f Jeff fixups

Author: valentinewallace

lightning/src/chain/channelmonitor.rs

@@ -153,11 +153,13 @@ pub enum ChannelMonitorUpdateErr {
 /// corrupted.
 /// Contains a human-readable error message.
 #[derive(Debug)]
-pub struct MonitorUpdateError {
-	/// The human-readable error message string.
-	pub msg: &'static str,
-	/// A monitor update may have an error but still need to be persisted to disk.
-	pub persist_updated_monitor: bool
+pub enum MonitorUpdateError {
+	/// While updating the monitor generated an error, still persist the monitor as it has changed
+	/// and needs to be updated on-disk. Contains a human-readable error message.
+	PersistMonitor(&'static str),
+	/// Updating the monitor generated an error, no need to persist the updated monitor. Contains
+	/// a human-readable error message.
+	NoPersistMonitor(&'static str)
 }
 
 /// An event to be processed by the ChannelManager.
@@ -265,10 +267,7 @@ impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref, D: Deref>
 	/// Creates a new object which can be used to monitor several channels given the chain
 	/// interface with which to register to receive notifications.
 	pub fn new(chain_source: Option<C>, broadcaster: T, logger: L, feeest: F, data_persister: D) -> Result<Self, ChannelMonitorUpdateErr> {
-		let monitors = match data_persister.load_channel_data() {
-			Ok(mons) => mons,
-			Err(e) => return Err(e)
-		};
+		let monitors = data_persister.load_channel_data()?;
 
 		Ok(Self {
 			monitors: Mutex::new(monitors),
@@ -294,14 +293,12 @@ impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref, D: Deref>
 			},
 			hash_map::Entry::Vacant(e) => e,
 		};
-		{
-			match self.data_persister.persist_channel_data(outpoint, &monitor) {
-				Err(e) => {
-					log_error!(self.logger, "Failed to persist new channel data");
-					return Err(e);
-				},
-				_ => {}
-			}
+		match self.data_persister.persist_channel_data(outpoint, &monitor) {
+			Err(e) => {
+				log_error!(self.logger, "Failed to persist new channel data");
+				return Err(e);
+			},
+			_ => {}
 		}
 		{
 			let funding_txo = monitor.get_funding_txo();
@@ -327,15 +324,21 @@ impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref, D: Deref>
 			log_trace!(self.logger, "Updating Channel Monitor for channel {}", log_funding_info!(orig_monitor));
 			let mut should_persist = true;
 			let res = orig_monitor.update_monitor(&update, &self.broadcaster, &self.logger);
-			if let Err(MonitorUpdateError{ msg, persist_updated_monitor }) = res {
-				log_error!(self.logger, "{}", msg);
-				should_persist = persist_updated_monitor;
+			match res {
+				Err(MonitorUpdateError::PersistMonitor(msg)) => {
+					log_error!(self.logger, "{}", msg);
+				},
+				Err(MonitorUpdateError::NoPersistMonitor(msg)) => {
+					log_error!(self.logger, "{}", msg);
+					should_persist = false;
+				},
+				Ok(()) => {},
 			}
 			if should_persist {
-					match self.data_persister.update_channel_data(outpoint, &update, orig_monitor) {
-						Err(e) => return Err(e),
-						_ => {}
-					}
+				match self.data_persister.update_channel_data(outpoint, &update, orig_monitor) {
+					Err(e) => return Err(e),
+					_ => {}
+				}
 			}
 			if res.is_err() { return Err(ChannelMonitorUpdateErr::PermanentFailure); }
 		} else {
@@ -1227,10 +1230,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// commitment transaction's secret, they are de facto pruned (we can use revocation key).
 	fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
 		if let Err(()) = self.commitment_secrets.provide_secret(idx, secret) {
-			return Err(MonitorUpdateError{
-				msg: "Previous secret did not match new one",
-				persist_updated_monitor: false
-			});
+			return Err(MonitorUpdateError::NoPersistMonitor("Previous secret did not match new one"));
 		}
 
 		// Prune HTLCs from the previous remote commitment tx so we don't generate failure/fulfill
@@ -1331,10 +1331,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// Panics if set_on_local_tx_csv has never been called.
 	fn provide_latest_local_commitment_tx_info(&mut self, commitment_tx: LocalCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
 		if self.local_tx_signed {
-			return Err(MonitorUpdateError{
-				msg: "A local commitment tx has already been signed, no new local commitment txn can be sent to our counterparty",
-				persist_updated_monitor: false
-			});
+			return Err(MonitorUpdateError::NoPersistMonitor(
+				"A local commitment tx has already been signed, no new local commitment txn can be sent to our counterparty"));
 		}
 		let txid = commitment_tx.txid();
 		let sequence = commitment_tx.unsigned_tx.input[0].sequence as u64;
@@ -1355,10 +1353,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		// for which you want to spend outputs. We're NOT robust again this scenario right
 		// now but we should consider it later.
 		if let Err(_) = self.onchain_tx_handler.provide_latest_local_tx(commitment_tx) {
-			return Err(MonitorUpdateError{
-				msg: "Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed",
-				persist_updated_monitor: false
-			});
+			return Err(MonitorUpdateError::PersistMonitor(
+				"Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed"));
 		}
 		self.current_local_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
 		mem::swap(&mut new_local_commitment_tx, &mut self.current_local_commitment_tx);