From: Elias Rohrer <dev@tnull.de>
Date: Wed, 27 Sep 2023 10:43:31 +0000 (+0200)
Subject: Don't retry overpaid values for `PartialFailure`s
X-Git-Tag: v0.0.117-rc1~15^2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=be1088ac080f05398ed009b1bee2730a4371341f;p=rust-lightning

Don't retry overpaid values for `PartialFailure`s

Previously, if an overpaid path would fail immediately, we'd retry a
`PartialFailure` with the full path amount, _including_ any overpayment.

Here, we now subtract the succeeded paths' values from the
net. value to exclude the overpaid amounts on retry.
---

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 4cc8d85ad..2522f99fb 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1354,12 +1354,14 @@ impl OutboundPayments {
 		}
 		let mut has_ok = false;
 		let mut has_err = false;
-		let mut pending_amt_unsent = 0;
+		let mut has_unsent = false;
 		let mut total_ok_fees_msat = 0;
+		let mut total_ok_amt_sent_msat = 0;
 		for (res, path) in results.iter().zip(route.paths.iter()) {
 			if res.is_ok() {
 				has_ok = true;
 				total_ok_fees_msat += path.fee_msat();
+				total_ok_amt_sent_msat += path.final_value_msat();
 			}
 			if res.is_err() { has_err = true; }
 			if let &Err(APIError::MonitorUpdateInProgress) = res {
@@ -1368,23 +1370,27 @@ impl OutboundPayments {
 				has_err = true;
 				has_ok = true;
 				total_ok_fees_msat += path.fee_msat();
+				total_ok_amt_sent_msat += path.final_value_msat();
 			} else if res.is_err() {
-				pending_amt_unsent += path.final_value_msat();
+				has_unsent = true;
 			}
 		}
 		if has_err && has_ok {
 			Err(PaymentSendFailure::PartialFailure {
 				results,
 				payment_id,
-				failed_paths_retry: if pending_amt_unsent != 0 {
+				failed_paths_retry: if has_unsent {
 					if let Some(route_params) = &route.route_params {
 						let mut route_params = route_params.clone();
 						// We calculate the leftover fee budget we're allowed to spend by
 						// subtracting the used fee from the total fee budget.
 						route_params.max_total_routing_fee_msat = route_params
 							.max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
-						route_params.final_value_msat = pending_amt_unsent;
 
+						// We calculate the remaining target amount by subtracting the succeded
+						// path values.
+						route_params.final_value_msat = route_params.final_value_msat
+							.saturating_sub(total_ok_amt_sent_msat);
 						Some(route_params)
 					} else { None }
 				} else { None },
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index e48ae7bd8..721767602 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2736,9 +2736,7 @@ fn retry_multi_path_single_failed_payment() {
 	let mut pay_params = route.route_params.clone().unwrap().payment_params;
 	pay_params.previously_failed_channels.push(chans[1].short_channel_id.unwrap());
 
-	// Note that the second request here requests the amount we originally failed to send,
-	// not the amount remaining on the full payment, which should be changed.
-	let mut retry_params = RouteParameters::from_payment_params_and_value(pay_params, 100_000_001);
+	let mut retry_params = RouteParameters::from_payment_params_and_value(pay_params, 100_000_000);
 	retry_params.max_total_routing_fee_msat = None;
 	route.route_params.as_mut().unwrap().final_value_msat = 100_000_000;
 	nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));