Replace WatchEventProvider with chain::Filter
[rust-lightning] / lightning / src / ln / channelmonitor.rs
index d17078554b4702c0af0020685c0b58fe2292076e..fd6334d7029e7ffccf180e3d4a21634c5a3882f1 100644 (file)
 //!
 //! ChannelMonitor objects are generated by ChannelManager in response to relevant
 //! messages/actions, and MUST be persisted to disk (and, preferably, remotely) before progress can
-//! be made in responding to certain messages, see ManyChannelMonitor for more.
+//! be made in responding to certain messages, see [`chain::Watch`] for more.
 //!
 //! Note that ChannelMonitors are an important part of the lightning trust model and a copy of the
 //! latest ChannelMonitor must always be actively monitoring for chain updates (and no out-of-date
 //! ChannelMonitors should do so). Thus, if you're building rust-lightning into an HSM or other
 //! security-domain-separated system design, you should consider having multiple paths for
 //! ChannelMonitors to get out of the HSM and onto monitoring devices.
+//!
+//! [`chain::Watch`]: ../../chain/trait.Watch.html
 
 use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::transaction::{TxOut,Transaction};
@@ -40,7 +42,9 @@ use ln::chan_utils;
 use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
-use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator};
+use chain;
+use chain::Filter;
+use chain::chaininterface::{ChainWatchedUtil, BroadcasterInterface, FeeEstimator};
 use chain::transaction::OutPoint;
 use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys};
 use util::logger::Logger;
@@ -48,9 +52,9 @@ use util::ser::{Readable, MaybeReadable, Writer, Writeable, U48};
 use util::{byte_utils, events};
 use util::events::Event;
 
-use std::collections::{HashMap, hash_map};
+use std::collections::{HashMap, HashSet, hash_map};
 use std::sync::Mutex;
-use std::{hash,cmp, mem};
+use std::{cmp, mem};
 use std::ops::Deref;
 use std::io::Error;
 
@@ -133,11 +137,24 @@ pub enum ChannelMonitorUpdateErr {
        TemporaryFailure,
        /// Used to indicate no further channel monitor updates will be allowed (eg we've moved on to a
        /// different watchtower and cannot update with all watchtowers that were previously informed
-       /// of this channel). This will force-close the channel in question (which will generate one
-       /// final ChannelMonitorUpdate which must be delivered to at least one ChannelMonitor copy).
+       /// of this channel).
+       ///
+       /// At reception of this error, ChannelManager will force-close the channel and return at
+       /// least a final ChannelMonitorUpdate::ChannelForceClosed which must be delivered to at
+       /// least one ChannelMonitor copy. Revocation secret MUST NOT be released and offchain channel
+       /// update must be rejected.
+       ///
+       /// This failure may also signal a failure to update the local persisted copy of one of
+       /// the channel monitor instance.
        ///
-       /// Should also be used to indicate a failure to update the local persisted copy of the channel
-       /// monitor.
+       /// Note that even when you fail a holder commitment transaction update, you must store the
+       /// update to ensure you can claim from it in case of a duplicate copy of this ChannelMonitor
+       /// broadcasts it (e.g distributed channel-monitor deployment)
+       ///
+       /// In case of distributed watchtowers deployment, the new version must be written to disk, as
+       /// state may have been stored but rejected due to a block forcing a commitment broadcast. This
+       /// storage is used to claim outputs of rejected state confirmed onchain by another watchtower,
+       /// lagging behind on block processing.
        PermanentFailure,
 }
 
@@ -159,8 +176,11 @@ pub enum MonitorEvent {
        CommitmentTxBroadcasted(OutPoint),
 }
 
