From a7da7220f9296ac2095d3799e6068d8dc96396a0 Mon Sep 17 00:00:00 2001 From: shaavan Date: Fri, 25 Jul 2025 22:50:24 +0530 Subject: [PATCH 1/6] Rename VerifiedInvoiceRequest to VerifiedInvoiceRequestLegacy In the upcoming commits, we will be phasing the current style of VerifiedInvoiceRequest, in favour of newer version. To keep the changes modular, and clean we rename the current VerifiedInvoiceRequest to VerifiedInvoiceRequestLegacy. --- lightning/src/offers/flow.rs | 11 ++++++----- lightning/src/offers/invoice_request.rs | 14 +++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 951d33de319..828c9b9a96b 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -41,7 +41,7 @@ use crate::offers::invoice::{ }; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{ - InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequest, + InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequestLegacy, }; use crate::offers::nonce::Nonce; use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder}; @@ -406,7 +406,7 @@ fn enqueue_onion_message_with_reply_paths( pub enum InvreqResponseInstructions { /// We are the recipient of this payment, and a [`Bolt12Invoice`] should be sent in response to /// the invoice request since it is now verified. - SendInvoice(VerifiedInvoiceRequest), + SendInvoice(VerifiedInvoiceRequestLegacy), /// We are a static invoice server and should respond to this invoice request by retrieving the /// [`StaticInvoice`] corresponding to the `recipient_id` and `invoice_id` and calling /// `OffersMessageFlow::enqueue_static_invoice`. @@ -920,7 +920,7 @@ where Ok(builder.into()) } - /// Creates a response for the provided [`VerifiedInvoiceRequest`]. + /// Creates a response for the provided [`VerifiedInvoiceRequestLegacy`]. /// /// A response can be either an [`OffersMessage::Invoice`] with additional [`MessageContext`], /// or an [`OffersMessage::InvoiceError`], depending on the [`InvoiceRequest`]. @@ -930,8 +930,9 @@ where /// - We fail to generate a valid signed [`Bolt12Invoice`] for the [`InvoiceRequest`]. pub fn create_response_for_invoice_request( &self, signer: &NS, router: &R, entropy_source: ES, - invoice_request: VerifiedInvoiceRequest, amount_msats: u64, payment_hash: PaymentHash, - payment_secret: PaymentSecret, usable_channels: Vec, + invoice_request: VerifiedInvoiceRequestLegacy, amount_msats: u64, + payment_hash: PaymentHash, payment_secret: PaymentSecret, + usable_channels: Vec, ) -> (OffersMessage, Option) where ES::Target: EntropySource, diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index dedbb27c674..fec08d76e06 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -592,7 +592,7 @@ pub struct InvoiceRequest { /// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending /// on whether the signing keys were derived. #[derive(Clone, Debug)] -pub struct VerifiedInvoiceRequest { +pub struct VerifiedInvoiceRequestLegacy { /// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made. pub offer_id: OfferId, @@ -745,7 +745,7 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( /// /// If the originating [`Offer`] was created using [`OfferBuilder::deriving_signing_pubkey`], /// then first use [`InvoiceRequest::verify_using_metadata`] or - /// [`InvoiceRequest::verify_using_recipient_data`] and then [`VerifiedInvoiceRequest`] methods + /// [`InvoiceRequest::verify_using_recipient_data`] and then [`VerifiedInvoiceRequestLegacy`] methods /// instead. /// /// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at @@ -801,10 +801,10 @@ macro_rules! invoice_request_verify_method { secp_ctx: &Secp256k1, #[cfg(c_bindings)] secp_ctx: &Secp256k1, - ) -> Result { + ) -> Result { let (offer_id, keys) = $self.contents.inner.offer.verify_using_metadata(&$self.bytes, key, secp_ctx)?; - Ok(VerifiedInvoiceRequest { + Ok(VerifiedInvoiceRequestLegacy { offer_id, #[cfg(not(c_bindings))] inner: $self, @@ -831,11 +831,11 @@ macro_rules! invoice_request_verify_method { secp_ctx: &Secp256k1, #[cfg(c_bindings)] secp_ctx: &Secp256k1, - ) -> Result { + ) -> Result { let (offer_id, keys) = $self.contents.inner.offer.verify_using_recipient_data( &$self.bytes, nonce, key, secp_ctx )?; - Ok(VerifiedInvoiceRequest { + Ok(VerifiedInvoiceRequestLegacy { offer_id, #[cfg(not(c_bindings))] inner: $self, @@ -961,7 +961,7 @@ macro_rules! invoice_request_respond_with_derived_signing_pubkey_methods { ( } } } -impl VerifiedInvoiceRequest { +impl VerifiedInvoiceRequestLegacy { offer_accessors!(self, self.inner.contents.inner.offer); invoice_request_accessors!(self, self.inner.contents); #[cfg(not(c_bindings))] From 24b7c4cfa475cd3e02425ad9e97d761e10eeddfa Mon Sep 17 00:00:00 2001 From: shaavan Date: Fri, 25 Jul 2025 22:51:40 +0530 Subject: [PATCH 2/6] f: Convert fields function to macro In the following commits we will introduce `fields` function for other types as well, so to keep code DRY we convert the function to a macro. --- lightning/src/offers/invoice_request.rs | 57 ++++++++++++++----------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index fec08d76e06..b1d8009202a 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -961,9 +961,41 @@ macro_rules! invoice_request_respond_with_derived_signing_pubkey_methods { ( } } } +macro_rules! fields_accessor { + ($self:ident, $inner:expr) => { + /// Fetch the [`InvoiceRequestFields`] for this verified invoice. + /// + /// These are fields which we expect to be useful when receiving a payment for this invoice + /// request, and include the returned [`InvoiceRequestFields`] in the + /// [`PaymentContext::Bolt12Offer`]. + /// + /// [`PaymentContext::Bolt12Offer`]: crate::blinded_path::payment::PaymentContext::Bolt12Offer + pub fn fields(&$self) -> InvoiceRequestFields { + let InvoiceRequestContents { + payer_signing_pubkey, + inner: InvoiceRequestContentsWithoutPayerSigningPubkey { + quantity, + payer_note, + .. + }, + } = &$inner; + + InvoiceRequestFields { + payer_signing_pubkey: *payer_signing_pubkey, + quantity: *quantity, + payer_note_truncated: payer_note + .clone() + .map(|s| UntrustedString(string_truncate_safe(s, PAYER_NOTE_LIMIT))), + human_readable_name: $self.offer_from_hrn().clone(), + } + } + }; +} + impl VerifiedInvoiceRequestLegacy { offer_accessors!(self, self.inner.contents.inner.offer); invoice_request_accessors!(self, self.inner.contents); + fields_accessor!(self, self.inner.contents); #[cfg(not(c_bindings))] invoice_request_respond_with_explicit_signing_pubkey_methods!( self, @@ -988,31 +1020,6 @@ impl VerifiedInvoiceRequestLegacy { self.inner, InvoiceWithDerivedSigningPubkeyBuilder ); - - /// Fetch the [`InvoiceRequestFields`] for this verified invoice. - /// - /// These are fields which we expect to be useful when receiving a payment for this invoice - /// request, and include the returned [`InvoiceRequestFields`] in the - /// [`PaymentContext::Bolt12Offer`]. - /// - /// [`PaymentContext::Bolt12Offer`]: crate::blinded_path::payment::PaymentContext::Bolt12Offer - pub fn fields(&self) -> InvoiceRequestFields { - let InvoiceRequestContents { - payer_signing_pubkey, - inner: InvoiceRequestContentsWithoutPayerSigningPubkey { quantity, payer_note, .. }, - } = &self.inner.contents; - - InvoiceRequestFields { - payer_signing_pubkey: *payer_signing_pubkey, - quantity: *quantity, - payer_note_truncated: payer_note - .clone() - // Truncate the payer note to `PAYER_NOTE_LIMIT` bytes, rounding - // down to the nearest valid UTF-8 code point boundary. - .map(|s| UntrustedString(string_truncate_safe(s, PAYER_NOTE_LIMIT))), - human_readable_name: self.offer_from_hrn().clone(), - } - } } /// `String::truncate(new_len)` panics if you split inside a UTF-8 code point, From a0921886780c97c1611046461cfdd4a30d423db1 Mon Sep 17 00:00:00 2001 From: shaavan Date: Fri, 25 Jul 2025 22:53:56 +0530 Subject: [PATCH 3/6] Introduce VerifiedInvoiceRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit reintroduces `VerifiedInvoiceRequest`, now parameterized by `SigningPubkeyStrategy`. The key motivation is to restrict which functions can be called on a `VerifiedInvoiceRequest` based on its strategy type. This enables compile-time guarantees — ensuring that an incorrect `InvoiceBuilder` cannot be constructed for a given request, and misuses are caught early. --- lightning/src/offers/invoice_request.rs | 65 ++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index b1d8009202a..e714668ddc4 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -71,6 +71,7 @@ use crate::io; use crate::ln::channelmanager::PaymentId; use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; +use crate::offers::invoice::{DerivedSigningPubkey, ExplicitSigningPubkey, SigningPubkeyStrategy}; use crate::offers::merkle::{ self, SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, }; @@ -96,7 +97,7 @@ use bitcoin::secp256k1::schnorr::Signature; use bitcoin::secp256k1::{self, Keypair, PublicKey, Secp256k1}; #[cfg(not(c_bindings))] -use crate::offers::invoice::{DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder}; +use crate::offers::invoice::InvoiceBuilder; #[cfg(c_bindings)] use crate::offers::invoice::{ InvoiceWithDerivedSigningPubkeyBuilder, InvoiceWithExplicitSigningPubkeyBuilder, @@ -615,6 +616,50 @@ pub struct VerifiedInvoiceRequestLegacy { pub keys: Option, } +/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify_using_metadata`] or +/// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending +/// on whether the signing keys were derived. +#[derive(Clone, Debug)] +pub struct VerifiedInvoiceRequest { + /// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made. + pub offer_id: OfferId, + + /// The verified request. + pub(crate) inner: InvoiceRequest, + + /// Keys for signing a [`Bolt12Invoice`] for the request. + /// + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + pub keys: S, +} + +/// An enum representing a verified invoice request, which can either have derived signing keys or +/// require explicit signing keys. +pub enum VerifiedInvoiceRequestEnum { + /// An invoice request with signing keys that can be derived from the metadata. + WithKeys(VerifiedInvoiceRequest), + /// An invoice request without derived signing keys, which must be explicitly provided. + WithoutKeys(VerifiedInvoiceRequest), +} + +impl VerifiedInvoiceRequestEnum { + /// Returns a reference to the underlying `InvoiceRequest`. + pub fn inner(&self) -> &InvoiceRequest { + match self { + VerifiedInvoiceRequestEnum::WithKeys(req) => &req.inner, + VerifiedInvoiceRequestEnum::WithoutKeys(req) => &req.inner, + } + } + + /// Returns the `OfferId` of the offer this invoice request is for. + pub fn offer_id(&self) -> OfferId { + match self { + VerifiedInvoiceRequestEnum::WithKeys(req) => req.offer_id, + VerifiedInvoiceRequestEnum::WithoutKeys(req) => req.offer_id, + } + } +} + /// The contents of an [`InvoiceRequest`], which may be shared with an [`Bolt12Invoice`]. /// /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice @@ -1022,6 +1067,24 @@ impl VerifiedInvoiceRequestLegacy { ); } +impl VerifiedInvoiceRequest { + offer_accessors!(self, self.inner.contents.inner.offer); + invoice_request_accessors!(self, self.inner.contents); + fields_accessor!(self, self.inner.contents); +} + +impl VerifiedInvoiceRequest { + offer_accessors!(self, self.inner.contents.inner.offer); + invoice_request_accessors!(self, self.inner.contents); + fields_accessor!(self, self.inner.contents); +} + +impl VerifiedInvoiceRequestEnum { + offer_accessors!(self, self.inner().contents.inner.offer); + invoice_request_accessors!(self, self.inner().contents); + fields_accessor!(self, self.inner().contents); +} + /// `String::truncate(new_len)` panics if you split inside a UTF-8 code point, /// which would leave the `String` containing invalid UTF-8. This function will /// instead truncate the string to the next smaller code point boundary so the From f89d83a6af5b6b7d1e0d7cb23533ee6c401fe6b7 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sat, 26 Jul 2025 17:43:03 +0530 Subject: [PATCH 4/6] Introduce VerifiedInvoiceRequestEnum usage This commit replaces the legacy `VerifiedInvoiceRequestLegacy` with the new `VerifiedInvoiceRequestEnum` type in the codebase. --- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/ln/offers_tests.rs | 20 ++-- lightning/src/offers/flow.rs | 72 +++++++------- lightning/src/offers/invoice.rs | 40 +++++--- lightning/src/offers/invoice_request.rs | 121 ++++++++++++------------ lightning/src/offers/offer.rs | 4 +- 6 files changed, 142 insertions(+), 119 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cc09fe5b842..36a1bfa6644 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7199,7 +7199,7 @@ where }; let payment_purpose_context = PaymentContext::Bolt12Offer(Bolt12OfferContext { - offer_id: verified_invreq.offer_id, + offer_id: verified_invreq.offer_id(), invoice_request: verified_invreq.fields(), }); let from_parts_res = events::PaymentPurpose::from_parts( @@ -14095,7 +14095,7 @@ where }; let amount_msats = match InvoiceBuilder::::amount_msats( - &invoice_request.inner + &invoice_request.inner() ) { Ok(amount_msats) => amount_msats, Err(error) => return Some((OffersMessage::InvoiceError(error.into()), responder.respond())), diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index ba9858be197..b622ea8f81a 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -57,7 +57,7 @@ use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, Init, NodeAnnou use crate::ln::outbound_payment::IDEMPOTENCY_TIMEOUT_TICKS; use crate::offers::invoice::Bolt12Invoice; use crate::offers::invoice_error::InvoiceError; -use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields}; +use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields, VerifiedInvoiceRequestEnum}; use crate::offers::nonce::Nonce; use crate::offers::parse::Bolt12SemanticError; use crate::onion_message::messenger::{Destination, MessageSendInstructions, NodeIdMessageRouter, NullMessageRouter, PeeledOnion}; @@ -2279,11 +2279,19 @@ fn fails_paying_invoice_with_unknown_required_features() { let secp_ctx = Secp256k1::new(); let created_at = alice.node.duration_since_epoch(); - let invoice = invoice_request - .verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).unwrap() - .respond_using_derived_keys_no_std(payment_paths, payment_hash, created_at).unwrap() - .features_unchecked(Bolt12InvoiceFeatures::unknown()) - .build_and_sign(&secp_ctx).unwrap(); + let verified_invoice_request = invoice_request + .verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).unwrap(); + + let invoice = match verified_invoice_request { + VerifiedInvoiceRequestEnum::WithKeys(request) => { + request.respond_using_derived_keys_no_std(payment_paths, payment_hash, created_at).unwrap() + .features_unchecked(Bolt12InvoiceFeatures::unknown()) + .build_and_sign(&secp_ctx).unwrap() + }, + VerifiedInvoiceRequestEnum::WithoutKeys(_) => { + panic!("Expected invoice request with keys"); + }, + }; // Enqueue an onion message containing the new invoice. let instructions = MessageSendInstructions::WithoutReplyPath { diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 828c9b9a96b..9657995380c 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -41,7 +41,7 @@ use crate::offers::invoice::{ }; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{ - InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequestLegacy, + InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequestEnum, }; use crate::offers::nonce::Nonce; use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder}; @@ -406,7 +406,7 @@ fn enqueue_onion_message_with_reply_paths( pub enum InvreqResponseInstructions { /// We are the recipient of this payment, and a [`Bolt12Invoice`] should be sent in response to /// the invoice request since it is now verified. - SendInvoice(VerifiedInvoiceRequestLegacy), + SendInvoice(VerifiedInvoiceRequestEnum), /// We are a static invoice server and should respond to this invoice request by retrieving the /// [`StaticInvoice`] corresponding to the `recipient_id` and `invoice_id` and calling /// `OffersMessageFlow::enqueue_static_invoice`. @@ -920,7 +920,7 @@ where Ok(builder.into()) } - /// Creates a response for the provided [`VerifiedInvoiceRequestLegacy`]. + /// Creates a response for the provided [`VerifiedInvoiceRequestEnum`]. /// /// A response can be either an [`OffersMessage::Invoice`] with additional [`MessageContext`], /// or an [`OffersMessage::InvoiceError`], depending on the [`InvoiceRequest`]. @@ -930,9 +930,8 @@ where /// - We fail to generate a valid signed [`Bolt12Invoice`] for the [`InvoiceRequest`]. pub fn create_response_for_invoice_request( &self, signer: &NS, router: &R, entropy_source: ES, - invoice_request: VerifiedInvoiceRequestLegacy, amount_msats: u64, - payment_hash: PaymentHash, payment_secret: PaymentSecret, - usable_channels: Vec, + invoice_request: VerifiedInvoiceRequestEnum, amount_msats: u64, payment_hash: PaymentHash, + payment_secret: PaymentSecret, usable_channels: Vec, ) -> (OffersMessage, Option) where ES::Target: EntropySource, @@ -945,7 +944,7 @@ where let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; let context = PaymentContext::Bolt12Offer(Bolt12OfferContext { - offer_id: invoice_request.offer_id, + offer_id: invoice_request.offer_id(), invoice_request: invoice_request.fields(), }); @@ -968,35 +967,36 @@ where #[cfg(not(feature = "std"))] let created_at = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64); - let response = if invoice_request.keys.is_some() { - #[cfg(feature = "std")] - let builder = invoice_request.respond_using_derived_keys(payment_paths, payment_hash); - #[cfg(not(feature = "std"))] - let builder = invoice_request.respond_using_derived_keys_no_std( - payment_paths, - payment_hash, - created_at, - ); - builder - .map(InvoiceBuilder::::from) - .and_then(|builder| builder.allow_mpp().build_and_sign(secp_ctx)) - .map_err(InvoiceError::from) - } else { - #[cfg(feature = "std")] - let builder = invoice_request.respond_with(payment_paths, payment_hash); - #[cfg(not(feature = "std"))] - let builder = invoice_request.respond_with_no_std(payment_paths, payment_hash, created_at); - builder - .map(InvoiceBuilder::::from) - .and_then(|builder| builder.allow_mpp().build()) - .map_err(InvoiceError::from) - .and_then(|invoice| { - #[cfg(c_bindings)] - let mut invoice = invoice; - invoice - .sign(|invoice: &UnsignedBolt12Invoice| signer.sign_bolt12_invoice(invoice)) - .map_err(InvoiceError::from) - }) + let response = match invoice_request { + VerifiedInvoiceRequestEnum::WithKeys(request) => { + #[cfg(feature = "std")] + let builder = request.respond_using_derived_keys(payment_paths, payment_hash); + #[cfg(not(feature = "std"))] + let builder = request.respond_using_derived_keys_no_std(payment_paths, payment_hash, created_at); + builder + .map(InvoiceBuilder::::from) + .and_then(|builder| builder.allow_mpp().build_and_sign(secp_ctx)) + .map_err(InvoiceError::from) + }, + VerifiedInvoiceRequestEnum::WithoutKeys(request) => { + #[cfg(feature = "std")] + let builder = request.respond_with(payment_paths, payment_hash); + #[cfg(not(feature = "std"))] + let builder = request.respond_with_no_std(payment_paths, payment_hash, created_at); + builder + .map(InvoiceBuilder::::from) + .and_then(|builder| builder.allow_mpp().build()) + .map_err(InvoiceError::from) + .and_then(|invoice| { + #[cfg(c_bindings)] + let mut invoice = invoice; + invoice + .sign(|invoice: &UnsignedBolt12Invoice| { + signer.sign_bolt12_invoice(invoice) + }) + .map_err(InvoiceError::from) + }) + }, }; match response { diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 198e544fefc..4b7ef4751f5 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1819,6 +1819,7 @@ mod tests { use crate::ln::msgs::DecodeError; use crate::offers::invoice_request::{ ExperimentalInvoiceRequestTlvStreamRef, InvoiceRequestTlvStreamRef, + VerifiedInvoiceRequestEnum, }; use crate::offers::merkle::{self, SignError, SignatureTlvStreamRef, TaggedHash, TlvStream}; use crate::offers::nonce::Nonce; @@ -2235,15 +2236,25 @@ mod tests { .build_and_sign() .unwrap(); - if let Err(e) = invoice_request + let verified_request = invoice_request .clone() .verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) - .unwrap() - .respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()) - .unwrap() - .build_and_sign(&secp_ctx) - { - panic!("error building invoice: {:?}", e); + .unwrap(); + + match verified_request { + VerifiedInvoiceRequestEnum::WithKeys(req) => { + let invoice = req + .respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()) + .unwrap() + .build_and_sign(&secp_ctx); + + if let Err(e) = invoice { + panic!("error building invoice: {:?}", e); + } + }, + VerifiedInvoiceRequestEnum::WithoutKeys(_) => { + panic!("expected invoice request with keys"); + }, } let expanded_key = ExpandedKey::new([41; 32]); @@ -2263,13 +2274,14 @@ mod tests { .build_and_sign() .unwrap(); - match invoice_request - .verify_using_metadata(&expanded_key, &secp_ctx) - .unwrap() - .respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()) - { - Ok(_) => panic!("expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::InvalidMetadata), + let verified_request = + invoice_request.verify_using_metadata(&expanded_key, &secp_ctx).unwrap(); + + match verified_request { + VerifiedInvoiceRequestEnum::WithKeys(_) => { + panic!("expected invoice request without keys") + }, + VerifiedInvoiceRequestEnum::WithoutKeys(_) => (), } } diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index e714668ddc4..ae41e43ab81 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -593,18 +593,18 @@ pub struct InvoiceRequest { /// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending /// on whether the signing keys were derived. #[derive(Clone, Debug)] -pub struct VerifiedInvoiceRequestLegacy { +pub struct VerifiedInvoiceRequest { /// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made. pub offer_id: OfferId, /// The verified request. pub(crate) inner: InvoiceRequest, - /// Keys used for signing a [`Bolt12Invoice`] if they can be derived. + /// Keys for signing a [`Bolt12Invoice`] for the request. /// #[cfg_attr( feature = "std", - doc = "If `Some`, must call [`respond_using_derived_keys`] when responding. Otherwise, call [`respond_with`]." + doc = "If `DerivedSigningPubkey`, must call [`respond_using_derived_keys`] when responding. Otherwise, call [`respond_with`]." )] #[cfg_attr(feature = "std", doc = "")] /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice @@ -613,23 +613,6 @@ pub struct VerifiedInvoiceRequestLegacy { doc = "[`respond_using_derived_keys`]: Self::respond_using_derived_keys" )] #[cfg_attr(feature = "std", doc = "[`respond_with`]: Self::respond_with")] - pub keys: Option, -} - -/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify_using_metadata`] or -/// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending -/// on whether the signing keys were derived. -#[derive(Clone, Debug)] -pub struct VerifiedInvoiceRequest { - /// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made. - pub offer_id: OfferId, - - /// The verified request. - pub(crate) inner: InvoiceRequest, - - /// Keys for signing a [`Bolt12Invoice`] for the request. - /// - /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice pub keys: S, } @@ -790,7 +773,7 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( /// /// If the originating [`Offer`] was created using [`OfferBuilder::deriving_signing_pubkey`], /// then first use [`InvoiceRequest::verify_using_metadata`] or - /// [`InvoiceRequest::verify_using_recipient_data`] and then [`VerifiedInvoiceRequestLegacy`] methods + /// [`InvoiceRequest::verify_using_recipient_data`] and then [`VerifiedInvoiceRequestEnum`] methods /// instead. /// /// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at @@ -846,17 +829,30 @@ macro_rules! invoice_request_verify_method { secp_ctx: &Secp256k1, #[cfg(c_bindings)] secp_ctx: &Secp256k1, - ) -> Result { + ) -> Result { let (offer_id, keys) = $self.contents.inner.offer.verify_using_metadata(&$self.bytes, key, secp_ctx)?; - Ok(VerifiedInvoiceRequestLegacy { - offer_id, + let inner = { #[cfg(not(c_bindings))] - inner: $self, + { $self } #[cfg(c_bindings)] - inner: $self.clone(), - keys, - }) + { $self.clone() } + }; + + let verified = match keys { + None => VerifiedInvoiceRequestEnum::WithoutKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: ExplicitSigningPubkey {}, + }), + Some(keys) => VerifiedInvoiceRequestEnum::WithKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: DerivedSigningPubkey(keys), + }), + }; + + Ok(verified) } /// Verifies that the request was for an offer created using the given key by checking a nonce @@ -876,18 +872,32 @@ macro_rules! invoice_request_verify_method { secp_ctx: &Secp256k1, #[cfg(c_bindings)] secp_ctx: &Secp256k1, - ) -> Result { + ) -> Result { let (offer_id, keys) = $self.contents.inner.offer.verify_using_recipient_data( &$self.bytes, nonce, key, secp_ctx )?; - Ok(VerifiedInvoiceRequestLegacy { - offer_id, + + let inner = { #[cfg(not(c_bindings))] - inner: $self, + { $self } #[cfg(c_bindings)] - inner: $self.clone(), - keys, - }) + { $self.clone() } + }; + + let verified = match keys { + None => VerifiedInvoiceRequestEnum::WithoutKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: ExplicitSigningPubkey {}, + }), + Some(keys) => VerifiedInvoiceRequestEnum::WithKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: DerivedSigningPubkey(keys), + }), + }; + + Ok(verified) } }; } @@ -990,10 +1000,7 @@ macro_rules! invoice_request_respond_with_derived_signing_pubkey_methods { ( return Err(Bolt12SemanticError::UnknownRequiredFeatures); } - let keys = match $self.keys { - None => return Err(Bolt12SemanticError::InvalidMetadata), - Some(keys) => keys, - }; + let keys = $self.keys.0; match $contents.contents.inner.offer.issuer_signing_pubkey() { Some(signing_pubkey) => debug_assert_eq!(signing_pubkey, keys.public_key()), @@ -1037,22 +1044,11 @@ macro_rules! fields_accessor { }; } -impl VerifiedInvoiceRequestLegacy { +impl VerifiedInvoiceRequest { offer_accessors!(self, self.inner.contents.inner.offer); invoice_request_accessors!(self, self.inner.contents); fields_accessor!(self, self.inner.contents); - #[cfg(not(c_bindings))] - invoice_request_respond_with_explicit_signing_pubkey_methods!( - self, - self.inner, - InvoiceBuilder - ); - #[cfg(c_bindings)] - invoice_request_respond_with_explicit_signing_pubkey_methods!( - self, - self.inner, - InvoiceWithExplicitSigningPubkeyBuilder - ); + #[cfg(not(c_bindings))] invoice_request_respond_with_derived_signing_pubkey_methods!( self, @@ -1067,16 +1063,23 @@ impl VerifiedInvoiceRequestLegacy { ); } -impl VerifiedInvoiceRequest { - offer_accessors!(self, self.inner.contents.inner.offer); - invoice_request_accessors!(self, self.inner.contents); - fields_accessor!(self, self.inner.contents); -} - impl VerifiedInvoiceRequest { offer_accessors!(self, self.inner.contents.inner.offer); invoice_request_accessors!(self, self.inner.contents); fields_accessor!(self, self.inner.contents); + + #[cfg(not(c_bindings))] + invoice_request_respond_with_explicit_signing_pubkey_methods!( + self, + self.inner, + InvoiceBuilder + ); + #[cfg(c_bindings)] + invoice_request_respond_with_explicit_signing_pubkey_methods!( + self, + self.inner, + InvoiceWithExplicitSigningPubkeyBuilder + ); } impl VerifiedInvoiceRequestEnum { @@ -3077,7 +3080,7 @@ mod tests { match invoice_request.verify_using_metadata(&expanded_key, &secp_ctx) { Ok(invoice_request) => { let fields = invoice_request.fields(); - assert_eq!(invoice_request.offer_id, offer.id()); + assert_eq!(invoice_request.offer_id(), offer.id()); assert_eq!( fields, InvoiceRequestFields { diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index bdc9e7c8b87..14543bc8431 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -1482,7 +1482,7 @@ mod tests { .build_and_sign() .unwrap(); match invoice_request.verify_using_metadata(&expanded_key, &secp_ctx) { - Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), + Ok(invoice_request) => assert_eq!(invoice_request.offer_id(), offer.id()), Err(_) => panic!("unexpected error"), } @@ -1563,7 +1563,7 @@ mod tests { .build_and_sign() .unwrap(); match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) { - Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), + Ok(invoice_request) => assert_eq!(invoice_request.offer_id(), offer.id()), Err(_) => panic!("unexpected error"), } From bc66286b91d52405b9989daa05aab86f70ae3909 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sat, 26 Jul 2025 18:00:01 +0530 Subject: [PATCH 5/6] Introduce specific InvoiceBuilders in OffersMessageFlow This change improves type safety and architectural clarity by introducing dedicated `InvoiceBuilder` methods tied to each variant of `VerifiedInvoiceRequestEnum`. With this change, users are now required to match on the enum variant before calling the corresponding builder method. This pushes the responsibility of selecting the correct builder to the user and ensures that invalid builder usage is caught at compile time, rather than relying on runtime checks. The signing logic has also been moved from the builder to the `ChannelManager`. This shift simplifies the builder's role and aligns it with the rest of the API, where builder methods return a configurable object that can be extended before signing. The result is a more consistent and predictable interface that separates concerns cleanly and makes future maintenance easier. --- lightning/src/ln/channelmanager.rs | 86 ++++++++++++-- lightning/src/offers/flow.rs | 179 +++++++++++++++++------------ 2 files changed, 181 insertions(+), 84 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 36a1bfa6644..cea719c0ac0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -92,10 +92,11 @@ use crate::ln::types::ChannelId; use crate::offers::async_receive_offer_cache::AsyncReceiveOfferCache; use crate::offers::flow::{InvreqResponseInstructions, OffersMessageFlow}; use crate::offers::invoice::{ - Bolt12Invoice, DerivedSigningPubkey, InvoiceBuilder, DEFAULT_RELATIVE_EXPIRY, + Bolt12Invoice, DerivedSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice, + DEFAULT_RELATIVE_EXPIRY, }; use crate::offers::invoice_error::InvoiceError; -use crate::offers::invoice_request::InvoiceRequest; +use crate::offers::invoice_request::{InvoiceRequest, VerifiedInvoiceRequestEnum}; use crate::offers::nonce::Nonce; use crate::offers::offer::Offer; use crate::offers::parse::Bolt12SemanticError; @@ -14112,16 +14113,79 @@ where }, }; - let entropy = &*self.entropy_source; - let (response, context) = self.flow.create_response_for_invoice_request( - &self.node_signer, &self.router, entropy, invoice_request, amount_msats, - payment_hash, payment_secret, self.list_usable_channels() - ); + let (result, context) = match invoice_request { + VerifiedInvoiceRequestEnum::WithKeys(request) => { + let result = self.flow.create_invoice_builder_from_invoice_request_with_keys( + &self.router, + &*self.entropy_source, + &request, + amount_msats, + payment_hash, + payment_secret, + self.list_usable_channels(), + ); - match context { - Some(context) => Some((response, responder.respond_with_reply_path(context))), - None => Some((response, responder.respond())) - } + match result { + Ok((builder, context)) => { + let res = builder + .build_and_sign(&self.secp_ctx) + .map_err(InvoiceError::from); + + (res, context) + }, + Err(error) => { + return Some(( + OffersMessage::InvoiceError(InvoiceError::from(error)), + responder.respond(), + )); + }, + } + }, + VerifiedInvoiceRequestEnum::WithoutKeys(request) => { + let result = self.flow.create_invoice_builder_from_invoice_request_without_keys( + &self.router, + &*self.entropy_source, + &request, + amount_msats, + payment_hash, + payment_secret, + self.list_usable_channels(), + ); + + match result { + Ok((builder, context)) => { + let res = builder. + build() + .map_err(InvoiceError::from) + .and_then(|invoice| { + #[cfg(c_bindings)] + let mut invoice = invoice; + invoice + .sign(|invoice: &UnsignedBolt12Invoice| self.node_signer.sign_bolt12_invoice(invoice)) + .map_err(InvoiceError::from) + }); + (res, context) + }, + Err(error) => { + return Some(( + OffersMessage::InvoiceError(InvoiceError::from(error)), + responder.respond(), + )); + }, + } + } + }; + + Some(match result { + Ok(invoice) => ( + OffersMessage::Invoice(invoice), + responder.respond_with_reply_path(context), + ), + Err(error) => ( + OffersMessage::InvoiceError(error), + responder.respond(), + ), + }) }, OffersMessage::Invoice(invoice) => { let payment_id = match self.flow.verify_bolt12_invoice(&invoice, context.as_ref()) { diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 9657995380c..b570dd158f4 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -37,11 +37,10 @@ use crate::ln::inbound_payment; use crate::offers::async_receive_offer_cache::AsyncReceiveOfferCache; use crate::offers::invoice::{ Bolt12Invoice, DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder, - UnsignedBolt12Invoice, DEFAULT_RELATIVE_EXPIRY, + DEFAULT_RELATIVE_EXPIRY, }; -use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{ - InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequestEnum, + InvoiceRequest, InvoiceRequestBuilder, VerifiedInvoiceRequest, VerifiedInvoiceRequestEnum, }; use crate::offers::nonce::Nonce; use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder}; @@ -52,7 +51,7 @@ use crate::onion_message::messenger::{Destination, MessageRouter, MessageSendIns use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::OnionMessageContents; use crate::routing::router::Router; -use crate::sign::{EntropySource, NodeSigner, ReceiveAuthKey}; +use crate::sign::{EntropySource, ReceiveAuthKey}; use crate::sync::{Mutex, RwLock}; use crate::types::payment::{PaymentHash, PaymentSecret}; use crate::util::ser::Writeable; @@ -920,94 +919,126 @@ where Ok(builder.into()) } - /// Creates a response for the provided [`VerifiedInvoiceRequestEnum`]. + /// Creates an [`InvoiceBuilder`] with [`DerivedSigningPubkey`] for the + /// provided [`VerifiedInvoiceRequest`]. /// - /// A response can be either an [`OffersMessage::Invoice`] with additional [`MessageContext`], - /// or an [`OffersMessage::InvoiceError`], depending on the [`InvoiceRequest`]. + /// Returns the invoice builder along with a [`MessageContext`] that can + /// later be used to respond to the counterparty. /// - /// An [`OffersMessage::InvoiceError`] will be generated if: - /// - We fail to generate valid payment paths to include in the [`Bolt12Invoice`]. - /// - We fail to generate a valid signed [`Bolt12Invoice`] for the [`InvoiceRequest`]. - pub fn create_response_for_invoice_request( - &self, signer: &NS, router: &R, entropy_source: ES, - invoice_request: VerifiedInvoiceRequestEnum, amount_msats: u64, payment_hash: PaymentHash, - payment_secret: PaymentSecret, usable_channels: Vec, - ) -> (OffersMessage, Option) + /// Use this method when you want to inspect or modify the [`InvoiceBuilder`] + /// before signing and generating the final [`Bolt12Invoice`]. + /// + /// # Errors + /// + /// Returns a [`Bolt12SemanticError`] if: + /// - User call the function with [`VerifiedInvoiceRequest`]. + /// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`]. + /// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`]. + pub fn create_invoice_builder_from_invoice_request_with_keys<'a, ES: Deref, R: Deref>( + &'a self, router: &R, entropy_source: ES, + invoice_request: &'a VerifiedInvoiceRequest, amount_msats: u64, + payment_hash: PaymentHash, payment_secret: PaymentSecret, + usable_channels: Vec, + ) -> Result<(InvoiceBuilder<'a, DerivedSigningPubkey>, MessageContext), Bolt12SemanticError> where ES::Target: EntropySource, - NS::Target: NodeSigner, + R::Target: Router, { let entropy = &*entropy_source; - let secp_ctx = &self.secp_ctx; + let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; + + let context = PaymentContext::Bolt12Offer(Bolt12OfferContext { + offer_id: invoice_request.offer_id, + invoice_request: invoice_request.fields(), + }); + + let payment_paths = self + .create_blinded_payment_paths( + router, + entropy, + usable_channels, + Some(amount_msats), + payment_secret, + context, + relative_expiry, + ) + .map_err(|_| Bolt12SemanticError::MissingPaths)?; + + #[cfg(feature = "std")] + let builder = invoice_request.respond_using_derived_keys(payment_paths, payment_hash); + #[cfg(not(feature = "std"))] + let builder = invoice_request.respond_using_derived_keys_no_std( + payment_paths, + payment_hash, + Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64), + ); + let builder = builder.map(|b| InvoiceBuilder::from(b).allow_mpp())?; + + let context = MessageContext::Offers(OffersContext::InboundPayment { payment_hash }); + + Ok((builder, context)) + } + /// Creates an [`InvoiceBuilder`] with [`ExplicitSigningPubkey`] for the + /// provided [`VerifiedInvoiceRequest`]. + /// + /// Returns the invoice builder along with a [`MessageContext`] that can + /// later be used to respond to the counterparty. + /// + /// Use this method when you want to inspect or modify the [`InvoiceBuilder`] + /// before signing and generating the final [`Bolt12Invoice`]. + /// + /// # Errors + /// + /// Returns a [`Bolt12SemanticError`] if: + /// - User call the function with [`VerifiedInvoiceRequest`]. + /// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`]. + /// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`]. + pub fn create_invoice_builder_from_invoice_request_without_keys<'a, ES: Deref, R: Deref>( + &'a self, router: &R, entropy_source: ES, + invoice_request: &'a VerifiedInvoiceRequest, amount_msats: u64, + payment_hash: PaymentHash, payment_secret: PaymentSecret, + usable_channels: Vec, + ) -> Result<(InvoiceBuilder<'a, ExplicitSigningPubkey>, MessageContext), Bolt12SemanticError> + where + ES::Target: EntropySource, + R::Target: Router, + { + let entropy = &*entropy_source; let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; let context = PaymentContext::Bolt12Offer(Bolt12OfferContext { - offer_id: invoice_request.offer_id(), + offer_id: invoice_request.offer_id, invoice_request: invoice_request.fields(), }); - let payment_paths = match self.create_blinded_payment_paths( - router, - entropy, - usable_channels, - Some(amount_msats), - payment_secret, - context, - relative_expiry, - ) { - Ok(paths) => paths, - Err(_) => { - let error = InvoiceError::from(Bolt12SemanticError::MissingPaths); - return (OffersMessage::InvoiceError(error.into()), None); - }, - }; + let payment_paths = self + .create_blinded_payment_paths( + router, + entropy, + usable_channels, + Some(amount_msats), + payment_secret, + context, + relative_expiry, + ) + .map_err(|_| Bolt12SemanticError::MissingPaths)?; + #[cfg(feature = "std")] + let builder = invoice_request.respond_with(payment_paths, payment_hash); #[cfg(not(feature = "std"))] - let created_at = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64); + let builder = invoice_request.respond_with_no_std( + payment_paths, + payment_hash, + Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64), + ); - let response = match invoice_request { - VerifiedInvoiceRequestEnum::WithKeys(request) => { - #[cfg(feature = "std")] - let builder = request.respond_using_derived_keys(payment_paths, payment_hash); - #[cfg(not(feature = "std"))] - let builder = request.respond_using_derived_keys_no_std(payment_paths, payment_hash, created_at); - builder - .map(InvoiceBuilder::::from) - .and_then(|builder| builder.allow_mpp().build_and_sign(secp_ctx)) - .map_err(InvoiceError::from) - }, - VerifiedInvoiceRequestEnum::WithoutKeys(request) => { - #[cfg(feature = "std")] - let builder = request.respond_with(payment_paths, payment_hash); - #[cfg(not(feature = "std"))] - let builder = request.respond_with_no_std(payment_paths, payment_hash, created_at); - builder - .map(InvoiceBuilder::::from) - .and_then(|builder| builder.allow_mpp().build()) - .map_err(InvoiceError::from) - .and_then(|invoice| { - #[cfg(c_bindings)] - let mut invoice = invoice; - invoice - .sign(|invoice: &UnsignedBolt12Invoice| { - signer.sign_bolt12_invoice(invoice) - }) - .map_err(InvoiceError::from) - }) - }, - }; + let builder = builder.map(|b| InvoiceBuilder::from(b).allow_mpp())?; - match response { - Ok(invoice) => { - let context = - MessageContext::Offers(OffersContext::InboundPayment { payment_hash }); + let context = MessageContext::Offers(OffersContext::InboundPayment { payment_hash }); - (OffersMessage::Invoice(invoice), Some(context)) - }, - Err(error) => (OffersMessage::InvoiceError(error.into()), None), - } + Ok((builder, context)) } /// Enqueues the created [`InvoiceRequest`] to be sent to the counterparty. @@ -1034,6 +1065,7 @@ where /// valid reply paths for the counterparty to send back the corresponding [`Bolt12Invoice`] /// or [`InvoiceError`]. /// + /// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError /// [`supports_onion_messages`]: crate::types::features::Features::supports_onion_messages pub fn enqueue_invoice_request( &self, invoice_request: InvoiceRequest, payment_id: PaymentId, nonce: Nonce, @@ -1079,6 +1111,7 @@ where /// reply paths for the counterparty to send back the corresponding [`InvoiceError`] if we fail /// to create blinded reply paths /// + /// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError /// [`supports_onion_messages`]: crate::types::features::Features::supports_onion_messages pub fn enqueue_invoice( &self, invoice: Bolt12Invoice, refund: &Refund, peers: Vec, From ff02fd5aa2e7dbf218681b5a84462c9eded164b8 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sat, 26 Jul 2025 18:18:20 +0530 Subject: [PATCH 6/6] Refactor: Introduce `get_payment_info` closure for invoice creation To ensure correct Bolt12 payment flow behavior, the `amount_msats` used for generating the `payment_hash`, `payment_secret`, and payment path must remain consistent. Previously, these steps could inadvertently diverge due to separate sources of `amount_msats`. This commit refactors the interface to use a `get_payment_info` closure, which captures the required variables and provides a single source of truth for both payment info (payment_hash, payment_secret) and path generation. This ensures consistency and eliminates subtle bugs that could arise from mismatched amounts across the flow. --- lightning/src/ln/channelmanager.rs | 66 +++++++++++------------------- lightning/src/offers/flow.rs | 35 +++++++++++----- 2 files changed, 47 insertions(+), 54 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cea719c0ac0..3c04a7b6947 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -91,10 +91,7 @@ use crate::ln::outbound_payment::{ use crate::ln::types::ChannelId; use crate::offers::async_receive_offer_cache::AsyncReceiveOfferCache; use crate::offers::flow::{InvreqResponseInstructions, OffersMessageFlow}; -use crate::offers::invoice::{ - Bolt12Invoice, DerivedSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice, - DEFAULT_RELATIVE_EXPIRY, -}; +use crate::offers::invoice::{Bolt12Invoice, UnsignedBolt12Invoice}; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{InvoiceRequest, VerifiedInvoiceRequestEnum}; use crate::offers::nonce::Nonce; @@ -12000,27 +11997,24 @@ where ) -> Result { let secp_ctx = &self.secp_ctx; - let amount_msats = refund.amount_msats(); - let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; - let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); - match self.create_inbound_payment(Some(amount_msats), relative_expiry, None) { - Ok((payment_hash, payment_secret)) => { - let entropy = &*self.entropy_source; - let builder = self.flow.create_invoice_builder_from_refund( - &self.router, entropy, refund, payment_hash, - payment_secret, self.list_usable_channels() - )?; - - let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?; + let entropy = &*self.entropy_source; + let builder = self.flow.create_invoice_builder_from_refund( + &self.router, entropy, refund, self.list_usable_channels(), + |amount_msats, relative_expiry| { + self.create_inbound_payment( + Some(amount_msats), + relative_expiry, + None + ).map_err(|()| Bolt12SemanticError::InvalidAmount) + } + )?; - self.flow.enqueue_invoice(invoice.clone(), refund, self.get_peers_for_blinded_path())?; + let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?; - Ok(invoice) - }, - Err(()) => Err(Bolt12SemanticError::InvalidAmount), - } + self.flow.enqueue_invoice(invoice.clone(), refund, self.get_peers_for_blinded_path())?; + Ok(invoice) } /// Pays for an [`Offer`] looked up using [BIP 353] Human Readable Names resolved by the DNS @@ -14095,22 +14089,12 @@ where Err(_) => return None, }; - let amount_msats = match InvoiceBuilder::::amount_msats( - &invoice_request.inner() - ) { - Ok(amount_msats) => amount_msats, - Err(error) => return Some((OffersMessage::InvoiceError(error.into()), responder.respond())), - }; - - let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; - let (payment_hash, payment_secret) = match self.create_inbound_payment( - Some(amount_msats), relative_expiry, None - ) { - Ok((payment_hash, payment_secret)) => (payment_hash, payment_secret), - Err(()) => { - let error = Bolt12SemanticError::InvalidAmount; - return Some((OffersMessage::InvoiceError(error.into()), responder.respond())); - }, + let get_payment_info = |amount_msats, relative_expiry| { + self.create_inbound_payment( + Some(amount_msats), + relative_expiry, + None + ).map_err(|_| Bolt12SemanticError::InvalidAmount) }; let (result, context) = match invoice_request { @@ -14119,10 +14103,8 @@ where &self.router, &*self.entropy_source, &request, - amount_msats, - payment_hash, - payment_secret, self.list_usable_channels(), + get_payment_info, ); match result { @@ -14146,10 +14128,8 @@ where &self.router, &*self.entropy_source, &request, - amount_msats, - payment_hash, - payment_secret, self.list_usable_channels(), + get_payment_info, ); match result { diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index b570dd158f4..62dad7974c0 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -866,13 +866,14 @@ where /// /// Returns an error if the refund targets a different chain or if no valid /// blinded path can be constructed. - pub fn create_invoice_builder_from_refund<'a, ES: Deref, R: Deref>( - &'a self, router: &R, entropy_source: ES, refund: &'a Refund, payment_hash: PaymentHash, - payment_secret: PaymentSecret, usable_channels: Vec, + pub fn create_invoice_builder_from_refund<'a, ES: Deref, R: Deref, F>( + &'a self, router: &R, entropy_source: ES, refund: &'a Refund, + usable_channels: Vec, get_payment_info: F, ) -> Result, Bolt12SemanticError> where ES::Target: EntropySource, R::Target: Router, + F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>, { if refund.chain() != self.chain_hash { return Err(Bolt12SemanticError::UnsupportedChain); @@ -884,6 +885,8 @@ where let amount_msats = refund.amount_msats(); let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; + let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?; + let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {}); let payment_paths = self .create_blinded_payment_paths( @@ -934,20 +937,25 @@ where /// - User call the function with [`VerifiedInvoiceRequest`]. /// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`]. /// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`]. - pub fn create_invoice_builder_from_invoice_request_with_keys<'a, ES: Deref, R: Deref>( + pub fn create_invoice_builder_from_invoice_request_with_keys<'a, ES: Deref, R: Deref, F>( &'a self, router: &R, entropy_source: ES, - invoice_request: &'a VerifiedInvoiceRequest, amount_msats: u64, - payment_hash: PaymentHash, payment_secret: PaymentSecret, - usable_channels: Vec, + invoice_request: &'a VerifiedInvoiceRequest, + usable_channels: Vec, get_payment_info: F, ) -> Result<(InvoiceBuilder<'a, DerivedSigningPubkey>, MessageContext), Bolt12SemanticError> where ES::Target: EntropySource, R::Target: Router, + F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>, { let entropy = &*entropy_source; let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; + let amount_msats = + InvoiceBuilder::::amount_msats(&invoice_request.inner)?; + + let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?; + let context = PaymentContext::Bolt12Offer(Bolt12OfferContext { offer_id: invoice_request.offer_id, invoice_request: invoice_request.fields(), @@ -995,19 +1003,24 @@ where /// - User call the function with [`VerifiedInvoiceRequest`]. /// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`]. /// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`]. - pub fn create_invoice_builder_from_invoice_request_without_keys<'a, ES: Deref, R: Deref>( + pub fn create_invoice_builder_from_invoice_request_without_keys<'a, ES: Deref, R: Deref, F>( &'a self, router: &R, entropy_source: ES, - invoice_request: &'a VerifiedInvoiceRequest, amount_msats: u64, - payment_hash: PaymentHash, payment_secret: PaymentSecret, - usable_channels: Vec, + invoice_request: &'a VerifiedInvoiceRequest, + usable_channels: Vec, get_payment_info: F, ) -> Result<(InvoiceBuilder<'a, ExplicitSigningPubkey>, MessageContext), Bolt12SemanticError> where ES::Target: EntropySource, R::Target: Router, + F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>, { let entropy = &*entropy_source; let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; + let amount_msats = + InvoiceBuilder::::amount_msats(&invoice_request.inner)?; + + let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?; + let context = PaymentContext::Bolt12Offer(Bolt12OfferContext { offer_id: invoice_request.offer_id, invoice_request: invoice_request.fields(),