From 7e085c5ce4a98e9ba8528810118d7b5c118eb45a Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Sat, 9 Mar 2024 18:58:01 -0600
Subject: [PATCH] Refactor handling of InvoiceRequest

In order to generate and InvoiceSent event, it would be cleaner to have
one location where a Bolt12Invoice is successfully generated. Refactor
the handling code to this end and clean-up line length by making some of
the type conversions more streamlined.
---
 lightning/src/ln/channelmanager.rs    | 48 +++++++++++----------------
 lightning/src/offers/invoice_error.rs | 14 ++++++++
 2 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 49ced4db3..ff3605b39 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -61,7 +61,6 @@ use crate::ln::wire::Encode;
 use crate::offers::invoice::{BlindedPayInfo, Bolt12Invoice, DEFAULT_RELATIVE_EXPIRY, DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice};
 use crate::offers::invoice_error::InvoiceError;
 use crate::offers::invoice_request::{DerivedPayerId, InvoiceRequestBuilder};
-use crate::offers::merkle::SignError;
 use crate::offers::offer::{Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
@@ -10356,7 +10355,7 @@ where
 					self.highest_seen_timestamp.load(Ordering::Acquire) as u64
 				);
 
-				if invoice_request.keys.is_some() {
+				let response = if invoice_request.keys.is_some() {
 					#[cfg(feature = "std")]
 					let builder = invoice_request.respond_using_derived_keys(
 						payment_paths, payment_hash
@@ -10365,12 +10364,10 @@ where
 					let builder = invoice_request.respond_using_derived_keys_no_std(
 						payment_paths, payment_hash, created_at
 					);
-					let builder: Result<InvoiceBuilder<DerivedSigningPubkey>, _> =
-						builder.map(|b| b.into());
-					match builder.and_then(|b| b.allow_mpp().build_and_sign(secp_ctx)) {
-						Ok(invoice) => Some(OffersMessage::Invoice(invoice)),
-						Err(error) => Some(OffersMessage::InvoiceError(error.into())),
-					}
+					builder
+						.map(InvoiceBuilder::<DerivedSigningPubkey>::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);
@@ -10378,29 +10375,24 @@ where
 					let builder = invoice_request.respond_with_no_std(
 						payment_paths, payment_hash, created_at
 					);
-					let builder: Result<InvoiceBuilder<ExplicitSigningPubkey>, _> =
-						builder.map(|b| b.into());
-					let response = builder.and_then(|builder| builder.allow_mpp().build())
-						.map_err(|e| OffersMessage::InvoiceError(e.into()))
+					builder
+						.map(InvoiceBuilder::<ExplicitSigningPubkey>::from)
+						.and_then(|builder| builder.allow_mpp().build())
+						.map_err(InvoiceError::from)
 						.and_then(|invoice| {
 							#[cfg(c_bindings)]
 							let mut invoice = invoice;
-							match invoice.sign(|invoice: &UnsignedBolt12Invoice|
-								self.node_signer.sign_bolt12_invoice(invoice)
-							) {
-								Ok(invoice) => Ok(OffersMessage::Invoice(invoice)),
-								Err(SignError::Signing) => Err(OffersMessage::InvoiceError(
-										InvoiceError::from_string("Failed signing invoice".to_string())
-								)),
-								Err(SignError::Verification(_)) => Err(OffersMessage::InvoiceError(
-										InvoiceError::from_string("Failed invoice signature verification".to_string())
-								)),
-							}
-						});
-					match response {
-						Ok(invoice) => Some(invoice),
-						Err(error) => Some(error),
-					}
+							invoice
+								.sign(|invoice: &UnsignedBolt12Invoice|
+									self.node_signer.sign_bolt12_invoice(invoice)
+								)
+								.map_err(InvoiceError::from)
+						})
+				};
+
+				match response {
+					Ok(invoice) => Some(OffersMessage::Invoice(invoice)),
+					Err(error) => Some(OffersMessage::InvoiceError(error.into())),
 				}
 			},
 			OffersMessage::Invoice(invoice) => {
diff --git a/lightning/src/offers/invoice_error.rs b/lightning/src/offers/invoice_error.rs
index 1b6345254..5ae5f457a 100644
--- a/lightning/src/offers/invoice_error.rs
+++ b/lightning/src/offers/invoice_error.rs
@@ -11,6 +11,7 @@
 
 use crate::io;
 use crate::ln::msgs::DecodeError;
+use crate::offers::merkle::SignError;
 use crate::offers::parse::Bolt12SemanticError;
 use crate::util::ser::{HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer};
 use crate::util::string::UntrustedString;
@@ -113,6 +114,19 @@ impl From<Bolt12SemanticError> for InvoiceError {
 	}
 }
 
+impl From<SignError> for InvoiceError {
+	fn from(error: SignError) -> Self {
+		let message = match error {
+			SignError::Signing => "Failed signing invoice",
+			SignError::Verification(_) => "Failed invoice signature verification",
+		};
+		InvoiceError {
+			erroneous_field: None,
+			message: UntrustedString(message.to_string()),
+		}
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::{ErroneousField, InvoiceError};
-- 
2.39.5