From f52bd0b8e3a0750f5a5fd2d2732b483b60c5eac7 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Thu, 18 Jul 2024 11:38:44 -0500
Subject: [PATCH 01/19] HMAC construction and verification for PaymentID

When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add functions for
constructing and verifying an HMAC over a Payment ID to allow for this.
---
 lightning/src/offers/signer.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs
index c15b94d4996..0aa51cd3338 100644
--- a/lightning/src/offers/signer.rs
+++ b/lightning/src/offers/signer.rs
@@ -36,6 +36,9 @@ const DERIVED_METADATA_AND_KEYS_HMAC_INPUT: &[u8; 16] = &[2; 16];
 const WITHOUT_ENCRYPTED_PAYMENT_ID_HMAC_INPUT: &[u8; 16] = &[3; 16];
 const WITH_ENCRYPTED_PAYMENT_ID_HMAC_INPUT: &[u8; 16] = &[4; 16];
 
+// HMAC input for a `PaymentId`. The HMAC is used in `OffersContext::OutboundPayment`.
+const PAYMENT_ID_HMAC_INPUT: &[u8; 16] = &[5; 16];
+
 /// Message metadata which possibly is derived from [`MetadataMaterial`] such that it can be
 /// verified.
 #[derive(Clone)]
@@ -391,3 +394,22 @@ fn hmac_for_message<'a>(
 
 	Ok(hmac)
 }
+
+pub(crate) fn hmac_for_payment_id(
+	payment_id: PaymentId, nonce: Nonce, expanded_key: &ExpandedKey,
+) -> Hmac<Sha256> {
+	const IV_BYTES: &[u8; IV_LEN] = b"LDK Payment ID ~";
+	let mut hmac = expanded_key.hmac_for_offer();
+	hmac.input(IV_BYTES);
+	hmac.input(&nonce.0);
+	hmac.input(PAYMENT_ID_HMAC_INPUT);
+	hmac.input(&payment_id.0);
+
+	Hmac::from_engine(hmac)
+}
+
+pub(crate) fn verify_payment_id(
+	payment_id: PaymentId, hmac: Hmac<Sha256>, nonce: Nonce, expanded_key: &ExpandedKey,
+) -> bool {
+	hmac_for_payment_id(payment_id, nonce, expanded_key) == hmac
+}

From 8119fbfaf9fb72a083c7e20250766a1eb4e0183b Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Thu, 18 Jul 2024 11:42:57 -0500
Subject: [PATCH 02/19] Add Sha256 HMAC (de)serialization

An HMAC needs to be included in OffersContext::OutboundPayment to
authenticate the included PaymentId. Implement Readable and Writeable to
allow for this.
---
 lightning/src/util/ser.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs
index 29c52f6a08b..5f203b74d97 100644
--- a/lightning/src/util/ser.rs
+++ b/lightning/src/util/ser.rs
@@ -33,7 +33,9 @@ use bitcoin::script::{self, ScriptBuf};
 use bitcoin::transaction::{OutPoint, Transaction, TxOut};
 use bitcoin::{consensus, Witness};
 use bitcoin::consensus::Encodable;
+use bitcoin::hashes::hmac::Hmac;
 use bitcoin::hashes::sha256d::Hash as Sha256dHash;
+use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hash_types::{Txid, BlockHash};
 use core::time::Duration;
 use crate::chain::ClaimId;
@@ -1033,6 +1035,21 @@ impl Readable for PartialSignatureWithNonce {
 	}
 }
 
+impl Writeable for Hmac<Sha256> {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+		w.write_all(&self[..])
+	}
+}
+
+impl Readable for Hmac<Sha256> {
+	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+		use bitcoin::hashes::Hash;
+
+		let buf: [u8; 32] = Readable::read(r)?;
+		Ok(Hmac::<Sha256>::from_byte_array(buf))
+	}
+}
+
 impl Writeable for Sha256dHash {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		w.write_all(&self[..])

From 0ca9faf0783ec7eecb5dac5c436411da71583ac7 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Thu, 18 Jul 2024 11:45:09 -0500
Subject: [PATCH 03/19] Add an HMAC to OffersContext::OutboundPayment

When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add an HMAC to
OffersContext::OutboundPayment such that the payment ID can be
authenticated prior to abandoning the payment.
---
 lightning/src/blinded_path/message.rs | 9 +++++++++
 lightning/src/ln/channelmanager.rs    | 9 ++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs
index 47444eb900d..313ac56bfcf 100644
--- a/lightning/src/blinded_path/message.rs
+++ b/lightning/src/blinded_path/message.rs
@@ -16,6 +16,8 @@ use bitcoin::secp256k1::{self, PublicKey, Secp256k1, SecretKey};
 #[allow(unused_imports)]
 use crate::prelude::*;
 
+use bitcoin::hashes::hmac::Hmac;
+use bitcoin::hashes::sha256::Hash as Sha256;
 use crate::blinded_path::{BlindedHop, BlindedPath, IntroductionNode, NextMessageHop, NodeIdLookUp};
 use crate::blinded_path::utils;
 use crate::io;
@@ -146,6 +148,12 @@ pub enum OffersContext {
 		/// [`Refund`]: crate::offers::refund::Refund
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		nonce: Nonce,
+
+		/// Authentication code for the [`PaymentId`], which should be checked when the context is
+		/// used with an [`InvoiceError`].
+		///
+		/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
+		hmac: Hmac<Sha256>,
 	},
 	/// Context used by a [`BlindedPath`] as a reply path for a [`Bolt12Invoice`].
 	///
@@ -173,6 +181,7 @@ impl_writeable_tlv_based_enum!(OffersContext,
 	(1, OutboundPayment) => {
 		(0, payment_id, required),
 		(1, nonce, required),
+		(2, hmac, required),
 	},
 	(2, InboundPayment) => {
 		(0, payment_hash, required),
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 4ce3ebc37fc..b359df9e5ed 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -68,6 +68,7 @@ use crate::offers::nonce::Nonce;
 use crate::offers::offer::{Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
+use crate::offers::signer;
 use crate::onion_message::async_payments::{AsyncPaymentsMessage, HeldHtlcAvailable, ReleaseHeldHtlc, AsyncPaymentsMessageHandler};
 use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
@@ -4227,7 +4228,7 @@ where
 			None if invoice.is_for_refund_without_paths() => {
 				invoice.verify_using_metadata(expanded_key, secp_ctx)
 			},
-			Some(&OffersContext::OutboundPayment { payment_id, nonce }) => {
+			Some(&OffersContext::OutboundPayment { payment_id, nonce, .. }) => {
 				invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx)
 			},
 			_ => Err(()),
@@ -8886,7 +8887,8 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 		let secp_ctx = &$self.secp_ctx;
 
 		let nonce = Nonce::from_entropy_source(entropy);
-		let context = OffersContext::OutboundPayment { payment_id, nonce };
+		let hmac = signer::hmac_for_payment_id(payment_id, nonce, expanded_key);
+		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac };
 		let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry))
 			.and_then(|paths| paths.into_iter().next().ok_or(()))
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
@@ -9021,7 +9023,8 @@ where
 		};
 		let invoice_request = builder.build_and_sign()?;
 
-		let context = OffersContext::OutboundPayment { payment_id, nonce };
+		let hmac = signer::hmac_for_payment_id(payment_id, nonce, expanded_key);
+		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac };
 		let reply_paths = self.create_blinded_paths(context)
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 

From bb445a3973957b5046e2354992901fb675198cad Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Thu, 18 Jul 2024 11:54:33 -0500
Subject: [PATCH 04/19] Authenticate payment_id from OffersContext

Before abandoning a payment when receiving an InvoiceError, verify that
the PaymentId included in the OffersContext with the included HMAC. This
prevents a malicious actor sending an InvoiceError with a known payment
id from abandoning our payment.
---
 lightning/src/ln/channelmanager.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index b359df9e5ed..9dc8270d8a5 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -10731,8 +10731,10 @@ where
 
 		let abandon_if_payment = |context| {
 			match context {
-				Some(OffersContext::OutboundPayment { payment_id, .. }) => {
-					self.abandon_payment(payment_id)
+				Some(OffersContext::OutboundPayment { payment_id, nonce, hmac }) => {
+					if signer::verify_payment_id(payment_id, hmac, nonce, expanded_key) {
+						self.abandon_payment(payment_id);
+					}
 				},
 				_ => {},
 			}

From 144d4882adf910d3daeab3ddba0b4e8793d7c25a Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 24 Jul 2024 16:33:02 -0500
Subject: [PATCH 05/19] Don't abandon payments for duplicate invoices

When making an outbound BOLT12 payment, multiple invoices may be
received for the same payment id. Instead of abandoning the payment when
a duplicate invoice received, simply ignore it without responding with
an InvoiceError. This prevents abandoning in-progress payments and
sending unnecessary onion messages.
---
 lightning/src/ln/channelmanager.rs            | 74 ++++++++++---------
 .../src/ln/max_payment_path_len_tests.rs      |  2 +-
 lightning/src/ln/offers_tests.rs              | 19 ++---
 3 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9dc8270d8a5..6b817e56bac 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -10853,43 +10853,51 @@ where
 					&self.logger, None, None, Some(invoice.payment_hash()),
 				);
 
-				let result = {
-					let features = self.bolt12_invoice_features();
-					if invoice.invoice_features().requires_unknown_bits_from(&features) {
-						Err(InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures))
-					} else if self.default_configuration.manually_handle_bolt12_invoices {
-						let event = Event::InvoiceReceived {
-							payment_id, invoice, context, responder,
-						};
-						self.pending_events.lock().unwrap().push_back((event, None));
-						return ResponseInstruction::NoResponse;
-					} else {
-						self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id)
-							.map_err(|e| {
-								log_trace!(logger, "Failed paying invoice: {:?}", e);
-								InvoiceError::from_string(format!("{:?}", e))
-							})
-					}
-				};
+				let features = self.bolt12_invoice_features();
+				if invoice.invoice_features().requires_unknown_bits_from(&features) {
+					log_trace!(
+						logger, "Invoice requires unknown features: {:?}",
+						invoice.invoice_features(),
+					);
+					abandon_if_payment(context);
 
-				match result {
-					Ok(_) => ResponseInstruction::NoResponse,
-					Err(err) => match responder {
-						Some(responder) => {
-							abandon_if_payment(context);
-							responder.respond(OffersMessage::InvoiceError(err))
-						},
+					let error = InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures);
+					let response = match responder {
+						Some(responder) => responder.respond(OffersMessage::InvoiceError(error)),
 						None => {
-							abandon_if_payment(context);
-							log_trace!(
-								logger,
-								"An error response was generated, but there is no reply_path specified \
-								for sending the response. Error: {}",
-								err
-							);
-							return ResponseInstruction::NoResponse;
+							log_trace!(logger, "No reply path to send error: {:?}", error);
+							ResponseInstruction::NoResponse
 						},
+					};
+					return response;
+				}
+
+				if self.default_configuration.manually_handle_bolt12_invoices {
+					let event = Event::InvoiceReceived {
+						payment_id, invoice, context, responder,
+					};
+					self.pending_events.lock().unwrap().push_back((event, None));
+					return ResponseInstruction::NoResponse;
+				}
+
+				match self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id) {
+					// Payments with SendingFailed error will already have been abandoned.
+					Err(Bolt12PaymentError::SendingFailed(e)) => {
+						log_trace!(logger, "Failed paying invoice: {:?}", e);
+
+						let err = InvoiceError::from_string(format!("{:?}", e));
+						match responder {
+							Some(responder) => responder.respond(OffersMessage::InvoiceError(err)),
+							None => {
+								log_trace!(logger, "No reply path to send error: {:?}", err);
+								ResponseInstruction::NoResponse
+							},
+						}
 					},
+					// Otherwise, don't abandon unknown, pending, or successful payments.
+					Err(Bolt12PaymentError::UnexpectedInvoice)
+						| Err(Bolt12PaymentError::DuplicateInvoice)
+						| Ok(()) => ResponseInstruction::NoResponse,
 				}
 			},
 			#[cfg(async_payments)]
diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs
index fd027ea62b6..096bcf9633c 100644
--- a/lightning/src/ln/max_payment_path_len_tests.rs
+++ b/lightning/src/ln/max_payment_path_len_tests.rs
@@ -388,7 +388,7 @@ fn bolt12_invoice_too_large_blinded_paths() {
 	let invoice_om = nodes[1].onion_messenger.next_onion_message_for_peer(nodes[0].node.get_our_node_id()).unwrap();
 	nodes[0].onion_messenger.handle_onion_message(&nodes[1].node.get_our_node_id(), &invoice_om);
 	// TODO: assert on the invoice error once we support replying to invoice OMs with failure info
-	nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Failed paying invoice: SendingFailed(OnionPacketSizeExceeded)", 1);
+	nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Failed paying invoice: OnionPacketSizeExceeded", 1);
 
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index a7fc92f527f..e774bc08162 100644
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -2027,16 +2027,13 @@ fn fails_paying_invoice_more_than_once() {
 	let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap();
 	david.onion_messenger.handle_onion_message(&charlie_id, &onion_message);
 
-	// David pays the first invoice
+	// David initiates paying the first invoice
 	let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
 	let (invoice1, _) = extract_invoice(david, &onion_message);
 
 	route_bolt12_payment(david, &[charlie, bob, alice], &invoice1);
 	expect_recent_payment!(david, RecentPaymentDetails::Pending, payment_id);
 
-	claim_bolt12_payment(david, &[charlie, bob, alice], payment_context);
-	expect_recent_payment!(david, RecentPaymentDetails::Fulfilled, payment_id);
-
 	disconnect_peers(alice, &[charlie]);
 
 	// Alice sends the second invoice
@@ -2054,13 +2051,11 @@ fn fails_paying_invoice_more_than_once() {
 	let (invoice2, _) = extract_invoice(david, &onion_message);
 	assert_eq!(invoice1.payer_metadata(), invoice2.payer_metadata());
 
-	// David sends an error instead of paying the second invoice
-	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
-	bob.onion_messenger.handle_onion_message(&david_id, &onion_message);
-
-	let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
-	alice.onion_messenger.handle_onion_message(&bob_id, &onion_message);
+	// David doesn't initiate paying the second invoice
+	assert!(david.onion_messenger.next_onion_message_for_peer(bob_id).is_none());
+	assert!(david.node.get_and_clear_pending_msg_events().is_empty());
 
-	let invoice_error = extract_invoice_error(alice, &onion_message);
-	assert_eq!(invoice_error, InvoiceError::from_string("DuplicateInvoice".to_string()));
+	// Complete paying the first invoice
+	claim_bolt12_payment(david, &[charlie, bob, alice], payment_context);
+	expect_recent_payment!(david, RecentPaymentDetails::Fulfilled, payment_id);
 }

From fbaf093ff4d24ef7474c7e99d55b74f3f51abc21 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 24 Jul 2024 17:05:42 -0500
Subject: [PATCH 06/19] Don't use UserAbandoned reason for auto-failing

A BOLT12 payment may be abandoned when handling the invoice or when
receiving an InvoiceError message. When abandoning the payment, don't
use UserAbandoned as the reason since that is meant for when the user
calls ChannelManager::abandon_payment.
---
 lightning/src/events/mod.rs        |  5 ++++-
 lightning/src/ln/channelmanager.rs | 34 ++++++++++++++++++------------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 3972ea470da..393acbc0242 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -501,7 +501,7 @@ impl_writeable_tlv_based_enum!(InterceptNextHop,
 /// The reason the payment failed. Used in [`Event::PaymentFailed`].
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum PaymentFailureReason {
-	/// The intended recipient rejected our payment.
+	/// The intended recipient rejected our payment or invoice request.
 	RecipientRejected,
 	/// The user chose to abandon this payment by calling [`ChannelManager::abandon_payment`].
 	///
@@ -528,10 +528,13 @@ pub enum PaymentFailureReason {
 	/// This error should generally never happen. This likely means that there is a problem with
 	/// your router.
 	UnexpectedError,
+	/// An invoice was received that required unknown features.
+	UnknownRequiredFeatures,
 }
 
 impl_writeable_tlv_based_enum!(PaymentFailureReason,
 	(0, RecipientRejected) => {},
+	(1, UnknownRequiredFeatures) => {},
 	(2, UserAbandoned) => {},
 	(4, RetriesExhausted) => {},
 	(6, PaymentExpired) => {},
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 6b817e56bac..17361a1628a 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4275,8 +4275,12 @@ where
 	///
 	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 	pub fn abandon_payment(&self, payment_id: PaymentId) {
+		self.abandon_payment_with_reason(payment_id, PaymentFailureReason::UserAbandoned)
+	}
+
+	fn abandon_payment_with_reason(&self, payment_id: PaymentId, reason: PaymentFailureReason) {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
-		self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events);
+		self.pending_outbound_payments.abandon_payment(payment_id, reason, &self.pending_events);
 	}
 
 	/// Send a spontaneous payment, which is a payment that does not require the recipient to have
@@ -10729,17 +10733,6 @@ where
 		let secp_ctx = &self.secp_ctx;
 		let expanded_key = &self.inbound_payment_key;
 
-		let abandon_if_payment = |context| {
-			match context {
-				Some(OffersContext::OutboundPayment { payment_id, nonce, hmac }) => {
-					if signer::verify_payment_id(payment_id, hmac, nonce, expanded_key) {
-						self.abandon_payment(payment_id);
-					}
-				},
-				_ => {},
-			}
-		};
-
 		match message {
 			OffersMessage::InvoiceRequest(invoice_request) => {
 				let responder = match responder {
@@ -10859,7 +10852,9 @@ where
 						logger, "Invoice requires unknown features: {:?}",
 						invoice.invoice_features(),
 					);
-					abandon_if_payment(context);
+					self.abandon_payment_with_reason(
+						payment_id, PaymentFailureReason::UnknownRequiredFeatures,
+					);
 
 					let error = InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures);
 					let response = match responder {
@@ -10916,10 +10911,21 @@ where
 					Some(OffersContext::InboundPayment { payment_hash }) => Some(payment_hash),
 					_ => None,
 				};
+
 				let logger = WithContext::from(&self.logger, None, None, payment_hash);
 				log_trace!(logger, "Received invoice_error: {}", invoice_error);
 
-				abandon_if_payment(context);
+				match context {
+					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac }) => {
+						if signer::verify_payment_id(payment_id, hmac, nonce, expanded_key) {
+							self.abandon_payment_with_reason(
+								payment_id, PaymentFailureReason::RecipientRejected,
+							);
+						}
+					},
+					_ => {},
+				}
+
 				ResponseInstruction::NoResponse
 			},
 		}

From cfd098048ecddf494c803b925643647ec3ce388a Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Fri, 2 Aug 2024 09:39:52 -0500
Subject: [PATCH 07/19] Don't include HMAC in Refund paths

Refunds are typically communicated via QR code, where a smaller size is
desirable. Make the HMAC in OutboundPayment data optional such that it
is elided from blinded paths used in refunds. This prevents abandoning
refunds if the reader sends an invoice_error instead of an invoice
message. However, this use case isn't necessary as the corresponding
outbound payment will either timeout when the refund expires or can be
explicitly abandoned by the creator.
---
 lightning/src/blinded_path/message.rs | 4 ++--
 lightning/src/ln/channelmanager.rs    | 7 +++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs
index 313ac56bfcf..26019c0369d 100644
--- a/lightning/src/blinded_path/message.rs
+++ b/lightning/src/blinded_path/message.rs
@@ -153,7 +153,7 @@ pub enum OffersContext {
 		/// used with an [`InvoiceError`].
 		///
 		/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
-		hmac: Hmac<Sha256>,
+		hmac: Option<Hmac<Sha256>>,
 	},
 	/// Context used by a [`BlindedPath`] as a reply path for a [`Bolt12Invoice`].
 	///
@@ -181,7 +181,7 @@ impl_writeable_tlv_based_enum!(OffersContext,
 	(1, OutboundPayment) => {
 		(0, payment_id, required),
 		(1, nonce, required),
-		(2, hmac, required),
+		(2, hmac, option),
 	},
 	(2, InboundPayment) => {
 		(0, payment_hash, required),
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 17361a1628a..5ebb3b2a8b3 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -8891,8 +8891,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 		let secp_ctx = &$self.secp_ctx;
 
 		let nonce = Nonce::from_entropy_source(entropy);
-		let hmac = signer::hmac_for_payment_id(payment_id, nonce, expanded_key);
-		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac };
+		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: None };
 		let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry))
 			.and_then(|paths| paths.into_iter().next().ok_or(()))
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
@@ -9028,7 +9027,7 @@ where
 		let invoice_request = builder.build_and_sign()?;
 
 		let hmac = signer::hmac_for_payment_id(payment_id, nonce, expanded_key);
-		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac };
+		let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) };
 		let reply_paths = self.create_blinded_paths(context)
 			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 
