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

Update BOLT 12 test vectors #2334

Merged

Conversation

jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Jun 2, 2023

The previous test vectors contained recurrences and older TLV types, and therefore couldn't be parsed. Update the tests with the latest test vectors from the spec and stop ignoring the tests. Additionally, the BOLT 12 test vectors had inadvertently left out a signature, but it has since been added. Include a signature check in the corresponding test for completeness.

@jkczyz jkczyz mentioned this pull request Jun 2, 2023
60 tasks
@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.15 🎉

Comparison is base (fb140b5) 90.34% compared to head (ecd283e) 91.49%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2334      +/-   ##
==========================================
+ Coverage   90.34%   91.49%   +1.15%     
==========================================
  Files         104      104              
  Lines       53392    65514   +12122     
  Branches    53392    65514   +12122     
==========================================
+ Hits        48237    59943   +11706     
- Misses       5155     5571     +416     
Impacted Files Coverage Δ
lightning/src/offers/merkle.rs 99.41% <100.00%> (+0.01%) ⬆️
lightning/src/offers/offer.rs 92.39% <100.00%> (+2.48%) ⬆️

... and 23 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@dunxen dunxen 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 the CI issue.

@jkczyz jkczyz force-pushed the 2023-06-bolt12-test-vectors branch from d952596 to 798ac45 Compare June 7, 2023 21:01
jkczyz added 3 commits June 7, 2023 16:55
These were added to help debug an encoding issue. However, the encoding
code was moved to the blinded_path module. Additionally, the test vector
used an old TLV encoding.
The previous test vectors contained recurrences and older TLV types, and
therefore couldn't be parsed. Update the tests with the latest test
vectors from the spec and stop ignoring the tests.
The BOLT 12 test vectors had inadvertently left out a signature, but it
has since been added. Include a signature check in the corresponding
test for completeness.
@jkczyz jkczyz force-pushed the 2023-06-bolt12-test-vectors branch from 798ac45 to ecd283e Compare June 7, 2023 21:58
@jkczyz
Copy link
Contributor Author

jkczyz commented Jun 7, 2023

LGTM. Just the CI issue.

Accidentally committed some errant code that was meant for a later commit. Should be fixed now.

Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

LGTM

@TheBlueMatt TheBlueMatt merged commit b6787a4 into lightningdevkit:main Jun 8, 2023
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