Rename APIError::MonitorUpdateFailed to MonitorUpdateInProgress
authorMatt Corallo <git@bluematt.me>
Thu, 29 Sep 2022 20:26:48 +0000 (20:26 +0000)
committerMatt Corallo <git@bluematt.me>
Thu, 29 Sep 2022 20:27:53 +0000 (20:27 +0000)
This much more accurately represents the error, indicating that a
monitor update is in progress asynchronously and may complete at a
later time.

fuzz/src/chanmon_consistency.rs
lightning-invoice/src/payment.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/util/errors.rs

index 7944d04ad1c33d1b4de0aed044f429df9f1c7860..a02d003e1b7a88ab2d70bb490d5280a079513f6d 100644 (file)
@@ -270,7 +270,7 @@ fn check_api_err(api_err: APIError) {
                                _ => panic!("{}", err),
                        }
                },
-               APIError::MonitorUpdateFailed => {
+               APIError::MonitorUpdateInProgress => {
                        // We can (obviously) temp-fail a monitor update
                },
                APIError::IncompatibleShutdownScript { .. } => panic!("Cannot send an incompatible shutdown script"),
index b9f4486339d199dbdc3d93f09cad652a64fe335c..e8e296294016f4ba8b9eb954e3d0df9f0c68d65f 100644 (file)
@@ -513,11 +513,11 @@ where
                                },
                                PaymentSendFailure::PartialFailure { failed_paths_retry, payment_id, results } => {
                                        // If a `PartialFailure` event returns a result that is an `Ok()`, it means that
-                                       // part of our payment is retried. When we receive `MonitorUpdateFailed`, it
+                                       // part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
                                        // means that we are still waiting for our channel monitor update to be completed.
                                        for (result, path) in results.iter().zip(route.paths.into_iter()) {
                                                match result {
-                                                       Ok(_) | Err(APIError::MonitorUpdateFailed) => {
+                                                       Ok(_) | Err(APIError::MonitorUpdateInProgress) => {
                                                                self.process_path_inflight_htlcs(payment_hash, path);
                                                        },
                                                        _ => {},
@@ -617,11 +617,11 @@ where
                        },
                        Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
                                // If a `PartialFailure` error contains a result that is an `Ok()`, it means that
-                               // part of our payment is retried. When we receive `MonitorUpdateFailed`, it
+                               // part of our payment is retried. When we receive `MonitorUpdateInProgress`, it
                                // means that we are still waiting for our channel monitor update to complete.
                                for (result, path) in results.iter().zip(route.unwrap().paths.into_iter()) {
                                        match result {
-                                               Ok(_) | Err(APIError::MonitorUpdateFailed) => {
+                                               Ok(_) | Err(APIError::MonitorUpdateInProgress) => {
                                                        self.process_path_inflight_htlcs(payment_hash, path);
                                                },
                                                _ => {},
@@ -796,7 +796,7 @@ mod tests {
        use std::time::{SystemTime, Duration};
        use time_utils::tests::SinceEpoch;
        use DEFAULT_EXPIRY_TIME;
-       use lightning::util::errors::APIError::{ChannelUnavailable, MonitorUpdateFailed};
+       use lightning::util::errors::APIError::{ChannelUnavailable, MonitorUpdateInProgress};
 
        fn invoice(payment_preimage: PaymentPreimage) -> Invoice {
                let payment_hash = Sha256::hash(&payment_preimage.0);
@@ -1718,7 +1718,7 @@ mod tests {
                        .fails_with_partial_failure(
                                retry.clone(), OnAttempt(1),
                                Some(vec![
-                                       Err(ChannelUnavailable { err: "abc".to_string() }), Err(MonitorUpdateFailed)
+                                       Err(ChannelUnavailable { err: "abc".to_string() }), Err(MonitorUpdateInProgress)
                                ]))
                        .expect_send(Amount::ForInvoice(final_value_msat));
 
@@ -1731,7 +1731,7 @@ mod tests {
                invoice_payer.pay_invoice(&invoice_to_pay).unwrap();
                let inflight_map = invoice_payer.create_inflight_map();
 
-               // Only the second path, which failed with `MonitorUpdateFailed` should be added to our
+               // Only the second path, which failed with `MonitorUpdateInProgress` should be added to our
                // inflight map because retries are disabled.
                assert_eq!(inflight_map.len(), 2);
        }
@@ -1750,7 +1750,7 @@ mod tests {
                        .fails_with_partial_failure(
                                retry.clone(), OnAttempt(1),
                                Some(vec![
-                                       Ok(()), Err(MonitorUpdateFailed)
+                                       Ok(()), Err(MonitorUpdateInProgress)
                                ]))
                        .expect_send(Amount::ForInvoice(final_value_msat));
 
@@ -2044,7 +2044,7 @@ mod tests {
                }
 
                fn fails_on_attempt(self, attempt: usize) -> Self {
-                       let failure = PaymentSendFailure::ParameterError(APIError::MonitorUpdateFailed);
+                       let failure = PaymentSendFailure::ParameterError(APIError::MonitorUpdateInProgress);
                        self.fails_with(failure, OnAttempt(attempt))
                }
 
index c0648eb2b8c3f8b5cafff2ce29a849e0d6c5b836..15d46b04688930d03525b6347d6b714a28713b28 100644 (file)
@@ -170,7 +170,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
        chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
 
        {
-               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)), false, APIError::MonitorUpdateFailed, {});
+               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)), false, APIError::MonitorUpdateInProgress, {});
                check_added_monitors!(nodes[0], 1);
        }
 
@@ -221,7 +221,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
        let (route, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000);
        {
                chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
-               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_2, &Some(payment_secret_2)), false, APIError::MonitorUpdateFailed, {});
+               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_2, &Some(payment_secret_2)), false, APIError::MonitorUpdateInProgress, {});
                check_added_monitors!(nodes[0], 1);
        }
 
