Prefer outbound SCID alias over real SCID when routing

lightning/src/ln/channel_state.rs

@@ -312,8 +312,8 @@ pub struct ChannelDetails {
 	/// Note that if [`inbound_scid_alias`] is set, it must be used for invoices and inbound
 	/// payments instead of this. See [`get_inbound_payment_scid`].
 	///
-	/// For channels with [`confirmations_required`] set to `Some(0)`, [`outbound_scid_alias`] may
-	/// be used in place of this in outbound routes. See [`get_outbound_payment_scid`].
+	/// [`outbound_scid_alias`] is preferred over this for outbound routes when set, as it remains
+	/// stable across splices. See [`get_outbound_payment_scid`].
 	///
 	/// When a channel is spliced, this continues to refer to the original pre-splice channel
 	/// state until the splice transaction reaches sufficient confirmations to be locked (and we
@@ -323,12 +323,10 @@ pub struct ChannelDetails {
 	/// [`outbound_scid_alias`]: Self::outbound_scid_alias
 	/// [`get_inbound_payment_scid`]: Self::get_inbound_payment_scid
 	/// [`get_outbound_payment_scid`]: Self::get_outbound_payment_scid
-	/// [`confirmations_required`]: Self::confirmations_required
 	pub short_channel_id: Option<u64>,
 	/// An optional [`short_channel_id`] alias for this channel, randomly generated by us and
-	/// usable in place of [`short_channel_id`] to reference the channel in outbound routes when
-	/// the channel has not yet been confirmed (as long as [`confirmations_required`] is
-	/// `Some(0)`).
+	/// preferred over [`short_channel_id`] for outbound routes when set. Unlike the real SCID,
+	/// the alias remains stable across splices.
 	///
 	/// This will be `None` as long as the channel is not available for routing outbound payments.
 	///
@@ -337,7 +335,6 @@ pub struct ChannelDetails {
 	/// exchange `splice_locked` messages with our peer).
 	///
 	/// [`short_channel_id`]: Self::short_channel_id
-	/// [`confirmations_required`]: Self::confirmations_required
 	pub outbound_scid_alias: Option<u64>,
 	/// An optional [`short_channel_id`] alias for this channel, randomly generated by our
 	/// counterparty and usable in place of [`short_channel_id`] in invoice route hints. Our
@@ -496,12 +493,13 @@ impl ChannelDetails {
 	/// This should be used in [`Route`]s to describe the first hop or in other contexts where
 	/// we're sending or forwarding a payment outbound over this channel.
 	///
-	/// This is either the [`ChannelDetails::short_channel_id`], if set, or the
-	/// [`ChannelDetails::outbound_scid_alias`]. See those for more information.
+	/// This is either the [`ChannelDetails::outbound_scid_alias`], if set, or the
+	/// [`ChannelDetails::short_channel_id`]. The alias is preferred because with splices,
+	/// the real SCID can change when a splice confirms, whereas the alias remains stable.
 	///
 	/// [`Route`]: crate::routing::router::Route
 	pub fn get_outbound_payment_scid(&self) -> Option<u64> {
-		self.short_channel_id.or(self.outbound_scid_alias)
+		self.outbound_scid_alias.or(self.short_channel_id)
 	}
 
 	/// Gets the funding output for this channel, if available.

lightning/src/ln/onion_route_tests.rs

@@ -816,7 +816,8 @@ fn test_onion_failure() {
 
 	// Our immediate peer sent UpdateFailMalformedHTLC because it couldn't understand the onion in
 	// the UpdateAddHTLC that we sent.
-	let short_channel_id = channels[0].0.contents.short_channel_id;
+	// The SCID in the failure should use the SCID alias as we specified in the route.
+	let short_channel_id = route.paths[0].hops[0].short_channel_id;
 	run_onion_failure_test(
 		"invalid_onion_version",
 		0,

lightning/src/ln/payment_tests.rs

@@ -247,13 +247,16 @@ fn mpp_retry_overpay() {
 	let (mut route, hash, payment_preimage, pay_secret) =
 		get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, amt_msat, max_fee);
 
-	// Check we overpay on the second path which we're about to fail.
+	// Check we overpay on the second path which we're about to fail. Path ordering is determined
+	// by alias SCID sort and is not stable, so we identify paths by first-hop pubkey.
 	assert_eq!(chan_1_update.contents.fee_proportional_millionths, 0);
-	let overpaid_amount_1 = route.paths[0].fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
+	let path_via_b = route.paths.iter().find(|p| p.hops[0].pubkey == node_b_id).unwrap();
+	let overpaid_amount_1 = path_via_b.fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
 	assert_eq!(overpaid_amount_1, 0);
 
 	assert_eq!(chan_2_update.contents.fee_proportional_millionths, 0);
-	let overpaid_amount_2 = route.paths[1].fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
+	let path_via_c = route.paths.iter().find(|p| p.hops[0].pubkey == node_c_id).unwrap();
+	let overpaid_amount_2 = path_via_c.fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
 
 	let total_overpaid_amount = overpaid_amount_1 + overpaid_amount_2;
 
@@ -301,11 +304,13 @@ fn mpp_retry_overpay() {
 	// Rebalance the channel so the second half of the payment can succeed.
 	send_payment(&nodes[3], &[&nodes[2]], 38_000_000);
 
-	// Retry the second half of the payment and make sure it succeeds.
-	let first_path_value = route.paths[0].final_value_msat();
+	// Retry the second half of the payment and make sure it succeeds. Identify the successful
+	// path (through nodes[1]) by first-hop pubkey, since path ordering is not stable.
+	let path_via_b_idx = route.paths.iter().position(|p| p.hops[0].pubkey == node_b_id).unwrap();
+	let first_path_value = route.paths[path_via_b_idx].final_value_msat();
 	assert_eq!(first_path_value, 36_000_000);
 
-	route.paths.remove(0);
+	route.paths.remove(path_via_b_idx);
 	route_params.final_value_msat -= first_path_value;
 	let chan_4_scid = chan_4_update.contents.short_channel_id;
 	route_params.payment_params.previously_failed_channels.push(chan_4_scid);
@@ -1825,8 +1830,18 @@ fn preflight_probes_yield_event() {
 	let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
 	let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
 
+	// Path order in route.paths is by alias SCID (ascending). Determine which res entry
+	// corresponds to which path by comparing alias SCIDs.
+	let node_b_id = nodes[1].node.get_our_node_id();
+	let node_c_id = nodes[2].node.get_our_node_id();
+	let chans = nodes[0].node.list_usable_channels();
+	let chan_to_b = chans.iter().find(|c| c.counterparty.node_id == node_b_id).unwrap();
+	let chan_to_c = chans.iter().find(|c| c.counterparty.node_id == node_c_id).unwrap();
+	let b_first = chan_to_b.get_outbound_payment_scid() < chan_to_c.get_outbound_payment_scid();
+	let (hash_b, hash_c) = if b_first { (res[0].0, res[1].0) } else { (res[1].0, res[0].0) };
+
 	let expected_route: &[(&[&Node], PaymentHash)] =
-		&[(&[&nodes[1], &nodes[3]], res[0].0), (&[&nodes[2], &nodes[3]], res[1].0)];
+		&[(&[&nodes[1], &nodes[3]], hash_b), (&[&nodes[2], &nodes[3]], hash_c)];
 
 	assert_eq!(res.len(), expected_route.len());
 
@@ -2121,7 +2136,7 @@ fn test_trivial_inflight_htlc_tracking() {
 		let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&node_a_id),
 			&NodeId::from_pubkey(&node_b_id),
-			channel_1.funding().get_short_channel_id().unwrap(),
+			channel_1.context().outbound_scid_alias(),
 		);
 		// First hop accounts for expected 1000 msat fee
 		assert_eq!(chan_1_used_liquidity, Some(501000));
@@ -2226,7 +2241,7 @@ fn test_holding_cell_inflight_htlcs() {
 		let used_liquidity = inflight_htlcs.used_liquidity_msat(
 			&NodeId::from_pubkey(&node_a_id),
 			&NodeId::from_pubkey(&node_b_id),
-			channel.funding().get_short_channel_id().unwrap(),
+			channel.context().outbound_scid_alias(),
 		);
 
 		assert_eq!(used_liquidity, Some(2000000));

lightning/src/ln/priv_short_conf_tests.rs

@@ -1108,16 +1108,16 @@ fn test_0conf_channel_reorg() {
 	mine_transaction(&nodes[1], &tx);
 	mine_transaction(&nodes[2], &tx);
 
-	// Send a payment using the channel's real SCID, which will be public in a few blocks once we
-	// can generate a channel_announcement.
+	// Send a payment using the channel's alias SCID. The channel itself will be public in a few
+	// blocks once we can generate a channel_announcement.
 	let bs_chans = nodes[1].node.list_usable_channels();
 	let bs_chan = bs_chans.iter().find(|chan| chan.counterparty.node_id == node_c_id).unwrap();
 	let original_scid = bs_chan.short_channel_id.unwrap();
 	assert_eq!(nodes[2].node.list_usable_channels()[0].short_channel_id.unwrap(), original_scid);
 
 	let (mut route, payment_hash, payment_preimage, payment_secret) =
 		get_route_and_payment_hash!(nodes[1], nodes[2], 10_000);
-	assert_eq!(route.paths[0].hops[0].short_channel_id, original_scid);
+	assert_eq!(route.paths[0].hops[0].short_channel_id, bs_chan.outbound_scid_alias.unwrap());
 	send_along_route_with_secret(
 		&nodes[1],
 		route.clone(),
@@ -1286,12 +1286,22 @@ fn test_0conf_channel_reorg() {
 
 	let onion = RecipientOnionFields::secret_only(payment_secret, 10_000);
 	let id = PaymentId([0; 32]);
-	nodes[1].node.send_payment_with_route(route, payment_hash, onion.clone(), id).unwrap();
+
+	// The route uses the alias SCID, which is stable across reorgs. To verify the old real SCID
+	// is invalidated after propagation delay, we explicitly build a route using original_scid.
+	let mut old_scid_route = route.clone();
+	old_scid_route.paths[0].hops[0].short_channel_id = original_scid;
+	nodes[1].node.send_payment_with_route(old_scid_route, payment_hash, onion.clone(), id).unwrap();
 	let mut conditions = PaymentFailedConditions::new();
 	conditions.reason = Some(PaymentFailureReason::RouteNotFound);
 	expect_payment_failed_conditions(&nodes[1], payment_hash, false, conditions);
 
-	nodes[0].node.send_payment_with_route(forwarded_route, payment_hash, onion, id).unwrap();
+	let mut old_scid_forwarded_route = forwarded_route.clone();
+	old_scid_forwarded_route.paths[0].hops[1].short_channel_id = original_scid;
+	nodes[0]
+		.node
+		.send_payment_with_route(old_scid_forwarded_route, payment_hash, onion, id)
+		.unwrap();
 	check_added_monitors(&nodes[0], 1);
 	let mut ev = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(ev.len(), 1);

lightning/src/ln/splicing_tests.rs

@@ -2678,7 +2678,7 @@ fn fail_splice_on_tx_complete_error() {
 
 	// Queue an outgoing HTLC to the holding cell. It should be freed once we exit quiescence.
 	let (route, payment_hash, _payment_preimage, payment_secret) =
-		get_route_and_payment_hash!(initiator, acceptor, 1_000_000);
+		get_route_and_payment_hash!(acceptor, initiator, 1_000_000);
 	let onion = RecipientOnionFields::secret_only(payment_secret, 1_000_000);
 	let payment_id = PaymentId(payment_hash.0);
 	acceptor.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();

lightning/src/routing/router.rs

@@ -9178,7 +9178,7 @@ mod tests {
 		assert_eq!(route.paths.len(), 1);
 		assert_eq!(route.get_total_amount(), amt_msat);
 		assert_eq!(route.paths[0].hops.len(), 2);
-		assert_eq!(route.paths[0].hops[0].short_channel_id, 1);
+		assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
 		assert_eq!(route.paths[0].hops[1].short_channel_id, 45);
 		assert_eq!(route.get_total_fees(), 123);
 	}