-/// Simple structure send back by ManyChannelMonitor in case of HTLC detected onchain from a
-/// forward channel and from which info are needed to update HTLC in a backward channel.
+/// Simple structure sent back by `chain::Watch` when an HTLC from a forward channel is detected on
+/// chain. Used to update the corresponding HTLC in the backward channel. Failing to pass the
+/// preimage claim backward will lead to loss of funds.
+///
+/// [`chain::Watch`]: ../../chain/trait.Watch.html
 #[derive(Clone, PartialEq)]
 pub struct HTLCUpdate {
        pub(super) payment_hash: PaymentHash,
@@ -169,111 +189,234 @@ pub struct HTLCUpdate {
 }
 impl_writeable!(HTLCUpdate, 0, { payment_hash, payment_preimage, source });
 
-/// A simple implementation of a ManyChannelMonitor and ChainListener. Can be used to create a
-/// watchtower or watch our own channels.
+/// An implementation of [`chain::Watch`] for monitoring channels.
 ///
-/// Note that you must provide your own key by which to refer to channels.
+/// Connected and disconnected blocks must be provided to `ChainMonitor` as documented by
+/// [`chain::Watch`]. May be used in conjunction with [`ChannelManager`] to monitor channels locally
+/// or used independently to monitor channels remotely.
 ///
-/// If you're accepting remote monitors (ie are implementing a watchtower), you must verify that
-/// users cannot overwrite a given channel by providing a duplicate key. ie you should probably
-/// index by a PublicKey which is required to sign any updates.
-///
-/// If you're using this for local monitoring of your own channels, you probably want to use
-/// `OutPoint` as the key, which will give you a ManyChannelMonitor implementation.
-///
-/// (C-not exported) due to an unconstrained generic in `Key`
-pub struct SimpleManyChannelMonitor<Key, ChanSigner: ChannelKeys, T: Deref, F: Deref, L: Deref, C: Deref>
-       where T::Target: BroadcasterInterface,
+/// [`chain::Watch`]: ../../chain/trait.Watch.html
+/// [`ChannelManager`]: ../channelmanager/struct.ChannelManager.html
+pub struct ChainMonitor<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref>
+       where C::Target: chain::Filter,
+        T::Target: BroadcasterInterface,
         F::Target: FeeEstimator,
         L::Target: Logger,
-        C::Target: ChainWatchInterface,
 {
        /// The monitors
-       pub monitors: Mutex<HashMap<Key, ChannelMonitor<ChanSigner>>>,
-       chain_monitor: C,
+       pub monitors: Mutex<HashMap<OutPoint, ChannelMonitor<ChanSigner>>>,
+       watch_events: Mutex<WatchEventCache>,
+       chain_source: Option<C>,
        broadcaster: T,
        logger: L,
        fee_estimator: F
 }
 
-impl<Key : Send + cmp::Eq + hash::Hash, ChanSigner: ChannelKeys, T: Deref + Sync + Send, F: Deref + Sync + Send, L: Deref + Sync + Send, C: Deref + Sync + Send>
-       ChainListener for SimpleManyChannelMonitor<Key, ChanSigner, T, F, L, C>
-       where T::Target: BroadcasterInterface,
+struct WatchEventCache {
+       watched: ChainWatchedUtil,
+       events: Vec<WatchEvent>,
+}
+
+/// An event indicating on-chain activity to watch for pertaining to a channel.
+enum WatchEvent {
+       /// Watch for a transaction with `txid` and having an output with `script_pubkey` as a spending
+       /// condition.
+       WatchTransaction {
+               /// Identifier of the transaction.
+               txid: Txid,
+
+               /// Spending condition for an output of the transaction.
+               script_pubkey: Script,
+       },
+       /// Watch for spends of a transaction output identified by `outpoint` having `script_pubkey` as
+       /// the spending condition.
+       WatchOutput {
+               /// Identifier for the output.
+               outpoint: OutPoint,
+
+               /// Spending condition for the output.
+               script_pubkey: Script,
+       }
+}
+
+impl WatchEventCache {
+       fn new() -> Self {
+               Self {
+                       watched: ChainWatchedUtil::new(),
+                       events: Vec::new(),
+               }
+       }
+
+       fn watch_tx(&mut self, txid: &Txid, script_pubkey: &Script) {
+               if self.watched.register_tx(txid, script_pubkey) {
+                       self.events.push(WatchEvent::WatchTransaction {
+                               txid: *txid,
+                               script_pubkey: script_pubkey.clone()
+                       });
+               }
+       }
+
+       fn watch_output(&mut self, outpoint: (&Txid, usize), script_pubkey: &Script) {
+               let (txid, index) = outpoint;
+               if self.watched.register_outpoint((*txid, index as u32), script_pubkey) {
+                       self.events.push(WatchEvent::WatchOutput {
+                               outpoint: OutPoint {
+                                       txid: *txid,
+                                       index: index as u16,
+                               },
+                               script_pubkey: script_pubkey.clone(),
+                       });
+               }
+       }
+
+       fn flush_events<C: Deref>(&mut self, chain_source: &Option<C>) -> bool where C::Target: chain::Filter {
+               let num_events = self.events.len();
+               match chain_source {
+                       &None => self.events.clear(),
+                       &Some(ref chain_source) => {
+                               for event in self.events.drain(..) {
+                                       match event {
+                                               WatchEvent::WatchTransaction { txid, script_pubkey } => {
+                                                       chain_source.register_tx(txid, script_pubkey)
+                                               },
+                                               WatchEvent::WatchOutput { outpoint, script_pubkey } => {
+                                                       chain_source.register_output(outpoint, script_pubkey)
+                                               },
+                                       }
+                               }
+                       }
+               }
+               num_events > 0
+       }
+
+       fn filter_block<'a>(&self, txdata: &[(usize, &'a Transaction)]) -> Vec<(usize, &'a Transaction)> {
+               let mut matched_txids = HashSet::new();
+               txdata.iter().filter(|&&(_, tx)| {
+                       // A tx matches the filter if it either matches the filter directly (via does_match_tx)
+                       // or if it is a descendant of another matched transaction within the same block.
+                       let mut matched = self.watched.does_match_tx(tx);
+                       for input in tx.input.iter() {
+                               if matched || matched_txids.contains(&input.previous_output.txid) {
+                                       matched = true;
+                                       break;
+                               }
+                       }
+                       if matched {
+                               matched_txids.insert(tx.txid());
+                       }
+                       matched
+               }).map(|e| *e).collect()
+       }
+}
+
+impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref> ChainMonitor<ChanSigner, C, T, F, L>
+       where C::Target: chain::Filter,
+             T::Target: BroadcasterInterface,
              F::Target: FeeEstimator,
              L::Target: Logger,
