channeld: hold shutdown until a signed splice locks

channeld/channeld.c

@@ -183,6 +183,9 @@ struct peer {
 	/* Send tx_abort on every splice we accept, once it completes */
 	bool dev_splice_abort_at_tx_complete;
 
+	/* Send shutdown without waiting for a pending splice to lock */
+	bool dev_shutdown_during_splice;
+
 	/* Signing second, send a warning instead of tx_signatures */
 	bool dev_splice_warn_instead_of_sigs;
 
@@ -554,6 +557,10 @@ static void check_mutual_splice_locked(struct peer *peer)
 	peer->splice_state->inflights = tal_free(peer->splice_state->inflights);
 	peer->splice_state->count = 0;
 	peer->splice_state->remote_locked_txid = tal_free(peer->splice_state->remote_locked_txid);
+
+	/* A shutdown waiting for this splice can go now. */
+	if (peer->send_shutdown)
+		start_commit_timer(peer);
 }
 
 static void implied_peer_splice_locked(struct peer *peer,
@@ -972,6 +979,25 @@ static bool shutdown_complete(const struct peer *peer)
  *     - MUST NOT send a `shutdown`.
  */
 /* So we only call this after reestablish or immediately after sending commit */
+static bool shared_input_finalized(const struct peer *peer,
+				   const struct inflight *inflight);
+static bool have_they_signed_inflight(const struct peer *peer,
+				      const struct inflight *inflight);
+
+/* A splice tx both sides have signed, so it can be (or has been) broadcast. */
+static bool have_signed_splice(const struct peer *peer)
+{
+	for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) {
+		const struct inflight *inflight = peer->splice_state->inflights[i];
+		if ((inflight->i_sent_sigs
+		     && (inflight->remote_tx_sigs
+			 || have_they_signed_inflight(peer, inflight)))
+		    || shared_input_finalized(peer, inflight))
+			return true;
+	}
+	return false;
+}
+
 static void maybe_send_shutdown(struct peer *peer)
 {
 	u8 *msg;
@@ -980,8 +1006,26 @@ static void maybe_send_shutdown(struct peer *peer)
 	if (!peer->send_shutdown)
 		return;
 
-	/* DTODO: Ensure 'shutdown' rules around splice are followed once those
-	 * rules get settled on spec */
+	/* BOLT-splice #2:
+	 *
+	 * A sending node:
+	 *...
+	 *   - MUST NOT send `shutdown` if there is a splice transaction that
+	 *     isn't locked yet.
+	 *
+	 * We only wait for one both sides have signed: any other may never
+	 * confirm, and waiting on it would let them stall the close (should
+	 * it confirm anyway, the mutual close loses and lightningd closes the
+	 * splice output instead).  Once they have sent `shutdown` we reply
+	 * anyway: a peer that has may never lock the splice, so waiting would
+	 * hang. */
+	if (have_signed_splice(peer)
+	    && !peer->shutdown_sent[REMOTE]
+	    && !peer->dev_shutdown_during_splice) {
+		peer_billboard(false, "Waiting for the splice to lock before"
+			       " sending shutdown");
+		return;
+	}
 
 	if (peer->shutdown_wrong_funding) {
 		tlvs = tlv_shutdown_tlvs_new(tmpctx);
@@ -4529,6 +4573,18 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
 				 " a splice while we have one pending. Did you"
 				 " mean to send SPLICE_ACK?");
 
+	/* BOLT-splice #2:
+	 *
+	 * The sending node:
+	 *...
+	 *   - MUST NOT send `splice_init` if it has previously sent `shutdown`.
+	 *
+	 * Nor will we take one (or an RBF) after sending ours.  While ours waits
+	 * for a splice to lock, an RBF of it is fine. */
+	if (peer->shutdown_sent[LOCAL] || peer->shutdown_sent[REMOTE])
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "Can't splice while shutting down");
+
 	peer->splicing = splicing_new(peer);
 
 	ictx = new_interactivetx_context(tmpctx, our_role,
@@ -7348,7 +7404,8 @@ static void init_channel(struct peer *peer)
 				    &peer->local_alias,
 				    &peer->dev_splice_abort_at_tx_complete,
 				    &peer->dev_splice_warn_instead_of_sigs,
-				    &peer->dev_delay_confirmed_stfu)) {
+				    &peer->dev_delay_confirmed_stfu,
+				    &peer->dev_shutdown_during_splice)) {
 		master_badmsg(WIRE_CHANNELD_INIT, msg);
 	}
 

channeld/channeld_wire.csv

@@ -86,6 +86,7 @@ msgdata,channeld_init,scid_alias,short_channel_id,
 msgdata,channeld_init,dev_splice_abort_at_tx_complete,bool,
 msgdata,channeld_init,dev_splice_warn_instead_of_sigs,bool,
 msgdata,channeld_init,dev_delay_confirmed_stfu,u32,
