Move ChannelMonitor state behind a Mutex
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index c3382caaea4f8f199b4bf40843788db682c5e6bf..a997694722f4c908e7c9f61698363617310ebc5f 100644 (file)
 //!
 //! [`chain::Watch`]: ../trait.Watch.html
 
-use bitcoin::blockdata::block::BlockHeader;
+use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::blockdata::transaction::{TxOut,Transaction};
 use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
-use bitcoin::consensus::encode;
 
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
@@ -39,25 +38,28 @@ use bitcoin::secp256k1;
 
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
+use chain;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
-use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys};
+use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
 use util::logger::Logger;
-use util::ser::{Readable, MaybeReadable, Writer, Writeable, U48};
+use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, U48};
 use util::byte_utils;
 use util::events::Event;
 
+use std::cell::RefCell;
 use std::collections::{HashMap, HashSet, hash_map};
 use std::{cmp, mem};
-use std::ops::Deref;
 use std::io::Error;
+use std::ops::Deref;
+use std::sync::Mutex;
 
 /// An update generated by the underlying Channel itself which contains some new information the
 /// ChannelMonitor should be made aware of.
-#[cfg_attr(any(test, feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 #[must_use]
 pub struct ChannelMonitorUpdate {
@@ -175,7 +177,7 @@ pub enum ChannelMonitorUpdateErr {
 /// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was
 /// corrupted.
 /// Contains a developer-readable error message.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct MonitorUpdateError(pub &'static str);
 
 /// An event to be processed by the ChannelManager.
@@ -252,6 +254,7 @@ pub(crate) const ANTI_REORG_DELAY: u32 = 6;
 /// end up force-closing the channel on us to claim it.
 pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
 
+// TODO(devrandom) replace this with HolderCommitmentTransaction
 #[derive(Clone, PartialEq)]
 struct HolderSignedTx {
        /// txid of the transaction in tx, just used to make comparison faster
@@ -485,7 +488,7 @@ enum OnchainEvent {
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
-#[cfg_attr(any(test, feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
        LatestHolderCommitmentTXInfo {
@@ -493,7 +496,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
        },
        LatestCounterpartyCommitmentTXInfo {
-               unsigned_commitment_tx: Transaction, // TODO: We should actually only need the txid here
+               commitment_txid: Txid,
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
                commitment_number: u64,
                their_revocation_point: PublicKey,
@@ -527,9 +530,9 @@ impl Writeable for ChannelMonitorUpdateStep {
                                        source.write(w)?;
                                }
                        }
-                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { ref unsigned_commitment_tx, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
+                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
                                1u8.write(w)?;
-                               unsigned_commitment_tx.write(w)?;
+                               commitment_txid.write(w)?;
                                commitment_number.write(w)?;
                                their_revocation_point.write(w)?;
                                (htlc_outputs.len() as u64).write(w)?;
@@ -573,7 +576,7 @@ impl Readable for ChannelMonitorUpdateStep {
                        },
                        1u8 => {
                                Ok(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
-                                       unsigned_commitment_tx: Readable::read(r)?,
+                                       commitment_txid: Readable::read(r)?,
                                        commitment_number: Readable::read(r)?,
                                        their_revocation_point: Readable::read(r)?,
                                        htlc_outputs: {
@@ -617,7 +620,20 @@ impl Readable for ChannelMonitorUpdateStep {
 /// get_and_clear_pending_monitor_events or get_and_clear_pending_events are serialized to disk and
 /// reloaded at deserialize-time. Thus, you must ensure that, when handling events, all events
 /// gotten are fully handled before re-serializing the new state.
-pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
+///
+/// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
+/// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
+/// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
+/// returned block hash and the the current chain and then reconnecting blocks to get to the
+/// best chain) upon deserializing the object!
+pub struct ChannelMonitor<Signer: Sign> {
+       #[cfg(test)]
+       pub(crate) inner: Mutex<ChannelMonitorImpl<Signer>>,
+       #[cfg(not(test))]
+       inner: Mutex<ChannelMonitorImpl<Signer>>,
+}
+
+pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        latest_update_id: u64,
        commitment_transaction_number_obscure_factor: u64,
 
@@ -626,7 +642,8 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        counterparty_payment_script: Script,
        shutdown_script: Script,
 
-       keys: ChanSigner,
+       channel_keys_id: [u8; 32],
+       holder_revocation_basepoint: PublicKey,
        funding_info: (OutPoint, Script),
        current_counterparty_commitment_txid: Option<Txid>,
        prev_counterparty_commitment_txid: Option<Txid>,
@@ -684,9 +701,9 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        outputs_to_watch: HashMap<Txid, Vec<(u32, Script)>>,
 
        #[cfg(test)]
-       pub onchain_tx_handler: OnchainTxHandler<ChanSigner>,
+       pub onchain_tx_handler: OnchainTxHandler<Signer>,
        #[cfg(not(test))]
-       onchain_tx_handler: OnchainTxHandler<ChanSigner>,
+       onchain_tx_handler: OnchainTxHandler<Signer>,
 
        // This is set when the Channel[Manager] generated a ChannelMonitorUpdate which indicated the
        // channel has been force-closed. After this is set, no further holder commitment transaction
@@ -714,14 +731,26 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
 #[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
 /// Used only in testing and fuzztarget to check serialization roundtrips don't change the
 /// underlying object
-impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
+impl<Signer: Sign> PartialEq for ChannelMonitor<Signer> {
+       fn eq(&self, other: &Self) -> bool {
+               let inner = self.inner.lock().unwrap();
+               let other = other.inner.lock().unwrap();
+               inner.eq(&other)
+       }
+}
+
+#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
+/// Used only in testing and fuzztarget to check serialization roundtrips don't change the
+/// underlying object
+impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
        fn eq(&self, other: &Self) -> bool {
                if self.latest_update_id != other.latest_update_id ||
                        self.commitment_transaction_number_obscure_factor != other.commitment_transaction_number_obscure_factor ||
                        self.destination_script != other.destination_script ||
                        self.broadcasted_holder_revokable_script != other.broadcasted_holder_revokable_script ||
                        self.counterparty_payment_script != other.counterparty_payment_script ||
-                       self.keys.pubkeys() != other.keys.pubkeys() ||
+                       self.channel_keys_id != other.channel_keys_id ||
+                       self.holder_revocation_basepoint != other.holder_revocation_basepoint ||
                        self.funding_info != other.funding_info ||
                        self.current_counterparty_commitment_txid != other.current_counterparty_commitment_txid ||
                        self.prev_counterparty_commitment_txid != other.prev_counterparty_commitment_txid ||
@@ -753,20 +782,19 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
        }
 }
 
-impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
-       /// Writes this monitor into the given writer, suitable for writing to disk.
-       ///
-       /// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
-       /// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
-       /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
-       /// returned block hash and the the current chain and then reconnecting blocks to get to the
-       /// best chain) upon deserializing the object!
-       pub fn serialize_for_disk<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
+impl<Signer: Sign> Writeable for ChannelMonitor<Signer> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                //TODO: We still write out all the serialization here manually instead of using the fancy
                //serialization framework we have, we should migrate things over to it.
                writer.write_all(&[SERIALIZATION_VERSION; 1])?;
                writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
 
+               self.inner.lock().unwrap().write(writer)
+       }
+}
+
+impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                self.latest_update_id.write(writer)?;
 
                // Set in initial Channel-object creation, so should always be set by now:
@@ -785,7 +813,8 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                self.counterparty_payment_script.write(writer)?;
                self.shutdown_script.write(writer)?;
 
-               self.keys.write(writer)?;
+               self.channel_keys_id.write(writer)?;
+               self.holder_revocation_basepoint.write(writer)?;
                writer.write_all(&self.funding_info.0.txid[..])?;
                writer.write_all(&byte_utils::be16_to_array(self.funding_info.0.index))?;
                self.funding_info.1.write(writer)?;
@@ -946,13 +975,13 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
        }
 }
 
-impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
-       pub(crate) fn new(keys: ChanSigner, shutdown_pubkey: &PublicKey,
-                       on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
-                       counterparty_htlc_base_key: &PublicKey, counterparty_delayed_payment_base_key: &PublicKey,
-                       on_holder_tx_csv: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
-                       commitment_transaction_number_obscure_factor: u64,
-                       initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
+impl<Signer: Sign> ChannelMonitor<Signer> {
+       pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_pubkey: &PublicKey,
+                         on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
+                         channel_parameters: &ChannelTransactionParameters,
+                         funding_redeemscript: Script, channel_value_satoshis: u64,
+                         commitment_transaction_number_obscure_factor: u64,
+                         initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<Signer> {
 
                assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
                let our_channel_close_key_hash = WPubkeyHash::hash(&shutdown_pubkey.serialize());
@@ -960,75 +989,290 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
                let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
-               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key: *counterparty_delayed_payment_base_key, counterparty_htlc_base_key: *counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
-
-               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), on_holder_tx_csv);
-
-               let holder_tx_sequence = initial_holder_commitment_tx.unsigned_tx.input[0].sequence as u64;
-               let holder_tx_locktime = initial_holder_commitment_tx.unsigned_tx.lock_time as u64;
-               let holder_commitment_tx = HolderSignedTx {
-                       txid: initial_holder_commitment_tx.txid(),
-                       revocation_key: initial_holder_commitment_tx.keys.revocation_key,
-                       a_htlc_key: initial_holder_commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: initial_holder_commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: initial_holder_commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: initial_holder_commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: initial_holder_commitment_tx.feerate_per_kw,
-                       htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+               let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap();
+               let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint;
+               let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint;
+               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
+
+               let channel_keys_id = keys.channel_keys_id();
+               let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;
+
+               // block for Rust 1.34 compat
+               let (holder_commitment_tx, current_holder_commitment_number) = {
+                       let trusted_tx = initial_holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+
+                       let tx_keys = trusted_tx.keys();
+                       let holder_commitment_tx = HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+                       };
+                       (holder_commitment_tx, trusted_tx.commitment_number())
                };
-               onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
+
+               let onchain_tx_handler =
+                       OnchainTxHandler::new(destination_script.clone(), keys,
+                       channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx.clone());
 
                let mut outputs_to_watch = HashMap::new();
                outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
 
                ChannelMonitor {
-                       latest_update_id: 0,
-                       commitment_transaction_number_obscure_factor,
+                       inner: Mutex::new(ChannelMonitorImpl {
+                               latest_update_id: 0,
+                               commitment_transaction_number_obscure_factor,
 
-                       destination_script: destination_script.clone(),
-                       broadcasted_holder_revokable_script: None,
-                       counterparty_payment_script,
-                       shutdown_script,
+                               destination_script: destination_script.clone(),
+                               broadcasted_holder_revokable_script: None,
+                               counterparty_payment_script,
+                               shutdown_script,
 
-                       keys,
-                       funding_info,
-                       current_counterparty_commitment_txid: None,
-                       prev_counterparty_commitment_txid: None,
+                               channel_keys_id,
+                               holder_revocation_basepoint,
+                               funding_info,
+                               current_counterparty_commitment_txid: None,
+                               prev_counterparty_commitment_txid: None,
 
-                       counterparty_tx_cache,
-                       funding_redeemscript,
-                       channel_value_satoshis,
-                       their_cur_revocation_points: None,
+                               counterparty_tx_cache,
+                               funding_redeemscript,
+                               channel_value_satoshis,
+                               their_cur_revocation_points: None,
 
-                       on_holder_tx_csv,
+                               on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
-                       commitment_secrets: CounterpartyCommitmentSecrets::new(),
-                       counterparty_claimable_outpoints: HashMap::new(),
-                       counterparty_commitment_txn_on_chain: HashMap::new(),
-                       counterparty_hash_commitment_number: HashMap::new(),
+                               commitment_secrets: CounterpartyCommitmentSecrets::new(),
+                               counterparty_claimable_outpoints: HashMap::new(),
+                               counterparty_commitment_txn_on_chain: HashMap::new(),
+                               counterparty_hash_commitment_number: HashMap::new(),
 
-                       prev_holder_signed_commitment_tx: None,
-                       current_holder_commitment_tx: holder_commitment_tx,
-                       current_counterparty_commitment_number: 1 << 48,
-                       current_holder_commitment_number: 0xffff_ffff_ffff - ((((holder_tx_sequence & 0xffffff) << 3*8) | (holder_tx_locktime as u64 & 0xffffff)) ^ commitment_transaction_number_obscure_factor),
+                               prev_holder_signed_commitment_tx: None,
+                               current_holder_commitment_tx: holder_commitment_tx,
+                               current_counterparty_commitment_number: 1 << 48,
+                               current_holder_commitment_number,
 
-                       payment_preimages: HashMap::new(),
-                       pending_monitor_events: Vec::new(),
-                       pending_events: Vec::new(),
+                               payment_preimages: HashMap::new(),
+                               pending_monitor_events: Vec::new(),
+                               pending_events: Vec::new(),
 
-                       onchain_events_waiting_threshold_conf: HashMap::new(),
-                       outputs_to_watch,
+                               onchain_events_waiting_threshold_conf: HashMap::new(),
+                               outputs_to_watch,
 
-                       onchain_tx_handler,
+                               onchain_tx_handler,
 
-                       lockdown_from_offchain: false,
-                       holder_tx_signed: false,
+                               lockdown_from_offchain: false,
+                               holder_tx_signed: false,
 
-                       last_block_hash: Default::default(),
-                       secp_ctx: Secp256k1::new(),
+                               last_block_hash: Default::default(),
+                               secp_ctx,
+                       }),
                }
        }
 
+       #[cfg(test)]
+       fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
+               self.inner.lock().unwrap().provide_secret(idx, secret)
+       }
+
+       /// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction.
+       /// The monitor watches for it to be broadcasted and then uses the HTLC information (and
+       /// possibly future revocation/preimage information) to claim outputs where possible.
+       /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
+       pub(crate) fn provide_latest_counterparty_commitment_tx<L: Deref>(
+               &self,
+               txid: Txid,
+               htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
+               commitment_number: u64,
+               their_revocation_point: PublicKey,
+               logger: &L,
+       ) where L::Target: Logger {
+               self.inner.lock().unwrap().provide_latest_counterparty_commitment_tx(
+                       txid, htlc_outputs, commitment_number, their_revocation_point, logger)
+       }
+
+       #[cfg(test)]
+       fn provide_latest_holder_commitment_tx(
+               &self,
+               holder_commitment_tx: HolderCommitmentTransaction,
+               htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
+       ) -> Result<(), MonitorUpdateError> {
+               self.inner.lock().unwrap().provide_latest_holder_commitment_tx(
+                       holder_commitment_tx, htlc_outputs)
+       }
+
+       #[cfg(test)]
+       pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
+               &self,
+               payment_hash: &PaymentHash,
+               payment_preimage: &PaymentPreimage,
+               broadcaster: &B,
+               fee_estimator: &F,
+               logger: &L,
+       ) where
+               B::Target: BroadcasterInterface,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
+       {
+               self.inner.lock().unwrap().provide_payment_preimage(
+                       payment_hash, payment_preimage, broadcaster, fee_estimator, logger)
+       }
+
+       pub(crate) fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(
+               &self,
+               broadcaster: &B,
+               logger: &L,
+       ) where
+               B::Target: BroadcasterInterface,
+               L::Target: Logger,
+       {
+               self.inner.lock().unwrap().broadcast_latest_holder_commitment_txn(broadcaster, logger)
+       }
+
+       /// Updates a ChannelMonitor on the basis of some new information provided by the Channel
+       /// itself.
+       ///
+       /// panics if the given update is not the next update by update_id.
+       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(
+               &self,
+               updates: &ChannelMonitorUpdate,
+               broadcaster: &B,
+               fee_estimator: &F,
+               logger: &L,
+       ) -> Result<(), MonitorUpdateError>
+       where
+               B::Target: BroadcasterInterface,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
+       {
+               self.inner.lock().unwrap().update_monitor(updates, broadcaster, fee_estimator, logger)
+       }
+
+       /// Gets the update_id from the latest ChannelMonitorUpdate which was applied to this
+       /// ChannelMonitor.
+       pub fn get_latest_update_id(&self) -> u64 {
+               self.inner.lock().unwrap().get_latest_update_id()
+       }
+
+       /// Gets the funding transaction outpoint of the channel this ChannelMonitor is monitoring for.
+       pub fn get_funding_txo(&self) -> (OutPoint, Script) {
+               self.inner.lock().unwrap().get_funding_txo().clone()
+       }
+
+       /// Gets a list of txids, with their output scripts (in the order they appear in the
+       /// transaction), which we must learn about spends of via block_connected().
+       ///
+       /// (C-not exported) because we have no HashMap bindings
+       pub fn get_outputs_to_watch(&self) -> HashMap<Txid, Vec<(u32, Script)>> {
+               self.inner.lock().unwrap().get_outputs_to_watch().clone()
+       }
+
+       /// Get the list of HTLCs who's status has been updated on chain. This should be called by
+       /// ChannelManager via [`chain::Watch::release_pending_monitor_events`].
+       ///
+       /// [`chain::Watch::release_pending_monitor_events`]: ../trait.Watch.html#tymethod.release_pending_monitor_events
+       pub fn get_and_clear_pending_monitor_events(&self) -> Vec<MonitorEvent> {
+               self.inner.lock().unwrap().get_and_clear_pending_monitor_events()
+       }
+
+       /// Gets the list of pending events which were generated by previous actions, clearing the list
+       /// in the process.
+       ///
+       /// 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(&self) -> Vec<Event> {
+               self.inner.lock().unwrap().get_and_clear_pending_events()
+       }
+
+       pub(crate) fn get_min_seen_secret(&self) -> u64 {
+               self.inner.lock().unwrap().get_min_seen_secret()
+       }
+
+       pub(crate) fn get_cur_counterparty_commitment_number(&self) -> u64 {
+               self.inner.lock().unwrap().get_cur_counterparty_commitment_number()
+       }
+
+       pub(crate) fn get_cur_holder_commitment_number(&self) -> u64 {
+               self.inner.lock().unwrap().get_cur_holder_commitment_number()
+       }
+
+       /// Used by ChannelManager deserialization to broadcast the latest holder state if its copy of
+       /// the Channel was out-of-date. You may use it to get a broadcastable holder toxic tx in case of
+       /// fallen-behind, i.e when receiving a channel_reestablish with a proof that our counterparty side knows
+       /// a higher revocation secret than the holder commitment number we are aware of. Broadcasting these
+       /// transactions are UNSAFE, as they allow counterparty side to punish you. Nevertheless you may want to
+       /// broadcast them if counterparty don't close channel with his higher commitment transaction after a
+       /// substantial amount of time (a month or even a year) to get back funds. Best may be to contact
+       /// out-of-band the other node operator to coordinate with him if option is available to you.
+       /// In any-case, choice is up to the user.
+       pub fn get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
+       where L::Target: Logger {
+               self.inner.lock().unwrap().get_latest_holder_commitment_txn(logger)
+       }
+
+       /// Unsafe test-only version of get_latest_holder_commitment_txn used by our test framework
+       /// to bypass HolderCommitmentTransaction state update lockdown after signature and generate
+       /// revoked commitment transaction.
+       #[cfg(any(test, feature = "unsafe_revoked_tx_signing"))]
+       pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
+       where L::Target: Logger {
+               self.inner.lock().unwrap().unsafe_get_latest_holder_commitment_txn(logger)
+       }
+
+       /// 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 new outputs to watch from `txdata`; after called, these are also included in
+       /// [`get_outputs_to_watch`].
+       ///
+       /// [`get_outputs_to_watch`]: #method.get_outputs_to_watch
+       pub fn block_connected<B: Deref, F: Deref, L: Deref>(
+               &self,
+               header: &BlockHeader,
+               txdata: &TransactionData,
+               height: u32,
+               broadcaster: B,
+               fee_estimator: F,
+               logger: L,
+       ) -> Vec<(Txid, Vec<(u32, TxOut)>)>
+       where
+               B::Target: BroadcasterInterface,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
+       {
+               self.inner.lock().unwrap().block_connected(
+                       header, txdata, height, broadcaster, fee_estimator, logger)
+       }
+
+       /// Determines if the disconnected block contained any transactions of interest and updates
+       /// appropriately.
+       pub fn block_disconnected<B: Deref, F: Deref, L: Deref>(
+               &self,
+               header: &BlockHeader,
+               height: u32,
+               broadcaster: B,
+               fee_estimator: F,
+               logger: L,
+       ) where
+               B::Target: BroadcasterInterface,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
+       {
+               self.inner.lock().unwrap().block_disconnected(
+                       header, height, broadcaster, fee_estimator, logger)
+       }
+}
+
+impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// 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).
@@ -1080,11 +1324,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                Ok(())
        }
 
