Skip to content

Commit

Permalink
sweep: make sure nil tx is handled
Browse files Browse the repository at this point in the history
After previous commit, it should be clear that the tx may be failed to
created in a `TxFailed` event. We now make sure to catch it to avoid
panic.
  • Loading branch information
yyforyongyu committed Nov 11, 2024
1 parent 29c3f66 commit 9a213eb
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 11 deletions.
18 changes: 15 additions & 3 deletions sweep/fee_bumper.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,16 @@ const (
// TxPublished is sent when the broadcast attempt is finished.
TxPublished BumpEvent = iota

// TxFailed is sent when the broadcast attempt fails.
// TxFailed is sent when the tx has encountered a fee-related error
// during its creation or broadcast, or an internal error from the fee
// bumper. In either case the inputs in this tx should be retried with
// either a different grouping strategy or an increased budget.
//
// NOTE: We also send this event when there's a third party spend
// event, and the sweeper will handle cleaning this up.
//
// TODO(yy): Remove the above usage once we remove sweeping non-CPFP
// anchors.
TxFailed

// TxReplaced is sent when the original tx is replaced by a new one.
Expand Down Expand Up @@ -270,8 +279,11 @@ func (b *BumpResult) String() string {
// Validate validates the BumpResult so it's safe to use.
func (b *BumpResult) Validate() error {
// Every result must have a tx except the error or fail case.
if b.Tx == nil && b.Event != TxFatal {
return fmt.Errorf("%w: nil tx", ErrInvalidBumpResult)
if b.Tx == nil {
isFail := b.Event == TxFailed || b.Event == TxFatal
if !isFail {
return fmt.Errorf("%w: nil tx", ErrInvalidBumpResult)
}
}

// Every result must have a known event.
Expand Down
14 changes: 7 additions & 7 deletions sweep/fee_bumper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,6 @@ func TestBumpResultValidate(t *testing.T) {
}
require.ErrorIs(t, b.Validate(), ErrInvalidBumpResult)

// A failed event without a tx will give an error.
b = BumpResult{
Event: TxFailed,
Err: errDummy,
}
require.ErrorIs(t, b.Validate(), ErrInvalidBumpResult)

// A fatal event without a failure reason will give an error.
b = BumpResult{
Event: TxFailed,
Expand All @@ -118,6 +111,13 @@ func TestBumpResultValidate(t *testing.T) {
}
require.NoError(t, b.Validate())

// Tx is allowed to be nil in a TxFailed event.
b = BumpResult{
Event: TxFailed,
Err: errDummy,
}
require.NoError(t, b.Validate())

// Tx is allowed to be nil in a TxFatal event.
b = BumpResult{
Event: TxFatal,
Expand Down
13 changes: 12 additions & 1 deletion sweep/sweeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,14 @@ func (s *UtxoSweeper) monitorFeeBumpResult(set InputSet,
// in sweeper and rely solely on this event to mark
// inputs as Swept?
if r.Event == TxConfirmed || r.Event == TxFailed {
// Exit if the tx is failed to be created.
if r.Tx == nil {
log.Debugf("Received %v for nil tx, "+
"exit monitor", r.Event)

return
}

log.Debugf("Received %v for sweep tx %v, exit "+
"fee bump monitor", r.Event,
r.Tx.TxHash())
Expand All @@ -1690,7 +1698,10 @@ func (s *UtxoSweeper) handleBumpEventTxFailed(resp *bumpResp) {
r := resp.result
tx, err := r.Tx, r.Err

log.Warnf("Fee bump attempt failed for tx=%v: %v", tx.TxHash(), err)
if tx != nil {
log.Warnf("Fee bump attempt failed for tx=%v: %v", tx.TxHash(),
err)
}

// NOTE: When marking the inputs as failed, we are using the input set
// instead of the inputs found in the tx. This is fine for current
Expand Down

0 comments on commit 9a213eb

Please sign in to comment.