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.

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

Jump to bottom

c-lightning should not disconnect when receiving a warning during quick close #4777

Open
t-bast opened this issue Sep 9, 2021 · 3 comments
Open

c-lightning should not disconnect when receiving a warning during quick close #4777

t-bast opened this issue Sep 9, 2021 · 3 comments

Comments

@t-bast
Copy link

t-bast commented Sep 9, 2021

This is related to lightning/bolts#904
Since c-lightning already supports warning messages, we should do this right now.

When c-lightning receives a warning during quick close (usually because the peer doesn't agree with the fee_range c-lightning sent), we really should not disconnect. If we stay connected and the node operator reads the warning message, they can reuse the close API to change the fee_range to something that the peer will agree with, and the mutual close is able to make progress.

But right now since c-lightning disconnects, on reconnection it will re-send the same fee_range automatically so it will instantly disconnect again, not giving the node operator an opportunity to update the fee_range.
@rustyrussell
Copy link
Contributor

Yes, we should update if they try to close again... but not disconnecting would be weird? We have nothing useful to say at this point.

@rustyrussell
Copy link
Contributor

(We always close after a warning, for this reason)

rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 13, 2021
@rustyrussell
tests: test that successive closes do update parameters (particularly…
9447381 
… feerate!).

Noted by @t-bast, this is how it should work, and this tests that it does.

See also: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 13, 2021
@rustyrussell
closingd: add notifications for feerate ranges.
65e340e 
This allows cmdline users to have more idea what's going on.

Inspired-by: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 13, 2021
@rustyrussell
closingd: add notifications for feerate ranges.
2b53587 
This allows cmdline users to have more idea what's going on.

Inspired-by: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 13, 2021
@rustyrussell
tests: test that successive closes do update parameters (particularly…
353452d 
… feerate!).

Noted by @t-bast, this is how it should work, and this tests that it does.

See also: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 13, 2021
@rustyrussell
closingd: add notifications for feerate ranges.
b365190 
This allows cmdline users to have more idea what's going on.

Inspired-by: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
@t-bast
Copy link
Author

t-bast commented Sep 13, 2021

In that case you should allow the node operator to call the close API while disconnected, that would work as well indeed.

rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 14, 2021
@rustyrussell
closingd: add notifications for feerate ranges.
2655e16 
This allows cmdline users to have more idea what's going on.

Inspired-by: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 15, 2021
@rustyrussell
tests: test that successive closes do update parameters (particularly…
e240c3f 
… feerate!).

Noted by @t-bast, this is how it should work, and this tests that it does.

See also: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Sep 15, 2021
@rustyrussell
closingd: add notifications for feerate ranges.
20dbce5 
This allows cmdline users to have more idea what's going on.

Inspired-by: ElementsProject#4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
cdecker pushed a commit that referenced this issue Sep 15, 2021
@rustyrussell @cdecker
tests: test that successive closes do update parameters (particularly…
f6a7a3f 
… feerate!).

Noted by @t-bast, this is how it should work, and this tests that it does.

See also: #4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker pushed a commit that referenced this issue Sep 15, 2021
@rustyrussell @cdecker
closingd: add notifications for feerate ranges.
d970cc0 
This allows cmdline users to have more idea what's going on.

Inspired-by: #4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rustyrussell @t-bast