fix: Judge a map an InboundLedger's walk abandoned

[bthomee]

Part 8/16 of a stack. Base: part 7 (bthomee/shamap-invalid-07-getmissingnodes).

High Level Overview of Change

An InboundLedger walk can abandon a map (mark it invalid) without that ever becoming visible through
the flags tryDB()/trigger() already check, since an empty "missing nodes" result reads identically
to "nothing left to fetch." Both call sites now ask InboundLedger::hasInvalidMap() first.

Context of Change

A walk hands back a bare list of hashes, so an empty result does not distinguish a satisfied map from
one that has been abandoned. InboundLedger::hasInvalidMap() reports the difference, and the two places
that read emptiness as nothing left to fetch ask it first.

tryDB() asks after both of its walks and before the settle below, because the walks set haveState_
and haveTransactions_ independently: one map can be abandoned while the other is merely incomplete, so
neither flag is set and the settle that would otherwise catch it never runs. trigger()'s
aggressive-retry branch asks before testing whether anything is still needed, since the
getNeededHashes() walk it just ran can reach the verdict itself. Where there is no header yet there is
no map to judge, and hasInvalidMap() reports false, which is right because getNeededHashes() has
then asked for the header and the non-empty branch is the one that runs. done() keeps the settle as
its backstop and gains a SOMETIMES() naming the race that still reaches it: trigger() walks the state
map with mtx_ released, so a walk on another thread can reach the verdict after the flags said the
acquisition was finished.

Both new cases stage a fetch pack rather than peer data, since a fetch pack is checked against each
node's own hash and never structurally, so a whole chain resolves locally without passing through
addKnownNode(). The first covers tryDB(): the transaction root is the chain, so its walk abandons
that map, while the state root is a hash nothing supplies, so that map is merely incomplete — the
asymmetry is what leaves both flags unset. The second covers trigger(), recording a timeout count
above kLedgerBecomeAggressiveThreshold directly rather than waiting for the timer chain.

API Impact

None of the checkboxes below apply: this is an internal xrpld correctness fix with no libxrpl,
public-API, or peer-protocol surface.

[Copilot]

Pull request overview

This PR fixes a correctness gap in InboundLedger acquisition where a SHAMap walk can abandon (invalidate) a map but still return an empty “missing nodes” result, which previously could be misinterpreted as “nothing left to fetch.” The change makes that “map became invalid” state visible to the two call sites (tryDB() and the aggressive-retry path in trigger()) that treat emptiness as completion.

Changes:

• Add InboundLedger::hasInvalidMap() to detect when either ledger map has been invalidated during acquisition.
• In tryDB() and trigger() (aggressive retry), explicitly fail the acquisition if a walk invalidated a map, instead of treating an empty result as completion.
• Add regression tests covering (1) local fetch-pack resolution that abandons only one map and (2) aggressive retry judging a locally-resolved invalid map (plus the headerless “no map to judge” case).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/xrpld/app/ledger/InboundLedger.h	Declares hasInvalidMap() with rationale about empty walk results vs abandoned maps.
src/xrpld/app/ledger/detail/InboundLedger.cpp	Implements hasInvalidMap() and adds invalid-map checks in tryDB(), trigger(), plus telemetry in done().
src/test/app/InboundLedger_test.cpp	Adds tests and helpers to reproduce the invalid-map-empty-walk edge cases and verify the new guards.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[github-actions]

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

[xrplf-ai-reviewer]

This diff adds InboundLedger::hasInvalidMap() and threads it into tryDB() and the two branches of trigger() so an empty "missing hashes" result from a walk that actually abandoned a map is no longer misread as "nothing left to fetch." The new guards are placed before the existing empty-result branches, the state-map section correctly withdraws complete_ alongside setting failed_ when the walk lands after the lock was re-acquired, and done() gets a documented SOMETIMES() marking the remaining (already-existing, now narrower) race. The two new tests stage fetch packs so a whole chain resolves locally without going through addKnownNode(), matching the described tryDB()/aggressive-retry asymmetry, and use setTimeouts()/clearProgress() to reach the aggressive branch without waiting on the real timer chain. I traced the flag/lock interactions in both call sites and didn't find a case where the new guard fires incorrectly or fails to fire when it should — the logic is consistent with the detailed comments and the rest of the stack. No changed lines stood out as bugs worth flagging.

[codecov]

Codecov Report

❌ Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/xrpld/app/ledger/detail/InboundLedger.cpp	57.9%	8 Missing ⚠️

📢 Thoughts on this report? Let us know!