Drop redundant parameters in sign_local_commitment_tx
[rust-lightning] / lightning / src / ln / channel.rs
index 317419702095cf7a8adc3e11415b2954b3d522bd..879df8cdd79d062c4edab4af6ad2b99a757e04c7 100644 (file)
@@ -18,9 +18,9 @@ use secp256k1;
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmonitor::ChannelMonitor;
-use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingForwardHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
-use ln::chan_utils::{LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys};
+use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep};
+use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::transaction::OutPoint;
@@ -35,6 +35,7 @@ use std;
 use std::default::Default;
 use std::{cmp,mem,fmt};
 use std::sync::{Arc};
+use std::ops::Deref;
 
 #[cfg(test)]
 pub struct ChannelValueStat {
@@ -240,11 +241,14 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        secp_ctx: Secp256k1<secp256k1::All>,
        channel_value_satoshis: u64,
 
+       latest_monitor_update_id: u64,
+
        #[cfg(not(test))]
        local_keys: ChanSigner,
        #[cfg(test)]
        pub(super) local_keys: ChanSigner,
        shutdown_pubkey: PublicKey,
+       destination_script: Script,
 
        // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction
        // generation start at 0 and count up...this simplifies some parts of implementation at the
@@ -269,7 +273,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        monitor_pending_funding_locked: bool,
        monitor_pending_revoke_and_ack: bool,
        monitor_pending_commitment_signed: bool,
-       monitor_pending_forwards: Vec<(PendingForwardHTLCInfo, u64)>,
+       monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
        monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
 
        // pending_update_fee is filled when sending and receiving update_fee
@@ -291,7 +295,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        holding_cell_update_fee: Option<u64>,
        next_local_htlc_id: u64,
        next_remote_htlc_id: u64,
-       channel_update_count: u32,
+       update_time_counter: u32,
        feerate_per_kw: u64,
 
        #[cfg(debug_assertions)]
@@ -303,6 +307,8 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 
        last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig)
 
+       funding_txo: Option<OutPoint>,
+
        /// The hash of the block in which the funding transaction reached our CONF_TARGET. We use this
        /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full
        /// series of block_connected/block_disconnected calls. Obviously this is not a guarantee as we
@@ -347,7 +353,10 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 
        their_shutdown_scriptpubkey: Option<Script>,
 
-       channel_monitor: ChannelMonitor<ChanSigner>,
+       /// Used exclusively to broadcast the latest local state, mostly a historical quirk that this
+       /// is here:
+       channel_monitor: Option<ChannelMonitor<ChanSigner>>,
+       commitment_secrets: CounterpartyCommitmentSecrets,
 
        network_sync: UpdateStatus,
 
@@ -359,11 +368,18 @@ pub const OUR_MAX_HTLCS: u16 = 50; //TODO
 /// on ice until the funding transaction gets more confirmations, but the LN protocol doesn't
 /// really allow for this, so instead we're stuck closing it out at that point.
 const UNCONF_THRESHOLD: u32 = 6;
-/// Exposing these two constants for use in test in ChannelMonitor
-pub const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
-pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
 const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4
 const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence: 4, script len: 1, witness lengths: 3/4, sig: 73/4, pubkey: 33/4, output: 31 (TODO: Wrong? Useless?)
+
+#[cfg(not(test))]
+const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
+#[cfg(test)]
+pub const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
+#[cfg(not(test))]
+const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
+#[cfg(test)]
+pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
+
 /// Maximmum `funding_satoshis` value, according to the BOLT #2 specification
 /// it's 2^24.
 pub const MAX_FUNDING_SATOSHIS: u64 = (1 << 24);
@@ -371,16 +387,16 @@ pub const MAX_FUNDING_SATOSHIS: u64 = (1 << 24);
 /// Used to return a simple Error back to ChannelManager. Will get converted to a
 /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our
 /// channel_id in ChannelManager.
