test: Cover visitDifferences node skipping

`visitDifferences` had no test coverage: `LedgerMaster::populateFetchPack` is
its only caller, and nothing exercised that path, so the private `hasLeafNode`
and `hasInnerNode` it consults were untested too. Codecov flagged the resulting
gap against an unrelated refactor, since that commit happened to touch a line
inside `hasLeafNode`.

Four cases: a map missing one item reports only the nodes on that item's path,
an identical map reports nothing, a null comparison map reports every node the
plain `visitNodes` walk sees, and a callback returning false stops the walk
where `populateFetchPack` would stop once a pack is full.

The first case uses 64 shared items rather than a couple, so the shared leaves
build inner nodes of their own and the walk has whole matching subtrees to
skip. Verified by mutation: with only two shared items, breaking
`hasInnerNode` to always claim a match left every test passing, since the tree
was too shallow for that branch to be taken. It also asserts that some inner
nodes are reported but not all, which is what distinguishes skipping a subtree
from walking it. Breaking `hasLeafNode` in either direction, or `hasInnerNode`,
now fails the case.

The maps are hashed via `getHash` before being compared, since node hashes are
computed on demand and `visitDifferences` returns early while the root hash is
still zero. Without that the walk visits nothing and every assertion here
would hold vacuously.

src/tests/libxrpl/shamap/SHAMapSync.cpp

@@ -2,11 +2,13 @@
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/random.h>
+#include <xrpl/basics/safe_cast.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/SHAMapLeafNode.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
 #include <xrpl/shamap/SHAMapTreeNode.h>
 
@@ -180,4 +182,143 @@ TEST_F(SHAMapSyncTest, sync)
     destination.invariants();
 }
 
+// `visitDifferences` walks this map and reports only the nodes the other map does not already
+// have, which is how a fetch pack is assembled (see LedgerMaster's populateFetchPack). It decides
+// what to skip by asking `hasInnerNode` and `hasLeafNode`, both private, so these tests are the
+// only way to reach them.
+//
+// Both answers matter to a peer waiting on the pack. Reporting a node it already has wastes space
+// in a size-limited message; skipping one it does not have leaves it unable to complete the
+// ledger. The tests below therefore check exactly which nodes come back, not just how many.
+
+// Node hashes are computed on demand, and `visitDifferences` returns early while the root hash is
+// still zero, so a map has to be hashed before it can be compared against another.
+static void
+finalize(SHAMap& map)
+{
+    map.setImmutable();
+    ASSERT_FALSE(map.getHash().isZero());
+}
+
+TEST_F(SHAMapSyncTest, visit_differences_reports_only_what_is_missing)
+{
+    TestNodeFamily f{j_};
+
+    // Enough shared items that they build inner nodes of their own: the walk then has whole
+    // matching subtrees to skip, which is what hasInnerNode decides. With only a couple of shared
+    // leaves the tree is too shallow for that path to be taken at all.
+    std::vector<boost::intrusive_ptr<SHAMapItem>> shared;
+    shared.reserve(64);
+    for (int i = 0; i < 64; ++i)
+        shared.push_back(makeRandomAS());
+
+    auto const extra = makeRandomAS();
+
+    SHAMap have{SHAMapType::FREE, f};
+    for (auto const& item : shared)
+        ASSERT_TRUE(have.addItem(SHAMapNodeType::TnAccountState, item));
+    finalize(have);
+
+    SHAMap want{SHAMapType::FREE, f};
+    for (auto const& item : shared)
+        ASSERT_TRUE(want.addItem(SHAMapNodeType::TnAccountState, item));
+    ASSERT_TRUE(want.addItem(SHAMapNodeType::TnAccountState, extra));
+    finalize(want);
+
+    std::vector<uint256> leaves;
+    std::size_t inners = 0;
+    want.visitDifferences(&have, [&leaves, &inners](SHAMapTreeNode const& node) {
+        if (node.isLeaf())
+            leaves.push_back(safeDowncast<SHAMapLeafNode const&>(node).peekItem()->key());
+        else
+            ++inners;
+        return true;
+    });
+
+    // Every shared leaf is already on the far side, so only `extra` is worth sending.
+    EXPECT_EQ(leaves, std::vector<uint256>{extra->key()});
+
+    // The inner nodes on `extra`'s path are reported, but the matching subtrees are skipped, so
+    // the walk must not have visited every inner node in the tree.
+    std::size_t allInners = 0;
+    want.visitNodes([&allInners](SHAMapTreeNode& node) {
+        if (!node.isLeaf())
+            ++allInners;
+        return true;
+    });
+    EXPECT_GT(inners, 0u);
+    EXPECT_LT(inners, allInners);
+}
+
+TEST_F(SHAMapSyncTest, visit_differences_against_identical_map_reports_nothing)
+{
+    TestNodeFamily f{j_};
+
+    auto const item = makeRandomAS();
+
+    SHAMap have{SHAMapType::FREE, f};
+    ASSERT_TRUE(have.addItem(SHAMapNodeType::TnAccountState, item));
+    finalize(have);
+
+    SHAMap want{SHAMapType::FREE, f};
+    ASSERT_TRUE(want.addItem(SHAMapNodeType::TnAccountState, item));
+    finalize(want);
+    ASSERT_EQ(want.getHash(), have.getHash());
+
+    std::size_t visited = 0;
+    want.visitDifferences(&have, [&visited]([[maybe_unused]] SHAMapTreeNode const& node) {
+        ++visited;
+        return true;
+    });
+
+    EXPECT_EQ(visited, 0u);
+}
+
+TEST_F(SHAMapSyncTest, visit_differences_against_no_map_reports_every_node)
+{
+    TestNodeFamily f{j_};
+
+    SHAMap want{SHAMapType::FREE, f};
+    for (int i = 0; i < 32; ++i)
+        ASSERT_TRUE(want.addItem(SHAMapNodeType::TnAccountState, makeRandomAS()));
+    finalize(want);
+
+    // A null `have` means the far side holds nothing, so every node counts as missing. Compared
+    // against visitNodes, which walks the same tree with no such filtering.
+    std::size_t differences = 0;
+    want.visitDifferences(nullptr, [&differences]([[maybe_unused]] SHAMapTreeNode const& node) {
+        ++differences;
+        return true;
+    });
+
+    std::size_t all = 0;
+    want.visitNodes([&all]([[maybe_unused]] SHAMapTreeNode& node) {
+        ++all;
+        return true;
+    });
+
+    EXPECT_GT(differences, 0u);
+    EXPECT_EQ(differences, all);
+}
+
+TEST_F(SHAMapSyncTest, visit_differences_stops_when_callback_returns_false)
+{
+    TestNodeFamily f{j_};
+
+    SHAMap want{SHAMapType::FREE, f};
+    for (int i = 0; i < 32; ++i)
+        ASSERT_TRUE(want.addItem(SHAMapNodeType::TnAccountState, makeRandomAS()));
+    finalize(want);
+
+    // Returning false is how populateFetchPack stops once the pack is full, so the walk must
+    // honour it rather than visiting the rest of the tree.
+    std::size_t visited = 0;
+    want.visitDifferences(nullptr, [&visited]([[maybe_unused]] SHAMapTreeNode const& node) {
+        ++visited;
+        return visited < 3;
+    });
+
+    EXPECT_EQ(visited, 3u);
+}
+
 }  // namespace xrpl::tests