Skip to content

Clarify Mandatory Field Length Requirements and Add Note on Low R Signatures in BOLT 11 #1243

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nGoline
Copy link

@nGoline nGoline commented Apr 1, 2025

This PR addresses the ambiguity in BOLT 11 regarding the handling of mandatory fields (p, h, s) when their lengths are incorrect. Specifically, it clarifies that:

  • A reader MUST fail the invoice if a mandatory field (p, h, s) is present with an incorrect length, rather than skipping it. This ensures that invoices with invalid mandatory fields are not misinterpreted as valid.

Additionally, this PR adds a note to the examples section regarding the use of Low R signatures. While Low R signatures save 1 byte in DER-encoded signatures, they are not enforced in the specification. This clarification is added to help implementers avoid confusion when testing against provided vectors, as the test vectors do not enforce Low R.

Changes

  1. Updated the "Requirements" section to explicitly state that invoices must fail if mandatory fields have incorrect lengths.
  2. Added a note to the examples section explaining that Low R signatures are not enforced in the specification, even though they can save space.

Context

This PR resolves the ambiguity raised in #1235. The issue highlighted a potential misinterpretation where a p field with an invalid length could be skipped, leading to an invalid invoice being treated as valid. It also addresses confusion caused by test vectors not enforcing Low R signatures, which caused discrepancies during testing with libraries like NBitcoin.

Testing

  • Verified that the clarification aligns with the behavior of major implementations by running a Differential Fuzzing.
  • Updated documentation to ensure consistency with the clarified requirements.

@nGoline nGoline force-pushed the resolve-bolt11-ambiguity branch from 73a9a24 to f2a747b Compare May 5, 2025 20:25
Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, ACK 60cc3b9 👍

`n` fields that do NOT have `data_length`s of 52, 52 or 53, respectively.
- MUST skip over `f` fields that use an unknown `version`.
- MUST fail the payment if any mandatory field (`p`, `h`, `s`, `n`) does not have the correct length (52, 52, 52, 53).
- MUST fail the payment if neither a `d` field nor a `h` field is present.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the writer side, the requirement is to have exactly one d or h field present, so if we make this change on the reader side we should also fail the payment if both d and h are present.

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.

3 participants