From e8de5e5d51a9bc8ea404a2630c724c404eb1fc0a Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 3 Jun 2026 11:41:19 -0400 Subject: [PATCH] Fix flawed pathfinding constraints will completely block DEX trades for MPT holders --- src/test/app/PathMPT_test.cpp | 66 +++++++++++++++++++++++++- src/xrpld/rpc/detail/AccountAssets.cpp | 8 ++-- src/xrpld/rpc/detail/MPT.h | 11 ++--- src/xrpld/rpc/detail/Pathfinder.cpp | 7 ++- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/test/app/PathMPT_test.cpp b/src/test/app/PathMPT_test.cpp index ec6e1c8fc9..0bc7f6d51d 100644 --- a/src/test/app/PathMPT_test.cpp +++ b/src/test/app/PathMPT_test.cpp @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include #include @@ -231,6 +233,67 @@ public: env.require(Balance("bob", usd(24))); } + void + maxedOutMPTPathfinding() + { + testcase("maxed-out MPT pathfinding"); + using namespace jtx; + + auto hasMPT = [](auto const& assets, MPT const& mpt) { + for (auto const& asset : assets) + { + if (asset.template holds() && asset.template get() == mpt.mpt()) + return true; + } + return false; + }; + + Env env = pathTestEnv(); + auto const gw = Account("gateway"); + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const carol = Account("carol"); + + env.fund(XRP(10'000), gw, alice, bob, carol); + env.close(); + + MPT const usd = + MPTTester({.env = env, .issuer = gw, .holders = {alice, bob, carol}, .maxAmt = 100}); + env(pay(gw, alice, usd(90))); + env(pay(gw, bob, usd(10))); + env.close(); + + auto const cache = + std::make_shared(env.current(), env.app().getJournal("AssetCache")); + + BEAST_EXPECT(hasMPT(accountSourceAssets(alice.id(), cache, false), usd)); + BEAST_EXPECT(hasMPT(accountDestAssets(bob.id(), cache, false), usd)); + BEAST_EXPECT(hasMPT(accountDestAssets(carol.id(), cache, false), usd)); + + // A fully minted issuance should not be advertised as issuer-side + // mintable source liquidity. + BEAST_EXPECT(!hasMPT(accountSourceAssets(gw.id(), cache, false), usd)); + + auto [st, sa, da] = findPaths(env, alice, bob, usd(5)); + BEAST_EXPECT(st.empty()); + BEAST_EXPECT(equal(sa, usd(5))); + BEAST_EXPECT(equal(da, usd(5))); + + env(offer(carol, usd(5), XRP(5))); + env.close(); + + std::tie(st, sa, da) = findPaths(env, alice, bob, drops(-1), usd(100).value()); + BEAST_EXPECT(sa == usd(5)); + BEAST_EXPECT(equal(da, XRP(5))); + if (BEAST_EXPECT(st.size() == 1 && st[0].size() == 1)) + { + auto const& pathElem = st[0][0]; + BEAST_EXPECT( + pathElem.isOffer() && pathElem.getIssuerID() == xrpAccount() && + pathElem.getCurrency() == xrpCurrency()); + } + } + void pathFind(bool const domainEnabled) { @@ -447,7 +510,7 @@ public: // // Background. The MPT-DEX refactor of `Pathfinder::Pathfinder` // (src/xrpld/rpc/detail/Pathfinder.cpp) replaced the original - // `mSrcAmount(srcAmount.value_or(...))` initialiser with an + // `mSrcAmount(srcAmount.value_or(...))` initializer with an // unconditional `amountFromPathAsset(...)` call. The latter always // returns the negative "no limit" STAmount sentinel, so the // `srcAmount` constructor parameter became dead code: @@ -566,6 +629,7 @@ public: noDirectPathNoIntermediaryNoAlternatives(); directPathNoIntermediary(); paymentAutoPathFind(); + maxedOutMPTPathfinding(); convertAllSendMaxRanking(); for (auto const domainEnabled : {false, true}) { diff --git a/src/xrpld/rpc/detail/AccountAssets.cpp b/src/xrpld/rpc/detail/AccountAssets.cpp index 67b9174fe3..0e71836b74 100644 --- a/src/xrpld/rpc/detail/AccountAssets.cpp +++ b/src/xrpld/rpc/detail/AccountAssets.cpp @@ -49,7 +49,7 @@ accountSourceAssets( { for (auto const& rspEntry : *mpts) { - if (!rspEntry.isZeroBalance() && !rspEntry.isMaxedOut()) + if (rspEntry.canSend(account)) assets.insert(rspEntry.getMptID()); } } @@ -86,8 +86,10 @@ accountDestAssets( { for (auto const& rspEntry : *mpts) { - if (rspEntry.isZeroBalance() && !rspEntry.isMaxedOut()) - assets.insert(rspEntry.getMptID()); + // Any cached MPT entry means this account already has an issuance + // or MPToken object. A maxed-out issuance does not prevent + // receiving existing MPT from another holder. + assets.insert(rspEntry.getMptID()); } } diff --git a/src/xrpld/rpc/detail/MPT.h b/src/xrpld/rpc/detail/MPT.h index 5cf2d5490f..334b710d50 100644 --- a/src/xrpld/rpc/detail/MPT.h +++ b/src/xrpld/rpc/detail/MPT.h @@ -31,14 +31,11 @@ public: return mptID_; } [[nodiscard]] bool - isZeroBalance() const + canSend(AccountID const& account) const { - return zeroBalance_; - } - [[nodiscard]] bool - isMaxedOut() const - { - return maxedOut_; + // A maxed-out issuance only prevents the issuer from creating more + // MPT. Holders can still send existing balances. + return account == getMPTIssuer(mptID_) ? !maxedOut_ : !zeroBalance_; } }; diff --git a/src/xrpld/rpc/detail/Pathfinder.cpp b/src/xrpld/rpc/detail/Pathfinder.cpp index 44aa7193ac..427c9e806e 100644 --- a/src/xrpld/rpc/detail/Pathfinder.cpp +++ b/src/xrpld/rpc/detail/Pathfinder.cpp @@ -828,7 +828,7 @@ Pathfinder::getPathsOut( if (pathAsset.get() != mpt.getMptID()) { } - else if (mpt.isZeroBalance() || mpt.isMaxedOut()) + else if (!mpt.canSend(account)) { } else if (bAuthRequired) @@ -1102,7 +1102,10 @@ Pathfinder::addLink( } if constexpr (kIsMpt) { - return asset.isZeroBalance() || asset.isMaxedOut() || + // `asset` came from uEndAccount's cached MPTs. + // `acct` is the next issuer hop, not the + // account whose balance is being tested. + return !asset.canSend(uEndAccount) || requireAuth(*ledger_, MPTIssue{asset}, acct); } };