Merge pull request #9470 from ziggie1984/fix-sweepInput-bug

Make BumpFee RPC user inputs more stricter.

Author: Roasbeef

cmd/commands/walletrpc_active.go

@@ -268,9 +268,10 @@ var bumpFeeCommand = cli.Command{
 		cli.Uint64Flag{
 			Name: "conf_target",
 			Usage: `
-	The deadline in number of blocks that the input should be spent within.
-	When not set, for new inputs, the default value (1008) is used; for
-	exiting inputs, their current values will be retained.`,
+	The conf target is the starting fee rate of the fee function expressed
+	in number of blocks. So instead of using sat_per_vbyte the conf target
+	can be specified and LND will query its fee estimator for the current
+	fee rate for the given target.`,
 		},
 		cli.Uint64Flag{
 			Name:   "sat_per_byte",
@@ -307,6 +308,14 @@ var bumpFeeCommand = cli.Command{
 	the budget for fee bumping; for existing inputs, their current budgets
 	will be retained.`,
 		},
+		cli.Uint64Flag{
+			Name: "deadline_delta",
+			Usage: `
+	The deadline delta in number of blocks that this input should be spent
+	within to bump the transaction. When specified also a budget value is
+	required. When the deadline is reached, ALL the budget will be spent as
+	fee.`,
+		},
 	},
 	Action: actionDecorator(bumpFee),
 }