-pub(super) enum ChannelError<ChanSigner: ChannelKeys> {
+pub(super) enum ChannelError {
        Ignore(&'static str),
        Close(&'static str),
        CloseDelayBroadcast {
                msg: &'static str,
-               update: Option<ChannelMonitor<ChanSigner>>,
+               update: ChannelMonitorUpdate,
        },
 }
 
-impl<ChanSigner: ChannelKeys> fmt::Debug for ChannelError<ChanSigner> {
+impl fmt::Debug for ChannelError {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                match self {
                        &ChannelError::Ignore(e) => write!(f, "Ignore : {}", e),
@@ -417,12 +433,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO
        }
 
-       fn derive_our_htlc_minimum_msat(_at_open_channel_feerate_per_kw: u64) -> u64 {
-               1000 // TODO
-       }
-
        // Constructors:
-       pub fn new_outbound(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface<ChanKeySigner = ChanSigner>>, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel<ChanSigner>, APIError> {
+       pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel<ChanSigner>, APIError>
+       where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
+             F::Target: FeeEstimator,
+       {
                let chan_keys = keys_provider.get_channel_keys(false, channel_value_satoshis);
 
                if channel_value_satoshis >= MAX_FUNDING_SATOSHIS {
@@ -444,12 +459,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
 
-               let secp_ctx = Secp256k1::new();
-               let channel_monitor = ChannelMonitor::new(chan_keys.clone(),
-                                                         chan_keys.funding_key(), chan_keys.revocation_base_key(), chan_keys.delayed_payment_base_key(),
-                                                         chan_keys.htlc_base_key(), chan_keys.payment_base_key(), &keys_provider.get_shutdown_pubkey(), config.own_channel_config.our_to_self_delay,
-                                                         keys_provider.get_destination_script(), logger.clone());
-
                Ok(Channel {
                        user_id: user_id,
                        config: config.channel_options.clone(),
@@ -457,11 +466,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        channel_id: keys_provider.get_channel_id(),
                        channel_state: ChannelState::OurInitSent as u32,
                        channel_outbound: true,
-                       secp_ctx: secp_ctx,
+                       secp_ctx: Secp256k1::new(),
                        channel_value_satoshis: channel_value_satoshis,
 
+                       latest_monitor_update_id: 0,
+
                        local_keys: chan_keys,
                        shutdown_pubkey: keys_provider.get_shutdown_pubkey(),
+                       destination_script: keys_provider.get_destination_script(),
+
                        cur_local_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
                        cur_remote_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
                        value_to_self_msat: channel_value_satoshis * 1000 - push_msat,
@@ -473,7 +486,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        holding_cell_update_fee: None,
                        next_local_htlc_id: 0,
                        next_remote_htlc_id: 0,
-                       channel_update_count: 1,
+                       update_time_counter: 1,
 
                        resend_order: RAACommitmentOrder::CommitmentFirst,
 
@@ -490,6 +503,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        last_sent_closing_fee: None,
 
+                       funding_txo: None,
                        funding_tx_confirmed_in: None,
                        short_channel_id: None,
                        last_block_connected: Default::default(),
@@ -501,7 +515,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_max_htlc_value_in_flight_msat: 0,
                        their_channel_reserve_satoshis: 0,
                        their_htlc_minimum_msat: 0,
-                       our_htlc_minimum_msat: Channel::<ChanSigner>::derive_our_htlc_minimum_msat(feerate),
+                       our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: 0,
                        our_to_self_delay: config.own_channel_config.our_to_self_delay,
                        their_max_accepted_htlcs: 0,
@@ -515,7 +529,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        their_shutdown_scriptpubkey: None,
 
-                       channel_monitor: channel_monitor,
+                       channel_monitor: None,
+                       commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
                        network_sync: UpdateStatus::Fresh,
 
@@ -523,7 +538,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                })
        }
 
-       fn check_remote_fee(fee_estimator: &FeeEstimator, feerate_per_kw: u32) -> Result<(), ChannelError<ChanSigner>> {
+       fn check_remote_fee<F: Deref>(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError>
+               where F::Target: FeeEstimator
+       {
                if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) {
                        return Err(ChannelError::Close("Peer's feerate much too low"));
                }
@@ -535,7 +552,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Creates a new channel from a remote sides' request for one.
        /// Assumes chain_hash has already been checked and corresponds with what we expect!
-       pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface<ChanKeySigner = ChanSigner>>, their_node_id: PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel<ChanSigner>, ChannelError<ChanSigner>> {
+       pub fn new_from_req<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, their_node_id: PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel<ChanSigner>, ChannelError>
+               where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
+          F::Target: FeeEstimator
+       {
                let mut chan_keys = keys_provider.get_channel_keys(true, msg.funding_satoshis);
                let their_pubkeys = ChannelPublicKeys {
                        funding_pubkey: msg.funding_pubkey,
@@ -643,12 +663,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
                }
 
-               let secp_ctx = Secp256k1::new();
-               let channel_monitor = ChannelMonitor::new(chan_keys.clone(),
-                                                         chan_keys.funding_key(), chan_keys.revocation_base_key(), chan_keys.delayed_payment_base_key(),
-                                                         chan_keys.htlc_base_key(), chan_keys.payment_base_key(), &keys_provider.get_shutdown_pubkey(), config.own_channel_config.our_to_self_delay,
-                                                         keys_provider.get_destination_script(), logger.clone());
-
                let their_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
                        match &msg.shutdown_scriptpubkey {
                                &OptionalField::Present(ref script) => {
@@ -670,17 +684,21 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                } else { None };
 
-               let mut chan = Channel {
+               let chan = Channel {
                        user_id: user_id,
                        config: local_config,
 
                        channel_id: msg.temporary_channel_id,
                        channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
                        channel_outbound: false,
-                       secp_ctx: secp_ctx,
+                       secp_ctx: Secp256k1::new(),
+
+                       latest_monitor_update_id: 0,
 
                        local_keys: chan_keys,
                        shutdown_pubkey: keys_provider.get_shutdown_pubkey(),
+                       destination_script: keys_provider.get_destination_script(),
+
                        cur_local_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
                        cur_remote_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
                        value_to_self_msat: msg.push_msat,
@@ -692,7 +710,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        holding_cell_update_fee: None,
                        next_local_htlc_id: 0,
                        next_remote_htlc_id: 0,
-                       channel_update_count: 1,
+                       update_time_counter: 1,
 
                        resend_order: RAACommitmentOrder::CommitmentFirst,
 
@@ -709,6 +727,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        last_sent_closing_fee: None,
 
+                       funding_txo: None,
                        funding_tx_confirmed_in: None,
                        short_channel_id: None,
                        last_block_connected: Default::default(),
@@ -721,7 +740,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
                        their_channel_reserve_satoshis: msg.channel_reserve_satoshis,
                        their_htlc_minimum_msat: msg.htlc_minimum_msat,
-                       our_htlc_minimum_msat: Channel::<ChanSigner>::derive_our_htlc_minimum_msat(msg.feerate_per_kw as u64),
+                       our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: msg.to_self_delay,
                        our_to_self_delay: config.own_channel_config.our_to_self_delay,
                        their_max_accepted_htlcs: msg.max_accepted_htlcs,
@@ -735,17 +754,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        their_shutdown_scriptpubkey,
 
-                       channel_monitor: channel_monitor,
+                       channel_monitor: None,
+                       commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
                        network_sync: UpdateStatus::Fresh,
 
                        logger,
                };
 
-               let obscure_factor = chan.get_commitment_transaction_number_obscure_factor();
-               let funding_redeemscript = chan.get_funding_redeemscript();
-               chan.channel_monitor.set_basic_channel_info(&msg.htlc_basepoint, &msg.delayed_payment_basepoint, msg.to_self_delay, funding_redeemscript, msg.funding_satoshis, obscure_factor);
-
                Ok(chan)
        }
 
@@ -804,7 +820,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let txins = {
                        let mut ins: Vec<TxIn> = Vec::new();
                        ins.push(TxIn {
-                               previous_output: self.channel_monitor.get_funding_txo().unwrap().into_bitcoin_outpoint(),
+                               previous_output: self.funding_txo.unwrap().into_bitcoin_outpoint(),
                                script_sig: Script::new(),
                                sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32),
                                witness: Vec::new(),
@@ -1023,7 +1039,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let txins = {
                        let mut ins: Vec<TxIn> = Vec::new();
                        ins.push(TxIn {
-                               previous_output: self.channel_monitor.get_funding_txo().unwrap().into_bitcoin_outpoint(),
+                               previous_output: self.funding_txo.unwrap().into_bitcoin_outpoint(),
                                script_sig: Script::new(),
                                sequence: 0xffffffff,
                                witness: Vec::new(),
@@ -1082,7 +1098,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// our counterparty!)
        /// The result is a transaction which we can revoke ownership of (ie a "local" transaction)
        /// TODO Some magic rust shit to compile-time check this?
-       fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError<ChanSigner>> {
+       fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
                let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
                let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
                let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
@@ -1095,7 +1111,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Creates a set of keys for build_commitment_transaction to generate a transaction which we
        /// will sign and send to our counterparty.
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
-       fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError<ChanSigner>> {
+       fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
                let payment_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_base_key());
@@ -1122,9 +1138,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
-       /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
-       /// Ok(_) if debug assertions are turned on and preconditions are met.
-       fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitor<ChanSigner>>), ChannelError<ChanSigner>> {
+       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
+       /// return Ok(_) if debug assertions are turned on or preconditions are met.
+       ///
+       /// Note that it is still possible to hit these assertions in case we find a preimage on-chain
+       /// but then have a reorg which settles on an HTLC-failure on chain.
+       fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
                // Either ChannelFunded got set (which means it won't be unset) or there is no way any
                // caller thought we could have something claimed (cause we wouldn't have accepted in an
                // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us,
@@ -1151,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                } else {
                                                        log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
                                                }
+                                               debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
                                                return Ok((None, None));
                                        },
                                        _ => {
@@ -1170,13 +1190,23 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                //
                // We have to put the payment_preimage in the channel_monitor right away here to ensure we
                // can claim it even if the channel hits the chain before we see their next commitment.
-               self.channel_monitor.provide_payment_preimage(&payment_hash_calc, &payment_preimage_arg);
+               self.latest_monitor_update_id += 1;
+               let monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
+                               payment_preimage: payment_preimage_arg.clone(),
+                       }],
+               };
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
 
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 {
                        for pending_update in self.holding_cell_htlc_updates.iter() {
                                match pending_update {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
+                                                       // Make sure we don't leave latest_monitor_update_id incremented here:
+                                                       self.latest_monitor_update_id -= 1;
+                                                       debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
                                                        return Ok((None, None));
                                                }
                                        },
@@ -1185,7 +1215,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                        log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
                                                        // TODO: We may actually be able to switch to a fulfill here, though its
                                                        // rare enough it may not be worth the complexity burden.
-                                                       return Ok((None, Some(self.channel_monitor.clone())));
+                                                       debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
+                                                       return Ok((None, Some(monitor_update)));
                                                }
                                        },
                                        _ => {}
@@ -1195,7 +1226,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
-                       return Ok((None, Some(self.channel_monitor.clone())));
+                       return Ok((None, Some(monitor_update)));
                }
 
                {
@@ -1203,7 +1234,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        if let InboundHTLCState::Committed = htlc.state {
                        } else {
                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
-                               return Ok((None, Some(self.channel_monitor.clone())));
+                               return Ok((None, Some(monitor_update)));
                        }
                        log_trace!(self, "Upgrading HTLC {} to LocalRemoved with a Fulfill!", log_bytes!(htlc.payment_hash.0));
                        htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone()));
@@ -1213,24 +1244,35 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        channel_id: self.channel_id(),
                        htlc_id: htlc_id_arg,
                        payment_preimage: payment_preimage_arg,
-               }), Some(self.channel_monitor.clone())))
+               }), Some(monitor_update)))
        }
 
-       pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitor<ChanSigner>>), ChannelError<ChanSigner>> {
+       pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitorUpdate>), ChannelError> {
                match self.get_update_fulfill_htlc(htlc_id, payment_preimage)? {
-                       (Some(update_fulfill_htlc), _) => {
+                       (Some(update_fulfill_htlc), Some(mut monitor_update)) => {
+                               let (commitment, mut additional_update) = self.send_commitment_no_status_check()?;
+                               // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
+                               // strictly increasing by one, so decrement it here.
+                               self.latest_monitor_update_id = monitor_update.update_id;
+                               monitor_update.updates.append(&mut additional_update.updates);
+                               Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
+                       },
+                       (Some(update_fulfill_htlc), None) => {
                                let (commitment, monitor_update) = self.send_commitment_no_status_check()?;
                                Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
                        },
-                       (None, Some(channel_monitor)) => Ok((None, Some(channel_monitor))),
+                       (None, Some(monitor_update)) => Ok((None, Some(monitor_update))),
                        (None, None) => Ok((None, None))
                }
        }
 
        /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
-       /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
-       /// Ok(_) if debug assertions are turned on and preconditions are met.
-       pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError<ChanSigner>> {
+       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
+       /// return Ok(_) if debug assertions are turned on or preconditions are met.
+       ///
+       /// Note that it is still possible to hit these assertions in case we find a preimage on-chain
+       /// but then have a reorg which settles on an HTLC-failure on chain.
+       pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Was asked to fail an HTLC when channel was not in an operational state");
                }
@@ -1246,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                match htlc.state {
                                        InboundHTLCState::Committed => {},
                                        InboundHTLCState::LocalRemoved(_) => {
+                                               debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
                                                return Ok(None);
                                        },
                                        _ => {
@@ -1266,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                match pending_update {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
+                                                       debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
                                                        return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
+                                                       debug_assert!(false, "Tried to fail an HTLC that was already failed");
                                                        return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
                                                }
                                        },
@@ -1298,7 +1343,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        // Message handlers:
 
-       pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> {
                // Check sanity of message fields:
                if !self.channel_outbound {
                        return Err(ChannelError::Close("Got an accept_channel message from an inbound peer"));
@@ -1400,16 +1445,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.their_cur_commitment_point = Some(msg.first_per_commitment_point);
                self.their_shutdown_scriptpubkey = their_shutdown_scriptpubkey;
 
-               let obscure_factor = self.get_commitment_transaction_number_obscure_factor();
-               let funding_redeemscript = self.get_funding_redeemscript();
-               self.channel_monitor.set_basic_channel_info(&msg.htlc_basepoint, &msg.delayed_payment_basepoint, msg.to_self_delay, funding_redeemscript, self.channel_value_satoshis, obscure_factor);
-
                self.channel_state = ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32;
 
                Ok(())
        }
 
-       fn funding_created_signature(&mut self, sig: &Signature) -> Result<(Transaction, LocalCommitmentTransaction, Signature, TxCreationKeys), ChannelError<ChanSigner>> {
+       fn funding_created_signature(&mut self, sig: &Signature) -> Result<(Transaction, LocalCommitmentTransaction, Signature, TxCreationKeys), ChannelError> {
                let funding_script = self.get_funding_redeemscript();
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
@@ -1434,7 +1475,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                &self.their_pubkeys.as_ref().expect("their_funding_pubkey() only allowed after accept_channel").funding_pubkey
        }
 
-       pub fn funding_created(&mut self, msg: &msgs::FundingCreated) -> Result<(msgs::FundingSigned, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       pub fn funding_created(&mut self, msg: &msgs::FundingCreated) -> Result<(msgs::FundingSigned, ChannelMonitor<ChanSigner>), ChannelError> {
                if self.channel_outbound {
                        return Err(ChannelError::Close("Received funding_created for an outbound channel?"));
                }
@@ -1444,28 +1485,47 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // channel.
                        return Err(ChannelError::Close("Received funding_created after we got the channel!"));
                }
-               if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
+               if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
                                self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
                let funding_txo = OutPoint::new(msg.funding_txid, msg.funding_output_index);
-               let funding_txo_script = self.get_funding_redeemscript().to_v0_p2wsh();
-               self.channel_monitor.set_funding_info((funding_txo, funding_txo_script));
+               self.funding_txo = Some(funding_txo.clone());
 
                let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature, local_keys) = match self.funding_created_signature(&msg.signature) {
                        Ok(res) => res,
                        Err(e) => {
-                               self.channel_monitor.unset_funding_info();
+                               self.funding_txo = None;
                                return Err(e);
                        }
                };
 
                // Now that we're past error-generating stuff, update our local state:
 
-               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
-               self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx, local_keys, self.feerate_per_kw, Vec::new());
+               let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
+               let funding_redeemscript = self.get_funding_redeemscript();
+               let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
+               macro_rules! create_monitor {
+                       () => { {
+                               let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
+                                                                             &self.shutdown_pubkey, self.our_to_self_delay,
+                                                                             &self.destination_script, (funding_txo, funding_txo_script.clone()),
+                                                                             &their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
+                                                                             self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
+                                                                             self.get_commitment_transaction_number_obscure_factor(),
+                                                                             self.logger.clone());
+
+                               channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
+                               channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), local_keys.clone(), self.feerate_per_kw, Vec::new()).unwrap();
+                               channel_monitor
+                       } }
+               }
+
+               self.channel_monitor = Some(create_monitor!());
+               let channel_monitor = create_monitor!();
+
                self.channel_state = ChannelState::FundingSent as u32;
                self.channel_id = funding_txo.to_channel_id();
                self.cur_remote_commitment_transaction_number -= 1;
@@ -1474,19 +1534,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok((msgs::FundingSigned {
                        channel_id: self.channel_id,
                        signature: our_signature
-               }, self.channel_monitor.clone()))
+               }, channel_monitor))
        }
 
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
-       pub fn funding_signed(&mut self, msg: &msgs::FundingSigned) -> Result<ChannelMonitor<ChanSigner>, ChannelError<ChanSigner>> {
+       pub fn funding_signed(&mut self, msg: &msgs::FundingSigned) -> Result<ChannelMonitorUpdate, (Option<ChannelMonitorUpdate>, ChannelError)> {
                if !self.channel_outbound {
-                       return Err(ChannelError::Close("Received funding_signed for an inbound channel?"));
+                       return Err((None, ChannelError::Close("Received funding_signed for an inbound channel?")));
                }
                if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 {
-                       return Err(ChannelError::Close("Received funding_signed in strange state!"));
+                       return Err((None, ChannelError::Close("Received funding_signed in strange state!")));
                }
-               if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
+               if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER - 1 ||
                                self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
@@ -1494,29 +1554,38 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let funding_script = self.get_funding_redeemscript();
 
-               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
+               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number).map_err(|e| (None, e))?;
                let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
                let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
                let their_funding_pubkey = &self.their_pubkeys.as_ref().unwrap().funding_pubkey;
 
                // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
-               secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, their_funding_pubkey), "Invalid funding_signed signature from peer");
+               if let Err(_) = self.secp_ctx.verify(&local_sighash, &msg.signature, their_funding_pubkey) {
+                       return Err((None, ChannelError::Close("Invalid funding_signed signature from peer")));
+               }
 
