Add BOLT12 support to LSPS2 via custom `Router` implementation by tnull · Pull Request #4463 · lightningdevkit/rust-lightning

tnull · 2026-03-05T13:36:46Z

Closes #4272.

This is an alternative approach to #4394 which leverages a custom Router implementation on the client side to inject the respective.

LDK Node integration PR over at lightningdevkit/ldk-node#817

ldk-reviews-bot · 2026-03-05T13:36:49Z

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-reviews-bot · 2026-03-07T13:37:42Z

🔔 1st Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-03-09T13:38:41Z

🔔 2nd Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-claude-review-bot · 2026-03-24T14:38:31Z

Review Summary — PR #4463 (Review pass 12)

One new issue found in the changelog file. All previously identified code bugs remain fixed.

Inline comments posted

pending_changelog/4463-LSPS2-BOLT12.txt:1-7 — Backward compatibility note has wrong type name (OnionMessageIntercepted::new_with_offline_peer_interception should be OnionMessenger::...) and the claim that downgrading will cause startup deserialization failures is inaccurate (these events are not persisted, and tag 37 is odd/safely skippable).

Verified prior fixes

All critical bugs from prior review passes confirmed resolved in this revision:

Deadlock in htlc_intercepted (cleanup_intercept_scids removed entirely)
InvoiceRequestReceived even event number (event removed)
pending_released_async_htlcs race condition (field removed)
? discarding valid paths in router loop (uses continue)
Empty paths returning Ok(vec![]) (guard added)
Type mismatches in tests (cltv_expiry_delta is u16 throughout)

Previously posted nits still applicable (not reposted)

router.rs:55 — Unused [Event::HTLCIntercepted] rustdoc link definition
router.rs:70-73 — Redundant intercept_scid parameter (mitigated by debug_assert_eq!)

codecov · 2026-03-24T16:53:41Z

Codecov Report

❌ Patch coverage is 90.26549% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.08%. Comparing base (066859b) to head (a0bcfcf).
⚠️ Report is 2911 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-liquidity/src/lsps2/router.rs	92.98%	27 Missing ⚠️
lightning/src/events/mod.rs	23.07%	9 Missing and 1 partial ⚠️
lightning/src/ln/channelmanager.rs	84.00%	4 Missing ⚠️
lightning/src/onion_message/messenger.rs	81.81%	2 Missing ⚠️
lightning/src/util/test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4463      +/-   ##
==========================================
+ Coverage   84.55%   87.08%   +2.52%     
==========================================
  Files         135      162      +27     
  Lines       76569   109690   +33121     
  Branches    76569   109690   +33121     
==========================================
+ Hits        64745    95519   +30774     
- Misses       9783    11688    +1905     
- Partials     2041     2483     +442

Flag	Coverage Δ
fuzzing-fake-hashes	`30.92% <0.00%> (?)`
fuzzing-real-hashes	`22.66% <6.15%> (?)`
tests	`86.18% <90.26%> (?)`

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.

jkczyz · 2026-04-09T20:51:31Z

-/// For **payment** blinded paths (in invoices), it returns the intercept SCID as the forwarding
-/// hop so that the LSP can intercept the HTLC and open a JIT channel.
+/// For **payment** blinded paths (in invoices), it appends paths using the intercept SCID as the
+/// forwarding hop so that the LSP can intercept the HTLC and open a JIT channel. Paths from the
+/// inner router (e.g., through pre-existing channels) are included as well, allowing payers to
+/// use existing inbound liquidity when available.


Seeing this in f538153. Should it belong in an earlier commit?

jkczyz · 2026-04-09T21:02:23Z

 pub struct TestMessageRouter<'a> {
 	pub inner: TestMessageRouterInternal<'a>,
 	pub peers_override: Mutex<Vec<PublicKey>>,
+	pub forward_node_scid_override: Mutex<HashMap<PublicKey, u64>>,


Is the TestMessageRouter changed needed anymore?

Hmm, we still use it in bolt12_lsps2_compact_message_path_test to test SCID-based interception. Do you have something else in mind?

Nah, just missed that.

