Merge pull request #8600 from GeorgeTsagk/chanfunding-coinselect-nofe…

…echeck

Add `maxFeeRatio` parameter to sanityCheckFee in psbt coin selection

cmd/commands/walletrpc_active.go

@@ -1252,6 +1252,12 @@ var fundPsbtCommand = cli.Command{
 			Value: defaultUtxoMinConf,
 		},
 		coinSelectionStrategyFlag,
+		cli.Float64Flag{
+			Name: "max_fee_ratio",
+			Usage: "the maximum fee to total output amount ratio " +
+				"that this psbt should adhere to",
+			Value: chanfunding.DefaultMaxFeeRatio,
+		},
 	},
 	Action: actionDecorator(fundPsbt),
 }
@@ -1390,6 +1396,8 @@ func fundPsbt(ctx *cli.Context) error {
 		}
 	}
 
+	req.MaxFeeRatio = ctx.Float64("max_fee_ratio")
+
 	walletClient, cleanUp := getWalletClient(ctx)
 	defer cleanUp()
 

docs/release-notes/release-notes-0.19.0.md

@@ -52,6 +52,9 @@
   `sat_per_kw` which allows for more precise
   fees](https://github.com/lightningnetwork/lnd/pull/9013).
 
+* [The `walletrpc.FundPsbt` method now has a new option to specify the maximum
+  fee to output amounts ratio.](https://github.com/lightningnetwork/lnd/pull/8600)
+
 ## lncli Additions
 
 * [A pre-generated macaroon root key can now be specified in `lncli create` and
@@ -62,6 +65,9 @@
   specify more precise fee
   rates](https://github.com/lightningnetwork/lnd/pull/9013).
 
+* The `lncli wallet fundpsbt` command now has a [`--max_fee_ratio` argument to
+  specify the max fees to output amounts ratio.](https://github.com/lightningnetwork/lnd/pull/8600)
+
 # Improvements
 ## Functional Updates
 
@@ -151,6 +157,7 @@
 * Boris Nagaev
 * CharlieZKSmith
 * Elle Mouton
+* George Tsagkarelis
 * Pins
 * Viktor Tigerström
 * Ziggie

lnrpc/walletrpc/walletkit.proto

@@ -1417,6 +1417,9 @@ message FundPsbtRequest {
 
     // The strategy to use for selecting coins during funding the PSBT.
     lnrpc.CoinSelectionStrategy coin_selection_strategy = 10;
+
+    // The max fee to total output amount ratio that this psbt should adhere to.
+    double max_fee_ratio = 12;
 }
 message FundPsbtResponse {
     /*

lnrpc/walletrpc/walletkit.swagger.json

@@ -1589,6 +1589,11 @@
         "coin_selection_strategy": {
           "$ref": "#/definitions/lnrpcCoinSelectionStrategy",
           "description": "The strategy to use for selecting coins during funding the PSBT."
+        },
+        "max_fee_ratio": {
+          "type": "number",
+          "format": "double",
+          "description": "The max fee to total output amount ratio that this psbt should adhere to."
         }
       }
     },

lnrpc/walletrpc/walletkit_server.go

@@ -1626,11 +1626,17 @@ func (w *WalletKit) FundPsbt(_ context.Context,
 			return nil, fmt.Errorf("unknown change output type")
 		}
 
+		maxFeeRatio := chanfunding.DefaultMaxFeeRatio
+
+		if req.MaxFeeRatio != 0 {
+			maxFeeRatio = req.MaxFeeRatio
+		}
+
 		// Run the actual funding process now, using the channel funding
 		// coin selection algorithm.
 		return w.fundPsbtCoinSelect(
 			account, changeIndex, packet, minConfs, changeType,
-			feeSatPerKW, coinSelectionStrategy,
+			feeSatPerKW, coinSelectionStrategy, maxFeeRatio,
 		)
 
 	// The template is specified as a RPC message. We need to create a new
@@ -1833,8 +1839,8 @@ func (w *WalletKit) fundPsbtInternalWallet(account string,
 func (w *WalletKit) fundPsbtCoinSelect(account string, changeIndex int32,
 	packet *psbt.Packet, minConfs int32,
 	changeType chanfunding.ChangeAddressType,
-	feeRate chainfee.SatPerKWeight, strategy base.CoinSelectionStrategy) (
-	*FundPsbtResponse, error) {
+	feeRate chainfee.SatPerKWeight, strategy base.CoinSelectionStrategy,
+	maxFeeRatio float64) (*FundPsbtResponse, error) {
 
 	// We want to make sure we don't select any inputs that are already
 	// specified in the template. To do that, we require those inputs to
@@ -1922,6 +1928,7 @@ func (w *WalletKit) fundPsbtCoinSelect(account string, changeIndex int32,
 		changeAmt, needMore, err := chanfunding.CalculateChangeAmount(
 			inputSum, outputSum, packetFeeNoChange,
 			packetFeeWithChange, changeDustLimit, changeType,
+			maxFeeRatio,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("error calculating change "+
@@ -1978,7 +1985,7 @@ func (w *WalletKit) fundPsbtCoinSelect(account string, changeIndex int32,
 
 		selectedCoins, changeAmount, err := chanfunding.CoinSelect(
 			feeRate, fundingAmount, changeDustLimit, coins,
-			strategy, estimator, changeType,
+			strategy, estimator, changeType, maxFeeRatio,
 		)
 		if err != nil {
 			return fmt.Errorf("error selecting coins: %w", err)

lnrpc/walletrpc/walletkit_server_test.go

@@ -162,10 +162,18 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		// packet in bytes.
 		expectedFee btcutil.Amount
 
+		// maxFeeRatio is the maximum fee to total output amount ratio
+		// that we consider valid.
+		maxFeeRatio float64
+
 		// expectedErr is the expected concrete error. If not nil, then
 		// the error must match exactly.
 		expectedErr error
 
+		// expectedContainedErrStr is the expected string to be
+		// contained in the returned error.
+		expectedContainedErrStr string
+
 		// expectedErrType is the expected error type. If not nil, then
 		// the error must be of this type.
 		expectedErrType error
@@ -178,6 +186,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:     -1,
 		feeRate:         chainfee.FeePerKwFloor,
+		maxFeeRatio:     chanfunding.DefaultMaxFeeRatio,
 		expectedErrType: &chanfunding.ErrInsufficientFunds{},
 	}, {
 		name: "1 p2wpkh utxo, add p2wkh change",
@@ -193,6 +202,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: 1,
 		expectedFee:             calcFee(0, 1, 1, 1, 0),
@@ -210,6 +220,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2TRChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: 1,
@@ -228,6 +239,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: -1,
 		expectedFee:             calcFee(0, 1, 1, 0, 0),
@@ -247,6 +259,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2WKHChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: -1,
@@ -267,6 +280,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2TRChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: -1,
@@ -285,6 +299,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             0,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.ExistingChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: 0,
@@ -303,6 +318,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             0,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.ExistingChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: 0,
@@ -321,6 +337,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             0,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.ExistingChangeAddress,
 		expectedUtxoIndexes:     []int{0},
 		expectChangeOutputIndex: 0,
@@ -369,6 +386,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             0,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.ExistingChangeAddress,
 		expectedUtxoIndexes:     []int{0, 1},
 		expectChangeOutputIndex: 0,
@@ -417,6 +435,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2TRChangeAddress,
 		expectedUtxoIndexes:     []int{0, 1},
 		expectChangeOutputIndex: 1,
@@ -456,6 +475,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2WKHChangeAddress,
 		expectedUtxoIndexes:     []int{},
 		expectChangeOutputIndex: 1,
@@ -497,10 +517,74 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:             -1,
 		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
 		changeType:              chanfunding.P2TRChangeAddress,
 		expectedUtxoIndexes:     []int{},
 		expectChangeOutputIndex: -1,
 		expectedFee:             calcFee(1, 0, 1, 0, 0),
+	}, {
+		name: "1 p2wpkh utxo, existing p2wkh change, invalid fee ratio",
+		utxos: []*lnwallet.Utxo{
+			{
+				Value:    250,
+				PkScript: p2wkhScript,
+			},
+		},
+		packet: makePacket(&wire.TxOut{
+			Value:    50,
+			PkScript: p2wkhScript,
+		}),
+		changeIndex:             0,
+		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio,
+		changeType:              chanfunding.ExistingChangeAddress,
+		expectedUtxoIndexes:     []int{0},
+		expectChangeOutputIndex: 0,
+		expectedFee:             calcFee(0, 1, 0, 1, 0),
+
+		expectedContainedErrStr: "fee 0.00000111 BTC exceeds max fee " +
+			"(0.00000027 BTC) on total output value",
+	}, {
+		name: "1 p2wpkh utxo, existing p2wkh change, negative feeratio",
+		utxos: []*lnwallet.Utxo{
+			{
+				Value:    250,
+				PkScript: p2wkhScript,
+			},
+		},
+		packet: makePacket(&wire.TxOut{
+			Value:    50,
+			PkScript: p2wkhScript,
+		}),
+		changeIndex:             0,
+		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             chanfunding.DefaultMaxFeeRatio * (-1),
+		changeType:              chanfunding.ExistingChangeAddress,
+		expectedUtxoIndexes:     []int{0},
+		expectChangeOutputIndex: 0,
+		expectedFee:             calcFee(0, 1, 0, 1, 0),
+
+		expectedContainedErrStr: "maxFeeRatio must be between 0.00 " +
+			"and 1.00 got -0.20",
+	}, {
+		name: "1 p2wpkh utxo, existing p2wkh change, big fee ratio",
+		utxos: []*lnwallet.Utxo{
+			{
+				Value:    250,
+				PkScript: p2wkhScript,
+			},
+		},
+		packet: makePacket(&wire.TxOut{
+			Value:    50,
+			PkScript: p2wkhScript,
+		}),
+		changeIndex:             0,
+		feeRate:                 chainfee.FeePerKwFloor,
+		maxFeeRatio:             0.85,
+		changeType:              chanfunding.ExistingChangeAddress,
+		expectedUtxoIndexes:     []int{0},
+		expectChangeOutputIndex: 0,
+		expectedFee:             calcFee(0, 1, 0, 1, 0),
 	}, {
 		name: "large existing p2tr input, fee estimation existing " +
 			"change output",
@@ -536,6 +620,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 		}),
 		changeIndex:                0,
 		feeRate:                    chainfee.FeePerKwFloor,
+		maxFeeRatio:                chanfunding.DefaultMaxFeeRatio,
 		changeType:                 chanfunding.ExistingChangeAddress,
 		expectedUtxoIndexes:        []int{},
 		expectChangeOutputIndex:    0,
@@ -575,6 +660,7 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 				"", tc.changeIndex, copiedPacket, 0,
 				tc.changeType, tc.feeRate,
 				rpcServer.cfg.CoinSelectionStrategy,
+				tc.maxFeeRatio,
 			)
 
 			switch {
@@ -588,6 +674,12 @@ func TestFundPsbtCoinSelect(t *testing.T) {
 				require.Error(tt, err)
 				require.ErrorAs(tt, err, &tc.expectedErr)
 
+				return
+			case tc.expectedContainedErrStr != "":
+				require.ErrorContains(
+					tt, err, tc.expectedContainedErrStr,
+				)
+
 				return
 			}