-               self.channel_monitor.provide_latest_local_commitment_tx_info(
-                       LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey),
-                       local_keys, self.feerate_per_kw, Vec::new());
+               self.latest_monitor_update_id += 1;
+               let monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
+                               commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey),
+                               local_keys, feerate_per_kw: self.feerate_per_kw, htlc_outputs: Vec::new(),
+                       }]
+               };
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
                self.channel_state = ChannelState::FundingSent as u32 | (self.channel_state & (ChannelState::MonitorUpdateFailed as u32));
                self.cur_local_commitment_transaction_number -= 1;
 
                if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                       Ok(self.channel_monitor.clone())
+                       Ok(monitor_update)
                } else {
-                       Err(ChannelError::Ignore("Previous monitor update failure prevented funding_signed from allowing funding broadcast"))
+                       Err((Some(monitor_update),
+                               ChannelError::Ignore("Previous monitor update failure prevented funding_signed from allowing funding broadcast")))
                }
        }
 
-       pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        return Err(ChannelError::Close("Peer sent funding_locked when we needed a channel_reestablish"));
                }
@@ -1527,7 +1596,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        self.channel_state |= ChannelState::TheirFundingLocked as u32;
                } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
                        self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
-                       self.channel_update_count += 1;
+                       self.update_time_counter += 1;
                } else if (self.channel_state & (ChannelState::ChannelFunded as u32) != 0 &&
                                 // Note that funding_signed/funding_created will have decremented both by 1!
                                 self.cur_local_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 &&
@@ -1587,7 +1656,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
        }
 
-       pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
                }
@@ -1597,6 +1666,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.amount_msat > self.channel_value_satoshis * 1000 {
                        return Err(ChannelError::Close("Remote side tried to send more than the total value of the channel"));
                }
+               if msg.amount_msat == 0 {
+                       return Err(ChannelError::Close("Remote side tried to send a 0-msat HTLC"));
+               }
                if msg.amount_msat < self.our_htlc_minimum_msat {
                        return Err(ChannelError::Close("Remote side tried to send less than our minimum HTLC value"));
                }
@@ -1661,7 +1733,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Marks an outbound HTLC which we have received update_fail/fulfill/malformed
        #[inline]
-       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<PaymentHash>, fail_reason: Option<HTLCFailReason>) -> Result<&HTLCSource, ChannelError<ChanSigner>> {
+       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<PaymentHash>, fail_reason: Option<HTLCFailReason>) -> Result<&HTLCSource, ChannelError> {
                for htlc in self.pending_outbound_htlcs.iter_mut() {
                        if htlc.htlc_id == htlc_id {
                                match check_preimage {
@@ -1686,7 +1758,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find"))
        }
 
-       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<HTLCSource, ChannelError<ChanSigner>> {
+       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<HTLCSource, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state"));
                }
@@ -1698,7 +1770,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|source| source.clone())
        }
 
-       pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got fail HTLC message when channel was not in an operational state"));
                }
@@ -1710,7 +1782,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       pub fn update_fail_malformed_htlc<'a>(&mut self, msg: &msgs::UpdateFailMalformedHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn update_fail_malformed_htlc<'a>(&mut self, msg: &msgs::UpdateFailMalformedHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got fail malformed HTLC message when channel was not in an operational state"));
                }
@@ -1722,20 +1794,20 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &FeeEstimator) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, Option<msgs::ClosingSigned>, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       pub fn commitment_signed<F: Deref>(&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &F) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate), (Option<ChannelMonitorUpdate>, ChannelError)> where F::Target: FeeEstimator {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(ChannelError::Close("Got commitment signed message when channel was not in an operational state"));
+                       return Err((None, ChannelError::Close("Got commitment signed message when channel was not in an operational state")));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent commitment_signed when we needed a channel_reestablish"));
+                       return Err((None, ChannelError::Close("Peer sent commitment_signed when we needed a channel_reestablish")));
                }
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK == BOTH_SIDES_SHUTDOWN_MASK && self.last_sent_closing_fee.is_some() {
-                       return Err(ChannelError::Close("Peer sent commitment_signed after we'd started exchanging closing_signeds"));
+                       return Err((None, ChannelError::Close("Peer sent commitment_signed after we'd started exchanging closing_signeds")));
                }
 
                let funding_script = self.get_funding_redeemscript();
 
-               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
+               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number).map_err(|e| (None, e))?;
 
                let mut update_fee = false;
                let feerate_per_kw = if !self.channel_outbound && self.pending_update_fee.is_some() {
@@ -1753,7 +1825,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let local_commitment_txid = local_commitment_tx.0.txid();
                let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
                log_trace!(self, "Checking commitment tx signature {} by key {} against tx {} with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_commitment_tx.0), encode::serialize_hex(&funding_script));
-               secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey()), "Invalid commitment tx signature from peer");
+               if let Err(_) = self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey()) {
+                       return Err((None, ChannelError::Close("Invalid commitment tx signature from peer")));
+               }
 
                //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
                if update_fee {
@@ -1761,12 +1835,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
 
                        if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
-                               return Err(ChannelError::Close("Funding remote cannot afford proposed new fee"));
+                               return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee")));
                        }
                }
 
                if msg.htlc_signatures.len() != local_commitment_tx.1 {
-                       return Err(ChannelError::Close("Got wrong number of HTLC signatures from remote"));
+                       return Err((None, ChannelError::Close("Got wrong number of HTLC signatures from remote")));
                }
 
                let mut htlcs_and_sigs = Vec::with_capacity(local_commitment_tx.2.len());
@@ -1776,7 +1850,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
                                log_trace!(self, "Checking HTLC tx signature {} by key {} against tx {} with redeemscript {}", log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(local_keys.b_htlc_key.serialize()), encode::serialize_hex(&htlc_tx), encode::serialize_hex(&htlc_redeemscript));
                                let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
