Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sweeper: rename Failed to Fatal and minor refactor #9446

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+266 −158
Open

sweeper: rename Failed to Fatal and minor refactor #9446

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f6e2137
sweep: rename `Failed` to `Fatal`
yyforyongyu Jan 20, 2025
0ac5bfa
sweep: shorten `storeRecord` method signature
yyforyongyu Jan 20, 2025
0ae9f1a
sweep: add `requestID` to `monitorRecord`
yyforyongyu Jan 20, 2025
ed4c7f7
sweep: refactor `storeRecord` to `updateRecord`
yyforyongyu Jan 20, 2025
a738e7f
docs: update release notes
yyforyongyu Jan 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions sweep/sweeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,12 @@ const (
// sweeping transactions confirmed, the remaining two will be excluded.
Excluded

// Failed is the state when a pending input has too many failed publish
// atttempts or unknown broadcast error is returned.
Failed
// Fatal is the final state of a pending input. Inputs ending in this
// state won't be retried. This could happen,
// - when a pending input has too many failed publish attempts;
// - the input has been spent by another party;
// - unknown broadcast error is returned.
Fatal
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another option could be "Terminal" since then it is very clear it wont be tried again

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but fatal is good too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fatal

)

// String gives a human readable text for the sweep states.
Expand All @@ -145,8 +148,8 @@ func (s SweepState) String() string {
case Excluded:
return "Excluded"

case Failed:
return "Failed"
case Fatal:
return "Fatal"

default:
return "Unknown"
Expand Down Expand Up @@ -215,7 +218,7 @@ func (p *SweeperInput) terminated() bool {
// If the input has reached a final state, that it's either
// been swept, or failed, or excluded, we will remove it from
// our sweeper.
case Failed, Swept, Excluded:
case Fatal, Swept, Excluded:
return true

default:
Expand Down Expand Up @@ -1264,7 +1267,7 @@ func (s *UtxoSweeper) handleNewInput(input *sweepInputMessage) error {
)
if err != nil {
err := fmt.Errorf("wait for spend: %w", err)
s.markInputFailed(pi, err)
s.markInputFatal(pi, err)

return err
}
Expand Down Expand Up @@ -1477,12 +1480,12 @@ func (s *UtxoSweeper) markInputsSwept(tx *wire.MsgTx, isOurTx bool) {
}
}

// markInputFailed marks the given input as failed and won't be retried. It
// markInputFatal marks the given input as fatal and won't be retried. It
// will also notify all the subscribers of this input.
func (s *UtxoSweeper) markInputFailed(pi *SweeperInput, err error) {
func (s *UtxoSweeper) markInputFatal(pi *SweeperInput, err error) {
log.Errorf("Failed to sweep input: %v, error: %v", pi, err)

pi.state = Failed
pi.state = Fatal

s.signalResult(pi, Result{Err: err})
}
Expand Down Expand Up @@ -1784,15 +1787,15 @@ func (s *UtxoSweeper) handleBumpEventTxFatal(resp *bumpResp) error {
}
}

// Mark the inputs as failed.
s.markInputsFailed(resp.set, r.Err)
// Mark the inputs as fatal.
s.markInputsFatal(resp.set, r.Err)

return nil
}

