fix: Restart the timer when reviving a timed-out TX-set acquisition

[bthomee]

Part 11/16 of a stack. Base: part 10 (bthomee/shamap-invalid-10-header-ledger-mutable).

High Level Overview of Change

TransactionAcquire::stillNeed() now restarts the retry timer whenever it revives a timed-out
acquisition, so a revived object actually resumes asking peers instead of silently waiting for
unprompted data.

Context of Change

TransactionAcquire::stillNeed() restarts the retry timer whenever it revives an acquisition, so a
revived object resumes asking rather than waiting for a peer to send data unprompted. Restarting is what
resumes it: the timer chain is what drives trigger(), and a timed-out acquisition has none pending.
expires_after() cancels any pending wait, so this cannot leave two chains running.

It also returns early when there is nothing to revive, so an acquisition that is still running keeps the
wait it already has. Consensus asks for a set it still needs once per round, and re-arming on every ask
would push the next tick back each time, so a set asked for more often than the interval would never
tick at all.

Two cases cover the halves. The first drives a real timeout chain to failure — cancel() alone never
arms a timer, so reviving from a faked failure would only prove a timer starts, not that a timed-out one
restarts — then revives it and checks a request goes out and data is examined again. The restart is
observed through a candidate peer that can only be re-offered by onTimer()'s broadcast:
RequestCountingPeerSet dedups by tracked id like the real peer set, so addPeers() never re-offers a
peer already selected, and a request reaching it again after stillNeed() is what distinguishes an
armed acquisition from one whose failure flag was merely cleared. The second asks again faster than the
interval, the way a short consensus round would, and checks a tick still gets through.

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

Fixes transaction-set acquisition revival by restarting the retry timer after timeout.

Changes:

• Documents revival behavior.
• Restarts timers only for failed acquisitions.
• Adds regression tests for revival and timer preservation.

Reviewed changes

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

File	Summary	Review findings
src/xrpld/app/ledger/detail/TransactionAcquire.h	Documents stillNeed() behavior.	None.
src/xrpld/app/ledger/detail/TransactionAcquire.cpp	Clears failure state and restarts the timer when needed.	None.
src/test/app/TransactionAcquire_test.cpp	Tests revival and timer behavior.	Moderate (2 votes): test does not exercise timer exhaustion. Nit (2 votes): add “that” to the comment.

Suppressed comments (1)

src/xrpld/app/ledger/detail/TransactionAcquire.cpp:287

• failed_ is also set for terminal map-invalid outcomes (for example when trigger() observes an invalid SHAMap), not only for timeout exhaustion. Clearing it here lets a later consensus lookup re-arm an acquisition whose map is permanently Invalid, contrary to the SHAMap contract that such a map cannot be repaired by retrying; it can briefly process more peer data and schedule unnecessary work. Track timeout-origin failure separately, or gate revival on the timeout failure state, and only clear failed_ for that case.

    if (!failed_)
        return;

    failed_ = false;

---

💡 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 1 comment.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!