Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compact blinded path creation #3011
Compact blinded path creation #3011
Changes from all commits
d792afb
e553a71
4956ade
0a6886e
e8354b2
e4661fe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably have some kind of discussion of how this makes paths shorter but if a channel closes will invalidate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make setting this optional somehow? I feel like if I'm building a super long-term offer I may have a different preference from something being scanned right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... arguably we shouldn't bother with it for reply paths, either. Just not sure exactly how we want to convey it through the
MessageRouter
trait. Currently, the caller makes the decision for the penultimate hop usingForwardNode
, but when adding more hops theMessageRouter
makes the decision since it needs aNetworkGraph
to find more hops. Similarly for the introduction node.So right now it's partly an implementation concern given you need a
NetworkGraph
. I guess we can just add a bool parameter and it say it is best effort? Any other ideas?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean... if there's an obvious "default behavior" that stands out, we could have a separate method, e.g.
create_long_term_blinded_paths
, or have aConfig
struct. That way we could also have a config setting for compact offers vs offers that don't need to be QR-scanned, or privacy-oriented offers that want longer blinded paths.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #3080 with separate
MessageRouter
methods, using compact paths forOffer::paths
andRefund::paths
and non-compact paths for onion message reply paths. Basically, the trait allows either for the caller to decide.As for how this is exposed in
ChannelManager
utilities, I'm not sure how we should handle short- vs long-lived offers. One thought was to chose the type of path based on the expiry. But for offers, the expiry is set by the user after the builder is returned and path already set. For refunds created viaChannelManager
, we require an expiration (even though the spec does not), though, so we could infer there.Alternatives would be:
create_offer_builder
for long-lived offerscreate_offer_builder
indicating if short- or long-livedOption
al absolute expiry parameter tocreate_offer_builder
used to infer which type of path to createAny preferences other alternatives?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with whichever of those options you think is best!