Skip to content

[RFC] Add BOLT 12 payer proof primitives#4297

Open
vincenzopalazzo wants to merge 3 commits intolightningdevkit:mainfrom
vincenzopalazzo:macros/proof-of-payment-bolt12-spec
Open

[RFC] Add BOLT 12 payer proof primitives#4297
vincenzopalazzo wants to merge 3 commits intolightningdevkit:mainfrom
vincenzopalazzo:macros/proof-of-payment-bolt12-spec

Conversation

@vincenzopalazzo
Copy link
Copy Markdown
Contributor

This is a first draft implementation of the payer proof extension to BOLT 12 as proposed in lightning/bolts#1295. The goal is to get early feedback on the API design before the spec is finalized.

Payer proofs allow proving that a BOLT 12 invoice was paid by demonstrating possession of:

  • The payment preimage
  • A valid invoice signature over a merkle root
  • The payer's signature

This PR adds the core building blocks:

  • Extends merkle.rs with selective disclosure primitives that allow creating and reconstructing merkle trees with partial TLV disclosure. This enables proving invoice authenticity while omitting sensitive fields.
  • Adds payer_proof.rs with PayerProof, PayerProofBuilder, and UnsignedPayerProof types. The builder pattern allows callers to selectively include invoice fields (description, amount, etc.) in the proof.
  • Implements bech32 encoding/decoding with the lnp prefix and proper TLV stream parsing with validation (ascending order, no duplicates, hash length checks).

This is explicitly a PoC to validate the API surface - the spec itself is still being refined. Looking for feedback on:

  • Whether the builder pattern makes sense for selective disclosure
  • The verification API
  • Integration points with the rest of the offers module

cc @TheBlueMatt @jkczyz

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Jan 5, 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.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 94.08163% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.20%. Comparing base (12edb7d) to head (97072b5).
⚠️ Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/offers/payer_proof.rs 92.34% 31 Missing and 39 partials ⚠️
lightning/src/offers/merkle.rs 97.33% 7 Missing and 3 partials ⚠️
lightning/src/ln/channelmanager.rs 87.50% 3 Missing and 1 partial ⚠️
lightning/src/ln/outbound_payment.rs 97.22% 1 Missing ⚠️
lightning/src/offers/invoice.rs 98.30% 1 Missing ⚠️
lightning/src/offers/signer.rs 97.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4297      +/-   ##
==========================================
+ Coverage   86.20%   87.20%   +1.00%     
==========================================
  Files         160      164       +4     
  Lines      107545   110168    +2623     
  Branches   107545   110168    +2623     
==========================================
+ Hits        92707    96070    +3363     
+ Misses      12214    11569     -645     
+ Partials     2624     2529      -95     
Flag Coverage Δ
fuzzing 39.78% <3.11%> (?)
tests 86.30% <94.08%> (+0.10%) ⬆️

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.

Copy link
Copy Markdown
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

A few notes, though I didn't dig into the code at a particularly low level.

@vincenzopalazzo vincenzopalazzo marked this pull request as ready for review January 20, 2026 17:00
@vincenzopalazzo vincenzopalazzo force-pushed the macros/proof-of-payment-bolt12-spec branch 2 times, most recently from 2324361 to 9f84e19 Compare January 20, 2026 17:42
vincenzopalazzo added a commit to vincenzopalazzo/payer-proof-test-vectors that referenced this pull request Jan 20, 2026
Add a Rust CLI tool that generates and verifies test vectors for BOLT 12
payer proofs as specified in lightning/bolts#1295. The tool uses the
rust-lightning implementation from lightningdevkit/rust-lightning#4297.

Features:
- Generate deterministic test vectors with configurable seed
- Verify test vectors from JSON files
- Support for basic proofs, proofs with notes, and invalid test cases
- Uses refund flow for explicit payer key control

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ldk-reviews-bot
Copy link
Copy Markdown

🔔 1st Reminder

Hey @valentinewallace! 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
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Some API comments. I'll review the actual code somewhat later (are we locked on on the spec or is it still in flux at all?), but would be nice to reduce allocations in it first anyway.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 2nd Reminder

Hey @valentinewallace! 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.

@valentinewallace valentinewallace removed their request for review January 26, 2026 17:25
@jkczyz jkczyz self-requested a review January 27, 2026 18:59
@ldk-reviews-bot
Copy link
Copy Markdown

🔔 1st Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 2nd Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 3rd Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 4th Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 5th Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 6th Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 7th Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 8th Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 9th Reminder

