From a9e63630639fd5c7968fe1268bf7b08a6eb81b27 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 6 Aug 2024 17:51:16 -0500 Subject: [PATCH] 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 a94d6c118..780713b33 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, }, - /// 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, /// 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, }, /// 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 7b903cd02..267fb2455 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 5b81251f6..b430196c4 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 c04e186f1..6585dfbe6 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()); } -- 2.39.5