refactor: Tighten VaultHelpers contracts and extend test coverage

- Document the mutate-then-transfer ordering that every helper follows and
  the caller obligation to discard the ApplyView on non-tesSUCCESS
  (transactor sandbox convention).
- Assert that removeVaultAssets is only called with `amount ==
  sfAssetsAvailable` when FinalRemoval::Yes, so a mis-specified amount can
  no longer zero the Vault fields while leaving dust on the pseudo-account.
- Mirror the `assetsAvailable <= assetsTotal` XRPL_ASSERT_PARTS from
  LoanSet/LoanPay in LoanManage::defaultLoan's fixCleanup3_4_0 branch for
  defense-in-depth parity.
- Extend VaultHelpers_test failure-path cases to pin the mutate-then-
  transfer observable, add a third-party-destination sub-test for
  removeVaultAssets, add a Legacy-vault fixture for moveVaultAssets with
  nonzero valueDelta, and add an MPT-backed fixture covering
  add/removeVaultAssets against the integral-asset transfer path.

Addresses review comments on #7983 from @gregtatcam, @xrplf-ai-reviewer,
and @copilot-pull-request-reviewer.

include/xrpl/ledger/helpers/VaultHelpers.h

@@ -153,6 +153,13 @@ getVaultScale(SLE::const_ref vault);
  * scale is appropriate for their own accounting (e.g. current vs. posterior
  * Vault scale); this helper does not perform any additional rounding.
  *
+ * Ordering: the Vault fields are mutated (and `view.update` called) before
+ * the underlying `accountSend`; on a non-tesSUCCESS return, the caller is
+ * responsible for discarding the ApplyView (transactors rely on the sandbox
+ * being thrown away on non-tesSUCCESS, which is the codebase convention).
+ * All in-tree callers are transactors that already return the propagated
+ * TER, so no additional rollback is required.
+ *
  * @param view The ledger view to apply changes to.
  * @param vault The vault SLE. Must not be null.
  * @param sender The account to transfer `amount` from.
@@ -202,6 +209,12 @@ enum class FinalRemoval : bool { No = false, Yes = true };
  * scale is appropriate for their own accounting; this helper does not
  * perform any additional rounding.
  *
+ * Ordering: same convention as addVaultAssets — the Vault fields are mutated
+ * before the transfer, and the ApplyView must be discarded by the caller on a
+ * non-tesSUCCESS return. The pre-transfer `amount > sfAssetsAvailable` guard
+ * still fires before any mutation, so a malformed clawback amount cannot
+ * modify the Vault at all.
+ *
  * @param view The ledger view to apply changes to.
  * @param vault The vault SLE. Must not be null.
  * @param recipient The account to transfer `amount` to. Must already be