-                               secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
+                               if let Err(_) = self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key) {
+                                       return Err((None, ChannelError::Close("Invalid HTLC tx signature from peer")));
+                               }
                                htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source));
                        } else {
                                htlcs_and_sigs.push((htlc, None, source));
@@ -1803,9 +1879,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let their_funding_pubkey = self.their_pubkeys.as_ref().unwrap().funding_pubkey;
 
-               self.channel_monitor.provide_latest_local_commitment_tx_info(
-                       LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey),
-                       local_keys, self.feerate_per_kw, htlcs_and_sigs);
+               self.latest_monitor_update_id += 1;
+               let mut monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
+                               commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey),
+                               local_keys, feerate_per_kw: self.feerate_per_kw, htlc_outputs: htlcs_and_sigs
+                       }]
+               };
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
 
                for htlc in self.pending_inbound_htlcs.iter_mut() {
                        let new_forward = if let &InboundHTLCState::RemoteAnnounced(ref forward_info) = &htlc.state {
@@ -1838,26 +1920,31 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                // If we were going to send a commitment_signed after the RAA, go ahead and do all
                                // the corresponding HTLC status updates so that get_last_commitment_update
                                // includes the right HTLCs.
-                               // Note that this generates a monitor update that we ignore! This is OK since we
-                               // won't actually send the commitment_signed that generated the update to the other
-                               // side until the latest monitor has been pulled from us and stored.
                                self.monitor_pending_commitment_signed = true;
-                               self.send_commitment_no_status_check()?;
+                               let (_, mut additional_update) = self.send_commitment_no_status_check().map_err(|e| (None, e))?;
+                               // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
+                               // strictly increasing by one, so decrement it here.
+                               self.latest_monitor_update_id = monitor_update.update_id;
+                               monitor_update.updates.append(&mut additional_update.updates);
                        }
                        // TODO: Call maybe_propose_first_closing_signed on restoration (or call it here and
                        // re-send the message on restoration)
-                       return Err(ChannelError::Ignore("Previous monitor update failure prevented generation of RAA"));
+                       return Err((Some(monitor_update), ChannelError::Ignore("Previous monitor update failure prevented generation of RAA")));
                }
 
-               let (our_commitment_signed, monitor_update, closing_signed) = if need_our_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 {
+               let (our_commitment_signed, closing_signed) = if need_our_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 {
                        // If we're AwaitingRemoteRevoke we can't send a new commitment here, but that's ok -
                        // we'll send one right away when we get the revoke_and_ack when we
                        // free_holding_cell_htlcs().
-                       let (msg, monitor) = self.send_commitment_no_status_check()?;
-                       (Some(msg), monitor, None)
+                       let (msg, mut additional_update) = self.send_commitment_no_status_check().map_err(|e| (None, e))?;
+                       // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
+                       // strictly increasing by one, so decrement it here.
+                       self.latest_monitor_update_id = monitor_update.update_id;
+                       monitor_update.updates.append(&mut additional_update.updates);
+                       (Some(msg), None)
                } else if !need_our_commitment {
-                       (None, self.channel_monitor.clone(), self.maybe_propose_first_closing_signed(fee_estimator))
-               } else { (None, self.channel_monitor.clone(), None) };
+                       (None, self.maybe_propose_first_closing_signed(fee_estimator))
+               } else { (None, None) };
 
                Ok((msgs::RevokeAndACK {
                        channel_id: self.channel_id,
@@ -1868,11 +1955,16 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Used to fulfill holding_cell_htlcs when we get a remote ack (or implicitly get it by them
        /// fulfilling or failing the last pending HTLC)
-       fn free_holding_cell_htlcs(&mut self) -> Result<Option<(msgs::CommitmentUpdate, ChannelMonitor<ChanSigner>)>, ChannelError<ChanSigner>> {
+       fn free_holding_cell_htlcs(&mut self) -> Result<Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, ChannelError> {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, 0);
                if self.holding_cell_htlc_updates.len() != 0 || self.holding_cell_update_fee.is_some() {
                        log_trace!(self, "Freeing holding cell with {} HTLC updates{}", self.holding_cell_htlc_updates.len(), if self.holding_cell_update_fee.is_some() { " and a fee update" } else { "" });
 
+                       let mut monitor_update = ChannelMonitorUpdate {
+                               update_id: self.latest_monitor_update_id + 1, // We don't increment this yet!
+                               updates: Vec::new(),
+                       };
+
                        let mut htlc_updates = Vec::new();
                        mem::swap(&mut htlc_updates, &mut self.holding_cell_htlc_updates);
                        let mut update_add_htlcs = Vec::with_capacity(htlc_updates.len());
@@ -1907,7 +1999,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                },
                                                &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
                                                        match self.get_update_fulfill_htlc(htlc_id, *payment_preimage) {
-                                                               Ok(update_fulfill_msg_option) => update_fulfill_htlcs.push(update_fulfill_msg_option.0.unwrap()),
+                                                               Ok((update_fulfill_msg_option, additional_monitor_update_opt)) => {
+                                                                       update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap());
+                                                                       if let Some(mut additional_monitor_update) = additional_monitor_update_opt {
+                                                                               monitor_update.updates.append(&mut additional_monitor_update.updates);
+                                                                       }
+                                                               },
                                                                Err(e) => {
                                                                        if let ChannelError::Ignore(_) = e {}
                                                                        else {
@@ -1957,7 +2054,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                } else {
                                                        None
                                                };
-                                       let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
+
+                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check()?;
+                                       // send_commitment_no_status_check and get_update_fulfill_htlc may bump latest_monitor_id
+                                       // but we want them to be strictly increasing by one, so reset it here.
+                                       self.latest_monitor_update_id = monitor_update.update_id;
+                                       monitor_update.updates.append(&mut additional_update.updates);
+
                                        Ok(Some((msgs::CommitmentUpdate {
                                                update_add_htlcs,
                                                update_fulfill_htlcs,
@@ -1979,7 +2082,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// waiting on this revoke_and_ack. The generation of this new commitment_signed may also fail,
        /// generating an appropriate error *after* the channel state has been updated based on the
        /// revoke_and_ack message.
-       pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &FeeEstimator) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option<msgs::ClosingSigned>, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       pub fn revoke_and_ack<F: Deref>(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &F) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate), ChannelError>
+               where F::Target: FeeEstimator
+       {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Close("Got revoke/ACK message when channel was not in an operational state"));
                }
@@ -1995,8 +2100,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                return Err(ChannelError::Close("Got a revoke commitment secret which didn't correspond to their current pubkey"));
                        }
                }
-               self.channel_monitor.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret)
-                       .map_err(|e| ChannelError::Close(e.0))?;
 
                if self.channel_state & ChannelState::AwaitingRemoteRevoke as u32 == 0 {
                        // Our counterparty seems to have burned their coins to us (by revoking a state when we
@@ -2009,6 +2112,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Received an unexpected revoke_and_ack"));
                }
 
+               self.commitment_secrets.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret)
+                       .map_err(|_| ChannelError::Close("Previous secrets did not match new one"))?;
+               self.latest_monitor_update_id += 1;
+               let mut monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::CommitmentSecret {
+                               idx: self.cur_remote_commitment_transaction_number + 1,
+                               secret: msg.per_commitment_secret,
+                       }],
+               };
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+
                // Update state now that we've passed all the can-fail calls...
                // (note that we may still fail to generate the new commitment_signed message, but that's
                // OK, we step the channel here and *then* if the new generation fails we can fail the
@@ -2134,28 +2249,44 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                // When the monitor updating is restored we'll call get_last_commitment_update(),
                                // which does not update state, but we're definitely now awaiting a remote revoke
                                // before we can step forward any more, so set it here.
-                               self.send_commitment_no_status_check()?;
+                               let (_, mut additional_update) = self.send_commitment_no_status_check()?;
+                               // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
+                               // strictly increasing by one, so decrement it here.
+                               self.latest_monitor_update_id = monitor_update.update_id;
+                               monitor_update.updates.append(&mut additional_update.updates);
                        }
                        self.monitor_pending_forwards.append(&mut to_forward_infos);
                        self.monitor_pending_failures.append(&mut revoked_htlcs);
-                       return Ok((None, Vec::new(), Vec::new(), None, self.channel_monitor.clone()));
+                       return Ok((None, Vec::new(), Vec::new(), None, monitor_update))
                }
 
                match self.free_holding_cell_htlcs()? {
-                       Some(mut commitment_update) => {
-                               commitment_update.0.update_fail_htlcs.reserve(update_fail_htlcs.len());
+                       Some((mut commitment_update, mut additional_update)) => {
+                               commitment_update.update_fail_htlcs.reserve(update_fail_htlcs.len());
                                for fail_msg in update_fail_htlcs.drain(..) {
-                                       commitment_update.0.update_fail_htlcs.push(fail_msg);
+                                       commitment_update.update_fail_htlcs.push(fail_msg);
                                }
-                               commitment_update.0.update_fail_malformed_htlcs.reserve(update_fail_malformed_htlcs.len());
+                               commitment_update.update_fail_malformed_htlcs.reserve(update_fail_malformed_htlcs.len());
                                for fail_msg in update_fail_malformed_htlcs.drain(..) {
-                                       commitment_update.0.update_fail_malformed_htlcs.push(fail_msg);
+                                       commitment_update.update_fail_malformed_htlcs.push(fail_msg);
                                }
-                               Ok((Some(commitment_update.0), to_forward_infos, revoked_htlcs, None, commitment_update.1))
+
+                               // free_holding_cell_htlcs may bump latest_monitor_id multiple times but we want them to be
+                               // strictly increasing by one, so decrement it here.
+                               self.latest_monitor_update_id = monitor_update.update_id;
+                               monitor_update.updates.append(&mut additional_update.updates);
+
+                               Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, None, monitor_update))
                        },
                        None => {
                                if require_commitment {
-                                       let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
+                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check()?;
+
+                                       // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
+                                       // strictly increasing by one, so decrement it here.
+                                       self.latest_monitor_update_id = monitor_update.update_id;
+                                       monitor_update.updates.append(&mut additional_update.updates);
+
                                        Ok((Some(msgs::CommitmentUpdate {
                                                update_add_htlcs: Vec::new(),
                                                update_fulfill_htlcs: Vec::new(),
@@ -2165,7 +2296,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                commitment_signed
                                        }), to_forward_infos, revoked_htlcs, None, monitor_update))
                                } else {
-                                       Ok((None, to_forward_infos, revoked_htlcs, self.maybe_propose_first_closing_signed(fee_estimator), self.channel_monitor.clone()))
+                                       Ok((None, to_forward_infos, revoked_htlcs, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update))
                                }
                        }
                }
