diff --git a/config.go b/config.go index 24ea3519368..3e1b4b14e96 100644 --- a/config.go +++ b/config.go @@ -681,6 +681,7 @@ func DefaultConfig() Config { }, Sweeper: &lncfg.Sweeper{ BatchWindowDuration: sweep.DefaultBatchWindowDuration, + MaxFeeRate: sweep.DefaultMaxFeeRate, }, Htlcswitch: &lncfg.Htlcswitch{ MailboxDeliveryTimeout: htlcswitch.DefaultMailboxDeliveryTimeout, diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 2b566c677ef..85e6df18574 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/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 diff --git a/itest/lnd_onchain_test.go b/itest/lnd_onchain_test.go index 7ea7d106e09..4c7a34d4847 100644 --- a/itest/lnd_onchain_test.go +++ b/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) diff --git a/lncfg/sweeper.go b/lncfg/sweeper.go index feca51ccb7a..08b12f6dabf 100644 --- a/lncfg/sweeper.go +++ b/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 } diff --git a/lnwallet/chainfee/rates.go b/lnwallet/chainfee/rates.go index f9b606f7497..34da5537449 100644 --- a/lnwallet/chainfee/rates.go +++ b/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 diff --git a/lnwallet/chainfee/rates_test.go b/lnwallet/chainfee/rates_test.go new file mode 100644 index 00000000000..11aa35adf6d --- /dev/null +++ b/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()) +} diff --git a/rpcserver.go b/rpcserver.go index 997db710310..01d5c5fe410 100644 --- a/rpcserver.go +++ b/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 diff --git a/sample-lnd.conf b/sample-lnd.conf index bf6bb47dc46..a491a5d038e 100644 --- a/sample-lnd.conf +++ b/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] diff --git a/server.go b/server.go index f82f9784e6f..8abb196ea77 100644 --- a/server.go +++ b/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, }) diff --git a/sweep/defaults.go b/sweep/defaults.go index 2f53cdb0a90..3ea49219001 100644 --- a/sweep/defaults.go +++ b/sweep/defaults.go @@ -2,6 +2,8 @@ package sweep import ( "time" + + "github.com/lightningnetwork/lnd/lnwallet/chainfee" ) var ( @@ -9,4 +11,9 @@ var ( // 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 ) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 4f53d2098d8..72b4f61dcdc 100644 --- a/sweep/sweeper.go +++ b/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,6 +1246,9 @@ 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 @@ -1252,8 +1256,8 @@ func (s *UtxoSweeper) getInputLists(cluster inputCluster, 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, ) } diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index 13f891857f3..eacf9012ed1 100644 --- a/sweep/sweeper_test.go +++ b/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. diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 576471032f3..c42d35f77da 100644 --- a/sweep/tx_input_set.go +++ b/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{ diff --git a/sweep/tx_input_set_test.go b/sweep/tx_input_set_test.go index c280f9f6a6e..899730d5e3e 100644 --- a/sweep/tx_input_set_test.go +++ b/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. diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index 20e80a00c41..e1f71e7cd98 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -39,7 +39,7 @@ type inputSet []input.Input // inputs are skipped. No input sets with a total value after fees below the // dust limit are returned. func generateInputPartitionings(sweepableInputs []txInput, - feePerKW chainfee.SatPerKWeight, maxInputsPerTx int, + feePerKW, maxFeeRate chainfee.SatPerKWeight, maxInputsPerTx int, wallet Wallet) ([]inputSet, error) { // Sort input by yield. We will start constructing input sets starting @@ -87,7 +87,9 @@ func generateInputPartitionings(sweepableInputs []txInput, // Start building a set of positive-yield tx inputs under the // condition that the tx will be published with the specified // fee rate. - txInputs := newTxInputSet(wallet, feePerKW, maxInputsPerTx) + txInputs := newTxInputSet( + wallet, feePerKW, maxFeeRate, maxInputsPerTx, + ) // From the set of sweepable inputs, keep adding inputs to the // input set until the tx output value no longer goes up or the @@ -137,11 +139,11 @@ func generateInputPartitionings(sweepableInputs []txInput, // sending any leftover change to the change script. func createSweepTx(inputs []input.Input, outputs []*wire.TxOut, changePkScript []byte, currentBlockHeight uint32, - feePerKw chainfee.SatPerKWeight, signer input.Signer) (*wire.MsgTx, - error) { + feePerKw, maxFeeRate chainfee.SatPerKWeight, + signer input.Signer) (*wire.MsgTx, error) { inputs, estimator, err := getWeightEstimate( - inputs, outputs, feePerKw, changePkScript, + inputs, outputs, feePerKw, maxFeeRate, changePkScript, ) if err != nil { return nil, err @@ -319,7 +321,8 @@ func createSweepTx(inputs []input.Input, outputs []*wire.TxOut, // getWeightEstimate returns a weight estimate for the given inputs. // Additionally, it returns counts for the number of csv and cltv inputs. func getWeightEstimate(inputs []input.Input, outputs []*wire.TxOut, - feeRate chainfee.SatPerKWeight, outputPkScript []byte) ([]input.Input, + feeRate, maxFeeRate chainfee.SatPerKWeight, + outputPkScript []byte) ([]input.Input, *weightEstimator, error) { // We initialize a weight estimator so we can accurately asses the @@ -327,7 +330,7 @@ func getWeightEstimate(inputs []input.Input, outputs []*wire.TxOut, // // TODO(roasbeef): can be more intelligent about buffering outputs to // be more efficient on-chain. - weightEstimate := newWeightEstimator(feeRate) + weightEstimate := newWeightEstimator(feeRate, maxFeeRate) // Our sweep transaction will always pay to the given set of outputs. for _, o := range outputs { diff --git a/sweep/txgenerator_test.go b/sweep/txgenerator_test.go index cb575e5a649..48dcacd4994 100644 --- a/sweep/txgenerator_test.go +++ b/sweep/txgenerator_test.go @@ -50,7 +50,9 @@ func TestWeightEstimate(t *testing.T) { 0x00, 0x00, 0x00, 0x00, } - _, estimator, err := getWeightEstimate(inputs, nil, 0, changePkScript) + _, estimator, err := getWeightEstimate( + inputs, nil, 0, 0, changePkScript, + ) require.NoError(t, err) weight := int64(estimator.weight()) @@ -151,7 +153,7 @@ func testUnknownScriptInner(t *testing.T, pkscript []byte, expectFail bool) { )) } - _, _, err := getWeightEstimate(inputs, nil, 0, pkscript) + _, _, err := getWeightEstimate(inputs, nil, 0, 0, pkscript) if expectFail { require.Error(t, err) } else { diff --git a/sweep/walletsweep.go b/sweep/walletsweep.go index f806d32764f..831b0151e48 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -180,10 +180,10 @@ type DeliveryAddr struct { // output, as specified by the change address. The sweep transaction will be // crafted with the target fee rate, and will use the utxoSource and // outpointLocker as sources for wallet funds. -func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, blockHeight uint32, - deliveryAddrs []DeliveryAddr, changeAddr btcutil.Address, - coinSelectLocker CoinSelectionLocker, utxoSource UtxoSource, - outpointLocker OutpointLocker, feeEstimator chainfee.Estimator, +func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, + blockHeight uint32, deliveryAddrs []DeliveryAddr, + changeAddr btcutil.Address, coinSelectLocker CoinSelectionLocker, + utxoSource UtxoSource, outpointLocker OutpointLocker, signer input.Signer, minConfs int32) (*WalletSweepPackage, error) { // TODO(roasbeef): turn off ATPL as well when available? @@ -320,8 +320,8 @@ func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, blockHeight uint32, // Finally, we'll ask the sweeper to craft a sweep transaction which // respects our fee preference and targets all the UTXOs of the wallet. sweepTx, err := createSweepTx( - inputsToSweep, txOuts, changePkScript, blockHeight, feeRate, - signer, + inputsToSweep, txOuts, changePkScript, blockHeight, + feeRate, maxFeeRate, signer, ) if err != nil { unlockOutputs() diff --git a/sweep/walletsweep_test.go b/sweep/walletsweep_test.go index f2196fc8998..4a7ceec7a74 100644 --- a/sweep/walletsweep_test.go +++ b/sweep/walletsweep_test.go @@ -297,7 +297,7 @@ func TestCraftSweepAllTxCoinSelectFail(t *testing.T) { utxoLocker := newMockOutpointLocker() _, err := CraftSweepAllTx( - 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, nil, + 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, nil, 0, ) @@ -323,7 +323,7 @@ func TestCraftSweepAllTxUnknownWitnessType(t *testing.T) { utxoLocker := newMockOutpointLocker() _, err := CraftSweepAllTx( - 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, nil, + 0, 0, 10, nil, nil, coinSelectLocker, utxoSource, utxoLocker, nil, 0, ) @@ -348,7 +348,6 @@ func TestCraftSweepAllTx(t *testing.T) { // First, we'll make a mock signer along with a fee estimator, We'll // use zero fees to we can assert a precise output value. signer := &mock.DummySigner{} - feeEstimator := newMockFeeEstimator(0, 0) // For our UTXO source, we'll pass in all the UTXOs that we know of, // other than the final one which is of an unknown witness type. @@ -358,8 +357,8 @@ func TestCraftSweepAllTx(t *testing.T) { utxoLocker := newMockOutpointLocker() sweepPkg, err := CraftSweepAllTx( - 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, - utxoLocker, feeEstimator, signer, 0, + 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, + utxoLocker, signer, 0, ) require.NoError(t, err, "unable to make sweep tx") diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go index c09adabdc8c..7dae25d80fc 100644 --- a/sweep/weight_estimator.go +++ b/sweep/weight_estimator.go @@ -16,13 +16,19 @@ type weightEstimator struct { parents map[chainhash.Hash]struct{} parentsFee btcutil.Amount parentsWeight int64 + + // maxFeeRate is the max allowed fee rate configured by the user. + maxFeeRate chainfee.SatPerKWeight } // newWeightEstimator instantiates a new sweeper weight estimator. -func newWeightEstimator(feeRate chainfee.SatPerKWeight) *weightEstimator { +func newWeightEstimator( + feeRate, maxFeeRate chainfee.SatPerKWeight) *weightEstimator { + return &weightEstimator{ - feeRate: feeRate, - parents: make(map[chainhash.Hash]struct{}), + feeRate: feeRate, + maxFeeRate: maxFeeRate, + parents: make(map[chainhash.Hash]struct{}), } } @@ -119,5 +125,24 @@ func (w *weightEstimator) fee() btcutil.Amount { fee = childFee } + // Exit early if maxFeeRate is not set. + if w.maxFeeRate == 0 { + return fee + } + + // Clamp the fee to the max fee rate. + maxFee := w.maxFeeRate.FeeForWeight(childWeight) + if fee > maxFee { + // Calculate the effective fee rate for logging. + childFeeRate := chainfee.SatPerKWeight( + fee * 1000 / btcutil.Amount(childWeight), + ) + log.Warnf("Child fee rate %v exceeds max allowed fee rate %v, "+ + "returning fee %v instead of %v", childFeeRate, + w.maxFeeRate, maxFee, fee) + + fee = maxFee + } + return fee } diff --git a/sweep/weight_estimator_test.go b/sweep/weight_estimator_test.go index 1952f531e86..350da9af3e9 100644 --- a/sweep/weight_estimator_test.go +++ b/sweep/weight_estimator_test.go @@ -17,7 +17,7 @@ import ( func TestWeightEstimator(t *testing.T) { testFeeRate := chainfee.SatPerKWeight(20000) - w := newWeightEstimator(testFeeRate) + w := newWeightEstimator(testFeeRate, 0) // Add an input without unconfirmed parent tx. input1 := input.MakeBaseInput( @@ -81,6 +81,46 @@ func TestWeightEstimator(t *testing.T) { require.Equal(t, expectedFee, w.fee()) } +// TestWeightEstimatorMaxFee tests that the weight estimator correctly caps the +// fee at the maximum allowed fee. +func TestWeightEstimatorMaxFee(t *testing.T) { + t.Parallel() + + testFeeRate := chainfee.SatPerKWeight(9_000) + maxFeeRate := chainfee.SatPerKWeight(10_000) + + w := newWeightEstimator(testFeeRate, maxFeeRate) + + // Define a parent transaction that pays a fee of 1000 sat/kw. + parentTxLowFee := &input.TxInfo{ + Weight: 100, + Fee: 100, + } + + // Add an output of the low-fee parent tx above. + childInput := input.MakeBaseInput( + &wire.OutPoint{}, input.CommitmentAnchor, + &input.SignDescriptor{}, 0, parentTxLowFee, + ) + require.NoError(t, w.add(&childInput)) + + // The child weight should be 322 weight uints. + const childWeight = 322 + require.Equal(t, childWeight, w.weight()) + + // Check the fee is capped at the maximum allowed fee. The + // calculations, + // + // totalWeight = childWeight + parentWeight = 422 + // fee = totalWeight * testFeeRate - parentsFee = + // 422 * 9_000 / 1000 - 100 = 3598 + // maxFee = childWeight * maxFeeRate = 422 * 10_000 / 1000 = 3220 + // + // Thus we cap at the maxFee. + expectedFee := maxFeeRate.FeeForWeight(childWeight) + require.Equal(t, expectedFee, w.fee()) +} + // TestWeightEstimatorAddOutput tests that adding the raw P2WKH output to the // estimator yield the same result as an estimated add. func TestWeightEstimatorAddOutput(t *testing.T) { @@ -100,10 +140,10 @@ func TestWeightEstimatorAddOutput(t *testing.T) { Value: 10000, } - w1 := newWeightEstimator(testFeeRate) + w1 := newWeightEstimator(testFeeRate, 0) w1.addOutput(txOut) - w2 := newWeightEstimator(testFeeRate) + w2 := newWeightEstimator(testFeeRate, 0) w2.addP2WKHOutput() // Estimate hhould be the same.