lightningd: keep peer connected when rejecting a dead channel's reestablish

lightningd/peer_control.c

@@ -2003,6 +2003,22 @@ static void send_reestablish(struct peer *peer,
 							 msg)));
 }
 
+/* Does this peer have another channel which still wants peer comms,
+ * other than the one we're rejecting? */
+static bool peer_has_other_live_channel(const struct peer *peer,
+					const struct channel_id *except)
+{
+	struct channel *c;
+
+	list_for_each(&peer->channels, c, list) {
+		if (channel_id_eq(&c->cid, except))
+			continue;
+		if (channel_state_wants_peercomms(c->state))
+			return true;
+	}
+	return false;
+}
+
 /* 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
@@ -2191,11 +2207,23 @@ 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. */
 	subd_send_msg(ld->connectd,
 		      take(towire_connectd_peer_send_msg(NULL, &peer->id,
 							 peer->connectd_counter,
 							 error)));
+
+	/* An error is channel-scoped, so don't tear down the connection if
+	 * the peer has other channels which still want to talk to us: we
+	 * would discard their messages.  This matters when a node recovers
+	 * from a static channel backup: it reestablishes every channel it
+	 * recovered, including ones which closed since the backup, and the
+	 * reestablish reply for the dead one must not stop us replying to
+	 * its still-live siblings. */
+	if (msgtype == WIRE_CHANNEL_REESTABLISH
+	    && peer_has_other_live_channel(peer, &channel_id))
+		return;
+
 	subd_send_msg(ld->connectd,
 		      take(towire_connectd_disconnect_peer(NULL,
 							&peer->id,

tests/test_misc.py

@@ -3093,6 +3093,40 @@ def test_recoverchannel(node_factory):
     assert stubs[0] == "c3a7b9d74a174497122bc52d74d6d69836acadc77e0429c6d8b68b48d5c9139a"
 
 
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "deletes database, which is assumed sqlite3")
+def test_recoverchannel_closed_sibling(node_factory, bitcoind):
+    """Recovering an SCB with a channel that has since closed must still
+    let the peer close the channels that are still live."""
+    l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True},
+                                             {'may_reconnect': True}])
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    # Open two channels, then close the first and settle it onchain.
+    c_dead, _ = l1.fundchannel(l2, 10**5)
+    c_live, _ = l1.fundchannel(l2, 10**5)
+    l1.rpc.close(c_dead)
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    sync_blockheight(bitcoind, [l1, l2])
+    wait_for(lambda: 'ONCHAIN' in [c['state'] for c in l1.rpc.listpeerchannels()['channels'] if c.get('short_channel_id') == c_dead])
+    wait_for(lambda: 'ONCHAIN' in [c['state'] for c in l2.rpc.listpeerchannels()['channels'] if c.get('short_channel_id') == c_dead])
+
+    scb = l2.rpc.staticbackup()['scb']
+
+    # Recover l2 from the backup.  It is fully synced, so it will not notice
+    # that the first channel is already spent (no new block is mined), and so
+    # reestablishes both it and the live channel.  l1 must reject the dead one
+    # without hanging up on the live one.
+    l2.stop()
+    os.unlink(os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, 'lightningd.sqlite3'))
+    l2.start()
+    sync_blockheight(bitcoind, [l2])
+    l2.rpc.recoverchannel(scb)
+    l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
+
+    wait_for(lambda: [c['state'] for c in l1.rpc.listpeerchannels()['channels']
+                      if c.get('short_channel_id') == c_live] == ['AWAITING_UNILATERAL'])
+
+
 def test_getemergencyrecoverdata(node_factory):
     """
     Test getemergencyrecoverdata