fix: Check the transaction map in the parallel ledger walk

`Ledger::walkLedger` returned `walkMapParallel`'s result directly when asked for a
parallel walk, which skipped the transaction map walk and the missing-node logging
that follow. `Application::loadOldLedger` is the only parallel caller, so `--load`,
`--ledger`, `--ledgerfile` and `--replay` never checked a ledger's transaction map.

The parallel result is now held in a local and combined with both maps' missing-node
lists at the end. The early return could not simply be deleted: `walkMapParallel`
reports a worker that cannot read the node store through its return value alone, so
dropping the boolean would lose that answer while `missingNodes1` stays empty.

This is an operational change for anyone using `--load` as a recovery step. A ledger
whose transaction map is missing a node is now refused at startup rather than
accepted. Refusing is the intent: the walk exists to decide whether every node of
both maps is available, and until now it answered for only one of them.

Two gtests in `src/tests/libxrpl/ledger/WalkLedger.cpp` build a ledger whose state
map is complete and whose transaction map holds only its root, then check that both
the parallel and the serial walk report it incomplete. A second case checks that a
ledger with two complete maps passes on both paths. The first fails against the
early return.

include/xrpl/ledger/Ledger.h

@@ -330,6 +330,18 @@ public:
     void
     updateSkipList();
 
+    /**
+     * Check that every node of both of this ledger's maps is available.
+     *
+     * The function walks the state map and then the transaction map, and logs
+     * what it could not read.
+     *
+     * @param j The journal to log missing nodes to.
+     * @param parallel Walk the state map on several threads. The transaction
+     *                 map is always walked on one thread.
+     * @return True when both maps are complete. False when either map is
+     *         missing a node.
+     */
     bool
     walkLedger(beast::Journal j, bool parallel = false) const;
 

include/xrpl/shamap/SHAMap.h

@@ -261,12 +261,17 @@ public:
     lowerBound(uint256 const& id) const;
 
     /**
-     * Visit every node in this SHAMap
+     * Visit every node in this SHAMap.
      *
-     * @param function called with every node visited.
-     * If function returns false, visitNodes exits.
+     * The walk ends at the first node it cannot read, so a caller that needs a
+     * complete walk to be correct must check the return value.
+     *
+     * @param function Called with every node visited. Returning false from it
+     *                 ends the walk, which on its own keeps the result true.
+     * @return True when the walk read every node it reached. False when a node
+     *         could not be read.
      */
-    void
+    bool
     visitNodes(std::function<bool(SHAMapTreeNode&)> const& function) const;
 
     /**
@@ -280,11 +285,12 @@ public:
     visitDifferences(SHAMap const* have, std::function<bool(SHAMapTreeNode const&)> const&) const;
 
     /**
-     * Visit every leaf node in this SHAMap
+     * Visit every leaf node in this SHAMap.
      *
-     * @param function called with every non inner node visited.
+     * @param function Called with every non-inner node visited.
+     * @return What visitNodes reports about the walk.
      */
-    void
+    bool
     visitLeaves(std::function<void(boost::intrusive_ptr<SHAMapItem const> const&)> const&) const;
 
     // comparison/sync functions

src/libxrpl/ledger/Ledger.cpp

@@ -743,18 +743,22 @@ Ledger::walkLedger(beast::Journal j, bool parallel) const
     std::vector<SHAMapMissingNode> missingNodes1;
     std::vector<SHAMapMissingNode> missingNodes2;
 
+    // Returning walkMapParallel's result directly would skip the transaction map walk
+    // below, and dropping it would lose an answer missingNodes1 does not carry: a worker
+    // that cannot read the node store reports that through the return value alone.
+    bool stateComplete = true;
+
     if (stateMap_.getHash().isZero() && !header_.accountHash.isZero() &&
         !stateMap_.fetchRoot(SHAMapHash{header_.accountHash}, nullptr))
     {
         missingNodes1.emplace_back(SHAMapType::STATE, SHAMapHash{header_.accountHash});
     }
+    else if (parallel)
+    {
+        stateComplete = stateMap_.walkMapParallel(missingNodes1, 32);
+    }
     else
     {
-        if (parallel)
-        {
-            return stateMap_.walkMapParallel(missingNodes1, 32);
-        }
-
         stateMap_.walkMap(missingNodes1, 32);
     }
 
@@ -785,7 +789,7 @@ Ledger::walkLedger(beast::Journal j, bool parallel) const
             stream << "First: " << missingNodes2[0].what();
         }
     }
-    return missingNodes1.empty() && missingNodes2.empty();
+    return stateComplete && missingNodes1.empty() && missingNodes2.empty();
 }
 
 bool

src/libxrpl/shamap/SHAMap.cpp

@@ -326,9 +326,6 @@ SHAMap::descend(SHAMapInnerNode& parent, unsigned int branch) const
         return node;
 
     node = fetchNode(parent.getChildHash(branch));
-    if (!node)
-        return {};
-
     node = parent.canonicalizeChild(branch, std::move(node));
     return node;
 }
@@ -340,7 +337,7 @@ SHAMap::descendNoStore(SHAMapInnerNode& parent, unsigned int branch) const
 {
     SHAMapTreeNodePtr ret = parent.getChild(branch);
     if (!ret && backed_)
-        ret = fetchNode(parent.getChildHash(branch));
+        ret = fetchNodeNT(parent.getChildHash(branch));
     return ret;
 }
 

src/libxrpl/shamap/SHAMapDelta.cpp

@@ -13,9 +13,11 @@
 #include <boost/smart_ptr/intrusive_ptr.hpp>
 
 #include <array>
+#include <exception>
 #include <mutex>
 #include <sstream>
 #include <stack>
+#include <string>
 #include <thread>
 #include <utility>
 #include <vector>
