Handle closed channels better. #8162
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
force-pushed
the
guilt/closed-channels-handle-better
branch
from
a5a3170 to
0c17cbe
Compare
We are supposed to allocate of the ctx we're passed, not tmpctx.
Doesn't matter for now, because we don't use this result with anything which outlives tmpctx,
but we're going to:
```
==47574==ERROR: AddressSanitizer: heap-use-after-free on address 0x6040005a8f38 at pc 0x55d3c584d252 bp 0x7ffddfb1b090 sp 0x7ffddfb1b088
READ of size 8 at 0x6040005a8f38 thread T0
#0 0x55d3c584d251 in json_add_closed_channel /home/runner/work/lightning/lightning/lightningd/closed_channel.c:27:3
#1 0x55d3c584ca5a in json_listclosedchannels /home/runner/work/lightning/lightning/lightningd/closed_channel.c:118:3
#2 0x55d3c58c0cbe in command_exec /home/runner/work/lightning/lightning/lightningd/jsonrpc.c:808:8
```
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/closed-channels-handle-better
branch
from
0c17cbe to
d54cd31
Compare
We're going to start loading them into memory for nicer responses if people try to reestablish closed channels, but we don't care about ones which were never actually opened. We could add a new state, but easier to simply remove them. Signed-off-by: Rusty Russell <[email protected]>
Use convenience variables. Signed-off-by: Rusty Russell <[email protected]>
Michael of Boltz told me this long ago, that when fees spiked some of their clients' opens got stuck. After two weeks their node forgot them, and when fees came back down the opens still failed. Unfortunately, I can't see an issue for this! We can give some grace: we don't want to waste too many resources, but 100 channels is nothing. The test needs to be adjusted to have *two* slow open channels, now. Changelog-Changed: Protocol: we won't forget still-opening channels after 2016 blocks, unless there are more than 100. Signed-off-by: Rusty Russell <[email protected]>
They're small, and this will allow us to efficiently respond to reestablish on them. Signed-off-by: Rusty Russell <[email protected]>
This was always false. peer_start_channeld was called in various places with the argument "NULL" instead of "false", which unfortunately compilers didn't complain about :( Signed-off-by: Rusty Russell <[email protected]>
We'll need this to send reestablish, and it is only small (max 47 sha256 per channel). Signed-off-by: Rusty Russell <[email protected]>
…closed channels. This may be useful for their recovery, though they should see the spend onchain. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: Protocol: We now reply to `channel_reestablish` even on long-closed channels.
rustyrussell
force-pushed
the
guilt/closed-channels-handle-better
branch
from
d54cd31 to
3afc03c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does several nice things: