From 0dfb24e661e1b5f286bd21812322cca8026c036f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 7 Oct 2021 23:46:13 +0000 Subject: [PATCH] Move `Persist` trait to chainmonitor as that's the only reference --- fuzz/src/utils/test_persister.rs | 4 +- lightning-background-processor/src/lib.rs | 5 +- lightning-block-sync/src/init.rs | 4 +- lightning-net-tokio/src/lib.rs | 2 +- lightning-persister/src/lib.rs | 7 +-- lightning/src/chain/chainmonitor.rs | 66 ++++++++++++++++++++--- lightning/src/chain/channelmonitor.rs | 47 ---------------- lightning/src/ln/channelmanager.rs | 3 +- lightning/src/util/test_utils.rs | 6 +-- 9 files changed, 73 insertions(+), 71 deletions(-) diff --git a/fuzz/src/utils/test_persister.rs b/fuzz/src/utils/test_persister.rs index 9cb5b60b6..5f4a5cf71 100644 --- a/fuzz/src/utils/test_persister.rs +++ b/fuzz/src/utils/test_persister.rs @@ -1,9 +1,9 @@ -use lightning::chain::channelmonitor; +use lightning::chain::{chainmonitor, channelmonitor}; use lightning::chain::transaction::OutPoint; use lightning::util::enforcing_trait_impls::EnforcingSigner; pub struct TestPersister {} -impl channelmonitor::Persist for TestPersister { +impl chainmonitor::Persist for TestPersister { fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> { Ok(()) } diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index fd1ad6fe2..e38a4a975 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -10,8 +10,7 @@ use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; -use lightning::chain::chainmonitor::ChainMonitor; -use lightning::chain::channelmonitor; +use lightning::chain::chainmonitor::{ChainMonitor, Persist}; use lightning::chain::keysinterface::{Sign, KeysInterface}; use lightning::ln::channelmanager::ChannelManager; use lightning::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler}; @@ -194,7 +193,7 @@ impl BackgroundProcessor { K::Target: 'static + KeysInterface, F::Target: 'static + FeeEstimator, L::Target: 'static + Logger, - P::Target: 'static + channelmonitor::Persist, + P::Target: 'static + Persist, CMH::Target: 'static + ChannelMessageHandler, RMH::Target: 'static + RoutingMessageHandler, UMH::Target: 'static + CustomMessageHandler, diff --git a/lightning-block-sync/src/init.rs b/lightning-block-sync/src/init.rs index d51cd3ba6..0bdc2a025 100644 --- a/lightning-block-sync/src/init.rs +++ b/lightning-block-sync/src/init.rs @@ -40,8 +40,8 @@ BlockSourceResult { /// /// use lightning::chain; /// use lightning::chain::Watch; +/// use lightning::chain::chainmonitor; /// use lightning::chain::chainmonitor::ChainMonitor; -/// use lightning::chain::channelmonitor; /// use lightning::chain::channelmonitor::ChannelMonitor; /// use lightning::chain::chaininterface::BroadcasterInterface; /// use lightning::chain::chaininterface::FeeEstimator; @@ -65,7 +65,7 @@ BlockSourceResult { /// F: FeeEstimator, /// L: Logger, /// C: chain::Filter, -/// P: channelmonitor::Persist, +/// P: chainmonitor::Persist, /// >( /// block_source: &mut B, /// chain_monitor: &ChainMonitor, diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs index 13a4c0d93..62c036b97 100644 --- a/lightning-net-tokio/src/lib.rs +++ b/lightning-net-tokio/src/lib.rs @@ -34,7 +34,7 @@ //! type Logger = dyn lightning::util::logger::Logger + Send + Sync; //! type ChainAccess = dyn lightning::chain::Access + Send + Sync; //! type ChainFilter = dyn lightning::chain::Filter + Send + Sync; -//! type DataPersister = dyn lightning::chain::channelmonitor::Persist + Send + Sync; +//! type DataPersister = dyn lightning::chain::chainmonitor::Persist + Send + Sync; //! type ChainMonitor = lightning::chain::chainmonitor::ChainMonitor, Arc, Arc, Arc, Arc>; //! type ChannelManager = Arc>; //! type PeerManager = Arc>; diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs index 6cfa540cf..60f5b0df6 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -18,7 +18,7 @@ use crate::util::DiskWriteable; use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr}; -use lightning::chain::channelmonitor; +use lightning::chain::chainmonitor; use lightning::chain::keysinterface::{Sign, KeysInterface}; use lightning::chain::transaction::OutPoint; use lightning::ln::channelmanager::ChannelManager; @@ -158,7 +158,7 @@ impl FilesystemPersister { } } -impl channelmonitor::Persist for FilesystemPersister { +impl chainmonitor::Persist for FilesystemPersister { fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor) -> Result<(), ChannelMonitorUpdateErr> { let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index); util::write_to_file(self.path_to_monitor_data(), filename, monitor) @@ -180,7 +180,8 @@ mod tests { use bitcoin::blockdata::block::{Block, BlockHeader}; use bitcoin::hashes::hex::FromHex; use bitcoin::Txid; - use lightning::chain::channelmonitor::{Persist, ChannelMonitorUpdateErr}; + use lightning::chain::chainmonitor::Persist; + use lightning::chain::channelmonitor::ChannelMonitorUpdateErr; use lightning::chain::transaction::OutPoint; use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors}; use lightning::ln::features::InitFeatures; diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 0d3f87645..c5468ca85 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -29,8 +29,7 @@ use bitcoin::hash_types::Txid; use chain; use chain::{Filter, WatchedOutput}; use chain::chaininterface::{BroadcasterInterface, FeeEstimator}; -use chain::channelmonitor; -use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, Balance, MonitorEvent, Persist, TransactionOutputs}; +use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, Balance, MonitorEvent, TransactionOutputs}; use chain::transaction::{OutPoint, TransactionData}; use chain::keysinterface::Sign; use util::logger::Logger; @@ -42,6 +41,57 @@ use prelude::*; use sync::RwLock; use core::ops::Deref; +/// `Persist` defines behavior for persisting channel monitors: this could mean +/// writing once to disk, and/or uploading to one or more backup services. +/// +/// Note that for every new monitor, you **must** persist the new `ChannelMonitor` +/// to disk/backups. And, on every update, you **must** persist either the +/// `ChannelMonitorUpdate` or the updated monitor itself. Otherwise, there is risk +/// of situations such as revoking a transaction, then crashing before this +/// revocation can be persisted, then unintentionally broadcasting a revoked +/// transaction and losing money. This is a risk because previous channel states +/// are toxic, so it's important that whatever channel state is persisted is +/// kept up-to-date. +pub trait Persist { + /// Persist a new channel's data. The data can be stored any way you want, but + /// the identifier provided by Rust-Lightning is the channel's outpoint (and + /// it is up to you to maintain a correct mapping between the outpoint and the + /// stored channel data). Note that you **must** persist every new monitor to + /// disk. See the `Persist` trait documentation for more details. + /// + /// See [`Writeable::write`] on [`ChannelMonitor`] for writing out a `ChannelMonitor` + /// and [`ChannelMonitorUpdateErr`] for requirements when returning errors. + /// + /// [`Writeable::write`]: crate::util::ser::Writeable::write + fn persist_new_channel(&self, id: OutPoint, data: &ChannelMonitor) -> Result<(), ChannelMonitorUpdateErr>; + + /// Update one channel's data. The provided `ChannelMonitor` has already + /// applied the given update. + /// + /// Note that on every update, you **must** persist either the + /// `ChannelMonitorUpdate` or the updated monitor itself to disk/backups. See + /// the `Persist` trait documentation for more details. + /// + /// If an implementer chooses to persist the updates only, they need to make + /// sure that all the updates are applied to the `ChannelMonitors` *before* + /// the set of channel monitors is given to the `ChannelManager` + /// deserialization routine. See [`ChannelMonitor::update_monitor`] for + /// applying a monitor update to a monitor. If full `ChannelMonitors` are + /// persisted, then there is no need to persist individual updates. + /// + /// Note that there could be a performance tradeoff between persisting complete + /// channel monitors on every update vs. persisting only updates and applying + /// them in batches. The size of each monitor grows `O(number of state updates)` + /// whereas updates are small and `O(1)`. + /// + /// See [`Writeable::write`] on [`ChannelMonitor`] for writing out a `ChannelMonitor`, + /// [`Writeable::write`] on [`ChannelMonitorUpdate`] for writing out an update, and + /// [`ChannelMonitorUpdateErr`] for requirements when returning errors. + /// + /// [`Writeable::write`]: crate::util::ser::Writeable::write + fn update_persisted_channel(&self, id: OutPoint, update: &ChannelMonitorUpdate, data: &ChannelMonitor) -> Result<(), ChannelMonitorUpdateErr>; +} + /// An implementation of [`chain::Watch`] for monitoring channels. /// /// Connected and disconnected blocks must be provided to `ChainMonitor` as documented by @@ -56,7 +106,7 @@ pub struct ChainMonitor, + P::Target: Persist, { /// The monitors pub monitors: RwLock>>, @@ -72,7 +122,7 @@ where C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, - P::Target: channelmonitor::Persist, + P::Target: Persist, { /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view /// of a channel and reacting accordingly based on transactions in the given chain data. See @@ -183,7 +233,7 @@ where T::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, - P::Target: channelmonitor::Persist, + P::Target: Persist, { fn block_connected(&self, block: &Block, height: u32) { let header = &block.header; @@ -212,7 +262,7 @@ where T::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, - P::Target: channelmonitor::Persist, + P::Target: Persist, { fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) { log_debug!(self.logger, "{} provided transactions confirmed at height {} in block {}", txdata.len(), height, header.block_hash()); @@ -260,7 +310,7 @@ where C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, - P::Target: channelmonitor::Persist, + P::Target: Persist, { /// Adds the monitor that watches the channel referred to by the given outpoint. /// @@ -344,7 +394,7 @@ impl even T::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, - P::Target: channelmonitor::Persist, + P::Target: Persist, { /// Processes [`SpendableOutputs`] events produced from each [`ChannelMonitor`] upon maturity. /// diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c4120e137..571a35fbe 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2877,53 +2877,6 @@ impl ChannelMonitorImpl { } } -/// `Persist` defines behavior for persisting channel monitors: this could mean -/// writing once to disk, and/or uploading to one or more backup services. -/// -/// Note that for every new monitor, you **must** persist the new `ChannelMonitor` -/// to disk/backups. And, on every update, you **must** persist either the -/// `ChannelMonitorUpdate` or the updated monitor itself. Otherwise, there is risk -/// of situations such as revoking a transaction, then crashing before this -/// revocation can be persisted, then unintentionally broadcasting a revoked -/// transaction and losing money. This is a risk because previous channel states -/// are toxic, so it's important that whatever channel state is persisted is -/// kept up-to-date. -pub trait Persist { - /// Persist a new channel's data. The data can be stored any way you want, but - /// the identifier provided by Rust-Lightning is the channel's outpoint (and - /// it is up to you to maintain a correct mapping between the outpoint and the - /// stored channel data). Note that you **must** persist every new monitor to - /// disk. See the `Persist` trait documentation for more details. - /// - /// See [`ChannelMonitor::write`] for writing out a `ChannelMonitor`, - /// and [`ChannelMonitorUpdateErr`] for requirements when returning errors. - fn persist_new_channel(&self, id: OutPoint, data: &ChannelMonitor) -> Result<(), ChannelMonitorUpdateErr>; - - /// Update one channel's data. The provided `ChannelMonitor` has already - /// applied the given update. - /// - /// Note that on every update, you **must** persist either the - /// `ChannelMonitorUpdate` or the updated monitor itself to disk/backups. See - /// the `Persist` trait documentation for more details. - /// - /// If an implementer chooses to persist the updates only, they need to make - /// sure that all the updates are applied to the `ChannelMonitors` *before* - /// the set of channel monitors is given to the `ChannelManager` - /// deserialization routine. See [`ChannelMonitor::update_monitor`] for - /// applying a monitor update to a monitor. If full `ChannelMonitors` are - /// persisted, then there is no need to persist individual updates. - /// - /// Note that there could be a performance tradeoff between persisting complete - /// channel monitors on every update vs. persisting only updates and applying - /// them in batches. The size of each monitor grows `O(number of state updates)` - /// whereas updates are small and `O(1)`. - /// - /// See [`ChannelMonitor::write`] for writing out a `ChannelMonitor`, - /// [`ChannelMonitorUpdate::write`] for writing out an update, and - /// [`ChannelMonitorUpdateErr`] for requirements when returning errors. - fn update_persisted_channel(&self, id: OutPoint, update: &ChannelMonitorUpdate, data: &ChannelMonitor) -> Result<(), ChannelMonitorUpdateErr>; -} - impl chain::Listen for (ChannelMonitor, T, F, L) where T::Target: BroadcasterInterface, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4d105b66f..475f36f43 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6221,8 +6221,7 @@ mod tests { #[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))] pub mod bench { use chain::Listen; - use chain::chainmonitor::ChainMonitor; - use chain::channelmonitor::Persist; + use chain::chainmonitor::{ChainMonitor, Persist}; use chain::keysinterface::{KeysManager, InMemorySigner}; use ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage}; use ln::features::{InitFeatures, InvoiceFeatures}; diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index aa3fcb550..246a50ebd 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -89,7 +89,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface { pub struct TestChainMonitor<'a> { pub added_monitors: Mutex)>>, pub latest_monitor_update_id: Mutex>, - pub chain_monitor: chainmonitor::ChainMonitor>, + pub chain_monitor: chainmonitor::ChainMonitor>, pub keys_manager: &'a TestKeysInterface, pub update_ret: Mutex>>, /// If this is set to Some(), after the next return, we'll always return this until update_ret @@ -101,7 +101,7 @@ pub struct TestChainMonitor<'a> { pub expect_channel_force_closed: Mutex>, } impl<'a> TestChainMonitor<'a> { - pub fn new(chain_source: Option<&'a TestChainSource>, broadcaster: &'a chaininterface::BroadcasterInterface, logger: &'a TestLogger, fee_estimator: &'a TestFeeEstimator, persister: &'a channelmonitor::Persist, keys_manager: &'a TestKeysInterface) -> Self { + pub fn new(chain_source: Option<&'a TestChainSource>, broadcaster: &'a chaininterface::BroadcasterInterface, logger: &'a TestLogger, fee_estimator: &'a TestFeeEstimator, persister: &'a chainmonitor::Persist, keys_manager: &'a TestKeysInterface) -> Self { Self { added_monitors: Mutex::new(Vec::new()), latest_monitor_update_id: Mutex::new(HashMap::new()), @@ -195,7 +195,7 @@ impl TestPersister { *self.update_ret.lock().unwrap() = ret; } } -impl channelmonitor::Persist for TestPersister { +impl chainmonitor::Persist for TestPersister { fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> { self.update_ret.lock().unwrap().clone() } -- 2.39.5