From 01a5cfdd29963fa596c53f1f86595cc46d6d30cc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 1 Jul 2025 14:41:13 +0930 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 a1fb13bd3841f4adf9e5427d5b7d52f54fe90b46 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:31 +0930 Subject: [PATCH 04/10] 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 | 2 +- 2 files changed, 7 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..648decc8cbad 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -423,7 +423,7 @@ 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) +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') 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 581fac54b9d13d689b4823c0acd585ff6a3893cc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 05/10] 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 | 78 ++++++++++++++++++++++++++++++---- 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/opening_control.c | 4 +- lightningd/peer_control.c | 2 +- 9 files changed, 115 insertions(+), 15 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 11e6be286656..bbdf288919ec 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,19 @@ 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! */ + /* Stub channels all have the same scid though, *and* get loaded from db! */ + if (channel->scid && !is_stub_scid(*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/opening_control.c b/lightningd/opening_control.c index 4735cc2303f1..e789e633c062 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -1581,8 +1581,8 @@ static struct channel *stub_chan(struct command *cmd, true, /* remote_channel_ready */ scid, NULL, - scid, - scid, + NULL, + NULL, &cid, /* The three arguments below are msatoshi_to_us, * msatoshi_to_us_min, and msatoshi_to_us_max. 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 e8c3e026f572be95f459c5dfb4d6f694f6f1418a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 06/10] bitcoin: have random_scid() function. Signed-off-by: Rusty Russell --- bitcoin/short_channel_id.c | 8 ++++++++ bitcoin/short_channel_id.h | 2 ++ lightningd/channel.c | 3 +-- wallet/db.c | 4 +--- wallet/test/run-wallet.c | 6 ++++++ 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/bitcoin/short_channel_id.c b/bitcoin/short_channel_id.c index 80e7d28a7e1d..cc1de68c4377 100644 --- a/bitcoin/short_channel_id.c +++ b/bitcoin/short_channel_id.c @@ -1,6 +1,7 @@ #include "config.h" #include #include +#include #include #include @@ -99,3 +100,10 @@ struct short_channel_id fromwire_short_channel_id(const u8 **cursor, size_t *max scid.u64 = fromwire_u64(cursor, max); return scid; } + +struct short_channel_id random_scid(void) +{ + struct short_channel_id scid; + randombytes_buf(&scid, sizeof(scid)); + return scid; +} diff --git a/bitcoin/short_channel_id.h b/bitcoin/short_channel_id.h index cf382d693804..1cd13f9a4cff 100644 --- a/bitcoin/short_channel_id.h +++ b/bitcoin/short_channel_id.h @@ -98,4 +98,6 @@ void towire_short_channel_id(u8 **pptr, struct short_channel_id short_channel_id); struct short_channel_id fromwire_short_channel_id(const u8 **cursor, size_t *max); +/* Set to random bytes */ +struct short_channel_id random_scid(void); #endif /* LIGHTNING_BITCOIN_SHORT_CHANNEL_ID_H */ diff --git a/lightningd/channel.c b/lightningd/channel.c index bbdf288919ec..6d2bda0c38fb 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -17,7 +17,6 @@ #include #include #include -#include #include #include @@ -273,7 +272,7 @@ 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)); + *channel->alias[LOCAL] = random_scid(); /* 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]); diff --git a/wallet/db.c b/wallet/db.c index ecae82a9871d..83cd366235dc 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -2020,13 +2020,11 @@ static void migrate_initialize_alias_local(struct lightningd *ld, tal_free(stmt); for (size_t i = 0; i < tal_count(ids); i++) { - struct short_channel_id alias; stmt = db_prepare_v2(db, SQL("UPDATE channels" " SET alias_local = ?" " WHERE id = ?;")); /* We don't even check for clashes! */ - randombytes_buf(&alias, sizeof(alias)); - db_bind_short_channel_id(stmt, alias); + db_bind_short_channel_id(stmt, random_scid()); db_bind_u64(stmt, ids[i]); db_exec_prepared_v2(stmt); tal_free(stmt); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 1195c79e0d0d..81b4004fd659 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1366,6 +1366,9 @@ static struct wallet *create_test_wallet(struct lightningd *ld, const tal_t *ctx CHECK_MSG(!wallet_err, wallet_err); w->max_channel_dbid = 0; + /* Create fresh channels map */ + ld->channels_by_scid = tal(ld, struct channel_scid_map); + channel_scid_map_init(ld->channels_by_scid); return w; } @@ -2091,6 +2094,9 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) /* do inflights get correctly added to the channel? */ wallet_inflight_add(w, inflight); + /* Hack to remove scids from htable so we don't clash! */ + chanmap_remove(ld, chan, *chan->alias[LOCAL]); + /* do inflights get correctly loaded from the database? */ CHECK_MSG(c2 = wallet_channel_load(w, chan->dbid), tal_fmt(w, "Load from DB")); From 3b03b52d84ce1ac4fc80a618e9ab60b9ca984dbd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 07/10] wallet: we can assume local_alias field is non-null. We have a migration which ensures this, but then I discovered that did *not* address channels without an SCID yet. So fixed the migration, and simpligied the code. Signed-off-by: Rusty Russell --- wallet/db.c | 6 +++--- wallet/test/run-wallet.c | 3 +++ wallet/wallet.c | 12 +++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/wallet/db.c b/wallet/db.c index 83cd366235dc..7b8e35d859df 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1026,7 +1026,7 @@ static struct migration dbmigrations[] = { {SQL("ALTER TABLE channels ADD remote_htlc_maximum_msat BIGINT DEFAULT NULL;"), NULL}, {SQL("ALTER TABLE channels ADD remote_htlc_minimum_msat BIGINT DEFAULT NULL;"), NULL}, {SQL("ALTER TABLE channels ADD last_stable_connection BIGINT DEFAULT 0;"), NULL}, - {NULL, migrate_initialize_alias_local}, + {NULL, NULL}, /* old migrate_initialize_alias_local */ {SQL("CREATE TABLE addresses (" " keyidx BIGINT," " addrtype INTEGER)"), NULL}, @@ -1043,6 +1043,7 @@ static struct migration dbmigrations[] = { {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}, + {NULL, migrate_initialize_alias_local}, }; /** @@ -2012,8 +2013,7 @@ static void migrate_initialize_alias_local(struct lightningd *ld, u64 *ids = tal_arr(tmpctx, u64, 0); stmt = db_prepare_v2(db, SQL("SELECT id FROM channels" - " WHERE scid IS NOT NULL" - " AND alias_local IS NULL;")); + " WHERE alias_local IS NULL;")); db_query_prepared(stmt); while (db_step(stmt)) tal_arr_expand(&ids, db_col_u64(stmt, "id")); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 81b4004fd659..a9e9069802c7 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1794,6 +1794,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) struct pubkey pk; struct node_id id; struct changed_htlc *last_commit; + struct short_channel_id local_alias; secp256k1_ecdsa_signature *sig = tal(w, secp256k1_ecdsa_signature); u8 *scriptpubkey = tal_arr(ctx, u8, 100); secp256k1_ecdsa_signature *node_sig1 = tal(w, secp256k1_ecdsa_signature); @@ -1850,6 +1851,8 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) /* Init channel inflights */ list_head_init(&c1.inflights); c1.type = type; + local_alias = random_scid(); + c1.alias[LOCAL] = &local_alias; db_begin_transaction(w->db); CHECK(!wallet_err); diff --git a/wallet/wallet.c b/wallet/wallet.c index bcbc6095df84..e46ab0014d52 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1814,7 +1814,8 @@ 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[LOCAL] = tal(tmpctx, struct short_channel_id); + *alias[LOCAL] = db_col_short_channel_id(stmt, "alias_local"); alias[REMOTE] = db_col_optional_scid(tmpctx, stmt, "alias_remote"); ok &= wallet_shachain_load(w, db_col_u64(stmt, "shachain_remote_id"), @@ -2094,7 +2095,8 @@ static struct closed_channel *wallet_stmt2closed_channel(const tal_t *ctx, cc->peer_id = db_col_optional(cc, stmt, "p.node_id", node_id); db_col_channel_id(stmt, "full_channel_id", &cc->cid); cc->scid = db_col_optional_scid(cc, stmt, "scid"); - cc->alias[LOCAL] = db_col_optional_scid(cc, stmt, "alias_local"); + cc->alias[LOCAL] = tal(cc, struct short_channel_id); + *cc->alias[LOCAL] = db_col_short_channel_id(stmt, "alias_local"); cc->alias[REMOTE] = db_col_optional_scid(cc, stmt, "alias_remote"); cc->opener = db_col_int(stmt, "funder"); cc->closer = db_col_int(stmt, "closer"); @@ -2649,11 +2651,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) db_bind_amount_msat(stmt, &chan->htlc_minimum_msat); db_bind_amount_msat(stmt, &chan->htlc_maximum_msat); - if (chan->alias[LOCAL] != NULL) - db_bind_short_channel_id(stmt, *chan->alias[LOCAL]); - else - db_bind_null(stmt); - + db_bind_short_channel_id(stmt, *chan->alias[LOCAL]); if (chan->alias[REMOTE] != NULL) db_bind_short_channel_id(stmt, *chan->alias[REMOTE]); else From d6f0ab552a634568d7c5b15aa869014225921715 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 08/10] lightningd: require local_alias in new_channel(). We allowed NULL for stub channels, but just don't put the stub scid into the hash tables. This cleans up all the callers to make it clear this is a non-optional parameter. We opencode channel_set_random_local_alias, since there's only one caller now. Signed-off-by: Rusty Russell --- lightningd/channel.c | 34 ++++++++------------- lightningd/channel.h | 4 +-- lightningd/opening_control.c | 14 +++++---- lightningd/test/run-invoice-select-inchan.c | 3 ++ wallet/test/run-db.c | 2 +- wallet/test/run-wallet.c | 3 +- wallet/wallet.c | 15 +++++---- 7 files changed, 35 insertions(+), 40 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 6d2bda0c38fb..03246165b939 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -268,16 +268,6 @@ static void chanmap_add(struct lightningd *ld, 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); - *channel->alias[LOCAL] = random_scid(); - /* 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; @@ -355,8 +345,12 @@ 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] = channel->alias[LOCAL] = NULL; - channel_set_random_local_alias(channel); + channel->alias[REMOTE] = NULL; + channel->alias[LOCAL] = tal(channel, struct short_channel_id); + *channel->alias[LOCAL] = random_scid(); + /* 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]); channel->shutdown_scriptpubkey[REMOTE] = NULL; channel->last_was_revoke = false; @@ -477,9 +471,9 @@ struct channel *new_channel(struct peer *peer, u64 dbid, struct amount_sat our_funds, bool remote_channel_ready, /* NULL or stolen */ - struct short_channel_id *scid, + struct short_channel_id *scid TAKES, struct short_channel_id *old_scids TAKES, - struct short_channel_id *alias_local TAKES, + struct short_channel_id alias_local, struct short_channel_id *alias_remote STEALS, struct channel_id *cid, struct amount_msat our_msat, @@ -605,22 +599,18 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->push = push; channel->our_funds = our_funds; channel->remote_channel_ready = remote_channel_ready; - channel->scid = tal_steal(channel, scid); + channel->scid = tal_dup_or_null(channel, struct short_channel_id, 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); + channel->alias[LOCAL] = tal_dup(channel, struct short_channel_id, &alias_local); /* All these possible short_channel_id variants go in the lookup table! */ /* Stub channels all have the same scid though, *and* get loaded from db! */ if (channel->scid && !is_stub_scid(*channel->scid)) chanmap_add(peer->ld, channel, *channel->scid); - if (channel->alias[LOCAL]) + if (!is_stub_scid(*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_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; @@ -727,7 +717,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, } /* scid is NULL when opening a new channel so we don't * need to set error in that case as well */ - if (scid && is_stub_scid(*scid)) + if (channel->scid && is_stub_scid(*channel->scid)) channel->error = towire_errorfmt(peer->ld, &channel->cid, "We can't be together anymore."); diff --git a/lightningd/channel.h b/lightningd/channel.h index 6cb964aed834..74e5282f76b6 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -390,9 +390,9 @@ struct channel *new_channel(struct peer *peer, u64 dbid, struct amount_sat our_funds, bool remote_channel_ready, /* NULL or stolen */ - struct short_channel_id *scid STEALS, + struct short_channel_id *scid TAKES, struct short_channel_id *old_scids TAKES, - struct short_channel_id *alias_local STEALS, + struct short_channel_id alias_local, struct short_channel_id *alias_remote STEALS, struct channel_id *cid, struct amount_msat our_msatoshi, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index e789e633c062..70ad39490625 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -110,6 +110,7 @@ wallet_commit_channel(struct lightningd *ld, struct timeabs timestamp; struct channel_stats zero_channel_stats; enum addrtype addrtype; + struct short_channel_id local_alias; /* We can't have any payments yet */ memset(&zero_channel_stats, 0, sizeof(zero_channel_stats)); @@ -172,6 +173,8 @@ wallet_commit_channel(struct lightningd *ld, else static_remotekey_start = 0x7FFFFFFFFFFFFFFF; + local_alias = random_scid(); + channel = new_channel(uc->peer, uc->dbid, NULL, /* No shachain yet */ CHANNELD_AWAITING_LOCKIN, @@ -190,7 +193,7 @@ wallet_commit_channel(struct lightningd *ld, false, /* !remote_channel_ready */ NULL, /* no scid yet */ NULL, /* no old scids */ - NULL, /* assign random local alias */ + local_alias, /* random local alias */ NULL, /* They haven't told us an alias yet */ cid, /* The three arguments below are msatoshi_to_us, @@ -1477,7 +1480,7 @@ static struct channel *stub_chan(struct command *cmd, struct peer *peer; struct pubkey localFundingPubkey; struct pubkey pk; - struct short_channel_id *scid; + struct short_channel_id scid; u32 blockht; u32 feerate; struct channel_stats zero_channel_stats; @@ -1555,10 +1558,9 @@ static struct channel *stub_chan(struct command *cmd, channel_info->old_remote_per_commit = pk; blockht = 100; - scid = tal(cmd, struct short_channel_id); /*To indicate this is an stub channel we keep it's scid to 1x1x1.*/ - if (!mk_short_channel_id(scid, 1, 1, 1)) + if (!mk_short_channel_id(&scid, 1, 1, 1)) fatal("Failed to make short channel 1x1x1!"); memset(&zero_channel_stats, 0, sizeof(zero_channel_stats)); @@ -1579,9 +1581,9 @@ static struct channel *stub_chan(struct command *cmd, AMOUNT_MSAT(0), AMOUNT_SAT(0), true, /* remote_channel_ready */ - scid, - NULL, + &scid, NULL, + scid, NULL, &cid, /* The three arguments below are msatoshi_to_us, diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 5dfcd4e2742b..fc7c642496ab 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -135,6 +135,9 @@ void channel_set_last_tx(struct channel *channel UNNEEDED, struct bitcoin_tx *tx UNNEEDED, const struct bitcoin_signature *sig UNNEEDED) { fprintf(stderr, "channel_set_last_tx called!\n"); abort(); } +/* Generated stub for channel_set_scid */ +void channel_set_scid(struct channel *channel UNNEEDED, const struct short_channel_id *new_scid UNNEEDED) +{ fprintf(stderr, "channel_set_scid called!\n"); abort(); } /* Generated stub for channel_state_name */ const char *channel_state_name(const struct channel *channel UNNEEDED) { fprintf(stderr, "channel_state_name called!\n"); abort(); } diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index a89acd5b0fcb..b773170f2db2 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -189,7 +189,7 @@ struct channel *new_channel(struct peer *peer UNNEEDED, u64 dbid 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_local UNNEEDED, struct short_channel_id *alias_remote STEALS UNNEEDED, struct channel_id *cid UNNEEDED, struct amount_msat our_msatoshi UNNEEDED, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index a9e9069802c7..7a9492323168 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1983,6 +1983,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) struct wally_psbt *funding_psbt; struct channel_info *channel_info = tal(w, struct channel_info); struct basepoints basepoints; + struct short_channel_id alias_local = random_scid(); secp256k1_ecdsa_signature *lease_commit_sig; u32 feerate, lease_blockheight_start; u64 dbid; @@ -2034,7 +2035,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) our_sats, 0, NULL, NULL, /* old scids */ - NULL, /* alias[LOCAL] */ + alias_local, NULL, /* alias[REMOTE] */ &cid, AMOUNT_MSAT(3333333000), diff --git a/wallet/wallet.c b/wallet/wallet.c index e46ab0014d52..6fecca4e48dc 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], *old_scids; + struct short_channel_id *scid, alias_local, *alias_remote, *old_scids; struct channel_id cid; struct channel *chan; u64 peer_dbid; @@ -1814,9 +1814,8 @@ 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] = tal(tmpctx, struct short_channel_id); - *alias[LOCAL] = db_col_short_channel_id(stmt, "alias_local"); - alias[REMOTE] = db_col_optional_scid(tmpctx, stmt, "alias_remote"); + alias_local = db_col_short_channel_id(stmt, "alias_local"); + alias_remote = db_col_optional_scid(tmpctx, stmt, "alias_remote"); ok &= wallet_shachain_load(w, db_col_u64(stmt, "shachain_remote_id"), &wshachain); @@ -1971,7 +1970,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm if (scid) remote_update->scid = *scid; else - remote_update->scid = *alias[LOCAL]; + remote_update->scid = alias_local; remote_update->fee_base = db_col_int(stmt, "remote_feerate_base"); remote_update->fee_ppm = db_col_int(stmt, "remote_feerate_ppm"); remote_update->cltv_delta = db_col_int(stmt, "remote_cltv_expiry_delta"); @@ -2028,10 +2027,10 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm push_msat, our_funding_sat, db_col_int(stmt, "funding_locked_remote") != 0, - scid, + take(scid), old_scids, - alias[LOCAL], - alias[REMOTE], + alias_local, + alias_remote, &cid, our_msat, msat_to_us_min, /* msatoshi_to_us_min */ From 1b628afbca6a7ec566aa9f7c0e77c6d1bdda3a5a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 09/10] 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 03246165b939..4e16b5134bfa 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -804,43 +804,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 c7ed96679f784567daf5c89022d4b80869211e9a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 21 Jul 2025 10:04:33 +0930 Subject: [PATCH 10/10] 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 648decc8cbad..897207afe8aa 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -425,10 +425,11 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor): @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') 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 @@ -449,3 +450,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'])