Add BOLT12 support to LSPS2 via custom Router implementation

[tnull]

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]

👋 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]

🔔 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]

🔔 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]

No issues found.

I re-reviewed the full current state of the PR (commit 26da1dde7). The design has been substantially refined since earlier passes, and the concerns from my prior review comments are now resolved by the current code:

• router.rs — create_blinded_payment_paths correctly uses continue on path-build failure (not ?), guards against returning an empty path set with Err(()), uses u16 consistently, increases the forward hop's max_cltv_expiry in the correct direction, and the code comment correctly references Event::HTLCIntercepted (this is the payment path). The metadata-decoder approach replaces the prior HashMap register/deregister design that several old comments targeted.
• events/mod.rs — OnionMessageIntercepted (event tag 37, odd) serializes field 0 only for NodeId next hops (backward compat) and the reader accepts both old (peer_node_id) and new (next_hop) encodings. The doc field naming/typo issues from prior passes are fixed. The previously-flagged even-tagged InvoiceRequestReceived event has been removed entirely.
• messenger.rs — the intercept_for_unknown_scids interception path is correct, and the "messager" typo is fixed.
• event.rs / msgs.rs / service.rs — the u32→u16 change for cltv_expiry_delta / lsp_cltv_expiry_delta is consistent across client, service API, message struct, and aligns with the BOLT cltv_expiry_delta width.
• pending_changelog/4463-LSPS2-BOLT12.txt — now accurate: it uses the correct OnionMessenger name and correctly caveats that LDK does not persist these events itself, so the downgrade risk only applies to users who manually persist them.

Security: the decoded payment metadata is authored by the recipient and authenticated via the offer's reply-path context HMAC, so a payer cannot inject malicious LSP parameters. Safe.

One low-confidence observation I deliberately did not file: router.rs:233 uses plain MIN_FINAL_CLTV_EXPIRY_DELTA, whereas the BOLT11 route-hint path uses MIN_FINAL_CLTV_EXPIRY_DELTA + 2. The end-to-end test bolt12_lsps2_end_to_end_test completes successfully (client emits PaymentClaimable and claims), demonstrating the plain value is functionally sufficient for the blinded-path case, so this is not a defect.

No inline comments posted this pass.

[codecov]

Codecov Report

❌ Patch coverage is 88.38174% with 56 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.13%. Comparing base (2313bd5) to head (4342483).
⚠️ Report is 169 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-liquidity/src/lsps2/router.rs	93.39%	27 Missing ⚠️
lightning/src/ln/channelmanager.rs	32.00%	16 Missing and 1 partial ⚠️
lightning/src/events/mod.rs	33.33%	5 Missing and 1 partial ⚠️
lightning/src/offers/flow.rs	66.66%	2 Missing and 1 partial ⚠️
lightning/src/onion_message/messenger.rs	86.66%	2 Missing ⚠️
lightning/src/util/test_utils.rs	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4463      +/-   ##
==========================================
+ Coverage   87.08%   87.13%   +0.04%     
==========================================
  Files         161      162       +1     
  Lines      109255   109707     +452     
  Branches   109255   109707     +452     
==========================================
+ Hits        95147    95589     +442     
- Misses      11627    11632       +5     
- Partials     2481     2486       +5     

Flag	Coverage Δ
fuzzing-fake-hashes	31.10% <7.35%> (+0.12%)	⬆️
fuzzing-real-hashes	22.79% <4.41%> (+0.13%)	⬆️
tests	86.20% <88.38%> (+0.03%)	⬆️

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.

[TheBlueMatt]

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

[tnull]

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.

[tnull]

Addressed pending comments, also had to rebase as we now had a minor conflict. Let me know if I can squash.

[TheBlueMatt]

Why do we need a public version of this? We already do it on timer tick and any time we connect to a peer, istm this is entirely redundant.

[tnull]

