sweeper: lower the default max fee rate and make it configurable

config.go

@@ -681,6 +681,7 @@ func DefaultConfig() Config {
 		},
 		Sweeper: &lncfg.Sweeper{
 			BatchWindowDuration: sweep.DefaultBatchWindowDuration,
+			MaxFeeRate:          sweep.DefaultMaxFeeRate,
 		},
 		Htlcswitch: &lncfg.Htlcswitch{
 			MailboxDeliveryTimeout: htlcswitch.DefaultMailboxDeliveryTimeout,

docs/release-notes/release-notes-0.18.0.md

@@ -21,6 +21,18 @@
 
 # New Features
 ## Functional Enhancements
+
+* A new config value,
+  [sweeper.maxfeerate](https://github.com/lightningnetwork/lnd/pull/7823), is
+  added so users can specify the max allowed fee rate when sweeping onchain
+  funds. The default value is 1000 sat/vb. Setting this value below 100 sat/vb
+  is not allowed, as low fee rate can cause transactions not confirming in
+  time, which could result in fund loss.
+  Please note that the actual fee rate to be used is deteremined by the fee
+  estimator used(for instance `bitcoind`), and this value is a cap on the max
+  allowed value. So it's expected that this cap is rarely hit unless there's
+  mempool congestion.
+
 ## RPC Additions
 ## lncli Additions
 

itest/lnd_onchain_test.go

@@ -229,11 +229,12 @@ func runCPFP(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) {
 
 	// We'll attempt to bump the fee of this transaction by performing a
 	// CPFP from Alice's point of view.
+	maxFeeRate := uint64(sweep.DefaultMaxFeeRate)
 	bumpFeeReq := &walletrpc.BumpFeeRequest{
 		Outpoint: op,
-		SatPerVbyte: uint64(
-			sweep.DefaultMaxFeeRate.FeePerKVByte() / 2000,
-		),
+		// We use a higher fee rate than the default max and expect the
+		// sweeper to cap the fee rate at the max value.
+		SatPerVbyte: maxFeeRate * 2,
 	}
 	bob.RPC.BumpFee(bumpFeeReq)
 
@@ -251,8 +252,11 @@ func runCPFP(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) {
 		"output txid not matched")
 	require.Equal(ht, pendingSweep.Outpoint.OutputIndex, op.OutputIndex,
 		"output index not matched")
-	require.Equal(ht, pendingSweep.SatPerVbyte, bumpFeeReq.SatPerVbyte,
-		"sweep sat per vbyte not matched")
+
+	// Also validate that the fee rate is capped at the max value.
+	require.Equalf(ht, maxFeeRate, pendingSweep.SatPerVbyte,
+		"sweep sat per vbyte not matched, want %v, got %v",
+		maxFeeRate, pendingSweep.SatPerVbyte)
 
 	// Mine a block to clean up the unconfirmed transactions.
 	ht.MineBlocksAndAssertNumTxes(1, 2)

lncfg/sweeper.go

@@ -3,11 +3,24 @@ package lncfg
 import (
 	"fmt"
 	"time"
+
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
+)
+
+const (
+	// MaxFeeRateFloor is the smallest config value allowed for the max fee
+	// rate in sat/vb.
+	MaxFeeRateFloor chainfee.SatPerVByte = 100
+
+	// MaxAllowedFeeRate is the largest fee rate in sat/vb that we allow
+	// when configuring the MaxFeeRate.
+	MaxAllowedFeeRate = 10_000
 )
 
 //nolint:lll
 type Sweeper struct {
-	BatchWindowDuration time.Duration `long:"batchwindowduration" description:"Duration of the sweep batch window. The sweep is held back during the batch window to allow more inputs to be added and thereby lower the fee per input."`
+	BatchWindowDuration time.Duration        `long:"batchwindowduration" description:"Duration of the sweep batch window. The sweep is held back during the batch window to allow more inputs to be added and thereby lower the fee per input."`
+	MaxFeeRate          chainfee.SatPerVByte `long:"maxfeerate" description:"Maximum fee rate in sat/vb that the sweeper is allowed to use when sweeping funds. Setting this value too low can result in transactions not being confirmed in time, causing HTLCs to expire hence potentially losing funds."`
 }
 
 // Validate checks the values configured for the sweeper.
@@ -16,5 +29,15 @@ func (s *Sweeper) Validate() error {
 		return fmt.Errorf("batchwindowduration must be positive")
 	}
 
+	// We require the max fee rate to be at least 100 sat/vbyte.
+	if s.MaxFeeRate < MaxFeeRateFloor {
+		return fmt.Errorf("maxfeerate must be >= 100 sat/vb")
+	}
+
+	// We require the max fee rate to be no greater than 10_000 sat/vbyte.
+	if s.MaxFeeRate > MaxAllowedFeeRate {
+		return fmt.Errorf("maxfeerate must be <= 10000 sat/vb")
+	}
+
 	return nil
 }

lnwallet/chainfee/rates.go

@@ -18,6 +18,24 @@ const (
 	AbsoluteFeePerKwFloor SatPerKWeight = 250
 )
 
+// SatPerVByte represents a fee rate in sat/vbyte.
+type SatPerVByte btcutil.Amount
+
+// FeePerKWeight converts the current fee rate from sat/vb to sat/kw.
+func (s SatPerVByte) FeePerKWeight() SatPerKWeight {
+	return SatPerKWeight(s * 1000 / blockchain.WitnessScaleFactor)
+}
+
+// FeePerKVByte converts the current fee rate from sat/vb to sat/kvb.
+func (s SatPerVByte) FeePerKVByte() SatPerKVByte {
+	return SatPerKVByte(s * 1000)
+}
+
+// String returns a human-readable string of the fee rate.
+func (s SatPerVByte) String() string {
+	return fmt.Sprintf("%v sat/vb", int64(s))
+}
+
 // SatPerKVByte represents a fee rate in sat/kb.
 type SatPerKVByte btcutil.Amount
 

lnwallet/chainfee/rates_test.go

@@ -0,0 +1,22 @@
+package chainfee
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestSatPerVByteConversion checks that the conversion from sat/vb to either
+// sat/kw or sat/kvb is correct.
+func TestSatPerVByteConversion(t *testing.T) {
+	t.Parallel()
+
+	// Create a test fee rate of 1 sat/vb.
+	rate := SatPerVByte(1)
+
+	// 1 sat/vb should be equal to 1000 sat/kvb.
+	require.Equal(t, SatPerKVByte(1000), rate.FeePerKVByte())
+
+	// 1 sat/vb should be equal to 250 sat/kw.
+	require.Equal(t, SatPerKWeight(250), rate.FeePerKWeight())
+}

rpcserver.go

@@ -1268,6 +1268,7 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 	var txid *chainhash.Hash
 
 	wallet := r.server.cc.Wallet
+	maxFeeRate := r.cfg.Sweeper.MaxFeeRate.FeePerKWeight()
 
 	// If the send all flag is active, then we'll attempt to sweep all the
 	// coins in the wallet in a single transaction (if possible),
@@ -1293,10 +1294,9 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 		// pay to the change address created above if we needed to
 		// reserve any value, the rest will go to targetAddr.
 		sweepTxPkg, err := sweep.CraftSweepAllTx(
-			feePerKw, uint32(bestHeight), nil, targetAddr, wallet,
-			wallet, wallet.WalletController,
-			r.server.cc.FeeEstimator, r.server.cc.Signer,
-			minConfs,
+			maxFeeRate, feePerKw, uint32(bestHeight), nil,
+			targetAddr, wallet, wallet, wallet.WalletController,
+			r.server.cc.Signer, minConfs,
 		)
 		if err != nil {
 			return nil, err
@@ -1347,11 +1347,10 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 			}
 
 			sweepTxPkg, err = sweep.CraftSweepAllTx(
-				feePerKw, uint32(bestHeight), outputs,
-				targetAddr, wallet, wallet,
+				maxFeeRate, feePerKw, uint32(bestHeight),
+				outputs, targetAddr, wallet, wallet,
 				wallet.WalletController,
-				r.server.cc.FeeEstimator, r.server.cc.Signer,
-				minConfs,
+				r.server.cc.Signer, minConfs,
 			)
 			if err != nil {
 				return nil, err

sample-lnd.conf

@@ -1578,6 +1578,11 @@
 ; window to allow more inputs to be added and thereby lower the fee per input.
 ; sweeper.batchwindowduration=30s
 
+; The max fee rate in sat/vb which can be used when sweeping funds. Setting
+; this value too low can result in transactions not being confirmed in time,
+; causing HTLCs to expire hence potentially losing funds.
+; sweeper.maxfeerate=1000
+
 
 [htlcswitch]
 

server.go

@@ -1071,7 +1071,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 		MaxInputsPerTx:       sweep.DefaultMaxInputsPerTx,
 		MaxSweepAttempts:     sweep.DefaultMaxSweepAttempts,
 		NextAttemptDeltaFunc: sweep.DefaultNextAttemptDeltaFunc,
-		MaxFeeRate:           sweep.DefaultMaxFeeRate,
+		MaxFeeRate:           cfg.Sweeper.MaxFeeRate,
 		FeeRateBucketSize:    sweep.DefaultFeeRateBucketSize,
 	})
 

sweep/defaults.go

@@ -2,11 +2,18 @@ package sweep
 
 import (
 	"time"
+
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 )
 
 var (
 	// DefaultBatchWindowDuration specifies duration of the sweep batch
 	// window. The sweep is held back during the batch window to allow more
 	// inputs to be added and thereby lower the fee per input.
 	DefaultBatchWindowDuration = 30 * time.Second
+
+	// DefaultMaxFeeRate is the default maximum fee rate allowed within the
+	// UtxoSweeper. The current value is equivalent to a fee rate of 1,000
+	// sat/vbyte.
+	DefaultMaxFeeRate chainfee.SatPerVByte = 1e3
 )

sweep/sweeper.go

@@ -21,11 +21,6 @@ import (
 )
 
 const (
-	// DefaultMaxFeeRate is the default maximum fee rate allowed within the
-	// UtxoSweeper. The current value is equivalent to a fee rate of 10,000
-	// sat/vbyte.
-	DefaultMaxFeeRate = chainfee.FeePerKwFloor * 1e4
-
 	// DefaultFeeRateBucketSize is the default size of fee rate buckets
 	// we'll use when clustering inputs into buckets with similar fee rates
 	// within the UtxoSweeper.
@@ -288,7 +283,7 @@ type UtxoSweeperConfig struct {
 
 	// MaxFeeRate is the the maximum fee rate allowed within the
 	// UtxoSweeper.
-	MaxFeeRate chainfee.SatPerKWeight
+	MaxFeeRate chainfee.SatPerVByte
 
 	// FeeRateBucketSize is the default size of fee rate buckets we'll use
 	// when clustering inputs into buckets with similar fee rates within the
@@ -484,9 +479,15 @@ func (s *UtxoSweeper) feeRateForPreference(
 		return 0, fmt.Errorf("fee preference resulted in invalid fee "+
 			"rate %v, minimum is %v", feeRate, s.relayFeeRate)
 	}
-	if feeRate > s.cfg.MaxFeeRate {
-		return 0, fmt.Errorf("fee preference resulted in invalid fee "+
-			"rate %v, maximum is %v", feeRate, s.cfg.MaxFeeRate)
+
+	// If the estimated fee rate is above the maximum allowed fee rate,
+	// default to the max fee rate.
+	if feeRate > s.cfg.MaxFeeRate.FeePerKWeight() {
+		log.Warnf("Estimated fee rate %v exceeds max allowed fee "+
+			"rate %v, using max fee rate instead", feeRate,
+			s.cfg.MaxFeeRate.FeePerKWeight())
+
+		return s.cfg.MaxFeeRate.FeePerKWeight(), nil
 	}
 
 	return feeRate, nil
@@ -1245,15 +1246,18 @@ func (s *UtxoSweeper) getInputLists(cluster inputCluster,
 		}
 	}
 
+	// Convert the max fee rate's unit from sat/vb to sat/kw.
+	maxFeeRate := s.cfg.MaxFeeRate.FeePerKWeight()
+
 	// If there is anything to retry, combine it with the new inputs and
 	// form input sets.
 	var allSets []inputSet
 	if len(retryInputs) > 0 {
 		var err error
 		allSets, err = generateInputPartitionings(
 			append(retryInputs, newInputs...),
-			cluster.sweepFeeRate, s.cfg.MaxInputsPerTx,
-			s.cfg.Wallet,
+			cluster.sweepFeeRate, maxFeeRate,
+			s.cfg.MaxInputsPerTx, s.cfg.Wallet,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("input partitionings: %v", err)
@@ -1262,8 +1266,8 @@ func (s *UtxoSweeper) getInputLists(cluster inputCluster,
 
 	// Create sets for just the new inputs.
 	newSets, err := generateInputPartitionings(
-		newInputs, cluster.sweepFeeRate, s.cfg.MaxInputsPerTx,
-		s.cfg.Wallet,
+		newInputs, cluster.sweepFeeRate, maxFeeRate,
+		s.cfg.MaxInputsPerTx, s.cfg.Wallet,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("input partitionings: %v", err)
@@ -1294,7 +1298,7 @@ func (s *UtxoSweeper) sweep(inputs inputSet, feeRate chainfee.SatPerKWeight,
 	// Create sweep tx.
 	tx, err := createSweepTx(
 		inputs, nil, s.currentOutputScript, uint32(currentHeight),
-		feeRate, s.cfg.Signer,
+		feeRate, s.cfg.MaxFeeRate.FeePerKWeight(), s.cfg.Signer,
 	)
 	if err != nil {
 		return fmt.Errorf("create sweep tx: %v", err)
@@ -1603,7 +1607,7 @@ func (s *UtxoSweeper) CreateSweepTx(inputs []input.Input, feePref FeePreference,
 
 	return createSweepTx(
 		inputs, nil, pkScript, currentBlockHeight, feePerKw,
-		s.cfg.Signer,
+		s.cfg.MaxFeeRate.FeePerKWeight(), s.cfg.Signer,
 	)
 }
 

sweep/sweeper_test.go

@@ -355,7 +355,7 @@ func assertTxFeeRate(t *testing.T, tx *wire.MsgTx,
 	outputAmt := tx.TxOut[0].Value
 
 	fee := btcutil.Amount(inputAmt - outputAmt)
-	_, estimator, err := getWeightEstimate(inputs, nil, 0, changePk)
+	_, estimator, err := getWeightEstimate(inputs, nil, 0, 0, changePk)
 	require.NoError(t, err)
 
 	txWeight := estimator.weight()
@@ -1186,7 +1186,7 @@ func TestBumpFeeRBF(t *testing.T) {
 
 	// We'll then attempt to bump its fee rate.
 	highFeePref := FeePreference{ConfTarget: 6}
-	highFeeRate := DefaultMaxFeeRate
+	highFeeRate := DefaultMaxFeeRate.FeePerKWeight()
 	ctx.estimator.blocksToFee[highFeePref.ConfTarget] = highFeeRate
 
 	// We should expect to see an error if a fee preference isn't provided.

sweep/tx_input_set.go

@@ -33,6 +33,9 @@ type txInputSetState struct {
 	// feeRate is the fee rate to use for the sweep transaction.
 	feeRate chainfee.SatPerKWeight
 
+	// maxFeeRate is the max allowed fee rate configured by the user.
+	maxFeeRate chainfee.SatPerKWeight
+
 	// inputTotal is the total value of all inputs.
 	inputTotal btcutil.Amount
 
@@ -61,7 +64,7 @@ type txInputSetState struct {
 // weightEstimate is the (worst case) tx weight with the current set of
 // inputs. It takes a parameter whether to add a change output or not.
 func (t *txInputSetState) weightEstimate(change bool) *weightEstimator {
-	weightEstimate := newWeightEstimator(t.feeRate)
+	weightEstimate := newWeightEstimator(t.feeRate, t.maxFeeRate)
 	for _, i := range t.inputs {
 		// Can ignore error, because it has already been checked when
 		// calculating the yields.
@@ -118,11 +121,12 @@ type txInputSet struct {
 }
 
 // newTxInputSet constructs a new, empty input set.
-func newTxInputSet(wallet Wallet, feePerKW chainfee.SatPerKWeight,
+func newTxInputSet(wallet Wallet, feePerKW, maxFeeRate chainfee.SatPerKWeight,
 	maxInputs int) *txInputSet {
 
 	state := txInputSetState{
-		feeRate: feePerKW,
+		feeRate:    feePerKW,
+		maxFeeRate: maxFeeRate,
 	}
 
 	b := txInputSet{

sweep/tx_input_set_test.go

@@ -16,7 +16,7 @@ func TestTxInputSet(t *testing.T) {
 		feeRate   = 1000
 		maxInputs = 10
 	)
-	set := newTxInputSet(nil, feeRate, maxInputs)
+	set := newTxInputSet(nil, feeRate, 0, maxInputs)
 
 	// Create a 300 sat input. The fee to sweep this input to a P2WKH output
 	// is 439 sats. That means that this input yields -139 sats and we
@@ -65,7 +65,7 @@ func TestTxInputSetFromWallet(t *testing.T) {
 	)
 
 	wallet := &mockWallet{}
-	set := newTxInputSet(wallet, feeRate, maxInputs)
+	set := newTxInputSet(wallet, feeRate, 0, maxInputs)
 
 	// Add a 500 sat input to the set. It yields positively, but doesn't
 	// reach the output dust limit.
@@ -134,7 +134,7 @@ func TestTxInputSetRequiredOutput(t *testing.T) {
 		feeRate   = 1000
 		maxInputs = 10
 	)
-	set := newTxInputSet(nil, feeRate, maxInputs)
+	set := newTxInputSet(nil, feeRate, 0, maxInputs)
 
 	// Attempt to add an input with a required txout below the dust limit.
 	// This should fail since we cannot trim such outputs.