@@ -283,21 +285,36 @@ bool
 SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMissing) const
 {
     if (!root_->isInner())  // root_ is only node, and we have it
-        return false;
+        return true;
+
+    // Only the nodes this call records count towards the result, so remember what the
+    // caller already had.
+    auto const initialMissing = missingNodes.size();
 
     using StackEntry = intr_ptr::SharedPtr<SHAMapInnerNode>;
     std::array<SHAMapTreeNodePtr, SHAMapInnerNode::kBranchFactor> topChildren;
     {
+        // This loop runs before the workers start, so it needs no lock.
         auto const& innerRoot = intr_ptr::staticPointerCast<SHAMapInnerNode>(root_);
         for (auto i = 0u; i < SHAMapInnerNode::kBranchFactor; ++i)
         {
-            if (!innerRoot->isEmptyBranch(i))
-                topChildren[i] = descendNoStore(*innerRoot, i);
+            if (innerRoot->isEmptyBranch(i))
+                continue;
+
+            topChildren[i] = descendNoStore(*innerRoot, i);
+            if (!topChildren[i])
+            {
+                // A root child that cannot be read hides its whole subtree. Record it here,
+                // because the loop below skips a null child without visiting it.
+                missingNodes.emplace_back(type_, innerRoot->getChildHash(i));
+                if (--maxMissing <= 0)
+                    return false;
+            }
         }
     }
     std::vector<std::thread> workers;
     workers.reserve(SHAMapInnerNode::kBranchFactor);
-    std::vector<SHAMapMissingNode> exceptions;
+    std::vector<std::string> exceptions;
     exceptions.reserve(SHAMapInnerNode::kBranchFactor);
 
     std::array<std::stack<StackEntry, std::vector<StackEntry>>, SHAMapInnerNode::kBranchFactor>
@@ -346,6 +363,14 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
                             else
                             {
                                 std::scoped_lock const l{m};
+
+                                // Another worker may have spent the budget while this one
+                                // was walking, so test it before adding to the list and not
+                                // only after. Testing only after lets every worker still
+                                // running add one more node than the caller asked for.
+                                if (maxMissing <= 0)
+                                    return;
+
                                 missingNodes.emplace_back(type_, node->getChildHash(i));
                                 if (--maxMissing <= 0)
                                     return;
@@ -353,10 +378,14 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
                         }
                     }
                 }
-                catch (SHAMapMissingNode const& e)
+                catch (std::exception const& e)
                 {
+                    // LCOV_EXCL_START
+                    // A worker must not let an exception leave its thread, so record it
+                    // and let the join below report it.
                     std::scoped_lock const l(m);
-                    exceptions.push_back(e);
+                    exceptions.emplace_back(e.what());
+                    // LCOV_EXCL_STOP
                 }
             },
             std::move(nodeStacks[rootChildIndex]));
@@ -366,14 +395,29 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
         worker.join();
 
     std::scoped_lock const l(m);
-    if (exceptions.empty())
-        return true;
-    std::stringstream ss;
-    ss << "Exception(s) in ledger load: ";
-    for (auto const& e : exceptions)
-        ss << e.what() << ", ";
-    JLOG(journal_.error()) << ss.str();
-    return false;
+    if (!exceptions.empty())
+    {
+        // LCOV_EXCL_START
+        std::stringstream ss;
+        ss << "Exception(s) in ledger load: ";
+        for (auto const& e : exceptions)
+            ss << e << ", ";
+        JLOG(journal_.error()) << ss.str();
+        return false;
+        // LCOV_EXCL_STOP
+    }
+
+    // A node the workers could not read is recorded in `missingNodes` rather than thrown,
+    // so the result has to consult it too.
+    auto const found = missingNodes.size() - initialMissing;
+    if (found != 0)
+    {
+        JLOG(journal_.error()) << "Missing node(s) in ledger load: " << found
+                               << ", first: " << missingNodes[initialMissing].what();
+        return false;
+    }
+
+    return true;
 }
 
 }  // namespace xrpl

src/libxrpl/shamap/SHAMapSync.cpp

