]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Expose an event when a payment has failed and retries complete
authorMatt Corallo <git@bluematt.me>
Fri, 10 Dec 2021 00:28:24 +0000 (00:28 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 15 Dec 2021 03:57:13 +0000 (03:57 +0000)
When a payment fails, a payer needs to know when they can consider
a payment as fully-failed, and when only some of the HTLCs in the
payment have failed. This isn't possible with the current event
scheme, as discovered recently and as described in the previous
commit.

This adds a new event which describes when a payment is fully and
irrevocably failed, generating it only after the payment has
expired or been marked as expired with
`ChannelManager::mark_retries_exceeded` *and* all HTLCs for it
have failed. With this, a payer can more simply deduce when a
payment has failed and use that to remove payment state or
finalize a payment failure.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/util/events.rs

index 1d3b292da876f7ccf5a1ff042551bb1d062ddb42..4caa5435eac0efdd3cf1e1259e99f0092121ed8e 100644 (file)
@@ -838,6 +838,10 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRA
 #[allow(dead_code)]
 const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
 
+/// The number of blocks before we consider an outbound payment for expiry if it doesn't have any
+/// pending HTLCs in flight.
+pub(crate) const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
+
 /// Information needed for constructing an invoice route hint for this channel.
 #[derive(Clone, Debug, PartialEq)]
 pub struct CounterpartyForwardingInfo {
@@ -2411,15 +2415,31 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// Signals that no further retries for the given payment will occur.
        ///
        /// After this method returns, any future calls to [`retry_payment`] for the given `payment_id`
-       /// will fail with [`PaymentSendFailure::ParameterError`].
+       /// will fail with [`PaymentSendFailure::ParameterError`]. If no such event has been generated,
+       /// an [`Event::PaymentFailed`] event will be generated as soon as there are no remaining
+       /// pending HTLCs for this payment.
+       ///
+       /// Note that calling this method does *not* prevent a payment from succeeding. You must still
+       /// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to
+       /// determine the ultimate status of a payment.
        ///
        /// [`retry_payment`]: Self::retry_payment
+       /// [`Event::PaymentFailed`]: events::Event::PaymentFailed
+       /// [`Event::PaymentSent`]: events::Event::PaymentSent
        pub fn abandon_payment(&self, payment_id: PaymentId) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
-                       let _ = payment.get_mut().mark_abandoned();
+                       if let Ok(()) = payment.get_mut().mark_abandoned() {
+                               if payment.get().remaining_parts() == 0 {
+                                       self.pending_events.lock().unwrap().push(events::Event::PaymentFailed {
+                                               payment_id,
+                                               payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                       });
+                                       payment.remove();
+                               }
+                       }
                }
        }
 
@@ -3250,22 +3270,28 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                        final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
                                                                })
                                                        } else { None };
