Prefer outbound SCID alias over real SCID when routing by Alkamal01 · Pull Request #4567 · lightningdevkit/rust-lightning

Alkamal01 · 2026-04-14T10:25:28Z

With splices, channels may spontaneously change SCIDs, making our assumption that SCIDs were more stable than SCID aliases in ChannelDetails::get_outbound_payment_scid wrong. This swaps which we prefer, and fixes up the tests that break as a result.

ldk-reviews-bot · 2026-04-14T10:25:31Z

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignment, please click here.

ldk-claude-review-bot · 2026-04-14T10:34:55Z

.gitignore

 no-std-check/target
 msrv-no-dev-deps-check/target
 lightning-tests/target
+.claude


Nit: This .claude gitignore entry is unrelated to the SCID alias change. Consider dropping it from this PR and submitting it separately (or as part of a repo-housekeeping commit).

ldk-claude-review-bot · 2026-04-14T10:34:56Z

lightning/src/ln/payment_tests.rs

 	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();

+	assert_eq!(res.len(), 2);


assert_eq!(res.len(), 2) is asserted here and again on line 1814 (assert_eq!(res.len(), expected_route.len())). The first one is redundant — consider removing one of them.

ldk-claude-review-bot · 2026-04-14T10:37:02Z

I've thoroughly reviewed every file and hunk in the diff, including examining the surrounding code for context. Let me also verify one thing about the field-level doc updates that I flagged in my prior review.

The field-level docs on short_channel_id (line 315) and outbound_scid_alias (lines 328-329) were actually already updated in this PR's diff — my prior comment was inaccurate. Those docs now correctly describe the alias as "preferred over" the real SCID, not as a 0-conf fallback.

No new bugs, security issues, or logic errors found.

Review Summary

All code changes are logically correct. The core preference swap in get_outbound_payment_scid() is sound, and all test updates properly adapt:

channel_state.rs: Core swap and doc updates are correct.
onion_route_tests.rs: Correctly uses the route's SCID (now alias) for failure matching instead of the channel announcement's real SCID.
payment_tests.rs: MPP tests properly identify paths by pubkey rather than assuming index order; inflight HTLC lookups correctly use alias for first hops; preflight probes test correctly determines path-to-result mapping via alias SCID comparison.
priv_short_conf_tests.rs: Reorg test correctly verifies alias-based routes work while explicitly constructing old-SCID routes to test invalidation.
splicing_tests.rs: Correctly fixes route to be built from sender's (acceptor's) perspective, since the sender's short_to_chan_info only contains its own outbound alias, not the peer's.
router.rs: Test correctly expects alias (44) instead of real SCID (1).

Prior review comments status:

priv_short_conf_tests.rs:1111-1112 — addressed (comment now correctly says "The channel itself will be public")
channel_state.rs:498 (stale field docs) — was already addressed in the diff; my prior comment was inaccurate
payment_tests.rs:1798 (redundant assert) — still applicable, minor nit

No new inline comments posted. No issues found.

TheBlueMatt

Thanks! One nit about comments but otherwise LGTM.

TheBlueMatt · 2026-04-14T13:24:30Z

lightning/src/ln/onion_route_tests.rs

 	// 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;
+	// get_outbound_payment_scid now prefers the alias SCID for the first hop, so the


claude loves to leave comments that describe what it did, describing things in terms of what it is changing and how things used to be, but that isn't actually a good comment. I'm not sure this needs a comment, but if it does it shouldn't be phrased as "X now does" but rather simply "The SCID in the failure should use the SCID alias as we specified in the route". This class of issues is repeated in a few places.

ldk-claude-review-bot · 2026-04-14T14:55:53Z

lightning/src/ln/channel_state.rs

 	///
-	/// 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.


Nit: The docstrings on short_channel_id (line 315-316) and outbound_scid_alias (lines 328-331) are now stale. They still say the alias is only used "in place of" the real SCID for 0-conf channels, but with this change the alias is now the preferred identifier for all outbound routes, not just a 0-conf fallback. Consider updating them to match the new semantics described here.

ldk-claude-review-bot · 2026-04-14T15:23:03Z

lightning/src/ln/priv_short_conf_tests.rs

+	// Send a payment using the channel's alias SCID, which will be public in a few blocks once
+	// we can generate a channel_announcement.


Nit: The comment now reads "Send a payment using the channel's alias SCID, which will be public in a few blocks…" — but the alias SCID itself never becomes public. It's the channel that becomes public (announced via the real SCID in channel_announcement). The original wording ("real SCID, which will be public") was accurate for the real SCID; consider rewording to avoid implying the alias is what gets announced:

Suggested change

// Send a payment using the channel's alias 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.

…ent_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.

codecov · 2026-04-14T17:06:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.00%. Comparing base (2ebc372) to head (841d912).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4567      +/-   ##
==========================================
- Coverage   87.01%   87.00%   -0.02%     
==========================================
  Files         163      163              
  Lines      108682   108742      +60     
  Branches   108682   108742      +60     
==========================================
+ Hits        94572    94613      +41     
- Misses      11631    11648      +17     
- Partials     2479     2481       +2

Flag	Coverage Δ
fuzzing	`40.19% <100.00%> (-0.04%)`	⬇️
tests	`86.09% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ldk-claude-review-bot reviewed Apr 14, 2026

View reviewed changes

ldk-reviews-bot requested a review from wpaulino April 14, 2026 10:35

Alkamal01 force-pushed the prefer-outbound-scid-alias-for-routing branch from b5156c6 to 181df48 Compare April 14, 2026 10:40

TheBlueMatt reviewed Apr 14, 2026

View reviewed changes

TheBlueMatt removed the request for review from wpaulino April 14, 2026 13:38

Alkamal01 force-pushed the prefer-outbound-scid-alias-for-routing branch from 181df48 to 1ee9685 Compare April 14, 2026 14:52

ldk-claude-review-bot reviewed Apr 14, 2026

View reviewed changes

Alkamal01 force-pushed the prefer-outbound-scid-alias-for-routing branch from 1ee9685 to c49f1b4 Compare April 14, 2026 15:18

ldk-claude-review-bot reviewed Apr 14, 2026

View reviewed changes

Alkamal01 force-pushed the prefer-outbound-scid-alias-for-routing branch from c49f1b4 to 841d912 Compare April 14, 2026 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prefer outbound SCID alias over real SCID when routing#4567

Prefer outbound SCID alias over real SCID when routing#4567
Alkamal01 wants to merge 1 commit intolightningdevkit:mainfrom
Alkamal01:prefer-outbound-scid-alias-for-routing

Alkamal01 commented Apr 14, 2026

Uh oh!

ldk-reviews-bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Uh oh!

ldk-claude-review-bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

TheBlueMatt left a comment

Uh oh!

TheBlueMatt Apr 14, 2026

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Uh oh!

codecov bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// Send a payment using the channel's alias SCID, which will be public in a few blocks once
		// we can generate a channel_announcement.

Conversation

Alkamal01 commented Apr 14, 2026

Uh oh!

ldk-reviews-bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Apr 14, 2026 •

edited

Loading

ldk-claude-review-bot commented Apr 14, 2026 •

edited

Loading

codecov bot commented Apr 14, 2026 •

edited

Loading