feat: Add MPT support to DEX by gregtatcam · Pull Request #5285 · XRPLF/rippled

gregtatcam · 2025-02-12T10:25:44Z

High Level Overview of Change

Add MPT support to DEX

Context of Change

Implements XLS-82d

Adds MPT support for the following transactions:

Payment
OfferCreate
CheckCash
All AMM transactions
Pathfinding

Retires FlowCross feature.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

Public API: New feature (new methods and/or new fields)
Public API: Breaking change (in general, breaking changes should only impact the next api_version)
libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
Peer protocol change (must be backward compatible or bump the peer protocol version)

Test Plan

Extended MPToken unit-test to test for basic MPT interactions with DEX.

codecov · 2025-02-12T15:09:13Z

Codecov Report

❌ Patch coverage is 88.97770% with 262 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.6%. Comparing base (6e24522) to head (281a7f9).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/tx/paths/MPTEndpointStep.cpp	73.3%	87 Missing ⚠️
src/libxrpl/tx/invariants/MPTInvariant.cpp	77.8%	18 Missing ⚠️
src/libxrpl/tx/paths/PaySteps.cpp	87.8%	16 Missing ⚠️
src/libxrpl/tx/transactors/check/CheckCash.cpp	88.1%	15 Missing ⚠️
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp	87.6%	11 Missing ⚠️
src/libxrpl/ledger/helpers/TokenHelpers.cpp	90.4%	9 Missing ⚠️
include/xrpl/protocol/AmountConversions.h	78.9%	8 Missing ⚠️
src/libxrpl/protocol/STParsedJSON.cpp	81.1%	7 Missing ⚠️
src/libxrpl/tx/transactors/dex/AMMClawback.cpp	85.7%	7 Missing ⚠️
src/libxrpl/tx/transactors/dex/AMMCreate.cpp	92.6%	7 Missing ⚠️
... and 28 more

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5285     +/-   ##
=========================================
+ Coverage     81.4%   81.6%   +0.1%     
=========================================
  Files          999    1004      +5     
  Lines        74423   75947   +1524     
  Branches      7557    7634     +77     
=========================================
+ Hits         60613   61948   +1335     
- Misses       13810   13999    +189

Files with missing lines	Coverage Δ
include/xrpl/ledger/OrderBookDB.h	`100.0% <ø> (ø)`
include/xrpl/ledger/PaymentSandbox.h	`100.0% <100.0%> (ø)`
include/xrpl/ledger/View.h	`100.0% <ø> (ø)`
include/xrpl/ledger/helpers/RippleStateHelpers.h	`100.0% <ø> (ø)`
include/xrpl/protocol/AMMCore.h	`100.0% <ø> (ø)`
include/xrpl/protocol/Asset.h	`97.7% <100.0%> (+1.4%)`	⬆️
include/xrpl/protocol/Concepts.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/LedgerFormats.h	`100.0% <ø> (ø)`
include/xrpl/protocol/MPTAmount.h	`91.2% <100.0%> (-8.8%)`	⬇️
include/xrpl/protocol/MPTIssue.h	`100.0% <100.0%> (ø)`
... and 127 more

... and 7 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 · 2025-02-18T23:33:53Z

haven't reviewed the changes but one invariant would be good to have is to ensure that the amount changed for OutstandingBalance in MPTokenIssuance should equal the net change of all the MPTokens modified in the transaction

include/xrpl/protocol/Book.h

src/libxrpl/protocol/Indexes.cpp

src/xrpld/app/misc/AMMUtils.h

src/xrpld/app/misc/detail/MPTUtils.cpp

include/xrpl/protocol/AmountConversions.h

src/libxrpl/tx/transactors/token/MPTUtils.cpp

* Payment * OfferCreate * CheckCash * All AMM transactions * Pathfinding Retire FlowCross feature

github-actions · 2026-03-30T18:07:20Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

gregtatcam · 2026-03-31T12:14:51Z

/ai-review

xrplf-ai-reviewer

ℹ️ Note: This is a large diff (1,119,614 chars). Complex issues deep in the diff may receive less attention.

