Add currency conversion support for BOLT 12 offers#3833
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
|
cc @jkczyz |
|
🔔 1st Reminder Hey @joostjager! This PR has been waiting for your review. |
|
Is this proposed change a response to a request from a specific user/users? |
|
Hi @joostjager! This PR is actually a continuation of the original thread that led to the The motivation behind it was to provide users with the ability to handle So, as a first step, we worked on refactoring most of the Offers-related code out of Hope that gives a clear picture of the intent behind this! Let me know if you have any thoughts or suggestions—would love to hear them. Thanks a lot! |
|
Another use case is Fedimint, where they'll want to include their own payment hash in the |
Does Fedimint plan to use the |
I believe with one. |
|
🔔 2nd Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 10th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 11th Reminder Hey @joostjager! This PR has been waiting for your review. |
|
Removing @joostjager for now to stop bot spam. @shaavan and I have been working through some variations of this approach. |
vincenzopalazzo
left a comment
vincenzopalazzo
left a comment
There was a problem hiding this comment.
Concept ACK for me
I was just looking around to sync with this Offer Flow
shaavan
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3833 +/- ##
==========================================
+ Coverage 84.55% 86.98% +2.43%
==========================================
Files 137 162 +25
Lines 77617 112310 +34693
Branches 77617 112310 +34693
==========================================
+ Hits 65627 97694 +32067
- Misses 9948 12103 +2155
- Partials 2042 2513 +471
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🔔 6th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
Updated .25 → .26 Thanks, @ldk-claude-review-bot - Changes:
|
|
Rebased .26 → .27 |
|
🔔 9th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 10th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 11th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 12th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
| #[cfg(c_bindings)] | ||
| #[derive(Clone)] | ||
| pub struct OfferWithExplicitMetadataBuilder<'a> { | ||
| pub struct OfferWithExplicitMetadataBuilder<'a, CC: CurrencyConversion> { |
There was a problem hiding this comment.
@TheBlueMatt Is this supported for bindings? Or do we need to pass the CurrencyConversion reference to the build method? I can't recall the exact restrictions. For instance, could we have the bindings store by value instead of reference? Or is that not supported either.
Add a `CurrencyConversion` trait for resolving currency-denominated amounts into millisatoshis. LDK cannot supply exchange rates itself, so applications provide this conversion logic as the foundation for fiat-denominated offer support.
Add unit tests for currency exchange-rate bounds covering fractional reference rates, basis-point tolerances, absolute tolerances, and invalid range construction. The tests lock in truncating tolerance arithmetic for fractional rates, preserve strict ordering of the accepted range around the reference rate, and verify rejection of underflowing lower bounds and overflowing upper bounds. AI-assisted: Added focused test coverage Co-Authored-By: OpenAI Codex <codex@openai.com>
Thread `CurrencyConversion` through `ChannelManager` type parameters and construction APIs. BOLT12 offer amounts will require currency conversion during invoice construction and payment validation. Wire the conversion dependency through `ChannelManager` now so later commits can use a single conversion path instead of duplicating conversion logic across call sites. Wire the dependency through the related test, fuzz, and helper scaffolding to support the new manager integration. AI-assisted: Dependency plumbing and scaffolding. Co-Authored-By: OpenAI Codex <codex@openai.com>
BOLT12 currency-denominated offers will require currency conversion during offer construction to validate the set amount. This commit handles the plumbing needed to introduce that behavior, threading `CurrencyConversion` through the relevant offer-building paths and scaffolding. The next commit will introduce the actual conversion logic. Keep the plumbing and logical changes separate to make the transition easier to review.
OfferBuilder previously rejected currency-denominated amounts even though CurrencyConversion had already been threaded through the builder APIs. Use that converter when building an offer so Amount::Currency can resolve to a payable millisatoshi range. Keep unsupported currencies and zero or excessive converted amounts rejected during build. This activates the builder behavior prepared by the preceding plumbing commit. Focused offer validation tests follow in a separate commit. AI-assisted: Split feature and test commits Co-Authored-By: OpenAI Codex <codex@openai.com>
Add offer-builder coverage for accepted and unsupported currency-denominated amounts, plus boundary cases where conversion floors to zero or exceeds the maximum payable millisatoshi amount. This keeps currency offer construction covered at the narrowest layer before later invoice request and payment flows derive amounts from those offers. AI-assisted: Wrote focused boundary tests Co-Authored-By: OpenAI Codex <codex@openai.com>
Currency-denominated offers cannot always be resolved to a final millisatoshi amount by the payer while creating an invoice request. Defer that amount check when building or parsing invoice requests for those offers. The payee can apply its conversion rate when deciding whether to issue the invoice, and the payer can verify the invoice amount when handling it. This keeps bitcoin-denominated offer amount checks unchanged while allowing currency-denominated offers to proceed through invoice request creation. AI-assisted: Documented deferred currency amount validation Co-Authored-By: OpenAI Codex <codex@openai.com>
The old `InvoiceRequest` amount accessor blurred two different concepts: the amount explicitly carried in the request TLV and the amount derived from the offer and quantity. Callers had to pair `amount_msats` with `has_amount_msats` to determine whether the amount was actually present in the request or synthesized on read. Split those meanings into separate accessors: - `amount_msats()` now returns only the amount explicitly requested in the invoice request. - `payable_amount_msats()` returns the payable amount for the invoice request, deriving it from the offer when needed. This removes `has_amount_msats()` and changes callers that need the payable amount to use `payable_amount_msats()` instead. As part of currency conversion support, `payable_amount_msats()` accepts a conversion trait parameter, allowing callers to derive payable amounts when the offer is currency-denominated.
Thread CurrencyConversion through the InvoiceBuilder construction flow and the related upstream APIs. This sets up the plumbing needed for currency-denominated invoice handling without introducing the actual verification logic yet. The plumbing and logical changes are separated to make the transition easier to review. The next commit adds payer-side invoice amount verification, completing the end-to-end currency conversion flow.
Currency-denominated offers may not include an explicit msat amount in the invoice request. During invoice building, we now use the configured currency conversion to either validate the requested amount or derive the payable amount from the offer amount. This completes the currency conversion support on the payee side. The next commit adds payer-side invoice amount verification, completing the end-to-end currency conversion flow.
Add tests covering invoice request and invoice response handling for currency-denominated offers. This combines coverage for the standard flow that derives the final invoice amount through currency conversion and the insufficient-msat request path that must be rejected while building the invoice response. The merged test coverage exercises both the positive and deferred- validation paths for currency-denominated invoice responses. AI-assisted: Planning and writing the tests Co-Authored-By: OpenAI Codex <codex@openai.com>
This completes the invoice handling side of currency conversion support. When paying an invoice for a currency-denominated offer, and the invoice request did not specify an explicit amount, we now use the configured CurrencyConversion to derive the acceptable msat range for the offer amount. The invoice is considered valid only if the quoted amount falls within that acceptable range, preventing the payer from being overcharged due to exchange-rate differences or unexpected invoice amounts.
Add payer-side tests for currency-denominated offers and invoices. Cover the standard payment flow, rejection of excessive invoices, handling of unverifiable fiat invoices, quantity-scaled invoice requests, and async static-invoice payments. Together these tests exercise the invoice amount verification paths introduced for currency-denominated offers. AI-assisted: Planned and wrote focused test coverage Co-Authored-By: OpenAI Codex <codex@openai.com>
Add a pending changelog entry for currency-denominated BOLT 12 offers. The entry calls out the new conversion requirement, the ChannelManager construction change, and validation for unsupported or invalid currency amounts.
|
Rebased .28 → .29 |
8 participants
This PR adds support for currency-denominated Offers in LDK’s BOLT 12 offer-handling flow.
Previously, Offers could only specify their amount in millisatoshis. However, BOLT 12 allows Offers to be denominated in other currencies such as fiat. Supporting this requires converting those currency amounts into millisatoshis at runtime when validating payments and constructing invoices.
Because exchange rates are external, time-dependent, and application-specific, LDK cannot perform these conversions itself. Instead, this PR introduces a
CurrencyConversiontrait which allows applications to provide their own logic for resolving currency-denominated amounts into millisatoshis. LDK remains exchange-rate agnostic and simply invokes this trait whenever a currency amount must be resolved.To make this conversion logic available throughout the BOLT 12 flow,
OffersMessageFlowis parameterized over aCurrencyConversionimplementation and the abstraction is threaded through the offer handling pipeline.With this in place:
OfferBuildercan now create Offers whose amounts are denominated in currencies instead of millisatoshis•
InvoiceRequesthandling can resolve Offer amounts when validating requests•
InvoiceBuilderenforces that the final invoice amount satisfies the Offer’s requirements after resolving any currency denominationCurrency validation is intentionally deferred until invoice construction when necessary, keeping earlier stages focused on structural validation while ensuring the final payable amount is correct.
Tests are added to cover the complete Offer → InvoiceRequest → Invoice flow when the original Offer amount is specified in a currency.