A walk that reaches a position only a leaf may occupy now marks the map
Invalid and abandons the descent instead of continuing:
SHAMapNodeID::getChildNodeID() throws past kLeafDepth, uncaught, all the
way to std::terminate(). It's reachable without going through
addKnownNode() at all - InboundLedgers::gotStaleData() stores any
parseable node from an unsolicited liAS_NODE reply into the fetch pack by
its own hash with no relatedness check - making this a conditioned remote
denial of service, not just a single bad packet.
As in addKnownNode(), the depth check runs before the full-below cache
lookup, for the same cache-doesn't-cover-depth reason. Callers must
re-check isValid() before reading an empty result as nothing left to
fetch, which getMissingNodes()'s docstring now says.
SHAMap::setImmutable() now returns [[nodiscard]] bool and refuses a map
already proven impossible; every state change goes through trySetState(),
whose compare-exchange can't leave Invalid however it interleaves with
another thread's. Ledger::setImmutable()/setAccepted() do the same one
level up, checking mapsValid() before touching anything and settling both
maps independently so neither is left mid-sync because the other refused.
All fourteen call sites now branch on the result. The two genesis paths
and buildLedgerImpl() call logicError(), since consensus can't tolerate an
invalid ledger; the load paths return early instead; InboundLedger and
TransactionAcquire withdraw complete_ alongside the failure, since for
them a refusal is an outcome a peer can produce.
SHAMap::addKnownNode() now reports invalid() for the two node shapes it
refuses to hook in: an inner node at kLeafDepth (a depth only a leaf may
occupy) and a node whose ID doesn't match where the descent stopped. Both
already read as bad data to callers, so neither counted as progress
before. No peer can satisfy a hash that reaches either shape, so retrying
is futile; the charge is a deterrent rather than a control, since the same
node can reach a map through a fetch pack or unsolicited object reply with
no peer to charge.
The depth check runs before the full-below cache lookup, since that cache
is keyed by hash (which doesn't cover depth) and shared across every map
in the family - checking depth first keeps the verdict independent of what
an unrelated map cached.
Background ledger acquisition reads and writes SHAMap::state_, ::full_,
::ledgerSeq_, and SHAMapInnerNode::fullBelowGen_ concurrently with the
thread driving it, so all four are now std::atomic. finishFetch() withdraws
full_ with an exchange behind a relaxed load, so exactly one reader thread
reports a gap; ledgerSeq_ stays relaxed both ways since it's only a
nodestore lookup hint.
Ledger::setFull() sets each map's sequence before its full flag, so the
release/exchange ordering makes the sequence visible to whichever thread's
exchange wins the gap report.
tryDB() can decide an acquisition can never succeed (a header hash/sequence
mismatch, or a zero account hash) without ever calling done(), so nothing
signals whatever is waiting, and logFailure() never records the hash in
recentFailures_ - the next round asks for the same doomed ledger again.
init() and trigger() now call done() on that path too, matching
checkLocal(), which already did.
DeepChain (src/tests/libxrpl/shamap/DeepChain.h) builds node chains for both
acquisition suites: fabricated chains that run to SHAMap::kLeafDepth, which
no valid tree can hold, and toLeaf() chains that complete an acquisition.
AcquireTestHelpers.h adds ChargeRecordingPeer, RequestCountingPeerSet
(deduping by tracked id like the real PeerSetImpl), packetFor(), waitFor(),
and tallyIs(), so both suites can drive an acquisition through its real
gotData() dispatch instead of reproducing it.
TransactionAcquire and InboundLedger drop final and take a defaulted
retryInterval, so tests can run a whole timeout chain in a fraction of a
second; nothing in production passes one.
Addresses Copilot review feedback on PR #8081.
SHAMapAddNode gains getBad() and getDuplicate() beside getGood(), so a
verdict can be read as counts instead of just a log string. get()'s wording
is pinned by src/tests/libxrpl/shamap/SHAMapAddNode.cpp, the one place that
depends on it.