-       /// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction.
-       /// The monitor watches for it to be broadcasted and then uses the HTLC information (and
-       /// possibly future revocation/preimage information) to claim outputs where possible.
-       /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
-       pub(crate) fn provide_latest_counterparty_commitment_tx_info<L: Deref>(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
+       pub(crate) fn provide_latest_counterparty_commitment_tx<L: Deref>(&mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
                // TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction
                // so that a remote monitor doesn't learn anything unless there is a malicious close.
                // (only maybe, sadly we cant do the same for local info, as we need to be aware of
@@ -1093,12 +1333,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        self.counterparty_hash_commitment_number.insert(htlc.payment_hash, commitment_number);
                }
 
-               let new_txid = unsigned_commitment_tx.txid();
-               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", new_txid, commitment_number, htlc_outputs.len());
-               log_trace!(logger, "New potential counterparty commitment transaction: {}", encode::serialize_hex(unsigned_commitment_tx));
+               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", txid, commitment_number, htlc_outputs.len());
                self.prev_counterparty_commitment_txid = self.current_counterparty_commitment_txid.take();
-               self.current_counterparty_commitment_txid = Some(new_txid);
-               self.counterparty_claimable_outpoints.insert(new_txid, htlc_outputs.clone());
+               self.current_counterparty_commitment_txid = Some(txid);
+               self.counterparty_claimable_outpoints.insert(txid, htlc_outputs.clone());
                self.current_counterparty_commitment_number = commitment_number;
                //TODO: Merge this into the other per-counterparty-transaction output storage stuff
                match self.their_cur_revocation_points {
@@ -1125,7 +1363,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                htlcs.push(htlc.0);
                        }
                }
