diff --git a/itest/lnd_onchain_test.go b/itest/lnd_onchain_test.go index e7a9b2a91..4c7a34d48 100644 --- a/itest/lnd_onchain_test.go +++ b/itest/lnd_onchain_test.go @@ -229,9 +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), + Outpoint: op, + // 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) @@ -249,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/rpcserver.go b/rpcserver.go index 26062d77c..dadaa51c6 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.FeeEstimator, r.server.cc.Signer, minConfs, ) if err != nil { return nil, err @@ -1347,8 +1347,8 @@ 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, diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 44e1656b4..c11895198 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -1245,6 +1245,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 +1255,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 +1265,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 +1297,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 +1606,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 b0ddbee2e..eacf9012e 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() diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 576471032..c42d35f77 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 c280f9f6a..899730d5e 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 20e80a00c..e1f71e7cd 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 cb575e5a6..48dcacd49 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 f806d3276..3ba6f0a35 100644 --- a/sweep/walletsweep.go +++ b/sweep/walletsweep.go @@ -180,11 +180,12 @@ 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, - signer input.Signer, minConfs int32) (*WalletSweepPackage, error) { +func CraftSweepAllTx(feeRate, maxFeeRate chainfee.SatPerKWeight, + blockHeight uint32, deliveryAddrs []DeliveryAddr, + changeAddr btcutil.Address, coinSelectLocker CoinSelectionLocker, + utxoSource UtxoSource, outpointLocker OutpointLocker, + feeEstimator chainfee.Estimator, signer input.Signer, + minConfs int32) (*WalletSweepPackage, error) { // TODO(roasbeef): turn off ATPL as well when available? @@ -320,8 +321,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 f2196fc89..3b0d9f61c 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, 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, nil, 0, ) @@ -358,7 +358,7 @@ func TestCraftSweepAllTx(t *testing.T) { utxoLocker := newMockOutpointLocker() sweepPkg, err := CraftSweepAllTx( - 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, + 0, 0, 10, nil, deliveryAddr, coinSelectLocker, utxoSource, utxoLocker, feeEstimator, signer, 0, ) require.NoError(t, err, "unable to make sweep tx") diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go index c09adabdc..7dae25d80 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 1952f531e..350da9af3 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.