fix: Add MPT/DEX assorted fixes by gregtatcam · Pull Request #6855 · XRPLF/rippled

gregtatcam · 2026-04-10T18:37:11Z

High Level Overview of Change

ValidMPTTransfer, check for valid MPTLocked, MPTCanTrade, MPTCanTransfer.
Replace canTrade with canTransfer in CheckCreate/Cash (Check is not DEX tx).
Change tecFROZEN to tecLOCKED for locked MPT.
Replace checkMPTTxAllowed() with canMPTTradeAndTransfer().
Fix double adjustOwnerCount() in AMMWithdraw.
Extend the unit-tests.

…EX tx).

Replace checkMPTTxAllowed() with canMPTTradeAndTransfer().

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

xrplf-ai-reviewer

Removed canTrade() creation-time guard needs audit — see inline comment at line 167.

Review by Claude Opus 4.6 · Prompt: V14

xrplf-ai-reviewer · 2026-04-10T18:41:58Z

    }

-    return canTrade(ctx.view, ctx.tx[sfSendMax].asset());
+    return tesSUCCESS;


Removed canTrade() check replaced with unconditional success — verify IOU tradeability is still enforced elsewhere at creation time.

The old code returned canTrade(ctx.view, ctx.tx[sfSendMax].asset()) here, which gated check creation on asset tradeability. The MPT path now has an explicit canTransfer guard (lines 148–153), but the IOU visitor branch (lines 103–135) should be audited to confirm an equivalent check exists. Dropping this shifts all IOU trade enforcement to cash time, creating a single point of failure if CheckCash is ever bypassed or misconfigured.

This is not an issue. canTrade() was added for MPT/DEX feature. canTrade() returns tesSUCCESS for Issue. Since CheckCreate is not a DEX transaction, this logic is reverted to pre MPT/DEX code-base; i.e., return tesSUCCESS. canTransfer() is added only for MPTIssue to check if MPTCanTransfer flag is enabled.

shawnxie999 · 2026-04-13T15:45:10Z

+                // Check once: if any involved account is frozen, the whole
+                // issuance transfer is considered frozen. Only need to check for
+                // frozen if there is a transfer of funds.
+                if (!frozen && isFrozen(view, account, MPTIssue{mptID}))


do we possibly want to include requireAuth as well?

Sure. This is going to be a check for the receivers only, right?

Would it also make sense to check for sender as well? Both should have auth when transferring

The problem with send is that the send object could be deleted, in which case requireAuth fails. I can call it with WeakAuth, but it still might fail. The only thing that I can do if it's deleted is to check for MPTRequireAuth flag on the MPTokenIssuance and if set then check MPTAuthorized flag on before object. If the latter is not set then the invariant fails.

Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com>

xrplf-ai-reviewer

Compile error (undeclared txnType) flagged in MPTInvariant.cpp; potential missing trading-restriction enforcement in CheckCreate.cpp — see inline comments.

Review by Claude Opus 4.6 · Prompt: V14

xrplf-ai-reviewer · 2026-04-13T17:33:36Z

+    // DEX transactions (AMM[Create,Deposit,Withdraw], cross-currency payments, offer creates) are
+    // subject to the MPTCanTrade flag in addition to the standard transfer rules.
+    // A payment is only DEX if it is a cross-currency payment.
+    auto const isDEX = [&tx, &txnType] {


txnType is captured by reference but never declared in scope — won't compile. Declare it before the lambda:

auto const txnType = tx.getTxnType(); auto const isDEX = [&tx, &txnType] {

xrplf-ai-reviewer · 2026-04-13T17:33:36Z

    }

-    return canTrade(ctx.view, ctx.tx[sfSendMax].asset());
+    return tesSUCCESS;


Unconditional tesSUCCESS replaces the prior canTrade call — confirm IOU trading restrictions are enforced elsewhere.

This is correct change.

xrplf-ai-reviewer

Looked through this one

Potential validation gap at line 167 — the unconditional canTrade call for all asset types was replaced with tesSUCCESS; the IOU path may be missing an equivalent check. See inline.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V14

xrplf-ai-reviewer · 2026-04-13T19:27:53Z

    }

-    return canTrade(ctx.view, ctx.tx[sfSendMax].asset());
+    return tesSUCCESS;


canTrade was previously called unconditionally for all asset types — confirm it was a no-op for IOU assets, or add an IOU branch validation to avoid silently dropping that check.

codecov · 2026-04-13T19:49:01Z

Codecov Report

❌ Patch coverage is 99.21875% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.6%. Comparing base (6a0ce46) to head (e85f010).
⚠️ Report is 7 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/tx/invariants/MPTInvariant.cpp	98.2%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6855     +/-   ##
=========================================
- Coverage     81.6%   81.6%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        75973   76030     +57     
  Branches      7610    7604      -6     
=========================================
+ Hits         61993   62038     +45     
- Misses       13980   13992     +12

