isComplete() is read without mtx_, so the flag must not be published
before the ledger it describes is settled - a second thread could
otherwise take a still-mid-sync ledger, and a mutable ledger reaching
LedgerHistory::insert() or LedgerMaster::switchLCL() calls logicError().
done() now owns the publication: it settles the ledger and only then sets
complete_, while trigger()/receiveNode() just set have-flags and leave the
verdict to done().
A walk hands back a bare list of hashes, so an empty result doesn't
distinguish a satisfied map from one the walk abandoned.
InboundLedger::hasInvalidMap() now reports the difference.
Three places that read emptiness as "nothing left to fetch" ask it first:
tryDB(), since its two walks set haveState_/haveTransactions_
independently and one map can be abandoned while the other is merely
incomplete; trigger()'s aggressive-retry branch, since the
getNeededHashes() walk it just ran can reach the verdict itself; and
trigger()'s state-map walk, the one walk that runs with mtx_ released.
That last one is asked outside the guard that re-reads the flags after
re-locking, since the verdict is about the map rather than about the
round: another thread can report the ledger complete while the lock is
released, and the guard would then drop the verdict, leaving a ledger
reported complete whose map cannot be the one the header names. The claim
is withdrawn alongside the failure there for that reason.
SHAMap::setImmutable() now returns [[nodiscard]] bool and refuses a map
already proven impossible. Every state change goes through trySetState(),
the only writer of state_ past construction, so the order between the
states is stated once: its compare-exchange can't leave Invalid however it
interleaves with another thread's, and setInvalid() stores through the same
funnel rather than behind its back. Invalid is stored unconditionally
there, since only the map itself reaches that verdict and a walk that
reaches it has to win against a thread settling the map; refusing to
overwrite Immutable would leave a map proven impossible reporting itself
sound, which is what nothing downstream could recover from.
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. The map hashes are
read before the maps are settled, since getHash() can unshare a dirty
tree, but written to the header only once both maps have made it. A walk
that invalidates a map in between therefore leaves the header describing
what the ledger was built from rather than a map that has since been
abandoned.
Every call site now branches 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. A test helper that cannot
reach BEAST_EXPECT throws instead, so a refusal cannot hand a broken
ledger to the assertions below.
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.
AcquireTestHelpers.h is the first src/test file to include one from
src/tests, so levelization records a new test.app > tests.libxrpl edge in
ordering.txt. No loop is introduced: nothing under src/tests includes
src/test.
Addresses Copilot review feedback on PR #8081.
