Skip to content

Commit 9d0a418

Browse files
committed
sweepbatcher: mask presign cancellation races
PresignSweepsGroup uses context-sensitive wallet and presigned-helper calls, but it previously returned their raw wrapped errors even when the caller context or batcher shutdown state had already become terminal. That leaves backend/helper errors visible during normal cancellation. Check for shutdown/cancellation before presigning and after fee lookup or presigning failures, preferring context.Canceled or ErrBatcherShuttingDown over lower-level errors. Log the original presign-path error before returning the shutdown or cancellation error so normal shutdown remains debuggable without changing the returned error. Add a regression test with a presigned helper that cancels the caller context while returning driver.ErrBadConn from SignTx. The test asserts PresignSweepsGroup reports context.Canceled and does not wrap the driver error, and runs against both mock and SQL-backed stores.
1 parent d278a2f commit 9d0a418

2 files changed

Lines changed: 109 additions & 1 deletion

File tree

sweepbatcher/sweep_batcher.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,13 +764,33 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input,
764764
return fmt.Errorf("presignedHelper is not installed")
765765
}
766766

767+
if err := b.shutdownOrCancelErrIfAny(ctx); err != nil {
768+
return err
769+
}
770+
767771
// Find the feerate needed to get into next block. Use conf_target=2,
768772
nextBlockFeeRate, err := b.wallet.EstimateFeeRate(ctx, 2)
769773
if err != nil {
774+
if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil {
775+
infof("PresignSweepsGroup EstimateFeeRate failed "+
776+
"during shutdown, returning %v instead of %v.",
777+
exitErr, err)
778+
779+
return exitErr
780+
}
781+
770782
return fmt.Errorf("failed to get nextBlockFeeRate: %w", err)
771783
}
772784
minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
773785
if err != nil {
786+
if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil {
787+
infof("PresignSweepsGroup MinRelayFee failed during "+
788+
"shutdown, returning %v instead of %v.",
789+
exitErr, err)
790+
791+
return exitErr
792+
}
793+
774794
return fmt.Errorf("failed to get minRelayFeeRate: %w", err)
775795
}
776796
destPkscript, err := txscript.PayToAddrScript(destAddress)
@@ -798,10 +818,23 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input,
798818
// outpoint in the batch.
799819
primarySweepID := sweeps[0].outpoint
800820

801-
return presign(
821+
err = presign(
802822
ctx, b.presignedHelper, destAddress, primarySweepID, sweeps,
803823
nextBlockFeeRate, minRelayFeeRate,
804824
)
825+
if err != nil {
826+
if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil {
827+
infof("PresignSweepsGroup presign failed during "+
828+
"shutdown, returning %v instead of %v.",
829+
exitErr, err)
830+
831+
return exitErr
832+
}
833+
834+
return err
835+
}
836+
837+
return nil
805838
}
806839

807840
// AddSweep loads information about sweeps from the store and fee rate source,

sweepbatcher/sweep_batcher_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3960,6 +3960,81 @@ func TestRunReturnsContextErrorOnErrChanCancellation(t *testing.T) {
39603960
runTests(t, testRunReturnsContextErrorOnErrChanCancellation)
39613961
}
39623962

3963+
// cancelingPresignedHelper is a PresignedHelper implementation that cancels
3964+
// the caller context while returning a driver-level signing error.
3965+
type cancelingPresignedHelper struct {
3966+
cancel context.CancelFunc
3967+
}
3968+
3969+
// DestPkScript satisfies the PresignedHelper interface. It is not used by
3970+
// PresignSweepsGroup, which already receives the destination address directly.
3971+
func (h *cancelingPresignedHelper) DestPkScript(context.Context,
3972+
wire.OutPoint) ([]byte, error) {
3973+
3974+
return nil, nil
3975+
}
3976+
3977+
// SignTx cancels the caller context and returns a driver-level error, matching
3978+
// the shutdown race this test exercises.
3979+
func (h *cancelingPresignedHelper) SignTx(context.Context, wire.OutPoint,
3980+
*wire.MsgTx, btcutil.Amount, chainfee.SatPerKWeight,
3981+
chainfee.SatPerKWeight, bool) (*wire.MsgTx, error) {
3982+
3983+
h.cancel()
3984+
3985+
return nil, driver.ErrBadConn
3986+
}
3987+
3988+
// CleanupTransactions satisfies the PresignedHelper interface. It is not
3989+
// exercised by this presigning-only test.
3990+
func (h *cancelingPresignedHelper) CleanupTransactions(context.Context,
3991+
[]wire.OutPoint) error {
3992+
3993+
return nil
3994+
}
3995+
3996+
// testPresignSweepsGroupReturnsContextErrorOnCancellation asserts that
3997+
// PresignSweepsGroup returns the context cancellation error if presigning fails
3998+
// while the caller context is being canceled.
3999+
func testPresignSweepsGroupReturnsContextErrorOnCancellation(t *testing.T,
4000+
_ testStore, batcherStore testBatcherStore) {
4001+
4002+
defer test.Guard(t)()
4003+
4004+
lnd := test.NewMockLnd()
4005+
ctx, cancel := context.WithCancel(t.Context())
4006+
4007+
// The store is not used by PresignSweepsGroup, but runTests passes
4008+
// both mock and SQL-backed stores so the test stays consistent with
4009+
// the rest of this file.
4010+
batcher := NewBatcher(
4011+
lnd.WalletKit, lnd.ChainNotifier, lnd.Signer,
4012+
testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams,
4013+
batcherStore, nil, WithPresignedHelper(
4014+
&cancelingPresignedHelper{cancel: cancel},
4015+
),
4016+
)
4017+
4018+
err := batcher.PresignSweepsGroup(
4019+
ctx, []Input{{
4020+
Value: btcutil.Amount(1_000_000),
4021+
Outpoint: wire.OutPoint{
4022+
Hash: chainhash.Hash{3, 3},
4023+
Index: 1,
4024+
},
4025+
}}, sweepTimeout, destAddr, nil,
4026+
)
4027+
require.ErrorIs(t, err, context.Canceled)
4028+
require.NotErrorIs(t, err, driver.ErrBadConn)
4029+
}
4030+
4031+
// TestPresignSweepsGroupReturnsContextErrorOnCancellation asserts that
4032+
// PresignSweepsGroup returns the context cancellation error if presigning fails
4033+
// while the caller context is being canceled.
4034+
func TestPresignSweepsGroupReturnsContextErrorOnCancellation(t *testing.T) {
4035+
runTests(t, testPresignSweepsGroupReturnsContextErrorOnCancellation)
4036+
}
4037+
39634038
// testSweepFetcher tests providing custom sweep fetcher to Batcher.
39644039
func testSweepFetcher(t *testing.T, store testStore,
39654040
batcherStore testBatcherStore) {

0 commit comments

Comments
 (0)