txconfirm: release abandoned fee input on CPFP reselect

[ellemouton]

Closes #664.

Problem

In broadcastWithCPFP, when the precise per-input vsize recompute grows
the package fee enough to push the originally selected fee input's change
below the dust limit, the broadcaster reselects a larger fee input — but
it had already reserveFeeInput'd (and wallet-LeaseOutput'd) the
original before the reselect and never released it. The abandoned UTXO
stays reserved and wallet-leased for the parent's whole lifetime (or until
DefaultFeeInputLeaseExpiry), even though the CPFP child only ever spends
the reselected input. It's a spurious UTXO lock (not fund loss) that
starves other fee selections.

Fix

Release the abandoned input's reservation + wallet lease
(releaseFeeOutpoint) when the reselect lands on a different outpoint.

The caveat that makes this non-trivial: selectFeeInput can
legitimately return a carried-over reservation on a fee bump — TRUC
package RBF requires the replacement child to double-spend the previous
child's fee input, so that input must stay reserved. The release therefore
skips a carried-over input: feeOutpointReserved records whether the input
was already reserved before the (idempotent) re-reservation, so a
freshly-selected-and-abandoned input is released while a carried-over one
is left intact.

Testing

TestCPFPReselectReleasesAbandonedFeeInput drives the exact reselect path
(a P2PKH fee input heavier than the OP_TRUE change proxy grows the fee and
forces a reselect off the small UTXO) and asserts the abandoned input is
dropped from the in-memory reservation set and its wallet lease is
released, while the reselected input stays reserved. It fails on main and
passes with the fix. Full txconfirm suite + lint-changed-local green.

🤖 Generated with Claude Code

[ellemouton]

@claude please review this PR.

It closes #664: broadcastWithCPFP reserved + wallet-leased a fee input, then on a precise-vsize reselect switched to a larger input without ever releasing the original — leaking the reservation/lease for the parent's lifetime.

The interesting part is the carried-over caveat, so please scrutinise:

• Correctness of feeOutpointReserved as the carried-over signal. I capture it before reserveFeeInput (which is idempotent and re-adds the outpoint), so on a fee bump a carried-over input reads true and is NOT released, while a freshly-selected input reads false and IS released on reselect. Is capturing before the re-reservation the right discriminator, and are there paths where an input is carried-over yet not yet in UsedFeeOutpoints at that point?

• TRUC RBF safety. The whole reason to skip carried-over inputs is that package RBF requires the replacement child to double-spend the previous child's fee input. Does the skip fully preserve that? And is there any scenario where a fresh input we now release was actually needed?

• No double-release / state-shell leak. releaseFeeOutpoint also prunes an empty parent-state entry — on reselect we release the old then immediately reserveFeeInput the new; any risk of dropping then mis-recreating state, or of releasing an outpoint that a later fallback path releases again?

Repro test TestCPFPReselectReleasesAbandonedFeeInput fails on main, passes here.

[claude]

Claude finished @ellemouton's task in 2m 29s —— View job

---

Review: txconfirm: release abandoned fee input on CPFP reselect

I traced the full reselect path in broadcastWithCPFP, feeOutpointReserved, reserveFeeInput, releaseFeeOutpoint, selectFeeInput/selectReservedFeeInput, and all downstream fallback releases. The fix is correct and conservative. Answering your three questions directly:

1. feeOutpointReserved as the carried-over discriminator ✅

Capturing before the idempotent reserveFeeInput is the right signal, and there is no path where an input is carried-over yet missing from UsedFeeOutpoints:

• reserveFeeInput always writes UsedFeeOutpoints[op] and UsedFeeInputs[op] together (broadcaster.go:452-453), and releaseFeeOutpoint deletes them together (:551-552). The two maps are never out of sync.
• The only way selectFeeInput returns a carried-over input is via selectReservedFeeInput, which reads exclusively from UsedFeeInputs (:1381). So anything selected as carried-over is by construction present in UsedFeeOutpoints, and feeOutpointReserved reads true. A first-submission input has no prior state → reads false. Discriminator is sound.

2. TRUC RBF safety ✅ (with one pre-existing observation, not a blocker)

• Skip preserves the invariant. On a bump, selectReservedFeeInput prefers the previously-reserved input, so feeInput reads carried-over and is never released — the replacement child keeps the input it must double-spend.
• Releasing a fresh input is safe. A "fresh" input is one selected this submission that no committed/broadcast child has ever spent (the reselect happens before any package submission). Nothing depends on it, so releasing it starves nothing.
• Nice property worth calling out: when the old input is fresh D but the reselect lands back on a carried-over C (because C is now the smallest reserved input covering the grown target), the discriminator correctly releases D and the child ends up spending C — RBF stays satisfied.
• Pre-existing (not introduced here): if a carried-over C no longer covers the grown target and the reselect switches to a fresh D, the skip keeps C reserved (good) but the child is built spending D only — so that particular child wouldn't double-spend C. This is orthogonal to the leak and predates this PR; keeping C reserved is strictly the safer choice. Flagging only so it's on the record.

3. Double-release / state-shell leak ✅

• Delete-then-recreate is benign. On a first-submission reselect where the abandoned input is the only reservation, releaseFeeOutpoint prunes the whole parentStates[txid] shell (:560-566) — but LastFeeRate/LastPackageFee are still 0 and PredictedFeeInputs empty at that point, so nothing of value is lost, and the immediately-following reserveFeeInput(reselected) recreates the entry via parentState(). If any predicted input or fee history did exist, the prune guard (len(PredictedFeeInputs)==0 etc.) leaves the entry intact. Either way the terminal state := b.parentStates[txid] at :1046 finds a live entry — no nil deref.
• No double-release. After the switch, feeInput = reselected, and every downstream fallback (:956, :966, :983, :1007, :1024, :1033) releases reselected.Outpoint, never the abandoned one. releaseFeeOutpoint is idempotent anyway (early-returns when !held, :547-549), so even a redundant call is a no-op.
• No lease race. Old and reselected are guaranteed distinct (reselected.Outpoint != feeInput.Outpoint guard), so the async release of the old lease and the new LeaseOutput touch different outpoints.

Minor: test coverage gap (suggestion, non-blocking)

TestCPFPReselectReleasesAbandonedFeeInput exercises the fresh-abandon path well (asserts both the in-memory drop and the async wallet-lease release, and pins that the reselected input's lease is not released). But the interesting caveat — that a carried-over input is not released on a bump reselect — has no direct regression test. A future refactor could invert the !feeInputCarriedOver guard and this suite would stay green. Consider adding a second test that: does an initial submission to reserve C, then a fee bump whose precise-vsize reselect switches off C, and asserts C stays in UsedFeeOutpoints and its lease is never released. That locks in the exact TRUC-safety behavior this PR is built around.

Verdict

LGTM. Correct discriminator, TRUC invariant preserved, no double-release or harmful state-shell churn. Only ask is the optional carried-over regression test to guard the caveat you flagged.

Todo

• Read reselect path & surrounding logic
• Scrutinise feeOutpointReserved carried-over signal
• Verify TRUC RBF safety of skip
• Check double-release / state-shell leak concerns
• Assess test correctness
• Post review
  · branch agent/cpfp-reselect-release-lease

[Roasbeef]

LGTM 🫛

[github-actions]

Successfully created backport PR for v0.1.x-branch:

• [v0.1.x-branch] Backport #1066: txconfirm: release abandoned fee input on CPFP reselect #1135