splice: Update to current spec

Updating splice related reestablish code to
lightning/bolts#1289
and
lightning/bolts#1160

Changelog-Changed: Breaking change -- if you have splicing enabled on a channel both nodes must upgrade in unison due to updating `channel_reestablish` for to new splice specifications

Author: ddustin

channeld/channeld.c

@@ -487,6 +487,32 @@ static void check_mutual_splice_locked(struct peer *peer)
 	peer->splice_state->remote_locked_txid = tal_free(peer->splice_state->remote_locked_txid);
 }
 
+static void implied_peer_splice_locked(struct peer *peer,
+				       struct bitcoin_txid splice_txid)
+{
+	/* If we've `mutual_splice_locked` but our peer hasn't, we can ignore
+	 * this message harmlessly */
+	if (!tal_count(peer->splice_state->inflights)) {
+		status_info("Peer implied redundant splice_locked, ignoring");
+		return;
+	}
+
+	/* If we've `mutual_splice_locked` but our peer hasn't, we can ignore
+	 * this message harmlessly */
+	if (!tal_count(peer->splice_state->inflights)) {
+		status_info("Peer implied redundant splice_locked, ignoring");
+		return;
+	}
+
+	peer->splice_state->remote_locked_txid = tal(peer->splice_state,
+						     struct bitcoin_txid);
+
+	*peer->splice_state->remote_locked_txid = splice_txid;
+
+	peer->splice_state->locked_ready[REMOTE] = true;
+	check_mutual_splice_locked(peer);
+}
+
 /* Our peer told us they saw our splice confirm on chain with `splice_locked`.
  * If we see it to we jump into transitioning to post-splice, otherwise we mark
  * a flag and wait until we see it on chain too. */
@@ -504,11 +530,6 @@ static void handle_peer_splice_locked(struct peer *peer, const u8 *msg)
 				"Peer sent duplicate splice_locked message %s",
 				tal_hex(tmpctx, msg));
 
-	peer->splice_state->remote_locked_txid = tal(peer->splice_state,
-						     struct bitcoin_txid);
-
-	*peer->splice_state->remote_locked_txid = splice_txid;
-
 	if (!channel_id_eq(&chanid, &peer->channel_id))
 		peer_failed_err(peer->pps, &chanid,
 				"Wrong splice lock channel id in %s "
@@ -523,8 +544,7 @@ static void handle_peer_splice_locked(struct peer *peer, const u8 *msg)
 		return;
 	}
 
-	peer->splice_state->locked_ready[REMOTE] = true;
-	check_mutual_splice_locked(peer);
+	implied_peer_splice_locked(peer, splice_txid);
 }
 
 static void handle_peer_channel_ready(struct peer *peer, const u8 *msg)
@@ -5491,8 +5511,8 @@ static void peer_reconnect(struct peer *peer,
 	bool dataloss_protect, check_extra_fields;
 	const u8 **premature_msgs = tal_arr(peer, const u8 *, 0);
 	struct inflight *inflight;
-	struct bitcoin_txid *local_next_funding, *remote_next_funding,
-			    *remote_your_last_funding;
+	struct tlv_channel_reestablish_tlvs_next_funding *local_next_funding,
+							 *remote_next_funding;
 	u64 send_next_commitment_number;
 
 	struct tlv_channel_reestablish_tlvs *send_tlvs, *recv_tlvs;
@@ -5535,17 +5555,25 @@ static void peer_reconnect(struct peer *peer,
 				 * tal off peer */
 				send_tlvs = tlv_channel_reestablish_tlvs_new(peer);
 			}
-			send_tlvs->next_funding = &inflight->outpoint.txid;
-
-			/* Eclair wants us to decrement commitment number to
-			 * indicate that we would like them to re-send
-			 * commitment signatures */
-			/* DTODO: Add bolt reference */
+			send_tlvs->next_funding = talz(send_tlvs, struct tlv_channel_reestablish_tlvs_next_funding);
+			send_tlvs->next_funding->next_funding_txid = inflight->outpoint.txid;
+
+			/* BOLT-??? #2:
+			 * The `next_funding.retransmit_flags` bitfield is used to let the
+			 * receiving peer know which messages they must retransmit for the
+			 * corresponding `next_funding_txid` after the reconnection:
+			 * | Bit Position  | Name                |
+			 * | ------------- | --------------------|
+			 * | 0             | `commitment_signed` |
+			 */
 			if (!inflight->last_tx)
-				send_next_commitment_number--;
+				send_tlvs->next_funding->retransmit_flags |= 1; /* commitment_signed */
 		}
 	}
 
