Merge branch 'develop' into vlntb/mem-leak-ledger-history-3

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

include/xrpl/basics/TaggedCache.h

@@ -153,6 +153,24 @@ private:
         Policy policy,
         Callback&& replaceCallback = nullptr);
 
+    /**
+     * canonicalizeImpl variant for callers that already hold `mutex_`.
+     *
+     * The unnamed `scoped_lock<mutex_type> const&` parameter is proof of
+     * ownership; this overload does not acquire the lock. Use it from other
+     * methods (e.g. fetchAndModify) that need to compose canonicalize with
+     * additional work under a single critical section, without relying on
+     * recursive-mutex re-entry.
+     */
+    template <class Policy, class Callback = std::nullptr_t>
+    bool
+    canonicalizeImpl(
+        std::scoped_lock<mutex_type> const&,
+        key_type const& key,
+        CanonicalizeClientPointerType<Policy> data,
+        Policy policy,
+        Callback&& replaceCallback = nullptr);
+
 public:
     /**
      * Replace aliased objects with originals.
@@ -239,9 +257,6 @@ public:
     bool
     retrieve(key_type const& key, T& data);
 
-    mutex_type&
-    peekMutex();
-
     std::vector<key_type>
     getKeys() const;
 
@@ -263,6 +278,22 @@ public:
     fetch(key_type const& digest, Handler const& h);
     // End CachedSLEs functions.
 
+    /**
+     * Fetch or create an entry and execute a callback while holding the lock.
+     *
+     * The entry for the given key is fetched from the cache or created if it
+     * doesn't exist. The callback is then invoked with a reference to the
+     * entry while the cache mutex is still held, allowing safe modification
+     * of the cached object.
+     *
+     * @param key The key to fetch or create
+     * @param callback Function to call with the entry under lock.
+     *                 Signature: void(T&)
+     */
+    template <class Callback>
+    void
+    fetchAndModify(key_type const& key, Callback&& callback);
+
 private:
     SharedPointerType
     initialFetch(key_type const& key, std::scoped_lock<mutex_type> const& l);

include/xrpl/basics/TaggedCache.ipp

@@ -4,6 +4,7 @@
 #include <xrpl/basics/Log.h>  // IWYU pragma: keep
 #include <xrpl/basics/TaggedCache.h>
 #include <xrpl/basics/scope.h>
+#include <xrpl/beast/utility/instrumentation.h>
 
 #include <algorithm>
 
@@ -335,6 +336,29 @@ template <class Policy, class Callback>
 inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
     canonicalizeImpl(
+        key_type const& key,
+        CanonicalizeClientPointerType<Policy> data,
+        Policy policy,
+        Callback&& replaceCallback)
+{
+    std::scoped_lock<mutex_type> const lock(mutex_);
+    return canonicalizeImpl(lock, key, data, policy, std::forward<Callback>(replaceCallback));
+}
+
+template <
+    class Key,
+    class T,
+    bool IsKeyCache,
+    class SharedWeakUnionPointer,
+    class SharedPointerType,
+    class Hash,
+    class KeyEqual,
+    class Mutex>
+template <class Policy, class Callback>
+inline bool
+TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
+    canonicalizeImpl(
+        std::scoped_lock<mutex_type> const&,
         key_type const& key,
         CanonicalizeClientPointerType<Policy> data,
         [[maybe_unused]] Policy policy,
@@ -354,8 +378,6 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     // passing a const argument is a compile error.
     constexpr bool replaceCached = std::is_same_v<Policy, detail::ReplaceCached>;
 
-    std::scoped_lock const lock(mutex_);
-
     auto cit = cache_.find(key);
 
     if (cit == cache_.end())
@@ -572,22 +594,6 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     return true;
 }
 
-template <
-    class Key,
-    class T,
-    bool IsKeyCache,
-    class SharedWeakUnionPointer,
-    class SharedPointerType,
-    class Hash,
-    class KeyEqual,
-    class Mutex>
-inline auto
-TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
-    peekMutex() -> mutex_type&
-{
-    return mutex_;
-}
-
 template <
     class Key,
     class T,
