lightningd: don't disconnect when sending error for unknown channel_reestablish

CHANGELOG.md

@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [Unreleased]
+
+### Fixed
+
+
+ - lightningd: stay connected when we send an error for an unknown channel_reestablish, fixing the reestablish reconnect loop ([#9427])
+
+[#9427]: https://github.com/ElementsProject/lightning/pull/9427
+
 ## [26.06.6] - 2026-07-20: "Quantum-Resistant Lightning Channel III"
 
 v26.06.3, v26.06.4, and v26.06.5 had issues during publishing with the pypi releases and were deleted.

lightningd/peer_control.c

@@ -112,6 +112,7 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid,
 	else
 		peer->their_features = NULL;
 
+	peer->unknown_channel_reestablishes = 0;
 	peer->dev_ignore_htlcs = false;
 
 	peer_node_id_map_add(ld->peers, peer);
@@ -1914,6 +1915,9 @@ void handle_peer_connected(struct lightningd *ld, const u8 *msg)
 	 * on peer commands, and it knows to ignore if it's wrong. */
 	peer->connectd_counter = connectd_counter;
 
+	/* Fresh connection, fresh spam allowance. */
+	peer->unknown_channel_reestablishes = 0;
+
 	/* We mark peer in "connecting" state until hooks have passed. */
 	assert(peer->connected == PEER_DISCONNECTED);
 	peer->connected = PEER_CONNECTING;
@@ -2003,6 +2007,11 @@ static void send_reestablish(struct peer *peer,
 							 msg)));
 }
 
+/* How many channel_reestablish for channels we don't know we answer without
+ * hanging up.  An honest peer needs one per stale channel; past that it's
+ * cheaper for us to make them reconnect (which connectd rate-limits). */
+#define MAX_UNKNOWN_CHANNEL_REESTABLISHES 10
+
 /* connectd tells us a peer has a message and we've not already attached
  * a subd.  Normally this is a race, but it happens for real when opening
  * a new channel, or referring to a channel we no longer want to talk to
@@ -2021,6 +2030,9 @@ void handle_peer_spoke(struct lightningd *ld, const u8 *msg)
 	int other_fd;
 	struct peer_fd *pfd;
 	char *errmsg;
+	/* Every error we send here hangs up, except the unknown-channel
+	 * reestablish case below. */
+	bool hangup = true;
 
 	if (!fromwire_connectd_peer_spoke(msg, msg, &id, &connectd_counter, &msgtype, &channel_id, &errmsg))
 		fatal("Connectd gave bad CONNECTD_PEER_SPOKE message %s",
@@ -2177,9 +2189,30 @@ void handle_peer_spoke(struct lightningd *ld, const u8 *msg)
 						"Channel is closed and forgotten");
 			goto send_error;
 		}
+		/* BOLT #1:
+		 *
+		 * A sending node:
+		 *...
+		 *  - when sending `error`:
+		 *    - MUST fail the channel(s) referred to by the error message.
+		 */
+		/* We don't know the channel, so there's nothing for us to
+		 * fail, and nothing tells us to drop the connection: `error`
+		 * exists so that we don't have to.  Staying up matters here,
+		 * because the peer needs this error to forget its own (saved,
+		 * commitment-ready) channel: hanging up races with the error
+		 * delivery, and then it retries on every reconnect (#8822).
+		 *
+		 * A hostile peer could use that to stream reestablishes for
+		 * random channel_ids down a single connection, so we only
+		 * tolerate a few before hanging up like we used to. */
+		if (++peer->unknown_channel_reestablishes
+		    <= MAX_UNKNOWN_CHANNEL_REESTABLISHES)
+			hangup = false;
+		break;
 	}
 
