Skip to content

js: exclude payer-proof data-range TLVs from invoice reconstruction#33

Merged
vincenzopalazzo merged 1 commit into
rustyrussell:masterfrom
vincenzopalazzo:claude/practical-black-439c0f
Jun 26, 2026
Merged

js: exclude payer-proof data-range TLVs from invoice reconstruction#33
vincenzopalazzo merged 1 commit into
rustyrussell:masterfrom
vincenzopalazzo:claude/practical-black-439c0f

Conversation

@vincenzopalazzo

Copy link
Copy Markdown
Collaborator

Summary

  • The TS payer-proof reader treated any non-signature-range TLV as an included invoice field, so an unknown TLV in the payer-proof data range (1001..=999_999_999) was mis-counted as an invoice merkle leaf and the proof was wrongly rejected on the proof_leaf_hashes count check.
  • Fix: a record is a genuine invoice field only when type < 240 (normal) or type >= 1_000_000_000 (experimental). This mirrors the rust-lightning reference (tlv_stream_iter), which strips both the signature range (240..=1000) and the data range (1001..=999_999_999) when reconstructing the invoice merkle root.
  • Effect: the reader now tolerates ignorable proof extensions (an unknown odd data-range TLV the writer signed over) instead of rejecting them — matching the reference impl in [RFC] Add BOLT 12 payer proof primitives lightningdevkit/rust-lightning#4297.

Review Notes

  • Production-safety review passed (2 rounds). The only types whose classification changed are [1006, 999_999_999]; no valid invoice field can live there per the invoice writer rule, and all 5 valid spec vectors + 23 invalid spec vectors still pass.
  • Not a forgery vector: proof_signature commits to everything outside 240..=1000 (data-range TLVs included), so an injected data-range TLV invalidates proof_signature unless the payer signed it. The invoice signature is reconstructed over invoice fields only, matching the issuer's original root.
  • Adds a regression test that signs a proof carrying an unknown odd data-range TLV (1007) and asserts it still verifies. It fails before the fix (leaf_hashes count (11) must match included non-signature TLV count (12)).

Decision Log

Hardest decision: whether to silently exclude unknown data-range TLVs (as done here, matching rust tlv_stream_iter) versus rejecting them. I went with exclude-and-tolerate because the reference does, and because proof_signature already commits to them so toleration is safe. The alternative — full BOLT TLV even/odd "it's ok to be odd" enforcement on the proof stream — is a broader, library-wide change (js/src/tlv.ts) that this repo doesn't do anywhere yet, so I kept it out of scope.

Alternatives rejected:

  • Reject any unknown data-range TLV: diverges from the rust reference and breaks forward-compat for ignorable extensions.
  • Hardcode the data-range bounds inline at the call site: less readable; extracted isIncludedInvoiceType instead to document the rule once.

Least confident about: the even/odd TLV semantics gap. An unknown even data-range TLV is silently excluded here but rust's typed TLV stream would reject it as an unknown mandatory field. That is a pre-existing, library-wide TLV-handling difference (the TS lib never enforces even/odd mandatory rules), not specific to payer proofs, so I left it for a follow-up. Happy to revisit if you want it enforced here.

Test plan

  • npx tsc --noEmit clean
  • npm test — offers 53, payer-proof 35 (was 34), generated 22, verify 3
  • Regression test fails without the fix, passes with it

@vincenzopalazzo vincenzopalazzo left a comment

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.

Self-review: I found one spec-compliance issue in payer-proof TLV parsing. The inline comment captures the specific concern.

Comment thread js/src/payer_proof.ts
The reader treated any non-signature-range TLV as an included invoice
field, so a TLV in the payer-proof data range (1001..=999_999_999) was
mis-counted as an invoice merkle leaf and the proof was wrongly rejected
on the leaf-hash count.

Classify a record as a genuine invoice field only when its type is < 240
(normal) or >= 1_000_000_000 (experimental), mirroring rust-lightning's
tlv_stream_iter. Within the data range the known proof fields 1001..=1005
are matched explicitly; any other type is handled per the BOLT "it's ok
to be odd" rule -- even types rejected at parse, odd types ignored (and
still committed by proof_signature).

Adds regression tests: an unknown odd data-range TLV (1007) signed into a
proof still verifies, while an unknown even one (1006) is rejected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vincenzopalazzo vincenzopalazzo force-pushed the claude/practical-black-439c0f branch from f01d52c to f14c01d Compare June 26, 2026 12:27
@vincenzopalazzo vincenzopalazzo merged commit 8c90caf into rustyrussell:master Jun 26, 2026
4 checks passed
vincenzopalazzo added a commit that referenced this pull request Jun 26, 2026
Bump version 0.1.1 -> 0.1.2 and add a changelog. This release ships the
payer-proof data-range TLV handling fix (PR #33): the reader excludes the
payer-proof data range (1001..=999_999_999) from invoice merkle
reconstruction and rejects unknown even TLVs in that range per the BOLT
"it's ok to be odd" rule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant