Default to padding blinded paths #4213
Conversation
TheBlueMatt
commented
Nov 10, 2025
TheBlueMatt
commented
Because they end up both being used to validate a `Bolt12Invoice`, we ended up with a single `OffersContext` both for inclusion in a `Refund` and an `InvoiceRequest`. However, this is ambiguous, and while it doesn't seem like an issue, it also seems like a nice property to only use a given `OffersContext` in one place. Further, in the next commit, we use `OffersContext` to figure out what we're building a blinded path for and changing behavior based on it, so its nice to be unambiguous. Thus, we split the single existing context into `OutboundPaymentInRefund` and `OutboundPaymentInInvReq`.
|
👋 I see @wpaulino was un-assigned. |
TheBlueMatt
force-pushed
the
2025-10-pad-use-non-compact-bps
branch
from
65206f0 to
c7d1621
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4213 +/- ##
=========================================
Coverage 89.33% 89.33%
=========================================
Files 180 180
Lines 138055 139336 +1281
Branches 138055 139336 +1281
=========================================
+ Hits 123326 124471 +1145
- Misses 12122 12243 +121
- Partials 2607 2622 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 10th Reminder Hey @valentinewallace! This PR has been waiting for your review. |
| expected_path_length: usize, | ||
| ) -> bool { | ||
| let introduction_node_id = resolve_introduction_node(lookup_node, path); | ||
| introduction_node_id == expected_introduction_node |
There was a problem hiding this comment.
We previously had a check that the intro node was encoded as an scid, should we restore that? Or maybe it changed due to the never-compact option
There was a problem hiding this comment.
We quite often use non-compact paths now, as we generally try to use them any time where we're not space-constrained, so such a check fails a handful of test.
| /// message, and thus an `Err` is returned. | ||
| /// message, and thus an `Err` is returned. The impact of this may be somewhat muted when | ||
| /// additional padding is added to the blinded path, but this protection is not complete. | ||
| pub struct NodeIdMessageRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref> |
There was a problem hiding this comment.
Not related to the PR per se, I'm wondering if we can get rid of this struct? I don't see it used anywhere and the docs don't indicate when it should be used
There was a problem hiding this comment.
The intent is that someone may want to override the router (eg using the flow's create_offer_builder_using_router).
There was a problem hiding this comment.
Ok, I still don't think the NodeIdMessageRouter docs make it clear when a user would want to actually do that. I assume it's for more stable blinded paths, i.e. avoiding using scids in a blinded message path ever.
There was a problem hiding this comment.
Oh, sorry I thought you meant how would it be used, not where.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
TheBlueMatt
force-pushed
the
2025-10-pad-use-non-compact-bps
branch
from
c7d1621 to
649ba34
Compare
|
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
valentinewallace
left a comment
valentinewallace
left a comment
There was a problem hiding this comment.
LGTM after squash. May want someone more familiar with the previous padding/dummy hop work to take a look
After much discussion in lightningdevkit#3246 we mostly decided to allow downstream developers to override whatever decisions the `DefaultMessageRouter` makes regarding blinded path selection by providing easy overrides for the selected `OnionMessageRouter`. We did not, however, actually select good defaults for `DefaultMessageRouter`. Here we add those defaults, taking advantage of the `MessageContext` we're given to detect why we're building a blinded path and selecting blinding and compaction parameters based on it. Specifically, if the blinded path is not being built for an offers context, we always use a non-compact blinded path and always pad it to four hops (including the recipient). However, if the blinded path is being built for an `Offers` context which implies it might need to fit in a QR code (or, worse, a payment onion), we reduce our padding and try to build a compact blinded path if possible. We retain the `NodeIdMessageRouter` to disable compact blinded path creation but use the same path-padding heuristic as for `DefaultMessageRouter`.
If we're building a blinded message path with extra dummy hops, we have to ensure we at least hide the length of the data in pre-final hops as otherwise the dummy hops are trivially obvious. Here we do so, taking an extra `bool` parameter to `BlindedMessagePath` constructors to decide whether to pad every hop to the existing `MESSAGE_PADDING_ROUND_OFF` or whether to only ensure that each non-final hop has an identical hop data length. In cases where the `DefaultMessageRouter` opts to use compact paths, it now also selects compact padding, whether short channel IDs are available or not.
TheBlueMatt
force-pushed
the
2025-10-pad-use-non-compact-bps
branch
from
649ba34 to
8efb3e1
Compare
|
Squashed and updated a few docs/comments: $ git diff-tree -U2 649ba3478 8efb3e171
diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs
index ffdb2f6403..75e2aaf3c5 100644
--- a/lightning/src/onion_message/functional_tests.rs
+++ b/lightning/src/onion_message/functional_tests.rs
@@ -696,5 +696,6 @@ fn test_blinded_path_padding_for_full_length_path() {
#[test]
fn test_blinded_path_compact_padding() {
- // Check that for a blinded path with compact padding, no extra padding is applied.
+ // Check that for a blinded path with non-SCID intermediate hops with compact padding, no extra
+ // padding is applied.
let nodes = create_nodes(4);
@@ -729,5 +730,6 @@ fn test_blinded_path_compact_padding() {
#[test]
fn test_compact_blinded_path_compact_padding() {
- // Check that for a blinded path with compact padding, no extra padding is applied.
+ // Check that for a blinded path with SCID intermediate hops with compact padding, no extra
+ // padding is applied.
let nodes = create_nodes(4);
diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs
index d6d4314196..03fc8dd4f7 100644
--- a/lightning/src/onion_message/messenger.rs
+++ b/lightning/src/onion_message/messenger.rs
@@ -773,4 +773,7 @@ where
/// paths.
///
+/// This may be useful in cases where you want a long-lived blinded path and anticipate channel(s)
+/// may close, but connections to specific peers will remain stable.
+///
/// This message router can only route to a directly connected [`Destination`].
/// |
|
✅ Added second reviewer: @wpaulino |
3 participants