-               self.counterparty_tx_cache.per_htlc.insert(new_txid, htlcs);
+               self.counterparty_tx_cache.per_htlc.insert(txid, htlcs);
        }
 
        /// Informs this monitor of the latest holder (ie broadcastable) commitment transaction. The
@@ -1133,22 +1371,25 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// 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_info(&mut self, commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-               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;
-               let mut new_holder_commitment_tx = HolderSignedTx {
-                       txid,
-                       revocation_key: commitment_tx.keys.revocation_key,
-                       a_htlc_key: commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: commitment_tx.feerate_per_kw,
-                       htlc_outputs,
+       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+               // block for Rust 1.34 compat
+               let mut new_holder_commitment_tx = {
+                       let trusted_tx = holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+                       let tx_keys = trusted_tx.keys();
+                       self.current_holder_commitment_number = trusted_tx.commitment_number();
+                       HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs,
+                       }
                };
-               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);
+               self.onchain_tx_handler.provide_latest_holder_tx(holder_commitment_tx);
                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 {
@@ -1159,7 +1400,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
        /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
        /// commitment_tx_infos which contain the payment hash have been revoked.
-       pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L)
+       fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L)
        where B::Target: BroadcasterInterface,
                    F::Target: FeeEstimator,
                    L::Target: Logger,