-                                                       self.pending_events.lock().unwrap().push(
-                                                               events::Event::PaymentPathFailed {
-                                                                       payment_id: Some(payment_id),
-                                                                       payment_hash,
-                                                                       rejected_by_dest: false,
-                                                                       network_update: None,
-                                                                       all_paths_failed: payment.get().remaining_parts() == 0,
-                                                                       path: path.clone(),
-                                                                       short_channel_id: None,
-                                                                       retry,
-                                                                       #[cfg(test)]
-                                                                       error_code: None,
-                                                                       #[cfg(test)]
-                                                                       error_data: None,
-                                                               }
-                                                       );
+                                                       let mut pending_events = self.pending_events.lock().unwrap();
+                                                       pending_events.push(events::Event::PaymentPathFailed {
+                                                               payment_id: Some(payment_id),
+                                                               payment_hash,
+                                                               rejected_by_dest: false,
+                                                               network_update: None,
+                                                               all_paths_failed: payment.get().remaining_parts() == 0,
+                                                               path: path.clone(),
+                                                               short_channel_id: None,
+                                                               retry,
+                                                               #[cfg(test)]
+                                                               error_code: None,
+                                                               #[cfg(test)]
+                                                               error_data: None,
+                                                       });
+                                                       if payment.get().abandoned() && payment.get().remaining_parts() == 0 {
+                                                               pending_events.push(events::Event::PaymentFailed {
+                                                                       payment_id,
+                                                                       payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                                               });
+                                                               payment.remove();
+                                                       }
                                                }
                                        } else {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3296,6 +3322,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                session_priv_bytes.copy_from_slice(&session_priv[..]);
                                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
                                let mut all_paths_failed = false;
+                               let mut full_failure_ev = None;
                                if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
                                        if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
                                                log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3307,6 +3334,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        }
                                        if payment.get().remaining_parts() == 0 {
                                                all_paths_failed = true;
+                                               if payment.get().abandoned() {
+                                                       full_failure_ev = Some(events::Event::PaymentFailed {
+                                                               payment_id,
+                                                               payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
+                                                       });
+                                                       payment.remove();
+                                               }
                                        }
                                } else {
                                        log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -3322,7 +3356,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        })
                                } else { None };
                                log_trace!(self.logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-                               match &onion_error {
+
+                               let path_failure = match &onion_error {
                                        &HTLCFailReason::LightningError { ref err } => {
 #[cfg(test)]
                                                let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
@@ -3331,22 +3366,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // TODO: If we decided to blame ourselves (or one of our channels) in
                                                // process_onion_failure we should close that channel as it implies our
                                                // next-hop is needlessly blaming us!
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentPathFailed {
-                                                               payment_id: Some(payment_id),
-                                                               payment_hash: payment_hash.clone(),
-                                                               rejected_by_dest: !payment_retryable,
-                                                               network_update,
-                                                               all_paths_failed,
-                                                               path: path.clone(),
-                                                               short_channel_id,
-                                                               retry,
+                                               events::Event::PaymentPathFailed {
+                                                       payment_id: Some(payment_id),
+                                                       payment_hash: payment_hash.clone(),
+                                                       rejected_by_dest: !payment_retryable,
+                                                       network_update,
+                                                       all_paths_failed,
+                                                       path: path.clone(),
+                                                       short_channel_id,
+                                                       retry,
 #[cfg(test)]
-                                                               error_code: onion_error_code,
+                                                       error_code: onion_error_code,
 #[cfg(test)]
-                                                               error_data: onion_error_data
-                                                       }
-                                               );
+                                                       error_data: onion_error_data
+                                               }
                                        },
                                        &HTLCFailReason::Reason {
 #[cfg(test)]
@@ -3361,24 +3394,25 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // ChannelDetails.
                                                // TODO: For non-temporary failures, we really should be closing the
                                                // channel here as we apparently can't relay through them anyway.
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentPathFailed {
-                                                               payment_id: Some(payment_id),
-                                                               payment_hash: payment_hash.clone(),
-                                                               rejected_by_dest: path.len() == 1,
-                                                               network_update: None,
-                                                               all_paths_failed,
-                                                               path: path.clone(),
-                                                               short_channel_id: Some(path.first().unwrap().short_channel_id),
-                                                               retry,
+                                               events::Event::PaymentPathFailed {
+                                                       payment_id: Some(payment_id),
+                                                       payment_hash: payment_hash.clone(),
+                                                       rejected_by_dest: path.len() == 1,
+                                                       network_update: None,
+                                                       all_paths_failed,
+                                                       path: path.clone(),
+                                                       short_channel_id: Some(path.first().unwrap().short_channel_id),
+                                                       retry,
 #[cfg(test)]
-                                                               error_code: Some(*failure_code),
+                                                       error_code: Some(*failure_code),
 #[cfg(test)]
-                                                               error_data: Some(data.clone()),
-                                                       }
-                                               );
+                                                       error_data: Some(data.clone()),
+                                               }
                                        }
-                               }
+                               };
+                               let mut pending_events = self.pending_events.lock().unwrap();
+                               pending_events.push(path_failure);
+                               if let Some(ev) = full_failure_ev { pending_events.push(ev); }
                        },
                        HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, .. }) => {
                                let err_packet = match onion_error {
@@ -4907,14 +4941,19 @@ where
                        inbound_payment.expiry_time > header.time as u64
                });
 