@@ -2200,7 +2331,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                })
        }
 
-       pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u64) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitor<ChanSigner>)>, ChannelError<ChanSigner>> {
+       pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u64) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> {
                match self.send_update_fee(feerate_per_kw) {
                        Some(update_fee) => {
                                let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
@@ -2285,7 +2416,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// which failed. The messages which were generated from that call which generated the
        /// monitor update failure must *not* have been sent to the remote end, and must instead
        /// have been dropped. They will be regenerated when monitor_updating_restored is called.
-       pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingForwardHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) {
+       pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, 0);
                self.monitor_pending_revoke_and_ack = resend_raa;
                self.monitor_pending_commitment_signed = resend_commitment;
@@ -2299,7 +2430,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Indicates that the latest ChannelMonitor update has been committed by the client
        /// successfully and we should restore normal operation. Returns messages which should be sent
        /// to the remote side.
-       pub fn monitor_updating_restored(&mut self) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingForwardHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option<msgs::FundingLocked>) {
+       pub fn monitor_updating_restored(&mut self) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option<msgs::FundingLocked>) {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
@@ -2351,7 +2482,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                (raa, commitment_update, order, forwards, failures, needs_broadcast_safe, funding_locked)
        }
 
-       pub fn update_fee(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::UpdateFee) -> Result<(), ChannelError<ChanSigner>> {
+       pub fn update_fee<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::UpdateFee) -> Result<(), ChannelError>
+               where F::Target: FeeEstimator
+       {
                if self.channel_outbound {
                        return Err(ChannelError::Close("Non-funding remote tried to update channel fee"));
                }
@@ -2360,7 +2493,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                Channel::<ChanSigner>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
                self.pending_update_fee = Some(msg.feerate_per_kw as u64);
-               self.channel_update_count += 1;
+               self.update_time_counter += 1;
                Ok(())
        }
 
@@ -2433,7 +2566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// May panic if some calls other than message-handling calls (which will all Err immediately)
        /// have been called between remove_uncommitted_htlcs_and_mark_paused and this call.
-       pub fn channel_reestablish(&mut self, msg: &msgs::ChannelReestablish) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, Option<ChannelMonitor<ChanSigner>>, RAACommitmentOrder, Option<msgs::Shutdown>), ChannelError<ChanSigner>> {
+       pub fn channel_reestablish(&mut self, msg: &msgs::ChannelReestablish) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, Option<ChannelMonitorUpdate>, RAACommitmentOrder, Option<msgs::Shutdown>), ChannelError> {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
                        // While BOLT 2 doesn't indicate explicitly we should error this channel here, it
                        // almost certainly indicates we are going to end up out-of-sync in some way, so we
@@ -2453,8 +2586,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided"));
                                        }
                                        if msg.next_remote_commitment_number > INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
-                                               self.channel_monitor.provide_rescue_remote_commitment_tx_info(data_loss.my_current_per_commitment_point);
-                                               return Err(ChannelError::CloseDelayBroadcast { msg: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting", update: Some(self.channel_monitor.clone())});
+                                               self.latest_monitor_update_id += 1;
+                                               let monitor_update = ChannelMonitorUpdate {
+                                                       update_id: self.latest_monitor_update_id,
+                                                       updates: vec![ChannelMonitorUpdateStep::RescueRemoteCommitmentTXInfo {
+                                                               their_current_per_commitment_point: data_loss.my_current_per_commitment_point
+                                                       }]
+                                               };
+                                               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+                                               return Err(ChannelError::CloseDelayBroadcast {
+                                                       msg: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting",
+                                                       update: monitor_update
+                                               });
                                        }
                                },
                                OptionalField::Absent => {}
@@ -2538,7 +2681,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                match self.free_holding_cell_htlcs() {
                                        Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)),
                                        Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast { .. }) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
-                                       Ok(Some((commitment_update, channel_monitor))) => return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(channel_monitor), self.resend_order.clone(), shutdown_msg)),
+                                       Ok(Some((commitment_update, monitor_update))) => return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), shutdown_msg)),
                                        Ok(None) => return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), shutdown_msg)),
                                }
                        } else {
@@ -2562,7 +2705,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
        }
 
-       fn maybe_propose_first_closing_signed(&mut self, fee_estimator: &FeeEstimator) -> Option<msgs::ClosingSigned> {
+       fn maybe_propose_first_closing_signed<F: Deref>(&mut self, fee_estimator: &F) -> Option<msgs::ClosingSigned>
+               where F::Target: FeeEstimator
+       {
                if !self.channel_outbound || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
                                self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32) != BOTH_SIDES_SHUTDOWN_MASK ||
                                self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() {
@@ -2590,7 +2735,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                })
        }
 
-       pub fn shutdown(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Vec<(HTLCSource, PaymentHash)>), ChannelError<ChanSigner>> {
+       pub fn shutdown<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Vec<(HTLCSource, PaymentHash)>), ChannelError>
+               where F::Target: FeeEstimator
+       {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish"));
                }
@@ -2629,7 +2776,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // From here on out, we may not fail!
 
                self.channel_state |= ChannelState::RemoteShutdownSent as u32;
-               self.channel_update_count += 1;
+               self.update_time_counter += 1;
 
                // We can't send our shutdown until we've committed all of our pending HTLCs, but the
                // remote side is unlikely to accept any new HTLCs, so we go ahead and "free" any holding
@@ -2659,7 +2806,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                };
 
                self.channel_state |= ChannelState::LocalShutdownSent as u32;
-               self.channel_update_count += 1;
+               self.update_time_counter += 1;
 
                Ok((our_shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs))
        }
@@ -2686,7 +2833,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                tx.input[0].witness.push(self.get_funding_redeemscript().into_bytes());
        }
 
-       pub fn closing_signed(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::ClosingSigned) -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError<ChanSigner>> {
+       pub fn closing_signed<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::ClosingSigned) -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError>
+               where F::Target: FeeEstimator
+       {
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK {
                        return Err(ChannelError::Close("Remote end sent us a closing_signed before both sides provided a shutdown"));
                }
@@ -2724,7 +2873,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        if last_fee == msg.fee_satoshis {
                                self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
                                self.channel_state = ChannelState::ShutdownComplete as u32;
-                               self.channel_update_count += 1;
+                               self.update_time_counter += 1;
                                return Ok((None, Some(closing_tx)));
                        }
                }
@@ -2774,7 +2923,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
 
                self.channel_state = ChannelState::ShutdownComplete as u32;