@@ -1212,10 +1453,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
        }
 
-       /// Updates a ChannelMonitor on the basis of some new information provided by the Channel
-       /// itself.
-       ///
-       /// panics if the given update is not the next update by update_id.
        pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError>
        where B::Target: BroadcasterInterface,
                    F::Target: FeeEstimator,
@@ -1239,11 +1476,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
                                        if self.lockdown_from_offchain { panic!(); }
-                                       self.provide_latest_holder_commitment_tx_info(commitment_tx.clone(), htlc_outputs.clone())?
-                               },
-                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } => {
+                                       self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())?
+                               }
+                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
-                                       self.provide_latest_counterparty_commitment_tx_info(&unsigned_commitment_tx, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
+                                       self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
                                },
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => {
                                        log_trace!(logger, "Updating ChannelMonitor with payment preimage");
@@ -1268,21 +1505,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                Ok(())
        }
 
-       /// Gets the update_id from the latest ChannelMonitorUpdate which was applied to this
-       /// ChannelMonitor.
        pub fn get_latest_update_id(&self) -> u64 {
                self.latest_update_id
        }
 
-       /// Gets the funding transaction outpoint of the channel this ChannelMonitor is monitoring for.
        pub fn get_funding_txo(&self) -> &(OutPoint, Script) {
                &self.funding_info
        }
 
