Avoid persisting ChannelManager on handle_tx_* errors

These errors will only ever affect our in-memory state, so there's no
need to persist the ChannelManager when we come across one. Note that
`tx_abort` is not included here because there is a possibility we force
close the channel, which we should persist.

Author: wpaulino

lightning/src/ln/channelmanager.rs

@@ -11252,7 +11252,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	>(
 		&self, counterparty_node_id: &PublicKey, channel_id: ChannelId,
 		tx_msg_handler: HandleTxMsgFn,
-	) -> Result<NotifyOption, MsgHandleErrInternal> {
+	) -> Result<(), MsgHandleErrInternal> {
 		let per_peer_state = self.per_peer_state.read().unwrap();
 		let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
 			debug_assert!(false);
@@ -11267,7 +11267,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 					Ok(msg_send) => {
 						let msg_send_event = msg_send.into_msg_send_event(*counterparty_node_id);
 						peer_state.pending_msg_events.push(msg_send_event);
-						Ok(NotifyOption::SkipPersistHandleEvents)
+						Ok(())
 					},
 					Err(InteractiveTxMsgError {
 						err,
@@ -11305,31 +11305,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 
 	fn internal_tx_add_input(
 		&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput,
-	) -> Result<NotifyOption, MsgHandleErrInternal> {
+	) -> Result<(), MsgHandleErrInternal> {
 		self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
 			channel.tx_add_input(msg, &self.logger)
 		})
 	}
 
 	fn internal_tx_add_output(
 		&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput,
-	) -> Result<NotifyOption, MsgHandleErrInternal> {
+	) -> Result<(), MsgHandleErrInternal> {
 		self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
 			channel.tx_add_output(msg, &self.logger)
 		})
 	}
 
 	fn internal_tx_remove_input(
 		&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput,
-	) -> Result<NotifyOption, MsgHandleErrInternal> {
+	) -> Result<(), MsgHandleErrInternal> {
 		self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
 			channel.tx_remove_input(msg, &self.logger)
 		})
 	}
 
 	fn internal_tx_remove_output(
 		&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput,
-	) -> Result<NotifyOption, MsgHandleErrInternal> {
+	) -> Result<(), MsgHandleErrInternal> {
 		self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
 			channel.tx_remove_output(msg, &self.logger)
 		})
@@ -16020,56 +16020,47 @@ impl<
 	fn handle_tx_add_input(&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput) {
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_add_input(counterparty_node_id, msg);
-			let persist = match &res {
-				Err(_) => NotifyOption::DoPersist,
-				Ok(persist) => *persist,
-			};
+			debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
 			let _ = self.handle_error(res, counterparty_node_id);
-			persist
+			NotifyOption::SkipPersistHandleEvents
 		});
 	}
 
 	fn handle_tx_add_output(&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput) {
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_add_output(counterparty_node_id, msg);
-			let persist = match &res {
-				Err(_) => NotifyOption::DoPersist,
-				Ok(persist) => *persist,
-			};
+			debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
 			let _ = self.handle_error(res, counterparty_node_id);
-			persist
+			NotifyOption::SkipPersistHandleEvents
 		});
 	}
 
 	fn handle_tx_remove_input(&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput) {
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_remove_input(counterparty_node_id, msg);
-			let persist = match &res {
-				Err(_) => NotifyOption::DoPersist,
-				Ok(persist) => *persist,
-			};
+			debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
 			let _ = self.handle_error(res, counterparty_node_id);
-			persist
+			NotifyOption::SkipPersistHandleEvents
 		});
 	}
 
 	fn handle_tx_remove_output(&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput) {
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_remove_output(counterparty_node_id, msg);
-			let persist = match &res {
-				Err(_) => NotifyOption::DoPersist,
-				Ok(persist) => *persist,
-			};
+			debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
 			let _ = self.handle_error(res, counterparty_node_id);
-			persist
+			NotifyOption::SkipPersistHandleEvents
 		});
 	}
 
 	fn handle_tx_complete(&self, counterparty_node_id: PublicKey, msg: &msgs::TxComplete) {
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_complete(counterparty_node_id, msg);
 			let persist = match &res {
-				Err(_) => NotifyOption::DoPersist,
+				Err(err) => {
+					debug_assert!(!err.closes_channel());
+					NotifyOption::SkipPersistHandleEvents
+				},
 				Ok(persist) => *persist,
 			};
 			let _ = self.handle_error(res, counterparty_node_id);