Skip to content

fix: restore missing returns in send_channel_announce_sigs #8986

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

Closed
vincenzopalazzo wants to merge 4 commits into from
+84 −26
Closed

fix: restore missing returns in send_channel_announce_sigs #8986

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be6c81a
fix: restore missing returns in send_channel_announce_sigs
vincenzopalazzo Mar 25, 2026
ce0cec7
tests: add regression test for reestablish announcement_sigs bug
vincenzopalazzo Mar 25, 2026
4fd8efb
fix: address PR review feedback
vincenzopalazzo Mar 26, 2026
7ee2066
tests: strengthen reestablish announcement sigs regression
vincenzopalazzo Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lightningd/channel_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ static void peer_channeld_reestablished(struct channel *channel, const u8* msg)
"bad channeld_reestablished %s",
tal_hex(channel, msg));

channel_gossip_channel_reestablished(channel, announcement_sigs_requested);
channel_gossip_channel_reestablished(channel);
}

void channel_fallen_behind(struct channel *channel)
Expand Down Expand Up @@ -1976,7 +1976,7 @@ bool peer_start_channeld(struct channel *channel,

/* "Reestablished" if we've just opened. */
if (!reconnected)
channel_gossip_channel_reestablished(channel, false);
channel_gossip_channel_reestablished(channel);

/* FIXME: DTODO: Use a pointer to a txid instead of zero'ing one out. */
memset(&txid, 0, sizeof(txid));
Expand Down
39 changes: 17 additions & 22 deletions lightningd/channel_gossip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,13 @@ static void send_channel_announce_sigs(struct channel *channel)
const u8 *ca, *msg;

/* Wait until we've exchanged reestablish messages */
if (!channel->reestablished)
if (!channel->reestablished) {
log_debug(channel->log, "channel_gossip: not sending channel_announcement_sigs until reestablished");
return;
}

if (cg->sent_sigs)
return;

ca = create_channel_announcement(tmpctx, channel, *channel->scid,
NULL, NULL, NULL, NULL);
Expand All @@ -714,13 +719,17 @@ static void send_channel_announce_sigs(struct channel *channel)

/* Double-check that HSM gave valid signatures. */
sha256_double(&hash, ca + offset, tal_count(ca) - offset);
if (!check_signed_hash(&hash, &local_node_sig, &ld->our_pubkey))
if (!check_signed_hash(&hash, &local_node_sig, &ld->our_pubkey)) {
channel_internal_error(channel,
"HSM returned an invalid node signature");
return;
}

if (!check_signed_hash(&hash, &local_bitcoin_sig, &channel->local_funding_pubkey))
if (!check_signed_hash(&hash, &local_bitcoin_sig, &channel->local_funding_pubkey)) {
channel_internal_error(channel,
"HSM returned an invalid bitcoin signature");
return;
}

msg = towire_announcement_signatures(NULL,
&channel->cid, *channel->scid,
Expand All @@ -729,16 +738,6 @@ static void send_channel_announce_sigs(struct channel *channel)
cg->sent_sigs = true;
}

static void send_channel_announce_sigs_once(struct channel *channel)
{
struct channel_gossip *cg = channel->channel_gossip;

if (cg->sent_sigs)
return;

send_channel_announce_sigs(channel);
}

/* Sends channel_announcement */
static void send_channel_announcement(struct channel *channel)
{
Expand Down Expand Up @@ -830,7 +829,7 @@ static void set_gossip_state(struct channel *channel,
case CGOSSIP_ANNOUNCED:
/* In case this snuck up on us (fast confirmations),
* make sure we sent sigs */
send_channel_announce_sigs_once(channel);
send_channel_announce_sigs(channel);

/* BOLT #7:
* A recipient node:
Expand Down Expand Up @@ -898,7 +897,7 @@ static void update_gossip_state(struct channel *channel)
return;
case CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS:
case CGOSSIP_WAITING_FOR_ANNOUNCE_DEPTH:
send_channel_announce_sigs_once(channel);
send_channel_announce_sigs(channel);
/* fall thru */
case CGOSSIP_WAITING_FOR_SCID:
case CGOSSIP_PRIVATE:
Expand Down Expand Up @@ -1006,7 +1005,7 @@ void channel_gossip_got_announcement_sigs(struct channel *channel,

send_our_sigs:
/* This only works once, so we won't spam them. */
send_channel_announce_sigs_once(channel);
send_channel_announce_sigs(channel);
}

/* Short channel id changed (splice, or reorg). */
Expand Down Expand Up @@ -1202,8 +1201,7 @@ static void channel_reestablished_stable(struct channel *channel)
}

/* Peer has connected and successfully reestablished channel. */
void channel_gossip_channel_reestablished(struct channel *channel,
bool announcement_sigs_requested)
void channel_gossip_channel_reestablished(struct channel *channel)
{
channel->reestablished = true;
tal_free(channel->stable_conn_timer);
Expand All @@ -1221,9 +1219,6 @@ void channel_gossip_channel_reestablished(struct channel *channel,
/* We can re-xmit sigs once per reconnect */
channel->channel_gossip->sent_sigs = false;

if (announcement_sigs_requested)
send_channel_announce_sigs(channel);

/* BOLT #7:
* - Upon reconnection (once the above timing requirements have
* been met):
Expand All @@ -1244,7 +1239,7 @@ void channel_gossip_channel_reestablished(struct channel *channel,
check_channel_gossip(channel);
return;
case CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS:
send_channel_announce_sigs_once(channel);
send_channel_announce_sigs(channel);
/* fall thru */
case CGOSSIP_PRIVATE:
case CGOSSIP_WAITING_FOR_ANNOUNCE_DEPTH:
Expand Down
3 changes: 1 addition & 2 deletions lightningd/channel_gossip.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ void channel_gossip_update_from_gossipd(struct channel *channel,
void channel_gossip_init_done(struct lightningd *ld);

/* Peer has connected and successfully reestablished channel. */
void channel_gossip_channel_reestablished(struct channel *channel,
bool announcement_sigs_requested);
void channel_gossip_channel_reestablished(struct channel *channel);

/* Peer has disconnected */
void channel_gossip_channel_disconnect(struct channel *channel);
Expand Down
35 changes: 35 additions & 0 deletions tests/test_gossip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,41 @@ def count_active(node):
wait_for(lambda: count_active(l2) == 2)


def test_reestablish_announcement_sigs(node_factory, bitcoind):
"""Regression test: peers must not disconnect after reestablishing
a channel that has already exchanged announcement_signatures.

A bug in send_channel_announce_sigs() caused it to unconditionally
send announcement_signatures on reconnect (missing early returns),
which made the remote peer drop the connection.
See: https://github.com/ElementsProject/lightning/issues/8978
"""
opts = {'dev-no-reconnect': None, 'may_reconnect': True}
l1, l2 = node_factory.get_nodes(2, opts=opts)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
scid, _ = l1.fundchannel(l2, 10**6)
bitcoind.generate_block(6)

# Wait for channel to be fully announced (both sides exchanged sigs)
l1.wait_channel_active(scid)
l2.wait_channel_active(scid)

# Disconnect and reconnect - channel should stay up
l1.rpc.disconnect(l2.info['id'], force=True)
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)

# Wait for reestablishment
l1.daemon.wait_for_log('channel_gossip: reestablished')

# Channel must remain connected (the bug caused immediate disconnect)
import time
Copy link
Copy Markdown
Collaborator

@sangbida sangbida Mar 26, 2026

Choose a reason for hiding this comment

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

This is redundant.

Copy link
Copy Markdown
Collaborator Author

@vincenzopalazzo vincenzopalazzo Mar 26, 2026

Choose a reason for hiding this comment

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

Fixed in 4fd8efb. Removed the redundant local import time; tests/test_gossip.py already imports it at module scope.

time.sleep(2)
channels = l1.rpc.listpeerchannels()['channels']
assert len(channels) == 1
assert channels[0]['peer_connected'] is True


def test_announce_address(node_factory, bitcoind):
"""Make sure our announcements are well formed."""

Expand Down
Loading