Rework auto retry send errors
authorValentine Wallace <vwallace@protonmail.com>
Fri, 10 Feb 2023 21:09:01 +0000 (15:09 -0600)
committerValentine Wallace <vwallace@protonmail.com>
Thu, 23 Feb 2023 20:50:23 +0000 (15:50 -0500)
Prior to this, we returned PaymentSendFailure from auto retry send payment
methods. This implied that we might return a PartialFailure from them, which
has never been the case. So it makes sense to rework the errors to be a better
fit for the methods.

We're taking error handling in a totally different direction now to make it
more asynchronous, see send_payment_internal for more information.

lightning-invoice/src/payment.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs

index 0ffbae5fad5004804a465b1cbcd1335dc2a34907..db47f9954800a2296785bdabd622aa31b7bd41c4 100644 (file)
@@ -17,7 +17,7 @@ use lightning::chain;
 use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use lightning::chain::keysinterface::{NodeSigner, SignerProvider, EntropySource};
 use lightning::ln::{PaymentHash, PaymentSecret};
-use lightning::ln::channelmanager::{ChannelManager, PaymentId, PaymentSendFailure, Retry};
+use lightning::ln::channelmanager::{ChannelManager, PaymentId, Retry, RetryableSendFailure};
 use lightning::routing::router::{PaymentParameters, RouteParameters, Router};
 use lightning::util::logger::Logger;
 
