Add PaymentMetadata and integrate it with PaymentDetail#552
Add PaymentMetadata and integrate it with PaymentDetail#552moisesPompilio wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
moisesPompilio
force-pushed
the
add-payment-metadata
branch
from
f22ac40 to
7f6e9f4
Compare
moisesPompilio
force-pushed
the
add-payment-metadata
branch
from
7f6e9f4 to
8fedde7
Compare
- Introduces the `PaymentMetadata` struct with a randomly generated 16-byte ID ([u8; 16]), lighter than the 32-byte ID used in `PaymentDetail`. - Adds a pointer from `PaymentDetail` to `PaymentMetadata`, available only for `PaymentKind::Bolt11`, `PaymentKind::Bolt12Offer` and `PaymentKind::Bolt12Refund` . - For `Bolt11`, the metadata reference is an `Option<PaymentMetadataId>`. - For `Bolt12Refund` and `Bolt12Offer`, it's a `Vec<PaymentMetadataId>`, allowing multiple metadata entries per detail.
moisesPompilio
force-pushed
the
add-payment-metadata
branch
from
8fedde7 to
024eae0
Compare
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Excuse the delay here. I now left some comments. Unfortunately still not sure if this approach is going in the right direction.
| /// The secret used by the payment. | ||
| secret: Option<PaymentSecret>, | ||
| /// The id of the payment metadata | ||
| payment_metadata_id: Option<PaymentMetadataId>, |
There was a problem hiding this comment.
Hmm, but as mentioned elsewhere, BOLT11 payment could also have multiple different metadata items associated with them? E.g., an invoice and some LSP fee limits.
| pub fn new() -> Self { | ||
| let mut random_bytes = [0u8; Self::LENGTH]; | ||
| rand::thread_rng().fill_bytes(&mut random_bytes); | ||
| let mut id = [0u8; 16]; |
There was a problem hiding this comment.
I still don't get why you need this separate array here, how is this different from random_bytes?
| /// This can be a BOLT 11 invoice, a BOLT 12 offer, or a BOLT 12 refund. | ||
| pub payment_metadata_detail: PaymentMetadataDetail, | ||
| /// The limits applying to how much fee we allow an LSP to deduct from the payment amount. | ||
| pub jit_channel_fee_limit: Option<JitChannelFeeLimits>, |
There was a problem hiding this comment.
Why would this be a constant field of PaymentMetadata?
| /// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md | ||
| Bolt11 { | ||
| /// The invoice associated with the payment. | ||
| invoice: String, |
There was a problem hiding this comment.
We'd def. don't want to store invoices as Strings, we have the Bolt11Invoice type with that.
| /// | ||
| /// This enum encapsulates various types of payment metadata, such as BOLT 11 invoices, | ||
| /// BOLT 12 offers, and BOLT 12 refunds, along with their associated details. | ||
| pub enum PaymentMetadataDetail { |
There was a problem hiding this comment.
Mhh, IIRC we discussed going in a different direction?
|
Discussed it offline and concluded that I'll take over the issue. |
3 participants
Add PaymentMetadata and integrate it with PaymentDetail