f Make payment cache removal happen at the callsite

Author: TheBlueMatt

lightning-invoice/src/payment.rs

@@ -313,14 +313,14 @@ where
 			// 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(true, payment_id, payment_hash, &retry);
+			let _ = self.retry_payment(payment_id, payment_hash, &retry);
 			Ok(payment_id)
 		} else {
 			self.pay_invoice_internal(invoice, amount_msats, retry_count + 1)
 		}
 	}
 
-	fn retry_payment(&self, other_paths_pending: bool, payment_id: PaymentId, payment_hash: PaymentHash, params: &RouteParameters)
+	fn retry_payment(&self, payment_id: PaymentId, payment_hash: PaymentHash, params: &RouteParameters)
 	-> Result<(), ()> {
 		let route;
 		{
@@ -339,11 +339,9 @@ where
 
 				if *attempts >= max_payment_attempts {
 					log_trace!(self.logger, "Payment {} exceeded maximum attempts; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
-					if !other_paths_pending { entry.remove(); }
 					return Err(());
 				} else if has_expired(params) {
 					log_trace!(self.logger, "Invoice expired for payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
-					if !other_paths_pending { entry.remove(); }
 					return Err(());
 				}
 
@@ -352,7 +350,6 @@ where
 				route = self.router.find_route(&payer, &params, Some(&first_hops.iter().collect::<Vec<_>>()), &self.scorer.lock());
 				if route.is_err() {
 					log_trace!(self.logger, "Failed to find a route for payment {}; not retrying (attempts: {})", log_bytes!(payment_hash.0), attempts);
-					if !other_paths_pending { entry.remove(); }
 					return Err(());
 				}
 			} else {
@@ -366,15 +363,14 @@ where
 			Err(PaymentSendFailure::ParameterError(_)) |
 			Err(PaymentSendFailure::PathParameterError(_)) => {
 				log_trace!(self.logger, "Failed to retry for payment {} due to bogus route/payment data, not retrying.", log_bytes!(payment_hash.0));
-				if !other_paths_pending { self.payment_cache.lock().unwrap().remove(&payment_hash); }
 				return Err(());
 			},
 			Err(PaymentSendFailure::AllFailedRetrySafe(_)) => {
-				self.retry_payment(other_paths_pending, payment_id, payment_hash, params)
+				self.retry_payment(payment_id, payment_hash, params)
 			},
 			Err(PaymentSendFailure::PartialFailure { results: _, failed_paths_retry, .. }) => {
 				if let Some(retry) = failed_paths_retry {
-					self.retry_payment(true, payment_id, payment_hash, &retry)
+					self.retry_payment(payment_id, payment_hash, &retry)
 				} else {
 					Ok(())
 				}
@@ -420,19 +416,17 @@ where
 
 				if *rejected_by_dest {
 					log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0));
-					if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); }
 				} else if payment_id.is_none() {
 					log_trace!(self.logger, "Payment {} has no id; not retrying", log_bytes!(payment_hash.0));
-					if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); }
 				} else if let Some(params) = retry {
-					if self.retry_payment(!all_paths_failed, payment_id.unwrap(), *payment_hash, params).is_ok() {
+					if self.retry_payment(payment_id.unwrap(), *payment_hash, params).is_ok() {
 						// We retried at least somewhat, don't provide the PaymentPathFailed event to the user.
 						return;
 					}
 				} else {
 					log_trace!(self.logger, "Payment {} missing retry params; not retrying", log_bytes!(payment_hash.0));
-					if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); }
 				}
+				if *all_paths_failed { self.payment_cache.lock().unwrap().remove(payment_hash); }
 			},
 			Event::PaymentSent { payment_hash, .. } => {
 				let mut payment_cache = self.payment_cache.lock().unwrap();