Several high-severity issues flagged inline: null dereference in removeDeletedMPTs, use-after-free race in AssetCache::getMPTs, potential server crash in NFTokenUtils via .get<Issue>() on MPT amounts, and a 0.1% tolerance applied to integer-denominated MPT amounts in checkNear. Additional correctness concerns around deleteAMMMPToken missing outstanding-amount accounting, strict aliasing UB in getMPTIssuer, uninitialized outstanding array, auto-authorization bypass in AMMCreate, and missing MPT issuer validation in path JSON parsing.

Review by Claude Opus 4.6 · Prompt: V12

xrplf-ai-reviewer · 2026-03-31T12:37:32Z

src/libxrpl/tx/paths/PaySteps.cpp

-
 bool
-checkNear(XRPAmount const& expected, XRPAmount const& actual)
+checkNear(MPTAmount const& expected, MPTAmount const& actual)


MPT gets IOU-style 0.1% tolerance instead of exact equality like XRP. Both are integer-denominated assets — under-delivery up to 0.1% would silently pass checkNear.

xrplf-ai-reviewer · 2026-03-31T12:37:32Z

src/libxrpl/tx/Transactor.cpp

+    // There could be at most two MPTs - one for each side of AMM pool
+    if (mpts.size() > 2)
+    {
+        JLOG(viewJ.error()) << "removeDeletedTrustLines: deleted mpts exceed 2 " << mpts.size();


Copy-paste bug: log says removeDeletedTrustLines but this is removeDeletedMPTs.

Suggested change

JLOG(viewJ.error()) << "removeDeletedTrustLines: deleted mpts exceed 2 " << mpts.size();

JLOG(viewJ.error()) << "removeDeletedMPTs: deleted mpts exceed 2 " << mpts.size();

xrplf-ai-reviewer · 2026-03-31T12:37:32Z

src/libxrpl/tx/Transactor.cpp

+    for (auto const& index : mpts)
+    {
+        if (auto const sleState = view.peek({ltMPTOKEN, index});
+            deleteAMMMPToken(view, sleState, (*sleState)[sfIssuer], viewJ) != tesSUCCESS)


Null dereference if view.peek() returns nullptr: (*sleState)[sfIssuer] crashes.

Suggested change

deleteAMMMPToken(view, sleState, (*sleState)[sfIssuer], viewJ) != tesSUCCESS)

sleState && deleteAMMMPToken(view, sleState, (*sleState)[sfIssuer], viewJ) != tesSUCCESS)

xrplf-ai-reviewer · 2026-03-31T12:37:32Z

include/xrpl/protocol/MPTIssue.h

