fix: Keep a settled acquisition registered so late replies stay bounded

[bthomee]

Part 16/17 of a stack. Base: part 15 (bthomee/shamap-invalid-15-invalidated-map-receivenode).

High Level Overview of Change

InboundTransactions::giveSet() now keeps a completed acquisition registered when the acquisition
itself is what supplied the set, so a late reply for that hash still reaches the late-reply allowance
instead of being charged outright.

Context of Change

giveSet() used to reset the map entry's acquire pointer unconditionally once a set arrived, so any
reply for that hash arriving after completion took gotData()'s ta == nullptr branch from then on —
charged outright, with no allowance in play at all, since takeNodesLocked() was never reached to apply
one. The tests added for the allowance in part 14 called acquire->takeNodes() directly, bypassing
gotData()/giveSet() entirely, so they never exercised this gap: the real production entry point for
peer replies never got as far as the allowance at all.

giveSet() now only resets the entry's acquire pointer when something other than the acquisition
itself supplied the set — a set arriving some other way still cancels an acquisition genuinely in
flight, but the acquisition completing on its own is not that. Keeping it alive until newRound() sweeps
the entry is what lets a late reply for this hash still reach getAcquire() and, through it,
takeNodesLocked()'s allowance.

This is a Copilot review finding on part 14 (#8093), split out into its own branch rather than folded
in: unlike the late-reply allowance bound fixed there, this gap is unchanged pre-existing behavior, not
something this stack makes worse, so there is no urgency tying it to that PR's release.

Covered by a new TransactionAcquire gtest/boost case driven end to end through
InboundTransactions::gotData(), including the async job done() hands off to, so the real completion
path is what proves the allowance survives it.

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 InboundTransactions::giveSet() where a completed TransactionAcquire was being unregistered immediately, causing late peer replies for the same hash to bypass TransactionAcquire::takeNodesLocked() and be charged outright without applying the “one free per peer asked” late-reply allowance.

Changes:

• Make InboundTransactions::giveSet() only reset/cancel the tracked acquisition when the set was supplied by something other than the acquisition itself (!fromAcquire).
• Add an end-to-end TransactionAcquire test that drives late replies through InboundTransactions::gotData() and the async done() -> giveSet() job path to verify the allowance survives completion.

Reviewed changes

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

File	Description
src/xrpld/app/ledger/detail/InboundTransactions.cpp	Preserve the acquisition pointer when giveSet() is invoked by the acquisition completing, so late replies still reach allowance logic instead of ta == nullptr charging.
src/test/app/TransactionAcquire_test.cpp	Adds an end-to-end regression test ensuring late-reply allowance remains effective after giveSet() runs via the async completion path.

---

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

[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.

[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.

[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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[xrplf-ai-reviewer]

This change fixes a real gap: giveSet() previously reset the map entry's acquire pointer unconditionally, which meant any late reply for a hash whose acquisition had just completed skipped the late-reply allowance in takeNodesLocked() entirely and was charged outright. The fix (if (!fromAcquire) inboundSet.acquire.reset();) only tears down the acquire pointer when something other than the acquisition itself supplied the set, letting a genuinely-still-in-flight acquisition still get cancelled by a set arriving another way, while a self-completed acquisition stays registered until newRound() sweeps it. The new gtest case drives this end-to-end through gotData()/done(), which is the right way to prove the allowance is actually reachable via the real peer-reply path rather than the direct takeNodes() calls used elsewhere in the suite, and it explicitly covers the free late reply followed by a charged replay. I don't see a correctness problem in what's shown here; the one open question — whether a completed-but-not-yet-swept acquisition could accumulate under sustained load before newRound() runs — is inherent to the described design and not something this diff makes worse, so I'm not flagging it.