@@ -700,6 +706,36 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
 }
 // End CachedSLEs functions.
 
+template <
+    class Key,
+    class T,
+    bool IsKeyCache,
+    class SharedWeakUnionPointer,
+    class SharedPointerType,
+    class Hash,
+    class KeyEqual,
+    class Mutex>
+template <class Callback>
+inline void
+TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
+    fetchAndModify(key_type const& key, Callback&& callback)
+{
+    static_assert(
+        !IsKeyCache, "fetchAndModify is only supported for value caches, not key-only caches");
+
+    std::scoped_lock<mutex_type> const lock(mutex_);
+
+    auto entry = std::make_shared<T>();
+    canonicalizeImpl(lock, key, entry, detail::ReplaceDynamically{}, [](SharedPointerType const&) {
+        return false;
+    });
+
+    ALWAYS(
+        entry != nullptr, "xrpl::TaggedCache::fetchAndModify : entry present after canonicalize");
+
+    callback(*entry);
+}
+
 template <
     class Key,
     class T,

src/test/app/LedgerHistory_test.cpp

@@ -79,6 +79,67 @@ public:
         return res;
     }
 
+    void
+    testHashIndexInvariant()
+    {
+        testcase("LedgerHistory hash/index invariant");
+        using namespace jtx;
+        using namespace std::chrono;
+
+        Env env{*this};
+        LedgerHistory lh{beast::insight::NullCollector::make(), env.app()};
+
+        // Create and insert validated ledgers
+        auto const genesis = makeLedger({}, env, lh, 0s);
+        auto const ledger1 = makeLedger(genesis, env, lh, 4s);
+        auto const ledger2 = makeLedger(ledger1, env, lh, 4s);
+        auto const ledger3 = makeLedger(ledger2, env, lh, 4s);
+
+        // Insert as validated (so they go into by_index)
+        lh.insert(genesis, true);
+        lh.insert(ledger1, true);
+        lh.insert(ledger2, true);
+        lh.insert(ledger3, true);
+
+        // Verify the hash/index invariant holds
+        // Can retrieve by sequence and get correct hash
+        BEAST_EXPECT(lh.getLedgerHash(genesis->header().seq) == genesis->header().hash);
+        BEAST_EXPECT(lh.getLedgerHash(ledger1->header().seq) == ledger1->header().hash);
+        BEAST_EXPECT(lh.getLedgerHash(ledger2->header().seq) == ledger2->header().hash);
+        BEAST_EXPECT(lh.getLedgerHash(ledger3->header().seq) == ledger3->header().hash);
+
+        // Can retrieve by sequence and get correct ledger
+        auto fetched1 = lh.getLedgerBySeq(ledger1->header().seq);
+        if (BEAST_EXPECT(fetched1 != nullptr))
+            BEAST_EXPECT(fetched1->header().hash == ledger1->header().hash);
+
+        auto fetched2 = lh.getLedgerBySeq(ledger2->header().seq);
+        if (BEAST_EXPECT(fetched2 != nullptr))
+            BEAST_EXPECT(fetched2->header().hash == ledger2->header().hash);
+
+        // Clear ledgers prior to ledger2's sequence
+        lh.clearLedgerCachePrior(ledger2->header().seq);
+
+        // Verify old entries are gone from the in-memory by_index map
+        // Note: getLedgerHash checks by_index directly without DB fallback
+        BEAST_EXPECT(lh.getLedgerHash(genesis->header().seq).isZero());
+        BEAST_EXPECT(lh.getLedgerHash(ledger1->header().seq).isZero());
+
+        // Verify newer entries are still present in by_index
+        BEAST_EXPECT(lh.getLedgerHash(ledger2->header().seq) == ledger2->header().hash);
+        BEAST_EXPECT(lh.getLedgerHash(ledger3->header().seq) == ledger3->header().hash);
+
+        // Verify newer entries remain retrievable and consistent
+        // getLedgerBySeq uses by_index first, then falls back to DB if needed
+        auto fetched2After = lh.getLedgerBySeq(ledger2->header().seq);
+        if (BEAST_EXPECT(fetched2After != nullptr))
+            BEAST_EXPECT(fetched2After->header().hash == ledger2->header().hash);
+
+        auto fetched3After = lh.getLedgerBySeq(ledger3->header().seq);
+        if (BEAST_EXPECT(fetched3After != nullptr))
+            BEAST_EXPECT(fetched3After->header().hash == ledger3->header().hash);
+    }
+
     void
     testHandleMismatch()
     {
@@ -170,12 +231,60 @@ public:
 
             BEAST_EXPECT(found);
         }
