fix: restore missing returns in send_channel_announce_sigs

lightningd/channel_control.c

@@ -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)
@@ -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));

lightningd/channel_gossip.c

@@ -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);
@@ -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,
@@ -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)
 {
@@ -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:
@@ -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:
@@ -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). */
@@ -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);
@@ -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):
@@ -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:

lightningd/channel_gossip.h

@@ -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);

tests/test_gossip.py

@@ -116,6 +116,70 @@ def count_active(node):
     wait_for(lambda: count_active(l2) == 2)
 
 
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "sqlite3-specific DB manip")
+def test_reestablish_announcement_sigs(node_factory):
+    """Regression test: announcement_signatures must wait until
+    channel reestablishment has completed after reconnect.
+
+    A bug in send_channel_announce_sigs() caused it to unconditionally
+    send announcement_signatures on reconnect (missing early returns),
+    before the peers had exchanged channel_reestablish.
+    See: https://github.com/ElementsProject/lightning/issues/8978
+    """
+    opts = {'may_reconnect': True, 'dev-no-reconnect': None}
+    l1, l2 = node_factory.line_graph(2, wait_for_announce=True, opts=opts)
+
+    def first_log_index(node, needle, start):
+        node.daemon.logs_catchup()
+        for idx, line in enumerate(node.daemon.logs[start:], start):
+            if needle in line:
+                return idx
+        return None
+
+    # Make l2 rely on its stored announcement state, and make l1 ask for
+    # announcement_signatures again on reconnect.
+    l2.stop()
+    gs_path = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, 'gossip_store')
+    os.unlink(gs_path)
+    l2.start()
+    wait_for(lambda: l2.rpc.listchannels()['channels'] != [])
+
+    l1.db_manip("UPDATE channels SET remote_ann_node_sig=NULL, remote_ann_bitcoin_sig=NULL")
+    gs_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store')
+    os.unlink(gs_path)
+    l1.restart()
+
+    l1.daemon.logs_catchup()
+    l2.daemon.logs_catchup()
+    l1_start = len(l1.daemon.logs)
+    l2_start = len(l2.daemon.logs)
+    l1.daemon.logsearch_start = l1_start
+    l2.daemon.logsearch_start = l2_start
+
+    l1.connect(l2)
+
+    l1.daemon.wait_for_log('channel_gossip: reestablished')
+    l2.daemon.wait_for_log('channel_gossip: reestablished')
+
+    l1.daemon.wait_for_logs(['peer_out WIRE_ANNOUNCEMENT_SIGNATURES',
+                             'peer_in WIRE_ANNOUNCEMENT_SIGNATURES'])
+    l2.daemon.wait_for_log('peer_out WIRE_ANNOUNCEMENT_SIGNATURES')
+
+    l1_reestablished = first_log_index(l1, 'channel_gossip: reestablished', l1_start)
+    l2_reestablished = first_log_index(l2, 'channel_gossip: reestablished', l2_start)
+    l1_announce = first_log_index(l1, 'peer_out WIRE_ANNOUNCEMENT_SIGNATURES', l1_start)
+    l2_announce = first_log_index(l2, 'peer_out WIRE_ANNOUNCEMENT_SIGNATURES', l2_start)
+
+    assert l1_reestablished is not None
+    assert l2_reestablished is not None
+    assert l1_announce > l1_reestablished
+    assert l2_announce > l2_reestablished
+
+    l1msgs = [l.split()[4] for l in l1.daemon.logs[l1_start:] if 'WIRE_ANNOUNCEMENT_SIGNATURES' in l]
+    assert sorted(l1msgs) == ['peer_in', 'peer_out']
+    assert len([l for l in l2.daemon.logs[l2_start:] if 'peer_out WIRE_ANNOUNCEMENT_SIGNATURES' in l]) == 1
+
+
 def test_announce_address(node_factory, bitcoind):
     """Make sure our announcements are well formed."""