Splice: Clone psbts instead of steal

The interaction betwen libwally and CLN’s memory management is tricky. Let’s dodge that problem and just clone the PSBTs.

Author: ddustin

channeld/channeld.c

@@ -3829,7 +3829,7 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
 	new_inflight->remote_funding = peer->splicing->remote_funding_pubkey;
 	new_inflight->outpoint = outpoint;
 	new_inflight->amnt = both_amount;
-	new_inflight->psbt = tal_steal(new_inflight, ictx->current_psbt);
+	new_inflight->psbt = clone_psbt(new_inflight, ictx->current_psbt);
 	new_inflight->splice_amnt = peer->splicing->accepter_relative;
 	new_inflight->last_tx = NULL;
 	new_inflight->i_am_initiator = false;
@@ -3858,13 +3858,10 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)
 	u8 *outmsg;
 	struct interactivetx_context *ictx;
 	struct bitcoin_tx *prev_tx;
+	struct wally_psbt *psbt = peer->splicing->current_psbt;
 	u32 sequence = 0;
 	u8 *scriptPubkey;
 
-	/* DTODO: Remove ictx from this function as its no longer used. */
-	ictx = new_interactivetx_context(tmpctx, TX_INITIATOR,
-					 peer->pps, peer->channel_id);
-
 	if (!fromwire_splice_ack(inmsg,
 				 &channel_id,
 				 &peer->splicing->accepter_relative,
@@ -3886,10 +3883,6 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)
 	peer->splice_state->locked_ready[LOCAL] = false;
 	peer->splice_state->locked_ready[REMOTE] = false;
 
-	ictx->next_update_fn = next_splice_step;
-	ictx->pause_when_complete = true;
-	ictx->desired_psbt = peer->splicing->current_psbt;
-
 	/* We go first as the receiver of the ack.
 	 *
 	 * BOLT-0d8b701614b09c6ee4172b04da2203e73deec7e2 #2:
@@ -3904,7 +3897,7 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)
 					     &peer->channel->funding_pubkey[LOCAL],
 					     &peer->splicing->remote_funding_pubkey);
 
-	input_index = ictx->desired_psbt->num_inputs;
+	input_index = psbt->num_inputs;
 
 	/* First we spend the existing channel outpoint
 	 *
@@ -3913,21 +3906,21 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)
 	 *     - MUST `tx_add_input` an input which spends the current funding
 	 *       transaction output.
 	 */
-	psbt_append_input(ictx->desired_psbt, &peer->channel->funding, sequence,
-			  NULL, wit_script, NULL);
+	psbt_append_input(psbt, &peer->channel->funding, sequence, NULL,
+			  wit_script, NULL);
 
 	/* Segwit requires us to store the value of the outpoint being spent,
 	 * so let's do that */
-	scriptPubkey = scriptpubkey_p2wsh(ictx->desired_psbt, wit_script);
-	psbt_input_set_wit_utxo(ictx->desired_psbt, input_index,
+	scriptPubkey = scriptpubkey_p2wsh(psbt, wit_script);
+	psbt_input_set_wit_utxo(psbt, input_index,
 				scriptPubkey, peer->channel->funding_sats);
 
 	/* We must loading the funding tx as our previous utxo */
 	prev_tx = bitcoin_tx_from_txid(peer, peer->channel->funding.txid);
-	psbt_input_set_utxo(ictx->desired_psbt, input_index, prev_tx->wtx);
+	psbt_input_set_utxo(psbt, input_index, prev_tx->wtx);
 
 	/* PSBT v2 requires this */
-	psbt_input_set_outpoint(ictx->desired_psbt, input_index,
+	psbt_input_set_outpoint(psbt, input_index,
 				peer->channel->funding);
 
 	/* Next we add the new channel outpoint, with a 0 amount for now. It
@@ -3939,26 +3932,23 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)
 	 *     - MUST `tx_add_output` a zero-value output which pays to the two
 	 *       funding keys using the higher of the two `generation` fields.
 	 */
-	psbt_append_output(ictx->desired_psbt,
-			   scriptpubkey_p2wsh(ictx->desired_psbt, new_wit_script),
+	psbt_append_output(psbt,
+			   scriptpubkey_p2wsh(psbt, new_wit_script),
 			   calc_balance(peer));
 
-	psbt_add_serials(ictx->desired_psbt, ictx->our_role);
-
-	ictx->shared_outpoint = tal(ictx, struct bitcoin_outpoint);
-	*ictx->shared_outpoint = peer->channel->funding;
-	ictx->funding_tx = prev_tx;
+	psbt_add_serials(psbt, TX_INITIATOR);
 
 	peer->splicing->tx_add_input_count = 0;
 	peer->splicing->tx_add_output_count = 0;
 
 	peer->splicing->mode = true;
 
 	/* Return the current PSBT to the channel_control to give to user. */
-	outmsg = towire_channeld_splice_confirmed_init(NULL,
-						       ictx->desired_psbt);
+	outmsg = towire_channeld_splice_confirmed_init(NULL, psbt);
 	wire_sync_write(MASTER_FD, take(outmsg));
 
+	/* We reset current_psbt to empty as now it represends the difference
+	 * what we've sent our peer so far */
 	tal_free(peer->splicing->current_psbt);
 	peer->splicing->current_psbt = create_psbt(peer->splicing, 0, 0, 0);
 }
@@ -3993,7 +3983,10 @@ static void splice_initiator_user_finalized(struct peer *peer)
 
 	ictx->next_update_fn = next_splice_step;
 	ictx->pause_when_complete = false;
-	ictx->desired_psbt = ictx->current_psbt = peer->splicing->current_psbt;
+	ictx->desired_psbt = ictx->current_psbt = clone_psbt(ictx,
+							     peer->splicing->current_psbt);
+	tal_free(peer->splicing->current_psbt);
+	peer->splicing->current_psbt = NULL;
 	ictx->tx_add_input_count = peer->splicing->tx_add_input_count;
 	ictx->tx_add_output_count = peer->splicing->tx_add_output_count;
 
@@ -4066,9 +4059,7 @@ static void splice_initiator_user_finalized(struct peer *peer)
 	 * normal in-memory copy of the psbt: peer->splicing/ictx->current_psbt.
 	 * Since we have to support using the inflight psbt anyway, we default
 	 * to it. */
-	new_inflight->psbt = tal_steal(new_inflight, ictx->current_psbt);
-	ictx->current_psbt = NULL;
-	peer->splicing->current_psbt = NULL;
+	new_inflight->psbt = clone_psbt(new_inflight, ictx->current_psbt);
 
 	current_push_val = relative_splice_balance_fundee(peer, our_role,
 							  new_inflight->psbt,
@@ -4145,6 +4136,8 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg)
 
 	/* Should already have a current_psbt from a previously initiated one */
 	assert(peer->splicing->current_psbt);
+	/* peer->splicing->current_psbt represents what PSBT we have sent to
+	 * our peer so far. */
 	ictx->current_psbt = peer->splicing->current_psbt;
 	ictx->tx_add_input_count = peer->splicing->tx_add_input_count;
 	ictx->tx_add_output_count = peer->splicing->tx_add_output_count;
@@ -4170,8 +4163,8 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg)
 
 	if (peer->splicing->current_psbt != ictx->current_psbt)
 		tal_free(peer->splicing->current_psbt);
-	peer->splicing->current_psbt = tal_steal(peer->splicing,
-						 ictx->current_psbt);
+	peer->splicing->current_psbt = clone_psbt(peer->splicing,
+						  ictx->current_psbt);
 
 	/* Peer may have modified our PSBT so we return it to the user here */
 	outmsg = towire_channeld_splice_confirmed_update(NULL,
@@ -4205,7 +4198,7 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg)
 		return;
 	}
 
-	if (!fromwire_channeld_splice_signed(tmpctx, inmsg, &signed_psbt,
+	if (!fromwire_channeld_splice_signed(inflight, inmsg, &signed_psbt,
 					     &peer->splicing->force_sign_first))
 		master_badmsg(WIRE_CHANNELD_SPLICE_SIGNED, inmsg);
 
@@ -4244,7 +4237,7 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg)
 			      fmt_bitcoin_txid(tmpctx, &current_psbt_txid));
 
 	tal_free(inflight->psbt);
-	inflight->psbt = tal_steal(inflight, signed_psbt);
+	inflight->psbt = clone_psbt(inflight, signed_psbt);
 
 	/* Save the user provided signatures to DB incase we have to
 	 * restart and reestablish later. */
@@ -4255,7 +4248,7 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg)
 
 	wire_sync_write(MASTER_FD, take(outmsg));
 
-	sign_first = do_i_sign_first(peer, signed_psbt, TX_INITIATOR,
+	sign_first = do_i_sign_first(peer, inflight->psbt, TX_INITIATOR,
 				     inflight->force_sign_first);
 
 	resume_splice_negotiation(peer, false, false, true, sign_first);
@@ -5612,6 +5605,7 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg)
 				if (bitcoin_txid_eq(&inflight->outpoint.txid,
 						    &txid)) {
 					inflight->is_locked = true;
+					assert(inflight->psbt);
 					msg = towire_channeld_update_inflight(NULL,
 									      inflight->psbt,
 									      NULL,