From b0a0ef6b66b7b936c5943cf08736f455e4785dbe Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Sat, 8 Aug 2026 14:49:25 +0200 Subject: [PATCH 1/3] lightningd: don't disconnect when sending error for unknown channel_reestablish When a peer sends WIRE_CHANNEL_REESTABLISH for a channel we don't know about (e.g. dual-funding, where we deleted the unsaved channel on disconnect but the peer saved it in DUALOPEND_OPEN_COMMIT_READY), we sent an error and hung up. Our error does make it onto the wire: disconnect_peer() -> drain_peer() in connectd/multiplex.c gives peer_outq 5 seconds to flush first. The race is on the receiving node: its connectd hands the error to dualopend and tears the subds down on EOF at the same time, and when dualopend loses that race lightningd only sees "Owning subdaemon dualopend died" (subd.c passes peer_fd=NULL, disconnect=false), so dualopen_errmsg() keeps the DUALOPEND_OPEN_COMMIT_READY channel for a later reconnect. It then reestablishes on every reconnect, and we error and hang up again. BOLT #1 only requires that a node sending `error` fails the channel(s) the error refers to; it never says to drop the connection, and here we don't know the channel, so there is nothing for us to fail. So send the error and stay connected: the peer's dualopend then reliably reads it and forgets the channel. Since we no longer hang up, bound how many unknown-channel reestablishes we answer on a single connection, so a peer can't use this to make us log and write errors indefinitely. Changelog-Fixed: dual-funding reconnect loop when peer doesn't know about a saved channel Fixes: https://github.com/ElementsProject/lightning/issues/8822 Signed-off-by: Vincenzo Palazzo --- lightningd/peer_control.c | 46 ++++++++++++++++++++++++++++++++++----- lightningd/peer_control.h | 5 +++++ tests/test_misc.py | 18 ++++++++++----- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index bfee7c084e6e..9659ef02002f 100644 --- a/lightningd/peer_control.c +++ b/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: diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index e5130ddd752b..f4e8468e3855 100644 --- a/lightningd/peer_control.h +++ b/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; }; diff --git a/tests/test_misc.py b/tests/test_misc.py index a87f956cdbb5..c0c9ce55414f 100644 --- a/tests/test_misc.py +++ b/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') From 339a2b5caee9b348078fd80d32b007cd2e1e6fcb Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Sat, 8 Aug 2026 14:49:33 +0200 Subject: [PATCH 2/3] tests: check we answer an unknown channel_reestablish without hanging up Sets up #8822: l1 drops the last tx_complete after dualopend has decided the commitment is ready, so l1 saves a DUALOPEND_OPEN_COMMIT_READY channel that l2 never saved. On reconnect l1 reestablishes a channel l2 doesn't know, and l2 has to answer with an error and stay connected. Note this pins the new behaviour rather than the loop itself: the loop is a race on l1's side (see the previous commit), and locally l1 wins it and forgets the channel even without the fix, so a strict xfail reproduction would just be flaky. The test does fail without the fix, on l2 hanging up. Signed-off-by: Vincenzo Palazzo --- tests/test_opening.py | 49 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_opening.py b/tests/test_opening.py index 82f2e6baf631..0f1d4fc6691c 100644 --- a/tests/test_opening.py +++ b/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): From 92767d627076bd87fb73d44a936319e5dd7b578f Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Mon, 17 Aug 2026 13:26:25 +0200 Subject: [PATCH 3/3] changelog: add Fixed entry for #9427 Signed-off-by: Vincenzo Palazzo --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd85a1e9c59..e69d59536278 100644 --- a/CHANGELOG.md +++ b/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.