Implement automatic payment retries in ChannelManager

TODO: more tests

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -52,6 +52,8 @@ use crate::ln::onion_utils;
 use crate::ln::onion_utils::HTLCFailReason;
 use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
 use crate::ln::payment_retry::{PaymentAttempts, Retry};
+#[cfg(feature = "std")]
+use crate::ln::payment_retry::has_expired;
 use crate::ln::wire::Encode;
 use crate::chain::keysinterface::{Sign, KeysInterface, KeysManager, Recipient};
 use crate::util::config::{UserConfig, ChannelConfig};
@@ -2607,6 +2609,47 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 		self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs)
 	}
 
+	/// Similar to [`ChannelManager::send_payment`], but will automatically find a route based on
+	/// `route_params` and retry failed payment paths based on `retry_strategy`.
+	pub fn send_payment_with_retry(&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), PaymentSendFailure> {
+		#[cfg(feature = "std")] {
+			if has_expired(&route_params) {
+				log_trace!(self.logger, "Invoice expired prior to send for payment {}", log_bytes!(payment_hash.0));
+				return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err: format!("Invoice expired prior to send for payment {}", log_bytes!(payment_hash.0))}));
+			}
+		}
+
+		let route = self.router.find_route(
+			&self.get_our_node_id(), &route_params, Some(&self.list_usable_channels().iter().collect::<Vec<_>>()),
+			self.compute_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
+		}))?;
+
+		log_trace!(self.logger, "Attempting to send payment with id {} and hash {} over route {}", log_bytes!(payment_id.0), log_bytes!(payment_hash.0), log_route!(route));
+		let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy)?;
+		match self.send_payment_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs) {
+			Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
+				self.retry_payment_auto_route(payment_id, route_params)
+			},
+			Err(PaymentSendFailure::PartialFailure { failed_paths_retry, .. }) => {
+				if let Some(retry_data) = failed_paths_retry {
+					// 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.retry_payment_auto_route(payment_id, retry_data);
+					Ok(())
+				} else {
+					// 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,
+		}
+	}
+
 	#[cfg(test)]
 	pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
 		self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, Retry::Attempts(0))
@@ -2732,16 +2775,58 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 				} 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");
+			if !self.pending_outbound_payments.lock().unwrap().get(&payment_id)
+				.map_or(false, |payment| payment.is_retryable())
+			{
+				// 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(())
 		}
 	}
 
+	fn retry_payment_auto_route(&self, payment_id: PaymentId, route_params: RouteParameters) -> Result<(), PaymentSendFailure> {
+		#[cfg(feature = "std")] {
+			if has_expired(&route_params) {
+				return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+					err: format!("Invoice expired for payment id {}; not retrying", log_bytes!(payment_id.0)),
+				}))
+			}
+		}
+
+		self.pending_outbound_payments.lock().unwrap().get_mut(&payment_id).map(|pmt| pmt.increment_attempts());
+		if !self.pending_outbound_payments.lock().unwrap().get(&payment_id)
+			.map_or(false, |payment| payment.is_retryable())
+		{
+			return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+				err: format!("Payment {} is not retryable", log_bytes!(payment_id.0)),
+			}))
+		}
+
+		let route = self.router.find_route(
+			&self.get_our_node_id(), &route_params, Some(&self.list_usable_channels().iter().collect::<Vec<_>>()),
+			self.compute_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
+		}))?;
+
+		match self.retry_payment(&route, payment_id) {
+			Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
+				self.retry_payment_auto_route(payment_id, route_params)
+			},
+			Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), .. }) => {
+				// Always return Ok for the same reason as noted in send_payment_with_retry.
+				let _ = self.retry_payment_auto_route(payment_id, retry);
+				Ok(())
+			},
+			res => res,
+		}
+	}
+
 	/// Retries a payment along the given [`Route`].
 	///
 	/// Errors returned are a superset of those returned from [`send_payment`], so see
@@ -3600,6 +3685,17 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 				}
 			}
 		}
+		{
+			let mut retryable_htlcs = Vec::new();
+			mem::swap(&mut retryable_htlcs, &mut self.retryable_htlcs.lock().unwrap());
+			for (payment_id, params) in retryable_htlcs {
+				// TODO: do we need to generate a PendingHTLCsForwardable event?
+				if let Err(e) = self.retry_payment_auto_route(payment_id, params) {
+					log_trace!(self.logger, "Errored retrying payment, marking as abandoned: {:?}", e);
+					self.abandon_payment(payment_id);
+				}
+			}
+		}
 
 		for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
 			self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
@@ -3966,7 +4062,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 				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!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 						return;
@@ -3975,6 +4072,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 						log_trace!(self.logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
 						return;
 					}
+					let is_retryable = payment.get().is_retryable();
 					if payment.get().remaining_parts() == 0 {
 						all_paths_failed = true;
 						if payment.get().abandoned() {
@@ -3985,10 +4083,11 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 							payment.remove();
 						}
 					}
+					is_retryable
 				} else {
 					log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 					return;
-				}
+				};
 				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 {
@@ -4027,6 +4126,15 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 						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 {
+							if let Some(retry_params) = &retry {
+								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),
+								});
+								self.retryable_htlcs.lock().unwrap().push((*payment_id, retry_params.clone()));
+							}
+						}
 						events::Event::PaymentPathFailed {
 							payment_id: Some(*payment_id),
 							payment_hash: payment_hash.clone(),
@@ -4046,6 +4154,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, R: Deref, L: Deref> ChannelManager<
 				let mut pending_events = self.pending_events.lock().unwrap();
 				pending_events.push(path_failure);
 				if let Some(ev) = full_failure_ev { pending_events.push(ev); }
+				if let Some(ev) = pending_retry_ev { pending_events.push(ev); }
 			},
 			HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
 				log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);

lightning/src/ln/payment_retry.rs

@@ -9,6 +9,8 @@
 
 //! Payment retry utilities for use in and with `ChannelManager`.
 
+#[cfg(feature = "std")]
+use crate::routing::router::RouteParameters;
 use crate::util::time::Time;
 #[cfg(all(not(feature = "no-std"), test))]
 use crate::util::time::tests::SinceEpoch;
@@ -46,6 +48,16 @@ impl Retry {
 	}
 }
 
+#[cfg(feature = "std")]
+pub(crate) 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
+}
+
 #[cfg(not(any(feature = "no-std", test)))]
 type ConfiguredTime = std::time::Instant;
 #[cfg(feature = "no-std")]