Rename APIError::MonitorUpdateFailed to MonitorUpdateInProgress

This much more accurately represents the error, indicating that a
monitor update is in progress asynchronously and may complete at a
later time.
This commit is contained in:
Matt Corallo 2022-09-29 20:26:48 +00:00
parent fb0a481696
commit f961daef33
5 changed files with 42 additions and 39 deletions

View 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"),

View 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))
}

View 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);

View 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;

View 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)
},