@@ -172,7 +172,7 @@ pub enum PaymentError {
        /// An error resulting from the provided [`Invoice`] or payment hash.
        Invoice(&'static str),
        /// An error occurring when sending a payment.
-       Sending(PaymentSendFailure),
+       Sending(RetryableSendFailure),
 }
 
 /// A trait defining behavior of an [`Invoice`] payer.
index 39cb9a7b5fe24d543674095fdbda3464b776ec10..3fd472925cf6948ec56a88986ed97a1368e99963 100644 (file)
@@ -76,7 +76,7 @@ use core::time::Duration;
 use core::ops::Deref;
 
 // Re-export this for use in the public API.
-pub use crate::ln::outbound_payment::{PaymentSendFailure, Retry};
+pub use crate::ln::outbound_payment::{PaymentSendFailure, Retry, RetryableSendFailure};
 
 // We hold various information about HTLC relay in the HTLC objects in Channel itself:
 //
@@ -2531,7 +2531,7 @@ where
 
        /// Similar to [`ChannelManager::send_payment`], but will automatically find a route based on
        /// `route_params` and retry failed payment paths based on `retry_strategy`.
-       pub fn send_payment_with_retry(&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), PaymentSendFailure> {
+       pub fn send_payment_with_retry(&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), RetryableSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments
                        .send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
@@ -2609,7 +2609,7 @@ where
        /// payments.
        ///
        /// [`PaymentParameters::for_keysend`]: crate::routing::router::PaymentParameters::for_keysend
-       pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, PaymentSendFailure> {
+       pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, RetryableSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
                        retry_strategy, route_params, &self.router, self.list_usable_channels(),
index 2ddd3e2e006a9935a8c7243800a98aa36278d60f..2d79033ac2e4ab899ecca95ceed71da2c0f6bec7 100644 (file)
@@ -312,9 +312,35 @@ impl<T: Time> Display for PaymentAttemptsUsingTime<T> {
        }
 }
 
-/// If a payment fails to send, it can be in one of several states. This enum is returned as the
-/// Err() type describing which state the payment is in, see the description of individual enum
-/// states for more.
+/// Indicates an immediate error on [`ChannelManager::send_payment_with_retry`]. Further errors
+/// may be surfaced later via [`Event::PaymentPathFailed`] and [`Event::PaymentFailed`].
+///
+/// [`ChannelManager::send_payment_with_retry`]: crate::ln::channelmanager::ChannelManager::send_payment_with_retry
+/// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
+/// [`Event::PaymentFailed`]: crate::util::events::Event::PaymentFailed
+#[derive(Clone, Debug)]
+pub enum RetryableSendFailure {
+       /// The provided [`PaymentParameters::expiry_time`] indicated that the payment has expired. Note
+       /// that this error is *not* caused by [`Retry::Timeout`].
+       ///
+       /// [`PaymentParameters::expiry_time`]: crate::routing::router::PaymentParameters::expiry_time
+       PaymentExpired,
+       /// We were unable to find a route to the destination.
+       RouteNotFound,
+       /// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
+       /// yet completed (i.e. generated an [`Event::PaymentSent`] or [`Event::PaymentFailed`]).
+       ///
+       /// [`PaymentId`]: crate::ln::channelmanager::PaymentId
+       /// [`Event::PaymentSent`]: crate::util::events::Event::PaymentSent
+       /// [`Event::PaymentFailed`]: crate::util::events::Event::PaymentFailed
+       DuplicatePayment,
+}
+
+/// If a payment fails to send with [`ChannelManager::send_payment`], it can be in one of several
+/// states. This enum is returned as the Err() type describing which state the payment is in, see
+/// the description of individual enum states for more.
+///
+/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
 #[derive(Clone, Debug)]
 pub enum PaymentSendFailure {
        /// A parameter which was passed to send_payment was invalid, preventing us from attempting to
@@ -399,7 +425,7 @@ impl OutboundPayments {
                first_hops: Vec<ChannelDetails>, compute_inflight_htlcs: IH, entropy_source: &ES,
                node_signer: &NS, best_block_height: u32, logger: &L,
                pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: SP,
-       ) -> Result<(), PaymentSendFailure>
+       ) -> Result<(), RetryableSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
@@ -409,10 +435,9 @@ impl OutboundPayments {
                SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                         u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
        {
-               self.pay_internal(payment_id, payment_hash, Some((payment_secret, None, retry_strategy)),
+               self.send_payment_internal(payment_id, payment_hash, payment_secret, None, retry_strategy,
                        route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer,
                        best_block_height, logger, pending_events, &send_payment_along_path)
-                       .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
 
        pub(super) fn send_payment_with_route<ES: Deref, NS: Deref, F>(
@@ -438,7 +463,7 @@ impl OutboundPayments {
                first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES,
                node_signer: &NS, best_block_height: u32, logger: &L,
                pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: SP
-       ) -> Result<PaymentHash, PaymentSendFailure>
+       ) -> Result<PaymentHash, RetryableSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
@@ -451,11 +476,10 @@ impl OutboundPayments {
                let preimage = payment_preimage
                        .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
                let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
-               self.pay_internal(payment_id, payment_hash, Some((&None, Some(preimage), retry_strategy)),
-                       route_params, router, first_hops, &inflight_htlcs, entropy_source, node_signer,
-                       best_block_height, logger, pending_events, &send_payment_along_path)
+               self.send_payment_internal(payment_id, payment_hash, &None, Some(preimage), retry_strategy,
+                       route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
+                       best_block_height, logger, pending_events, send_payment_along_path)
                        .map(|()| payment_hash)
-                       .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
 
        pub(super) fn send_spontaneous_payment_with_route<ES: Deref, NS: Deref, F>(
@@ -522,12 +546,7 @@ impl OutboundPayments {
                        }
                        core::mem::drop(outbounds);
                        if let Some((payment_id, payment_hash, route_params)) = retry_id_route_params {
-                               if let Err(e) = self.pay_internal(payment_id, payment_hash, None, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path) {
-                                       log_info!(logger, "Errored retrying payment: {:?}", e);
-                                       // If we error on retry, there is no chance of the payment succeeding and no HTLCs have
-                                       // been irrevocably committed to, so we can safely abandon.
-                                       self.abandon_payment(payment_id, pending_events);
-                               }
+                               self.retry_payment_internal(payment_id, payment_hash, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path)
                        } else { break }
                }
 
@@ -553,14 +572,18 @@ impl OutboundPayments {
                        !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 && !pmt.is_fulfilled())
        }
 
-       /// Will return `Ok(())` iff at least one HTLC is sent for the payment.
-       fn pay_internal<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
-               &self, payment_id: PaymentId, payment_hash: PaymentHash,
-               initial_send_info: Option<(&Option<PaymentSecret>, Option<PaymentPreimage>, Retry)>,
-               route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
-               inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
-               logger: &L, pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: &SP,
-       ) -> Result<(), PaymentSendFailure>
+       /// Errors immediately on [`RetryableSendFailure`] error conditions. Otherwise, further errors may
+       /// be surfaced asynchronously via [`Event::PaymentPathFailed`] and [`Event::PaymentFailed`].
+       ///
+       /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
+       /// [`Event::PaymentFailed`]: crate::util::events::Event::PaymentFailed
+       fn send_payment_internal<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
+               &self, payment_id: PaymentId, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>,
+               keysend_preimage: Option<PaymentPreimage>, retry_strategy: Retry, route_params: RouteParameters,
+               router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L,
+               pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: SP,
+       ) -> Result<(), RetryableSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
@@ -568,53 +591,148 @@ impl OutboundPayments {
                L::Target: Logger,
                IH: Fn() -> InFlightHtlcs,
                SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
-                  u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
+                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
        {
                #[cfg(feature = "std")] {
                        if has_expired(&route_params) {
-                               return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
-                                       err: format!("Invoice expired for payment id {}", log_bytes!(payment_id.0)),
-                               }))
+                               return Err(RetryableSendFailure::PaymentExpired)
                        }
                }
 
                let route = router.find_route(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs(),
-               ).map_err(|e| PaymentSendFailure::ParameterError(APIError::APIMisuseError {
-                       err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound
-               }))?;
-
-               let res = if let Some((payment_secret, keysend_preimage, retry_strategy)) = initial_send_info {
-                       let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, keysend_preimage, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?;
-                       self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
-               } else {
-                       self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
+                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs()
+               ).map_err(|_| RetryableSendFailure::RouteNotFound)?;
+
+               let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret,
+                       payment_id, keysend_preimage, &route, Some(retry_strategy),
+                       Some(route_params.payment_params.clone()), entropy_source, best_block_height)
+                       .map_err(|_| RetryableSendFailure::DuplicatePayment)?;
+
+               let res = self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None,
+                       onion_session_privs, node_signer, best_block_height, &send_payment_along_path);
+               log_info!(logger, "Result sending payment with id {}: {:?}", log_bytes!(payment_id.0), res);
+               if let Err(e) = res {
+                       self.handle_pay_route_err(e, payment_id, payment_hash, route, route_params, router, first_hops, &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path);
+               }
+               Ok(())
+       }
+
+       fn retry_payment_internal<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
+               &self, payment_id: PaymentId, payment_hash: PaymentHash, route_params: RouteParameters,
+               router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: &IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L,
+               pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: &SP,
+       )
+       where
+               R::Target: Router,
+               ES::Target: EntropySource,
+               NS::Target: NodeSigner,
+               L::Target: Logger,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
+       {
+               #[cfg(feature = "std")] {
+                       if has_expired(&route_params) {
+                               log_error!(logger, "Payment params expired on retry, abandoning payment {}", log_bytes!(payment_id.0));
+                               self.abandon_payment(payment_id, pending_events);
+                               return
+                       }
+               }
+
+               let route = match router.find_route(
+                       &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
+                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs()
+               ) {
+                       Ok(route) => route,
+                       Err(e) => {
+                               log_error!(logger, "Failed to find a route on retry, abandoning payment {}: {:#?}", log_bytes!(payment_id.0), e);
+                               self.abandon_payment(payment_id, pending_events);
+                               return
+                       }
                };
