refactor: Invert ObjectHasPseudoAccount to check deletion

Change ObjectHasPseudoAccount from checking "object exists →
pseudo-account exists" to "object deleted → pseudo-account deleted".

Together with AccountRootsDeletedClean (which enforces the reverse:
pseudo-account deleted → object deleted), this pair of invariants
guarantees that an object and its pseudo-account are always deleted
as a unit.

include/xrpl/tx/invariants/InvariantCheck.h

@@ -386,8 +386,11 @@ public:
 };
 
 /*
- * Verify that all objects that have associated pseudo-accounts, always have the said
- * pseudo-accounts.
+ * Verify that when an object with an associated pseudo-account is deleted,
+ * its pseudo-account is also deleted.
+ *
+ * The reverse (pseudo-account deleted → object deleted) is enforced by
+ * AccountRootsDeletedClean via getPseudoAccountFields().
  */
 class ObjectHasPseudoAccount
 {
@@ -399,7 +402,7 @@ public:
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
 
 private:
-    std::vector<SLE::const_pointer> sles_;
+    std::vector<SLE::const_pointer> deletedObjSles_;
 };
 // additional invariant checks can be declared above and then added to this
 // tuple

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -1072,25 +1072,24 @@ ValidAmounts::finalize(
 void
 ObjectHasPseudoAccount::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after)
 {
-    // If an object is deleted, pseudo-account is also deleted
-    if (isDelete)
+    if (!isDelete)
         return;
 
-    // After should never be null when isDelete = false, if it is, something went horribly wrong
-    if (!after)
+    // Before should never be null when isDelete = true
+    if (!before)
     {
         XRPL_ASSERT(
-            after,
-            "xrpl::ObjectHasPseudoAccount::visitEntry : modified ledger entry missing after state");
+            before,
+            "xrpl::ObjectHasPseudoAccount::visitEntry : deleted ledger entry missing before state");
         return;  // LCOV_EXCL_LINE
     }
 
