Skip to content
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

feature: advertise required feature bit for TLV onions #6336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Roasbeef
Copy link
Member

In this commit, we start to advertise the required feature bit for the
new modern TLV onion format. In a future change, we'll remove the logic
for being able to encode+encode the payload. For now, we still have
tests that exercise being able to use the new and old formats (as well
as both of them in a single route), so we'll continue to retain that
behavior for now.

Implements lightning/bolts#962.

In this commit, we start to advertise the required feature bit for the
new modern TLV onion format. In a future change, we'll remove the logic
for being able to encode+encode the payload. For now, we still have
tests that exercise being able to use the new and old formats (as well
as both of them in a single route), so we'll continue to retain that
behavior for now.

Implements lightning/bolts#962.
Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Just for completeness sake, the one other place I think we can maybe change to using the required bit instead of the optional bit in this pr (instead of waiting for the follow-up pr where we remove all the legacy code for the optional bit) is for when we create the JIT keysend and AMP invoices. currently we just put in the optional feature bit there & persist those invoices.

@Roasbeef
Copy link
Member Author

So instead, we might want to leave the feature bit as is, and modify the SendToRoute call to default to using the TLV payload: lightning/bolts#962 (comment)

@Roasbeef
Copy link
Member Author

Made this PR to implement the change above: #6385

@Roasbeef
Copy link
Member Author

Moving this to 0.16 given that we're introducing behavior in 0.14.3 to stop allowing the legacy onions to be sent at all.

@Roasbeef Roasbeef modified the milestones: v0.15.0, v0.16.0 Apr 12, 2022
@Roasbeef Roasbeef modified the milestones: v0.16.0, v0.17.0 Aug 23, 2022
@saubyk saubyk added P3 might get fixed, nice to have P1 MUST be fixed or reviewed and removed P3 might get fixed, nice to have labels Aug 8, 2023
@saubyk saubyk removed this from the Low Priority milestone Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants