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.

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

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

lightning-invoice/src/payment.rs

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

lightning/src/ln/chanmon_update_fail_tests.rs

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

lightning/src/ln/channelmanager.rs

@@ -1166,12 +1166,12 @@ 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 Errs 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
+	/// 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.
@@ -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;

lightning/src/util/errors.rs

@@ -48,7 +48,7 @@ pub enum APIError {
 	},
 	/// An attempt to call watch/update_channel returned an Err (ie you did this!), causing the
 	/// attempted action to fail.
-	MonitorUpdateFailed,
+	MonitorUpdateInProgress,
 	/// [`KeysInterface::get_shutdown_scriptpubkey`] returned a shutdown scriptpubkey incompatible
 	/// with the channel counterparty as negotiated in [`InitFeatures`].
 	///
@@ -70,7 +70,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)
 			},