-               self.channel_update_count += 1;
+               self.update_time_counter += 1;
 
                Ok((Some(msgs::ClosingSigned {
                        channel_id: self.channel_id,
@@ -2800,7 +2949,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if self.channel_state < ChannelState::FundingCreated as u32 {
                        panic!("Can't get a channel monitor until funding has been created");
                }
-               &mut self.channel_monitor
+               self.channel_monitor.as_mut().unwrap()
        }
 
        /// Guaranteed to be Some after both FundingLocked messages have been exchanged (and, thus,
@@ -2813,7 +2962,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Returns the funding_txo we either got from our peer, or were given by
        /// get_outbound_funding_created.
        pub fn get_funding_txo(&self) -> Option<OutPoint> {
-               self.channel_monitor.get_funding_txo()
+               self.funding_txo
        }
 
        /// Allowed in any state (including after shutdown)
@@ -2886,8 +3035,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        /// Allowed in any state (including after shutdown)
-       pub fn get_channel_update_count(&self) -> u32 {
-               self.channel_update_count
+       pub fn get_update_time_counter(&self) -> u32 {
+               self.update_time_counter
+       }
+
+       pub fn get_latest_monitor_update_id(&self) -> u64 {
+               self.latest_monitor_update_id
        }
 
        pub fn should_announce(&self) -> bool {
@@ -2900,7 +3053,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Gets the fee we'd want to charge for adding an HTLC output to this Channel
        /// Allowed in any state (including after shutdown)
-       pub fn get_our_fee_base_msat(&self, fee_estimator: &FeeEstimator) -> u32 {
+       pub fn get_our_fee_base_msat<F: Deref>(&self, fee_estimator: &F) -> u32
+               where F::Target: FeeEstimator
+       {
                // For lack of a better metric, we calculate what it would cost to consolidate the new HTLC
                // output value back into a transaction with the regular channel output:
 
@@ -2993,8 +3148,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
                        for (ref tx, index_in_block) in txn_matched.iter().zip(indexes_of_txn_matched) {
-                               if tx.txid() == self.channel_monitor.get_funding_txo().unwrap().txid {
-                                       let txo_idx = self.channel_monitor.get_funding_txo().unwrap().index as usize;
+                               if tx.txid() == self.funding_txo.unwrap().txid {
+                                       let txo_idx = self.funding_txo.unwrap().index as usize;
                                        if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
                                                        tx.output[txo_idx].value != self.channel_value_satoshis {
                                                if self.channel_outbound {
@@ -3007,7 +3162,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                        panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
                                                }
                                                self.channel_state = ChannelState::ShutdownComplete as u32;
-                                               self.channel_update_count += 1;
+                                               self.update_time_counter += 1;
                                                return Err(msgs::ErrorMessage {
                                                        channel_id: self.channel_id(),
                                                        data: "funding tx had wrong script/value".to_owned()
@@ -3033,7 +3188,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                if header.bitcoin_hash() != self.last_block_connected {
                        self.last_block_connected = header.bitcoin_hash();
-                       self.channel_monitor.last_block_hash = self.last_block_connected;
+                       self.update_time_counter = cmp::max(self.update_time_counter, header.time);
+                       if let Some(channel_monitor) = self.channel_monitor.as_mut() {
+                               channel_monitor.last_block_hash = self.last_block_connected;
+                       }
                        if self.funding_tx_confirmations > 0 {
                                if self.funding_tx_confirmations == self.minimum_depth as u64 {
                                        let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
@@ -3041,7 +3199,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                true
                                        } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
                                                self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
-                                               self.channel_update_count += 1;
+                                               self.update_time_counter += 1;
                                                true
                                        } else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
                                                // We got a reorg but not enough to trigger a force close, just update
@@ -3093,14 +3251,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        self.funding_tx_confirmations = self.minimum_depth as u64 - 1;
                }
                self.last_block_connected = header.bitcoin_hash();
-               self.channel_monitor.last_block_hash = self.last_block_connected;
+               if let Some(channel_monitor) = self.channel_monitor.as_mut() {
+                       channel_monitor.last_block_hash = self.last_block_connected;
+               }
                false
        }
 
        // Methods to get unprompted messages to send to the remote end (or where we already returned
        // something in the handler for the message that prompted this message):
 
-       pub fn get_open_channel(&self, chain_hash: Sha256dHash, fee_estimator: &FeeEstimator) -> msgs::OpenChannel {
+       pub fn get_open_channel<F: Deref>(&self, chain_hash: Sha256dHash, fee_estimator: &F) -> msgs::OpenChannel
+               where F::Target: FeeEstimator
+       {
                if !self.channel_outbound {
                        panic!("Tried to open a channel for an inbound channel?");
                }
@@ -3170,7 +3332,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
-       fn get_outbound_funding_created_signature(&mut self) -> Result<(Signature, Transaction), ChannelError<ChanSigner>> {
+       fn get_outbound_funding_created_signature(&mut self) -> Result<(Signature, Transaction), ChannelError> {
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
                Ok((self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx)
@@ -3184,27 +3346,25 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Note that channel_id changes during this call!
        /// Do NOT broadcast the funding transaction until after a successful funding_signed call!
        /// If an Err is returned, it is a ChannelError::Close.
-       pub fn get_outbound_funding_created(&mut self, funding_txo: OutPoint) -> Result<(msgs::FundingCreated, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       pub fn get_outbound_funding_created(&mut self, funding_txo: OutPoint) -> Result<(msgs::FundingCreated, ChannelMonitor<ChanSigner>), ChannelError> {
                if !self.channel_outbound {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
                if self.channel_state != (ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32) {
                        panic!("Tried to get a funding_created messsage at a time other than immediately after initial handshake completion (or tried to get funding_created twice)");
                }
-               if self.channel_monitor.get_min_seen_secret() != (1 << 48) ||
+               if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
                                self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
-               let funding_txo_script = self.get_funding_redeemscript().to_v0_p2wsh();
-               self.channel_monitor.set_funding_info((funding_txo, funding_txo_script));
-
+               self.funding_txo = Some(funding_txo.clone());
                let (our_signature, commitment_tx) = match self.get_outbound_funding_created_signature() {
                        Ok(res) => res,
                        Err(e) => {
                                log_error!(self, "Got bad signatures: {:?}!", e);
-                               self.channel_monitor.unset_funding_info();
+                               self.funding_txo = None;
                                return Err(e);
                        }
                };
@@ -3212,7 +3372,28 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let temporary_channel_id = self.channel_id;
 
                // Now that we're past error-generating stuff, update our local state:
-               self.channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
+
+               let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
+               let funding_redeemscript = self.get_funding_redeemscript();
+               let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
+               macro_rules! create_monitor {
+                       () => { {
+                               let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
+                                                                             &self.shutdown_pubkey, self.our_to_self_delay,
+                                                                             &self.destination_script, (funding_txo, funding_txo_script.clone()),
+                                                                             &their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
+                                                                             self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
+                                                                             self.get_commitment_transaction_number_obscure_factor(),
+                                                                             self.logger.clone());
+
+                               channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
+                               channel_monitor
+                       } }
+               }
+
+               self.channel_monitor = Some(create_monitor!());
+               let channel_monitor = create_monitor!();
+
                self.channel_state = ChannelState::FundingCreated as u32;
                self.channel_id = funding_txo.to_channel_id();
                self.cur_remote_commitment_transaction_number -= 1;
@@ -3222,7 +3403,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        funding_txid: funding_txo.txid,
                        funding_output_index: funding_txo.index,
                        signature: our_signature
-               }, self.channel_monitor.clone()))
+               }, channel_monitor))
        }
 
        /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our
@@ -3233,7 +3414,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// closing).
        /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
        /// https://github.com/lightningnetwork/lightning-rfc/issues/468
-       pub fn get_channel_announcement(&self, our_node_id: PublicKey, chain_hash: Sha256dHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError<ChanSigner>> {
+       pub fn get_channel_announcement(&self, our_node_id: PublicKey, chain_hash: Sha256dHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
                if !self.config.announced_channel {
                        return Err(ChannelError::Ignore("Channel is not available for public announcements"));
                }
@@ -3270,17 +3451,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                assert_eq!(self.channel_state & ChannelState::PeerDisconnected as u32, ChannelState::PeerDisconnected as u32);
                assert_ne!(self.cur_remote_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER);
                let data_loss_protect = if self.cur_remote_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER {
-                       let remote_last_secret = self.channel_monitor.get_secret(self.cur_remote_commitment_transaction_number + 2).unwrap();
+                       let remote_last_secret = self.commitment_secrets.get_secret(self.cur_remote_commitment_transaction_number + 2).unwrap();
                        log_trace!(self, "Enough info to generate a Data Loss Protect with per_commitment_secret {}", log_bytes!(remote_last_secret));
                        OptionalField::Present(DataLossProtect {
                                your_last_per_commitment_secret: remote_last_secret,
                                my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number + 1))
                        })
                } else {
-                       log_debug!(self, "We don't seen yet any revoked secret, if this channnel has already been updated it means we are fallen-behind, you should wait for other peer closing");
+                       log_info!(self, "Sending a data_loss_protect with no previous remote per_commitment_secret");
                        OptionalField::Present(DataLossProtect {
                                your_last_per_commitment_secret: [0;32],
-                               my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number))
+                               my_current_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number + 1))
                        })
                };
                msgs::ChannelReestablish {
@@ -3317,7 +3498,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// HTLCs on the wire or we wouldn't be able to determine what they actually ACK'ed.
        /// You MUST call send_commitment prior to any other calls on this Channel
        /// If an Err is returned, it's a ChannelError::Ignore!
-       pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError<ChanSigner>> {
+       pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) {
                        return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down"));
                }
@@ -3325,6 +3506,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if amount_msat > self.channel_value_satoshis * 1000 {
                        return Err(ChannelError::Ignore("Cannot send more than the total value of the channel"));
                }
+
+               if amount_msat == 0 {
+                       return Err(ChannelError::Ignore("Cannot send 0-msat HTLC"));
+               }
+
                if amount_msat < self.their_htlc_minimum_msat {
                        return Err(ChannelError::Ignore("Cannot send less than their minimum HTLC value"));
                }
@@ -3394,7 +3580,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Always returns a ChannelError::Close if an immediately-preceding (read: the
        /// last call to this Channel) send_htlc returned Ok(Some(_)) and there is an Err.
        /// May panic if called except immediately after a successful, Ok(Some(_))-returning send_htlc.
-       pub fn send_commitment(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       pub fn send_commitment(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Cannot create commitment tx until channel is fully established");
                }
@@ -3426,7 +3612,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.send_commitment_no_status_check()
        }
        /// Only fails in case of bad keys
-       fn send_commitment_no_status_check(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitor<ChanSigner>), ChannelError<ChanSigner>> {
+       fn send_commitment_no_status_check(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> {
                // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we
                // fail to generate this, we still are at least at a position where upgrading their status
                // is acceptable.
@@ -3450,20 +3636,31 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let (res, remote_commitment_tx, htlcs) = match self.send_commitment_no_state_update() {
                        Ok((res, (remote_commitment_tx, mut htlcs))) => {
                                // Update state now that we've passed all the can-fail calls...
-                               let htlcs_no_ref = htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
+                               let htlcs_no_ref: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
+                                       htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
                                (res, remote_commitment_tx, htlcs_no_ref)
                        },
                        Err(e) => return Err(e),
                };
 
-               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_commitment_tx, htlcs, self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
+               self.latest_monitor_update_id += 1;
+               let monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::LatestRemoteCommitmentTXInfo {
+                               unsigned_commitment_tx: remote_commitment_tx.clone(),
+                               htlc_outputs: htlcs.clone(),
+                               commitment_number: self.cur_remote_commitment_transaction_number,
+                               their_revocation_point: self.their_cur_commitment_point.unwrap()
+                       }]
+               };
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
                self.channel_state |= ChannelState::AwaitingRemoteRevoke as u32;
-               Ok((res, self.channel_monitor.clone()))
+               Ok((res, monitor_update))
        }
 
        /// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation
        /// when we shouldn't change HTLC/channel state.
-       fn send_commitment_no_state_update(&self) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError<ChanSigner>> {
+       fn send_commitment_no_state_update(&self) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> {
                let mut feerate_per_kw = self.feerate_per_kw;
                if let Some(feerate) = self.pending_update_fee {
                        if self.channel_outbound {
@@ -3511,7 +3708,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// to send to the remote peer in one go.
        /// Shorthand for calling send_htlc() followed by send_commitment(), see docs on those for
        /// more info.
-       pub fn send_htlc_and_commit(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitor<ChanSigner>)>, ChannelError<ChanSigner>> {
+       pub fn send_htlc_and_commit(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> {
                match self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet)? {
                        Some(update_add_htlc) => {
                                let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
@@ -3550,7 +3747,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                } else {
                        self.channel_state |= ChannelState::LocalShutdownSent as u32;
                }
-               self.channel_update_count += 1;
+               self.update_time_counter += 1;
 
                // Go ahead and drop holding cell updates as we'd rather fail payments than wait to send
                // our shutdown until we've committed all of the pending changes.
@@ -3577,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
        /// Also returns the list of payment_hashes for channels which we can safely fail backwards
        /// immediately (others we will have to allow to time out).
-       pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
+       pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
                assert!(self.channel_state != ChannelState::ShutdownComplete as u32);
 
                // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
@@ -3599,8 +3796,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                self.channel_state = ChannelState::ShutdownComplete as u32;
-               self.channel_update_count += 1;
-               (self.channel_monitor.get_latest_local_commitment_txn(), dropped_outbound_htlcs)
+               self.update_time_counter += 1;
+               self.latest_monitor_update_id += 1;
+               (self.funding_txo.clone(), ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
+               }, dropped_outbound_htlcs)
        }
 }
 
@@ -3628,9 +3829,9 @@ impl Writeable for InboundHTLCRemovalReason {
        }
 }
 
-impl<R: ::std::io::Read> Readable<R> for InboundHTLCRemovalReason {
-       fn read(reader: &mut R) -> Result<Self, DecodeError> {
-               Ok(match <u8 as Readable<R>>::read(reader)? {
+impl Readable for InboundHTLCRemovalReason {
+       fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               Ok(match <u8 as Readable>::read(reader)? {
                        0 => InboundHTLCRemovalReason::FailRelay(Readable::read(reader)?),
                        1 => InboundHTLCRemovalReason::FailMalformed((Readable::read(reader)?, Readable::read(reader)?)),
                        2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
@@ -3655,8 +3856,11 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                self.channel_outbound.write(writer)?;
                self.channel_value_satoshis.write(writer)?;
 
+               self.latest_monitor_update_id.write(writer)?;
+
                self.local_keys.write(writer)?;
                self.shutdown_pubkey.write(writer)?;
+               self.destination_script.write(writer)?;
 
                self.cur_local_commitment_transaction_number.write(writer)?;
                self.cur_remote_commitment_transaction_number.write(writer)?;
@@ -3697,18 +3901,6 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                        }
                }
 
-               macro_rules! write_option {
-                       ($thing: expr) => {
-                               match &$thing {
-                                       &None => 0u8.write(writer)?,
-                                       &Some(ref v) => {
-                                               1u8.write(writer)?;
-                                               v.write(writer)?;
-                                       },
-                               }
-                       }
-               }
-
                (self.pending_outbound_htlcs.len() as u64).write(writer)?;
                for htlc in self.pending_outbound_htlcs.iter() {
                        htlc.htlc_id.write(writer)?;
@@ -3726,15 +3918,15 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                                },
                                &OutboundHTLCState::RemoteRemoved(ref fail_reason) => {
                                        2u8.write(writer)?;
-                                       write_option!(*fail_reason);
+                                       fail_reason.write(writer)?;
                                },
                                &OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref fail_reason) => {
                                        3u8.write(writer)?;
-                                       write_option!(*fail_reason);
+                                       fail_reason.write(writer)?;
                                },
                                &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) => {
                                        4u8.write(writer)?;
-                                       write_option!(*fail_reason);
+                                       fail_reason.write(writer)?;
                                },
                        }
                }
@@ -3785,12 +3977,12 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                        fail_reason.write(writer)?;
                }
 
-               write_option!(self.pending_update_fee);
-               write_option!(self.holding_cell_update_fee);
+               self.pending_update_fee.write(writer)?;
+               self.holding_cell_update_fee.write(writer)?;
 
                self.next_local_htlc_id.write(writer)?;
                (self.next_remote_htlc_id - dropped_inbound_htlcs).write(writer)?;
-               self.channel_update_count.write(writer)?;
+               self.update_time_counter.write(writer)?;
                self.feerate_per_kw.write(writer)?;
 
                match self.last_sent_closing_fee {
@@ -3803,8 +3995,9 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                        None => 0u8.write(writer)?,
                }
 
-               write_option!(self.funding_tx_confirmed_in);
-               write_option!(self.short_channel_id);
+               self.funding_txo.write(writer)?;
+               self.funding_tx_confirmed_in.write(writer)?;
+               self.short_channel_id.write(writer)?;
 
                self.last_block_connected.write(writer)?;
                self.funding_tx_confirmations.write(writer)?;
@@ -3820,21 +4013,23 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                self.their_max_accepted_htlcs.write(writer)?;
                self.minimum_depth.write(writer)?;
 
-               write_option!(self.their_pubkeys);
-               write_option!(self.their_cur_commitment_point);
+               self.their_pubkeys.write(writer)?;
+               self.their_cur_commitment_point.write(writer)?;
 
-               write_option!(self.their_prev_commitment_point);
+               self.their_prev_commitment_point.write(writer)?;
                self.their_node_id.write(writer)?;
 
-               write_option!(self.their_shutdown_scriptpubkey);
+               self.their_shutdown_scriptpubkey.write(writer)?;
+
+               self.commitment_secrets.write(writer)?;
 
-               self.channel_monitor.write_for_disk(writer)?;
+               self.channel_monitor.as_ref().unwrap().write_for_disk(writer)?;
                Ok(())
        }
 }
 
-impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R, Arc<Logger>> for Channel<ChanSigner> {
-       fn read(reader: &mut R, logger: Arc<Logger>) -> Result<Self, DecodeError> {
+impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<ChanSigner> {
+       fn read<R : ::std::io::Read>(reader: &mut R, logger: Arc<Logger>) -> Result<Self, DecodeError> {
                let _ver: u8 = Readable::read(reader)?;
                let min_ver: u8 = Readable::read(reader)?;
                if min_ver > SERIALIZATION_VERSION {
@@ -3849,8 +4044,11 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                let channel_outbound = Readable::read(reader)?;
                let channel_value_satoshis = Readable::read(reader)?;
 
+               let latest_monitor_update_id = Readable::read(reader)?;
+
                let local_keys = Readable::read(reader)?;
                let shutdown_pubkey = Readable::read(reader)?;
+               let destination_script = Readable::read(reader)?;
 
                let cur_local_commitment_transaction_number = Readable::read(reader)?;
                let cur_remote_commitment_transaction_number = Readable::read(reader)?;
@@ -3864,7 +4062,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                                amount_msat: Readable::read(reader)?,
                                cltv_expiry: Readable::read(reader)?,
                                payment_hash: Readable::read(reader)?,
-                               state: match <u8 as Readable<R>>::read(reader)? {
+                               state: match <u8 as Readable>::read(reader)? {
                                        1 => InboundHTLCState::AwaitingRemoteRevokeToAnnounce(Readable::read(reader)?),
                                        2 => InboundHTLCState::AwaitingAnnouncedRemoteRevoke(Readable::read(reader)?),
                                        3 => InboundHTLCState::Committed,
@@ -3883,7 +4081,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                                cltv_expiry: Readable::read(reader)?,
                                payment_hash: Readable::read(reader)?,
                                source: Readable::read(reader)?,
-                               state: match <u8 as Readable<R>>::read(reader)? {
+                               state: match <u8 as Readable>::read(reader)? {
                                        0 => OutboundHTLCState::LocalAnnounced(Box::new(Readable::read(reader)?)),
                                        1 => OutboundHTLCState::Committed,
                                        2 => OutboundHTLCState::RemoteRemoved(Readable::read(reader)?),
@@ -3897,7 +4095,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                let holding_cell_htlc_update_count: u64 = Readable::read(reader)?;
                let mut holding_cell_htlc_updates = Vec::with_capacity(cmp::min(holding_cell_htlc_update_count as usize, OUR_MAX_HTLCS as usize*2));
                for _ in 0..holding_cell_htlc_update_count {
-                       holding_cell_htlc_updates.push(match <u8 as Readable<R>>::read(reader)? {
+                       holding_cell_htlc_updates.push(match <u8 as Readable>::read(reader)? {
                                0 => HTLCUpdateAwaitingACK::AddHTLC {
                                        amount_msat: Readable::read(reader)?,
                                        cltv_expiry: Readable::read(reader)?,
@@ -3917,7 +4115,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                        });
                }
 
-               let resend_order = match <u8 as Readable<R>>::read(reader)? {
+               let resend_order = match <u8 as Readable>::read(reader)? {
                        0 => RAACommitmentOrder::CommitmentFirst,
                        1 => RAACommitmentOrder::RevokeAndACKFirst,
                        _ => return Err(DecodeError::InvalidValue),
@@ -3944,15 +4142,16 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
 
                let next_local_htlc_id = Readable::read(reader)?;
                let next_remote_htlc_id = Readable::read(reader)?;
-               let channel_update_count = Readable::read(reader)?;
+               let update_time_counter = Readable::read(reader)?;
                let feerate_per_kw = Readable::read(reader)?;
 
-               let last_sent_closing_fee = match <u8 as Readable<R>>::read(reader)? {
+               let last_sent_closing_fee = match <u8 as Readable>::read(reader)? {
                        0 => None,
                        1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)),
                        _ => return Err(DecodeError::InvalidValue),
                };
 
+               let funding_txo = Readable::read(reader)?;
                let funding_tx_confirmed_in = Readable::read(reader)?;
                let short_channel_id = Readable::read(reader)?;
 
@@ -3977,6 +4176,8 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                let their_node_id = Readable::read(reader)?;
 
                let their_shutdown_scriptpubkey = Readable::read(reader)?;
+               let commitment_secrets = Readable::read(reader)?;
+
                let (monitor_last_block, channel_monitor) = ReadableArgs::read(reader, logger.clone())?;
                // We drop the ChannelMonitor's last block connected hash cause we don't actually bother
                // doing full block connection operations on the internal ChannelMonitor copies
@@ -3994,8 +4195,11 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                        secp_ctx: Secp256k1::new(),
                        channel_value_satoshis,
 
+                       latest_monitor_update_id,
+
                        local_keys,
                        shutdown_pubkey,
+                       destination_script,
 
                        cur_local_commitment_transaction_number,
                        cur_remote_commitment_transaction_number,
@@ -4017,7 +4221,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
                        holding_cell_update_fee,
                        next_local_htlc_id,
                        next_remote_htlc_id,
-                       channel_update_count,
+                       update_time_counter,
                        feerate_per_kw,
 
                        #[cfg(debug_assertions)]
@@ -4027,6 +4231,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
 
                        last_sent_closing_fee,
 
+                       funding_txo,
                        funding_tx_confirmed_in,
                        short_channel_id,
                        last_block_connected,
@@ -4051,7 +4256,8 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
 
                        their_shutdown_scriptpubkey,
 
-                       channel_monitor,
+                       channel_monitor: Some(channel_monitor),
+                       commitment_secrets,
 
                        network_sync: UpdateStatus::Fresh,
 
@@ -4062,22 +4268,28 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
 
 #[cfg(test)]
 mod tests {
+       use bitcoin::BitcoinHash;
        use bitcoin::util::bip143;
        use bitcoin::consensus::encode::serialize;
        use bitcoin::blockdata::script::{Script, Builder};
-       use bitcoin::blockdata::transaction::Transaction;
+       use bitcoin::blockdata::transaction::{Transaction, TxOut};
+       use bitcoin::blockdata::constants::genesis_block;
        use bitcoin::blockdata::opcodes;
+       use bitcoin::network::constants::Network;
        use bitcoin_hashes::hex::FromHex;
        use hex;
        use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
        use ln::channel::{Channel,ChannelKeys,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,TxCreationKeys};
        use ln::channel::MAX_FUNDING_SATOSHIS;
+       use ln::features::InitFeatures;
+       use ln::msgs::{OptionalField, DataLossProtect};
        use ln::chan_utils;
        use ln::chan_utils::{LocalCommitmentTransaction, ChannelPublicKeys};
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
        use chain::keysinterface::{InMemoryChannelKeys, KeysInterface};
        use chain::transaction::OutPoint;
        use util::config::UserConfig;
+       use util::enforcing_trait_impls::EnforcingChannelKeys;
        use util::test_utils;
        use util::logger::Logger;
        use secp256k1::{Secp256k1, Message, Signature, All};
@@ -4087,6 +4299,7 @@ mod tests {
        use bitcoin_hashes::hash160::Hash as Hash160;
        use bitcoin_hashes::Hash;
        use std::sync::Arc;
+       use rand::{thread_rng,Rng};
 
        struct TestFeeEstimator {
                fee_est: u64
@@ -4134,6 +4347,70 @@ mod tests {
                PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap())
        }
 
+       #[test]
+       fn channel_reestablish_no_updates() {
+               let feeest = TestFeeEstimator{fee_est: 15000};
+               let logger : Arc<Logger> = Arc::new(test_utils::TestLogger::new());
+               let secp_ctx = Secp256k1::new();
+               let mut seed = [0; 32];
+               let mut rng = thread_rng();
+               rng.fill_bytes(&mut seed);
+               let network = Network::Testnet;
+               let keys_provider = test_utils::TestKeysInterface::new(&seed, network, logger.clone() as Arc<Logger>);
+
+               // Go through the flow of opening a channel between two nodes.
+
+               // Create Node A's channel
+               let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
+               let config = UserConfig::default();
+               let mut node_a_chan = Channel::<EnforcingChannelKeys>::new_outbound(&&feeest, &&keys_provider, node_a_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap();
+
+               // Create Node B's channel by receiving Node A's open_channel message
+               let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash(), &&feeest);
+               let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
+               let mut node_b_chan = Channel::<EnforcingChannelKeys>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::supported(), &open_channel_msg, 7, logger, &config).unwrap();
+
+               // Node B --> Node A: accept channel
+               let accept_channel_msg = node_b_chan.get_accept_channel();
+               node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::supported()).unwrap();
+
+               // Node A --> Node B: funding created
+               let output_script = node_a_chan.get_funding_redeemscript();
+               let tx = Transaction { version: 1, lock_time: 0, input: Vec::new(), output: vec![TxOut {
+                       value: 10000000, script_pubkey: output_script.clone(),
+               }]};
+               let funding_outpoint = OutPoint::new(tx.txid(), 0);
+               let (funding_created_msg, _) = node_a_chan.get_outbound_funding_created(funding_outpoint).unwrap();
+               let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg).unwrap();
+
+               // Node B --> Node A: funding signed
+               let _ = node_a_chan.funding_signed(&funding_signed_msg);
+
+               // Now disconnect the two nodes and check that the commitment point in
+               // Node B's channel_reestablish message is sane.
+               node_b_chan.remove_uncommitted_htlcs_and_mark_paused();
+               let expected_commitment_point = PublicKey::from_secret_key(&secp_ctx, &node_b_chan.build_local_commitment_secret(node_b_chan.cur_local_commitment_transaction_number + 1));
+               let msg = node_b_chan.get_channel_reestablish();
+               match msg.data_loss_protect {
+                       OptionalField::Present(DataLossProtect { my_current_per_commitment_point, .. }) => {
+                               assert_eq!(expected_commitment_point, my_current_per_commitment_point);
+                       },
+                       _ => panic!()
+               }
+
+               // Check that the commitment point in Node A's channel_reestablish message
+               // is sane.
+               node_a_chan.remove_uncommitted_htlcs_and_mark_paused();
+               let expected_commitment_point = PublicKey::from_secret_key(&secp_ctx, &node_a_chan.build_local_commitment_secret(node_a_chan.cur_local_commitment_transaction_number + 1));
+               let msg = node_a_chan.get_channel_reestablish();
+               match msg.data_loss_protect {
+                       OptionalField::Present(DataLossProtect { my_current_per_commitment_point, .. }) => {
+                               assert_eq!(expected_commitment_point, my_current_per_commitment_point);
+                       },
+                       _ => panic!()
+               }
+       }
+
        #[test]
        fn outbound_commitment_test() {
                // Test vectors from BOLT 3 Appendix C:
@@ -4141,7 +4418,7 @@ mod tests {
                let logger : Arc<Logger> = Arc::new(test_utils::TestLogger::new());
                let secp_ctx = Secp256k1::new();
 
-               let chan_keys = InMemoryChannelKeys::new(
+               let mut chan_keys = InMemoryChannelKeys::new(
                        &secp_ctx,
                        SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
@@ -4151,22 +4428,22 @@ mod tests {
 
                        // These aren't set in the test vectors:
                        [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff],
-                       7000000000,
+                       10_000_000,
                );
 
                assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
                                hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
-               let keys_provider: Arc<KeysInterface<ChanKeySigner = InMemoryChannelKeys>> = Arc::new(Keys { chan_keys });
+               let keys_provider = Keys { chan_keys: chan_keys.clone() };
 
                let their_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let mut config = UserConfig::default();
                config.channel_options.announced_channel = false;
-               let mut chan = Channel::<InMemoryChannelKeys>::new_outbound(&feeest, &keys_provider, their_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test
+               let mut chan = Channel::<InMemoryChannelKeys>::new_outbound(&&feeest, &&keys_provider, their_node_id, 10_000_000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test
                chan.their_to_self_delay = 144;
                chan.our_dust_limit_satoshis = 546;
 
                let funding_info = OutPoint::new(Sha256dHash::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), 0);
-               chan.channel_monitor.set_funding_info((funding_info, Script::new()));
+               chan.funding_txo = Some(funding_info);
 
                let their_pubkeys = ChannelPublicKeys {
                        funding_pubkey: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
@@ -4175,6 +4452,7 @@ mod tests {
                        delayed_payment_basepoint: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
                        htlc_basepoint: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444")
                };
+               chan_keys.set_remote_channel_pubkeys(&their_pubkeys);
 
                assert_eq!(their_pubkeys.payment_basepoint.serialize()[..],
                           hex::decode("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]);
@@ -4198,6 +4476,7 @@ mod tests {
 
                let mut unsigned_tx: (Transaction, Vec<HTLCOutputInCommitment>);
 
+               let mut localtx;
                macro_rules! test_commitment {
                        ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
                                unsigned_tx = {
@@ -4212,8 +4491,8 @@ mod tests {
                                let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &redeemscript, chan.channel_value_satoshis)[..]).unwrap();
                                secp_ctx.verify(&sighash, &their_signature, chan.their_funding_pubkey()).unwrap();
 
-                               let mut localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey());
-                               localtx.add_local_sig(chan.local_keys.funding_key(), &redeemscript, chan.channel_value_satoshis, &chan.secp_ctx);
+                               localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey());
+                               chan_keys.sign_local_commitment(&mut localtx, &chan.secp_ctx);
 
                                assert_eq!(serialize(localtx.with_valid_witness())[..],
                                                hex::decode($tx_hex).unwrap()[..]);
@@ -4221,11 +4500,11 @@ mod tests {
                }
 
                macro_rules! test_htlc_output {
-                       ( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr ) => {
+                       ( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
                                let remote_signature = Signature::from_der(&hex::decode($their_sig_hex).unwrap()[..]).unwrap();
 
                                let ref htlc = unsigned_tx.1[$htlc_idx];
-                               let mut htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw);
+                               let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
                                let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
                                secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.b_htlc_key).unwrap();
@@ -4242,8 +4521,12 @@ mod tests {
                                        assert!(preimage.is_some());
                                }
 
-                               chan_utils::sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, &keys.per_commitment_point, chan.local_keys.htlc_base_key(), &chan.secp_ctx).unwrap();
-                               assert_eq!(serialize(&htlc_tx)[..],
+                               let mut per_htlc = Vec::new();
+                               per_htlc.push((htlc.clone(), Some(remote_signature), None));
+                               localtx.set_htlc_cache(keys.clone(), chan.feerate_per_kw, per_htlc);
+                               chan_keys.sign_htlc_transaction(&mut localtx, $htlc_idx, preimage, chan.their_to_self_delay, &chan.secp_ctx);
+
+                               assert_eq!(serialize(localtx.htlc_with_valid_witness($htlc_idx).as_ref().unwrap())[..],
                                                hex::decode($tx_hex).unwrap()[..]);
                        };
                }