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

Fix close fail retry #7072

Merged

Conversation

rustyrussell
Copy link
Contributor

There are actually other bugs in the same logic, so it would be nice to fix.

Fixes #7014

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tabs vs spaces, it's weird.  No code changes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/fix-close-fail-retry branch 2 times, most recently from 4c74ff4 to ce62b47 Compare February 14, 2024 06:23
@rustyrussell rustyrussell added this to the v24.02 milestone Feb 14, 2024
We do `memeq(a, tal_bytelen(a), b, tal_bytelen(b))` remarkably often...

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…tination.

Watchtowers changed the code so that we *always* have a channel->shutdown_scriptpubkey[LOCAL]
(see new_channel()).  The previous code had several problems:

1. It tested this for NULL, unnecessarily.
2. It allowed overriding if it was a default, *even* if we were already using it.
3. If the peer opened without option_shutdown_anysegwit, but upgraded before we closed,
   we would not recognize the default.
4. It set the final scriptpubkey (and other things!) even if the command failed.

Changelog-Fixed: JSON-RPC: `close` with `destination` works even if prior `destination` was rejected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…isting nodes.

This will solve the problem for users who already hit the bug fixed by
the previous patch!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Contributor Author

Changed to fix typos in fuzzing and make typecheck in tal_arr_eq more general.

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 9376425

lightningd/lightningd.h Show resolved Hide resolved
@cdecker
Copy link
Member

cdecker commented Feb 16, 2024

ACK 9376425

@cdecker cdecker merged commit 4c5c53c into ElementsProject:master Feb 16, 2024
36 checks passed
@cdecker cdecker deleted the guilt/fix-close-fail-retry branch February 16, 2024 16:44
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.

Attempting to close channel that does not have option_shutdown_anysegwit enabled fails, becomes uncloseable
3 participants