bolt11: reject duplicate payment hashes in invoices by Roasbeef · Pull Request #1357 · lightning/bolts

Roasbeef · 2026-09-10T20:03:00Z

In this PR, we modify the spec to reject an invoice with duplicate payment hashes.

Unfortunately there was a recent hack o fa popular telegram p2p bot that exploited this ambiguity. Some libraries took the first payment hash, while some took the last. If you were running a system that exhibited both variants of the behavior (diff between libraries, or nodes, etc), then your system could be tricked into thinking it never paid out a withdrawl, thereby leading to a vuln that could drain the system.

IMO there's no reason an invoice should have > 1 payment hash, so we should just reject it.

Test vectors have been updated accordingly.

In this commit, we correct the existing tagged-field vector to match the reader requirement for fixed-length fields. The vector previously labeled malformed p, h, s, and n fields as ignored, even though a reader must fail the payment when their lengths are invalid. We now mark the invoice and those fields as invalid, while leaving the unknown fallback version marked as ignored.

In this commit, we require readers to fail a payment when an invoice contains more than one p field. The prior payer rule selected the first field, which allowed two components to interpret the same signed invoice using different payment hashes. A valid invoice now has one unambiguous p field, and the payer must use it.

In this commit, we add two signed invalid invoices with two valid-length p fields. One uses distinct hashes to catch first- or last-field selection, while the other repeats the same hash to ensure implementations count fields instead of deduplicating values. Both vectors use the spec's documented key and have valid signatures and checksums, leaving the duplicate p fields as the intended rejection condition.

In this commit, we move the duplicate payment hash invoice vectors into a machine-readable JSON file. The BOLT 11 document now links to the canonical vectors instead of embedding a second copy.

In this commit, we move every encoded BOLT 11 invoice into the machine-readable test vector file. The prose keeps the detailed breakdowns, while the JSON file becomes the single source for both valid and invalid invoice strings.

TheBlueMatt

Ugh. Yea, should do, obviously.

TheBlueMatt · 2026-09-14T11:09:18Z


 # Examples

+Machine-readable invoice test vectors for all examples below are provided in


Seems weird to move the invoice data to a json file but leave the descriptions here, makes it much harder to parse.

MPins · 2026-09-16T11:27:57Z

Concept ACK on the duplicate rule.

One suggestion on the test vectors. Marking the "fields which must be ignored" invoice as invalid is correct, but it was also the only example exercising two forward-compatibility paths. I walked the tagged fields of all 28 vectors in the proposed bolt11/invoice-test.json: the unknown field type 2 and the f field with unknown version 19 appear in exactly one invoice — the one this PR marks "valid": false — and none of the 15 valid vectors carries an unknown field type or an unknown fallback version. So after this PR nothing positive covers either path.

MPins · 2026-09-16T11:42:13Z

+> ### Same, but with unknown fields and invalid fixed-length fields.

-> ### Same, but including fields which must be ignored.
-> lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqqsgq2qrqqqfppnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqppnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqhpnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqhp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqspnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqsp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnp5qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnpkqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqz599y53s3ujmcfjp5xrdap68qxymkqphwsexhmhr8wdz5usdzkzrse33chw6dlp3jhuhge9ley7j2ayx36kawe7kmgg8sv5ugdyusdcqzn8z9x


Marking this invoice as invalid is correct, but it was also the only example exercising two forward-compatibility paths:

the unknown field type 2

the f field with unknown version 19

saubyk

Concept Ack

ekzyis

Concept ACK

