From bd5eec8e1feb114233f4e9a0cd2df18b65b6586f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 21 Feb 2024 16:00:05 +0800 Subject: [PATCH] sweep: refactor `markInputsPendingPublish` to take `InputSet` This commit changes `markInputsPendingPublish` to take `InputSet` only. This is needed for the following commits as we won't be able to know the tx being created beforehand, yet we still want to make sure these inputs won't be grouped to another input set as it complicates our RBF process. --- sweep/sweeper.go | 41 +++++------------------ sweep/sweeper_test.go | 77 ++++++++++++++++++------------------------- 2 files changed, 40 insertions(+), 78 deletions(-) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 12b3efc24..92c6646d2 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -794,10 +794,7 @@ func (s *UtxoSweeper) sweep(set InputSet) error { // Reschedule the inputs that we just tried to sweep. This is done in // case the following publish fails, we'd like to update the inputs' // publish attempts and rescue them in the next sweep. - err = s.markInputsPendingPublish(tr, tx.TxIn) - if err != nil { - return err - } + s.markInputsPendingPublish(set) log.Debugf("Publishing sweep tx %v, num_inputs=%v, height=%v", tx.TxHash(), len(tx.TxIn), s.currentHeight) @@ -832,31 +829,19 @@ func (s *UtxoSweeper) sweep(set InputSet) error { return nil } -// markInputsPendingPublish saves the sweeping tx to db and updates the pending -// inputs with the given tx inputs. It also increments the `publishAttempts`. -func (s *UtxoSweeper) markInputsPendingPublish(tr *TxRecord, - inputs []*wire.TxIn) error { - - // Add tx to db before publication, so that we will always know that a - // spend by this tx is ours. Otherwise if the publish doesn't return, - // but did publish, we'd lose track of this tx. Even republication on - // startup doesn't prevent this, because that call returns a double - // spend error then and would also not add the hash to the store. - err := s.cfg.Store.StoreTx(tr) - if err != nil { - return fmt.Errorf("store tx: %w", err) - } - +// markInputsPendingPublish updates the pending inputs with the given tx +// inputs. It also increments the `publishAttempts`. +func (s *UtxoSweeper) markInputsPendingPublish(set InputSet) { // Reschedule sweep. - for _, input := range inputs { - pi, ok := s.pendingInputs[input.PreviousOutPoint] + for _, input := range set.Inputs() { + pi, ok := s.pendingInputs[*input.OutPoint()] if !ok { // It could be that this input is an additional wallet // input that was attached. In that case there also // isn't a pending input to update. log.Debugf("Skipped marking input as pending "+ "published: %v not found in pending inputs", - input.PreviousOutPoint) + input.OutPoint()) continue } @@ -868,7 +853,7 @@ func (s *UtxoSweeper) markInputsPendingPublish(tr *TxRecord, if pi.terminated() { log.Errorf("Expect input %v to not have terminated "+ "state, instead it has %v", - input.PreviousOutPoint, pi.state) + input.OutPoint, pi.state) continue } @@ -876,19 +861,9 @@ func (s *UtxoSweeper) markInputsPendingPublish(tr *TxRecord, // Update the input's state. pi.state = StatePendingPublish - // Record the fees and fee rate of this tx to prepare possible - // RBF. - pi.rbf = fn.Some(RBFInfo{ - Txid: tr.Txid, - FeeRate: chainfee.SatPerKWeight(tr.FeeRate), - Fee: btcutil.Amount(tr.Fee), - }) - // Record another publish attempt. pi.publishAttempts++ } - - return nil } // markInputsPublished updates the sweeping tx in db and marks the list of diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index a870c2789..f6891db92 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -1897,87 +1897,74 @@ func TestMarkInputsPendingPublish(t *testing.T) { require := require.New(t) - // Create a mock sweeper store. - mockStore := NewMockSweeperStore() - - // Create a test TxRecord and a dummy error. - dummyTR := &TxRecord{} - dummyErr := errors.New("dummy error") - // Create a test sweeper. - s := New(&UtxoSweeperConfig{ - Store: mockStore, - }) + s := New(&UtxoSweeperConfig{}) + + // Create a mock input set. + set := &MockInputSet{} + defer set.AssertExpectations(t) // Create three testing inputs. // // inputNotExist specifies an input that's not found in the sweeper's // `pendingInputs` map. - inputNotExist := &wire.TxIn{ - PreviousOutPoint: wire.OutPoint{Index: 1}, - } + inputNotExist := &input.MockInput{} + defer inputNotExist.AssertExpectations(t) + + inputNotExist.On("OutPoint").Return(&wire.OutPoint{Index: 0}) // inputInit specifies a newly created input. - inputInit := &wire.TxIn{ - PreviousOutPoint: wire.OutPoint{Index: 2}, - } - s.pendingInputs[inputInit.PreviousOutPoint] = &pendingInput{ + inputInit := &input.MockInput{} + defer inputInit.AssertExpectations(t) + + inputInit.On("OutPoint").Return(&wire.OutPoint{Index: 1}) + + s.pendingInputs[*inputInit.OutPoint()] = &pendingInput{ state: StateInit, } // inputPendingPublish specifies an input that's about to be published. - inputPendingPublish := &wire.TxIn{ - PreviousOutPoint: wire.OutPoint{Index: 3}, - } - s.pendingInputs[inputPendingPublish.PreviousOutPoint] = &pendingInput{ + inputPendingPublish := &input.MockInput{} + defer inputPendingPublish.AssertExpectations(t) + + inputPendingPublish.On("OutPoint").Return(&wire.OutPoint{Index: 2}) + + s.pendingInputs[*inputPendingPublish.OutPoint()] = &pendingInput{ state: StatePendingPublish, } // inputTerminated specifies an input that's terminated. - inputTerminated := &wire.TxIn{ - PreviousOutPoint: wire.OutPoint{Index: 4}, - } - s.pendingInputs[inputTerminated.PreviousOutPoint] = &pendingInput{ + inputTerminated := &input.MockInput{} + defer inputTerminated.AssertExpectations(t) + + inputTerminated.On("OutPoint").Return(&wire.OutPoint{Index: 3}) + + s.pendingInputs[*inputTerminated.OutPoint()] = &pendingInput{ state: StateExcluded, } - // First, check that when an error is returned from db, it's properly - // returned here. - mockStore.On("StoreTx", dummyTR).Return(dummyErr).Once() - err := s.markInputsPendingPublish(dummyTR, nil) - require.ErrorIs(err, dummyErr) - - // Then, check that the target input has will be correctly marked as - // published. - // - // Mock the store to return nil - mockStore.On("StoreTx", dummyTR).Return(nil).Once() - // Mark the test inputs. We expect the non-exist input and the // inputTerminated to be skipped, and the rest to be marked as pending // publish. - err = s.markInputsPendingPublish(dummyTR, []*wire.TxIn{ + set.On("Inputs").Return([]input.Input{ inputNotExist, inputInit, inputPendingPublish, inputTerminated, }) - require.NoError(err) + s.markInputsPendingPublish(set) // We expect unchanged number of pending inputs. require.Len(s.pendingInputs, 3) // We expect the init input's state to become pending publish. require.Equal(StatePendingPublish, - s.pendingInputs[inputInit.PreviousOutPoint].state) + s.pendingInputs[*inputInit.OutPoint()].state) // We expect the pending-publish to stay unchanged. require.Equal(StatePendingPublish, - s.pendingInputs[inputPendingPublish.PreviousOutPoint].state) + s.pendingInputs[*inputPendingPublish.OutPoint()].state) // We expect the terminated to stay unchanged. require.Equal(StateExcluded, - s.pendingInputs[inputTerminated.PreviousOutPoint].state) - - // Assert mocked statements are executed as expected. - mockStore.AssertExpectations(t) + s.pendingInputs[*inputTerminated.OutPoint()].state) } // TestMarkInputsPublished checks that given a list of inputs with different