Skip to content

Commit 8dca9ee

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 160b991 commit 8dca9ee

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
@@ -929,7 +929,16 @@ pub enum PaymentSendFailure {
929929
/// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the
930930
/// case of Ok(())) or will send once channel_monitor_updated is called on the next-hop channel
931931
/// with the latest update_id.
932-
PartialFailure(Vec<Result<(), APIError>>),
932+
PartialFailure {
933+
/// The errors themselves, in the same order as the route hops.
934+
results: Vec<Result<(), APIError>>,
935+
/// If some paths failed without irrevocably committing to the new HTLC(s), this will
936+
/// contain a [`RouteParameters`] object which can be used to calculate a new route that
937+
/// will pay all remaining unpaid balance.
938+
failed_paths_retry: Option<RouteParameters>,
939+
/// The payment id for the payment, which is now at least partially pending.
940+
payment_id: PaymentId,
941+
},
933942
}
934943

935944
macro_rules! handle_error {
@@ -2218,19 +2227,35 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22182227
}
22192228
let mut has_ok = false;
22202229
let mut has_err = false;
2221-
for res in results.iter() {
2230+
let mut pending_amt_unsent = 0;
2231+
let mut max_unsent_cltv_delta = 0;
2232+
for (res, path) in results.iter().zip(route.paths.iter()) {
22222233
if res.is_ok() { has_ok = true; }
22232234
if res.is_err() { has_err = true; }
22242235
if let &Err(APIError::MonitorUpdateFailed) = res {
22252236
// MonitorUpdateFailed is inherently unsafe to retry, so we call it a
22262237
// PartialFailure.
22272238
has_err = true;
22282239
has_ok = true;
2229-
break;
2240+
} else if res.is_err() {
2241+
pending_amt_unsent += path.last().unwrap().fee_msat;
2242+
max_unsent_cltv_delta = cmp::max(max_unsent_cltv_delta, path.last().unwrap().cltv_expiry_delta);
22302243
}
22312244
}
22322245
if has_err && has_ok {
2233-
Err(PaymentSendFailure::PartialFailure(results))
2246+
Err(PaymentSendFailure::PartialFailure {
2247+
results,
2248+
payment_id,
2249+
failed_paths_retry: if pending_amt_unsent != 0 {
2250+
if let Some(payee) = &route.payee {
2251+
Some(RouteParameters {
2252+
payee: payee.clone(),
2253+
final_value_msat: pending_amt_unsent,
2254+
final_cltv_expiry_delta: max_unsent_cltv_delta,
2255+
})
2256+
} else { None }
2257+
} else { None },
2258+
})
22342259
} else if has_err {
22352260
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
22362261
} 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)