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

lightning-invoice/src/payment.rs

@@ -1563,7 +1563,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

@@ -171,7 +171,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);
 	}
 
@@ -222,7 +222,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);
 	}
 
@@ -286,7 +286,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);
 	}
 
@@ -1963,12 +1963,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

@@ -1172,12 +1172,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.
@@ -2476,13 +2476,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!(),
 						}
@@ -2533,12 +2534,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
@@ -2602,8 +2603,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)
 			},