Fwiw, I was confused about the sender vs. receiver requirements for parsing fields with a fixed length (#1305). I mentioned in the LDK Discord that these two statements seemed to be in conflict if the invoice contained both a valid and an invalid p field:

[a payer] SHOULD use the first p field as the payment hash.

[a reader] MUST fail the payment if any field with fixed data_length (p, h, s, n) does not have the correct length (52, 52, 52, 53).

It was confusing because it wasn't clear whether the fixed-length check applies after we pick the first p, or whether it applies to any p, which implies there might be multiple.

I mentioned that I would try to make this less confusing. Maybe I would have found this vulnerability if I had done that 😅

I think that with "MUST use the first p field as the payment hash" and "MUST fail the payment if more than one p field is present" from this PR, it is now clearer how to handle p.

ziggie1984 · 2026-09-21T16:20:55Z

Concept ACK. Two things, one substantive and one about the vector format.

The same rule arguably belongs on `s`

The writer requirements already mandate both:

MUST include exactly one p field.

MUST include exactly one s field.

but this PR only adds the matching reader rule for p. The reader side for s is still phrased in the singular with no rule for what happens when several are present:

if a valid s field is not provided:

MUST fail the payment.

otherwise:

MUST use the s field as payment_secret

That is the same ambiguity being closed for p: "the s field" does not say which one, so one implementation takes the first and another takes the last, and two components fed the same signed invoice disagree on the payment_secret. The consequences differ from the payment-hash case — a mismatched secret mostly means the payment is rejected at the recipient rather than a bookkeeping split — but a system that decides "paid" or "not paid" by matching secrets has the same shape of bug available to it, and @ekzyis's point above is that the p wording was already confusing enough to hide one.

Since the writer rule exists for both fields, the cheapest fix is to extend the new line:

MUST fail the payment if more than one p or s field is present.

Or, more generally, fail on any repeat of a field the writer section specifies as "exactly one" or "one" (p, s, d/h, x, c, n, m), leaving f, r and b repeatable. I'd understand keeping this PR to p if the broader rule needs its own discussion, but s seems in scope given the writer rule is already there.

The vectors record the verdict but not the reason

Building on @TheBlueMatt's point about the split between the JSON and the descriptions: as it stands each entry carries only valid: true|false, which lets an implementation pass a vector by rejecting the invoice for an unrelated reason.

Concretely, I ran the 28 proposed vectors through lnd's BOLT 11 decoder, before and after lightningnetwork/lnd#11190 (which implements this PR's p rule):

base: 22/28 agree, after: 25/28. The three that flip are exactly the duplicate-p cases, which is the intended effect.
Unknown fields and invalid fixed-length fields is marked invalid here because several fixed-length fields have the wrong data_length. lnd rejects it with invoice contains multiple payment hashes — the right verdict for the wrong reason, and it would keep passing even if lnd never implemented the length rule. (That invoice now violates two separate MUSTs, so it can't distinguish them.)
Three valid: false vectors are accepted outright by lnd today: Unknown required feature 100, Missing required s field, and Non canonical signature (high-S) with 'n' field defined. Those are pre-existing divergences, not something this PR causes — but they're invisible in a pass/fail-only format until someone reads the output by hand.

Suggestion: give each invalid vector an expected-failure tag (e.g. "failure": "duplicate_payment_hash" / "invalid_field_length" / "unknown_required_feature"), and give the valid ones the expected decoded values (payment_hash, amount_msat, payment_secret, …). If the JSON is going to be the machine-readable source of truth, it may as well carry the assertions rather than just the verdict — that also gives the duplicate-p rule a vector that can only be passed by implementing it.

In this commit, we address the reviewer feedback on the duplicate payment hash rejection. We move the release notes entry from BOLT Spec Updates to Bug Fixes in the v0.22.0 notes, since rejecting a second payment hash field is deliberately ahead of the current BOLT 11 text; the entry now carries an interop note and links lightning/bolts#1357 as the motivation. We also add the same entry to the v0.20.5 and v0.21.4 notes, as the fix will be backported to those releases. We replace the stale comment above parse32Bytes, which quoted the old BOLT 11 rule that a reader must skip over 32-byte fields with an unsupported length. The current reader requirements say the opposite: a reader must fail the payment if a fixed-length field (p, h, s, n) does not have the correct length. The new comment states that reporting the field as absent is a known divergence, and explains why the end result for the payment hash is the same either way. In the tests, we keep a valid variant of the flipped vector that drops the duplicate payment hash but retains the wrong-length h and n fields, so the coverage that such fields are skipped survives. The duplicate vector stays in the decode table, now pinning ErrDuplicatePaymentHash through the new wantErr field rather than asserting any decode error. We also replace the rapid property test with two table rows in TestParseTaggedFields covering identical and distinct payment hashes, since the drawn bytes could not influence the outcome.

In this commit, we reject BOLT 11 invoices that contain more than one payment hash (`p`) field. `zpay32.Decode` previously kept the first supported payment hash and ignored later fields, so the interpreted payment hash depended on field order. The decoder now tracks whether a `p` field has appeared separately from whether its contents parsed successfully, and any later `p` field returns `ErrDuplicatePaymentHash`. This covers identical hashes, distinct hashes, and a malformed first field followed by a valid one. The change is deliberately stricter than the current BOLT 11 text, which tells a reader to use the first `p` field; it is motivated by lightning/bolts#1357, and will be backported to the v0.20.5 and v0.21.4 releases. We also replace the stale comment above parse32Bytes, which quoted the old BOLT 11 rule that a reader must skip over 32-byte fields with an unsupported length. The current reader requirements say the opposite: a reader must fail the payment if a fixed-length field (p, h, s, n) does not have the correct length. The new comment states that reporting the field as absent is a known divergence, and explains why the end result for the payment hash is the same either way. In the tests, we cover identical and distinct duplicate hashes, a malformed first field followed by a valid one, and the signed vectors. A valid variant of the flipped vector keeps the wrong-length h and n fields, so the coverage that such fields are skipped survives. The duplicate vector in the decode table pins ErrDuplicatePaymentHash through a new wantErr field rather than asserting any decode error.

In this commit, we reject BOLT 11 invoices that contain more than one payment hash (`p`) field. `zpay32.Decode` previously kept the first supported payment hash and ignored later fields, so the interpreted payment hash depended on field order. The decoder now tracks whether a `p` field has appeared separately from whether its contents parsed successfully, and any later `p` field returns `ErrDuplicatePaymentHash`. This covers identical hashes, distinct hashes, and a malformed first field followed by a valid one. The change is deliberately stricter than the current BOLT 11 text, which tells a reader to use the first `p` field; it is motivated by lightning/bolts#1357, and will be backported to the v0.20.5 and v0.21.4 releases. We also replace the stale comment above parse32Bytes, which quoted the old BOLT 11 rule that a reader must skip over 32-byte fields with an unsupported length. The current reader requirements say the opposite: a reader must fail the payment if a fixed-length field (p, h, s, n) does not have the correct length. The new comment states that reporting the field as absent is a known divergence, and explains why the end result for the payment hash is the same either way. In the tests, we cover identical and distinct duplicate hashes, a malformed first field followed by a valid one, and the signed vectors. A valid variant of the flipped vector keeps the wrong-length h and n fields, so the coverage that such fields are skipped survives. The duplicate vector in the decode table pins ErrDuplicatePaymentHash through a new wantErr field rather than asserting any decode error. (cherry picked from commit e2f2706)

Abdulkbk

cACK

Abdulkbk · 2026-09-26T22:22:38Z

 A reader:
  - MUST skip over `f` fields that use an unknown `version`.
  - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) does not have the correct length (52, 52, 52, 53).
