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 89.34% 89.37% +0.02%
==========================================
Files 180 180
Lines 138480 140045 +1565
Branches 138480 140045 +1565
==========================================
+ Hits 123730 125164 +1434
- Misses 12129 12295 +166
+ Partials 2621 2586 -35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🔔 3rd Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
shaavan
force-pushed
the
currency
branch
2 times, most recently
from
1eb4160 to
bd2a110
Compare
|
🔔 4th Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review. |
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.
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.
This commit completes the second part of currency conversion support for offers by adding validation for currency-denominated amounts during offer construction. With the plumbing introduced in the previous commit now in place, `OfferBuilder` can support currency-denominated offer amounts and expose the related amount-setting APIs publicly. Add minimal tests to verify that the public amount APIs work correctly and that fiat-denominated offers build successfully.
For currency-denominated offers, the payer may not reliably derive the final msat amount during invoice request creation. Instead, defer the final amount resolution to invoice creation (for the payee) and invoice handling (for the payer), where the currency conversion can be verified against the offer's fiat amount. This also updates a previously failing test to cover the new behavior. Additional reasoning is documented in the commit.
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()` returns 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. As part of the recent currency conversion support, `payable_amount_msats()` now accepts a conversion trait parameter, allowing callers to derive payable amounts even when the offer is currency-denominated. This lays the groundwork for future commits adding currency conversion support to `Bolt12Invoice` creation and handling logic.
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 end-to-end and payer-side tests for currency-denominated offers and invoices. This consolidates coverage for the standard payment flow, excessive invoice rejection, unverifiable fiat invoices when conversion support is unavailable, and quantity-scaled invoice requests. The combined coverage exercises the main invoice amount verification paths introduced by currency-denominated offer support. AI-assisted: Planning and writing the tests Co-Authored-By: OpenAI Codex <codex@openai.com>
| @@ -1000,9 +1034,10 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> { | |||
| .map_err(|_| Bolt12SemanticError::MissingPaths)?; | |||
|
|
|||
| #[cfg(all(feature = "std", not(fuzzing)))] | |||
| let builder = invoice_request.respond_using_derived_keys(payment_paths, payment_hash); | |||
| let builder = invoice_request.respond_using_derived_keys(converter, payment_paths, payment_hash); | |||
There was a problem hiding this comment.
The currency amount is resolved twice from converter here: once at line 1012 (InvoiceBuilder::amount_msats), whose result is embedded into the blinded payment paths at line 1029, and again inside respond_using_derived_keys → InvoiceBuilder::for_offer → Self::amount_msats(invoice_request, converter) at line 1037, which sets the actual invoice amount_msats.
For a CurrencyConversion that returns time-varying exchange rates (the intended fiat use case), the two lookups can return different values. The amount baked into the blinded payment path constraints (line 1029) would then disagree with the amount the payer is quoted in the invoice, which can break routing/claim consistency. The same pattern exists in create_invoice_builder_from_invoice_request_without_keys (lines 1081/1106).
Consider resolving the amount once and threading the resolved value into the invoice builder, rather than re-deriving it inside respond_with/respond_using_derived_keys.
|
🔔 6th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
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.