From 2fbe59985d3a86cc4eacbc545484d5543f05af7f Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 21 Jan 2025 10:32:07 -0500 Subject: [PATCH 01/35] 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 ca20eb990e6e..be138bc8321a 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3329,7 +3329,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; @@ -3360,8 +3360,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); @@ -3645,7 +3645,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 fc278a78a2d7e88b7f8fb528b6e375b810467c6a Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:41:33 -0500 Subject: [PATCH 02/35] 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. --- channeld/channeld.c | 204 +++++++++++++++++++++++------------ channeld/channeld_wire.csv | 1 + channeld/inflight.c | 12 +++ channeld/inflight.h | 3 +- lightningd/channel.c | 1 + lightningd/channel.h | 5 + lightningd/channel_control.c | 11 +- wallet/db.c | 1 + wallet/test/run-wallet.c | 10 ++ wallet/wallet.c | 51 +++++---- 10 files changed, 204 insertions(+), 95 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index be138bc8321a..160dc70c8efb 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3384,7 +3384,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->locked_scid); wire_sync_write(MASTER_FD, take(msg)); } @@ -3454,7 +3455,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->locked_scid); wire_sync_write(MASTER_FD, take(msg)); msg = towire_channeld_splice_sending_sigs(tmpctx, &final_txid); @@ -3623,7 +3625,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->locked_scid); wire_sync_write(MASTER_FD, take(msg)); } @@ -3849,11 +3852,12 @@ 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; new_inflight->force_sign_first = peer->splicing->force_sign_first; + new_inflight->locked_scid = NULL; current_push_val = relative_splice_balance_fundee(peer, our_role,ictx->current_psbt, outpoint.n, splice_funding_index); @@ -3875,15 +3879,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, @@ -3905,10 +3905,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: @@ -3923,7 +3919,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 * @@ -3932,21 +3928,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 @@ -3958,15 +3954,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; @@ -3974,10 +3966,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); } @@ -4012,7 +4005,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; @@ -4078,15 +4074,14 @@ 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->locked_scid = NULL; /* 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 * 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, @@ -4108,7 +4103,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->locked_scid); wire_sync_write(MASTER_FD, take(outmsg)); sign_first = do_i_sign_first(peer, new_inflight->psbt, our_role, @@ -4162,6 +4158,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; @@ -4187,8 +4185,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, @@ -4222,7 +4220,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); @@ -4271,17 +4269,18 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) } 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. */ outmsg = towire_channeld_update_inflight(NULL, inflight->psbt, inflight->last_tx, - &inflight->last_sig); + &inflight->last_sig, + inflight->locked_scid); 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); @@ -5541,6 +5540,7 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) struct pubkey point; bool splicing; struct bitcoin_txid txid; + struct inflight *inflight_match; if (!fromwire_channeld_funding_depth(tmpctx, msg, @@ -5554,14 +5554,55 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) if (peer->shutdown_sent[LOCAL]) return; - if (depth < peer->channel->minimum_depth) { - peer->depth_togo = peer->channel->minimum_depth - depth; - } else { - peer->depth_togo = 0; + if (splicing) { + if (depth < peer->channel->minimum_depth) + return; - /* For splicing we only update the short channel id on mutual - * splice lock */ - if (splicing) { + assert(peer->channel_ready[LOCAL]); + assert(peer->channel_ready[REMOTE]); + + if(!peer->splice_state->locked_ready[LOCAL]) { + assert(scid); + + inflight_match = NULL; + 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)) { + if (inflight_match) + peer_failed_err(peer->pps, + &peer->channel_id, + "It should be" + " impossible" + " for two" + " inflights to" + "match, %s", + fmt_bitcoin_txid(tmpctx, &txid)); + assert(scid); + assert(inflight->psbt); + inflight->locked_scid = tal_dup(inflight, + struct short_channel_id, + scid); + msg = towire_channeld_update_inflight(NULL, + inflight->psbt, + NULL, + NULL, + inflight->locked_scid); + wire_sync_write(MASTER_FD, take(msg)); + inflight_match = inflight; + } + } + + if (!inflight_match) { + status_debug("Ignoring stale fudning depth" + " notification %s for splice depth" + " check", + fmt_bitcoin_txid(tmpctx, &txid)); + return; + } + + /* For splicing we only update the short channel id on mutual + * splice lock */ peer->splice_state->short_channel_id = *scid; status_debug("Current channel id is %s, " "splice_short_channel_id now set to %s", @@ -5569,20 +5610,38 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) peer->short_channel_ids[LOCAL]), fmt_short_channel_id(tmpctx, peer->splice_state->short_channel_id)); - } else { - status_debug("handle_funding_depth: Setting short_channel_ids[LOCAL] to %s", - fmt_short_channel_id(tmpctx, - (scid ? *scid : peer->local_alias))); - /* If we know an actual short_channel_id prefer to use - * that, otherwise fill in the alias. From channeld's - * point of view switching from zeroconf to an actual - * funding scid is just a reorg. */ - if (scid) - peer->short_channel_ids[LOCAL] = *scid; - else - peer->short_channel_ids[LOCAL] = peer->local_alias; + + 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; + check_mutual_splice_locked(peer); } + return; + } + + if (depth < peer->channel->minimum_depth) { + peer->depth_togo = peer->channel->minimum_depth - depth; + } else { + peer->depth_togo = 0; + + status_debug("handle_funding_depth: Setting short_channel_ids[LOCAL] to %s", + fmt_short_channel_id(tmpctx, + (scid ? *scid : peer->local_alias))); + /* If we know an actual short_channel_id prefer to use + * that, otherwise fill in the alias. From channeld's + * point of view switching from zeroconf to an actual + * funding scid is just a reorg. */ + if (scid) + peer->short_channel_ids[LOCAL] = *scid; + else + peer->short_channel_ids[LOCAL] = peer->local_alias; + if (!peer->channel_ready[LOCAL]) { status_debug("channel_ready: sending commit index" " %"PRIu64": %s", @@ -5603,17 +5662,6 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg) peer->channel_ready[LOCAL] = true; check_mutual_channel_ready(peer); - } else if(splicing && !peer->splice_state->locked_ready[LOCAL]) { - assert(scid); - - msg = towire_splice_locked(NULL, &peer->channel_id); - - peer->splice_state->locked_txid = txid; - - peer_write(peer->pps, take(msg)); - - peer->splice_state->locked_ready[LOCAL] = true; - check_mutual_splice_locked(peer); } } @@ -6080,6 +6128,7 @@ static void init_channel(struct peer *peer) struct secret last_remote_per_commit_secret; struct penalty_base *pbases; struct channel_type *channel_type; + bool found_locked_inflight; assert(!(fcntl(MASTER_FD, F_GETFL) & O_NONBLOCK)); @@ -6144,6 +6193,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]->locked_scid) { + 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", diff --git a/channeld/channeld_wire.csv b/channeld/channeld_wire.csv index a05a3acbc4df..c2833504153a 100644 --- a/channeld/channeld_wire.csv +++ b/channeld/channeld_wire.csv @@ -272,6 +272,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,locked_scid,?short_channel_id, # channeld->master: A funding error has occured msgtype,channeld_splice_funding_error,7220 diff --git a/channeld/inflight.c b/channeld/inflight.c index d06142bf324d..88e7ac60d409 100644 --- a/channeld/inflight.c +++ b/channeld/inflight.c @@ -1,6 +1,7 @@ #include "config.h" #include #include +#include #include #include @@ -25,6 +26,14 @@ 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); + int has_locked_scid = fromwire_u8(cursor, max); + if (has_locked_scid) { + inflight->locked_scid = tal(inflight, struct short_channel_id); + *inflight->locked_scid = fromwire_short_channel_id(cursor, max); + } + else { + inflight->locked_scid = NULL; + } return inflight; } @@ -44,4 +53,7 @@ void towire_inflight(u8 **pptr, const struct inflight *inflight) } towire_bool(pptr, inflight->i_am_initiator); towire_bool(pptr, inflight->force_sign_first); + towire_u8(pptr, inflight->locked_scid ? 1 : 0); + if (inflight->locked_scid) + towire_short_channel_id(pptr, *inflight->locked_scid); } diff --git a/channeld/inflight.h b/channeld/inflight.h index 2c296f43148e..73d9d1ad1f38 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; @@ -18,7 +19,7 @@ struct inflight { struct bitcoin_signature last_sig; bool i_am_initiator; bool force_sign_first; - bool is_locked; + struct short_channel_id *locked_scid; }; struct inflight *fromwire_inflight(const tal_t *ctx, const u8 **cursor, size_t *max); diff --git a/lightningd/channel.c b/lightningd/channel.c index 87e3f2d96e56..2a25d36a1ca7 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -207,6 +207,7 @@ new_inflight(struct channel *channel, inflight->i_am_initiator = i_am_initiator; inflight->force_sign_first = force_sign_first; + inflight->locked_scid = NULL; inflight->splice_locked_memonly = false; list_add_tail(&channel->inflights, &inflight->list); diff --git a/lightningd/channel.h b/lightningd/channel.h index 5a802ce6b038..94de5c8b81be 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. */ + struct short_channel_id *locked_scid; + /* 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/lightningd/channel_control.c b/lightningd/channel_control.c index 2f6c17338594..5b30aaebce18 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -875,9 +875,10 @@ static void handle_update_inflight(struct lightningd *ld, struct bitcoin_txid txid; struct bitcoin_tx *last_tx; struct bitcoin_signature *last_sig; + struct short_channel_id *locked_scid; if (!fromwire_channeld_update_inflight(tmpctx, msg, &psbt, &last_tx, - &last_sig)) { + &last_sig, &locked_scid)) { channel_internal_error(channel, "bad channel_add_inflight %s", tal_hex(channel, msg)); @@ -905,6 +906,8 @@ static void handle_update_inflight(struct lightningd *ld, if (last_sig) inflight->last_sig = *last_sig; + inflight->locked_scid = tal_steal(inflight, locked_scid); + tal_wally_start(); if (wally_psbt_combine(inflight->funding_psbt, psbt) != WALLY_OK) { channel_internal_error(channel, @@ -1793,13 +1796,11 @@ bool peer_start_channeld(struct channel *channel, infcopy->amnt = inflight->funding->total_funds; infcopy->remote_tx_sigs = inflight->remote_tx_sigs; infcopy->splice_amnt = inflight->funding->splice_amnt; - if (inflight->last_tx) - infcopy->last_tx = tal_dup(infcopy, struct bitcoin_tx, inflight->last_tx); - else - infcopy->last_tx = NULL; + infcopy->last_tx = tal_dup_or_null(infcopy, struct bitcoin_tx, inflight->last_tx); infcopy->last_sig = inflight->last_sig; infcopy->i_am_initiator = inflight->i_am_initiator; infcopy->force_sign_first = inflight->force_sign_first; + infcopy->locked_scid = tal_dup_or_null(infcopy, struct short_channel_id, inflight->locked_scid); tal_wally_start(); wally_psbt_clone_alloc(inflight->funding_psbt, 0, &infcopy->psbt); diff --git a/wallet/db.c b/wallet/db.c index 2f67add763df..b40c94c9d40c 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1040,6 +1040,7 @@ static struct migration dbmigrations[] = { {SQL("ALTER TABLE channel_htlcs ADD updated_index BIGINT DEFAULT 0"), NULL}, {SQL("CREATE INDEX channel_htlcs_updated_idx ON channel_htlcs (updated_index)"), NULL}, {NULL, migrate_initialize_channel_htlcs_wait_indexes}, + {SQL("ALTER TABLE channel_funding_inflights ADD locked_scid BIGINT DEFAULT 0;"), NULL}, }; /** diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index d82a8c72c84e..122dd5de811c 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1652,6 +1652,11 @@ 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->locked_scid == !i2->locked_scid); + if (i1->locked_scid) + CHECK(memeq(i1->locked_scid, sizeof(*i1->locked_scid), + i2->locked_scid, sizeof(*i2->locked_scid))); + CHECK(i1->splice_locked_memonly == i2->splice_locked_memonly); return true; } @@ -2073,6 +2078,9 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) 0, false, false); + inflight->splice_locked_memonly = true; + inflight->locked_scid = tal(inflight, struct short_channel_id); + memset(inflight->locked_scid, 7, sizeof(struct short_channel_id)); inflight_set_last_tx(inflight, last_tx, sig); @@ -2100,6 +2108,8 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) 0, false, false); + inflight->splice_locked_memonly = false; + inflight->locked_scid = NULL; 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 43f7e290a8cc..0502600817d5 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1414,8 +1414,9 @@ void wallet_inflight_add(struct wallet *w, struct channel_inflight *inflight) ", i_am_initiator" ", force_sign_first" ", remote_funding" + ", locked_scid" ") VALUES (" - "?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); + "?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); db_bind_u64(stmt, inflight->channel->dbid); db_bind_txid(stmt, &inflight->funding->outpoint.txid); @@ -1458,6 +1459,10 @@ 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); + if (inflight->locked_scid) + db_bind_short_channel_id(stmt, *inflight->locked_scid); + else + db_bind_null(stmt); db_exec_prepared_v2(stmt); assert(!stmt->error); @@ -1486,17 +1491,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_scid 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_scid=?" // 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) { @@ -1506,6 +1512,10 @@ void wallet_inflight_save(struct wallet *w, db_bind_null(stmt); db_bind_null(stmt); } + if (inflight->locked_scid) + db_bind_short_channel_id(stmt, *inflight->locked_scid); + else + db_bind_null(stmt); db_bind_u64(stmt, inflight->channel->dbid); db_bind_txid(stmt, &inflight->funding->outpoint.txid); db_bind_int(stmt, inflight->funding->outpoint.n); @@ -1513,6 +1523,20 @@ void wallet_inflight_save(struct wallet *w, db_exec_prepared_v2(take(stmt)); } +static struct short_channel_id *db_col_optional_scid(const tal_t *ctx, + struct db_stmt *stmt, + const char *colname) +{ + struct short_channel_id *scid; + + if (db_col_is_null(stmt, colname)) + return NULL; + + scid = tal(ctx, struct short_channel_id); + *scid = db_col_short_channel_id(stmt, colname); + return scid; +} + void wallet_channel_clear_inflights(struct wallet *w, struct channel *chan) { @@ -1604,6 +1628,8 @@ wallet_stmt2inflight(struct wallet *w, struct db_stmt *stmt, i_am_initiator, force_sign_first); + inflight->locked_scid = db_col_optional_scid(inflight, stmt, "locked_scid"); + /* last_tx is null for not yet committed * channels + static channel backup recoveries */ if (!db_col_is_null(stmt, "last_tx")) { @@ -1655,6 +1681,7 @@ static bool wallet_channel_load_inflights(struct wallet *w, ", i_am_initiator" ", force_sign_first" ", remote_funding" + ", locked_scid" " FROM channel_funding_inflights" " WHERE channel_id = ?" " ORDER BY funding_feerate")); @@ -1703,20 +1730,6 @@ static bool wallet_channel_config_load(struct wallet *w, const u64 id, return ok; } -static struct short_channel_id *db_col_optional_scid(const tal_t *ctx, - struct db_stmt *stmt, - const char *colname) -{ - struct short_channel_id *scid; - - if (db_col_is_null(stmt, colname)) - return NULL; - - scid = tal(ctx, struct short_channel_id); - *scid = db_col_short_channel_id(stmt, colname); - return scid; -} - static struct channel_state_change **wallet_state_change_get(const tal_t *ctx, struct wallet *w, u64 channel_id) From fbc11e488d38b80e01ede363685e2f48c7a77ecd Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sat, 1 Feb 2025 10:52:50 -0500 Subject: [PATCH 03/35] 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 160dc70c8efb..4641ef768e61 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5199,6 +5199,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 f44daa801c7eb392d6e34810b5fbf649b2b17d2d Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 12:02:31 -0500 Subject: [PATCH 04/35] 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 5b30aaebce18..e1ae935ebdd8 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -50,6 +50,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) @@ -399,6 +401,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)); @@ -433,6 +442,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); @@ -2276,6 +2292,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, @@ -2322,6 +2344,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))); @@ -2352,6 +2380,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)); @@ -2383,6 +2417,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 e40b360898301025ba16a1da7cc701cdcc0da6d6 Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Mon, 27 Jan 2025 14:39:36 +0100 Subject: [PATCH 05/35] 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 4641ef768e61..92b617389099 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4164,6 +4164,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 d08f07924f29..ef4bc61474ee 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -229,12 +229,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); @@ -242,6 +255,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 828387f781b7eaeba0f8886bc173136a0c309fcc Mon Sep 17 00:00:00 2001 From: Richard Myers Date: Wed, 22 Jan 2025 15:42:39 +0100 Subject: [PATCH 06/35] splice: Add splice_txid to splice_locked message --- channeld/channeld.c | 12 ++++++++---- tests/fuzz/fuzz-wire-splice_locked.c | 5 +++-- wire/peer_wire.csv | 1 + 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 92b617389099..fac75b198dcd 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)); @@ -5220,7 +5221,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)) { @@ -5266,7 +5268,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." 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 e820e7aedd15..f3ef243bc636 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -229,6 +229,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 e7aa9dba9c08d8244cefb131c8f42b03afff4057 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 4 Feb 2025 15:23:31 -0500 Subject: [PATCH 07/35] 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 | 33 +++++++++++++++++++++++++++++---- channeld/splice.c | 1 + channeld/splice.h | 2 ++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index fac75b198dcd..1240f123e25d 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -444,8 +444,22 @@ 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", + 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)); peer->splice_state->await_commitment_succcess = true; @@ -473,7 +487,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 +501,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 +522,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 +537,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 06a2bb47e4df5f80fb0f2c797fb264303ecba5d7 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 7 Feb 2025 13:17:09 -0500 Subject: [PATCH 08/35] 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 | 50 +++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/common/psbt_open.c b/common/psbt_open.c index 42fe1e1808a6..c2bf8558a190 100644 --- a/common/psbt_open.c +++ b/common/psbt_open.c @@ -61,14 +61,17 @@ 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]; + psbt->num_inputs = 1; /* Sort the inputs, so serializing them is ok */ wally_map_sort(&psbt->inputs[0].unknowns, 0); @@ -88,16 +91,22 @@ 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; + psbt->num_inputs = 0; + 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 +114,10 @@ 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]; + psbt->num_outputs = 1; + /* Sort the outputs, so serializing them is ok */ wally_map_sort(&psbt->outputs[0].unknowns, 0); @@ -120,9 +131,12 @@ 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; + psbt->num_outputs = 0; + tal_free(psbt); + tal_free(copy); + return bytes; } @@ -131,10 +145,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 +156,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 38193d9fb63e75f7650eb604951676114d16b1fb Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 7 Feb 2025 14:00:01 -0500 Subject: [PATCH 09/35] 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 1240f123e25d..99e088b108d1 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4186,7 +4186,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; @@ -4213,10 +4215,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 f3fb53a378d1f8e7622a2eb42b2539617752b940 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Sun, 16 Feb 2025 15:13:01 -0500 Subject: [PATCH 10/35] PSBT: Change bitcoin_tx routine to use TAKES MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bitcoin_tx_with_psbt` would somewhat opaquely steal the passed `psbt` value. This caused a bug where code made a `bitcoin_tx` using a psbt without realizing the value was stolen. Because the resulting `bitcoin_tx` was placed in tmpctx it was not immediately clear that using `psbt` afterwards was an error until the tmpctx was cleared — creating a valgrind backtrace far from the actual issue. Switching to the routine to using TAKES and adding documentation in the header, makes it explicitly clear which operation the user is doing — helping prevent future regressions of this kind. Changelog-None --- bitcoin/tx.c | 6 +++++- bitcoin/tx.h | 3 ++- db/bindings.c | 2 +- lightningd/channel.c | 4 +--- lightningd/peer_control.c | 7 ++----- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index bb9aa3bd8061..869bc6897979 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -548,9 +548,13 @@ void bitcoin_tx_finalize(struct bitcoin_tx *tx) assert(bitcoin_tx_check(tx)); } -struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt STEALS) +struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt TAKES) { size_t locktime; + + if (!taken(psbt)) + psbt = clone_psbt(NULL, psbt); + wally_psbt_get_locktime(psbt, &locktime); struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, psbt->num_inputs, diff --git a/bitcoin/tx.h b/bitcoin/tx.h index fc6844035fc1..2549cd5e2b61 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -111,7 +111,8 @@ bool bitcoin_txid_to_hex(const struct bitcoin_txid *txid, char *hexstr, size_t hexstr_len); /* Create a bitcoin_tx from a psbt */ -struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt); +struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, + struct wally_psbt *psbt TAKES); /* Internal de-linearization functions. */ /* Pull a bitcoin tx, and create a PSBT wrapper for it */ diff --git a/db/bindings.c b/db/bindings.c index e8b83785b753..9107bc5c28bd 100644 --- a/db/bindings.c +++ b/db/bindings.c @@ -501,7 +501,7 @@ struct bitcoin_tx *db_col_psbt_to_tx(const tal_t *ctx, struct db_stmt *stmt, con struct wally_psbt *psbt = db_col_psbt(ctx, stmt, colname); if (!psbt) return NULL; - return bitcoin_tx_with_psbt(ctx, psbt); + return bitcoin_tx_with_psbt(ctx, take(psbt)); } struct channel_type *db_col_channel_type(const tal_t *ctx, struct db_stmt *stmt, diff --git a/lightningd/channel.c b/lightningd/channel.c index 2a25d36a1ca7..94ff06c7b842 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -220,12 +220,10 @@ void inflight_set_last_tx(struct channel_inflight *inflight, struct bitcoin_tx *last_tx, const struct bitcoin_signature last_sig) { - struct wally_psbt *last_tx_psbt_clone; assert(inflight->last_tx == NULL); assert(last_tx); - last_tx_psbt_clone = clone_psbt(inflight, last_tx->psbt); - inflight->last_tx = bitcoin_tx_with_psbt(inflight, last_tx_psbt_clone); + inflight->last_tx = bitcoin_tx_with_psbt(inflight, last_tx->psbt); inflight->last_sig = last_sig; } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 56ea80828a9a..03e8644d1ed4 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2105,8 +2105,6 @@ void update_channel_from_inflight(struct lightningd *ld, struct channel *channel, const struct channel_inflight *inflight) { - struct wally_psbt *psbt_copy; - channel->funding = inflight->funding->outpoint; channel->funding_sats = inflight->funding->total_funds; @@ -2141,10 +2139,9 @@ void update_channel_from_inflight(struct lightningd *ld, channel->opener, &inflight->lease_blockheight_start); - /* Make a 'clone' of this tx */ - psbt_copy = clone_psbt(channel, inflight->last_tx->psbt); channel_set_last_tx(channel, - bitcoin_tx_with_psbt(channel, psbt_copy), + bitcoin_tx_with_psbt(channel, + inflight->last_tx->psbt), &inflight->last_sig); /* If the remote side rotated their pubkey during splice, update now */ From df16c461f684a6eae27f685fd54ad685b5ce632e Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 17 Feb 2025 21:18:36 -0500 Subject: [PATCH 11/35] PSBT: Add audi_psbt routine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A routine that audit’s and asserts PSBT memory to confirm it has a sane memory allocation hierarchy. Changelog-None --- bitcoin/psbt.c | 73 +++++++++++++++++++++++++++++++++ bitcoin/psbt.h | 11 +++++ bitcoin/test/run-psbt-from-tx.c | 1 + channeld/channeld.c | 20 +++++++++ common/test/run-psbt_diff.c | 14 ++++--- 5 files changed, 114 insertions(+), 5 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index f42261d89e53..dff12b040fad 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -102,6 +102,79 @@ struct wally_psbt *combine_psbt(const tal_t *ctx, return combined_psbt; } +static bool parent_or_grandparent(const tal_t *goal, const tal_t *child) +{ + const tal_t *parent = tal_parent(child); + if (!parent) + return false; + return parent == goal || parent_or_grandparent(goal, parent); +} + +#define NULL_OR_MATCH_P(item, parent) \ + ((item) == NULL || parent_or_grandparent((parent), (item))) + +static void audit_map(const tal_t *ctx, const struct wally_map *map) +{ + assert(NULL_OR_MATCH_P(map->items, ctx)); + for (size_t i = 0; i < map->num_items; i++) { + assert(NULL_OR_MATCH_P(map->items[i].key, ctx)); + assert(NULL_OR_MATCH_P(map->items[i].value, ctx)); + assert(!map->items[i].key + || tal_bytelen(map->items[i].key) + == map->items[i].key_len); + assert(!map->items[i].value + || tal_bytelen(map->items[i].value) + == map->items[i].value_len); + } +} + +void audit_psbt(const tal_t *ctx, const struct wally_psbt *psbt) +{ + assert(psbt); + assert(NULL_OR_MATCH_P(psbt->tx, ctx)); + assert(NULL_OR_MATCH_P(psbt->inputs, ctx)); + assert(NULL_OR_MATCH_P(psbt->outputs, ctx)); + audit_map(ctx, &psbt->unknowns); + audit_map(ctx, &psbt->global_xpubs); +#ifndef WALLY_ABI_NO_ELEMENTS + audit_map(ctx, &psbt->global_scalars); +#endif + for (size_t i = 0; i < psbt->num_inputs; i++) { + assert(NULL_OR_MATCH_P(psbt->inputs[i].utxo, ctx)); + assert(NULL_OR_MATCH_P(psbt->inputs[i].witness_utxo, ctx)); + assert(NULL_OR_MATCH_P(psbt->inputs[i].final_witness, ctx)); + audit_map(ctx, &psbt->inputs[i].keypaths); + audit_map(ctx, &psbt->inputs[i].signatures); + audit_map(ctx, &psbt->inputs[i].unknowns); + audit_map(ctx, &psbt->inputs[i].preimages); + audit_map(ctx, &psbt->inputs[i].psbt_fields); + audit_map(ctx, &psbt->inputs[i].taproot_leaf_signatures); + audit_map(ctx, &psbt->inputs[i].taproot_leaf_scripts); + audit_map(ctx, &psbt->inputs[i].taproot_leaf_hashes); + audit_map(ctx, &psbt->inputs[i].taproot_leaf_paths); + /* DTODO: Investigate if taproot wally maps have child maps */ +#ifndef WALLY_ABI_NO_ELEMENTS + assert(NULL_OR_MATCH_P(psbt->inputs[i].pegin_tx, ctx)); + assert(NULL_OR_MATCH_P(psbt->inputs[i].pegin_witness, ctx)); + audit_map(ctx, &psbt->inputs[i].pset_fields); +#endif + } + for (size_t i = 0; i < psbt->num_outputs; i++) { + assert(NULL_OR_MATCH_P(psbt->outputs[i].script, ctx)); + assert(psbt->outputs[i].script_len + == tal_bytelen(psbt->outputs[i].script)); + audit_map(ctx, &psbt->outputs[i].keypaths); + audit_map(ctx, &psbt->outputs[i].unknowns); + audit_map(ctx, &psbt->outputs[i].psbt_fields); + audit_map(ctx, &psbt->outputs[i].taproot_tree); + audit_map(ctx, &psbt->outputs[i].taproot_leaf_hashes); + audit_map(ctx, &psbt->outputs[i].taproot_leaf_paths); +#ifndef WALLY_ABI_NO_ELEMENTS + audit_map(ctx, &psbt->outputs[i].pset_fields); +#endif + } +} + bool psbt_is_finalized(const struct wally_psbt *psbt) { size_t is_finalized; diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index 1335431ff6f5..a44bba06636e 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -61,6 +61,17 @@ struct wally_psbt *combine_psbt(const tal_t *ctx, const struct wally_psbt *psbt0, const struct wally_psbt *psbt1); +/** + * audit_psbt - Audit the memory structure of the PSBT. + * + * This checks all known memory allocations in the PSBT and asserts that they + * are all allocated with 'ctx' being it's parent. + * + * ctx - the ctx all memory *should* be attached to + * psbt - the PSBT to audit. + * */ +void audit_psbt(const tal_t *ctx, const struct wally_psbt *psbt); + /** * psbt_is_finalized - Check if tx is ready to be extracted * diff --git a/bitcoin/test/run-psbt-from-tx.c b/bitcoin/test/run-psbt-from-tx.c index d813fd4d3931..2b55c420b0a2 100644 --- a/bitcoin/test/run-psbt-from-tx.c +++ b/bitcoin/test/run-psbt-from-tx.c @@ -114,6 +114,7 @@ int main(int argc, char *argv[]) const struct wally_map_item *final_scriptsig = wally_map_get_integer(&tx2->psbt->inputs[0].psbt_fields, /* PSBT_IN_FINAL_SCRIPTSIG */ 0x07); assert(final_scriptsig->value_len > 0); assert(tx2->psbt->inputs[0].final_witness != NULL); + audit_psbt(tx2->psbt, tx2->psbt); common_shutdown(); return 0; diff --git a/channeld/channeld.c b/channeld/channeld.c index 99e088b108d1..4b8954340fa8 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3677,6 +3677,8 @@ static void resume_splice_negotiation(struct peer *peer, new_output_index); wire_sync_write(MASTER_FD, take(msg)); } + + audit_psbt(current_psbt, current_psbt); } static struct inflight *inflights_new(struct peer *peer) @@ -3995,10 +3997,15 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg) outmsg = towire_channeld_splice_confirmed_init(NULL, psbt); wire_sync_write(MASTER_FD, take(outmsg)); + audit_psbt(psbt, psbt); + /* 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); + + audit_psbt(peer->splicing->current_psbt, peer->splicing->current_psbt); + assert(tal_parent(peer->splicing->current_psbt) != tmpctx); } /* This occurs when the user has marked they are done making changes to the @@ -4102,6 +4109,8 @@ static void splice_initiator_user_finalized(struct peer *peer) new_inflight->force_sign_first = peer->splicing->force_sign_first; new_inflight->locked_scid = NULL; + audit_psbt(ictx->current_psbt, ictx->current_psbt); + /* 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 * normal in-memory copy of the psbt: peer->splicing/ictx->current_psbt. @@ -4144,6 +4153,9 @@ static void splice_initiator_user_finalized(struct peer *peer) true, !sign_first); wire_sync_write(MASTER_FD, take(outmsg)); + + audit_psbt(new_inflight->psbt, new_inflight->psbt); + assert(tal_parent(new_inflight->psbt) != tmpctx); } /* During a splice the user may call splice_update mulitple times adding @@ -4227,6 +4239,8 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) ictx->current_psbt, false, false); wire_sync_write(MASTER_FD, take(outmsg)); + audit_psbt(peer->splicing->current_psbt, peer->splicing->current_psbt); + assert(tal_parent(peer->splicing->current_psbt) != tmpctx); } /* This occurs when the user has signed the final version of the PSBT. At this @@ -4317,7 +4331,13 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) sign_first = do_i_sign_first(peer, inflight->psbt, TX_INITIATOR, inflight->force_sign_first); + audit_psbt(inflight->psbt, inflight->psbt); + assert(tal_parent(inflight->psbt) != tmpctx); + resume_splice_negotiation(peer, false, false, true, sign_first); + + audit_psbt(inflight->psbt, inflight->psbt); + assert(tal_parent(inflight->psbt) != tmpctx); } /* This occurs once our 'stfu' transition was successful. */ diff --git a/common/test/run-psbt_diff.c b/common/test/run-psbt_diff.c index 37f4fdf495dc..6d355697792d 100644 --- a/common/test/run-psbt_diff.c +++ b/common/test/run-psbt_diff.c @@ -162,7 +162,7 @@ int main(int argc, const char *argv[]) tal_wally_start(); if (wally_psbt_clone_alloc(end, flags, &start) != WALLY_OK) abort(); - tal_wally_end(tmpctx); + tal_wally_end_onto(tmpctx, start, struct wally_psbt); diff_count(start, end, 0, 0); diff_count(end, start, 0, 0); @@ -175,7 +175,7 @@ int main(int argc, const char *argv[]) tal_wally_start(); if (wally_psbt_clone_alloc(end, flags, &start) != WALLY_OK) abort(); - tal_wally_end(tmpctx); + tal_wally_end_onto(tmpctx, start, struct wally_psbt); add_in_out_with_serial(end, 5, 2); diff_count(start, end, 1, 0); diff_count(end, start, 0, 1); @@ -184,7 +184,7 @@ int main(int argc, const char *argv[]) tal_wally_start(); if (wally_psbt_clone_alloc(end, flags, &start) != WALLY_OK) abort(); - tal_wally_end(tmpctx); + tal_wally_end_onto(tmpctx, start, struct wally_psbt); add_in_out_with_serial(end, 15, 3); diff_count(start, end, 1, 0); diff_count(end, start, 0, 1); @@ -193,7 +193,7 @@ int main(int argc, const char *argv[]) tal_wally_start(); if (wally_psbt_clone_alloc(end, flags, &start) != WALLY_OK) abort(); - tal_wally_end(tmpctx); + tal_wally_end_onto(tmpctx, start, struct wally_psbt); add_in_out_with_serial(end, 11, 4); diff_count(start, end, 1, 0); diff_count(end, start, 0, 1); @@ -205,7 +205,7 @@ int main(int argc, const char *argv[]) tal_wally_start(); if (wally_psbt_clone_alloc(end, flags, &start) != WALLY_OK) abort(); - tal_wally_end(tmpctx); + tal_wally_end_onto(tmpctx, start, struct wally_psbt); psbt_rm_output(end, 0); psbt_rm_input(end, 0); add_in_out_with_serial(end, 5, 5); @@ -219,6 +219,7 @@ int main(int argc, const char *argv[]) psbt_input_set_unknown(start, &start->inputs[1], key, val, tal_bytelen(val)); /* Swap locations */ + assert(end->inputs[1].unknowns.num_items > 1); struct wally_map_item tmp; tmp = end->inputs[1].unknowns.items[0]; end->inputs[1].unknowns.items[0] = end->inputs[1].unknowns.items[1]; @@ -232,6 +233,9 @@ int main(int argc, const char *argv[]) check_psbt_comparison(); + audit_psbt(start, start); + audit_psbt(end, end); + /* No memory leaks please */ common_shutdown(); return 0; From 312db53bee725c6367ec84ad4cb31ce0542ff1c9 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 17 Feb 2025 21:21:25 -0500 Subject: [PATCH 12/35] PSBT: tal_wally updates and docs Default wally_tal_ctx to NULL, add extra asserts and tal_checks, and documentation explaning the usage of tal_wally_start/end. Changelog-None --- common/setup.c | 1 + common/utils.c | 11 +++++++++-- common/utils.h | 24 ++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/common/setup.c b/common/setup.c index 8db367eec9d6..56ff6828473a 100644 --- a/common/setup.c +++ b/common/setup.c @@ -11,6 +11,7 @@ static void *cln_wally_tal(size_t size) { assert(wally_tal_ctx); + assert(tal_check(wally_tal_ctx, "cln_wally_tal ctx check")); return tal_arr_label(wally_tal_ctx, u8, size, "cln_wally_tal"); } diff --git a/common/utils.c b/common/utils.c index a942a12ae7da..f51094f41eeb 100644 --- a/common/utils.c +++ b/common/utils.c @@ -10,7 +10,7 @@ #include #include -const tal_t *wally_tal_ctx; +const tal_t *wally_tal_ctx = NULL; secp256k1_context *secp256k1_ctx; const tal_t *tmpctx; @@ -35,8 +35,14 @@ void tal_wally_start(void) void tal_wally_end(const tal_t *parent) { tal_t *p; + if (!wally_tal_ctx) { + /* This makes valgrind show us backtraces! */ + *(u8 *)wally_tal_ctx = '\0'; + abort(); + } while ((p = tal_first(wally_tal_ctx)) != NULL) { - /* Refuse to make a loop! */ + /* Refuse to make a loop! + * Did you mean to use tal_wally_end_onto? */ assert(p != parent); /* Don't steal backtrace from wally_tal_ctx! */ if (tal_name(p) && streq(tal_name(p), "backtrace")) { @@ -45,6 +51,7 @@ void tal_wally_end(const tal_t *parent) } tal_steal(parent, p); } + assert(tal_check(wally_tal_ctx, "tal_wally_end ctx check")); wally_tal_ctx = tal_free(wally_tal_ctx); } diff --git a/common/utils.h b/common/utils.h index 4fd54aab3680..26992fdb06b0 100644 --- a/common/utils.h +++ b/common/utils.h @@ -112,12 +112,32 @@ void clean_tmpctx(void); /* Call this before any libwally function which allocates. */ void tal_wally_start(void); -/* Then call this to reparent everything onto this parent */ +/* Then call this to reparent everything onto this parent. + * + * This method should *not* be used for creation of psbt objects, + * tal_wally_end_onto should be used instead! + * + * Typical usage is to end on the PSBT being modified. + */ void tal_wally_end(const tal_t *parent); /* ... or this if you want to reparent onto something which is * allocated by libwally here. Fixes up this from_wally obj to have a - * proper tal_name, too! */ + * proper tal_name, too! + * + * This results in a tal hierarchy like so: + * parent -> from_wally -> [everything wally allocated] + * + * This is typically used when allocating a new psbt and ensuring all the psbt's + * member objects are children of the PSBT itself. + * + * For example: + * 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); + */ #define tal_wally_end_onto(parent, from_wally, type) \ tal_wally_end_onto_( \ (parent), (from_wally), \ From 2fa0bcf8304c9562c4e4cd34f495787e7dc781a5 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 17 Feb 2025 21:22:33 -0500 Subject: [PATCH 13/35] splice: Prevent user from signing an unfinal splice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An extra check to ensure the user doesn’t try to sign a splice that wasn’t finalized. --- channeld/channeld.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/channeld/channeld.c b/channeld/channeld.c index 4b8954340fa8..24a89c8414e6 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4267,6 +4267,15 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) wire_sync_write(MASTER_FD, take(msg)); return; } + if (!inflight) { + msg = towire_channeld_splice_state_error(NULL, "Can't sign a" + " splice until the" + " splice is finalized" + " using" + " splice_update"); + wire_sync_write(MASTER_FD, take(msg)); + return; + } if (!fromwire_channeld_splice_signed(inflight, inmsg, &signed_psbt, &peer->splicing->force_sign_first)) From 746a1add2753b0df521f0e8ff99eef2bcec5df3d Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 17 Feb 2025 21:23:56 -0500 Subject: [PATCH 14/35] PSBT: Clean up PSBT chunk allocations Cleaning up the memory hierarchy of PSBT usage in splicing and `psbt_finalize_input` --- channeld/channeld.c | 4 ++-- common/interactivetx.c | 2 +- common/psbt_internal.c | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 24a89c8414e6..623fbb65a2f2 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -4236,7 +4236,7 @@ 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)); audit_psbt(peer->splicing->current_psbt, peer->splicing->current_psbt); @@ -4277,7 +4277,7 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) return; } - if (!fromwire_channeld_splice_signed(inflight, inmsg, &signed_psbt, + if (!fromwire_channeld_splice_signed(tmpctx, inmsg, &signed_psbt, &peer->splicing->force_sign_first)) master_badmsg(WIRE_CHANNELD_SPLICE_SIGNED, inmsg); diff --git a/common/interactivetx.c b/common/interactivetx.c index ef4bc61474ee..0a72a7df658d 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -588,7 +588,7 @@ char *process_interactivetx_updates(const tal_t *ctx, tal_wally_start(); wally_psbt_input_set_utxo(in, tx->wtx); - tal_wally_end(ictx); + tal_wally_end(ictx->current_psbt); psbt_input_set_serial_id(ictx->current_psbt, in, serial_id); diff --git a/common/psbt_internal.c b/common/psbt_internal.c index 4243c9a02cc1..4b0c98ef97db 100644 --- a/common/psbt_internal.c +++ b/common/psbt_internal.c @@ -106,10 +106,12 @@ void psbt_finalize_input(const tal_t *ctx, redeem_script->value, redeem_script->value_len, 0); u8 *final_scriptsig = - bitcoin_scriptsig_redeem(NULL, + bitcoin_scriptsig_redeem(ctx, take(redeemscript)); + tal_wally_start(); wally_psbt_input_set_final_scriptsig(in, final_scriptsig, tal_bytelen(final_scriptsig)); - wally_psbt_input_set_redeem_script(in, tal_arr(NULL, u8, 0), 0); + wally_psbt_input_set_redeem_script(in, tal_arr(in, u8, 0), 0); + tal_wally_end(ctx); } } From 083ec093f8e2e4b58b3f0de43eb4f3b2743e7e7b Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 14 Mar 2025 13:21:14 -0400 Subject: [PATCH 15/35] splice: Clean error statement Error statement was misformatted and caused a crash instead of reporting the error. Changelog-None --- lightningd/peer_control.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 03e8644d1ed4..f247e05a1a76 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2118,7 +2118,7 @@ void update_channel_from_inflight(struct lightningd *ld, "Updaing channel view for splice causes" " an invalid satoshi amount wrapping," " channel: %s, initial funds: %s, splice" - " banace change: %s", + " balance change: "PRIi64, fmt_channel_id(tmpctx, &channel->cid), fmt_amount_sat(tmpctx, channel->our_funds), From 9dfc62b6bdefddf27e185568fe12d8b6a84080cb Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 17 Apr 2025 13:14:44 -0400 Subject: [PATCH 16/35] splice: Update `our_funds` during splice_lock The value of `our_funds` inlightningd is the funds added to the channel during creation. Splicing is a quasi-creation event. This change makes our pending funds be considered funding funds at the moment of splice confirmation. Changelog-None --- lightningd/channel_control.c | 10 +++++----- lightningd/dual_open_control.c | 3 ++- lightningd/peer_control.c | 10 ++++++++-- lightningd/peer_control.h | 3 ++- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index e1ae935ebdd8..e7923f5dc52b 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -1112,6 +1112,11 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg) " locked_txid %s", fmt_bitcoin_txid(tmpctx, &locked_txid)); + wallet_htlcsigs_confirm_inflight(channel->peer->ld->wallet, channel, + &inflight->funding->outpoint); + + update_channel_from_inflight(channel->peer->ld, channel, inflight, true); + /* Stash prev funding data so we can log it after scid is updated * (to get the blockheight) */ prev_our_msats = channel->our_msat; @@ -1122,11 +1127,6 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg) channel->msat_to_us_min.millisatoshis += splice_amnt * 1000; /* Raw: splicing */ channel->msat_to_us_max.millisatoshis += splice_amnt * 1000; /* Raw: splicing */ - wallet_htlcsigs_confirm_inflight(channel->peer->ld->wallet, channel, - &inflight->funding->outpoint); - - update_channel_from_inflight(channel->peer->ld, channel, inflight); - /* Remember that we got the lockin */ wallet_channel_save(channel->peer->ld->wallet, channel); diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 8f971c5ffd13..0043f33725f8 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1056,7 +1056,8 @@ static enum watch_result opening_depth_cb(struct lightningd *ld, &inflight->channel->peer->id); if (depth >= inflight->channel->minimum_depth) - update_channel_from_inflight(ld, inflight->channel, inflight); + update_channel_from_inflight(ld, inflight->channel, inflight, + false); dualopend_tell_depth(inflight->channel, txid, depth); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index f247e05a1a76..e4f3bcc0c27f 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -356,7 +356,7 @@ static enum watch_result closed_inflight_depth_cb(struct lightningd *ld, return KEEP_WATCHING; /* This is now the main tx. */ - update_channel_from_inflight(ld, inflight->channel, inflight); + update_channel_from_inflight(ld, inflight->channel, inflight, false); channel_fail_saw_onchain(inflight->channel, REASON_UNKNOWN, tx, @@ -2103,13 +2103,19 @@ void peer_disconnect_done(struct lightningd *ld, const u8 *msg) void update_channel_from_inflight(struct lightningd *ld, struct channel *channel, - const struct channel_inflight *inflight) + const struct channel_inflight *inflight, + bool is_splice) { channel->funding = inflight->funding->outpoint; channel->funding_sats = inflight->funding->total_funds; channel->our_funds = inflight->funding->our_funds; + /* At this point, our_msat *becomes* our_funds because the splice + * confirms. Any excess millisats stay in our_msats */ + if (is_splice) + channel->our_funds = amount_msat_to_sat_round_down(channel->our_msat); + if (!amount_sat_add_sat_s64(&channel->our_funds, channel->our_funds, inflight->funding->splice_amnt)) { diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 84d11f2ca797..ddfeecf3ca20 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -128,7 +128,8 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel, void update_channel_from_inflight(struct lightningd *ld, struct channel *channel, - const struct channel_inflight *inflight); + const struct channel_inflight *inflight, + bool is_splice); void channel_watch_funding(struct lightningd *ld, struct channel *channel); From 2f196dc22731a82e60e41274bfc0a2d4ccbf2b35 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 17 Apr 2025 14:47:34 -0400 Subject: [PATCH 17/35] splice: Allow commit_sig batch in any order Other implementations are sending commit_sig batches in different orders. We add support for them being in any order by ordering the batch of messages after receiving them. Changelog-Changed: Increase interop compatability by loosening requirement that commitment signed messages be received in a particular order and sorting them internally. --- channeld/channeld.c | 176 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 155 insertions(+), 21 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 623fbb65a2f2..c542e9f8bea5 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1889,6 +1889,10 @@ struct commitsig_info { * * `commit_index` 0 refers to the funding commit. `commit_index` 1 and above * refer to inflight splices. + * + * `msg_batch` refers to the entire batch of messages in this commit_sig bundle + * with index 0 being the funding commit_sig and the rest being inflights. The + * inflight msgs must be in the same order as the inflight array. */ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, const u8 *msg, @@ -1899,7 +1903,8 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, s64 remote_splice_amnt, u64 local_index, const struct pubkey *local_per_commit, - bool allow_empty_commit) + bool allow_empty_commit, + const u8 **msg_batch) { struct commitsig_info *result; struct channel_id channel_id; @@ -1913,8 +1918,6 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, size_t i; const struct hsm_htlc *htlcs; const u8 * msg2; - u8 *splice_msg; - int type; struct bitcoin_outpoint outpoint; struct amount_sat funding_sats; struct channel_id active_id; @@ -2144,9 +2147,17 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, result->old_secret = old_secret; /* Only the parent call continues from here. * Return for all child calls. */ - if(commit_index) + if (commit_index) return result; + if (tal_count(msg_batch) - 1 < tal_count(peer->splice_state->inflights)) + peer_failed_err(peer->pps, &peer->channel_id, + "commit_sig batch was too small (%zu). It must" + " include a commit sig for all inflights plus" + " channel funding (req: %zu).", + tal_count(msg_batch), + tal_count(peer->splice_state->inflights)); + commitsigs = tal_arr(NULL, const struct commitsig*, 0); /* We expect multiple consequtive commit_sig messages if we have * inflight splices. Since consequtive is requred, we recurse for @@ -2156,23 +2167,13 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, peer->channel->funding_sats); s64 sub_splice_amnt = peer->splice_state->inflights[i]->splice_amnt; - splice_msg = peer_read(tmpctx, peer->pps); - check_tx_abort(peer, splice_msg); - /* Check type for cleaner failure message */ - type = fromwire_peektype(msg); - if (type != WIRE_COMMITMENT_SIGNED) - peer_failed_err(peer->pps, &peer->channel_id, - "Expected splice related " - "WIRE_COMMITMENT_SIGNED but got %s", - peer_wire_name(type)); - /* We purposely just store the last commit msg in result */ - result = handle_peer_commit_sig(peer, splice_msg, i + 1, + result = handle_peer_commit_sig(peer, msg_batch[i + 1], i + 1, peer->splice_state->inflights[i]->remote_funding, changed_htlcs, sub_splice_amnt, funding_diff - sub_splice_amnt, local_index, local_per_commit, - allow_empty_commit); + allow_empty_commit, NULL); old_secret = result->old_secret; tal_arr_expand(&commitsigs, result->commitsig); tal_steal(commitsigs, result); @@ -2201,6 +2202,136 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, return result; } +/* Returns 0 if this is for funding, and 1+ for items in inflight array. + * Returns -1 if the value is not recognized. + */ +static int commit_index_from_msg(const u8 *msg, struct peer *peer) +{ + struct channel_id funding_id; + struct channel_id channel_id; + struct bitcoin_signature commit_sig; + secp256k1_ecdsa_signature *raw_sigs; + struct tlv_commitment_signed_tlvs *cs_tlv + = tlv_commitment_signed_tlvs_new(tmpctx); + fromwire_commitment_signed(tmpctx, msg, &channel_id, &commit_sig.s, + &raw_sigs, &cs_tlv); + + derive_channel_id(&funding_id, &peer->channel->funding); + if (channel_id_eq(&funding_id, &channel_id)) + return 0; + + for (int i = 0; i < tal_count(peer->splice_state->inflights); i++) { + struct channel_id splice_id; + derive_channel_id(&splice_id, + &peer->splice_state->inflights[i]->outpoint); + + if (channel_id_eq(&splice_id, &channel_id)) + return i + 1; + } + + return -1; +} + +static int commit_cmp(const void *a, const void *n, void *peer) +{ + int commit_index_a = commit_index_from_msg(*(u8**)a, peer); + int commit_index_n = commit_index_from_msg(*(u8**)n, peer); + + if (commit_index_a == commit_index_n) + return 0; + + /* Unrecognized commits go on the end */ + if (commit_index_a == -1) + return 1; + + if (commit_index_n == -1) + return -1; + + /* Otherwise we sort by commit_index */ + return commit_index_a - commit_index_n; +} + +static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, + const u8 *msg, + u32 commit_index, + struct pubkey remote_funding, + const struct htlc **changed_htlcs, + s64 splice_amnt, + s64 remote_splice_amnt, + u64 local_index, + const struct pubkey *local_per_commit, + bool allow_empty_commit) +{ + struct channel_id channel_id; + struct bitcoin_signature commit_sig; + secp256k1_ecdsa_signature *raw_sigs; + u16 batch_size; + const u8 **msg_batch; + enum peer_wire type; + struct tlv_commitment_signed_tlvs *cs_tlv + = tlv_commitment_signed_tlvs_new(tmpctx); + status_info("fromwire_commitment_signed(%p) primary", msg); + if (!fromwire_commitment_signed(tmpctx, msg, + &channel_id, &commit_sig.s, &raw_sigs, + &cs_tlv)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad commit_sig %s", tal_hex(msg, msg)); + + /* Default batch_size is 1 */ + batch_size = 1; + if (cs_tlv->splice_info && cs_tlv->splice_info->batch_size) + batch_size = cs_tlv->splice_info->batch_size; + + msg_batch = tal_arr(tmpctx, const u8*, batch_size); + msg_batch[0] = msg; + status_info("msg_batch[0]: %p", msg_batch[0]); + + /* Already received commitment signed once, so start at i = 1 */ + for (u16 i = 1; i < batch_size; i++) { + struct tlv_commitment_signed_tlvs *sub_cs_tlv + = tlv_commitment_signed_tlvs_new(tmpctx); + u8 *sub_msg = peer_read(tmpctx, peer->pps); + check_tx_abort(peer, sub_msg); + + /* Check type for cleaner failure message */ + type = fromwire_peektype(sub_msg); + if (type != WIRE_COMMITMENT_SIGNED) + peer_failed_err(peer->pps, &peer->channel_id, + "Expected splice related " + "WIRE_COMMITMENT_SIGNED but got %s", + peer_wire_name(type)); + status_info("fromwire_commitment_signed(%p) splice index %d", sub_msg, (int)i); + if (!fromwire_commitment_signed(tmpctx, sub_msg, + &channel_id, &commit_sig.s, + &raw_sigs, &sub_cs_tlv)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad commit_sig %s" + " in commit_sig batch:" + " [%"PRIu16"/%"PRIu16"]", + tal_hex(sub_msg, sub_msg), i, batch_size); + + if (!sub_cs_tlv->splice_info + || sub_cs_tlv->splice_info->batch_size != batch_size) + peer_failed_err(peer->pps, &peer->channel_id, + "batch_size value mismatch in" + " commit_sig bundle, item [%"PRIu16 + "/%"PRIu16"] %s", i, batch_size, + tal_hex(sub_msg, sub_msg)); + + msg_batch[i] = sub_msg; + status_info("msg_batch[%d]: %p", (int)i, msg_batch[i]); + } + + status_info("Sorting the msg_batch of tal_count %d, batch_size: %d", (int)tal_count(msg_batch), (int)batch_size); + asort(msg_batch, tal_count(msg_batch), commit_cmp, peer); + + return handle_peer_commit_sig(peer, msg, commit_index, remote_funding, + changed_htlcs, splice_amnt, + remote_splice_amnt, local_index, + local_per_commit, allow_empty_commit, + msg_batch); +} + /* Pops the penalty base for the given commitnum from our internal list. There * may not be one, in which case we return NULL and leave the list * unmodified. */ @@ -2847,7 +2978,8 @@ static struct commitsig *interactive_send_commitments(struct peer *peer, remote_splice_amnt, next_index_local - 1, &my_current_per_commitment_point, - true); + true, + NULL); } } @@ -4571,10 +4703,12 @@ static void peer_in(struct peer *peer, const u8 *msg) handle_peer_add_htlc(peer, msg); return; case WIRE_COMMITMENT_SIGNED: - handle_peer_commit_sig(peer, msg, 0, - peer->channel->funding_pubkey[REMOTE], - NULL, 0, 0, peer->next_index[LOCAL], - &peer->next_local_per_commit, false); + handle_peer_commit_sig_batch(peer, msg, 0, + peer->channel->funding_pubkey[REMOTE], + NULL, 0, 0, + peer->next_index[LOCAL], + &peer->next_local_per_commit, + false); return; case WIRE_UPDATE_FEE: handle_peer_feechange(peer, msg); From 3fac5650776d99561c9fe1e2c691afd60203815e Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 17 Apr 2025 17:06:27 -0400 Subject: [PATCH 18/35] splice: Update to Eclair style of reestablish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update to use Eclair’s spec’d version of reestablish. Changelog-None --- channeld/channeld.c | 71 ++++++++++++++++------ tests/fuzz/fuzz-wire-channel_reestablish.c | 10 +++ tests/test_connection.py | 3 + wire/peer_wire.csv | 14 ++--- 4 files changed, 68 insertions(+), 30 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index c542e9f8bea5..be1767e9da11 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -51,6 +51,8 @@ #include #include +#define EXPERIMENTAL_UPGRADE_ENABLED 0 + /* stdin == requests, 3 == peer, 4 = HSM */ #define MASTER_FD STDIN_FILENO #define HSM_FD 4 @@ -356,6 +358,7 @@ static bool handle_master_request_later(struct peer *peer, const u8 *msg) return false; } +#if EXPERIMENTAL_UPGRADE_ENABLED /* Compare, with false if either is NULL */ static bool match_type(const u8 *t1, const u8 *t2) { @@ -385,6 +388,7 @@ static void set_channel_type(struct channel *channel, const u8 *type) wire_sync_write(MASTER_FD, take(towire_channeld_upgraded(NULL, channel->type))); } +#endif static void lock_signer_outpoint(const struct bitcoin_outpoint *outpoint) { @@ -5165,6 +5169,7 @@ static bool capture_premature_msg(const u8 ***shit_lnd_says, const u8 *msg) return true; } +#if EXPERIMENTAL_UPGRADE_ENABLED /* Unwrap a channel_type into a raw byte array for the wire: can be NULL */ static u8 *to_bytearr(const tal_t *ctx, const struct channel_type *channel_type TAKES) @@ -5183,6 +5188,7 @@ static u8 *to_bytearr(const tal_t *ctx, ret = tal_dup_talarr(ctx, u8, channel_type->features); return ret; } +#endif static void peer_reconnect(struct peer *peer, const struct secret *last_remote_per_commit_secret) @@ -5219,6 +5225,7 @@ static void peer_reconnect(struct peer *peer, send_tlvs = NULL; +#if EXPERIMENTAL_UPGRADE_ENABLED if (peer->experimental_upgrade) { /* Subtle: we free tmpctx below as we loop, so tal off peer */ send_tlvs = tlv_channel_reestablish_tlvs_new(peer); @@ -5257,6 +5264,7 @@ static void peer_reconnect(struct peer *peer, peer->channel))); } } +#endif inflight = last_inflight(peer); @@ -5284,6 +5292,34 @@ static void peer_reconnect(struct peer *peer, } } + if (feature_negotiated(peer->our_features, peer->their_features, + OPT_SPLICE)) { + if (!send_tlvs) { + /* Subtle: we free tmpctx below as we loop, so + * tal off 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; + + 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)); + } + } + } + /* BOLT #2: * * - upon reconnection: @@ -5396,26 +5432,6 @@ 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, - &inflight->outpoint.txid))); - 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 */ @@ -5470,6 +5486,19 @@ static void peer_reconnect(struct peer *peer, } } + /* 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; + + peer_write(peer->pps, + take(towire_splice_locked(NULL, + &peer->channel_id, + &itr->outpoint.txid))); + peer->splice_state->locked_ready[LOCAL] = true; + } + /* BOLT #2: * * - if `next_commitment_number` is 1 in both the @@ -5634,6 +5663,7 @@ static void peer_reconnect(struct peer *peer, /* (If we had sent `closing_signed`, we'd be in closingd). */ maybe_send_shutdown(peer); +#if EXPERIMENTAL_UPGRADE_ENABLED if (recv_tlvs->desired_channel_type) status_debug("They sent desired_channel_type [%s]", fmt_featurebits(tmpctx, @@ -5711,6 +5741,7 @@ static void peer_reconnect(struct peer *peer, if (type) set_channel_type(peer->channel, type); } +#endif tal_free(send_tlvs); diff --git a/tests/fuzz/fuzz-wire-channel_reestablish.c b/tests/fuzz/fuzz-wire-channel_reestablish.c index 29d9dc5bce0a..85a6a7975c3c 100644 --- a/tests/fuzz/fuzz-wire-channel_reestablish.c +++ b/tests/fuzz/fuzz-wire-channel_reestablish.c @@ -6,6 +6,8 @@ #include #include +#define EXPERIMENTAL_UPGRADE_ENABLED 0 + struct channel_reestablish { struct channel_id channel_id; u64 next_commitment_number; @@ -46,6 +48,11 @@ 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)) + return false; +#if EXPERIMENTAL_UPGRADE_ENABLED if (!tal_arr_eq(x->tlvs->next_to_send, y->tlvs->next_to_send)) return false; if (!tal_arr_eq(x->tlvs->desired_channel_type, y->tlvs->desired_channel_type)) @@ -54,6 +61,9 @@ static bool equal(const struct channel_reestablish *x, return false; return tal_arr_eq(x->tlvs->upgradable_channel_type, y->tlvs->upgradable_channel_type); +#else + return true; +#endif } void run(const u8 *data, size_t size) diff --git a/tests/test_connection.py b/tests/test_connection.py index 1118bea512ae..83b3ffc62d36 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -3745,6 +3745,7 @@ def test_openchannel_init_alternate(node_factory, executor): print("nothing to do") +@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields") def test_upgrade_statickey(node_factory, executor): """l1 doesn't have option_static_remotekey, l2 offers it.""" l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True, @@ -3783,6 +3784,7 @@ def test_upgrade_statickey(node_factory, executor): l2.daemon.wait_for_log(r"They sent desired_channel_type \[12\]") +@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields") def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind): """We test penalty before/after, and unilateral before/after""" l1, l2 = node_factory.get_nodes(2, opts=[{'may_reconnect': True, @@ -3940,6 +3942,7 @@ def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind): wait_for(lambda: len(l2.rpc.listpeerchannels()['channels']) == 0) +@unittest.skip("experimental-upgrade-protocol TLV fields conflict with splicing TLV fields") def test_upgrade_statickey_fail(node_factory, executor, bitcoind): """We reconnect at all points during retransmit, and we won't upgrade.""" l1_disconnects = ['-WIRE_COMMITMENT_SIGNED', diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index f3ef243bc636..418fa78534e8 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -320,16 +320,10 @@ 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 tlvdata,channel_reestablish_tlvs,next_funding,next_funding_txid,sha256, -tlvtype,channel_reestablish_tlvs,next_funding,0 -tlvdata,channel_reestablish_tlvs,next_funding,next_funding_txid,sha256, -tlvtype,channel_reestablish_tlvs,next_to_send,1 -tlvdata,channel_reestablish_tlvs,next_to_send,commitment_number,tu64, -tlvtype,channel_reestablish_tlvs,desired_channel_type,3 -tlvdata,channel_reestablish_tlvs,desired_channel_type,type,byte,... -tlvtype,channel_reestablish_tlvs,current_channel_type,5 -tlvdata,channel_reestablish_tlvs,current_channel_type,type,byte,... -tlvtype,channel_reestablish_tlvs,upgradable_channel_type,7 -tlvdata,channel_reestablish_tlvs,upgradable_channel_type,type,byte,... +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, msgtype,announcement_signatures,259 msgdata,announcement_signatures,channel_id,channel_id, msgdata,announcement_signatures,short_channel_id,short_channel_id, From 13f8d155ceb1e2deca3ca84f05ce1893a0f046d6 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Tue, 29 Apr 2025 17:59:51 -0400 Subject: [PATCH 19/35] splice: Decrement `next_commitment_number` for Eclair Eclair requires `next_commitment_number` to be decremented to resend the individual splice commitment_signed message. --- channeld/channeld.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index be1767e9da11..764d0788e5a7 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5207,6 +5207,7 @@ static void peer_reconnect(struct peer *peer, const u8 **premature_msgs = tal_arr(peer, const u8 *, 0); struct inflight *inflight; struct bitcoin_txid *local_next_funding, *remote_next_funding; + u64 send_next_commitment_number; struct tlv_channel_reestablish_tlvs *send_tlvs, *recv_tlvs; @@ -5268,6 +5269,7 @@ static void peer_reconnect(struct peer *peer, inflight = last_inflight(peer); + send_next_commitment_number = peer->next_index[LOCAL]; if (inflight && (!inflight->last_tx || !inflight->remote_tx_sigs)) { if (missing_user_signatures(peer, inflight->i_am_initiator @@ -5289,6 +5291,12 @@ static void peer_reconnect(struct peer *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_next_commitment_number--; } } @@ -5346,7 +5354,7 @@ static void peer_reconnect(struct peer *peer, if (channel_has(peer->channel, OPT_STATIC_REMOTEKEY)) { msg = towire_channel_reestablish (NULL, &peer->channel_id, - peer->next_index[LOCAL], + send_next_commitment_number, peer->revocations_received, last_remote_per_commit_secret, /* Can send any (valid) point here */ @@ -5362,7 +5370,7 @@ static void peer_reconnect(struct peer *peer, */ msg = towire_channel_reestablish (NULL, &peer->channel_id, - peer->next_index[LOCAL], + send_next_commitment_number, peer->revocations_received, last_remote_per_commit_secret, &my_current_per_commitment_point, From 25e6f81cdcd1887c73d0fb0c4e3dfc32f1f9062c Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Wed, 30 Apr 2025 17:35:24 -0400 Subject: [PATCH 20/35] splice: Only send or recv commit sig if needed Be more conservative about when we request of send commit sig for splice to match the Eclair behavior. --- channeld/channeld.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 764d0788e5a7..0c6c10c03b74 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5296,7 +5296,8 @@ static void peer_reconnect(struct peer *peer, * indicate that we would like them to re-send * commitment signatures */ /* DTODO: Add bolt reference */ - send_next_commitment_number--; + if (!inflight->last_tx) + send_next_commitment_number--; } } @@ -5437,7 +5438,7 @@ static void peer_reconnect(struct peer *peer, status_info("Resuming splice negotation."); resume_splice_negotiation(peer, false, - true, + !inflight->last_tx, false, true); } else if (bitcoin_txid_eq(remote_next_funding, @@ -5450,8 +5451,8 @@ static void peer_reconnect(struct peer *peer, if (local_next_funding) assume_stfu_mode(peer); resume_splice_negotiation(peer, - true, - local_next_funding, + next_commitment_number == peer->next_index[REMOTE] - 1, + local_next_funding && !inflight->last_tx, true, local_next_funding); } else if (bitcoin_txid_eq(remote_next_funding, @@ -5615,7 +5616,10 @@ static void peer_reconnect(struct peer *peer, PRIu64, next_commitment_number); - retransmit_commitment_signed = true; + if (!recv_tlvs || !recv_tlvs->next_funding) + retransmit_commitment_signed = true; + else + retransmit_commitment_signed = false; /* BOLT #2: * From 3e6c57b56364e2e4063008b452d30140476693fc Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 1 May 2025 12:52:42 -0400 Subject: [PATCH 21/35] splice: Enable the receiving of splice RBF Turn on the receiving of splice RBF attempts. --- channeld/channeld.c | 76 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 0c6c10c03b74..c60f31340d9d 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -60,6 +60,7 @@ #define VALID_STFU_MESSAGE(msg) \ ((msg) == WIRE_SPLICE || \ (msg) == WIRE_SPLICE_ACK || \ + (msg) == WIRE_TX_INIT_RBF || \ (msg) == WIRE_TX_ABORT) #define SAT_MIN(a, b) (amount_sat_less((a), (b)) ? (a) : (b)) @@ -3898,6 +3899,9 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) struct amount_msat current_push_val; const enum tx_role our_role = TX_ACCEPTER; u8 *abort_msg; + enum peer_wire type; + struct tlv_tx_init_rbf_tlvs *init_rbf_tlvs; + struct tlv_tx_ack_rbf_tlvs *ack_rbf_tlvs; /* Can't start a splice with another splice still active */ assert(!peer->splicing); @@ -3906,14 +3910,46 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) ictx = new_interactivetx_context(tmpctx, our_role, peer->pps, peer->channel_id); - if (!fromwire_splice(inmsg, - &channel_id, - &peer->splicing->opener_relative, - &funding_feerate_perkw, - &locktime, - &peer->splicing->remote_funding_pubkey)) - peer_failed_warn(peer->pps, &peer->channel_id, - "Bad wire_splice %s", tal_hex(tmpctx, inmsg)); + type = fromwire_peektype(inmsg); + + if (type == WIRE_SPLICE) { + if (!fromwire_splice(inmsg, + &channel_id, + &peer->splicing->opener_relative, + &funding_feerate_perkw, + &locktime, + &peer->splicing->remote_funding_pubkey)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad wire_splice %s", + tal_hex(tmpctx, inmsg)); + if (last_inflight(peer)) { + peer_failed_warn(peer->pps, &peer->channel_id, + "Can't splice_init because we already" + " have a pending splice (did you mean" + " to splice RBF?)"); + } + } + else if (type == WIRE_TX_INIT_RBF) { + if (!fromwire_tx_init_rbf(tmpctx, inmsg, + &channel_id, + &locktime, + &funding_feerate_perkw, + &init_rbf_tlvs)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad tx_init_rbf %s", + tal_hex(tmpctx, inmsg)); + if (!init_rbf_tlvs + || !init_rbf_tlvs->funding_output_contribution) + peer_failed_warn(peer->pps, &peer->channel_id, + "tx_init_rbf must contain tlv with a" + " funding_output_contribution value"); + peer->splicing->opener_relative = *init_rbf_tlvs->funding_output_contribution; + if (!last_inflight(peer)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Can't tx_init_rbf because we have no" + " pending splice"); + peer->splicing->remote_funding_pubkey = last_inflight(peer)->remote_funding; + } peer->splice_state->await_commitment_succcess = false; @@ -3936,10 +3972,24 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) /* TODO: Add plugin hook for user to adjust accepter amount */ peer->splicing->accepter_relative = 0; - msg = towire_splice_ack(NULL, - &peer->channel_id, - peer->splicing->accepter_relative, - &peer->channel->funding_pubkey[LOCAL]); + if (type == WIRE_SPLICE) { + msg = towire_splice_ack(NULL, + &peer->channel_id, + peer->splicing->accepter_relative, + &peer->channel->funding_pubkey[LOCAL]); + } else if (type == WIRE_TX_INIT_RBF) { + ack_rbf_tlvs = tlv_tx_ack_rbf_tlvs_new(tmpctx); + ack_rbf_tlvs->funding_output_contribution = tal(ack_rbf_tlvs, s64); + *ack_rbf_tlvs->funding_output_contribution = 0; + ack_rbf_tlvs->require_confirmed_inputs = NULL; + msg = towire_tx_ack_rbf(NULL, + &peer->channel_id, + ack_rbf_tlvs); + } else { + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "message type unsupported"); + msg = NULL; /* Squelch unused warning */ + } peer->splicing->mode = true; @@ -4739,6 +4789,7 @@ static void peer_in(struct peer *peer, const u8 *msg) handle_stfu(peer, msg); return; case WIRE_SPLICE: + case WIRE_TX_INIT_RBF: splice_accepter(peer, msg); return; case WIRE_SPLICE_ACK: @@ -4768,7 +4819,6 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_TX_SIGNATURES: handle_unexpected_tx_sigs(peer, msg); return; - case WIRE_TX_INIT_RBF: case WIRE_TX_ACK_RBF: break; From ae7aeecf15519fb6c91d1a5012efaa1fbbd29952 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 1 May 2025 16:33:22 -0400 Subject: [PATCH 22/35] splice: Splice script should not abort on sign MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The result of splice_signed can fail for many reasons that are non-critical (already in mempool for instance). Don’t abort channels in this case as that causes a force close. --- plugins/spender/splice.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/plugins/spender/splice.c b/plugins/spender/splice.c index f6963e5e6dca..1c8ce72fa9ff 100644 --- a/plugins/spender/splice.c +++ b/plugins/spender/splice.c @@ -730,6 +730,26 @@ static struct command_result *splice_signed_get_result(struct command *cmd, return continue_splice(splice_cmd->cmd, splice_cmd); } +static struct command_result *splice_signed_error_pkg(struct command *cmd, + const char *methodname, + const char *buf, + const jsmntok_t *error, + struct splice_index_pkg *pkg) +{ + struct splice_cmd *splice_cmd = pkg->splice_cmd; + struct abort_pkg *abort_pkg; + + splice_cmd->wetrun = false; + + abort_pkg = tal(cmd->plugin, struct abort_pkg); + abort_pkg->splice_cmd = tal_steal(abort_pkg, pkg->splice_cmd); + abort_pkg->str = tal_strndup(abort_pkg, buf + error->start, + error->end - error->start); + abort_pkg->code = -1; + + return make_error(cmd, abort_pkg, "splice_signed_error"); +} + static struct command_result *splice_signed(struct command *cmd, struct splice_cmd *splice_cmd, size_t index) @@ -743,7 +763,8 @@ static struct command_result *splice_signed(struct command *cmd, pkg->index = index; req = jsonrpc_request_start(cmd, "splice_signed", - splice_signed_get_result, splice_error_pkg, + splice_signed_get_result, + splice_signed_error_pkg, pkg); json_add_channel_id(req->js, "channel_id", action->channel_id); From 9d9d6bc2df6c1b99857e1f21f7c701767ab3e4e4 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 1 May 2025 16:35:21 -0400 Subject: [PATCH 23/35] splice: Enable user splice RBF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow user’s to RBF existing splices. For now this is done by simple executing an additional splice command, in the future this will can also be done with dedicated RPCs. Changelog-Added: Enabled the ability to RBF splices --- channeld/channeld.c | 79 ++++++++++++++++++++++++++++-------- lightningd/channel_control.c | 7 ++-- 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index c60f31340d9d..a39f00e722ef 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -61,6 +61,7 @@ ((msg) == WIRE_SPLICE || \ (msg) == WIRE_SPLICE_ACK || \ (msg) == WIRE_TX_INIT_RBF || \ + (msg) == WIRE_TX_ACK_RBF || \ (msg) == WIRE_TX_ABORT) #define SAT_MIN(a, b) (amount_sat_less((a), (b)) ? (a) : (b)) @@ -4097,14 +4098,41 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg) struct wally_psbt *psbt = peer->splicing->current_psbt; u32 sequence = 0; u8 *scriptPubkey; + enum peer_wire type; + struct tlv_tx_ack_rbf_tlvs *ack_rbf_tlvs; - if (!fromwire_splice_ack(inmsg, - &channel_id, - &peer->splicing->accepter_relative, - &peer->splicing->remote_funding_pubkey)) - peer_failed_warn(peer->pps, &peer->channel_id, - "Bad wire_splice_ack %s", - tal_hex(tmpctx, inmsg)); + type = fromwire_peektype(inmsg); + + if (type == WIRE_SPLICE_ACK) { + if (!fromwire_splice_ack(inmsg, + &channel_id, + &peer->splicing->accepter_relative, + &peer->splicing->remote_funding_pubkey)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad wire_splice_ack %s", + tal_hex(tmpctx, inmsg)); + } else if (type == WIRE_TX_ACK_RBF) { + if (!fromwire_tx_ack_rbf(tmpctx, + inmsg, + &channel_id, + &ack_rbf_tlvs)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Bad tx_ack_rbf %s", + tal_hex(tmpctx, inmsg)); + + if (!ack_rbf_tlvs + || !ack_rbf_tlvs->funding_output_contribution) + peer_failed_warn(peer->pps, &peer->channel_id, + "tx_ack_rbf must contain tlv with a" + " funding_output_contribution value"); + peer->splicing->accepter_relative = *ack_rbf_tlvs->funding_output_contribution; + + if (!last_inflight(peer)) + peer_failed_err(peer->pps, &peer->channel_id, + "Can't handle tx_ack_rbf because we" + " have no pending splice"); + peer->splicing->remote_funding_pubkey = last_inflight(peer)->remote_funding; + } if (!channel_id_eq(&channel_id, &peer->channel_id)) peer_failed_warn(peer->pps, &peer->channel_id, @@ -4538,12 +4566,29 @@ static void splice_initiator_user_signed(struct peer *peer, const u8 *inmsg) /* This occurs once our 'stfu' transition was successful. */ static void handle_splice_stfu_success(struct peer *peer) { - u8 *msg = towire_splice(tmpctx, - &peer->channel_id, - peer->splicing->opener_relative, - peer->splicing->feerate_per_kw, - peer->splicing->current_psbt->fallback_locktime, - &peer->channel->funding_pubkey[LOCAL]); + u8 *msg; + struct tlv_tx_init_rbf_tlvs *init_rbf_tlvs; + if (!last_inflight(peer)) { + msg = towire_splice(tmpctx, + &peer->channel_id, + peer->splicing->opener_relative, + peer->splicing->feerate_per_kw, + peer->splicing->current_psbt->fallback_locktime, + &peer->channel->funding_pubkey[LOCAL]); + } + else { /* RBF attempt */ + init_rbf_tlvs = tlv_tx_init_rbf_tlvs_new(tmpctx); + init_rbf_tlvs->funding_output_contribution = tal(init_rbf_tlvs, s64); + *init_rbf_tlvs->funding_output_contribution = peer->splicing->opener_relative; + init_rbf_tlvs->require_confirmed_inputs = NULL; + + msg = towire_tx_init_rbf(tmpctx, + &peer->channel_id, + peer->splicing->current_psbt->fallback_locktime, + peer->splicing->feerate_per_kw, + init_rbf_tlvs); + } + peer->splice_state->await_commitment_succcess = false; peer_write(peer->pps, take(msg)); } @@ -4617,8 +4662,9 @@ static void handle_splice_init(struct peer *peer, const u8 *inmsg) return; } - status_debug("Getting handle_splice_init psbt version %d", - peer->splicing->current_psbt->version); + status_debug("Getting handle_splice_init psbt version %d (RBF?: %s)", + peer->splicing->current_psbt->version, + last_inflight(peer) ? "y" : "n"); if (skip_stfu) { handle_splice_stfu_success(peer); @@ -4793,6 +4839,7 @@ static void peer_in(struct peer *peer, const u8 *msg) splice_accepter(peer, msg); return; case WIRE_SPLICE_ACK: + case WIRE_TX_ACK_RBF: splice_initiator(peer, msg); return; case WIRE_SPLICE_LOCKED: @@ -4819,8 +4866,6 @@ static void peer_in(struct peer *peer, const u8 *msg) case WIRE_TX_SIGNATURES: handle_unexpected_tx_sigs(peer, msg); return; - case WIRE_TX_ACK_RBF: - break; case WIRE_CHANNEL_REESTABLISH: handle_unexpected_reestablish(peer, msg); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index e7923f5dc52b..ffd3fd4f1055 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -2204,11 +2204,12 @@ static struct command_result *channel_for_splice(struct command *cmd, "abnormal owner state %s", (*channel)->owner->name); - if ((*channel)->state != CHANNELD_NORMAL) + if ((*channel)->state != CHANNELD_NORMAL + && (*channel)->state != CHANNELD_AWAITING_SPLICE) return command_fail(cmd, SPLICE_INVALID_CHANNEL_STATE, - "Channel needs to be in normal state but " - "is in state %s", + "Channel needs to be in normal or awaiting" + " splice state but is in state %s", channel_state_name(*channel)); return NULL; From b25079000fa1d31310981dfe0c2c23b4a3b5e943 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 1 May 2025 17:36:08 -0400 Subject: [PATCH 24/35] splice: Add verbose log messages for new reestablish TLV --- channeld/channeld.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/channeld/channeld.c b/channeld/channeld.c index a39f00e722ef..f87cb1ce8c81 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5408,6 +5408,10 @@ static void peer_reconnect(struct peer *peer, 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]; @@ -5424,6 +5428,25 @@ static void peer_reconnect(struct peer *peer, } } + 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) + : "NULL", + send_tlvs && send_tlvs->my_current_funding_locked_txid + ? fmt_bitcoin_txid(tmpctx, + send_tlvs->my_current_funding_locked_txid) + : "NULL", + send_next_commitment_number); + /* BOLT #2: * * - upon reconnection: From cd03fdc7e9e0b996cbb46a52155eca908f032385 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 2 May 2025 13:36:52 -0400 Subject: [PATCH 25/35] gossip: Limit announcement sigs reply A splice where reestablish happens at the right moment causes an infinite loop of announcement signatures being sent back and forth. Limit the announcement sigs we send in response to announcement sigs to once per channel session. ChangelogNone --- lightningd/channel.c | 2 ++ lightningd/channel.h | 3 +++ lightningd/channel_gossip.c | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 94ff06c7b842..6d27be037f21 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -322,6 +322,7 @@ struct channel *new_unsaved_channel(struct peer *peer, /* Nothing happened yet */ memset(&channel->stats, 0, sizeof(channel->stats)); channel->state_changes = tal_arr(channel, struct channel_state_change *, 0); + channel->replied_to_announcement_sigs = false; /* No shachain yet */ channel->their_shachain.id = 0; @@ -632,6 +633,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->num_onchain_spent_calls = 0; channel->stats = *stats; channel->state_changes = tal_steal(channel, state_changes); + channel->replied_to_announcement_sigs = false; /* Populate channel->channel_gossip */ channel_gossip_init(channel, take(peer_update)); diff --git a/lightningd/channel.h b/lightningd/channel.h index 94de5c8b81be..53c28be3016e 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -353,6 +353,9 @@ struct channel { /* Our change history. */ struct channel_state_change **state_changes; + + /* Have we replied to announcement_signatures once? */ + bool replied_to_announcement_sigs; }; /* Is channel owned (and should be talking to peer) */ diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c index 76a503c9875d..0cfb8935ca41 100644 --- a/lightningd/channel_gossip.c +++ b/lightningd/channel_gossip.c @@ -688,7 +688,10 @@ void channel_gossip_got_announcement_sigs(struct channel *channel, * - MUST respond with its own `announcement_signatures` * message. */ - send_channel_announce_sigs(channel); + if(!channel->replied_to_announcement_sigs) { + send_channel_announce_sigs(channel); + channel->replied_to_announcement_sigs = true; + } check_channel_gossip(channel); return; } From 1b0eda4b5e539274e8314e0d6293c7f77bd1c707 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Fri, 2 May 2025 18:48:42 -0400 Subject: [PATCH 26/35] splice: Update test for new reestablish behavior New reestablish behvaior allows splices to resume on reestablish if only commit sigs were exchanged. Update the `test_commit_crash_splice` to reflect this (and turn it back on). --- tests/test_splicing.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index 575bcfac366a..cf7b33d6dbc5 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -269,7 +269,6 @@ def test_invalid_splice(node_factory, bitcoind): assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0 -@unittest.skip("Test is flaky causing CI to be unusable.") @pytest.mark.openchannel('v1') @pytest.mark.openchannel('v2') @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @@ -293,23 +292,18 @@ def test_commit_crash_splice(node_factory, bitcoind): # The splicing inflight should have been left pending in the DB assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 1 - l1.daemon.wait_for_log(r'Restarting channeld after tx_abort on CHANNELD_NORMAL channel') + l1.daemon.wait_for_log(r'peer_out WIRE_CHANNEL_REESTABLISH') + l1.daemon.wait_for_log(r'Got reestablish commit=1 revoke=0 inflights: 1, active splices: 1') + l1.daemon.wait_for_log(r'Splice resume check with local_next_funding: sent, remote_next_funding: received, inflights: 1') + l1.daemon.wait_for_log(r'Splice negotation, will not send commit, not recv commit, send signature, recv signature as initiator') assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 1 - result = l1.rpc.splice_init(chan_id, -105000, l1.rpc.addpsbtoutput(100000)['psbt']) - result = l1.rpc.splice_update(chan_id, result['psbt']) - assert(result['commitments_secured'] is False) - result = l1.rpc.splice_update(chan_id, result['psbt']) - assert(result['commitments_secured'] is True) - result = l1.rpc.splice_signed(chan_id, result['psbt']) - l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') mempool = bitcoind.rpc.getrawmempool(True) assert len(list(mempool.keys())) == 1 - assert result['txid'] in list(mempool.keys()) bitcoind.generate_block(6, wait_for_mempool=1) From 899433223ba34637e4c39f4451826eb65cbdbec0 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 11:50:11 -0400 Subject: [PATCH 27/35] splice: Add support for `tx_abort` during RBF We do this by adding a specific txid the tx_abort applies to and performing checks based on that. If the txid is NULL or unrecognized than no inflights are dropped from DB. If we recognize it than we do the check to see if we signed it and, if not, we let lightningd remove it from DB. --- channeld/channeld.c | 53 +++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index f87cb1ce8c81..a6407648c7f7 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1777,10 +1777,11 @@ static bool missing_user_signatures(const struct peer *peer, return false; } -static void check_tx_abort(struct peer *peer, const u8 *msg) +/* `txid` is which tx to abort (aka remove inflight). NULL means just restart + * channeld and dont abort any specific inflight. */ +static void check_tx_abort(struct peer *peer, const u8 *msg, struct bitcoin_txid *txid) { - struct inflight *inflight = last_inflight(peer); - struct bitcoin_outpoint *outpoint; + struct inflight *inflight; struct channel_id channel_id; u8 *data; char *reason; @@ -1788,11 +1789,20 @@ static void check_tx_abort(struct peer *peer, const u8 *msg) if (fromwire_peektype(msg) != WIRE_TX_ABORT) return; - if (have_i_signed_inflight(peer, inflight)) { - peer_failed_err(peer->pps, &peer->channel_id, "tx_abort" - " is not allowed after I have sent my" - " signature. msg: %s", - tal_hex(tmpctx, msg)); + inflight = NULL; + for (size_t i = 0; txid && i < tal_count(peer->splice_state->inflights); i++) { + struct inflight *itr = peer->splice_state->inflights[i]; + if (!bitcoin_txid_eq(&itr->outpoint.txid, txid)) + continue; + if (have_i_signed_inflight(peer, inflight)) { + peer_failed_err(peer->pps, &peer->channel_id, "tx_abort" + " is not allowed after I have sent my" + " signature. msg: %s txid: %s", + tal_hex(tmpctx, msg), + fmt_bitcoin_txid(tmpctx, + &itr->outpoint.txid)); + } + inflight = itr; } if (!fromwire_tx_abort(tmpctx, msg, &channel_id, &data)) @@ -1804,10 +1814,6 @@ static void check_tx_abort(struct peer *peer, const u8 *msg) peer_write(peer->pps, take(towire_tx_abort(NULL, &peer->channel_id, NULL))); - outpoint = NULL; - if (inflight) - outpoint = &inflight->outpoint; - status_info("Send tx_abort to master"); reason = sanitize_error(tmpctx, msg, &peer->channel_id); @@ -1816,7 +1822,9 @@ static void check_tx_abort(struct peer *peer, const u8 *msg) wire_sync_write(MASTER_FD, take(towire_channeld_splice_abort(NULL, false, - outpoint, + inflight + ? &inflight->outpoint + : NULL, tal_fmt(tmpctx, "Peer aborted" " for reason: %s", @@ -2297,7 +2305,7 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, struct tlv_commitment_signed_tlvs *sub_cs_tlv = tlv_commitment_signed_tlvs_new(tmpctx); u8 *sub_msg = peer_read(tmpctx, peer->pps); - check_tx_abort(peer, sub_msg); + check_tx_abort(peer, sub_msg, NULL); /* Check type for cleaner failure message */ type = fromwire_peektype(sub_msg); @@ -2960,7 +2968,7 @@ static struct commitsig *interactive_send_commitments(struct peer *peer, WIRE_TX_SIGNATURES, WIRE_TX_ABORT); - check_tx_abort(peer, msg); + check_tx_abort(peer, msg, &inflight->outpoint.txid); if (msg_received) *msg_received = msg; @@ -3538,7 +3546,7 @@ static void resume_splice_negotiation(struct peer *peer, recv_commitments, &msg_received); - check_tx_abort(peer, msg_received); + check_tx_abort(peer, msg_received, &inflight->outpoint.txid); if (their_commit) { if (inflight->last_tx != their_commit->tx) @@ -3654,7 +3662,7 @@ static void resume_splice_negotiation(struct peer *peer, type = fromwire_peektype(msg); - check_tx_abort(peer, msg); + check_tx_abort(peer, msg, &inflight->outpoint.txid); if (handle_peer_error_or_warning(peer->pps, msg)) return; @@ -4018,7 +4026,7 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg) peer_failed_err(peer->pps, &peer->channel_id, "Interactive splicing error: %s", error); - check_tx_abort(peer, abort_msg); + check_tx_abort(peer, abort_msg, NULL); assert(ictx->pause_when_complete == false); peer->splicing->sent_tx_complete = true; @@ -4270,7 +4278,7 @@ static void splice_initiator_user_finalized(struct peer *peer) peer_failed_warn(peer->pps, &peer->channel_id, "Splice interactivetx error: %s", error); - check_tx_abort(peer, abort_msg); + check_tx_abort(peer, abort_msg, NULL); /* With pause_when_complete fase, this assert should never fail */ assert(peer->splicing->received_tx_complete); @@ -4436,7 +4444,7 @@ static void splice_initiator_user_update(struct peer *peer, const u8 *inmsg) peer_failed_warn(peer->pps, &peer->channel_id, "Splice update error: %s", error); - check_tx_abort(peer, abort_msg); + check_tx_abort(peer, abort_msg, NULL); peer->splicing->tx_add_input_count = ictx->tx_add_input_count; peer->splicing->tx_add_output_count = ictx->tx_add_output_count; @@ -4747,7 +4755,7 @@ static void peer_in(struct peer *peer, const u8 *msg) if (handle_peer_error_or_warning(peer->pps, msg)) return; - check_tx_abort(peer, msg); + check_tx_abort(peer, msg, NULL); /* If we're in STFU mode and aren't waiting for a STFU mode * specific message, the only valid message was tx_abort */ @@ -4846,7 +4854,7 @@ static void peer_in(struct peer *peer, const u8 *msg) handle_peer_splice_locked(peer, msg); return; case WIRE_TX_ABORT: - check_tx_abort(peer, msg); + check_tx_abort(peer, msg, NULL); return; case WIRE_INIT: case WIRE_OPEN_CHANNEL: @@ -6434,7 +6442,6 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNELD_SPLICE_FEERATE_ERROR: case WIRE_CHANNELD_SPLICE_FUNDING_ERROR: case WIRE_CHANNELD_SPLICE_ABORT: - check_tx_abort(peer, msg); case WIRE_CHANNELD_CONFIRMED_STFU: break; case WIRE_CHANNELD_DEV_REENABLE_COMMIT: From ededb7c9f84e208339e1b10e6cb2502c0c908be0 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 12:12:13 -0400 Subject: [PATCH 28/35] splice: Fix txid watch for splice RBF Remove a check that prevents txid watching on splice RBF attempts. --- lightningd/channel_control.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index ffd3fd4f1055..a0c288022d3b 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -753,15 +753,21 @@ static void handle_splice_sending_sigs(struct lightningd *ld, " splice_confirmed_signed txid %s", fmt_bitcoin_txid(tmpctx, &txid)); - /* Signing a splice after it has confirmed is safe and can happen during - * reestablish if one node is late seeing blocks */ - if (channel->state == CHANNELD_AWAITING_SPLICE) - return; + /* 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)); + } cc = splice_command_for_chan(ld, channel); /* If matching user command found, this was a user intiated splice */ channel_set_state(channel, - CHANNELD_NORMAL, + channel->state, CHANNELD_AWAITING_SPLICE, cc ? REASON_USER : REASON_REMOTE, "Splice signatures sent"); From 98f63709b13b34e7d88479be3028dd193e7a47ca Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 13:38:14 -0400 Subject: [PATCH 29/35] splice: Improve log message for handle_peer_commit_sig --- channeld/channeld.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index a6407648c7f7..295f9bf4fa1b 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1942,8 +1942,9 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, remote_funding }; status_debug("handle_peer_commit_sig(splice: %d, remote_splice: %d," - " index: %"PRIu64")", - (int)splice_amnt, (int)remote_splice_amnt, local_index); + " commit_index: %"PRIu32", local_index: %"PRIu64", msg: %p)", + (int)splice_amnt, (int)remote_splice_amnt, commit_index, + local_index, msg); struct tlv_commitment_signed_tlvs *cs_tlv = tlv_commitment_signed_tlvs_new(tmpctx); @@ -2057,8 +2058,9 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, dump_htlcs(peer->channel, "receiving commit_sig"); peer_failed_warn(peer->pps, &peer->channel_id, "Bad commit_sig signature %"PRIu64" %s for tx" - " %s wscript %s key %s feerate %u. Cur funding" - " %s, splice_info: %s, race_await_commit: %s," + " %s wscript %s key %s feerate %u. Outpoint" + " %s, funding_sats: %s, splice_info: %s," + " race_await_commit: %s," " inflight splice count: %zu", local_index, fmt_bitcoin_signature(msg, &commit_sig), @@ -2066,7 +2068,8 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, tal_hex(msg, funding_wscript), fmt_pubkey(msg, &remote_funding), channel_feerate(peer->channel, LOCAL), - fmt_channel_id(tmpctx, &active_id), + fmt_bitcoin_outpoint(tmpctx, &outpoint), + fmt_amount_sat(tmpctx, funding_sats), cs_tlv && cs_tlv->splice_info ? fmt_channel_id(tmpctx, &cs_tlv->splice_info->funding_txid) From 537cfdf8cfe35581c2c7c5690f33de7134f1a33c Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 15:46:50 -0400 Subject: [PATCH 30/35] splice: Changing encoding of TLV funding_txid Using the wrong encoding flips the bytes. We have to use sha256 as the spec says to prevent them from flipping. --- channeld/channeld.c | 19 +++++++++---------- wire/peer_wire.csv | 2 +- wire/test/run-peer-wire.c | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 295f9bf4fa1b..9e7b4e4e305a 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1201,7 +1201,7 @@ static u8 *send_commit_part(const tal_t *ctx, struct tlv_commitment_signed_tlvs_splice_info); cs_tlv->splice_info->batch_size = batch_size; - derive_channel_id(&cs_tlv->splice_info->funding_txid, funding); + cs_tlv->splice_info->funding_txid = funding->txid; } txs = channel_txs(tmpctx, funding, funding_sats, &htlc_map, @@ -1934,7 +1934,6 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, const u8 * msg2; struct bitcoin_outpoint outpoint; struct amount_sat funding_sats; - struct channel_id active_id; const struct commitsig **commitsigs; int remote_anchor_outnum; struct pubkey funding_pubkeys[NUM_SIDES] = @@ -1960,16 +1959,16 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, * ... * - MUST ignore `commitment_signed` messages where `splice_channel_id` * does not match the `channel_id` of the confirmed splice. */ - derive_channel_id(&active_id, &peer->channel->funding); if (peer->splice_state->await_commitment_succcess && !tal_count(peer->splice_state->inflights) && cs_tlv && cs_tlv->splice_info) { - if (!channel_id_eq(&active_id, - &cs_tlv->splice_info->funding_txid)) { + if (!bitcoin_txid_eq(&peer->channel->funding.txid, + &cs_tlv->splice_info->funding_txid)) { status_info("Ignoring stale commit_sig for channel_id" " %s, as %s is locked in now.", - fmt_channel_id(tmpctx, - &cs_tlv->splice_info->funding_txid), - fmt_channel_id(tmpctx, &active_id)); + fmt_bitcoin_txid(tmpctx, + &cs_tlv->splice_info->funding_txid), + fmt_bitcoin_txid(tmpctx, + &peer->channel->funding.txid)); return NULL; } } @@ -2071,8 +2070,8 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, fmt_bitcoin_outpoint(tmpctx, &outpoint), fmt_amount_sat(tmpctx, funding_sats), cs_tlv && cs_tlv->splice_info - ? fmt_channel_id(tmpctx, - &cs_tlv->splice_info->funding_txid) + ? fmt_bitcoin_txid(tmpctx, + &cs_tlv->splice_info->funding_txid) : "N/A", peer->splice_state->await_commitment_succcess ? "yes" : "no", diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index 418fa78534e8..b4b8230aff23 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -301,7 +301,7 @@ msgdata,commitment_signed,htlc_signature,signature,num_htlcs msgdata,commitment_signed,splice_channel_id,commitment_signed_tlvs, tlvtype,commitment_signed_tlvs,splice_info,0 tlvdata,commitment_signed_tlvs,splice_info,batch_size,u16, -tlvdata,commitment_signed_tlvs,splice_info,funding_txid,channel_id, +tlvdata,commitment_signed_tlvs,splice_info,funding_txid,sha256, msgtype,revoke_and_ack,133 msgdata,revoke_and_ack,channel_id,channel_id, msgdata,revoke_and_ack,per_commitment_secret,byte,32 diff --git a/wire/test/run-peer-wire.c b/wire/test/run-peer-wire.c index 8700d201e01b..cbb457ac27bb 100644 --- a/wire/test/run-peer-wire.c +++ b/wire/test/run-peer-wire.c @@ -40,9 +40,9 @@ static void set_scid(struct short_channel_id *scid) memset(scid, 2, sizeof(struct short_channel_id)); } -static void set_cid(struct channel_id *cid) +static void set_bitcoin_txid(struct bitcoin_txid *cid) { - memset(cid, 2, sizeof(struct channel_id)); + memset(cid, 2, sizeof(struct bitcoin_txid)); } /* Size up to field. */ @@ -1028,7 +1028,7 @@ int main(int argc, char *argv[]) cs.tlvs = tlv_commitment_signed_tlvs_new(tmpctx); cs.tlvs->splice_info = tal(ctx, struct tlv_commitment_signed_tlvs_splice_info); cs.tlvs->splice_info->batch_size = 1; - set_cid(&cs.tlvs->splice_info->funding_txid); + set_bitcoin_txid(&cs.tlvs->splice_info->funding_txid); msg = towire_struct_commitment_signed(ctx, &cs); cs2 = fromwire_struct_commitment_signed(ctx, msg); From a5289230c6d25a375d8deb8a1f3f97341d13d75a Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 15:57:54 -0400 Subject: [PATCH 31/35] splice: Add test for splice RBF A test that does an RBF and a few HTLCs at various phases of the splice RBF --- tests/test_splicing.py | 59 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index cf7b33d6dbc5..184c4d447847 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -1,5 +1,4 @@ from fixtures import * # noqa: F401,F403 -from flaky import flaky from pyln.client import RpcError import pytest import unittest @@ -12,7 +11,6 @@ @pytest.mark.openchannel('v1') @pytest.mark.openchannel('v2') @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') -@flaky def test_splice(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=True, opts={'experimental-splicing': None}) @@ -49,6 +47,63 @@ def test_splice(node_factory, bitcoind): assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0 +@pytest.mark.openchannel('v1') +@pytest.mark.openchannel('v2') +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +def test_splice_rbf(node_factory, bitcoind): + l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=True, opts={'experimental-splicing': None}) + + chan_id = l1.get_channel_id(l2) + + funds_result = l1.rpc.addpsbtoutput(100000) + + # Pay with fee by subjtracting 5000 from channel balance + result = l1.rpc.splice_init(chan_id, -105000, funds_result['psbt']) + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is False) + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is True) + result = l1.rpc.splice_signed(chan_id, result['psbt']) + + l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + + mempool = bitcoind.rpc.getrawmempool(True) + assert len(list(mempool.keys())) == 1 + assert result['txid'] in list(mempool.keys()) + + inv = l2.rpc.invoice(10**2, '1', 'no_1') + l1.rpc.pay(inv['bolt11']) + + funds_result = l1.rpc.addpsbtoutput(100000) + + # Pay with fee by subjtracting 5000 from channel balance + result = l1.rpc.splice_init(chan_id, -110000, funds_result['psbt']) + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is False) + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is True) + result = l1.rpc.splice_signed(chan_id, result['psbt']) + + l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_AWAITING_SPLICE') + l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_AWAITING_SPLICE') + + inv = l2.rpc.invoice(10**2, '2', 'no_2') + l1.rpc.pay(inv['bolt11']) + + bitcoind.generate_block(6, wait_for_mempool=1) + + l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + + inv = l2.rpc.invoice(10**2, '3', 'no_3') + l1.rpc.pay(inv['bolt11']) + + # Check that the splice doesn't generate a unilateral close transaction + time.sleep(5) + assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0 + + @pytest.mark.openchannel('v1') @pytest.mark.openchannel('v2') @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') From 27571a80f52b6af2812da850d56ff680e8eb9970 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 17:30:59 -0400 Subject: [PATCH 32/35] splice: Verbose logging for Eclair interop --- channeld/channeld.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 9e7b4e4e305a..f665521416b3 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2253,16 +2253,28 @@ static int commit_cmp(const void *a, const void *n, void *peer) int commit_index_a = commit_index_from_msg(*(u8**)a, peer); int commit_index_n = commit_index_from_msg(*(u8**)n, peer); - if (commit_index_a == commit_index_n) + status_debug("commit_cmp a: %p, n: %p result: %d & %d", + *(u8**)a, *(u8**)n, commit_index_a, commit_index_n); + + if (commit_index_a == commit_index_n) { + status_debug("commit_cmp: return 0"); return 0; + } /* Unrecognized commits go on the end */ - if (commit_index_a == -1) + if (commit_index_a == -1) { + status_debug("commit_cmp: return 1"); return 1; + } - if (commit_index_n == -1) + if (commit_index_n == -1) { + status_debug("commit_cmp: return -1"); return -1; + } + status_debug("commit_cmp: return %d - %d = %d", + commit_index_a, commit_index_n, + commit_index_a - commit_index_n); /* Otherwise we sort by commit_index */ return commit_index_a - commit_index_n; } @@ -2300,7 +2312,7 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, msg_batch = tal_arr(tmpctx, const u8*, batch_size); msg_batch[0] = msg; - status_info("msg_batch[0]: %p", msg_batch[0]); + status_debug("msg_batch[0]: %p", msg_batch[0]); /* Already received commitment signed once, so start at i = 1 */ for (u16 i = 1; i < batch_size; i++) { @@ -2316,7 +2328,7 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, "Expected splice related " "WIRE_COMMITMENT_SIGNED but got %s", peer_wire_name(type)); - status_info("fromwire_commitment_signed(%p) splice index %d", sub_msg, (int)i); + status_debug("fromwire_commitment_signed(%p) splice index %d", sub_msg, (int)i); if (!fromwire_commitment_signed(tmpctx, sub_msg, &channel_id, &commit_sig.s, &raw_sigs, &sub_cs_tlv)) @@ -2335,10 +2347,10 @@ static struct commitsig_info *handle_peer_commit_sig_batch(struct peer *peer, tal_hex(sub_msg, sub_msg)); msg_batch[i] = sub_msg; - status_info("msg_batch[%d]: %p", (int)i, msg_batch[i]); + status_debug("msg_batch[%d]: %p", (int)i, msg_batch[i]); } - status_info("Sorting the msg_batch of tal_count %d, batch_size: %d", (int)tal_count(msg_batch), (int)batch_size); + status_debug("Sorting the msg_batch of tal_count %d, batch_size: %d", (int)tal_count(msg_batch), (int)batch_size); asort(msg_batch, tal_count(msg_batch), commit_cmp, peer); return handle_peer_commit_sig(peer, msg, commit_index, remote_funding, From f7d1ce80fb1560857dece83534a5fd87ded65cdb Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 17:31:38 -0400 Subject: [PATCH 33/35] splice: Message sorting should be using txid parsing Update from old channel_id non-spec version --- channeld/channeld.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index f665521416b3..27106bf63ffd 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2223,7 +2223,7 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, */ static int commit_index_from_msg(const u8 *msg, struct peer *peer) { - struct channel_id funding_id; + struct bitcoin_txid funding_txid; struct channel_id channel_id; struct bitcoin_signature commit_sig; secp256k1_ecdsa_signature *raw_sigs; @@ -2232,18 +2232,18 @@ static int commit_index_from_msg(const u8 *msg, struct peer *peer) fromwire_commitment_signed(tmpctx, msg, &channel_id, &commit_sig.s, &raw_sigs, &cs_tlv); - derive_channel_id(&funding_id, &peer->channel->funding); - if (channel_id_eq(&funding_id, &channel_id)) - return 0; + if (!cs_tlv || !cs_tlv->splice_info) + return -1; - for (int i = 0; i < tal_count(peer->splice_state->inflights); i++) { - struct channel_id splice_id; - derive_channel_id(&splice_id, - &peer->splice_state->inflights[i]->outpoint); + funding_txid = cs_tlv->splice_info->funding_txid; - if (channel_id_eq(&splice_id, &channel_id)) + if (bitcoin_txid_eq(&funding_txid, &peer->channel->funding.txid)) + return 0; + + for (int i = 0; i < tal_count(peer->splice_state->inflights); i++) + if (bitcoin_txid_eq(&funding_txid, + &peer->splice_state->inflights[i]->outpoint.txid)) return i + 1; - } return -1; } From f6134b80d0357115f421d2fda9a1404f3174c598 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 5 May 2025 17:31:57 -0400 Subject: [PATCH 34/35] splice: Add more explicit error messages To make failure casee more clear --- channeld/channeld.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/channeld/channeld.c b/channeld/channeld.c index 27106bf63ffd..29b4719eecb9 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2015,6 +2015,22 @@ static struct commitsig_info *handle_peer_commit_sig(struct peer *peer, if (commit_index) { outpoint = peer->splice_state->inflights[commit_index - 1]->outpoint; funding_sats = peer->splice_state->inflights[commit_index - 1]->amnt; + + if (!cs_tlv || !cs_tlv->splice_info) + peer_failed_err(peer->pps, &peer->channel_id, + "Must set funding_txid for each" + " extra commitment_signed message."); + + status_info("handle_peer_commit_sig for inflight outpoint %s", fmt_bitcoin_txid(tmpctx, &peer->splice_state->inflights[commit_index - 1]->outpoint.txid)); + status_info("handle_peer_commit_sig cs_tlv->splice_info->funding_txid %s", fmt_bitcoin_txid(tmpctx, &cs_tlv->splice_info->funding_txid)); + + if (!bitcoin_txid_eq(&peer->splice_state->inflights[commit_index - 1]->outpoint.txid, + &cs_tlv->splice_info->funding_txid)) + peer_failed_err(peer->pps, &peer->channel_id, + "Expected commit sig message for %s but" + " got %s", + fmt_bitcoin_txid(tmpctx, &peer->splice_state->inflights[commit_index - 1]->outpoint.txid), + fmt_bitcoin_txid(tmpctx, &cs_tlv->splice_info->funding_txid)); } else { outpoint = peer->channel->funding; From 9b74028c0e23b5105e51f5dae3d45da557670d4c Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Mon, 12 May 2025 17:27:00 -0400 Subject: [PATCH 35/35] splice: Wait for mempool in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases the CI is so quick and / or slow that the mempool doesn’t have the transaction by the time we’re looking for it. Move the mempool check into a wait_for. --- tests/test_splicing.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index 184c4d447847..0f1c9359a566 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -30,8 +30,8 @@ def test_splice(node_factory, bitcoind): l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 assert result['txid'] in list(mempool.keys()) bitcoind.generate_block(6, wait_for_mempool=1) @@ -68,8 +68,8 @@ def test_splice_rbf(node_factory, bitcoind): l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 assert result['txid'] in list(mempool.keys()) inv = l2.rpc.invoice(10**2, '1', 'no_1') @@ -249,8 +249,8 @@ def test_splice_out(node_factory, bitcoind): l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 assert result['txid'] in list(mempool.keys()) bitcoind.generate_block(6, wait_for_mempool=1) @@ -307,8 +307,8 @@ def test_invalid_splice(node_factory, bitcoind): result = l1.rpc.signpsbt(result['psbt']) result = l1.rpc.splice_signed(chan_id, result['signed_psbt']) + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 assert result['txid'] in list(mempool.keys()) bitcoind.generate_block(6, wait_for_mempool=1) @@ -357,15 +357,14 @@ def test_commit_crash_splice(node_factory, bitcoind): l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') - mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) bitcoind.generate_block(6, wait_for_mempool=1) l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') - time.sleep(1) + time.sleep(5) assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 0 @@ -407,8 +406,8 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor): l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1) mempool = bitcoind.rpc.getrawmempool(True) - assert len(list(mempool.keys())) == 1 assert result['txid'] in list(mempool.keys()) bitcoind.generate_block(1, wait_for_mempool=1)