splice: prevent splice going to onchaind & race prevention

Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called *anyway* on restart. This is now fixed.

Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.

Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.

Changelog-Fixed: Splice: bugfix for restart related race condition interacting with adversarial close detection.

lightningd/channel.c

@@ -176,6 +176,7 @@ new_inflight(struct channel *channel,
 	inflight->lease_amt = lease_amt;
 
 	inflight->i_am_initiator = i_am_initiator;
+	inflight->splice_locked_memonly = false;
 
 	list_add_tail(&channel->inflights, &inflight->list);
 	tal_add_destructor(inflight, destroy_inflight);

lightningd/channel.h

@@ -63,6 +63,16 @@ struct channel_inflight {
 
 	/* Did I initate this splice attempt? */
 	bool i_am_initiator;
+
+	/* Note: This field is not stored in the database.
+	 *
+	 * After splice_locked, we need a way to stop the chain watchers from
+	 * thinking the old channel was spent.
+	 *
+	 * Leaving the inflight in memory-only with splice_locked true leaves
+	 * moves the responsiblity of cleaning up the inflight to the watcher,
+	 * avoiding any potential race conditions. */
+	bool splice_locked_memonly;
 };
 
 struct open_attempt {

lightningd/channel_control.c

@@ -825,10 +825,21 @@ 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);
 
-	/* Empty out the inflights */
 	log_debug(channel->log, "lightningd, splice_locked clearing inflights");
+
+	/* Take out the successful inflight from the list temporarily */
+	list_del(&inflight->list);
+
 	wallet_channel_clear_inflights(channel->peer->ld->wallet, channel);
 
+	/* Put the successful inflight back in as a memory-only object.
+	 * peer_control's funding_spent function will pick this up and clean up
+	 * our inflight.
+	 *
+	 * This prevents any potential race conditions between us and them. */
+	inflight->splice_locked_memonly = true;
+	list_add_tail(&channel->inflights, &inflight->list);
+
 	lockin_complete(channel, CHANNELD_AWAITING_SPLICE);
 }
 

lightningd/peer_control.c

@@ -1953,18 +1953,26 @@ static enum watch_result funding_spent(struct channel *channel,
 
 	bitcoin_txid(tx, &txid);
 
-	wallet_channeltxs_add(channel->peer->ld->wallet, channel,
-			      WIRE_ONCHAIND_INIT, &txid, 0, block->height);
-
 	/* If we're doing a splice, we expect the funding transaction to be
 	 * spent, so don't freak out and just keep watching in that case */
 	list_for_each(&channel->inflights, inflight, list) {
 		if (bitcoin_txid_eq(&txid,
 				    &inflight->funding->outpoint.txid)) {
+			/* splice_locked is a special flag that indicates this
+			 * is a memory-only inflight acting as a race condition
+			 * safeguard. When we see this, it is our responsability
+			 * to clean up this memory-only inflight. */
+			if (inflight->splice_locked_memonly) {
+				tal_free(inflight);
+				return DELETE_WATCH;
+			}
 			return KEEP_WATCHING;
 		}
 	}
 
+	wallet_channeltxs_add(channel->peer->ld->wallet, channel,
+			      WIRE_ONCHAIND_INIT, &txid, 0, block->height);
+
 	return onchaind_funding_spent(channel, tx, block->height);
 }