Skip to content

Add a payment_metadata map in blinded * path contexts#4584

Open
TheBlueMatt wants to merge 6 commits intolightningdevkit:mainfrom
TheBlueMatt:2026-04-bolt12-custom-data
Open

Add a payment_metadata map in blinded * path contexts#4584
TheBlueMatt wants to merge 6 commits intolightningdevkit:mainfrom
TheBlueMatt:2026-04-bolt12-custom-data

Conversation

@TheBlueMatt
Copy link
Copy Markdown
Collaborator

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 * 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

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.

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.
@TheBlueMatt TheBlueMatt requested a review from tnull May 1, 2026 02:04
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 1, 2026

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@TheBlueMatt TheBlueMatt force-pushed the 2026-04-bolt12-custom-data branch 2 times, most recently from 7c4e653 to ee6ba89 Compare May 1, 2026 02:05
Comment thread lightning/src/onion_message/messenger.rs
Comment thread lightning/src/blinded_path/message.rs Outdated
Comment thread lightning/src/blinded_path/payment.rs
@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented May 1, 2026

I've thoroughly reviewed the entire PR diff, cross-referencing serialization macros, flow propagation, pattern match exhaustiveness, backward compatibility, and deserialization safety. All areas I previously flagged have been verified as resolved (typos fixed, doc references corrected).

After this comprehensive re-review, I found no new issues beyond those already covered by my prior inline comments.

No new issues found.

Prior inline comments status:

  • lightning/src/blinded_path/message.rs:407 — "tighly" typo: verified fixed
  • lightning/src/blinded_path/payment.rs:621 — "tighly" typo across all doc comments: verified fixed
  • lightning/src/onion_message/messenger.rs:475OffersContext::StaticInvoiceRequested::payment_metadata doc reference: verified the actual code references OffersContext::InvoiceRequest::payment_metadata which is correct (my prior comment was based on a misread; the reference exists as a field on the InvoiceRequest variant)

Verification summary of key areas:

  • TLV serialization: All payment_metadata fields use odd type 1 (backward-compatible optional). Ascending TLV order maintained in all structs/enums.
  • (option, encoding) macro expansion: BigSizeKeyedMap<&BTreeMap<...>>, AccountableBool<&bool>, WithoutLength<&&Features> all have matching Writeable impls for the reference-based expansion path.
  • BigSizeKeyedMap deserialization: Uses LengthReadable with LengthLimitedRead, bounding total bytes by TLV length. Duplicate keys rejected. No pre-allocation from declared count.
  • Metadata propagation: Correct through all three paths — (1) offer message context → channelmanager extraction → payment context, (2) router override in create_blinded_payment_paths, (3) AsyncBolt12OfferContextBolt12OfferContext conversion for keysend.
  • Pattern match exhaustiveness: All 9 pattern matches on OffersContext::InvoiceRequest and all constructions of the variant across 6 files correctly include payment_metadata.
  • Test coverage: Three new tests cover the three main injection paths.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

❌ Patch coverage is 85.26316% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.23%. Comparing base (42e198c) to head (6bfdc0d).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/util/ser.rs 74.07% 1 Missing and 6 partials ⚠️
lightning/src/blinded_path/payment.rs 71.42% 6 Missing ⚠️
lightning/src/util/test_utils.rs 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4584      +/-   ##
==========================================
- Coverage   87.15%   86.23%   -0.92%     
==========================================
  Files         161      159       -2     
  Lines      109251   109248       -3     
  Branches   109251   109248       -3     
==========================================
- Hits        95215    94209    -1006     
- Misses      11560    12423     +863     
- Partials     2476     2616     +140     
Flag Coverage Δ
fuzzing-fake-hashes ?
fuzzing-real-hashes ?
tests 86.23% <85.26%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheBlueMatt TheBlueMatt force-pushed the 2026-04-bolt12-custom-data branch from ee6ba89 to 792846c Compare May 1, 2026 11:20
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
@TheBlueMatt TheBlueMatt force-pushed the 2026-04-bolt12-custom-data branch from 792846c to 98cff64 Compare May 1, 2026 11:25
@ldk-reviews-bot
Copy link
Copy Markdown

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Copy link
Copy Markdown
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Looks good, but it would be nice to provide a bit more ergonomic/self-descriptive types here. At the very least we should document what the semantics of the u64s are (or rather, that there are ~none and the user is free to set them).

Tagging @jkczyz as second reviewer on this.

Comment thread lightning/src/blinded_path/payment.rs
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
pub payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above: what are we expecting users to set the u64 to?

Shouldn't we rather use a dedicated PaymentMetadata type for this, or even an Option<Box<dyn Writeable>> or similar?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Answered at #4584 (comment) but I do really want to force users into the K-V-map box here. Can do a type if we want but its a bit awkward that we have a separate BOLT 11 and BOLT 12 PaymentMetadata type...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The Box<dyn Writeable> would get a bit ugly at read time. You'd need to parameterize the onion message parsing with your types.

@tnull tnull requested a review from jkczyz May 5, 2026 12:18
///
/// [`RecipientOnionFields::payment_metadata`]: crate::ln::outbound_payment::RecipientOnionFields::payment_metadata
/// [`Bolt11Invoice::payment_metadata`]: lightning_invoice::Bolt11Invoice::payment_metadata
pub payment_metadata: Option<BTreeMap<u64, Vec<u8>>>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why use a map? Wouldn't a user's wrapper struct be just as efficient encoding -- or could be even more efficient if each part is of a fixed length? Deserialization would be simpler, too, if they didn't need to iterate a map.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The intention is both to force users to think about forward compatibility from day one (in the sense that its already a K-V) but also because we want to reserve some keys in LDK (eg in #4463).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The intention is both to force users to think about forward compatibility from day one (in the sense that its already a K-V)

Isn't that what using our serialization macros provide? Blinded paths are also not expected to be useable indefinitely, so the compatibility argument isn't as strong, IMO.

FWIW, the rationale given in the commit message only mentions composability.

but also because we want to reserve some keys in LDK (eg in #4463).

Wouldn't it be simpler to add a dedicated field for anything LDK-specific? Then we wouldn't need custom parsing / serialization logic (for each variant of PaymentContext) or need to worry about checking if users took a reserved type. This would need to be added somewhere in the code vs a simple declaration in the impl_writeable_tlv_based uses.

@TheBlueMatt TheBlueMatt added this to the 0.3 milestone May 5, 2026
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
@TheBlueMatt TheBlueMatt force-pushed the 2026-04-bolt12-custom-data branch from b8375a7 to 6bfdc0d Compare May 5, 2026 19:21
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.

5 participants