Merge branch 'develop' into ximinez/loanpay-fee-overflow-develop

- Add test cases to exercise the LoanPay fee cap calculation
- Fix a bug in the fee cap calculation.

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

@@ -139,6 +139,23 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
     // If making an overpayment, count it as a full payment because it will do
     // about the same amount of work, if not more.
     NumberRoundModeGuard const mg(tx.isFlag(tfLoanOverpayment) ? Number::upward : Number::downward);
+
+    static_assert(loanMaximumPaymentsPerTransaction % loanPaymentsPerFeeIncrement == 0);
+    std::int64_t constexpr maxFeeIncrements =
+        loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement;
+
+    if (view.rules().enabled(fixSecurity3_1_3) &&
+        amount >= regularPayment * loanMaximumPaymentsPerTransaction)
+    {
+        // The payment handler will never process more than
+        // loanMaximumPaymentsPerTransaction payments (including overpayments),
+        // and one fee increment is charged for every
+        // loanPaymentsPerFeeIncrement, so don't charge more than
+        // loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement fee
+        // increments.
+        return maxFeeIncrements * normalCost;
+    }
+
     // Estimate how many payments will be made
     Number const numPaymentEstimate = static_cast<std::int64_t>(amount / regularPayment);
 
@@ -147,6 +164,10 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
     auto const feeIncrements = std::max(
         std::int64_t(1),
         static_cast<std::int64_t>(numPaymentEstimate / loanPaymentsPerFeeIncrement));
+    XRPL_ASSERT(
+        !view.rules().enabled(fixSecurity3_1_3) || feeIncrements <= maxFeeIncrements,
+        "xrpl::LoanPay::calculateBaseFee : number of fee increments is in "
+        "range");
 
     return feeIncrements * normalCost;
 }

src/test/app/AMM_test.cpp

@@ -8,7 +8,6 @@
 #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>
@@ -20,7 +19,6 @@
 #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>
@@ -37,7 +35,6 @@
 #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>
@@ -7063,200 +7060,10 @@ 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

@@ -4746,15 +4746,17 @@ protected:
     }
 
     void
-    testDosLoanPay()
+    testDosLoanPay(FeatureBitset features)
     {
+        bool const feeCapped = features[fixSecurity3_1_3];
+
         // From FIND-005
-        testcase << "DoS LoanPay";
+        testcase << "DoS LoanPay: fee calculation " << (feeCapped ? "capped" : "uncapped");
 
         using namespace jtx;
         using namespace std::chrono_literals;
         using namespace Lending;
-        Env env(*this, all);
+        Env env(*this, features);
 
         Account const issuer{"issuer"};
         Account const lender{"lender"};
@@ -4763,6 +4765,8 @@ protected:
         env.fund(XRP(1'000'000), issuer, lender, borrower);
         env.close();
 
+        BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixSecurity3_1_3));
+
         PrettyAsset const iouAsset = issuer[iouCurrency];
         env(trust(lender, iouAsset(100'000'000)));
         env(trust(borrower, iouAsset(100'000'000)));
@@ -4775,51 +4779,106 @@ protected:
         using namespace loan;
 
         auto const loanSetFee = fee(env.current()->fees().base * 2);