-               match res {
-                       Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
-                               let retry_res = self.pay_internal(payment_id, payment_hash, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
-                               log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
-                               if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = &retry_res {
-                                       if err.starts_with("Retries exhausted ") { return res; }
-                               }
-                               retry_res
+
+               let res = self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer,
+                       best_block_height, send_payment_along_path);
+               log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), res);
+               if let Err(e) = res {
+                       self.handle_pay_route_err(e, payment_id, payment_hash, route, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
+               }
+       }
+
+       fn handle_pay_route_err<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
+               &self, err: PaymentSendFailure, payment_id: PaymentId, payment_hash: PaymentHash, route: Route,
+               route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: &IH,
+               entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L,
+               pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: &SP,
+       )
+       where
+               R::Target: Router,
+               ES::Target: EntropySource,
+               NS::Target: NodeSigner,
+               L::Target: Logger,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
+       {
+               match err {
+                       PaymentSendFailure::AllFailedResendSafe(errs) => {
+                               Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
+                               self.retry_payment_internal(payment_id, payment_hash, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
                        },
-                       Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), .. }) => {
+                       PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. } => {
+                               Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
                                // Some paths were sent, even if we failed to send the full MPP value our recipient may
                                // misbehave and claim the funds, at which point we have to consider the payment sent, so
                                // return `Ok()` here, ignoring any retry errors.
-                               let retry_res = self.pay_internal(payment_id, payment_hash, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
-                               log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
-                               Ok(())
+                               self.retry_payment_internal(payment_id, payment_hash, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
                        },
-                       Err(PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. }) => {
+                       PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. } => {
                                // This may happen if we send a payment and some paths fail, but only due to a temporary
                                // monitor failure or the like, implying they're really in-flight, but we haven't sent the
                                // initial HTLC-Add messages yet.
-                               Ok(())
                        },
-                       res => res,
+                       PaymentSendFailure::PathParameterError(results) => {
+                               Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
+                               self.abandon_payment(payment_id, pending_events);
+                       },
+                       PaymentSendFailure::ParameterError(e) => {
+                               log_error!(logger, "Failed to send to route due to parameter error: {:?}. Your router is buggy", e);
+                               self.abandon_payment(payment_id, pending_events);
+                       },
+                       PaymentSendFailure::DuplicatePayment => debug_assert!(false), // unreachable
+               }
+       }
+
+       fn push_payment_path_failed_evs<I: ExactSizeIterator + Iterator<Item = Result<(), APIError>>>(
+               payment_id: PaymentId, payment_hash: PaymentHash, paths: Vec<Vec<RouteHop>>, path_results: I,
+               pending_events: &Mutex<Vec<events::Event>>
+       ) {
+               let mut events = pending_events.lock().unwrap();
+               debug_assert_eq!(paths.len(), path_results.len());
+               for (path, path_res) in paths.into_iter().zip(path_results) {
+                       if let Err(e) = path_res {
+                               let failed_scid = if let APIError::InvalidRoute { .. } = e {
+                                       None
+                               } else {
+                                       Some(path[0].short_channel_id)
+                               };
+                               events.push(events::Event::PaymentPathFailed {
+                                       payment_id: Some(payment_id),
+                                       payment_hash,
+                                       payment_failed_permanently: false,
+                                       network_update: None,
+                                       all_paths_failed: false,
+                                       path,
+                                       short_channel_id: failed_scid,
+                                       retry: None,
+                                       #[cfg(test)]
+                                       error_code: None,
+                                       #[cfg(test)]
+                                       error_data: None,
+                               });
+                       }
                }
        }
 