@@ -31,28 +31,36 @@
 
 namespace xrpl {
 
-void
+bool
 SHAMap::visitLeaves(
     std::function<void(boost::intrusive_ptr<SHAMapItem const> const& item)> const& leafFunction)
     const
 {
-    visitNodes([&leafFunction](SHAMapTreeNode& node) {
+    return visitNodes([&leafFunction](SHAMapTreeNode& node) {
         if (!node.isInner())
             leafFunction(safeDowncast<SHAMapLeafNode&>(node).peekItem());
         return true;
     });
 }
 
-void
+bool
 SHAMap::visitNodes(std::function<bool(SHAMapTreeNode&)> const& function) const
 {
     if (!root_)
-        return;
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE("xrpl::SHAMap::visitNodes : non-null root");
+        return true;
+        // LCOV_EXCL_STOP
+    }
 
-    function(*root_);
+    // The root is readable by definition, so a stop here still reports a complete walk.
+    if (!function(*root_))
+        return true;
 
+    // A map whose root is a leaf holds one node, and this walk just visited it.
     if (!root_->isInner())
-        return;
+        return true;
 
     using StackEntry = std::pair<unsigned int, intr_ptr::SharedPtr<SHAMapInnerNode>>;
     std::stack<StackEntry, std::vector<StackEntry>> stack;
@@ -67,8 +75,17 @@ SHAMap::visitNodes(std::function<bool(SHAMapTreeNode&)> const& function) const
             if (!node->isEmptyBranch(pos))
             {
                 SHAMapTreeNodePtr const child = descendNoStore(*node, pos);
+                if (!child)
+                {
+                    // No caller can use a partial walk, so end it here rather than pay
+                    // for the rest of the map. The callers that act on the result log
+                    // it at a level an operator reads, so keep this one at debug.
+                    JLOG(journal_.debug())
+                        << "visitNodes: unreadable child node " << node->getChildHash(pos);
+                    return false;
+                }
                 if (!function(*child))
-                    return;
+                    return true;
 
                 if (child->isLeaf())
                 {
@@ -103,6 +120,8 @@ SHAMap::visitNodes(std::function<bool(SHAMapTreeNode&)> const& function) const
         std::tie(pos, node) = stack.top();
         stack.pop();
     }
+
+    return true;
 }
 
 void

src/test/app/LedgerReplay_test.cpp

@@ -28,17 +28,28 @@
 
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/random.h>
 #include <xrpl/beast/net/IPAddress.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/xor_shift_engine.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/Ledger.h>
+#include <xrpl/nodestore/Database.h>
+#include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/LedgerHeader.h>
 #include <xrpl/protocol/RippleLedgerHash.h>
+#include <xrpl/protocol/Serializer.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/server/Handoff.h>
+#include <xrpl/shamap/Family.h>
+#include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapItem.h>
+#include <xrpl/shamap/SHAMapMissingNode.h>  // SHAMapType
+#include <xrpl/shamap/SHAMapTreeNode.h>
 
 #include <boost/asio/ip/address.hpp>
 
@@ -64,6 +75,7 @@
 #include <string>
 #include <thread>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 namespace xrpl::test {
@@ -1090,6 +1102,111 @@ struct LedgerReplayer_test : public beast::unit_test::Suite
         }
     }
 
+    /**
+     * A ledger whose transaction map cannot be read in full must draw an error reply
+     * rather than a short transaction list.
+     *
+     * The two maps are built in memory and only part of them is written to the store: the
+     * whole state map, so the ledger loads, and the transaction map's root alone. The
+     * nodes below that root are never written, so the ledger the handler reads back from
+     * the store cannot reach them.
+     */
+    void
+    testReplayDeltaIncompleteTxMap()
+    {
+        static constexpr auto kItems = 200;
+        static constexpr auto kSeed = 0xdeadbeefU;
+
+        // Far enough ahead of the server's own history that the missing-node handler finds
+        // no hash to acquire and returns at once.
+        static constexpr auto kSeqAhead = 1000;
+
+        testcase("ReplayDelta incomplete tx map");
+
+        LedgerServer server(*this, {.initLedgers = 1});
+        auto& family = server.app.getNodeFamily();
+
+        // The items only have to be well formed, because the walk stops before the handler
+        // reads any of them.
+        beast::xor_shift_engine engine{kSeed};
+        auto const makeItem = [&engine]() {
+            static constexpr auto kWordsPerItem = 3;
+
+            Serializer s;
+            for (auto word = 0; word < kWordsPerItem; ++word)
+                s.add32(randInt<std::uint32_t>(engine));
+            return makeShamapitem(s.getSHA512Half(), s.slice());
+        };
+
+        SHAMap stateMap{SHAMapType::STATE, family};
+        SHAMap txMap{SHAMapType::TRANSACTION, family};
+        for (auto i = 0; i < kItems; ++i)
+        {
+            stateMap.addItem(SHAMapNodeType::TnAccountState, makeItem());
+            txMap.addItem(SHAMapNodeType::TnTransactionNm, makeItem());
+        }
+        stateMap.setImmutable();
+        txMap.setImmutable();
+
+        // Read both hashes before storing anything. getHash() is what computes the node
+        // hashes, through unshare(), so storing first would write every node under a stale
+        // hash and nothing would be findable afterwards.
+        LedgerHeader header = server.ledgerMaster.getClosedLedger()->header();
+        header.seq += kSeqAhead;
+        header.accountHash = stateMap.getHash().asUInt256();
+        header.txHash = txMap.getHash().asUInt256();
+        header.hash = calculateLedgerHash(header);
+
+        // Write a map's nodes into the store. visitNodes reports the root first, so
+        // rootOnly lets that one call through and stops at the next.
+        auto const storeMap = [&family](SHAMap const& map, bool rootOnly) {
+            auto stored = 0;
+            map.visitNodes([&family, rootOnly, &stored](SHAMapTreeNode& node) {
+                if (rootOnly && stored > 0)
+                    return false;
+
+                Serializer s;
+                node.serializeWithPrefix(s);
+                family.db().store(
+                    NodeObjectType::AccountNode,
+                    std::move(s.modData()),
+                    node.getHash().asUInt256(),
+                    0);
+                ++stored;
+                return true;
+            });
+            return stored;
+        };
+
+        BEAST_EXPECT(storeMap(stateMap, false) > 1);
+        BEAST_EXPECT(storeMap(txMap, true) == 1);
+
+        bool loaded = false;
+        auto const holed = std::make_shared<Ledger const>(
+            header,
+            loaded,
+            false,
+            server.ledgerMaster.getClosedLedger()->rules(),
+            server.ledgerMaster.getClosedLedger()->fees(),
+            family,
+            server.app.getLogs().journal("Ledger"));
+
+        // Both roots are readable, so the ledger loads and the handler accepts it.
+        BEAST_EXPECT(loaded);
+        BEAST_EXPECT(holed->isImmutable());
+        server.ledgerMaster.storeLedger(holed);
+
+        auto request = std::make_shared<protocol::TMReplayDeltaRequest>();
+        request->set_ledgerhash(header.hash.data(), header.hash.size());
+        auto const reply = server.msgHandler.processReplayDeltaRequest(request);
+
+        // The reply names the missing nodes and carries no partial payload.
+        BEAST_EXPECT(reply.has_error());
+        BEAST_EXPECT(reply.error() == protocol::TMReplyError::reNO_NODE);
+        BEAST_EXPECT(reply.transaction_size() == 0);
+        BEAST_EXPECT(!reply.has_ledgerheader());
+    }
+
     void
     testTaskParameter()
     {
@@ -1492,6 +1609,7 @@ struct LedgerReplayer_test : public beast::unit_test::Suite
     {
         testProofPath();
         testReplayDelta();
+        testReplayDeltaIncompleteTxMap();
         testTruncatedHeader();
         testTaskParameter();
         testConfig();

src/tests/libxrpl/ledger/WalkLedger.cpp

@@ -0,0 +1,181 @@
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/random.h>
+#include <xrpl/beast/hash/uhash.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/xor_shift_engine.h>
+#include <xrpl/ledger/Ledger.h>
+#include <xrpl/nodestore/NodeObject.h>
+#include <xrpl/protocol/Fees.h>
+#include <xrpl/protocol/LedgerHeader.h>
+#include <xrpl/protocol/Rules.h>
+#include <xrpl/protocol/Serializer.h>
+#include <xrpl/shamap/SHAMap.h>
+#include <xrpl/shamap/SHAMapItem.h>
+#include <xrpl/shamap/SHAMapMissingNode.h>  // SHAMapType
+#include <xrpl/shamap/SHAMapTreeNode.h>
+
+#include <boost/smart_ptr/intrusive_ptr.hpp>
+
+#include <gtest/gtest.h>
+#include <helpers/TestFamily.h>
+#include <helpers/TestSink.h>
+
+#include <cstdint>
+#include <unordered_set>
+#include <utility>
+
+namespace xrpl::tests {
+
+namespace {
+
+// Fixed seed, so a failure reproduces when the test runs on its own.
+constexpr std::uint32_t kSeed = 0x5eed1234U;
+
+/**
+ * Build a SHAMapItem holding random data.
+ *
+ * @param engine The random engine to draw the data from.
+ * @return The new item.
+ */
+boost::intrusive_ptr<SHAMapItem>
+makeRandomItem(beast::xor_shift_engine& engine)
+{
+    static constexpr auto kWordsPerItem = 3;
+
+    Serializer s;
+    for (auto word = 0; word < kWordsPerItem; ++word)
+        s.add32(randInt<std::uint32_t>(engine));
+    return makeShamapitem(s.getSHA512Half(), s.slice());
+}
+
+/**
+ * Write a map's nodes into a family's node store.
+ *
+ * @param map The map to read. It must be complete.
+ * @param family The family whose store receives the nodes.
+ * @param rootOnly Write only the root node, leaving every node below it absent.
+ */
+void
+storeMap(SHAMap const& map, test::TestFamily& family, bool rootOnly)
+{
+    // visitNodes reports the root first, so rootOnly lets that first call through and
+    // stops at the one after it.
+    int stored = 0;
+    map.visitNodes([&family, rootOnly, &stored](SHAMapTreeNode& node) {
+        if (rootOnly && stored > 0)
+            return false;
+
+        Serializer s;
+        node.serializeWithPrefix(s);
+        family.db().store(
+            NodeObjectType::AccountNode, std::move(s.modData()), node.getHash().asUInt256(), 0);
+        ++stored;
+        return true;
+    });
+}
+
+}  // namespace
+
+// walkLedger must not report a ledger whose transaction map is missing a node as
+// complete. The parallel path used to return the state map's result directly and never
+// walk the transaction map at all, so Application::loadOldLedger accepted such a ledger.
+// Both paths must agree, so this checks the serial one on the same ledger.
+TEST(WalkLedger, parallel_walk_checks_the_transaction_map)
+{
+    static constexpr auto kItems = 200;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    test::TestFamily source{j};
+    test::TestFamily dest{j};
+
+    // Two complete maps, built in the source family.
+    SHAMap stateMap{SHAMapType::STATE, source};
+    SHAMap txMap{SHAMapType::TRANSACTION, source};
+    for (auto i = 0; i < kItems; ++i)
+    {
+        stateMap.addItem(SHAMapNodeType::TnAccountState, makeRandomItem(engine));
+        txMap.addItem(SHAMapNodeType::TnTransactionNm, makeRandomItem(engine));
+    }
+    stateMap.setImmutable();
+    txMap.setImmutable();
+
+    // Read both hashes before storing anything. getHash() is what computes the node
+    // hashes, through unshare(), so storing first would write every node under a stale
+    // hash and nothing would be findable afterwards.
+    LedgerHeader header;
+    header.seq = 1;
+    header.accountHash = stateMap.getHash().asUInt256();
+    header.txHash = txMap.getHash().asUInt256();
+    header.hash = calculateLedgerHash(header);
+
+    // The destination family gets the whole state map, so the state walk succeeds, and
+    // only the transaction map's root, so every node below it is unreadable. The first
+    // unreadable node makes SHAMap::finishFetch call TestFamily::missingNodeAcquireBySeq,
+    // which throws, so expect one "finishFetch exception" warning in the log below.
+    storeMap(stateMap, dest, false);
+    storeMap(txMap, dest, true);
+
+    bool loaded = false;
+    Ledger const ledger{
+        header,
+        loaded,
+        false,
+        Rules{std::unordered_set<uint256, beast::Uhash<>>{}},
+        Fees{},
+        dest,
+        j};
+
+    // Both roots are readable, so the ledger loads.
+    ASSERT_TRUE(loaded);
+
+    EXPECT_FALSE(ledger.walkLedger(j, true));
+    EXPECT_FALSE(ledger.walkLedger(j, false));
+}
+
+// A ledger whose maps are both complete must pass, on the parallel path too.
+TEST(WalkLedger, parallel_walk_accepts_a_complete_ledger)
+{
+    static constexpr auto kItems = 200;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    test::TestFamily source{j};
+    test::TestFamily dest{j};
+
+    SHAMap stateMap{SHAMapType::STATE, source};
+    SHAMap txMap{SHAMapType::TRANSACTION, source};
+    for (auto i = 0; i < kItems; ++i)
+    {
+        stateMap.addItem(SHAMapNodeType::TnAccountState, makeRandomItem(engine));
+        txMap.addItem(SHAMapNodeType::TnTransactionNm, makeRandomItem(engine));
+    }
+    stateMap.setImmutable();
+    txMap.setImmutable();
+
+    // getHash() before storeMap: see the note in the test above.
+    LedgerHeader header;
+    header.seq = 1;
+    header.accountHash = stateMap.getHash().asUInt256();
+    header.txHash = txMap.getHash().asUInt256();
+    header.hash = calculateLedgerHash(header);
+
+    storeMap(stateMap, dest, false);
+    storeMap(txMap, dest, false);
+
+    bool loaded = false;
+    Ledger const ledger{
+        header,
+        loaded,
+        false,
+        Rules{std::unordered_set<uint256, beast::Uhash<>>{}},
+        Fees{},
+        dest,
+        j};
+    ASSERT_TRUE(loaded);
+
+    EXPECT_TRUE(ledger.walkLedger(j, true));
+    EXPECT_TRUE(ledger.walkLedger(j, false));
+}
+
+}  // namespace xrpl::tests

src/tests/libxrpl/shamap/SHAMapMissingNode.cpp

@@ -0,0 +1,501 @@
+#include <xrpl/shamap/SHAMapMissingNode.h>
+
+#include <xrpl/basics/Blob.h>
+#include <xrpl/basics/SHAMapHash.h>
+#include <xrpl/basics/Slice.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/xor_shift_engine.h>
+#include <xrpl/protocol/Serializer.h>
+#include <xrpl/shamap/SHAMap.h>
+#include <xrpl/shamap/SHAMapAddNode.h>
+#include <xrpl/shamap/SHAMapNodeID.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
+
+#include <gtest/gtest.h>
+#include <helpers/TestSink.h>
+#include <shamap/common.h>
+
+#include <cstddef>
+#include <cstdint>
+#include <utility>
+#include <vector>
+
+namespace xrpl::tests {
+
+namespace {
+
+// Fixed seed, so a failure reproduces when the test runs on its own.
+constexpr std::uint32_t kSeed = 0xdeadbeefU;
+
+// Largest number of nodes getMissingNodes may report per round.
+constexpr int kMaxNodesPerRequest = 2048;
+
+/**
+ * Copy part of a SHAMap into another SHAMap.
+ *
+ * The function answers at most nodeLimit of the requests that
+ * dest.getMissingNodes() makes. The remaining subtrees stay absent from the
+ * destination family's database, which is the state this test needs.
+ *
+ * @param source The complete map to copy from.
+ * @param dest The map to copy into. It must be in the synching state.
+ * @param nodeLimit The largest number of missing-node requests to answer.
+ * @return Nothing. The function reports a fatal gtest failure on error, so
+ *         wrap the call in ASSERT_NO_FATAL_FAILURE.
+ */
+void
+copyPartialMap(SHAMap const& source, SHAMap& dest, std::size_t nodeLimit)
+{
+    std::vector<SHAMapNodeData> rootData;
+    if (!source.getNodeFat(SHAMapNodeID{}, rootData, false, 0))
+        FAIL() << "Could not get root node";
+
+    auto rootNode = SHAMapTreeNode::makeFromWire(makeSlice(rootData[0].data));
+    if (!rootNode)
+        FAIL() << "Could not deserialize root node";
+    if (!dest.addRootNode(source.getHash(), std::move(rootNode), nullptr).isGood())
+        FAIL() << "Could not add root node";
+
+    std::size_t answered = 0;
+    while (answered < nodeLimit)
+    {
+        auto const missing = dest.getMissingNodes(kMaxNodesPerRequest, nullptr);
+        if (missing.empty())
+            break;
+
+        for (auto const& request : missing)
+        {
+            if (answered >= nodeLimit)
+                return;
+            ++answered;
+
+            std::vector<SHAMapNodeData> nodeData;
+            if (!source.getNodeFat(request.first, nodeData, false, 0))
+                continue;
+
+            for (auto const& entry : nodeData)
+            {
+                auto node = SHAMapTreeNode::makeFromWire(makeSlice(entry.data));
+                if (!node)
+                    FAIL() << "Could not deserialize node " << entry.nodeID;
+                if (!dest.addKnownNode(entry.nodeID, std::move(node), nullptr).isGood())
+                    FAIL() << "Could not add known node " << entry.nodeID;
+            }
+        }
+    }
+}
+
+/**
+ * Lift a leaf out of a one-item map in the wire format a peer would send it in.
+ *
+ * A locally built map always has an inner node for its root, so this is how a
+ * test reaches the one-node shape that addRootNode accepts.
+ *
+ * @param map A map holding exactly one item.
+ * @param wire Set to the leaf's wire form.
+ * @param hash Set to the leaf's hash.
+ * @return Nothing. The function reports a fatal gtest failure on error, so
+ *         wrap the call in ASSERT_NO_FATAL_FAILURE.
+ */
+void
+extractSoleLeaf(SHAMap const& map, Blob& wire, SHAMapHash& hash)
+{
+    auto leaves = 0;
+    map.visitNodes([&](SHAMapTreeNode& node) {
+        if (!node.isInner())
+        {
+            ++leaves;
+            Serializer s;
+            node.serializeForWire(s);
+            wire = s.modData();
+            hash = node.getHash();
+        }
+        return true;
+    });
+    if (leaves != 1)
+        FAIL() << "Expected one leaf, found " << leaves;
+}
+
+struct NodeCounts
+{
+    bool complete;
+    int inner;
+    int leaves;
+};
+
+/**
+ * Count the inner and leaf nodes that visitNodes reaches.
+ *
+ * @param map The map to walk.
+ * @return The counts, plus what visitNodes reported about the walk.
+ */
+NodeCounts
+countNodes(SHAMap const& map)
+{
+    int inner = 0;
+    int leaves = 0;
+    bool const complete = map.visitNodes([&inner, &leaves](SHAMapTreeNode& node) {
+        if (node.isInner())
+        {
+            ++inner;
+        }
+        else
+        {
+            ++leaves;
+        }
+        return true;
+    });
+    return {.complete = complete, .inner = inner, .leaves = leaves};
+}
+
+}  // namespace
+
+// visitLeaves on a map with unreadable child nodes must not throw. It must end the walk
+// at the first one and report that the walk was incomplete.
+TEST(SHAMapMissingNode, visit_leaves_stops_at_an_unreadable_node)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kNodesToCopy = 3uz;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    // The source map is complete, so it holds one leaf per item added.
+    auto beforeCount = 0;
+    EXPECT_TRUE(source.visitLeaves([&beforeCount](auto const&) { ++beforeCount; }));
+    EXPECT_EQ(beforeCount, kItems);
+
+    // Copy only a few nodes, so most subtrees stay absent from destFamily's
+    // database. This is the state a node reaches when a child is evicted after
+    // a database rotation, or when a sync is still incomplete.
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, kNodesToCopy));
+
+    // The walk must return, must report that it was incomplete, and must reach fewer
+    // leaves than the source holds.
+    auto afterCount = 0;
+    auto complete = true;
+    EXPECT_NO_THROW(complete = dest.visitLeaves([&afterCount](auto const&) { ++afterCount; }));
+    EXPECT_FALSE(complete);
+    EXPECT_LT(afterCount, beforeCount);
+}
+
+// walkMap must report the nodes it cannot read rather than throwing, so that
+// Ledger::walkLedger can return false and let its caller re-acquire the ledger.
+TEST(SHAMapMissingNode, walk_map_reports_missing_nodes)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kNodesToCopy = 3uz;
+    static constexpr auto kMaxMissing = 32;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, kNodesToCopy));
+
+    std::vector<SHAMapMissingNode> missing;
+    EXPECT_NO_THROW(dest.walkMap(missing, kMaxMissing));
+    EXPECT_FALSE(missing.empty());
+
+    // A complete map reports nothing missing.
+    std::vector<SHAMapMissingNode> none;
+    EXPECT_NO_THROW(source.walkMap(none, kMaxMissing));
+    EXPECT_TRUE(none.empty());
+}
+
+// walkMapParallel must report a partial map as incomplete. Its workers record an
+// unreadable child instead of throwing, so the result has to consult that list and not
+// only the exceptions the workers caught.
+TEST(SHAMapMissingNode, walk_map_parallel_reports_missing_nodes)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kNodesToCopy = 3uz;
+    static constexpr auto kMaxMissing = 32;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, kNodesToCopy));
+
+    std::vector<SHAMapMissingNode> missing;
+    EXPECT_FALSE(dest.walkMapParallel(missing, kMaxMissing));
+    EXPECT_FALSE(missing.empty());
+
+    // A complete map reports nothing missing.
+    std::vector<SHAMapMissingNode> none;
+    EXPECT_TRUE(source.walkMapParallel(none, kMaxMissing));
+    EXPECT_TRUE(none.empty());
+}
+
+// An empty branch of the root holds nothing, so the parallel walk must skip it rather
+// than report it missing. Three items cannot fill sixteen branches, whereas the larger
+// maps in this file leave none of them empty.
+TEST(SHAMapMissingNode, walk_map_parallel_skips_empty_root_branches)
+{
+    static constexpr auto kItems = 3;
+    static constexpr auto kMaxMissing = 32;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily family{j};
+
+    SHAMap map{SHAMapType::FREE, family};
+    for (auto i = 0; i < kItems; ++i)
+        map.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    map.setImmutable();
+
+    // The root has to be inner, or the walk returns before it reads a single branch.
+    auto const [complete, inner, leaves] = countNodes(map);
+    EXPECT_TRUE(complete);
+    EXPECT_EQ(leaves, kItems);
+    EXPECT_GT(inner, 0);
+
+    std::vector<SHAMapMissingNode> missing;
+    EXPECT_TRUE(map.walkMapParallel(missing, kMaxMissing));
+    EXPECT_TRUE(missing.empty());
+}
+
+// walkMapParallel reads the root's children before it starts any worker, and skips a null
+// one. That pass has to record the miss itself, or a map holding nothing but its root
+// reports as complete.
+TEST(SHAMapMissingNode, walk_map_parallel_reports_missing_root_children)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kMaxMissing = 32;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    // Copy the root and nothing else, so every one of its children is unreadable and no
+    // worker ever runs.
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, 0));
+
+    std::vector<SHAMapMissingNode> missing;
+    EXPECT_FALSE(dest.walkMapParallel(missing, kMaxMissing));
+    EXPECT_FALSE(missing.empty());
+}
+
+// Stopping the walk through the visitor is not the same thing as an unreadable node, so it
+// must not make the result false. A stop at the root counts too; the root's answer used to
+// be discarded.
+TEST(SHAMapMissingNode, early_stop_on_a_complete_map_reports_complete)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kVisitsBeforeStop = 3;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily family{j};
+
+    SHAMap map{SHAMapType::FREE, family};
+    for (auto i = 0; i < kItems; ++i)
+        map.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    map.setImmutable();
+
+    auto visits = 0;
+    EXPECT_TRUE(map.visitNodes([&visits](SHAMapTreeNode&) {
+        ++visits;
+        return visits < kVisitsBeforeStop;
+    }));
+    EXPECT_EQ(visits, kVisitsBeforeStop);
+
+    // Stopping at the root leaves the rest of the map unvisited.
+    auto rootVisits = 0;
+    EXPECT_TRUE(map.visitNodes([&rootVisits](SHAMapTreeNode&) {
+        ++rootVisits;
+        return false;
+    }));
+    EXPECT_EQ(rootVisits, 1);
+}
+
+// A map holding nothing but its root cannot read any branch, so the walk visits the root
+// alone and reports itself incomplete.
+TEST(SHAMapMissingNode, a_root_only_map_reports_incomplete)
+{
+    static constexpr auto kItems = 200;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, 0));
+
+    auto visits = 0;
+    EXPECT_FALSE(dest.visitNodes([&visits](SHAMapTreeNode&) {
+        ++visits;
+        return true;
+    }));
+    EXPECT_EQ(visits, 1);
+}
+
+// visitNodes on a complete map must reach every node.
+TEST(SHAMapMissingNode, visit_nodes_complete_map)
+{
+    static constexpr auto kItems = 50;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily family{j};
+
+    SHAMap map{SHAMapType::FREE, family};
+    for (auto i = 0; i < kItems; ++i)
+        map.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    map.setImmutable();
+
+    // The walk must report itself complete, and the map must hold one leaf per item
+    // added, plus the inner nodes that hold those leaves.
+    auto const [complete, inner, leaves] = countNodes(map);
+    EXPECT_TRUE(complete);
+    EXPECT_EQ(leaves, kItems);
+    EXPECT_GT(inner, 0);
+}
+
+// The budget caps what a walk records. Once it is spent the walk stops, and the parallel
+// walk spends it on the root's children before it starts a worker.
+TEST(SHAMapMissingNode, a_spent_missing_node_budget_stops_the_walk)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kMaxMissing = 1;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    // Copy the root and nothing else, so more than one child is unreadable and the
+    // budget of one runs out.
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, 0));
+
+    std::vector<SHAMapMissingNode> serial;
+    EXPECT_NO_THROW(dest.walkMap(serial, kMaxMissing));
+    EXPECT_EQ(serial.size(), 1u);
+
+    std::vector<SHAMapMissingNode> parallel;
+    EXPECT_FALSE(dest.walkMapParallel(parallel, kMaxMissing));
+    EXPECT_EQ(parallel.size(), 1u);
+}
+
+// A map whose root is a leaf holds one node and has it, so every walk reports it
+// complete. addRootNode is the only way into that shape.
+TEST(SHAMapMissingNode, a_leaf_rooted_map_reports_complete)
+{
+    static constexpr auto kMaxMissing = 32;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    Blob wire;
+    SHAMapHash hash;
+    ASSERT_NO_FATAL_FAILURE(extractSoleLeaf(source, wire, hash));
+
+    SHAMap dest{SHAMapType::FREE, hash.asUInt256(), destFamily};
+    dest.setSynching();
+    auto leaf = SHAMapTreeNode::makeFromWire(makeSlice(wire));
+    ASSERT_TRUE(leaf);
+    ASSERT_TRUE(dest.addRootNode(hash, std::move(leaf), nullptr).isGood());
+
+    auto visits = 0;
+    EXPECT_TRUE(dest.visitNodes([&visits](SHAMapTreeNode&) {
+        ++visits;
+        return true;
+    }));
+    EXPECT_EQ(visits, 1);
+
+    std::vector<SHAMapMissingNode> serial;
+    EXPECT_NO_THROW(dest.walkMap(serial, kMaxMissing));
+    EXPECT_TRUE(serial.empty());
+
+    std::vector<SHAMapMissingNode> parallel;
+    EXPECT_TRUE(dest.walkMapParallel(parallel, kMaxMissing));
+    EXPECT_TRUE(parallel.empty());
+}
+
+// The missing-node budget belongs to the whole walk, not to each worker. Every worker that
+// is still running shares one counter, so it has to be tested before a node is added to the
+// list. Testing it only afterwards let a 16-worker walk record 17 nodes against a cap of 2.
+TEST(SHAMapMissingNode, a_missing_node_budget_is_shared_by_every_worker)
+{
+    static constexpr auto kItems = 200;
+    static constexpr auto kMaxMissing = 2;
+
+    // Enough to make all sixteen of the root's children readable inner nodes, so sixteen
+    // workers start and each one finds unreadable children below its own.
+    static constexpr auto kNodesToCopy = 16uz;
+
+    beast::Journal const j{TestSink::instance()};
+    beast::xor_shift_engine engine{kSeed};
+    TestNodeFamily sourceFamily{j};
+    TestNodeFamily destFamily{j};
+
+    SHAMap source{SHAMapType::FREE, sourceFamily};
+    for (auto i = 0; i < kItems; ++i)
+        source.addItem(SHAMapNodeType::TnAccountState, makeRandomAccountStateItem(engine));
+    source.setImmutable();
+
+    SHAMap dest{SHAMapType::FREE, source.getHash().asUInt256(), destFamily};
+    dest.setSynching();
+    ASSERT_NO_FATAL_FAILURE(copyPartialMap(source, dest, kNodesToCopy));
+
+    std::vector<SHAMapMissingNode> missing;
+    EXPECT_FALSE(dest.walkMapParallel(missing, kMaxMissing));
+    EXPECT_FALSE(missing.empty());
+    EXPECT_LE(missing.size(), static_cast<std::size_t>(kMaxMissing));
+}
+
+}  // namespace xrpl::tests

