[RFC] Add BOLT 12 payer proof primitives#4297
[RFC] Add BOLT 12 payer proof primitives#4297vincenzopalazzo wants to merge 20 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4297 +/- ##
==========================================
- Coverage 87.15% 86.15% -1.00%
==========================================
Files 161 158 -3
Lines 109251 110380 +1129
Branches 109251 110380 +1129
==========================================
- Hits 95215 95097 -118
- Misses 11560 12633 +1073
- Partials 2476 2650 +174
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
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.
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
2 times, most recently
from
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
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.
|
🔔 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. |
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
5 times, most recently
from
fb8c68c to
9ad5c35
Compare
Rename the old PaidBolt12Invoice enum to Bolt12InvoiceType, move it out of events, and update outbound payment plumbing to store the renamed invoice type directly.
Encapsulate the paid invoice, preimage, and payer nonce in the PaidBolt12Invoice struct and surface it through Event::PaymentSent::bolt12_invoice. To support the nonce round-trip, plumb payment_nonce through HTLCSource::OutboundRoute, SendAlongPathArgs, PendingOutboundPayment::Retryable and the outbound payment internals, and extract it from the OffersContext variants so payers can later re-derive the payer signing key from the same nonce used for the invoice request. Update expect_payment_sent, claim_payment, claim_payment_along_route and the async-payments test assertions to surface and consume the PaidBolt12Invoice. Also add Writeable/Readable impls for sha256::Hash in util::ser so PaidBolt12Invoice serialization compiles. Co-Authored-By: Jeffrey Czyz <jkczyz@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
from
86f5b71 to
3d080ad
Compare
|
Ok I rebased to fix the conflict (hope it was real and not just a github bug) and I pushed as a fixup commit the last iteration done on the specification lightning/bolts#1295 (comment) I would like to give another pass myself, but I think it is good to put it out there for early review too |
| //! | ||
| //! # Proposed authentication change (full payer-proof merkle root) | ||
| //! | ||
| //! The current spec (BOLT 12 PR 1295) signs only `SHA256(payer_signature.note || | ||
| //! invoice_merkle_root)` with `invreq_payer_id`. Tamper-resistance for the | ||
| //! remaining payer-proof TLVs (`preimage`, `omitted_tlvs`, `missing_hashes`, | ||
| //! `leaf_hashes`) is *transitive*: each one has a separate binding (the | ||
| //! preimage hashes to `invoice_payment_hash`; the rest reconstruct the invoice | ||
| //! merkle root that the issuer's `signature` covers). It works only because | ||
| //! every payer_proof TLV today is either part of the already-signed invoice or | ||
| //! has an out-of-band binding to it. Any future payer-side TLV outside both | ||
| //! categories silently loses authentication — see | ||
| //! <https://github.com/lightning/bolts/pull/1295#discussion_r3160114065>. | ||
| //! | ||
| //! T-bast's proposal is to sign the merkle root of *all* payer_proof TLVs and | ||
| //! to extract `note` into its own TLV (a normal merkle leaf instead of being | ||
| //! bundled inside the `payer_signature` TLV). Under that scheme: | ||
| //! | ||
| //! 1. `payer_signature` becomes a plain `bip340sig` like every other signature | ||
| //! TLV in BOLT 12. | ||
| //! 2. `note` becomes a dedicated TLV (`PAYER_PROOF_PROOF_NOTE_TYPE`, see | ||
| //! below); the type number is provisional and may move when the spec | ||
| //! reallocates payer-proof data TLVs out of the `SIGNATURE_TYPES` range. | ||
| //! 3. The `payer_signature` is a tagged signature over the merkle root of all | ||
| //! payer_proof TLVs except the `payer_signature` TLV itself, computed | ||
| //! exactly as `signature` is computed for invoices/offers/invoice requests. | ||
| //! | ||
| //! Verifier flow under the new scheme is two signature checks (instead of the | ||
| //! current implicit chain of three): | ||
| //! | ||
| //! 1. Verify `payer_signature` against the payer-proof merkle root using | ||
| //! `invreq_payer_id`. | ||
| //! 2. Reconstruct the invoice merkle root from `leaf_hashes`, | ||
| //! `missing_hashes`, `omitted_tlvs`, and the disclosed invoice TLVs, then | ||
| //! verify the issuer's `signature` against it using `invoice_node_id`. | ||
| //! | ||
| //! `SHA256(preimage) == invoice_payment_hash` remains a separate check. | ||
| //! | ||
| //! This branch carries the design and a placeholder TLV constant for the new | ||
| //! `payer_note` TLV; the structural code change to switch over has been | ||
| //! deferred until the spec settles on the final TLV layout (in particular | ||
| //! whether the data-bearing payer-proof TLVs `preimage`/`omitted_tlvs`/ | ||
| //! `missing_hashes`/`leaf_hashes` move out of the `SIGNATURE_TYPES` range so | ||
| //! they can be merkle leaves under the standard BOLT 12 signing convention). | ||
| //! Tracking issue: <https://github.com/lightning/bolts/issues/1332>. |
There was a problem hiding this comment.
We may want to remove after the spec meeting if this change is confirmed
| /// docs for the authentication scheme. | ||
| payer_signature: Signature, | ||
| /// Optional payer-supplied note. Its own TLV (a regular merkle leaf) | ||
| /// per BOLT 12 PR 1295 commit `0f2b026`. |
There was a problem hiding this comment.
pushed the code before Rusty updated the spec, so this needs to be removed if the change is confimed
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
from
2521206 to
5b1d61f
Compare
|
🔔 6th Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
from
85f64d3 to
18e65c8
Compare
|
🔔 8th Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
jkczyz
left a comment
jkczyz
left a comment
There was a problem hiding this comment.
Some draft review comments that I meant to publish. I think they're still relevant after the spec changes.
| // --- Serialization Round-Trip --- | ||
| // The proof can be serialized to a bech32 string (lnp...) for sharing. | ||
| let encoded = proof.to_string(); | ||
| assert!(encoded.starts_with("lnp1")); | ||
|
|
||
| // Round-trip through TLV bytes: re-parse the raw bytes (verification happens at parse time). | ||
| let decoded = PayerProof::try_from(proof.bytes().to_vec()).unwrap(); | ||
| assert_eq!(decoded.payment_preimage(), proof.payment_preimage()); | ||
| assert_eq!(decoded.payment_hash(), proof.payment_hash()); | ||
| assert_eq!(decoded.payer_signing_pubkey(), proof.payer_signing_pubkey()); | ||
| assert_eq!(decoded.issuer_signing_pubkey(), proof.issuer_signing_pubkey()); | ||
| assert_eq!(decoded.merkle_root(), proof.merkle_root()); |
There was a problem hiding this comment.
This should already be tested in the unit tests.
| // --- Test 6: Round-trip the proof with note through TLV bytes --- | ||
| let encoded = proof_with_note.to_string(); | ||
| assert!(encoded.starts_with("lnp1")); | ||
|
|
||
| let decoded = PayerProof::try_from(proof_with_note.bytes().to_vec()).unwrap(); | ||
| assert_eq!(decoded.payer_note().map(|p| p.0), Some("Paid for coffee")); | ||
| assert_eq!(decoded.payment_preimage(), payment_preimage); |
| /// `PAYER_METADATA_TYPE` (type 0) cannot be included (per spec). | ||
| PayerMetadataNotAllowed, | ||
| /// TLV types in the signature/payer-proof range (`SIGNATURE_TYPES`, i.e. | ||
| /// `240..=1000`) or in the unsupported gap between `SIGNATURE_TYPES` and | ||
| /// `EXPERIMENTAL_OFFER_TYPES` cannot be included. | ||
| DisallowedTlvType, |
There was a problem hiding this comment.
Let's just merge these and have it include the type number.
| pub fn build( | ||
| self, payer_note: Option<String>, | ||
| ) -> Result<UnsignedPayerProof<'a>, PayerProofError> { |
There was a problem hiding this comment.
Could we just make payer_note a builder method? That avoids passing None if we don't want it.
| /// the payment flow. | ||
| /// | ||
| /// [`InvoiceRequest::payer_note`]: crate::offers::invoice_request::InvoiceRequest::payer_note | ||
| pub fn build_and_sign(self, payer_note: Option<String>) -> Result<PayerProof, PayerProofError> { |
| // --- Test 1: Wrong preimage is rejected --- | ||
| let wrong_preimage = PaymentPreimage([0xDE; 32]); | ||
| let wrong_paid = payer_proof::PaidBolt12Invoice::new( | ||
| Bolt12InvoiceType::Bolt12Invoice(invoice.clone()), wrong_preimage, Some(payer_nonce), | ||
| ); | ||
| assert!(matches!(wrong_paid.prove_payer(), Err(PayerProofError::PreimageMismatch))); | ||
|
|
||
| // --- Test 2: Wrong payment_id causes key derivation failure --- | ||
| let expanded_key = bob.keys_manager.get_expanded_key(); | ||
| let secp_ctx = Secp256k1::new(); | ||
| let paid_invoice = payer_proof::PaidBolt12Invoice::new( | ||
| Bolt12InvoiceType::Bolt12Invoice(invoice.clone()), | ||
| payment_preimage, | ||
| Some(payer_nonce), | ||
| ); | ||
| let wrong_payment_id = PaymentId([0xFF; 32]); | ||
| let result = paid_invoice.prove_payer_derived(&expanded_key, wrong_payment_id, &secp_ctx); | ||
| assert!(matches!(result, Err(PayerProofError::KeyDerivationFailed))); | ||
|
|
||
| // --- Test 3: Wrong nonce causes key derivation failure --- | ||
| let wrong_nonce = Nonce::from_entropy_source(&chanmon_cfgs[0].keys_manager); | ||
| let wrong_nonce_paid = payer_proof::PaidBolt12Invoice::new( | ||
| Bolt12InvoiceType::Bolt12Invoice(invoice.clone()), payment_preimage, Some(wrong_nonce), | ||
| ); | ||
| let result = wrong_nonce_paid.prove_payer_derived(&expanded_key, payment_id, &secp_ctx); | ||
| assert!(matches!(result, Err(PayerProofError::KeyDerivationFailed))); |
There was a problem hiding this comment.
Do these really need to be tested here? Unit tests would be better.
| // --- Test 4: Minimal proof (only required fields) --- | ||
| let minimal_proof = paid_invoice | ||
| .prove_payer_derived(&expanded_key, payment_id, &secp_ctx).unwrap() | ||
| .build_and_sign(None) | ||
| .unwrap(); | ||
| // --- Test 5: Proof with selective disclosure and payer note --- | ||
| let proof_with_note = paid_invoice | ||
| .prove_payer_derived(&expanded_key, payment_id, &secp_ctx).unwrap() | ||
| .include_offer_description() | ||
| .include_offer_issuer() | ||
| .include_invoice_amount() | ||
| .include_invoice_created_at() | ||
| .build_and_sign(Some("Paid for coffee".into())) | ||
| .unwrap(); | ||
| assert_eq!(proof_with_note.payer_note().map(|p| p.0), Some("Paid for coffee")); | ||
|
|
||
| // Both proofs should verify and have the same core fields | ||
| assert_eq!(minimal_proof.payment_preimage(), proof_with_note.payment_preimage()); | ||
| assert_eq!(minimal_proof.payment_hash(), proof_with_note.payment_hash()); | ||
| assert_eq!(minimal_proof.payer_signing_pubkey(), proof_with_note.payer_signing_pubkey()); | ||
| assert_eq!(minimal_proof.issuer_signing_pubkey(), proof_with_note.issuer_signing_pubkey()); | ||
|
|
||
| // The merkle roots are the same since both reconstruct from the same invoice | ||
| assert_eq!(minimal_proof.merkle_root(), proof_with_note.merkle_root()); |
There was a problem hiding this comment.
We can probably fold these into the previous test. No need to repeat all the setup just to create different proofs.
|
|
||
| const EXPERIMENTAL_TEST_TLV_TYPE: u64 = 1_000_000_001; | ||
|
|
||
| fn write_tlv_record<T: Writeable>(bytes: &mut Vec<u8>, tlv_type: u64, value: &T) { |
There was a problem hiding this comment.
Why do all these tests manually construct the TLVs? Can they use the invoice builder instead?
|
I asked Claude to compare the implementation against the spec and to look for any missing test coverage. Here's its report. @vincenzopalazzo See the section on identified gaps. The earlier sections are just showing its work. BOLT 1295 Payer Proof — Spec Compliance ReviewContextBranch Source files:
The PR base commit is Spec → Code Map1. TLV Field Definitions (
|
| Spec TLV | Type | Implementation |
|---|---|---|
signature |
240 (bip340sig) | payer_proof.rs:162 (PAYER_PROOF_ISSUER_SIGNATURE_TYPE); declared in tlv_stream! at payer_proof.rs:501-506 |
proof_signature |
241 (bip340sig) | payer_proof.rs:163 (PAYER_PROOF_PROOF_SIGNATURE_TYPE); same tlv_stream! |
proof_preimage |
1001 (32*byte) | payer_proof.rs:164; declared at payer_proof.rs:511-519 |
proof_omitted_tlvs |
1002 (...*bigsize) | payer_proof.rs:165 (named PAYER_PROOF_OMITTED_TLVS_TYPE, field name proof_omitted_markers) |
proof_missing_hashes |
1003 (...*sha256) | payer_proof.rs:166 |
proof_leaf_hashes |
1004 (...*sha256) | payer_proof.rs:167 |
proof_note |
1005 (...*utf8) | payer_proof.rs:168 |
| Offer/invreq/invoice fields (2-22, 80-91, 160-176) | per-type | Parsed via OfferTlvStream, InvoiceRequestTlvStream, InvoiceTlvStream from existing modules |
HRP lnp defined at payer_proof.rs:174. Signature tag "lightningpayer_proofproof_signature" at payer_proof.rs:178.
2. Writer Requirements
| Spec rule | Implementation | Test |
|---|---|---|
MUST NOT include invreq_metadata |
include_type rejects type 0 at payer_proof.rs:368-370; serialization filters invoice TLVs by included_types (which never contains 0) at payer_proof.rs:594-599 |
test_invreq_metadata_not_allowed payer_proof.rs:1490 (only checks the constant); test_parsing_rejects_payer_metadata payer_proof.rs:1632 (parse-time) |
MUST include invreq_payer_id, invoice_payment_hash, invoice_node_id, signature, invoice_features (if present) |
default_included_types payer_proof.rs:267-276; signature always copied via serialize_payer_proof payer_proof.rs:602-606 |
Indirect via creates_and_verifies_payer_proof_after_offer_payment offers_tests.rs:2694 |
MUST include proof_preimage |
Always written at payer_proof.rs:611-619 |
Round-trip tests (e.g., test_round_trip_with_trailing_experimental_tlvs payer_proof.rs:1834) |
Marker calculation per compute_omitted_markers rules |
merkle.rs:423-440 (compute_omitted_markers) |
test_omitted_markers_edge_cases payer_proof.rs:1308, test_omitted_markers_all_included payer_proof.rs:1328. GAP: see §A below |
proof_omitted_tlvs MAY be omitted when empty |
Conditional emit at payer_proof.rs:608-610 |
Implicit (round-trip with all-included) |
proof_note MAY be included |
Optional field threaded through builder/sign | creates_payer_proof_with_note_and_selective_disclosure offers_tests.rs:2805 |
proof_missing_hashes in post-order DFS smallest-to-largest TLV order |
build_tree_dfs merkle.rs:457-477 (recursive split with left-first traversal, push after both children processed) |
test_selective_disclosure_round_trip merkle.rs:944, test_missing_hashes_for_synthetic_tree merkle.rs:1001 |
MUST copy signature into payer_proof |
payer_proof.rs:602-606 always sets invoice_signature: Some(...) |
Round-trip tests |
proof_signature over merkle-root, with first_tlv = 0x0000 |
proof_signature_hash payer_proof.rs:450-452 calls from_valid_tlv_stream_bytes_with_empty_nonce_seed (merkle.rs:51-58) which uses [0x00, 0x00] as nonce seed |
Indirect via round-trip tests |
3. Reader Requirements (MUST reject if…)
| Spec rule | Implementation | Test |
|---|---|---|
missing invreq_payer_id |
payer_proof.rs:788 (MissingPayerSigningPubkey) |
GAP: no direct test |
missing invoice_payment_hash |
payer_proof.rs:791 (MissingPaymentHash) |
GAP: no direct test |
missing invoice_node_id |
payer_proof.rs:793 (MissingSigningPubkey) |
GAP: no direct test |
missing signature (240) |
payer_proof.rs:795 (MissingSignature) |
GAP: no direct test |
missing proof_preimage |
payer_proof.rs:797 |
GAP: no direct test |
missing proof_missing_hashes |
payer_proof.rs:802-803 |
test_parsing_rejects_missing_proof_missing_hashes payer_proof.rs:1816 |
missing proof_leaf_hashes |
payer_proof.rs:804-805 |
test_parsing_rejects_missing_proof_leaf_hashes payer_proof.rs:1826 |
missing proof_signature |
payer_proof.rs:798-799 |
GAP: no direct test |
SHA256(proof_preimage) ≠ invoice_payment_hash |
payer_proof.rs:870-873 |
GAP: no direct test (builder-side PreimageMismatch is tested) |
proof_omitted_tlvs not strictly ascending |
validate_omitted_markers_for_parsing ascending check at payer_proof.rs:938-940 |
test_validate_omitted_markers_rejects_non_ascending payer_proof.rs:1394 |
proof_omitted_tlvs contains 0 |
payer_proof.rs:922-925 |
test_validate_omitted_markers_rejects_zero payer_proof.rs:1347 |
proof_omitted_tlvs outside ranges 1-239 / 1_000_000_000-3_999_999_999 |
payer_proof.rs:929-935 (rejects 240-1000 / 1001-999_999_999 / ≥4_000_000_000) |
test_validate_omitted_markers_rejects_signature_types, …rejects_data_range_types, …rejects_above_experimental_range (payer_proof.rs:1357, :1369, :1383) |
proof_omitted_tlvs contains an included TLV number |
payer_proof.rs:942-945 |
test_validate_omitted_markers_rejects_included_types payer_proof.rs:1405 |
proof_omitted_tlvs not minimized (M-1 ∉ {included, prev_marker, 0 if first}) |
payer_proof.rs:947-964 |
test_validate_omitted_markers_rejects_non_minimized payer_proof.rs:1442, …non_minimized_run_start payer_proof.rs:1454, plus accepts: …accepts_trailing_run, …accepts_valid, …accepts_leading_run, …accepts_consecutive_included |
proof_leaf_hashes count ≠ non-signature TLV count |
payer_proof.rs:857-859 |
GAP: no direct test (the merkle-side test_reconstruction_fails_with_wrong_missing_hashes is adjacent but tests a different field) |
Insufficient proof_missing_hashes to reconstruct root |
merkle.rs:381-388 (InsufficientMissingHashes) |
test_reconstruction_fails_with_wrong_missing_hashes merkle.rs:1050 |
Invalid invoice signature |
payer_proof.rs:876-882 |
GAP: no direct test |
Invalid proof_signature |
payer_proof.rs:887-893 |
Indirect via test_round_trip_rejects_unknown_odd_data_range_tlv payer_proof.rs:1643 (modifies bytes, observes proof_signature failure) |
4. Invoice Writer Requirement (Added in this Spec)
"MUST NOT set any non-signature TLV fields outside the inclusive ranges: 0 to 239 and 1000000000 to 3999999999."
This applies to the invoice writer, not the payer-proof. It is enforced implicitly by lightning/src/offers/invoice.rs: the Bolt12Invoice builder only emits TLVs in OFFER_TYPES (1..80), INVOICE_REQUEST_TYPES (80..160), INVOICE_TYPES (160..240), and the experimental ranges (1_000_000_000+ via EXPERIMENTAL_OFFER_TYPES/EXPERIMENTAL_INVOICE_REQUEST_TYPES/EXPERIMENTAL_INVOICE_TYPES). Parser likewise rejects unknown types via the all-bytes-consumed check in ParsedMessage. No test directly asserts an invoice with type 1500 or type 4_000_000_000 is rejected, but the existing parser machinery makes that a structural guarantee.
5. Merkle / Selective Disclosure
| Spec aspect | Implementation | Test |
|---|---|---|
Per-TLV hash = Branch(Leaf(record), Nonce(type_bytes)), nonce_tag seeded by first TLV (= TLV 0 / invreq_metadata for invoices) |
compute_selective_disclosure merkle.rs:375-415 |
test_selective_disclosure_computation payer_proof.rs:1239 |
proof_missing_hashes is the omitted-side hash at each "exactly one branch entirely omitted" internal node, in post-order DFS |
build_tree_dfs merkle.rs:457-477 |
test_missing_hashes_for_synthetic_tree merkle.rs:1001 |
| Reconstruction reverses the same DFS | reconstruct_merkle_root / reconstruct_merkle_root_dfs merkle.rs:355-410 |
test_selective_disclosure_round_trip merkle.rs:944, test_reconstruction_fails_with_wrong_missing_hashes merkle.rs:1050 |
| Invoice merkle root excludes fields 1001..=999_999_999 (proof data) | tlv_stream_iter payer_proof.rs:834-841 filters SIGNATURE_TYPES and PAYER_PROOF_DATA_TYPES |
Indirect via end-to-end round-trip |
proof_signature merkle root uses [0x00, 0x00] as nonce seed |
from_valid_tlv_stream_bytes_with_empty_nonce_seed merkle.rs:51-58 |
Indirect via round-trip |
Identified Gaps
A. Mis-named "spec example" test (low severity, doc only)
test_omitted_markers_spec_example (payer_proof.rs:1271) uses included = {10, 40} and asserts omitted_markers == [11, 12, 41, 42]. The actual BOLT 1295 spec example uses included = {40} only and the expected markers are [1, 2, 3, 41, 42]. The implementation correctly handles the real spec case (verified by tracing compute_omitted_markers), but the test does not directly exercise it.
B. Spec test vectors are NOT verified (high severity)
check_against_spec_vectors at payer_proof.rs:2056 is #[ignore]d with the message "spec test vectors need to be regenerated against the current full-tree signing scheme". Until the upstream draft stabilizes its test vectors, cross-implementation interop is unverified. The four embedded vectors (PAYER_PROOF_VECTORS at payer_proof.rs:1942-1983) are not exercised at all.
C. Reader-rejection tests for "missing required field" are sparse (medium severity)
For each MUST-include field — invreq_payer_id (88), invoice_payment_hash (168), invoice_node_id (176), signature (240), proof_preimage (1001), proof_signature (241), proof_missing_hashes (1003), proof_leaf_hashes (1004) — the spec requires rejection if missing. Only the last two have direct tests (test_parsing_rejects_missing_proof_*). The other six rely on existing Bolt12SemanticError variants returned by ParsedPayerProofFields::try_from, but no test strips one of those TLVs and asserts the parse fails.
D. No direct test for preimage / signature mismatch on the parse path (medium severity)
PayerProof::try_from performs three cryptographic checks at payer_proof.rs:870-893: preimage hash, invoice signature, proof signature. None has a direct test that takes a valid proof, perturbs exactly that one field, and asserts a parse rejection. test_round_trip_rejects_unknown_odd_data_range_tlv payer_proof.rs:1643 is the closest analogue (modifies bytes after signing, observes failure) but only covers the appended-TLV case, not edits to the preimage / leaf hashes / missing hashes.
E. No direct test for proof_leaf_hashes count mismatch (medium severity)
The check at payer_proof.rs:857-859 rejects when leaf_hashes.len() != included_records.len(). Stripping one hash from proof_leaf_hashes (or duplicating one) should fail with DecodeError::InvalidValue. Not currently tested.
F. Theoretical edge case: marker overflow into invalid range (low severity, unreachable for real invoices)
compute_omitted_markers (merkle.rs:423-440) computes each marker as prev_value + 1 without checking bounds. If a single run of omitted TLVs spans across the gap from 239 to 1_000_000_000 (i.e., 240+ consecutive omitted non-signature TLVs in the 1-239 region followed by experimental TLVs), markers could fall in the invalid range 240-999_999_999. Real BOLT 12 invoices have ≤ ~15 non-signature TLVs, so this is unreachable in practice; the reader will reject such markers via validate_omitted_markers_for_parsing either way. The writer should still ideally debug_assert! or refuse construction. Not a correctness bug for current invoice shapes; flag for documentation.
G. End-to-end test asymmetry: no "third-party verification" test (low severity)
creates_and_verifies_payer_proof_after_offer_payment offers_tests.rs:2694 builds a proof and round-trips it through PayerProof::try_from, but it does not exercise the third-party verifier scenario where someone receives a lnp1... bech32 string from a different party and must verify it without knowing the original payment context. The test is close to this — try_from does run all signature checks — but does not explicitly demonstrate proof-of-payment to a verifier who didn't make the payment.
Plan to Fix
The implementation is correct on every spec rule I traced. The gaps are predominantly test coverage, with one doc-only naming fix and one deferred upstream dependency (test vectors). Recommended changes, in priority order:
Fix 1 (gap A): Rename or split the misleading test
In lightning/src/offers/payer_proof.rs:
- Rename
test_omitted_markers_spec_example→test_omitted_markers_two_included_runs(it tests the two-included-runs shape, not the spec example). - Add a new
test_omitted_markers_spec_examplethat usesincluded = {40}for the TLV stream[0, 10, 20, 30, 40, 50, 60]and assertsomitted_markers == [1, 2, 3, 41, 42]andmissing_hashes.len() == 3(for [TLV50, TLV60, left_subtree]).
Fix 2 (gap C): Add tests for each "missing required field" rejection
In the existing helper pattern (proof_bytes_without_tlv at payer_proof.rs:1803), add tests modeled on test_parsing_rejects_missing_proof_missing_hashes:
test_parsing_rejects_missing_payer_id— strip TLV 88, assertMissingPayerSigningPubkey.test_parsing_rejects_missing_payment_hash— strip TLV 168, assertMissingPaymentHash.test_parsing_rejects_missing_node_id— strip TLV 176, assertMissingSigningPubkey.test_parsing_rejects_missing_invoice_signature— strip TLV 240, assertMissingSignature.test_parsing_rejects_missing_proof_signature— strip TLV 241, assertMissingSignature.test_parsing_rejects_missing_proof_preimage— strip TLV 1001, assertDecode(InvalidValue).
Use build_round_trip_proof_with_disclosed_fields payer_proof.rs:1157 as the source proof.
Fix 3 (gap D): Add tamper-detection tests
Add three tests that take a valid proof, mutate one byte in a specific TLV, and assert parse failure:
test_parsing_rejects_modified_preimage— flip a byte in theproof_preimagevalue, assert preimage-hash mismatch.test_parsing_rejects_modified_leaf_hash— flip a byte inproof_leaf_hashes, assert invoice signature verification fails.test_parsing_rejects_modified_missing_hash— flip a byte inproof_missing_hashes, assert invoice signature verification fails.
These complement test_round_trip_rejects_unknown_odd_data_range_tlv (which covers the proof_signature failure path).
Fix 4 (gap E): Add proof_leaf_hashes count mismatch test
test_parsing_rejects_leaf_hashes_count_mismatch — take a valid proof, drop the first 32 bytes of the proof_leaf_hashes TLV's value (one hash short), re-emit, assert Decode(InvalidValue). The drop must preserve TLV framing (re-encode the length).
Fix 5 (gap F): Add debug_assert! and unit test for marker bounds
In compute_omitted_markers (merkle.rs:423-440), add a debug_assert!(marker <= 239 || (1_000_000_000..=3_999_999_999).contains(&marker)) after the *prev_value = marker line. Also add compute_omitted_markers_panics_on_overflow (under #[cfg(test)]) using a synthetic tlv_data with 240+ consecutive omitted TLVs starting from type 1.
This formalizes the invariant. It cannot fire for real invoices but will catch future spec changes that allow more TLV slots.
Fix 6 (gap B): Re-enable spec vectors when upstream stabilizes
This is upstream-dependent. Track lightning/bolts#1295. When the spec settles and produces final test vectors, regenerate PAYER_PROOF_VECTORS payer_proof.rs:1942 and remove #[ignore] from check_against_spec_vectors payer_proof.rs:2055. Until then, document the dependency in the module-level doc comment of payer_proof.rs:10-18.
Fix 7 (gap G): Optional — third-party verification example
In lightning/src/ln/offers_tests.rs, extend creates_and_verifies_payer_proof_after_offer_payment (or add a sibling test) to demonstrate: produce a proof on Bob's side, transfer the bech32 string to Alice (via let bech32 = payer_proof.to_string(); let received: PayerProof = bech32.parse().unwrap();), and verify the disclosed fields against an independently held offer reference. Optional but improves documentation value.
Critical Files to Modify
lightning/src/offers/payer_proof.rs— rename existing test, add ~9 new tests in the existingmod tests.lightning/src/offers/merkle.rs— adddebug_assert!incompute_omitted_markers, add panics-on-overflow test.lightning/src/ln/offers_tests.rs— optional new test for third-party verification.
Reused Patterns
proof_bytes_without_tlvpayer_proof.rs:1803-1811— reuse for stripping TLVs in Fix 2.build_round_trip_proof_with_disclosed_fieldspayer_proof.rs:1157-1236— source proof for Fix 2/3/4.write_tlv_record_bytespayer_proof.rs:1015-1019— for byte-level construction.report_bech32_mismatchpayer_proof.rs:2006-2051— already in place for Fix 6 when re-enabled.
Verification
Run the new tests:
cargo +1.75.0 test -p lightning --features std offers::payer_proof
cargo +1.75.0 test -p lightning --features std offers::merkle
cargo +1.75.0 test -p lightning --features std --test '*' creates_and_verifies_payer_proof
Confirm none of the existing tests regress, and the renamed test_omitted_markers_spec_example actually mirrors the spec example values from BOLT 1295.
vincenzopalazzo
force-pushed
the
macros/proof-of-payment-bolt12-spec
branch
from
65fcd6c to
046685a
Compare
6 participants
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