Skip to content

Commit 1ee9685

Browse files
committed
Prefer outbound_scid_alias over short_channel_id in get_outbound_payment_scid
With splicing, the real SCID changes when a splice confirms while the alias remains stable. Prefer the alias so routes stay valid across splice confirmations. Update all tests that depended on the previous (real-SCID-first) behavior, including inflight HTLC tracking lookups and multi-path route ordering.
1 parent 9f73a98 commit 1ee9685

6 files changed

Lines changed: 49 additions & 20 deletions

File tree

lightning/src/ln/channel_state.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,12 +496,13 @@ impl ChannelDetails {
496496
/// This should be used in [`Route`]s to describe the first hop or in other contexts where
497497
/// we're sending or forwarding a payment outbound over this channel.
498498
///
499-
/// This is either the [`ChannelDetails::short_channel_id`], if set, or the
500-
/// [`ChannelDetails::outbound_scid_alias`]. See those for more information.
499+
/// This is either the [`ChannelDetails::outbound_scid_alias`], if set, or the
500+
/// [`ChannelDetails::short_channel_id`]. The alias is preferred because with splices,
501+
/// the real SCID can change when a splice confirms, whereas the alias remains stable.
501502
///
502503
/// [`Route`]: crate::routing::router::Route
503504
pub fn get_outbound_payment_scid(&self) -> Option<u64> {
504-
self.short_channel_id.or(self.outbound_scid_alias)
505+
self.outbound_scid_alias.or(self.short_channel_id)
505506
}
506507

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

lightning/src/ln/onion_route_tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,8 @@ fn test_onion_failure() {
816816

817817
// Our immediate peer sent UpdateFailMalformedHTLC because it couldn't understand the onion in
818818
// the UpdateAddHTLC that we sent.
819-
let short_channel_id = channels[0].0.contents.short_channel_id;
819+
// The SCID in the failure matches the alias SCID we specified in the route.
820+
let short_channel_id = route.paths[0].hops[0].short_channel_id;
820821
run_onion_failure_test(
821822
"invalid_onion_version",
822823
0,

lightning/src/ln/payment_tests.rs

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,16 @@ fn mpp_retry_overpay() {
247247
let (mut route, hash, payment_preimage, pay_secret) =
248248
get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, amt_msat, max_fee);
249249

250-
// Check we overpay on the second path which we're about to fail.
250+
// Check we overpay on the second path which we're about to fail. Path ordering is determined
251+
// by alias SCID sort and is not stable, so we identify paths by first-hop pubkey.
251252
assert_eq!(chan_1_update.contents.fee_proportional_millionths, 0);
252-
let overpaid_amount_1 = route.paths[0].fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
253+
let path_via_b = route.paths.iter().find(|p| p.hops[0].pubkey == node_b_id).unwrap();
254+
let overpaid_amount_1 = path_via_b.fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
253255
assert_eq!(overpaid_amount_1, 0);
254256

255257
assert_eq!(chan_2_update.contents.fee_proportional_millionths, 0);
256-
let overpaid_amount_2 = route.paths[1].fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
258+
let path_via_c = route.paths.iter().find(|p| p.hops[0].pubkey == node_c_id).unwrap();
259+
let overpaid_amount_2 = path_via_c.fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
257260

258261
let total_overpaid_amount = overpaid_amount_1 + overpaid_amount_2;
259262

@@ -301,11 +304,13 @@ fn mpp_retry_overpay() {
301304
// Rebalance the channel so the second half of the payment can succeed.
302305
send_payment(&nodes[3], &[&nodes[2]], 38_000_000);
303306

304-
// Retry the second half of the payment and make sure it succeeds.
305-
let first_path_value = route.paths[0].final_value_msat();
307+
// Retry the second half of the payment and make sure it succeeds. Identify the successful
308+
// path (through nodes[1]) by first-hop pubkey, since path ordering is not stable.
309+
let path_via_b_idx = route.paths.iter().position(|p| p.hops[0].pubkey == node_b_id).unwrap();
310+
let first_path_value = route.paths[path_via_b_idx].final_value_msat();
306311
assert_eq!(first_path_value, 36_000_000);
307312

308-
route.paths.remove(0);
313+
route.paths.remove(path_via_b_idx);
309314
route_params.final_value_msat -= first_path_value;
310315
let chan_4_scid = chan_4_update.contents.short_channel_id;
311316
route_params.payment_params.previously_failed_channels.push(chan_4_scid);
@@ -1790,8 +1795,18 @@ fn preflight_probes_yield_event() {
17901795
let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
17911796
let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
17921797

1798+
// Path order in route.paths is by alias SCID (ascending). Determine which res entry
1799+
// corresponds to which path by comparing alias SCIDs.
1800+
let node_b_id = nodes[1].node.get_our_node_id();
1801+
let node_c_id = nodes[2].node.get_our_node_id();
1802+
let chans = nodes[0].node.list_usable_channels();
1803+
let chan_to_b = chans.iter().find(|c| c.counterparty.node_id == node_b_id).unwrap();
1804+
let chan_to_c = chans.iter().find(|c| c.counterparty.node_id == node_c_id).unwrap();
1805+
let b_first = chan_to_b.get_outbound_payment_scid() < chan_to_c.get_outbound_payment_scid();
1806+
let (hash_b, hash_c) = if b_first { (res[0].0, res[1].0) } else { (res[1].0, res[0].0) };
1807+
17931808
let expected_route: &[(&[&Node], PaymentHash)] =
1794-
&[(&[&nodes[1], &nodes[3]], res[0].0), (&[&nodes[2], &nodes[3]], res[1].0)];
1809+
&[(&[&nodes[1], &nodes[3]], hash_b), (&[&nodes[2], &nodes[3]], hash_c)];
17951810

17961811
assert_eq!(res.len(), expected_route.len());
17971812

@@ -2086,7 +2101,7 @@ fn test_trivial_inflight_htlc_tracking() {
20862101
let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
20872102
&NodeId::from_pubkey(&node_a_id),
20882103
&NodeId::from_pubkey(&node_b_id),
2089-
channel_1.funding().get_short_channel_id().unwrap(),
2104+
channel_1.context().outbound_scid_alias(),
20902105
);
20912106
// First hop accounts for expected 1000 msat fee
20922107
assert_eq!(chan_1_used_liquidity, Some(501000));
@@ -2191,7 +2206,7 @@ fn test_holding_cell_inflight_htlcs() {
21912206
let used_liquidity = inflight_htlcs.used_liquidity_msat(
21922207
&NodeId::from_pubkey(&node_a_id),
21932208
&NodeId::from_pubkey(&node_b_id),
2194-
channel.funding().get_short_channel_id().unwrap(),
2209+
channel.context().outbound_scid_alias(),
21952210
);
21962211

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

lightning/src/ln/priv_short_conf_tests.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,16 +1108,16 @@ fn test_0conf_channel_reorg() {
11081108
mine_transaction(&nodes[1], &tx);
11091109
mine_transaction(&nodes[2], &tx);
11101110

1111-
// Send a payment using the channel's real SCID, which will be public in a few blocks once we
1112-
// can generate a channel_announcement.
1111+
// Send a payment using the channel's alias SCID, which will be public in a few blocks once
1112+
// we can generate a channel_announcement.
11131113
let bs_chans = nodes[1].node.list_usable_channels();
11141114
let bs_chan = bs_chans.iter().find(|chan| chan.counterparty.node_id == node_c_id).unwrap();
11151115
let original_scid = bs_chan.short_channel_id.unwrap();
11161116
assert_eq!(nodes[2].node.list_usable_channels()[0].short_channel_id.unwrap(), original_scid);
11171117

11181118
let (mut route, payment_hash, payment_preimage, payment_secret) =
11191119
get_route_and_payment_hash!(nodes[1], nodes[2], 10_000);
1120-
assert_eq!(route.paths[0].hops[0].short_channel_id, original_scid);
1120+
assert_eq!(route.paths[0].hops[0].short_channel_id, bs_chan.outbound_scid_alias.unwrap());
11211121
send_along_route_with_secret(
11221122
&nodes[1],
11231123
route.clone(),
@@ -1286,12 +1286,22 @@ fn test_0conf_channel_reorg() {
12861286

12871287
let onion = RecipientOnionFields::secret_only(payment_secret, 10_000);
12881288
let id = PaymentId([0; 32]);
1289-
nodes[1].node.send_payment_with_route(route, payment_hash, onion.clone(), id).unwrap();
1289+
1290+
// The route uses the alias SCID, which is stable across reorgs. To verify the old real SCID
1291+
// is invalidated after propagation delay, we explicitly build a route using original_scid.
1292+
let mut old_scid_route = route.clone();
1293+
old_scid_route.paths[0].hops[0].short_channel_id = original_scid;
1294+
nodes[1].node.send_payment_with_route(old_scid_route, payment_hash, onion.clone(), id).unwrap();
12901295
let mut conditions = PaymentFailedConditions::new();
12911296
conditions.reason = Some(PaymentFailureReason::RouteNotFound);
12921297
expect_payment_failed_conditions(&nodes[1], payment_hash, false, conditions);
12931298

1294-
nodes[0].node.send_payment_with_route(forwarded_route, payment_hash, onion, id).unwrap();
1299+
let mut old_scid_forwarded_route = forwarded_route.clone();
1300+
old_scid_forwarded_route.paths[0].hops[1].short_channel_id = original_scid;
1301+
nodes[0]
1302+
.node
1303+
.send_payment_with_route(old_scid_forwarded_route, payment_hash, onion, id)
1304+
.unwrap();
12951305
check_added_monitors(&nodes[0], 1);
12961306
let mut ev = nodes[0].node.get_and_clear_pending_msg_events();
12971307
assert_eq!(ev.len(), 1);

lightning/src/ln/splicing_tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2677,8 +2677,10 @@ fn fail_splice_on_tx_complete_error() {
26772677
let _ = complete_splice_handshake(initiator, acceptor);
26782678

26792679
// Queue an outgoing HTLC to the holding cell. It should be freed once we exit quiescence.
2680+
// Build the route from acceptor's perspective so the first-hop SCID is acceptor's own
2681+
// outbound alias, which is what acceptor's channel lookup uses.
26802682
let (route, payment_hash, _payment_preimage, payment_secret) =
2681-
get_route_and_payment_hash!(initiator, acceptor, 1_000_000);
2683+
get_route_and_payment_hash!(acceptor, initiator, 1_000_000);
26822684
let onion = RecipientOnionFields::secret_only(payment_secret, 1_000_000);
26832685
let payment_id = PaymentId(payment_hash.0);
26842686
acceptor.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();

lightning/src/routing/router.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9178,7 +9178,7 @@ mod tests {
91789178
assert_eq!(route.paths.len(), 1);
91799179
assert_eq!(route.get_total_amount(), amt_msat);
91809180
assert_eq!(route.paths[0].hops.len(), 2);
9181-
assert_eq!(route.paths[0].hops[0].short_channel_id, 1);
9181+
assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
91829182
assert_eq!(route.paths[0].hops[1].short_channel_id, 45);
91839183
assert_eq!(route.get_total_fees(), 123);
91849184
}

0 commit comments

Comments
 (0)