fix: Count partial batch progress in a TX-set reply

[bthomee]

Part 12/16 of a stack. Base: part 11 (bthomee/shamap-invalid-11-revive-timer).

High Level Overview of Change

TransactionAcquire::takeNodes() now accumulates one combined verdict across a whole batch instead of
stopping at the first bad node, so a packet that ends badly still gets credit — and clears the
timeout-postponing progress flag — for the good nodes ahead of it, matching what
InboundLedger::receiveNode() already does.

Context of Change

TransactionAcquire::takeNodes() accumulates one SHAMapAddNode across the batch and hands it back, so
a packet ending on one bad node still counts the nodes hooked in ahead of it. A root the set already
holds counts as a duplicate rather than passing unremarked, which is what tells a batch of nodes we
already have from one that was never examined at all.

The body moves to takeNodesLocked() and takeNodes() becomes a thin wrapper that takes the lock and
records what the batch achieved on the one exit — several of the inner exits stop the batch early, and
nothing in the compiler catches one that forgets to record progress. Progress turns on the batch being
useful rather than good, so a batch of nothing but duplicates records none: that flag is what keeps the
next timer tick from counting a timeout, and a reply carrying nothing new advanced nothing. It costs
retry budget on the ordinary second responder to a fan-out and nothing else.

InboundTransactions::gotData() asks isGood() rather than isUseful() for the useless-data charge,
which keeps that duplicate free — now that the verdict distinguishes a duplicate from an empty tally,
the useful test would charge for it while the batch that was never examined still needs charging.

testPartialBatchIsCounted feeds two good nodes followed by one at a position it cannot occupy, and
checks the verdict names both halves and that the flag is set; then a batch of nothing but a root
already held, checking it is good, not useful, and records nothing; then that root alongside a node the
set does need, checking the duplicate is reported as one while the other records the progress.

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 improves TransactionAcquire::takeNodes() accounting so partial success in a TX-set node batch is preserved (good nodes counted) even when the batch ends with a bad node, and “progress” is recorded based on actual map advancement rather than “overall good.” This aligns TX-set acquisition behavior with how inbound ledger node batches are already handled.

Changes:

• Refactors TransactionAcquire::takeNodes() to delegate to takeNodesLocked() and record progress on a single exit path based on SHAMapAddNode::isUseful().
• Accumulates a combined SHAMapAddNode verdict across the whole batch, including explicit duplicate counting for an already-held root.
• Updates InboundTransactions::gotData() to charge useless-data fees based on !san.isGood() (so duplicate-only replies are not charged) and adds a focused regression test.

Reviewed changes

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

File	Description
src/xrpld/app/ledger/detail/TransactionAcquire.h	Documents batch-level verdict semantics and introduces takeNodesLocked() for single-exit progress recording.
src/xrpld/app/ledger/detail/TransactionAcquire.cpp	Implements batch verdict accumulation, early-exit returns that preserve prior counts, and progress recording only when the batch added nodes.
src/xrpld/app/ledger/detail/InboundTransactions.cpp	Adjusts useless-data charging to use isGood() so duplicate-only replies aren’t penalized while empty/unprocessed replies still are.
src/test/app/TransactionAcquire_test.cpp	Adds testPartialBatchIsCounted to validate partial-batch tallying, duplicate handling, and progress flag behavior.

---

💡 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 4 out of 4 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]

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.

[github-actions]

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

[codecov]

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/xrpld/app/ledger/detail/TransactionAcquire.cpp	90.9%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!