+{
+    static_assert(sizeof(MPTID) == (sizeof(std::uint32_t) + sizeof(AccountID)));
+    AccountID const* accountId =
+        reinterpret_cast<AccountID const*>(mptid.data() + sizeof(std::uint32_t));


Strict aliasing violation: reinterpret_cast from uint8_t* to AccountID const* is UB. In a consensus system, compiler-version-dependent behavior is a real risk. Use std::memcpy or std::bit_cast.

xrplf-ai-reviewer · 2026-03-31T12:37:32Z

include/xrpl/tx/invariants/MPTInvariant.h

+    enum Order { Before = 0, After = 1 };
+    struct MPTData
+    {
+        std::array<std::int64_t, After + 1> outstanding;


outstanding lacks a default initializer unlike mptAmount{0}. Default-construction yields indeterminate values.

Suggested change

std::array<std::int64_t, After + 1> outstanding;

std::array<std::int64_t, After + 1> outstanding{};

See: std::array

xrplf-ai-reviewer · 2026-03-31T12:37:33Z

src/libxrpl/protocol/STParsedJSON.cpp

                        }

-                        p.emplace_back(uAccount, uCurrency, uIssuer, hasCurrency);
+                        p.emplace_back(uAccount, uAsset, uIssuer, hasAsset);


When mpt_issuance_id is present but no issuer field, uIssuer stays as zero-AccountID. The issuer check only fires when issuer IS provided. Fix: auto-populate uIssuer from the MPTID when isMPT and no issuer was supplied.

Issuer can be a path element. If it's not included in the path then it should not be set.

xrplf-ai-reviewer · 2026-03-31T12:37:33Z

src/libxrpl/tx/paths/PaySteps.cpp

+        // can be different from the issuer. If cur is MPT then curAsset
+        // is just set to MPTID.
+        if (curAsset.holds<MPTIssue>() && cur->hasCurrency())
        {


curAsset = Issue{} sets a zero-account issuer when transitioning from MPT to Currency. If curAsset.getIssuer() is used before a subsequent account/issuer node sets it, the zero issuer propagates into step construction. Is this intentional?

The issuer is set in the code block right after this transition.

xrplf-ai-reviewer · 2026-03-31T12:37:33Z

src/libxrpl/tx/paths/PaySteps.cpp

+        // implied to be the second-to-last step of the path. If normPath.back
+        // is an offer, which sells MPT then the added path element account is
+        // the MPT's issuer.
+        if (!((normPath.back().isAccount() &&


lastAsset.getPathAsset() != deliver compares PathAsset vs Asset — previously this only compared currencies. If operator!= also compares issuers, path normalization behavior changes and could affect payment routing correctness. Verify the semantics match the original intent.

For currencies it compares the currency only. For MPT, it compares MPTID, which must include the issuer.

xrplf-ai-reviewer · 2026-03-31T12:37:33Z

include/xrpl/tx/paths/Offer.h


-template <class TIn, class TOut>
+template <StepAmount TIn, StepAmount TOut>
 TAmounts<TIn, TOut>


TOffer::send() unconditionally passes AllowMPTOverflow::Yes. DEX offer settlement can temporarily cause MPT sfOutstandingAmount to exceed sfMaximumAmount. Is there reconciliation logic ensuring final outstanding stays within bounds?

The logic balances it out. MPTInvariant has the ultimate overflow check.

xrplf-ai-reviewer · 2026-03-31T12:37:33Z

include/xrpl/ledger/ApplyView.h

+     * BookStep limits it's output to 150USD. This in turn limits A3's send
+     * amount to 150XRP: A1 buys 100XRP and sells 100USD to A3. This doesn't
+     * change OutstandingAmount. GW buys 50XRP and sells 50USD to A3. This
+     * changes OutstandingAmount 10 1,000USD.


Typo in doc comment: 10 should be to.

Suggested change

* changes OutstandingAmount 10 1,000USD.

* changes OutstandingAmount to 1,000USD.

github-actions · 2026-03-31T16:03:21Z

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

github-actions · 2026-03-31T20:02:32Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

shawnxie999 · 2026-03-31T20:35:57Z

src/libxrpl/tx/paths/MPTEndpointStep.cpp

 MPTEndpointStep<TDerived>::lineQualityIn(ReadView const& v) const
 {
    // dst quality in
    return QUALITY_ONE;


nit - exclude line cov

Why should it be excluded?

hmm its seems codecov is complaining that this line is not covered

shawnxie999 · 2026-03-31T20:36:27Z

src/libxrpl/tx/paths/MPTEndpointStep.cpp

        JLOG(j_.debug()) << "MPTEndpointStep: specified bad account.";
        return temBAD_PATH;
    }

    if (src_ == dst_)
    {
        JLOG(j_.debug()) << "MPTEndpointStep: same src and dst.";
        return temBAD_PATH;
    }

    auto const sleSrc = ctx.view.read(keylet::account(src_));
    if (!sleSrc)
    {
        JLOG(j_.warn()) << "MPTEndpointStep: can't receive MPT from non-existent issuer: " << src_;
        return terNO_ACCOUNT;
    }


nit - exclude line cov for all returns

shawnxie999 · 2026-03-31T20:37:07Z

src/libxrpl/tx/paths/MPTEndpointStep.cpp

            UNREACHABLE(
                "xrpl::MPTEndpointStep::check : prev seen book without a "
                "prev step");
            return temBAD_PATH_LOOP;


nit - exclude line cov

shawnxie999 · 2026-03-31T21:32:25Z

src/libxrpl/tx/paths/MPTEndpointStep.cpp

        // limiting node
        MPTAmount const actualIn = mulRatio(maxSrcToDst, srcQOut, QUALITY_ONE, /*roundUp*/ true);
        // Don't have to factor in dstQIn since it's always QUALITY_ONE
        MPTAmount const out = maxSrcToDst;
        setCacheLimiting(actualIn, maxSrcToDst, out, srcDebtDir);
        auto const ter = rippleCredit(
            sb,
            src_,
            dst_,


nit - might be worth adding a comment that this code will not be reachable, as executing forward direction should not be limiting since the reverse direction already did the job

If something is unreachable, consider adding the UNREACHABLE assertion.

github-actions · 2026-04-01T13:52:03Z

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

github-actions · 2026-04-01T17:46:45Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

gregtatcam force-pushed the feature/mpt-v2e branch 3 times, most recently from 0f1e958 to 5134103 Compare February 12, 2025 14:47

gregtatcam requested review from shawnxie999 and vlntb February 13, 2025 18:50

gregtatcam force-pushed the feature/mpt-v2e branch 5 times, most recently from 8e91fa9 to 9962c46 Compare March 5, 2025 17:00

gregtatcam force-pushed the feature/mpt-v2e branch 8 times, most recently from 8e0faf6 to 2c76683 Compare March 15, 2025 22:25

gregtatcam force-pushed the feature/mpt-v2e branch 2 times, most recently from 2b18e49 to 77ef360 Compare March 20, 2025 19:00

shawnxie999 reviewed Mar 25, 2025

View reviewed changes

include/xrpl/protocol/AmountConversions.h Outdated Show resolved Hide resolved

src/libxrpl/tx/transactors/token/MPTUtils.cpp Outdated Show resolved Hide resolved

gregtatcam force-pushed the feature/mpt-v2e branch 3 times, most recently from 70aac26 to c7be58f Compare March 27, 2025 16:48

mvadari mentioned this pull request Mar 27, 2025

fix: add MPTCurrency type instead of MPTAmount for Issues XRPLF/xrpl-py#822

Merged

2 tasks

gregtatcam added 2 commits April 3, 2025 14:28

Add MPT support to DEX:

a0ee987

* Payment * OfferCreate * CheckCash * All AMM transactions * Pathfinding Retire FlowCross feature

Extend AMM/MPT test framework to facilitate MPT/DEX unit-tests

7a42ba9

Fix clang-tidy errors

adabcac

xrplf-ai-reviewer bot reviewed Mar 31, 2026

View reviewed changes

Address AI feedback

06b6c5c

github-actions bot added the PR: has conflicts label Mar 31, 2026

gregtatcam added 4 commits March 31, 2026 13:49

Merge branch 'develop' into feature/mpt-v2e

4485977

Fix clang-format

4e3d3ef

Merge branch 'develop' into feature/mpt-v2e

36dec12

Fix clang-format

97189fb

github-actions bot removed the PR: has conflicts label Mar 31, 2026

shawnxie999 reviewed Mar 31, 2026

View reviewed changes

Merge branch 'develop' into feature/mpt-v2e

07b915f

shawnxie999 reviewed Mar 31, 2026

View reviewed changes

Fix clang-tidy const errors.

4ab4d7a

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

shawnxie999 approved these changes Apr 1, 2026

View reviewed changes

gregtatcam added 2 commits April 1, 2026 13:37

Minor refactorings

1ef173b

Merge branch 'develop' into feature/mpt-v2e

583cbdf

github-actions bot removed the PR: has conflicts label Apr 1, 2026

gregtatcam added 4 commits April 1, 2026 14:33

Fix clang-tidy errors

50c6914

Enforce ValidTaker type restriction on both TakerPays and TakerGets.

fea848b

Fix clang-tidy const

8baf529

Fix comment

281a7f9

	JLOG(viewJ.error()) << "removeDeletedTrustLines: deleted mpts exceed 2 " << mpts.size();
	JLOG(viewJ.error()) << "removeDeletedMPTs: deleted mpts exceed 2 " << mpts.size();

	deleteAMMMPToken(view, sleState, (*sleState)[sfIssuer], viewJ) != tesSUCCESS)
	sleState && deleteAMMMPToken(view, sleState, (*sleState)[sfIssuer], viewJ) != tesSUCCESS)

	std::array<std::int64_t, After + 1> outstanding;
	std::array<std::int64_t, After + 1> outstanding{};

	* changes OutstandingAmount 10 1,000USD.
	* changes OutstandingAmount to 1,000USD.

Conversation

gregtatcam commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Test Plan

Uh oh!

codecov bot commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shawnxie999 commented Feb 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 30, 2026

Uh oh!

gregtatcam commented Mar 31, 2026

Uh oh!

xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 31, 2026

Uh oh!

github-actions bot commented Mar 31, 2026

Uh oh!

shawnxie999 Mar 31, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gregtatcam commented Feb 12, 2025 •

edited

Loading

codecov bot commented Feb 12, 2025 •

edited

Loading

shawnxie999 Mar 31, 2026 •

edited

Loading