-    switch (after->getType())
+    switch (before->getType())
     {
         case ltAMM:
         case ltVAULT:
         case ltLOAN_BROKER:
-            sles_.push_back(after);
+            deletedObjSles_.push_back(before);
             break;
         default:
             return;
@@ -1108,28 +1107,32 @@ ObjectHasPseudoAccount::finalize(
     if (!view.rules().enabled(fixCleanup3_3_0))
         return true;
 
-    if (sles_.empty())
+    if (deletedObjSles_.empty())
         return true;
 
+    auto const typeName = [](SLE const& sle) {
+        if (auto item = LedgerFormats::getInstance().findByType(sle.getType()))
+            return item->getName();
+        return std::to_string(sle.getType());
+    };
+
     bool failed = false;
-    for (auto const& sle : sles_)
+    for (auto const& sle : deletedObjSles_)
     {
-        // For current ledger entry types, pseudo-account is identified by `sfAccount` field.
         if (!sle->isFieldPresent(sfAccount))
         {
-            JLOG(j.fatal()) << "Invariant failed: ledger entry " << sle->getSType()
+            JLOG(j.fatal()) << "Invariant failed: deleted " << typeName(*sle)
                             << " is missing pseudo-account field";
             failed = true;
             continue;
         }
 
+        // The pseudo-account must NOT exist on the ledger after the object is deleted.
         bool const exists = view.read(keylet::account(sle->getAccountID(sfAccount))) != nullptr;
-
-        // The pseudo-account must exist on the ledger
-        if (!exists)
+        if (exists)
         {
-            JLOG(j.fatal()) << "Invariant failed: ledger entry " << sle->getSType()
-                            << " pseudo-account does not exist";
+            JLOG(j.fatal()) << "Invariant failed: deleted " << typeName(*sle)
+                            << " without deleting its pseudo-account";
             failed = true;
         }
     }

src/test/app/Invariants_test.cpp

@@ -2741,7 +2741,8 @@ class Invariants_test : public beast::unit_test::Suite
             });
 
         doInvariantCheck(
-            {"vault updated by a wrong transaction type"},
+            {"vault updated by a wrong transaction type",
+             "deleted Vault without deleting its pseudo-account"},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {
                 auto const keylet = keylet::vault(a1.id(), ac.view().seq());
                 auto sleVault = ac.view().peek(keylet);
@@ -2752,7 +2753,7 @@ class Invariants_test : public beast::unit_test::Suite
             },
             XRPAmount{},
             STTx{ttPAYMENT, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2798,7 +2799,8 @@ class Invariants_test : public beast::unit_test::Suite
             {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
 
         doInvariantCheck(
-            {"vault deleted by a wrong transaction type"},
+            {"vault deleted by a wrong transaction type",
+             "deleted Vault without deleting its pseudo-account"},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {
                 auto const keylet = keylet::vault(a1.id(), ac.view().seq());
                 auto sleVault = ac.view().peek(keylet);
@@ -2809,7 +2811,7 @@ class Invariants_test : public beast::unit_test::Suite
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2818,7 +2820,8 @@ class Invariants_test : public beast::unit_test::Suite
             });
 
         doInvariantCheck(
-            {"vault operation updated more than single vault"},
+            {"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(), ac.view().seq());
@@ -2838,7 +2841,7 @@ class Invariants_test : public beast::unit_test::Suite
             },
             XRPAmount{},
             STTx{ttVAULT_DELETE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 {
@@ -2874,7 +2877,8 @@ class Invariants_test : public beast::unit_test::Suite
             {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
 
         doInvariantCheck(
-            {"deleted vault must also delete shares"},
+            {"deleted vault must also delete shares",
+             "deleted Vault without deleting its pseudo-account"},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {
                 auto const keylet = keylet::vault(a1.id(), ac.view().seq());
                 auto sleVault = ac.view().peek(keylet);
@@ -2885,7 +2889,7 @@ class Invariants_test : public beast::unit_test::Suite
             },
             XRPAmount{},
             STTx{ttVAULT_DELETE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -4892,71 +4896,116 @@ class Invariants_test : public beast::unit_test::Suite
         using namespace jtx;
 
         auto const amendments = defaultAmendments() | fixCleanup3_3_0;
-        // An account that is never funded, so it won't exist in any view.
-        AccountID const ghostId = Account{"Ghost"}.id();
 
-        doInvariantCheck(
-            Env{*this, amendments},
-            {{"is missing pseudo-account field"}},
-            [](Account const& a1, Account const&, ApplyContext& ac) {
-                auto const brokerKeylet = keylet::loanbroker(a1.id(), ac.view().seq());
-                auto sle = std::make_shared<SLE>(brokerKeylet);
-                sle->makeFieldAbsent(sfAccount);
-                auto const page = ac.view().dirInsert(
-                    keylet::ownerDir(a1.id()), sle->key(), describeOwnerDir(a1.id()));
-                if (!page)
-                    return false;
-                sle->setFieldU64(sfOwnerNode, *page);
-                sle->setFieldU32(sfOwnerCount, 1);
-                ac.view().insert(sle);
-                return true;
-            });
+        // Vault: object deleted without its pseudo-account
+        {
+            Keylet vaultKeylet = keylet::amendments();
+            doInvariantCheck(
+                Env{*this, amendments},
+                {{"deleted Vault without deleting its pseudo-account"}},
+                [&vaultKeylet](Account const&, Account const&, ApplyContext& ac) {
+                    auto sle = ac.view().peek(vaultKeylet);
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                XRPAmount{},
+                STTx{ttVAULT_DELETE, [](STObject&) {}},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+                [&vaultKeylet](Account const& a1, Account const&, Env& env) {
+                    Vault const vault{env};
+                    auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
+                    env(tx);
+                    vaultKeylet = keylet;
+                    return true;
+                });
+        }
 
-        // Vault: pseudo-account is referenced but does not exist on the ledger.
-        doInvariantCheck(
-            Env{*this, amendments},
-            {{"pseudo-account does not exist"}},
-            [ghostId](Account const& a1, Account const&, ApplyContext& ac) {
-                auto const vaultKeylet = keylet::vault(a1.id(), ac.view().seq());
-                auto sle = std::make_shared<SLE>(vaultKeylet);
-                sle->setAccountID(sfAccount, ghostId);
-                auto const page = ac.view().dirInsert(
-                    keylet::ownerDir(a1.id()), sle->key(), describeOwnerDir(a1.id()));
-                if (!page)
-                    return false;
-                sle->setFieldU64(sfOwnerNode, *page);
-                ac.view().insert(sle);
-                return true;
-            });
+        // AMM: object deleted without its pseudo-account
+        {
+            uint256 ammID{};
+            Account const gw{"gw"};
+            doInvariantCheck(
+                Env{*this, amendments},
+                {{"deleted AMM without deleting its pseudo-account"}},
+                [&ammID](Account const&, Account const&, ApplyContext& ac) {
+                    auto sle = ac.view().peek(keylet::amm(ammID));
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                XRPAmount{},
+                STTx{ttAMM_DELETE, [](STObject&) {}},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+                [&ammID, &gw](Account const&, Account const&, Env& env) {
+                    env.fund(XRP(1'000), gw);
+                    AMM const amm(env, gw, XRP(100), gw["USD"](100));
+                    ammID = amm.ammID();
+                    return true;
+                });
+        }
 
-        // AMM: pseudo-account is referenced but does not exist on the ledger.
-        doInvariantCheck(
-            Env{*this, amendments},
-            {{"pseudo-account does not exist"}},
-            [ghostId](Account const&, Account const&, ApplyContext& ac) {
-                auto const ammKeylet = keylet::amm(uint256(1u));
-                auto sle = std::make_shared<SLE>(ammKeylet);
-                sle->setAccountID(sfAccount, ghostId);
-                ac.view().insert(sle);
-                return true;
-            });
+        // LoanBroker: object deleted without its pseudo-account
+        {
+            Keylet loanBrokerKeylet = keylet::amendments();
+            doInvariantCheck(
+                Env{*this, amendments},
+                {{"deleted LoanBroker without deleting its pseudo-account"}},
+                [&loanBrokerKeylet](Account const&, Account const&, ApplyContext& ac) {
+                    auto sle = ac.view().peek(loanBrokerKeylet);
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                XRPAmount{},
+                STTx{ttLOAN_BROKER_DELETE, [](STObject&) {}},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
+                [&loanBrokerKeylet, this](Account const& a1, Account const&, Env& env) {
+                    PrettyAsset const xrpAsset{xrpIssue(), 1'000'000};
+                    loanBrokerKeylet = this->createLoanBroker(a1, env, xrpAsset);
+                    return BEAST_EXPECT(env.le(loanBrokerKeylet));
+                });
+        }
 
-        // LoanBroker: pseudo-account is referenced but does not exist on the ledger.
-        doInvariantCheck(
-            Env{*this, amendments},
-            {{"pseudo-account does not exist"}},
-            [ghostId](Account const& a1, Account const&, ApplyContext& ac) {
-                auto const brokerKeylet = keylet::loanbroker(a1.id(), ac.view().seq());
-                auto sle = std::make_shared<SLE>(brokerKeylet);
-                sle->setAccountID(sfAccount, ghostId);
-                auto const page = ac.view().dirInsert(
-                    keylet::ownerDir(a1.id()), sle->key(), describeOwnerDir(a1.id()));
-                if (!page)
-                    return false;
-                sle->setFieldU64(sfOwnerNode, *page);
-                ac.view().insert(sle);
-                return true;
-            });
+        // Deleted object missing sfAccount field (defensive check).
+        // Manually construct the view to place a vault SLE without
+        // sfAccount into the base ledger, then erase it.
+        {
+            Env env{*this, amendments};
+            Account const a1{"A1"};
+            Account const a2{"A2"};
+            env.fund(XRP(1000), a1, a2);
+            env.close();
+
+            OpenView ov{*env.current()};
+
+            auto const vaultKeylet = keylet::vault(a1.id(), ov.seq());
+            auto sleVault = std::make_shared<SLE>(vaultKeylet);
+            sleVault->makeFieldAbsent(sfAccount);
+            ov.rawInsert(sleVault);
+
+            STTx tx{ttVAULT_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 sle = ac.view().peek(vaultKeylet);
+            if (!BEAST_EXPECT(sle))
+                return;
+            ac.view().erase(sle);
+
+            auto transactor = makeTransactor(ac);
+            if (!BEAST_EXPECT(transactor))
+                return;
+            TER const result = transactor->checkInvariants(tesSUCCESS, XRPAmount{});
+            BEAST_EXPECT(result == tecINVARIANT_FAILED);
+            BEAST_EXPECT(sink.messages().str().contains("is missing pseudo-account field"));
+        }
     }
 
 public: