From 4134b1c00a7885e509a900f043c3ea9a22855225 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 3 Apr 2024 16:13:49 +0800 Subject: [PATCH] sweep: make sure max fee rate can be reached Previously we don't allow confTarget to be 0, which ended up the final position being never reached. We fix it here by allowing confTarget to be 0 in case the deadline has already been passed for a given input. --- sweep/fee_bumper.go | 4 ++-- sweep/fee_bumper_test.go | 13 ++++++++----- sweep/fee_function.go | 15 ++++++++------- sweep/fee_function_test.go | 25 +++++++++++++++++-------- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/sweep/fee_bumper.go b/sweep/fee_bumper.go index e7569d5da..c6ea8aad4 100644 --- a/sweep/fee_bumper.go +++ b/sweep/fee_bumper.go @@ -951,11 +951,11 @@ func calcCurrentConfTarget(currentHeight, deadline int32) uint32 { // If we are already past the deadline, we will set the conf target to // be 1. - if deadlineDelta <= 0 { + if deadlineDelta < 0 { log.Warnf("Deadline is %d blocks behind current height %v", -deadlineDelta, currentHeight) - confTarget = 1 + confTarget = 0 } else { confTarget = uint32(deadlineDelta) } diff --git a/sweep/fee_bumper_test.go b/sweep/fee_bumper_test.go index fe45910a5..38036934c 100644 --- a/sweep/fee_bumper_test.go +++ b/sweep/fee_bumper_test.go @@ -189,9 +189,9 @@ func TestCalcCurrentConfTarget(t *testing.T) { require.EqualValues(t, 100, conf) // When the current block height is 200 and deadline height is 100, the - // conf target should be 1 since the deadline has passed. + // conf target should be 0 since the deadline has passed. conf = calcCurrentConfTarget(int32(200), int32(100)) - require.EqualValues(t, 1, conf) + require.EqualValues(t, 0, conf) } // TestInitializeFeeFunction tests the initialization of the fee function. @@ -218,7 +218,8 @@ func TestInitializeFeeFunction(t *testing.T) { DeliveryAddress: changePkScript, Inputs: []input.Input{&inp}, Budget: btcutil.Amount(1000), - MaxFeeRate: feerate, + MaxFeeRate: feerate * 10, + DeadlineHeight: 10, } // Mock the fee estimator to return an error. @@ -889,7 +890,8 @@ func TestBroadcastSuccess(t *testing.T) { DeliveryAddress: changePkScript, Inputs: []input.Input{&inp}, Budget: btcutil.Amount(1000), - MaxFeeRate: feerate, + MaxFeeRate: feerate * 10, + DeadlineHeight: 10, } // Send the req and expect no error. @@ -930,7 +932,8 @@ func TestBroadcastFail(t *testing.T) { DeliveryAddress: changePkScript, Inputs: []input.Input{&inp}, Budget: btcutil.Amount(1000), - MaxFeeRate: feerate, + MaxFeeRate: feerate * 10, + DeadlineHeight: 10, } // Mock the fee estimator to return the testing fee rate. diff --git a/sweep/fee_function.go b/sweep/fee_function.go index 59da96d78..acf6e3d84 100644 --- a/sweep/fee_function.go +++ b/sweep/fee_function.go @@ -113,9 +113,15 @@ var _ FeeFunction = (*LinearFeeFunction)(nil) func NewLinearFeeFunction(maxFeeRate chainfee.SatPerKWeight, confTarget uint32, estimator chainfee.Estimator) (*LinearFeeFunction, error) { - // Sanity check conf target. + // If the deadline has already been reached, there's nothing the fee + // function can do. In this case, we'll use the max fee rate + // immediately. if confTarget == 0 { - return nil, fmt.Errorf("width must be greater than zero") + return &LinearFeeFunction{ + startingFeeRate: maxFeeRate, + endingFeeRate: maxFeeRate, + currentFeeRate: maxFeeRate, + }, nil } l := &LinearFeeFunction{ @@ -193,11 +199,6 @@ func (l *LinearFeeFunction) Increment() (bool, error) { // // NOTE: part of the FeeFunction interface. func (l *LinearFeeFunction) IncreaseFeeRate(confTarget uint32) (bool, error) { - // If the new position is already at the end, we return an error. - if confTarget == 0 { - return false, ErrMaxPosition - } - newPosition := uint32(0) // Only calculate the new position when the conf target is less than diff --git a/sweep/fee_function_test.go b/sweep/fee_function_test.go index fb75bcc1c..461f9e3bb 100644 --- a/sweep/fee_function_test.go +++ b/sweep/fee_function_test.go @@ -22,10 +22,16 @@ func TestLinearFeeFunctionNew(t *testing.T) { minRelayFeeRate := chainfee.SatPerKWeight(100) confTarget := uint32(6) - // Assert init fee function with zero conf value returns an error. + // Assert init fee function with zero conf value will end up using the + // max fee rate. f, err := NewLinearFeeFunction(maxFeeRate, 0, estimator) - rt.ErrorContains(err, "width must be greater than zero") - rt.Nil(f) + rt.NoError(err) + rt.NotNil(f) + + // Assert the internal state. + rt.Equal(maxFeeRate, f.startingFeeRate) + rt.Equal(maxFeeRate, f.endingFeeRate) + rt.Equal(maxFeeRate, f.currentFeeRate) // When the fee estimator returns an error, it's returned. // @@ -240,11 +246,6 @@ func TestLinearFeeFunctionIncreaseFeeRate(t *testing.T) { rt.NoError(err) rt.False(increased) - // Test that when we use a conf target of 0, we get an error. - increased, err = f.IncreaseFeeRate(0) - rt.ErrorIs(err, ErrMaxPosition) - rt.False(increased) - // We now increase the fee rate from conf target 8 to 1 and assert we // get no error and true. for i := uint32(1); i < confTarget; i++ { @@ -262,4 +263,12 @@ func TestLinearFeeFunctionIncreaseFeeRate(t *testing.T) { // Check public method returns the expected fee rate. rt.Equal(estimatedFeeRate+delta, f.FeeRate()) } + + // Test that when we use a conf target of 0, we get the ending fee + // rate. + increased, err = f.IncreaseFeeRate(0) + rt.NoError(err) + rt.True(increased) + rt.Equal(confTarget, f.position) + rt.Equal(maxFeeRate, f.currentFeeRate) }