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 4cc8d85ad19e29d77525c741bd1b59175f4ca7e8..2522f99fbe85dbecd751034215207cbda23ad5ab 100644 (file)
--- 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 e48ae7bd8dcffeb54f165c958baaba9d9510ae5b..72176760243bf25cae3dd3adfcb656a4be6417cd 100644 (file)
--- 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()));