Files with missing lines	Coverage Δ
include/xrpl/tx/invariants/InvariantCheck.h	`100.0% <ø> (ø)`
src/libxrpl/ledger/helpers/MPTokenHelpers.cpp	`96.1% <100.0%> (+<0.1%)`	⬆️
src/libxrpl/ledger/helpers/TokenHelpers.cpp	`95.4% <100.0%> (+0.1%)`	⬆️
src/libxrpl/tx/transactors/check/CheckCash.cpp	`90.6% <100.0%> (+<0.1%)`	⬆️
src/libxrpl/tx/transactors/check/CheckCreate.cpp	`99.1% <100.0%> (+0.1%)`	⬆️
src/libxrpl/tx/transactors/dex/AMMClawback.cpp	`98.1% <100.0%> (+2.6%)`	⬆️
src/libxrpl/tx/transactors/dex/AMMCreate.cpp	`90.9% <100.0%> (+0.1%)`	⬆️
src/libxrpl/tx/transactors/dex/AMMDeposit.cpp	`96.4% <100.0%> (+0.3%)`	⬆️
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp	`94.3% <100.0%> (+0.2%)`	⬆️
src/libxrpl/tx/transactors/dex/OfferCreate.cpp	`93.3% <100.0%> (+0.1%)`	⬆️
... and 1 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

shawnxie999 · 2026-04-13T20:40:01Z

+    ReadView const& view,
+    beast::Journal const& j)
+{
+    // AMMClawback is called by the issuer, so freeze restrictions do not apply.


nit - this comment can probably be removed/changed

shawnxie999 · 2026-04-13T20:54:41Z

+                if (!frozen && isFrozen(view, account, MPTIssue{mptID}))
+                {
+                    frozen = true;
+                }


Could we combine the auth checks with the freeze logic here? That way we wouldn’t need separate frozen and reqAuth booleans—we could instead use a single illegalTransfer boolean that covers both cases. (it probably doesn't hurt to check auth for the sender)

* Combine frozen and reqAuth. * Check senders and receivers for reqAuth.

xrplf-ai-reviewer

LGTM - clean fixes across the board.

Review by Claude Opus 4.6 · Prompt: V14

shawnxie999

lgtm

xrplf-ai-reviewer

LGTM - solid fixes.

Review by Claude Opus 4.6 · Prompt: V15

…eck.

xrplf-ai-reviewer

LGTM - solid fixes across MPT/DEX handling.

Review by Claude Opus 4.6 · Prompt: V15

github-actions · 2026-04-17T17:10:36Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

gregtatcam and others added 9 commits April 9, 2026 11:52

Add ValidMPTTransfer invariant.

a91e8dd

Replace canTrade with canTransfer in CheckCreate/Cash (Check is not D…

d26d931

…EX tx).

Change tecFROZEN to tecLOCKED for locked MPT.

12eb050

Replace checkMPTTxAllowed() with canMPTTradeAndTransfer().

Fix double adjustOwnerCount() in AMMWithdraw.

0a896b7

Update src/libxrpl/tx/invariants/MPTInvariant.cpp

63f270f

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

Address AI review

77df21b

Improve ValidMPTTransfer::finalize()

1a2c0e7

Add ValidMPTTransfer invariant unit-tests.

a839b8b

Add CheckCash can't transfer failure unit-test.

50cacdd

gregtatcam requested review from shawnxie999 and vlntb April 10, 2026 18:37

xrplf-ai-reviewer bot reviewed Apr 10, 2026

View reviewed changes

shawnxie999 reviewed Apr 13, 2026

View reviewed changes

Comment thread src/libxrpl/ledger/helpers/MPTokenHelpers.cpp

shawnxie999 reviewed Apr 13, 2026

View reviewed changes

Comment thread src/libxrpl/tx/invariants/MPTInvariant.cpp Outdated

shawnxie999 reviewed Apr 13, 2026

View reviewed changes

Update src/libxrpl/tx/invariants/MPTInvariant.cpp

62921eb

Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com>

xrplf-ai-reviewer bot reviewed Apr 13, 2026

View reviewed changes

gregtatcam added 2 commits April 13, 2026 13:39

Fix MPTInvariant refactoring

5439264

Add requireAuth invariant

a2ab6c7

xrplf-ai-reviewer bot reviewed Apr 13, 2026

View reviewed changes

shawnxie999 reviewed Apr 13, 2026

View reviewed changes

Refactor requireAuth handling in ValidMPTTransfer.

70fa0d7

* Combine frozen and reqAuth. * Check senders and receivers for reqAuth.

xrplf-ai-reviewer bot approved these changes Apr 14, 2026

View reviewed changes

shawnxie999 approved these changes Apr 14, 2026

View reviewed changes

Merge branch 'develop' into gregtatcam/mpt/assorted-fixes-dev

af17ab2

xrplf-ai-reviewer bot approved these changes Apr 15, 2026

View reviewed changes

Fix AMMClawback return code on AllowTrustLineClawback and NoFreeze ch…

e85f010

…eck.

xrplf-ai-reviewer bot approved these changes Apr 15, 2026

View reviewed changes

github-actions bot added the PR: has conflicts label Apr 17, 2026

Conversation

gregtatcam commented Apr 10, 2026

High Level Overview of Change

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

gregtatcam Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shawnxie999 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gregtatcam Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

shawnxie999 Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregtatcam Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gregtatcam Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shawnxie999 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

shawnxie999 Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

shawnxie999 left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shawnxie999 Apr 13, 2026 •

edited

Loading

codecov bot commented Apr 13, 2026 •

edited

Loading

shawnxie999 Apr 13, 2026 •

edited

Loading