diff --git a/src/tests/libxrpl/shamap/SHAMapSync.cpp b/src/tests/libxrpl/shamap/SHAMapSync.cpp index e4bcbd89705..0bb27f9cbb0 100644 --- a/src/tests/libxrpl/shamap/SHAMapSync.cpp +++ b/src/tests/libxrpl/shamap/SHAMapSync.cpp @@ -2,11 +2,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -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> 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 leaves; + std::size_t inners = 0; + want.visitDifferences(&have, [&leaves, &inners](SHAMapTreeNode const& node) { + if (node.isLeaf()) + leaves.push_back(safeDowncast(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{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