Fix test error

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

include/xrpl/tx/invariants/LoanBrokerInvariant.h

@@ -24,6 +24,10 @@ namespace xrpl {
  *    `OwnerCount` is zero and its pre-state `DebtTotal` rounds to zero at the
  *    vault's `AssetsTotal` scale, as `LoanBrokerDelete::preclaim` requires.
  * 3. At most one `ltLOAN_BROKER` may be deleted in a single transaction.
+ * 4. From `fixCleanup3_5_0` onward, a successful `ttLOAN_BROKER_DELETE` must
+ *    erase exactly one `ltLOAN_BROKER`, it must be the one identified by the
+ *    transaction's `LoanBrokerID`, and the transaction must not leave any other
+ *    broker created or modified.
  *
  */
 class ValidLoanBroker

include/xrpl/tx/invariants/LoanInvariant.h

@@ -42,6 +42,10 @@ namespace xrpl {
  *          `PaymentRemaining` strictly decrease; `NextPaymentDueDate`
  *          advances by N * `PaymentInterval`, N > 0.
  *       `PaymentRemaining == 0` after: pinned by checks 1 and 5b.
+ *    h. From `fixCleanup3_5_0` onward, a successful `ttLOAN_DELETE` must erase
+ *       exactly one `ltLOAN`, it must be the one identified by the
+ *       transaction's `LoanID`, and the transaction must not leave any other
+ *       Loan created or modified.
  *
  */
 class ValidLoan

include/xrpl/tx/invariants/MPTInvariant.h

@@ -45,7 +45,9 @@ class ValidMPTIssuance
     /**
      * MPTokens and RippleStates deleted during apply. finalize() checks each
      * holder's AccountRoot to detect vault pseudo-account holdings deleted
-     * outside VaultDelete. All these checks are gated on fixCleanup3_2_0.
+     * outside VaultDelete. All these checks are gated on fixCleanup3_2_0. From
+     * fixCleanup3_5_0 a VaultDelete is checked as well, catching the holdings
+     * of a vault that survives the transaction.
      */
     std::vector<std::shared_ptr<SLE const>> deletedHoldings_;
 

include/xrpl/tx/invariants/VaultInvariant.h

@@ -46,6 +46,14 @@ namespace xrpl {
  * - vault withdrawal may not succeed when the vault phase is Investment
  * - closed-ended loan origination (ttLOAN_SET) may only succeed when the
  *   vault phase is Investment
+ * - from fixCleanup3_5_0 onward, at most one ltVAULT may be erased, it must be
+ *   the one identified by a ttVAULT_DELETE transaction's VaultID, and that
+ *   transaction must not leave any other vault created or modified
+ *
+ * Deletion of a vault pseudo-account's holding is enforced by
+ * ValidMPTIssuance (see MPTInvariant.cpp): from fixCleanup3_5_0 onward a
+ * ttVAULT_DELETE may only erase the holdings of the pseudo-account it deletes,
+ * not those of a vault that survives the transaction.
  *
  * Immutability of VaultKind, SubscriptionDate and RedemptionDate is enforced
  * by NoModifiedUnmodifiableFields (see InvariantCheck.cpp). From
@@ -100,6 +108,10 @@ private:
     std::vector<Shares> afterMPTs_;
     std::vector<Vault> beforeVault_;
     std::vector<Shares> beforeMPTs_;
+    // Pre-state of the vaults erased by this transaction. A subset of
+    // beforeVault_, which holds the pre-state of every touched vault and so
+    // cannot on its own tell a deleted vault from a modified one.
+    std::vector<Vault> deletedVault_;
     std::unordered_map<uint256, DeltaInfo> deltas_;
 
     /**

src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp

@@ -31,6 +31,9 @@ ValidLoanBroker::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref
     //   (a) only ttLOAN_BROKER_DELETE removes a broker
     //   (b) at most one broker is removed per transaction
     //   (c) DebtTotal and OwnerCount were zero before deletion
+    //   (d) a ttLOAN_BROKER_DELETE removes exactly one broker, it is the one
+    //       named by the transaction, and no other broker was created or
+    //       modified alongside it
     // `before` is the pre-transaction state, which is what
     // LoanBrokerDelete::preclaim reads. Erased trust lines and MPTokens need no
     // special handling here: the `if (after)` branch below already records them.
@@ -114,7 +117,7 @@ ValidLoanBroker::goodZeroDirectory(
 bool
 ValidLoanBroker::finalize(
     STTx const& tx,
-    TER const,
+    TER const result,
     XRPAmount const,
     ReadView const& view,
     beast::Journal const& j)
@@ -122,6 +125,8 @@ ValidLoanBroker::finalize(
     // Loan Brokers will not exist on ledger if the Lending Protocol amendment
     // is not enabled, so there's no need to check it.
 
+    bool const fix350Enabled = view.rules().enabled(fixCleanup3_5_0);
+
     // Deletion invariants (featureLendingProtocolV1_1). At most one
     // LoanBroker may be removed per transaction, and only by
     // ttLOAN_BROKER_DELETE, and only when its pre-state OwnerCount is zero and
@@ -130,7 +135,9 @@ ValidLoanBroker::finalize(
     // LoanBrokerDelete-must-not-touch-any-loan rule: even a broker that has
     // finished paying off every loan may still hold non-zero exposure until
     // its LoanBrokerCoverWithdraw settles, and neither state is safe to
-    // delete.
+    // delete. From fixCleanup3_5_0 a successful ttLOAN_BROKER_DELETE must
+    // remove a broker, that broker must be the one named by the transaction,
+    // and no other broker may be touched alongside it.
     if (view.rules().enabled(featureLendingProtocolV1_1))
     {
         if (multipleBrokerDeletions_)
@@ -177,6 +184,39 @@ ValidLoanBroker::finalize(
                     << "Invariant failed: Loan Broker deleted with non-zero owner count";
                 return false;
             }
+            // Checked last so that the more specific diagnostics above are
+            // reported first.
+            if (fix350Enabled)
+            {
+                auto const deletedKey = deletedBroker_->key();
+                // The erased broker is itself collected in brokers_, so only a
+                // different directly-touched broker is a violation. Brokers
+                // reached indirectly, through a pseudo-account, trust line or
+                // MPToken, carry no brokerAfter and are not touched in this sense.
+                if (std::ranges::any_of(brokers_, [&deletedKey](auto const& entry) {
+                        return entry.second.brokerAfter && entry.first != deletedKey;
+                    }))
+                {
+                    JLOG(j.fatal()) << "Invariant failed: " <<  //
+                        "Loan Broker deletion must not create or modify another Loan Broker";
+                    return false;
+                }
+                if (!tx.isFieldPresent(sfLoanBrokerID) ||
+                    deletedKey != keylet::loanBroker(tx[sfLoanBrokerID]).key)
+                {
+                    JLOG(j.fatal()) << "Invariant failed: " <<  //
+                        "deleted Loan Broker does not match the LoanBrokerID in the transaction";
+                    return false;
+                }
+            }
+        }
+        else if (fix350Enabled && tx.getTxnType() == ttLOAN_BROKER_DELETE && isTesSuccess(result))
+        {
+            // LoanBrokerDelete's privileges do not require LoanBrokerDelete to erase broker
+            // objects, and nothing else requires it.
+            JLOG(j.fatal()) << "Invariant failed: " <<  //
+                "Loan Broker deletion succeeded without deleting a Loan Broker";
+            return false;
         }
     }
 

src/libxrpl/tx/invariants/LoanInvariant.cpp

@@ -52,6 +52,7 @@ ValidLoan::finalize(
 
     auto const txType = tx.getTxnType();
     bool const lpV11Enabled = view.rules().enabled(featureLendingProtocolV1_1);
+    bool const fix350Enabled = view.rules().enabled(fixCleanup3_5_0);
 
     // Without featureLendingProtocolV1_1 an erased Loan is subject to the same
     // per-entry checks as any modified Loan. From V1_1 onward it is only subject
@@ -271,6 +272,42 @@ ValidLoan::finalize(
                            "other than LoanDelete";
         return false;
     }
+
+    // A successful ttLOAN_DELETE must erase exactly one Loan, that Loan must be
+    // the one named by its LoanID, and no other Loan may be created or
+    // modified. Keyed off the transaction type.
+    // LoanDelete has no privileges, so nothing else requires it to erase
+    // anything.
+    if (lpV11Enabled && fix350Enabled && txType == ttLOAN_DELETE && isTesSuccess(result))
+    {
+        if (deletedLoans_.empty())
+        {
+            JLOG(j.fatal()) << "Invariant failed: loan deletion succeeded "
+                               "without deleting a loan";
+            return false;
+        }
+
+        if (deletedLoans_.size() > 1)
+        {
+            JLOG(j.fatal()) << "Invariant failed: more than one Loan deleted";
+            return false;
+        }
+
+        if (!loans_.empty())
+        {
+            JLOG(j.fatal()) << "Invariant failed: loan deletion must not "
+                               "create or modify another loan";
+            return false;
+        }
+
+        if (deletedLoans_.front().first->key() != keylet::loan(tx[sfLoanID]).key)
+        {
+            JLOG(j.fatal()) << "Invariant failed: deleted loan does not match "
+                               "the LoanID in the transaction";
+            return false;
+        }
+    }
+
     return true;
 }
 

src/libxrpl/tx/invariants/MPTInvariant.cpp

@@ -142,6 +142,10 @@ ValidMPTIssuance::finalize(
     //   - A vault pseudo-account's MPToken or RippleState may only be
     //     deleted by VaultDelete; the share's sfReferenceHolding pointer
     //     must not dangle outside that controlled lifecycle.
+    // Post-fixCleanup3_5_0:
+    //   - That last rule narrows further: a VaultDelete may only erase the
+    //     holdings of the pseudo-account it deletes, not those of a vault that
+    //     survives the transaction.
     if (rules.enabled(fixCleanup3_2_0))
     {
         // Not an amendment gate like the same-named flags below, just an
@@ -159,7 +163,11 @@ ValidMPTIssuance::finalize(
                                "MPTokenIssuance by a non-VaultCreate transaction";
             invariantPasses = false;
         }
-        if (!deletedHoldings_.empty() && tx.getTxnType() != ttVAULT_DELETE)
+        // From fixCleanup3_5_0 a VaultDelete may only erase the holdings of the
+        // vault it deletes, not those of any other vault.
+        bool const exemptHoldings =
+            tx.getTxnType() == ttVAULT_DELETE && !rules.enabled(fixCleanup3_5_0);
+        if (!deletedHoldings_.empty() && !exemptHoldings)
         {
             auto const isVaultPseudo = [&](AccountID const& acct) {
                 auto const sle = view.read(keylet::account(acct));
@@ -184,8 +192,16 @@ ValidMPTIssuance::finalize(
                 }
                 if (offending)
                 {
-                    JLOG(j.fatal()) << "Invariant failed: vault pseudo-account holding "
-                                       "deleted by a non-VaultDelete transaction";
+                    if (tx.getTxnType() == ttVAULT_DELETE)
+                    {
+                        JLOG(j.fatal()) << "Invariant failed: vault deletion deleted another "
+                                           "vault pseudo-account's holding";
+                    }
+                    else
+                    {
+                        JLOG(j.fatal()) << "Invariant failed: vault pseudo-account holding "
+                                           "deleted by a non-VaultDelete transaction";
+                    }
                     invariantPasses = false;
                 }
             }

src/libxrpl/tx/invariants/VaultInvariant.cpp

@@ -108,6 +108,8 @@ ValidVault::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref afte
         {
             case ltVAULT:
                 beforeVault_.push_back(Vault::make(*before));
+                if (isDelete)
+                    deletedVault_.push_back(beforeVault_.back());
                 break;
             case ltMPTOKEN_ISSUANCE:
                 // At this moment we have no way of telling if this object holds
@@ -418,6 +420,7 @@ ValidVault::finalize(
 {
     bool const enforce = view.rules().enabled(featureSingleAssetVault);
     bool const fix340Enabled = view.rules().enabled(fixCleanup3_4_0);
+    bool const fix350Enabled = view.rules().enabled(fixCleanup3_5_0);
 
     if (!isTesSuccess(ret))
         return true;  // Do not perform checks
@@ -447,6 +450,16 @@ ValidVault::finalize(
         return !enforce;  // Also not a vault operation
     }
 
+    // Counted explicitly, and ahead of the check below, which conflates
+    // deletions with modifications. Bounding the deletions on their own keeps
+    // the rule auditable here rather than as a consequence of how beforeVault_
+    // happens to be populated, and matches ValidLoan and ValidLoanBroker.
+    if (fix350Enabled && deletedVault_.size() > 1)
+    {
+        JLOG(j.fatal()) << "Invariant failed: more than one vault deleted";
+        return false;  // That's all we can do here
+    }
+
     if (beforeVault_.size() > 1 || afterVault_.size() > 1)
     {
         JLOG(j.fatal()) <<  //
@@ -516,8 +529,35 @@ ValidVault::finalize(
             result = false;
         }
 
+        // The vault erased must be the one the transaction names. Reaching here
+        // means txnType is ttVAULT_DELETE, so sfVaultID is present. Checked last
+        // so that the more specific diagnostics above are reported first.
+        if (fix350Enabled &&
+            (!tx.isFieldPresent(sfVaultID) || beforeVault.key != keylet::vault(tx[sfVaultID]).key))
+        {
+            JLOG(j.fatal()) << "Invariant failed: deleted vault does not match "
+                               "the VaultID in the transaction";
+            result = false;
+        }
+
         return result;
     }
+
+    // Reaching here means a vault survives the transaction, so any vault erased
+    // alongside it must be a different one. The single-vault check above counts
+    // beforeVault_ and afterVault_ separately and so cannot see this pairing,
+    // which would otherwise leave the checks below comparing the pre-state of
+    // the erased vault against the post-state of the surviving one. Checked
+    // ahead of the ttVAULT_DELETE case below, which covers the same state but
+    // misreports it as a deletion that erased nothing, and ahead of the assert
+    // on the surviving vault's identity, which fires only in debug builds.
+    if (fix350Enabled && !deletedVault_.empty())
+    {
+        JLOG(j.fatal()) << "Invariant failed: vault deletion must not "
+                           "create or modify another vault";
+        return false;  // That's all we can do here
+    }
+
     if (txnType == ttVAULT_DELETE)
     {
         JLOG(j.fatal()) << "Invariant failed: vault deletion succeeded without "

src/test/app/invariants/InvariantsPseudoAccount_test.cpp

@@ -543,6 +543,28 @@ class InvariantsPseudoAccount_test : public InvariantsBase
                 STTx{ttACCOUNT_SET, [](STObject&) {}},
                 {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
                 createLoanBroker);
+
+            // A LoanBrokerDelete must remove the broker its LoanBrokerID
+            // names. The broker here is freshly created, so its DebtTotal and
+            // OwnerCount are both zero and every other deletion check passes,
+            // leaving only the identity check to fire.
+            doInvariantCheck(
+                {{"deleted Loan Broker does not match the LoanBrokerID in the transaction"}},
+                [&](Account const&, Account const&, ApplyContext& ac) {
+                    if (loanBrokerKeylet.type != ltLOAN_BROKER)
+                        return false;
+                    auto sleBroker = ac.view().peek(loanBrokerKeylet);
+                    if (!BEAST_EXPECT(sleBroker))
+                        return false;
+                    ac.view().erase(sleBroker);
+                    return true;
+                },
+                XRPAmount{},
+                STTx{
+                    ttLOAN_BROKER_DELETE,
+                    [](STObject& tx) { tx.setFieldH256(sfLoanBrokerID, uint256(42)); }},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+                createLoanBroker);
         }
 
         // A LoanBrokerDelete must not remove a broker whose pre-transaction
@@ -728,6 +750,78 @@ class InvariantsPseudoAccount_test : public InvariantsBase
                 STTx{ttLOAN_BROKER_DELETE, [](STObject&) {}},
                 {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
                 createTwoBrokers);
+
+            // Erasing one broker while touching another is not caught by the
+            // multiple-deletion check, which only counts erasures. The erased
+            // broker is itself collected alongside the modified one, so the
+            // check has to disregard it and look for a different broker.
+            doInvariantCheck(
+                {{"Loan Broker deletion must not create or modify another Loan Broker"}},
+                [&](Account const&, Account const&, ApplyContext& ac) {
+                    auto sle1 = ac.view().peek(loanBrokerKeylet1);
+                    auto sle2 = ac.view().peek(loanBrokerKeylet2);
+                    if (!BEAST_EXPECT(sle1 && sle2))
+                        return false;
+                    ac.view().erase(sle1);
+                    ac.view().update(sle2);
+                    return true;
+                },
+                XRPAmount{},
+                STTx{ttLOAN_BROKER_DELETE, [](STObject&) {}},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+                createTwoBrokers);
+        }
+
+        // A LoanBrokerDelete that modifies a broker without deleting one must
+        // fail the invariant, but only with fixCleanup3_5_0 enabled and only
+        // when the transaction succeeded.
+        {
+            using Outcome = std::pair<TER, std::string>;
+            auto const deleteNoBroker = [&, this](FeatureBitset features, TER initialResult) {
+                Env env{*this, features};
+                Account const a1{"A1"};
+                Account const a2{"A2"};
+                env.fund(XRP(1000), a1, a2);
+                env.close();
+                PrettyAsset const xrpAsset{xrpIssue(), 1'000'000};
+                auto const brokerKeylet = createLoanBroker(a1, env, xrpAsset);
+                if (!BEAST_EXPECT(env.le(brokerKeylet)))
+                    return Outcome{initialResult, {}};
+                env.close();
+
+                OpenView ov{*env.current()};
+                STTx const tx{ttLOAN_BROKER_DELETE, [](STObject&) {}};
+                test::StreamSink sink{beast::Severity::Warning};
+                beast::Journal const jlog{sink};
+                ApplyContext ac{
+                    env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+                CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+                auto sleBroker = ac.view().peek(brokerKeylet);
+                if (!BEAST_EXPECT(sleBroker))
+                    return Outcome{initialResult, {}};
+                ac.view().update(sleBroker);
+
+                auto transactor = makeTransactor(ac);
+                if (!BEAST_EXPECT(transactor))
+                    return Outcome{initialResult, {}};
+                TER const ter = transactor->checkInvariants(
+                    initialResult, XRPAmount{}, Transactor::InvariantScope::Full);
+                return Outcome{ter, sink.messages().str()};
+            };
+
+            static constexpr char const* kNoBrokerDeleted =
+                "Loan Broker deletion succeeded without deleting a Loan Broker";
+
+            auto const [ter, log] = deleteNoBroker(all_, tesSUCCESS);
+            BEAST_EXPECT(log.contains(kNoBrokerDeleted));
+            BEAST_EXPECT(ter == tecINVARIANT_FAILED);
+            // Gated on fixCleanup3_5_0: before it, this state went unreported.
+            BEAST_EXPECT(!deleteNoBroker(all_ - fixCleanup3_5_0, tesSUCCESS)
+                              .second.contains(kNoBrokerDeleted));
+            // Gated on the transaction having succeeded, too. A LoanBrokerDelete
+            // that ends in a tec deletes nothing, and must not be faulted for it.
+            BEAST_EXPECT(!deleteNoBroker(all_, tecKILLED).second.contains(kNoBrokerDeleted));
         }
     }
 

src/test/app/invariants/InvariantsVault_test.cpp

@@ -375,43 +375,53 @@ class InvariantsVault_test : public InvariantsBase
                 return true;
             });
 
+        Precheck const eraseTwoVaults = [](Account const& a1, Account const& a2, ApplyContext& ac) {
+            for (auto const& a : {a1, a2})
+            {
+                auto const keylet = keylet::vault(a.id(), SeqProxy::rawSequence(ac.view().seq()));
+                auto sleVault = ac.view().peek(keylet);
+                if (!sleVault)
+                    return false;
+                ac.view().erase(sleVault);
+            }
+            return true;
+        };
+
+        Preclose const createTwoVaults = [](Account const& a1, Account const& a2, Env& env) {
+            Vault const vault{env};
+            for (auto const& a : {a1, a2})
+            {
+                auto [tx, _] = vault.create({.owner = a, .asset = xrpIssue()});
+                env(tx);
+            }
+            return true;
+        };
+
+        // Erasing two vaults is reported by the deletion count check, which is
+        // counted ahead of the general single-vault check so that it is
+        // reachable at all -- every erased vault is also recorded as a
+        // "before" state, so the general check would otherwise always win.
         doInvariantCheck(
-            {"vault operation updated more than single vault",
-             "deleted Vault without deleting its pseudo-account"},
-            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
-                {
-                    auto const keylet =
-                        keylet::vault(a1.id(), SeqProxy::rawSequence(ac.view().seq()));
-                    auto sleVault = ac.view().peek(keylet);
-                    if (!sleVault)
-                        return false;
-                    ac.view().erase(sleVault);
-                }
-                {
-                    auto const keylet =
-                        keylet::vault(a2.id(), SeqProxy::rawSequence(ac.view().seq()));
-                    auto sleVault = ac.view().peek(keylet);
-                    if (!sleVault)
-                        return false;
-                    ac.view().erase(sleVault);
-                }
-                return true;
-            },
+            {"more than one vault deleted", "deleted Vault without deleting its pseudo-account"},
+            eraseTwoVaults,
             XRPAmount{},
             STTx{ttVAULT_DELETE, [](STObject&) {}},
             {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
-            [&](Account const& a1, Account const& a2, Env& env) {
-                Vault const vault{env};
-                {
-                    auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
-                    env(tx);
-                }
-                {
-                    auto [tx, _] = vault.create({.owner = a2, .asset = xrpIssue()});
-                    env(tx);
-                }
-                return true;
-            });
+            createTwoVaults);
+
+        // Without fixCleanup3_5_0 the deletion count check is skipped and the
+        // same state falls through to the general single-vault check, which
+        // still rejects it. This pins the gate: the amendment sharpens the
+        // diagnostic, it does not decide whether the transaction is rejected.
+        doInvariantCheck(
+            makeEnv(all_ - fixCleanup3_5_0),
+            {"vault operation updated more than single vault",
+             "deleted Vault without deleting its pseudo-account"},
+            eraseTwoVaults,
+            XRPAmount{},
+            STTx{ttVAULT_DELETE, [](STObject&) {}},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+            createTwoVaults);
 
         doInvariantCheck(
             {"vault operation updated more than single vault"},
@@ -482,6 +492,221 @@ class InvariantsVault_test : public InvariantsBase
                 return true;
             });
 
+        // A VaultDelete must erase the vault its VaultID names. The vault here
+        // is perfectly deletable -- empty, with its share issuance erased
+        // alongside it -- so every other deletion check passes and only the
+        // identity check can fire. This is what a transactor that resolved the
+        // wrong keylet would leave behind.
+        doInvariantCheck(
+            {"deleted vault does not match the VaultID in the transaction"},
+            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const keylet = keylet::vault(a1.id(), SeqProxy::rawSequence(ac.view().seq()));
+                auto sleVault = ac.view().peek(keylet);
+                if (!sleVault)
+                    return false;
+                auto sleShares = ac.view().peek(keylet::mptokenIssuance((*sleVault)[sfShareMPTID]));
+                if (!sleShares)
+                    return false;
+                ac.view().erase(sleVault);
+                ac.view().erase(sleShares);
+                return true;
+            },
+            XRPAmount{},
+            STTx{ttVAULT_DELETE, [](STObject& tx) { tx.setFieldH256(sfVaultID, uint256(42)); }},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+            [&](Account const& a1, Account const& a2, Env& env) {
+                Vault const vault{env};
+                auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
+                env(tx);
+                return true;
+            });
+
+        // Erasing one vault while another survives the transaction is not
+        // caught by the single-vault check, which counts the "before" and
+        // "after" collections separately and so sees one entry in each. Left
+        // unchecked, the value comparisons would measure the erased vault's
+        // pre-state against the surviving vault's post-state.
+        doInvariantCheck(
+            {"vault deletion must not create or modify another vault"},
+            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const sequence = ac.view().seq();
+                auto sleVault =
+                    ac.view().peek(keylet::vault(a1.id(), SeqProxy::rawSequence(sequence)));
+                if (!sleVault)
+                    return false;
+                ac.view().erase(sleVault);
+                // A second, newly created vault leaves the "after" collection
+                // non-empty while the erased vault is the only "before" entry.
+                auto sleOther =
+                    std::make_shared<SLE>(keylet::vault(a2.id(), SeqProxy::rawSequence(sequence)));
+                auto const vaultPage = ac.view().dirInsert(
+                    keylet::ownerDir(a2.id()), sleOther->key(), describeOwnerDir(a2.id()));
+                sleOther->setFieldU64(sfOwnerNode, *vaultPage);
+                sleOther->setAccountID(sfAccount, a2.id());
+                ac.view().insert(sleOther);
+                return true;
+            },
+            XRPAmount{},
+            STTx{ttVAULT_SET, [](STObject&) {}},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+            [&](Account const& a1, Account const& a2, Env& env) {
+                Vault const vault{env};
+                auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
+                env(tx);
+                return true;
+            });
+
+        // The same pairing on ttVAULT_DELETE, the one transaction type allowed
+        // to erase a vault. It reports this diagnostic only because the check
+        // is made ahead of the case that reads the surviving vault and reports
+        // a deletion that erased nothing.
+        //
+        // Erase a1's vault and create a bare one for a2, leaving the erased
+        // vault the only "before" entry and the new one the only "after" entry.
+        Precheck const eraseAndAdd = [](Account const& a1, Account const& a2, ApplyContext& ac) {
+            auto const sequence = ac.view().seq();
+            auto sleVault = ac.view().peek(keylet::vault(a1.id(), SeqProxy::rawSequence(sequence)));
+            if (!sleVault)
+                return false;
+            ac.view().erase(sleVault);
+            auto sleOther =
+                std::make_shared<SLE>(keylet::vault(a2.id(), SeqProxy::rawSequence(sequence)));
+            auto const vaultPage = ac.view().dirInsert(
+                keylet::ownerDir(a2.id()), sleOther->key(), describeOwnerDir(a2.id()));
+            sleOther->setFieldU64(sfOwnerNode, *vaultPage);
+            sleOther->setAccountID(sfAccount, a2.id());
+            ac.view().insert(sleOther);
+            return true;
+        };
+
+        Preclose const createOneVault = [](Account const& a1, Account const& a2, Env& env) {
+            Vault const vault{env};
+            auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
+            env(tx);
+            return true;
+        };
+
+        doInvariantCheck(
+            {"vault deletion must not create or modify another vault"},
+            eraseAndAdd,
+            XRPAmount{},
+            STTx{ttVAULT_DELETE, [](STObject&) {}},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+            createOneVault);
+
+        // Without fixCleanup3_5_0 the same state falls through to that case and
+        // is reported as a deletion that erased nothing. This pins the gate:
+        // the amendment sharpens the diagnostic, it does not decide whether the
+        // transaction is rejected.
+        doInvariantCheck(
+            makeEnv(all_ - fixCleanup3_5_0),
+            {"vault deletion succeeded without deleting a vault"},
+            eraseAndAdd,
+            XRPAmount{},
+            STTx{ttVAULT_DELETE, [](STObject&) {}},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+            createOneVault);
+
+        // A VaultDelete that deletes a1's vault must not erase a2's vault's
+        // trust line along with it. The third leg checks that deleting a1's vault
+        // on its own stays silent.
+        {
+            using Holding = std::pair<TER, std::string>;
+            auto const deleteVaultAndHolding = [&, this](FeatureBitset features, bool eraseOther) {
+                Env env{*this, features};
+                Account const gw{"gw"};
+                Account const a1{"A1"};
+                Account const a2{"A2"};
+                env.fund(XRP(1000), gw, a1, a2);
+                env.close();
+
+                // An IOU vault, so that the pseudo-account holds the asset in a
+                // RippleState of its own. An XRP vault has no holding to erase.
+                PrettyAsset const asset{gw["IOU"]};
+                Vault const vault{env};
+                auto [txCreate1, vaultKeylet1] = vault.create({.owner = a1, .asset = asset});
+                env(txCreate1);
+                auto [txCreate2, vaultKeylet2] = vault.create({.owner = a2, .asset = asset});
+                env(txCreate2);
+                env.close();
+
+                OpenView ov{*env.current()};
+                STTx const tx{ttVAULT_DELETE, [&vaultKeylet1](STObject& obj) {
+                                  obj.setFieldH256(sfVaultID, vaultKeylet1.key);
+                              }};
+                test::StreamSink sink{beast::Severity::Warning};
+                beast::Journal const jlog{sink};
+                ApplyContext ac{
+                    env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+                CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+                // Read (don't touch) the vaults: a pseudo-account's holding is
+                // reachable from the share issuance, which records it in
+                // sfReferenceHolding for every non-XRP vault from
+                // fixCleanup3_2_0 onward.
+                auto const holdingKeylet = [&](Keylet const& k) -> std::optional<Keylet> {
+                    auto const sleVault = ac.view().read(k);
+                    if (!sleVault)
+                        return std::nullopt;
+                    auto const sleShares =
+                        ac.view().read(keylet::mptokenIssuance(sleVault->at(sfShareMPTID)));
+                    if (!sleShares || !sleShares->isFieldPresent(sfReferenceHolding))
+                        return std::nullopt;
+                    return keylet::unchecked(sleShares->getFieldH256(sfReferenceHolding));
+                };
+                auto const holding1 = holdingKeylet(vaultKeylet1);
+                auto const holding2 = holdingKeylet(vaultKeylet2);
+                if (!BEAST_EXPECT(holding1 && holding2))
+                    return Holding{tesSUCCESS, {}};
+
+                auto sleVault1 = ac.view().peek(vaultKeylet1);
+                if (!BEAST_EXPECT(sleVault1))
+                    return Holding{tesSUCCESS, {}};
+                auto sleShares1 =
+                    ac.view().peek(keylet::mptokenIssuance(sleVault1->at(sfShareMPTID)));
+                auto slePseudo1 = ac.view().peek(keylet::account(sleVault1->at(sfAccount)));
+                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+                auto sleHolding1 = ac.view().peek(*holding1);
+                if (!BEAST_EXPECT(sleShares1 && slePseudo1 && sleHolding1))
+                    return Holding{tesSUCCESS, {}};
+                ac.view().erase(sleHolding1);
+                ac.view().erase(sleShares1);
+                ac.view().erase(slePseudo1);
+                ac.view().erase(sleVault1);
+
+                if (eraseOther)
+                {
+                    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+                    auto sleHolding2 = ac.view().peek(*holding2);
+                    if (!BEAST_EXPECT(sleHolding2))
+                        return Holding{tesSUCCESS, {}};
+                    ac.view().erase(sleHolding2);
+                }
+
+                auto transactor = makeTransactor(ac);
+                if (!BEAST_EXPECT(transactor))
+                    return Holding{tesSUCCESS, {}};
+                TER const ter = transactor->checkInvariants(
+                    tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full);
+                return Holding{ter, sink.messages().str()};
+            };
+
+            static constexpr char const* kOtherHolding =
+                "vault deletion deleted another vault pseudo-account's holding";
+
+            auto const [terHolding, logHolding] = deleteVaultAndHolding(all_, true);
+            BEAST_EXPECT(logHolding.contains(kOtherHolding));
+            BEAST_EXPECT(terHolding == tecINVARIANT_FAILED);
+            // Gated on fixCleanup3_5_0: before it the transaction type alone
+            // excused the other vault's erased holding.
+            BEAST_EXPECT(!deleteVaultAndHolding(all_ - fixCleanup3_5_0, true)
+                              .second.contains(kOtherHolding));
+            // The deleted vault's own holding stays exempt. Its pseudo-account
+            // is gone from the post-transaction view, so nothing there
+            // identifies the erased holding as a vault's.
+            BEAST_EXPECT(!deleteVaultAndHolding(all_, false).second.contains(kOtherHolding));
+        }
+
         doInvariantCheck(
             {"vault operation succeeded without modifying a vault"},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {
@@ -1465,6 +1690,166 @@ class InvariantsVault_test : public InvariantsBase
                 STTx{ttACCOUNT_SET, [](STObject&) {}},
                 {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
                 precloseLoan);
+
+            // A LoanDelete must erase the loan its LoanID names. Naming a
+            // different loan is what a transactor that resolved the wrong
+            // keylet would leave behind.
+            doInvariantCheck(
+                {"deleted loan does not match the LoanID in the transaction"},
+                eraseLoan,
+                XRPAmount{},
+                STTx{ttLOAN_DELETE, [](STObject& tx) { tx.setFieldH256(sfLoanID, uint256(42)); }},
+                {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+                precloseLoan);
+        }
+
+        // A LoanDelete that erases one loan must not touch another. Two loans
+        // are seeded against a real broker so that both are pre-existing: the
+        // second is modified rather than created, which would otherwise trip
+        // the "created by a transaction other than LoanSet" check first.
+        {
+            Env env{*this, all_};
+            Account const a1{"A1"};
+            Account const a2{"A2"};
+            env.fund(XRP(1000), a1, a2);
+            auto const keys = createClosedXrpBroker(a1, env);
+            if (BEAST_EXPECT(keys))
+            {
+                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+                auto const& brokerKeylet = keys->second;
+
+                OpenView ov{*env.current()};
+                auto const loanKeylet1 = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(1));
+                auto const loanKeylet2 = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(2));
+                ov.rawInsert(makeLoanSle(brokerKeylet.key, 1, a1.id()));
+                ov.rawInsert(makeLoanSle(brokerKeylet.key, 2, a1.id()));
+
+                STTx const tx{ttLOAN_DELETE, [](STObject&) {}};
+                test::StreamSink sink{beast::Severity::Warning};
+                beast::Journal const jlog{sink};
+                ApplyContext ac{
+                    env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+                CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+                auto sleLoan1 = ac.view().peek(loanKeylet1);
+                auto sleLoan2 = ac.view().peek(loanKeylet2);
+                if (BEAST_EXPECT(sleLoan1 && sleLoan2))
+                {
+                    ac.view().erase(sleLoan1);
+                    ac.view().update(sleLoan2);
+
+                    auto transactor = makeTransactor(ac);
+                    if (BEAST_EXPECT(transactor))
+                    {
+                        TER const result = transactor->checkInvariants(
+                            tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full);
+                        BEAST_EXPECT(result == tecINVARIANT_FAILED);
+                        BEAST_EXPECT(sink.messages().str().contains(
+                            "loan deletion must not create or modify another loan"));
+                    }
+                }
+            }
+        }
+
+        // A LoanDelete must not erase more than one loan. The bound is new in
+        // fixCleanup3_5_0: before it ValidLoan checked only that the erasing
+        // transaction was a LoanDelete, so any number of loans could go at
+        // once. Both loans are seeded directly, so erasing them is the only
+        // thing ValidLoan has to object to.
+        for (bool const withFix : {true, false})
+        {
+            Env env{*this, withFix ? all_ : all_ - fixCleanup3_5_0};
+            Account const a1{"A1"};
+            Account const a2{"A2"};
+            env.fund(XRP(1000), a1, a2);
+            auto const keys = createClosedXrpBroker(a1, env);
+            if (!BEAST_EXPECT(keys))
+                continue;
+            // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+            auto const& brokerKeylet = keys->second;
+
+            OpenView ov{*env.current()};
+            auto const loanKeylet1 = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(1));
+            auto const loanKeylet2 = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(2));
+            ov.rawInsert(makeLoanSle(brokerKeylet.key, 1, a1.id()));
+            ov.rawInsert(makeLoanSle(brokerKeylet.key, 2, a1.id()));
+
+            STTx const tx{ttLOAN_DELETE, [](STObject&) {}};
+            test::StreamSink sink{beast::Severity::Warning};
+            beast::Journal const jlog{sink};
+            ApplyContext ac{
+                env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+            CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+            auto sleLoan1 = ac.view().peek(loanKeylet1);
+            auto sleLoan2 = ac.view().peek(loanKeylet2);
+            if (!BEAST_EXPECT(sleLoan1 && sleLoan2))
+                continue;
+            ac.view().erase(sleLoan1);
+            ac.view().erase(sleLoan2);
+
+            auto transactor = makeTransactor(ac);
+            if (!BEAST_EXPECT(transactor))
+                continue;
+            TER const result = transactor->checkInvariants(
+                tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full);
+            BEAST_EXPECT(sink.messages().str().contains("more than one Loan deleted") == withFix);
+            if (withFix)
+                BEAST_EXPECT(result == tecINVARIANT_FAILED);
+        }
+
+        // A LoanDelete that modifies a loan without deleting one must fail the
+        // invariant, but only with fixCleanup3_5_0 enabled and only when the
+        // transaction succeeded.
+        {
+            using Outcome = std::pair<TER, std::string>;
+            auto const deleteNoLoan = [&, this](FeatureBitset features, TER initialResult) {
+                Env env{*this, features};
+                Account const a1{"A1"};
+                Account const a2{"A2"};
+                env.fund(XRP(1000), a1, a2);
+                auto const keys = createClosedXrpBroker(a1, env);
+                if (!BEAST_EXPECT(keys))
+                    return Outcome{initialResult, {}};
+                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+                auto const& brokerKeylet = keys->second;
+
+                OpenView ov{*env.current()};
+                auto const loanKeylet = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(1));
+                ov.rawInsert(makeLoanSle(brokerKeylet.key, 1, a1.id()));
+
+                STTx const tx{ttLOAN_DELETE, [](STObject&) {}};
+                test::StreamSink sink{beast::Severity::Warning};
+                beast::Journal const jlog{sink};
+                ApplyContext ac{
+                    env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+                CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+                auto sleLoan = ac.view().peek(loanKeylet);
+                if (!BEAST_EXPECT(sleLoan))
+                    return Outcome{initialResult, {}};
+                ac.view().update(sleLoan);
+
+                auto transactor = makeTransactor(ac);
+                if (!BEAST_EXPECT(transactor))
+                    return Outcome{initialResult, {}};
+                TER const ter = transactor->checkInvariants(
+                    initialResult, XRPAmount{}, Transactor::InvariantScope::Full);
+                return Outcome{ter, sink.messages().str()};
+            };
+
+            static constexpr char const* kNoLoanDeleted =
+                "loan deletion succeeded without deleting a loan";
+
+            auto const [ter, log] = deleteNoLoan(all_, tesSUCCESS);
+            BEAST_EXPECT(log.contains(kNoLoanDeleted));
+            BEAST_EXPECT(ter == tecINVARIANT_FAILED);
+            // Gated on fixCleanup3_5_0: before it, this state went unreported.
+            BEAST_EXPECT(
+                !deleteNoLoan(all_ - fixCleanup3_5_0, tesSUCCESS).second.contains(kNoLoanDeleted));
+            // Gated on the transaction having succeeded, too. A LoanDelete that
+            // ends in a tec deletes nothing, and must not be faulted for it.
+            BEAST_EXPECT(!deleteNoLoan(all_, tecKILLED).second.contains(kNoLoanDeleted));
         }
 
         STTx const loanSetTx{