Skip to content

Commit a153dbe

Browse files
okekefrancis112claude
andcommitted
fix: prefer outbound SCID alias over real SCID for spliced channels
With channel splicing, channels may spontaneously change their on-chain short channel IDs (SCIDs) as new funding transactions are created. However, SCID aliases remain stable across splices. Change get_outbound_payment_scid() to prefer outbound_scid_alias over short_channel_id (previously was the opposite). This ensures first-hop routing uses stable identifiers that won't change when channels are spliced. Updated tests to reflect that: - First hop uses the alias (from get_outbound_payment_scid) - Subsequent hops use real SCIDs from network gossip Closes #3268 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent b7a50a6 commit a153dbe

4 files changed

Lines changed: 87 additions & 59 deletions

File tree

lightning/src/ln/channel_state.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -499,12 +499,12 @@ impl ChannelDetails {
499499
/// This should be used in [`Route`]s to describe the first hop or in other contexts where
500500
/// we're sending or forwarding a payment outbound over this channel.
501501
///
502-
/// This is either the [`ChannelDetails::short_channel_id`], if set, or the
503-
/// [`ChannelDetails::outbound_scid_alias`]. See those for more information.
502+
/// This is either the [`ChannelDetails::outbound_scid_alias`], if set, or the
503+
/// [`ChannelDetails::short_channel_id`]. See those for more information.
504504
///
505505
/// [`Route`]: crate::routing::router::Route
506506
pub fn get_outbound_payment_scid(&self) -> Option<u64> {
507-
self.short_channel_id.or(self.outbound_scid_alias)
507+
self.outbound_scid_alias.or(self.short_channel_id)
508508
}
509509

510510
/// Gets the funding output for this channel, if available.

lightning/src/ln/payment_tests.rs

Lines changed: 70 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,18 @@ fn mpp_retry_overpay() {
247247
get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, amt_msat, max_fee);
248248

249249
// Check we overpay on the second path which we're about to fail.
250+
// Find which path goes through each node (paths may be in any order).
251+
let (path_to_b_idx, path_to_c_idx) =
252+
if route.paths[0].hops[0].pubkey == node_b_id { (0, 1) } else { (1, 0) };
253+
250254
assert_eq!(chan_1_update.contents.fee_proportional_millionths, 0);
251-
let overpaid_amount_1 = route.paths[0].fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
255+
let overpaid_amount_1 =
256+
route.paths[path_to_b_idx].fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
252257
assert_eq!(overpaid_amount_1, 0);
253258

254259
assert_eq!(chan_2_update.contents.fee_proportional_millionths, 0);
255-
let overpaid_amount_2 = route.paths[1].fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
260+
let overpaid_amount_2 =
261+
route.paths[path_to_c_idx].fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
256262

257263
let total_overpaid_amount = overpaid_amount_1 + overpaid_amount_2;
258264

@@ -301,11 +307,12 @@ fn mpp_retry_overpay() {
301307
send_payment(&nodes[3], &[&nodes[2]], 38_000_000);
302308

303309
// Retry the second half of the payment and make sure it succeeds.
304-
let first_path_value = route.paths[0].final_value_msat();
305-
assert_eq!(first_path_value, 36_000_000);
310+
// The path to node B (path_to_b_idx) succeeded with 36M; remove it so we retry only the C path.
311+
let succeeded_path_value = route.paths[path_to_b_idx].final_value_msat();
312+
assert_eq!(succeeded_path_value, 36_000_000);
306313

307-
route.paths.remove(0);
308-
route_params.final_value_msat -= first_path_value;
314+
route.paths.remove(path_to_b_idx);
315+
route_params.final_value_msat -= succeeded_path_value;
309316
let chan_4_scid = chan_4_update.contents.short_channel_id;
310317
route_params.payment_params.previously_failed_channels.push(chan_4_scid);
311318
// Check the remaining max total routing fee for the second attempt accounts only for 1_000 msat
@@ -1781,13 +1788,32 @@ fn preflight_probes_yield_event() {
17811788
.unwrap();
17821789

17831790
let recv_value = 50_000_000;
1784-
let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
1785-
let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
1791+
let route_params =
1792+
RouteParameters::from_payment_params_and_value(payment_params.clone(), recv_value);
1793+
1794+
// Compute the route first to determine path ordering (probe results follow route path order).
1795+
let node_0_id = nodes[0].node.get_our_node_id();
1796+
let node_1_id = nodes[1].node.get_our_node_id();
1797+
let usable_channels = nodes[0].node.list_usable_channels();
1798+
let first_hops = usable_channels.iter().collect::<Vec<_>>();
1799+
let inflight_htlcs = nodes[0].node.compute_inflight_htlcs();
1800+
let route = nodes[0]
1801+
.router
1802+
.router
1803+
.find_route(&node_0_id, &route_params, Some(&first_hops), inflight_htlcs)
1804+
.unwrap();
17861805

1787-
let expected_route: &[(&[&Node], PaymentHash)] =
1788-
&[(&[&nodes[1], &nodes[3]], res[0].0), (&[&nodes[2], &nodes[3]], res[1].0)];
1806+
// Determine which path index goes to node 1 vs node 2
1807+
let (path_to_1_idx, path_to_2_idx) =
1808+
if route.paths[0].hops[0].pubkey == node_1_id { (0, 1) } else { (1, 0) };
17891809

1790-
assert_eq!(res.len(), expected_route.len());
1810+
let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
1811+
assert_eq!(res.len(), 2);
1812+
1813+
let expected_route: &[(&[&Node], PaymentHash)] = &[
1814+
(&[&nodes[1], &nodes[3]], res[path_to_1_idx].0),
1815+
(&[&nodes[2], &nodes[3]], res[path_to_2_idx].0),
1816+
];
17911817

17921818
send_probe_along_route(&nodes[0], expected_route);
17931819

@@ -2029,29 +2055,40 @@ fn test_trivial_inflight_htlc_tracking() {
20292055
// Send and claim the payment. Inflight HTLCs should be empty.
20302056
let (_, payment_hash, _, payment_id) = send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 500000);
20312057
let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs();
2032-
{
2033-
let mut per_peer_lock;
2034-
let mut peer_state_lock;
2035-
let channel_1 =
2036-
get_channel_ref!(&nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan_1_id);
20372058

2059+
// Inflight HTLCs are tracked by the SCID used in routes:
2060+
// - First hop uses alias via get_outbound_payment_scid()
2061+
// - Subsequent hops use the real SCID from gossip/network graph
2062+
let chan_1_scid = nodes[0]
2063+
.node
2064+
.list_channels()
2065+
.iter()
2066+
.find(|c| c.channel_id == chan_1_id)
2067+
.unwrap()
2068+
.get_outbound_payment_scid()
2069+
.unwrap();
2070+
let chan_2_scid = nodes[1]
2071+
.node
2072+
.list_channels()
2073+
.iter()
2074+
.find(|c| c.channel_id == chan_2_id)
2075+
.unwrap()
2076+
.short_channel_id
2077+
.unwrap();
2078+
2079+
{
20382080
let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
20392081
&NodeId::from_pubkey(&node_a_id),
20402082
&NodeId::from_pubkey(&node_b_id),
2041-
channel_1.funding().get_short_channel_id().unwrap(),
2083+
chan_1_scid,
20422084
);
20432085
assert_eq!(chan_1_used_liquidity, None);
20442086
}
20452087
{
2046-
let mut per_peer_lock;
2047-
let mut peer_state_lock;
2048-
let channel_2 =
2049-
get_channel_ref!(&nodes[1], nodes[2], per_peer_lock, peer_state_lock, chan_2_id);
2050-
20512088
let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
20522089
&NodeId::from_pubkey(&node_b_id),
20532090
&NodeId::from_pubkey(&node_c_id),
2054-
channel_2.funding().get_short_channel_id().unwrap(),
2091+
chan_2_scid,
20552092
);
20562093

20572094
assert_eq!(chan_2_used_liquidity, None);
@@ -2071,29 +2108,19 @@ fn test_trivial_inflight_htlc_tracking() {
20712108
route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 500000);
20722109
let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs();
20732110
{
2074-
let mut per_peer_lock;
2075-
let mut peer_state_lock;
2076-
let channel_1 =
2077-
get_channel_ref!(&nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan_1_id);
2078-
20792111
let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
20802112
&NodeId::from_pubkey(&node_a_id),
20812113
&NodeId::from_pubkey(&node_b_id),
2082-
channel_1.funding().get_short_channel_id().unwrap(),
2114+
chan_1_scid,
20832115
);
20842116
// First hop accounts for expected 1000 msat fee
20852117
assert_eq!(chan_1_used_liquidity, Some(501000));
20862118
}
20872119
{
2088-
let mut per_peer_lock;
2089-
let mut peer_state_lock;
2090-
let channel_2 =
2091-
get_channel_ref!(&nodes[1], nodes[2], per_peer_lock, peer_state_lock, chan_2_id);
2092-
20932120
let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
20942121
&NodeId::from_pubkey(&node_b_id),
20952122
&NodeId::from_pubkey(&node_c_id),
2096-
channel_2.funding().get_short_channel_id().unwrap(),
2123+
chan_2_scid,
20972124
);
20982125

20992126
assert_eq!(chan_2_used_liquidity, Some(500000));
@@ -2113,28 +2140,18 @@ fn test_trivial_inflight_htlc_tracking() {
21132140

21142141
let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs();
21152142
{
2116-
let mut per_peer_lock;
2117-
let mut peer_state_lock;
2118-
let channel_1 =
2119-
get_channel_ref!(&nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan_1_id);
2120-
21212143
let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
21222144
&NodeId::from_pubkey(&node_a_id),
21232145
&NodeId::from_pubkey(&node_b_id),
2124-
channel_1.funding().get_short_channel_id().unwrap(),
2146+
chan_1_scid,
21252147
);
21262148
assert_eq!(chan_1_used_liquidity, None);
21272149
}
21282150
{
2129-
let mut per_peer_lock;
2130-
let mut peer_state_lock;
2131-
let channel_2 =
2132-
get_channel_ref!(&nodes[1], nodes[2], per_peer_lock, peer_state_lock, chan_2_id);
2133-
21342151
let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat(
21352152
&NodeId::from_pubkey(&node_b_id),
21362153
&NodeId::from_pubkey(&node_c_id),
2137-
channel_2.funding().get_short_channel_id().unwrap(),
2154+
chan_2_scid,
21382155
);
21392156
assert_eq!(chan_2_used_liquidity, None);
21402157
}
@@ -2176,15 +2193,16 @@ fn test_holding_cell_inflight_htlcs() {
21762193
let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs();
21772194

21782195
{
2179-
let mut per_peer_lock;
2180-
let mut peer_state_lock;
2181-
let channel =
2182-
get_channel_ref!(&nodes[0], nodes[1], per_peer_lock, peer_state_lock, channel_id);
2196+
// Inflight HTLCs are tracked by the SCID used in routes, which is now the alias.
2197+
// Use the route's SCID (from get_outbound_payment_scid) to query inflight liquidity.
2198+
let chan_details =
2199+
nodes[0].node.list_channels().into_iter().find(|c| c.channel_id == channel_id).unwrap();
2200+
let route_scid = chan_details.get_outbound_payment_scid().unwrap();
21832201

21842202
let used_liquidity = inflight_htlcs.used_liquidity_msat(
21852203
&NodeId::from_pubkey(&node_a_id),
21862204
&NodeId::from_pubkey(&node_b_id),
2187-
channel.funding().get_short_channel_id().unwrap(),
2205+
route_scid,
21882206
);
21892207

21902208
assert_eq!(used_liquidity, Some(2000000));

lightning/src/ln/priv_short_conf_tests.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,11 +1117,12 @@ fn test_0conf_channel_reorg() {
11171117
let bs_chans = nodes[1].node.list_usable_channels();
11181118
let bs_chan = bs_chans.iter().find(|chan| chan.counterparty.node_id == node_c_id).unwrap();
11191119
let original_scid = bs_chan.short_channel_id.unwrap();
1120+
let outbound_scid = bs_chan.get_outbound_payment_scid().unwrap();
11201121
assert_eq!(nodes[2].node.list_usable_channels()[0].short_channel_id.unwrap(), original_scid);
11211122

11221123
let (mut route, payment_hash, payment_preimage, payment_secret) =
11231124
get_route_and_payment_hash!(nodes[1], nodes[2], 10_000);
1124-
assert_eq!(route.paths[0].hops[0].short_channel_id, original_scid);
1125+
assert_eq!(route.paths[0].hops[0].short_channel_id, outbound_scid);
11251126
send_along_route_with_secret(
11261127
&nodes[1],
11271128
route.clone(),
@@ -1290,12 +1291,21 @@ fn test_0conf_channel_reorg() {
12901291

12911292
let onion = RecipientOnionFields::secret_only(payment_secret);
12921293
let id = PaymentId([0; 32]);
1293-
nodes[1].node.send_payment_with_route(route, payment_hash, onion.clone(), id).unwrap();
1294+
// Use the original (real) SCID in the route to test that it no longer works after the
1295+
// channel announcement propagation delay.
1296+
let mut old_scid_route = route.clone();
1297+
old_scid_route.paths[0].hops[0].short_channel_id = original_scid;
1298+
nodes[1].node.send_payment_with_route(old_scid_route, payment_hash, onion.clone(), id).unwrap();
12941299
let mut conditions = PaymentFailedConditions::new();
12951300
conditions.reason = Some(PaymentFailureReason::RouteNotFound);
12961301
expect_payment_failed_conditions(&nodes[1], payment_hash, false, conditions);
12971302

1298-
nodes[0].node.send_payment_with_route(forwarded_route, payment_hash, onion, id).unwrap();
1303+
let mut old_scid_forwarded_route = forwarded_route.clone();
1304+
old_scid_forwarded_route.paths[0].hops[1].short_channel_id = original_scid;
1305+
nodes[0]
1306+
.node
1307+
.send_payment_with_route(old_scid_forwarded_route, payment_hash, onion, id)
1308+
.unwrap();
12991309
check_added_monitors(&nodes[0], 1);
13001310
let mut ev = nodes[0].node.get_and_clear_pending_msg_events();
13011311
assert_eq!(ev.len(), 1);

lightning/src/routing/router.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9054,7 +9054,7 @@ mod tests {
90549054
assert_eq!(route.paths.len(), 1);
90559055
assert_eq!(route.get_total_amount(), amt_msat);
90569056
assert_eq!(route.paths[0].hops.len(), 2);
9057-
assert_eq!(route.paths[0].hops[0].short_channel_id, 1);
9057+
assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
90589058
assert_eq!(route.paths[0].hops[1].short_channel_id, 45);
90599059
assert_eq!(route.get_total_fees(), 123);
90609060
}

0 commit comments

Comments
 (0)