+               let mut pending_events = self.pending_events.lock().unwrap();
                let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-               outbounds.retain(|_, payment| {
-                       const PAYMENT_EXPIRY_BLOCKS: u32 = 3;
+               outbounds.retain(|payment_id, payment| {
                        if payment.remaining_parts() != 0 { return true }
-                       if let PendingOutboundPayment::Retryable { starting_block_height, .. } = payment {
-                               return *starting_block_height + PAYMENT_EXPIRY_BLOCKS > height
-                       }
-                       true
+                       if let PendingOutboundPayment::Retryable { starting_block_height, payment_hash, .. } = payment {
+                               if *starting_block_height + PAYMENT_EXPIRY_BLOCKS <= height {
+                                       log_info!(self.logger, "Timing out payment with id {} and hash {}", log_bytes!(payment_id.0), log_bytes!(payment_hash.0));
+                                       pending_events.push(events::Event::PaymentFailed {
+                                               payment_id: *payment_id, payment_hash: *payment_hash,
+                                       });
+                                       false
+                               } else { true }
+                       } else { true }
                });
        }
 
index 84f4860d47a41c8260aab9d9a0576fb57de4e3a4..8ff793ed08b723280c3f089eb2ce74cb7bd2ad53 100644 (file)
@@ -1211,6 +1211,7 @@ pub struct PaymentFailedConditions<'a> {
        pub(crate) expected_htlc_error_data: Option<(u16, &'a [u8])>,
        pub(crate) expected_blamed_scid: Option<u64>,
        pub(crate) expected_blamed_chan_closed: Option<bool>,
+       pub(crate) expected_mpp_parts_remain: bool,
 }
 
 impl<'a> PaymentFailedConditions<'a> {
@@ -1219,8 +1220,13 @@ impl<'a> PaymentFailedConditions<'a> {
                        expected_htlc_error_data: None,
                        expected_blamed_scid: None,
                        expected_blamed_chan_closed: None,
+                       expected_mpp_parts_remain: false,
                }
        }
+       pub fn mpp_parts_remain(mut self) -> Self {
+               self.expected_mpp_parts_remain = true;
+               self
+       }
        pub fn blamed_scid(mut self, scid: u64) -> Self {
                self.expected_blamed_scid = Some(scid);
                self
@@ -1246,6 +1252,7 @@ macro_rules! expect_payment_failed_with_update {
 #[cfg(test)]
 macro_rules! expect_payment_failed {
        ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
+               #[allow(unused_mut)]
                let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new();
                $(
                        conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data);
@@ -1259,8 +1266,8 @@ macro_rules! expect_payment_failed_conditions {
        ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $conditions: expr) => {
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
-               match events[0] {
-                       Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data, ref path, ref retry, ref network_update, .. } => {
+               let expected_payment_id = match events[0] {
+                       Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data, ref path, ref retry, ref payment_id, ref network_update, .. } => {
                                assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
                                assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
                                assert!(retry.is_some(), "expected retry.is_some()");
@@ -1292,10 +1299,24 @@ macro_rules! expect_payment_failed_conditions {
                                                None => panic!("Expected update"),
                                        }
                                }
+
+                               payment_id.unwrap()
                        },
                        _ => panic!("Unexpected event"),
                };
-       };
+               if !$conditions.expected_mpp_parts_remain {
+                       $node.node.abandon_payment(expected_payment_id);
+                       let events = $node.node.get_and_clear_pending_events();
+                       assert_eq!(events.len(), 1);
+                       match events[0] {
+                               Event::PaymentFailed { ref payment_hash, ref payment_id } => {
+                                       assert_eq!(*payment_hash, $expected_payment_hash, "unexpected second payment_hash");
+                                       assert_eq!(*payment_id, expected_payment_id);
+                               }
+                               _ => panic!("Unexpected second event"),
+                       }
+               }
+       }
 }
 
 pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
@@ -1598,16 +1619,29 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
                        commitment_signed_dance!(origin_node, prev_node, next_msgs.as_ref().unwrap().1, false);
                        let events = origin_node.node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 1);
-                       match events[0] {
-                               Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, .. } => {
+                       let expected_payment_id = match events[0] {
+                               Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, ref payment_id, .. } => {
                                        assert_eq!(payment_hash, our_payment_hash);
                                        assert!(rejected_by_dest);
                                        assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
                                        for (idx, hop) in expected_route.iter().enumerate() {
                                                assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
                                        }
+                                       payment_id.unwrap()
                                },
                                _ => panic!("Unexpected event"),
+                       };
+                       if i == expected_paths.len() - 1 {
+                               origin_node.node.abandon_payment(expected_payment_id);
+                               let events = origin_node.node.get_and_clear_pending_events();
+                               assert_eq!(events.len(), 1);
+                               match events[0] {
+                                       Event::PaymentFailed { ref payment_hash, ref payment_id } => {
+                                               assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
+                                               assert_eq!(*payment_id, expected_payment_id);
+                                       }
+                                       _ => panic!("Unexpected second event"),
+                               }
                        }
                }
        }
