From 948ebc67f01a6b17c04687c8c4b35431d01192e3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 16:53:57 -0500 Subject: [PATCH 01/12] Copy PaymentAttempts from invoice module to outbound_payment module --- lightning/src/ln/outbound_payment.rs | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 161f6ecc14b..a4def55cfeb 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -22,9 +22,14 @@ use crate::routing::router::{PaymentParameters, Route, RouteHop, RouteParameters use crate::util::errors::APIError; use crate::util::events; use crate::util::logger::Logger; +use crate::util::time::Time; +#[cfg(all(not(feature = "no-std"), test))] +use crate::util::time::tests::SinceEpoch; use core::cmp; +use core::fmt::{self, Display, Formatter}; use core::ops::Deref; + use crate::prelude::*; use crate::sync::Mutex; @@ -182,6 +187,48 @@ impl PendingOutboundPayment { } } +pub(crate) type PaymentAttempts = PaymentAttemptsUsingTime; + +/// Storing minimal payment attempts information required for determining if a outbound payment can +/// be retried. +pub(crate) struct PaymentAttemptsUsingTime { + /// This count will be incremented only after the result of the attempt is known. When it's 0, + /// it means the result of the first attempt is not known yet. + pub(crate) count: usize, + /// This field is only used when retry is `Retry::Timeout` which is only build with feature std + first_attempted_at: T +} + +#[cfg(not(any(feature = "no-std", test)))] +type ConfiguredTime = std::time::Instant; +#[cfg(feature = "no-std")] +type ConfiguredTime = crate::util::time::Eternity; +#[cfg(all(not(feature = "no-std"), test))] +type ConfiguredTime = SinceEpoch; + +impl PaymentAttemptsUsingTime { + pub(crate) fn new() -> Self { + PaymentAttemptsUsingTime { + count: 0, + first_attempted_at: T::now() + } + } +} + +impl Display for PaymentAttemptsUsingTime { + fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { + #[cfg(feature = "no-std")] + return write!(f, "attempts: {}", self.count); + #[cfg(not(feature = "no-std"))] + return write!( + f, + "attempts: {}, duration: {}s", + self.count, + T::now().duration_since(self.first_attempted_at).as_secs() + ); + } +} + /// 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. From cbb75e27b22c443e625a983134666c28675112e3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 17:02:17 -0500 Subject: [PATCH 02/12] Copy Retry from invoice module to outbound_payment module Also configure it such that in std tests, it will use SinceEpoch instead of Instant so time can be manually advanced. --- lightning/src/ln/outbound_payment.rs | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a4def55cfeb..a9c1e393d2b 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -187,6 +187,36 @@ impl PendingOutboundPayment { } } +/// Strategies available to retry payment path failures. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum Retry { + /// Max number of attempts to retry payment. + /// + /// Note that this is the number of *path* failures, not full payment retries. For multi-path + /// payments, if this is less than the total number of paths, we will never even retry all of the + /// payment's paths. + Attempts(usize), + #[cfg(not(feature = "no-std"))] + /// Time elapsed before abandoning retries for a payment. + Timeout(core::time::Duration), +} + +impl Retry { + pub(crate) fn is_retryable_now(&self, attempts: &PaymentAttempts) -> bool { + match (self, attempts) { + (Retry::Attempts(max_retry_count), PaymentAttempts { count, .. }) => { + max_retry_count > count + }, + #[cfg(all(not(feature = "no-std"), not(test)))] + (Retry::Timeout(max_duration), PaymentAttempts { first_attempted_at, .. }) => + *max_duration >= std::time::Instant::now().duration_since(*first_attempted_at), + #[cfg(all(not(feature = "no-std"), test))] + (Retry::Timeout(max_duration), PaymentAttempts { first_attempted_at, .. }) => + *max_duration >= SinceEpoch::now().duration_since(*first_attempted_at), + } + } +} + pub(crate) type PaymentAttempts = PaymentAttemptsUsingTime; /// Storing minimal payment attempts information required for determining if a outbound payment can From ae3453393227f932eef574735f8feee4278eccc8 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 17:13:47 -0500 Subject: [PATCH 03/12] Parameterize add_new_pending_payment with retry strategy and route params --- lightning/src/ln/channelmanager.rs | 4 ++-- lightning/src/ln/outbound_payment.rs | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d08f8da5558..ba54827d9a7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -53,7 +53,7 @@ use crate::ln::onion_utils::HTLCFailReason; use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT}; #[cfg(test)] use crate::ln::outbound_payment; -use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment}; +use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment, Retry}; use crate::ln::wire::Encode; use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, Sign, SignerProvider}; use crate::util::config::{UserConfig, ChannelConfig}; @@ -2457,7 +2457,7 @@ where #[cfg(test)] pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, route: &Route) -> Result, PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); - self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, &self.entropy_source, best_block_height) + self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, Retry::Attempts(0), &self.entropy_source, best_block_height) } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a9c1e393d2b..e230002a04e 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -359,7 +359,7 @@ impl OutboundPayments { F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { - let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?; self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) } @@ -378,7 +378,7 @@ impl OutboundPayments { None => PaymentPreimage(entropy_source.get_secure_random_bytes()), }; let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner()); - let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { Ok(()) => Ok(payment_hash), @@ -477,7 +477,7 @@ impl OutboundPayments { } let route = Route { paths: vec![hops], payment_params: None }; - let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { Ok(()) => Ok((payment_hash, payment_id)), @@ -488,14 +488,15 @@ impl OutboundPayments { #[cfg(test)] pub(super) fn test_add_new_pending_payment( &self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, - route: &Route, entropy_source: &ES, best_block_height: u32 + route: &Route, retry_strategy: Retry, entropy_source: &ES, best_block_height: u32 ) -> Result, PaymentSendFailure> where ES::Target: EntropySource { - self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, entropy_source, best_block_height) + self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, retry_strategy, None, entropy_source, best_block_height) } pub(super) fn add_new_pending_payment( &self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, - route: &Route, entropy_source: &ES, best_block_height: u32 + route: &Route, retry_strategy: Retry, route_params: Option, + entropy_source: &ES, best_block_height: u32 ) -> Result, PaymentSendFailure> where ES::Target: EntropySource { let mut onion_session_privs = Vec::with_capacity(route.paths.len()); for _ in 0..route.paths.len() { From c0a22f717432cbf7f0c0cf60f3a247588617b7ae Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 18:22:56 -0500 Subject: [PATCH 04/12] Store retry data in PendingOutboundPayment::Retryable Used in upcoming commit(s) to automatically retry HTLCs in ChannelManager --- lightning/src/ln/channelmanager.rs | 5 ++++- lightning/src/ln/outbound_payment.rs | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ba54827d9a7..b18ddaeba6b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -53,7 +53,7 @@ use crate::ln::onion_utils::HTLCFailReason; use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT}; #[cfg(test)] use crate::ln::outbound_payment; -use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment, Retry}; +use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, Retry}; use crate::ln::wire::Encode; use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, Sign, SignerProvider}; use crate::util::config::{UserConfig, ChannelConfig}; @@ -7278,6 +7278,9 @@ where hash_map::Entry::Vacant(entry) => { let path_fee = path.get_path_fees(); entry.insert(PendingOutboundPayment::Retryable { + retry_strategy: Retry::Attempts(0), + attempts: PaymentAttempts::new(), + route_params: None, session_privs: [session_priv_bytes].iter().map(|a| *a).collect(), payment_hash: htlc.payment_hash, payment_secret, diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index e230002a04e..a4211ea34d6 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -40,6 +40,9 @@ pub(crate) enum PendingOutboundPayment { session_privs: HashSet<[u8; 32]>, }, Retryable { + retry_strategy: Retry, + attempts: PaymentAttempts, + route_params: Option, session_privs: HashSet<[u8; 32]>, payment_hash: PaymentHash, payment_secret: Option, @@ -73,6 +76,17 @@ pub(crate) enum PendingOutboundPayment { } impl PendingOutboundPayment { + fn increment_attempts(&mut self) { + if let PendingOutboundPayment::Retryable { attempts, .. } = self { + attempts.count += 1; + } + } + fn is_retryable_now(&self) -> bool { + if let PendingOutboundPayment::Retryable { retry_strategy, attempts, .. } = self { + return retry_strategy.is_retryable_now(&attempts) + } + false + } pub(super) fn is_fulfilled(&self) -> bool { match self { PendingOutboundPayment::Fulfilled { .. } => true, @@ -508,6 +522,9 @@ impl OutboundPayments { hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment), hash_map::Entry::Vacant(entry) => { let payment = entry.insert(PendingOutboundPayment::Retryable { + retry_strategy, + attempts: PaymentAttempts::new(), + route_params, session_privs: HashSet::new(), pending_amt_msat: 0, pending_fee_msat: Some(0), @@ -911,8 +928,11 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (0, session_privs, required), (1, pending_fee_msat, option), (2, payment_hash, required), + (not_written, retry_strategy, (static_value, Retry::Attempts(0))), (4, payment_secret, option), + (not_written, attempts, (static_value, PaymentAttempts::new())), (6, total_msat, required), + (not_written, route_params, (static_value, None)), (8, pending_amt_msat, required), (10, starting_block_height, required), }, From 6351a99935e81320abefb4c7e51c05c5fc8a62df Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 24 Jan 2023 14:26:57 -0500 Subject: [PATCH 05/12] Decode onion fail outside of outbound_payments lock It's not ideal to do all this computation while the lock is held. We also want to decode the failure *before* taking the lock, so we can store the failed scid in the relevant outbound for retry in the next commit(s). --- lightning/src/ln/outbound_payment.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a4211ea34d6..0a5a964b33b 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -783,6 +783,11 @@ impl OutboundPayments { payment_params: &Option, probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1, pending_events: &Mutex>, logger: &L ) where L::Target: Logger { + #[cfg(test)] + let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source); + #[cfg(not(test))] + let (network_update, short_channel_id, payment_retryable, _, _) = onion_error.decode_onion_failure(secp_ctx, logger, &source); + let mut session_priv_bytes = [0; 32]; session_priv_bytes.copy_from_slice(&session_priv[..]); let mut outbounds = self.pending_outbound_payments.lock().unwrap(); @@ -811,6 +816,7 @@ impl OutboundPayments { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); return } + core::mem::drop(outbounds); let mut retry = if let Some(payment_params_data) = payment_params { let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); Some(RouteParameters { @@ -822,11 +828,6 @@ impl OutboundPayments { log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0)); let path_failure = { - #[cfg(test)] - let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source); - #[cfg(not(test))] - let (network_update, short_channel_id, payment_retryable, _, _) = onion_error.decode_onion_failure(secp_ctx, logger, &source); - if payment_is_probe(payment_hash, &payment_id, probing_cookie_secret) { if !payment_retryable { events::Event::ProbeSuccessful { From 686ef083162758f980786b3e5a85f4412cd4b847 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 6 Jan 2023 18:39:51 -0500 Subject: [PATCH 06/12] Generate PendingHTLCsForwardable upon retryable payment --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/outbound_payment.rs | 25 ++++++++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b18ddaeba6b..9cf58c91825 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -388,7 +388,7 @@ impl MsgHandleErrInternal { /// Event::PendingHTLCsForwardable for the API guidelines indicating how long should be waited). /// This provides some limited amount of privacy. Ideally this would range from somewhere like one /// second to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly. -const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100; +pub(super) const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100; /// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should /// be sent in the order they appear in the return value, however sometimes the order needs to be diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 0a5a964b33b..a971ea8b7f0 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -15,7 +15,7 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::ln::channelmanager::{HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId}; +use crate::ln::channelmanager::{HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId}; use crate::ln::msgs::DecodeError; use crate::ln::onion_utils::HTLCFailReason; use crate::routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, RoutePath}; @@ -29,6 +29,7 @@ use crate::util::time::tests::SinceEpoch; use core::cmp; use core::fmt::{self, Display, Formatter}; use core::ops::Deref; +use core::time::Duration; use crate::prelude::*; use crate::sync::Mutex; @@ -87,6 +88,11 @@ impl PendingOutboundPayment { } false } + pub fn insert_previously_failed_scid(&mut self, scid: u64) { + if let PendingOutboundPayment::Retryable { route_params: Some(params), .. } = self { + params.payment_params.previously_failed_channels.push(scid); + } + } pub(super) fn is_fulfilled(&self) -> bool { match self { PendingOutboundPayment::Fulfilled { .. } => true, @@ -793,7 +799,8 @@ impl OutboundPayments { 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) { + let mut pending_retry_ev = None; + let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) { if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); return @@ -802,6 +809,10 @@ impl OutboundPayments { log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0)); return } + let is_retryable_now = payment.get().is_retryable_now(); + if let Some(scid) = short_channel_id { + payment.get_mut().insert_previously_failed_scid(scid); + } if payment.get().remaining_parts() == 0 { all_paths_failed = true; if payment.get().abandoned() { @@ -812,10 +823,11 @@ impl OutboundPayments { payment.remove(); } } + is_retryable_now } else { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); return - } + }; core::mem::drop(outbounds); let mut retry = if let Some(payment_params_data) = payment_params { let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); @@ -850,6 +862,12 @@ impl OutboundPayments { if let Some(scid) = short_channel_id { retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid)); } + if payment_retryable && attempts_remaining && retry.is_some() { + debug_assert!(full_failure_ev.is_none()); + pending_retry_ev = Some(events::Event::PendingHTLCsForwardable { + time_forwardable: Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS), + }); + } events::Event::PaymentPathFailed { payment_id: Some(*payment_id), payment_hash: payment_hash.clone(), @@ -869,6 +887,7 @@ impl OutboundPayments { let mut pending_events = pending_events.lock().unwrap(); pending_events.push(path_failure); if let Some(ev) = full_failure_ev { pending_events.push(ev); } + if let Some(ev) = pending_retry_ev { pending_events.push(ev); } } pub(super) fn abandon_payment(&self, payment_id: PaymentId) -> Option { From 72a7da8d5175f656d6e4b73b7ab7f5b2ee9a89f5 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 23:29:45 -0500 Subject: [PATCH 07/12] Remove AllPathsFailed outbounds at send_payment_internal callsites instead This makes it easier to retry payments if all paths fail on initial send, in in which case we'll want to hold off on removing the pending payment --- lightning/src/ln/outbound_payment.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a971ea8b7f0..c2d0a6b62c9 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -380,7 +380,9 @@ impl OutboundPayments { u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) + self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, + onion_session_privs, node_signer, best_block_height, send_payment_along_path) + .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) } pub(super) fn send_spontaneous_payment( @@ -402,7 +404,10 @@ impl OutboundPayments { match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { Ok(()) => Ok(payment_hash), - Err(e) => Err(e) + Err(e) => { + self.remove_outbound_if_all_failed(payment_id, &e); + Err(e) + } } } @@ -501,7 +506,10 @@ impl OutboundPayments { match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { Ok(()) => Ok((payment_hash, payment_id)), - Err(e) => Err(e) + Err(e) => { + self.remove_outbound_if_all_failed(payment_id, &e); + Err(e) + } } } @@ -648,10 +656,6 @@ impl OutboundPayments { } else { None }, }) } else if has_err { - // If we failed to send any paths, we should remove the new PaymentId from the - // `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`. - let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some(); - debug_assert!(removed, "We should always have a pending payment to remove here"); Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect())) } else { Ok(()) @@ -673,6 +677,16 @@ impl OutboundPayments { self.send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, node_signer, best_block_height, send_payment_along_path) + .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) + } + + // If we failed to send any paths, we should remove the new PaymentId from the + // `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`. + fn remove_outbound_if_all_failed(&self, payment_id: PaymentId, err: &PaymentSendFailure) { + if let &PaymentSendFailure::AllFailedResendSafe(_) = err { + let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some(); + debug_assert!(removed, "We should always have a pending payment to remove here"); + } } pub(super) fn claim_htlc( From d776dee3a553ee9ba3d2ce217207f42f10ed091f Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 6 Jan 2023 19:39:40 -0500 Subject: [PATCH 08/12] Retry HTLCs in process_pending_htlc_forwards --- lightning/src/ln/channelmanager.rs | 6 ++ lightning/src/ln/outbound_payment.rs | 117 ++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9cf58c91825..8dc3059d485 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3250,6 +3250,12 @@ where } } + let best_block_height = self.best_block.read().unwrap().height(); + self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(), + || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, &self.logger, + |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)); + for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) { self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination); } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index c2d0a6b62c9..4b6278d1949 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -15,10 +15,10 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::ln::channelmanager::{HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId}; +use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId}; use crate::ln::msgs::DecodeError; use crate::ln::onion_utils::HTLCFailReason; -use crate::routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, RoutePath}; +use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router}; use crate::util::errors::APIError; use crate::util::events; use crate::util::logger::Logger; @@ -237,6 +237,16 @@ impl Retry { } } +#[cfg(feature = "std")] +pub(super) fn has_expired(route_params: &RouteParameters) -> bool { + if let Some(expiry_time) = route_params.payment_params.expiry_time { + if let Ok(elapsed) = std::time::SystemTime::UNIX_EPOCH.elapsed() { + return elapsed > core::time::Duration::from_secs(expiry_time) + } + } + false +} + pub(crate) type PaymentAttempts = PaymentAttemptsUsingTime; /// Storing minimal payment attempts information required for determining if a outbound payment can @@ -411,6 +421,109 @@ impl OutboundPayments { } } + pub(super) fn check_retry_payments( + &self, router: &R, first_hops: FH, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, + best_block_height: u32, logger: &L, send_payment_along_path: SP, + ) + where + R::Target: Router, + ES::Target: EntropySource, + NS::Target: NodeSigner, + SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, + u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, + IH: Fn() -> InFlightHtlcs, + FH: Fn() -> Vec, + L::Target: Logger, + { + loop { + let mut outbounds = self.pending_outbound_payments.lock().unwrap(); + let mut retry_id_route_params = None; + for (pmt_id, pmt) in outbounds.iter_mut() { + if pmt.is_retryable_now() { + if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, route_params: Some(params), .. } = pmt { + if pending_amt_msat < total_msat { + retry_id_route_params = Some((*pmt_id, params.clone())); + pmt.increment_attempts(); + break + } + } + } + } + if let Some((payment_id, route_params)) = retry_id_route_params { + core::mem::drop(outbounds); + if let Err(e) = self.pay_internal(payment_id, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, &send_payment_along_path) { + log_trace!(logger, "Errored retrying payment: {:?}", e); + } + } else { break } + } + } + + fn pay_internal( + &self, payment_id: PaymentId, route_params: RouteParameters, router: &R, + first_hops: Vec, inflight_htlcs: InFlightHtlcs, entropy_source: &ES, + node_signer: &NS, best_block_height: u32, send_payment_along_path: &F + ) -> Result<(), PaymentSendFailure> + where + R::Target: Router, + ES::Target: EntropySource, + NS::Target: NodeSigner, + F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, + u32, PaymentId, &Option, [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)), + })) + } + } + + let route = router.find_route( + &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params, + Some(&first_hops.iter().collect::>()), &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 = self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path); + match res { + Err(PaymentSendFailure::AllFailedResendSafe(_)) => { + let mut outbounds = self.pending_outbound_payments.lock().unwrap(); + if let Some(payment) = outbounds.get_mut(&payment_id) { + let retryable = payment.is_retryable_now(); + if retryable { + payment.increment_attempts(); + } else { return res } + } else { return res } + core::mem::drop(outbounds); + self.pay_internal(payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path) + }, + Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => { + let mut outbounds = self.pending_outbound_payments.lock().unwrap(); + if let Some(payment) = outbounds.get_mut(&payment_id) { + let retryable = payment.is_retryable_now(); + if retryable { + payment.increment_attempts(); + } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) } + } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) } + core::mem::drop(outbounds); + + // 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 _ = self.pay_internal(payment_id, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path); + Ok(()) + }, + Err(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, + } + } + pub(super) fn retry_payment_with_route( &self, route: &Route, payment_id: PaymentId, entropy_source: &ES, node_signer: &NS, best_block_height: u32, send_payment_along_path: F From acf9292d585fe558a33a2192e2e5e025f0c1edb2 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 19 Dec 2022 00:19:47 -0500 Subject: [PATCH 09/12] Support sending payments with a retry strategy in ChannelManager --- lightning/src/ln/channelmanager.rs | 14 +++++++- lightning/src/ln/outbound_payment.rs | 52 +++++++++++++++++++++------- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8dc3059d485..21fba778791 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -45,7 +45,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No #[cfg(any(feature = "_test_utils", test))] use crate::ln::features::InvoiceFeatures; use crate::routing::gossip::NetworkGraph; -use crate::routing::router::{DefaultRouter, InFlightHtlcs, PaymentParameters, Route, RouteHop, RoutePath, Router}; +use crate::routing::router::{DefaultRouter, InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router}; use crate::routing::scoring::ProbabilisticScorer; use crate::ln::msgs; use crate::ln::onion_utils; @@ -2446,6 +2446,18 @@ where self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } + /// 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, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), PaymentSendFailure> { + 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, + &self.router, self.list_usable_channels(), self.compute_inflight_htlcs(), + &self.entropy_source, &self.node_signer, best_block_height, + |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + } + #[cfg(test)] fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 4b6278d1949..65e9029d49d 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -378,6 +378,25 @@ impl OutboundPayments { } } + pub(super) fn send_payment( + &self, payment_hash: PaymentHash, payment_secret: &Option, payment_id: PaymentId, + retry_strategy: Retry, route_params: RouteParameters, router: &R, + first_hops: Vec, inflight_htlcs: InFlightHtlcs, entropy_source: &ES, + node_signer: &NS, best_block_height: u32, send_payment_along_path: F + ) -> Result<(), PaymentSendFailure> + where + R::Target: Router, + ES::Target: EntropySource, + NS::Target: NodeSigner, + F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, + u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, + { + self.pay_internal(payment_id, Some((payment_hash, payment_secret, retry_strategy)), + route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, + best_block_height, &send_payment_along_path) + .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) + } + pub(super) fn send_payment_with_route( &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, payment_id: PaymentId, entropy_source: &ES, node_signer: &NS, best_block_height: u32, @@ -391,7 +410,7 @@ impl OutboundPayments { { let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?; self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, - onion_session_privs, node_signer, best_block_height, send_payment_along_path) + onion_session_privs, node_signer, best_block_height, &send_payment_along_path) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) } @@ -412,7 +431,7 @@ impl OutboundPayments { let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner()); let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { + match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok(payment_hash), Err(e) => { self.remove_outbound_if_all_failed(payment_id, &e); @@ -451,7 +470,7 @@ impl OutboundPayments { } if let Some((payment_id, route_params)) = retry_id_route_params { core::mem::drop(outbounds); - if let Err(e) = self.pay_internal(payment_id, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, &send_payment_along_path) { + if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, &send_payment_along_path) { log_trace!(logger, "Errored retrying payment: {:?}", e); } } else { break } @@ -459,9 +478,11 @@ impl OutboundPayments { } fn pay_internal( - &self, payment_id: PaymentId, route_params: RouteParameters, router: &R, - first_hops: Vec, inflight_htlcs: InFlightHtlcs, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, send_payment_along_path: &F + &self, payment_id: PaymentId, + initial_send_info: Option<(PaymentHash, &Option, Retry)>, + route_params: RouteParameters, router: &R, first_hops: Vec, + inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32, + send_payment_along_path: &F ) -> Result<(), PaymentSendFailure> where R::Target: Router, @@ -485,7 +506,12 @@ impl OutboundPayments { err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound }))?; - let res = self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path); + let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info { + let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy, Some(route_params.clone()), entropy_source, best_block_height)?; + self.send_payment_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) + }; match res { Err(PaymentSendFailure::AllFailedResendSafe(_)) => { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); @@ -496,7 +522,7 @@ impl OutboundPayments { } else { return res } } else { return res } core::mem::drop(outbounds); - self.pay_internal(payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path) + self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path) }, Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); @@ -511,7 +537,7 @@ impl OutboundPayments { // 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 _ = self.pay_internal(payment_id, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path); + let _ = self.pay_internal(payment_id, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path); Ok(()) }, Err(PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. }) => { @@ -591,7 +617,7 @@ impl OutboundPayments { })), } }; - self.send_payment_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, send_payment_along_path) + self.send_payment_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, &send_payment_along_path) } pub(super) fn send_probe( @@ -617,7 +643,7 @@ impl OutboundPayments { let route = Route { paths: vec![hops], payment_params: None }; let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) { + match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok((payment_hash, payment_id)), Err(e) => { self.remove_outbound_if_all_failed(payment_id, &e); @@ -674,7 +700,7 @@ impl OutboundPayments { &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>, node_signer: &NS, best_block_height: u32, - send_payment_along_path: F + send_payment_along_path: &F ) -> Result<(), PaymentSendFailure> where NS::Target: NodeSigner, @@ -789,7 +815,7 @@ impl OutboundPayments { { self.send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, node_signer, best_block_height, - send_payment_along_path) + &send_payment_along_path) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) } From 2f49c8170c35579102d23aa3c090fa35e13ea2bd Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 19 Dec 2022 00:38:54 -0500 Subject: [PATCH 10/12] Test ChannelManager automatic retries --- lightning/src/ln/outbound_payment.rs | 96 ++++++ lightning/src/ln/payment_tests.rs | 490 ++++++++++++++++++++++++++- lightning/src/util/test_utils.rs | 22 +- 3 files changed, 606 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 65e9029d49d..2a6ff04abef 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -1114,3 +1114,99 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (2, payment_hash, required), }, ); + +#[cfg(test)] +mod tests { + use bitcoin::blockdata::constants::genesis_block; + use bitcoin::network::constants::Network; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + + use crate::ln::PaymentHash; + use crate::ln::channelmanager::{PaymentId, PaymentSendFailure}; + use crate::ln::msgs::{ErrorAction, LightningError}; + use crate::ln::outbound_payment::{OutboundPayments, Retry}; + use crate::routing::gossip::NetworkGraph; + use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters}; + use crate::sync::Arc; + use crate::util::errors::APIError; + use crate::util::test_utils; + + #[test] + #[cfg(feature = "std")] + fn fails_paying_after_expiration() { + do_fails_paying_after_expiration(false); + do_fails_paying_after_expiration(true); + } + #[cfg(feature = "std")] + fn do_fails_paying_after_expiration(on_retry: bool) { + let outbound_payments = OutboundPayments::new(); + let logger = test_utils::TestLogger::new(); + let genesis_hash = genesis_block(Network::Testnet).header.block_hash(); + let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger)); + let router = test_utils::TestRouter::new(network_graph); + let secp_ctx = Secp256k1::new(); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let past_expiry_time = std::time::SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() - 2; + let payment_params = PaymentParameters::from_node_id( + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap())) + .with_expiry_time(past_expiry_time); + let expired_route_params = RouteParameters { + payment_params, + final_value_msat: 0, + final_cltv_expiry_delta: 0, + }; + let err = if on_retry { + outbound_payments.pay_internal( + PaymentId([0; 32]), None, expired_route_params, &&router, vec![], InFlightHtlcs::new(), + &&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + } else { + 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, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + }; + if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err { + assert!(err.contains("Invoice expired")); + } else { panic!("Unexpected error"); } + } + + #[test] + fn find_route_error() { + do_find_route_error(false); + do_find_route_error(true); + } + fn do_find_route_error(on_retry: bool) { + let outbound_payments = OutboundPayments::new(); + let logger = test_utils::TestLogger::new(); + let genesis_hash = genesis_block(Network::Testnet).header.block_hash(); + let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger)); + let router = test_utils::TestRouter::new(network_graph); + let secp_ctx = Secp256k1::new(); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + router.expect_find_route(Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })); + + let payment_params = PaymentParameters::from_node_id( + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap())); + let route_params = RouteParameters { + payment_params, + final_value_msat: 0, + final_cltv_expiry_delta: 0, + }; + let err = if on_retry { + outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]), + &Route { paths: vec![], payment_params: None }, Retry::Attempts(1), Some(route_params.clone()), + &&keys_manager, 0).unwrap(); + outbound_payments.pay_internal( + PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(), + &&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + } else { + 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, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + }; + if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err { + assert!(err.contains("Failed to find a route")); + } else { panic!("Unexpected error"); } + } +} diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 2a9a180f3a9..086370eb74e 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -17,10 +17,12 @@ use crate::chain::keysinterface::EntropySource; use crate::chain::transaction::OutPoint; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; +use crate::ln::features::InvoiceFeatures; use crate::ln::msgs; use crate::ln::msgs::ChannelMessageHandler; +use crate::ln::outbound_payment::Retry; use crate::routing::gossip::RoutingFees; -use crate::routing::router::{get_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters}; +use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters}; use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider}; use crate::util::test_utils; use crate::util::errors::APIError; @@ -34,6 +36,11 @@ use crate::prelude::*; use crate::ln::functional_test_utils::*; use crate::routing::gossip::NodeId; +#[cfg(feature = "std")] +use { + crate::util::time::tests::SinceEpoch, + std::time::{SystemTime, Duration} +}; #[test] fn retry_single_path_payment() { @@ -1575,3 +1582,484 @@ fn do_test_intercepted_payment(test: InterceptTest) { assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.0)) }); } } + +#[derive(PartialEq)] +enum AutoRetry { + Success, + FailAttempts, + FailTimeout, + FailOnRestart, +} + +#[test] +fn automatic_retries() { + do_automatic_retries(AutoRetry::Success); + do_automatic_retries(AutoRetry::FailAttempts); + do_automatic_retries(AutoRetry::FailTimeout); + do_automatic_retries(AutoRetry::FailOnRestart); +} +fn do_automatic_retries(test: AutoRetry) { + // Test basic automatic payment retries in ChannelManager. See individual `test` variant comments + // below. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + + let persister; + let new_chain_monitor; + let node_0_deserialized; + + let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); + let channel_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2; + let channel_id_2 = create_announced_chan_between_nodes(&nodes, 2, 1).2; + + // Marshall data to send the payment + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let amt_msat = 1000; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat); + + macro_rules! pass_failed_attempt_with_retry_along_path { + ($failing_channel_id: expr, $expect_pending_htlcs_forwardable: expr) => { + // Send a payment attempt that fails due to lack of liquidity on the second hop + check_added_monitors!(nodes[0], 1); + let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + let mut update_add = update_0.update_add_htlcs[0].clone(); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add); + commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true); + expect_pending_htlcs_forwardable_ignore!(nodes[1]); + nodes[1].node.process_pending_htlc_forwards(); + expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], + vec![HTLCDestination::NextHopChannel { + node_id: Some(nodes[2].node.get_our_node_id()), + channel_id: $failing_channel_id, + }]); + nodes[1].node.process_pending_htlc_forwards(); + let update_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + check_added_monitors!(&nodes[1], 1); + assert!(update_1.update_fail_htlcs.len() == 1); + let fail_msg = update_1.update_fail_htlcs[0].clone(); + + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg); + commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false); + + // Ensure the attempt fails and a new PendingHTLCsForwardable event is generated for the retry + let mut events = nodes[0].node.get_and_clear_pending_events(); + match events[0] { + Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => { + assert_eq!(payment_hash, ev_payment_hash); + assert_eq!(payment_failed_permanently, false); + }, + _ => panic!("Unexpected event"), + } + if $expect_pending_htlcs_forwardable { + assert_eq!(events.len(), 2); + match events[1] { + Event::PendingHTLCsForwardable { .. } => {}, + _ => panic!("Unexpected event"), + } + } else { assert_eq!(events.len(), 1) } + } + } + + if test == AutoRetry::Success { + // Test that we can succeed on the first retry. + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + pass_failed_attempt_with_retry_along_path!(channel_id_2, true); + + // Open a new channel with liquidity on the second hop so we can find a route for the retry + // attempt, since the initial second hop channel will be excluded from pathfinding + create_announced_chan_between_nodes(&nodes, 1, 2); + + // We retry payments in `process_pending_htlc_forwards` + nodes[0].node.process_pending_htlc_forwards(); + check_added_monitors!(nodes[0], 1); + let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], amt_msat, payment_hash, Some(payment_secret), msg_events.pop().unwrap(), true, None); + claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage); + } else if test == AutoRetry::FailAttempts { + // Ensure ChannelManager will not retry a payment if it has run out of payment attempts. + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + pass_failed_attempt_with_retry_along_path!(channel_id_2, true); + + // Open a new channel with no liquidity on the second hop so we can find a (bad) route for + // the retry attempt, since the initial second hop channel will be excluded from pathfinding + let channel_id_3 = create_announced_chan_between_nodes(&nodes, 2, 1).2; + + // We retry payments in `process_pending_htlc_forwards` + nodes[0].node.process_pending_htlc_forwards(); + pass_failed_attempt_with_retry_along_path!(channel_id_3, false); + + // Ensure we won't retry a second time. + nodes[0].node.process_pending_htlc_forwards(); + let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 0); + + nodes[0].node.abandon_payment(PaymentId(payment_hash.0)); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => { + assert_eq!(payment_hash, *ev_payment_hash); + assert_eq!(PaymentId(payment_hash.0), *ev_payment_id); + }, + _ => panic!("Unexpected event"), + } + } else if test == AutoRetry::FailTimeout { + #[cfg(not(feature = "no-std"))] { + // Ensure ChannelManager will not retry a payment if it times out due to Retry::Timeout. + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Timeout(Duration::from_secs(60))).unwrap(); + pass_failed_attempt_with_retry_along_path!(channel_id_2, true); + + // Advance the time so the second attempt fails due to timeout. + SinceEpoch::advance(Duration::from_secs(61)); + + // Make sure we don't retry again. + nodes[0].node.process_pending_htlc_forwards(); + let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 0); + + nodes[0].node.abandon_payment(PaymentId(payment_hash.0)); + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => { + assert_eq!(payment_hash, *ev_payment_hash); + assert_eq!(PaymentId(payment_hash.0), *ev_payment_id); + }, + _ => panic!("Unexpected event"), + } + } + } else if test == AutoRetry::FailOnRestart { + // Ensure ChannelManager will not retry a payment after restart, even if there were retry + // attempts remaining prior to restart. + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(2)).unwrap(); + pass_failed_attempt_with_retry_along_path!(channel_id_2, true); + + // Open a new channel with no liquidity on the second hop so we can find a (bad) route for + // the retry attempt, since the initial second hop channel will be excluded from pathfinding + let channel_id_3 = create_announced_chan_between_nodes(&nodes, 2, 1).2; + + // Ensure the first retry attempt fails, with 1 retry attempt remaining + nodes[0].node.process_pending_htlc_forwards(); + pass_failed_attempt_with_retry_along_path!(channel_id_3, true); + + // Restart the node and ensure that ChannelManager does not use its remaining retry attempt + let node_encoded = nodes[0].node.encode(); + let chan_1_monitor_serialized = get_monitor!(nodes[0], channel_id_1).encode(); + reload_node!(nodes[0], node_encoded, &[&chan_1_monitor_serialized], persister, new_chain_monitor, node_0_deserialized); + + // Make sure we don't retry again. + nodes[0].node.process_pending_htlc_forwards(); + let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 0); + + nodes[0].node.abandon_payment(PaymentId(payment_hash.0)); + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => { + assert_eq!(payment_hash, *ev_payment_hash); + assert_eq!(PaymentId(payment_hash.0), *ev_payment_id); + }, + _ => panic!("Unexpected event"), + } + } +} + +#[test] +fn auto_retry_partial_failure() { + // Test that we'll retry appropriately on send partial failure and retry partial failure. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + let chan_3_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + + // Marshall data to send the payment + let amt_msat = 20_000; + let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + // Ensure the first monitor update (for the initial send path1 over chan_1) succeeds, but the + // second (for the initial send path2 over chan_2) fails. + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure); + // Ensure third monitor update (for the retry1's path1 over chan_1) succeeds, but the fourth (for + // the retry1's path2 over chan_3) fails, and monitor updates succeed after that. + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + + // Configure the initial send, retry1 and retry2's paths. + let send_route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_id, + channel_features: nodes[1].node.channel_features(), + fee_msat: amt_msat / 2, + cltv_expiry_delta: 100, + }], + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_2_id, + channel_features: nodes[1].node.channel_features(), + fee_msat: amt_msat / 2, + cltv_expiry_delta: 100, + }], + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + }; + let retry_1_route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_id, + channel_features: nodes[1].node.channel_features(), + fee_msat: amt_msat / 4, + cltv_expiry_delta: 100, + }], + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_3_id, + channel_features: nodes[1].node.channel_features(), + fee_msat: amt_msat / 4, + cltv_expiry_delta: 100, + }], + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + }; + let retry_2_route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_id, + channel_features: nodes[1].node.channel_features(), + fee_msat: amt_msat / 4, + cltv_expiry_delta: 100, + }], + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + }; + nodes[0].router.expect_find_route(Ok(send_route)); + nodes[0].router.expect_find_route(Ok(retry_1_route)); + nodes[0].router.expect_find_route(Ok(retry_2_route)); + + // 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); + match closed_chan_events[0] { + Event::ChannelClosed { .. } => {}, + _ => panic!("Unexpected event"), + } + match closed_chan_events[1] { + Event::ChannelClosed { .. } => {}, + _ => 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 + let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + + // First message is the first update_add, remaining messages are broadcasting channel updates and + // errors for the permfailed channels + assert_eq!(msg_events.len(), 5); + let mut payment_event = SendEvent::from_event(msg_events.remove(0)); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg); + check_added_monitors!(nodes[1], 1); + let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa); + check_added_monitors!(nodes[0], 1); + let as_second_htlc_updates = SendEvent::from_node(&nodes[0]); + + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs); + check_added_monitors!(nodes[0], 1); + let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa); + check_added_monitors!(nodes[1], 1); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.msgs[0]); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.msgs[1]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.commitment_msg); + check_added_monitors!(nodes[1], 1); + let (bs_second_raa, bs_second_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_cs); + check_added_monitors!(nodes[0], 1); + let as_second_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa); + check_added_monitors!(nodes[1], 1); + + expect_pending_htlcs_forwardable_ignore!(nodes[1]); + nodes[1].node.process_pending_htlc_forwards(); + expect_payment_claimable!(nodes[1], payment_hash, payment_secret, amt_msat); + nodes[1].node.claim_funds(payment_preimage); + expect_payment_claimed!(nodes[1], payment_hash, amt_msat); + let bs_claim_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + assert_eq!(bs_claim_update.update_fulfill_htlcs.len(), 1); + + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_claim_update.update_fulfill_htlcs[0]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_claim_update.commitment_signed); + check_added_monitors!(nodes[0], 1); + let (as_third_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_third_raa); + check_added_monitors!(nodes[1], 4); + let bs_second_claim_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs); + check_added_monitors!(nodes[1], 1); + let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.update_fulfill_htlcs[0]); + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.update_fulfill_htlcs[1]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.commitment_signed); + check_added_monitors!(nodes[0], 1); + let (as_fourth_raa, as_fourth_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_fourth_raa); + check_added_monitors!(nodes[1], 1); + + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_fourth_cs); + check_added_monitors!(nodes[1], 1); + let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa); + check_added_monitors!(nodes[0], 1); + expect_payment_sent!(nodes[0], payment_preimage); +} + +#[test] +fn auto_retry_zero_attempts_send_error() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + + // Marshall data to send the payment + let amt_msat = 20_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + 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"); } + 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 + check_added_monitors!(nodes[0], 2); +} + +#[test] +fn fails_paying_after_rejected_by_payee() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + + // Marshall data to send the payment + let amt_msat = 20_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + check_added_monitors!(nodes[0], 1); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let mut payment_event = SendEvent::from_event(events.pop().unwrap()); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + check_added_monitors!(nodes[1], 0); + commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[1]); + expect_payment_claimable!(&nodes[1], payment_hash, payment_secret, amt_msat); + + nodes[1].node.fail_htlc_backwards(&payment_hash); + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], [HTLCDestination::FailedPayment { payment_hash }]); + pass_failed_payment_back(&nodes[0], &[&[&nodes[1]]], false, payment_hash); +} diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 221cc1c6258..42c664e5b3d 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -20,6 +20,7 @@ use crate::chain::keysinterface; use crate::ln::channelmanager; use crate::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; use crate::ln::{msgs, wire}; +use crate::ln::msgs::LightningError; use crate::ln::script::ShutdownScript; use crate::routing::gossip::NetworkGraph; use crate::routing::router::{find_route, InFlightHtlcs, Route, RouteHop, RouteParameters, Router, ScorerAccountingForInFlightHtlcs}; @@ -76,11 +77,17 @@ impl chaininterface::FeeEstimator for TestFeeEstimator { pub struct TestRouter<'a> { pub network_graph: Arc>, + pub next_routes: Mutex>>, } impl<'a> TestRouter<'a> { pub fn new(network_graph: Arc>) -> Self { - Self { network_graph } + Self { network_graph, next_routes: Mutex::new(VecDeque::new()), } + } + + pub fn expect_find_route(&self, result: Result) { + let mut expected_routes = self.next_routes.lock().unwrap(); + expected_routes.push_back(result); } } @@ -89,6 +96,9 @@ impl<'a> Router for TestRouter<'a> { &self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&channelmanager::ChannelDetails]>, inflight_htlcs: &InFlightHtlcs ) -> Result { + if let Some(find_route_res) = self.next_routes.lock().unwrap().pop_front() { + return find_route_res + } let logger = TestLogger::new(); find_route( payer, params, &self.network_graph, first_hops, &logger, @@ -102,6 +112,16 @@ impl<'a> Router for TestRouter<'a> { fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {} } +#[cfg(feature = "std")] // If we put this on the `if`, we get "attributes are not yet allowed on `if` expressions" on 1.41.1 +impl<'a> Drop for TestRouter<'a> { + fn drop(&mut self) { + if std::thread::panicking() { + return; + } + assert!(self.next_routes.lock().unwrap().is_empty()); + } +} + pub struct OnlyReadsKeysInterface {} impl EntropySource for OnlyReadsKeysInterface { From ad486a4596a97ed1274d9cd6a62d90eeb11e7ea3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 4 Jan 2023 18:32:12 -0500 Subject: [PATCH 11/12] Payment retries: copy tests from InvoicePayer As part of migrating payment retries from InvoicePayer to ChannelManager, several tests don't need a rewrite and can be pretty much copied and pasted. --- lightning/src/ln/payment_tests.rs | 325 ++++++++++++++++++++++++++++++ 1 file changed, 325 insertions(+) diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 086370eb74e..ac7938f84be 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -2063,3 +2063,328 @@ fn fails_paying_after_rejected_by_payee() { expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], [HTLCDestination::FailedPayment { payment_hash }]); pass_failed_payment_back(&nodes[0], &[&[&nodes[1]]], false, payment_hash); } + +#[test] +fn retry_multi_path_single_failed_payment() { + // Tests that we can/will retry after a single path of an MPP payment failed immediately + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + let chans = nodes[0].node.list_usable_channels(); + let mut route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chans[0].short_channel_id.unwrap(), + channel_features: nodes[1].node.channel_features(), + fee_msat: 10_000, + cltv_expiry_delta: 100, + }], + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chans[1].short_channel_id.unwrap(), + channel_features: nodes[1].node.channel_features(), + fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than + cltv_expiry_delta: 100, + }], + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + }; + nodes[0].router.expect_find_route(Ok(route.clone())); + // On retry, split the payment across both channels. + route.paths[0][0].fee_msat = 50_000_001; + route.paths[1][0].fee_msat = 50_000_000; + nodes[0].router.expect_find_route(Ok(route.clone())); + + let amt_msat = 100_010_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(htlc_msgs.len(), 2); + check_added_monitors!(nodes[0], 2); +} + +#[test] +fn immediate_retry_on_failure() { + // Tests that we can/will retry immediately after a failure + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + let chans = nodes[0].node.list_usable_channels(); + let mut route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chans[0].short_channel_id.unwrap(), + channel_features: nodes[1].node.channel_features(), + fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than + cltv_expiry_delta: 100, + }], + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + }; + nodes[0].router.expect_find_route(Ok(route.clone())); + // On retry, split the payment across both channels. + route.paths.push(route.paths[0].clone()); + route.paths[0][0].short_channel_id = chans[1].short_channel_id.unwrap(); + route.paths[0][0].fee_msat = 50_000_000; + route.paths[1][0].fee_msat = 50_000_001; + nodes[0].router.expect_find_route(Ok(route.clone())); + + let amt_msat = 100_010_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(htlc_msgs.len(), 2); + check_added_monitors!(nodes[0], 2); +} + +#[test] +fn no_extra_retries_on_back_to_back_fail() { + // In a previous release, we had a race where we may exceed the payment retry count if we + // get two failures in a row with the second having `all_paths_failed` set. + // Generally, when we give up trying to retry a payment, we don't know for sure what the + // current state of the ChannelManager event queue is. Specifically, we cannot be sure that + // there are not multiple additional `PaymentPathFailed` or even `PaymentSent` events + // pending which we will see later. Thus, when we previously removed the retry tracking map + // entry after a `all_paths_failed` `PaymentPathFailed` event, we may have dropped the + // retry entry even though more events for the same payment were still pending. This led to + // us retrying a payment again even though we'd already given up on it. + // + // We now have a separate event - `PaymentFailed` which indicates no HTLCs remain and which + // is used to remove the payment retry counter entries instead. This tests for the specific + // excess-retry case while also testing `PaymentFailed` generation. + + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id; + let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id; + + let mut route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_scid, + channel_features: nodes[1].node.channel_features(), + fee_msat: 0, + cltv_expiry_delta: 100, + }, RouteHop { + pubkey: nodes[2].node.get_our_node_id(), + node_features: nodes[2].node.node_features(), + short_channel_id: chan_2_scid, + channel_features: nodes[2].node.channel_features(), + fee_msat: 100_000_000, + cltv_expiry_delta: 100, + }], + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_scid, + channel_features: nodes[1].node.channel_features(), + fee_msat: 0, + cltv_expiry_delta: 100, + }, RouteHop { + pubkey: nodes[2].node.get_our_node_id(), + node_features: nodes[2].node.node_features(), + short_channel_id: chan_2_scid, + channel_features: nodes[2].node.channel_features(), + fee_msat: 100_000_000, + cltv_expiry_delta: 100, + }] + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())), + }; + nodes[0].router.expect_find_route(Ok(route.clone())); + // On retry, we'll only be asked for one path + route.paths.remove(1); + nodes[0].router.expect_find_route(Ok(route.clone())); + + let amt_msat = 100_010_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + let htlc_updates = SendEvent::from_node(&nodes[0]); + check_added_monitors!(nodes[0], 1); + assert_eq!(htlc_updates.msgs.len(), 1); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &htlc_updates.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &htlc_updates.commitment_msg); + check_added_monitors!(nodes[1], 1); + let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa); + check_added_monitors!(nodes[0], 1); + let second_htlc_updates = SendEvent::from_node(&nodes[0]); + + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs); + check_added_monitors!(nodes[0], 1); + let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &second_htlc_updates.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &second_htlc_updates.commitment_msg); + check_added_monitors!(nodes[1], 1); + let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa); + check_added_monitors!(nodes[1], 1); + let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_fail_update.commitment_signed); + check_added_monitors!(nodes[0], 1); + let (as_second_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa); + check_added_monitors!(nodes[1], 1); + let bs_second_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs); + check_added_monitors!(nodes[1], 1); + let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.update_fail_htlcs[0]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.commitment_signed); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa); + check_added_monitors!(nodes[0], 1); + let (as_third_raa, as_fourth_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_third_raa); + check_added_monitors!(nodes[1], 1); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_fourth_cs); + check_added_monitors!(nodes[1], 1); + let bs_fourth_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_fourth_raa); + check_added_monitors!(nodes[0], 1); + + // At this point A has sent two HTLCs which both failed due to lack of fee. It now has two + // pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second + // with it set. The first event will use up the only retry we are allowed, with the second + // `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd + // treated this as "HTLC complete" and dropped the retry counter, causing us to retry again + // if the final HTLC failed. + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 4); + match events[0] { + Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => { + assert_eq!(payment_hash, ev_payment_hash); + assert_eq!(payment_failed_permanently, false); + }, + _ => panic!("Unexpected event"), + } + match events[1] { + Event::PendingHTLCsForwardable { .. } => {}, + _ => panic!("Unexpected event"), + } + match events[2] { + Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => { + assert_eq!(payment_hash, ev_payment_hash); + assert_eq!(payment_failed_permanently, false); + }, + _ => panic!("Unexpected event"), + } + match events[3] { + Event::PendingHTLCsForwardable { .. } => {}, + _ => panic!("Unexpected event"), + } + + nodes[0].node.process_pending_htlc_forwards(); + let retry_htlc_updates = SendEvent::from_node(&nodes[0]); + check_added_monitors!(nodes[0], 1); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &retry_htlc_updates.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], &retry_htlc_updates.commitment_msg, false, true); + let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true); + + let mut 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, .. } => { + assert_eq!(payment_hash, ev_payment_hash); + assert_eq!(payment_failed_permanently, false); + }, + _ => panic!("Unexpected event"), + } + nodes[0].node.abandon_payment(PaymentId(payment_hash.0)); + events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => { + assert_eq!(payment_hash, *ev_payment_hash); + assert_eq!(PaymentId(payment_hash.0), *ev_payment_id); + }, + _ => panic!("Unexpected event"), + } +} From 6d819796f2efedb924601c3e3bbbb23b2aefb131 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 23 Jan 2023 22:45:59 -0500 Subject: [PATCH 12/12] Disambiguate send_payment_internal from pay_internal --- lightning/src/ln/outbound_payment.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 2a6ff04abef..8867b2e96ae 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -409,7 +409,7 @@ impl OutboundPayments { u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, + 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) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) } @@ -431,7 +431,7 @@ impl OutboundPayments { let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner()); let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { + match self.pay_route_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok(payment_hash), Err(e) => { self.remove_outbound_if_all_failed(payment_id, &e); @@ -508,7 +508,7 @@ impl OutboundPayments { let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info { let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy, Some(route_params.clone()), entropy_source, best_block_height)?; - self.send_payment_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) + 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) }; @@ -617,7 +617,7 @@ impl OutboundPayments { })), } }; - self.send_payment_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, &send_payment_along_path) + self.pay_route_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, &send_payment_along_path) } pub(super) fn send_probe( @@ -643,7 +643,7 @@ impl OutboundPayments { let route = Route { paths: vec![hops], payment_params: None }; let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; - match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { + match self.pay_route_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok((payment_hash, payment_id)), Err(e) => { self.remove_outbound_if_all_failed(payment_id, &e); @@ -696,7 +696,7 @@ impl OutboundPayments { } } - fn send_payment_internal( + fn pay_route_internal( &self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>, node_signer: &NS, best_block_height: u32, @@ -813,7 +813,7 @@ impl OutboundPayments { F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { - self.send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, + self.pay_route_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })