fix: restore missing returns in send_channel_announce_sigs#8986
Open
vincenzopalazzo wants to merge 2 commits intoElementsProject:masterfrom
Open
fix: restore missing returns in send_channel_announce_sigs#8986vincenzopalazzo wants to merge 2 commits intoElementsProject:masterfrom
vincenzopalazzo wants to merge 2 commits intoElementsProject:masterfrom
Conversation
Commit 9fb10b8 ("fixup! reestablish: Send announcement sigs when asked") converted send_channel_announce_sigs() from bool to void but removed all early-return statements. This causes the function to unconditionally send announcement_signatures even when: 1. The channel hasn't finished reestablishment yet 2. The HSM returned invalid signatures (channel_internal_error was called but execution continued) Additionally, channel_gossip_channel_reestablished() called send_channel_announce_sigs() directly when announcement_sigs_requested was true, bypassing the gossip state machine. This sent unexpected announcement_signatures to the peer, causing the remote side to drop the connection (channeld exits with status_peer_connection_lost/62208). The bug only manifests between two v26.04rc1 peers because only v26.04rc1 sets the my_current_funding_locked TLV with retransmit_flags that triggers announcement_sigs_requested=true. Fix by: - Restoring early returns in send_channel_announce_sigs() - Moving the sent_sigs guard into send_channel_announce_sigs() itself - Removing the send_channel_announce_sigs_once() wrapper - Removing the announcement_sigs_requested bypass that called send_channel_announce_sigs() outside the state machine Fixes: ElementsProject#8978 Changelog-Fixed: Peers no longer disconnect immediately after channel reestablishment.
vincenzopalazzo
force-pushed
the
claude/flamboyant-mclaren
branch
from
2d5c90d to
be6c81a
Compare
Verify that a channel stays connected after disconnect/reconnect when it has already fully exchanged announcement_signatures. The bug in send_channel_announce_sigs() caused unconditional sending of announcement_signatures on reconnect, making the peer drop the connection. Fixes: ElementsProject#8978
sangbida
reviewed
Mar 26, 2026
| l1.daemon.wait_for_log('channel_gossip: reestablished') | ||
|
|
||
| # Channel must remain connected (the bug caused immediate disconnect) | ||
| import time |
sangbida
approved these changes
Mar 26, 2026
Collaborator
sangbida
left a comment
sangbida
left a comment
There was a problem hiding this comment.
This looks good, just have a minor nit.
madelinevibes
added
26.04 bug
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
status_peer_connection_lost)send_channel_announce_sigs()when converting frombooltovoid, andchannel_gossip_channel_reestablished()called it directly bypassing the gossip state machinesent_sigsguard intosend_channel_announce_sigs(), and removes the unsafe direct call pathDetail
The
send_channel_announce_sigs()function had three missing returns:!channel->reestablishedcheck (logged but fell through)channel_internal_errorfor invalid node signaturechannel_internal_errorfor invalid bitcoin signatureAdditionally, the
announcement_sigs_requestedparameter causedsend_channel_announce_sigs()to be called directly (bypassing the state machine) when the peer setmy_current_funding_lockedTLV withretransmit_flags & 1. This sent unexpectedannouncement_signaturesto the peer, causing it to drop the connection.The bug only manifests between two v26.04rc1 peers because only v26.04rc1 sets the new
my_current_funding_lockedTLV.Fixes #8978
Test plan