Exactly because we auto-request offers when connecting to a peer we found that we need this refresh during the LDK Node integration. Otherwise, there would already a refresh without the intercept SCIDs inflight when we start negotiating the LSPS2 flow and start injecting the SCIDs, which fails the flow. The alternative workaround would be a busy-loop that calls timer_tick_occurred until we can successfully generate an offer, but that's way uglier, hence why we added this API.

[TheBlueMatt]

Hmm, I don't think this currently works? If we build a static invoice when we connect to peers while we're negotiating LSPS2 (ie we have no LSPS2 parameters configured yet) we'll upload the static invoice to our static invoice storage server and there will be a window during which we cannot receive because the static invoice that is stored is bogus. LSPS2BOLT12Router really needs to fail until we have configured parameters so that we won't upload a bogus static invoice.

The docs here really need to mention that this should never really be required unless using LSPS.

[tnull]

Hmm, I think you're right, but not quite sure what the best API for this would be? I guess we could add a flag that temporarily will have the router error, but that seems like a very race-y approach when potentially dealing with potentially a) multiple in-flight connections b) multiple LSPs c) multiple subsequent or parallel JIT flows per LSP.

Do you have any specific API in mind that would allow for these cases?

[TheBlueMatt]

I think the only way to do it would be to have the router error until its configured, yea. I think that should be mostly-fine, though - if we are building a blinded path and don't have any LSP information then having the router return a blinded path is just gonna result in a payment failure anyway (and probably error since we don't have any channels!). It seems fine to hard-code that.

[TheBlueMatt]

Can't this race? We probably need a bit more smarts in the flow to make sure we always return a pre-completed Future if async offers are already available in a race-free way.

[tnull]

Can't this race? We probably need a bit more smarts in the flow to make sure we always return a pre-completed Future if async offers are already available in a race-free way.

I'm not quite sure I understand what you mean, could you expand? What would race what?

[TheBlueMatt]

If the async receive offer becomes ready between the get_async_receive_offer returning Err and hitting this we might block forever, no?

[tnull]

Added back a commit that derives Hash for OfferId, as we need that downstream (slightly orthogonal, but connected to this PR).

[TheBlueMatt]

Does this need updating with the changes from #4584?

[tnull]

Does this need updating with the changes from #4584?

Yes, probably we should now wait for that to land first.

[tnull]

Now updated to use the new payment-metadata-in-BOLT12-context flow. Also updated lightningdevkit/ldk-node#817 to use this.

[TheBlueMatt]

Why do we need this? Doesn't our router inject the metadata for us?

[tnull]

The MessageRouter in LDK Node will do that for regular offers, but that doesn't work for the async path:

For regular offers, ldk-node does this:

• Wraps MessageRouter.
• When create_offer_builder_using_router(...) builds blinded invoice-request paths, the wrapper sees MessageContext::Offers(InvoiceRequest { payment_metadata, .. }).
• It writes LSPS2 metadata there.
• Later, when an invoice request arrives, LDK turns that into PaymentContext::Bolt12Offer { payment_metadata }.
• Then LSPS2BOLT12Router can decode it while building the actual invoice payment paths.

For async static invoices, the order is different:

• The recipient proactively builds a StaticInvoice before any payer invoice request exists.
• The static invoice’s payment paths are built via Router::create_blinded_payment_paths.
• That call receives PaymentContext::AsyncBolt12Offer { payment_metadata }.
• The MessageRouter is not involved in that payment-path construction, so it has no place to inject the metadata.

[TheBlueMatt]

Right, then shouldn't we wrap the router rather than adding a new top level method here?

[TheBlueMatt]

Hmm, I don't think this currently works? If we build a static invoice when we connect to peers while we're negotiating LSPS2 (ie we have no LSPS2 parameters configured yet) we'll upload the static invoice to our static invoice storage server and there will be a window during which we cannot receive because the static invoice that is stored is bogus. LSPS2BOLT12Router really needs to fail until we have configured parameters so that we won't upload a bogus static invoice.

The docs here really need to mention that this should never really be required unless using LSPS.

[ldk-reviews-bot]

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