Provide payment retry data when an MPP payment failed partially
authorMatt Corallo <git@bluematt.me>
Tue, 26 Oct 2021 21:39:31 +0000 (21:39 +0000)
committerMatt Corallo <git@bluematt.me>
Sat, 30 Oct 2021 01:53:19 +0000 (01:53 +0000)
This will allow `InvoicePayer` to properly retry payments that only
partially failed to send.

fuzz/src/chanmon_consistency.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs

index 464e784d5142e81a2d8e07310b1dc8bad7d15aa4..acdf3cb3ca2dc3b2f902aa1a323e645b837d2368 100644 (file)
@@ -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); } }
                },
        }
 }
index feefd8c18f1ecce8e9776f348b75bcb16f010f32..0ae3ff29d87ed82e5c26beecc225c25401658fad 100644 (file)
@@ -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!(); }
index 53d332be01c812f04638b39df9d5f9dd98888e13..cdde8bd85325f9a68f5ed1ed4510c891428421c7 100644 (file)
@@ -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<Result<(), APIError>>),
+       PartialFailure {
+               /// The errors themselves, in the same order as the route hops.
+               results: Vec<Result<(), APIError>>,
+               /// 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<RouteParameters>,
+               /// 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 {
index bcb1ac1f1ad24cd7c07ee523d22d4cdaa97843f1..14bc6beec5fb14a2d42981f2ce06a936b1dea9b2 100644 (file)
@@ -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!(),
                                }