Skip to content

Make BumpFee RPC user inputs more stricter. #9470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Roasbeef merged 2 commits into from
Feb 7, 2025
Merged

Make BumpFee RPC user inputs more stricter. #9470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34c4d12
walletrpc: add new deadline-delta param to bumpfee rpc
ziggie1984 Feb 6, 2025
022b358
docs: update release-notes
ziggie1984 Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions cmd/commands/walletrpc_active.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
ziggie1984 marked this conversation as resolved.
Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update the docs for bumpclosefee. In addition, we should add the new param deadline_delta in the cli too.

Copy link
Copy Markdown
Collaborator Author

@ziggie1984 ziggie1984 Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good point, updated the bumpclosefee and bumpforceclose RPCs as well.

Copy link
Copy Markdown
Collaborator Author

@ziggie1984 ziggie1984 Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also added the conf_target (target_conf) to the bumpforceclose RPC

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",
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.`,
Comment thread
ziggie1984 marked this conversation as resolved.
},
cli.Uint64Flag{
Name: "sat_per_byte",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions docs/release-notes/release-notes-0.18.5.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions itest/lnd_onchain_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
93 changes: 65 additions & 28 deletions itest/lnd_sweep_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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).
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
Loading