src/tests/libxrpl/shamap/SHAMapSync.cpp

@@ -4,7 +4,6 @@
 #include <xrpl/basics/random.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/xor_shift_engine.h>
-#include <xrpl/protocol/Serializer.h>
 #include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapItem.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
@@ -18,7 +17,6 @@
 
 #include <chrono>
 #include <cstddef>
-#include <cstdint>
 #include <list>
 #include <utility>
 #include <vector>
@@ -34,13 +32,7 @@ protected:
     boost::intrusive_ptr<SHAMapItem>
     makeRandomAS()
     {
-        static constexpr auto kWordsPerState = 3uz;
-
-        Serializer s;
-
-        for (auto word = 0uz; word < kWordsPerState; ++word)
-            s.add32(randInt<std::uint32_t>(eng_));
-        return makeShamapitem(s.getSHA512Half(), s.slice());
+        return makeRandomAccountStateItem(eng_);
     }
 
     bool

src/tests/libxrpl/shamap/common.h

@@ -4,16 +4,21 @@
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/chrono.h>
 #include <xrpl/basics/contract.h>
+#include <xrpl/basics/random.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/config/BasicConfig.h>
 #include <xrpl/config/Constants.h>
 #include <xrpl/nodestore/Database.h>
 #include <xrpl/nodestore/DummyScheduler.h>
 #include <xrpl/nodestore/Manager.h>