@@ -1243,13 +1361,13 @@ mod tests {
        use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 
        use crate::ln::PaymentHash;
-       use crate::ln::channelmanager::{PaymentId, PaymentSendFailure};
+       use crate::ln::channelmanager::PaymentId;
        use crate::ln::msgs::{ErrorAction, LightningError};
-       use crate::ln::outbound_payment::{OutboundPayments, Retry};
+       use crate::ln::outbound_payment::{OutboundPayments, Retry, RetryableSendFailure};
        use crate::routing::gossip::NetworkGraph;
        use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters};
        use crate::sync::{Arc, Mutex};
-       use crate::util::errors::APIError;
+       use crate::util::events::Event;
        use crate::util::test_utils;
 
        #[test]
@@ -1280,20 +1398,24 @@ mod tests {
                        final_cltv_expiry_delta: 0,
                };
                let pending_events = Mutex::new(Vec::new());
-               let err = if on_retry {
-                       outbound_payments.pay_internal(
-                               PaymentId([0; 32]), PaymentHash([0; 32]), None, expired_route_params, &&router, vec![],
+               if on_retry {
+                       outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]), None,
+                       &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)),
+                       Some(expired_route_params.payment_params.clone()), &&keys_manager, 0).unwrap();
+                       outbound_payments.retry_payment_internal(
+                               PaymentId([0; 32]), PaymentHash([0; 32]), expired_route_params, &&router, vec![],
                                &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
-                               &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+                               &|_, _, _, _, _, _, _, _, _| Ok(()));
+                       let events = pending_events.lock().unwrap();
+                       assert_eq!(events.len(), 1);
+                       if let Event::PaymentFailed { .. } = events[0] { } else { panic!("Unexpected event"); }
                } else {
-                       outbound_payments.send_payment(
+                       let err = outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params,
                                &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
-                               &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
-               };
-               if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
-                       assert!(err.contains("Invoice expired"));
-               } else { panic!("Unexpected error"); }
+                               &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err();
+                       if let RetryableSendFailure::PaymentExpired = err { } else { panic!("Unexpected error"); }
+               }
        }
 
        #[test]