@@ -344,11 +353,12 @@ func bumpFee(ctx *cli.Context) error {
 	}
 
 	resp, err := client.BumpFee(ctxc, &walletrpc.BumpFeeRequest{
-		Outpoint:    protoOutPoint,
-		TargetConf:  uint32(ctx.Uint64("conf_target")),
-		Immediate:   immediate,
-		Budget:      ctx.Uint64("budget"),
-		SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
+		Outpoint:      protoOutPoint,
+		TargetConf:    uint32(ctx.Uint64("conf_target")),
+		Immediate:     immediate,
+		Budget:        ctx.Uint64("budget"),
+		SatPerVbyte:   ctx.Uint64("sat_per_vbyte"),
+		DeadlineDelta: uint32(ctx.Uint64("deadline_delta")),
 	})
 	if err != nil {
 		return err
@@ -377,9 +387,10 @@ var bumpCloseFeeCommand = cli.Command{
 		cli.Uint64Flag{
 			Name: "conf_target",
 			Usage: `
-	The deadline in number of blocks that the input should be spent within.
-	When not set, for new inputs, the default value (1008) is used; for
-	exiting inputs, their current values will be retained.`,
+	The conf target is the starting fee rate of the fee function expressed
+	in number of blocks. So instead of using sat_per_vbyte the conf target
+	can be specified and LND will query its fee estimator for the current
+	fee rate for the given target.`,
 		},
 		cli.Uint64Flag{
 			Name:   "sat_per_byte",
@@ -435,8 +446,17 @@ var bumpForceCloseFeeCommand = cli.Command{
 		cli.Uint64Flag{
 			Name: "conf_target",
 			Usage: `
-	The deadline in number of blocks that the anchor output should be spent
-	within to bump the closing transaction.`,
+	The conf target is the starting fee rate of the fee function expressed
+	in number of blocks. So instead of using sat_per_vbyte the conf target
+	can be specified and LND will query its fee estimator for the current
+	fee rate for the given target.`,
+		},
+		cli.Uint64Flag{
+			Name: "deadline_delta",
+			Usage: `
+	The deadline delta in number of blocks that the anchor output should
+	be spent within to bump the closing transaction. When the deadline is
+	reached, ALL the budget will be spent as fees.`,
 		},
 		cli.Uint64Flag{
 			Name:   "sat_per_byte",
@@ -513,10 +533,11 @@ func bumpForceCloseFee(ctx *cli.Context) error {
 	resp, err := walletClient.BumpForceCloseFee(
 		ctxc, &walletrpc.BumpForceCloseFeeRequest{
 			ChanPoint:       rpcChannelPoint,
-			DeadlineDelta:   uint32(ctx.Uint64("conf_target")),
 			Budget:          ctx.Uint64("budget"),
 			Immediate:       immediate,
 			StartingFeerate: ctx.Uint64("sat_per_vbyte"),
+			TargetConf:      uint32(ctx.Uint64("conf_target")),
+			DeadlineDelta:   uint32(ctx.Uint64("deadline_delta")),
 		})
 	if err != nil {
 		return err

docs/release-notes/release-notes-0.18.5.md

@@ -50,6 +50,15 @@
 
 * [Improved user experience](https://github.com/lightningnetwork/lnd/pull/9454)
  by returning a custom error code when HTLC carries incorrect custom records.
+
+* [Make input validation stricter](https://github.com/lightningnetwork/lnd/pull/9470)
+  when using the `BumpFee`, `BumpCloseFee(deprecated)` and `BumpForceCloseFee` 
+  RPCs. For the `BumpFee` RPC the new param `deadline_delta` is introduced. For
+  the `BumpForceCloseFee` RPC the param `conf_target` was added. The conf_target
+  changed in its meaning for all the RPCs which had it before. Now it is used
+  for estimating the starting fee rate instead of being treated as the deadline,
+  and it cannot be set together with `StartingFeeRate`. Moreover if the user now
+  specifies the `deadline_delta` param, the budget value has to be set as well.
 
 ## Tooling and Documentation
 

itest/lnd_onchain_test.go

@@ -490,12 +490,12 @@ func testAnchorThirdPartySpend(ht *lntest.HarnessTest) {
 		commit.DeadlineHeight, anchor.DeadlineHeight)
 
 	// Update the anchor sweep's deadline and budget so it will always be
-	// swpet.
+	// swept.
 	bumpFeeReq := &walletrpc.BumpFeeRequest{
-		Outpoint:   anchor.Outpoint,
-		TargetConf: uint32(deadline + 100),
-		Budget:     uint64(anchor.AmountSat * 10),
-		Immediate:  true,
+		Outpoint:      anchor.Outpoint,
+		DeadlineDelta: uint32(deadline + 100),
+		Budget:        uint64(anchor.AmountSat * 10),
+		Immediate:     true,
 	}
 	alice.RPC.BumpFee(bumpFeeReq)
 

itest/lnd_sweep_test.go

@@ -1723,11 +1723,42 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	// min relay fee rate is used as the current fee rate.
 	assertFeeRateEqual(startFeeRate)
 
+	// First we test the case where we specify the conf target to increase
+	// the starting fee rate of the fee function.
+	confTargetFeeRate := chainfee.SatPerVByte(50)
+	ht.SetFeeEstimateWithConf(confTargetFeeRate.FeePerKWeight(), 3)
+
+	// Second bump request - we will specify the conf target and expect a
+	// starting fee rate that is estimated using the provided estimator.
+	// - starting fee rate: 50 sat/vbyte (conf target 3).
+	// - deadline: 1008 (default deadline).
+	// - budget: 50% of the input value.
+	bumpFeeReq = &walletrpc.BumpFeeRequest{
+		Outpoint: op,
+		// We use a force param to create the sweeping tx immediately.
+		Immediate:  true,
+		TargetConf: 3,
+	}
+
+	alice.RPC.BumpFee(bumpFeeReq)
+
+	// Alice's old sweeping tx should be replaced.
+	ht.AssertTxNotInMempool(sweepTx1.TxHash())
+
+	// Assert the pending sweep is created with the expected values:
+	// - broadcast attempts: 2.
+	// - starting fee rate: 50 sat/vbyte (conf target 3).
+	// - deadline: 1008 (default deadline).
+	// - budget: 50% of the input value.
+	sweepTx2 := assertPendingSweepResp(
+		2, uint64(value/2), deadline, uint64(confTargetFeeRate),
+	)
+
 	// testFeeRate sepcifies a starting fee rate in sat/vbyte.
 	const testFeeRate = uint64(100)
 
-	// Second bump request - we will specify the fee rate and expect a fee
-	// func that has,
+	// Third bump request - we will specify the fee rate and expect a fee
+	// func to change the starting fee rate of the fee function,
 	// - starting fee rate: 100 sat/vbyte.
 	// - deadline: 1008 (default deadline).
 	// - budget: 50% of the input value.
@@ -1740,15 +1771,15 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	alice.RPC.BumpFee(bumpFeeReq)
 
 	// Alice's old sweeping tx should be replaced.
-	ht.AssertTxNotInMempool(sweepTx1.TxHash())
+	ht.AssertTxNotInMempool(sweepTx2.TxHash())
 
 	// Assert the pending sweep is created with the expected values:
-	// - broadcast attempts: 2.
+	// - broadcast attempts: 3.
 	// - starting fee rate: 100 sat/vbyte.
 	// - deadline: 1008 (default deadline).
 	// - budget: 50% of the input value.
-	sweepTx2 := assertPendingSweepResp(
-		2, uint64(value/2), deadline, testFeeRate,
+	sweepTx3 := assertPendingSweepResp(
+		3, uint64(value/2), deadline, testFeeRate,
 	)
 
 	// We expect the requested starting fee rate to be the current fee
@@ -1758,7 +1789,7 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	// testBudget specifies a budget in sats.
 	testBudget := uint64(float64(value) * 0.1)
 
-	// Third bump request - we will specify the budget and expect a fee
+	// Fourth bump request - we will specify the budget and expect a fee
 	// func that has,
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
 	// - deadline: 1008 (default deadline).
@@ -1772,14 +1803,14 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	alice.RPC.BumpFee(bumpFeeReq)
 
 	// Alice's old sweeping tx should be replaced.
-	ht.AssertTxNotInMempool(sweepTx2.TxHash())
+	ht.AssertTxNotInMempool(sweepTx3.TxHash())
 
 	// Assert the pending sweep is created with the expected values:
-	// - broadcast attempts: 3.
+	// - broadcast attempts: 4.
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
 	// - deadline: 1008 (default deadline).
 	// - budget: 10% of the input value.
-	sweepTx3 := assertPendingSweepResp(3, testBudget, deadline, 0)
+	sweepTx4 := assertPendingSweepResp(4, testBudget, deadline, testFeeRate)
 
 	// We expect the current fee rate to be increased because we ensure the
 	// initial broadcast always succeeds.
@@ -1789,34 +1820,37 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	testDeadlineDelta := uint32(100)
 	deadlineHeight := uint32(currentHeight) + testDeadlineDelta
 
-	// Fourth bump request - we will specify the deadline and expect a fee
+	// Fifth bump request - we will specify the deadline and expect a fee
 	// func that has,
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
 	// - deadline: 100.
 	// - budget: 10% of the input value, stays unchanged.
 	bumpFeeReq = &walletrpc.BumpFeeRequest{
 		Outpoint: op,
 		// We use a force param to create the sweeping tx immediately.
-		Immediate:  true,
-		TargetConf: testDeadlineDelta,
+		Immediate:     true,
+		DeadlineDelta: testDeadlineDelta,
+		Budget:        testBudget,
 	}
 	alice.RPC.BumpFee(bumpFeeReq)
 
 	// Alice's old sweeping tx should be replaced.
-	ht.AssertTxNotInMempool(sweepTx3.TxHash())
+	ht.AssertTxNotInMempool(sweepTx4.TxHash())
 
 	// Assert the pending sweep is created with the expected values:
-	// - broadcast attempts: 4.
+	// - broadcast attempts: 5.
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
 	// - deadline: 100.
 	// - budget: 10% of the input value, stays unchanged.
-	sweepTx4 := assertPendingSweepResp(4, testBudget, deadlineHeight, 0)
+	sweepTx5 := assertPendingSweepResp(
+		5, testBudget, deadlineHeight, testFeeRate,
+	)
 
 	// We expect the current fee rate to be increased because we ensure the
 	// initial broadcast always succeeds.
 	assertFeeRateGreater(testFeeRate)
 
-	// Fifth bump request - we test the behavior of `Immediate` - every
+	// Sixth bump request - we test the behavior of `Immediate` - every
 	// time it's called, the fee function will keep increasing the fee rate
 	// until the broadcast can succeed. The fee func that has,
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
@@ -1830,14 +1864,16 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 	alice.RPC.BumpFee(bumpFeeReq)
 
 	// Alice's old sweeping tx should be replaced.
-	ht.AssertTxNotInMempool(sweepTx4.TxHash())
+	ht.AssertTxNotInMempool(sweepTx5.TxHash())
 
 	// Assert the pending sweep is created with the expected values:
-	// - broadcast attempts: 5.
+	// - broadcast attempts: 6.
 	// - starting fee rate: 100 sat/vbyte, stays unchanged.
 	// - deadline: 100, stays unchanged.
 	// - budget: 10% of the input value, stays unchanged.
-	sweepTx5 := assertPendingSweepResp(5, testBudget, deadlineHeight, 0)
+	sweepTx6 := assertPendingSweepResp(
+		6, testBudget, deadlineHeight, testFeeRate,
+	)
 
 	// We expect the current fee rate to be increased because we ensure the
 	// initial broadcast always succeeds.
@@ -1855,26 +1891,27 @@ func runBumpFee(ht *lntest.HarnessTest, alice *node.HarnessNode) {
 		// We use a force param to create the sweeping tx immediately.
 		Immediate:   true,
 		SatPerVbyte: startFeeRate,
-		Budget:      smallBudget,
-		TargetConf:  uint32(sweep.DefaultDeadlineDelta),
+		// The budget and the deadline delta must be set together.
+		Budget:        smallBudget,
+		DeadlineDelta: uint32(sweep.DefaultDeadlineDelta),
 	}
 	alice.RPC.BumpFee(bumpFeeReq)
 
 	// Assert the pending sweep is created with the expected values:
-	// - broadcast attempts: 6.
+	// - broadcast attempts: 7.
 	// - starting fee rate: 1 sat/vbyte.
 	// - deadline: 1008.
 	// - budget: 1000 sats.
-	sweepTx6 := assertPendingSweepResp(
-		6, smallBudget, deadline, startFeeRate,
+	sweepTx7 := assertPendingSweepResp(
+		7, smallBudget, deadline, startFeeRate,
 	)
 
 	// Since this budget is too small to cover the RBF, we expect the
 	// sweeping attempt to fail.
 	//
-	require.Equal(ht, sweepTx5.TxHash(), sweepTx6.TxHash(), "tx5 should "+
-		"not be replaced: tx5=%v, tx6=%v", sweepTx5.TxHash(),
-		sweepTx6.TxHash())
+	require.Equal(ht, sweepTx6.TxHash(), sweepTx7.TxHash(), "tx6 should "+
+		"not be replaced: tx6=%v, tx7=%v", sweepTx6.TxHash(),
+		sweepTx7.TxHash())
 
 	// We expect the current fee rate to be increased because we ensure the
 	// initial broadcast always succeeds.