[RFC] Add BOLT 12 payer proof primitives#4297
[RFC] Add BOLT 12 payer proof primitives#4297vincenzopalazzo wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4297 +/- ##
==========================================
- Coverage 85.86% 85.63% -0.23%
==========================================
Files 159 160 +1
Lines 104302 105267 +965
Branches 104302 105267 +965
==========================================
+ Hits 89558 90149 +591
- Misses 12246 12599 +353
- Partials 2498 2519 +21
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:
|
TheBlueMatt
left a comment
There was a problem hiding this comment.
A few notes, though I didn't dig into the code at a particularly low level.
| self.included_types.insert(164); | ||
| self | ||
| } | ||
|
|
There was a problem hiding this comment.
Probably worth having a generic "select by id" thing (maybe requiring it be in the experimental range?), given we'll add support for custom TLVs eventually.
There was a problem hiding this comment.
There's already an include_type(tlv_type: u64) method that allows including any TLV by its type number (except invreq_metadata which is forbidden by spec). Is that what you're looking for, or did you want something more restrictive that only allows experimental types (>= 1,000,000,000)?
lightning/src/offers/merkle.rs
Outdated
| let mut is_included: Vec<bool> = vec![false; num_leaves]; | ||
| let mut min_types: Vec<u64> = vec![u64::MAX; num_leaves]; |
There was a problem hiding this comment.
Would be nice to avoid the unnecessary allocations here.
lightning/src/offers/merkle.rs
Outdated
| let branch_tag = tagged_hash_engine(sha256::Hash::hash("LnBranch".as_bytes())); | ||
|
|
||
| let mut hashes: Vec<Option<sha256::Hash>> = vec![None; num_leaves]; | ||
| let mut is_included: Vec<bool> = vec![false; num_leaves]; |
There was a problem hiding this comment.
Same here, there's lot of allocations that we should be able to avoid.
| invoice_created_at: Option<Duration>, | ||
| #[allow(dead_code)] | ||
| invoice_features: Option<Bolt12InvoiceFeatures>, | ||
| } |
There was a problem hiding this comment.
We presumably want a place to store custom included TLVs.
2324361 to
9f84e19
Compare
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 <noreply@anthropic.com>
|
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
TheBlueMatt
left a comment
There was a problem hiding this comment.
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.
lightning/src/offers/payer_proof.rs
Outdated
| /// The derived key must match the `payer_id` in the original invoice for the signature | ||
| /// to be valid. | ||
| pub fn sign_with_derived_key( | ||
| self, expanded_key: &ExpandedKey, nonce: Nonce, note: Option<&str>, |
There was a problem hiding this comment.
Hmm, the Nonce is stored in the invoice iirc so it would be nice to not have to pass this back in. Not sure if its better to store the nonce in the unsigned proof or just not have an unsigned proof at all (having the last step of the builder be signing, rather than building). What's the rationale for having separate unsigned + signed types?
There was a problem hiding this comment.
Addressed the second part of this — removed the public UnsignedPayerProof type entirely and moved signing into the builder, matching the build_and_sign pattern used by InvoiceRequestBuilder and InvoiceBuilder. The builder now has:
build_and_sign(sign_fn, note)— for explicit signing with a closurebuild_and_sign_with_derived_key(expanded_key, nonce, note)— for derived key signing
Regarding the first part (not having to pass the Nonce back in): I may be missing something, but from tracing through the code, I think the nonce isn't actually stored in the invoice's payer metadata for the DerivedSigningPubkey path. When derive_metadata_and_keys runs in signer.rs, it only puts the encrypted_payment_id into the resulting Metadata::Bytes — the nonce is consumed during derivation.
In the ChannelManager flow, the nonce is carried separately through the OffersContext::OutboundPaymentForOffer { payment_id, nonce } in the blinded path context, and verify_using_payer_data reconstructs the metadata from that context rather than reading it from the invoice.
So I kept nonce as a parameter on build_and_sign_with_derived_key. Happy to change this if I'm wrong about the metadata layout though!
|
🔔 2nd Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @jkczyz! This PR has been waiting for your review. |
83b32bd to
59a84c3
Compare
Implements payer proofs for BOLT 12 invoices as specified in lightning/bolts#1295. A payer proof cryptographically demonstrates that a BOLT 12 invoice was paid using selective disclosure of invoice fields, the payment preimage, and signatures from both the invoice issuer and the payer.
59a84c3 to
9b3865f
Compare
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:
This PR adds the core building blocks:
This is explicitly a PoC to validate the API surface - the spec itself is still being refined. Looking for feedback on:
cc @TheBlueMatt @jkczyz