[0.1] Reject offer_amount of 0 as invalid per BOLT 12#4487
Merged
TheBlueMatt merged 1 commit intolightningdevkit:0.1from Mar 17, 2026
Merged
[0.1] Reject offer_amount of 0 as invalid per BOLT 12#4487TheBlueMatt merged 1 commit intolightningdevkit:0.1from
TheBlueMatt merged 1 commit intolightningdevkit:0.1from
Conversation
|
I've assigned @valentinewallace as a reviewer! |
TheBlueMatt
changed the title
4 tasks
Collaborator
|
No new issues found beyond the previously flagged misleading test comment on line 1523. The builder and TLV deserialization logic is correct. |
Per the spec clarification in lightning/bolts#1316: - Writers MUST set offer_amount greater than zero when present - Readers MUST NOT respond to offers where offer_amount is zero Reject amount_msats(0) in the builder with InvalidAmount, and reject parsed offers with amount=0 (with or without currency) during TLV deserialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Backport of a06c446 Conflicts resolved in: * lightning/src/offers/offer.rs Compared to the upstream commit, this instead allows downstream code to pass a 0 amount but converts it to no-amount to ensure upgraded readers of the built offer will accept it. This avoids changing the API in a backport.
TheBlueMatt
force-pushed
the
2026-03-4324-0.1
branch
from
333fadc to
c6d0e46
Compare
Comment on lines
+385
to
+387
| if amount_msats == 0 { | ||
| $self.offer.amount = None; | ||
| } |
Contributor
There was a problem hiding this comment.
I am wondering if this is something that we want to do or return a Bolt12SemanticError::InvalidAmount from an API point of view?
Collaborator
Author
There was a problem hiding this comment.
Its an API change in the public API. IMO we should strongly avoid backporting API changes.
Contributor
There was a problem hiding this comment.
Its an API change in the public API. IMO we should strongly avoid backporting API changes.
Cool :) I was just pointing it out to make sure that we were not introducing it by mistake!
vincenzopalazzo
added a commit
to vincenzopalazzo/lndk
that referenced
this pull request
Mar 17, 2026
Per the spec clarification in lightning/bolts#1316: - Writers MUST set offer_amount greater than zero when present - Readers MUST NOT respond to offers where offer_amount is zero Reader side (parse.rs): validate_amount() now rejects Amount::Bitcoin { amount_msats: 0 } unconditionally at the top of the match arm, before checking user-provided amounts. Previously it treated amount=0 as "no amount set" and allowed payment if the user provided an amount. Writer side (server.rs, handler.rs, lnd_requests.rs): The gRPC CreateOfferRequest previously did `unwrap_or(0)` on the optional amount field, causing offers created without an amount to write offer_amount=0 into the TLV stream. Changed amount_msats to Option<u64> through the full chain (CreateOfferParams -> CreateOfferArgs -> create_offer), so offer_amount is omitted entirely when no amount is specified. Dependency (Cargo.toml): Updated rust-lightning to include the cherry-pick of lightningdevkit/rust-lightning#4487 which normalizes amount_msats(0) to None in the OfferBuilder and rejects offer_amount=0 during TLV deserialization. Added [patch] section to ensure ldk-sample uses the same fork, avoiding duplicate crate versions. Previous test failure (before fix): ---- offers::parse::tests::test_reject_offer_with_zero_amount stdout ---- thread 'offers::parse::tests::test_reject_offer_with_zero_amount' panicked at src/offers/parse.rs:140:9: Offers with amount=0 must be rejected per BOLT 12 spec (bolts #1316) test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 80 filtered out Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Per the spec clarification in lightning/bolts#1316:
Reject amount_msats(0) in the builder with InvalidAmount, and reject parsed offers with amount=0 (with or without currency) during TLV deserialization.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Backport of a06c446
Conflicts resolved in:
Compared to the upstream commit, this instead allows downstream code to pass a 0 amount but converts it to no-amount to ensure upgraded readers of the built offer will accept it. This avoids changing the API in a backport.
Backport of #4324