-	/* Weird message?  Log and reply with error. */
+	/* Unknown channel, or a weird message?  Log and reply with error. */
 	log_peer_unusual(ld->log, &peer->id,
 			 "Unknown channel %s for %s",
 			 fmt_channel_id(tmpctx,
@@ -2191,15 +2224,16 @@ void handle_peer_spoke(struct lightningd *ld, const u8 *msg)
 send_error:
 	log_peer_debug(ld->log, &peer->id, "Telling connectd to send error %s",
 		       tal_hex(tmpctx, error));
-	/* Get connectd to send error and close. */
+	/* Get connectd to send error, and (usually) close. */
 	subd_send_msg(ld->connectd,
 		      take(towire_connectd_peer_send_msg(NULL, &peer->id,
 							 peer->connectd_counter,
 							 error)));
-	subd_send_msg(ld->connectd,
-		      take(towire_connectd_disconnect_peer(NULL,
-							&peer->id,
-							peer->connectd_counter)));
+	if (hangup)
+		subd_send_msg(ld->connectd,
+			      take(towire_connectd_disconnect_peer(NULL,
+								&peer->id,
+								peer->connectd_counter)));
 	return;
 
 tell_connectd:

lightningd/peer_control.h

@@ -65,6 +65,11 @@ struct peer {
 	/* If we open a channel our direction will be this */
 	u8 direction;
 
+	/* How many channel_reestablish for channels we don't know have we seen
+	 * on this connection?  We answer those with an error and stay
+	 * connected, so we bound it to keep it from being a spam lever. */
+	u32 unknown_channel_reestablishes;
+
 	/* Swallow incoming HTLCs (for testing) */
 	bool dev_ignore_htlcs;
 };

tests/test_misc.py

@@ -3332,17 +3332,23 @@ def test_restorefrompeer(node_factory, bitcoind):
     l1.start()
     assert l1.daemon.is_in_log('Server started with public key')
 
-    # If this happens fast enough, connect fails with "disconnected
-    # during connection"
-    try:
-        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    except RpcError as err:
-        assert "disconnected during connection" in err.error['message']
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
 
     l1.daemon.wait_for_log('peer_in WIRE_PEER_STORAGE_RETRIEVAL')
 
+    # We lost our db, so l2's channel_reestablish is for a channel we don't
+    # know: we answer with an error, but we don't hang up on them any more.
+    l1.daemon.wait_for_log('Unknown channel .* for WIRE_CHANNEL_REESTABLISH')
+    assert only_one(l1.rpc.listpeers()['peers'])['connected']
+
     assert l1.rpc.restorefrompeer()['stubs'][0] == _['channel_id']
 
+    # We need to reconnect so the stub channel triggers the bogus
+    # channel_reestablish flow in peer_connected_hook_final.
+    # (l1 no longer disconnects on unknown channel_reestablish.)
+    l1.rpc.disconnect(l2.info['id'], force=True)
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
     l1.daemon.wait_for_log('Sending a bogus channel_reestablish message to make the peer unilaterally close the channel.')
     l1.daemon.wait_for_log('peer_out WIRE_ERROR')
 

tests/test_opening.py

@@ -271,6 +271,55 @@ def test_v2_open_sigs_reconnect_1(node_factory, bitcoind):
     l2.daemon.wait_for_log(r'to CHANNELD_NORMAL')
 
 
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+@pytest.mark.openchannel('v2')
+def test_v2_open_reestablish_unknown_channel(node_factory, bitcoind):
+    """ Reconnect loop from #8822.
+
+    l1 drops the last tx_complete on the floor *after* dualopend has decided
+    the commitment is ready, so l1 saves the channel in
+    DUALOPEND_OPEN_COMMIT_READY while l2 is still waiting and throws its
+    unsaved channel away.  On reconnect l1 reestablishes a channel l2 has
+    never heard of.
+
+    l2 answers with an error and stays connected: that's what lets l1's
+    dualopend actually read the error and forget the channel.  When we hung
+    up instead, the error raced the disconnect on l1's side, and whenever it
+    lost that race l1 reestablished again on every reconnect.
+    """
+    l1, l2 = node_factory.get_nodes(2,
+                                    opts=[{'disconnect': ['-WIRE_TX_COMPLETE'],
+                                           'may_reconnect': True,
+                                           'dev-no-reconnect': None},
+                                          {'may_reconnect': True,
+                                           'dev-no-reconnect': None}])
+
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['p2tr'], (2**24) / 10**8 + 0.01)
+    bitcoind.generate_block(1)
+    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0)
+
+    with pytest.raises(RpcError):
+        l1.rpc.fundchannel(l2.info['id'], 100000)
+
+    # We saved it, they didn't.
+    wait_for(lambda: [c['state'] for c in l1.rpc.listpeerchannels()['channels']]
+             == ['DUALOPEND_OPEN_COMMIT_READY'])
+    wait_for(lambda: l2.rpc.listpeerchannels()['channels'] == [])
+
+    # One reconnect has to be enough: l2 tells us it doesn't know the channel
+    # and we forget it.
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    l2.daemon.wait_for_log('Unknown channel .* for WIRE_CHANNEL_REESTABLISH')
+    l1.daemon.wait_for_log('peer_in WIRE_ERROR')
+    wait_for(lambda: l1.rpc.listpeerchannels()['channels'] == [])
+
+    # Neither side hung up over it: that's the whole point, an error we send
+    # after hanging up may never be read.
+    assert only_one(l1.rpc.listpeers()['peers'])['connected']
+    assert only_one(l2.rpc.listpeers()['peers'])['connected']
+
+
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
 @pytest.mark.openchannel('v2')
 def test_v2_open_sigs_out_of_order(node_factory, bitcoind):