jkczyz · 2026-04-09T21:06:25Z

+/// Override closure type for [`TestRouter::override_create_blinded_payment_paths`].
+///
+/// This closure is called instead of the default [`Router::create_blinded_payment_paths`]
+/// implementation when set, receiving the actual [`ReceiveTlvs`] so tests can construct custom
+/// blinded payment paths using the same TLVs the caller generated.
+pub type BlindedPaymentPathOverrideFn = Box<
+	dyn Fn(
+			PublicKey,
+			ReceiveAuthKey,
+			Vec<ChannelDetails>,
+			ReceiveTlvs,
+			Option<u64>,
+		) -> Result<Vec<BlindedPaymentPath>, ()>
+		+ Send
+		+ Sync,
+>;


Hmm... maybe we can have TestRouter wrap a dyn Router instead and provide a way to override it?

I had explored this before, but unfortunately Router is not dyn-compatible (due to the generic type-parameter on create_blinded_payment_paths), and making it a parameter seemed like a pretty invasive change to our test code (i.e., many places need changing, as it would bubble up).

That said, maybe it would be preferable to change Router to be dyn compatible, also so that in LDK Node we could determine the used router at build time? Though that seems a bit out-of-scope for this pr?

jkczyz

Feel free to squash.

jkczyz · 2026-04-14T03:45:15Z

 pub struct TestMessageRouter<'a> {
 	pub inner: TestMessageRouterInternal<'a>,
 	pub peers_override: Mutex<Vec<PublicKey>>,
+	pub forward_node_scid_override: Mutex<HashMap<PublicKey, u64>>,


Nah, just missed that.

Introduce `LSPS2BOLT12Router` to allow registering LSPS2 invoice parameters and build blinded payment paths through the negotiated intercept SCIDs. All other routing behavior still delegates to the wrapped router. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

Describe how `InvoiceParametersReady` feeds both the existing `BOLT11` route-hint flow and the new `LSPS2BOLT12Router` registration path for `BOLT12` offers. Co-Authored-By: HAL 9000

Exercise the LSPS2 buy flow and assert that registered LSPS2 parameters produce a blinded payment path whose first forwarding hop uses the negotiated intercept SCID. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

Allow tests to inject a custom `create_blinded_payment_paths` hook while preserving the normal `ReceiveTlvs` bindings. This makes it possible to exercise LSPS2-specific `BOLT12` path construction in integration tests. Co-Authored-By: HAL 9000

Cover the full offer-payment flow from onion-message invoice exchange through HTLC interception, JIT channel opening, and settlement. This confirms the LSPS2 router and service handler work together in the integrated path. Co-Authored-By: HAL 9000

Test the full end-to-end flow of an async payment through an LSPS2 JIT channel: static invoice server setup, `LSPS2BOLT12Router` registration, async payment onion message exchange (`HeldHtlcAvailable`/`ReleaseHeldHtlc`), HTLC interception, JIT channel open, and payment claim. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

To enable suppoort for async payments via LSPS2 JIT channels, we expose explicit async receive-offer refresh and readiness waiting so integrators can sequence external setup before relying on a ready async offer, instead of polling timer ticks. Generated with AI assistance. Co-Authored-By: HAL 9000

The BOLT 7 wire format defines `cltv_expiry_delta` as a 2-byte field, and LDK uses u16 for it everywhere (`RouteHintHop`, `ChannelUpdateInfo`, `UnsignedChannelUpdate`). Align the LSPS2 types accordingly. serde_json will reject values exceeding `u16::MAX` during deserialization, so a counterparty sending an out-of-range value is handled gracefully. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

tnull · 2026-04-14T08:15:03Z

Feel free to squash.

Squashed without further changes.

ldk-reviews-bot · 2026-04-16T08:15:45Z

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-18T08:16:06Z

🔔 2nd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-20T08:16:54Z

🔔 3rd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt · 2026-04-21T13:08:56Z

