From c70942e5788c46cbf3a6a83c3d5d6a275d35c6b3 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 21 Jan 2025 10:32:07 -0500 Subject: [PATCH 01/15] Splice: Rotating funding pubkey fix Interop testing with Eclair revealed an issue with remote funding key rotation. This searches for the funding output using the rotated remote funding pubkey instead of the furrent funding pubkey. Also update the variable name to be more clear which this represents. Changelog-Changed: Interop fixes for compatability with Eclair --- channeld/channeld.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index c00081a78e0e..f7f81d44dbdf 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3306,7 +3306,7 @@ static void resume_splice_negotiation(struct peer *peer, struct bitcoin_tx *bitcoin_tx; u32 splice_funding_index; const u8 *msg, *sigmsg; - u32 chan_output_index; + u32 new_output_index; struct pubkey *their_pubkey; struct bitcoin_tx *final_tx; struct bitcoin_txid final_txid; @@ -3337,8 +3337,8 @@ static void resume_splice_negotiation(struct peer *peer, &peer->channel->funding_pubkey[LOCAL], &peer->channel->funding_pubkey[REMOTE]); - find_channel_output(peer, current_psbt, &chan_output_index, - &peer->channel->funding_pubkey[REMOTE]); + find_channel_output(peer, current_psbt, &new_output_index, + &inflight->remote_funding); splice_funding_index = find_channel_funding_input(current_psbt, &peer->channel->funding); @@ -3622,7 +3622,7 @@ static void resume_splice_negotiation(struct peer *peer, final_tx = bitcoin_tx_with_psbt(tmpctx, current_psbt); msg = towire_channeld_splice_confirmed_signed(tmpctx, final_tx, - chan_output_index); + new_output_index); wire_sync_write(MASTER_FD, take(msg)); } } From 453a4e174a05bbc72443f2e95504e84855bc2323 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:41:33 -0500 Subject: [PATCH 02/15] splice: Add locked field to inflight db This is needed to remember if a splice was locked and reconnect occurs mid `splice_locked` attempted so it can be resumed in reestablish. --- lightningd/channel.c | 1 + lightningd/channel.h | 5 +++++ wallet/db.c | 1 + wallet/test/run-wallet.c | 6 ++++++ wallet/wallet.c | 17 ++++++++++++----- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index f379891aa3d0..8e8d9c803ac6 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -200,6 +200,7 @@ new_inflight(struct channel *channel, inflight->i_am_initiator = i_am_initiator; inflight->force_sign_first = force_sign_first; + inflight->is_locked = false; inflight->splice_locked_memonly = false; list_add_tail(&channel->inflights, &inflight->list); diff --git a/lightningd/channel.h b/lightningd/channel.h index 268e7884a90f..a24bba85afe0 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -81,6 +81,11 @@ struct channel_inflight { /* On reestablish recovery; should I sign first? */ bool force_sign_first; + /* Has this inflight reached sufficent depth on chain? This is needed + * for splices that need to coordinate `splice_locked` with their + * peer through reconnect flows. */ + bool is_locked; + /* Note: This field is not stored in the database. * * After splice_locked, we need a way to stop the chain watchers from diff --git a/wallet/db.c b/wallet/db.c index 82287106d015..b14766a29c90 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1030,6 +1030,7 @@ static struct migration dbmigrations[] = { {SQL("ALTER TABLE channel_funding_inflights ADD remote_funding BLOB DEFAULT NULL;"), NULL}, {SQL("ALTER TABLE peers ADD last_known_address BLOB DEFAULT NULL;"), NULL}, {SQL("ALTER TABLE channels ADD close_attempt_height INTEGER DEFAULT 0;"), NULL}, + {SQL("ALTER TABLE channel_funding_inflights ADD locked_onchain INTEGER DEFAULT 0;"), NULL}, }; /** diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 33ef2ba5f40a..d47898bc49d7 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1616,6 +1616,8 @@ static bool channel_inflightseq(struct channel_inflight *i1, CHECK(i1->lease_chan_max_msat == i2->lease_chan_max_msat); CHECK(i1->lease_chan_max_ppt == i2->lease_chan_max_ppt); CHECK(i1->lease_blockheight_start == i2->lease_blockheight_start); + CHECK(i1->is_locked == i2->is_locked); + CHECK(i1->splice_locked_memonly == i2->splice_locked_memonly); return true; } @@ -2037,6 +2039,8 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) 0, false, false); + inflight->splice_locked_memonly = true; + inflight->is_locked = true; inflight_set_last_tx(inflight, last_tx, sig); @@ -2064,6 +2068,8 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) 0, false, false); + inflight->splice_locked_memonly = false; + inflight->is_locked = false; inflight_set_last_tx(inflight, last_tx, sig); wallet_inflight_add(w, inflight); CHECK_MSG(c2 = wallet_channel_load(w, chan->dbid), diff --git a/wallet/wallet.c b/wallet/wallet.c index 03da02be0989..6e28989fccda 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1244,8 +1244,9 @@ void wallet_inflight_add(struct wallet *w, struct channel_inflight *inflight) ", i_am_initiator" ", force_sign_first" ", remote_funding" + ", locked_onchain" ") VALUES (" - "?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); + "?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); db_bind_u64(stmt, inflight->channel->dbid); db_bind_txid(stmt, &inflight->funding->outpoint.txid); @@ -1288,6 +1289,7 @@ void wallet_inflight_add(struct wallet *w, struct channel_inflight *inflight) db_bind_pubkey(stmt, inflight->funding->splice_remote_funding); else db_bind_null(stmt); + db_bind_int(stmt, inflight->is_locked); db_exec_prepared_v2(stmt); assert(!stmt->error); @@ -1316,17 +1318,18 @@ void wallet_inflight_save(struct wallet *w, struct db_stmt *stmt; /* The *only* thing you can update on an * inflight is the funding PSBT (to add sigs) - * and the last_tx/last_sig if this is for a splice */ + * and the last_tx/last_sig or locked_onchain if this is for a splice */ stmt = db_prepare_v2(w->db, SQL("UPDATE channel_funding_inflights SET" " funding_psbt=?" // 0 ", funding_tx_remote_sigs_received=?" // 1 ", last_tx=?" // 2 ", last_sig=?" // 3 + ", locked_onchain=?" // 4 " WHERE" - " channel_id=?" // 4 - " AND funding_tx_id=?" // 5 - " AND funding_tx_outnum=?")); // 6 + " channel_id=?" // 5 + " AND funding_tx_id=?" // 6 + " AND funding_tx_outnum=?")); // 7 db_bind_psbt(stmt, inflight->funding_psbt); db_bind_int(stmt, inflight->remote_tx_sigs); if (inflight->last_tx) { @@ -1336,6 +1339,7 @@ void wallet_inflight_save(struct wallet *w, db_bind_null(stmt); db_bind_null(stmt); } + db_bind_int(stmt, inflight->is_locked); db_bind_u64(stmt, inflight->channel->dbid); db_bind_txid(stmt, &inflight->funding->outpoint.txid); db_bind_int(stmt, inflight->funding->outpoint.n); @@ -1434,6 +1438,8 @@ wallet_stmt2inflight(struct wallet *w, struct db_stmt *stmt, i_am_initiator, force_sign_first); + inflight->is_locked = db_col_int(stmt, "locked_onchain"); + /* last_tx is null for not yet committed * channels + static channel backup recoveries */ if (!db_col_is_null(stmt, "last_tx")) { @@ -1485,6 +1491,7 @@ static bool wallet_channel_load_inflights(struct wallet *w, ", i_am_initiator" ", force_sign_first" ", remote_funding" + ", locked_onchain" " FROM channel_funding_inflights" " WHERE channel_id = ?" " ORDER BY funding_feerate")); From 6f785a533206c3b36bc0d7b758994925dc643441 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:42:26 -0500 Subject: [PATCH 03/15] splice: serialize inflight is_locked field This field should be serialized for communication between channeld and lightningd. --- channeld/inflight.c | 2 ++ channeld/inflight.h | 1 + 2 files changed, 3 insertions(+) diff --git a/channeld/inflight.c b/channeld/inflight.c index d06142bf324d..ed4d2a042f2b 100644 --- a/channeld/inflight.c +++ b/channeld/inflight.c @@ -25,6 +25,7 @@ struct inflight *fromwire_inflight(const tal_t *ctx, const u8 **cursor, size_t * } inflight->i_am_initiator = fromwire_bool(cursor, max); inflight->force_sign_first = fromwire_bool(cursor, max); + inflight->is_locked = fromwire_bool(cursor, max); return inflight; } @@ -44,4 +45,5 @@ void towire_inflight(u8 **pptr, const struct inflight *inflight) } towire_bool(pptr, inflight->i_am_initiator); towire_bool(pptr, inflight->force_sign_first); + towire_bool(pptr, inflight->is_locked); } diff --git a/channeld/inflight.h b/channeld/inflight.h index 2c296f43148e..88a09bf067ea 100644 --- a/channeld/inflight.h +++ b/channeld/inflight.h @@ -7,6 +7,7 @@ #include struct inflight { + /* The new channel outpoint */ struct bitcoin_outpoint outpoint; struct pubkey remote_funding; struct amount_sat amnt; From a5066a13aef9f19000ee4a37f48c7c70afb3f502 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:46:29 -0500 Subject: [PATCH 04/15] splice: Pass is_locked between channeld and ld To support resuming `splice_locked` across channel reconnect, we need to pass inflight `is_locked` between lightningd and channeld. This implements that interface between channeld and lightningd so each inflight should have up to date `is_locked` values between restarts. --- channeld/channeld.c | 32 +++++++++++++++++++++++++++----- channeld/channeld_wire.csv | 1 + lightningd/channel_control.c | 6 +++++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index f7f81d44dbdf..bb616059115e 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3361,7 +3361,8 @@ static void resume_splice_negotiation(struct peer *peer, msg = towire_channeld_update_inflight(NULL, current_psbt, their_commit->tx, - &their_commit->commit_signature); + &their_commit->commit_signature, + inflight->is_locked); wire_sync_write(MASTER_FD, take(msg)); } @@ -3431,7 +3432,8 @@ static void resume_splice_negotiation(struct peer *peer, inflight->force_sign_first) && send_signature) { msg = towire_channeld_update_inflight(NULL, current_psbt, - NULL, NULL); + NULL, NULL, + inflight->is_locked); wire_sync_write(MASTER_FD, take(msg)); msg = towire_channeld_splice_sending_sigs(tmpctx, &final_txid); @@ -3600,7 +3602,8 @@ static void resume_splice_negotiation(struct peer *peer, if (recv_signature) { /* We let core validate our peer's signatures are correct. */ msg = towire_channeld_update_inflight(NULL, current_psbt, NULL, - NULL); + NULL, + inflight->is_locked); wire_sync_write(MASTER_FD, take(msg)); } @@ -3831,6 +3834,7 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) new_inflight->last_tx = NULL; new_inflight->i_am_initiator = false; new_inflight->force_sign_first = peer->splicing->force_sign_first; + new_inflight->is_locked = false; current_push_val = relative_splice_balance_fundee(peer, our_role,ictx->current_psbt, outpoint.n, splice_funding_index); @@ -4055,6 +4059,7 @@ static void splice_initiator_user_finalized(struct peer *peer) new_inflight->last_tx = NULL; new_inflight->i_am_initiator = true; new_inflight->force_sign_first = peer->splicing->force_sign_first; + new_inflight->is_locked = false; /* Switch over to using inflight psbt. This allows us to be reentrant. * On restart we *will* have inflight psbt but we will not have any @@ -4085,7 +4090,8 @@ static void splice_initiator_user_finalized(struct peer *peer) outmsg = towire_channeld_update_inflight(NULL, new_inflight->psbt, their_commit->tx, - &their_commit->commit_signature); + &their_commit->commit_signature, + new_inflight->is_locked); wire_sync_write(MASTER_FD, take(outmsg)); sign_first = do_i_sign_first(peer, new_inflight->psbt, our_role, @@ -4244,7 +4250,8 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) * restart and reestablish later. */ outmsg = towire_channeld_update_inflight(NULL, inflight->psbt, inflight->last_tx, - &inflight->last_sig); + &inflight->last_sig, + inflight->is_locked); wire_sync_write(MASTER_FD, take(outmsg)); @@ -5593,6 +5600,20 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) } else if(splicing && !peer->splice_state->locked_ready[LOCAL]) { assert(scid); + for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) { + struct inflight *inflight = peer->splice_state->inflights[i]; + if (bitcoin_txid_eq(&inflight->outpoint.txid, + &txid)) { + inflight->is_locked = true; + msg = towire_channeld_update_inflight(NULL, + inflight->psbt, + NULL, + NULL, + inflight->is_locked); + wire_sync_write(MASTER_FD, take(msg)); + } + } + msg = towire_splice_locked(NULL, &peer->channel_id); peer->splice_state->locked_txid = txid; @@ -6068,6 +6089,7 @@ static void init_channel(struct peer *peer) struct penalty_base *pbases; bool reestablish_only; struct channel_type *channel_type; + bool found_locked_inflight; assert(!(fcntl(MASTER_FD, F_GETFL) & O_NONBLOCK)); diff --git a/channeld/channeld_wire.csv b/channeld/channeld_wire.csv index e9139ef435c0..e844a2f0f4f1 100644 --- a/channeld/channeld_wire.csv +++ b/channeld/channeld_wire.csv @@ -273,6 +273,7 @@ msgtype,channeld_update_inflight,7219 msgdata,channeld_update_inflight,psbt,wally_psbt, msgdata,channeld_update_inflight,last_tx,?bitcoin_tx, msgdata,channeld_update_inflight,last_sig,?bitcoin_signature, +msgdata,channeld_update_inflight,is_locked,bool, # channeld->master: A funding error has occured msgtype,channeld_splice_funding_error,7220 diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 9fcb7f5ebecc..f6737594962d 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -886,9 +886,10 @@ static void handle_update_inflight(struct lightningd *ld, struct bitcoin_txid txid; struct bitcoin_tx *last_tx; struct bitcoin_signature *last_sig; + bool is_locked; if (!fromwire_channeld_update_inflight(tmpctx, msg, &psbt, &last_tx, - &last_sig)) { + &last_sig, &is_locked)) { channel_internal_error(channel, "bad channel_add_inflight %s", tal_hex(channel, msg)); @@ -916,6 +917,8 @@ static void handle_update_inflight(struct lightningd *ld, if (last_sig) inflight->last_sig = *last_sig; + inflight->is_locked = is_locked; + tal_wally_start(); if (wally_psbt_combine(inflight->funding_psbt, psbt) != WALLY_OK) { channel_internal_error(channel, @@ -1812,6 +1815,7 @@ bool peer_start_channeld(struct channel *channel, infcopy->last_sig = inflight->last_sig; infcopy->i_am_initiator = inflight->i_am_initiator; infcopy->force_sign_first = inflight->force_sign_first; + infcopy->is_locked = inflight->is_locked; tal_wally_start(); wally_psbt_clone_alloc(inflight->funding_psbt, 0, &infcopy->psbt); From d3816dbc7fd1890583efe16d7696098a2a311227 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:50:18 -0500 Subject: [PATCH 05/15] splice: Update `locked_txid` on restart Use the inflight information to set a correct `locked_txid` value on restart. This is critical for handling interrupted `splice_locked` events that need to be resumed on reconnect/reestablish. --- channeld/channeld.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/channeld/channeld.c b/channeld/channeld.c index bb616059115e..e4700eef8add 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -6155,6 +6155,21 @@ static void init_channel(struct peer *peer) peer->final_ext_key = tal_dup(peer, struct ext_key, &final_ext_key); peer->splice_state->count = tal_count(peer->splice_state->inflights); + found_locked_inflight = false; + for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) { + if (peer->splice_state->inflights[i]->is_locked) { + if (found_locked_inflight) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "There should never be two splice" + " candidates locked on chain at" + " once. First %s. Second %s", + fmt_bitcoin_txid(tmpctx, &peer->splice_state->locked_txid), + fmt_bitcoin_txid(tmpctx, &peer->splice_state->inflights[i]->outpoint.txid)); + peer->splice_state->locked_txid = peer->splice_state->inflights[i]->outpoint.txid; + found_locked_inflight = true; + } + } + status_debug("option_static_remotekey = %u," " option_anchor_outputs = %u" " option_anchors_zero_fee_htlc_tx = %u", From 0b52099a51318aa102761d23f411a19216349630 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:52:50 -0500 Subject: [PATCH 06/15] splice: Resume `splice_locked` on reestablish A new case where `splice_locked` must be sent again on reestablish. This handles the case where `splice_locked` did not complete locally or remotely and must be resumed. --- channeld/channeld.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/channeld/channeld.c b/channeld/channeld.c index e4700eef8add..dfbe85fe85d7 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5177,6 +5177,25 @@ static void peer_reconnect(struct peer *peer, true, false, true); + } else if (inflight->is_locked + && bitcoin_txid_eq(remote_next_funding, + &inflight->outpoint.txid)) { + if (!bitcoin_txid_eq(&inflight->outpoint.txid, + &peer->splice_state->locked_txid)) + peer_failed_err(peer->pps, + &peer->channel_id, + "Invalid splice was resumed %s," + " should be %s", + fmt_bitcoin_txid(tmpctx, + &inflight->outpoint.txid), + fmt_bitcoin_txid(tmpctx, + &peer->splice_state->locked_txid)); + status_info("Splice is not confirmed but locked on" + " chain -- resending splice_locked"); + peer_write(peer->pps, + take(towire_splice_locked(NULL, + &peer->channel_id))); + peer->splice_state->locked_ready[LOCAL] = true; } else if (bitcoin_txid_eq(remote_next_funding, &inflight->outpoint.txid)) { /* Don't send sigs unless we have theirs */ From 8feb43af74156f918db4bd3e8efef3345d0e5c23 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 11:19:53 -0500 Subject: [PATCH 07/15] Splice: Clone psbts instead of steal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The interaction betwen libwally and CLN’s memory management is tricky. Let’s dodge that problem and just clone the PSBTs. Clean up some unused PSBT / ictx code while we’re at it --- channeld/channeld.c | 63 ++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index dfbe85fe85d7..14f0ebf88aad 100644 --- a/channeld/channeld.c +++ b/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; @@ -3856,15 +3856,11 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg) size_t input_index; const u8 *wit_script, *new_wit_script; 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 +3882,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 +3896,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 +3905,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,15 +3931,11 @@ 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; @@ -3955,10 +3943,11 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg) 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 +3982,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 +4058,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 +4135,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 +4162,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 +4197,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 +4236,7 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) fmt_bitcoin_txid(tmpctx, ¤t_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 +4247,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); @@ -5624,6 +5616,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, From 3935033dc30caae7dc630dbff5bad08e8cd81a8c Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 12:02:31 -0500 Subject: [PATCH 08/15] Splice: Update PSBT version handling Upscale user provided PSBTs to v2 and convert them back to user preference when returned. --- lightningd/channel_control.c | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index f6737594962d..4a9275840339 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -49,6 +49,8 @@ struct splice_command { struct channel_id **channel_ids; /* For multi-channel stfu command: the pending result */ struct stfu_result **results; + /* The user provided PSBT's version */ + u32 user_psbt_ver; }; void channel_update_feerates(struct lightningd *ld, const struct channel *channel) @@ -409,6 +411,13 @@ static void handle_splice_confirmed_init(struct lightningd *ld, return; } + if (psbt->version != cc->user_psbt_ver + && !psbt_set_version(psbt, cc->user_psbt_ver)) + channel_internal_error(channel, "Splice failed to convert from" + " internal version "PRIu32" to user" + " version "PRIu32, psbt->version, + cc->user_psbt_ver); + struct json_stream *response = json_stream_success(cc->cmd); json_add_string(response, "psbt", fmt_wally_psbt(tmpctx, psbt)); @@ -443,6 +452,13 @@ static void handle_splice_confirmed_update(struct lightningd *ld, return; } + if (psbt->version != cc->user_psbt_ver + && !psbt_set_version(psbt, cc->user_psbt_ver)) + channel_internal_error(channel, "Splice failed to convert from" + " internal version "PRIu32" to user" + " version "PRIu32, psbt->version, + cc->user_psbt_ver); + struct json_stream *response = json_stream_success(cc->cmd); json_add_string(response, "psbt", fmt_wally_psbt(tmpctx, psbt)); json_add_bool(response, "commitments_secured", commitments_secured); @@ -2271,6 +2287,12 @@ static struct command_result *json_splice_init(struct command *cmd, cc->channel = channel; cc->channel_ids = NULL; cc->results = NULL; + cc->user_psbt_ver = initialpsbt->version; + + if (initialpsbt->version != 2 && !psbt_set_version(initialpsbt, 2)) + return command_fail(cmd, + SPLICE_INPUT_ERROR, + "Splice failed to convert to v2"); msg = towire_channeld_splice_init(NULL, initialpsbt, *relative_amount, *feerate_per_kw, *force_feerate, @@ -2317,6 +2339,12 @@ static struct command_result *json_splice_update(struct command *cmd, cc->channel = channel; cc->channel_ids = NULL; cc->results = NULL; + cc->user_psbt_ver = psbt->version; + + if (psbt->version != 2 && !psbt_set_version(psbt, 2)) + return command_fail(cmd, + SPLICE_INPUT_ERROR, + "Splice failed to convert to v2"); subd_send_msg(channel->owner, take(towire_channeld_splice_update(NULL, psbt))); @@ -2347,6 +2375,12 @@ static struct command_result *single_splice_signed(struct command *cmd, cc->channel = channel; cc->channel_ids = NULL; cc->results = NULL; + cc->user_psbt_ver = psbt->version; + + if (psbt->version != 2 && !psbt_set_version(psbt, 2)) + return command_fail(cmd, + SPLICE_INPUT_ERROR, + "Splice failed to convert to v2"); msg = towire_channeld_splice_signed(tmpctx, psbt, sign_first); subd_send_msg(channel->owner, take(msg)); @@ -2378,6 +2412,9 @@ static struct command_result *json_splice_signed(struct command *cmd, return command_fail(cmd, SPLICE_INPUT_ERROR, "PSBT failed to validate."); + log_debug(cmd->ld->log, "splice_signed input PSBT version %d", + psbt->version); + /* If a single channel is specified, we do that and finish. */ if (channel) { if (command_check_only(cmd)) From 34cb1a84806e58a33b60b04d8085c78246471e1a Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Mon, 27 Jan 2025 14:39:36 +0100 Subject: [PATCH 09/15] Add debug outputs and fix prevtx issue --- channeld/channeld.c | 4 ++++ common/interactivetx.c | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 14f0ebf88aad..d5613d608fca 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4141,6 +4141,10 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) ictx->tx_add_input_count = peer->splicing->tx_add_input_count; ictx->tx_add_output_count = peer->splicing->tx_add_output_count; + ictx->shared_outpoint = tal(ictx, struct bitcoin_outpoint); + *ictx->shared_outpoint = peer->channel->funding; + ictx->funding_tx = bitcoin_tx_from_txid(peer, peer->channel->funding.txid); + /* If there no are no changes, we consider the splice user finalized */ if (!interactivetx_has_changes(ictx, ictx->desired_psbt)) { splice_initiator_user_finalized(peer); diff --git a/common/interactivetx.c b/common/interactivetx.c index 194063cab8f8..7050feef222c 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -227,12 +227,25 @@ static char *send_next(const tal_t *ctx, /* If this is the shared channel input, we send funding txid in * tlvs and do not send prevtx */ + if (ictx->shared_outpoint) { + status_debug("ictx->shared_outpoint=%s", + fmt_bitcoin_outpoint(tmpctx, + ictx->shared_outpoint)); + } + else { + status_debug("ictx->shared_outpoint = NULL"); + } + status_debug("point=%s", fmt_bitcoin_outpoint(tmpctx, &point)); + if (ictx->shared_outpoint && bitcoin_outpoint_eq(&point, ictx->shared_outpoint)) { - struct tlv_tx_add_input_tlvs *tlvs = tal(tmpctx, struct tlv_tx_add_input_tlvs); + struct tlv_tx_add_input_tlvs *tlvs = tlv_tx_add_input_tlvs_new(tmpctx); tlvs->shared_input_txid = tal_dup(tlvs, struct bitcoin_txid, &point.txid); + status_debug("Adding shared input %s", + tal_hexstr(ctx, &serial_id, + sizeof(serial_id))); msg = towire_tx_add_input(NULL, cid, serial_id, NULL, in->input.index, in->input.sequence, tlvs); @@ -240,6 +253,9 @@ static char *send_next(const tal_t *ctx, msg = towire_tx_add_input(NULL, cid, serial_id, prevtx, in->input.index, in->input.sequence, NULL); + status_debug("Adding splice input %s", + tal_hexstr(ctx, &serial_id, + sizeof(serial_id))); } tal_arr_remove(&set->added_ins, 0); From 151ae779a49d7d4d5e44e76d13609c385130651d Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Wed, 22 Jan 2025 15:42:39 +0100 Subject: [PATCH 10/15] Fixed splice_locked messgae to include splice_txid field --- channeld/channeld.c | 17 +++++++++++------ tests/fuzz/fuzz-wire-splice_locked.c | 5 +++-- wire/peer_wire.csv | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index d5613d608fca..812f2a117b71 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -511,13 +511,14 @@ static void check_mutual_splice_locked(struct peer *peer) } /* Our peer told us they saw our splice confirm on chain with `splice_locked`. - * If we see it to we jump into tansitioning to post-splice, otherwise we mark + * 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. */ static void handle_peer_splice_locked(struct peer *peer, const u8 *msg) { struct channel_id chanid; + struct bitcoin_txid splice_txid; - if (!fromwire_splice_locked(msg, &chanid)) + if (!fromwire_splice_locked(msg, &chanid, &splice_txid)) peer_failed_warn(peer->pps, &peer->channel_id, "Bad splice_locked %s", tal_hex(msg, msg)); @@ -5190,7 +5191,8 @@ static void peer_reconnect(struct peer *peer, " chain -- resending splice_locked"); peer_write(peer->pps, take(towire_splice_locked(NULL, - &peer->channel_id))); + &peer->channel_id, + &inflight->outpoint.txid))); peer->splice_state->locked_ready[LOCAL] = true; } else if (bitcoin_txid_eq(remote_next_funding, &inflight->outpoint.txid)) { @@ -5236,7 +5238,9 @@ static void peer_reconnect(struct peer *peer, status_info("We have no pending splice but peer" " expects one; resending splice_lock"); peer_write(peer->pps, - take(towire_splice_locked(NULL, &peer->channel_id))); + take(towire_splice_locked(NULL, + &peer->channel_id, + &peer->channel->funding.txid))); } else { splice_abort(peer, "next_funding_txid not recognized." @@ -5630,10 +5634,11 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) } } - msg = towire_splice_locked(NULL, &peer->channel_id); - peer->splice_state->locked_txid = txid; + msg = towire_splice_locked(NULL, &peer->channel_id, + &txid); + peer_write(peer->pps, take(msg)); peer->splice_state->locked_ready[LOCAL] = true; diff --git a/tests/fuzz/fuzz-wire-splice_locked.c b/tests/fuzz/fuzz-wire-splice_locked.c index d1f7b1b6bfc1..d2fcf1291fd1 100644 --- a/tests/fuzz/fuzz-wire-splice_locked.c +++ b/tests/fuzz/fuzz-wire-splice_locked.c @@ -5,18 +5,19 @@ struct splice_locked { struct channel_id channel_id; + struct bitcoin_txid txid; }; static void *encode(const tal_t *ctx, const struct splice_locked *s) { - return towire_splice_locked(ctx, &s->channel_id); + return towire_splice_locked(ctx, &s->channel_id, &s->txid); } static struct splice_locked *decode(const tal_t *ctx, const void *p) { struct splice_locked *s = tal(ctx, struct splice_locked); - if (fromwire_splice_locked(p, &s->channel_id)) + if (fromwire_splice_locked(p, &s->channel_id, &s->txid)) return s; return tal_free(s); } diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index c36d90807b08..183a2c143cc4 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -223,6 +223,7 @@ msgdata,splice_ack,relative_satoshis,s64, msgdata,splice_ack,funding_pubkey,point, msgtype,splice_locked,77, msgdata,splice_locked,channel_id,channel_id, +msgdata,splice_locked,splice_txid,sha256, msgtype,shutdown,38 msgdata,shutdown,channel_id,channel_id, msgdata,shutdown,len,u16, From 1e667eb5b282df1b950fffe2188d8432049faea9 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 15:23:31 -0500 Subject: [PATCH 11/15] splice: Add check for correct txid in `splice_locked` Check that the peer sent the correct txid in their `splice_locked` message. We have to check this later on in `check_mutal_splice_locked` so we store the value in `splice_state` --- channeld/channeld.c | 31 +++++++++++++++++++++++++++---- channeld/splice.c | 1 + channeld/splice.h | 2 ++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 812f2a117b71..ba2c083e399e 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -444,8 +444,20 @@ static void check_mutual_splice_locked(struct peer *peer) if (short_channel_id_eq(peer->short_channel_ids[LOCAL], peer->splice_state->short_channel_id)) - peer_failed_warn(peer->pps, &peer->channel_id, - "Duplicate splice_locked events detected"); + peer_failed_err(peer->pps, &peer->channel_id, + "Duplicate splice_locked events detected" + " by scid check"); + + if (!peer->splice_state->remote_locked_txid + || !bitcoin_txid_eq(peer->splice_state->remote_locked_txid, + &peer->splice_state->locked_txid)) + peer_failed_err(peer->pps, &peer->channel_id, + "splice_locked message txid %s does not match" + " our locked txid %s", + fmt_bitcoin_txid(tmpctx, + peer->splice_state->remote_locked_txid), + fmt_bitcoin_txid(tmpctx, + &peer->splice_state->locked_txid)); peer->splice_state->await_commitment_succcess = true; @@ -473,7 +485,7 @@ static void check_mutual_splice_locked(struct peer *peer) inflight = peer->splice_state->inflights[i]; if (!inflight) - peer_failed_warn(peer->pps, &peer->channel_id, + peer_failed_err(peer->pps, &peer->channel_id, "Unable to find inflight txid amoung %zu" " inflights. new funding txid: %s", tal_count(peer->splice_state->inflights), @@ -487,7 +499,7 @@ static void check_mutual_splice_locked(struct peer *peer) inflight->amnt, inflight->splice_amnt); if (error) - peer_failed_warn(peer->pps, &peer->channel_id, + peer_failed_err(peer->pps, &peer->channel_id, "Splice lock unable to update funding. %s", error); @@ -508,6 +520,7 @@ 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); } /* Our peer told us they saw our splice confirm on chain with `splice_locked`. @@ -522,6 +535,16 @@ static void handle_peer_splice_locked(struct peer *peer, const u8 *msg) peer_failed_warn(peer->pps, &peer->channel_id, "Bad splice_locked %s", tal_hex(msg, msg)); + if (peer->splice_state->remote_locked_txid) + peer_failed_err(peer->pps, &chanid, + "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 " diff --git a/channeld/splice.c b/channeld/splice.c index 3ef245f7d9ed..e72b4af54830 100644 --- a/channeld/splice.c +++ b/channeld/splice.c @@ -11,6 +11,7 @@ struct splice_state *splice_state_new(const tal_t *ctx) splice_state->locked_ready[REMOTE] = false; splice_state->await_commitment_succcess = false; splice_state->inflights = NULL; + splice_state->remote_locked_txid = NULL; return splice_state; } diff --git a/channeld/splice.h b/channeld/splice.h index 92bea28284e4..be6f7ff583d1 100644 --- a/channeld/splice.h +++ b/channeld/splice.h @@ -21,6 +21,8 @@ struct splice_state { bool await_commitment_succcess; /* The txid of which splice inflight was confirmed */ struct bitcoin_txid locked_txid; + /* The txid our peer locked their splice on */ + struct bitcoin_txid *remote_locked_txid; /* The number of splices that are active (awaiting confirmation) */ u32 count; }; From b2fefae561692b2bbc9e8e5ad23a0a74788fe94c Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 15:53:14 -0500 Subject: [PATCH 12/15] fixup! splice: Add check for correct txid in `splice_locked` --- channeld/channeld.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index ba2c083e399e..98013f6809df 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -454,8 +454,10 @@ static void check_mutual_splice_locked(struct peer *peer) peer_failed_err(peer->pps, &peer->channel_id, "splice_locked message txid %s does not match" " our locked txid %s", - fmt_bitcoin_txid(tmpctx, - peer->splice_state->remote_locked_txid), + peer->splice_state->remote_locked_txid + ? fmt_bitcoin_txid(tmpctx, + peer->splice_state->remote_locked_txid) + : "NULL", fmt_bitcoin_txid(tmpctx, &peer->splice_state->locked_txid)); From f690cc745fd3cab90491c665c0febe1bc2ec0c9d Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 7 Feb 2025 13:17:09 -0500 Subject: [PATCH 13/15] PSBT: Fix compare to not mutate memory PSBT changeset routines were using linearize_output which mutated the memory of the objects it was comparing. This commit fixes that and also cleans up the memory usage to be more clear and more guarentee there is no memory corruption. Changelog-None --- common/psbt_open.c | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/common/psbt_open.c b/common/psbt_open.c index 42fe1e1808a6..a54085e61ee0 100644 --- a/common/psbt_open.c +++ b/common/psbt_open.c @@ -61,14 +61,16 @@ static int compare_outputs_at(const struct output_set *a, } static const u8 *linearize_input(const tal_t *ctx, - const struct wally_psbt_input *in) + const struct wally_psbt *parent, + size_t index) { + struct wally_psbt *copy = clone_psbt(NULL, parent); struct wally_psbt *psbt = create_psbt(NULL, 1, 0, 0); + struct wally_psbt_input dummy_in; size_t byte_len; - psbt->inputs[0] = *in; - psbt->num_inputs++; - + dummy_in = psbt->inputs[0]; + psbt->inputs[0] = copy->inputs[index]; /* Sort the inputs, so serializing them is ok */ wally_map_sort(&psbt->inputs[0].unknowns, 0); @@ -88,16 +90,21 @@ static const u8 *linearize_input(const tal_t *ctx, const u8 *bytes = psbt_get_bytes(ctx, psbt, &byte_len); - /* Hide the inputs we added, so it doesn't get freed */ - psbt->num_inputs--; + psbt->inputs[0] = dummy_in; + tal_free(psbt); + tal_free(copy); + return bytes; } static const u8 *linearize_output(const tal_t *ctx, - const struct wally_psbt_output *out) + const struct wally_psbt *parent, + size_t index) { - struct wally_psbt *psbt = create_psbt(NULL, 1, 1, 0); + struct wally_psbt *copy = clone_psbt(NULL, parent); + struct wally_psbt *psbt = create_psbt(NULL, 0, 1, 0); + struct wally_psbt_output dummy_out; size_t byte_len; struct bitcoin_outpoint outpoint; @@ -105,8 +112,9 @@ static const u8 *linearize_output(const tal_t *ctx, memset(&outpoint, 1, sizeof(outpoint)); psbt_append_input(psbt, &outpoint, 0, NULL, NULL, NULL); - psbt->outputs[0] = *out; - psbt->num_outputs++; + dummy_out = psbt->outputs[0]; + psbt->outputs[0] = copy->outputs[index]; + /* Sort the outputs, so serializing them is ok */ wally_map_sort(&psbt->outputs[0].unknowns, 0); @@ -120,9 +128,10 @@ static const u8 *linearize_output(const tal_t *ctx, const u8 *bytes = psbt_get_bytes(ctx, psbt, &byte_len); - /* Hide the outputs we added, so it doesn't get freed */ - psbt->num_outputs--; + psbt->outputs[0] = dummy_out; tal_free(psbt); + tal_free(copy); + return bytes; } @@ -131,10 +140,8 @@ static bool input_identical(const struct wally_psbt *a, const struct wally_psbt *b, size_t b_index) { - const u8 *a_in = linearize_input(tmpctx, - &a->inputs[a_index]); - const u8 *b_in = linearize_input(tmpctx, - &b->inputs[b_index]); + const u8 *a_in = linearize_input(tmpctx, a, a_index); + const u8 *b_in = linearize_input(tmpctx, b, b_index); return tal_arr_eq(a_in, b_in); } @@ -144,10 +151,8 @@ static bool output_identical(const struct wally_psbt *a, const struct wally_psbt *b, size_t b_index) { - const u8 *a_out = linearize_output(tmpctx, - &a->outputs[a_index]); - const u8 *b_out = linearize_output(tmpctx, - &b->outputs[b_index]); + const u8 *a_out = linearize_output(tmpctx, a, a_index); + const u8 *b_out = linearize_output(tmpctx, b, b_index); return tal_arr_eq(a_out, b_out); } From 057982b77e912a7c2a6290768085eaa454d71d2d Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 7 Feb 2025 14:00:01 -0500 Subject: [PATCH 14/15] splice: Use clone instead of steal for PSBT Update splice flows to use the new `clone_psbt` method instead of stealing back and forth. --- channeld/channeld.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 98013f6809df..11046ba7794e 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4163,7 +4163,9 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) 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; + + /* clone the current psbt over to ictx */ + ictx->current_psbt = clone_psbt(ictx, 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; @@ -4190,10 +4192,12 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) peer->splicing->tx_add_input_count = ictx->tx_add_input_count; peer->splicing->tx_add_output_count = ictx->tx_add_output_count; - if (peer->splicing->current_psbt != ictx->current_psbt) - tal_free(peer->splicing->current_psbt); + /* clone the ictx psbt back onto current */ + tal_free(peer->splicing->current_psbt); peer->splicing->current_psbt = clone_psbt(peer->splicing, ictx->current_psbt); + tal_free(ictx->current_psbt); + ictx->current_psbt = NULL; /* Peer may have modified our PSBT so we return it to the user here */ outmsg = towire_channeld_splice_confirmed_update(NULL, From f6494701ac960419315d6a75b6eea3f961322655 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 20:13:59 -0500 Subject: [PATCH 15/15] Test commit for CI --- bitcoin/psbt.c | 15 ++-- channeld/channeld.c | 132 +++++++++++++++++++++++++++++------ common/interactivetx.c | 46 +++++++++++- common/psbt_open.c | 49 +++++++++++++ lightningd/channel_control.c | 2 + 5 files changed, 215 insertions(+), 29 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index a1e19c401b19..7f387da29e3f 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -38,12 +38,15 @@ struct wally_psbt *create_psbt(const tal_t *ctx, size_t num_inputs, size_t num_o struct wally_psbt *clone_psbt(const tal_t *ctx, const struct wally_psbt *psbt) { - struct wally_psbt *clone; - tal_wally_start(); - if (wally_psbt_clone_alloc(psbt, 0, &clone) != WALLY_OK) - abort(); - tal_wally_end_onto(ctx, clone, struct wally_psbt); - return clone; + struct wally_psbt *result; + size_t bytes_written; + const u8 *psbt_bytes; + + psbt_bytes = psbt_get_bytes(NULL, psbt, &bytes_written); + result = psbt_from_bytes(ctx, psbt_bytes, bytes_written); + tal_free(psbt_bytes); + + return result; } struct wally_psbt *new_psbt(const tal_t *ctx, const struct wally_tx *wtx) diff --git a/channeld/channeld.c b/channeld/channeld.c index 11046ba7794e..03b4df2c2043 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3359,7 +3359,7 @@ static void resume_splice_negotiation(struct peer *peer, recv_signature ? "" : "not ", our_role == TX_INITIATOR ? "initiator" : "accepter"); - wit_script = bitcoin_redeem_2of2(tmpctx, + wit_script = bitcoin_redeem_2of2(current_psbt, &peer->channel->funding_pubkey[LOCAL], &peer->channel->funding_pubkey[REMOTE]); @@ -3610,8 +3610,9 @@ static void resume_splice_negotiation(struct peer *peer, " Most likely you are missing" " signatures."); - psbt_finalize_input(current_psbt, in, - inws[j++]); + tal_wally_start(); + psbt_finalize_input(current_psbt, in, inws[j++]); + tal_wally_end(current_psbt); } final_tx = bitcoin_tx_with_psbt(tmpctx, current_psbt); @@ -3654,6 +3655,18 @@ static void resume_splice_negotiation(struct peer *peer, new_output_index); wire_sync_write(MASTER_FD, take(msg)); } + + size_t dummy; + psbt_get_bytes(NULL, inflight->psbt, &dummy); + psbt_get_bytes(NULL, current_psbt, &dummy); + clone_psbt(NULL, inflight->psbt); + clone_psbt(NULL, current_psbt); + assert(inflight->psbt == current_psbt); + + /* The below code fixes the testssss */ + struct wally_psbt *copy = inflight->psbt; + inflight->psbt = clone_psbt(inflight, inflight->psbt); + tal_free(copy); } static struct inflight *inflights_new(struct peer *peer) @@ -3841,7 +3854,7 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) funding_feerate_perkw, both_amount, peer->splicing->accepter_relative, - ictx->current_psbt, + clone_psbt(tmpctx, ictx->current_psbt), false, peer->splicing->force_sign_first); @@ -3862,15 +3875,34 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) new_inflight->force_sign_first = peer->splicing->force_sign_first; new_inflight->is_locked = false; - current_push_val = relative_splice_balance_fundee(peer, our_role,ictx->current_psbt, + size_t dummy; + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + + current_push_val = relative_splice_balance_fundee(peer, our_role, ictx->current_psbt, outpoint.n, splice_funding_index); + + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + update_hsmd_with_splice(peer, new_inflight, our_role, current_push_val); + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + update_view_from_inflights(peer); + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + peer->splice_state->count++; + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + resume_splice_negotiation(peer, true, true, true, true); + + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + + new_inflight->psbt = clone_psbt(new_inflight, ictx->current_psbt); + clean_tmpctx(); + + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); } /* splice_initiator runs when splice_ack is received by the other side. It @@ -3992,6 +4024,7 @@ static void splice_initiator_user_finalized(struct peer *peer) char *error; u32 chan_output_index, splice_funding_index; struct wally_psbt_output *new_chan_output; + struct wally_psbt *psbt; struct inflight *new_inflight; struct bitcoin_txid current_psbt_txid; struct amount_sat both_amount; @@ -4003,7 +4036,7 @@ static void splice_initiator_user_finalized(struct peer *peer) /* We must loading the funding tx as our previous utxo */ prev_tx = bitcoin_tx_from_txid(peer, peer->channel->funding.txid); - ictx = new_interactivetx_context(tmpctx, our_role, + ictx = new_interactivetx_context(NULL, our_role, peer->pps, peer->channel_id); ictx->next_update_fn = next_splice_step; @@ -4015,60 +4048,82 @@ static void splice_initiator_user_finalized(struct peer *peer) ictx->tx_add_input_count = peer->splicing->tx_add_input_count; ictx->tx_add_output_count = peer->splicing->tx_add_output_count; + clone_psbt(NULL, ictx->current_psbt); + ictx->shared_outpoint = tal(ictx, struct bitcoin_outpoint); *ictx->shared_outpoint = peer->channel->funding; ictx->funding_tx = prev_tx; - error = process_interactivetx_updates(tmpctx, ictx, + clone_psbt(NULL, ictx->current_psbt); + + error = process_interactivetx_updates(ictx, ictx, &peer->splicing->received_tx_complete, &abort_msg); if (error) peer_failed_warn(peer->pps, &peer->channel_id, "Splice interactivetx error: %s", error); + clone_psbt(NULL, ictx->current_psbt); // dustin + check_tx_abort(peer, abort_msg); + psbt = ictx->current_psbt; + + clone_psbt(NULL, psbt); + /* With pause_when_complete fase, this assert should never fail */ assert(peer->splicing->received_tx_complete); peer->splicing->sent_tx_complete = true; - psbt_sort_by_serial_id(ictx->current_psbt); + clone_psbt(NULL, psbt); - new_chan_output = find_channel_output(peer, ictx->current_psbt, + new_chan_output = find_channel_output(peer, psbt, &chan_output_index, &peer->splicing->remote_funding_pubkey); + + clone_psbt(NULL, psbt); - splice_funding_index = find_channel_funding_input(ictx->current_psbt, + splice_funding_index = find_channel_funding_input(psbt, &peer->channel->funding); + + clone_psbt(NULL, psbt); // lisa - both_amount = check_balances(peer, our_role, ictx->current_psbt, + both_amount = check_balances(peer, our_role, psbt, chan_output_index, splice_funding_index); new_chan_output->amount = both_amount.satoshis; /* Raw: type conv */ + + clone_psbt(NULL, psbt); - psbt_elements_normalize_fees(ictx->current_psbt); + psbt_elements_normalize_fees(psbt); + + clone_psbt(NULL, psbt); status_debug("Splice adding inflight: %s", - fmt_wally_psbt(tmpctx, ictx->current_psbt)); + fmt_wally_psbt(tmpctx, psbt)); - psbt_txid(tmpctx, ictx->current_psbt, ¤t_psbt_txid, NULL); + psbt_txid(ictx, psbt, ¤t_psbt_txid, NULL); + + clone_psbt(NULL, psbt); - outmsg = towire_channeld_add_inflight(tmpctx, + outmsg = towire_channeld_add_inflight(NULL, &peer->splicing->remote_funding_pubkey, ¤t_psbt_txid, chan_output_index, peer->splicing->feerate_per_kw, amount_sat(new_chan_output->amount), peer->splicing->opener_relative, - ictx->current_psbt, + psbt, true, peer->splicing->force_sign_first); + + clone_psbt(NULL, psbt); master_wait_sync_reply(tmpctx, peer, take(outmsg), WIRE_CHANNELD_GOT_INFLIGHT); new_inflight = inflights_new(peer); - psbt_txid(tmpctx, ictx->current_psbt, &new_inflight->outpoint.txid, + psbt_txid(new_inflight, psbt, &new_inflight->outpoint.txid, NULL); new_inflight->remote_funding = peer->splicing->remote_funding_pubkey; new_inflight->outpoint.n = chan_output_index; @@ -4084,7 +4139,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 = clone_psbt(new_inflight, ictx->current_psbt); + new_inflight->psbt = clone_psbt(new_inflight, psbt); current_push_val = relative_splice_balance_fundee(peer, our_role, new_inflight->psbt, @@ -4113,14 +4168,32 @@ static void splice_initiator_user_finalized(struct peer *peer) sign_first = do_i_sign_first(peer, new_inflight->psbt, our_role, peer->splicing->force_sign_first); + assert(new_inflight->psbt); + if (!sign_first) resume_splice_negotiation(peer, false, false, false, true); + size_t dummy; + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + + assert(new_inflight->psbt); + struct wally_psbt *old_psbt = new_inflight->psbt; + new_inflight->psbt = clone_psbt(new_inflight, new_inflight->psbt); + tal_free(old_psbt); + outmsg = towire_channeld_splice_confirmed_update(NULL, new_inflight->psbt, true, !sign_first); wire_sync_write(MASTER_FD, take(outmsg)); + + tal_free(ictx); + + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); + + clean_tmpctx(); + + psbt_get_bytes(NULL, new_inflight->psbt, &dummy); } /* During a splice the user may call splice_update mulitple times adding @@ -4141,7 +4214,7 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) return; } - ictx = new_interactivetx_context(tmpctx, TX_INITIATOR, + ictx = new_interactivetx_context(NULL, TX_INITIATOR, peer->pps, peer->channel_id); if (!fromwire_channeld_splice_update(ictx, inmsg, &ictx->desired_psbt)) @@ -4152,6 +4225,7 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) " splice when not in" " splice mode."); wire_sync_write(MASTER_FD, take(msg)); + tal_free(ictx); return; } @@ -4177,10 +4251,11 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) if (!interactivetx_has_changes(ictx, ictx->desired_psbt)) { splice_initiator_user_finalized(peer); tal_steal(last_inflight(peer), last_inflight(peer)->psbt); + tal_free(ictx); return; } - error = process_interactivetx_updates(tmpctx, ictx, + error = process_interactivetx_updates(ictx, ictx, &peer->splicing->received_tx_complete, &abort_msg); if (error) @@ -4201,9 +4276,10 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) /* Peer may have modified our PSBT so we return it to the user here */ outmsg = towire_channeld_splice_confirmed_update(NULL, - ictx->current_psbt, + peer->splicing->current_psbt, false, false); wire_sync_write(MASTER_FD, take(outmsg)); + tal_free(ictx); } /* This occurs when the user has signed the final version of the PSBT. At this @@ -4285,6 +4361,13 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) inflight->force_sign_first); resume_splice_negotiation(peer, false, false, true, sign_first); + + size_t dummy; + psbt_get_bytes(NULL, inflight->psbt, &dummy); + + clean_tmpctx(); + + psbt_get_bytes(NULL, inflight->psbt, &dummy); } /* This occurs once our 'stfu' transition was successful. */ @@ -5650,15 +5733,22 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) for (size_t i = 0; i < tal_count(peer->splice_state->inflights); i++) { struct inflight *inflight = peer->splice_state->inflights[i]; + size_t dummy; + psbt_get_bytes(NULL, inflight->psbt, &dummy); + psbt_get_bytes(NULL, inflight->psbt, &dummy); + psbt_get_bytes(NULL, inflight->psbt, &dummy); if (bitcoin_txid_eq(&inflight->outpoint.txid, &txid)) { inflight->is_locked = true; assert(inflight->psbt); + psbt_get_bytes(NULL, inflight->psbt, &dummy); msg = towire_channeld_update_inflight(NULL, inflight->psbt, NULL, NULL, inflight->is_locked); + + psbt_get_bytes(NULL, inflight->psbt, &dummy); wire_sync_write(MASTER_FD, take(msg)); } } diff --git a/common/interactivetx.c b/common/interactivetx.c index 7050feef222c..bef57b476b60 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -391,19 +391,36 @@ char *process_interactivetx_updates(const tal_t *ctx, if (received_tx_complete) they_complete = *received_tx_complete; - /* Build change_set and handle PSBT variables */ - ictx->change_set = tal_free(ictx->change_set); + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); /* Call next_update_fn or default to 'desired_psbt' */ next_psbt = ictx->next_update_fn(ictx, ictx); + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + /* Returning NULL from next_update_fn is the same as using `current_psbt` * with no changes -- both indicate no changes */ if (!next_psbt) next_psbt = ictx->current_psbt; + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + ictx->change_set = get_changes(ctx, ictx, next_psbt); + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); // this fails + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + /* If current_psbt and next_psbt are the same, dont double free it! * Otherwise we advance `current_psbt` to `next_psbt` and begin * processing the change set in `ictx->change_set` */ @@ -414,6 +431,11 @@ char *process_interactivetx_updates(const tal_t *ctx, ictx->current_psbt = next_psbt; } + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + /* As initiator we always start with a single send to start it off */ if (ictx->our_role == TX_INITIATOR) { error = send_next(ctx, ictx, &we_complete); @@ -426,6 +448,11 @@ char *process_interactivetx_updates(const tal_t *ctx, } } + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + /* Loop through tx update turns with peer */ while (!(we_complete && they_complete)) { struct channel_id cid; @@ -439,10 +466,20 @@ char *process_interactivetx_updates(const tal_t *ctx, if (received_tx_complete) *received_tx_complete = false; + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + msg = read_next_msg(ctx, ictx, &error); if (error) return error; + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + t = fromwire_peektype(msg); switch (t) { case WIRE_TX_ADD_INPUT: { @@ -803,6 +840,11 @@ char *process_interactivetx_updates(const tal_t *ctx, send_next(ctx, ictx, &we_complete); } + if (ictx->current_psbt) + clone_psbt(NULL, ictx->current_psbt); + if (ictx->desired_psbt) + clone_psbt(NULL, ictx->desired_psbt); + /* Sort psbt! */ psbt_sort_by_serial_id(ictx->current_psbt); diff --git a/common/psbt_open.c b/common/psbt_open.c index a54085e61ee0..b4e8086ffd6a 100644 --- a/common/psbt_open.c +++ b/common/psbt_open.c @@ -285,79 +285,128 @@ struct psbt_changeset *psbt_get_changeset(const tal_t *ctx, size_t i = 0, j = 0; struct psbt_changeset *set; + clone_psbt(NULL, orig); + clone_psbt(NULL, new); + psbt_sort_by_serial_id(orig); + + clone_psbt(NULL, orig); + clone_psbt(NULL, new); + psbt_sort_by_serial_id(new); + clone_psbt(NULL, orig); + clone_psbt(NULL, new); + set = new_changeset(ctx); + clone_psbt(NULL, orig); + clone_psbt(NULL, new); + /* Find the input diff */ while (i < orig->num_inputs || j < new->num_inputs) { + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (i >= orig->num_inputs) { ADD(input, set->added_ins, new, j); j++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (j >= new->num_inputs) { ADD(input, set->rm_ins, orig, i); i++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); result = compare_serials(&orig->inputs[i].unknowns, &new->inputs[j].unknowns); + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (result == -1) { ADD(input, set->rm_ins, orig, i); i++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (result == 1) { ADD(input, set->added_ins, new, j); j++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (!input_identical(orig, i, new, j)) { ADD(input, set->rm_ins, orig, i); ADD(input, set->added_ins, new, j); } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); i++; j++; + clone_psbt(NULL, orig); + clone_psbt(NULL, new); } + + clone_psbt(NULL, orig); + clone_psbt(NULL, new); /* Find the output diff */ i = 0; j = 0; while (i < orig->num_outputs || j < new->num_outputs) { + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (i >= orig->num_outputs) { ADD(output, set->added_outs, new, j); j++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (j >= new->num_outputs) { ADD(output, set->rm_outs, orig, i); i++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); result = compare_serials(&orig->outputs[i].unknowns, &new->outputs[j].unknowns); + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (result == -1) { ADD(output, set->rm_outs, orig, i); i++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (result == 1) { ADD(output, set->added_outs, new, j); j++; continue; } + clone_psbt(NULL, orig); + clone_psbt(NULL, new); if (!output_identical(orig, i, new, j)) { ADD(output, set->rm_outs, orig, i); ADD(output, set->added_outs, new, j); } i++; j++; + clone_psbt(NULL, orig); + clone_psbt(NULL, new); } + clone_psbt(NULL, orig); // crash here + clone_psbt(NULL, new); + return set; } diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 4a9275840339..27f37de7c5e3 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -452,6 +452,8 @@ static void handle_splice_confirmed_update(struct lightningd *ld, return; } + assert(psbt); + if (psbt->version != cc->user_psbt_ver && !psbt_set_version(psbt, cc->user_psbt_ver)) channel_internal_error(channel, "Splice failed to convert from"