test: Fix clang-tidy issues in new Vault+AMM tests

- AMM_test: directly include <xrpl/protocol/MPTIssue.h>
  (misc-include-cleaner).
- Vault_test::testMutateCanTransferAfterDeposit: declare local Vault
  const (misc-const-correctness).

src/libxrpl/ledger/helpers/LendingHelpers.cpp

@@ -1857,18 +1857,8 @@ loanMakePayment(
 
     // -------------------------------------------------------------
     // overpayment handling
-    //
-    // If the "fixSecurity3_1_3" amendment is enabled, truncate "amount",
-    // at the loan scale. If the raw value is used, the overpayment
-    // amount could be meaningless dust. Trying to process such a small
-    // amount will, at best, waste time when all the result values round
-    // to zero. At worst, it can cause logical errors with tiny amounts
-    // of interest that don't add up correctly.
-    auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3)
-        ? roundToAsset(asset, amount, loanScale, Number::towards_zero)
-        : amount;
     if (paymentType == LoanPaymentType::overpayment && loan->isFlag(lsfLoanOverpayment) &&
-        paymentRemainingProxy > 0 && totalPaid < roundedAmount &&
+        paymentRemainingProxy > 0 && totalPaid < amount &&
         numPayments < loanMaximumPaymentsPerTransaction)
     {
         TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)};