-       /// Gets a list of txids, with their output scripts (in the order they appear in the
-       /// transaction), which we must learn about spends of via block_connected().
-       ///
-       /// (C-not exported) because we have no HashMap bindings
        pub fn get_outputs_to_watch(&self) -> &HashMap<Txid, Vec<(u32, Script)>> {
                // If we've detected a counterparty commitment tx on chain, we must include it in the set
                // of outputs to watch for spends of, otherwise we're likely to lose user funds. Because
@@ -1293,22 +1523,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                &self.outputs_to_watch
        }
 
-       /// Get the list of HTLCs who's status has been updated on chain. This should be called by
-       /// ChannelManager via [`chain::Watch::release_pending_monitor_events`].
-       ///
-       /// [`chain::Watch::release_pending_monitor_events`]: ../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);
                ret
        }
 
-       /// Gets the list of pending events which were generated by previous actions, clearing the list
-       /// in the process.
-       ///
-       /// 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> {
                let mut ret = Vec::new();
                mem::swap(&mut ret, &mut self.pending_events);
@@ -1361,7 +1581,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        let secret = self.get_secret(commitment_number).unwrap();
                        let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
-                       let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.keys.pubkeys().revocation_basepoint));
+                       let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.holder_revocation_basepoint));
                        let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.counterparty_tx_cache.counterparty_delayed_payment_base_key));
 
                        let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.counterparty_tx_cache.on_counterparty_tx_csv, &delayed_key);
@@ -1696,81 +1916,54 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                (claim_requests, (commitment_txid, watch_outputs))
        }
 
-       /// Used by ChannelManager deserialization to broadcast the latest holder state if its copy of
-       /// the Channel was out-of-date. You may use it to get a broadcastable holder toxic tx in case of
-       /// fallen-behind, i.e when receiving a channel_reestablish with a proof that our counterparty side knows
-       /// a higher revocation secret than the holder commitment number we are aware of. Broadcasting these
-       /// transactions are UNSAFE, as they allow counterparty side to punish you. Nevertheless you may want to
-       /// broadcast them if counterparty don't close channel with his higher commitment transaction after a
-       /// substantial amount of time (a month or even a year) to get back funds. Best may be to contact
-       /// out-of-band the other node operator to coordinate with him if option is available to you.
-       /// In any-case, choice is up to the user.
        pub fn get_latest_holder_commitment_txn<L: Deref>(&mut self, logger: &L) -> Vec<Transaction> where L::Target: Logger {
                log_trace!(logger, "Getting signed latest holder commitment transaction!");
                self.holder_tx_signed = true;
-               if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript) {
-                       let txid = commitment_tx.txid();
-                       let mut res = vec![commitment_tx];
-                       for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
-                               if let Some(vout) = htlc.0.transaction_output_index {
-                                       let preimage = if !htlc.0.offered {
-                                                       if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
-                                                               // We can't build an HTLC-Success transaction without the preimage
-                                                               continue;
-                                                       }
-                                               } else { None };
-                                       if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
-                                                       &::bitcoin::OutPoint { txid, vout }, &preimage) {
-                                               res.push(htlc_tx);
+               let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
+               let txid = commitment_tx.txid();
+               let mut res = vec![commitment_tx];
+               for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
+                       if let Some(vout) = htlc.0.transaction_output_index {
+                               let preimage = if !htlc.0.offered {
+                                       if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
+                                               // We can't build an HTLC-Success transaction without the preimage
+                                               continue;
                                        }
+                               } else { None };
+                               if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
+                                       &::bitcoin::OutPoint { txid, vout }, &preimage) {
+                                       res.push(htlc_tx);
                                }
                        }
-                       // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
-                       // The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation.
-                       return res
                }
-               Vec::new()
+               // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
+               // The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation.
+               return res;
        }
 
-       /// Unsafe test-only version of get_latest_holder_commitment_txn used by our test framework
-       /// to bypass HolderCommitmentTransaction state update lockdown after signature and generate
-       /// revoked commitment transaction.
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
-       pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&mut self, logger: &L) -> Vec<Transaction> where L::Target: Logger {
+       fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&mut self, logger: &L) -> Vec<Transaction> where L::Target: Logger {
                log_trace!(logger, "Getting signed copy of latest holder commitment transaction!");
-               if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_holder_tx(&self.funding_redeemscript) {
-                       let txid = commitment_tx.txid();
-                       let mut res = vec![commitment_tx];
-                       for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
-                               if let Some(vout) = htlc.0.transaction_output_index {
-                                       let preimage = if !htlc.0.offered {
-                                                       if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
-                                                               // We can't build an HTLC-Success transaction without the preimage
-                                                               continue;
-                                                       }
-                                               } else { None };
-                                       if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx(
-                                                       &::bitcoin::OutPoint { txid, vout }, &preimage) {
-                                               res.push(htlc_tx);
+               let commitment_tx = self.onchain_tx_handler.get_fully_signed_copy_holder_tx(&self.funding_redeemscript);
+               let txid = commitment_tx.txid();
+               let mut res = vec![commitment_tx];
+               for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
+                       if let Some(vout) = htlc.0.transaction_output_index {
+                               let preimage = if !htlc.0.offered {
+                                       if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
+                                               // We can't build an HTLC-Success transaction without the preimage
+                                               continue;
                                        }
+                               } else { None };
+                               if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx(
+                                       &::bitcoin::OutPoint { txid, vout }, &preimage) {
+                                       res.push(htlc_tx);
                                }
                        }
-                       return res
                }
-               Vec::new()
+               return res
        }
 
-       /// 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 new outputs to watch from `txdata`; after called, these are also included in
-       /// [`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, txdata: &TransactionData, height: u32, broadcaster: B, fee_estimator: F, logger: L)-> Vec<(Txid, Vec<(u32, TxOut)>)>
                where B::Target: BroadcasterInterface,
                      F::Target: FeeEstimator,
