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.

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 393acbc024284a36c0833698423e3bb55f233163..a94d6c118ce62de0d317e4975a4defcccdb01dbb 100644 (file)
--- 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 c1d65fd1a118f2f3e669950d2e2ed2db8b4cf97e..9d246f1a741840a72ca663f54e64cd1a3fb50098 100644 (file)
--- 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 9bc48f12f0a965117682beec31a06b2f97dda9ab..52669637d00d4cc14c4075f700ced537d2601752 100644 (file)
--- 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 617c58c2e4887c2e34a994c80bb326801e37dc10..7b903cd02b9a82c381831fef5729698be07d2aa1 100644 (file)
--- 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 5f1b7123d5ad455f1b45553c3505d639a838a804..2845ddd9a414a4391d5ad35855be6d90a8eea86e 100644 (file)
--- 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 99063334a28eb502e2c214cd662fc12a50e568cd..617df3ce7cc46ac265f66004c9cf3de355b005e7 100644 (file)
--- 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 ee0f731b23f5387e7e24abda085d89a3f23e095d..5b81251f6c1b1802e9751c4e487d715f80c7ec90 100644 (file)
--- 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 bd28f96988fc570407b01c0ecc5dcedc330d2233..94c2cfd2ee412c7fcce38630be526242f72b8c50 100644 (file)
--- 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 5cc31f60500922d72058d4f05ccb8cc2dad8d787..c04e186f1d3a702164b5813b316f9f8a7617800b 100644 (file)
--- 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 3ab055d2adaa52a6d147f196dc8d6c0a91fa82c1..083c32e53938e23c8b38e72c971f9a921b6d33ce 100644 (file)
--- 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());
                },