Fix and modernize lightning-invoice API #1057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
from
6d3321f
to
669b3ea
Compare
This swaps out our doctest example invoices for real LDK-generated invoices on a real LDK node.
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
from
669b3ea
to
f631178
Compare
Codecov Report
@@ Coverage Diff @@
## main #1057 +/- ##
==========================================
+ Coverage 90.82% 91.66% +0.84%
==========================================
Files 65 65
Lines 32801 36884 +4083
==========================================
+ Hits 29791 33811 +4020
- Misses 3010 3073 +63
Continue to review full report at Codecov.
|
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
2 times, most recently
from
618d801
to
11f0698
Compare
|
Updated but note still failing one last test case that I think t-bast will update cc lightning/bolts#898 (comment) |
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
from
11f0698
to
3e1b78b
Compare
|
Updated again now that the last error is fixed, should be good to go! |
jkczyz
approved these changes
Aug 31, 2021
kaiwolfram
reviewed
Aug 31, 2021
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
from
3e1b78b
to
a5c5848
Compare
This adds two additional tests from the BOLT 11 invalid invoice tests, fixing the two errors that broke them. It fixes a panic on the "nonrecoverable signature" test and makes the error variant more sensible on the bogus SI prefix test.
The BOLT 11 invalid invoice test vectors suggest failing to parse invoices which have an amount which is not a whole number of millisatoshis. lightning-invoice, however, happily parses such invoices. While we could continue to parse them, failing them makes for one less check on the user code side, so we might as well. In order to keep the invoice creation less likely to fail, we also switch the Builder amount-setting function to use millisatoshis.
This adds the final missing BOLT 11 failure test, checking for unknown required feature flags before accepting an invoice.
BOLT 11 states that a reader "MUST skip over...`p`, `h`, `s` or `n` fields that do NOT have data_lengths of 52, 52, 52 or 53, respectively." Here we do so by simply ignoring any invalid-length field.
This pulls the BOLT 11 test vectors from lightning/bolts#898, tweaking our tests to properly handle them.
TheBlueMatt
force-pushed
the
2021-08-invoice-fails
branch
from
a5c5848
to
6879348
Compare
|
Squashed without changes, will take after CI: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes lightningdevkit/ldk-garbagecollected#41 and requires payment_secrets