From 249f7ab6c7eb15ca1b7059f823268650ea6c79c1 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 5 Jun 2026 14:08:34 -0400 Subject: [PATCH] Enforce MPT CanTransfer on AMM LPTokens transfers --- include/xrpl/ledger/View.h | 19 +++ src/libxrpl/ledger/View.cpp | 27 +++++ src/libxrpl/ledger/helpers/TokenHelpers.cpp | 24 +++- src/libxrpl/tx/paths/DirectStep.cpp | 10 ++ src/test/app/LPTokenTransfer_test.cpp | 128 ++++++++++++++++++++ 5 files changed, 205 insertions(+), 3 deletions(-) diff --git a/include/xrpl/ledger/View.h b/include/xrpl/ledger/View.h index 255413e459..7f8fef60e7 100644 --- a/include/xrpl/ledger/View.h +++ b/include/xrpl/ledger/View.h @@ -65,6 +65,25 @@ isLPTokenFrozen( Asset const& asset, Asset const& asset2); +/** Check whether an AMM LPToken may be transferred between @p from and @p to. + + @p lpTokenIssuer is the issuer of the LPToken being moved. If it is not an + AMM account the token is not an LPToken and the transfer is unconditionally + permitted. Otherwise, for each MPT pool asset of that AMM, canTransfer() must + permit the transfer (which exempts the MPT issuer). Non-MPT pool assets are + always transferable by this check, so it is implicitly gated by + featureMPTokensV2 (MPTs can only be AMM pool assets once V2 is enabled). + + @return tesSUCCESS if permitted, otherwise the canTransfer() failure code + (e.g. tecNO_AUTH) of the first MPT pool asset that disallows it. + */ +[[nodiscard]] TER +canTransferLPToken( + ReadView const& view, + AccountID const& from, + AccountID const& to, + AccountID const& lpTokenIssuer); + // Return the list of enabled amendments [[nodiscard]] std::set getEnabledAmendments(ReadView const& view); diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index fdd7998609..a7699dae71 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -129,6 +129,33 @@ isLPTokenFrozen( return isFrozen(view, account, asset) || isFrozen(view, account, asset2); } +TER +canTransferLPToken( + ReadView const& view, + AccountID const& from, + AccountID const& to, + AccountID const& lpTokenIssuer) +{ + // Only AMM-issued LPTokens are subject to this check. The LPToken's issuer + // is the AMM account; if it is not an AMM, this is not an LPToken. + auto const sleIssuer = view.read(keylet::account(lpTokenIssuer)); + if (!sleIssuer || !sleIssuer->isFieldPresent(sfAMMID)) + return tesSUCCESS; + + auto const sleAmm = view.read(keylet::amm((*sleIssuer)[sfAMMID])); + if (!sleAmm) + return tecINTERNAL; // LCOV_EXCL_LINE + + auto const transferable = [&](Asset const& a) -> TER { + if (!a.holds()) + return tesSUCCESS; + return canTransfer(view, a.get(), from, to); + }; + if (auto const err = transferable((*sleAmm)[sfAsset]); !isTesSuccess(err)) + return err; + return transferable((*sleAmm)[sfAsset2]); +} + bool areCompatible( ReadView const& validLedger, diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 7191456868..ad15a0ceef 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -34,7 +34,7 @@ namespace xrpl { -// Forward declaration for function that remains in View.h/cpp +// Forward declarations for functions that remain in View.h/cpp bool isLPTokenFrozen( ReadView const& view, @@ -42,6 +42,13 @@ isLPTokenFrozen( Asset const& asset, Asset const& asset2); +TER +canTransferLPToken( + ReadView const& view, + AccountID const& from, + AccountID const& to, + AccountID const& lpTokenIssuer); + //------------------------------------------------------------------------------ // // Freeze checking (Asset-based) @@ -282,8 +289,19 @@ accountHolds( } // IOU: Return balance on trust line modulo freeze - SLE::const_pointer const sle = - getLineIfUsable(view, account, currency, issuer, zeroIfFrozen, j); + SLE::const_pointer sle = getLineIfUsable(view, account, currency, issuer, zeroIfFrozen, j); + + // An LPToken whose AMM pool contains an MPT that forbids transfers is not + // spendable, mirroring the pool-asset freeze handling in getLineIfUsable so + // that frozen and non-transferable LPTokens behave identically. issuer is + // the LPToken's AMM account; canTransferLPToken is a no-op for non-AMM + // issuers and non-MPT pool assets, so this is implicitly gated by + // featureMPTokensV2. + if (sle && zeroIfFrozen == FreezeHandling::ZeroIfFrozen && + !isTesSuccess(canTransferLPToken(view, account, account, issuer))) + { + sle = nullptr; + } return getTrustLineBalance(view, sle, account, currency, issuer, returnSpendable, j); } diff --git a/src/libxrpl/tx/paths/DirectStep.cpp b/src/libxrpl/tx/paths/DirectStep.cpp index cc6e75ea3d..a01dbab02b 100644 --- a/src/libxrpl/tx/paths/DirectStep.cpp +++ b/src/libxrpl/tx/paths/DirectStep.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -848,6 +849,15 @@ DirectStepI::check(StrandContext const& ctx) const auto const ter = checkFreeze(ctx.view, src_, dst_, currency_); if (!isTesSuccess(ter)) return ter; + + // An LPToken redeemed against its AMM (dst_ is the LPToken issuer on + // this hop) cannot move if a pool asset is an MPT that forbids + // transfers between these accounts. Treat it like a frozen pool asset + // (return terNO_LINE) so frozen and non-transferable LPTokens behave + // identically. A no-op unless dst_ is an AMM whose pool holds such an + // MPT (so it is implicitly gated by featureMPTokensV2). + if (!isTesSuccess(canTransferLPToken(ctx.view, src_, dst_, dst_))) + return terNO_LINE; } // If previous step was a direct step then we need to check diff --git a/src/test/app/LPTokenTransfer_test.cpp b/src/test/app/LPTokenTransfer_test.cpp index 4bf2db9515..d7cd81bf58 100644 --- a/src/test/app/LPTokenTransfer_test.cpp +++ b/src/test/app/LPTokenTransfer_test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -20,6 +21,8 @@ #include #include +#include + namespace xrpl::test { class LPTokenTransfer_test : public jtx::AMMTest @@ -430,6 +433,129 @@ class LPTokenTransfer_test : public jtx::AMMTest } } + void + testMPTCanTransferDirectStep(FeatureBitset features) + { + testcase("MPT CanTransfer DirectStep"); + + using namespace jtx; + + // An MPT can only be an AMM pool asset once featureMPTokensV2 is + // enabled, so this behavior is only meaningful when V2 is present, and + // is independent of fixFrozenLPTokenTransfer. + if (!features[featureMPTokensV2]) + return; + + // gw issues an MPT used as one of the AMM pool assets. gw (the MPT + // issuer) seeds the pool and hands LP tokens to alice. Transferring LP + // tokens between two non-issuer holders is only permitted when the + // pool MPT allows transfers (lsfMPTCanTransfer); issuer-involving + // transfers are always permitted. The check fires on the redeem step + // against the AMM account via canTransferLPToken(). + auto scenario = [&](std::uint32_t mptFlags) { + Env env{*this, features}; + env.fund(XRP(30'000), gw_, alice_, bob_); + env.close(); + + // gw is the MPT issuer, so it may seed the pool regardless of + // whether the MPT permits third-party transfers. + MPT const btc = MPTTester( + {.env = env, .issuer = gw_, .holders = {alice_}, .pay = 1'000, .flags = mptFlags}); + + AMM const amm(env, gw_, XRP(10'000), btc(10'000)); + auto const lpIssue = amm.lptIssue(); + + env.trust(STAmount{lpIssue, 100'000}, alice_); + env.trust(STAmount{lpIssue, 100'000}, bob_); + env.close(); + + // Issuer-involving LP token transfer is always allowed (gw is the + // pool MPT's issuer), even when the MPT lacks CanTransfer. + env(pay(gw_, alice_, STAmount{lpIssue, 1'000})); + env.close(); + + // Transfer between two non-issuer holders is allowed only if the + // pool MPT has CanTransfer set; otherwise the redeem step against + // the AMM account blocks it, behaving like a frozen pool asset + // (terNO_LINE -> tecPATH_DRY). + if ((mptFlags & tfMPTCanTransfer) != 0u) + { + env(pay(alice_, bob_, STAmount{lpIssue, 100})); + } + else + { + env(pay(alice_, bob_, STAmount{lpIssue, 100}), Ter(tecPATH_DRY)); + } + env.close(); + }; + + // Pool MPT without CanTransfer blocks third-party LP token transfers. + scenario(tfMPTCanTrade); + + // Pool MPT with CanTransfer allows them. + scenario(tfMPTCanTrade | tfMPTCanTransfer); + } + + void + testMPTCanTransferOffer(FeatureBitset features) + { + testcase("MPT CanTransfer Offer"); + + using namespace jtx; + + if (!features[featureMPTokensV2]) + return; + + // Parity with frozen LP tokens for the order book: a non-transferable + // pool MPT makes the LP token un-spendable (canTransferLPToken zeroes + // the spendable balance in accountHolds, just as isLPTokenFrozen does), + // so an offer to sell it cannot be funded - the same tecUNFUNDED_OFFER + // outcome as freezing a pool asset (see testOfferCreation). + auto scenario = [&](std::uint32_t mptFlags) { + Env env{*this, features}; + env.fund(XRP(30'000), gw_, carol_); + env.close(); + + MPT const btc = MPTTester( + {.env = env, .issuer = gw_, .holders = {carol_}, .pay = 1'000, .flags = mptFlags}); + + AMM const amm(env, gw_, XRP(10'000), btc(10'000)); + auto const lpIssue = amm.lptIssue(); + + env.trust(STAmount{lpIssue, 100'000}, carol_); + env.close(); + + // gw (the pool MPT issuer) seeds carol_ with LP tokens; issuer + // involving transfers are always allowed. + env(pay(gw_, carol_, STAmount{lpIssue, 1'000})); + env.close(); + + // carol_ tries to create an offer to sell the LP token. + if ((mptFlags & tfMPTCanTransfer) != 0u) + { + env(offer(carol_, XRP(10), STAmount{lpIssue, 10}), Txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol_, 1)); + } + else + { + // Non-transferable pool MPT => LP token un-spendable => the + // sell offer is unfunded, just as if a pool asset were frozen. + env(offer(carol_, XRP(10), STAmount{lpIssue, 10}), + Txflags(tfPassive), + Ter(tecUNFUNDED_OFFER)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol_, 0)); + } + }; + + // Pool MPT without CanTransfer: LP token sell offer is unfunded. + scenario(tfMPTCanTrade); + + // Pool MPT with CanTransfer: LP token sell offer is created. + scenario(tfMPTCanTrade | tfMPTCanTransfer); + } + public: void run() override @@ -444,6 +570,8 @@ public: testOfferCrossing(features); testCheck(features); testNFTOffers(features); + testMPTCanTransferDirectStep(features); + testMPTCanTransferOffer(features); } } };