-        C::Target: ChainWatchInterface,
 {
-       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[usize]) {
-               let block_hash = header.block_hash();
+       /// 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 connected block. See
+       /// [`ChannelMonitor::block_connected`] for details. Any HTLCs that were resolved on chain will
+       /// be returned by [`chain::Watch::release_pending_monitor_events`].
+       ///
+       /// Calls back to [`chain::Filter`] if any monitor indicated new outputs to watch, returning
+       /// `true` if so. Subsequent calls must not exclude any transactions matching the new outputs
+       /// nor any in-block descendants of such transactions. It is not necessary to re-fetch the block
+       /// to obtain updated `txdata`.
+       ///
+       /// [`ChannelMonitor::block_connected`]: struct.ChannelMonitor.html#method.block_connected
+       /// [`chain::Watch::release_pending_monitor_events`]: ../../chain/trait.Watch.html#tymethod.release_pending_monitor_events
+       /// [`chain::Filter`]: ../../chain/trait.Filter.html
+       pub fn block_connected(&self, header: &BlockHeader, txdata: &[(usize, &Transaction)], height: u32) -> bool {
+               let mut watch_events = self.watch_events.lock().unwrap();
+               let matched_txn = watch_events.filter_block(txdata);
                {
                        let mut monitors = self.monitors.lock().unwrap();
                        for monitor in monitors.values_mut() {
-                               let txn_outputs = monitor.block_connected(txn_matched, height, &block_hash, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+                               let txn_outputs = monitor.block_connected(header, &matched_txn, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
 
                                for (ref txid, ref outputs) in txn_outputs {
                                        for (idx, output) in outputs.iter().enumerate() {
-                                               self.chain_monitor.install_watch_outpoint((txid.clone(), idx as u32), &output.script_pubkey);
+                                               watch_events.watch_output((txid, idx), &output.script_pubkey);
                                        }
                                }
                        }
                }
+               watch_events.flush_events(&self.chain_source)
        }
 
-       fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) {
-               let block_hash = header.block_hash();
+       /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
+       /// of a channel based on the disconnected block. See [`ChannelMonitor::block_disconnected`] for
+       /// details.
+       ///
+       /// [`ChannelMonitor::block_disconnected`]: struct.ChannelMonitor.html#method.block_disconnected
+       pub fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) {
                let mut monitors = self.monitors.lock().unwrap();
                for monitor in monitors.values_mut() {
-                       monitor.block_disconnected(disconnected_height, &block_hash, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+                       monitor.block_disconnected(header, disconnected_height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
                }
        }
 }
 
-impl<Key : Send + cmp::Eq + hash::Hash + 'static, ChanSigner: ChannelKeys, T: Deref, F: Deref, L: Deref, C: Deref> SimpleManyChannelMonitor<Key, ChanSigner, T, F, L, C>
-       where T::Target: BroadcasterInterface,
+impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref> ChainMonitor<ChanSigner, C, T, F, L>
+       where C::Target: chain::Filter,
+             T::Target: BroadcasterInterface,
              F::Target: FeeEstimator,
              L::Target: Logger,
-        C::Target: ChainWatchInterface,
 {
-       /// 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_monitor: C, broadcaster: T, logger: L, feeest: F) -> SimpleManyChannelMonitor<Key, ChanSigner, T, F, L, C> {
-               let res = SimpleManyChannelMonitor {
+       /// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
+       ///
+       /// When an optional chain source implementing [`chain::Filter`] is provided, the chain monitor
+       /// will call back to it indicating transactions and outputs of interest. This allows clients to
+       /// pre-filter blocks or only fetch blocks matching a compact filter. Otherwise, clients may
+       /// always need to fetch full blocks absent another means for determining which blocks contain
+       /// transactions relevant to the watched channels.
+       ///
+       /// [`chain::Filter`]: ../../chain/trait.Filter.html
+       pub fn new(chain_source: Option<C>, broadcaster: T, logger: L, feeest: F) -> Self {
+               Self {
                        monitors: Mutex::new(HashMap::new()),
-                       chain_monitor,
+                       watch_events: Mutex::new(WatchEventCache::new()),
+                       chain_source,
                        broadcaster,
                        logger,
                        fee_estimator: feeest,
-               };
-
-               res
+               }
        }
 
-       /// Adds or updates the monitor which monitors the channel referred to by the given key.
-       pub fn add_monitor_by_key(&self, key: Key, monitor: ChannelMonitor<ChanSigner>) -> Result<(), MonitorUpdateError> {
+       /// Adds the monitor that watches the channel referred to by the given outpoint.
+       ///
+       /// Calls back to [`chain::Filter`] with the funding transaction and outputs to watch.
+       ///
+       /// [`chain::Filter`]: ../../chain/trait.Filter.html
+       fn add_monitor(&self, outpoint: OutPoint, monitor: ChannelMonitor<ChanSigner>) -> Result<(), MonitorUpdateError> {
+               let mut watch_events = self.watch_events.lock().unwrap();
                let mut monitors = self.monitors.lock().unwrap();
-               let entry = match monitors.entry(key) {
-                       hash_map::Entry::Occupied(_) => return Err(MonitorUpdateError("Channel monitor for given key is already present")),
+               let entry = match monitors.entry(outpoint) {
+                       hash_map::Entry::Occupied(_) => return Err(MonitorUpdateError("Channel monitor for given outpoint is already present")),
                        hash_map::Entry::Vacant(e) => e,
                };
                {
                        let funding_txo = monitor.get_funding_txo();
                        log_trace!(self.logger, "Got new Channel Monitor for channel {}", log_bytes!(funding_txo.0.to_channel_id()[..]));
-                       self.chain_monitor.install_watch_tx(&funding_txo.0.txid, &funding_txo.1);
-                       self.chain_monitor.install_watch_outpoint((funding_txo.0.txid, funding_txo.0.index as u32), &funding_txo.1);
+                       watch_events.watch_tx(&funding_txo.0.txid, &funding_txo.1);
+                       watch_events.watch_output((&funding_txo.0.txid, funding_txo.0.index as usize), &funding_txo.1);
                        for (txid, outputs) in monitor.get_outputs_to_watch().iter() {
                                for (idx, script) in outputs.iter().enumerate() {
-                                       self.chain_monitor.install_watch_outpoint((*txid, idx as u32), script);
+                                       watch_events.watch_output((txid, idx), script);
                                }
                        }
                }
                entry.insert(monitor);
+               watch_events.flush_events(&self.chain_source);
                Ok(())
        }
 
-       /// Updates the monitor which monitors the channel referred to by the given key.
-       pub fn update_monitor_by_key(&self, key: Key, update: ChannelMonitorUpdate) -> Result<(), MonitorUpdateError> {
+       /// Updates the monitor that watches the channel referred to by the given outpoint.
+       fn update_monitor(&self, outpoint: OutPoint, update: ChannelMonitorUpdate) -> Result<(), MonitorUpdateError> {
                let mut monitors = self.monitors.lock().unwrap();
-               match monitors.get_mut(&key) {
+               match monitors.get_mut(&outpoint) {
                        Some(orig_monitor) => {
                                log_trace!(self.logger, "Updating Channel Monitor for channel {}", log_funding_info!(orig_monitor));
                                orig_monitor.update_monitor(update, &self.broadcaster, &self.logger)
@@ -283,29 +426,29 @@ impl<Key : Send + cmp::Eq + hash::Hash + 'static, ChanSigner: ChannelKeys, T: De
        }
 }
 
-impl<ChanSigner: ChannelKeys, T: Deref + Sync + Send, F: Deref + Sync + Send, L: Deref + Sync + Send, C: Deref + Sync + Send> ManyChannelMonitor for SimpleManyChannelMonitor<OutPoint, ChanSigner, T, F, L, C>
-       where T::Target: BroadcasterInterface,
+impl<ChanSigner: ChannelKeys, C: Deref + Sync + Send, T: Deref + Sync + Send, F: Deref + Sync + Send, L: Deref + Sync + Send> chain::Watch for ChainMonitor<ChanSigner, C, T, F, L>
+       where C::Target: chain::Filter,
+             T::Target: BroadcasterInterface,
              F::Target: FeeEstimator,
              L::Target: Logger,
-        C::Target: ChainWatchInterface,
 {
        type Keys = ChanSigner;
 
-       fn add_monitor(&self, funding_txo: OutPoint, monitor: ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
-               match self.add_monitor_by_key(funding_txo, monitor) {
+       fn watch_channel(&self, funding_txo: OutPoint, monitor: ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
+               match self.add_monitor(funding_txo, monitor) {
                        Ok(_) => Ok(()),
                        Err(_) => Err(ChannelMonitorUpdateErr::PermanentFailure),
                }
        }
 
-       fn update_monitor(&self, funding_txo: OutPoint, update: ChannelMonitorUpdate) -> Result<(), ChannelMonitorUpdateErr> {
-               match self.update_monitor_by_key(funding_txo, update) {
+       fn update_channel(&self, funding_txo: OutPoint, update: ChannelMonitorUpdate) -> Result<(), ChannelMonitorUpdateErr> {
+               match self.update_monitor(funding_txo, update) {
                        Ok(_) => Ok(()),
                        Err(_) => Err(ChannelMonitorUpdateErr::PermanentFailure),
                }
        }
 
-       fn get_and_clear_pending_monitor_events(&self) -> Vec<MonitorEvent> {
+       fn release_pending_monitor_events(&self) -> Vec<MonitorEvent> {
                let mut pending_monitor_events = Vec::new();
                for chan in self.monitors.lock().unwrap().values_mut() {
                        pending_monitor_events.append(&mut chan.get_and_clear_pending_monitor_events());
@@ -314,11 +457,11 @@ impl<ChanSigner: ChannelKeys, T: Deref + Sync + Send, F: Deref + Sync + Send, L:
        }
 }
 
-impl<Key : Send + cmp::Eq + hash::Hash, ChanSigner: ChannelKeys, T: Deref, F: Deref, L: Deref, C: Deref> events::EventsProvider for SimpleManyChannelMonitor<Key, ChanSigner, T, F, L, C>
-       where T::Target: BroadcasterInterface,
+impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref> events::EventsProvider for ChainMonitor<ChanSigner, C, T, F, L>
+       where C::Target: chain::Filter,
+             T::Target: BroadcasterInterface,
              F::Target: FeeEstimator,
              L::Target: Logger,
-        C::Target: ChainWatchInterface,
 {
        fn get_and_clear_pending_events(&self) -> Vec<Event> {
                let mut pending_events = Vec::new();
@@ -824,6 +967,10 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        // Set once we've signed a holder commitment transaction and handed it over to our
        // OnchainTxHandler. After this is set, no future updates to our holder commitment transactions
        // may occur, and we fail any such monitor updates.
+       //
+       // In case of update rejection due to a locally already signed commitment transaction, we
+       // nevertheless store update content to track in case of concurrent broadcast by another
+       // remote monitor out-of-order with regards to the block view.
        holder_tx_signed: bool,
 
        // We simply modify last_block_hash in Channel's block_connected so that serialization is
@@ -835,70 +982,6 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        secp_ctx: Secp256k1<secp256k1::All>, //TODO: dedup this a bit...
 }
 
-/// Simple trait indicating ability to track a set of ChannelMonitors and multiplex events between
-/// them. Generally should be implemented by keeping a local SimpleManyChannelMonitor and passing
-/// events to it, while also taking any add/update_monitor events and passing them to some remote
-/// server(s).
-///
-/// In general, you must always have at least one local copy in memory, which must never fail to
-/// update (as it is responsible for broadcasting the latest state in case the channel is closed),
-/// and then persist it to various on-disk locations. If, for some reason, the in-memory copy fails
-/// to update (eg out-of-memory or some other condition), you must immediately shut down without
-/// taking any further action such as writing the current state to disk. This should likely be
-/// accomplished via panic!() or abort().
-///
-/// Note that any updates to a channel's monitor *must* be applied to each instance of the
-/// channel's monitor everywhere (including remote watchtowers) *before* this function returns. If
-/// an update occurs and a remote watchtower is left with old state, it may broadcast transactions
-/// which we have revoked, allowing our counterparty to claim all funds in the channel!
-///
-/// User needs to notify implementors of ManyChannelMonitor when a new block is connected or
-/// disconnected using their `block_connected` and `block_disconnected` methods. However, rather
-/// than calling these methods directly, the user should register implementors as listeners to the
-/// BlockNotifier and call the BlockNotifier's `block_(dis)connected` methods, which will notify
-/// all registered listeners in one go.
-pub trait ManyChannelMonitor: Send + Sync {
-       /// The concrete type which signs for transactions and provides access to our channel public
-       /// keys.
-       type Keys: ChannelKeys;
-
-       /// Adds a monitor for the given `funding_txo`.
-       ///
-       /// Implementer must also ensure that the funding_txo txid *and* outpoint are registered with
-       /// any relevant ChainWatchInterfaces such that the provided monitor receives block_connected
-       /// callbacks with the funding transaction, or any spends of it.
-       ///
-       /// Further, the implementer must also ensure that each output returned in
-       /// monitor.get_outputs_to_watch() is registered to ensure that the provided monitor learns about
-       /// any spends of any of the outputs.
-       ///
-       /// Any spends of outputs which should have been registered which aren't passed to
-       /// ChannelMonitors via block_connected may result in FUNDS LOSS.
-       fn add_monitor(&self, funding_txo: OutPoint, monitor: ChannelMonitor<Self::Keys>) -> Result<(), ChannelMonitorUpdateErr>;
-
-       /// Updates a monitor for the given `funding_txo`.
-       ///
-       /// Implementer must also ensure that the funding_txo txid *and* outpoint are registered with
-       /// any relevant ChainWatchInterfaces such that the provided monitor receives block_connected
-       /// callbacks with the funding transaction, or any spends of it.
-       ///
-       /// Further, the implementer must also ensure that each output returned in
-       /// monitor.get_watch_outputs() is registered to ensure that the provided monitor learns about
-       /// any spends of any of the outputs.
-       ///
-       /// Any spends of outputs which should have been registered which aren't passed to
-       /// ChannelMonitors via block_connected may result in FUNDS LOSS.
-       fn update_monitor(&self, funding_txo: OutPoint, monitor: ChannelMonitorUpdate) -> Result<(), ChannelMonitorUpdateErr>;
-
-       /// Used by ChannelManager to get list of HTLC resolved onchain and which needed to be updated
-       /// with success or failure.
-       ///
-       /// You should probably just call through to
-       /// ChannelMonitor::get_and_clear_pending_monitor_events() for each ChannelMonitor and return
-       /// the full list.
-       fn get_and_clear_pending_monitor_events(&self) -> Vec<MonitorEvent>;
-}
-
 #[cfg(any(test, feature = "fuzztarget"))]
 /// Used only in testing and fuzztarget to check serialization roundtrips don't change the
 /// underlying object
@@ -1167,12 +1250,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        feerate_per_kw: initial_holder_commitment_tx.feerate_per_kw,
                        htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
                };
-               // Returning a monitor error before updating tracking points means in case of using
-               // a concurrent watchtower implementation for same channel, if this one doesn't
-               // reject update as we do, you MAY have the latest holder valid commitment tx onchain
-               // for which you want to spend outputs. We're NOT robust again this scenario right
-               // now but we should consider it later.
-               onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx).unwrap();
+               onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
 
                ChannelMonitor {
                        latest_update_id: 0,
@@ -1327,9 +1405,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// up-to-date as our holder commitment transaction is updated.
        /// Panics if set_on_holder_tx_csv has never been called.
        pub(super) fn provide_latest_holder_commitment_tx_info(&mut self, commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-               if self.holder_tx_signed {
-                       return Err(MonitorUpdateError("A holder commitment tx has already been signed, no new holder commitment txn can be sent to our counterparty"));
-               }
                let txid = commitment_tx.txid();
                let sequence = commitment_tx.unsigned_tx.input[0].sequence as u64;
                let locktime = commitment_tx.unsigned_tx.lock_time as u64;
@@ -1343,17 +1418,13 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        feerate_per_kw: commitment_tx.feerate_per_kw,
                        htlc_outputs: htlc_outputs,
                };
-               // Returning a monitor error before updating tracking points means in case of using
-               // a concurrent watchtower implementation for same channel, if this one doesn't
-               // reject update as we do, you MAY have the latest holder valid commitment tx onchain
-               // 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_holder_tx(commitment_tx) {
-                       return Err(MonitorUpdateError("Holder commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed"));
-               }
+               self.onchain_tx_handler.provide_latest_holder_tx(commitment_tx);
                self.current_holder_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
                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"));
+               }
                Ok(())
        }
 
@@ -1446,7 +1517,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        }
 
        /// Get the list of HTLCs who's status has been updated on chain. This should be called by
-       /// ChannelManager via ManyChannelMonitor::get_and_clear_pending_monitor_events().
+       /// ChannelManager via [`chain::Watch::release_pending_monitor_events`].
+       ///
+       /// [`chain::Watch::release_pending_monitor_events`]: ../../chain/trait.Watch.html#tymethod.release_pending_monitor_events
        pub fn get_and_clear_pending_monitor_events(&mut self) -> Vec<MonitorEvent> {
                let mut ret = Vec::new();
                mem::swap(&mut ret, &mut self.pending_monitor_events);
@@ -1456,7 +1529,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// Gets the list of pending events which were generated by previous actions, clearing the list
        /// in the process.
        ///
-       /// This is called by ManyChannelMonitor::get_and_clear_pending_events() and is equivalent to
+       /// This is called by ChainMonitor::get_and_clear_pending_events() and is equivalent to
        /// EventsProvider::get_and_clear_pending_events() except that it requires &mut self as we do
        /// no internal locking in ChannelMonitors.
        pub fn get_and_clear_pending_events(&mut self) -> Vec<Event> {
@@ -1875,17 +1948,23 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                Vec::new()
        }
 
-       /// Called by SimpleManyChannelMonitor::block_connected, which implements
-       /// ChainListener::block_connected.
-       /// Eventually this should be pub and, roughly, implement ChainListener, however this requires
-       /// &mut self, as well as returns new spendable outputs and outpoints to watch for spending of
-       /// on-chain.
-       fn block_connected<B: Deref, F: Deref, L: Deref>(&mut self, txn_matched: &[&Transaction], height: u32, block_hash: &BlockHash, broadcaster: B, fee_estimator: F, logger: L)-> Vec<(Txid, Vec<TxOut>)>
+       /// Processes transactions in a newly connected block, which may result in any of the following:
+       /// - update the monitor's state against resolved HTLCs
+       /// - punish the counterparty in the case of seeing a revoked commitment transaction
+       /// - force close the channel and claim/timeout incoming/outgoing HTLCs if near expiration
+       /// - detect settled outputs for later spending
+       /// - schedule and bump any in-flight claims
+       ///
+       /// Returns any transaction outputs from `txn_matched` that spends of should be watched for.
+       /// After called these are also available via [`get_outputs_to_watch`].
+       ///
+       /// [`get_outputs_to_watch`]: #method.get_outputs_to_watch
+       pub fn block_connected<B: Deref, F: Deref, L: Deref>(&mut self, header: &BlockHeader, txn_matched: &[(usize, &Transaction)], height: u32, broadcaster: B, fee_estimator: F, logger: L)-> Vec<(Txid, Vec<TxOut>)>
                where B::Target: BroadcasterInterface,
                      F::Target: FeeEstimator,
                                        L::Target: Logger,
        {
-               for tx in txn_matched {
+               for &(_, tx) in txn_matched {
                        let mut output_val = 0;
                        for out in tx.output.iter() {
                                if out.value > 21_000_000_0000_0000 { panic!("Value-overflowing transaction provided to block connected"); }
@@ -1894,10 +1973,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        }
                }
 
+               let block_hash = header.block_hash();
                log_trace!(logger, "Block {} at height {} connected with {} txn matched", block_hash, height, txn_matched.len());
+
                let mut watch_outputs = Vec::new();
                let mut claimable_outpoints = Vec::new();
-               for tx in txn_matched {
+               for &(_, tx) in txn_matched {
                        if tx.input.len() == 1 {
                                // Assuming our keys were not leaked (in which case we're screwed no matter what),
                                // commitment transactions and HTLC transactions will all only ever have one input,
@@ -1974,7 +2055,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
                self.onchain_tx_handler.block_connected(txn_matched, claimable_outpoints, height, &*broadcaster, &*fee_estimator, &*logger);
 
-               self.last_block_hash = block_hash.clone();
+               self.last_block_hash = block_hash;
                for &(ref txid, ref output_scripts) in watch_outputs.iter() {
                        self.outputs_to_watch.insert(txid.clone(), output_scripts.iter().map(|o| o.script_pubkey.clone()).collect());
                }
@@ -1982,12 +2063,16 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                watch_outputs
        }
 
-       fn block_disconnected<B: Deref, F: Deref, L: Deref>(&mut self, height: u32, block_hash: &BlockHash, broadcaster: B, fee_estimator: F, logger: L)
+       /// Determines if the disconnected block contained any transactions of interest and updates
+       /// appropriately.
+       pub fn block_disconnected<B: Deref, F: Deref, L: Deref>(&mut self, header: &BlockHeader, height: u32, broadcaster: B, fee_estimator: F, logger: L)
                where B::Target: BroadcasterInterface,
                      F::Target: FeeEstimator,
                      L::Target: Logger,
        {
+               let block_hash = header.block_hash();
                log_trace!(logger, "Block {} at height {} disconnected", block_hash, height);
+
                if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + ANTI_REORG_DELAY - 1)) {
                        //We may discard:
                        //- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected
@@ -1996,7 +2081,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
                self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator, logger);
 
-               self.last_block_hash = block_hash.clone();
+               self.last_block_hash = block_hash;
        }
 
        fn would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {