Skip to content

Commit

Permalink
Don't retry overpaid values for PartialFailures
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tnull committed Sep 28, 2023
1 parent 8c99e34 commit be1088a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
14 changes: 10 additions & 4 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 },
Expand Down
4 changes: 1 addition & 3 deletions lightning/src/ln/payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down

0 comments on commit be1088a

Please sign in to comment.