From be1088ac080f05398ed009b1bee2730a4371341f Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 27 Sep 2023 12:43:31 +0200 Subject: [PATCH] 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. --- lightning/src/ln/outbound_payment.rs | 14 ++++++++++---- lightning/src/ln/payment_tests.rs | 4 +--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 4cc8d85a..2522f99f 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 e48ae7bd..72176760 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())); -- 2.30.2