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.

Author: jkczyz

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.
 		/// TODO: Will downgrading cause this to be None for UnknownRequiredFeatures?
 		reason: Option<PaymentFailureReason>,
 	},
@@ -1636,12 +1621,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.
@@ -2078,13 +2057,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()

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),
 ///         // ...
 ///     #     _ => {},
 ///     }
@@ -4260,15 +4258,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) {
@@ -8845,7 +8841,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.
@@ -8961,7 +8957,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
 	///

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"),
 	}
 }
 

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());
 	}