rust-lightning
rust-lightning copied to clipboard
Enable Creation of Offers and Refunds Without Blinded Path
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:
Bothcreate_offer_builderandcreate_refund_buildernow 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 likecreate_compact_blinded_pathsandcreate_blinded_paths_using_absolute_expiryare now redundant and have been removed. This makes the flow cleaner and easier to maintain. -
Router-Driven Flexibility:
TheDefaultMessageRouternow uses aRouterConfigto 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 Report
Attention: Patch coverage is 96.10028% with 14 lines in your changes missing coverage. Please review.
Project coverage is 88.95%. Comparing base (
ebe571a) to head (596cf47). Report is 47 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3246 +/- ##
==========================================
+ Coverage 88.93% 88.95% +0.01%
==========================================
Files 168 173 +5
Lines 121962 123516 +1554
Branches 121962 123516 +1554
==========================================
+ Hits 108471 109870 +1399
- Misses 11088 11194 +106
- Partials 2403 2452 +49
| Flag | Coverage Δ | |
|---|---|---|
| fuzzing | 22.28% <0.00%> (-0.42%) |
:arrow_down: |
| tests | 88.77% <96.10%> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Updated from pr3246.02 to pr3246.03 (diff):
Changes:
- Fix docs.
- Introduce tests for offer and refund with no blinded paths.
Updated from pr3246.03 to pr3246.04 (diff):
Changes:
- Rebase on main, to resolve merge conflicts.
- Fix CI.
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
matchto avoidmutvariables.
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.
Updated from pr3246.09 to pr3246.10 (diff): Addressed @jkczyz comments
Changes:
- Update
BlindedPathTypedocumentation. - Update code to amend all returned paths by
MessageRouterto offers, and refund - DRY
create_offer_builder, andcreate_refund_builder
Updated from pr3246.10 to pr3246.11 (diff): Addressed @bluematt & @jkczyz comments
Changes:
- Reverted the "Unified Flow" commits, hence re-introducing the two function flows for blinded path creation.
- Squashed the rest of the commits.
- Moved BlindedPathType to channelmanager, as it is only used there.
- Update
DefaultMessageRouter::create_compact_blinded_pathsto only return a single path.
Updated from pr3246.13 to pr3246.14 (diff):
Changes:
- Rebase on #3412
- Introduce the "Router-based" approach.
- With this a user can either pass the
DefaultMessageRouter, configured to return a particular type of blinded path (or no blinded paths), or can pass a custom message router to the create_{offer/refund}_builder, to return customised blinded paths.
@TheBlueMatt, @jkczyz
A gentle ping! Would love to get your thoughts on this new approach, when you get a moment! Thanks a lot! :)
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.
Updated from pr3246.16 to pr3246.17 (diff):
Changes
- Reverted the rebase from #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!
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_EXPIRYtest 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.
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?).
I'm definitely okay with having methods to override the router used, and like the changes to not decide in
ChannelManagerif 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 aCompactMessageRouter? 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.
Updated from pr3246.18 to pr3246.19 (diff): Addressed @jkczyz comments
Changes:
- Expanded
NullMessageRouter, andCompactMessageRouterdocument.
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?
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.
@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!
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 aCompactMessageRouteris 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.
Updated from pr3246.19 to pr3246.20 (diff): Addressed @TheBlueMatt comments
Changes:
- Update
DefaultMessageRouterto create CompactBlindedMessagePathby default. - When a user wants to create full-length blinded path, they can parameterise using the
NodeIdMessageRouter
Updated from pr3246.20 to pr3246.21 (diff):
Changes:
- Rebase on main
- Clean-up commits history and messages.
- Address other minor nit-changes.