From db71e8284ec423c323956017bdcc254242f70fbd Mon Sep 17 00:00:00 2001 From: Bart <11445373+bthomee@users.noreply.github.com> Date: Wed, 10 Jun 2026 16:04:21 -0400 Subject: [PATCH] Review feedback --- src/libxrpl/shamap/SHAMap.cpp | 4 ++++ src/tests/libxrpl/shamap/SHAMapMissingNode.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libxrpl/shamap/SHAMap.cpp b/src/libxrpl/shamap/SHAMap.cpp index b4e0ccb937..abfffa7d2f 100644 --- a/src/libxrpl/shamap/SHAMap.cpp +++ b/src/libxrpl/shamap/SHAMap.cpp @@ -146,7 +146,11 @@ SHAMap::walkTowardsKey(uint256 const& id, SharedPtrNodeStack* stack) const inNode.adopt(descend(inner.get(), branch)); if (!inNode) + { + JLOG(journal_.warn()) << "xrpl::SHAMap::walkTowardsKey: missing child node " + << inner->getChildHash(branch); return nullptr; + } nodeID = nodeID.getChildNodeID(branch); } diff --git a/src/tests/libxrpl/shamap/SHAMapMissingNode.cpp b/src/tests/libxrpl/shamap/SHAMapMissingNode.cpp index 5a5db9ba58..80073dad93 100644 --- a/src/tests/libxrpl/shamap/SHAMapMissingNode.cpp +++ b/src/tests/libxrpl/shamap/SHAMapMissingNode.cpp @@ -24,8 +24,8 @@ namespace { // Seed for the random number generator used to create the test data. constexpr std::uint32_t kSeed = 0xdeadbeefU; -// Sync source into dest, delivering at most nodeLimit inner nodes. Returns the number of nodes -// actually delivered to dest. +// Sync source into dest, delivering at most nodeLimit nodes requested by dest.getMissingNodes(). +// This intentionally leaves parts of the tree absent to simulate missing subtrees. void syncWithLimit(SHAMap const& source, SHAMap& dest, std::size_t nodeLimit) { @@ -44,7 +44,7 @@ syncWithLimit(SHAMap const& source, SHAMap& dest, std::size_t nodeLimit) if (missing.empty()) break; - for (auto const& [nodeID, hash] : missing) + for (auto const& [nodeID, _] : missing) { if (delivered >= nodeLimit) return; @@ -90,7 +90,7 @@ TEST(SHAMapMissingNode, visitLeavesSkipsMissingNodes) // child nodes evicted after a rotation. SHAMap dest(SHAMapType::FREE, source.getHash().asUInt256(), destFamily); dest.setSynching(); - syncWithLimit(source, dest, 3); + ASSERT_NO_FATAL_FAILURE(syncWithLimit(source, dest, 3)); // Count the total number of leaf nodes in the dest map. Since most subtrees are now missing, // the traversal will result in fewer nodes being visited. The function must not throw.