From 01a5cfdd29963fa596c53f1f86595cc46d6d30cc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 14:41:13 +0930 Subject: [PATCH 1/7] pytest: test if we correctly route using old scids after splice Spoiler: we don't! Signed-off-by: Rusty Russell --- tests/test_splicing.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index 0f1c9359a566..79d8619aa80f 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -421,3 +421,31 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor): # 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.xfail(strict=True) +def test_route_by_old_scid(node_factory, bitcoind): + l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True, opts={'experimental-splicing': None}) + + # Get pre-splice route. + inv = l3.rpc.invoice(10000000, 'test_route_by_old_scid', 'test_route_by_old_scid') + route = l1.rpc.getroute(l3.info['id'], 10000000, 1)['route'] + + # Do a splice + funds_result = l2.rpc.fundpsbt("109000sat", "slow", 166, excess_as_change=True) + chan_id = l2.get_channel_id(l3) + result = l2.rpc.splice_init(chan_id, 100000, funds_result['psbt']) + result = l2.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is False) + result = l2.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is True) + result = l2.rpc.signpsbt(result['psbt']) + result = l2.rpc.splice_signed(chan_id, result['signed_psbt']) + + wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_AWAITING_SPLICE') + bitcoind.generate_block(6, wait_for_mempool=1) + wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_NORMAL') + + # Now l1 tries to send using old scid: should work + l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret']) + l1.rpc.waitsendpay(inv['payment_hash']) From 21d9f08e2b101b66480007b01f33430bdf22aa72 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 14:42:13 +0930 Subject: [PATCH 2/7] wallet: remove now-gratuitous counters from update statements. When we had to use the number to the db_bind call, these annotations made sense, but since 0bcff1e76d6796e20a26c883ad83bc8fad17efeb (for v23.08) we removed that. So remove all the counters, which are simple overhead if we want to change something. Signed-off-by: Rusty Russell --- wallet/wallet.c | 124 ++++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/wallet/wallet.c b/wallet/wallet.c index 75f3b6fae18a..b2d67fae6772 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1494,15 +1494,15 @@ void wallet_inflight_save(struct wallet *w, * 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 + " funding_psbt=?" + ", funding_tx_remote_sigs_received=?" + ", last_tx=?" + ", last_sig=?" + ", locked_scid=?" " WHERE" - " channel_id=?" // 5 - " AND funding_tx_id=?" // 6 - " AND funding_tx_outnum=?")); // 7 + " channel_id=?" + " AND funding_tx_id=?" + " AND funding_tx_outnum=?")); db_bind_psbt(stmt, inflight->funding_psbt); db_bind_int(stmt, inflight->remote_tx_sigs); if (inflight->last_tx) { @@ -2519,61 +2519,61 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) wallet_channel_config_save(w, &chan->our_config); stmt = db_prepare_v2(w->db, SQL("UPDATE channels SET" - " shachain_remote_id=?," // 0 - " scid=?," // 1 - " full_channel_id=?," // 2 - " state=?," // 3 - " funder=?," // 4 - " channel_flags=?," // 5 - " minimum_depth=?," // 6 - " next_index_local=?," // 7 - " next_index_remote=?," // 8 - " next_htlc_id=?," // 9 - " funding_tx_id=?," // 10 - " funding_tx_outnum=?," // 11 - " funding_satoshi=?," // 12 - " our_funding_satoshi=?," // 13 - " funding_locked_remote=?," // 14 - " push_msatoshi=?," // 15 - " msatoshi_local=?," // 16 + " shachain_remote_id=?," + " scid=?," + " full_channel_id=?," + " state=?," + " funder=?," + " channel_flags=?," + " minimum_depth=?," + " next_index_local=?," + " next_index_remote=?," + " next_htlc_id=?," + " funding_tx_id=?," + " funding_tx_outnum=?," + " funding_satoshi=?," + " our_funding_satoshi=?," + " funding_locked_remote=?," + " push_msatoshi=?," + " msatoshi_local=?," " shutdown_scriptpubkey_remote=?," - " shutdown_keyidx_local=?," // 18 - " channel_config_local=?," // 19 - " last_tx=?, last_sig=?," // 20 + 21 - " last_was_revoke=?," // 22 - " min_possible_feerate=?," // 23 - " max_possible_feerate=?," // 24 - " msatoshi_to_us_min=?," // 25 - " msatoshi_to_us_max=?," // 26 - " feerate_base=?," // 27 - " feerate_ppm=?," // 28 - " remote_upfront_shutdown_script=?," // 29 - " local_static_remotekey_start=?," // 30 - " remote_static_remotekey_start=?," // 31 - " channel_type=?," // 32 - " shutdown_scriptpubkey_local=?," // 33 - " closer=?," // 34 - " state_change_reason=?," // 35 - " shutdown_wrong_txid=?," // 36 - " shutdown_wrong_outnum=?," // 37 - " lease_expiry=?," // 38 - " lease_commit_sig=?," // 39 - " lease_chan_max_msat=?," // 40 - " lease_chan_max_ppt=?," // 41 - " htlc_minimum_msat=?," // 42 - " htlc_maximum_msat=?," // 43 - " alias_local=?," // 44 - " alias_remote=?," // 45 - " ignore_fee_limits=?," // 46 - " remote_feerate_base=?," // 47 - " remote_feerate_ppm=?," // 48 - " remote_cltv_expiry_delta=?," // 49 - " remote_htlc_minimum_msat=?," // 50 - " remote_htlc_maximum_msat=?," // 51 - " last_stable_connection=?," // 52 - " require_confirm_inputs_remote=?," // 53 - " close_attempt_height=?" // 54 - " WHERE id=?")); // 55 + " shutdown_keyidx_local=?," + " channel_config_local=?," + " last_tx=?, last_sig=?," + " last_was_revoke=?," + " min_possible_feerate=?," + " max_possible_feerate=?," + " msatoshi_to_us_min=?," + " msatoshi_to_us_max=?," + " feerate_base=?," + " feerate_ppm=?," + " remote_upfront_shutdown_script=?," + " local_static_remotekey_start=?," + " remote_static_remotekey_start=?," + " channel_type=?," + " shutdown_scriptpubkey_local=?," + " closer=?," + " state_change_reason=?," + " shutdown_wrong_txid=?," + " shutdown_wrong_outnum=?," + " lease_expiry=?," + " lease_commit_sig=?," + " lease_chan_max_msat=?," + " lease_chan_max_ppt=?," + " htlc_minimum_msat=?," + " htlc_maximum_msat=?," + " alias_local=?," + " alias_remote=?," + " ignore_fee_limits=?," + " remote_feerate_base=?," + " remote_feerate_ppm=?," + " remote_cltv_expiry_delta=?," + " remote_htlc_minimum_msat=?," + " remote_htlc_maximum_msat=?," + " last_stable_connection=?," + " require_confirm_inputs_remote=?," + " close_attempt_height=?" + " WHERE id=?")); db_bind_u64(stmt, chan->their_shachain.id); if (chan->scid) db_bind_short_channel_id(stmt, *chan->scid); From d912287fa6be212c77abccaeb23e414acbfd831b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 15:57:11 +0930 Subject: [PATCH 3/7] lightningd: save previous short_channel_ids during splice, and keep in db. There can be any number of these, and it will be useful to allow routing by older scids (when other nodes haven't seen our gossip, or even before we *can* announce the new post-splice channel). Signed-off-by: Rusty Russell --- lightningd/channel.c | 16 ++++++++++++++++ lightningd/channel.h | 7 +++++++ lightningd/channel_control.c | 4 ++++ lightningd/opening_control.c | 2 ++ wallet/db.c | 1 + wallet/test/run-db.c | 1 + wallet/test/run-wallet.c | 3 ++- wallet/wallet.c | 7 ++++++- 8 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 94ff06c7b842..be3f7bf72526 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -242,6 +242,19 @@ struct open_attempt *new_channel_open_attempt(struct channel *channel) return oa; } +void channel_add_old_scid(struct channel *channel, + struct short_channel_id old_scid) +{ + /* If this is not public, we skip */ + if (!(channel->channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)) + return; + + if (!channel->old_scids) + channel->old_scids = tal_dup(channel, struct short_channel_id, &old_scid); + else + tal_arr_expand(&channel->old_scids, old_scid); +} + struct channel *new_unsaved_channel(struct peer *peer, u32 feerate_base, u32 feerate_ppm) @@ -275,6 +288,7 @@ struct channel *new_unsaved_channel(struct peer *peer, channel->last_htlc_sigs = NULL; channel->remote_channel_ready = false; channel->scid = NULL; + channel->old_scids = NULL; channel->next_index[LOCAL] = 1; channel->next_index[REMOTE] = 1; channel->next_htlc_id = 0; @@ -411,6 +425,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, bool remote_channel_ready, /* NULL or stolen */ struct short_channel_id *scid, + struct short_channel_id *old_scids TAKES, struct short_channel_id *alias_local TAKES, struct short_channel_id *alias_remote STEALS, struct channel_id *cid, @@ -538,6 +553,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->our_funds = our_funds; channel->remote_channel_ready = remote_channel_ready; channel->scid = tal_steal(channel, scid); + channel->old_scids = tal_dup_talarr(channel, struct short_channel_id, old_scids); channel->alias[LOCAL] = tal_dup_or_null(channel, struct short_channel_id, alias_local); /* We always make sure this is set (historical channels from db might not) */ if (!channel->alias[LOCAL]) { diff --git a/lightningd/channel.h b/lightningd/channel.h index f133dd2b4e36..cbc53e50bb6c 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -214,6 +214,8 @@ struct channel { bool remote_channel_ready; /* Channel if locked locally. */ struct short_channel_id *scid; + /* Old scids if we were spliced */ + struct short_channel_id *old_scids; /* Alias used for option_zeroconf, or option_scid_alias, if * present. LOCAL are all the alias we told the peer about and @@ -388,6 +390,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, bool remote_channel_ready, /* NULL or stolen */ struct short_channel_id *scid STEALS, + struct short_channel_id *old_scids TAKES, struct short_channel_id *alias_local STEALS, struct short_channel_id *alias_remote STEALS, struct channel_id *cid, @@ -487,6 +490,10 @@ u32 channel_last_funding_feerate(const struct channel *channel); /* Only set completely_eliminate for never-existed channels */ void delete_channel(struct channel *channel STEALS, bool completely_eliminate); +/* Add a historic (public) short_channel_id to this channel */ +void channel_add_old_scid(struct channel *channel, + struct short_channel_id old_scid); + const char *channel_state_name(const struct channel *channel); const char *channel_state_str(enum channel_state state); diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index a2447136a200..a254023be0ff 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -815,12 +815,16 @@ bool depthcb_update_scid(struct channel *channel, lockin_has_completed(channel, false); } else { + struct short_channel_id old_scid = *channel->scid; + /* We freaked out if required when original was * removed, so just update now */ log_info(channel->log, "Short channel id changed from %s->%s", fmt_short_channel_id(tmpctx, *channel->scid), fmt_short_channel_id(tmpctx, scid)); *channel->scid = scid; + /* In case we broadcast it before (e.g. splice!) */ + channel_add_old_scid(channel, old_scid); channel_gossip_scid_changed(channel); } diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index f3e14005123c..4735cc2303f1 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -189,6 +189,7 @@ wallet_commit_channel(struct lightningd *ld, local_funding, false, /* !remote_channel_ready */ NULL, /* no scid yet */ + NULL, /* no old scids */ NULL, /* assign random local alias */ NULL, /* They haven't told us an alias yet */ cid, @@ -1579,6 +1580,7 @@ static struct channel *stub_chan(struct command *cmd, AMOUNT_SAT(0), true, /* remote_channel_ready */ scid, + NULL, scid, scid, &cid, diff --git a/wallet/db.c b/wallet/db.c index 569182d5e592..ecae82a9871d 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1042,6 +1042,7 @@ static struct migration dbmigrations[] = { {NULL, NULL}, /* Old, incorrect channel_htlcs_wait_indexes migration */ {SQL("ALTER TABLE channel_funding_inflights ADD locked_scid BIGINT DEFAULT 0;"), NULL}, {NULL, migrate_initialize_channel_htlcs_wait_indexes_and_fixup_forwards}, + {SQL("ALTER TABLE channels ADD old_scids BLOB DEFAULT NULL;"), NULL}, }; /** diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index 7c18dede5127..a89acd5b0fcb 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -188,6 +188,7 @@ struct channel *new_channel(struct peer *peer UNNEEDED, u64 dbid UNNEEDED, bool remote_channel_ready UNNEEDED, /* NULL or stolen */ struct short_channel_id *scid STEALS UNNEEDED, + struct short_channel_id *old_scids TAKES UNNEEDED, struct short_channel_id *alias_local STEALS UNNEEDED, struct short_channel_id *alias_remote STEALS UNNEEDED, struct channel_id *cid UNNEEDED, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 5787f640c25f..1195c79e0d0d 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -2026,7 +2026,8 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) &outpoint, funding_sats, AMOUNT_MSAT(0), our_sats, - 0, false, + 0, NULL, + NULL, /* old scids */ NULL, /* alias[LOCAL] */ NULL, /* alias[REMOTE] */ &cid, diff --git a/wallet/wallet.c b/wallet/wallet.c index b2d67fae6772..bcbc6095df84 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1774,7 +1774,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm struct channel_info channel_info; struct fee_states *fee_states; struct height_states *height_states; - struct short_channel_id *scid, *alias[NUM_SIDES]; + struct short_channel_id *scid, *alias[NUM_SIDES], *old_scids; struct channel_id cid; struct channel *chan; u64 peer_dbid; @@ -1813,6 +1813,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm } scid = db_col_optional_scid(tmpctx, stmt, "scid"); + old_scids = db_col_short_channel_id_arr(tmpctx, stmt, "old_scids"); alias[LOCAL] = db_col_optional_scid(tmpctx, stmt, "alias_local"); alias[REMOTE] = db_col_optional_scid(tmpctx, stmt, "alias_remote"); @@ -2027,6 +2028,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm our_funding_sat, db_col_int(stmt, "funding_locked_remote") != 0, scid, + old_scids, alias[LOCAL], alias[REMOTE], &cid, @@ -2241,6 +2243,7 @@ static bool wallet_channels_load_active(struct wallet *w) " id" ", peer_id" ", scid" + ", old_scids" ", full_channel_id" ", channel_config_local" ", channel_config_remote" @@ -2521,6 +2524,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) stmt = db_prepare_v2(w->db, SQL("UPDATE channels SET" " shachain_remote_id=?," " scid=?," + " old_scids=?," " full_channel_id=?," " state=?," " funder=?," @@ -2580,6 +2584,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) else db_bind_null(stmt); + db_bind_short_channel_id_arr(stmt, chan->old_scids); db_bind_channel_id(stmt, &chan->cid); db_bind_int(stmt, channel_state_in_db(chan->state)); db_bind_int(stmt, chan->opener); From 30afa746ecda1a5bbf665454d48b0f2397bd28c2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 15:57:21 +0930 Subject: [PATCH 4/7] lightningd: consider old scids when looking up channels (for routing). Changelog-Fixed: Protocol: we now allow routing through old short-channel-ids once a splice is done (previously we would refuse, leading to a 6 block gap in service). Signed-off-by: Rusty Russell --- lightningd/channel.c | 6 ++++++ tests/test_splicing.py | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index be3f7bf72526..11e6be286656 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -781,6 +781,12 @@ struct channel *any_channel_by_scid(struct lightningd *ld, if (chan->scid && short_channel_id_eq(scid, *chan->scid)) return chan; + + /* Look through any old pre-splice channel ids */ + for (size_t i = 0; i < tal_count(chan->old_scids); i++) { + if (short_channel_id_eq(scid, chan->old_scids[i])) + return chan; + } } } return NULL; diff --git a/tests/test_splicing.py b/tests/test_splicing.py index 79d8619aa80f..dd0c51647876 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -423,7 +423,6 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor): assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0 -@pytest.mark.xfail(strict=True) def test_route_by_old_scid(node_factory, bitcoind): l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True, opts={'experimental-splicing': None}) From 8746c18220a2c9e02455810a4c75f2a810dc0087 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 15:57:28 +0930 Subject: [PATCH 5/7] lightningd: maintain a hash table of short_channel_id, for faster lookup. This contains real scids, as well as aliases, and old scids. Signed-off-by: Rusty Russell --- lightningd/channel.c | 77 ++++++++++++++++++++++++++++++---- lightningd/channel.h | 30 +++++++++++++ lightningd/channel_control.c | 4 +- lightningd/dual_open_control.c | 4 +- lightningd/lightningd.c | 4 ++ lightningd/lightningd.h | 2 + lightningd/memdump.c | 2 + lightningd/peer_control.c | 2 +- 8 files changed, 112 insertions(+), 13 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 11e6be286656..cd36cdfb400a 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -242,6 +242,60 @@ struct open_attempt *new_channel_open_attempt(struct channel *channel) return oa; } +static void chanmap_remove(struct lightningd *ld, + const struct channel *channel, + struct short_channel_id scid) +{ + struct scid_to_channel *scc = channel_scid_map_get(ld->channels_by_scid, scid); + assert(scc->channel == channel); + tal_free(scc); +} + +static void destroy_scid_to_channel(struct scid_to_channel *scc, + struct lightningd *ld) +{ + if (!channel_scid_map_del(ld->channels_by_scid, scc)) + abort(); +} + +static void chanmap_add(struct lightningd *ld, + struct channel *channel, + struct short_channel_id scid) +{ + struct scid_to_channel *scc = tal(channel, struct scid_to_channel); + scc->channel = channel; + scc->scid = scid; + channel_scid_map_add(ld->channels_by_scid, scc); + tal_add_destructor2(scc, destroy_scid_to_channel, ld); +} + +static void channel_set_random_local_alias(struct channel *channel) +{ + assert(channel->alias[LOCAL] == NULL); + channel->alias[LOCAL] = tal(channel, struct short_channel_id); + randombytes_buf(channel->alias[LOCAL], sizeof(struct short_channel_id)); + /* We don't check for uniqueness. We would crash on a clash, but your machine is + * probably broken beyond repair if it gets two equal 64 bit numbers */ + chanmap_add(channel->peer->ld, channel, *channel->alias[LOCAL]); +} + +void channel_set_scid(struct channel *channel, const struct short_channel_id *new_scid) +{ + struct lightningd *ld = channel->peer->ld; + + /* Get rid of old one (if any) */ + if (channel->scid != NULL) { + chanmap_remove(ld, channel, *channel->scid); + channel->scid = tal_free(channel->scid); + } + + /* Add new one (if any) */ + if (new_scid) { + channel->scid = tal_dup(channel, struct short_channel_id, new_scid); + chanmap_add(ld, channel, *new_scid); + } +} + void channel_add_old_scid(struct channel *channel, struct short_channel_id old_scid) { @@ -253,6 +307,8 @@ void channel_add_old_scid(struct channel *channel, channel->old_scids = tal_dup(channel, struct short_channel_id, &old_scid); else tal_arr_expand(&channel->old_scids, old_scid); + + chanmap_add(channel->peer->ld, channel, old_scid); } struct channel *new_unsaved_channel(struct peer *peer, @@ -300,10 +356,8 @@ struct channel *new_unsaved_channel(struct peer *peer, = CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE; channel->shutdown_wrong_funding = NULL; channel->closing_feerate_range = NULL; - channel->alias[REMOTE] = NULL; - /* We don't even bother checking for clashes. */ - channel->alias[LOCAL] = tal(channel, struct short_channel_id); - randombytes_buf(channel->alias[LOCAL], sizeof(struct short_channel_id)); + channel->alias[REMOTE] = channel->alias[LOCAL] = NULL; + channel_set_random_local_alias(channel); channel->shutdown_scriptpubkey[REMOTE] = NULL; channel->last_was_revoke = false; @@ -555,11 +609,18 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->scid = tal_steal(channel, scid); channel->old_scids = tal_dup_talarr(channel, struct short_channel_id, old_scids); channel->alias[LOCAL] = tal_dup_or_null(channel, struct short_channel_id, alias_local); + /* All these possible short_channel_id variants go in the lookup table! */ + if (channel->scid) + chanmap_add(peer->ld, channel, *channel->scid); + if (channel->alias[LOCAL]) + chanmap_add(peer->ld, channel, *channel->alias[LOCAL]); + for (size_t i = 0; i < tal_count(channel->old_scids); i++) + chanmap_add(peer->ld, channel, channel->old_scids[i]); + /* We always make sure this is set (historical channels from db might not) */ - if (!channel->alias[LOCAL]) { - channel->alias[LOCAL] = tal(channel, struct short_channel_id); - randombytes_buf(channel->alias[LOCAL], sizeof(struct short_channel_id)); - } + if (!channel->alias[LOCAL]) + channel_set_random_local_alias(channel); + channel->alias[REMOTE] = tal_steal(channel, alias_remote); /* Haven't gotten one yet. */ channel->cid = *cid; channel->our_msat = our_msat; diff --git a/lightningd/channel.h b/lightningd/channel.h index cbc53e50bb6c..6cb964aed834 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -1,6 +1,7 @@ #ifndef LIGHTNING_LIGHTNINGD_CHANNEL_H #define LIGHTNING_LIGHTNINGD_CHANNEL_H #include "config.h" +#include #include #include #include @@ -840,6 +841,35 @@ struct channel *peer_any_channel_bystate(struct peer *peer, struct channel *channel_by_dbid(struct lightningd *ld, const u64 dbid); +struct scid_to_channel { + struct short_channel_id scid; + struct channel *channel; +}; + +static inline const struct short_channel_id scid_to_channel_key(const struct scid_to_channel *scidchan) +{ + return scidchan->scid; +} + +static inline bool scid_to_channel_eq_scid(const struct scid_to_channel *scidchan, + struct short_channel_id scid) +{ + return short_channel_id_eq(scidchan->scid, scid); +} + +/* Define channel_scid_map */ +HTABLE_DEFINE_NODUPS_TYPE(struct scid_to_channel, + scid_to_channel_key, + short_channel_id_hash, + scid_to_channel_eq_scid, + channel_scid_map); + +/* The only allowed way to set channel->scid */ +void channel_set_scid(struct channel *channel, const struct short_channel_id *new_scid); + +/* The only allowed way to set channel->alias[LOCAL] */ +void channel_set_local_alias(struct channel *channel, struct short_channel_id alias_scid); + /* Includes both real scids and aliases. If !privacy_leak_ok, then private * channels' real scids are not included. */ struct channel *any_channel_by_scid(struct lightningd *ld, diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index a254023be0ff..f6f3f129a875 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -803,7 +803,7 @@ bool depthcb_update_scid(struct channel *channel, if (!channel->scid) { wallet_annotate_txout(ld->wallet, outpoint, TX_CHANNEL_FUNDING, channel->dbid); - channel->scid = tal_dup(channel, struct short_channel_id, &scid); + channel_set_scid(channel, &scid); /* If we have a zeroconf channel, i.e., no scid yet * but have exchange `channel_ready` messages, then we @@ -822,7 +822,7 @@ bool depthcb_update_scid(struct channel *channel, log_info(channel->log, "Short channel id changed from %s->%s", fmt_short_channel_id(tmpctx, *channel->scid), fmt_short_channel_id(tmpctx, scid)); - *channel->scid = scid; + channel_set_scid(channel, &scid); /* In case we broadcast it before (e.g. splice!) */ channel_add_old_scid(channel, old_scid); channel_gossip_scid_changed(channel); diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index b96ac946d35a..874fb40f7c13 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1038,7 +1038,7 @@ static enum watch_result opening_depth_cb(struct lightningd *ld, if (!inflight->channel->scid) { wallet_annotate_txout(ld->wallet, &inflight->funding->outpoint, TX_CHANNEL_FUNDING, inflight->channel->dbid); - inflight->channel->scid = tal_dup(inflight->channel, struct short_channel_id, &scid); + channel_set_scid(inflight->channel, &scid); wallet_channel_save(ld->wallet, inflight->channel); } else if (!short_channel_id_eq(*inflight->channel->scid, scid)) { /* We freaked out if required when original was @@ -1046,7 +1046,7 @@ static enum watch_result opening_depth_cb(struct lightningd *ld, log_info(inflight->channel->log, "Short channel id changed from %s->%s", fmt_short_channel_id(tmpctx, *inflight->channel->scid), fmt_short_channel_id(tmpctx, scid)); - *inflight->channel->scid = scid; + channel_set_scid(inflight->channel, &scid); wallet_channel_save(ld->wallet, inflight->channel); } diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 68b2e2462ec5..f5007f32b085 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -209,6 +209,10 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->peers_by_dbid = tal(ld, struct peer_dbid_map); peer_dbid_map_init(ld->peers_by_dbid); + /*~ This speeds lookups for short_channel_ids to their channels. */ + ld->channels_by_scid = tal(ld, struct channel_scid_map); + channel_scid_map_init(ld->channels_by_scid); + /*~ For multi-part payments, we need to keep some incoming payments * in limbo until we get all the parts, or we time them out. */ ld->htlc_sets = tal(ld, struct htlc_set_map); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 65383c46bdad..7cb7d1d26fc7 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -215,6 +215,8 @@ struct lightningd { struct peer_node_id_map *peers; /* And those in database by dbid */ struct peer_dbid_map *peers_by_dbid; + /* Here are all our channels and their aliases */ + struct channel_scid_map *channels_by_scid; /* Outstanding connect commands. */ struct list_head connects; diff --git a/lightningd/memdump.c b/lightningd/memdump.c index c999db71a0d0..b2f32a10636f 100644 --- a/lightningd/memdump.c +++ b/lightningd/memdump.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -202,6 +203,7 @@ static bool lightningd_check_leaks(struct command *cmd) memleak_scan_htable(memtable, &ld->htlc_sets->raw); memleak_scan_htable(memtable, &ld->peers->raw); memleak_scan_htable(memtable, &ld->peers_by_dbid->raw); + memleak_scan_htable(memtable, &ld->channels_by_scid->raw); memleak_scan_htable(memtable, &ld->closed_channels->raw); wallet_memleak_scan(memtable, ld->wallet); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index ae944a691e72..35b88089991f 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2225,7 +2225,7 @@ static enum watch_result funding_depth_cb(struct lightningd *ld, /* That's not entirely unexpected in early states */ log_debug(channel->log, "Funding tx %s reorganized out!", fmt_bitcoin_txid(tmpctx, txid)); - channel->scid = tal_free(channel->scid); + channel_set_scid(channel, NULL); return KEEP_WATCHING; /* But it's often Bad News in later states */ From e596d9f79d096c9d8bc145c62d2b1f6fe63e5981 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 15:58:28 +0930 Subject: [PATCH 6/7] lightningd: use the hash table to lookup scids. This replaces the old "iterate through each peer, then each peer's channel" suboptimality. A bit of care required that we don't expose scids if we're forwarding, but that was already carefully handled. Signed-off-by: Rusty Russell --- lightningd/channel.c | 59 +++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index cd36cdfb400a..28e49a9b4f9a 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -814,43 +814,28 @@ struct channel *any_channel_by_scid(struct lightningd *ld, struct short_channel_id scid, bool privacy_leak_ok) { - struct peer *p; - struct channel *chan; - struct peer_node_id_map_iter it; - - /* FIXME: Support lookup by scid directly! */ - for (p = peer_node_id_map_first(ld->peers, &it); - p; - p = peer_node_id_map_next(ld->peers, &it)) { - list_for_each(&p->channels, chan, list) { - /* BOLT #2: - * - MUST always recognize the `alias` as a - * `short_channel_id` for incoming HTLCs to this - * channel. - */ - if (chan->alias[LOCAL] && - short_channel_id_eq(scid, *chan->alias[LOCAL])) - return chan; - /* BOLT #2: - * - if `channel_type` has `option_scid_alias` set: - * - MUST NOT allow incoming HTLCs to this channel - * using the real `short_channel_id` - */ - if (!privacy_leak_ok - && channel_type_has(chan->type, OPT_SCID_ALIAS)) - continue; - if (chan->scid - && short_channel_id_eq(scid, *chan->scid)) - return chan; - - /* Look through any old pre-splice channel ids */ - for (size_t i = 0; i < tal_count(chan->old_scids); i++) { - if (short_channel_id_eq(scid, chan->old_scids[i])) - return chan; - } - } - } - return NULL; + const struct scid_to_channel *scc = channel_scid_map_get(ld->channels_by_scid, scid); + if (!scc) + return NULL; + + /* BOLT #2: + * - MUST always recognize the `alias` as a `short_channel_id` for + * incoming HTLCs to this channel. + */ + if (scc->channel->alias[LOCAL] + && short_channel_id_eq(scid, *scc->channel->alias[LOCAL])) + return scc->channel; + + /* BOLT #2: + * - if `channel_type` has `option_scid_alias` set: + * - MUST NOT allow incoming HTLCs to this channel using the real + * `short_channel_id` + */ + /* This means any scids other than the alias (handled above) cannot be exposed */ + if (!privacy_leak_ok && channel_type_has(scc->channel->type, OPT_SCID_ALIAS)) + return NULL; + + return scc->channel; } struct channel *channel_by_dbid(struct lightningd *ld, const u64 dbid) From f4566cd339d898895c66bfaa5d7c7ee6bbcb4ae5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 16:03:52 +0930 Subject: [PATCH 7/7] pytest: test persistence of old scids, even if we spliced multiple times. Signed-off-by: Rusty Russell --- tests/test_splicing.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index dd0c51647876..46452728017f 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -424,10 +424,11 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor): def test_route_by_old_scid(node_factory, bitcoind): - l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True, opts={'experimental-splicing': None}) + l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True, opts={'experimental-splicing': None, 'may_reconnect': True}) # Get pre-splice route. inv = l3.rpc.invoice(10000000, 'test_route_by_old_scid', 'test_route_by_old_scid') + inv2 = l3.rpc.invoice(10000000, 'test_route_by_old_scid2', 'test_route_by_old_scid2') route = l1.rpc.getroute(l3.info['id'], 10000000, 1)['route'] # Do a splice @@ -448,3 +449,28 @@ def test_route_by_old_scid(node_factory, bitcoind): # Now l1 tries to send using old scid: should work l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret']) l1.rpc.waitsendpay(inv['payment_hash']) + + # Let's splice again, so the original scid is two behind the times. + l3.fundwallet(200000) + funds_result = l3.rpc.fundpsbt("109000sat", "slow", 166, excess_as_change=True) + chan_id = l3.get_channel_id(l2) + result = l3.rpc.splice_init(chan_id, 100000, funds_result['psbt']) + result = l3.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is False) + result = l3.rpc.splice_update(chan_id, result['psbt']) + assert(result['commitments_secured'] is True) + result = l3.rpc.signpsbt(result['psbt']) + result = l3.rpc.splice_signed(chan_id, result['signed_psbt']) + + wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_AWAITING_SPLICE') + bitcoind.generate_block(6, wait_for_mempool=1) + wait_for(lambda: only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['state'] == 'CHANNELD_NORMAL') + + # Now restart l2, make sure it remembers the original! + l2.restart() + l2.rpc.connect(l1.info['id'], 'localhost', l1.port) + l2.rpc.connect(l3.info['id'], 'localhost', l3.port) + + wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['connected'] is True) + l1.rpc.sendpay(route, inv2['payment_hash'], payment_secret=inv2['payment_secret']) + l1.rpc.waitsendpay(inv2['payment_hash'])