lightningd: send CHANNEL_REESTABLISH ourselves on closed channels.

We used to fire up channeld to send this, but:
1. That's silly, we have all the information to make it ourselves.
2. We didn't do it if there was an error on the channel, which as of 24.02
   there always is!
3. When it did work, running channeld *stops* onchaind, indefinitely slowing recovery.

Fixes: Blockstream/greenlight#433
Changelog-Fixed: Protocol: we once again send CHANNEL_REESTABLISH responses on closing channels.
Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

lightningd/peer_control.c

@@ -1721,6 +1721,46 @@ void peer_connected(struct lightningd *ld, const u8 *msg)
 	plugin_hook_call_peer_connected(ld, cmd_id, hook_payload);
 }
 
+static void send_reestablish(struct lightningd *ld, struct channel *channel)
+{
+	u8 *msg;
+	struct secret last_remote_per_commit_secret;
+	u64 num_revocations;
+
+	/* BOLT #2:
+	 *     - if `next_revocation_number` equals 0:
+	 *       - MUST set `your_last_per_commitment_secret` to all zeroes
+	 *     - otherwise:
+	 *       - MUST set `your_last_per_commitment_secret` to the last
+	 *         `per_commitment_secret` it received
+	 */
+	num_revocations = revocations_received(&channel->their_shachain.chain);
+	if (num_revocations == 0)
+		memset(&last_remote_per_commit_secret, 0,
+		       sizeof(last_remote_per_commit_secret));
+	else if (!shachain_get_secret(&channel->their_shachain.chain,
+				      num_revocations-1,
+				      &last_remote_per_commit_secret)) {
+		channel_fail_permanent(channel,
+				       REASON_LOCAL,
+				       "Could not get revocation secret %"PRIu64,
+				       num_revocations-1);
+		return;
+	}
+
+	msg = towire_channel_reestablish(tmpctx, &channel->cid,
+					 channel->next_index[LOCAL],
+					 num_revocations,
+					 &last_remote_per_commit_secret,
+					 &channel->channel_info.remote_per_commit,
+					 /* No upgrade for you, since we're closed! */
+					 NULL);
+	subd_send_msg(ld->connectd,
+		      take(towire_connectd_peer_send_msg(NULL, &channel->peer->id,
+							 channel->peer->connectd_counter,
+							 msg)));
+}
+
 /* 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
@@ -1749,6 +1789,11 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 	/* Do we know what channel they're talking about? */
 	channel = find_channel_by_id(peer, &channel_id);
 	if (channel) {
+		/* In this case, we'll send an error below, but send reestablish reply first
+		 * in case they lost their state and need it */
+		if (msgtype == WIRE_CHANNEL_REESTABLISH && channel_state_closed(channel->state))
+			send_reestablish(ld, channel);
+
 		/* If we have a canned error for this channel, send it now */
 		if (channel->error) {
 			error = channel->error;
@@ -1790,26 +1835,6 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 			return;
 		}
 
-		if (msgtype == WIRE_CHANNEL_REESTABLISH) {
-			log_debug(channel->log,
-				  "Reestablish on %s channel: using channeld to reply",
-				  channel_state_name(channel));
-			if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-				log_broken(channel->log,
-					   "Failed to create socketpair: %s",
-					   strerror(errno));
-				error = towire_warningfmt(tmpctx, &channel->cid,
-							  "Trouble in paradise?");
-				goto send_error;
-			}
-			if (peer_start_channeld(channel, new_peer_fd(tmpctx, fds[0]), NULL, true, true)) {
-				goto tell_connectd;
-			}
-			/* FIXME: Send informative error? */
-			close(fds[1]);
-			return;
-		}
-
 		/* Send generic error. */
 		error = towire_errorfmt(tmpctx, &channel_id,
 					"channel in state %s",

lightningd/test/run-invoice-select-inchan.c

@@ -901,6 +901,11 @@ void report_subd_memleak(struct leak_detect *leak_detect UNNEEDED, struct subd *
 void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
 			   bool cooperative UNNEEDED, const struct bitcoin_tx *close_tx UNNEEDED)
 { fprintf(stderr, "resolve_close_command called!\n"); abort(); }
+/* Generated stub for shachain_get_secret */
+bool shachain_get_secret(const struct shachain *shachain UNNEEDED,
+			 u64 commit_num UNNEEDED,
+			 struct secret *preimage UNNEEDED)
+{ fprintf(stderr, "shachain_get_secret called!\n"); abort(); }
 /* Generated stub for start_leak_request */
 void start_leak_request(const struct subd_req *req UNNEEDED,
 			struct leak_detect *leak_detect UNNEEDED)

tests/test_closing.py

@@ -4126,7 +4126,6 @@ def test_anchorspend_using_to_remote(node_factory, bitcoind, anchors):
     bitcoind.generate_block(1, wait_for_mempool=2)
 
 
-@pytest.mark.xfail(strict=True)
 def test_onchain_reestablish_reply(node_factory, bitcoind, executor):
     l1, l2, l3 = node_factory.line_graph(3, opts={'may_reconnect': True,
                                                   'dev-no-reconnect': None})

wallet/test/run-wallet.c

@@ -986,6 +986,9 @@ void topology_add_sync_waiter_(const tal_t *ctx UNNEEDED,
 /* Generated stub for towire_announcement_signatures */
 u8 *towire_announcement_signatures(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED, struct short_channel_id short_channel_id UNNEEDED, const secp256k1_ecdsa_signature *node_signature UNNEEDED, const secp256k1_ecdsa_signature *bitcoin_signature UNNEEDED)
 { fprintf(stderr, "towire_announcement_signatures called!\n"); abort(); }
+/* Generated stub for towire_channel_reestablish */
+u8 *towire_channel_reestablish(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED, u64 next_commitment_number UNNEEDED, u64 next_revocation_number UNNEEDED, const struct secret *your_last_per_commitment_secret UNNEEDED, const struct pubkey *my_current_per_commitment_point UNNEEDED, const struct tlv_channel_reestablish_tlvs *channel_reestablish UNNEEDED)
+{ fprintf(stderr, "towire_channel_reestablish called!\n"); abort(); }
 /* Generated stub for towire_channeld_dev_memleak */
 u8 *towire_channeld_dev_memleak(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "towire_channeld_dev_memleak called!\n"); abort(); }