@@ -1322,22 +1444,24 @@ mod tests {
                        Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
 
                let pending_events = Mutex::new(Vec::new());
-               let err = if on_retry {
+               if on_retry {
                        outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]), None,
                                &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)),
                                Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap();
-                       outbound_payments.pay_internal(
-                               PaymentId([0; 32]), PaymentHash([0; 32]), None, route_params, &&router, vec![],
+                       outbound_payments.retry_payment_internal(
+                               PaymentId([0; 32]), PaymentHash([0; 32]), route_params, &&router, vec![],
                                &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
-                               &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+                               &|_, _, _, _, _, _, _, _, _| Ok(()));
+                       let events = pending_events.lock().unwrap();
+                       assert_eq!(events.len(), 1);
+                       if let Event::PaymentFailed { .. } = events[0] { } else { panic!("Unexpected event"); }
                } else {
-                       outbound_payments.send_payment(
+                       let err = outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params,
                                &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
-                               &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
-               };
-               if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
-                       assert!(err.contains("Failed to find a route"));
-               } else { panic!("Unexpected error"); }
+                               &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err();
+                       if let RetryableSendFailure::RouteNotFound = err {
+                       } else { panic!("Unexpected error"); }
+               }
        }
 }
index 3285cf4d7a5b41de63766f1c92776bd19f01d364..7c90f5601049dd515671d0a5cfb75638d36240c9 100644 (file)
@@ -1869,15 +1869,23 @@ fn auto_retry_partial_failure() {
        // Send a payment that will partially fail on send, then partially fail on retry, then succeed.
        nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(3)).unwrap();
        let closed_chan_events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(closed_chan_events.len(), 2);
+       assert_eq!(closed_chan_events.len(), 4);
        match closed_chan_events[0] {
                Event::ChannelClosed { .. } => {},
                _ => panic!("Unexpected event"),
        }
        match closed_chan_events[1] {
+               Event::PaymentPathFailed { .. } => {},
+               _ => panic!("Unexpected event"),
+       }
+       match closed_chan_events[2] {
                Event::ChannelClosed { .. } => {},
                _ => panic!("Unexpected event"),
        }
+       match closed_chan_events[3] {
+               Event::PaymentPathFailed { .. } => {},
+               _ => panic!("Unexpected event"),
+       }
 
        // Pass the first part of the payment along the path.
        check_added_monitors!(nodes[0], 5); // three outbound channel updates succeeded, two permanently failed
@@ -1993,11 +2001,13 @@ fn auto_retry_zero_attempts_send_error() {
        };
 
        chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-       let err = nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap_err();
-       if let PaymentSendFailure::AllFailedResendSafe(_) = err {
-       } else { panic!("Unexpected error"); }
+       nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
        assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 2); // channel close messages
-       assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 1); // channel close event
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 3);
+       if let Event::ChannelClosed { .. } = events[0] { } else { panic!(); }
+       if let Event::PaymentPathFailed { .. } = events[1] { } else { panic!(); }
+       if let Event::PaymentFailed { .. } = events[2] { } else { panic!(); }
        check_added_monitors!(nodes[0], 2);
 }
 
@@ -2121,6 +2131,16 @@ fn retry_multi_path_single_failed_payment() {
        }
 
        nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
+                       network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
+                       assert_eq!(payment_hash, ev_payment_hash);
+                       assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
+               },
+               _ => panic!("Unexpected event"),
+       }
        let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(htlc_msgs.len(), 2);
        check_added_monitors!(nodes[0], 2);
@@ -2182,6 +2202,16 @@ fn immediate_retry_on_failure() {
                }, Ok(route.clone()));
 
        nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
+       let events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(events.len(), 1);
+       match events[0] {
+               Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
+               network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
+                       assert_eq!(payment_hash, ev_payment_hash);
+                       assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
+               },
+               _ => panic!("Unexpected event"),
+       }
        let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(htlc_msgs.len(), 2);
        check_added_monitors!(nodes[0], 2);