@@ -1836,15 +2029,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                }
                if should_broadcast {
                        self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
-                       if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript) {
-                               self.holder_tx_signed = true;
-                               let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
-                               let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
-                               if !new_outputs.is_empty() {
-                                       watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
-                               }
-                               claimable_outpoints.append(&mut new_outpoints);
+                       let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
+                       self.holder_tx_signed = true;
+                       let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
+                       let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
+                       if !new_outputs.is_empty() {
+                               watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
                        }
+                       claimable_outpoints.append(&mut new_outpoints);
                }
                if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) {
                        for ev in events {
@@ -1893,8 +2085,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                watch_outputs
        }
 
-       /// 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,
@@ -2188,22 +2378,24 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                break;
                        } else if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
                                if broadcasted_holder_revokable_script.0 == outp.script_pubkey {
-                                       spendable_output =  Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
+                                       spendable_output =  Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor {
                                                outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
                                                per_commitment_point: broadcasted_holder_revokable_script.1,
                                                to_self_delay: self.on_holder_tx_csv,
                                                output: outp.clone(),
-                                               key_derivation_params: self.keys.key_derivation_params(),
                                                revocation_pubkey: broadcasted_holder_revokable_script.2.clone(),
-                                       });
+                                               channel_keys_id: self.channel_keys_id,
+                                               channel_value_satoshis: self.channel_value_satoshis,
+                                       }));
                                        break;
                                }
                        } else if self.counterparty_payment_script == outp.script_pubkey {
-                               spendable_output = Some(SpendableOutputDescriptor::StaticOutputCounterpartyPayment {
+                               spendable_output = Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor {
                                        outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
                                        output: outp.clone(),
-                                       key_derivation_params: self.keys.key_derivation_params(),
-                               });
+                                       channel_keys_id: self.channel_keys_id,
+                                       channel_value_satoshis: self.channel_value_satoshis,
+                               }));
                                break;
                        } else if outp.script_pubkey == self.shutdown_script {
                                spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
@@ -2238,7 +2430,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 /// 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<Keys: ChannelKeys>: Send + Sync {
+pub trait Persist<ChannelSigner: Sign>: Send + Sync {
        /// 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
@@ -2250,7 +2442,7 @@ pub trait Persist<Keys: ChannelKeys>: Send + Sync {
        ///
        /// [`ChannelMonitor::serialize_for_disk`]: struct.ChannelMonitor.html#method.serialize_for_disk
        /// [`ChannelMonitorUpdateErr`]: enum.ChannelMonitorUpdateErr.html
-       fn persist_new_channel(&self, id: OutPoint, data: &ChannelMonitor<Keys>) -> Result<(), ChannelMonitorUpdateErr>;
+       fn persist_new_channel(&self, id: OutPoint, data: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr>;
 
        /// Update one channel's data. The provided `ChannelMonitor` has already
        /// applied the given update.
@@ -2279,13 +2471,30 @@ pub trait Persist<Keys: ChannelKeys>: Send + Sync {
        /// [`ChannelMonitor::serialize_for_disk`]: struct.ChannelMonitor.html#method.serialize_for_disk
        /// [`ChannelMonitorUpdate::write`]: struct.ChannelMonitorUpdate.html#method.write
        /// [`ChannelMonitorUpdateErr`]: enum.ChannelMonitorUpdateErr.html
-       fn update_persisted_channel(&self, id: OutPoint, update: &ChannelMonitorUpdate, data: &ChannelMonitor<Keys>) -> Result<(), ChannelMonitorUpdateErr>;
+       fn update_persisted_channel(&self, id: OutPoint, update: &ChannelMonitorUpdate, data: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr>;
+}
+
+impl<Signer: Sign, T: Deref, F: Deref, L: Deref> chain::Listen for (RefCell<ChannelMonitor<Signer>>, T, F, L)
+where
+       T::Target: BroadcasterInterface,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
+{
+       fn block_connected(&self, block: &Block, height: u32) {
+               let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
+               self.0.borrow_mut().block_connected(&block.header, &txdata, height, &*self.1, &*self.2, &*self.3);
+       }
+
+       fn block_disconnected(&self, header: &BlockHeader, height: u32) {
+               self.0.borrow_mut().block_disconnected(header, height, &*self.1, &*self.2, &*self.3);
+       }
 }
 
 const MAX_ALLOC_SIZE: usize = 64*1024;
 
-impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor<ChanSigner>) {
-       fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
+               for (BlockHash, ChannelMonitor<Signer>) {
+       fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
                macro_rules! unwrap_obj {
                        ($key: expr) => {
                                match $key {
@@ -2318,7 +2527,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor
                let counterparty_payment_script = Readable::read(reader)?;
                let shutdown_script = Readable::read(reader)?;
 
-               let keys = Readable::read(reader)?;
+               let channel_keys_id = Readable::read(reader)?;
+               let holder_revocation_basepoint = Readable::read(reader)?;
                // Technically this can fail and serialize fail a round-trip, but only for serialization of
                // barely-init'd ChannelMonitors that we can't do anything with.
                let outpoint = OutPoint {
@@ -2518,56 +2728,62 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor
                                return Err(DecodeError::InvalidValue);
                        }
                }
-               let onchain_tx_handler = Readable::read(reader)?;
+               let onchain_tx_handler = ReadableArgs::read(reader, keys_manager)?;
 
                let lockdown_from_offchain = Readable::read(reader)?;
                let holder_tx_signed = Readable::read(reader)?;
 
+               let mut secp_ctx = Secp256k1::new();
+               secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
+
                Ok((last_block_hash.clone(), ChannelMonitor {
-                       latest_update_id,
-                       commitment_transaction_number_obscure_factor,
+                       inner: Mutex::new(ChannelMonitorImpl {
+                               latest_update_id,
+                               commitment_transaction_number_obscure_factor,
 
-                       destination_script,
-                       broadcasted_holder_revokable_script,
-                       counterparty_payment_script,
-                       shutdown_script,
+                               destination_script,
+                               broadcasted_holder_revokable_script,
+                               counterparty_payment_script,
+                               shutdown_script,
 
-                       keys,
-                       funding_info,
-                       current_counterparty_commitment_txid,
-                       prev_counterparty_commitment_txid,
+                               channel_keys_id,
+                               holder_revocation_basepoint,
+                               funding_info,
+                               current_counterparty_commitment_txid,
+                               prev_counterparty_commitment_txid,
 
-                       counterparty_tx_cache,
-                       funding_redeemscript,
-                       channel_value_satoshis,
-                       their_cur_revocation_points,
+                               counterparty_tx_cache,
+                               funding_redeemscript,
+                               channel_value_satoshis,
+                               their_cur_revocation_points,
 
-                       on_holder_tx_csv,
+                               on_holder_tx_csv,
 
-                       commitment_secrets,
-                       counterparty_claimable_outpoints,
-                       counterparty_commitment_txn_on_chain,
-                       counterparty_hash_commitment_number,
+                               commitment_secrets,
+                               counterparty_claimable_outpoints,
+                               counterparty_commitment_txn_on_chain,
+                               counterparty_hash_commitment_number,
 
-                       prev_holder_signed_commitment_tx,
-                       current_holder_commitment_tx,
-                       current_counterparty_commitment_number,
-                       current_holder_commitment_number,
+                               prev_holder_signed_commitment_tx,
+                               current_holder_commitment_tx,
+                               current_counterparty_commitment_number,
+                               current_holder_commitment_number,
 
-                       payment_preimages,
-                       pending_monitor_events,
-                       pending_events,
+                               payment_preimages,
+                               pending_monitor_events,
+                               pending_events,
 
-                       onchain_events_waiting_threshold_conf,
-                       outputs_to_watch,
+                               onchain_events_waiting_threshold_conf,
+                               outputs_to_watch,
 
-                       onchain_tx_handler,
+                               onchain_tx_handler,
 
-                       lockdown_from_offchain,
-                       holder_tx_signed,
+                               lockdown_from_offchain,
+                               holder_tx_signed,
 
-                       last_block_hash,
-                       secp_ctx: Secp256k1::new(),
+                               last_block_hash,
+                               secp_ctx,
+                       }),
                }))
        }
 }
@@ -2589,12 +2805,12 @@ mod tests {
        use ln::channelmanager::{PaymentPreimage, PaymentHash};
        use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
        use ln::chan_utils;
-       use ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction};
+       use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
        use bitcoin::secp256k1::key::{SecretKey,PublicKey};
        use bitcoin::secp256k1::Secp256k1;
        use std::sync::{Arc, Mutex};
-       use chain::keysinterface::InMemoryChannelKeys;
+       use chain::keysinterface::InMemorySigner;
 
        #[test]
        fn test_prune_preimages() {
@@ -2645,12 +2861,12 @@ mod tests {
                macro_rules! test_preimages_exist {
                        ($preimages_slice: expr, $monitor: expr) => {
                                for preimage in $preimages_slice {
-                                       assert!($monitor.payment_preimages.contains_key(&preimage.1));
+                                       assert!($monitor.inner.lock().unwrap().payment_preimages.contains_key(&preimage.1));
                                }
                        }
                }
 
-               let keys = InMemoryChannelKeys::new(
+               let keys = InMemorySigner::new(
                        &secp_ctx,
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        SecretKey::from_slice(&[41; 32]).unwrap(),
@@ -2659,23 +2875,42 @@ mod tests {
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        [41; 32],
                        0,
-                       (0, 0)
+                       [0; 32]
                );
 
+               let counterparty_pubkeys = ChannelPublicKeys {
+                       funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
+                       revocation_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
+                       payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()),
+                       delayed_payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap()),
+                       htlc_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())
+               };
+               let funding_outpoint = OutPoint { txid: Default::default(), index: u16::max_value() };
+               let channel_parameters = ChannelTransactionParameters {
+                       holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+                       holder_selected_contest_delay: 66,
+                       is_outbound_from_holder: true,
+                       counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
+                               pubkeys: counterparty_pubkeys,
+                               selected_contest_delay: 67,
+                       }),
+                       funding_outpoint: Some(funding_outpoint),
+               };
                // Prune with one old state and a holder commitment tx holding a few overlaps with the
                // old state.
-               let mut monitor = ChannelMonitor::new(keys,
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
-                       (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
-                       10, Script::new(), 46, 0, HolderCommitmentTransaction::dummy());
-
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
+               let monitor = ChannelMonitor::new(Secp256k1::new(), keys,
+                                                 &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
+                                                 (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
+                                                 &channel_parameters,
+                                                 Script::new(), 46, 0,
+                                                 HolderCommitmentTransaction::dummy());
+
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
+               let dummy_txid = dummy_tx.txid();
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
                for &(ref preimage, ref hash) in preimages.iter() {
                        monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger);
                }
@@ -2684,31 +2919,31 @@ mod tests {
                let mut secret = [0; 32];
                secret[0..32].clone_from_slice(&hex::decode("7cc854b54e3e0dcdb010d7a3fee464a9687be6e8db3be6854c475621e007a5dc").unwrap());
                monitor.provide_secret(281474976710655, secret.clone()).unwrap();
-               assert_eq!(monitor.payment_preimages.len(), 15);
+               assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 15);
                test_preimages_exist!(&preimages[0..10], monitor);
                test_preimages_exist!(&preimages[15..20], monitor);
 
                // Now provide a further secret, pruning preimages 15-17
                secret[0..32].clone_from_slice(&hex::decode("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap());
                monitor.provide_secret(281474976710654, secret.clone()).unwrap();
-               assert_eq!(monitor.payment_preimages.len(), 13);
+               assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 13);
                test_preimages_exist!(&preimages[0..10], monitor);
                test_preimages_exist!(&preimages[17..20], monitor);
 
                // Now update holder commitment tx info, pruning only element 18 as we still care about the
                // previous commitment tx's preimages too
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
                monitor.provide_secret(281474976710653, secret.clone()).unwrap();
-               assert_eq!(monitor.payment_preimages.len(), 12);
+               assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
                test_preimages_exist!(&preimages[0..10], monitor);
                test_preimages_exist!(&preimages[18..20], monitor);
 
                // But if we do it again, we'll prune 5-10
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
                monitor.provide_secret(281474976710652, secret.clone()).unwrap();
-               assert_eq!(monitor.payment_preimages.len(), 5);
+               assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);
                test_preimages_exist!(&preimages[0..5], monitor);
        }
 
@@ -2781,7 +3016,7 @@ mod tests {
                                sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs);
                        }
                }
-               assert_eq!(base_weight + OnchainTxHandler::<InMemoryChannelKeys>::get_witnesses_weight(&inputs_des[..]),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs));
+               assert_eq!(base_weight + OnchainTxHandler::<InMemorySigner>::get_witnesses_weight(&inputs_des[..]),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs));
 
                // Claim tx with 1 offered HTLCs, 3 received HTLCs
                claim_tx.input.clear();
@@ -2805,7 +3040,7 @@ mod tests {
                                sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs);
                        }
                }
-               assert_eq!(base_weight + OnchainTxHandler::<InMemoryChannelKeys>::get_witnesses_weight(&inputs_des[..]),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs));
+               assert_eq!(base_weight + OnchainTxHandler::<InMemorySigner>::get_witnesses_weight(&inputs_des[..]),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs));
 
                // Justice tx with 1 revoked HTLC-Success tx output
                claim_tx.input.clear();
@@ -2827,7 +3062,7 @@ mod tests {
                                sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs);
                        }
                }
-               assert_eq!(base_weight + OnchainTxHandler::<InMemoryChannelKeys>::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_des.len() - sum_actual_sigs));
+               assert_eq!(base_weight + OnchainTxHandler::<InMemorySigner>::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_des.len() - sum_actual_sigs));
        }
 
        // Further testing is done in the ChannelManager integration tests.