index 1bf4f67de76f98d178b1dccbe1709c7bfd5d820a..d693c41d07e300c8228bda16ffb52d18d213fa11 100644 (file)
@@ -19,7 +19,7 @@ use chain::transaction::OutPoint;
 use chain::keysinterface::BaseSign;
 use ln::{PaymentPreimage, PaymentSecret, PaymentHash};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT};
-use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA};
+use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, PAYMENT_EXPIRY_BLOCKS };
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
 use ln::chan_utils::{HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, HTLCOutputInCommitment};
@@ -3149,9 +3149,10 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
        mine_transaction(&nodes[1], &revoked_local_txn[0]);
        check_added_monitors!(nodes[1], 1);
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+       assert!(ANTI_REORG_DELAY > PAYMENT_EXPIRY_BLOCKS); // We assume payments will also expire
 
        let events = nodes[1].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), if deliver_bs_raa { 2 } else { 3 });
+       assert_eq!(events.len(), if deliver_bs_raa { 2 } else { 4 });
        match events[0] {
                Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => { },
                _ => panic!("Unexepected event"),
@@ -3164,6 +3165,12 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
        }
        if !deliver_bs_raa {
                match events[2] {
+                       Event::PaymentFailed { ref payment_hash, .. } => {
+                               assert_eq!(*payment_hash, fourth_payment_hash);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+               match events[3] {
                        Event::PendingHTLCsForwardable { .. } => { },
                        _ => panic!("Unexpected event"),
                };
@@ -4181,7 +4188,14 @@ fn do_test_holding_cell_htlc_add_timeouts(forwarded_htlc: bool) {
                }
                expect_payment_failed_with_update!(nodes[0], second_payment_hash, false, chan_2.0.contents.short_channel_id, false);
        } else {
-               expect_payment_failed!(nodes[1], second_payment_hash, true);
+               let events = nodes[1].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 2);
+               if let Event::PaymentPathFailed { ref payment_hash, .. } = events[0] {
+                       assert_eq!(*payment_hash, second_payment_hash);
+               } else { panic!("Unexpected event"); }
+               if let Event::PaymentFailed { ref payment_hash, .. } = events[1] {
+                       assert_eq!(*payment_hash, second_payment_hash);
+               } else { panic!("Unexpected event"); }
        }
 }
 
@@ -5837,7 +5851,14 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
                check_added_monitors!(nodes[0], 1);
                check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
        } else {
-               expect_payment_failed!(nodes[0], our_payment_hash, true);
+               let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 2);
+               if let Event::PaymentPathFailed { ref payment_hash, .. } = events[0] {
+                       assert_eq!(*payment_hash, our_payment_hash);
+               } else { panic!("Unexpected event"); }
+               if let Event::PaymentFailed { ref payment_hash, .. } = events[1] {
+                       assert_eq!(*payment_hash, our_payment_hash);
+               } else { panic!("Unexpected event"); }
        }
 }
 
index 518ccd3b0efe5fd6fc75e4ba72eaadbec690205c..82c14645fb8c8bc32ac6c9114bb29bdff718fa6c 100644 (file)
@@ -67,7 +67,7 @@ fn retry_single_path_payment() {
        check_added_monitors!(nodes[1], 1);
        nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
-       expect_payment_failed!(nodes[0], payment_hash, false);
+       expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
 
        // Rebalance the channel so the retry succeeds.
        send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000);
@@ -170,7 +170,7 @@ fn mpp_retry() {
        check_added_monitors!(nodes[2], 1);
        nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false);
-       expect_payment_failed!(nodes[0], payment_hash, false);
+       expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
 
        // Rebalance the channel so the second half of the payment can succeed.
        send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
@@ -437,7 +437,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
                confirm_transaction(&nodes[0], &as_htlc_timeout_txn[0]);
        }
        nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
-       expect_payment_failed!(nodes[0], payment_hash, false);
+       expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain());
 
        // Finally, retry the payment (which was reloaded from the ChannelMonitor when nodes[0] was
        // reloaded) via a route over the new channel, which work without issue and eventually be