-        Number const principalRequest{1, 3};
+        Number const principalRequest{3959'37, -2};
         auto const baseFee = env.current()->fees().base;
 
-        auto createJson = env.json(
+        auto const createJson = env.json(
             set(borrower, broker.brokerID, principalRequest),
             fee(loanSetFee),
-            json(sfCounterpartySignature, Json::objectValue));
-
-        createJson["ClosePaymentFee"] = "0";
-        createJson["GracePeriod"] = 60;
-        createJson["InterestRate"] = 20930;
-        createJson["LateInterestRate"] = 77049;
-        createJson["LatePaymentFee"] = "0";
-        createJson["LoanServiceFee"] = "0";
-        createJson["OverpaymentFee"] = 7;
-        createJson["OverpaymentInterestRate"] = 66653;
-        createJson["PaymentInterval"] = 60;
-        createJson["PaymentTotal"] = 3239184;
-        createJson["PrincipalRequested"] = "3959.37";
+            json(sfCounterpartySignature, Json::objectValue),
+            closePaymentFee(0),
+            gracePeriod(60),
+            interestRate(TenthBips32(20930)),
+            lateInterestRate(TenthBips32(77049)),
+            latePaymentFee(0),
+            loanServiceFee(0),
+            overpaymentFee(TenthBips32(7)),
+            overpaymentInterestRate(TenthBips32(66653)),
+            paymentInterval(60),
+            paymentTotal(3239184));
 
+        // There are enough payments due on this loan that it only needs to be
+        // created once, and can be paid on multiple times. Just don't create a
+        // gazillion test cases.
         auto const brokerStateBefore = env.le(keylet::loanbroker(broker.brokerID));
         auto const loanSequence = brokerStateBefore->at(sfLoanSequence);
         auto const keylet = keylet::loan(broker.brokerID, loanSequence);
 
-        createJson = env.json(createJson, sig(sfCounterpartySignature, lender));
-        env(createJson, ter(tesSUCCESS));
+        env(createJson, sig(sfCounterpartySignature, lender));
         env.close();
 
-        auto const stateBefore = getCurrentState(env, broker, keylet);
-        BEAST_EXPECT(stateBefore.paymentRemaining == 3239184);
-        BEAST_EXPECT(stateBefore.paymentRemaining > loanMaximumPaymentsPerTransaction);
+        auto const roundedPayment = [&]() {
+            auto const stateBefore = getCurrentState(env, broker, keylet);
+            BEAST_EXPECT(stateBefore.paymentRemaining == 3239184);
 
-        auto loanPayTx = env.json(pay(borrower, keylet.key, STAmount{broker.asset, Number{}}));
-        Number const amount{395937, -2};
-        loanPayTx["Amount"]["value"] = to_string(amount);
-        XRPAmount const payFee{
-            baseFee *
-            std::int64_t(amount / stateBefore.periodicPayment / loanPaymentsPerFeeIncrement + 1)};
-        env(loanPayTx, ter(tesSUCCESS), fee(payFee));
-        env.close();
+            return roundToAsset(
+                iouAsset, stateBefore.periodicPayment, stateBefore.loanScale, Number::upward);
+        }();
 
-        auto const stateAfter = getCurrentState(env, broker, keylet);
-        BEAST_EXPECT(
-            stateAfter.paymentRemaining ==
-            stateBefore.paymentRemaining - loanMaximumPaymentsPerTransaction);
+        auto test = [&](int const payFactor,
+                        int const feeFactor,
+                        TER const expectedTer = tesSUCCESS) {
+            auto const stateBefore = getCurrentState(env, broker, keylet);
+            BEAST_EXPECT(stateBefore.paymentRemaining <= 3239184);
+            BEAST_EXPECT(stateBefore.paymentRemaining > loanMaximumPaymentsPerTransaction);
+
+            Number const amount = roundedPayment * payFactor;
+            auto loanPayTx = env.json(pay(borrower, keylet.key, STAmount{broker.asset, amount}));
+            XRPAmount const payFee{baseFee * feeFactor};
+            env(loanPayTx, ter(expectedTer), fee(payFee));
+            env.close();
+            auto const expectedChange = isTesSuccess(expectedTer)
+                ? std::min(loanMaximumPaymentsPerTransaction, payFactor)
+                : 0;
+
+            auto const stateAfter = getCurrentState(env, broker, keylet);
+            BEAST_EXPECT(
+                stateAfter.paymentRemaining == stateBefore.paymentRemaining - expectedChange);
+        };
+
+        std::int64_t constexpr maxFeeIncrements =
+            loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement;
+
+        TER const failWithoutFix = feeCapped ? (TER)tesSUCCESS : (TER)telINSUF_FEE_P;
+
+        // * Amount well above threshold -> capped fee
+        // The original test case - way over the limit - more fee is always ok
+        test(1819878, 363976);
+        // The capped fee is only sufficient if the amendment is enabled.
+        test(1819878, maxFeeIncrements, failWithoutFix);
+
+        // * Amount exactly at threshold -> capped fee
+        test(loanMaximumPaymentsPerTransaction, maxFeeIncrements);
+        // More fee is always ok
+        test(loanMaximumPaymentsPerTransaction, maxFeeIncrements + 10);
+
+        // * Amount below threshold -> normal calculation
+        test(1, 1);
+        test(loanPaymentsPerFeeIncrement * 2, 2);
+        test(0, 0, temBAD_AMOUNT);
+        test(0, 1, temBAD_AMOUNT);
+        // Fee difference rounds evenly
+        test(
+            loanMaximumPaymentsPerTransaction - 10,
+            ((loanMaximumPaymentsPerTransaction - 10) / loanPaymentsPerFeeIncrement) - 1,
+            telINSUF_FEE_P);
+        test(
+            loanMaximumPaymentsPerTransaction - 10,
+            ((loanMaximumPaymentsPerTransaction - 10) / loanPaymentsPerFeeIncrement));
+        // More fee is always ok
+        test(
+            loanMaximumPaymentsPerTransaction - 10,
+            ((loanMaximumPaymentsPerTransaction - 10) / loanPaymentsPerFeeIncrement) + 3);
+        // Fee rounds up
+        for (int under = 1; under < loanPaymentsPerFeeIncrement; ++under)
+        {
+            test(loanMaximumPaymentsPerTransaction - under, maxFeeIncrements - 1, telINSUF_FEE_P);
+            test(loanMaximumPaymentsPerTransaction - under, maxFeeIncrements);
+        }
+        // Only when you get one less fee increment can you pay less
+        test(loanMaximumPaymentsPerTransaction - loanPaymentsPerFeeIncrement, maxFeeIncrements - 1);
+        // And again, more fee is always ok.
+        test(loanMaximumPaymentsPerTransaction - loanPaymentsPerFeeIncrement, maxFeeIncrements);
     }
 
     void
@@ -7232,7 +7291,8 @@ public:
         testLoanPayDebtDecreaseInvariant();
         testWrongMaxDebtBehavior();
         testLoanPayComputePeriodicPaymentValidTotalInterestInvariant();
-        testDosLoanPay();
+        testDosLoanPay(all | fixSecurity3_1_3);
+        testDosLoanPay(all - fixSecurity3_1_3);
         testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant();
         testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant();
         testLoanNextPaymentDueDateOverflow();

src/test/app/Vault_test.cpp

@@ -6139,141 +6139,6 @@ 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
@@ -6297,7 +6162,6 @@ public:
         testAssetsMaximum();
         testBug6_LimitBypassWithShares();
         testRemoveEmptyHoldingLockedAmount();
-        testMutateCanTransferAfterDeposit();
     }
 };