Merged
Conversation
8ab8694 to
39e8a45
Compare
We should explicitly filter features based on where they can be included (`init`, `node_announcement` or `invoice`) as specified in Bolt 9. We also introduce the option_payment_metadata feature which helps our test cases since it's only allowed in invoices.
This commit doesn't contain any logic, it simply prefixes some classes to make it obvious that they are payment-related, rename files and moves some classes. We will update the payment onion, so it was a good time to do this small refactoring which will also be necessary for onion messages.
Add support for lightning/bolts#912 Whenever we find a payment metadata field in an invoice, we send it in the onion payload for the final recipient. We include a payment metadata in every invoice we generate. This lets us see whether our payers support it or not, which is important data to have before we make it mandatory and use it for storage-less invoices.
39e8a45 to
0791667
Compare
sstone
reviewed
Feb 8, 2022
| nodeParams.features, | ||
| randomBytes32(), | ||
| // We always include a payment metadata in our invoices, which lets us test whether senders support it | ||
| randomBytes(64).toByteVector(), |
Member
Author
There was a problem hiding this comment.
Updated in aa81e7d to use a single byte to avoid making invoices too big
| description, | ||
| PaymentRequest.DEFAULT_MIN_FINAL_EXPIRY_DELTA, | ||
| invoiceFeatures, | ||
| nodeParams.features, |
Member
There was a problem hiding this comment.
why not nodeParams.features.initFeatures() ?
Member
Author
There was a problem hiding this comment.
Done in aebef0c, the feature bits were already filtered inside PaymentRequest.create but it still makes sense to do it here as well.
abd2f4c to
aebef0c
Compare
sstone
approved these changes
Feb 8, 2022
This file contains hidden or 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
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.
Add support for
option_payment_metadataas specified in lightning/bolts#912This feature lets wallets generate "light" invoices without storing them in the local DB until they are paid.
The trick is to simply put the important data (mainly
payment_secretandtotal_amount) in an encrypted blob inside the invoice, and verify its presence and integrity when receiving a payment that contains apayment_metadatafield (and insert it in the DB on-the-fly).This PR only implements support for the sender side, as recipients will only be able to leverage this features once all senders support it.
We do include a
payment_metadatain all invoices we generate, which lets us know whether our payers support this feature: this is important data to have before we decide to make it mandatory and actually implement storage-less invoices.We also take this opportunity to have our first invoice-only feature to implement feature bit filtering, as required by the spec (
IN9flags as specified in bolt 9): see the first commit of the PR.The second commit does not contain any logic and simply moves data classes around and rename them, it's best reviewed independently.
The third commit contains the actual logic for
option_payment_metadata.