+	/* BOLT-??? #2:
+	 *  - if `option_splice` was negotiated:
+	 */
 	if (feature_negotiated(peer->our_features, peer->their_features,
 			       OPT_SPLICE)) {
 		if (!send_tlvs) {
@@ -5554,46 +5582,67 @@ static void peer_reconnect(struct peer *peer,
 			send_tlvs = tlv_channel_reestablish_tlvs_new(peer);
 		}
 
-		if (peer->channel_ready[REMOTE])
-			send_tlvs->your_last_funding_locked_txid = &peer->channel->funding.txid;
-
-		send_tlvs->my_current_funding_locked_txid = &peer->channel->funding.txid;
-		status_debug("Setting send_tlvs->my_current_funding_locked_txid"
-			     " to %s",
-			     fmt_bitcoin_txid(tmpctx,
-			     		      &peer->channel->funding.txid));
-
 		for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) {
 			struct inflight *itr = peer->splice_state->inflights[i];
 			if (itr->locked_scid) {
-				send_tlvs->my_current_funding_locked_txid = &itr->outpoint.txid;
-				status_debug("Overriding send_tlvs->my_current_"
-					     "funding_locked_txid to %s because"
-					     " inflight is locked to scid %s",
-					     fmt_bitcoin_txid(tmpctx,
-					     		      &itr->outpoint.txid),
-					     fmt_short_channel_id(tmpctx,
-					     			  *itr->locked_scid));
+				peer->splice_state->short_channel_id = *itr->locked_scid;
+				peer->splice_state->locked_txid = itr->outpoint.txid;
+				peer->splice_state->locked_ready[LOCAL] = true;
 			}
 		}
+
+		/* BOLT-??? #2:
+		 *    - if a splice transaction reached acceptable depth while disconnected:
+		 *      - MUST include `my_current_funding_locked` with the txid of the latest such transaction.
+		 *    - otherwise, if it has already sent `splice_locked` for any transaction:
+		 *      - MUST include `my_current_funding_locked` with the txid of the last `splice_locked` it sent.
+		 */
+		if (peer->splice_state->locked_ready[LOCAL]) {
+
+			send_tlvs->my_current_funding_locked = talz(send_tlvs, struct tlv_channel_reestablish_tlvs_my_current_funding_locked);
+			send_tlvs->my_current_funding_locked->my_current_funding_locked_txid = peer->splice_state->locked_txid;
+			status_debug("Setting send_tlvs->my_current_funding"
+				     "_locked_txid to splice txid %s",
+				     fmt_bitcoin_txid(tmpctx,
+				     		      &peer->splice_state->locked_txid));
+		}
+		/* BOLT-??? #2:
+		 *    - otherwise, if it has already sent `channel_ready`:
+		 *      - MUST include `my_current_funding_locked` with the txid of the channel funding transaction.
+		 */
+		else if (peer->channel_ready[LOCAL]) {
+
+			send_tlvs->my_current_funding_locked = talz(send_tlvs, struct tlv_channel_reestablish_tlvs_my_current_funding_locked);
+			send_tlvs->my_current_funding_locked->my_current_funding_locked_txid = peer->channel->funding.txid;
+			status_debug("Setting send_tlvs->my_current_funding"
+				     "_locked_txid to channel txid %s",
+				     fmt_bitcoin_txid(tmpctx,
+				     		      &peer->channel->funding.txid));
+		}
+		/* BOLT-??? #2:
+		 *    - otherwise (it has never sent `channel_ready` or `splice_locked`):
+		 *      - MUST NOT include `my_current_funding_locked`.
+		*/
+		else {
+			status_debug("Not setting send_tlvs->my_current_funding"
+				     "_locked_txid (funding txid %s)",
+				     fmt_bitcoin_txid(tmpctx,
+				     		      &peer->channel->funding.txid));
+			assert(!send_tlvs->my_current_funding_locked);
+		}
 	}
 
 	status_debug("Sending channel_reestablish with"
 		     " next_funding_tx_id: %s,"
-		     " your_last_funding_locked: %s,"
 		     " my_current_funding_locked: %s,"
 		     " next_local_commit_number: %"PRIu64",",
 		     send_tlvs && send_tlvs->next_funding
 		     	? fmt_bitcoin_txid(tmpctx,
-		     			   send_tlvs->next_funding)
-		     	: "NULL",
-		     send_tlvs && send_tlvs->your_last_funding_locked_txid
-		     	? fmt_bitcoin_txid(tmpctx,
-		     			   send_tlvs->your_last_funding_locked_txid)
+		     			   &send_tlvs->next_funding->next_funding_txid)
 		     	: "NULL",