@@ -10916,7 +10915,7 @@ where
 				log_trace!(logger, "Received invoice_error: {}", invoice_error);
 
 				match context {
-					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac }) => {
+					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) }) => {
 						if signer::verify_payment_id(payment_id, hmac, nonce, expanded_key) {
 							self.abandon_payment_with_reason(
 								payment_id, PaymentFailureReason::RecipientRejected,

From e4ca166e6298eb57d82d635854ca7cdee0d13279 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Mon, 29 Jul 2024 16:10:43 -0500
Subject: [PATCH 08/19] Move BOLT12 invoice features check

When handling a BOLT12 invoice, and invoice error is sent if the invoice
contains unknown required features. However, since the payment is still
in state AwaitingInvoice, abandoning it results in losing the reason
since an InvoiceRequestFailed event would be generated. Move the check
to PendingOutboundPayments such that the payment is first moved to state
InvoiceReceived so that a PaymentFailed event is generated instead.
---
 lightning/src/ln/channelmanager.rs   | 57 +++++++++++-----------------
 lightning/src/ln/outbound_payment.rs | 48 +++++++++++++----------
 2 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 5ebb3b2a8b3..617c58c2e48 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4238,9 +4238,10 @@ where
 	fn send_payment_for_verified_bolt12_invoice(&self, invoice: &Bolt12Invoice, payment_id: PaymentId) -> Result<(), Bolt12PaymentError> {
 		let best_block_height = self.best_block.read().unwrap().height;
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
+		let features = self.bolt12_invoice_features();
 		self.pending_outbound_payments
 			.send_payment_for_bolt12_invoice(
-				invoice, payment_id, &self.router, self.list_usable_channels(),
+				invoice, payment_id, &self.router, self.list_usable_channels(), features,
 				|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self,
 				&self.secp_ctx, best_block_height, &self.logger, &self.pending_events,
 				|args| self.send_payment_along_path(args)
@@ -10845,27 +10846,6 @@ where
 					&self.logger, None, None, Some(invoice.payment_hash()),
 				);
 
-				let features = self.bolt12_invoice_features();
-				if invoice.invoice_features().requires_unknown_bits_from(&features) {
-					log_trace!(
-						logger, "Invoice requires unknown features: {:?}",
-						invoice.invoice_features(),
-					);
-					self.abandon_payment_with_reason(
-						payment_id, PaymentFailureReason::UnknownRequiredFeatures,
-					);
-
-					let error = InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures);
-					let response = match responder {
-						Some(responder) => responder.respond(OffersMessage::InvoiceError(error)),
-						None => {
-							log_trace!(logger, "No reply path to send error: {:?}", error);
-							ResponseInstruction::NoResponse
-						},
-					};
-					return response;
-				}
-
 				if self.default_configuration.manually_handle_bolt12_invoices {
 					let event = Event::InvoiceReceived {
 						payment_id, invoice, context, responder,
@@ -10874,24 +10854,31 @@ where
 					return ResponseInstruction::NoResponse;
 				}
 
-				match self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id) {
-					// Payments with SendingFailed error will already have been abandoned.
+				let error = match self.send_payment_for_verified_bolt12_invoice(
+					&invoice, payment_id,
+				) {
+					Err(Bolt12PaymentError::UnknownRequiredFeatures) => {
+						log_trace!(
+							logger, "Invoice requires unknown features: {:?}",
+							invoice.invoice_features()
+						);
+						InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures)
+					},
 					Err(Bolt12PaymentError::SendingFailed(e)) => {
 						log_trace!(logger, "Failed paying invoice: {:?}", e);
-
-						let err = InvoiceError::from_string(format!("{:?}", e));
-						match responder {
-							Some(responder) => responder.respond(OffersMessage::InvoiceError(err)),
-							None => {
-								log_trace!(logger, "No reply path to send error: {:?}", err);
-								ResponseInstruction::NoResponse
-							},
-						}
+						InvoiceError::from_string(format!("{:?}", e))
 					},
-					// Otherwise, don't abandon unknown, pending, or successful payments.
 					Err(Bolt12PaymentError::UnexpectedInvoice)
 						| Err(Bolt12PaymentError::DuplicateInvoice)
-						| Ok(()) => ResponseInstruction::NoResponse,
+						| Ok(()) => return ResponseInstruction::NoResponse,
+				};
+
+				match responder {
+					Some(responder) => responder.respond(OffersMessage::InvoiceError(error)),
+					None => {
+						log_trace!(logger, "No reply path to send error: {:?}", error);
+						ResponseInstruction::NoResponse
+					},
 				}
 			},
 			#[cfg(async_payments)]
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 859b3467296..5cc31f60500 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -19,6 +19,7 @@ use crate::events::{self, PaymentFailureReason};
 use crate::ln::types::{PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::channelmanager::{EventCompletionAction, HTLCSource, PaymentId};
+use crate::ln::features::Bolt12InvoiceFeatures;
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason};
 use crate::offers::invoice::Bolt12Invoice;
@@ -510,6 +511,8 @@ pub enum Bolt12PaymentError {
 	UnexpectedInvoice,
 	/// Payment for an invoice with the corresponding [`PaymentId`] was already initiated.
 	DuplicateInvoice,
+	/// 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.
 	SendingFailed(RetryableSendFailure),
 }
@@ -783,9 +786,9 @@ impl OutboundPayments {
 		R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref
 	>(
 		&self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R,
-		first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
-		node_id_lookup: &NL, secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32,
-		logger: &L,
+		first_hops: Vec<ChannelDetails>, features: Bolt12InvoiceFeatures, inflight_htlcs: IH,
+		entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
+		secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
 		pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
 		send_payment_along_path: SP,
 	) -> Result<(), Bolt12PaymentError>
@@ -819,6 +822,13 @@ impl OutboundPayments {
 			hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
 		}
 
+		if invoice.invoice_features().requires_unknown_bits_from(&features) {
+			self.abandon_payment(
+				payment_id, PaymentFailureReason::UnknownRequiredFeatures, pending_events,
+			);
+			return Err(Bolt12PaymentError::UnknownRequiredFeatures);
+		}
+
 		let mut payment_params = PaymentParameters::from_bolt12_invoice(&invoice);
 
 		// Advance any blinded path where the introduction node is our node.
@@ -1951,7 +1961,7 @@ mod tests {
 	use crate::events::{Event, PathFailure, PaymentFailureReason};
 	use crate::ln::types::PaymentHash;
 	use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
-	use crate::ln::features::{ChannelFeatures, NodeFeatures};
+	use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, NodeFeatures};
 	use crate::ln::msgs::{ErrorAction, LightningError};
 	use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure, StaleExpiration};
 	#[cfg(feature = "std")]
@@ -2319,9 +2329,9 @@ mod tests {
 
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(
-				&invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager,
-				&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
-				|_| panic!()
+				&invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(),
+				|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {},
+				&secp_ctx, 0, &&logger, &pending_events, |_| panic!()
 			),
 			Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)),
 		);
@@ -2380,9 +2390,9 @@ mod tests {
 
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(
-				&invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager,
-				&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
-				|_| panic!()
+				&invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(),
+				|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {},
+				&secp_ctx, 0, &&logger, &pending_events, |_| panic!()
 			),
 			Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::RouteNotFound)),
 		);
@@ -2454,9 +2464,9 @@ mod tests {
 		assert!(!outbound_payments.has_pending_payments());
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(
-				&invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager,
-				&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
-				|_| panic!()
+				&invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(),
+				|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {},
+				&secp_ctx, 0, &&logger, &pending_events, |_| panic!()
 			),
 			Err(Bolt12PaymentError::UnexpectedInvoice),
 		);
@@ -2472,9 +2482,9 @@ mod tests {
 
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(
-				&invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager,
-				&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
-				|_| Ok(())
+				&invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(),
+				|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {},
+				&secp_ctx, 0, &&logger, &pending_events, |_| Ok(())
 			),
 			Ok(()),
 		);
@@ -2483,9 +2493,9 @@ mod tests {
 
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(
-				&invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager,
-				&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
-				|_| panic!()
+				&invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(),
+				|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {},
+				&secp_ctx, 0, &&logger, &pending_events, |_| panic!()
 			),
 			Err(Bolt12PaymentError::DuplicateInvoice),
 		);

From 05db67b5c3200adde275d562607da095e41eb8a4 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 31 Jul 2024 15:12:40 -0500
Subject: [PATCH 09/19] Use a smaller bit for UnknownFeature

When testing Bolt12Invoice unknown require feature handling, a large
feature bit can cause SendError::TooBigPacket when creating an onion
message. Use a smaller feature bit for UnknownFeature, which also has
the added benefit of reducing test output.
---
 lightning-types/src/features.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs
index 3aec5358d2b..54587dc980a 100644
--- a/lightning-types/src/features.rs
+++ b/lightning-types/src/features.rs
@@ -567,7 +567,7 @@ mod sealed {
 
 	#[cfg(any(test, feature = "_test_utils"))]
 	define_feature!(
-		123456789,
+		12345,
 		UnknownFeature,
 		[
 			NodeContext,
@@ -1117,7 +1117,7 @@ mod tests {
 		features.set_unknown_feature_required();
 		assert!(features.requires_unknown_bits());
 		assert!(features.supports_unknown_bits());
-		assert_eq!(features.required_unknown_bits_from(&ChannelFeatures::empty()), vec![123456788]);
+		assert_eq!(features.required_unknown_bits_from(&ChannelFeatures::empty()), vec![12344]);
 
 		let mut features = ChannelFeatures::empty();
 		features.set_unknown_feature_optional();
@@ -1127,17 +1127,17 @@ mod tests {
 
 		let mut features = ChannelFeatures::empty();
 		features.set_unknown_feature_required();
-		features.set_custom_bit(123456786).unwrap();
+		features.set_custom_bit(12346).unwrap();
 		assert!(features.requires_unknown_bits());
 		assert!(features.supports_unknown_bits());
 		assert_eq!(
 			features.required_unknown_bits_from(&ChannelFeatures::empty()),
-			vec![123456786, 123456788]
+			vec![12344, 12346]
 		);
 
 		let mut limiter = ChannelFeatures::empty();
 		limiter.set_unknown_feature_optional();
-		assert_eq!(features.required_unknown_bits_from(&limiter), vec![123456786]);
+		assert_eq!(features.required_unknown_bits_from(&limiter), vec![12346]);
 	}
 
 	#[test]

From 19dec4b36fb38825565d8cb8ebd4b6bb1ba081a1 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 31 Jul 2024 15:57:14 -0500
Subject: [PATCH 10/19] Add InvoiceBuilder::features_unchecked

In order to test handling of unknown required features in a
Bolt12Invoice, add a test-only function to allow setting arbitrary
feature bits.
---
 lightning/src/offers/invoice.rs        |  7 +++++++
 lightning/src/offers/invoice_macros.rs | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs
index 835a077f294..5a9cb349ce1 100644
--- a/lightning/src/offers/invoice.rs
+++ b/lightning/src/offers/invoice.rs
@@ -117,6 +117,8 @@ use crate::ln::features::{BlindedHopFeatures, Bolt12InvoiceFeatures, InvoiceRequ
 use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
 use crate::ln::msgs::DecodeError;
 use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_methods_common};
+#[cfg(test)]
+use crate::offers::invoice_macros::invoice_builder_methods_test;
 use crate::offers::invoice_request::{INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
 use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, WithoutSignatures, self};
 use crate::offers::nonce::Nonce;
@@ -385,6 +387,9 @@ impl<'a> InvoiceBuilder<'a, DerivedSigningPubkey> {
 impl<'a, S: SigningPubkeyStrategy> InvoiceBuilder<'a, S> {
 	invoice_builder_methods!(self, Self, Self, self, S, mut);
 	invoice_builder_methods_common!(self, Self, self.invoice.fields_mut(), Self, self, S, Bolt12Invoice, mut);
+
+	#[cfg(test)]
+	invoice_builder_methods_test!(self, Self, self.invoice.fields_mut(), Self, self, mut);
 }
 
 #[cfg(all(c_bindings, not(test)))]
@@ -399,6 +404,7 @@ impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> {
 	invoice_explicit_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, &mut Self, self, ExplicitSigningPubkey);
 	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, ExplicitSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self);
 }
 
 #[cfg(all(c_bindings, not(test)))]
@@ -413,6 +419,7 @@ impl<'a> InvoiceWithDerivedSigningPubkeyBuilder<'a> {
 	invoice_derived_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, &mut Self, self, DerivedSigningPubkey);
 	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, DerivedSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self);
 }
 
 #[cfg(c_bindings)]
diff --git a/lightning/src/offers/invoice_macros.rs b/lightning/src/offers/invoice_macros.rs
index b79bb8c9e88..e1875fbc87f 100644
--- a/lightning/src/offers/invoice_macros.rs
+++ b/lightning/src/offers/invoice_macros.rs
@@ -82,6 +82,21 @@ macro_rules! invoice_builder_methods_common { (
 	}
 } }
 
