From 827da23e968daf20f06691f78966e603389beda1 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 b6eee429ca2..c0232469be8 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::{Amount, DerivedMetadata, Offer, OfferBuilder}; @@ -393,7 +393,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`. @@ -902,7 +902,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`]. @@ -912,8 +912,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 27f32bcc1d2..eebdca90259 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 96ae45775ed065f3bb78048703ff80eaee056eec 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 | 59 ++++++++++++++----------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index eebdca90259..fa7ae1e4b64 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -961,9 +961,43 @@ 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() + // 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(), + } + } + }; +} + 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 +1022,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 b2d4198d97278851d4001d18eb8f92604d646a80 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 | 78 ++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index fa7ae1e4b64..1bd015a1404 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,63 @@ 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, +} + +/// Represents a [`VerifiedInvoiceRequest`], along with information about how the resulting +/// [`Bolt12Invoice`] should be signed. +/// +/// The signing strategy determines whether the signing keys are: +/// - Derived either from the originating [`Offer`]’s metadata or recipient_data, or +/// - Explicitly provided. +/// +/// This distinction is required to produce a valid, signed [`Bolt12Invoice`] from a verified request. +/// +/// For more on key derivation strategies, see: +/// [`InvoiceRequest::verify_using_metadata`] and [`InvoiceRequest::verify_using_recipient_data`]. +/// +/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice +pub enum InvoiceRequestVerifiedFromOffer { + /// A verified invoice request that uses signing keys derived from the originating [`Offer`]’s metadata or recipient_data. + DerivedKeys(VerifiedInvoiceRequest), + /// A verified invoice request that requires explicitly provided signing keys to sign the resulting [`Bolt12Invoice`]. + /// + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + ExplicitKeys(VerifiedInvoiceRequest), +} + +impl InvoiceRequestVerifiedFromOffer { + /// Returns a reference to the underlying `InvoiceRequest`. + fn inner(&self) -> &InvoiceRequest { + match self { + InvoiceRequestVerifiedFromOffer::DerivedKeys(req) => &req.inner, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(req) => &req.inner, + } + } + + /// Returns the `OfferId` of the offer this invoice request is for. + pub fn offer_id(&self) -> OfferId { + match self { + InvoiceRequestVerifiedFromOffer::DerivedKeys(req) => req.offer_id, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(req) => req.offer_id, + } + } +} + /// The contents of an [`InvoiceRequest`], which may be shared with an [`Bolt12Invoice`]. /// /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice @@ -1024,6 +1082,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 InvoiceRequestVerifiedFromOffer { + 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 20334f57e29a8a2acfc078d520ddee950fac16f6 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sat, 26 Jul 2025 17:43:03 +0530 Subject: [PATCH 4/6] Introduce InvoiceRequestVerifiedFromOffer usage This commit replaces the legacy `VerifiedInvoiceRequestLegacy` with the new `InvoiceRequestVerifiedFromOffer` type in the codebase. --- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/ln/offers_tests.rs | 20 ++-- lightning/src/offers/flow.rs | 69 ++++++------- lightning/src/offers/invoice.rs | 40 +++++--- lightning/src/offers/invoice_request.rs | 123 ++++++++++++------------ lightning/src/offers/offer.rs | 4 +- 6 files changed, 142 insertions(+), 118 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e4614351ee9..6f0ff01b15e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7422,7 +7422,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( @@ -14330,7 +14330,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 6c56ecc4270..2a1059b0392 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, InvoiceRequestVerifiedFromOffer}; use crate::offers::nonce::Nonce; use crate::offers::parse::Bolt12SemanticError; use crate::onion_message::messenger::{Destination, MessageSendInstructions, NodeIdMessageRouter, NullMessageRouter, PeeledOnion}; @@ -2276,11 +2276,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 { + InvoiceRequestVerifiedFromOffer::DerivedKeys(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() + }, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(_) => { + 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 c0232469be8..61c25d0cca0 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, InvoiceRequestVerifiedFromOffer, }; use crate::offers::nonce::Nonce; use crate::offers::offer::{Amount, DerivedMetadata, Offer, OfferBuilder}; @@ -393,7 +393,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(InvoiceRequestVerifiedFromOffer), /// 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`. @@ -902,7 +902,7 @@ where Ok(builder.into()) } - /// Creates a response for the provided [`VerifiedInvoiceRequestLegacy`]. + /// Creates a response for the provided [`InvoiceRequestVerifiedFromOffer`]. /// /// A response can be either an [`OffersMessage::Invoice`] with additional [`MessageContext`], /// or an [`OffersMessage::InvoiceError`], depending on the [`InvoiceRequest`]. @@ -912,7 +912,7 @@ 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, + invoice_request: InvoiceRequestVerifiedFromOffer, amount_msats: u64, payment_hash: PaymentHash, payment_secret: PaymentSecret, usable_channels: Vec, ) -> (OffersMessage, Option) @@ -927,7 +927,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(), }); @@ -950,35 +950,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 { + InvoiceRequestVerifiedFromOffer::DerivedKeys(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) + }, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(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 9751f52b046..0724ab89be9 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, + InvoiceRequestVerifiedFromOffer, }; 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 { + InvoiceRequestVerifiedFromOffer::DerivedKeys(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); + } + }, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(_) => { + 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 { + InvoiceRequestVerifiedFromOffer::DerivedKeys(_) => { + panic!("expected invoice request without keys") + }, + InvoiceRequestVerifiedFromOffer::ExplicitKeys(_) => (), } } diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 1bd015a1404..6983f0eca4c 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, } @@ -657,7 +640,7 @@ pub enum InvoiceRequestVerifiedFromOffer { impl InvoiceRequestVerifiedFromOffer { /// Returns a reference to the underlying `InvoiceRequest`. - fn inner(&self) -> &InvoiceRequest { + pub(crate) fn inner(&self) -> &InvoiceRequest { match self { InvoiceRequestVerifiedFromOffer::DerivedKeys(req) => &req.inner, InvoiceRequestVerifiedFromOffer::ExplicitKeys(req) => &req.inner, @@ -803,7 +786,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 [`InvoiceRequestVerifiedFromOffer`] methods /// instead. /// /// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at @@ -859,17 +842,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 => InvoiceRequestVerifiedFromOffer::ExplicitKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: ExplicitSigningPubkey {}, + }), + Some(keys) => InvoiceRequestVerifiedFromOffer::DerivedKeys(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 @@ -889,18 +885,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 => InvoiceRequestVerifiedFromOffer::ExplicitKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: ExplicitSigningPubkey {}, + }), + Some(keys) => InvoiceRequestVerifiedFromOffer::DerivedKeys(VerifiedInvoiceRequest { + offer_id, + inner, + keys: DerivedSigningPubkey(keys), + }), + }; + + Ok(verified) } }; } @@ -1003,10 +1013,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()), @@ -1052,22 +1059,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<'_, ExplicitSigningPubkey> - ); - #[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, @@ -1082,16 +1078,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<'_, ExplicitSigningPubkey> + ); + #[cfg(c_bindings)] + invoice_request_respond_with_explicit_signing_pubkey_methods!( + self, + self.inner, + InvoiceWithExplicitSigningPubkeyBuilder + ); } impl InvoiceRequestVerifiedFromOffer { @@ -3092,7 +3095,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 5b613091c6b..de376b2d2d4 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 edf8bdb9387cb8c11282f0093c5a584fcd394e1b 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 | 84 ++++++++++++-- lightning/src/offers/flow.rs | 177 +++++++++++++++++------------ 2 files changed, 178 insertions(+), 83 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6f0ff01b15e..2e9d72eff8d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -94,7 +94,8 @@ 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; @@ -14347,16 +14348,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 61c25d0cca0..26549fc0979 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, InvoiceRequestVerifiedFromOffer, + InvoiceRequest, InvoiceRequestBuilder, InvoiceRequestVerifiedFromOffer, VerifiedInvoiceRequest, }; use crate::offers::nonce::Nonce; use crate::offers::offer::{Amount, DerivedMetadata, Offer, OfferBuilder}; @@ -57,8 +56,7 @@ use crate::onion_message::messenger::{ 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::offers::static_invoice::{StaticInvoice, StaticInvoiceBuilder}; use crate::sync::{Mutex, RwLock}; use crate::types::payment::{PaymentHash, PaymentSecret}; @@ -902,95 +900,126 @@ where Ok(builder.into()) } - /// Creates a response for the provided [`InvoiceRequestVerifiedFromOffer`]. + /// Creates an [`InvoiceBuilder`] with [`DerivedSigningPubkey`] for the + /// provided [`VerifiedInvoiceRequest`]. + /// + /// Returns the invoice builder along with a [`MessageContext`] that can + /// later be used to respond to the counterparty. /// - /// A response can be either an [`OffersMessage::Invoice`] with additional [`MessageContext`], - /// or an [`OffersMessage::InvoiceError`], depending on the [`InvoiceRequest`]. + /// Use this method when you want to inspect or modify the [`InvoiceBuilder`] + /// before signing and generating the final [`Bolt12Invoice`]. /// - /// 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: InvoiceRequestVerifiedFromOffer, amount_msats: u64, + /// # 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, - ) -> (OffersMessage, Option) + ) -> 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 { - InvoiceRequestVerifiedFromOffer::DerivedKeys(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) - }, - InvoiceRequestVerifiedFromOffer::ExplicitKeys(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. @@ -1017,6 +1046,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, @@ -1062,6 +1092,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 71669fd05f7912f05d4f9325d631b1029f24420d 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 | 72 +++++++++++------------------- lightning/src/offers/flow.rs | 35 ++++++++++----- 2 files changed, 50 insertions(+), 57 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e9d72eff8d..5fb4110d3b4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -93,12 +93,9 @@ 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; +use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestVerifiedFromOffer}; use crate::offers::nonce::Nonce; use crate::offers::offer::Offer; use crate::offers::parse::Bolt12SemanticError; @@ -12240,27 +12237,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 @@ -14330,34 +14324,22 @@ 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 { - VerifiedInvoiceRequestEnum::WithKeys(request) => { + InvoiceRequestVerifiedFromOffer::DerivedKeys(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(), + get_payment_info, ); match result { @@ -14376,15 +14358,13 @@ where }, } }, - VerifiedInvoiceRequestEnum::WithoutKeys(request) => { + InvoiceRequestVerifiedFromOffer::ExplicitKeys(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(), + get_payment_info, ); match result { diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 26549fc0979..6178f4b2ef0 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -847,13 +847,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); @@ -865,6 +866,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( @@ -915,20 +918,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(), @@ -976,19 +984,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(),