fix: reject offers with offer_amount=0 per BOLT 12#253
Open
vincenzopalazzo wants to merge 3 commits intolndk-org:masterfrom
Open
fix: reject offers with offer_amount=0 per BOLT 12#253vincenzopalazzo wants to merge 3 commits intolndk-org:masterfrom
vincenzopalazzo wants to merge 3 commits intolndk-org:masterfrom
Conversation
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>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #253 +/- ##
======================================
Coverage 6.88% 6.88%
======================================
Files 1 1
Lines 218 218
======================================
Hits 15 15
Misses 203 203 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tall The amount_msats field on CreateOfferParams was changed to Option<u64> but integration tests were not updated. Also fix security_audit CI by adding --locked to cargo install to avoid MSRV conflicts with transitive dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
offer_amountmust be > 0 when present, readers must reject offers withoffer_amount=0validate_amount()now rejectsamount=0unconditionally instead of treating it as "no amount"CreateOfferRequestwithout an amount now omitsoffer_amountentirely instead of writingoffer_amount=0viaunwrap_or(0)offer_amount=0at the TLV parsing levelContext
Backport reference: lightningdevkit/rust-lightning#4324 (comment)
The LNDK fork of rust-lightning (based on 0.1.3) did not include the
offer_amount=0fix. Additionally, LNDK's own gRPC handler convertedNoneamounts to0, producing spec-noncompliant offers that other implementations would reject.Test plan
test_reject_offer_with_zero_amount: verifiesOfferBuildernormalizesamount_msats(0)toNone, andvalidate_amountrejectsAmount::Bitcoin { amount_msats: 0 }both with and without user-provided amountstest_create_offer_no_amount: verifies offer created without an amount hasoffer.amount() == None(omitted from TLV)🤖 Generated with Claude Code