-		     send_tlvs && send_tlvs->my_current_funding_locked_txid
+		     send_tlvs && send_tlvs->my_current_funding_locked
 		     	? fmt_bitcoin_txid(tmpctx,
-		     			   send_tlvs->my_current_funding_locked_txid)
+		     			   &send_tlvs->my_current_funding_locked->my_current_funding_locked_txid)
 		     	: "NULL",
 		     send_next_commitment_number);
 
@@ -5709,7 +5758,7 @@ static void peer_reconnect(struct peer *peer,
 						  !inflight->last_tx,
 						  false,
 						  true);
-		} else if (bitcoin_txid_eq(remote_next_funding,
+		} else if (bitcoin_txid_eq(&remote_next_funding->next_funding_txid,
 					   &inflight->outpoint.txid)) {
 			/* Don't send sigs unless we have theirs */
 			assert(local_next_funding || inflight->remote_tx_sigs);
@@ -5719,11 +5768,13 @@ static void peer_reconnect(struct peer *peer,
 			if (local_next_funding)
 				assume_stfu_mode(peer);
 			resume_splice_negotiation(peer,
-						  next_commitment_number == peer->next_index[REMOTE] - 1,
+						  remote_next_funding
+						  	? remote_next_funding->retransmit_flags & 1
+						  	: false,
 						  local_next_funding && !inflight->last_tx,
 						  true,
 						  local_next_funding);
-		} else if (bitcoin_txid_eq(remote_next_funding,
+		} else if (bitcoin_txid_eq(&remote_next_funding->next_funding_txid,
 					   &peer->channel->funding.txid)) {
 			peer_failed_err(peer->pps,
 					&peer->channel_id,
@@ -5732,7 +5783,7 @@ static void peer_reconnect(struct peer *peer,
 					" active funding txid %s. Should be %s"
 					" or NULL",
 					fmt_bitcoin_txid(tmpctx,
-							 remote_next_funding),
+							 &remote_next_funding->next_funding_txid),
 					fmt_bitcoin_txid(tmpctx,
 							 &peer->channel->funding.txid),
 					fmt_bitcoin_txid(tmpctx,
@@ -5743,71 +5794,21 @@ static void peer_reconnect(struct peer *peer,
 					"Invalid reestablish with unrecognized"
 					" next_funding txid %s, should be %s",
 					fmt_bitcoin_txid(tmpctx,
-							 remote_next_funding),
+							 &remote_next_funding->next_funding_txid),
 					fmt_bitcoin_txid(tmpctx,
 							 &inflight->outpoint.txid));
 		}
 	} else if (remote_next_funding) { /* No current inflight */
 		/* If our peer is trying to negotiate details about a splice
 		 * that is already onchain, jump ahead to sending splice_lock */
-		if (bitcoin_txid_eq(remote_next_funding,
-				    &peer->channel->funding.txid)) {
+		if (bitcoin_txid_eq(&remote_next_funding->next_funding_txid,
+				    &peer->channel->funding.txid))
 			status_info("We have no pending splice but peer"
-				    " is negotiating one; resending"
-				    " splice_lock %s",
+				    " is negotiating one that matches current"
+				    " channel, ignoring it: %s",
 				    fmt_bitcoin_outpoint(tmpctx, &peer->channel->funding));
-			peer_write(peer->pps,
-				   take(towire_splice_locked(NULL,
-				   			     &peer->channel_id,
-				   			     &peer->channel->funding.txid)));
-		}
-		else {
-			splice_abort(peer, "next_funding_txid not recognized."
-					     " Sending tx_abort.");
-		}
-	}
-
-	/* Re-send `splice_locked` if an inflight is locked */
-	for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) {
-		struct inflight *itr = peer->splice_state->inflights[i];
-		if (!itr->locked_scid)
-			continue;
-
-		status_info("Resending splice_locked because an inflight %s is"
-			    " locked",
-			    fmt_bitcoin_outpoint(tmpctx, &itr->outpoint));
-		peer_write(peer->pps,
-			   take(towire_splice_locked(NULL,
-						     &peer->channel_id,
-						     &itr->outpoint.txid)));
-		peer->splice_state->locked_ready[LOCAL] = true;
-	}
-
-	/* If no inflight, no splice negotiation, but
-	   `your_last_funding_locked_txid is stale, re-send `splice_locked`. */
-	if (!inflight && !remote_next_funding
-	    && feature_negotiated(peer->our_features, peer->their_features,
-				  OPT_SPLICE)) {
-		remote_your_last_funding = recv_tlvs
-			? recv_tlvs->your_last_funding_locked_txid : NULL;
-		if (remote_your_last_funding
-			&& !bitcoin_txid_eq(&peer->channel->funding.txid,
-					    remote_your_last_funding)) {
-			status_info("Resending splice_locked with no inflight,"
-				    " no splice negotation, but we did recv"
-				    " remote_your_last_funding value of %s"
-				    " instead of %s. Our sent splice_locked"
-				    " value is %s.",
-				    remote_your_last_funding
-				    	? fmt_bitcoin_txid(tmpctx, remote_your_last_funding)
-				    	: "NULL",
-				    fmt_bitcoin_outpoint(tmpctx, &peer->channel->funding),
-				    fmt_bitcoin_txid(tmpctx, &peer->channel->funding.txid));
-			peer_write(peer->pps,
-				   take(towire_splice_locked(NULL,
-							     &peer->channel_id,
-							     &peer->channel->funding.txid)));
-		}
+		else
+			splice_abort(peer, "next_funding_txid not recognized.");
 	}
 
 	/* BOLT #2:
@@ -5835,6 +5836,26 @@ static void peer_reconnect(struct peer *peer,
 		peer_write(peer->pps, take(msg));
 	}
 
+	/* BOLT-??? #2
+	 * A receiving node:
+	 *   - if splice transactions are pending and `my_current_funding_locked` matches one of
+	 *     those splice transactions, for which it hasn't received `splice_locked` yet:
+  	 */
+	if (inflight && recv_tlvs && recv_tlvs->my_current_funding_locked) {
+		for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) {
+			struct inflight *itr = peer->splice_state->inflights[i];
+			if (!bitcoin_txid_eq(&itr->outpoint.txid,
+					     &recv_tlvs->my_current_funding_locked->my_current_funding_locked_txid))
+				continue;
+			/* BOLT-??? #2
+			 *     - MUST process `my_current_funding_locked` as if it was receiving `splice_locked`
+			 *       for this `txid`.
+			 */
+			implied_peer_splice_locked(peer, itr->outpoint.txid);
+			break;
+		}
+	}
+
 	/* Note: next_index is the index of the current commit we're working
 	 * on, but BOLT #2 refers to the *last* commit index, so we -1 where
 	 * required. */

openingd/dualopend.c

@@ -3986,8 +3986,11 @@ static void do_reconnect_dance(struct state *state)
 	 *   - MUST NOT set `next_funding_txid`.
 	 */
 	tlvs = tlv_channel_reestablish_tlvs_new(tmpctx);
-	if (!tx_state->remote_funding_sigs_rcvd)
-		tlvs->next_funding = &tx_state->funding.txid;
+	if (!tx_state->remote_funding_sigs_rcvd) {
+		tlvs->next_funding = talz(tlvs, struct tlv_channel_reestablish_tlvs_next_funding);
+		tlvs->next_funding->next_funding_txid = tx_state->funding.txid;
+		tlvs->next_funding->retransmit_flags = 1; /* COMMITMENT_SIGNED */
+	}
 
 	msg = towire_channel_reestablish
 		(NULL, &state->channel_id, 1, 0,
@@ -4060,7 +4063,7 @@ static void do_reconnect_dance(struct state *state)
 	 */
 	if (tlvs->next_funding) {
 		/* Does this match ours? */
-		if (bitcoin_txid_eq(tlvs->next_funding, &tx_state->funding.txid)) {
+		if (bitcoin_txid_eq(&tlvs->next_funding->next_funding_txid, &tx_state->funding.txid)) {
 			bool send_our_sigs = true;
 			char *err;
 			/* We haven't gotten their tx_sigs */
@@ -4089,7 +4092,7 @@ static void do_reconnect_dance(struct state *state)
 			open_abort(state, "Sent next_funding_txid %s doesn't match ours %s",
 
 				   fmt_bitcoin_txid(tmpctx,
-						    tlvs->next_funding),
+						    &tlvs->next_funding->next_funding_txid),
 				   fmt_bitcoin_txid(tmpctx,
 						    &tx_state->funding.txid));
 			return;

tests/fuzz/fuzz-wire-channel_reestablish.c

@@ -48,9 +48,7 @@ static bool equal(const struct channel_reestablish *x,
 
 	if (!tal_arr_eq(x->tlvs->next_funding, y->tlvs->next_funding))
 		return false;
-	if (!tal_arr_eq(x->tlvs->your_last_funding_locked_txid, y->tlvs->your_last_funding_locked_txid))
-		return false;
-	if (!tal_arr_eq(x->tlvs->my_current_funding_locked_txid, y->tlvs->my_current_funding_locked_txid))
+	if (!tal_arr_eq(x->tlvs->my_current_funding_locked, y->tlvs->my_current_funding_locked))
 		return false;
 #if EXPERIMENTAL_UPGRADE_ENABLED
 	if (!tal_arr_eq(x->tlvs->next_to_send, y->tlvs->next_to_send))

wire/peer_wire.csv

@@ -326,12 +326,12 @@ msgdata,channel_reestablish,next_commitment_number,u64,
 msgdata,channel_reestablish,next_revocation_number,u64,
 msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32
 msgdata,channel_reestablish,my_current_per_commitment_point,point,
-tlvtype,channel_reestablish_tlvs,next_funding,0
+tlvtype,channel_reestablish_tlvs,next_funding,1
 tlvdata,channel_reestablish_tlvs,next_funding,next_funding_txid,sha256,
-tlvtype,channel_reestablish_tlvs,your_last_funding_locked_txid,1
-tlvdata,channel_reestablish_tlvs,your_last_funding_locked_txid,your_last_funding_locked_txid,sha256,
-tlvtype,channel_reestablish_tlvs,my_current_funding_locked_txid,3
-tlvdata,channel_reestablish_tlvs,my_current_funding_locked_txid,my_current_funding_locked_txid,sha256,
+tlvdata,channel_reestablish_tlvs,next_funding,retransmit_flags,byte,
+tlvtype,channel_reestablish_tlvs,my_current_funding_locked,5
+tlvdata,channel_reestablish_tlvs,my_current_funding_locked,my_current_funding_locked_txid,sha256,
+tlvdata,channel_reestablish_tlvs,my_current_funding_locked,retransmit_flags,byte,
 msgtype,announcement_signatures,259
 msgdata,announcement_signatures,channel_id,channel_id,
 msgdata,announcement_signatures,short_channel_id,short_channel_id,