On initial send retries, avoid previously failed scids

Previously, we would have tried the same failed channels over and over until
retries are exhausted.

Author: valentinewallace

lightning/src/ln/outbound_payment.rs

@@ -731,8 +731,8 @@ impl OutboundPayments {
 
 	fn handle_pay_route_err<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
 		&self, err: PaymentSendFailure, payment_id: PaymentId, payment_hash: PaymentHash, route: Route,
-		route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: &IH,
-		entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L,
+		mut route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
+		inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L,
 		pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: &SP,
 	)
 	where
@@ -746,11 +746,11 @@ impl OutboundPayments {
 	{
 		match err {
 			PaymentSendFailure::AllFailedResendSafe(errs) => {
-				push_payment_path_failed_evs(payment_id, payment_hash, route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
+				push_path_failed_evs_and_scids(payment_id, payment_hash, Some(&mut route_params), route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
 				self.retry_payment_internal(payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
 			},
 			PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. } => {
-				push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
+				push_path_failed_evs_and_scids(payment_id, payment_hash, None, route.paths, results.into_iter(), pending_events);
 				// Some paths were sent, even if we failed to send the full MPP value our recipient may
 				// misbehave and claim the funds, at which point we have to consider the payment sent, so
 				// return `Ok()` here, ignoring any retry errors.
@@ -762,7 +762,7 @@ impl OutboundPayments {
 				// initial HTLC-Add messages yet.
 			},
 			PaymentSendFailure::PathParameterError(results) => {
-				push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
+				push_path_failed_evs_and_scids(payment_id, payment_hash, None, route.paths, results.into_iter(), pending_events);
 				self.abandon_payment(payment_id, pending_events);
 			},
 			PaymentSendFailure::ParameterError(e) => {
@@ -1270,14 +1270,17 @@ impl OutboundPayments {
 	}
 }
 
-fn push_payment_path_failed_evs<I: Iterator<Item = Result<(), APIError>>>(
-	payment_id: PaymentId, payment_hash: PaymentHash, paths: Vec<Vec<RouteHop>>, path_results: I,
-	pending_events: &Mutex<Vec<events::Event>>
+fn push_path_failed_evs_and_scids<I: Iterator<Item = Result<(), APIError>>>(
+	payment_id: PaymentId, payment_hash: PaymentHash, mut route_params: Option<&mut RouteParameters>,
+	paths: Vec<Vec<RouteHop>>, path_results: I, pending_events: &Mutex<Vec<events::Event>>
 ) {
 	let mut events = pending_events.lock().unwrap();
 	for (path, path_res) in paths.into_iter().zip(path_results) {
 		if path_res.is_err() {
 			let scid = path[0].short_channel_id;
+			if let Some(ref mut params) = route_params {
+				params.payment_params.previously_failed_channels.push(scid);
+			}
 			events.push(events::Event::PaymentPathFailed {
 				payment_id: Some(payment_id),
 				payment_hash,

lightning/src/ln/payment_tests.rs

@@ -2196,9 +2196,11 @@ fn immediate_retry_on_failure() {
 	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;
+	let mut pay_params = route_params.payment_params.clone();
+	pay_params.previously_failed_channels.push(chans[0].short_channel_id.unwrap());
 	nodes[0].router.expect_find_route(RouteParameters {
-			payment_params: route_params.payment_params.clone(),
-			final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV
+			payment_params: pay_params, final_value_msat: amt_msat,
+			final_cltv_expiry_delta: TEST_FINAL_CLTV
 		}, Ok(route.clone()));
 
 	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();