Skip to content

Fix failed integration tests due to removal of ammendaments #882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix failed integration tests due to removal of ammendaments #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .ci-config/rippled.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ validators.txt
# Devnet amendments as of June 28th, 2023
NegativeUNL
fixRemoveNFTokenAutoTrustLine
NonFungibleTokensV1
CheckCashMakesTrustLine
fixRmSmallIncreasedQOffers
fixSTAmountCanonicalize
Expand Down Expand Up @@ -156,10 +155,8 @@ fix1512
fix1373
MultiSign
Checks
NonFungibleTokensV1_1
# 1.10.0 Amendments
DisallowIncoming
fixNonFungibleTokensV1_2
fixTrustLinesToSelf
fixUniversalNumber
ImmediateOfferKilled
Expand All @@ -170,7 +167,6 @@ ExpandedSignerList
AMM
Clawback
fixReducedOffersV1
fixNFTokenRemint
# 2.0.0 Amendments
XChainBridge
DID
Expand Down Expand Up @@ -202,7 +198,6 @@ PermissionedDomains
SingleAssetVault
fixFrozenLPTokenTransfer
fixInvalidTxFlags
PermissionDelegation
PermissionedDEX
Batch
TokenEscrow
Expand Down
60 changes: 59 additions & 1 deletion tests/integration/it_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from xrpl.models.amounts.issued_currency_amount import IssuedCurrencyAmount
from xrpl.models.currencies.issued_currency import IssuedCurrency
from xrpl.models.currencies.xrp import XRP
from xrpl.models.requests import Ledger
from xrpl.models.requests import Feature, Ledger
from xrpl.models.requests.account_objects import AccountObjects, AccountObjectType
from xrpl.models.transactions import MPTokenAuthorize, MPTokenIssuanceCreate
from xrpl.models.transactions.account_set import AccountSet, AccountSetAsfFlag
Expand Down Expand Up @@ -636,3 +636,61 @@ def create_mpt_token_and_authorize_source(
sign_and_reliable_submission(payment_tx, issuer, client=client)

return mpt_issuance_id


async def is_amendment_enabled_async(
client: AsyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.

Args:
client: The async client to use for the request.
amendment_name: The name of the amendment to check.

Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = await client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_id, feature_data in features.items():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
Comment on lines +641 to +667
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Refactor to eliminate unused loop variable.

The feature_id loop variable is never used. Consider iterating over .values() directly for cleaner code.

Apply this diff:

-            for feature_id, feature_data in features.items():
+            for feature_data in features.values():
                 if (
                     isinstance(feature_data, dict)
                     and feature_data.get("name") == amendment_name
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def is_amendment_enabled_async(
client: AsyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.
Args:
client: The async client to use for the request.
amendment_name: The name of the amendment to check.
Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = await client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_id, feature_data in features.items():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
async def is_amendment_enabled_async(
client: AsyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.
Args:
client: The async client to use for the request.
amendment_name: The name of the amendment to check.
Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = await client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_data in features.values():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
🧰 Tools
🪛 Ruff (0.14.3)

659-659: Loop control variable feature_id not used within loop body

Rename unused feature_id to _feature_id

(B007)

665-665: Consider moving this statement to an else block

(TRY300)

666-666: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
In tests/integration/it_utils.py around lines 641 to 667, the for-loop iterates
over features.items() but never uses the feature_id variable; change the loop to
iterate over features.values() (e.g., for feature_data in features.values():)
and remove the unused feature_id binding so the body uses feature_data as before
and returns feature_data.get("enabled", False).



def is_amendment_enabled(
client: SyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.

Args:
client: The sync client to use for the request.
amendment_name: The name of the amendment to check.

Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_id, feature_data in features.items():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
Comment on lines +670 to +696
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Apply the same refactoring to the sync version.

The sync function has the same unused loop variable issue as the async version.

Apply this diff:

-            for feature_id, feature_data in features.items():
+            for feature_data in features.values():
                 if (
                     isinstance(feature_data, dict)
                     and feature_data.get("name") == amendment_name
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def is_amendment_enabled(
client: SyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.
Args:
client: The sync client to use for the request.
amendment_name: The name of the amendment to check.
Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_id, feature_data in features.items():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
def is_amendment_enabled(
client: SyncClient,
amendment_name: str,
) -> bool:
"""
Check if a specific amendment is enabled on the XRPL server.
Args:
client: The sync client to use for the request.
amendment_name: The name of the amendment to check.
Returns:
True if the amendment is enabled, False otherwise.
"""
try:
response = client.request(Feature())
if response.is_successful() and "features" in response.result:
features = response.result["features"]
for feature_data in features.values():
if (
isinstance(feature_data, dict)
and feature_data.get("name") == amendment_name
):
return feature_data.get("enabled", False)
return False
except Exception:
return False
🧰 Tools
🪛 Ruff (0.14.3)

688-688: Loop control variable feature_id not used within loop body

Rename unused feature_id to _feature_id

(B007)

694-694: Consider moving this statement to an else block

(TRY300)

695-695: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
In tests/integration/it_utils.py around lines 670 to 696, the sync
is_amendment_enabled function uses an unused loop variable (feature_id) in "for
feature_id, feature_data in features.items()"; change the loop to iterate over
values only (for feature_data in features.values()) so the unused variable is
removed while preserving behavior; keep the rest of the logic intact and return
False on exceptions as before.

25 changes: 25 additions & 0 deletions tests/integration/transactions/test_delegate_set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from tests.integration.integration_test_case import IntegrationTestCase
from tests.integration.it_utils import (
fund_wallet_async,
is_amendment_enabled_async,
sign_and_reliable_submission_async,
test_async_and_sync,
)
Expand All @@ -22,6 +23,14 @@
class TestDelegateSet(IntegrationTestCase):
@test_async_and_sync(globals())
async def test_delegation_with_no_permission(self, client):
# Check if PermissionDelegation amendment is enabled
is_enabled = await is_amendment_enabled_async(client, "PermissionDelegation")
if not is_enabled:
self.skipTest(
"Skipping DelegateSet test: PermissionDelegation amendment "
"is not enabled on the server"
)

# Note: Using WALLET, DESTINATION accounts could pollute the test results
alice = Wallet.create()
await fund_wallet_async(alice)
Expand All @@ -48,6 +57,14 @@ async def test_delegation_with_no_permission(self, client):

@test_async_and_sync(globals())
async def test_delegate_set_workflow(self, client):
# Check if PermissionDelegation amendment is enabled
is_enabled = await is_amendment_enabled_async(client, "PermissionDelegation")
if not is_enabled:
self.skipTest(
"Skipping DelegateSet test: PermissionDelegation amendment "
"is not enabled on the server"
)

# Note: Using WALLET, DESTINATION accounts could pollute the test results
alice = Wallet.create()
await fund_wallet_async(alice)
Expand Down Expand Up @@ -122,6 +139,14 @@ async def test_delegate_set_workflow(self, client):

@test_async_and_sync(globals())
async def test_fetch_delegate_account_objects(self, client):
# Check if PermissionDelegation amendment is enabled
is_enabled = await is_amendment_enabled_async(client, "PermissionDelegation")
if not is_enabled:
self.skipTest(
"Skipping DelegateSet test: PermissionDelegation amendment "
"is not enabled on the server"
)

# Note: Using WALLET, DESTINATION accounts could pollute the test results
alice = Wallet.create()
await fund_wallet_async(alice)
Expand Down
Loading