@@ -285,7 +285,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
        let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
        {
                chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
-               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_2, &Some(payment_secret_2)), false, APIError::MonitorUpdateFailed, {});
+               unwrap_send_err!(nodes[0].node.send_payment(&route, payment_hash_2, &Some(payment_secret_2)), false, APIError::MonitorUpdateInProgress, {});
                check_added_monitors!(nodes[0], 1);
        }
 
@@ -1962,12 +1962,12 @@ fn test_path_paused_mpp() {
        chanmon_cfgs[0].persister.set_next_update_ret(Some(ChannelMonitorUpdateStatus::InProgress));
 
        // 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.
+       // the second got a MonitorUpdateInProgress 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)) {
                assert_eq!(results.len(), 2);
                if let Ok(()) = results[0] {} else { panic!(); }
-               if let Err(APIError::MonitorUpdateFailed) = results[1] {} else { panic!(); }
+               if let Err(APIError::MonitorUpdateInProgress) = results[1] {} else { panic!(); }
        } else { panic!(); }
        check_added_monitors!(nodes[0], 2);
        chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
index bd3c6d501caaf71e07ef67654584aca04836b510..dbeee41618a79162d3905acc6c69338b9cdab730 100644 (file)
@@ -1166,13 +1166,13 @@ pub enum PaymentSendFailure {
        /// in over-/re-payment.
        ///
        /// The results here are ordered the same as the paths in the route object which was passed to
-       /// send_payment, and any Errs which are not APIError::MonitorUpdateFailed can be safely
-       /// retried (though there is currently no API with which to do so).
+       /// send_payment, and any `Err`s which are not [`APIError::MonitorUpdateInProgress`] can be
+       /// safely retried via [`ChannelManager::retry_payment`].
        ///
-       /// Any entries which contain Err(APIError::MonitorUpdateFailed) or Ok(()) MUST NOT be retried
-       /// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the
-       /// case of Ok(())) or will send once a [`MonitorEvent::Completed`] is provided for the
-       /// next-hop channel with the latest update_id.
+       /// Any entries which contain `Err(APIError::MonitorUpdateInprogress)` or `Ok(())` MUST NOT be
+       /// retried as they will result in over-/re-payment. These HTLCs all either successfully sent
+       /// (in the case of `Ok(())`) or will send once a [`MonitorEvent::Completed`] is provided for
+       /// the next-hop channel with the latest update_id.
        PartialFailure {
                /// The errors themselves, in the same order as the route hops.
                results: Vec<Result<(), APIError>>,
@@ -2469,13 +2469,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                insert_outbound_payment!();
                                                        },
                                                        (ChannelMonitorUpdateStatus::InProgress, Err(_)) => {
-                                                               // Note that MonitorUpdateFailed here indicates (per function docs)
-                                                               // that we will resend the commitment update once monitor updating
-                                                               // is restored. Therefore, we must return an error indicating that
-                                                               // it is unsafe to retry the payment wholesale, which we do in the
-                                                               // send_payment check for MonitorUpdateFailed, below.
+                                                               // Note that MonitorUpdateInProgress here indicates (per function
+                                                               // docs) that we will resend the commitment update once monitor
+                                                               // updating completes. Therefore, we must return an error
+                                                               // indicating that it is unsafe to retry the payment wholesale,
+                                                               // which we do in the send_payment check for
+                                                               // MonitorUpdateInProgress, below.
                                                                insert_outbound_payment!(); // Only do this after possibly break'ing on Perm failure above.
-                                                               return Err(APIError::MonitorUpdateFailed);
+                                                               return Err(APIError::MonitorUpdateInProgress);
                                                        },
                                                        _ => unreachable!(),
                                                }
