Enable Creation of Offers and Refunds Without Blinded Path #3246

shaavan · 2024-08-16T08:09:12Z

PR Description:

An Offer and a Refund can exist without a blinded path, where the signing_pubkey is used to determine the destination. While we could already handle Offers and Refunds without a blinded path, we didn’t have a way to create them like this. This PR addresses that gap and simplifies the blinded path creation flow.

The Key Changes:

Pass Router to Builders:
Both create_offer_builder and create_refund_builder now take a router as a parameter. This gives users control over how blinded paths are created. For example, users can now choose not to include any blinded paths, or use a custom router for specific behavior.
Unified Blinded Path Creation:
The Blinded Path creation has been simplified by only keeping a single function for Blinded Path creation, i.e., create_blinded_path. Older, separate variants like create_compact_blinded_paths and create_blinded_paths_using_absolute_expiry are now redundant and have been removed. This makes the flow cleaner and easier to maintain.
Router-Driven Flexibility:
The DefaultMessageRouter now uses a RouterConfig to allow it to create any kind of blinded path (if any) to create. This simplifies the flow, while maintaining the same functionality.

This PR makes blinded path creation more straightforward and gives users better control, while preserving the core logic of the MessageRouter.

blocked on #3412

codecov · 2024-08-16T08:18:56Z

Codecov Report

Attention: Patch coverage is 96.56489% with 9 lines in your changes missing coverage. Please review.

Project coverage is 91.12%. Comparing base (830ffa0) to head (49c6c3a).
Report is 143 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/onion_message/messenger.rs	82.05%	7 Missing ⚠️
lightning-dns-resolver/src/lib.rs	50.00%	1 Missing ⚠️
lightning/src/ln/channelmanager.rs	99.29%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3246      +/-   ##
==========================================
+ Coverage   89.26%   91.12%   +1.85%     
==========================================
  Files         155      156       +1     
  Lines      119968   141224   +21256     
  Branches   119968   141224   +21256     
==========================================
+ Hits       107092   128688   +21596     
+ Misses      10265    10003     -262     
+ Partials     2611     2533      -78

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

shaavan · 2024-08-19T12:38:27Z

Updated from pr3246.01 to pr3246.02 (diff):

Changes:

Rebase on main to resolve merge conflicts.

shaavan · 2024-08-19T13:29:40Z

Updated from pr3246.02 to pr3246.03 (diff):

Changes:

Fix docs.
Introduce tests for offer and refund with no blinded paths.

shaavan · 2024-08-20T12:17:51Z

Updated from pr3246.03 to pr3246.04 (diff):

Changes:

Rebase on main, to resolve merge conflicts.
Fix CI.

lightning/src/onion_message/messenger.rs

lightning/src/ln/channelmanager.rs

jkczyz · 2024-09-04T03:37:00Z

lightning/src/ln/offers_tests.rs