@@ -1877,7 +1867,7 @@ loanMakePayment(
         // It shouldn't be possible for the overpayment to be greater than
         // totalValueOutstanding, because that would have been processed as
         // another normal payment. But cap it just in case.
-        Number const overpayment = std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy);
+        Number const overpayment = std::min(amount - totalPaid, *totalValueOutstandingProxy);
 
         detail::ExtendedPaymentComponents const overpaymentComponents =
             detail::computeOverpaymentComponents(

src/libxrpl/tx/transactors/lending/LoanPay.cpp

@@ -30,7 +30,6 @@
 #include <bit>
 #include <cstdint>
 #include <memory>
-#include <vector>
 
 namespace xrpl {
 
@@ -144,7 +143,6 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
     Number const numPaymentEstimate = static_cast<std::int64_t>(amount / regularPayment);
 
     // Charge one base fee per paymentsPerFeeIncrement payments, rounding up.
-    // This set round is safe because there's a mode guard just above
     Number::setround(Number::upward);
     auto const feeIncrements = std::max(
         std::int64_t(1),
@@ -442,10 +440,9 @@ LoanPay::doApply()
     // Vault object state changes
     view.update(vaultSle);
 
-    Number const assetsAvailableBefore = *assetsAvailableProxy;
-    Number const assetsTotalBefore = *assetsTotalProxy;
 #if !NDEBUG
     {
+        Number const assetsAvailableBefore = *assetsAvailableProxy;
         Number const pseudoAccountBalanceBefore = accountHolds(
             view,
             vaultPseudoAccount,
@@ -469,6 +466,16 @@ LoanPay::doApply()
         "xrpl::LoanPay::doApply",
         "assets available must not be greater than assets outstanding");
 
+    if (*assetsAvailableProxy > *assetsTotalProxy)
+    {
+        // LCOV_EXCL_START
+        JLOG(j_.fatal()) << "Vault assets available must not be greater "
+                            "than assets outstanding. Available: "
+                         << *assetsAvailableProxy << ", Total: " << *assetsTotalProxy;
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
+
     JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw
                      << ", total paid to vault rounded: " << totalPaidToVaultRounded
                      << ", total paid to broker: " << totalPaidToBroker
@@ -494,68 +501,12 @@ LoanPay::doApply()
     associateAsset(*vaultSle, asset);
 
     // Duplicate some checks after rounding
-    Number const assetsAvailableAfter = *assetsAvailableProxy;
-    Number const assetsTotalAfter = *assetsTotalProxy;
-
     XRPL_ASSERT_PARTS(
-        assetsAvailableAfter <= assetsTotalAfter,
+        *assetsAvailableProxy <= *assetsTotalProxy,
         "xrpl::LoanPay::doApply",
         "assets available must not be greater than assets outstanding");
-    if (assetsAvailableAfter == assetsAvailableBefore)
-    {
-        // An unchanged assetsAvailable indicates that the amount paid to the
-        // vault was zero, or rounded to zero. That should be impossible, but I
-        // can't rule it out for extreme edge cases, so fail gracefully if it
-        // happens.
-        //
-        // LCOV_EXCL_START
-        JLOG(j_.warn()) << "LoanPay: Vault assets available unchanged after rounding: "  //
-                        << "Before: " << assetsAvailableBefore                           //
-                        << ", After: " << assetsAvailableAfter;
-        return tecPRECISION_LOSS;
-        // LCOV_EXCL_STOP
-    }
-    if (paymentParts->valueChange != beast::zero && assetsTotalAfter == assetsTotalBefore)
-    {
-        // Non-zero valueChange with an unchanged assetsTotal indicates that the
-        // actual value change rounded to zero. That should be impossible, but I
-        // can't rule it out for extreme edge cases, so fail gracefully if it
-        // happens.
-        //
-        // LCOV_EXCL_START
-        JLOG(j_.warn())
-            << "LoanPay: Vault assets expected change, but unchanged after rounding: "  //
-            << "Before: " << assetsTotalBefore                                          //
-            << ", After: " << assetsTotalAfter                                          //
-            << ", ValueChange: " << paymentParts->valueChange;
-        return tecPRECISION_LOSS;
-        // LCOV_EXCL_STOP
-    }
-    if (paymentParts->valueChange == beast::zero && assetsTotalAfter != assetsTotalBefore)
-    {
-        // A change in assetsTotal when there was no valueChange indicates that
-        // something really weird happened. That should be flat out impossible.
-        //
-        // LCOV_EXCL_START
-        JLOG(j_.fatal()) << "LoanPay: Vault assets changed unexpectedly after rounding: "  //
-                         << "Before: " << assetsTotalBefore                                //
-                         << ", After: " << assetsTotalAfter                                //
-                         << ", ValueChange: " << paymentParts->valueChange;
-        return tecINTERNAL;
-        // LCOV_EXCL_STOP
-    }
-    if (assetsAvailableAfter > assetsTotalAfter)
-    {
-        // Assets available are not allowed to be larger than assets total.
-        // LCOV_EXCL_START
-        JLOG(j_.fatal()) << "LoanPay: Vault assets available must not be greater "
-                            "than assets outstanding. Available: "
-                         << assetsAvailableAfter << ", Total: " << assetsTotalAfter;
-        return tecINTERNAL;
-        // LCOV_EXCL_STOP
-    }
 
-    // These three values are used to check that funds are conserved after the transfers
+#if !NDEBUG
     auto const accountBalanceBefore = accountHolds(
         view,
         account_,
@@ -584,6 +535,7 @@ LoanPay::doApply()
               ahIGNORE_AUTH,
               j_,
               SpendableHandling::shFULL_BALANCE);
+#endif
 
     if (totalPaidToVaultRounded != beast::zero)
     {
@@ -619,22 +571,19 @@ LoanPay::doApply()
         return ter;
 
 #if !NDEBUG
-    {
-        Number const pseudoAccountBalanceAfter = accountHolds(
-            view,
-            vaultPseudoAccount,
-            asset,
-            FreezeHandling::fhIGNORE_FREEZE,
-            AuthHandling::ahIGNORE_AUTH,
-            j_);
-        XRPL_ASSERT_PARTS(
-            assetsAvailableAfter == pseudoAccountBalanceAfter,
-            "xrpl::LoanPay::doApply",
-            "vault pseudo balance agrees after");
-    }
-#endif
+    Number const assetsAvailableAfter = *assetsAvailableProxy;
+    Number const pseudoAccountBalanceAfter = accountHolds(
+        view,
+        vaultPseudoAccount,
+        asset,
+        FreezeHandling::fhIGNORE_FREEZE,
+        AuthHandling::ahIGNORE_AUTH,
+        j_);
+    XRPL_ASSERT_PARTS(
+        assetsAvailableAfter == pseudoAccountBalanceAfter,
+        "xrpl::LoanPay::doApply",
+        "vault pseudo balance agrees after");
 
-    // Check that funds are conserved
     auto const accountBalanceAfter = accountHolds(
         view,
         account_,
@@ -663,121 +612,14 @@ LoanPay::doApply()
               ahIGNORE_AUTH,
               j_,
               SpendableHandling::shFULL_BALANCE);
-    auto const balanceScale = [&]() {
-        // Find a reasonable scale to use for the balance comparisons.
-        //
-        // First find the minimum and maximum exponent of all the non-zero balances, before and
-        // after. If min and max are equal, use that value. If they are not, use "max + 1" to reduce
-        // rounding discrepancies without making the result meaningless. Cap the scale at
-        // STAmount::cMaxOffset, just in case the numbers are all very large.
-        std::vector<int> exponents;
 
-        for (auto const& a : {
-                 accountBalanceBefore,
-                 vaultBalanceBefore,
-                 brokerBalanceBefore,
-                 accountBalanceAfter,
-                 vaultBalanceAfter,
-                 brokerBalanceAfter,
-             })
-        {
-            // Exclude zeroes
-            if (a != beast::zero)
-                exponents.push_back(a.exponent());
-        }
-        if (exponents.empty())
-        {
-            UNREACHABLE("xrpl::LoanPay::doApply : all zeroes");
-            return 0;
-        }
-        auto const [minItr, maxItr] = std::ranges::minmax_element(exponents);
-        auto const min = *minItr;
-        auto const max = *maxItr;
-        JLOG(j_.trace()) << "Min scale: " << min << ", max scale: " << max;
-        // IOU rounding can be interesting. We want all the balance checks to agree, but don't want
-        // to round to such an extreme that it becomes meaningless.  e.g. Everything rounds to one
-        // digit. So add 1 to the max (reducing the number of digits after the decimal point by 1)
-        // if the scales are not already all the same.
-        return std::min(min == max ? max : max + 1, STAmount::cMaxOffset);
-    }();
-
-    // No object changes are made below this point
     XRPL_ASSERT_PARTS(
-        Number::getround() == Number::to_nearest,
-        "xrpl::LoanPay::doApply",
-        "Number rounding to_nearest");
-    NumberRoundModeGuard const mg(Number::to_nearest);
-
-    auto const accountBalanceBeforeRounded = roundToScale(accountBalanceBefore, balanceScale);
-    auto const vaultBalanceBeforeRounded = roundToScale(vaultBalanceBefore, balanceScale);
-    auto const brokerBalanceBeforeRounded = roundToScale(brokerBalanceBefore, balanceScale);
-
-    auto const totalBalanceBefore = accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore;
-    auto const totalBalanceBeforeRounded = roundToScale(totalBalanceBefore, balanceScale);
-
-    JLOG(j_.trace()) << "Before: "  //
-                     << "account " << Number(accountBalanceBeforeRounded) << " ("
-                     << Number(accountBalanceBefore) << ")"
-                     << ", vault " << Number(vaultBalanceBeforeRounded) << " ("
-                     << Number(vaultBalanceBefore) << ")"
-                     << ", broker " << Number(brokerBalanceBeforeRounded) << " ("
-                     << Number(brokerBalanceBefore) << ")"
-                     << ", total " << Number(totalBalanceBeforeRounded) << " ("
-                     << Number(totalBalanceBefore) << ")";
-
-    auto const accountBalanceAfterRounded = roundToScale(accountBalanceAfter, balanceScale);
-    auto const vaultBalanceAfterRounded = roundToScale(vaultBalanceAfter, balanceScale);
-    auto const brokerBalanceAfterRounded = roundToScale(brokerBalanceAfter, balanceScale);
-
-    auto const totalBalanceAfter = accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter;
-    auto const totalBalanceAfterRounded = roundToScale(totalBalanceAfter, balanceScale);
-
-    JLOG(j_.trace()) << "After: "  //
-                     << "account " << Number(accountBalanceAfterRounded) << " ("
-                     << Number(accountBalanceAfter) << ")"
-                     << ", vault " << Number(vaultBalanceAfterRounded) << " ("
-                     << Number(vaultBalanceAfter) << ")"
-                     << ", broker " << Number(brokerBalanceAfterRounded) << " ("
-                     << Number(brokerBalanceAfter) << ")"
-                     << ", total " << Number(totalBalanceAfterRounded) << " ("
-                     << Number(totalBalanceAfter) << ")";
-
-    auto const accountBalanceChange = accountBalanceAfter - accountBalanceBefore;
-    auto const vaultBalanceChange = vaultBalanceAfter - vaultBalanceBefore;
-    auto const brokerBalanceChange = brokerBalanceAfter - brokerBalanceBefore;
-
-    auto const totalBalanceChange = accountBalanceChange + vaultBalanceChange + brokerBalanceChange;
-    auto const totalBalanceChangeRounded = roundToScale(totalBalanceChange, balanceScale);
-
-    JLOG(j_.trace()) << "Changes: "                                    //
-                     << "account " << to_string(accountBalanceChange)  //
-                     << ", vault " << to_string(vaultBalanceChange)    //
-                     << ", broker " << to_string(brokerBalanceChange)  //
-                     << ", total " << to_string(totalBalanceChangeRounded) << " ("
-                     << Number(totalBalanceChange) << ")";
-
-    if (totalBalanceBeforeRounded != totalBalanceAfterRounded)
-    {
-        JLOG(j_.warn()) << "Total rounded balances don't match"
-                        << (totalBalanceChangeRounded == beast::zero ? ", but total changes do"
-                                                                     : "");
-    }
-    if (totalBalanceChangeRounded != beast::zero)
-    {
-        JLOG(j_.warn()) << "Total balance changes don't match"
-                        << (totalBalanceBeforeRounded == totalBalanceAfterRounded
-                                ? ", but total balances do"
-                                : "");
-    }
-
-    // Rounding for IOUs can be weird, so check a few different ways to show
-    // that funds are conserved.
-    XRPL_ASSERT_PARTS(
-        totalBalanceBeforeRounded == totalBalanceAfterRounded ||
-            totalBalanceChangeRounded == beast::zero,
+        accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore ==
+            accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter,
         "xrpl::LoanPay::doApply",
         "funds are conserved (with rounding)");
-
+    XRPL_ASSERT_PARTS(
+        accountBalanceAfter >= beast::zero, "xrpl::LoanPay::doApply", "positive account balance");
     XRPL_ASSERT_PARTS(
         accountBalanceAfter < accountBalanceBefore || account_ == asset.getIssuer(),
         "xrpl::LoanPay::doApply",
@@ -798,6 +640,7 @@ LoanPay::doApply()
         vaultBalanceAfter > vaultBalanceBefore || brokerBalanceAfter > brokerBalanceBefore,
         "xrpl::LoanPay::doApply",
         "vault and/or broker balance increased");
+#endif
 
     return tesSUCCESS;
 }

src/test/app/AMM_test.cpp

@@ -8,6 +8,7 @@
 #include <test/jtx/escrow.h>
 #include <test/jtx/fee.h>
 #include <test/jtx/flags.h>
+#include <test/jtx/mpt.h>
 #include <test/jtx/offer.h>
 #include <test/jtx/paths.h>
 #include <test/jtx/pay.h>
@@ -19,6 +20,7 @@
 #include <test/jtx/ter.h>
 #include <test/jtx/trust.h>
 #include <test/jtx/txflags.h>
+#include <test/jtx/vault.h>
 
 #include <xrpl/basics/Number.h>
 #include <xrpl/basics/base_uint.h>
@@ -35,6 +37,7 @@
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/Quality.h>
 #include <xrpl/protocol/Rules.h>
@@ -7060,10 +7063,200 @@ private:
             {all});
     }
 
+    // Create a single-asset vault, deposit assets so the depositor receives
+    // shares (an MPT issued by the vault pseudo-account), then pair those
+    // shares with XRP in an AMM. Finally do a single-asset deposit of more
+    // shares into the AMM.
+    void
+    testVaultSharesAMM()
+    {
+        testcase("Vault Shares paired with XRP in AMM");
+
+        using namespace jtx;
+
+        // Vaults rely on featureSingleAssetVault (which the AMM_test class
+        // strips by default). MPT-AMM pairs require featureMPTokensV2.
+        FeatureBitset const features{
+            jtx::testable_amendments() | featureSingleAssetVault | featureMPTokensV2};
+
+        Env env{*this, features};
+
+        Account const owner{"vaultOwner"};
+        env.fund(XRP(1'000'000), owner);
+        env.close();
+
+        // Use XRP as the vault asset for simplicity.
+        PrettyAsset const asset{xrpIssue(), 1'000'000};
+
+        Vault const vault{env};
+        auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = asset});
+        env(vaultTx);
+        env.close();
+        if (!BEAST_EXPECT(env.le(vaultKeylet)))
+            return;
+
+        // Deposit 10,000 XRP into the vault. Owner receives shares (MPT)
+        // issued by the vault's pseudo-account.
+        env(vault.deposit(
+            {.depositor = owner, .id = vaultKeylet.key, .amount = asset(10'000).value()}));
+        env.close();
+
+        auto const vaultSle = env.le(vaultKeylet);
+        if (!BEAST_EXPECT(vaultSle))
+            return;
+        MPTID const shareMptID = vaultSle->at(sfShareMPTID);
+        MPTIssue const shareIssue{shareMptID};
+        // The share MPT is issued by the vault's pseudo-account. Memoize so
+        // env.balance() can format share amounts.
+        env.memoize(Account{"vaultPseudo", vaultSle->at(sfAccount)});
+
+        // XRP vaults use scale=6, so a 10,000 XRP deposit yields
+        // 10,000 * 1e6 = 10^10 share units (raw MPT amount).
+        STAmount const sharesHeld = env.balance(owner, shareIssue);
+        BEAST_EXPECT(sharesHeld.mantissa() == 10'000'000'000ull);
+        BEAST_EXPECT(sharesHeld.asset() == shareIssue);
+
+        // Seed the AMM with half the shares + 5,000 XRP.
+        STAmount const halfShares(shareIssue, std::uint64_t{5'000'000'000});
+        AMM ammOwner(env, owner, halfShares, XRP(5'000));
+        BEAST_EXPECT(ammOwner.ammExists());
+
+        // Single-asset deposit: add 2,500,000,000 more shares (a quarter of
+        // the original holding) to the share side of the pool.
+        STAmount const extraShares(shareIssue, std::uint64_t{2'500'000'000});
+        ammOwner.deposit(owner, extraShares);
+
+        // The share-side pool should now equal halfShares + extraShares,
+        // while the XRP-side balance is unchanged at 5,000 XRP.
+        auto const [shareBalance, xrpBalance, lpt] = ammOwner.balances(shareIssue, xrpIssue());
+        BEAST_EXPECT(shareBalance == halfShares + extraShares);
+        BEAST_EXPECT(xrpBalance == XRP(5'000));
+        // Owner now holds the original 10B shares minus what was put into the
+        // AMM (5B seed + 2.5B single-asset deposit) = 2.5B.
+        STAmount const expectedOwnerShares(shareIssue, std::uint64_t{2'500'000'000});
+        BEAST_EXPECT(env.balance(owner, shareIssue) == expectedOwnerShares);
+    }
+
+    // Create a Vault whose underlying asset is a lockable / clawback-able
+    // MPT. Pair the vault shares with XRP in an AMM. Transfer half of the
+    // owner's LP tokens to a second account, then issuer-lock the
+    // underlying MPT, then try to transfer LP tokens / cash out again.
+    //
+    // Locking the underlying MPT cascades up via
+    // `isVaultPseudoAccountFrozen`: the vault-share MPT is treated as
+    // frozen because its underlying is locked. So:
+    //   - LP-token Payment after lock fails (`tecPATH_DRY`).
+    //   - AMM withdrawal of LP tokens fails (`tecFROZEN`).
+    // The LP tokens are effectively stuck for as long as the underlying
+    // MPT remains locked.
+    void
+    testLockedVaultMPTCashOut()
+    {
+        testcase("Cash out LP Tokens after vault MPT locked");
+
+        using namespace jtx;
+
+        FeatureBitset const features{
+            jtx::testable_amendments() | featureSingleAssetVault | featureMPTokensV2};
+
+        Env env{*this, features};
+
+        Account const issuer{"issuer"};
+        Account const owner{"vaultOwner"};
+        Account const trader{"trader"};
+
+        env.fund(XRP(1'000'000), issuer, owner, trader);
+        env.close();
+
+        // Underlying MPT supports lock + clawback. MPTDEXFlags adds
+        // CanTransfer + CanTrade so the vault and AMM can route it.
+        MPTTester mpt(
+            {.env = env,
+             .issuer = issuer,
+             .holders = {owner},
+             .pay = 100'000,
+             .flags = tfMPTCanLock | tfMPTCanClawback | MPTDEXFlags});
+        PrettyAsset const asset = MPT(mpt);
+
+        // Create the vault.
+        Vault const vault{env};
+        auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = asset});
+        env(vaultTx);
+        env.close();
+        if (!BEAST_EXPECT(env.le(vaultKeylet)))
+            return;
+
+        // Deposit 50,000 of the underlying MPT.
+        env(vault.deposit(
+            {.depositor = owner, .id = vaultKeylet.key, .amount = asset(50'000).value()}));
+        env.close();
+
+        auto const vaultSle = env.le(vaultKeylet);
+        if (!BEAST_EXPECT(vaultSle))
+            return;
+        MPTID const shareMptID = vaultSle->at(sfShareMPTID);
+        MPTIssue const shareIssue{shareMptID};
+        env.memoize(Account{"vaultPseudo", vaultSle->at(sfAccount)});
+
+        // MPT vaults use scale=0, so 50,000 deposit -> 50,000 share units.
+        STAmount const sharesHeld = env.balance(owner, shareIssue);
+        BEAST_EXPECT(sharesHeld.mantissa() == 50'000);
+
+        // Create the AMM: 25,000 vault shares + 1,000 XRP.
+        STAmount const seedShares(shareIssue, std::uint64_t{25'000});
+        AMM ammOwner(env, owner, seedShares, XRP(1'000));
+        BEAST_EXPECT(ammOwner.ammExists());
+
+        // The AMM pseudo-account issues the LP tokens; memoize so
+        // env.balance() can format LP-token amounts.
+        env.memoize(Account{"ammPseudo", ammOwner.ammAccount()});
+
+        // Owner's LP token balance after AMM creation.
+        auto const lptIssue = ammOwner.lptIssue();
+        STAmount const lptOwner0 = env.balance(owner, lptIssue);
+        STAmount const lptZero(lptIssue, std::uint32_t{0});
+        BEAST_EXPECT(lptOwner0 != lptZero);
+
+        // Trader needs a trust line to receive LP tokens.
+        STAmount const lptTrustLimit(lptIssue, std::uint64_t{1'000'000'000});
+        env(trust(trader, lptTrustLimit));
+        env.close();
+
+        // Step 1: transfer half the LP tokens from owner -> trader.
+        STAmount const halfLpt(lptIssue, lptOwner0.mantissa() / 2, lptOwner0.exponent());
+        env(pay(owner, trader, halfLpt));
+        env.close();
+        BEAST_EXPECT(env.balance(trader, lptIssue) == halfLpt);
+
+        // Step 2: issuer locks the underlying MPT.
+        mpt.set({.flags = tfMPTLock});
+        env.close();
+
+        // Step 3: transfer LP tokens again. The lock on the underlying MPT
+        // cascades through the vault-share issuance via
+        // isVaultPseudoAccountFrozen, so the AMM-routed Payment fails.
+        STAmount const quarterLpt(lptIssue, lptOwner0.mantissa() / 4, lptOwner0.exponent());
+        env(pay(owner, trader, quarterLpt), ter(tecPATH_DRY));
+        env.close();
+        // Trader's balance is still just the half from before the lock.
+        BEAST_EXPECT(env.balance(trader, lptIssue) == halfLpt);
+
+        // Step 4: try to cash out the LP tokens. The AMM withdrawal must
+        // touch the vault-share side, which is now treated as frozen
+        // because its underlying is locked, so the withdrawal fails.
+        ammOwner.withdrawAll(trader, std::nullopt, ter(tecFROZEN));
+        env.close();
+        // Trader still holds the LP tokens; nothing was redeemed.
+        BEAST_EXPECT(env.balance(trader, lptIssue) == halfLpt);
+        BEAST_EXPECT(env.balance(trader, shareIssue) == STAmount(shareIssue, std::uint64_t{0}));
+    }
+
     void
     run() override
     {
         FeatureBitset const all{testable_amendments()};
+        testVaultSharesAMM();
+        testLockedVaultMPTCashOut();
         testInvalidInstance();
         testInstanceCreate();
         testInvalidDeposit(all);

src/test/app/Loan_test.cpp

@@ -370,11 +370,16 @@ protected:
                         env.balance(vaultPseudo, broker.asset).number());
                     if (ownerCount == 0)
                     {
-                        // The Vault must be perfectly balanced if there
-                        // are no loans outstanding
+                        // Allow some slop for rounding IOUs
+
+                        // TODO: This needs to be an exact match once all the
+                        // other rounding issues are worked out.
                         auto const total = vaultSle->at(sfAssetsTotal);
                         auto const available = vaultSle->at(sfAssetsAvailable);
-                        env.test.BEAST_EXPECT(total == available);
+                        env.test.BEAST_EXPECT(
+                            total == available ||
+                            (!broker.asset.integral() && available != 0 &&
+                             ((total - available) / available < Number(1, -6))));
                         env.test.BEAST_EXPECT(vaultSle->at(sfLossUnrealized) == 0);
                     }
                 }
@@ -7063,140 +7068,6 @@ protected:
         BEAST_EXPECT(afterSecondCoverAvailable == 0);
     }
 
-    void
-    testYieldTheftRounding(std::uint32_t flags)
-    {
-        testcase("Rounding manipulation does not permit yield theft");
-        using namespace jtx;
-        using namespace loan;
-
-        // 1. Setup Environment
-        Env env(*this, all);
-        Account const issuer{"issuer"};
-        Account const lender{"lender"};
-        Account const borrower{"borrower"};
-
-        env.fund(XRP(1000), issuer, lender, borrower);
-        env.close();
-
-        // 2. Asset Selection
-        PrettyAsset const iou = issuer["USD"];
-        env(trust(lender, iou(100'000'000)));
-        env(trust(borrower, iou(100'000'000)));
-        env(pay(issuer, lender, iou(100'000'000)));
-        env(pay(issuer, borrower, iou(100'000'000)));
-        env.close();
-
-        // 3. Create Vault and Broker with High Debt Limit (100M)
-        auto const brokerInfo = createVaultAndBroker(
-            env,
-            iou,
-            lender,
-            {
-                .vaultDeposit = 5'000'000,
-                .debtMax = Number{100'000'000},
-                .coverDeposit = 500'000,
-            });
-        auto const [currentSeq, vaultKeylet] = [&]() {
-            auto const brokerSle = env.le(keylet::loanbroker(brokerInfo.brokerID));
-            auto const currentSeq = brokerSle->at(sfLoanSequence);
-            auto const vaultKeylet = keylet::vault(brokerSle->at(sfVaultID));
-            return std::make_tuple(currentSeq, vaultKeylet);
-        }();
-
-        // 4. Loan Parameters (Attack Vector)
-        Number const principal = 1'000'000;
-        TenthBips32 const interestRate = TenthBips32{1};  // 0.001%
-        std::uint32_t const paymentInterval = 86400;
-        std::uint32_t const paymentTotal = 3650;
-
-        auto const loanSetFee = fee(env.current()->fees().base * 2);
-        env(set(borrower, brokerInfo.brokerID, iou(principal).value(), flags),
-            sig(sfCounterpartySignature, lender),
-            loan::interestRate(interestRate),
-            loan::paymentInterval(paymentInterval),
-            loan::paymentTotal(paymentTotal),
-            fee(loanSetFee));
-        env.close();
-
-        // --- RETRIEVE OBJECTS & SETUP ATTACK ---
-
-        auto borrowerBalance = [&]() { return env.balance(borrower, iou); };
-        auto const borrowerScale = static_cast<STAmount const&>(borrowerBalance()).exponent();
-
-        auto const loanKeylet = keylet::loan(brokerInfo.brokerID, currentSeq);
-        auto const maybePeriodicPayment = [&]() -> std::optional<STAmount> {
-            auto const loanSle = env.le(loanKeylet);
-            if (!BEAST_EXPECT(loanSle))
-                return std::nullopt;
-            // Construct Payment
-            return STAmount{iou, loanSle->at(sfPeriodicPayment)};
-        }();
-        if (!maybePeriodicPayment)
-            return;
-        auto const periodicPayment = *maybePeriodicPayment;
-        auto const roundedPayment = roundToScale(periodicPayment, borrowerScale, Number::upward);
-
-        // ATTACK: Add dust buffer (1e-9) to force 'excess' logic execution
-        STAmount const paymentBuffer{iou, Number(1, -9)};
-        STAmount const attackPayment = periodicPayment + paymentBuffer;
-
-        auto const maybeInitialVaultAssets = [&]() -> std::optional<Number> {
-            auto const vault = env.le(vaultKeylet);
-            if (!BEAST_EXPECT(vault))
-                return std::nullopt;
-            return vault->at(sfAssetsTotal);
-        }();
-        if (!maybeInitialVaultAssets)
-            return;
-        auto const initialVaultAssets = *maybeInitialVaultAssets;
-
-        // 5. Execution Loop
-        int yieldTheftCount = 0;
-        auto previousAssetsTotal = initialVaultAssets;
-
-        for (int i = 0; i < 100; ++i)
-        {
-            auto const balanceBefore = borrowerBalance();
-            env(pay(borrower, loanKeylet.key, attackPayment, flags));
-            env.close();
-            auto const borrowerDelta = balanceBefore - borrowerBalance();
-            BEAST_EXPECT(borrowerDelta.signum() == roundedPayment.signum());
-
-            auto const loanSle = env.le(loanKeylet);
-            if (!BEAST_EXPECT(loanSle))
-                break;
-            auto const updatedPayment = STAmount{iou, loanSle->at(sfPeriodicPayment)};
-            BEAST_EXPECT(
-                (roundToScale(updatedPayment, borrowerScale, Number::upward) == roundedPayment));
-            BEAST_EXPECT(
-                (updatedPayment == periodicPayment) ||
-                (flags == tfLoanOverpayment && i >= 2 && updatedPayment < periodicPayment));
-
-            auto const currentVaultSle = env.le(vaultKeylet);
-            if (!BEAST_EXPECT(currentVaultSle))
-                break;
-
-            auto const currentAssetsTotal = currentVaultSle->at(sfAssetsTotal);
-            auto const delta = currentAssetsTotal - previousAssetsTotal;
-
-            BEAST_EXPECT(
-                (delta == beast::zero && borrowerDelta <= roundedPayment) ||
-                (delta > beast::zero && borrowerDelta > roundedPayment));
-
-            // If tx succeeded but Assets Total didn't change, interest was
-            // stolen.
-            if (delta == beast::zero && borrowerDelta > roundedPayment)
-            {
-                yieldTheftCount++;
-            }
-
-            previousAssetsTotal = currentAssetsTotal;
-        }
-
-        BEAST_EXPECTS(yieldTheftCount == 0, std::to_string(yieldTheftCount));
-    }
-
     // Tests that vault withdrawals work correctly when the vault has unrealized
     // loss from an impaired loan, ensuring the invariant check properly
     // accounts for the loss.
@@ -7335,11 +7206,6 @@ public:
         testLoanPayLateFullPaymentBypassesPenalties();
         testLoanCoverMinimumRoundingExploit();
 #endif
-        for (auto const flags : {0u, tfLoanOverpayment})
-        {
-            testYieldTheftRounding(flags);
-        }
-
         testWithdrawReflectsUnrealizedLoss();
         testInvalidLoanSet();
 

src/test/app/Vault_test.cpp

@@ -6139,6 +6139,141 @@ class Vault_test : public beast::unit_test::suite
         runTest(amendments);
     }
 
+    // Issuer mutates the underlying MPT's lsfMPTCanTransfer / lsfMPTCanTrade
+    // flags after holders have already deposited into a vault. Demonstrates:
+    //
+    //   - VaultDeposit and VaultWithdraw both go through `canTransfer`,
+    //     so clearing lsfMPTCanTransfer freezes every holder's funds in
+    //     the vault until the issuer re-enables the flag (`tecNO_AUTH`).
+    //
+    //   - The issuer is exempt: `canTransfer` short-circuits when either
+    //     side of the transfer is the issuer, so the issuer can still
+    //     deposit and withdraw.
+    //
+    //   - lsfMPTCanTrade is *not* checked by VaultDeposit/VaultWithdraw at
+    //     all — clearing it has no effect on vault I/O. (It only gates
+    //     DEX/AMM operations via `canTrade`.)
+    void
+    testMutateCanTransferAfterDeposit()
+    {
+        using namespace test::jtx;
+        testcase("MPT vault: clearing CanTransfer/CanTrade after deposit");
+
+        Env env{*this, testable_amendments() | featureSingleAssetVault};
+
+        Account const issuer{"issuer"};
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+
+        env.fund(XRP(1'000), issuer, alice, bob);
+        env.close();
+
+        // MPT is transferable, tradable, lockable, and clawback-capable. Both
+        // CanTransfer and CanTrade are mutable so the issuer can flip them
+        // later via MPTokenIssuanceSet.
+        MPTTester mptt{env, issuer, mptInitNoFund};
+        mptt.create(
+            {.flags = tfMPTCanTransfer | tfMPTCanTrade | tfMPTCanLock | tfMPTCanClawback,
+             .mutableFlags = tmfMPTCanMutateCanTransfer | tmfMPTCanMutateCanTrade});
+        PrettyAsset const asset = mptt.issuanceID();
+
+        mptt.authorize({.account = alice});
+        mptt.authorize({.account = bob});
+        env(pay(issuer, alice, asset(100'000)));
+        env(pay(issuer, bob, asset(100'000)));
+        env.close();
+
+        Vault const vault{env};
+        auto [createTx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
+        env(createTx);
+        env.close();
+        BEAST_EXPECT(env.le(vaultKeylet));
+
+        // Both holders deposit. Issuer also deposits (issuer can be a
+        // depositor too) so we can later confirm the issuer-exempt path.
+        env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50'000)}));
+        env(vault.deposit({.depositor = bob, .id = vaultKeylet.key, .amount = asset(30'000)}));
+        env(vault.deposit({.depositor = issuer, .id = vaultKeylet.key, .amount = asset(20'000)}));
+        env.close();
+
+        // -- 1. Issuer clears lsfMPTCanTransfer ---------------------------
+        mptt.set({.mutableFlags = tmfMPTClearCanTransfer});
+        env.close();
+
+        {
+            auto const sle = env.le(keylet::mptIssuance(asset.raw().get<MPTIssue>().getMptID()));
+            BEAST_EXPECT(sle && !sle->isFlag(lsfMPTCanTransfer));
+            BEAST_EXPECT(sle && sle->isFlag(lsfMPTCanTrade));
+        }
+
+        // 2. Holder deposits and withdrawals are blocked: vault pseudo-
+        //    account is neither sender nor receiver = issuer, so
+        //    canTransfer returns tecNO_AUTH.
+        env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(1'000)}),
+            ter(tecNO_AUTH));
+        env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = asset(1'000)}),
+            ter(tecNO_AUTH));
+        env(vault.withdraw({.depositor = bob, .id = vaultKeylet.key, .amount = asset(1'000)}),
+            ter(tecNO_AUTH));
+        env.close();
+
+        // 3. Issuer-as-depositor is exempt — `canTransfer` short-circuits
+        //    on the issuer side. Both deposit and withdraw succeed.
+        env(vault.deposit({.depositor = issuer, .id = vaultKeylet.key, .amount = asset(5'000)}));
+        env(vault.withdraw({.depositor = issuer, .id = vaultKeylet.key, .amount = asset(5'000)}));
+        env.close();
+
+        // 3b. A holder can also escape by withdrawing *to the issuer* via
+        //     sfDestination. `canTransfer`'s issuer short-circuit fires on
+        //     `to == issuer`, so the withdrawal succeeds even though
+        //     CanTransfer is cleared. The holder's shares are burned and
+        //     the underlying MPT lands at the issuer (presumably part of
+        //     an off-ledger redemption arrangement).
+        auto const aliceMptBefore = env.balance(alice, asset);
+        auto withdrawToIssuer =
+            vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = asset(2'000)});
+        withdrawToIssuer[sfDestination] = issuer.human();
+        env(withdrawToIssuer);
+        env.close();
+        // Alice's MPT balance is unchanged — the asset went to the issuer,
+        // not back to her — but her share holding was burned.
+        BEAST_EXPECT(env.balance(alice, asset) == aliceMptBefore);
+
+        // -- 4. Also clear lsfMPTCanTrade. Vault paths don't consult
+        //       CanTrade, so this changes nothing for vault I/O. ----------
+        mptt.set({.mutableFlags = tmfMPTClearCanTrade});
+        env.close();
+
+        {
+            auto const sle = env.le(keylet::mptIssuance(asset.raw().get<MPTIssue>().getMptID()));
+            BEAST_EXPECT(sle && !sle->isFlag(lsfMPTCanTrade));
+        }
+
+        // Holder ops still fail the same way (CanTransfer-driven), and the
+        // issuer is still exempt.
+        env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = asset(1'000)}),
+            ter(tecNO_AUTH));
+        env(vault.deposit({.depositor = issuer, .id = vaultKeylet.key, .amount = asset(1'000)}));
+        env.close();
+
+        // -- 5. Re-enable CanTransfer; leave CanTrade cleared. ------------
+        mptt.set({.mutableFlags = tmfMPTSetCanTransfer});
+        env.close();
+
+        // Holders can now withdraw all their stake — confirms CanTrade is
+        // not consulted by the vault transactors. Alice already redeemed
+        // 2,000 to the issuer, so only 48,000 remains for her.
+        env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = asset(48'000)}));
+        env(vault.withdraw({.depositor = bob, .id = vaultKeylet.key, .amount = asset(30'000)}));
+        env.close();
+
+        {
+            auto const sle = env.le(keylet::mptIssuance(asset.raw().get<MPTIssue>().getMptID()));
+            BEAST_EXPECT(sle && sle->isFlag(lsfMPTCanTransfer));
+            BEAST_EXPECT(sle && !sle->isFlag(lsfMPTCanTrade));
+        }
+    }
+
 public:
     void
     run() override
@@ -6162,6 +6297,7 @@ public:
         testAssetsMaximum();
         testBug6_LimitBypassWithShares();
         testRemoveEmptyHoldingLockedAmount();
+        testMutateCanTransferAfterDeposit();
     }
 };