// markInputsFailed marks all inputs found in the tx as failed. It will also
// markInputsFatal marks all inputs in the input set as failed. It will also
// notify all the subscribers of these inputs.
func (s *UtxoSweeper) markInputsFailed(set InputSet, err error) {
func (s *UtxoSweeper) markInputsFatal(set InputSet, err error) {
for _, inp := range set.Inputs() {
outpoint := inp.OutPoint()

Expand All @@ -1816,7 +1819,7 @@ func (s *UtxoSweeper) markInputsFailed(set InputSet, err error) {
continue
}

s.markInputFailed(input, err)
s.markInputFatal(input, err)
}
}

Expand Down
40 changes: 20 additions & 20 deletions sweep/sweeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,21 +215,21 @@ func TestMarkInputsPublishFailed(t *testing.T) {
// published.
// - inputSwept specifies an input that's swept.
// - inputExcluded specifies an input that's excluded.
// - inputFailed specifies an input that's failed.
// - inputFatal specifies an input that's fatal.
var (
inputInit = createMockInput(t, s, Init)
inputPendingPublish = createMockInput(t, s, PendingPublish)
inputPublished = createMockInput(t, s, Published)
inputPublishFailed = createMockInput(t, s, PublishFailed)
inputSwept = createMockInput(t, s, Swept)
inputExcluded = createMockInput(t, s, Excluded)
inputFailed = createMockInput(t, s, Failed)
inputFatal = createMockInput(t, s, Fatal)
)

// Gather all inputs.
set.On("Inputs").Return([]input.Input{
inputInit, inputPendingPublish, inputPublished,
inputPublishFailed, inputSwept, inputExcluded, inputFailed,
inputPublishFailed, inputSwept, inputExcluded, inputFatal,
})

// Mark the test inputs. We expect the non-exist input and the
Expand Down Expand Up @@ -264,7 +264,7 @@ func TestMarkInputsPublishFailed(t *testing.T) {
require.Equal(Excluded, s.inputs[inputExcluded.OutPoint()].state)

// We expect the failed input to stay unchanged.
require.Equal(Failed, s.inputs[inputFailed.OutPoint()].state)
require.Equal(Fatal, s.inputs[inputFatal.OutPoint()].state)

// Assert mocked statements are executed as expected.
mockStore.AssertExpectations(t)
Expand Down Expand Up @@ -437,7 +437,7 @@ func TestUpdateSweeperInputs(t *testing.T) {
// These inputs won't hit RequiredLockTime so we won't mock.
input4 := &SweeperInput{state: Swept, Input: inp1}
input5 := &SweeperInput{state: Excluded, Input: inp1}
input6 := &SweeperInput{state: Failed, Input: inp1}
input6 := &SweeperInput{state: Fatal, Input: inp1}

// Mock the input to have a locktime in the future so it will NOT be
// returned.
Expand Down Expand Up @@ -575,7 +575,7 @@ func TestDecideStateAndRBFInfo(t *testing.T) {
require.Equal(Published, state)
}

// TestMarkInputFailed checks that the input is marked as failed as expected.
// TestMarkInputFatal checks that the input is marked as expected.
func TestMarkInputFailed(t *testing.T) {
t.Parallel()

Expand All @@ -596,10 +596,10 @@ func TestMarkInputFailed(t *testing.T) {
}

// Call the method under test.
s.markInputFailed(pi, errors.New("dummy error"))
s.markInputFatal(pi, errors.New("dummy error"))

// Assert the state is updated.
require.Equal(t, Failed, pi.state)
require.Equal(t, Fatal, pi.state)
}

// TestSweepPendingInputs checks that `sweepPendingInputs` correctly executes
Expand Down Expand Up @@ -1102,41 +1102,41 @@ func TestMarkInputsFailed(t *testing.T) {
// published.
// - inputSwept specifies an input that's swept.
// - inputExcluded specifies an input that's excluded.
// - inputFailed specifies an input that's failed.
// - inputFatal specifies an input that's fatal.
var (
inputInit = createMockInput(t, s, Init)
inputPendingPublish = createMockInput(t, s, PendingPublish)
inputPublished = createMockInput(t, s, Published)
inputPublishFailed = createMockInput(t, s, PublishFailed)
inputSwept = createMockInput(t, s, Swept)
inputExcluded = createMockInput(t, s, Excluded)
inputFailed = createMockInput(t, s, Failed)
inputFatal = createMockInput(t, s, Fatal)
)

// Gather all inputs.
set.On("Inputs").Return([]input.Input{
inputInit, inputPendingPublish, inputPublished,
inputPublishFailed, inputSwept, inputExcluded, inputFailed,
inputPublishFailed, inputSwept, inputExcluded, inputFatal,
})

// Mark the test inputs. We expect the non-exist input and
// inputSwept/inputExcluded/inputFailed to be skipped.
s.markInputsFailed(set, errDummy)
// inputSwept/inputExcluded/inputFatal to be skipped.
s.markInputsFatal(set, errDummy)

// We expect unchanged number of pending inputs.
require.Len(s.inputs, 7)

// We expect the init input's to be marked as failed.
require.Equal(Failed, s.inputs[inputInit.OutPoint()].state)
// We expect the init input's to be marked as fatal.
require.Equal(Fatal, s.inputs[inputInit.OutPoint()].state)

// We expect the pending-publish input to be marked as failed.
require.Equal(Failed, s.inputs[inputPendingPublish.OutPoint()].state)
require.Equal(Fatal, s.inputs[inputPendingPublish.OutPoint()].state)

// We expect the published input to be marked as failed.
require.Equal(Failed, s.inputs[inputPublished.OutPoint()].state)
// We expect the published input to be marked as fatal.
require.Equal(Fatal, s.inputs[inputPublished.OutPoint()].state)

// We expect the publish failed input to be markd as failed.
require.Equal(Failed, s.inputs[inputPublishFailed.OutPoint()].state)
require.Equal(Fatal, s.inputs[inputPublishFailed.OutPoint()].state)

// We expect the swept input to stay unchanged.
require.Equal(Swept, s.inputs[inputSwept.OutPoint()].state)
Expand All @@ -1145,7 +1145,7 @@ func TestMarkInputsFailed(t *testing.T) {
require.Equal(Excluded, s.inputs[inputExcluded.OutPoint()].state)

// We expect the failed input to stay unchanged.
require.Equal(Failed, s.inputs[inputFailed.OutPoint()].state)
require.Equal(Fatal, s.inputs[inputFatal.OutPoint()].state)
}

// TestHandleBumpEventTxFatal checks that `handleBumpEventTxFatal` correctly
Expand Down