+
+        // Reverse order: validatedLedger arrives first, then builtLedger
+        // detects the mismatch. Covers the mismatch branch in builtLedger.
+        {
+            bool found = false;
+            Env env{
+                *this,
+                envconfig(),
+                std::make_unique<CheckMessageLogs>("MISMATCH on close time", &found)};
+            LedgerHistory lh{beast::insight::NullCollector::make(), env.app()};
+            auto const genesis = makeLedger({}, env, lh, 0s);
+            auto const ledgerA = makeLedger(genesis, env, lh, 4s);
+            auto const ledgerB = makeLedger(genesis, env, lh, 40s);
+
+            uint256 const dummyTxHash{1};
+            lh.validatedLedger(ledgerB, dummyTxHash);
+            lh.builtLedger(ledgerA, dummyTxHash, {});
+
+            BEAST_EXPECT(found);
+        }
+    }
+
+    void
+    testFixIndex()
+    {
+        testcase("LedgerHistory fixIndex");
+        using namespace jtx;
+        using namespace std::chrono;
+
+        Env env{*this};
+        LedgerHistory lh{beast::insight::NullCollector::make(), env.app()};
+
+        auto const genesis = makeLedger({}, env, lh, 0s);
+        auto const ledger1 = makeLedger(genesis, env, lh, 4s);
+        lh.insert(ledger1, true);
+
+        // Unknown index: returns true, no repair.
+        BEAST_EXPECT(lh.fixIndex(999, ledger1->header().hash));
+
+        // Known index with the same hash: returns true, no repair.
+        BEAST_EXPECT(lh.fixIndex(ledger1->header().seq, ledger1->header().hash));
+
+        // Known index with a different hash: returns false and repairs.
+        uint256 const bogusHash{42};
+        BEAST_EXPECT(!lh.fixIndex(ledger1->header().seq, bogusHash));
+        BEAST_EXPECT(lh.getLedgerHash(ledger1->header().seq) == bogusHash);
     }
 
     void
     run() override
     {
+        testHashIndexInvariant();
         testHandleMismatch();
+        testFixIndex();
     }
 };
 

src/tests/libxrpl/basics/TaggedCache.cpp

@@ -243,4 +243,65 @@ TEST(TaggedCacheTest, tagged_cache)
     }
 }
 
