Skip to content

Commit 9db2306

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 7ae8787 commit 9db2306

File tree

4 files changed

+35
-9
lines changed

4 files changed

+35
-9
lines changed

fuzz/src/chanmon_consistency.rs

+2-2
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

+1-1
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

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

934943
macro_rules! handle_error {
@@ -2217,7 +2226,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22172226
}
22182227
let mut has_ok = false;
22192228
let mut has_err = false;
2220-
for res in results.iter() {
2229+
let mut pending_amt_unsent = 0;
2230+
let mut max_unsent_cltv_delta = 0;
2231+
for (res, path) in results.iter().zip(route.paths.iter()) {
22212232
if res.is_ok() { has_ok = true; }
22222233
if res.is_err() { has_err = true; }
22232234
if let &Err(APIError::MonitorUpdateFailed) = res {
@@ -2226,10 +2237,25 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22262237
has_err = true;
22272238
has_ok = true;
22282239
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);
22292243
}
22302244
}
22312245
if has_err && has_ok {
2232-
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+
})
22332259
} else if has_err {
22342260
Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
22352261
} else {

lightning/src/ln/functional_test_utils.rs

+3-3
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)