+msgdata,channeld_init,dev_shutdown_during_splice,bool,
 
 # channeld->lightningd: successfully negotated reestablishment.
 msgtype,channeld_reestablished,1101

lightningd/channel_control.c

@@ -704,7 +704,8 @@ static void handle_splice_confirmed_signed(struct lightningd *ld,
 	inflight->remote_tx_sigs = true;
 	wallet_inflight_save(ld->wallet, inflight);
 
-	if (channel->state != CHANNELD_AWAITING_SPLICE) {
+	if (channel->state != CHANNELD_AWAITING_SPLICE
+	    && channel->state != CHANNELD_SHUTTING_DOWN) {
 		log_debug(channel->log,
 			  "Would broadcast splice, but state %s"
 			  " isn't CHANNELD_AWAITING_SPLICE",
@@ -726,8 +727,10 @@ static enum watch_result splice_depth_cb(struct lightningd *ld,
 {
 	/* Usually, we're here because we're awaiting a splice, but
 	 * we could also mutual shutdown, or that weird splice_locked_memonly
-	 * hack... */
-	if (inflight->channel->state != CHANNELD_AWAITING_SPLICE) {
+	 * hack...  channeld holds a shutdown until the splice locks, so
+	 * keep telling it the depth. */
+	if (inflight->channel->state != CHANNELD_AWAITING_SPLICE
+	    && inflight->channel->state != CHANNELD_SHUTTING_DOWN) {
 		log_debug(inflight->channel->log, "Splice inflight event but not"
 			  " in AWAITING_SPLICE, ending watch of txid %s",
 			 fmt_bitcoin_txid(tmpctx, &inflight->funding->outpoint.txid));
@@ -832,22 +835,26 @@ static void handle_splice_sending_sigs(struct lightningd *ld,
 
 	/* We can get here because of a splice RBF or because re-signing during
 	 * or because of a splice RBF. In the latter case, we will be adding
-	 * adding a harmless second txid watch on the inflight. */
-	if (channel->state != CHANNELD_NORMAL
-		&& channel->state != CHANNELD_AWAITING_SPLICE) {
-		log_unusual(channel->log, "Setting state to"
-			    " CHANNELD_AWAITING_SPLICE but existing channel"
-			    " state is unexpected value %s",
-			    channel_state_str(channel->state));
-	}
+	 * adding a harmless second txid watch on the inflight.  A close
+	 * requested meanwhile stays: channeld holds the shutdown until the
+	 * splice locks. */
+	if (channel->state != CHANNELD_SHUTTING_DOWN) {
+		if (channel->state != CHANNELD_NORMAL
+			&& channel->state != CHANNELD_AWAITING_SPLICE) {
+			log_unusual(channel->log, "Setting state to"
+				    " CHANNELD_AWAITING_SPLICE but existing channel"
+				    " state is unexpected value %s",
+				    channel_state_str(channel->state));
+		}
 
-	cc = splice_command_for_chan(ld, channel);
-	/* If matching user command found, this was a user intiated splice */
-	channel_set_state(channel,
-			  channel->state,
-			  CHANNELD_AWAITING_SPLICE,
-			  cc ? REASON_USER : REASON_REMOTE,
-			  "Splice signatures sent");
+		cc = splice_command_for_chan(ld, channel);
+		/* If matching user command found, this was a user intiated splice */
+		channel_set_state(channel,
+				  channel->state,
+				  CHANNELD_AWAITING_SPLICE,
+				  cc ? REASON_USER : REASON_REMOTE,
+				  "Splice signatures sent");
+	}
 
 	watch_splice_inflight(ld, inflight);
 
@@ -1266,6 +1273,7 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
 	s64 splice_amnt;
 	struct channel_inflight *inflight;
 	struct bitcoin_txid locked_txid;
+	u32 locked_blockheight;
 
 	if (!fromwire_channeld_got_splice_locked(msg, &funding_sats,
 						 &splice_amnt,
@@ -1299,8 +1307,14 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
 	/* Remember that we got the lockin */
 	wallet_channel_save(channel->peer->ld->wallet, channel);
 
-	/* Update the scid and tell everyone */
-	change_scid(channel, *inflight->locked_scid);
+	/* inflight is freed below, along with the rest. */
+	locked_blockheight = short_channel_id_blocknum(*inflight->locked_scid);
+
+	/* Update the scid and tell everyone, unless we're shutting down:
+	 * gossip already has the channel dying under its scid (see
+	 * channel_apply_scid). */
+	if (!channel_state_closing(channel->state))
+		change_scid(channel, *inflight->locked_scid);
 
 	/* Now watch the new funding tx */
 	channel_watch_funding(channel->peer->ld, channel);
@@ -1310,8 +1324,7 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
 			      prev_our_msats,
 			      prev_funding_sats,
 			      &prev_funding_out,
-			      channel->scid ?
-			      short_channel_id_blocknum(*channel->scid) : 0,
+			      locked_blockheight,
 			      &locked_txid,
 			      inflight);
 
@@ -1329,7 +1342,12 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
 	 * funding_spend_watch. */
 	channel_watch_inflight_outs(channel->peer->ld, channel);
 
-	lockin_complete(channel, CHANNELD_AWAITING_SPLICE);
+	/* A shutdown was waiting for this: stay in CHANNELD_SHUTTING_DOWN,
+	 * but record the new funding like lockin_complete would. */
+	if (channel->state == CHANNELD_SHUTTING_DOWN)
+		channel_record_open(channel, locked_blockheight, true);
+	else
+		lockin_complete(channel, CHANNELD_AWAITING_SPLICE);
 }
 
 /* We were informed by channeld that channel is ready (reached mindepth) */
@@ -2102,7 +2120,8 @@ bool peer_start_channeld(struct channel *channel,
 				       *channel->alias[LOCAL],
 				       ld->dev_splice_abort_at_tx_complete,
 				       ld->dev_splice_warn_instead_of_sigs,
-				       ld->dev_delay_confirmed_stfu);
+				       ld->dev_delay_confirmed_stfu,
+				       ld->dev_shutdown_during_splice);
 
 	/* We don't expect a response: we are triggered by funding_depth_cb. */
 	subd_send_msg(channel->owner, take(initmsg));

lightningd/lightningd.c

@@ -148,6 +148,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	ld->dev_splice_without_unified_sigs = false;
 	ld->dev_splice_coinbase_input = false;
 	ld->dev_splice_abort_at_tx_complete = false;
+	ld->dev_shutdown_during_splice = false;
 	ld->dev_splice_rbf_any_feerate = false;
 	ld->dev_splice_warn_instead_of_sigs = false;
 	ld->dev_splice_abort_after_sigs = false;

lightningd/lightningd.h

@@ -375,6 +375,10 @@ struct lightningd {
 	 * completes, so the initiator's handling of it can be tested. */
 	bool dev_splice_abort_at_tx_complete;
 
+	/* Send shutdown without waiting for a pending splice to lock, as
+	 * peers without that rule do. */
+	bool dev_shutdown_during_splice;
+
 	/* Let splice_init propose an RBF below 25/24 of the last feerate, so
 	 * the peer's refusal can be tested. */
 	bool dev_splice_rbf_any_feerate;

lightningd/options.c

@@ -917,6 +917,10 @@ static void dev_register_opts(struct lightningd *ld)
 		     opt_set_bool,
 		     &ld->dev_splice_abort_at_tx_complete,
 		     "Send tx_abort on every splice we accept, once it completes");
+	clnopt_noarg("--dev-shutdown-during-splice", OPT_DEV,
+		     opt_set_bool,
+		     &ld->dev_shutdown_during_splice,
+		     "Send shutdown without waiting for a pending splice to lock");
 	clnopt_noarg("--dev-splice-rbf-any-feerate", OPT_DEV,
 		     opt_set_bool,
 		     &ld->dev_splice_rbf_any_feerate,

lightningd/peer_control.c

@@ -3177,9 +3177,18 @@ static void setup_peer(struct peer *peer)
 			continue;
 
 		/* Normal cases where we watch funding */
+		/* channeld holds a shutdown until a pending splice locks,
+		 * so keep watching it (not dual-open candidates, which are
+		 * not splices). */
+		case CHANNELD_SHUTTING_DOWN:
+			channel_watch_funding(ld, channel);
+			list_for_each(&channel->inflights, inflight, list)
+				if (inflight->funding->splice_remote_funding)
+					watch_splice_inflight(ld, inflight);
+			break;
+
 		case CHANNELD_AWAITING_LOCKIN:
 		case CHANNELD_NORMAL:
-		case CHANNELD_SHUTTING_DOWN:
 		case CLOSINGD_SIGEXCHANGE:
 		/* We still want to watch spend, to tell onchaind: */
 		case CLOSINGD_COMPLETE:

tests/plugins/channeld_fakenet.c

@@ -1086,6 +1086,7 @@ static struct channel *handle_init(struct info *info, const u8 *init_msg)
 	bool dev_splice_abort_at_tx_complete;
 	bool dev_splice_warn_instead_of_sigs;
 	u32 dev_delay_confirmed_stfu;
+	bool dev_shutdown_during_splice;
 	struct channel *channel;
 	const u8 *wscript;
 	char *err_reason;
@@ -1154,7 +1155,8 @@ static struct channel *handle_init(struct info *info, const u8 *init_msg)
 				    &local_alias,
 				    &dev_splice_abort_at_tx_complete,
 				    &dev_splice_warn_instead_of_sigs,
-				    &dev_delay_confirmed_stfu))
+				    &dev_delay_confirmed_stfu,
+				    &dev_shutdown_during_splice))
 		abort();
 
 	status_debug("Parsed init...");