feat: Support DynamicMPT (XLS-94) - #1332
Conversation
|
This PR needs to be updated to match the latest Dynamic MPT spec and xrpld changes. |
| {mutableFieldItems.map((field) => ( | ||
| <div className="header-box-item" key={field.key}> | ||
| <div className="item-name">{field.label}</div> | ||
| <div className="flag-status-group"> | ||
| <div | ||
| className="flag-status mutable" | ||
| data-testid="mutable-badge" | ||
| title={t('mutable_field_tooltip')} | ||
| > | ||
| {t('mutable')} | ||
| </div> |
There was a problem hiding this comment.
Can you check with Julian whether it looks okay to have the two buttons on a single line? The new layout doesn’t look great to me. Also, should the buttons to be left-aligned?
|
/ai-review |
There was a problem hiding this comment.
Solid, well-tested implementation of DynamicMPT (XLS-94) flag decoding and UI surfacing. Two things worth a second look: the TX_FLAGS map for MPTokenIssuanceSet drops the old 0x1000/0x2000 bit mappings entirely rather than keeping them alongside the renamed ones, and the new 'Mutable' pill logic in Settings.tsx wires up immutableFlag for six of the seven capability flags but skips canConfidentialAmount even though a corresponding lsifMPTCanHoldConfidentialBalance bit exists in the immutable-flags map.
There was a problem hiding this comment.
Well-structured addition of DynamicMPT (XLS-94) support: new ImmutableFlags decoding on MPTokenIssuanceCreate/MPTokenIssuanceSet/MPTokenIssuance, and a 'Mutable' badge UI in the MPT Settings panel. I cross-checked all the new bit-flag maps (MPT_IMMUTABLE_FLAGS in transactionUtils.ts, MPT_ISSUANCE_IMMUTABLE_FLAGS in rippled/lib/utils.ts, and the immutableFlag wiring in Settings.tsx) against each other and against the new test fixtures/mock data — the bit values and flag names are internally consistent, and the added/updated unit tests match the implemented badge-visibility logic (capability enable-only, badge hidden once enabled or once locked, field rows for Metadata/TransferFee hidden once locked). No correctness, security, or resource-handling bugs found in the changed lines. Two low-confidence/low-value items worth a quick look below.
There was a problem hiding this comment.
Solid, well-tested implementation of DynamicMPT (XLS-94) decoding and UI. Main finding: the ImmutableFlags bit-to-name mapping is defined twice with parallel but independently-maintained tables (tif* names in transactionUtils.ts and lsif* names in rippled/lib/utils.ts), which is a duplication risk if rippled ever changes/adds a bit value. No correctness or security issues found in the changed lines otherwise.
There was a problem hiding this comment.
The core DynamicMPT decoding/UI logic (immutable flags on MPTokenIssuanceCreate/Set, Settings page badges) looks correct and is well covered by tests. The one thing worth fixing before merge is that the ImmutableFlags bit-to-name tables are duplicated verbatim (same bit values, different name prefixes) across two files instead of being derived from a single source, which the comments themselves acknowledge ("Same bit values as the ledger lsif* map"). Everything else (badge logic, parser wiring, i18n key usage) is consistent with existing patterns.
There was a problem hiding this comment.
Solid feature addition: decodes MutableFlags/ImmutableFlags on MPTokenIssuanceCreate/Set and the MPTokenIssuance ledger object, and adds a clear Mutable/Immutable UI on the MPT Settings panel. Bit-value mappings for the new capability flags were cross-checked against the Settings component's flag→immutableFlag wiring and all line up correctly (no copy-paste mismatch found). The one thing worth tightening before merge is that the tif*/lsif* immutable-flag bit constants are defined twice in two different files with identical values — a future spec tweak to one map without the other would silently desync ledger-object decoding from transaction decoding.
| "resolved": "https://registry.npmjs.org/xrpl/-/xrpl-4.6.0.tgz", | ||
| "integrity": "sha512-0nXZfqDHRJ6bsDv1WtA9MdCYalMtXuxVa9mtLdqT3xypRKf2LwT5DbuGL/kHcVfuqk3B+ly+SFARlrnX+LHtRQ==", | ||
| "version": "5.1.0", | ||
| "resolved": "https://artifactory.ops.ripple.com/artifactory/api/npm/ripple-npm/xrpl/-/xrpl-5.1.0.tgz", |
There was a problem hiding this comment.
Artifactory URLs on xrpl deps block external installs. Regenerate package-lock.json against public npm registry:
Ensure .npmrc points to https://registry.npmjs.org, then:
rm -rf node_modules package-lock.json && npm install
Or confirm https://artifactory.ops.ripple.com is publicly reachable before merging.
There was a problem hiding this comment.
This MR wires up DynamicMPT (XLS-94) decoding and UI across the MPTokenIssuanceCreate/Set transactions and the MPT ledger object/Settings page. The logic is consistent and well tested (bit values, badge visibility rules, and translations line up correctly across the files I checked). Two things worth a second look before merging: the ImmutableFlags bit-to-name map is now duplicated verbatim in two separate files, and one capability's enabled-flag name doesn't match its immutable-lock-flag name — worth a quick sanity check against the XLS-94 spec/rippled source to make sure that's intentional and not a copy-paste slip.
High Level Overview of Change
MutableFlagsfield onMPTokenIssuanceCreateand theMPTokenIssuanceledger object, plus the newMutableFlags / MPTokenMetadata / TransferFeefields onMPTokenIssuanceSet.Mutablepill on capabilities the issuer can still enabled, hidden once a capability is enabled (capabilities are enable-only in the merged scheme), plusMutablerows forMetadataandTransfer Fee.Context of Change
Spec: https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0094-dynamic-MPT
Corresponding merged rippled PRs:
#5705 – Support DynamicMPT XLS-94d (core implementation)
#5797 – Rename mutable flags for DynamicMPT XLS-94d
#5820 – Rename flags for DynamicMPT
#6834 – Fix DynamicMPT for Attackathon
#7439 – Remove clear mutable flags for DynamicMPT XLS-94
Type of Change
Before / After
MPTokenIssuanceCreate — with ImmutableFlags
MPTokenIssuanceSet — enable CanEscrow + CanTrade
MPTokenIssuanceSet — update metadata + TransferFee + lock via ImmutableFlags
MPT page with new Settings