@@ -238,6 +251,17 @@ clawbackVaultAssets(
  * perform any additional rounding, except when `finalRemoval` is Yes (see
  * FinalRemoval).
  *
+ * Ordering: same convention as addVaultAssets — the Vault fields are mutated
+ * before the transfer, and the ApplyView must be discarded by the caller on a
+ * non-tesSUCCESS return.
+ *
+ * When `finalRemoval` is Yes, callers must pass `amount ==
+ * *vault->at(sfAssetsAvailable)` (the pre-call value): the helper asserts
+ * this invariant and would otherwise leave dust on the Vault's pseudo-account
+ * despite hard-resetting the fields to zero. VaultWithdraw's final-removal
+ * path pins `amount = allAvailable` immediately before calling this helper
+ * to satisfy the contract.
+ *
  * @param ctx The apply-view context to apply changes to.
  * @param vault The vault SLE. Must not be null.
  * @param senderAcct The account that submitted the withdrawal transaction.
@@ -245,9 +269,9 @@ clawbackVaultAssets(
  * @param priorBalance The XRP reserve base, passed through to doWithdraw for
  *                      creating a holding for `dstAcct` when required.
  * @param amount The amount to subtract from sfAssetsAvailable, and to
- *               transfer from the Vault's pseudo-account to `dstAcct`.
- *               Ignored (other than for the transfer) when `finalRemoval` is
- *               Yes.
+ *               transfer from the Vault's pseudo-account to `dstAcct`. When
+ *               `finalRemoval` is Yes, must equal the Vault's pre-call
+ *               sfAssetsAvailable so the transfer drains the pseudo-account.
  * @param j Journal for logging.
  * @param finalRemoval Whether this is the Vault's final removal (see
  *                      FinalRemoval).
@@ -284,6 +308,10 @@ removeVaultAssets(
  * addEmptyHolding and requireAuth performed by the caller beforehand); this
  * helper does not create holdings or check authorization.
  *
+ * Ordering: same convention as addVaultAssets — the Vault fields are mutated
+ * before the transfer, and the ApplyView must be discarded by the caller on
+ * a non-tesSUCCESS return.
+ *
  * sfAssetsAvailable is decreased by an STAmount built from the sum of the
  * recipients' Numbers, so for a very large recipient list whose sum exceeds
  * STAmount's ~16 significant digits, this could round differently than

src/libxrpl/ledger/helpers/VaultHelpers.cpp

@@ -306,9 +306,31 @@ removeVaultAssets(
     [[maybe_unused]] Asset const asset = vault->at(sfAsset);
     XRPL_ASSERT(amount.asset() == asset, "xrpl::removeVaultAssets : amount matches vault asset");
     XRPL_ASSERT(amount >= beast::kZero, "xrpl::removeVaultAssets : amount is non-negative");
+    // On a final removal both fields are hard-reset to zero, so the amount
+    // being withdrawn must equal the Vault's pre-mutation sfAssetsAvailable
+    // — otherwise the caller would leave residual funds on the Vault's
+    // pseudo-account after the Vault-level bookkeeping says the position
+    // is fully unwound. VaultWithdraw enforces this by pinning
+    // `assetsWithdrawn = allAvailable` immediately before setting
+    // FinalRemoval::Yes. The equality is checked via STAmount so both
+    // sides go through the same asset-precision normalization used by
+    // VaultWithdraw when it built `allAvailable`.
+    // Constructed outside the assert macro because unprotected commas
+    // inside `{}` initializers are parsed as extra macro arguments.
+    [[maybe_unused]] STAmount const availableSnapshot{asset, Number(vault->at(sfAssetsAvailable))};
+    XRPL_ASSERT(
+        finalRemoval == FinalRemoval::No || amount == availableSnapshot,
+        "xrpl::removeVaultAssets : final removal amount equals sfAssetsAvailable");
 
     applyRemoveVaultAssets(ctx.view, vault, amount, finalRemoval);
 
+    // The amount==0 short-circuit is defensive: every in-tree caller
+    // (VaultWithdraw) computes `assetsWithdrawn` from a positive
+    // sharesRedeemed and errors out earlier on a zero-share withdrawal, so
+    // production traffic cannot reach this path with amount==0.
+    // doWithdraw would otherwise still succeed for a zero amount, but
+    // short-circuiting here makes the invariant explicit at the single
+    // mutation point.
     if (amount == beast::kZero)
         return tesSUCCESS;  // LCOV_EXCL_LINE
 

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

@@ -3,6 +3,7 @@
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/Number.h>
 #include <xrpl/beast/utility/Zero.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/View.h>
@@ -327,6 +328,24 @@ LoanManage::defaultLoan(
     {
         STAmount const amount{vaultAsset, defaultCovered};
         STAmount const writeOff{vaultAsset, totalDefaultAmount};
+
+        // Defense-in-depth mirror of the invariant check performed on the
+        // LoanSet.cpp/LoanPay.cpp addVaultAssets paths: project the raw
+        // post-mutation Vault fields (pre-rounding) and assert Available <=
+        // Total. Redundant with the tefBAD_LEDGER guard above (which is
+        // strictly stronger, since Total - Available >= totalDefaultAmount
+        // implies the post-write inequality), but kept for structural
+        // parity — if either of the addVaultAssets deltas ever drifts, this
+        // fires before the SLE mutation lands.
+        [[maybe_unused]] Number const assetsAvailableAfterRaw =
+            Number(vaultSle->at(sfAssetsAvailable)) + defaultCovered;
+        [[maybe_unused]] Number const assetsTotalAfterRaw =
+            Number(vaultSle->at(sfAssetsTotal)) + (defaultCovered - totalDefaultAmount);
+        XRPL_ASSERT_PARTS(
+            assetsAvailableAfterRaw <= assetsTotalAfterRaw,
+            "xrpl::LoanManage::defaultLoan",
+            "assets available must not be greater than assets outstanding");
+
         return addVaultAssets(
             view, vaultSle, brokerSle->at(sfAccount), amount, amount - writeOff, j);
     }

src/test/app/VaultHelpers_test.cpp

@@ -1,6 +1,7 @@
 #include <test/jtx/Account.h>
 #include <test/jtx/Env.h>
 #include <test/jtx/amount.h>
+#include <test/jtx/mpt.h>
 #include <test/jtx/noop.h>
 #include <test/jtx/pay.h>
 #include <test/jtx/trust.h>
@@ -14,11 +15,14 @@
 #include <xrpl/ledger/helpers/VaultHelpers.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Asset.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Keylet.h>
+#include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STNumber.h>  // IWYU pragma: keep
 #include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxFlags.h>
 
 namespace xrpl::test {
 
@@ -146,14 +150,25 @@ class VaultHelpers_test : public beast::unit_test::Suite
         // If the underlying accountSend fails (here: sender has no trust
         // line for the vault asset and is not its issuer, so cannot source
         // the IOU), the helper propagates the non-tes error rather than
-        // silently swallowing it.
+        // silently swallowing it. The Vault SLE fields have already been
+        // mutated by the time accountSend fails (the helper follows the
+        // "mutate-then-transfer" ordering documented in VaultHelpers.h);
+        // this test pins that observable so the ordering contract is
+        // visible and future changes have to knowingly break it. In
+        // production the transactor's ApplyView sandbox is discarded on a
+        // non-tesSUCCESS return, so the caller never observes the
+        // mutation.
         {
             Account const stranger{"stranger"};
             env.fund(XRP(10'000), stranger);
             env.close();
+            Number const totalBeforeFail = vault->at(sfAssetsTotal);
+            Number const availableBeforeFail = vault->at(sfAssetsAvailable);
             STAmount const ten{vaultAsset, 10};
             auto const ter = addVaultAssets(view, vault, stranger, ten, ten, env.journal);
             BEAST_EXPECT(!isTesSuccess(ter));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBeforeFail + 10);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBeforeFail + 10);
         }
     }
 
@@ -240,14 +255,22 @@ class VaultHelpers_test : public beast::unit_test::Suite
         // propagates the non-tes error rather than silently swallowing it.
         // In production the recipient is always the asset issuer, which
         // implicitly holds its own asset; this synthetic third-party
-        // recipient stands in only to exercise the failure branch.
+        // recipient stands in only to exercise the failure branch. As with
+        // addVaultAssets, the Vault SLE fields are mutated (decreased)
+        // before the transfer attempt, matching the documented mutate-
+        // then-transfer contract; the transactor sandbox is what makes the
+        // mutation invisible to callers on failure.
         {
             Account const stranger{"stranger"};
             env.fund(XRP(10'000), stranger);
             env.close();
+            Number const totalBeforeFail = vault->at(sfAssetsTotal);
+            Number const availableBeforeFail = vault->at(sfAssetsAvailable);
             STAmount const ten{vaultAsset, 10};
             auto const failTer = clawbackVaultAssets(view, vault, stranger, ten, env.journal);
             BEAST_EXPECT(!isTesSuccess(failTer));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBeforeFail - 10);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBeforeFail - 10);
         }
     }
 
@@ -261,12 +284,17 @@ class VaultHelpers_test : public beast::unit_test::Suite
         Account const issuer{"issuer"};
         Account const owner{"owner"};
         Account const depositor{"depositor"};
-        env.fund(XRP(10'000), issuer, owner, depositor);
+        // Third-party withdrawal destination for the dstAcct != senderAcct
+        // sub-test below. Set up before the ApplyViewImpl snapshot so its
+        // account root and trust line are visible to the helper.
+        Account const charlie{"charlie"};
+        env.fund(XRP(10'000), issuer, owner, depositor, charlie);
         env.close();
 
         PrettyAsset const asset = issuer["USD"];
         env(trust(owner, asset(1'000'000)));
         env(trust(depositor, asset(1'000'000)));
+        env(trust(charlie, asset(1'000'000)));
         env(pay(issuer, depositor, asset(10'000)));
         env.close();
 
@@ -276,10 +304,14 @@ class VaultHelpers_test : public beast::unit_test::Suite
         env(v.deposit({.depositor = depositor, .id = vaultKeylet.key, .amount = asset(1'000)}));
         env.close();
 
-        // doWithdraw only reads ctx.tx on the third-party-destination path;
-        // for a self-withdrawal (senderAcct == dstAcct, exercised below) it
-        // is unused, so a trivial signed noop stands in for a real
-        // VaultWithdraw transaction.
+        // doWithdraw uses ctx.tx on the third-party-destination path (both
+        // verifyDepositPreauth and getEffectiveTxReserveSponsor read it);
+        // for a self-withdrawal (senderAcct == dstAcct) it is unused. A
+        // signed noop from `depositor` stands in for a real VaultWithdraw
+        // transaction here: verifyDepositPreauth passes for a destination
+        // without lsfDepositAuth, and getEffectiveTxReserveSponsor returns
+        // a null sponsor because the destination differs from tx.Account
+        // (see SponsorHelpers.cpp:getEffectiveTxReserveSponsor).
         auto const dummyTx = env.jt(noop(depositor)).stx;
         if (!BEAST_EXPECT(dummyTx))
             return;
@@ -330,25 +362,99 @@ class VaultHelpers_test : public beast::unit_test::Suite
             BEAST_EXPECT(depositorBalanceAfter == depositorBalanceBefore + hundred);
         }
 
-        // FinalRemoval::Yes hard-resets both fields to exactly zero,
-        // regardless of the passed-in amount (deliberately an understated
-        // amount here, to prove the fields aren't merely decremented by
-        // it).
+        // Third-party destination: senderAcct != dstAcct exercises
+        // doWithdraw's verifyDepositPreauth branch (a self-withdrawal
+        // instead hits addEmptyHolding). `charlie` already holds a trust
+        // line for the asset, so no holding creation is needed on the
+        // destination side. The Vault SLE moves by `amount`, and the
+        // funds land on `charlie` rather than on the transaction sender.
+        {
+            Number const totalBefore3P = vault->at(sfAssetsTotal);
+            Number const availableBefore3P = vault->at(sfAssetsAvailable);
+            auto const charlieBalanceBefore3P = accountHolds(
+                view,
+                charlie,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            auto const depositorBalanceBefore3P = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+
+            STAmount const fifty{vaultAsset, 50};
+            auto const ter = removeVaultAssets(
+                ctx, vault, depositor, charlie, XRPAmount{0}, fifty, env.journal, FinalRemoval::No);
+            auto const charlieBalanceAfter3P = accountHolds(
+                view,
+                charlie,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            auto const depositorBalanceAfter3P = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+
+            BEAST_EXPECT(isTesSuccess(ter));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBefore3P - 50);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBefore3P - 50);
+            BEAST_EXPECT(charlieBalanceAfter3P == charlieBalanceBefore3P + fifty);
+            BEAST_EXPECT(depositorBalanceAfter3P == depositorBalanceBefore3P);
+        }
+
+        // FinalRemoval::Yes hard-resets both fields to exactly zero and
+        // drains the Vault's pseudo-account: the caller must pass
+        // `amount == pre-call sfAssetsAvailable` (VaultWithdraw pins that
+        // via `assetsWithdrawn = allAvailable` before setting the flag),
+        // and the recipient's balance is expected to increase by exactly
+        // that amount. The passed-in STAmount is used only for the
+        // pseudo-account -> recipient transfer; the Vault fields are
+        // reset regardless of it (as long as it matches sfAssetsAvailable
+        // -- other values would trip the helper's precondition
+        // XRPL_ASSERT and are covered by the transactor contract, not
+        // this unit).
         {
             Number const totalBeforeFinal = vault->at(sfAssetsTotal);
-            STAmount const one{vaultAsset, 1};
+            Number const availableBeforeFinal = vault->at(sfAssetsAvailable);
+            auto const depositorBalanceBeforeFinal = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+
+            STAmount const allAvailable{vaultAsset, availableBeforeFinal};
             auto const ter = removeVaultAssets(
                 ctx,
                 vault,
                 depositor,
                 depositor,
                 XRPAmount{0},
-                one,
+                allAvailable,
                 env.journal,
                 FinalRemoval::Yes);
+            auto const depositorBalanceAfterFinal = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+
             BEAST_EXPECT(isTesSuccess(ter));
             BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == Number{0});
             BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == Number{0});
+            BEAST_EXPECT(depositorBalanceAfterFinal == depositorBalanceBeforeFinal + allAvailable);
             BEAST_EXPECTS(
                 totalBeforeFinal != Number(0),
                 "fixture sanity: total was nonzero before the final removal");
@@ -480,6 +586,220 @@ class VaultHelpers_test : public beast::unit_test::Suite
         }
     }
 
+    // Legacy-version Vault fixture: featureLendingProtocolV1_1 is off, so
+    // VaultCreate does not set sfLEVersion and getVaultVersion() resolves
+    // to Legacy. That is the only vault variant on which
+    // moveVaultAssets permits a nonzero valueDelta (see the
+    // "moveVaultAssets : nonzero valueDelta requires Legacy vault version"
+    // assertion in the helper).
+    void
+    testMoveVaultAssetsLegacy()
+    {
+        testcase("moveVaultAssets: Legacy vault, nonzero valueDelta");
+        using namespace jtx;
+
+        Env env(*this, testableAmendments() - featureLendingProtocolV1_1);
+        Account const issuer{"issuer"};
+        Account const owner{"owner"};
+        Account const depositor{"depositor"};
+        Account const borrower{"borrower"};
+        Account const feeRecipient{"feeRecipient"};
+        env.fund(XRP(10'000), issuer, owner, depositor, borrower, feeRecipient);
+        env.close();
+
+        PrettyAsset const asset = issuer["USD"];
+        env(trust(owner, asset(1'000'000)));
+        env(trust(depositor, asset(1'000'000)));
+        env(trust(borrower, asset(1'000'000)));
+        env(trust(feeRecipient, asset(1'000'000)));
+        env(pay(issuer, depositor, asset(10'000)));
+        env.close();
+
+        auto const vaultKeylet = setupVault(env, asset, owner);
+
+        Vault const v{env};
+        env(v.deposit({.depositor = depositor, .id = vaultKeylet.key, .amount = asset(1'000)}));
+        env.close();
+
+        auto const open = env.current();
+        ApplyViewImpl view(&*open, TapNone);
+        auto const vault = view.peek(vaultKeylet);
+        if (!BEAST_EXPECT(vault))
+            return;
+        // Confirm the fixture actually produced a Legacy vault (sfLEVersion
+        // absent). If this changes upstream the helper's precondition would
+        // reject the nonzero valueDelta below, which would only fire via an
+        // XRPL_ASSERT — checking it explicitly keeps the failure mode
+        // legible.
+        if (!BEAST_EXPECTS(
+                getVaultVersion(vault) == VaultVersion::Legacy,
+                "fixture: featureLendingProtocolV1_1 disabled produces Legacy vault"))
+            return;
+
+        Asset const vaultAsset = vault->at(sfAsset);
+
+        Number const totalBefore = vault->at(sfAssetsTotal);
+        Number const availableBefore = vault->at(sfAssetsAvailable);
+        auto const borrowerBalanceBefore = accountHolds(
+            view,
+            borrower,
+            vaultAsset,
+            FreezeHandling::IgnoreFreeze,
+            AuthHandling::IgnoreAuth,
+            env.journal);
+        auto const feeRecipientBalanceBefore = accountHolds(
+            view,
+            feeRecipient,
+            vaultAsset,
+            FreezeHandling::IgnoreFreeze,
+            AuthHandling::IgnoreAuth,
+            env.journal);
+
+        // Nonzero valueDelta increases sfAssetsTotal independently of the
+        // funds actually moved from the Vault's pseudo-account to the
+        // recipients: this mirrors a Legacy-vault loan origination, where
+        // recognizing accrued interest into sfAssetsTotal is decoupled
+        // from the cash disbursement decrementing sfAssetsAvailable.
+        MultiplePaymentDestinations const recipients{
+            {borrower, Number{80}},
+            {feeRecipient, Number{20}},
+        };
+        STAmount const valueDelta{vaultAsset, 5};
+        auto const ter = moveVaultAssets(view, vault, recipients, valueDelta, env.journal);
+        auto const borrowerBalanceAfter = accountHolds(
+            view,
+            borrower,
+            vaultAsset,
+            FreezeHandling::IgnoreFreeze,
+            AuthHandling::IgnoreAuth,
+            env.journal);
+        auto const feeRecipientBalanceAfter = accountHolds(
+            view,
+            feeRecipient,
+            vaultAsset,
+            FreezeHandling::IgnoreFreeze,
+            AuthHandling::IgnoreAuth,
+            env.journal);
+        STAmount const eighty{vaultAsset, 80};
+        STAmount const twenty{vaultAsset, 20};
+
+        BEAST_EXPECT(isTesSuccess(ter));
+        BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBefore + 5);
+        BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBefore - 100);
+        BEAST_EXPECT(borrowerBalanceAfter == borrowerBalanceBefore + eighty);
+        BEAST_EXPECT(feeRecipientBalanceAfter == feeRecipientBalanceBefore + twenty);
+    }
+
+    // MPT-backed Vault exercises add/remove helpers on an integral asset,
+    // where accountSend/doWithdraw take the MPT code paths instead of the
+    // IOU trust-line paths used by the primary testAddVaultAssets/
+    // testRemoveVaultAssets fixtures. Field-mutation contracts are
+    // asset-agnostic and stay identical, but this confirms the helpers
+    // compose correctly with the MPT transfer implementations.
+    void
+    testHelpersMPT()
+    {
+        testcase("addVaultAssets / removeVaultAssets: MPT-backed vault");
+        using namespace jtx;
+
+        Env env(*this);
+        Account const issuer{"issuer"};
+        Account const owner{"owner"};
+        Account const depositor{"depositor"};
+        env.fund(XRP(10'000), issuer, owner, depositor);
+        env.close();
+
+        MPTTester mptt{env, issuer, kMptInitNoFund};
+        mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
+        mptt.authorize({.account = owner});
+        mptt.authorize({.account = depositor});
+        PrettyAsset const asset = mptt.issuanceID();
+        env(pay(issuer, depositor, asset(10'000)));
+        env.close();
+
+        auto const vaultKeylet = setupVault(env, asset, owner);
+
+        Vault const v{env};
+        env(v.deposit({.depositor = depositor, .id = vaultKeylet.key, .amount = asset(1'000)}));
+        env.close();
+
+        auto const dummyTx = env.jt(noop(depositor)).stx;
+        if (!BEAST_EXPECT(dummyTx))
+            return;
+
+        auto const open = env.current();
+        ApplyViewImpl view(&*open, TapNone);
+        auto const vault = view.peek(vaultKeylet);
+        if (!BEAST_EXPECT(vault))
+            return;
+        Asset const vaultAsset = vault->at(sfAsset);
+        ApplyViewContext const ctx{.view = view, .tx = *dummyTx};
+
+        Number const totalBefore = vault->at(sfAssetsTotal);
+        Number const availableBefore = vault->at(sfAssetsAvailable);
+        auto const depositorBalanceBefore = accountHolds(
+            view,
+            depositor,
+            vaultAsset,
+            FreezeHandling::IgnoreFreeze,
+            AuthHandling::IgnoreAuth,
+            env.journal);
+
+        // addVaultAssets: same field-delta contract as the IOU fixture,
+        // but accountSend routes through the MPT transfer code path.
+        {
+            STAmount const fifty{vaultAsset, 50};
+            auto const ter = addVaultAssets(view, vault, depositor, fifty, fifty, env.journal);
+            auto const depositorBalanceAfter = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            BEAST_EXPECT(isTesSuccess(ter));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBefore + 50);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBefore + 50);
+            BEAST_EXPECT(depositorBalanceAfter == depositorBalanceBefore - fifty);
+        }
+
+        // removeVaultAssets (non-final, self-withdrawal): fields decrease
+        // by amount; doWithdraw's MPT path routes the transfer back to
+        // the depositor.
+        {
+            Number const totalBeforeRm = vault->at(sfAssetsTotal);
+            Number const availableBeforeRm = vault->at(sfAssetsAvailable);
+            auto const depositorBalanceBeforeRm = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            STAmount const thirty{vaultAsset, 30};
+            auto const ter = removeVaultAssets(
+                ctx,
+                vault,
+                depositor,
+                depositor,
+                XRPAmount{0},
+                thirty,
+                env.journal,
+                FinalRemoval::No);
+            auto const depositorBalanceAfterRm = accountHolds(
+                view,
+                depositor,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            BEAST_EXPECT(isTesSuccess(ter));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBeforeRm - 30);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBeforeRm - 30);
+            BEAST_EXPECT(depositorBalanceAfterRm == depositorBalanceBeforeRm + thirty);
+        }
+    }
+
 public:
     void
     run() override
@@ -488,6 +808,8 @@ public:
         testClawbackVaultAssets();
         testRemoveVaultAssets();
         testMoveVaultAssets();
+        testMoveVaultAssetsLegacy();
+        testHelpersMPT();
     }
 };