+#[cfg(test)]
+macro_rules! invoice_builder_methods_test { (
+	$self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr
+	$(, $self_mut: tt)?
+) => {
+	#[cfg_attr(c_bindings, allow(dead_code))]
+	pub(crate) fn features_unchecked(
+		$($self_mut)* $self: $self_type, features: Bolt12InvoiceFeatures
+	) -> $return_type {
+		$invoice_fields.features = features;
+		$return_value
+	}
+
+} }
+
 macro_rules! invoice_accessors_common { ($self: ident, $contents: expr, $invoice_type: ty) => {
 	/// Paths to the recipient originating from publicly reachable nodes, including information
 	/// needed for routing payments across them.
@@ -133,3 +148,5 @@ macro_rules! invoice_accessors_common { ($self: ident, $contents: expr, $invoice
 
 pub(super) use invoice_accessors_common;
 pub(super) use invoice_builder_methods_common;
+#[cfg(test)]
+pub(super) use invoice_builder_methods_test;

From 16bbb7b3dc89d6769bb85fbbc4f9822aa7711af8 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 31 Jul 2024 16:08:20 -0500
Subject: [PATCH 11/19] Remove unused macro param

---
 lightning/src/offers/invoice.rs        | 10 +++++-----
 lightning/src/offers/invoice_macros.rs |  2 +-
 lightning/src/offers/static_invoice.rs |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs
index 5a9cb349ce1..3fd2e73aa5b 100644
--- a/lightning/src/offers/invoice.rs
+++ b/lightning/src/offers/invoice.rs
@@ -386,7 +386,7 @@ impl<'a> InvoiceBuilder<'a, DerivedSigningPubkey> {
 
 impl<'a, S: SigningPubkeyStrategy> InvoiceBuilder<'a, S> {
 	invoice_builder_methods!(self, Self, Self, self, S, mut);
-	invoice_builder_methods_common!(self, Self, self.invoice.fields_mut(), Self, self, S, Bolt12Invoice, mut);
+	invoice_builder_methods_common!(self, Self, self.invoice.fields_mut(), Self, self, Bolt12Invoice, mut);
 
 	#[cfg(test)]
 	invoice_builder_methods_test!(self, Self, self.invoice.fields_mut(), Self, self, mut);
@@ -396,14 +396,14 @@ impl<'a, S: SigningPubkeyStrategy> InvoiceBuilder<'a, S> {
 impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> {
 	invoice_explicit_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, (), (), ExplicitSigningPubkey);
-	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), (), (), ExplicitSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), (), (), Bolt12Invoice);
 }
 
 #[cfg(all(c_bindings, test))]
 impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> {
 	invoice_explicit_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, &mut Self, self, ExplicitSigningPubkey);
-	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, ExplicitSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, Bolt12Invoice);
 	invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self);
 }
 
@@ -411,14 +411,14 @@ impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> {
 impl<'a> InvoiceWithDerivedSigningPubkeyBuilder<'a> {
 	invoice_derived_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, (), (), DerivedSigningPubkey);
-	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), (), (), DerivedSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), (), (), Bolt12Invoice);
 }
 
 #[cfg(all(c_bindings, test))]
 impl<'a> InvoiceWithDerivedSigningPubkeyBuilder<'a> {
 	invoice_derived_signing_pubkey_builder_methods!(self, &mut Self);
 	invoice_builder_methods!(self, &mut Self, &mut Self, self, DerivedSigningPubkey);
-	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, DerivedSigningPubkey, Bolt12Invoice);
+	invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, Bolt12Invoice);
 	invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self);
 }
 
diff --git a/lightning/src/offers/invoice_macros.rs b/lightning/src/offers/invoice_macros.rs
index e1875fbc87f..c603c352ce9 100644
--- a/lightning/src/offers/invoice_macros.rs
+++ b/lightning/src/offers/invoice_macros.rs
@@ -11,7 +11,7 @@
 
 macro_rules! invoice_builder_methods_common { (
 	$self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr,
-	$type_param: ty, $invoice_type: ty $(, $self_mut: tt)?
+	$invoice_type: ty $(, $self_mut: tt)?
 ) => {
 	#[doc = concat!("Sets the [`", stringify!($invoice_type), "::relative_expiry`]")]
 	#[doc = concat!("as seconds since [`", stringify!($invoice_type), "::created_at`].")]
diff --git a/lightning/src/offers/static_invoice.rs b/lightning/src/offers/static_invoice.rs
index 614903bdfcc..c91496e17a1 100644
--- a/lightning/src/offers/static_invoice.rs
+++ b/lightning/src/offers/static_invoice.rs
@@ -156,7 +156,7 @@ impl<'a> StaticInvoiceBuilder<'a> {
 		Ok(invoice)
 	}
 
-	invoice_builder_methods_common!(self, Self, self.invoice, Self, self, S, StaticInvoice, mut);
+	invoice_builder_methods_common!(self, Self, self.invoice, Self, self, StaticInvoice, mut);
 }
 
 /// A semantically valid [`StaticInvoice`] that hasn't been signed.

From 4567ce501a2f7c07670a0dbe6d3bd25a7ca5af1b Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 31 Jul 2024 17:04:39 -0500
Subject: [PATCH 12/19] Test Bolt12Invoice with unknown required features

---
 lightning/src/ln/offers_tests.rs | 125 ++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 3 deletions(-)

diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index e774bc08162..ee0f731b23f 100644
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -41,20 +41,24 @@
 //! blinded paths are used.
 
 use bitcoin::network::Network;
-use bitcoin::secp256k1::PublicKey;
+use bitcoin::secp256k1::{PublicKey, Secp256k1};
 use core::time::Duration;
 use crate::blinded_path::{BlindedPath, IntroductionNode};
 use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext};
-use crate::events::{Event, MessageSendEventsProvider, PaymentPurpose};
+use crate::blinded_path::message::{MessageContext, OffersContext};
+use crate::events::{Event, MessageSendEventsProvider, PaymentFailureReason, PaymentPurpose};
 use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self};
+use crate::ln::features::Bolt12InvoiceFeatures;
 use crate::ln::functional_test_utils::*;
+use crate::ln::inbound_payment::ExpandedKey;
 use crate::ln::msgs::{ChannelMessageHandler, Init, NodeAnnouncement, OnionMessage, OnionMessageHandler, RoutingMessageHandler, SocketAddress, UnsignedGossipMessage, UnsignedNodeAnnouncement};
 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::nonce::Nonce;
 use crate::offers::parse::Bolt12SemanticError;
-use crate::onion_message::messenger::{Destination, PeeledOnion};
+use crate::onion_message::messenger::{Destination, PeeledOnion, new_pending_onion_message};
 use crate::onion_message::offers::OffersMessage;
 use crate::onion_message::packet::ParsedOnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
@@ -184,6 +188,15 @@ fn claim_bolt12_payment<'a, 'b, 'c>(
 	claim_payment(node, path, payment_preimage);
 }
 
