From 707fef340b4b8e8c9d6ee144c636841100386898 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 16 May 2026 19:15:59 -0500 Subject: [PATCH] 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. --- sweepbatcher/sweep_batcher.go | 35 +++++++++++++- sweepbatcher/sweep_batcher_test.go | 75 ++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index aade3375..e2c3c1d5 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -764,13 +764,33 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, return fmt.Errorf("presignedHelper is not installed") } + if err := b.shutdownOrCancelErrIfAny(ctx); err != nil { + return err + } + // Find the feerate needed to get into next block. Use conf_target=2, nextBlockFeeRate, err := b.wallet.EstimateFeeRate(ctx, 2) if err != nil { + if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil { + infof("PresignSweepsGroup EstimateFeeRate failed "+ + "during shutdown, returning %v instead of %v.", + exitErr, err) + + return exitErr + } + return fmt.Errorf("failed to get nextBlockFeeRate: %w", err) } minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) if err != nil { + if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil { + infof("PresignSweepsGroup MinRelayFee failed during "+ + "shutdown, returning %v instead of %v.", + exitErr, err) + + return exitErr + } + return fmt.Errorf("failed to get minRelayFeeRate: %w", err) } destPkscript, err := txscript.PayToAddrScript(destAddress) @@ -798,10 +818,23 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, // outpoint in the batch. primarySweepID := sweeps[0].outpoint - return presign( + err = presign( ctx, b.presignedHelper, destAddress, primarySweepID, sweeps, nextBlockFeeRate, minRelayFeeRate, ) + if err != nil { + if exitErr := b.shutdownOrCancelErrIfAny(ctx); exitErr != nil { + infof("PresignSweepsGroup presign failed during "+ + "shutdown, returning %v instead of %v.", + exitErr, err) + + return exitErr + } + + return err + } + + return nil } // AddSweep loads information about sweeps from the store and fee rate source, diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index c8f68517..648dc4c6 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -3960,6 +3960,81 @@ func TestRunReturnsContextErrorOnErrChanCancellation(t *testing.T) { runTests(t, testRunReturnsContextErrorOnErrChanCancellation) } +// cancelingPresignedHelper is a PresignedHelper implementation that cancels +// the caller context while returning a driver-level signing error. +type cancelingPresignedHelper struct { + cancel context.CancelFunc +} + +// DestPkScript satisfies the PresignedHelper interface. It is not used by +// PresignSweepsGroup, which already receives the destination address directly. +func (h *cancelingPresignedHelper) DestPkScript(context.Context, + wire.OutPoint) ([]byte, error) { + + return nil, nil +} + +// SignTx cancels the caller context and returns a driver-level error, matching +// the shutdown race this test exercises. +func (h *cancelingPresignedHelper) SignTx(context.Context, wire.OutPoint, + *wire.MsgTx, btcutil.Amount, chainfee.SatPerKWeight, + chainfee.SatPerKWeight, bool) (*wire.MsgTx, error) { + + h.cancel() + + return nil, driver.ErrBadConn +} + +// CleanupTransactions satisfies the PresignedHelper interface. It is not +// exercised by this presigning-only test. +func (h *cancelingPresignedHelper) CleanupTransactions(context.Context, + []wire.OutPoint) error { + + return nil +} + +// testPresignSweepsGroupReturnsContextErrorOnCancellation asserts that +// PresignSweepsGroup returns the context cancellation error if presigning fails +// while the caller context is being canceled. +func testPresignSweepsGroupReturnsContextErrorOnCancellation(t *testing.T, + _ testStore, batcherStore testBatcherStore) { + + defer test.Guard(t)() + + lnd := test.NewMockLnd() + ctx, cancel := context.WithCancel(t.Context()) + + // The store is not used by PresignSweepsGroup, but runTests passes + // both mock and SQL-backed stores so the test stays consistent with + // the rest of this file. + batcher := NewBatcher( + lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, + testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, + batcherStore, nil, WithPresignedHelper( + &cancelingPresignedHelper{cancel: cancel}, + ), + ) + + err := batcher.PresignSweepsGroup( + ctx, []Input{{ + Value: btcutil.Amount(1_000_000), + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{3, 3}, + Index: 1, + }, + }}, sweepTimeout, destAddr, nil, + ) + require.ErrorIs(t, err, context.Canceled) + require.NotErrorIs(t, err, driver.ErrBadConn) +} + +// TestPresignSweepsGroupReturnsContextErrorOnCancellation asserts that +// PresignSweepsGroup returns the context cancellation error if presigning fails +// while the caller context is being canceled. +func TestPresignSweepsGroupReturnsContextErrorOnCancellation(t *testing.T) { + runTests(t, testPresignSweepsGroupReturnsContextErrorOnCancellation) +} + // testSweepFetcher tests providing custom sweep fetcher to Batcher. func testSweepFetcher(t *testing.T, store testStore, batcherStore testBatcherStore) {