From 719ca5b229336ead40daf7d971e91feb726c3c10 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 30 Apr 2024 17:39:45 +0800 Subject: [PATCH] sweep: remove redundant error from `Broadcast` --- sweep/fee_bumper.go | 10 +++++----- sweep/fee_bumper_test.go | 15 +++++---------- sweep/mock_test.go | 6 +++--- sweep/sweeper.go | 11 +---------- sweep/sweeper_test.go | 9 ++------- 5 files changed, 16 insertions(+), 35 deletions(-) diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index 1e159fa12..c2902bb60 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -65,7 +65,7 @@ type Bumper interface { // and monitors its confirmation status for potential fee bumping. It // returns a chan that the caller can use to receive updates about the // broadcast result and potential RBF attempts. - Broadcast(req *BumpRequest) (<-chan *BumpResult, error) + Broadcast(req *BumpRequest) <-chan *BumpResult } // BumpEvent represents the event of a fee bumping attempt. @@ -382,9 +382,9 @@ func (t *TxPublisher) isNeutrinoBackend() bool { // RBF-compliant unless the budget specified cannot cover the fee. // // NOTE: part of the Bumper interface. -func (t *TxPublisher) Broadcast(req *BumpRequest) (<-chan *BumpResult, error) { - log.Tracef("Received broadcast request: %s", lnutils.SpewLogClosure( - req)) +func (t *TxPublisher) Broadcast(req *BumpRequest) <-chan *BumpResult { + log.Tracef("Received broadcast request: %s", + lnutils.SpewLogClosure(req)) // Store the request. requestID, record := t.storeInitialRecord(req) @@ -398,7 +398,7 @@ func (t *TxPublisher) Broadcast(req *BumpRequest) (<-chan *BumpResult, error) { t.handleInitialBroadcast(record, requestID) } - return subscriber, nil + return subscriber } // storeInitialRecord initializes a monitor record and saves it in the map. diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index 50c1fdced..ba41d6569 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -978,8 +978,7 @@ func TestBroadcast(t *testing.T) { } // Send the req and expect no error. - resultChan, err := tp.Broadcast(req) - require.NoError(t, err) + resultChan := tp.Broadcast(req) require.NotNil(t, resultChan) // Validate the record was stored. @@ -1029,8 +1028,7 @@ func TestBroadcastImmediate(t *testing.T) { chainfee.SatPerKWeight(0), errDummy).Once() // Send the req and expect no error. - resultChan, err := tp.Broadcast(req) - require.NoError(t, err) + resultChan := tp.Broadcast(req) require.NotNil(t, resultChan) // Validate the record was removed due to an error returned in initial @@ -1541,8 +1539,7 @@ func TestHandleInitialBroadcastSuccess(t *testing.T) { } // Register the testing record use `Broadcast`. - resultChan, err := tp.Broadcast(req) - require.NoError(t, err) + resultChan := tp.Broadcast(req) // Grab the monitor record from the map. rid := tp.requestCounter.Load() @@ -1613,8 +1610,7 @@ func TestHandleInitialBroadcastFail(t *testing.T) { mock.Anything).Return(errDummy).Once() // Register the testing record use `Broadcast`. - resultChan, err := tp.Broadcast(req) - require.NoError(t, err) + resultChan := tp.Broadcast(req) // Grab the monitor record from the map. rid := tp.requestCounter.Load() @@ -1647,8 +1643,7 @@ func TestHandleInitialBroadcastFail(t *testing.T) { mock.Anything, mock.Anything).Return(errDummy).Once() // Register the testing record use `Broadcast`. - resultChan, err = tp.Broadcast(req) - require.NoError(t, err) + resultChan = tp.Broadcast(req) // Grab the monitor record from the map. rid = tp.requestCounter.Load() diff --git a/sweep/mock_test.go b/sweep/mock_test.go index d42b0320d..f9471f22a 100644 --- a/sweep/mock_test.go +++ b/sweep/mock_test.go @@ -284,14 +284,14 @@ type MockBumper struct { var _ Bumper = (*MockBumper)(nil) // Broadcast broadcasts the transaction to the network. -func (m *MockBumper) Broadcast(req *BumpRequest) (<-chan *BumpResult, error) { +func (m *MockBumper) Broadcast(req *BumpRequest) <-chan *BumpResult { args := m.Called(req) if args.Get(0) == nil { - return nil, args.Error(1) + return nil } - return args.Get(0).(chan *BumpResult), args.Error(1) + return args.Get(0).(chan *BumpResult) } // MockFeeFunction is a mock implementation of the FeeFunction interface. diff --git a/sweep/sweeper.go b/sweep/sweeper.go index ed3954801..fb4f212fc 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -838,16 +838,7 @@ func (s *UtxoSweeper) sweep(set InputSet) error { // Broadcast will return a read-only chan that we will listen to for // this publish result and future RBF attempt. - resp, err := s.cfg.Publisher.Broadcast(req) - if err != nil { - log.Errorf("Initial broadcast failed: %v, inputs=\n%v", err, - inputTypeSummary(set.Inputs())) - - // TODO(yy): find out which input is causing the failure. - s.markInputsPublishFailed(set) - - return err - } + resp := s.cfg.Publisher.Broadcast(req) // Successfully sent the broadcast attempt, we now handle the result by // subscribing to the result chan and listen for future updates about diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index 2e1be71c2..5cdbba297 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -673,13 +673,8 @@ func TestSweepPendingInputs(t *testing.T) { setNeedWallet, normalSet, }) - // Mock `Broadcast` to return an error. This should cause the - // `createSweepTx` inside `sweep` to fail. This is done so we can - // terminate the method early as we are only interested in testing the - // workflow in `sweepPendingInputs`. We don't need to test `sweep` here - // as it should be tested in its own unit test. - dummyErr := errors.New("dummy error") - publisher.On("Broadcast", mock.Anything).Return(nil, dummyErr).Twice() + // Mock `Broadcast` to return a result. + publisher.On("Broadcast", mock.Anything).Return(nil).Twice() // Call the method under test. s.sweepPendingInputs(pis)