From d73e43428028b691453b0d5fe5e8608e4e9b1190 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 26 Oct 2021 21:39:31 +0000 Subject: [PATCH] Provide payment retry data when an MPP payment failed partially This will allow `InvoicePayer` to properly retry payments that only partially failed to send. --- fuzz/src/chanmon_consistency.rs | 4 +-- lightning/src/ln/chanmon_update_fail_tests.rs | 2 +- lightning/src/ln/channelmanager.rs | 33 ++++++++++++++++--- lightning/src/ln/functional_test_utils.rs | 6 ++-- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 464e784d5..acdf3cb3c 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -271,8 +271,8 @@ fn check_payment_err(send_err: PaymentSendFailure) { PaymentSendFailure::AllFailedRetrySafe(per_path_results) => { for api_err in per_path_results { check_api_err(api_err); } }, - PaymentSendFailure::PartialFailure(per_path_results) => { - for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } } + PaymentSendFailure::PartialFailure { results, .. } => { + for res in results { if let Err(api_err) = res { check_api_err(api_err); } } }, } } diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index feefd8c18..0ae3ff29d 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -1950,7 +1950,7 @@ fn test_path_paused_mpp() { // Now check that we get the right return value, indicating that the first path succeeded but // the second got a MonitorUpdateFailed err. This implies PaymentSendFailure::PartialFailure as // some paths succeeded, preventing retry. - if let Err(PaymentSendFailure::PartialFailure(results)) = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)) { + if let Err(PaymentSendFailure::PartialFailure { results, ..}) = nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)) { assert_eq!(results.len(), 2); if let Ok(()) = results[0] {} else { panic!(); } if let Err(APIError::MonitorUpdateFailed) = results[1] {} else { panic!(); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 53d332be0..cdde8bd85 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -946,7 +946,16 @@ pub enum PaymentSendFailure { /// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the /// case of Ok(())) or will send once channel_monitor_updated is called on the next-hop channel /// with the latest update_id. - PartialFailure(Vec>), + PartialFailure { + /// The errors themselves, in the same order as the route hops. + results: Vec>, + /// If some paths failed without irrevocably committing to the new HTLC(s), this will + /// contain a [`RouteParameters`] object which can be used to calculate a new route that + /// will pay all remaining unpaid balance. + failed_paths_retry: Option, + /// The payment id for the payment, which is now at least partially pending. + payment_id: PaymentId, + }, } macro_rules! handle_error { @@ -2236,7 +2245,9 @@ impl ChannelMana } let mut has_ok = false; let mut has_err = false; - for res in results.iter() { + let mut pending_amt_unsent = 0; + let mut max_unsent_cltv_delta = 0; + for (res, path) in results.iter().zip(route.paths.iter()) { if res.is_ok() { has_ok = true; } if res.is_err() { has_err = true; } if let &Err(APIError::MonitorUpdateFailed) = res { @@ -2244,11 +2255,25 @@ impl ChannelMana // PartialFailure. has_err = true; has_ok = true; - break; + } else if res.is_err() { + pending_amt_unsent += path.last().unwrap().fee_msat; + max_unsent_cltv_delta = cmp::max(max_unsent_cltv_delta, path.last().unwrap().cltv_expiry_delta); } } if has_err && has_ok { - Err(PaymentSendFailure::PartialFailure(results)) + Err(PaymentSendFailure::PartialFailure { + results, + payment_id, + failed_paths_retry: if pending_amt_unsent != 0 { + if let Some(payee) = &route.payee { + Some(RouteParameters { + payee: payee.clone(), + final_value_msat: pending_amt_unsent, + final_cltv_expiry_delta: max_unsent_cltv_delta, + }) + } else { None } + } else { None }, + }) } else if has_err { Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect())) } else { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index bcb1ac1f1..14bc6beec 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -482,9 +482,9 @@ macro_rules! unwrap_send_err { _ => panic!(), } }, - &Err(PaymentSendFailure::PartialFailure(ref fails)) if !$all_failed => { - assert_eq!(fails.len(), 1); - match fails[0] { + &Err(PaymentSendFailure::PartialFailure { ref results, .. }) if !$all_failed => { + assert_eq!(results.len(), 1); + match results[0] { Err($type) => { $check }, _ => panic!(), } -- 2.39.5