From 87b687962237c3bfc596a0963b1ef2bc2e9ba6f7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 29 Nov 2021 21:36:12 +0000 Subject: [PATCH] Drop `MonitorUpdateErr` in favor of opaque errors. We don't expect users to ever change behavior based on the string contained in a `MonitorUpdateErr`, except log it, so there's little reason to not just log it ourselves and return a `()` for errors. We do so here, simplifying the callsite in `ChainMonitor` as well. --- lightning/src/chain/chainmonitor.rs | 4 +-- lightning/src/chain/channelmonitor.rs | 44 +++++++++++---------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 3c8f9bb56..4ff3bba11 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -639,8 +639,8 @@ where C::Target: chain::Filter, let monitor = &monitor_state.monitor; log_trace!(self.logger, "Updating ChannelMonitor for channel {}", log_funding_info!(monitor)); let update_res = monitor.update_monitor(&update, &self.broadcaster, &self.fee_estimator, &self.logger); - if let Err(e) = &update_res { - log_error!(self.logger, "Failed to update ChannelMonitor for channel {}: {:?}", log_funding_info!(monitor), e); + if update_res.is_err() { + log_error!(self.logger, "Failed to update ChannelMonitor for channel {}.", log_funding_info!(monitor)); } // Even if updating the monitor returns an error, the monitor's state will // still be changed. So, persist the updated monitor despite the error. diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 2ae4bd3de..5f456c7bf 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -115,14 +115,6 @@ impl Readable for ChannelMonitorUpdate { } } -/// General Err type for ChannelMonitor actions. Generally, this implies that the data provided is -/// inconsistent with the ChannelMonitor being called. eg for ChannelMonitor::update_monitor this -/// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was -/// corrupted. -/// Contains a developer-readable error message. -#[derive(Clone, Debug)] -pub struct MonitorUpdateError(pub &'static str); - /// An event to be processed by the ChannelManager. #[derive(Clone, PartialEq)] pub enum MonitorEvent { @@ -1052,7 +1044,7 @@ impl ChannelMonitor { } #[cfg(test)] - fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> { + fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> { self.inner.lock().unwrap().provide_secret(idx, secret) } @@ -1074,12 +1066,10 @@ impl ChannelMonitor { #[cfg(test)] fn provide_latest_holder_commitment_tx( - &self, - holder_commitment_tx: HolderCommitmentTransaction, + &self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, - ) -> Result<(), MonitorUpdateError> { - self.inner.lock().unwrap().provide_latest_holder_commitment_tx( - holder_commitment_tx, htlc_outputs) + ) -> Result<(), ()> { + self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ()) } #[cfg(test)] @@ -1120,7 +1110,7 @@ impl ChannelMonitor { broadcaster: &B, fee_estimator: &F, logger: &L, - ) -> Result<(), MonitorUpdateError> + ) -> Result<(), ()> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -1695,9 +1685,9 @@ impl ChannelMonitorImpl { /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen /// counterparty 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> { + fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> { if let Err(()) = self.commitment_secrets.provide_secret(idx, secret) { - return Err(MonitorUpdateError("Previous secret did not match new one")); + return Err("Previous secret did not match new one"); } // Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill @@ -1789,7 +1779,7 @@ impl ChannelMonitorImpl { /// is important that any clones of this channel monitor (including remote clones) by kept /// up-to-date as our holder commitment transaction is updated. /// Panics if set_on_holder_tx_csv has never been called. - fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), MonitorUpdateError> { + fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), &'static str> { // block for Rust 1.34 compat let mut new_holder_commitment_tx = { let trusted_tx = holder_commitment_tx.trust(); @@ -1812,7 +1802,7 @@ impl ChannelMonitorImpl { mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx); self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx); if self.holder_tx_signed { - return Err(MonitorUpdateError("Latest holder commitment signed has already been signed, update is rejected")); + return Err("Latest holder commitment signed has already been signed, update is rejected"); } Ok(()) } @@ -1876,7 +1866,7 @@ impl ChannelMonitorImpl { self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); } - pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError> + pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -1902,8 +1892,8 @@ impl ChannelMonitorImpl { if self.lockdown_from_offchain { panic!(); } if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone()) { log_error!(logger, "Providing latest holder commitment transaction failed/was refused:"); - log_error!(logger, " {}", e.0); - ret = Err(e); + log_error!(logger, " {}", e); + ret = Err(()); } } ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => { @@ -1918,8 +1908,8 @@ impl ChannelMonitorImpl { log_trace!(logger, "Updating ChannelMonitor with commitment secret"); if let Err(e) = self.provide_secret(*idx, *secret) { log_error!(logger, "Providing latest counterparty commitment secret failed/was refused:"); - log_error!(logger, " {}", e.0); - ret = Err(e); + log_error!(logger, " {}", e); + ret = Err(()); } }, ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => { @@ -1947,9 +1937,9 @@ impl ChannelMonitorImpl { self.latest_update_id = updates.update_id; if ret.is_ok() && self.funding_spend_seen { - ret = Err(MonitorUpdateError("Counterparty attempted to update commitment after funding was spent")); - } - ret + log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent"); + Err(()) + } else { ret } } pub fn get_latest_update_id(&self) -> u64 { -- 2.39.5