From e34c2667d7b29cc38d83e8ec7190cd7769ec4e9f Mon Sep 17 00:00:00 2001
From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com>
Date: Sun, 24 May 2026 16:37:16 -0400
Subject: [PATCH] fix: Skip deleted book directories and non-root modifications
 in `ValidBookDirectory` invariant (#7312)

---
 .../tx/invariants/DirectoryInvariant.cpp      | 18 ++++--
 src/test/app/Invariants_test.cpp              | 64 +++++++++++++++++++
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp
index 2a4fac07d0..1624a19830 100644
--- a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp
+++ b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp
@@ -40,19 +40,27 @@ badExchangeRate(SLE const& dir)
 
 void
 ValidBookDirectory::visitEntry(
-    bool,
+    bool isDelete,
     std::shared_ptr<SLE const> const& before,
     std::shared_ptr<SLE const> const& after)
 {
     // New root directories must have matching exchange-rate metadata. New
-    // child directories must point to an existing root.
+    // child directories, and modified directories that change sfRootIndex, must
+    // point to an existing root.
 
-    // Only validate newly-created directories; LedgerStateFix handles legacy
-    // bad exchange-rate metadata.
-    if (badBookDirectory_ || before || !after || after->getType() != ltDIR_NODE)
+    // Only validate newly-created directories and sfRootIndex changes;
+    // LedgerStateFix handles legacy bad exchange-rate metadata. Skip deletions
+    // because `after` is not guaranteed to be null.
+    if (badBookDirectory_ || isDelete || !after || after->getType() != ltDIR_NODE)
         return;
 
     auto const rootIndex = after->getFieldH256(sfRootIndex);
+    // Ignore ordinary modifications that do not change which root this
+    // directory belongs to. That tolerates legacy bad exchange-rate metadata
+    // during normal operation while still checking sfRootIndex changes.
+    if (before && before->getFieldH256(sfRootIndex) == rootIndex)
+        return;
+
     if (after->key() == rootIndex && !badBookDirectory_)
     {
         badBookDirectory_ = badBookDirectory_ || badExchangeRate(*after);
diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp
index cc16948553..c8a6e813de 100644
--- a/src/test/app/Invariants_test.cpp
+++ b/src/test/app/Invariants_test.cpp
@@ -2137,6 +2137,70 @@ class Invariants_test : public beast::unit_test::Suite
             BEAST_EXPECT(
                 invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
         }
+
+        // A bad root is rejected when added, ignored when a legacy bad root is
+        // modified without changing sfRootIndex or deleted, and checked when a
+        // modified directory changes sfRootIndex.
+        {
+            Env env{*this, defaultAmendments()};
+            Account const a1{"A1"};
+            env.fund(XRP(1000), a1);
+            env.close();
+
+            OpenView view{*env.current()};
+            auto const directoryQuality = STAmount::kURateOne;
+            auto const rootDir = getBookRootKey(a1, directoryQuality);
+            auto const missingRootDir = getBookRootKey(a1, directoryQuality + 1);
+            auto const badRoot = makeRootPage(rootDir, directoryQuality + 1);
+            view.rawInsert(badRoot);
+
+            test::StreamSink sink{beast::Severity::Warning};
+            beast::Journal const jlog{sink};
+
+            {
+                // add
+                ValidBookDirectory invariant;
+                invariant.visitEntry(false, nullptr, badRoot);
+
+                BEAST_EXPECT(
+                    !invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
+            }
+            {
+                // modify (without changing the sfRootIndex)
+                ValidBookDirectory invariant;
+                invariant.visitEntry(false, badRoot, badRoot);
+
+                BEAST_EXPECT(
+                    invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
+            }
+            {
+                // modify (changing sfRootIndex to a missing root)
+                auto const childBefore = makeChildPage(rootDir);
+                auto const childAfter = std::make_shared<SLE>(*childBefore, childBefore->key());
+                childAfter->setFieldH256(sfRootIndex, missingRootDir.key);
+
+                ValidBookDirectory invariant;
+                invariant.visitEntry(false, childBefore, childAfter);
+
+                test::StreamSink missingRootSink{beast::Severity::Warning};
+                beast::Journal const missingRootJlog{missingRootSink};
+                BEAST_EXPECT(!invariant.finalize(
+                    makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, missingRootJlog));
+                BEAST_EXPECT(
+                    missingRootSink.messages().str().find("book directory root missing") !=
+                    std::string::npos);
+            }
+            {
+                // delete
+                view.rawErase(badRoot);
+                BEAST_EXPECT(!view.exists(rootDir));
+
+                ValidBookDirectory invariant;
+                invariant.visitEntry(true, badRoot, badRoot);
+                BEAST_EXPECT(
+                    invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
+            }
+        }
     }
 
     Keylet