+TEST(TaggedCacheTest, fetch_and_modify)
+{
+    using namespace std::chrono_literals;
+    beast::Journal const journal{TestSink::instance()};
+
+    TestStopwatch clock;
+    clock.set(0);
+
+    struct MutableValue
+    {
+        int counter = 0;
+        std::string name;
+    };
+
+    using Key = LedgerIndex;
+    using MutCache = TaggedCache<Key, MutableValue>;
+
+    MutCache mc("mutable_test", 2, 2s, clock, journal);
+
+    // A. Insert on miss: fetchAndModify creates entry and mutates it.
+    mc.fetchAndModify(5, [](MutableValue& v) {
+        v.counter = 42;
+        v.name = "initial";
+    });
+
+    EXPECT_EQ(mc.getCacheSize(), 1);
+    EXPECT_EQ(mc.getTrackSize(), 1);
+
+    // Verify the mutation persisted.
+    auto p1 = mc.fetch(5);
+    ASSERT_NE(p1, nullptr);
+    EXPECT_EQ(p1->counter, 42);
+    EXPECT_EQ(p1->name, "initial");
+
+    // Keep a second strong pointer to verify in-place modification.
+    auto p2 = mc.fetch(5);
+    ASSERT_NE(p2, nullptr);
+    EXPECT_EQ(p1.get(), p2.get());  // Same object
+
+    // B. Modify existing object on hit.
+    mc.fetchAndModify(5, [](MutableValue& v) {
+        v.counter += 10;
+        v.name = "modified";
+    });
+
+    // No new entry was created.
+    EXPECT_EQ(mc.getCacheSize(), 1);
+    EXPECT_EQ(mc.getTrackSize(), 1);
+
+    // The same object was mutated (both strong pointers see the change).
+    EXPECT_EQ(p1->counter, 52);
+    EXPECT_EQ(p1->name, "modified");
+    EXPECT_EQ(p2->counter, 52);
+
+    // Fresh fetch returns the same object identity.
+    auto p3 = mc.fetch(5);
+    ASSERT_NE(p3, nullptr);
+    EXPECT_EQ(p3.get(), p1.get());
+    EXPECT_EQ(p3->counter, 52);
+}
+
 }  // namespace xrpl

src/xrpld/app/ledger/LedgerHistory.cpp

@@ -13,36 +13,28 @@
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/json/json_value.h>
-#include <xrpl/json/to_string.h>  // IWYU pragma: keep
+#include <xrpl/json/json_writer.h>
+#include <xrpl/ledger/Ledger.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/RippleLedgerHash.h>
 #include <xrpl/protocol/Rules.h>
 #include <xrpl/protocol/TxMeta.h>
-#include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapItem.h>
 
 #include <algorithm>
+#include <cstddef>
 #include <memory>