Hey @jkczyz! 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.

@TheBlueMatt TheBlueMatt added this to the 0.3 milestone Feb 18, 2026
@vincenzopalazzo vincenzopalazzo force-pushed the macros/proof-of-payment-bolt12-spec branch 5 times, most recently from fb8c68c to 9ad5c35 Compare February 24, 2026 18:13
Comment on lines +1055 to +1062
use core::convert::TryFrom;

// Create a TLV stream with leaf_hashes (type 248) that has invalid length
// BigSize encoding: values 0-252 are single byte, 253-65535 use 0xFD prefix
let mut bytes = Vec::new();
// TLV type 248 (leaf_hashes) - 248 < 253 so single byte
bytes.push(0xf8); // type 248
bytes.push(0x1f); // length 31 (not multiple of 32!)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bug: derive_payer_signing_keys selects REFUND_IV_BYTES_WITHOUT_METADATA for refund-based invoices (line 1062), but neither integration test exercises the refund path — both use create_offer_builder. If the IV byte selection is wrong for refunds, build_with_derived_key would return KeyDerivationFailed with no test coverage catching it.

Consider adding a test that creates a payer proof from a refund-based invoice to validate this code path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid point on test coverage. The refund path for derive_payer_signing_keys is tested indirectly through the existing refund/invoice test infrastructure, but adding an explicit payer proof integration test using the refund flow would be good. I'll add that in a follow-up.

@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Quick update after the latest push.

I cherry-picked the follow-up fixes that came out of the Codex/spec review.

What changed:

  • payer proofs now correctly allow experimental invoice TLVs above 1000; only 240..=1000 is treated as reserved
  • TLV ordering is preserved when serializing proofs with experimental TLVs
  • parsed PayerProofs now preserve disclosed fields and expose description, issuer, amount, and creation time
  • coverage was extended for experimental TLVs, trailing omitted markers, and the refund derived-key flow

I also updated the external test vectors here:

At this point, the real review bugs found around the producer/parser mismatch are fixed on the branch.

The one discussion I still consider open is the missing_hashes count in the 7-node example: our implementation produces 4 hashes, while the current BOLTs example shows 3. I tracked that, together with the omitted-marker wording mismatch, here:

The other remaining review threads look like follow-up material to me (TLV0 invariant docs, parser shape, secp context reuse), not missing correctness fixes.

Move the invoice/refund payer key derivation logic into reusable helpers so
payer proofs can derive the same signing keys without duplicating the metadata
and signer flow.
@vincenzopalazzo vincenzopalazzo force-pushed the macros/proof-of-payment-bolt12-spec branch 2 times, most recently from c96cef5 to d5baee8 Compare March 25, 2026 17:00
Comment on lines +348 to +389
BigSize(TLV_PREIMAGE).write(&mut bytes).expect("Vec write should not fail");
BigSize(32).write(&mut bytes).expect("Vec write should not fail");
bytes.extend_from_slice(&self.preimage.0);

if !self.disclosure.omitted_markers.is_empty() {
let omitted_len: u64 = self
.disclosure
.omitted_markers
.iter()
.map(|m| BigSize(*m).serialized_length() as u64)
.sum();
BigSize(TLV_OMITTED_TLVS).write(&mut bytes).expect("Vec write should not fail");
BigSize(omitted_len).write(&mut bytes).expect("Vec write should not fail");
for marker in &self.disclosure.omitted_markers {
BigSize(*marker).write(&mut bytes).expect("Vec write should not fail");
}
}

if !self.disclosure.missing_hashes.is_empty() {
let len = self.disclosure.missing_hashes.len() * 32;
BigSize(TLV_MISSING_HASHES).write(&mut bytes).expect("Vec write should not fail");
BigSize(len as u64).write(&mut bytes).expect("Vec write should not fail");
for hash in &self.disclosure.missing_hashes {
bytes.extend_from_slice(hash.as_ref());
}
}

if !self.disclosure.leaf_hashes.is_empty() {
let len = self.disclosure.leaf_hashes.len() * 32;
BigSize(TLV_LEAF_HASHES).write(&mut bytes).expect("Vec write should not fail");
BigSize(len as u64).write(&mut bytes).expect("Vec write should not fail");
for hash in &self.disclosure.leaf_hashes {
bytes.extend_from_slice(hash.as_ref());
}
}