+#include <xrpl/protocol/Serializer.h>
 #include <xrpl/shamap/Family.h>
 #include <xrpl/shamap/FullBelowCache.h>
+#include <xrpl/shamap/SHAMapItem.h>
 #include <xrpl/shamap/TreeNodeCache.h>
 
+#include <boost/smart_ptr/intrusive_ptr.hpp>
+
 #include <chrono>
 #include <cstdint>
 #include <memory>
@@ -21,6 +26,25 @@
 
 namespace xrpl::tests {
 
+/**
+ * Build a SHAMapItem holding random account-state data.
+ *
+ * @param engine The random engine to draw the data from. Pass an engine with a
+ *               fixed seed to make a test reproducible on its own.
+ * @return The new item.
+ */
+template <class Engine>
+boost::intrusive_ptr<SHAMapItem>
+makeRandomAccountStateItem(Engine& engine)
+{
+    static constexpr auto kWordsPerState = 3uz;
+
+    Serializer s;
+    for (auto word = 0uz; word < kWordsPerState; ++word)
+        s.add32(randInt<std::uint32_t>(engine));
+    return makeShamapitem(s.getSHA512Half(), s.slice());
+}
+
 class TestNodeFamily : public Family
 {
 private:

src/xrpld/app/consensus/RCLConsensus.cpp

@@ -391,6 +391,8 @@ RCLConsensus::Adaptor::onClose(
         LedgerIndex const seq = prevLedger->header().seq + 1;
         CensorshipDetector<TxID, LedgerIndex>::TxIDSeqVec proposed;
 
+        // initialSet is unbacked, so the walk has no node store to fail to read and
+        // cannot report an incomplete result.
         initialSet->visitLeaves(
             [&proposed, seq](boost::intrusive_ptr<SHAMapItem const> const& item) {
                 proposed.emplace_back(item->key(), seq);
@@ -539,6 +541,8 @@ RCLConsensus::Adaptor::doAccept(
     {
         std::vector<TxID> accepted;
 
+        // The consensus transaction set is unbacked, so the walk has no node store to
+        // fail to read and cannot report an incomplete result.
         result.txns.map->visitLeaves(
             [&accepted](boost::intrusive_ptr<SHAMapItem const> const& item) {
                 accepted.push_back(item->key());

src/xrpld/app/ledger/detail/LedgerReplayMsgHandler.cpp

@@ -222,9 +222,18 @@ LedgerReplayMsgHandler::processReplayDeltaRequest(
     reply.set_ledgerheader(nData.getDataPtr(), nData.getLength());
     // pack transactions
     auto const& txMap = ledger->txMap();
-    txMap.visitLeaves([&](boost::intrusive_ptr<SHAMapItem const> const& txNode) {
-        reply.add_transaction(txNode->data(), txNode->size());
-    });
+    if (!txMap.visitLeaves([&](boost::intrusive_ptr<SHAMapItem const> const& txNode) {
+            reply.add_transaction(txNode->data(), txNode->size());
+        }))
+    {
+        // The transaction list above is incomplete, so name the missing nodes instead.
+        // The ledger itself is present, and an error reply carries no partial payload.
+        JLOG(journal_.debug()) << "getReplayDelta: Incomplete tx map for ledger " << ledgerHash;
+        reply.clear_transaction();
+        reply.clear_ledgerheader();
+        reply.set_error(protocol::TMReplyError::reNO_NODE);
+        return reply;
+    }
 
     JLOG(journal_.debug()) << "getReplayDelta for ledger " << ledgerHash << " txMap hash "
                            << txMap.getHash().asUInt256();

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -25,7 +25,6 @@
 #include <xrpl/protocol/Serializer.h>
 #include <xrpl/server/NetworkOPs.h>
 #include <xrpl/server/State.h>
-#include <xrpl/shamap/SHAMapMissingNode.h>
 #include <xrpl/shamap/SHAMapTreeNode.h>
 
 #include <boost/algorithm/string/predicate.hpp>
@@ -408,17 +407,17 @@ SHAMapStoreImp::run()
             JLOG(journal_.debug()) << "copying ledger " << validatedSeq;
             std::uint64_t nodeCount = 0;
 
-            try
-            {
-                validatedLedger->stateMap().snapShot(false)->visitNodes(
+            // A partial copy must not be followed by a rotation: clearPrior() above has
+            // already advanced minimumOnline_, and rotate() below deletes the archive, so
+            // a node the walk did not reach would have no remaining copy. Abandon this
+            // cycle and retry on a later ledger instead.
+            if (!validatedLedger->stateMap().snapShot(false)->visitNodes(
                     [this, &nodeCount](SHAMapTreeNode const& node) {
                         return copyNode(nodeCount, node);
-                    });
-            }
-            catch (SHAMapMissingNode const& e)
+                    }))
             {
-                JLOG(journal_.error())
-                    << "Missing node while copying ledger before rotate: " << e.what();
+                JLOG(journal_.error()) << "Missing node while copying ledger " << validatedSeq
+                                       << " before rotate; abandoning this rotation";
                 continue;
             }