+	assert_eq!(refund.payer_id(), alice_id);
+	assert!(refund.paths().is_empty());
+}
+
 /// Checks that blinded paths are compact for short-lived offers.
 #[test]
 fn creates_short_lived_offer() {


The naming at least for the following tests is no longer relevant, it seems.

As far as I understand, with the introduction of BlindedPathParameters, it seems like the behavior these tests were checking isn't relevant anymore. Should we think about removing these tests?

Let's wait on the resolution of #3246 (comment). We'll ultimately want to test cases allowed by whatever interface is exposed to the user.

lightning/src/ln/offers_tests.rs

shaavan · 2024-09-04T11:05:53Z

Updated from pr3246.04 to pr3246.05 (diff):

Changes:

Rebase on main.

lightning/src/onion_message/messenger.rs

shaavan · 2024-09-05T20:37:45Z

Updated from pr3246.05 to pr3246.06 (diff):
Addressed @jkczyz comments

Changes:

Removed the global constant PATHS_PLACEHOLDER, and instead use a default constructor, and local DEFAULT_VALUE.
Remove the redundant functions in their appropriate commits.
Use match to avoid mut variables.

shaavan · 2024-09-19T15:40:34Z

Updated from pr3246.06 to pr3246.07 (diff):

Changes:

Introduce a new approach using the BlindedPathType enum.
The enum allows for the specification of the type of Blinded Path (Compact or Full) that a user can specify to create the desired type of Blinded Path.
Update offer_builder and refund_builder so that user can explicitly specify the type of Blinded Path they want to create.
Update the Blinded Path creation flow so that only one function flow is responsible for creating both kinds of Blinded Paths.

shaavan · 2024-09-19T16:03:19Z

Updated from pr3246.07 to pr3246.08 (diff):

Changes:

Rebase on main to resolve merge conflicts.

shaavan · 2024-10-03T12:30:51Z

Updated from pr3246.08 to pr3246.09 (diff):

Changes:

Rebase on main, and fix ci.

lightning/src/onion_message/messenger.rs

jkczyz · 2024-10-04T22:51:51Z

lightning/src/ln/channelmanager.rs

+				let context = MessageContext::Offers(context);
+				let path = $self
+					.create_blinded_paths(context)
+					.and_then(|paths| paths.into_iter().next().ok_or(()))


Hmm... @TheBlueMatt I don't think this interface is sufficient to allow someone to specify they want more than one path in the offer, unless we include all the paths returned by the MessageRouter instead of just one.

Yea, IMO we should include all paths - if the router wants to do something funky in the offer, so be it.

What should we have DefaultMessageRouter do?

What we currently do (use the context of why we're asking for a path to decide how many paths to include)?

I suppose DefaultMessageRouter could use both MessageContex and BlindedPathType -- or which MessageRouter method is called, if we revert to that state -- to determine if more than one path should be returned. That way we'd allow users to pass BlindedPathType::Full to create_offer_builder such that they can create an offer with more than one path (e.g., when an offer doesn't need to be in a QR code).

Right, that makes sense but then I'm confused about your previous comment about keeping get interface the same as it is?

If we change MessageRouter to only have one method, then we need to pass in BlindedPathType. But then the peers parameter will need to be Vec<MessageForwardNode> even when used with BlindedPathType::Full instead of Vec<PublicKey>. So callers could pass in a MessageForwardNode with an scid even though it isn't expected, resulting in the last hop being compact.

So callers could pass in a MessageForwardNode with an scid even though it isn't expected, resulting in the last hop being compact.

Makes sense! I have reintroduced the two function flow back in pr3246.11

Also, I have updated the DefaultMessageRouter's create_compact_blinded_paths to only return a single path in pr3246.11

I'm confused why this is a concern, if the MessageRouter insists on a specific type of blinded path in general, so what? Ideally the MessageRouter gets to pick what it wants to do, though I think we're not allowed to have a compact introduction point in reply paths? Even in that case, though, ISTM we can communicate the restrictions and if the MessageRouter is buggy we can just fail.

I'm confused why this is a concern, if the MessageRouter insists on a specific type of blinded path in general, so what? Ideally the MessageRouter gets to pick what it wants to do,

Nothing is preventing a MessageRouter from doing so. It would just need access to the channels.

It's just kinda ugly for DefaultMessageRouter to need to clear short_channel_id depending on how it is called. Plus, ChannelManager needs to do additional lookups to get this information just to have it discarded. I'm fine either way, but it doesn't optimize the usual case (i.e., use non-compact paths for reply paths).

though I think we're not allowed to have a compact introduction point in reply paths? Even in that case, though, ISTM we can communicate the restrictions and if the MessageRouter is buggy we can just fail.

Yeah, though it doesn't look like we are enforcing this at the moment.

lightning/src/ln/channelmanager.rs

lightning/src/onion_message/messenger.rs

shaavan · 2024-10-11T14:28:53Z

Updated from pr3246.09 to pr3246.10 (diff):
Addressed @jkczyz comments

Changes:

Update BlindedPathType documentation.
Update code to amend all returned paths by MessageRouter to offers, and refund
DRY create_offer_builder, and create_refund_builder

lightning/src/ln/channelmanager.rs

shaavan · 2024-12-16T14:45:47Z

Updated from pr3246.14 to pr3246.15 (diff):

Changes:

Rebase on main, to resolve merge conflicts

shaavan · 2025-01-11T14:46:43Z

Updated from pr3246.15 to pr3246.16 (diff):
Addressed @jkczyz comments:

Changes:

Introduce create_{offer, refund}_builder_using_router, to create blinded paths using custom router.

shaavan · 2025-03-28T12:01:40Z

Updated from pr3246.16 to pr3246.17 (diff):

Changes

Reverted the rebase from Introduce OffersMessageFlow #3412.

Reasoning
Since the approach in #3412 is still under discussion, I felt it might be better to un-rebase this PR for now. This keeps the thread active and allows us to continue the conversation here in parallel with the ongoing OffersMessageFlow work.

Open to thoughts or suggestions, of course!

lightning/src/onion_message/messenger.rs

lightning/src/ln/channelmanager.rs

Previously, the `compact_paths` flag was only used to determine whether to use a compact introduction node when creating compact blinded paths. With the upcoming change to accept `MessageForwardNode` in `create_blinded_paths`, there's a risk of SCIDs being passed (and used) even when the user intends to create a full-length blinded path. This patch updates the logic in `create_blinded_paths_from_iter` to ignore SCIDs unless `compact_paths` is explicitly true—preserving correct behavior for full-length blinded paths.

shaavan · 2025-04-02T13:06:48Z

Updated from pr3246.17 to pr3246.18 (diff):
Addressed @jkczyz comments

Changes:

Update create_blinded_paths_from_iter() to ignore scid when user wants to create full-length blinded paths.
Make MAX_SHORT_LIVED_RELATIVE_EXPIRY test only const.
Update documentation on ways user can create compact blinded paths.
Correct create_offer_builder_using_router documentation.
DRY up create_{offer_,refund}_builder.

jkczyz

Let's go ahead an squash. @TheBlueMatt the approach is essentially what is described in #3246 (comment) without the helpers like without_blinded_paths (i.e. constructing the alternative MessagesRouter implementations using a constructor.

lightning/src/onion_message/messenger.rs

TheBlueMatt · 2025-04-15T17:52:20Z

I'm definitely okay with having methods to override the router used, and like the changes to not decide in ChannelManager if a path should be compact or not, but I'm a bit confused because now it seems like we default to doing the wrong thing - if an offer is short-lived we never build a compact blinded path unless the user manually asks for it by passing a CompactMessageRouter? That seems like an important regression (unless we think actually its not worth it to have compact BPs in short-lived offers cause most offers are long-lived?).

jkczyz · 2025-04-15T20:10:08Z

I'm definitely okay with having methods to override the router used, and like the changes to not decide in ChannelManager if a path should be compact or not, but I'm a bit confused because now it seems like we default to doing the wrong thing - if an offer is short-lived we never build a compact blinded path unless the user manually asks for it by passing a CompactMessageRouter? That seems like an important regression (unless we think actually its not worth it to have compact BPs in short-lived offers cause most offers are long-lived?).

It could create compact paths by default if ChannelManager were parameterized by CompactMessageRouter. And besides from having the user explicitly pass CompactMessageRouter to a one of the methods, how could it be done without having ChannelManager decide?

I guess by passing the Option<Duration> through from ChannelManager to MessageRouter? Not sure it if is even worth having CompactMessageRouter then. NullMessageRouter may be sufficient to pass as an override. Don't have strong opinion, though it would be nice if create_offer_builder didn't need an optional parameter.

To prepare for supporting both standard and compact blinded paths, this commit updates the `create_blinded_paths` function to take a `Vec<MessageForwardNode>` as input. This change ensures the function has all the information it needs to handle both types of blinded path creation. This refactor that sets the stage for upcoming enhancements.

Introduced two new default routers: - `CompactMessageRouter` – for creating compact blinded paths - `NullMessageRouter` – for intentionally creating no blinded paths This makes the purpose of each router explicit and ensures their functions remain unique. It also builds on the flexibility introduced in the previous commit, where `create_blinded_paths` was updated to support a MessageForwardNode.

Added a new helper function to encapsulate the logic for fetching peers used in blinded path creation. This keeps the code DRY by reducing duplication and makes the logic easier to reuse across different message routers.

Introduced two new builder functions that accept a custom `MessageRouter` as input. This gives users the flexibility to plug in their own router implementation, enabling custom logic for blinded path creation when building Offers or Refunds. It’s a step toward greater extensibility and control in how paths are constructed.

This change updates the default `create_offer_builder` to always use `create_blinded_paths`, enabling deterministic creation of only full-length blinded paths. Now that we provide `create_offer_builder_using_router`, users who need custom path logic (like compact blinded path) can pass in their own `MessageRouter`. This allows the default builder to remain simple and consistent, while offering flexibility through the alternate function.

Aligned the `create_refund_builder` logic with `create_offer_builder` by updating it to use `create_blinded_paths`. Now that custom routers can be passed via `create_refund_builder_using_router`, the default builder can consistently generate full-length blinded paths without branching logic. This simplifies the default behavior while keeping flexibility available for custom use cases.

With the recent updates to `create_blinded_paths` supporting flexible routing through `MessageRouter`, the specialized variants—`create_compact_blinded_paths` and `create_blinded_paths_using_absolute_expiry`—have become unnecessary. This commit removes those now-redundant functions to simplify the codebase and reduce maintenance overhead.

Introduced tests to validate the behavior of Offers and Refunds created without blinded paths, using `NullMessageRouter`.

shaavan · 2025-04-18T18:32:30Z

Updated from pr3246.18 to pr3246.19 (diff):
Addressed @jkczyz comments

Changes:

Expanded NullMessageRouter, and CompactMessageRouter document.

TheBlueMatt · 2025-04-29T18:34:45Z

It could create compact paths by default if ChannelManager were parameterized by CompactMessageRouter. And besides from having the user explicitly pass CompactMessageRouter to a one of the methods, how could it be done without having ChannelManager decide?

I guess by passing the Option through from ChannelManager to MessageRouter?

Right, this is what I was thinking - give the router enough information to decide and let it decide, then let users override the router if they want to override the decision.

Not sure it if is even worth having CompactMessageRouter then. NullMessageRouter may be sufficient to pass as an override.

I guess if you want to always use a compact path (even for long-lived paths)? Could also have some kind of configurable router that just lets you pick the time limit.

Don't have strong opinion, though it would be nice if create_offer_builder didn't need an optional parameter.

Agreed, it can be a separate function tho.

shaavan · 2025-05-05T15:25:25Z

@TheBlueMatt

I see the point you're suggesting, Matt, and I agree there’s some convenience in having the router automatically decide the blinded path type based on Option<Duration>. But I think having the user explicitly call the offer builder with a CompactMessageRouter is just as convenient—and arguably cleaner.

The current approach has the benefit of being explicit and predictable. By default, we always generate a full-length path, providing a consistent baseline across all offers. If a user wants a compact path—for example, for short-lived offers—they can opt into it clearly via the router they pass in. Conversely, if the user parameterizes ChannelManager with a CompactMessageRouter, they’ll consistently get compact paths—unless they manually override that choice.

This trades away automatic decision-making by the router in favor of something I believe is more valuable in the long run: user awareness and control. It also avoids introducing heuristics that could surprise users unless they’re quite familiar with the internal logic.

In that sense, prioritizing clear defaults and explicit opt-in behavior, I believe, makes the design more intuitive and easier to reason about over time.

Curious to hear your thoughts—thanks!

jkczyz · 2025-05-06T23:11:55Z

I see the point you're suggesting, Matt, and I agree there’s some convenience in having the router automatically decide the blinded path type based on Option<Duration>. But I think having the user explicitly call the offer builder with a CompactMessageRouter is just as convenient—and arguably cleaner.

Regarding this point, I tend to agree that removing the Option<Duration> from methods like create_offer_builder is more desirable. The existing behavior uses this to set the expiry and provide additional information for creating the path. But upon returning an OfferBuilder, the user can then override the expiry with a longer one and be left with a long-lived offer using compact blinded paths.

shaavan marked this pull request as draft August 16, 2024 12:09

shaavan force-pushed the blinded_api branch from 53a3d18 to 6fa28be Compare August 16, 2024 12:09

jkczyz self-requested a review August 16, 2024 14:36

shaavan mentioned this pull request Aug 19, 2024

Exploring the Integration of Dummy Hops into Blinded Paths #3252

Open

shaavan force-pushed the blinded_api branch from 6fa28be to 52efbc6 Compare August 19, 2024 12:38

shaavan force-pushed the blinded_api branch from 52efbc6 to 79d6fdd Compare August 19, 2024 13:29

shaavan marked this pull request as ready for review August 19, 2024 13:30

shaavan force-pushed the blinded_api branch from 79d6fdd to 0d9ac2a Compare August 20, 2024 12:17

jkczyz reviewed Sep 4, 2024

View reviewed changes

shaavan force-pushed the blinded_api branch from 0d9ac2a to cbaf88a Compare September 4, 2024 11:05

TheBlueMatt reviewed Sep 5, 2024

View reviewed changes

lightning/src/onion_message/messenger.rs Outdated Show resolved Hide resolved

shaavan force-pushed the blinded_api branch from cbaf88a to 7a18051 Compare September 5, 2024 20:37

shaavan force-pushed the blinded_api branch from 7a18051 to bc00884 Compare September 19, 2024 15:40

shaavan force-pushed the blinded_api branch from bc00884 to 3bcc420 Compare September 19, 2024 16:03

shaavan changed the title ~~Introduce BlindedPathParams~~ Introduce BlindedPathType enum Sep 24, 2024

shaavan force-pushed the blinded_api branch from 3bcc420 to d6dde1e Compare October 3, 2024 12:30

jkczyz reviewed Oct 4, 2024

View reviewed changes

shaavan force-pushed the blinded_api branch from d6dde1e to 6817512 Compare October 11, 2024 14:28

jkczyz reviewed Oct 11, 2024

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

shaavan force-pushed the blinded_api branch from 6817512 to fe33775 Compare October 19, 2024 11:57

shaavan force-pushed the blinded_api branch from ee51aed to 268d590 Compare January 11, 2025 14:46

jkczyz mentioned this pull request Jan 30, 2025

rustfmt: Run on onion_message #3576

Merged

shaavan force-pushed the blinded_api branch 2 times, most recently from dd1401f to 3c3224d Compare March 28, 2025 12:01

jkczyz reviewed Apr 1, 2025

View reviewed changes

shaavan force-pushed the blinded_api branch 2 times, most recently from f4b5919 to ba0b3f1 Compare April 2, 2025 13:06

shaavan requested a review from jkczyz April 4, 2025 12:39

jkczyz reviewed Apr 15, 2025

View reviewed changes

lightning/src/onion_message/messenger.rs Outdated Show resolved Hide resolved

lightning/src/onion_message/messenger.rs Show resolved Hide resolved

shaavan force-pushed the blinded_api branch from ba0b3f1 to 7b9b2ea Compare April 18, 2025 18:29

shaavan added 7 commits April 19, 2025 00:01

Refactor: Introduce get_peers_for_blinded_path utility

16bf41c

Added a new helper function to encapsulate the logic for fetching peers used in blinded path creation. This keeps the code DRY by reducing duplication and makes the logic easier to reuse across different message routers.

Test: Add coverage for Offers and Refunds without blinded paths

49c6c3a

Introduced tests to validate the behavior of Offers and Refunds created without blinded paths, using `NullMessageRouter`.

shaavan force-pushed the blinded_api branch from 7b9b2ea to 49c6c3a Compare April 18, 2025 18:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Creation of Offers and Refunds Without Blinded Path #3246

Enable Creation of Offers and Refunds Without Blinded Path #3246

shaavan commented Aug 16, 2024 •

edited

Loading

codecov bot commented Aug 16, 2024 •

edited

Loading

shaavan commented Aug 19, 2024

shaavan commented Aug 19, 2024

shaavan commented Aug 20, 2024

jkczyz Sep 4, 2024

shaavan Sep 5, 2024

jkczyz Sep 5, 2024

shaavan commented Sep 4, 2024

shaavan commented Sep 5, 2024

shaavan commented Sep 19, 2024

shaavan commented Sep 19, 2024

shaavan commented Oct 3, 2024

jkczyz Oct 4, 2024

TheBlueMatt Oct 8, 2024

jkczyz Oct 8, 2024

TheBlueMatt Oct 11, 2024

jkczyz Oct 11, 2024

TheBlueMatt Oct 17, 2024

jkczyz Oct 17, 2024

shaavan Oct 19, 2024 •

edited

Loading

TheBlueMatt Oct 19, 2024

jkczyz Oct 21, 2024

shaavan commented Oct 11, 2024

shaavan commented Dec 16, 2024

shaavan commented Jan 11, 2025

shaavan commented Mar 28, 2025

shaavan commented Apr 2, 2025

jkczyz left a comment

TheBlueMatt commented Apr 15, 2025

jkczyz commented Apr 15, 2025

shaavan commented Apr 18, 2025

TheBlueMatt commented Apr 29, 2025

shaavan commented May 5, 2025

jkczyz commented May 6, 2025

Enable Creation of Offers and Refunds Without Blinded Path #3246

Are you sure you want to change the base?

Enable Creation of Offers and Refunds Without Blinded Path #3246

Conversation

shaavan commented Aug 16, 2024 • edited Loading

PR Description:

The Key Changes:

codecov bot commented Aug 16, 2024 • edited Loading

Codecov Report

shaavan commented Aug 19, 2024

shaavan commented Aug 19, 2024

shaavan commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaavan commented Sep 4, 2024

shaavan commented Sep 5, 2024

shaavan commented Sep 19, 2024

shaavan commented Sep 19, 2024

shaavan commented Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaavan Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaavan commented Oct 11, 2024

shaavan commented Dec 16, 2024

shaavan commented Jan 11, 2025

shaavan commented Mar 28, 2025

Changes

shaavan commented Apr 2, 2025

jkczyz left a comment

Choose a reason for hiding this comment

TheBlueMatt commented Apr 15, 2025

jkczyz commented Apr 15, 2025

shaavan commented Apr 18, 2025

TheBlueMatt commented Apr 29, 2025

shaavan commented May 5, 2025

jkczyz commented May 6, 2025

shaavan commented Aug 16, 2024 •

edited

Loading

codecov bot commented Aug 16, 2024 •

edited

Loading

shaavan Oct 19, 2024 •

edited

Loading