Fix flawed pathfinding constraints will completely block DEX trades for MPT holders

src/test/app/PathMPT_test.cpp

@@ -14,6 +14,8 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/RPCHandler.h>
 #include <xrpld/rpc/Role.h>
+#include <xrpld/rpc/detail/AccountAssets.h>
+#include <xrpld/rpc/detail/AssetCache.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
@@ -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<MPTID>() && asset.template get<MPTID>() == 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<AssetCache>(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})
         {

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());
         }
     }
 

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_;
     }
 };
 

src/xrpld/rpc/detail/Pathfinder.cpp

@@ -828,7 +828,7 @@ Pathfinder::getPathsOut(
                         if (pathAsset.get<MPTID>() != 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);
                             }
                         };