Support for Dynamic MPT (XLS-94d) #877
Conversation
WalkthroughAdds Dynamic Multi-Purpose Tokens (XLS-94d) support: new mutable-flag enums and validation in MPT transaction models, a new Serialized field (MutableFlags) in binary codec definitions, comprehensive unit and integration tests, rippled config / changelog updates, and docstring fixes for ripple epoch. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,230,201,0.4)
participant User
participant Client
participant MPTModel
participant Validator
participant Ledger
end
User->>Client: Submit MPTokenIssuanceCreate (mutable_flags?)
Client->>MPTModel: Construct transaction
MPTModel->>Validator: _get_errors()
Validator->>Validator: Check mutable_flags non-zero & valid bits
alt invalid
Validator-->>MPTModel: XRPLModelException / errors
MPTModel-->>Client: Reject
else valid
Validator-->>MPTModel: OK
Client->>Ledger: Submit create
Ledger-->>Client: Success (stored with MutableFlags)
end
User->>Client: Submit MPTokenIssuanceSet (mutable_flags, metadata, fee)
Client->>MPTModel: Construct transaction
MPTModel->>Validator: _get_errors()
Validator->>Validator: Validate set/clear conflicts, metadata hex/length, fee bounds, field interactions
alt invalid
Validator-->>MPTModel: XRPLModelException / errors
MPTModel-->>Client: Reject
else valid
Validator-->>MPTModel: OK (warnings may be returned)
Client->>Ledger: Submit set
Ledger-->>Client: Success (updated issuance)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
xrpl/utils/time_conversions.py (1)
16-18: Fix MAX_XRPL_TIME off-by-one error by setting it to 232 - 1.**XRPL time is a 32-bit unsigned integer counting seconds since the "Ripple Epoch": 2000-01-01 00:00:00 UTC. A 32-bit unsigned integer has a maximum value of 2^32 - 1, not 2^32.
Currently,
MAX_XRPL_TIME = 2**32combined with the> MAX_XRPL_TIMEchecks allows the value 2^32 to pass validation, even though it cannot be represented in a 32-bit unsigned integer. This causes an overflow.Changing
MAX_XRPL_TIMEto2**32 - 1(4,294,967,295) fixes the boundary error while preserving the existing comparison logic. The test file lacks explicit boundary tests for these edge cases—consider adding tests that verify 2^32 - 1 is accepted and 2^32 is rejected.- MAX_XRPL_TIME: Final[int] = 2**32 - """The maximum time that can be expressed on the XRPL""" + MAX_XRPL_TIME: Final[int] = 2**32 - 1 + """The maximum Ripple time offset (in seconds) expressible on the XRPL (2^32 - 1)"""xrpl/core/binarycodec/definitions/definitions.json (1)
3370-3377: Add XChainAddAccountCreateAttestation transaction type code to definitions.jsonXChainAddAccountCreateAttestation is a supported XRPL transaction type, yet it's missing from the TRANSACTION_TYPES mapping in definitions.json. The model class, enum, and tests all exist in the codebase, and codec fixtures contain examples of this transaction type. Without the mapping, binary encoding/decoding will fail for this transaction.
Add to xrpl/core/binarycodec/definitions/definitions.json at line ~3375 (the numeric gap between 45 and 47 indicates code 46):
"XChainAddClaimAttestation": 45, + "XChainAddAccountCreateAttestation": 46, "XChainModifyBridge": 47
🧹 Nitpick comments (10)
CHANGELOG.md (1)
8-8: Nit: unify bracket style for section headers.Headers mix single and double brackets (e.g., “[[Unreleased]]” vs “[4.2.0]”). Consider standardizing for consistency.
tests/unit/models/transactions/test_mptoken_issuance_create.py (1)
131-159: Good coverage for valid, zero, and reserved‑bit cases.Consider two quick additions:
- Assert negative mutable_flags (e.g., -1) is rejected.
- Assert serialization includes the proper XRPL field name (MutableFlags) via
to_dict().Happy to draft these test cases if useful.
xrpl/models/transactions/mptoken_issuance_create.py (1)
183-195: Build the valid mask programmatically to avoid drift.Hard-coding the OR of every enum member is easy to miss during future updates. Derive the mask from the enum at runtime (or define a single module-level constant computed from the enum) to keep it source-of-truth. The suggested refactoring is appropriate:
- valid_mutable_flags = ( - MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_CAN_LOCK.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_REQUIRE_AUTH.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_CAN_ESCROW.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_CAN_TRADE.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_CAN_TRANSFER.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_CAN_CLAWBACK.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_METADATA.value - | MPTokenIssuanceCreateMutableFlag.TMF_MPT_CAN_MUTATE_TRANSFER_FEE.value - ) + valid_mutable_flags = 0 + for f in MPTokenIssuanceCreateMutableFlag: + valid_mutable_flags |= f.valueAlso verified: The binary-codec definition uses "type": "UInt32" for MutableFlags, confirming an unsigned 32-bit type as intended.
tests/unit/models/transactions/test_mptoken_issuance_set.py (2)
52-56: Avoid asserting the entire error dict stringAsserting exact string of a dict is brittle (ordering/formatting). Prefer substring checks or parsing.
Apply:
- self.assertEqual( - error.exception.args[0], - "{'flags': \"flag conflict: both TF_MPT_LOCK and TF_MPT_UNLOCK can't be set" - '"}', - ) + self.assertIn("TF_MPT_LOCK and TF_MPT_UNLOCK", error.exception.args[0])
157-166: Conflict tests are solidClear coverage of set/clear flag conflicts. Consider adding a test for “unknown/reserved bits” once validation is added in the model (see model comment).
Also applies to: 167-176
xrpl/models/transactions/mptoken_issuance_set.py (2)
139-144: Docstring nit“apply to all any accounts” → “apply to all accounts holding MPTs.”
- If omitted, this transaction will apply to all any accounts holding MPTs. + If omitted, this transaction applies to all accounts holding MPTs.
198-206: Flags + dynamic fieldsYou allow flags=0 with dynamic fields. If the intent is “no Flags at all,” consider requiring flags is None. If flags=0 is intentional, add a brief comment to document it.
- if has_dynamic_fields and self.flags is not None: + # flags=0 allowed for backwards-compat; non-zero disallowed + if has_dynamic_fields and self.flags is not None:tests/integration/transactions/test_mptoken_issuance_dynamic_mpt.py (3)
1-1: Spec reference consistencyDocstring says “XLS-94”; PR title uses “XLS-94d”. Align naming to avoid confusion.
-"""Integration tests for DynamicMPT (XLS-94) feature.""" +"""Integration tests for DynamicMPT (XLS-94d) feature."""
22-29: Avoid duplicating ledger flag constants in testsHardcoding LSF_* can drift from source. Prefer importing from a single module (if available) or deriving from the model’s enum to reduce duplication.
50-58: Tx->mpt_issuance_id extractionAccessing meta["mpt_issuance_id"] is central to these tests. If upstream ever renames this, tests will fail. Consider a small helper to fetch it with a clearer error when missing.
Also applies to: 99-105, 156-160, 216-220, 263-267, 340-344, 404-408, 456-459
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.ci-config/rippled.cfg(1 hunks)CHANGELOG.md(1 hunks)tests/integration/transactions/test_mptoken_issuance_dynamic_mpt.py(1 hunks)tests/unit/models/transactions/test_mptoken_issuance_create.py(2 hunks)tests/unit/models/transactions/test_mptoken_issuance_set.py(2 hunks)xrpl/core/binarycodec/definitions/definitions.json(4 hunks)xrpl/models/transactions/__init__.py(2 hunks)xrpl/models/transactions/mptoken_issuance_create.py(3 hunks)xrpl/models/transactions/mptoken_issuance_set.py(5 hunks)xrpl/utils/time_conversions.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/unit/models/transactions/test_mptoken_issuance_create.py (4)
xrpl/models/transactions/mptoken_issuance_create.py (3)
MPTokenIssuanceCreate(89-208)MPTokenIssuanceCreateFlag(25-37)MPTokenIssuanceCreateMutableFlag(40-69)xrpl/models/base_model.py (1)
is_valid(295-302)tests/unit/models/transactions/test_mptoken_issuance_set.py (1)
test_tx_mutable_flags_zero_fails(148-155)xrpl/models/exceptions.py (1)
XRPLModelException(6-9)
tests/integration/transactions/test_mptoken_issuance_dynamic_mpt.py (9)
tests/integration/integration_test_case.py (1)
IntegrationTestCase(9-18)tests/integration/it_utils.py (2)
sign_and_reliable_submission_async(209-245)test_async_and_sync(296-400)xrpl/models/requests/account_objects.py (2)
AccountObjects(48-71)AccountObjectType(19-43)xrpl/models/requests/tx.py (1)
Tx(20-87)xrpl/models/transactions/mptoken_issuance_create.py (3)
MPTokenIssuanceCreate(89-208)MPTokenIssuanceCreateFlag(25-37)MPTokenIssuanceCreateMutableFlag(40-69)xrpl/models/transactions/mptoken_issuance_set.py (2)
MPTokenIssuanceSet(127-298)MPTokenIssuanceSetMutableFlag(46-111)xrpl/utils/str_conversions.py (1)
str_to_hex(4-16)xrpl/wallet/main.py (2)
classic_address(34-41)address(24-29)xrpl/models/response.py (1)
is_successful(74-82)
tests/unit/models/transactions/test_mptoken_issuance_set.py (5)
xrpl/models/exceptions.py (1)
XRPLModelException(6-9)xrpl/models/transactions/mptoken_issuance_set.py (3)
MPTokenIssuanceSet(127-298)MPTokenIssuanceSetFlag(26-43)MPTokenIssuanceSetMutableFlag(46-111)xrpl/utils/str_conversions.py (1)
str_to_hex(4-16)xrpl/models/base_model.py (1)
is_valid(295-302)tests/unit/models/transactions/test_mptoken_issuance_create.py (1)
test_tx_mutable_flags_zero_fails(142-148)
xrpl/models/transactions/__init__.py (2)
xrpl/models/transactions/mptoken_issuance_create.py (1)
MPTokenIssuanceCreateMutableFlag(40-69)xrpl/models/transactions/mptoken_issuance_set.py (2)
MPTokenIssuanceSet(127-298)MPTokenIssuanceSetMutableFlag(46-111)
xrpl/models/transactions/mptoken_issuance_set.py (3)
xrpl/models/transactions/transaction.py (2)
Transaction(184-549)has_flag(393-419)xrpl/models/transactions/types/transaction_type.py (1)
TransactionType(6-70)xrpl/models/utils.py (2)
require_kwargs_on_init(317-370)validate_mptoken_metadata(145-275)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.13)
- GitHub Check: Integration test (3.8)
- GitHub Check: Integration test (3.12)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.10)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (11)
.ci-config/rippled.cfg (1)
209-209: Confirm amendment name + rippled version; ensure features apply in CI.Please verify:
- The amendment identifier is exactly “DynamicMPT” upstream, and the CI rippled image includes it (rippled PR #5705 or later).
- If CI runs rippled in standalone, the [features] stanza may be ignored (noted in Lines 91–92). Ensure your harness enables the amendment effectively (e.g., by using a compatible image or alternative enablement).
CHANGELOG.md (1)
10-13: LGTM — clear user‑facing note for XLS‑94d.xrpl/utils/time_conversions.py (1)
26-26: Docstrings correctly updated to 2000‑01‑01T00:00Z Ripple Epoch.Also applies to: 56-56, 79-79, 111-111
xrpl/models/transactions/mptoken_issuance_create.py (2)
40-71: New MutableFlag enum looks good and aligns with field/flag mutability intent.
137-144: Public fieldmutable_flagsaddition is documented and non‑breaking.tests/unit/models/transactions/test_mptoken_issuance_create.py (1)
6-10: Import surface looks right after re‑exporting the new enum.xrpl/models/transactions/__init__.py (1)
53-54: Public export additions look goodNew mutable-flag enums are properly imported and re-exported. No issues spotted.
Also applies to: 60-61, 177-178, 182-183
tests/unit/models/transactions/test_mptoken_issuance_set.py (1)
148-156: Good coverage for zero mutable_flagsCovers the invalid zero case; aligns with model validation.
xrpl/core/binarycodec/definitions/definitions.json (1)
683-692: MutableFlags field additionAdding MutableFlags (UInt32, signing) aligns with the new model fields. LGTM.
tests/integration/transactions/test_mptoken_issuance_dynamic_mpt.py (2)
61-78: Assertions are focused and robustGood use of uppercase hex comparisons and explicit field presence/absence checks. Looks solid.
Also applies to: 112-116, 191-195, 245-247, 373-381, 433-435, 484-485
335-361: End-to-end combo update test is valuableCombining flag updates with transfer fee exercises key interactions. Nice coverage.
…ons for amendment PRs
| "terLAST": -91, | ||
| "terNO_ACCOUNT": -96, | ||
| "terNO_AMM": -87, | ||
| "tedADDRESS_COLLISION": -86, |
There was a problem hiding this comment.
Can you confirm that terADDRESS_COLLISION error code is present in the definitions.json file? This is an error code that is returned from VaultCreate transaction.
There was a problem hiding this comment.
I reverted the changes in 2ab1fcb to resolve the issue.
| ) | ||
|
|
||
| # Check for zero value | ||
| if self.mutable_flags == 0: |
There was a problem hiding this comment.
Isn't this case covered with the mask check above? Why do you need to check the zero case separately?
There was a problem hiding this comment.
Good question. We need to handle the zero case separately because 0 & any bit pattern always results in 0, meaning line 199 would evaluate to False and we will fail to catch it.
| account=_ACCOUNT, | ||
| mutable_flags=0, | ||
| ) | ||
| self.assertIn("mutable_flags cannot be 0", error.exception.args[0]) |
There was a problem hiding this comment.
| self.assertIn("mutable_flags cannot be 0", error.exception.args[0]) | |
| self.assertIn("MPTokenIssuanceCreate: mutable_flags cannot be 0", error.exception.args[0]) |
Users might find it useful if the transaction name is included in the error message.
There was a problem hiding this comment.
I partially agree with you. That said,
- The error occurs during the construction of the MPTokenIssuanceCreate object, so the context should already be clear.
- The error dictionary format
{'field_name': 'error_message'}follows the standard pattern used throughout the xrpl-py library. Other validation errors in the same file also use this format without including the class name prefix.
Let me know you still think that we should add the transaction name as part of the message.
| "VaultSet": 66, | ||
| "VaultWithdraw": 69, | ||
| "XChainAccountCreateCommit": 44, | ||
| "XChainAddAccountCreateAttestation": 46, |
There was a problem hiding this comment.
@kuan121 The lack of this field is causing the integration tests to fail.
The client library is unable to understand the transaction serialization for the XChainAddAccountCreateAttestation transaction.
There was a problem hiding this comment.
I reverted the changes in 2ab1fcb to resolve the issue.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/pull_request_template.md (1)
44-47: Nice addition; tighten the ask with a spec link and examples.Consider linking the exact XLS-94d section and naming a few typical tem* you expect covered (e.g., temINVALID_FLAG, temBAD_TRANSFER_FEE, temMALFORMED) so authors know the bar and reviewers can verify quickly.
tests/unit/models/transactions/test_mptoken_issuance_set.py (2)
181-246: DRY the set/clear conflict tests with parametrization.You can cover all pairs in one test using subTests to cut repetition and speed up maintenance.
@@ - def test_mutable_flags_set_clear_can_lock_conflict(self): - ... - def test_mutable_flags_set_clear_require_auth_conflict(self): - ... - # (repeated per pair) + def test_mutable_flags_set_clear_conflicts(self): + pairs = [ + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_LOCK, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_LOCK, + "CAN_LOCK"), + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_REQUIRE_AUTH, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_REQUIRE_AUTH, + "REQUIRE_AUTH"), + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_ESCROW, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_ESCROW, + "CAN_ESCROW"), + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_TRADE, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_TRADE, + "CAN_TRADE"), + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_TRANSFER, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_TRANSFER, + "CAN_TRANSFER"), + (MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_CLAWBACK, + MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_CLAWBACK, + "CAN_CLAWBACK"), + ] + for set_flag, clear_flag, name in pairs: + with self.subTest(name=name), self.assertRaises(XRPLModelException) as err: + MPTokenIssuanceSet( + account=_ACCOUNT, + mptoken_issuance_id=_TOKEN_ID, + mutable_flags=set_flag | clear_flag, + ) + self.assertIn(f"Cannot set and clear {name}", err.exception.args[0])
13-15: Silence false-positive secret detectors on test constants.Token ID is a fixture, not a credential. Add an inline suppression to avoid CI noise.
-_TOKEN_ID = "000004C463C52827307480341125DA0577DEFC38405B0E3E" +_TOKEN_ID = "000004C463C52827307480341125DA0577DEFC38405B0E3E" # noqa: S105 # gitleaks:allowxrpl/models/transactions/mptoken_issuance_set.py (2)
213-230: Avoid recomputing the valid_mask on every call.Hoist a module‑level constant derived from the Enum to reduce duplication and keep source‑of‑truth centralized.
+from functools import reduce @@ -class MPTokenIssuanceSetMutableFlag(int, Enum): +class MPTokenIssuanceSetMutableFlag(int, Enum): ... +# Bitmask of all valid TMF bits +_VALID_MUTABLE_FLAG_MASK: Final[int] = reduce( + lambda acc, f: acc | f.value, MPTokenIssuanceSetMutableFlag, 0 +) @@ - valid_mask = ( - MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_LOCK.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_LOCK.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_REQUIRE_AUTH.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_REQUIRE_AUTH.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_ESCROW.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_ESCROW.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_TRADE.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_TRADE.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_TRANSFER.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_TRANSFER.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_SET_CAN_CLAWBACK.value - | MPTokenIssuanceSetMutableFlag.TMF_MPT_CLEAR_CAN_CLAWBACK.value - ) - if self.mutable_flags & ~valid_mask: + if self.mutable_flags & ~_VALID_MUTABLE_FLAG_MASK: errors["mutable_flags"] = "mutable_flags contains invalid bits"
145-166: Docs: small nits.
- “apply to all any accounts” → “apply to all accounts”.
- Double‑check flag names in prose (“TMF_MPT_CAN_MUTATE_*”) match the spec’s canonical names.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/pull_request_template.md(1 hunks)tests/unit/models/transactions/test_mptoken_issuance_create.py(2 hunks)tests/unit/models/transactions/test_mptoken_issuance_set.py(3 hunks)xrpl/core/binarycodec/definitions/definitions.json(2 hunks)xrpl/models/transactions/mptoken_issuance_set.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/models/transactions/test_mptoken_issuance_create.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-13T18:43:34.412Z
Learnt from: Patel-Raj11
PR: XRPLF/xrpl-py#840
File: xrpl/core/binarycodec/definitions/definitions.json:2594-2602
Timestamp: 2025-05-13T18:43:34.412Z
Learning: The `definitions.json` file in XRPL is a generated file and should not be directly edited. Constraints like maximum array lengths are enforced at the model level (e.g., in transaction models) rather than in the binary codec layer.
Applied to files:
xrpl/core/binarycodec/definitions/definitions.json
📚 Learning: 2025-02-11T21:11:19.326Z
Learnt from: ckeshava
PR: XRPLF/xrpl-py#772
File: tools/generate_definitions.py:0-0
Timestamp: 2025-02-11T21:11:19.326Z
Learning: In XRPL's binary codec format, large unsigned integers (UINT128, UINT160, UINT192, UINT256) should be consistently treated as Hash types, while smaller UINTs are translated to UInt types. This is reflected in generate_tx_models.py where they are mapped to "str" type.
Applied to files:
xrpl/core/binarycodec/definitions/definitions.json
🧬 Code graph analysis (2)
tests/unit/models/transactions/test_mptoken_issuance_set.py (4)
xrpl/models/exceptions.py (1)
XRPLModelException(6-9)xrpl/models/transactions/mptoken_issuance_set.py (3)
MPTokenIssuanceSet(127-316)MPTokenIssuanceSetFlag(26-43)MPTokenIssuanceSetMutableFlag(46-111)xrpl/utils/str_conversions.py (1)
str_to_hex(4-16)xrpl/models/base_model.py (1)
is_valid(295-302)
xrpl/models/transactions/mptoken_issuance_set.py (2)
xrpl/models/transactions/transaction.py (1)
has_flag(393-419)xrpl/models/utils.py (2)
require_kwargs_on_init(317-370)validate_mptoken_metadata(145-275)
🪛 Gitleaks (8.28.0)
tests/unit/models/transactions/test_mptoken_issuance_set.py
[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.14.1)
tests/unit/models/transactions/test_mptoken_issuance_set.py
14-14: Possible hardcoded password assigned to: "_TOKEN_ID"
(S105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Integration test (3.13)
- GitHub Check: Integration test (3.12)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.14)
- GitHub Check: Integration test (3.8)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.10)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
xrpl/core/binarycodec/definitions/definitions.json (2)
3356-3359: Confirm these Vault entries came from the generator (no semantic changes).*Values appear unchanged; if this churn wasn’t produced by the defs generator, please revert to avoid noisy diffs in a generated file.
Based on learnings
683-692: All verification checks passed. MutableFlags, terADDRESS_COLLISION, and XChainAddAccountCreateAttestation are correctly defined and wired.tests/unit/models/transactions/test_mptoken_issuance_set.py (1)
61-74: Add a test to lock in behavior for flags=0 with dynamic fields.Model allows flags==0 when dynamic fields are present. If spec prefers “no Flags at all” in this mode, assert reject; otherwise, assert accept to prevent regressions.
xrpl/models/transactions/mptoken_issuance_set.py (1)
198-206: Confirm policy: allow Flags==0 with dynamic fields?Current logic rejects non‑zero Flags but accepts Flags==0 when any of mutable_flags/metadata/transfer_fee are set. Verify XLS-94d intent; if the intent is “no Flags in DynamicMPT mode,” reject any provided Flags (even zero). Otherwise, consider normalizing by dropping Flags on serialization.
High Level Overview of Change
Add support of Dynamic MPT (XLS-94d)
Context of Change
Type of Change
Did you update CHANGELOG.md?
Test Plan