+	pub fn register_intercept_scid(
+		&self, intercept_scid: u64, invoice_params: LSPS2Bolt12InvoiceParameters,
+	) -> Option<LSPS2Bolt12InvoiceParameters> {
+		debug_assert_eq!(intercept_scid, invoice_params.intercept_scid);


lol why on earth are we providing both just to assert that they're the same?

Because I found it preferable to still communicate that the parameters will be keyed by intercept SCID, especially since we also want to remove them by intercept SCID. And dropping the intercept SCID from the parameters is also not great.

TheBlueMatt · 2026-04-21T13:19:12Z

+/// [`MessageRouter`]: lightning::onion_message::messenger::MessageRouter
+/// [`Event::OnionMessageIntercepted`]: lightning::events::Event::OnionMessageIntercepted
+/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
+pub struct LSPS2BOLT12Router<R: Router, ES: EntropySource + Send + Sync> {


I don't believe the Send + Sync bounds are required.

TheBlueMatt · 2026-04-21T13:25:11Z

+						}))
+					} else {
+						debug_assert!(
+							false,


Huh? Just Err?

TheBlueMatt · 2026-04-21T13:26:05Z

+
+When downgrading to LDK v0.2 and prior, `OnionMessageIntercepted` events that
+hold the new `NextMessageHop::ShortChannelId` will fail to be deserialized and
+hence will be discarded.


No? We'll fail to deserialize and fail to load entirely. This is likely a real issue...Given the number of nodes with offline onion message interception out there its probably not a huge issue so maybe we ignore it but need to carefully document it and probably it should be a new config option.

No? We'll fail to deserialize and fail to load entirely.

Wait, we don't persist OnionMessageIntercepted events at all in LDK, right? I think I got mislead by the pre-existing serialization logic, which AFAIU just got added as we thought the LSPs might want to persist the whole event? And even in the LDK Node integration, we so far don't persist anything, let alone the whole LDK event? So we should be good just dropping all the serialization logic, as well as the new upgrade/downgrade tests?

EDIT: Ah, actually, we implement the serialization logic just for serialized_length. Discussed this before here: #2995 (comment)

So we may be good to just change it arbitrarily, remove all upgrade/downgrade tests and add a fat note there that it's never actually serialized? Or maybe we should find another way to calculate serialized_length for this variant in enqueue_intercepted_events to avoid further confusion in the future?

TheBlueMatt · 2026-04-21T13:28:01Z

+	/// Waits for an async receive offer to become ready after the interactive static-invoice
+	/// protocol completes.
+	#[cfg(feature = "std")]
+	pub fn await_async_receive_offer(&self, max_wait: Duration) -> Result<Offer, ()> {


Bleh, why are we only exposing a non-async version of this? At a minimum we should expose an async version as well.

TheBlueMatt · 2026-04-21T13:29:01Z


 	pending_async_payments_messages: Mutex<Vec<(AsyncPaymentsMessage, MessageSendInstructions)>>,
 	async_receive_offer_cache: Mutex<AsyncReceiveOfferCache>,
+	#[cfg(feature = "std")]


Why is this gated on std?

TheBlueMatt · 2026-04-21T13:29:14Z

@@ -1537,7 +1602,6 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
 			},
 			_ => return None,
 		};


What's with the unnecessary diff in this file?

TheBlueMatt · 2026-04-21T13:29:27Z

+	}
+
+	#[cfg(feature = "std")]
+	pub(crate) fn wait_for_async_receive_offer_ready(&self, max_wait: Duration) -> bool {


Let's just expose the Future?

tnull requested review from TheBlueMatt and jkczyz March 5, 2026 13:36

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from 2cb0546 to 25ab3bc Compare March 5, 2026 14:05

TheBlueMatt reviewed Mar 5, 2026

View reviewed changes

Comment thread lightning/src/onion_message/messenger.rs Outdated

Comment thread lightning-liquidity/src/lsps2/router.rs Outdated

Comment thread lightning/src/onion_message/messenger.rs Outdated

tnull added this to Weekly Goals Mar 5, 2026

tnull moved this to Goal: Merge in Weekly Goals Mar 5, 2026

tnull self-assigned this Mar 5, 2026

jkczyz reviewed Mar 9, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/router.rs

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from 25ab3bc to 5786409 Compare March 24, 2026 14:34

ldk-claude-review-bot reviewed Mar 24, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/service.rs Outdated

ldk-claude-review-bot reviewed Mar 24, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/router.rs Outdated

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from 5786409 to 98a9e9d Compare March 24, 2026 14:50

ldk-claude-review-bot reviewed Mar 24, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/service.rs Outdated

ldk-claude-review-bot reviewed Mar 24, 2026

View reviewed changes

Comment thread lightning-liquidity/tests/lsps2_integration_tests.rs Outdated

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch 2 times, most recently from 8800d48 to 7ca886d Compare March 24, 2026 15:14

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from 7ca886d to 2ff16d7 Compare March 25, 2026 08:23

ldk-claude-review-bot reviewed Mar 25, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/service.rs Outdated

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch 4 times, most recently from bcc4e10 to 5602e07 Compare March 25, 2026 12:27

ldk-claude-review-bot reviewed Mar 25, 2026

View reviewed changes

Comment thread lightning-liquidity/src/lsps2/service.rs Outdated

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch 2 times, most recently from ea05389 to 3acf915 Compare March 25, 2026 13:24

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from bf0ead8 to ff32073 Compare April 9, 2026 08:00

tnull mentioned this pull request Apr 9, 2026

Fix async release before HTLC decode #4548

Merged

jkczyz reviewed Apr 9, 2026

View reviewed changes

tnull removed the blocked on dependent pr label Apr 13, 2026

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from ff32073 to 1e29ea3 Compare April 13, 2026 10:49

tnull requested a review from jkczyz April 13, 2026 10:49

jkczyz reviewed Apr 14, 2026

View reviewed changes

tnull added 8 commits April 14, 2026 10:14

Document the LSPS2 BOLT12 router flow

f648e9a

Describe how `InvoiceParametersReady` feeds both the existing `BOLT11` route-hint flow and the new `LSPS2BOLT12Router` registration path for `BOLT12` offers. Co-Authored-By: HAL 9000

Test LSPS2 router payment-path generation for BOLT12

652dc8c

Exercise the LSPS2 buy flow and assert that registered LSPS2 parameters produce a blinded payment path whose first forwarding hop uses the negotiated intercept SCID. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

tnull force-pushed the 2026-03-lsps2-bolt12-alt branch from 1e29ea3 to 607e924 Compare April 14, 2026 08:14

tnull requested review from TheBlueMatt and jkczyz April 14, 2026 08:15

jkczyz previously approved these changes Apr 15, 2026

View reviewed changes

Comment thread lightning-tests/src/upgrade_downgrade_tests.rs

Add pending_changelog entry

d598b14

tnull dismissed jkczyz’s stale review via d598b14 April 15, 2026 08:05

jkczyz previously approved these changes Apr 15, 2026

View reviewed changes

TheBlueMatt reviewed Apr 21, 2026

View reviewed changes

f Changelog

a0bcfcf

tnull dismissed jkczyz’s stale review via a0bcfcf April 22, 2026 08:33

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

View reviewed changes

Comment thread pending_changelog/4463-LSPS2-BOLT12.txt

Conversation

tnull commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldk-reviews-bot commented Mar 7, 2026

Uh oh!

ldk-reviews-bot commented Mar 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Review Summary — PR #4463 (Review pass 12)

Inline comments posted

Verified prior fixes

Previously posted nits still applicable (not reposted)

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull commented Apr 14, 2026

Uh oh!

Uh oh!

ldk-reviews-bot commented Apr 16, 2026

Uh oh!

ldk-reviews-bot commented Apr 18, 2026

Uh oh!

ldk-reviews-bot commented Apr 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull commented Mar 5, 2026 •

edited

Loading

ldk-reviews-bot commented Mar 5, 2026 •

edited

Loading

ldk-claude-review-bot commented Mar 24, 2026 •

edited

Loading

codecov Bot commented Mar 24, 2026 •

edited

Loading

tnull Apr 22, 2026 •

edited

Loading