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

@@ -11196,7 +11196,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, bool)> {
+	) -> Result<(), (MsgHandleErrInternal, bool)> {
 		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);
@@ -11211,7 +11211,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,
@@ -11251,31 +11251,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, bool)> {
+	) -> Result<(), (MsgHandleErrInternal, bool)> {
 		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, bool)> {
+	) -> Result<(), (MsgHandleErrInternal, bool)> {
 		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, bool)> {
+	) -> Result<(), (MsgHandleErrInternal, bool)> {
 		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, bool)> {
+	) -> Result<(), (MsgHandleErrInternal, bool)> {
 		self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
 			channel.tx_remove_output(msg, &self.logger)
 		})
@@ -15995,11 +15995,16 @@ 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 (mut persist, exited_quiescence) = match &res {
-				Err((_, exited_quiescence)) => (NotifyOption::DoPersist, *exited_quiescence),
-				Ok(persist) => (*persist, false),
+			let exited_quiescence = match &res {
+				Err((err, exited_quiescence)) => {
+					debug_assert!(!err.closes_channel());
+					*exited_quiescence
+				},
+				Ok(_) => false,
 			};
 			let _ = self.handle_error(res.map_err(|(err, _)| err), counterparty_node_id);
+
+			let mut persist = NotifyOption::SkipPersistHandleEvents;
 			if exited_quiescence {
 				// Note that we're not able to do this inline in `internal_tx_add_input` as we
 				// usually do because we first need to send the `tx_abort` in `handle_error` above
@@ -16015,11 +16020,16 @@ impl<
 	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 (mut persist, exited_quiescence) = match &res {
-				Err((_, exited_quiescence)) => (NotifyOption::DoPersist, *exited_quiescence),
-				Ok(persist) => (*persist, false),
+			let exited_quiescence = match &res {
+				Err((err, exited_quiescence)) => {
+					debug_assert!(!err.closes_channel());
+					*exited_quiescence
+				},
+				Ok(_) => false,
 			};
 			let _ = self.handle_error(res.map_err(|(err, _)| err), counterparty_node_id);
+
+			let mut persist = NotifyOption::SkipPersistHandleEvents;
 			if exited_quiescence {
 				// Note that we're not able to do this inline in `internal_tx_add_output` as we
 				// usually do because we first need to send the `tx_abort` in `handle_error` above
@@ -16035,11 +16045,16 @@ impl<
 	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 (mut persist, exited_quiescence) = match &res {
-				Err((_, exited_quiescence)) => (NotifyOption::DoPersist, *exited_quiescence),
-				Ok(persist) => (*persist, false),
+			let exited_quiescence = match &res {
+				Err((err, exited_quiescence)) => {
+					debug_assert!(!err.closes_channel());
+					*exited_quiescence
+				},
+				Ok(_) => false,
 			};
 			let _ = self.handle_error(res.map_err(|(err, _)| err), counterparty_node_id);
+
+			let mut persist = NotifyOption::SkipPersistHandleEvents;
 			if exited_quiescence {
 				// Note that we're not able to do this inline in `internal_tx_remove_input` as we
 				// usually do because we first need to send the `tx_abort` in `handle_error` above
@@ -16055,11 +16070,16 @@ impl<
 	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 (mut persist, exited_quiescence) = match &res {
-				Err((_, exited_quiescence)) => (NotifyOption::DoPersist, *exited_quiescence),
-				Ok(persist) => (*persist, false),
+			let exited_quiescence = match &res {
+				Err((err, exited_quiescence)) => {
+					debug_assert!(!err.closes_channel());
+					*exited_quiescence
+				},
+				Ok(_) => false,
 			};
 			let _ = self.handle_error(res.map_err(|(err, _)| err), counterparty_node_id);
+
+			let mut persist = NotifyOption::SkipPersistHandleEvents;
 			if exited_quiescence {
 				// Note that we're not able to do this inline in `internal_tx_remove_output` as we
 				// usually do because we first need to send the `tx_abort` in `handle_error` above
@@ -16076,10 +16096,14 @@ impl<
 		let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
 			let res = self.internal_tx_complete(counterparty_node_id, msg);
 			let (mut persist, exited_quiescence) = match &res {
-				Err((_, exited_quiescence)) => (NotifyOption::DoPersist, *exited_quiescence),
+				Err((err, exited_quiescence)) => {
+					debug_assert!(!err.closes_channel());
+					(NotifyOption::SkipPersistHandleEvents, *exited_quiescence)
+				},
 				Ok(persist) => (*persist, false),
 			};
 			let _ = self.handle_error(res.map_err(|(err, _)| err), counterparty_node_id);
+
 			if exited_quiescence {
 				// Note that we're not able to do this inline in `internal_tx_complete` as we
 				// usually do because we first need to send the `tx_abort` in `handle_error` above