Add a payment_metadata map in blinded * path contexts#4584
Add a payment_metadata map in blinded * path contexts#4584TheBlueMatt wants to merge 4 commits intolightningdevkit:mainfrom
payment_metadata map in blinded * path contexts#4584Conversation
We almost certainly don't want to be moving `option` TLVs during serialization, and while we had logic elsewhere to work around this previously its nice not to have to in the future.
Similar to how BOLT 11 payments can use a `payment_metadata` to provide arbitrary bytes in the invoice to be communicated back to them when receiving, its useful to be able to provide some bytes which are communicated back upon receiving a payment. Here we do so in the BOLT 12 blinded path contexts, offering a `BTreeMap<u64, Vec<u8>>` instead to enable more easily including multiple sets of data. Also note that a `Router` building a blinded path is allowed to modify the `payment_metadata` without breaking the payment. Tests by claude
|
👋 Thanks for assigning @tnull as a reviewer! |
f876c82 to
7c4e653
Compare
Similar to how BOLT 11 payments can use a `payment_metadata` to provide arbitrary bytes in the invoice to be communicated back to them when receiving, its useful to be able to provide some bytes which are communicated back upon receiving a payment. Here we do so in the BOLT 12 blinded message path contexts, offering a `BTreeMap<u64, Vec<u8>>` instead to enable more easily including multiple sets of data. We don't yet wire it up to the public `ChannelManager` API, but do allow selecting values for those using the manual `OffersMessageFlow`. Tests by claude
7c4e653 to
ee6ba89
Compare
| /// [`OffersContext::InvoiceRequest::payment_metadata`] prior to blinded path construction is | ||
| /// allowed. |
There was a problem hiding this comment.
Bug: OffersContext::StaticInvoiceRequested::payment_metadata does not exist. The StaticInvoiceRequested variant only has recipient_id, invoice_slot, and path_absolute_expiry fields — no payment_metadata was added to it in this PR.
This will either be a broken rustdoc link or misleading documentation. Either add payment_metadata to StaticInvoiceRequested or remove this reference.
| /// needs to be "stored" by a payment recipient for their own internal use, provided back to | ||
| /// them with the payment. | ||
| /// | ||
| /// Note that because this is included in the payment onion, its size must be tighly |
There was a problem hiding this comment.
Typo: "tighly" → "tightly"
| /// provided to payers via [`Bolt11Invoice::payment_metadata`]) and can be used any time data | ||
| /// needs to be "stored" by a payment recipient for their own internal use, provided back to | ||
| /// them with the payment. | ||
| /// |
There was a problem hiding this comment.
Typo: "tighly" → "tightly" (same typo appears in all four payment_metadata doc comments in this file — Bolt12OfferContext, AsyncBolt12OfferContext, and Bolt12RefundContext)
Review SummaryNo new issues found beyond what was flagged in the prior review pass. Previously flagged issues (still applicable):
Verification of correctness across the PR:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4584 +/- ##
==========================================
- Coverage 87.15% 87.14% -0.01%
==========================================
Files 161 161
Lines 109251 109322 +71
Branches 109251 109322 +71
==========================================
+ Hits 95215 95270 +55
- Misses 11560 11571 +11
- Partials 2476 2481 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
We do so both in the blinded message and payment paths, supporting async payments when data is injected in the blinded payment paths (eg via the
Router). We don't expose building offers with metadata yet.