closingd: send option_dataloss_protect fields when reestablishing.

Travis caught an error where this happened: when closingd reconnects it
was sending the reestablish message without the option_dataloss_protect
fields.  That causes the peer to fail the channel!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

CHANGELOG.md

@@ -25,6 +25,8 @@ changes.
 
 ### Fixed
 
+- protocol: reconnection during closing negotiation now supports
+  `option_data_loss_protect` properly.
 - `--bind-addr=<path>` fixed for nodes using local sockets (eg. testing).
 - Unannounced local channels were forgotten for routing on restart until reconnection occurred.
 

closingd/closing_wire.csv

@@ -27,6 +27,7 @@ closing_init,,channel_reestablish_len,u16
 closing_init,,channel_reestablish,channel_reestablish_len*u8
 closing_init,,final_scriptpubkey_len,u16
 closing_init,,final_scriptpubkey,final_scriptpubkey_len*u8
+closing_init,,last_remote_secret,struct secret
 
 # We received an offer, save signature.
 closing_received_signature,2002

closingd/closingd.c

@@ -112,11 +112,36 @@ static void do_reconnect(struct crypto_state *cs,
 			 const u64 next_index[NUM_SIDES],
 			 u64 revocations_received,
 			 const u8 *channel_reestablish,
-			 const u8 *final_scriptpubkey)
+			 const u8 *final_scriptpubkey,
+			 const struct secret *last_remote_per_commit_secret)
 {
 	u8 *msg;
 	struct channel_id their_channel_id;
 	u64 next_local_commitment_number, next_remote_revocation_number;
+	struct pubkey my_current_per_commitment_point;
+	struct secret *s;
+
+	/* Our current per-commitment point is the commitment point in the last
+	 * received signed commitment; HSM gives us that and the previous
+	 * secret (which we don't need). */
+	msg = towire_hsm_get_per_commitment_point(NULL,
+						  next_index[LOCAL]-1);
+	if (!wire_sync_write(HSM_FD, take(msg)))
+		status_failed(STATUS_FAIL_HSM_IO,
+			      "Writing get_per_commitment_point to HSM: %s",
+			      strerror(errno));
+
+	msg = wire_sync_read(tmpctx, HSM_FD);
+	if (!msg)
+		status_failed(STATUS_FAIL_HSM_IO,
+			      "Reading resp get_per_commitment_point reply: %s",
+			      strerror(errno));
+	if (!fromwire_hsm_get_per_commitment_point_reply(tmpctx, msg,
+					&my_current_per_commitment_point,
+					&s))
+		status_failed(STATUS_FAIL_HSM_IO,
+			      "Bad per_commitment_point reply %s",
+			      tal_hex(tmpctx, msg));
 
 	/* BOLT #2:
 	 *
@@ -135,9 +160,14 @@ static void do_reconnect(struct crypto_state *cs,
 	 *   - MUST set `next_remote_revocation_number` to the commitment number
 	 *     of the next `revoke_and_ack` message it expects to receive.
 	 */
-	msg = towire_channel_reestablish(NULL, channel_id,
+
+	/* We're always allowed to send extra fields, so we send dataloss_protect
+	 * even if we didn't negotiate it */
+	msg = towire_channel_reestablish_option_data_loss_protect(NULL, channel_id,
 					 next_index[LOCAL],
-					 revocations_received);
+					 revocations_received,
+					 last_remote_per_commit_secret,
+					 &my_current_per_commitment_point);
 	sync_crypto_write(cs, PEER_FD, take(msg));
 
 	/* They might have already send reestablish, which triggered us */
@@ -509,6 +539,7 @@ int main(int argc, char *argv[])
 	u64 next_index[NUM_SIDES], revocations_received;
 	enum side whose_turn;
 	u8 *channel_reestablish;
+	struct secret last_remote_per_commit_secret;
 
 	subdaemon_setup(argc, argv);
 
@@ -534,7 +565,8 @@ int main(int argc, char *argv[])
 				   &next_index[REMOTE],
 				   &revocations_received,
 				   &channel_reestablish,
-				   &final_scriptpubkey))
+				   &final_scriptpubkey,
+				   &last_remote_per_commit_secret))
 		master_badmsg(WIRE_CLOSING_INIT, msg);
 
 	status_trace("out = %s/%s",
@@ -553,7 +585,8 @@ int main(int argc, char *argv[])
 	if (reconnected)
 		do_reconnect(&cs, &channel_id,
 			     next_index, revocations_received,
-			     channel_reestablish, final_scriptpubkey);
+			     channel_reestablish, final_scriptpubkey,
+			     &last_remote_per_commit_secret);
 
 	/* We don't need this any more */
 	tal_free(final_scriptpubkey);

lightningd/closing_control.c

@@ -159,6 +159,7 @@ void peer_start_closingd(struct channel *channel,
 	u64 num_revocations;
 	struct amount_msat their_msat;
 	int hsmfd;
+	struct secret last_remote_per_commit_secret;
 	struct lightningd *ld = channel->peer->ld;
 
 	if (!channel->remote_shutdown_scriptpubkey) {
@@ -168,7 +169,8 @@ void peer_start_closingd(struct channel *channel,
 	}
 
 	hsmfd = hsm_get_client_fd(ld, &channel->peer->id, channel->dbid,
-				  HSM_CAP_SIGN_CLOSING_TX);
+				  HSM_CAP_SIGN_CLOSING_TX
+				  | HSM_CAP_COMMITMENT_POINT);
 
 	channel_set_owner(channel,
 			  new_channel_subd(ld,
@@ -235,6 +237,27 @@ void peer_start_closingd(struct channel *channel,
 		channel_fail_permanent(channel, "our_msat overflow on closing");
 		return;
 	}
+
+	/* BOLT #2:
+	 *
+	 *   - if it supports `option_data_loss_protect`:
+	 *     - if `next_remote_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
+	 */
+	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,
+				       "Could not get revocation secret %"PRIu64,
+				       num_revocations-1);
+		return;
+	}
 	initmsg = towire_closing_init(tmpctx,
 				      cs,
 				      &channel->funding_txid,
@@ -256,7 +279,8 @@ void peer_start_closingd(struct channel *channel,
 				      num_revocations,
 				      channel_reestablish,
 				      p2wpkh_for_keyidx(tmpctx, ld,
-							channel->final_key_idx));
+							channel->final_key_idx),
+				      &last_remote_per_commit_secret);
 
 	/* We don't expect a response: it will give us feedback on
 	 * signatures sent and received, then closing_complete. */