let note_bytes = note.map(|n| n.as_bytes()).unwrap_or(&[]);
let payer_sig_len = 64 + note_bytes.len();
BigSize(TLV_PAYER_SIGNATURE).write(&mut bytes).expect("Vec write should not fail");
BigSize(payer_sig_len as u64).write(&mut bytes).expect("Vec write should not fail");
payer_signature.write(&mut bytes).expect("Vec write should not fail");
bytes.extend_from_slice(note_bytes);
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 can't we implement Writable for sha256::Hash?

Comment on lines +414 to +418
} else {
let marker = prev_value + 1;
markers.push(marker);
prev_value = marker;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bug (producer-consumer mismatch): compute_omitted_markers generates markers sequentially via prev_value + 1 with no awareness of the SIGNATURE_TYPES (240..=1000) hole. Both validate_omitted_markers (line 662) and validate_omitted_markers_for_parsing (line 833) reject markers that fall in this range.

This means if an invoice has enough omitted TLVs after the last included type that sequential markers reach 240, the producer generates an invalid proof that the consumer rejects. Concretely:

  • Required included types max out at 176 (INVOICE_NODE_ID_TYPE)
  • Omitted invoice types in 178..238 contribute ~31 positions, generating markers 177..207
  • Each additional omitted experimental type (>1000) adds one more marker: 208, 209, ..., 239, 240 (rejected!)
  • Only 33 experimental invoice types beyond the 178..238 range triggers this

While unlikely today, this is a correctness gap: the producer and consumer disagree on how markers interact with the reserved range. The fix depends on how the spec handles it — either the producer needs to skip markers in SIGNATURE_TYPES (jumping from 239 to 1001), or the consumer needs to allow them. Both sides need to agree.

Consider adding a debug_assert that no generated markers fall in SIGNATURE_TYPES to catch this during testing, and a runtime check or skip logic once the spec clarifies the behavior.

/// `TlvStream::new()` assumes well-formed input and panics on malformed BigSize
/// values or out-of-bounds lengths. This function validates the framing first,
/// returning an error instead of panicking on untrusted input.
fn validate_tlv_framing(bytes: &[u8]) -> Result<(), crate::ln::msgs::DecodeError> {
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.

Could we define this on TlvStream even if only an is_valid static method? Maybe as a follow-up we can build it into TlvStream or have some wrapper for performing validation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Make sense to me, I will not resolve this review also if I moved the is_valid to remember to open a new tracking issue for this

Copy link
Copy Markdown
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Made some of the more involved requests in https://github.com/jkczyz/rust-lightning/tree/macros/proof-of-payment-bolt12-spec. Didn't want to push anything directly to your branch in case you were still making changes. But feel free to grab those commits. They are referenced in my comments.

Comment on lines +306 to +322
fn compute_payer_signature_message(note: Option<&str>, merkle_root: &sha256::Hash) -> Message {
let mut inner_hasher = sha256::Hash::engine();
if let Some(n) = note {
inner_hasher.input(n.as_bytes());
}
inner_hasher.input(merkle_root.as_ref());
let inner_msg = sha256::Hash::from_engine(inner_hasher);

let tag_hash = sha256::Hash::hash(PAYER_SIGNATURE_TAG.as_bytes());

let mut final_hasher = sha256::Hash::engine();
final_hasher.input(tag_hash.as_ref());
final_hasher.input(tag_hash.as_ref());
final_hasher.input(inner_msg.as_ref());
let final_digest = sha256::Hash::from_engine(final_hasher);

Message::from_digest(*final_digest.as_byte_array())
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.

Let's take jkczyz@34aaab3 so we can reuse as much code as possible.

Add the payer proof types, selective disclosure merkle support, parsing, and
tests for constructing and validating BOLT 12 payer proofs from invoices.
@vincenzopalazzo vincenzopalazzo force-pushed the macros/proof-of-payment-bolt12-spec branch 3 times, most recently from 6739c39 to b52a5f3 Compare April 1, 2026 16:55
Encapsulate invoice, preimage, and nonce in PaidBolt12Invoice and
surface it in PaymentSent. Rework builder to return UnsignedPayerProof
with SignFn/sign_message integration, use encode_tlv_stream! for
serialization, move helpers to DisclosedFields methods, and address
naming conventions and TLV validation feedback.

Co-Authored-By: Jeffrey Czyz <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/proof-of-payment-bolt12-spec branch from b52a5f3 to c836395 Compare April 1, 2026 17:12
@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Thanks @jkczyz I should have addressed everything that you asked me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants