Drop risky unwraps in PaymentParameters::from_* methods

Previoously, we added a bunch of constructors for `PaymentParameters`
that would panic if we ever misused the wrong APIs on a `Clear` or
`Blinded` payee. While this was not immediately reachable, we here
refactor these constructors to avoid the risk going forward.

Author: tnull

lightning/src/ln/channelmanager.rs

@@ -12518,6 +12518,10 @@ where
 						);
 						InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures)
 					},
+					Err(Bolt12PaymentError::InvalidInvoice) => {
+						log_trace!($logger, "Failed paying invalid/incompatible invoice");
+						return None;
+					},
 					Err(Bolt12PaymentError::SendingFailed(e)) => {
 						log_trace!($logger, "Failed paying invoice: {:?}", e);
 						InvoiceError::from_string(format!("{:?}", e))

lightning/src/ln/offers_tests.rs

@@ -1186,7 +1186,7 @@ fn pays_bolt12_invoice_asynchronously() {
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
 
-	// Re-process the same onion message to ensure idempotency — 
+	// Re-process the same onion message to ensure idempotency —
 	// we should not generate a duplicate `InvoiceReceived` event.
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
 
@@ -2302,7 +2302,7 @@ fn rejects_keysend_to_non_static_invoice_path() {
 
 	// Pay the invoice via keysend now that we have the preimage and make sure the recipient fails it
 	// due to incorrect payment context.
-	let pay_params = PaymentParameters::from_bolt12_invoice(&invoice);
+	let pay_params = PaymentParameters::from_bolt12_invoice(&invoice).unwrap();
 	let route_params = RouteParameters::from_payment_params_and_value(pay_params, amt_msat);
 	let keysend_payment_id = PaymentId([2; 32]);
 	let payment_hash = nodes[0].node.send_spontaneous_payment(

lightning/src/ln/outbound_payment.rs

@@ -593,6 +593,8 @@ pub enum Bolt11PaymentError {
 	/// [`Bolt11Invoice`]: lightning_invoice::Bolt11Invoice
 	/// [`ChannelManager::pay_for_bolt11_invoice`]: crate::ln::channelmanager::ChannelManager::pay_for_bolt11_invoice
 	InvalidAmount,
+	/// An invalid or incompatible invoice was provided to [`ChannelManager::pay_for_bolt11_invoice`].
+	InvalidInvoice,
 	/// The invoice was valid for the corresponding [`PaymentId`], but sending the payment failed.
 	SendingFailed(RetryableSendFailure),
 }
@@ -604,6 +606,8 @@ pub enum Bolt12PaymentError {
 	UnexpectedInvoice,
 	/// Payment for an invoice with the corresponding [`PaymentId`] was already initiated.
 	DuplicateInvoice,
+	/// An invalid or incompatible invoice was provided to [`ChannelManager::pay_for_bolt12_invoice`].
+	InvalidInvoice,
 	/// The invoice was valid for the corresponding [`PaymentId`], but required unknown features.
 	UnknownRequiredFeatures,
 	/// The invoice was valid for the corresponding [`PaymentId`], but sending the payment failed.
@@ -902,6 +906,7 @@ impl OutboundPayments {
 		recipient_onion.payment_metadata = invoice.payment_metadata().map(|v| v.clone());
 
 		let payment_params = PaymentParameters::from_bolt11_invoice(invoice)
+			.map_err(|_| Bolt11PaymentError::InvalidInvoice)?
 			.with_user_config_ignoring_fee_limit(route_params_config);
 
 		let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, amount);
@@ -949,6 +954,7 @@ impl OutboundPayments {
 
 		let mut route_params = RouteParameters::from_payment_params_and_value(
 			PaymentParameters::from_bolt12_invoice(&invoice)
+				.map_err(|_| Bolt12PaymentError::InvalidInvoice)?
 				.with_user_config_ignoring_fee_limit(params_config), invoice.amount_msats()
 		);
 		if let Some(max_fee_msat) = params_config.max_total_routing_fee_msat {
@@ -1127,6 +1133,7 @@ impl OutboundPayments {
 					let keysend_preimage = PaymentPreimage(entropy_source.get_secure_random_bytes());
 					let payment_hash = PaymentHash(Sha256::hash(&keysend_preimage.0).to_byte_array());
 					let pay_params = PaymentParameters::from_static_invoice(invoice)
+						.map_err(|_| Bolt12PaymentError::InvalidInvoice)?
 						.with_user_config_ignoring_fee_limit(*route_params_config);
 					let mut route_params = RouteParameters::from_payment_params_and_value(pay_params, amount_msat);
 					route_params.max_total_routing_fee_msat = route_params_config.max_total_routing_fee_msat;
@@ -3034,7 +3041,7 @@ mod tests {
 		assert!(outbound_payments.has_pending_payments());
 
 		let route_params = RouteParameters::from_payment_params_and_value(
-			PaymentParameters::from_bolt12_invoice(&invoice),
+			PaymentParameters::from_bolt12_invoice(&invoice).unwrap(),
 			invoice.amount_msats(),
 		);
 		router.expect_find_route(route_params, Err(""));
@@ -3086,7 +3093,7 @@ mod tests {
 			.sign(recipient_sign).unwrap();
 
 		let route_params = RouteParameters {
-			payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
+			payment_params: PaymentParameters::from_bolt12_invoice(&invoice).unwrap(),
 			final_value_msat: invoice.amount_msats(),
 			max_total_routing_fee_msat: Some(1234),
 		};

lightning/src/routing/router.rs

@@ -921,41 +921,46 @@ impl PaymentParameters {
 	/// Creates parameters for paying to a blinded payee from the provided invoice. Sets
 	/// [`Payee::Blinded::route_hints`], [`Payee::Blinded::features`], and
 	/// [`PaymentParameters::expiry_time`].
-	pub fn from_bolt11_invoice(invoice: &Bolt11Invoice) -> Self {
+	///
+	/// Will return an error if an incompatible invoice is provided.
+	pub fn from_bolt11_invoice(invoice: &Bolt11Invoice) -> Result<Self, ()>{
 		let mut payment_params = Self::from_node_id(
 			invoice.recover_payee_pub_key(),
 			invoice.min_final_cltv_expiry_delta() as u32,
 		)
-		.with_route_hints(invoice.route_hints())
-		.unwrap();
+		.with_route_hints(invoice.route_hints())?;
 
 		if let Some(expiry) = invoice.expires_at() {
 			payment_params = payment_params.with_expiry_time(expiry.as_secs());
 		}
 		if let Some(features) = invoice.features() {
-			payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
+			payment_params = payment_params.with_bolt11_features(features.clone())?;
 		}
 
-		payment_params
+		Ok(payment_params)
 	}
 
 	/// Creates parameters for paying to a blinded payee from the provided invoice. Sets
 	/// [`Payee::Blinded::route_hints`], [`Payee::Blinded::features`], and
 	/// [`PaymentParameters::expiry_time`].
-	pub fn from_bolt12_invoice(invoice: &Bolt12Invoice) -> Self {
-		Self::blinded(invoice.payment_paths().to_vec())
-			.with_bolt12_features(invoice.invoice_features().clone()).unwrap()
-			.with_expiry_time(invoice.created_at().as_secs().saturating_add(invoice.relative_expiry().as_secs()))
+	///
+	/// Will return an error if an incompatible invoice is provided.
+	pub fn from_bolt12_invoice(invoice: &Bolt12Invoice) -> Result<Self, ()> {
+		Ok(Self::blinded(invoice.payment_paths().to_vec())
+			.with_bolt12_features(invoice.invoice_features().clone())?
+			.with_expiry_time(invoice.created_at().as_secs().saturating_add(invoice.relative_expiry().as_secs())))
 	}
 
 	/// Creates parameters for paying to a blinded payee from the provided invoice. Sets
 	/// [`Payee::Blinded::route_hints`], [`Payee::Blinded::features`], and
 	/// [`PaymentParameters::expiry_time`].
+	///
+	/// Will return an error if an incompatible invoice is provided.
 	#[cfg(async_payments)]
-	pub fn from_static_invoice(invoice: &StaticInvoice) -> Self {
-		Self::blinded(invoice.payment_paths().to_vec())
-			.with_bolt12_features(invoice.invoice_features().clone()).unwrap()
-			.with_expiry_time(invoice.created_at().as_secs().saturating_add(invoice.relative_expiry().as_secs()))
+	pub fn from_static_invoice(invoice: &StaticInvoice) -> Result<Self, ()> {
+		Ok(Self::blinded(invoice.payment_paths().to_vec())
+			.with_bolt12_features(invoice.invoice_features().clone())?
+			.with_expiry_time(invoice.created_at().as_secs().saturating_add(invoice.relative_expiry().as_secs())))
 	}
 
 	/// Creates parameters for paying to a blinded payee from the provided blinded route hints.