docs: Correct the reachability claims on the leaf depth guards

The `visitDifferences` guard said an inner node at leaf depth could be seeded by a peer through an
earlier fetch-pack exchange, while the code marks the branch `UNREACHABLE`, which is documented as a
line that must not be reached in normal use or under fuzzing. Both cannot hold: a peer-reachable
assert is a peer-triggered abort. The code is the correct half. `addKnownNode` marks the map invalid
instead of hooking such a node in, fetch-pack blobs are checked against their content hash in
`LedgerMaster::getFetchPack`, and the parent child-hash slots they attach to chain up to a validated
root, so provoking this needs a preimage rather than a crafted message.

The comment now attributes the branch to a defect or a corrupt store, and records why the node is
still reported into the pack before its children are skipped: the wire form of an inner node carries
only child hashes, so the bad depth is never transmitted, and the recipient hooks blobs in by hash at
positions its own traversal picks. Withholding it would instead surface as a peer that cannot
complete a ledger, with nothing to diagnose locally.

The `hasLeafNode` comment credited its caller with a bound that does not apply. That guard limits the
depth of the caller's own traversal, whereas `hasLeafNode` runs on the map passed in and descends
from that map's root, so the check here is the only thing between a malformed map and the throw in
`getChildNodeID`, not a second line of defense. `hasInnerNode` is the one bounded by its caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bart
2026-08-21 17:35:11 -04:00
parent 90ef662bc7
commit 5891da909d

View File

@@ -143,9 +143,12 @@ SHAMap::visitDifferences(
if (!function(*node))
return;
// Nibbles run out at kLeafDepth, so only a leaf belongs there. An inner node at that depth
// means the store holds one, possibly seeded by a peer via an earlier fetch-pack exchange;
// skip its children rather than descending into it.
// Nibbles run out at kLeafDepth, so only a leaf belongs there. A well-formed map never
// holds an inner node at that depth: addKnownNode marks the map invalid rather than hooking
// one in, and fetch-pack data is hash-verified against a validated root, so reaching this
// means a defect or a corrupt store, not something a peer can provoke. Report the node
// anyway - the wire form carries no depth, and the recipient hooks blobs in by hash - but
// skip the children rather than letting getChildNodeID throw on them.
if (nodeID.getDepth() >= kLeafDepth)
{
// LCOV_EXCL_START
@@ -760,9 +763,9 @@ SHAMap::hasLeafNode(uint256 const& tag, SHAMapHash const& targetNodeHash) const
do
{
// Same kLeafDepth hazard as in visitDifferences above. That's the only caller and already
// bails out before reaching here; this is a second line of defense since the loop below
// descends on its own and doesn't depend on the caller's bound.
// Same kLeafDepth hazard as in visitDifferences above. That guard bounds the caller's own
// traversal, not the map queried here, and the loop below descends from this map's root
// independently, so this check is what keeps a malformed map from reaching getChildNodeID.
if (nodeID.getDepth() >= kLeafDepth)
{
// LCOV_EXCL_START