Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BOLT #11 Ambiguity: p Field Skipping vs. Mandatory Payment Hash #1235

Open
nGoline opened this issue Mar 7, 2025 · 2 comments
Open

BOLT #11 Ambiguity: p Field Skipping vs. Mandatory Payment Hash #1235

nGoline opened this issue Mar 7, 2025 · 2 comments

Comments

@nGoline
Copy link

nGoline commented Mar 7, 2025

I’ve noticed a potential ambiguity in BOLT 11 regarding the p field:

  • The spec states that a writer MUST include exactly one p field.
  • However, under the “Tagged Fields” → “Requirements” section, it also says a reader MUST skip:
    “… unknown fields, OR an f field with unknown version, OR p, h, s or n fields that do not have data_lengths of 52, 52, 52 or 53, respectively.”

This can be misread to mean that if a p field doesn’t match the required length (e.g. zero length), the invoice parser should skip it rather than fail.

However, skipping a zero-length p field effectively means “no payment hash field is present,” which conflicts with the writer's requirement that p is mandatory. We know from the spec that the invoice must have a valid p field to be payable.

Why is this confusing?

A literal reading of “MUST skip over p fields that do not have data_length of 52” could be interpreted as:

“If data_length is not 52 for p, just skip it. Move on.”

But skipping a mandatory field is the same as omitting it. This leads to the question:

Is an invoice valid if we skip the p field because its data_length is 0 or invalid?

Why am I asking this now?

While running some Differential Fuzz Tests between LND and NLightning, we found out that for the decoding of the following invoice, LND fails and NLightning succeeds:
lnbc1pxtpcnsdz5cw9huz3z7wctpfpnptxcsdz5w9chzcw9huz3z7wc309t5cw9huz3z7wc3tpfpxpcnsdz5w9chzu3z7qqqqqqqqqqqqqqqqqqqqqqqquq44p9

A live demo for NLightning's Decoder can be found here

Potential Fix / Clarification

It might help to clarify in BOLT 11 that while you can skip unknown field types, you must fail if a mandatory known field (like p) doesn’t meet its specified data length. Perhaps something like:

“A reader MUST fail the invoice if a mandatory field (p, h, s, n) is present with an incorrect length, rather than skipping it.”

This would remove any possible interpretation that a zero-length p field can be skipped and the invoice somehow remain valid.

@t-bast
Copy link
Collaborator

t-bast commented Mar 20, 2025

Can you please open a PR that fixes this? I don't think anyone else will do it based on this issue alone.

@nGoline
Copy link
Author

nGoline commented Mar 21, 2025

I had some issues while testing against the provided vectors, specifically on the signatures of the invoices.
I'm using NBitcoin as the underlying Bitcoin library and it has a setting that defaults to producing signatures using Low R. After a few hours of going through the code (NLightning and NBitcoin) I realized all the test vectors (including BOLT3) do not enforce Low R on the signatures, which caused me a lot of headaches.
I'll add this information to the same PR, if that's ok.

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

No branches or pull requests

2 participants