Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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
Collaborator

Choose a reason for hiding this comment

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

This is redundant.

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