index 959b180d5101330f18edfe631d29315cf1660f24..5184a02031ddd6719048719368e46bd261a95f7a 100644 (file)
@@ -225,14 +225,20 @@ pub enum Event {
                /// [`Route::get_total_fees`]: crate::routing::router::Route::get_total_fees
                fee_paid_msat: Option<u64>,
        },
-       /// Indicates an outbound payment we made failed. Probably some intermediary node dropped
+       /// Indicates an outbound HTLC we sent failed. Probably some intermediary node dropped
        /// something. You may wish to retry with a different route.
+       ///
+       /// Note that this does *not* indicate that all paths for an MPP payment have failed, see
+       /// [`Event::PaymentFailed`] and [`all_paths_failed`].
+       ///
+       /// [`all_paths_failed`]: Self::all_paths_failed
        PaymentPathFailed {
                /// The id returned by [`ChannelManager::send_payment`] and used with
-               /// [`ChannelManager::retry_payment`].
+               /// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
                ///
                /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
                /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
+               /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
                payment_id: Option<PaymentId>,
                /// The hash that was given to [`ChannelManager::send_payment`].
                ///
@@ -254,6 +260,20 @@ pub enum Event {
                /// For both single-path and multi-path payments, this is set if all paths of the payment have
                /// failed. This will be set to false if (1) this is an MPP payment and (2) other parts of the
                /// larger MPP payment were still in flight when this event was generated.
+               ///
+               /// Note that if you are retrying individual MPP parts, using this value to determine if a
+               /// payment has fully failed is race-y. Because multiple failures can happen prior to events
+               /// being processed, you may retry in response to a first failure, with a second failure
+               /// (with `all_paths_failed` set) still pending. Then, when the second failure is processed
+               /// you will see `all_paths_failed` set even though the retry of the first failure still
+               /// has an associated in-flight HTLC. See (1) for an example of such a failure.
+               ///
+               /// If you wish to retry individual MPP parts and learn when a payment has failed, you must
+               /// call [`ChannelManager::abandon_payment`] and wait for a [`Event::PaymentFailed`] event.
+               ///
+               /// (1) <https://github.com/lightningdevkit/rust-lightning/issues/1164>
+               ///
+               /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
                all_paths_failed: bool,
                /// The payment path that failed.
                path: Vec<RouteHop>,
@@ -274,6 +294,27 @@ pub enum Event {
 #[cfg(test)]
                error_data: Option<Vec<u8>>,
        },
+       /// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events
+       /// provide failure information for each MPP part in the payment.
+       ///
+       /// This event is provided once there are no further pending HTLCs for the payment and the
+       /// payment is no longer retryable, either due to a several-block timeout or because
+       /// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment.
+       ///
+       /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
+       PaymentFailed {
+               /// The id returned by [`ChannelManager::send_payment`] and used with
+               /// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+               /// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
+               /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
+               payment_id: PaymentId,
+               /// The hash that was given to [`ChannelManager::send_payment`].
+               ///
+               /// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
+               payment_hash: PaymentHash,
+       },
        /// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at
        /// a time in the future.
        ///
@@ -462,6 +503,13 @@ impl Writeable for Event {
                                        (4, path, vec_type)
                                })
                        },
+                       &Event::PaymentFailed { ref payment_id, ref payment_hash } => {
+                               15u8.write(writer)?;
+                               write_tlv_fields!(writer, {
+                                       (0, payment_id, required),
+                                       (2, payment_hash, required),
+                               })
+                       },
                        // Note that, going forward, all new events must only write data inside of
                        // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
                        // data via `write_tlv_fields`.
@@ -639,6 +687,21 @@ impl MaybeReadable for Event {
                                };
                                f()
                        },
+                       15u8 => {
+                               let f = || {
+                                       let mut payment_hash = PaymentHash([0; 32]);
+                                       let mut payment_id = PaymentId([0; 32]);
+                                       read_tlv_fields!(reader, {
+                                               (0, payment_id, required),
+                                               (2, payment_hash, required),
+                                       });
+                                       Ok(Some(Event::PaymentFailed {
+                                               payment_id,
+                                               payment_hash,
+                                       }))
+                               };
+                               f()
+                       },
                        // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
                        // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
                        // reads.