Skip to content

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

Merged
Roasbeef merged 2 commits intomasterfrom
bolt11-test-vectors-payment-secret
Aug 17, 2021
Merged

Make payment secret mandatory and update Bolt 11 test vectors#887
Roasbeef merged 2 commits intomasterfrom
bolt11-test-vectors-payment-secret

Conversation

@t-bast
Copy link
Copy Markdown
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 t-bast requested review from Roasbeef and rustyrussell July 12, 2021 09:54
t-bast added a commit to ACINQ/eclair that referenced this pull request Jul 12, 2021
Copy link
Copy Markdown
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...

@t-bast
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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