-#include <mutex>
 #include <optional>
 #include <utility>
 #include <vector>
 
 namespace xrpl {
 
-// FIXME: Need to clean up ledgers by index at some point
-
 LedgerHistory::LedgerHistory(beast::insight::Collector::ptr const& collector, Application& app)
     : app_(app)
     , collector_(collector)
     , mismatchCounter_(collector->makeCounter("ledger.history", "mismatch"))
-    , ledgersByHash_(
-          "LedgerCache",
-          app_.config().getValueFor(SizedItem::LedgerSize),
-          std::chrono::seconds{app_.config().getValueFor(SizedItem::LedgerAge)},
-          stopwatch(),
-          app_.getJournal("TaggedCache"))
     , consensusValidated_(
           "ConsensusValidated",
           64,
@@ -51,6 +43,13 @@ LedgerHistory::LedgerHistory(beast::insight::Collector::ptr const& collector, Ap
           app_.getJournal("TaggedCache"))
     , j_(app.getJournal("LedgerHistory"))
 {
+    auto lockedMaps = ledgerMaps_.lock();
+    lockedMaps->byHash = std::make_unique<LedgerMaps::LedgersByHash>(
+        "LedgerCache",
+        app_.config().getValueFor(SizedItem::LedgerSize),
+        std::chrono::seconds{app_.config().getValueFor(SizedItem::LedgerAge)},
+        stopwatch(),
+        app_.getJournal("TaggedCache"));
 }
 
 bool
@@ -62,11 +61,11 @@ LedgerHistory::insert(std::shared_ptr<Ledger const> const& ledger, bool validate
     XRPL_ASSERT(
         ledger->stateMap().getHash().isNonZero(), "xrpl::LedgerHistory::insert : nonzero hash");
 
-    std::unique_lock const sl(ledgersByHash_.peekMutex());
-
-    bool const alreadyHad = ledgersByHash_.canonicalizeReplaceCache(ledger->header().hash, ledger);
+    auto lockedMaps = ledgerMaps_.lock();
+    bool const alreadyHad =
+        lockedMaps->byHash->canonicalizeReplaceCache(ledger->header().hash, ledger);
     if (validated)
-        ledgersByIndex_[ledger->header().seq] = ledger->header().hash;
+        lockedMaps->byIndex[ledger->header().seq] = ledger->header().hash;
 
     return alreadyHad;
 }
@@ -74,8 +73,8 @@ LedgerHistory::insert(std::shared_ptr<Ledger const> const& ledger, bool validate
 LedgerHash
 LedgerHistory::getLedgerHash(LedgerIndex index)
 {
-    std::unique_lock const sl(ledgersByHash_.peekMutex());
-    if (auto it = ledgersByIndex_.find(index); it != ledgersByIndex_.end())
+    auto lockedMaps = ledgerMaps_.lock();
+    if (auto it = lockedMaps->byIndex.find(index); it != lockedMaps->byIndex.end())
         return it->second;
     return {};
 }
@@ -83,18 +82,16 @@ LedgerHistory::getLedgerHash(LedgerIndex index)
 std::shared_ptr<Ledger const>
 LedgerHistory::getLedgerBySeq(LedgerIndex index)
 {
+    uint256 hash;
     {
-        std::unique_lock sl(ledgersByHash_.peekMutex());
-        auto it = ledgersByIndex_.find(index);
-
-        if (it != ledgersByIndex_.end())
-        {
-            uint256 const hash = it->second;
-            sl.unlock();
-            return getLedgerByHash(hash);
-        }
+        auto lockedMaps = ledgerMaps_.lock();
+        if (auto it = lockedMaps->byIndex.find(index); it != lockedMaps->byIndex.end())
+            hash = it->second;
     }
 
+    if (!hash.isZero())
+        return getLedgerByHash(hash);
+
     Rules const rules{app_.config().features};
     Fees const fees = app_.config().fees.toFees();
     std::shared_ptr<Ledger const> ret = loadByIndex(index, rules, fees, app_);
@@ -107,12 +104,11 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index)
 
     {
         // Add this ledger to the local tracking by index
-        std::unique_lock const sl(ledgersByHash_.peekMutex());
-
+        auto lockedMaps = ledgerMaps_.lock();
         XRPL_ASSERT(
             ret->isImmutable(), "xrpl::LedgerHistory::getLedgerBySeq : immutable result ledger");
-        ledgersByHash_.canonicalizeReplaceClient(ret->header().hash, ret);
-        ledgersByIndex_[ret->header().seq] = ret->header().hash;
+        lockedMaps->byHash->canonicalizeReplaceClient(ret->header().hash, ret);
+        lockedMaps->byIndex[ret->header().seq] = ret->header().hash;
         return (ret->header().seq == index) ? ret : nullptr;
     }
 }
@@ -120,7 +116,11 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index)
 std::shared_ptr<Ledger const>
 LedgerHistory::getLedgerByHash(LedgerHash const& hash)
 {
-    auto ret = ledgersByHash_.fetch(hash);
+    std::shared_ptr<Ledger const> ret;
+    {
+        auto lockedMaps = ledgerMaps_.lock();
+        ret = lockedMaps->byHash->fetch(hash);
+    }
 
     if (ret)
     {
@@ -147,13 +147,29 @@ LedgerHistory::getLedgerByHash(LedgerHash const& hash)
     XRPL_ASSERT(
         ret->header().hash == hash,
         "xrpl::LedgerHistory::getLedgerByHash : loaded ledger hash match");
-    ledgersByHash_.canonicalizeReplaceClient(ret->header().hash, ret);
+    {
+        auto lockedMaps = ledgerMaps_.lock();
+        lockedMaps->byHash->canonicalizeReplaceClient(ret->header().hash, ret);
+    }
     XRPL_ASSERT(
         ret->header().hash == hash, "xrpl::LedgerHistory::getLedgerByHash : result hash match");
 
     return ret;
 }
 
+namespace {
+
+// Captures the sibling's contribution when builtLedger or validatedLedger
+// detects a hash mismatch, so handleMismatch can run outside the cache lock.
+struct MismatchInputs
+{
+    LedgerHash otherHash;
+    std::optional<uint256> otherConsensusHash;
+    json::Value consensus;
+};
+
+}  // namespace
+
 static void
 logOne(ReadView const& ledger, uint256 const& tx, char const* msg, beast::Journal& j)
 {
@@ -190,7 +206,7 @@ logMetadataDifference(
     auto builtMetaData = getMeta(builtLedger, tx);
 
     XRPL_ASSERT(
-        validMetaData || builtMetaData, "xrpl::log_metadata_difference : some metadata present");
+        validMetaData || builtMetaData, "xrpl::logMetadataDifference : some metadata present");
 
     if (validMetaData && builtMetaData)
     {
@@ -267,7 +283,7 @@ logMetadataDifference(
                                 << " Index: " << validMetaData->getIndex() << " Nodes:\n"
                                 << validNodes.getJson(JsonOptions::Values::None);
             }
-            else  // nodes_diff
+            else  // nodesDiff
             {
                 JLOG(j.debug()) << "MISMATCH on TX " << tx << ": Different nodes!";
                 JLOG(j.debug()) << " Built:"
@@ -433,34 +449,44 @@ LedgerHistory::builtLedger(
     LedgerHash const hash = ledger->header().hash;
     XRPL_ASSERT(!hash.isZero(), "xrpl::LedgerHistory::builtLedger : nonzero hash");
 
-    std::unique_lock const sl(consensusValidated_.peekMutex());
+    std::optional<MismatchInputs> mismatch;
 
-    auto entry = std::make_shared<CvEntry>();
-    consensusValidated_.canonicalizeReplaceClient(index, entry);
+    consensusValidated_.fetchAndModify(index, [&](CvEntry& entry) {
+        if (entry.validated && !entry.built)
+        {
+            SOMETIMES(
+                entry.validated.value() != hash,
+                "xrpl::LedgerHistory::builtLedger : validated-first mismatch");
+            if (entry.validated.value() != hash)
+            {
+                JLOG(j_.error()) << "MISMATCH: seq=" << index
+                                 << " validated:" << entry.validated.value() << " then:" << hash;
+                mismatch = MismatchInputs{
+                    .otherHash = entry.validated.value(),
+                    .otherConsensusHash = entry.validatedConsensusHash,
+                    .consensus = consensus};
+            }
+            else
+            {
+                // We validated a ledger and then built it locally
+                JLOG(j_.debug()) << "MATCH: seq=" << index << " late";
+            }
+        }
 
-    if (entry->validated && !entry->built)
+        entry.built.emplace(hash);
+        entry.builtConsensusHash.emplace(consensusHash);
+        entry.consensus.emplace(std::move(consensus));
+    });
+
+    if (mismatch)
     {
-        if (entry->validated.value() != hash)
-        {
-            JLOG(j_.error()) << "MISMATCH: seq=" << index
-                             << " validated:" << entry->validated.value() << " then:" << hash;
-            handleMismatch(
-                hash,
-                entry->validated.value(),
-                consensusHash,
-                entry->validatedConsensusHash,
-                consensus);
-        }
-        else
-        {
-            // We validated a ledger and then built it locally
-            JLOG(j_.debug()) << "MATCH: seq=" << index << " late";
-        }
+        handleMismatch(
+            hash,
+            mismatch->otherHash,
+            consensusHash,
+            mismatch->otherConsensusHash,
+            mismatch->consensus);
     }
-
-    entry->built.emplace(hash);
-    entry->builtConsensusHash.emplace(consensusHash);
-    entry->consensus.emplace(std::move(consensus));
 }
 
 void
@@ -472,49 +498,64 @@ LedgerHistory::validatedLedger(
     LedgerHash const hash = ledger->header().hash;
     XRPL_ASSERT(!hash.isZero(), "xrpl::LedgerHistory::validatedLedger : nonzero hash");
 
-    std::unique_lock const sl(consensusValidated_.peekMutex());
+    std::optional<MismatchInputs> mismatch;
 
-    auto entry = std::make_shared<CvEntry>();
-    consensusValidated_.canonicalizeReplaceClient(index, entry);
+    consensusValidated_.fetchAndModify(index, [&](CvEntry& entry) {
+        if (entry.built && !entry.validated)
+        {
+            XRPL_ASSERT(
+                entry.consensus.has_value(),
+                "xrpl::LedgerHistory::validatedLedger : consensus set when built set");
+            SOMETIMES(
+                entry.built.value() != hash,
+                "xrpl::LedgerHistory::validatedLedger : built-first mismatch");
+            if (entry.built.value() != hash)
+            {
+                JLOG(j_.error()) << "MISMATCH: seq=" << index << " built:" << entry.built.value()
+                                 << " then:" << hash;
+                mismatch = MismatchInputs{
+                    .otherHash = entry.built.value(),
+                    .otherConsensusHash = entry.builtConsensusHash,
+                    .consensus =
+                        entry.consensus.value()};  // NOLINT(bugprone-unchecked-optional-access)
+                                                   // consensus always emplaced with built
+            }
+            else
+            {
+                // We built a ledger locally and then validated it
+                JLOG(j_.debug()) << "MATCH: seq=" << index;
+            }
+        }
 
-    if (entry->built && !entry->validated)
+        entry.validated.emplace(hash);
+        entry.validatedConsensusHash = consensusHash;
+    });
+
+    if (mismatch)
     {
-        if (entry->built.value() != hash)
-        {
-            JLOG(j_.error()) << "MISMATCH: seq=" << index << " built:" << entry->built.value()
-                             << " then:" << hash;
-            handleMismatch(
-                entry->built.value(),
-                hash,
-                entry->builtConsensusHash,
-                consensusHash,
-                entry->consensus.value());  // NOLINT(bugprone-unchecked-optional-access) consensus
-                                            // always emplaced with built
-        }
-        else
-        {
-            // We built a ledger locally and then validated it
-            JLOG(j_.debug()) << "MATCH: seq=" << index;
-        }
+        handleMismatch(
+            mismatch->otherHash,
+            hash,
+            mismatch->otherConsensusHash,
+            consensusHash,
+            mismatch->consensus);
     }
-
-    entry->validated.emplace(hash);
-    entry->validatedConsensusHash = consensusHash;
 }
 
 /**
- * Ensure ledgers_by_hash_ doesn't have the wrong hash for a particular index
+ * Ensure byHash doesn't have the wrong hash for a particular index
  */
 bool
 LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash)
 {
-    std::unique_lock const sl(ledgersByHash_.peekMutex());
-    auto it = ledgersByIndex_.find(ledgerIndex);
-
-    if ((it != ledgersByIndex_.end()) && (it->second != ledgerHash))
+    auto lockedMaps = ledgerMaps_.lock();
+    if (auto it = lockedMaps->byIndex.find(ledgerIndex); it != lockedMaps->byIndex.end())
     {
-        it->second = ledgerHash;
-        return false;
+        if (it->second != ledgerHash)
+        {
+            it->second = ledgerHash;
+            return false;
+        }
     }
     return true;
 }
@@ -522,12 +563,47 @@ LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash)
 void
 LedgerHistory::clearLedgerCachePrior(LedgerIndex seq)
 {
-    for (LedgerHash const it : ledgersByHash_.getKeys())
+    std::size_t hashesCleared = 0;
+    std::size_t indexesCleared = 0;
+    std::size_t cacheSize = 0;
+    std::size_t indexSize = 0;
+
+    std::vector<LedgerHash> const keys = [this] {
+        auto lockedMaps = ledgerMaps_.lock();
+        return lockedMaps->byHash->getKeys();
+    }();
+
+    for (LedgerHash const& it : keys)
     {
         auto const ledger = getLedgerByHash(it);
         if (!ledger || ledger->header().seq < seq)
-            ledgersByHash_.del(it, false);
+        {
+            auto lockedMaps = ledgerMaps_.lock();
+            lockedMaps->byHash->del(it, false);
+            ++hashesCleared;
+        }
     }
+
+    {
+        auto lockedMaps = ledgerMaps_.lock();
+        cacheSize = lockedMaps->byHash->size();
+
+        auto& byIndex = lockedMaps->byIndex;
+        std::size_t const sizeBefore = byIndex.size();
+        byIndex.erase(byIndex.begin(), byIndex.lower_bound(seq));
+        indexSize = byIndex.size();
+        indexesCleared = sizeBefore - indexSize;
+
+        ALWAYS(
+            byIndex.empty() || byIndex.begin()->first >= seq,
+            "xrpl::LedgerHistory::clearLedgerCachePrior : byIndex pruned to seq");
+    }
+
+    JLOG(j_.debug()) << "LedgersByHash: cleared " << hashesCleared << " entries before seq " << seq
+                     << " (total now " << cacheSize << ")";
+
+    JLOG(j_.debug()) << "LedgersByIndex: cleared " << indexesCleared << " index entries before seq "
+                     << seq << " (total now " << indexSize << ")";
 }
 
 }  // namespace xrpl

src/xrpld/app/ledger/LedgerHistory.h

@@ -2,6 +2,8 @@
 
 #include <xrpld/app/main/Application.h>
 
+#include <xrpl/basics/Mutex.hpp>
+#include <xrpl/basics/TaggedCache.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/insight/Collector.h>
 #include <xrpl/beast/insight/Counter.h>
@@ -13,6 +15,7 @@
 
 #include <map>
 #include <memory>
+#include <mutex>
 #include <optional>
 
 namespace xrpl {
@@ -35,13 +38,14 @@ public:
     insert(std::shared_ptr<Ledger const> const& ledger, bool validated);
 
     /**
-     * Get the ledgers_by_hash cache hit rate
+     * Get the byHash cache hit rate
      * @return the hit rate
      */
     float
     getCacheHitRate()
     {
-        return ledgersByHash_.getHitRate();
+        auto lockedMaps = ledgerMaps_.lock();
+        return lockedMaps->byHash->getHitRate();
     }
 
     /**
@@ -70,7 +74,11 @@ public:
     void
     sweep()
     {
-        ledgersByHash_.sweep();
+        auto* const byHash = [this] {
+            auto lockedMaps = ledgerMaps_.lock();
+            return lockedMaps->byHash.get();
+        }();
+        byHash->sweep();
         consensusValidated_.sweep();
     }
 
@@ -124,9 +132,15 @@ private:
     beast::insight::Collector::ptr collector_;
     beast::insight::Counter mismatchCounter_;
 
-    using LedgersByHash = TaggedCache<LedgerHash, Ledger const>;
+    struct LedgerMaps
+    {
+        using LedgersByHash = TaggedCache<LedgerHash, Ledger const>;
 
-    LedgersByHash ledgersByHash_;
+        std::unique_ptr<LedgersByHash> byHash;
+        std::map<LedgerIndex, LedgerHash> byIndex;  // validated ledgers
+    };
+
+    xrpl::Mutex<LedgerMaps, std::recursive_mutex> ledgerMaps_;
 
     // Maps ledger indexes to the corresponding hashes
     // For debug and logging purposes
@@ -146,9 +160,6 @@ private:
     using ConsensusValidated = TaggedCache<LedgerIndex, CvEntry>;
     ConsensusValidated consensusValidated_;
 
-    // Maps ledger indexes to the corresponding hash.
-    std::map<LedgerIndex, LedgerHash> ledgersByIndex_;  // validated ledgers
-
     beast::Journal j_;
 };