diff --git a/txconfirm/broadcaster.go b/txconfirm/broadcaster.go index 326e7b98d..dd528c9ad 100644 --- a/txconfirm/broadcaster.go +++ b/txconfirm/broadcaster.go @@ -479,6 +479,26 @@ func (b *CPFPBroadcaster) reserveFeeInput(ctx context.Context, } } +// feeOutpointReserved reports whether the parent txid already holds a +// reservation on the given outpoint. It distinguishes a fee input carried over +// from a prior committed bump (which TRUC package RBF requires the replacement +// child to keep double-spending, so it must stay reserved) from one freshly +// selected during the current submission (safe to release if abandoned). It +// does not create a parent state entry, so a pre-reservation probe cannot leak +// an empty shell. +func (b *CPFPBroadcaster) feeOutpointReserved(parentTxid chainhash.Hash, + op wire.OutPoint) bool { + + state, ok := b.parentStates[parentTxid] + if !ok || state.UsedFeeOutpoints == nil { + return false + } + + _, held := state.UsedFeeOutpoints[op] + + return held +} + // releaseWalletLease calls wallet.ReleaseOutput with the package-scoped // LockID, silently ignoring "unknown output" / "not leased" class // errors since those are a normal consequence of a wallet that is @@ -889,6 +909,13 @@ func (b *CPFPBroadcaster) broadcastWithCPFP(ctx context.Context, height int32, err) } + // Record whether this input was carried over from a prior committed + // bump before we (re-)reserve it. If a later reselect abandons it, we + // only release inputs freshly selected this submission: a carried-over + // input must stay reserved because TRUC package RBF requires the + // replacement child to double-spend the previous child's fee input. + feeInputCarriedOver := b.feeOutpointReserved(txid, feeInput.Outpoint) + b.reserveFeeInput(ctx, txid, feeInput) // If the chosen fee-input's actual script class differs from the @@ -932,6 +959,20 @@ func (b *CPFPBroadcaster) broadcastWithCPFP(ctx context.Context, height int32, ) } if reselected.Outpoint != feeInput.Outpoint { + // The child will spend the reselected input, so + // release the abandoned original's reservation + // and wallet lease -- otherwise it stays locked + // for the parent's whole lifetime while nothing + // spends it (#664). Skip a carried-over input: + // TRUC RBF still needs the previous child's fee + // input reserved, so releasing it would drop an + // input the replacement must double-spend. + if !feeInputCarriedOver { + b.releaseFeeOutpoint( + ctx, txid, feeInput.Outpoint, + ) + } + feeInput = reselected b.reserveFeeInput(ctx, txid, feeInput) } diff --git a/txconfirm/broadcaster_test.go b/txconfirm/broadcaster_test.go index 0ebeb8048..46ed1550c 100644 --- a/txconfirm/broadcaster_test.go +++ b/txconfirm/broadcaster_test.go @@ -1058,6 +1058,102 @@ func TestCPFPBumpFailsClosedAtFeeRateCap(t *testing.T) { }) } +// TestCPFPReselectReleasesAbandonedFeeInput reproduces wavelength#664: when the +// precise-vsize fee recompute forces a reselect to a larger fee UTXO, the +// originally reserved input must be released -- both the in-memory reservation +// and the wallet lease -- rather than staying locked for the parent's whole +// lifetime while the child spends the reselected input instead. +func TestCPFPReselectReleasesAbandonedFeeInput(t *testing.T) { + t.Parallel() + + const feeRate = 5 + + chain := newFakeChainSourceRef(100) + chain.feeRate = feeRate + + parent := makeTestTx(true) + txid := parent.TxHash() + + // Same shape as TestCPFPReselectsAfterPreciseFeeGrowth: a P2PKH fee + // input is heavier than the OP_TRUE change proxy, so the precise vsize + // recompute grows the fee and forces a reselect off the small UTXO. + opTrue := []byte{txscript.OP_TRUE} + changeVSize := estimateChildVSize(opTrue, opTrue) + p2pkhScript := p2pkhTestPkScript(t) + preciseVSize := estimateChildVSize(p2pkhScript, opTrue) + require.Greater(t, preciseVSize, changeVSize) + + baseFee, err := computePackageFee(parent, feeRate, changeVSize) + require.NoError(t, err) + preciseFee, err := computePackageFee(parent, feeRate, preciseVSize) + require.NoError(t, err) + require.Greater(t, preciseFee, baseFee) + + // small (A) clears the initial threshold but not the grown one, so it + // is selected + reserved first and then abandoned for large (B). + small := &walletcore.Utxo{ + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{ + 0xaa, + }, + Index: 0, + }, + Amount: baseFee + DustLimit, + PkScript: p2pkhScript, + } + large := &walletcore.Utxo{ + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{ + 0xbb, + }, + Index: 0, + }, + Amount: preciseFee + DustLimit + 1000, + PkScript: p2pkhScript, + } + + w := &fakeWallet{utxos: []*walletcore.Utxo{small, large}} + b := NewCPFPBroadcaster( + BroadcasterConfig{ + ChainSource: chain, + Wallet: w, + }, + ) + + _, err = b.Submit(t.Context(), 100, &BroadcastRequest{ + Tx: parent, Label: "reselect-release", + }) + require.NoError(t, err) + + // The in-memory reservation must hold the reselected input (B) and NOT + // the abandoned original (A). + state, ok := b.parentStates[txid] + require.True(t, ok) + require.Contains(t, state.UsedFeeOutpoints, large.Outpoint) + require.NotContains( + t, state.UsedFeeOutpoints, small.Outpoint, + "abandoned fee input must not stay reserved (#664)", + ) + + // The wallet lease on the abandoned input must be released. Lease + // releases run asynchronously, so wait for the call to land. + require.Eventually(t, func() bool { + releaseCalls, _ := w.releaseSnapshot() + for _, op := range releaseCalls { + if op == small.Outpoint { + return true + } + } + + return false + }, time.Second, 10*time.Millisecond, + "abandoned fee input's wallet lease must be released") + + // The reselected input's lease must NOT have been released. + releaseCalls, _ := w.releaseSnapshot() + require.NotContains(t, releaseCalls, large.Outpoint) +} + // makeCheckpointShapedParent builds a v3/TRUC parent that mirrors a finalized // OOR checkpoint / fraud-response tx: its sole non-anchor input already carries // a real (pre-signed) taproot key-spend witness, it has a taproot checkpoint