Skip to content

Commit 576394b

Browse files
committed
Provide payment retry data when an MPP payment failed partially
This will allow `InvoicePayer` to properly retry payments that only partially failed to send.
1 parent e94f7cf commit 576394b

File tree

4 files changed

+35
-10
lines changed

4 files changed

+35
-10
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ fn check_payment_err(send_err: PaymentSendFailure) {
271271
PaymentSendFailure::AllFailedRetrySafe(per_path_results) => {
272272
for api_err in per_path_results { check_api_err(api_err); }
273273
},
274-
PaymentSendFailure::PartialFailure(per_path_results) => {
275-
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
274+
PaymentSendFailure::PartialFailure { results, .. } => {
275+
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
276276
},
277277
}
278278
}

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1950,7 +1950,7 @@ fn test_path_paused_mpp() {
19501950
// Now check that we get the right return value, indicating that the first path succeeded but
19511951
// the second got a MonitorUpdateFailed err. This implies PaymentSendFailure::PartialFailure as
19521952
// some paths succeeded, preventing retry.
1953-
if let Err(PaymentSendFailure::PartialFailure(results)) = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)) {
1953+
if let Err(PaymentSendFailure::PartialFailure { results, ..}) = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)) {
19541954
assert_eq!(results.len(), 2);
19551955
if let Ok(()) = results[0] {} else { panic!(); }
19561956
if let Err(APIError::MonitorUpdateFailed) = results[1] {} else { panic!(); }

lightning/src/ln/channelmanager.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,16 @@ pub enum PaymentSendFailure {
946946
/// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the
947947
/// case of Ok(())) or will send once channel_monitor_updated is called on the next-hop channel
948948
/// with the latest update_id.
949-
PartialFailure(Vec<Result<(), APIError>>),
949+
PartialFailure {
950+
/// The errors themselves, in the same order as the route hops.
951+
results: Vec<Result<(), APIError>>,
952+
/// If some paths failed without irrevocably committing to the new HTLC(s), this will
953+
/// contain a [`RouteParameters`] object which can be used to calculate a new route that
954+
/// will pay all remaining unpaid balance.
955+
failed_paths_retry: Option<RouteParameters>,
956+
/// The payment id for the payment, which is now at least partially pending.
957+
payment_id: PaymentId,
958+
},
950959
}
951960

952961
macro_rules! handle_error {
@@ -2236,19 +2245,35 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22362245
}
22372246
let mut has_ok = false;
22382247
let mut has_err = false;
2239-
for res in results.iter() {
2248+
let mut pending_amt_unsent = 0;
2249+
let mut max_unsent_cltv_delta = 0;
2250+
for (res, path) in results.iter().zip(route.paths.iter()) {
22402251
if res.is_ok() { has_ok = true; }
22412252
if res.is_err() { has_err = true; }
22422253
if let &Err(APIError::MonitorUpdateFailed) = res {
22432254
// MonitorUpdateFailed is inherently unsafe to retry, so we call it a
22442255
// PartialFailure.
22452256
has_err = true;
22462257
has_ok = true;
2247-
break;
2258+
} else if res.is_err() {
2259+
pending_amt_unsent += path.last().unwrap().fee_msat;
2260+
max_unsent_cltv_delta = cmp::max(max_unsent_cltv_delta, path.last().unwrap().cltv_expiry_delta);
22482261
}
22492262
}
22502263
if has_err && has_ok {
2251-
Err(PaymentSendFailure::PartialFailure(results))
2264+
Err(PaymentSendFailure::PartialFailure {
2265+
results,
2266+
payment_id,
2267+
failed_paths_retry: if pending_amt_unsent != 0 {
2268+
if let Some(payee) = &route.payee {
2269+
Some(RouteParameters {
2270+
payee: payee.clone(),
2271+
final_value_msat: pending_amt_unsent,
2272+
final_cltv_expiry_delta: max_unsent_cltv_delta,
2273+
})
2274+
} else { None }
2275+
} else { None },
2276+
})
22522277
} else if has_err {
22532278
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
22542279
} else {

lightning/src/ln/functional_test_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,9 +482,9 @@ macro_rules! unwrap_send_err {
482482
_ => panic!(),
483483
}
484484
},
485-
&Err(PaymentSendFailure::PartialFailure(ref fails)) if !$all_failed => {
486-
assert_eq!(fails.len(), 1);
487-
match fails[0] {
485+
&Err(PaymentSendFailure::PartialFailure { ref results, .. }) if !$all_failed => {
486+
assert_eq!(results.len(), 1);
487+
match results[0] {
488488
Err($type) => { $check },
489489
_ => panic!(),
490490
}

0 commit comments

Comments
 (0)