Refactor: Introduce get_payment_info closure for invoice creation

shaavan · shaavan · commit d2dd871c0449 · 2025-09-26T23:29:50.000+05:30
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.
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -92,10 +92,7 @@ use crate::ln::outbound_payment::{
 use crate::ln::types::ChannelId;
 use crate::offers::async_receive_offer_cache::AsyncReceiveOfferCache;
 use crate::offers::flow::{HeldHtlcReplyPath, 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, InvoiceRequestVerifiedFromOffer};
 use crate::offers::nonce::Nonce;
@@ -12737,27 +12734,24 @@ where
 	) -> Result<Bolt12Invoice, Bolt12SemanticError> {
 		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
@@ -14876,22 +14870,12 @@ where
 					Err(_) => return None,
 				};
 
-				let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::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 {
@@ -14900,10 +14884,8 @@ where
 							&self.router,
 							&*self.entropy_source,
 							&request,
-							amount_msats,
-							payment_hash,
-							payment_secret,
 							self.list_usable_channels(),
+							get_payment_info,
 						);
 
 						match result {
@@ -14927,10 +14909,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
@@ -884,13 +884,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<ChannelDetails>,
+	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<ChannelDetails>, get_payment_info: F,
 	) -> Result<InvoiceBuilder<'a, DerivedSigningPubkey>, 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);
@@ -902,6 +903,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(
@@ -951,20 +954,25 @@ where
 	/// Returns a [`Bolt12SemanticError`] if:
 	/// - 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>(
 		&self, router: &R, entropy_source: ES,
-		invoice_request: &'a VerifiedInvoiceRequest<DerivedSigningPubkey>, amount_msats: u64,
-		payment_hash: PaymentHash, payment_secret: PaymentSecret,
-		usable_channels: Vec<ChannelDetails>,
+		invoice_request: &'a VerifiedInvoiceRequest<DerivedSigningPubkey>,
+		usable_channels: Vec<ChannelDetails>, 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::<DerivedSigningPubkey>::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(),
@@ -1011,19 +1019,24 @@ where
 	/// Returns a [`Bolt12SemanticError`] if:
 	/// - 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>(
 		&self, router: &R, entropy_source: ES,
-		invoice_request: &'a VerifiedInvoiceRequest<ExplicitSigningPubkey>, amount_msats: u64,
-		payment_hash: PaymentHash, payment_secret: PaymentSecret,
-		usable_channels: Vec<ChannelDetails>,
+		invoice_request: &'a VerifiedInvoiceRequest<ExplicitSigningPubkey>,
+		usable_channels: Vec<ChannelDetails>, 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::<DerivedSigningPubkey>::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(),
