sweep: make sure the budget is used up at deadlineHeight-1

sweep/fee_function.go

@@ -86,11 +86,14 @@ type LinearFeeFunction struct {
 	currentFeeRate chainfee.SatPerKWeight
 
 	// width is the number of blocks between the starting block height
-	// and the deadline block height.
+	// and the deadline block height minus one.
+	//
+	// NOTE: We do minus one from the conf target here because we want to
+	// max out the budget before the deadline height is reached.
 	width uint32
 
-	// position is the number of blocks between the starting block height
+	// position is the fee function's current position, given a width of w,
-	// and the current block height.
+	// a valid position should lie in range [0, w].
 	position uint32
 
 	// deltaFeeRate is the fee rate (msat/kw) increase per block.
@@ -116,10 +119,10 @@ func NewLinearFeeFunction(maxFeeRate chainfee.SatPerKWeight,
 	startingFeeRate fn.Option[chainfee.SatPerKWeight]) (
 	*LinearFeeFunction, error) {
 
-	// If the deadline has already been reached, there's nothing the fee
+	// If the deadline is one block away or has already been reached,
-	// function can do. In this case, we'll use the max fee rate
+	// there's nothing the fee function can do. In this case, we'll use the
-	// immediately.
+	// max fee rate immediately.
-	if confTarget == 0 {
+	if confTarget <= 1 {
 		return &LinearFeeFunction{
 			startingFeeRate: maxFeeRate,
 			endingFeeRate:   maxFeeRate,
@@ -129,7 +132,7 @@ func NewLinearFeeFunction(maxFeeRate chainfee.SatPerKWeight,
 
 	l := &LinearFeeFunction{
 		endingFeeRate: maxFeeRate,
-		width:         confTarget,
+		width:         confTarget - 1,
 		estimator:     estimator,
 	}
 
@@ -153,18 +156,18 @@ func NewLinearFeeFunction(maxFeeRate chainfee.SatPerKWeight,
 
 	// The starting and ending fee rates are in sat/kw, so we need to
 	// convert them to msat/kw by multiplying by 1000.
-	delta := btcutil.Amount(end - start).MulF64(1000 / float64(confTarget))
+	delta := btcutil.Amount(end - start).MulF64(1000 / float64(l.width))
 	l.deltaFeeRate = mSatPerKWeight(delta)
 
 	// We only allow the delta to be zero if the width is one - when the
 	// delta is zero, it means the starting and ending fee rates are the
 	// same, which means there's nothing to increase, so any width greater
 	// than 1 doesn't provide any utility. This could happen when the
-	// sweeper is offered to sweep an input that has passed its deadline.
+	// budget is too small.
 	if l.deltaFeeRate == 0 && l.width != 1 {
 		log.Errorf("Failed to init fee function: startingFeeRate=%v, "+
 			"endingFeeRate=%v, width=%v, delta=%v", start, end,
-			confTarget, l.deltaFeeRate)
+			l.width, l.deltaFeeRate)
 
 		return nil, fmt.Errorf("fee rate delta is zero")
 	}
@@ -175,7 +178,7 @@ func NewLinearFeeFunction(maxFeeRate chainfee.SatPerKWeight,
 
 	log.Debugf("Linear fee function initialized with startingFeeRate=%v, "+
 		"endingFeeRate=%v, width=%v, delta=%v", start, end,
-		confTarget, l.deltaFeeRate)
+		l.width, l.deltaFeeRate)
 
 	return l, nil
 }
@@ -211,12 +214,12 @@ func (l *LinearFeeFunction) IncreaseFeeRate(confTarget uint32) (bool, error) {
 	newPosition := uint32(0)
 
 	// Only calculate the new position when the conf target is less than
-	// the function's width - the width is the initial conf target, and we
+	// the function's width - the width is the initial conf target-1, and
-	// expect the current conf target to decrease over time. However, we
+	// we expect the current conf target to decrease over time. However, we
 	// still allow the supplied conf target to be greater than the width,
 	// and we won't increase the fee rate in that case.
-	if confTarget < l.width {
+	if confTarget < l.width+1 {
-		newPosition = l.width - confTarget
+		newPosition = l.width + 1 - confTarget
 		log.Tracef("Increasing position from %v to %v", l.position,
 			newPosition)
 	}
@@ -290,7 +293,7 @@ func (l *LinearFeeFunction) estimateFeeRate(
 	// (1008), we will use the min relay fee instead.
 	if confTarget >= chainfee.MaxBlockTarget {
 		minFeeRate := l.estimator.RelayFeePerKW()
-		log.Debugf("Conf target %v is greater than max block target, "+
+		log.Infof("Conf target %v is greater than max block target, "+
 			"using min relay fee rate %v", confTarget, minFeeRate)
 
 		return minFeeRate, nil

sweep/fee_function_test.go

@@ -8,22 +8,20 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestLinearFeeFunctionNew tests the NewLinearFeeFunction function.
+// TestLinearFeeFunctionNewMaxFeeRateUsed tests when the conf target is <= 1,
-func TestLinearFeeFunctionNew(t *testing.T) {
+// the max fee rate is used.
+func TestLinearFeeFunctionNewMaxFeeRateUsed(t *testing.T) {
 	t.Parallel()
 
 	rt := require.New(t)
 
 	// Create a mock fee estimator.
 	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
 
 	// Create testing params.
 	maxFeeRate := chainfee.SatPerKWeight(10000)
-	estimatedFeeRate := chainfee.SatPerKWeight(500)
-	minRelayFeeRate := chainfee.SatPerKWeight(100)
-	confTarget := uint32(6)
 	noStartFeeRate := fn.None[chainfee.SatPerKWeight]()
-	startFeeRate := chainfee.SatPerKWeight(1000)
 
 	// Assert init fee function with zero conf value will end up using the
 	// max fee rate.
@@ -36,23 +34,56 @@ func TestLinearFeeFunctionNew(t *testing.T) {
 	rt.Equal(maxFeeRate, f.endingFeeRate)
 	rt.Equal(maxFeeRate, f.currentFeeRate)
 
-	// When the fee estimator returns an error, it's returned.
+	// Assert init fee function with conf of one will end up using the max
-	//
+	// fee rate.
-	// Mock the fee estimator to return an error.
+	f, err = NewLinearFeeFunction(maxFeeRate, 1, estimator, noStartFeeRate)
-	estimator.On("EstimateFeePerKW", confTarget).Return(
+	rt.NoError(err)
-		chainfee.SatPerKWeight(0), errDummy).Once()
+	rt.NotNil(f)
 
-	f, err = NewLinearFeeFunction(
+	// Assert the internal state.
-		maxFeeRate, confTarget, estimator, noStartFeeRate,
+	rt.Equal(maxFeeRate, f.startingFeeRate)
-	)
+	rt.Equal(maxFeeRate, f.endingFeeRate)
-	rt.ErrorIs(err, errDummy)
+	rt.Equal(maxFeeRate, f.currentFeeRate)
-	rt.Nil(f)
+}
 
-	// When the starting feerate is greater than the ending feerate, the
+// TestLinearFeeFunctionNewZeroFeeRateDelta tests when the fee rate delta is
-	// starting feerate is capped.
+// zero, it will return an error except when the width is one.
+func TestLinearFeeFunctionNewZeroFeeRateDelta(t *testing.T) {
+	t.Parallel()
+
+	rt := require.New(t)
+
+	// Create a mock fee estimator.
+	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
+
+	// Create testing params.
+	maxFeeRate := chainfee.SatPerKWeight(10000)
+	estimatedFeeRate := chainfee.SatPerKWeight(500)
+	confTarget := uint32(6)
+	noStartFeeRate := fn.None[chainfee.SatPerKWeight]()
+
+	// When the calculated fee rate delta is 0, an error should be returned
+	// when the width is not one.
 	//
 	// Mock the fee estimator to return the fee rate.
-	smallConf := uint32(1)
+	estimator.On("EstimateFeePerKW", confTarget).Return(
+		// The starting fee rate is the max fee rate.
+		maxFeeRate, nil).Once()
+	estimator.On("RelayFeePerKW").Return(estimatedFeeRate).Once()
+
+	f, err := NewLinearFeeFunction(
+		maxFeeRate, confTarget, estimator, noStartFeeRate,
+	)
+	rt.ErrorContains(err, "fee rate delta is zero")
+	rt.Nil(f)
+
+	// When the calculated fee rate delta is 0, an error should NOT be
+	// returned when the width is one, and the starting feerate is capped
+	// at the max fee rate.
+	//
+	// Mock the fee estimator to return the fee rate.
+	smallConf := uint32(2)
 	estimator.On("EstimateFeePerKW", smallConf).Return(
 		// The fee rate is greater than the max fee rate.
 		maxFeeRate+1, nil).Once()
@@ -64,18 +95,41 @@ func TestLinearFeeFunctionNew(t *testing.T) {
 	rt.NoError(err)
 	rt.NotNil(f)
 
-	// When the calculated fee rate delta is 0, an error should be returned.
+	// Assert the internal state.
-	//
+	rt.Equal(maxFeeRate, f.startingFeeRate)
-	// Mock the fee estimator to return the fee rate.
+	rt.Equal(maxFeeRate, f.endingFeeRate)
-	estimator.On("EstimateFeePerKW", confTarget).Return(
+	rt.Equal(maxFeeRate, f.currentFeeRate)
-		// The starting fee rate is the max fee rate.
+	rt.Zero(f.deltaFeeRate)
-		maxFeeRate, nil).Once()
+	rt.Equal(smallConf-1, f.width)
-	estimator.On("RelayFeePerKW").Return(estimatedFeeRate).Once()
+}
 
-	f, err = NewLinearFeeFunction(
+// TestLinearFeeFunctionNewEsimator tests the NewLinearFeeFunction function
+// properly reacts to the response or error returned from the fee estimator.
+func TestLinearFeeFunctionNewEsimator(t *testing.T) {
+	t.Parallel()
+
+	rt := require.New(t)
+
+	// Create a mock fee estimator.
+	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
+
+	// Create testing params.
+	maxFeeRate := chainfee.SatPerKWeight(10000)
+	minRelayFeeRate := chainfee.SatPerKWeight(100)
+	confTarget := uint32(6)
+	noStartFeeRate := fn.None[chainfee.SatPerKWeight]()
+
+	// When the fee estimator returns an error, it's returned.
+	//
+	// Mock the fee estimator to return an error.
+	estimator.On("EstimateFeePerKW", confTarget).Return(
+		chainfee.SatPerKWeight(0), errDummy).Once()
+
+	f, err := NewLinearFeeFunction(
 		maxFeeRate, confTarget, estimator, noStartFeeRate,
 	)
-	rt.ErrorContains(err, "fee rate delta is zero")
+	rt.ErrorIs(err, errDummy)
 	rt.Nil(f)
 
 	// When the conf target is >= 1008, the min relay fee should be used.
@@ -95,16 +149,36 @@ func TestLinearFeeFunctionNew(t *testing.T) {
 	rt.Equal(maxFeeRate, f.endingFeeRate)
 	rt.Equal(minRelayFeeRate, f.currentFeeRate)
 	rt.NotZero(f.deltaFeeRate)
-	rt.Equal(largeConf, f.width)
+	rt.Equal(largeConf-1, f.width)
+}
+
+// TestLinearFeeFunctionNewSuccess tests we can create the fee function
+// successfully.
+func TestLinearFeeFunctionNewSuccess(t *testing.T) {
+	t.Parallel()
+
+	rt := require.New(t)
+
+	// Create a mock fee estimator.
+	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
+
+	// Create testing params.
+	maxFeeRate := chainfee.SatPerKWeight(10000)
+	estimatedFeeRate := chainfee.SatPerKWeight(500)
+	minRelayFeeRate := chainfee.SatPerKWeight(100)
+	confTarget := uint32(6)
+	noStartFeeRate := fn.None[chainfee.SatPerKWeight]()
+	startFeeRate := chainfee.SatPerKWeight(1000)
 
 	// Check a successfully created fee function.
 	//
 	// Mock the fee estimator to return the fee rate.
 	estimator.On("EstimateFeePerKW", confTarget).Return(
 		estimatedFeeRate, nil).Once()
-	estimator.On("RelayFeePerKW").Return(estimatedFeeRate).Once()
+	estimator.On("RelayFeePerKW").Return(minRelayFeeRate).Once()
 
-	f, err = NewLinearFeeFunction(
+	f, err := NewLinearFeeFunction(
 		maxFeeRate, confTarget, estimator, noStartFeeRate,
 	)
 	rt.NoError(err)
@@ -115,7 +189,7 @@ func TestLinearFeeFunctionNew(t *testing.T) {
 	rt.Equal(maxFeeRate, f.endingFeeRate)
 	rt.Equal(estimatedFeeRate, f.currentFeeRate)
 	rt.NotZero(f.deltaFeeRate)
-	rt.Equal(confTarget, f.width)
+	rt.Equal(confTarget-1, f.width)
 
 	// Check a successfully created fee function using the specified
 	// starting fee rate.
@@ -131,7 +205,10 @@ func TestLinearFeeFunctionNew(t *testing.T) {
 
 	// Assert the customized starting fee rate is used.
 	rt.Equal(startFeeRate, f.startingFeeRate)
+	rt.Equal(maxFeeRate, f.endingFeeRate)
 	rt.Equal(startFeeRate, f.currentFeeRate)
+	rt.NotZero(f.deltaFeeRate)
+	rt.Equal(confTarget-1, f.width)
 }
 
 // TestLinearFeeFunctionFeeRateAtPosition checks the expected feerate is
@@ -201,12 +278,13 @@ func TestLinearFeeFunctionIncrement(t *testing.T) {
 
 	// Create a mock fee estimator.
 	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
 
 	// Create testing params. These params are chosen so the delta value is
 	// 100.
-	maxFeeRate := chainfee.SatPerKWeight(1000)
+	maxFeeRate := chainfee.SatPerKWeight(900)
 	estimatedFeeRate := chainfee.SatPerKWeight(100)
-	confTarget := uint32(9)
+	confTarget := uint32(9) // This means the width is 8.
 
 	// Mock the fee estimator to return the fee rate.
 	estimator.On("EstimateFeePerKW", confTarget).Return(
@@ -219,8 +297,8 @@ func TestLinearFeeFunctionIncrement(t *testing.T) {
 	)
 	rt.NoError(err)
 
-	// We now increase the position from 1 to 9.
+	// We now increase the position from 1 to 8.
-	for i := uint32(1); i <= confTarget; i++ {
+	for i := uint32(1); i <= confTarget-1; i++ {
 		// Increase the fee rate.
 		increased, err := f.Increment()
 		rt.NoError(err)
@@ -236,7 +314,7 @@ func TestLinearFeeFunctionIncrement(t *testing.T) {
 		rt.Equal(estimatedFeeRate+delta, f.FeeRate())
 	}
 
-	// Now the position is at 9th, increase it again should give us an
+	// Now the position is at 8th, increase it again should give us an
 	// error.
 	increased, err := f.Increment()
 	rt.ErrorIs(err, ErrMaxPosition)
@@ -252,12 +330,13 @@ func TestLinearFeeFunctionIncreaseFeeRate(t *testing.T) {
 
 	// Create a mock fee estimator.
 	estimator := &chainfee.MockEstimator{}
+	defer estimator.AssertExpectations(t)
 
 	// Create testing params. These params are chosen so the delta value is
 	// 100.
-	maxFeeRate := chainfee.SatPerKWeight(1000)
+	maxFeeRate := chainfee.SatPerKWeight(900)
 	estimatedFeeRate := chainfee.SatPerKWeight(100)
-	confTarget := uint32(9)
+	confTarget := uint32(9) // This means the width is 8.
 
 	// Mock the fee estimator to return the fee rate.
 	estimator.On("EstimateFeePerKW", confTarget).Return(
@@ -281,9 +360,9 @@ func TestLinearFeeFunctionIncreaseFeeRate(t *testing.T) {
 	rt.NoError(err)
 	rt.False(increased)
 
-	// We now increase the fee rate from conf target 8 to 1 and assert we
+	// We now increase the fee rate from conf target 8 to 2 and assert we
 	// get no error and true.
-	for i := uint32(1); i < confTarget; i++ {
+	for i := uint32(1); i < confTarget-1; i++ {
 		// Increase the fee rate.
 		increased, err := f.IncreaseFeeRate(confTarget - i)
 		rt.NoError(err)
@@ -299,11 +378,17 @@ func TestLinearFeeFunctionIncreaseFeeRate(t *testing.T) {
 		rt.Equal(estimatedFeeRate+delta, f.FeeRate())
 	}
 
-	// Test that when we use a conf target of 0, we get the ending fee
+	// Test that when we use a conf target of 1, we get the ending fee
 	// rate.
-	increased, err = f.IncreaseFeeRate(0)
+	increased, err = f.IncreaseFeeRate(1)
 	rt.NoError(err)
 	rt.True(increased)
-	rt.Equal(confTarget, f.position)
+	rt.Equal(confTarget-1, f.position)
 	rt.Equal(maxFeeRate, f.currentFeeRate)
+
+	// Test that when we use a conf target of 0, ErrMaxPosition is
+	// returned.
+	increased, err = f.IncreaseFeeRate(0)
+	rt.ErrorIs(err, ErrMaxPosition)
+	rt.False(increased)
 }