+fn extract_offer_nonce<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> Nonce {
+	match node.onion_messenger.peel_onion_message(message) {
+		Ok(PeeledOnion::Receive(_, Some(MessageContext::Offers(OffersContext::InvoiceRequest { nonce })), _)) => nonce,
+		Ok(PeeledOnion::Receive(_, context, _)) => panic!("Unexpected onion message context: {:?}", context),
+		Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"),
+		Err(e) => panic!("Failed to process onion message {:?}", e),
+	}
+}
+
 fn extract_invoice_request<'a, 'b, 'c>(
 	node: &Node<'a, 'b, 'c>, message: &OnionMessage
 ) -> (InvoiceRequest, BlindedPath) {
@@ -2059,3 +2072,109 @@ fn fails_paying_invoice_more_than_once() {
 	claim_bolt12_payment(david, &[charlie, bob, alice], payment_context);
 	expect_recent_payment!(david, RecentPaymentDetails::Fulfilled, payment_id);
 }
+
+#[test]
+fn fails_paying_invoice_with_unknown_required_features() {
+	let mut accept_forward_cfg = test_default_channel_config();
+	accept_forward_cfg.accept_forwards_to_priv_channels = true;
+
+	// Clearing route_blinding prevents forming any payment paths since the node is unannounced.
+	let mut features = channelmanager::provided_init_features(&accept_forward_cfg);
+	features.set_onion_messages_optional();
+	features.set_route_blinding_optional();
+
+	let chanmon_cfgs = create_chanmon_cfgs(6);
+	let node_cfgs = create_node_cfgs(6, &chanmon_cfgs);
+
+	*node_cfgs[1].override_init_features.borrow_mut() = Some(features);
+
+	let node_chanmgrs = create_node_chanmgrs(
+		6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None]
+	);
+	let nodes = create_network(6, &node_cfgs, &node_chanmgrs);
+
+	create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000);
+	create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000);
+
+	let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]);
+	let alice_id = alice.node.get_our_node_id();
+	let bob_id = bob.node.get_our_node_id();
+	let charlie_id = charlie.node.get_our_node_id();
+	let david_id = david.node.get_our_node_id();
+
+	disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]);
+	disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]);
+
+	let offer = alice.node
+		.create_offer_builder(None).unwrap()
+		.amount_msats(10_000_000)
+		.build().unwrap();
+
+	let payment_id = PaymentId([1; 32]);
+	david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None)
+		.unwrap();
+
+	connect_peers(david, bob);
+
+	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
+	bob.onion_messenger.handle_onion_message(&david_id, &onion_message);
+
+	connect_peers(alice, charlie);
+
+	let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
+	alice.onion_messenger.handle_onion_message(&bob_id, &onion_message);
+
+	let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
+	let nonce = extract_offer_nonce(alice, &onion_message);
+
+	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
+	charlie.onion_messenger.handle_onion_message(&alice_id, &onion_message);
+
+	// Drop the invoice in favor for one with unknown required features.
+	let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap();
+	let (invoice, _) = extract_invoice(david, &onion_message);
+
+	let payment_paths = invoice.payment_paths().to_vec();
+	let payment_hash = invoice.payment_hash();
+
+	let expanded_key = ExpandedKey::new(&alice.keys_manager.get_inbound_payment_key_material());
+	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();
+
+	// Enqueue an onion message containing the new invoice.
+	let pending_message = new_pending_onion_message(
+		OffersMessage::Invoice(invoice), Destination::BlindedPath(reply_path), None
+	);
+	alice.node.pending_offers_messages.lock().unwrap().push(pending_message);
+
+	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
+	charlie.onion_messenger.handle_onion_message(&alice_id, &onion_message);
+
+	let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap();
+	david.onion_messenger.handle_onion_message(&charlie_id, &onion_message);
+
+	// Confirm that david drops this failed payment from his pending outbound payments.
+	match get_event!(david, Event::PaymentFailed) {
+		Event::PaymentFailed {
+			payment_id: event_payment_id,
+			payment_hash: event_payment_hash,
+			reason: Some(event_reason),
+		} => {
+			assert_eq!(event_payment_id, payment_id);
+			assert_eq!(event_payment_hash, payment_hash);
+			assert_eq!(event_reason, PaymentFailureReason::UnknownRequiredFeatures);
+		},
+		_ => panic!("Expected Event::PaymentFailed with reason"),
+	}
+}

From 14153edeed100bc311bd1eca95ac4717c4d6583c Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Tue, 6 Aug 2024 17:17:52 -0500
Subject: [PATCH 13/19] Make payment_hash optional in Event::PaymentFailed