+  - MUST fail the payment if more than one `p` field is present.


should we rephrase this to something like "MUST fail unless exactly one p field is present" such that it's clear we should fail if p < 1?

A reader of BOLT11 must fail a `p`, `h`, `s` or `n` of the wrong length, a writer must include one `p`, `s`, `d` or `h`, and a reader should treat an `x`, `c` or `9` with a leading zero word as invalid. A writer must use no leading zero in an amount. `_first` returned the first field of a tag whose length fit and skipped the rest. It now checks every occurrence of the tag and raises. `sign` writes a zero `x` or `c` as an empty field, the minimal one, which the reader accepts. BOLT11's example "Same, but including fields which must be ignored" contradicts the reader requirement. It is listed as invalid, with its bytes unchanged, and `tests/_data/README.md` says why and names lightning/bolts#1357, the pending upstream change. The tests build the refused invoices field by field and sign them. Signed-off-by: Ferdinando Ametrano <ferdinando@ametrano.net>

…ite (closes #260) A reader of BOLT11 must fail a `p`, `h`, `s` or `n` of the wrong length, and should treat an `x`, `c` or `9` with a leading zero word as invalid. A writer must include exactly one `p`, one `s` and one `d` or `h`, and must use no leading zero in an amount. A second `n` is refused too, as Electrum does. `_first` returned the first field of a tag whose length fit and skipped the rest. It now checks every occurrence of the tag and raises. `sign` writes a zero `x` or `c` as an empty field, the minimal one, which the reader accepts. BOLT11's example "Same, but including fields which must be ignored" contradicts the reader requirement. It is listed as invalid, with its bytes unchanged, and `tests/_data/README.md` says why and names lightning/bolts#1357, the pending upstream change. The tests build the refused invoices field by field and sign them. Signed-off-by: Ferdinando Ametrano <ferdinando@ametrano.net>

…ite (closes #260) (#275) A reader of BOLT11 must fail a `p`, `h`, `s` or `n` of the wrong length, and should treat an `x`, `c` or `9` with a leading zero word as invalid. A writer must include exactly one `p`, one `s` and one `d` or `h`, and must use no leading zero in an amount. A second `n` is refused too, as Electrum does. `_first` returned the first field of a tag whose length fit and skipped the rest. It now checks every occurrence of the tag and raises. `sign` writes a zero `x` or `c` as an empty field, the minimal one, which the reader accepts. BOLT11's example "Same, but including fields which must be ignored" contradicts the reader requirement. It is listed as invalid, with its bytes unchanged, and `tests/_data/README.md` says why and names lightning/bolts#1357, the pending upstream change. The tests build the refused invoices field by field and sign them. Signed-off-by: Ferdinando Ametrano <ferdinando@ametrano.net>

kaloudis · 2026-10-07T21:05:24Z

Concept ACK

Instead of keeping the first well-formed `p` field, decode now throws `Invoice contains multiple payment hashes` when a second `p` field is present, counted by presence regardless of length or value. This follows lightning/bolts#1357 and matches lnd's zpay32 since lightningnetwork/lnd#11190, so an invoice ZEUS refuses can never be tracked or verified under a hash the node did not settle on. The other singleton tags keep first-wins with wrong-length skipping, which is still lnd's behavior for them.

Bolt11Utils.decode applied singleton tags last-wins, so an invoice that repeated a tag such as payment_hash made ZEUS track, display, or verify a different value than the node settles on. This is the bug class used against lnp2pBot via the `invoices` npm package. decode now matches lnd's zpay32: * More than one `p` field throws `Invoice contains multiple payment hashes`, counted by presence regardless of length or value, per lightning/bolts#1357 and lightningnetwork/lnd#11190. * An invoice with no well-formed `p` field throws `No valid payment hash found`. * The other singleton tags (payment_secret, description, description_hash, payee, expiry, min_final_cltv_expiry, metadata) keep their first occurrence, and fixed-size fields with the wrong word length (52 for p/s/h, 53 for n) are skipped. * The `sections` array still records every occurrence.

The writer section already requires exactly one `p` and exactly one `s` field, but the reader rule only rejected a second `p`. Nothing rejected an invoice with no `p` at all, and a repeated `s` was left to each implementation, which reintroduces the same first-vs-last split for the payment secret.

Moving the invoices into the JSON file while leaving their breakdowns in the markdown made the examples hard to follow, as each breakdown no longer had the string it breaks down. Put every invoice back under its heading, including the upper case and invalid examples, and generate the JSON from the markdown so the two can't drift. The JSON descriptions now match the markdown headings, and the two duplicate `p` vectors get markdown entries of their own. The unknown fields example is now invalid, so it moves to the invalid section with its breakdown.

The unknown fields example broke several rules at once (duplicate `p`, `h`, `s` and `n` fields, each with a bad length), so a reader could reject it for any one of them and still pass. It was also the only vector exercising an unknown field type and an `f` field with an unknown version, so marking it invalid left nothing positive covering either. Split it in two. The valid "fields which must be ignored" example is back, carrying just the unknown `2` field and the version 19 `f` field. A new invalid vector carries a single `n` field of the wrong length, which a reader that skips the field would accept. We also add vectors for a missing `p` field and for two distinct `s` fields, so each part of the exactly-one rule has a vector that only that rule rejects. The new invoices are signed with the same key and RFC6979 nonces as the other examples.

Roasbeef · 2026-10-19T16:55:20Z

Pushed three commits that address the review so far.

@Abdulkbk good catch, there was no reader rule that failed an invoice with no p field at all. @ziggie1984 agreed that s is in scope, since the writer rule already says exactly one. Both are now covered by a single reader rule:

MUST fail the payment unless exactly one p field and exactly one s field are present.

I kept the other "one" fields (x, c, n, m, d/h repeats) out of this PR, as widening it further probably deserves its own discussion.

@TheBlueMatt the invoice strings are back under their headings in the markdown, including the upper case and invalid examples. The JSON is now generated from the markdown, and its descriptions match the headings, so the two can't drift.

@MPins the "fields which must be ignored" example is back as a valid vector, now carrying only the unknown 2 field and the version 19 f field, so both forward-compat paths have positive coverage again.

@ziggie1984 on the vectors, every invalid vector now has a single defect. The old multi-defect invoice is replaced by an invoice whose only problem is an n field of the wrong length, which a reader that skips the field would accept. There are also new vectors for a missing p field and for two distinct s fields. I held off on the failure-reason tags for now, since the reason names would need agreement across implementations. That seems better as a follow-up.

All 31 vectors were run through lnd's zpay32. Every valid vector decodes, and every invalid vector lnd rejects fails for the intended reason. lnd still accepts five of the invalid vectors: the three you found (unknown required feature 100, missing s, high-S with n), plus the two that test the new rules (the bad length n field and two s fields). Those are lnd bugs, and we'll fix them on that side.

This also resolves #1358, since the old "must be ignored" example is no longer silently accepting bad-length fields.

TheBlueMatt

didnt check the test cases but i assume a clanker did it right

Roasbeef added 5 commits September 10, 2026 12:53

bolt11: move invoice vectors to JSON

86eea52

In this commit, we move the duplicate payment hash invoice vectors into a machine-readable JSON file. The BOLT 11 document now links to the canonical vectors instead of embedding a second copy.

bolt11: move all invoice vectors to JSON

03ac814

In this commit, we move every encoded BOLT 11 invoice into the machine-readable test vector file. The prose keeps the detailed breakdowns, while the JSON file becomes the single source for both valid and invalid invoice strings.

kaloudis mentioned this pull request Sep 10, 2026

utils: match lnd on duplicate BOLT11 singleton tags ZeusLN/zeus#4631

Open

13 of 29 tasks

TheBlueMatt reviewed Sep 14, 2026

View reviewed changes

MPins mentioned this pull request Sep 16, 2026

bolt11 encoding examples out-of-date #1358

Closed

MPins reviewed Sep 16, 2026

View reviewed changes

saubyk approved these changes Sep 18, 2026

View reviewed changes

saubyk mentioned this pull request Sep 18, 2026

zpay32: reject duplicate payment hash fields lightningnetwork/lnd#11190

Merged

8 of 9 tasks

ekzyis reviewed Sep 18, 2026

View reviewed changes

SomberNight mentioned this pull request Sep 18, 2026

Add BOLT11DecodeException and InvalidBitcoinURI wraps for missing uncaught exceptions. spesmilo/electrum#10940

Merged

Roasbeef mentioned this pull request Sep 21, 2026

Lightning Specification Meeting 2026/09/21 #1354

Open

15 of 22 tasks

This was referenced Sep 23, 2026

[v0.20.x-branch] Backport #11190: zpay32: reject duplicate payment hash fields lightningnetwork/lnd#11235

Merged

[v0.21.x-branch] Backport #11190: zpay32: reject duplicate payment hash fields lightningnetwork/lnd#11236

Merged

Abdulkbk approved these changes Sep 26, 2026

View reviewed changes

fametrano mentioned this pull request Oct 7, 2026

Refuse the BOLT11 invoices a reader must fail or a writer must not write (closes #260) btclib-org/btclib-wallet#275

Merged

Roasbeef mentioned this pull request Oct 19, 2026

Lightning Specification Meeting 2026/10/19 #1365

Open

14 of 25 tasks

Roasbeef added 3 commits October 19, 2026 11:52

TheBlueMatt approved these changes Oct 19, 2026

View reviewed changes

TheBlueMatt merged commit b383d14 into master Oct 19, 2026

Roasbeef mentioned this pull request Oct 19, 2026

zpay32: enforce BOLT 11 fixed lengths, single s, low-S with n lightningnetwork/lnd#11357

Open

deadmanoz mentioned this pull request Oct 20, 2026

November 2026 Perth BitDevs PerthBitDevs/PerthBitDevs#44

Open

Navigation Menu

Repository navigation

bolt11: reject duplicate payment hashes in invoices - #1357

bolt11: reject duplicate payment hashes in invoices#1357
TheBlueMatt merged 8 commits into
masterfrom
bolt11-single-payment-hash

Roasbeef commented Sep 10, 2026

Uh oh!

TheBlueMatt left a comment

Uh oh!

TheBlueMatt Sep 14, 2026

Uh oh!

MPins commented Sep 16, 2026

Uh oh!

MPins Sep 16, 2026

Uh oh!

saubyk left a comment

Uh oh!

ekzyis left a comment •

edited

Loading

Uh oh!

ziggie1984 commented Sep 21, 2026

Uh oh!

Abdulkbk left a comment

Uh oh!

Abdulkbk Sep 26, 2026

Uh oh!

kaloudis commented Oct 7, 2026

Uh oh!

Roasbeef commented Oct 19, 2026

Uh oh!

TheBlueMatt left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants


		# Examples

		Machine-readable invoice test vectors for all examples below are provided in

Conversation

Roasbeef commented Sep 10, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

MPins commented Sep 16, 2026

Uh oh!

MPins Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

saubyk left a comment

Choose a reason for hiding this comment

Uh oh!

ekzyis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziggie1984 commented Sep 21, 2026

The same rule arguably belongs on s

The vectors record the verdict but not the reason

Uh oh!

Abdulkbk left a comment

Choose a reason for hiding this comment

Uh oh!

Abdulkbk Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

kaloudis commented Oct 7, 2026

Uh oh!

Roasbeef commented Oct 19, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ekzyis left a comment •

edited

Loading

The same rule arguably belongs on `s`