Skip to content
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.

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

Minor improvements in the lncli api regarding channel closures. #8350

Merged
merged 4 commits into from Jan 16, 2024

Conversation

ziggie1984
Copy link
Collaborator

@ziggie1984 ziggie1984 commented Jan 6, 2024

Fixes #8309 in renaming the command to account for the fact that only force closes can be bumped using bumpclosefee.

Moreover it enables the max_fee_rate rpc option for the closechannel cmd. By setting higher values than the normal x3 value of the ideal fee rate, the coop close can have a higher prob. to succeed.

@ziggie1984 ziggie1984 marked this pull request as ready for review January 6, 2024 23:22
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice improvements! Definitely need more clarity on those commands. Just a few nits and suggestions from my side.

cmd/lncli/commands.go Outdated Show resolved Hide resolved
cmd/lncli/commands.go Outdated Show resolved Hide resolved
cmd/lncli/walletrpc_active.go Show resolved Hide resolved
cmd/lncli/walletrpc_active.go Show resolved Hide resolved
@ziggie1984 ziggie1984 force-pushed the rename-bumpclosefee branch 3 times, most recently from f8d26a9 to b554256 Compare January 8, 2024 17:47
BumpFee now returns a status signaling the success of the operation.
Replace (hide) the old bumpclosefee cmd with the new
bumpforceclosefee cmd to avoid confusion which closing
transactions can be bumped.
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

TargetConf: uint32(ctx.Uint64("conf_target")),
SatPerVbyte: ctx.Uint64(feeRateFlag),
Force: true,
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: closing parenthesis on new line.

@@ -1067,6 +1067,8 @@ message BumpFeeRequest {
}

message BumpFeeResponse {
// The status of the bump fee operation.
string status = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

LGTM🙏

@lightninglabs-deploy
Copy link

@ziggie1984, remember to re-request review from reviewers when ready

@yyforyongyu yyforyongyu merged commit 1bc1295 into lightningnetwork:master Jan 16, 2024
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: "lncli closechannel" may sometimes ignore the --sat_per_vbyte argument
4 participants