When abandoning a BOLT12 payment before a Bolt12Invoice is received, an
Event::InvoiceRequestFailed is generated and the abandonment reason is
lost. Make payment_hash optional in Event::PaymentFailed so that
Event::InvoiceRequestFailed can be removed in favor of it.
---
 lightning/src/events/mod.rs                   | 17 +++++++++++++++--
 lightning/src/ln/blinded_payment_tests.rs     |  2 +-
 lightning/src/ln/chanmon_update_fail_tests.rs |  2 +-
 lightning/src/ln/channelmanager.rs            |  3 ++-
 lightning/src/ln/functional_test_utils.rs     |  4 ++--
 lightning/src/ln/functional_tests.rs          | 18 +++++++++---------
 lightning/src/ln/offers_tests.rs              |  2 +-
 lightning/src/ln/onion_route_tests.rs         |  2 +-
 lightning/src/ln/outbound_payment.rs          | 12 ++++++------
 lightning/src/ln/payment_tests.rs             | 10 +++++-----
 10 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 393acbc0242..a94d6c118ce 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -879,10 +879,12 @@ pub enum Event {
 		///
 		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 		payment_id: PaymentId,
-		/// The hash that was given to [`ChannelManager::send_payment`].
+		/// The hash that was given to [`ChannelManager::send_payment`]. `None` if the payment failed
+		/// before receiving an invoice when paying a BOLT12 [`Offer`].
 		///
 		/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
-		payment_hash: PaymentHash,
+		/// [`Offer`]: crate::offers::offer::Offer
+		payment_hash: Option<PaymentHash>,
 		/// The reason the payment failed. This is only `None` for events generated or serialized
 		/// by versions prior to 0.0.115.
 		reason: Option<PaymentFailureReason>,
@@ -1555,10 +1557,15 @@ impl Writeable for Event {
 			},
 			&Event::PaymentFailed { ref payment_id, ref payment_hash, ref reason } => {
 				15u8.write(writer)?;
+				let (payment_hash, invoice_received) = match payment_hash {
+					Some(payment_hash) => (payment_hash, true),
+					None => (&PaymentHash([0; 32]), false),
+				};
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
 					(1, reason, option),
 					(2, payment_hash, required),
+					(3, invoice_received, required),
 				})
 			},
 			&Event::OpenChannelRequest { .. } => {
@@ -1932,11 +1939,17 @@ impl MaybeReadable for Event {
 					let mut payment_hash = PaymentHash([0; 32]);
 					let mut payment_id = PaymentId([0; 32]);
 					let mut reason = None;
+					let mut invoice_received: Option<bool> = None;
 					read_tlv_fields!(reader, {
 						(0, payment_id, required),
 						(1, reason, upgradable_option),
 						(2, payment_hash, required),
+						(3, invoice_received, option),
 					});
+					let payment_hash = match invoice_received {
+						Some(invoice_received) => invoice_received.then(|| payment_hash),
+						None => (payment_hash != PaymentHash([0; 32])).then(|| payment_hash),
+					};
 					Ok(Some(Event::PaymentFailed {
 						payment_id,
 						payment_hash,
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index c1d65fd1a11..9d246f1a741 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -1068,7 +1068,7 @@ fn blinded_path_retries() {
 	assert_eq!(evs.len(), 1);
 	match evs[0] {
 		Event::PaymentFailed { payment_hash: ev_payment_hash, reason, .. } => {
-			assert_eq!(ev_payment_hash, payment_hash);
+			assert_eq!(ev_payment_hash, Some(payment_hash));
 			// We have 1 retry attempt remaining, but we're out of blinded paths to try.
 			assert_eq!(reason, Some(PaymentFailureReason::RouteNotFound));
 		},
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 9bc48f12f0a..52669637d00 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -1776,7 +1776,7 @@ fn test_monitor_update_on_pending_forwards() {
 	} else { panic!("Unexpected event!"); }
 	match events[2] {
 		Event::PaymentFailed { payment_hash, .. } => {
-			assert_eq!(payment_hash, payment_hash_1);
+			assert_eq!(payment_hash, Some(payment_hash_1));
 		},
 		_ => panic!("Unexpected event"),
 	}
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 617c58c2e48..7b903cd02b9 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1681,7 +1681,8 @@ where
 /// channel_manager.process_pending_events(&|event| {
 ///     match event {
 ///         Event::PaymentSent { payment_hash, .. } => println!("Paid {}", payment_hash),
-///         Event::PaymentFailed { payment_hash, .. } => println!("Failed paying {}", payment_hash),
+///         Event::PaymentFailed { payment_hash: Some(payment_hash), .. } =>
+///             println!("Failed paying {}", payment_hash),
 ///         // ...
 ///     #     _ => {},
 ///     }
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 5f1b7123d5a..2845ddd9a41 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -2520,7 +2520,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 	if !conditions.expected_mpp_parts_remain {
 		match &payment_failed_events[1] {
 			Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
-				assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
+				assert_eq!(*payment_hash, Some(expected_payment_hash), "unexpected second payment_hash");
 				assert_eq!(*payment_id, expected_payment_id);
 				assert_eq!(reason.unwrap(), if expected_payment_failed_permanently {
 					PaymentFailureReason::RecipientRejected
@@ -3162,7 +3162,7 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
 			if i == expected_paths.len() - 1 {
 				match events[1] {
 					Event::PaymentFailed { ref payment_hash, ref payment_id, ref reason } => {
-						assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
+						assert_eq!(*payment_hash, Some(our_payment_hash), "unexpected second payment_hash");
 						assert_eq!(*payment_id, expected_payment_id);
 						assert_eq!(reason.unwrap(), expected_fail_reason);
 					}
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 99063334a28..617df3ce7cc 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -3380,7 +3380,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 	)));
 	assert!(events.iter().any(|ev| matches!(
 		ev,
-		Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash
+		Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == Some(fourth_payment_hash)
 	)));
 
 	nodes[1].node.process_pending_htlc_forwards();
@@ -3442,7 +3442,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 			}
 			match events[1] {
 				Event::PaymentFailed { ref payment_hash, .. } => {
-					assert_eq!(*payment_hash, first_payment_hash);
+					assert_eq!(*payment_hash, Some(first_payment_hash));
 				},
 				_ => panic!("Unexpected event"),
 			}
@@ -3454,7 +3454,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 			}
 			match events[3] {
 				Event::PaymentFailed { ref payment_hash, .. } => {
-					assert_eq!(*payment_hash, second_payment_hash);
+					assert_eq!(*payment_hash, Some(second_payment_hash));
 				},
 				_ => panic!("Unexpected event"),
 			}
@@ -3466,7 +3466,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 			}
 			match events[5] {
 				Event::PaymentFailed { ref payment_hash, .. } => {
-					assert_eq!(*payment_hash, third_payment_hash);
+					assert_eq!(*payment_hash, Some(third_payment_hash));
 				},
 				_ => panic!("Unexpected event"),
 			}
@@ -3572,7 +3572,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
 	}
 	match events[1] {
 		Event::PaymentFailed { payment_hash, .. } => {
-			assert_eq!(payment_hash, failed_payment_hash);
+			assert_eq!(payment_hash, Some(failed_payment_hash));
 		},
 		_ => panic!("Unexpected event"),
 	}
@@ -3851,7 +3851,7 @@ fn test_simple_peer_disconnect() {
 		}
 		match events[3] {
 			Event::PaymentFailed { payment_hash, .. } => {
-				assert_eq!(payment_hash, payment_hash_5);
+				assert_eq!(payment_hash, Some(payment_hash_5));
 			},
 			_ => panic!("Unexpected event"),
 		}
@@ -6007,7 +6007,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
 	}
 	match &events[1] {
 		&Event::PaymentFailed { ref payment_hash, .. } => {
-			assert_eq!(our_payment_hash.clone(), *payment_hash);
+			assert_eq!(Some(our_payment_hash), *payment_hash);
 		},
 		_ => panic!("Unexpected event"),
 	}
@@ -6095,7 +6095,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
 	}
 	match &events[1] {
 		&Event::PaymentFailed { ref payment_hash, .. } => {
-			assert_eq!(payment_hash_2.clone(), *payment_hash);
+			assert_eq!(Some(payment_hash_2), *payment_hash);
 		},
 		_ => panic!("Unexpected event"),
 	}
@@ -7048,7 +7048,7 @@ fn test_channel_failed_after_message_with_badonion_node_perm_bits_set() {
 	}
 	match events_5[1] {
 		Event::PaymentFailed { payment_hash, .. } => {
-			assert_eq!(payment_hash, our_payment_hash);
+			assert_eq!(payment_hash, Some(our_payment_hash));
 		},
 		_ => panic!("Unexpected event"),
 	}
diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index ee0f731b23f..5b81251f6c1 100644
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -2168,7 +2168,7 @@ fn fails_paying_invoice_with_unknown_required_features() {
 	match get_event!(david, Event::PaymentFailed) {
 		Event::PaymentFailed {
 			payment_id: event_payment_id,
-			payment_hash: event_payment_hash,
+			payment_hash: Some(event_payment_hash),
 			reason: Some(event_reason),
 		} => {
 			assert_eq!(event_payment_id, payment_id);
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index bd28f96988f..94c2cfd2ee4 100644
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -215,7 +215,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(
 	}
 	match events[1] {
 		Event::PaymentFailed { payment_hash: ev_payment_hash, payment_id: ev_payment_id, reason: ref ev_reason } => {
-			assert_eq!(*payment_hash, ev_payment_hash);
+			assert_eq!(Some(*payment_hash), ev_payment_hash);
 			assert_eq!(payment_id, ev_payment_id);
 			assert_eq!(if expected_retryable {
 				PaymentFailureReason::RetriesExhausted
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 5cc31f60500..c04e186f1d3 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -961,7 +961,7 @@ impl OutboundPayments {
 				if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = pmt {
 					pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
 						payment_id: *pmt_id,
-						payment_hash: *payment_hash,
+						payment_hash: Some(*payment_hash),
 						reason: *reason,
 					}, None));
 					retain = false;
@@ -1127,7 +1127,7 @@ impl OutboundPayments {
 					if $payment.get().remaining_parts() == 0 {
 						pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
 							payment_id,
-							payment_hash,
+							payment_hash: Some(payment_hash),
 							reason: *reason,
 						}, None));
 						$payment.remove();
@@ -1795,7 +1795,7 @@ impl OutboundPayments {
 					if !payment_is_probe {
 						full_failure_ev = Some(events::Event::PaymentFailed {
 							payment_id: *payment_id,
-							payment_hash: *payment_hash,
+							payment_hash: Some(*payment_hash),
 							reason: *reason,
 						});
 					}
@@ -1864,7 +1864,7 @@ impl OutboundPayments {
 				if payment.get().remaining_parts() == 0 {
 					pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
 						payment_id,
-						payment_hash: *payment_hash,
+						payment_hash: Some(*payment_hash),
 						reason: *reason,
 					}, None));
 					payment.remove();
@@ -2337,7 +2337,7 @@ mod tests {
 		);
 		assert!(!outbound_payments.has_pending_payments());
 
-		let payment_hash = invoice.payment_hash();
+		let payment_hash = Some(invoice.payment_hash());
 		let reason = Some(PaymentFailureReason::PaymentExpired);
 
 		assert!(!pending_events.lock().unwrap().is_empty());
@@ -2398,7 +2398,7 @@ mod tests {
 		);
 		assert!(!outbound_payments.has_pending_payments());
 
-		let payment_hash = invoice.payment_hash();
+		let payment_hash = Some(invoice.payment_hash());
 		let reason = Some(PaymentFailureReason::RouteNotFound);
 
 		assert!(!pending_events.lock().unwrap().is_empty());
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 3ab055d2ada..083c32e5393 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2268,7 +2268,7 @@ fn do_automatic_retries(test: AutoRetry) {
 			} else {
 				match events[1] {
 					Event::PaymentFailed { payment_hash: ev_payment_hash, .. } => {
-						assert_eq!(payment_hash, ev_payment_hash);
+						assert_eq!(Some(payment_hash), ev_payment_hash);
 					},
 					_ => panic!("Unexpected event"),
 				}
@@ -2351,7 +2351,7 @@ fn do_automatic_retries(test: AutoRetry) {
 			assert_eq!(events.len(), 1);
 			match events[0] {
 				Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
-					assert_eq!(payment_hash, *ev_payment_hash);
+					assert_eq!(Some(payment_hash), *ev_payment_hash);
 					assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
 					assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
 				},
@@ -2388,7 +2388,7 @@ fn do_automatic_retries(test: AutoRetry) {
 		assert_eq!(events.len(), 1);
 		match events[0] {
 			Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
-				assert_eq!(payment_hash, *ev_payment_hash);
+				assert_eq!(Some(payment_hash), *ev_payment_hash);
 				assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
 				assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
 			},
@@ -2409,7 +2409,7 @@ fn do_automatic_retries(test: AutoRetry) {
 		assert_eq!(events.len(), 1);
 		match events[0] {
 			Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
-				assert_eq!(payment_hash, *ev_payment_hash);
+				assert_eq!(Some(payment_hash), *ev_payment_hash);
 				assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
 				assert_eq!(PaymentFailureReason::RouteNotFound, ev_reason.unwrap());
 			},
@@ -3099,7 +3099,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 	}
 	match events[1] {
 		Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id, reason: ref ev_reason } => {
-			assert_eq!(payment_hash, *ev_payment_hash);
+			assert_eq!(Some(payment_hash), *ev_payment_hash);
 			assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
 			assert_eq!(PaymentFailureReason::RetriesExhausted, ev_reason.unwrap());
 		},

From a9e63630639fd5c7968fe1268bf7b08a6eb81b27 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Tue, 6 Aug 2024 17:51:16 -0500
Subject: [PATCH 14/19] Remove Event::InvoiceRequestFailed

Now that Event::PaymentFailed has an option payment_hash, it can be used
in replace of Event::InvoiceRequestFailed. This allows for including a
reason when abandoning a payment before an invoice is received.
---
 lightning/src/events/mod.rs          | 30 ++++++----------------------
 lightning/src/ln/channelmanager.rs   | 16 ++++++---------
 lightning/src/ln/offers_tests.rs     | 14 ++++++-------
 lightning/src/ln/outbound_payment.rs | 21 ++++++++++++-------
 4 files changed, 33 insertions(+), 48 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index a94d6c118ce..780713b331b 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -772,22 +772,6 @@ pub enum Event {
 		/// Sockets for connecting to the node.
 		addresses: Vec<msgs::SocketAddress>,
 	},
-	/// Indicates a request for an invoice failed to yield a response in a reasonable amount of time
-	/// or was explicitly abandoned by [`ChannelManager::abandon_payment`]. This may be for an
-	/// [`InvoiceRequest`] sent for an [`Offer`] or for a [`Refund`] that hasn't been redeemed.
-	///
-	/// # Failure Behavior and Persistence
-	/// This event will eventually be replayed after failures-to-handle (i.e., the event handler
-	/// returning `Err(ReplayEvent ())`) and will be persisted across restarts.
-	///
-	/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
-	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-	/// [`Offer`]: crate::offers::offer::Offer
-	/// [`Refund`]: crate::offers::refund::Refund
-	InvoiceRequestFailed {
-		/// The `payment_id` to have been associated with payment for the requested invoice.
-		payment_id: PaymentId,
-	},
 	/// Indicates a [`Bolt12Invoice`] in response to an [`InvoiceRequest`] or a [`Refund`] was
 	/// received.
 	///
@@ -886,7 +870,8 @@ pub enum Event {
 		/// [`Offer`]: crate::offers::offer::Offer
 		payment_hash: Option<PaymentHash>,
 		/// The reason the payment failed. This is only `None` for events generated or serialized
-		/// by versions prior to 0.0.115.
+		/// by versions prior to 0.0.115 or when deserializing an `Event::InvoiceRequestFailed`,
+		/// which was removed in 0.0.124.
 		reason: Option<PaymentFailureReason>,
 	},
 	/// Indicates that a path for an outbound payment was successful.
@@ -1644,12 +1629,6 @@ impl Writeable for Event {
 					(8, funding_txo, required),
 				});
 			},
-			&Event::InvoiceRequestFailed { ref payment_id } => {
-				33u8.write(writer)?;
-				write_tlv_fields!(writer, {
-					(0, payment_id, required),
-				})
-			},
 			&Event::ConnectionNeeded { .. } => {
 				35u8.write(writer)?;
 				// Never write ConnectionNeeded events as buffered onion messages aren't serialized.
@@ -2092,13 +2071,16 @@ impl MaybeReadable for Event {
 				};
 				f()
 			},
+			// This was Event::InvoiceRequestFailed prior to version 0.0.124.
 			33u8 => {
 				let mut f = || {
 					_init_and_read_len_prefixed_tlv_fields!(reader, {
 						(0, payment_id, required),
 					});
-					Ok(Some(Event::InvoiceRequestFailed {
+					Ok(Some(Event::PaymentFailed {
 						payment_id: payment_id.0.unwrap(),
+						payment_hash: None,
+						reason: None,
 					}))
 				};
 				f()
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7b903cd02b9..267fb245526 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1745,8 +1745,7 @@ where
 /// ```
 ///
 /// Use [`pay_for_offer`] to initiated payment, which sends an [`InvoiceRequest`] for an [`Offer`]
-/// and pays the [`Bolt12Invoice`] response. In addition to success and failure events,
-/// [`ChannelManager`] may also generate an [`Event::InvoiceRequestFailed`].
+/// and pays the [`Bolt12Invoice`] response.
 ///
 /// ```
 /// # use lightning::events::{Event, EventsProvider};
@@ -1788,7 +1787,6 @@ where
 ///     match event {
 ///         Event::PaymentSent { payment_id: Some(payment_id), .. } => println!("Paid {}", payment_id),
 ///         Event::PaymentFailed { payment_id, .. } => println!("Failed paying {}", payment_id),
-///         Event::InvoiceRequestFailed { payment_id, .. } => println!("Failed paying {}", payment_id),
 ///         // ...
 ///     #     _ => {},
 ///     }
@@ -4265,15 +4263,13 @@ where
 	/// # Requested Invoices
 	///
 	/// In the case of paying a [`Bolt12Invoice`] via [`ChannelManager::pay_for_offer`], abandoning
-	/// the payment prior to receiving the invoice will result in an [`Event::InvoiceRequestFailed`]
-	/// and prevent any attempts at paying it once received. The other events may only be generated
-	/// once the invoice has been received.
+	/// the payment prior to receiving the invoice will result in an [`Event::PaymentFailed`] and
+	/// prevent any attempts at paying it once received.
 	///
 	/// # Restart Behavior
 	///
 	/// If an [`Event::PaymentFailed`] is generated and we restart without first persisting the
-	/// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated; likewise for
-	/// [`Event::InvoiceRequestFailed`].
+	/// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated.
 	///
 	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 	pub fn abandon_payment(&self, payment_id: PaymentId) {
@@ -8853,7 +8849,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 	///
 	/// To revoke the refund, use [`ChannelManager::abandon_payment`] prior to receiving the
 	/// invoice. If abandoned, or an invoice isn't received before expiration, the payment will fail
-	/// with an [`Event::InvoiceRequestFailed`].
+	/// with an [`Event::PaymentFailed`].
 	///
 	/// If `max_total_routing_fee_msat` is not specified, The default from
 	/// [`RouteParameters::from_payment_params_and_value`] is applied.
@@ -8969,7 +8965,7 @@ where
 	///
 	/// To revoke the request, use [`ChannelManager::abandon_payment`] prior to receiving the
 	/// invoice. If abandoned, or an invoice isn't received in a reasonable amount of time, the
-	/// payment will fail with an [`Event::InvoiceRequestFailed`].
+	/// payment will fail with an [`Event::PaymentFailed`].
 	///
 	/// # Privacy
 	///
diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index 5b81251f6c1..b430196c4cf 100644
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -1332,7 +1332,7 @@ fn fails_authentication_when_handling_invoice_request() {
 	assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None);
 
 	david.node.abandon_payment(payment_id);
-	get_event!(david, Event::InvoiceRequestFailed);
+	get_event!(david, Event::PaymentFailed);
 
 	// Send the invoice request to Alice using an invalid blinded path.
 	let payment_id = PaymentId([2; 32]);
@@ -1419,7 +1419,7 @@ fn fails_authentication_when_handling_invoice_for_offer() {
 	david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None)
 		.unwrap();
 	david.node.abandon_payment(payment_id);
-	get_event!(david, Event::InvoiceRequestFailed);
+	get_event!(david, Event::PaymentFailed);
 
 	// Don't send the invoice request, but grab its reply path to use with a different request.
 	let invalid_reply_path = {
@@ -1547,7 +1547,7 @@ fn fails_authentication_when_handling_invoice_for_refund() {
 
 	expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id);
 	david.node.abandon_payment(payment_id);
-	get_event!(david, Event::InvoiceRequestFailed);
+	get_event!(david, Event::PaymentFailed);
 
 	// Send the invoice to David using an invalid blinded path.
 	let invalid_path = refund.paths().first().unwrap().clone();
@@ -1931,11 +1931,11 @@ fn fails_sending_invoice_without_blinded_payment_paths_for_offer() {
 	assert_eq!(invoice_error, InvoiceError::from(Bolt12SemanticError::MissingPaths));
 
 	// Confirm that david drops this failed payment from his pending outbound payments.
-	match get_event!(david, Event::InvoiceRequestFailed) {
-		Event::InvoiceRequestFailed { payment_id: pay_id } => {
-			assert_eq!(pay_id, payment_id)
+	match get_event!(david, Event::PaymentFailed) {
+		Event::PaymentFailed { payment_id: actual_payment_id, .. } => {
+			assert_eq!(payment_id, actual_payment_id);
 		},
-		_ => panic!("No Event::InvoiceRequestFailed"),
+		_ => panic!("No Event::PaymentFailed"),
 	}
 }
 
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index c04e186f1d3..6585dfbe67c 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1708,9 +1708,12 @@ impl OutboundPayments {
 					},
 				};
 				if is_stale {
-					pending_events.push_back(
-						(events::Event::InvoiceRequestFailed { payment_id: *payment_id }, None)
-					);
+					let event = events::Event::PaymentFailed {
+						payment_id: *payment_id,
+						payment_hash: None,
+						reason: None,
+					};
+					pending_events.push_back((event, None));
 					false
 				} else {
 					true
@@ -1870,8 +1873,10 @@ impl OutboundPayments {
 					payment.remove();
 				}
 			} else if let PendingOutboundPayment::AwaitingInvoice { .. } = payment.get() {
-				pending_events.lock().unwrap().push_back((events::Event::InvoiceRequestFailed {
+				pending_events.lock().unwrap().push_back((events::Event::PaymentFailed {
 					payment_id,
+					payment_hash: None,
+					reason: Some(reason),
 				}, None));
 				payment.remove();
 			}
@@ -2200,7 +2205,7 @@ mod tests {
 		assert!(!pending_events.lock().unwrap().is_empty());
 		assert_eq!(
 			pending_events.lock().unwrap().pop_front(),
-			Some((Event::InvoiceRequestFailed { payment_id }, None)),
+			Some((Event::PaymentFailed { payment_id, payment_hash: None, reason: None }, None)),
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 
@@ -2249,7 +2254,7 @@ mod tests {
 		assert!(!pending_events.lock().unwrap().is_empty());
 		assert_eq!(
 			pending_events.lock().unwrap().pop_front(),
-			Some((Event::InvoiceRequestFailed { payment_id }, None)),
+			Some((Event::PaymentFailed { payment_id, payment_hash: None, reason: None }, None)),
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 
@@ -2289,7 +2294,9 @@ mod tests {
 		assert!(!pending_events.lock().unwrap().is_empty());
 		assert_eq!(
 			pending_events.lock().unwrap().pop_front(),
-			Some((Event::InvoiceRequestFailed { payment_id }, None)),
+			Some((Event::PaymentFailed {
+				payment_id, payment_hash: None, reason: Some(PaymentFailureReason::UserAbandoned),
+			}, None)),
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 	}

From f00b782a1c0b07eee31a8f9101af0d271646f15c Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 7 Aug 2024 16:15:35 -0500
Subject: [PATCH 15/19] Add pending changelog for Event::PaymentFailed

---
 .../3192-invoice-request-failed-event.txt            | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 pending_changelog/3192-invoice-request-failed-event.txt

diff --git a/pending_changelog/3192-invoice-request-failed-event.txt b/pending_changelog/3192-invoice-request-failed-event.txt
new file mode 100644
index 00000000000..f1ab8df5bc9
--- /dev/null
+++ b/pending_changelog/3192-invoice-request-failed-event.txt
@@ -0,0 +1,12 @@
+## API Updates
+ * `Event::PaymentFailed` now uses an `Option` for its payment hash, which will
+   be `None` if the invoice hasn't been received yet for a BOLT12 payment
+   (#3192).
+ * `Event::PaymentFailed` is now generated instead `Event::InvoiceRequestFailed`,
+   which has been removed. When deserialized the latter will be converted to the
+   former with `None` for the payment hash (#3192).
+
+## Backwards Compatibility
+ * Any `Event::PaymentFailed` generated without a payment hash will deserialize
+   with `PaymentHash([0; 32])` when downgrading. This can be treated like an
+   `Event::InvoiceRequestFailed` (#3192).

From 457ba24ee5cfc66e756b298b323f107755278438 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Tue, 6 Aug 2024 18:13:45 -0500
Subject: [PATCH 16/19] Add PaymentFailureReason::InvoiceRequestExpired

Now that Event::PaymentFailed is generated when an InvoiceRequest times
out, define a new PaymentFailureReason for this situation.
---
 lightning/src/events/mod.rs          |  8 +++++---
 lightning/src/ln/outbound_payment.rs | 14 +++++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 780713b331b..51bf7832ae5 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -530,12 +530,15 @@ pub enum PaymentFailureReason {
 	UnexpectedError,
 	/// An invoice was received that required unknown features.
 	UnknownRequiredFeatures,
+	/// A [`Bolt12Invoice`] was not received in a reasonable amount of time.
+	InvoiceRequestExpired,
 }
 
 impl_writeable_tlv_based_enum!(PaymentFailureReason,
 	(0, RecipientRejected) => {},
 	(1, UnknownRequiredFeatures) => {},
 	(2, UserAbandoned) => {},
+	(3, InvoiceRequestExpired) => {},
 	(4, RetriesExhausted) => {},
 	(6, PaymentExpired) => {},
 	(8, RouteNotFound) => {},
@@ -870,8 +873,7 @@ pub enum Event {
 		/// [`Offer`]: crate::offers::offer::Offer
 		payment_hash: Option<PaymentHash>,
 		/// The reason the payment failed. This is only `None` for events generated or serialized
-		/// by versions prior to 0.0.115 or when deserializing an `Event::InvoiceRequestFailed`,
-		/// which was removed in 0.0.124.
+		/// by versions prior to 0.0.115.
 		reason: Option<PaymentFailureReason>,
 	},
 	/// Indicates that a path for an outbound payment was successful.
@@ -2080,7 +2082,7 @@ impl MaybeReadable for Event {
 					Ok(Some(Event::PaymentFailed {
 						payment_id: payment_id.0.unwrap(),
 						payment_hash: None,
-						reason: None,
+						reason: Some(PaymentFailureReason::InvoiceRequestExpired),
 					}))
 				};
 				f()
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 6585dfbe67c..a2e9e6b9b01 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1711,7 +1711,7 @@ impl OutboundPayments {
 					let event = events::Event::PaymentFailed {
 						payment_id: *payment_id,
 						payment_hash: None,
-						reason: None,
+						reason: Some(PaymentFailureReason::InvoiceRequestExpired),
 					};
 					pending_events.push_back((event, None));
 					false
@@ -2205,7 +2205,11 @@ mod tests {
 		assert!(!pending_events.lock().unwrap().is_empty());
 		assert_eq!(
 			pending_events.lock().unwrap().pop_front(),
-			Some((Event::PaymentFailed { payment_id, payment_hash: None, reason: None }, None)),
+			Some((Event::PaymentFailed {
+				payment_id,
+				payment_hash: None,
+				reason: Some(PaymentFailureReason::InvoiceRequestExpired),
+			}, None)),
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 
@@ -2254,7 +2258,11 @@ mod tests {
 		assert!(!pending_events.lock().unwrap().is_empty());
 		assert_eq!(
 			pending_events.lock().unwrap().pop_front(),
-			Some((Event::PaymentFailed { payment_id, payment_hash: None, reason: None }, None)),
+			Some((Event::PaymentFailed {
+				payment_id,
+				payment_hash: None,
+				reason: Some(PaymentFailureReason::InvoiceRequestExpired),
+			}, None)),
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 

From 1563186c2b40098c1872d718ed75891c2a42c702 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Tue, 6 Aug 2024 18:33:51 -0500
Subject: [PATCH 17/19] Add PaymentFailureReason::InvoiceRequestRejected

Instead of re-using PaymentFailureReason::RecipientRejected, define a
new InvoiceRequestRejected variant for when an InvoiceError is received
instead of a Bolt12Invoice. This allows user to differentiate the cause
of the failure.
---
 lightning/src/events/mod.rs        | 7 ++++++-
 lightning/src/ln/channelmanager.rs | 2 +-
 lightning/src/ln/offers_tests.rs   | 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 51bf7832ae5..b993de76e66 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -501,7 +501,7 @@ impl_writeable_tlv_based_enum!(InterceptNextHop,
 /// The reason the payment failed. Used in [`Event::PaymentFailed`].
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum PaymentFailureReason {
-	/// The intended recipient rejected our payment or invoice request.
+	/// The intended recipient rejected our payment.
 	RecipientRejected,
 	/// The user chose to abandon this payment by calling [`ChannelManager::abandon_payment`].
 	///
@@ -532,6 +532,10 @@ pub enum PaymentFailureReason {
 	UnknownRequiredFeatures,
 	/// A [`Bolt12Invoice`] was not received in a reasonable amount of time.
 	InvoiceRequestExpired,
+	/// An [`InvoiceRequest`] for the payment was rejected by the recipient.
+	///
+	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
+	InvoiceRequestRejected,
 }
 
 impl_writeable_tlv_based_enum!(PaymentFailureReason,
@@ -540,6 +544,7 @@ impl_writeable_tlv_based_enum!(PaymentFailureReason,
 	(2, UserAbandoned) => {},
 	(3, InvoiceRequestExpired) => {},
 	(4, RetriesExhausted) => {},
+	(5, InvoiceRequestRejected) => {},
 	(6, PaymentExpired) => {},
 	(8, RouteNotFound) => {},
 	(10, UnexpectedError) => {},
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 267fb245526..c6645956b18 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -10902,7 +10902,7 @@ where
 					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) }) => {
 						if signer::verify_payment_id(payment_id, hmac, nonce, expanded_key) {
 							self.abandon_payment_with_reason(
-								payment_id, PaymentFailureReason::RecipientRejected,
+								payment_id, PaymentFailureReason::InvoiceRequestRejected,
 							);
 						}
 					},
diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index b430196c4cf..fd496b5f639 100644
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -1932,8 +1932,9 @@ fn fails_sending_invoice_without_blinded_payment_paths_for_offer() {
 
 	// Confirm that david drops this failed payment from his pending outbound payments.
 	match get_event!(david, Event::PaymentFailed) {
-		Event::PaymentFailed { payment_id: actual_payment_id, .. } => {
+		Event::PaymentFailed { payment_id: actual_payment_id, reason, .. } => {
 			assert_eq!(payment_id, actual_payment_id);
+			assert_eq!(reason, Some(PaymentFailureReason::InvoiceRequestRejected));
 		},
 		_ => panic!("No Event::PaymentFailed"),
 	}

From bb94320e4095b23e4bdc025ade7a59d181f47a90 Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 7 Aug 2024 17:16:54 -0500
Subject: [PATCH 18/19] Make PaymentFailureReason upgradable

This allows downgrading to version 0.0.124 or later and using None for a
PaymentFailureReason that was added after.
---
 lightning/src/events/mod.rs          | 7 ++++---
 lightning/src/ln/outbound_payment.rs | 5 +++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index b993de76e66..938100a901c 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -538,7 +538,7 @@ pub enum PaymentFailureReason {
 	InvoiceRequestRejected,
 }
 
-impl_writeable_tlv_based_enum!(PaymentFailureReason,
+impl_writeable_tlv_based_enum_upgradable!(PaymentFailureReason,
 	(0, RecipientRejected) => {},
 	(1, UnknownRequiredFeatures) => {},
 	(2, UserAbandoned) => {},
@@ -878,7 +878,8 @@ pub enum Event {
 		/// [`Offer`]: crate::offers::offer::Offer
 		payment_hash: Option<PaymentHash>,
 		/// The reason the payment failed. This is only `None` for events generated or serialized
-		/// by versions prior to 0.0.115.
+		/// by versions prior to 0.0.115, or when downgrading to 0.0.124 or later with a reason that
+		/// was added after.
 		reason: Option<PaymentFailureReason>,
 	},
 	/// Indicates that a path for an outbound payment was successful.
@@ -1939,7 +1940,7 @@ impl MaybeReadable for Event {
 					Ok(Some(Event::PaymentFailed {
 						payment_id,
 						payment_hash,
-						reason,
+						reason: _init_tlv_based_struct_field!(reason, upgradable_option),
 					}))
 				};
 				f()
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index a2e9e6b9b01..c36f2c5c1a9 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -96,7 +96,8 @@ pub(crate) enum PendingOutboundPayment {
 	Abandoned {
 		session_privs: HashSet<[u8; 32]>,
 		payment_hash: PaymentHash,
-		/// Will be `None` if the payment was serialized before 0.0.115.
+		/// Will be `None` if the payment was serialized before 0.0.115 or if downgrading to 0.0.124
+		/// or later with a reason that was added after.
 		reason: Option<PaymentFailureReason>,
 	},
 }
@@ -1940,7 +1941,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	},
 	(3, Abandoned) => {
 		(0, session_privs, required),
-		(1, reason, option),
+		(1, reason, upgradable_option),
 		(2, payment_hash, required),
 	},
 	(5, AwaitingInvoice) => {

From 075a2e36b97cf4602be5ebf294cfde0a3c21413e Mon Sep 17 00:00:00 2001
From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Fri, 9 Aug 2024 11:07:35 -0500
Subject: [PATCH 19/19] Make PaymentFailureReason downgradable

The PaymentFailureReason variants for invoice request failures will
cause downgrades to break. Instead, use a new TLV for the reason and
continue to write the old TLV, only use None for the new reasons.
---
 lightning/src/events/mod.rs                   | 39 +++++++++++++++++--
 .../3192-invoice-request-failed-event.txt     |  6 +++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 938100a901c..63f19ed222b 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -502,6 +502,12 @@ impl_writeable_tlv_based_enum!(InterceptNextHop,
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum PaymentFailureReason {
 	/// The intended recipient rejected our payment.
+	///
+	/// Also used for [`UnknownRequiredFeatures`] and [`InvoiceRequestRejected`] when downgrading to
+	/// version prior to 0.0.124.
+	///
+	/// [`UnknownRequiredFeatures`]: Self::UnknownRequiredFeatures
+	/// [`InvoiceRequestRejected`]: Self::InvoiceRequestRejected
 	RecipientRejected,
 	/// The user chose to abandon this payment by calling [`ChannelManager::abandon_payment`].
 	///
@@ -517,7 +523,10 @@ pub enum PaymentFailureReason {
 	/// The payment expired while retrying, based on the provided
 	/// [`PaymentParameters::expiry_time`].
 	///
+	/// Also used for [`InvoiceRequestExpired`] when downgrading to version prior to 0.0.124.
+	///
 	/// [`PaymentParameters::expiry_time`]: crate::routing::router::PaymentParameters::expiry_time
+	/// [`InvoiceRequestExpired`]: Self::InvoiceRequestExpired
 	PaymentExpired,
 	/// We failed to find a route while retrying the payment.
 	///
@@ -878,8 +887,8 @@ pub enum Event {
 		/// [`Offer`]: crate::offers::offer::Offer
 		payment_hash: Option<PaymentHash>,
 		/// The reason the payment failed. This is only `None` for events generated or serialized
-		/// by versions prior to 0.0.115, or when downgrading to 0.0.124 or later with a reason that
-		/// was added after.
+		/// by versions prior to 0.0.115, or when downgrading to a version with a reason that was
+		/// added after.
 		reason: Option<PaymentFailureReason>,
 	},
 	/// Indicates that a path for an outbound payment was successful.
@@ -1554,11 +1563,30 @@ impl Writeable for Event {
 					Some(payment_hash) => (payment_hash, true),
 					None => (&PaymentHash([0; 32]), false),
 				};
+				let legacy_reason = match reason {
+					None => &None,
+					// Variants available prior to version 0.0.124.
+					Some(PaymentFailureReason::RecipientRejected)
+						| Some(PaymentFailureReason::UserAbandoned)
+						| Some(PaymentFailureReason::RetriesExhausted)
+						| Some(PaymentFailureReason::PaymentExpired)
+						| Some(PaymentFailureReason::RouteNotFound)
+						| Some(PaymentFailureReason::UnexpectedError) => reason,
+					// Variants introduced at version 0.0.124 or later. Prior versions fail to parse
+					// unknown variants, while versions 0.0.124 or later will use None.
+					Some(PaymentFailureReason::UnknownRequiredFeatures) =>
+						&Some(PaymentFailureReason::RecipientRejected),
+					Some(PaymentFailureReason::InvoiceRequestExpired) =>
+						&Some(PaymentFailureReason::RetriesExhausted),
+					Some(PaymentFailureReason::InvoiceRequestRejected) =>
+						&Some(PaymentFailureReason::RecipientRejected),
+				};
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
-					(1, reason, option),
+					(1, legacy_reason, option),
 					(2, payment_hash, required),
 					(3, invoice_received, required),
+					(5, reason, option),
 				})
 			},
 			&Event::OpenChannelRequest { .. } => {
@@ -1926,17 +1954,20 @@ impl MaybeReadable for Event {
 					let mut payment_hash = PaymentHash([0; 32]);
 					let mut payment_id = PaymentId([0; 32]);
 					let mut reason = None;
+					let mut legacy_reason = None;
 					let mut invoice_received: Option<bool> = None;
 					read_tlv_fields!(reader, {
 						(0, payment_id, required),
-						(1, reason, upgradable_option),
+						(1, legacy_reason, upgradable_option),
 						(2, payment_hash, required),
 						(3, invoice_received, option),
+						(5, reason, upgradable_option),
 					});
 					let payment_hash = match invoice_received {
 						Some(invoice_received) => invoice_received.then(|| payment_hash),
 						None => (payment_hash != PaymentHash([0; 32])).then(|| payment_hash),
 					};
+					let reason = reason.or(legacy_reason);
 					Ok(Some(Event::PaymentFailed {
 						payment_id,
 						payment_hash,
diff --git a/pending_changelog/3192-invoice-request-failed-event.txt b/pending_changelog/3192-invoice-request-failed-event.txt
index f1ab8df5bc9..26d736c0e41 100644
--- a/pending_changelog/3192-invoice-request-failed-event.txt
+++ b/pending_changelog/3192-invoice-request-failed-event.txt
@@ -10,3 +10,9 @@
  * Any `Event::PaymentFailed` generated without a payment hash will deserialize
    with `PaymentHash([0; 32])` when downgrading. This can be treated like an
    `Event::InvoiceRequestFailed` (#3192).
+ * An `Event::PaymentFailed` generated with one of the following
+   `PaymentFailureReason`s will deserialize with the corresponding reason after
+   downgrading to a version prior to 0.0.124:
+   - `UnknownRequiredFeatures` to `RecipientRejected`,
+   - `InvoiceRequestExpired` to `RetriesExhausted`, and
+   - `InvoiceRequestRejected` to `RecipientRejected` (#3192).