@@ -2526,12 +2527,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// PaymentSendFailure for more info.
        ///
        /// In general, a path may raise:
-       ///  * APIError::RouteError when an invalid route or forwarding parameter (cltv_delta, fee,
+       ///  * [`APIError::RouteError`] when an invalid route or forwarding parameter (cltv_delta, fee,
        ///    node public key) is specified.
-       ///  * APIError::ChannelUnavailable if the next-hop channel is not available for updates
+       ///  * [`APIError::ChannelUnavailable`] if the next-hop channel is not available for updates
        ///    (including due to previous monitor update failure or new permanent monitor update
        ///    failure).
-       ///  * APIError::MonitorUpdateFailed if a new monitor update failure prevented sending the
+       ///  * [`APIError::MonitorUpdateInProgress`] if a new monitor update failure prevented sending the
        ///    relevant updates.
        ///
        /// Note that depending on the type of the PaymentSendFailure the HTLC may have been
@@ -2595,8 +2596,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                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 {
-                               // MonitorUpdateFailed is inherently unsafe to retry, so we call it a
+                       if let &Err(APIError::MonitorUpdateInProgress) = res {
+                               // MonitorUpdateInProgress is inherently unsafe to retry, so we call it a
                                // PartialFailure.
                                has_err = true;
                                has_ok = true;
index 15dd202b9c83a96064257c3fa528592021594eb8..ad699354233cb705fd41221cee58293a3a62c3d2 100644 (file)
@@ -46,13 +46,15 @@ pub enum APIError {
                /// A human-readable error message
                err: String
        },
-       /// An attempt to call watch/update_channel returned a
-       /// [`ChannelMonitorUpdateStatus::InProgress`] indicating the persistence of a monitor update
-       /// is awaiting async resolution. Once it resolves the attempted action should complete
-       /// automatically.
+       /// An attempt to call [`chain::Watch::watch_channel`]/[`chain::Watch::update_channel`]
+       /// returned a [`ChannelMonitorUpdateStatus::InProgress`] indicating the persistence of a
+       /// monitor update is awaiting async resolution. Once it resolves the attempted action should
+       /// complete automatically.
        ///
+       /// [`chain::Watch::watch_channel`]: crate::chain::Watch::watch_channel
+       /// [`chain::Watch::update_channel`]: crate::chain::Watch::update_channel
        /// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
-       MonitorUpdateFailed,
+       MonitorUpdateInProgress,
        /// [`KeysInterface::get_shutdown_scriptpubkey`] returned a shutdown scriptpubkey incompatible
        /// with the channel counterparty as negotiated in [`InitFeatures`].
        ///
@@ -74,7 +76,7 @@ impl fmt::Debug for APIError {
                        APIError::FeeRateTooHigh {ref err, ref feerate} => write!(f, "{} feerate: {}", err, feerate),
                        APIError::RouteError {ref err} => write!(f, "Route error: {}", err),
                        APIError::ChannelUnavailable {ref err} => write!(f, "Channel unavailable: {}", err),
-                       APIError::MonitorUpdateFailed => f.write_str("Client indicated a channel monitor update failed"),
+                       APIError::MonitorUpdateInProgress => f.write_str("Client indicated a channel monitor update is in progress but not yet complete"),
                        APIError::IncompatibleShutdownScript { ref script } => {
                                write!(f, "Provided a scriptpubkey format not accepted by peer: {}", script)
                        },