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

Make payment secret mandatory and update Bolt 11 test vectors #887

Merged
merged 2 commits into from
Aug 17, 2021

Conversation

t-bast
Copy link
Collaborator

@t-bast t-bast commented Jul 12, 2021

Update Bolt 11 test vectors to always include a payment secret.
We want to make it mandatory in invoices which would make the existing test vectors invalid.

t-bast added a commit to ACINQ/eclair that referenced this pull request Jul 12, 2021
Copy link
Collaborator

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Seems OK, but the checksums and signatures in the breakdowns are incorrect.

Let me write a tool to generate these, and I can create another diff...

11-payment-encoding.md Outdated Show resolved Hide resolved
@t-bast
Copy link
Collaborator Author

t-bast commented Jul 19, 2021

Seems OK, but the checksums and signatures in the breakdowns are incorrect.

Right, good catch. Don't worry, I'll fix these today!

@rustyrussell
Copy link
Collaborator

rustyrussell commented Jul 19, 2021

Already done, might as well take my changes!

Also, validated the vectors, thanks!

See the guilt/pr-887 branch for three more commits: https://github.com/lightningnetwork/lightning-rfc/tree/guilt/pr-887

@t-bast t-bast force-pushed the bolt11-test-vectors-payment-secret branch from fe4a664 to 02b8026 Compare July 19, 2021 12:32
@t-bast
Copy link
Collaborator Author

t-bast commented Jul 19, 2021

Great, I had done it in parallel and I just checked your commits, we find the same results so we should be good to go ;)

Update Bolt 11 test vectors to always include a payment secret.

We want to make it mandatory in invoices which would make the existing
test vectors invalid.
@t-bast t-bast force-pushed the bolt11-test-vectors-payment-secret branch from 424898a to 42bd71d Compare July 19, 2021 12:41
@rustyrussell
Copy link
Collaborator

Ack 42bd71d

@t-bast t-bast changed the title Add payment secret to Bolt 11 test vectors Make payment secret mandatory and update Bolt 11 test vectors Aug 10, 2021
Copy link
Collaborator

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack ec1d4dc

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Didn't check the new vectors, but text changes look good.

Copy link
Collaborator

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🐳

@Roasbeef Roasbeef merged commit 07c7cae into master Aug 17, 2021
@t-bast t-bast deleted the bolt11-test-vectors-payment-secret branch August 17, 2021 06:49
t-bast added a commit to ACINQ/eclair that referenced this pull request Aug 17, 2021
t-bast added a commit to ACINQ/eclair that referenced this pull request Sep 9, 2021
t-bast added a commit to ACINQ/eclair that referenced this pull request Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants