Commit to payment_metadata in inbound payment HMAC#4528
Commit to payment_metadata in inbound payment HMAC#4528TheBlueMatt wants to merge 2 commits intolightningdevkit:mainfrom
Conversation
TheBlueMatt
commented
Mar 31, 2026
`payment_metadata` is a separate concept at the BOLT 11 layer (similar to payment secret, but arbitrary-sized) and at the BOLT 12 layer, so referring to payment information as "payment metadata" is confusing. Instead, use simply "payment info".
When payment_metadata is set in a BOLT 11 invoice, users expect to receive it back as-is in the payment onion. In order to ensure it isn't tampered with, they presumably will add an HMAC, or worse, not add one and forget that it can be tampered with. Instead, here we include it in the HMAC computation for the payment secret. This ensures that the sender must relay the correct metadata for the payment to be accepted by the receiver, binding the metadata to the payment cryptographically. The metadata is only included in the HMAC when present, so existing payments without metadata continue to verify correctly. However, this does break receiving payments with metadata today. On an upgrade this seems acceptable to me given we have seen almost no use of payment metadata in practice. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I've assigned @valentinewallace as a reviewer! |
| @@ -0,0 +1,6 @@ | |||
| # Backwards compat | |||
| * Payment metadata is now comitted to in the HMAC used to build payment secrets. | |||
There was a problem hiding this comment.
Typo: "comitted" → "committed"
| * Payment metadata is now comitted to in the HMAC used to build payment secrets. | |
| * Payment metadata is now committed to in the HMAC used to build payment secrets. |
| if let Some(metadata) = payment_metadata { | ||
| hmac.input(metadata); | ||
| } |
There was a problem hiding this comment.
Design consideration: None and Some(&[]) are cryptographically indistinguishable here (and at every other HMAC site in this PR) since hmac.input(&[]) is a no-op. This means:
- A payment created with
payment_metadata = Nonewill verify successfully if the sender sendspayment_metadata = Some(vec![])in the onion (and vice versa).
In practice this seems harmless since empty metadata carries no information, but if you want to formally distinguish "no metadata" from "empty metadata", you could prefix the HMAC input with a flag byte (e.g., hmac.input(&[1u8]) when metadata is Some, hmac.input(&[0u8]) when None).
| pub fn get_payment_preimage( | ||
| &self, payment_hash: PaymentHash, payment_secret: PaymentSecret, | ||
| payment_metadata: Option<&[u8]>, |
There was a problem hiding this comment.
The doc comment for this public API method doesn't mention the new payment_metadata parameter. Callers need to know that if a payment was created with payment_metadata via create_inbound_payment, the same metadata must be provided here for preimage derivation to succeed. Without this documentation, a caller may pass None and get a confusing APIMisuseError.
| pub fn create_inbound_payment( | ||
| &self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32, | ||
| min_final_cltv_expiry_delta: Option<u16>, | ||
| min_final_cltv_expiry_delta: Option<u16>, payment_metadata: Option<&[u8]>, |
There was a problem hiding this comment.
The doc comment for create_inbound_payment doesn't document the new payment_metadata parameter. Worth adding a note explaining that when provided, the metadata is committed to in the payment secret HMAC, so the sender must relay it exactly for payment verification to succeed.
Review SummaryInline comments posted:
Cross-cutting observations:
|