Add TODOs
[rust-lightning] / lightning / src / ln / channel.rs
index 53946720e69f48a3c54f39d59923f259f97e6ecf..306d4c5954b9cb4a3d89aa7bb373f044e85020d6 100644 (file)
@@ -25,17 +25,19 @@ use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::transaction::OutPoint;
 use chain::keysinterface::{ChannelKeys, KeysInterface};
 use util::transaction_utils;
-use util::ser::{Readable, ReadableArgs, Writeable, Writer};
-use util::logger::{Logger, LogHolder};
+use util::ser::{Readable, Writeable, Writer};
+use util::logger::Logger;
 use util::errors::APIError;
 use util::config::{UserConfig,ChannelConfig};
 
 use std;
 use std::default::Default;
 use std::{cmp,mem,fmt};
-use std::sync::{Arc};
 use std::ops::Deref;
 
+#[cfg(all(test, feature = "mutation_testing"))]
+use mutagen::mutate;
+
 #[cfg(test)]
 pub struct ChannelValueStat {
        pub value_to_self_msat: u64,
@@ -329,9 +331,9 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        #[cfg(not(test))]
        their_max_htlc_value_in_flight_msat: u64,
        //get_our_max_htlc_value_in_flight_msat(): u64,
-       /// minimum channel reserve for **self** to maintain - set by them.
-       their_channel_reserve_satoshis: u64,
-       //get_our_channel_reserve_satoshis(): u64,
+       /// minimum channel reserve for self to maintain - set by them.
+       local_channel_reserve_satoshis: u64,
+       // get_remote_channel_reserve_satoshis(channel_value_sats: u64): u64
        their_htlc_minimum_msat: u64,
        our_htlc_minimum_msat: u64,
        their_to_self_delay: u16,
@@ -358,15 +360,13 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        commitment_secrets: CounterpartyCommitmentSecrets,
 
        network_sync: UpdateStatus,
-
-       logger: Arc<Logger>,
 }
 
 pub const OUR_MAX_HTLCS: u16 = 50; //TODO
 /// Confirmation count threshold at which we close a channel. Ideally we'd keep the channel around
 /// 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;
+const UNCONF_THRESHOLD: u32 = 1;
 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?)
 
@@ -417,10 +417,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                channel_value_satoshis * 1000 / 10 //TODO
        }
 
-       /// Returns a minimum channel reserve value **they** need to maintain
+       /// Returns a minimum channel reserve value the remote needs to maintain,
+       /// required by us.
        ///
        /// Guaranteed to return a value no larger than channel_value_satoshis
-       pub(crate) fn get_our_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
+       pub(crate) fn get_remote_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
                let (q, _) = channel_value_satoshis.overflowing_div(100);
                cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
        }
@@ -430,7 +431,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        // Constructors:
-       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>
+       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, config: &UserConfig) -> Result<Channel<ChanSigner>, APIError>
        where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
              F::Target: FeeEstimator,
        {
@@ -449,7 +450,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               if Channel::<ChanSigner>::get_our_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
+               if Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
                        return Err(APIError::FeeRateTooHigh{err: format!("Not enough reserve above dust limit can be found at current fee rate({})", background_feerate), feerate: background_feerate});
                }
 
@@ -509,7 +510,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_dust_limit_satoshis: 0,
                        our_dust_limit_satoshis: Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate),
                        their_max_htlc_value_in_flight_msat: 0,
-                       their_channel_reserve_satoshis: 0,
+                       local_channel_reserve_satoshis: 0,
                        their_htlc_minimum_msat: 0,
                        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,
@@ -529,8 +530,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
                        network_sync: UpdateStatus::Fresh,
-
-                       logger,
                })
        }
 
@@ -540,7 +539,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                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"));
                }
-               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 {
+               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 200 {
                        return Err(ChannelError::Close("Peer's feerate much too high"));
                }
                Ok(())
@@ -548,7 +547,7 @@ 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<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>
+       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, config: &UserConfig) -> Result<Channel<ChanSigner>, ChannelError>
                where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
           F::Target: FeeEstimator
        {
@@ -635,15 +634,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
 
                let our_dust_limit_satoshis = Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate);
-               let our_channel_reserve_satoshis = Channel::<ChanSigner>::get_our_channel_reserve_satoshis(msg.funding_satoshis);
-               if our_channel_reserve_satoshis < our_dust_limit_satoshis {
+               let remote_channel_reserve_satoshis = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(msg.funding_satoshis);
+               if remote_channel_reserve_satoshis < our_dust_limit_satoshis {
                        return Err(ChannelError::Close("Suitable channel reserve not found. aborting"));
                }
                if msg.channel_reserve_satoshis < our_dust_limit_satoshis {
                        return Err(ChannelError::Close("channel_reserve_satoshis too small"));
                }
-               if our_channel_reserve_satoshis < msg.dust_limit_satoshis {
-                       return Err(ChannelError::Close("Dust limit too high for our channel reserve"));
+               if remote_channel_reserve_satoshis < msg.dust_limit_satoshis {
+                       return Err(ChannelError::Close("Dust limit too high for the channel reserve we require the remote to keep"));
                }
 
                // check if the funder's amount for the initial commitment tx is sufficient
@@ -655,7 +654,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let to_local_msat = msg.push_msat;
                let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT;
-               if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= our_channel_reserve_satoshis * 1000 {
+               if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= remote_channel_reserve_satoshis * 1000 {
                        return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
                }
 
@@ -734,7 +733,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_dust_limit_satoshis: msg.dust_limit_satoshis,
                        our_dust_limit_satoshis: our_dust_limit_satoshis,
                        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,
+                       local_channel_reserve_satoshis: msg.channel_reserve_satoshis,
                        their_htlc_minimum_msat: msg.htlc_minimum_msat,
                        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,
@@ -754,8 +753,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        commitment_secrets: CounterpartyCommitmentSecrets::new(),
 
                        network_sync: UpdateStatus::Fresh,
-
-                       logger,
                };
 
                Ok(chan)
@@ -810,7 +807,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Note that below-dust HTLCs are included in the third return value, but not the second, and
        /// sources are provided only for outbound HTLCs in the third return value.
        #[inline]
-       fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) {
+       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64, logger: &L) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger {
                let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);
 
                let txins = {
@@ -832,7 +829,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let mut local_htlc_total_msat = 0;
                let mut value_to_self_msat_offset = 0;
 
-               log_trace!(self, "Building commitment transaction number {} (really {} xor {}) for {}, generated by {} with fee {}...", commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number), self.get_commitment_transaction_number_obscure_factor(), if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
+               log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for {}, generated by {} with fee {}...", commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number), self.get_commitment_transaction_number_obscure_factor(), if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
 
                macro_rules! get_htlc_in_commitment {
                        ($htlc: expr, $offered: expr) => {
@@ -851,25 +848,25 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                if $outbound == local { // "offered HTLC output"
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
-                                               log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                txouts.push((TxOut {
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
-                                               log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                } else {
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
-                                               log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                txouts.push((TxOut { // "received HTLC output"
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
-                                               log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                }
@@ -889,7 +886,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                add_htlc_output!(htlc, false, None, state_name);
                                remote_htlc_total_msat += htlc.amount_msat;
                        } else {
-                               log_trace!(self, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
+                               log_trace!(logger, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
                                match &htlc.state {
                                        &InboundHTLCState::LocalRemoved(ref reason) => {
                                                if generated_by_local {
@@ -916,7 +913,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
                                local_htlc_total_msat += htlc.amount_msat;
                        } else {
-                               log_trace!(self, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
+                               log_trace!(logger, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
                                match htlc.state {
                                        OutboundHTLCState::AwaitingRemoteRevokeToRemove(None)|OutboundHTLCState::AwaitingRemovedRemoteRevoke(None) => {
                                                value_to_self_msat_offset -= htlc.amount_msat as i64;
@@ -949,9 +946,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        } else {
                                self.max_commitment_tx_output_remote.lock().unwrap()
                        };
-                       debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.their_channel_reserve_satoshis as i64);
+                       debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.local_channel_reserve_satoshis as i64);
                        max_commitment_tx_output.0 = cmp::max(max_commitment_tx_output.0, value_to_self_msat as u64);
-                       debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
+                       debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
                        max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64);
                }
 
@@ -966,7 +963,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let value_to_b = if local { value_to_remote } else { value_to_self };
 
                if value_to_a >= (dust_limit_satoshis as i64) {
-                       log_trace!(self, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
+                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
                        txouts.push((TxOut {
                                script_pubkey: chan_utils::get_revokeable_redeemscript(&keys.revocation_key,
                                                                                       if local { self.their_to_self_delay } else { self.our_to_self_delay },
@@ -976,7 +973,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                if value_to_b >= (dust_limit_satoshis as i64) {
-                       log_trace!(self, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
+                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
                        let static_payment_pk = if local {
                                self.their_pubkeys.as_ref().unwrap().payment_point
                        } else {
@@ -1143,7 +1140,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        ///
        /// 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> {
+       fn get_update_fulfill_htlc<L: Deref>(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> where L::Target: Logger {
                // 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,
@@ -1168,7 +1165,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        InboundHTLCState::LocalRemoved(ref reason) => {
                                                if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
                                                } 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()));
+                                                       log_warn!(logger, "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));
@@ -1197,7 +1194,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                payment_preimage: payment_preimage_arg.clone(),
                        }],
                };
-               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone(), logger).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() {
@@ -1212,7 +1209,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
-                                                       log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
+                                                       log_warn!(logger, "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.
                                                        debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
@@ -1222,7 +1219,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        _ => {}
                                }
                        }
-                       log_trace!(self, "Adding HTLC claim to holding_cell! Current state: {}", self.channel_state);
+                       log_trace!(logger, "Adding HTLC claim to holding_cell! Current state: {}", self.channel_state);
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
@@ -1236,7 +1233,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
                                return Ok((None, Some(monitor_update)));
                        }
-                       log_trace!(self, "Upgrading HTLC {} to LocalRemoved with a Fulfill!", log_bytes!(htlc.payment_hash.0));
+                       log_trace!(logger, "Upgrading HTLC {} to LocalRemoved with a Fulfill!", log_bytes!(htlc.payment_hash.0));
                        htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone()));
                }
 
@@ -1247,10 +1244,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }), 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<ChannelMonitorUpdate>), ChannelError> {
-               match self.get_update_fulfill_htlc(htlc_id, payment_preimage)? {
+       pub fn get_update_fulfill_htlc_and_commit<L: Deref>(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitorUpdate>), ChannelError> where L::Target: Logger {
+               match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger)? {
                        (Some(update_fulfill_htlc), Some(mut monitor_update)) => {
-                               let (commitment, mut additional_update) = self.send_commitment_no_status_check()?;
+                               let (commitment, mut additional_update) = self.send_commitment_no_status_check(logger)?;
                                // 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;
@@ -1258,7 +1255,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
                        },
                        (Some(update_fulfill_htlc), None) => {
-                               let (commitment, monitor_update) = self.send_commitment_no_status_check()?;
+                               let (commitment, monitor_update) = self.send_commitment_no_status_check(logger)?;
                                Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
                        },
                        (None, Some(monitor_update)) => Ok((None, Some(monitor_update))),
@@ -1363,7 +1360,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.channel_reserve_satoshis < self.our_dust_limit_satoshis {
                        return Err(ChannelError::Close("Peer never wants payout outputs?"));
                }
-               if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) {
+               if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) {
                        return Err(ChannelError::Close("Dust limit is bigger than our channel reverse"));
                }
                if msg.htlc_minimum_msat >= (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000 {
@@ -1425,7 +1422,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
                self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
-               self.their_channel_reserve_satoshis = msg.channel_reserve_satoshis;
+               self.local_channel_reserve_satoshis = msg.channel_reserve_satoshis;
                self.their_htlc_minimum_msat = msg.htlc_minimum_msat;
                self.their_to_self_delay = msg.to_self_delay;
                self.their_max_accepted_htlcs = msg.max_accepted_htlcs;
@@ -1450,21 +1447,21 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       fn funding_created_signature(&mut self, sig: &Signature) -> Result<(Transaction, LocalCommitmentTransaction, Signature), ChannelError> {
+       fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(Transaction, LocalCommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
                let funding_script = self.get_funding_redeemscript();
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
-               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_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw, logger).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)[..]);
 
                // They sign the "local" commitment transaction...
-               log_trace!(self, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
+               log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
                secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");
 
                let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 
                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;
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
                let remote_signature = 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)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?.0;
 
@@ -1476,7 +1473,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> {
+       pub fn funding_created<L: Deref>(&mut self, msg: &msgs::FundingCreated, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor<ChanSigner>), ChannelError> where L::Target: Logger {
                if self.channel_outbound {
                        return Err(ChannelError::Close("Received funding_created for an outbound channel?"));
                }
@@ -1495,7 +1492,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let funding_txo = OutPoint::new(msg.funding_txid, msg.funding_output_index);
                self.funding_txo = Some(funding_txo.clone());
 
-               let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature) = match self.funding_created_signature(&msg.signature) {
+               let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature) = match self.funding_created_signature(&msg.signature, logger) {
                        Ok(res) => res,
                        Err(e) => {
                                self.funding_txo = None;
@@ -1516,10 +1513,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                              &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(),
-                                                                             local_initial_commitment_tx.clone(),
-                                                                             self.logger.clone());
+                                                                             local_initial_commitment_tx.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_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap(), logger);
                                channel_monitor
                        } }
                }
@@ -1540,7 +1536,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// 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> {
+       pub fn funding_signed<L: Deref>(&mut self, msg: &msgs::FundingSigned, logger: &L) -> Result<ChannelMonitor<ChanSigner>, ChannelError> where L::Target: Logger {
                if !self.channel_outbound {
                        return Err(ChannelError::Close("Received funding_signed for an inbound channel?"));
                }
@@ -1556,10 +1552,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let funding_script = self.get_funding_redeemscript();
 
                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;
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
-               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_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw, logger).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;
@@ -1582,10 +1578,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                              &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(),
-                                                                             local_commitment_tx,
-                                                                             self.logger.clone());
+                                                                             local_commitment_tx);
 
-                               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_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap(), logger);
 
                                channel_monitor
                        } }
@@ -1698,7 +1693,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if htlc_inbound_value_msat + msg.amount_msat > Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) {
                        return Err(ChannelError::Close("Remote HTLC add would put them over our max HTLC value"));
                }
-               // Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
+               // Check remote_channel_reserve_satoshis (we're getting paid, so they have to at least meet
                // the reserve_satoshis we told them to always have as direct payment so that they lose
                // something if we punish them for broadcasting an old state).
                // Note that we don't really care about having a small/no to_remote output in our local
@@ -1713,13 +1708,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let mut removed_outbound_total_msat = 0;
                for ref htlc in self.pending_outbound_htlcs.iter() {
                        if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(None) = htlc.state {
+debug_assert!(false, "This should be triggerable, and we should add a test case that does so!");
                                removed_outbound_total_msat += htlc.amount_msat;
                        } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(None) = htlc.state {
                                removed_outbound_total_msat += htlc.amount_msat;
                        }
                }
-               if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
-                       return Err(ChannelError::Close("Remote HTLC add would put them over their reserve value"));
+               if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
+                       return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
                }
                if self.next_remote_htlc_id != msg.htlc_id {
                        return Err(ChannelError::Close("Remote skipped HTLC ID"));
@@ -1755,6 +1751,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        None => {},
                                        Some(payment_hash) =>
                                                if payment_hash != htlc.payment_hash {
+                                                       println!("FAIL: {:?}, {:?}", htlc.payment_hash, payment_hash);
                                                        return Err(ChannelError::Close("Remote tried to fulfill HTLC with an incorrect preimage"));
                                                }
                                };
@@ -1797,7 +1794,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       pub fn update_fail_malformed_htlc<'a>(&mut self, msg: &msgs::UpdateFailMalformedHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
+       pub fn update_fail_malformed_htlc(&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"));
                }
@@ -1809,7 +1806,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       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 {
+       pub fn commitment_signed<F: Deref, L: Deref>(&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &F, logger: &L) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate), (Option<ChannelMonitorUpdate>, ChannelError)>
+       where F::Target: FeeEstimator,
+                               L::Target: Logger
+       {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err((None, ChannelError::Close("Got commitment signed message when channel was not in an operational state")));
                }
@@ -1833,13 +1833,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                };
 
                let mut local_commitment_tx = {
-                       let mut commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, feerate_per_kw);
+                       let mut commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, feerate_per_kw, logger);
                        let htlcs_cloned: Vec<_> = commitment_tx.2.drain(..).map(|htlc| (htlc.0, htlc.1.map(|h| h.clone()))).collect();
                        (commitment_tx.0, commitment_tx.1, htlcs_cloned)
                };
                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 {} (sighash {}) with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_commitment_tx.0), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
+               log_trace!(logger, "Checking commitment tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_commitment_tx.0), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
                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")));
                }
@@ -1849,7 +1849,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        let num_htlcs = local_commitment_tx.1;
                        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 {
+                       let remote_reserve_we_require = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+                       if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + remote_reserve_we_require {
                                return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee")));
                        }
                }
@@ -1867,7 +1868,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                let htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
                                let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
-                               log_trace!(self, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(local_keys.b_htlc_key.serialize()), encode::serialize_hex(&htlc_tx), log_bytes!(htlc_sighash[..]), encode::serialize_hex(&htlc_redeemscript));
+                               log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(local_keys.b_htlc_key.serialize()), encode::serialize_hex(&htlc_tx), log_bytes!(htlc_sighash[..]), encode::serialize_hex(&htlc_redeemscript));
                                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")));
                                }
@@ -1907,7 +1908,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                htlc_outputs: htlcs_and_sigs
                        }]
                };
-               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone(), logger).unwrap();
 
                for htlc in self.pending_inbound_htlcs.iter_mut() {
                        let new_forward = if let &InboundHTLCState::RemoteAnnounced(ref forward_info) = &htlc.state {
@@ -1941,7 +1942,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                // the corresponding HTLC status updates so that get_last_commitment_update
                                // includes the right HTLCs.
                                self.monitor_pending_commitment_signed = true;
-                               let (_, mut additional_update) = self.send_commitment_no_status_check().map_err(|e| (None, e))?;
+                               let (_, mut additional_update) = self.send_commitment_no_status_check(logger).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;
@@ -1956,7 +1957,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // 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, mut additional_update) = self.send_commitment_no_status_check().map_err(|e| (None, e))?;
+                       let (msg, mut additional_update) = self.send_commitment_no_status_check(logger).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;
@@ -1975,10 +1976,10 @@ 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, ChannelMonitorUpdate)>, ChannelError> {
+       fn free_holding_cell_htlcs<L: Deref>(&mut self, logger: &L) -> Result<Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
                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 { "" });
+                       log_trace!(logger, "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!
@@ -2007,10 +2008,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                Err(e) => {
                                                                        match e {
                                                                                ChannelError::Ignore(ref msg) => {
-                                                                                       log_info!(self, "Failed to send HTLC with payment_hash {} due to {}", log_bytes!(payment_hash.0), msg);
+                                                                                       log_info!(logger, "Failed to send HTLC with payment_hash {} due to {}", log_bytes!(payment_hash.0), msg);
                                                                                },
                                                                                _ => {
-                                                                                       log_info!(self, "Failed to send HTLC with payment_hash {} resulting in a channel closure during holding_cell freeing", log_bytes!(payment_hash.0));
+                                                                                       log_info!(logger, "Failed to send HTLC with payment_hash {} resulting in a channel closure during holding_cell freeing", log_bytes!(payment_hash.0));
                                                                                },
                                                                        }
                                                                        err = Some(e);
@@ -2018,7 +2019,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                        }
                                                },
                                                &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
-                                                       match self.get_update_fulfill_htlc(htlc_id, *payment_preimage) {
+                                                       match self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) {
                                                                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 {
@@ -2075,7 +2076,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                        None
                                                };
 
-                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check()?;
+                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check(logger)?;
                                        // 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;
@@ -2102,8 +2103,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<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
+       pub fn revoke_and_ack<F: Deref, L: Deref>(&mut self, msg: &msgs::RevokeAndACK, fee_estimator: &F, logger: &L) -> Result<(Option<msgs::CommitmentUpdate>, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option<msgs::ClosingSigned>, ChannelMonitorUpdate), ChannelError>
+               where F::Target: FeeEstimator,
+                                       L::Target: Logger,
        {
                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"));
@@ -2142,7 +2144,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                secret: msg.per_commitment_secret,
                        }],
                };
-               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone(), logger).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
@@ -2153,7 +2155,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.their_cur_commitment_point = Some(msg.next_per_commitment_point);
                self.cur_remote_commitment_transaction_number -= 1;
 
-               log_trace!(self, "Updating HTLCs on receipt of RAA...");
+               log_trace!(logger, "Updating HTLCs on receipt of RAA...");
                let mut to_forward_infos = Vec::new();
                let mut revoked_htlcs = Vec::new();
                let mut update_fail_htlcs = Vec::new();
@@ -2165,7 +2167,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // Take references explicitly so that we can hold multiple references to self.
                        let pending_inbound_htlcs: &mut Vec<_> = &mut self.pending_inbound_htlcs;
                        let pending_outbound_htlcs: &mut Vec<_> = &mut self.pending_outbound_htlcs;
-                       let logger = LogHolder { logger: &self.logger };
 
                        // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
                        pending_inbound_htlcs.retain(|htlc| {
@@ -2269,7 +2270,7 @@ 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.
-                               let (_, mut additional_update) = self.send_commitment_no_status_check()?;
+                               let (_, mut additional_update) = self.send_commitment_no_status_check(logger)?;
                                // 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;
@@ -2280,7 +2281,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Ok((None, Vec::new(), Vec::new(), None, monitor_update))
                }
 
-               match self.free_holding_cell_htlcs()? {
+               match self.free_holding_cell_htlcs(logger)? {
                        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(..) {
@@ -2300,7 +2301,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        },
                        None => {
                                if require_commitment {
-                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check()?;
+                                       let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check(logger)?;
 
                                        // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
                                        // strictly increasing by one, so decrement it here.
@@ -2351,23 +2352,24 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                })
        }
 
-       pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u64) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> {
+       pub fn send_update_fee_and_commit<L: Deref>(&mut self, feerate_per_kw: u64, logger: &L) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
                match self.send_update_fee(feerate_per_kw) {
                        Some(update_fee) => {
-                               let (commitment_signed, monitor_update) = self.send_commitment_no_status_check()?;
+                               let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?;
                                Ok(Some((update_fee, commitment_signed, monitor_update)))
                        },
                        None => Ok(None)
                }
        }
 
+       #[cfg_attr(all(test, feature = "mutation_testing"), mutate)]
        /// Removes any uncommitted HTLCs, to be used on peer disconnection, including any pending
        /// HTLCs that we intended to add but haven't as we were waiting on a remote revoke.
        /// Returns the set of PendingHTLCStatuses from remote uncommitted HTLCs (which we're
        /// implicitly dropping) and the payment_hashes of HTLCs we tried to add but are dropping.
        /// No further message handling calls may be made until a channel_reestablish dance has
        /// completed.
-       pub fn remove_uncommitted_htlcs_and_mark_paused(&mut self) -> Vec<(HTLCSource, PaymentHash)> {
+       pub fn remove_uncommitted_htlcs_and_mark_paused<L: Deref>(&mut self, logger: &L) -> Vec<(HTLCSource, PaymentHash)> where L::Target: Logger {
                let mut outbound_drops = Vec::new();
 
                assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
@@ -2426,7 +2428,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                });
                self.channel_state |= ChannelState::PeerDisconnected as u32;
-               log_debug!(self, "Peer disconnection resulted in {} remote-announced HTLC drops and {} waiting-to-locally-announced HTLC drops on channel {}", outbound_drops.len(), inbound_drop_count, log_bytes!(self.channel_id()));
+               log_debug!(logger, "Peer disconnection resulted in {} remote-announced HTLC drops and {} waiting-to-locally-announced HTLC drops on channel {}", outbound_drops.len(), inbound_drop_count, log_bytes!(self.channel_id()));
                outbound_drops
        }
 
@@ -2447,10 +2449,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.channel_state |= ChannelState::MonitorUpdateFailed as u32;
        }
 
+       #[cfg_attr(all(test, feature = "mutation_testing"), mutate)]
        /// 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<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option<msgs::FundingLocked>) {
+       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option<msgs::FundingLocked>) where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
@@ -2488,13 +2491,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        Some(self.get_last_revoke_and_ack())
                } else { None };
                let commitment_update = if self.monitor_pending_commitment_signed {
-                       Some(self.get_last_commitment_update())
+                       Some(self.get_last_commitment_update(logger))
                } else { None };
+//XXX: Should we free_holding_cell_htlcs() here if we dont need a cs normally?
 
                self.monitor_pending_revoke_and_ack = false;
                self.monitor_pending_commitment_signed = false;
                let order = self.resend_order.clone();
-               log_trace!(self, "Restored monitor updating resulting in {}{} commitment update and {} RAA, with {} first",
+               log_trace!(logger, "Restored monitor updating resulting in {}{} commitment update and {} RAA, with {} first",
                        if needs_broadcast_safe { "a funding broadcast safe, " } else { "" },
                        if commitment_update.is_some() { "a" } else { "no" },
                        if raa.is_some() { "an" } else { "no" },
@@ -2517,6 +2521,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
+       #[cfg_attr(all(test, feature = "mutation_testing"), mutate)]
        fn get_last_revoke_and_ack(&self) -> msgs::RevokeAndACK {
                let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number));
                let per_commitment_secret = chan_utils::build_commitment_secret(self.local_keys.commitment_seed(), self.cur_local_commitment_transaction_number + 2);
@@ -2527,7 +2532,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
        }
 
-       fn get_last_commitment_update(&self) -> msgs::CommitmentUpdate {
+       fn get_last_commitment_update<L: Deref>(&self, logger: &L) -> msgs::CommitmentUpdate where L::Target: Logger {
                let mut update_add_htlcs = Vec::new();
                let mut update_fulfill_htlcs = Vec::new();
                let mut update_fail_htlcs = Vec::new();
@@ -2575,18 +2580,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               log_trace!(self, "Regenerated latest commitment update with {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds",
+               log_trace!(logger, "Regenerated latest commitment update with {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds",
                                update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len());
                msgs::CommitmentUpdate {
                        update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs,
                        update_fee: None,
-                       commitment_signed: self.send_commitment_no_state_update().expect("It looks like we failed to re-generate a commitment_signed we had previously sent?").0,
+                       commitment_signed: self.send_commitment_no_state_update(logger).expect("It looks like we failed to re-generate a commitment_signed we had previously sent?").0,
                }
        }
 
        /// 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<ChannelMonitorUpdate>, RAACommitmentOrder, Option<msgs::Shutdown>), ChannelError> {
+       pub fn channel_reestablish<L: Deref>(&mut self, msg: &msgs::ChannelReestablish, logger: &L) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, Option<ChannelMonitorUpdate>, RAACommitmentOrder, Option<msgs::Shutdown>), ChannelError> where L::Target: Logger {
                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
@@ -2679,9 +2684,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                if msg.next_local_commitment_number == our_next_remote_commitment_number {
                        if required_revoke.is_some() {
-                               log_debug!(self, "Reconnected channel {} with only lost outbound RAA", log_bytes!(self.channel_id()));
+                               log_debug!(logger, "Reconnected channel {} with only lost outbound RAA", log_bytes!(self.channel_id()));
                        } else {
-                               log_debug!(self, "Reconnected channel {} with no loss", log_bytes!(self.channel_id()));
+                               log_debug!(logger, "Reconnected channel {} with no loss", log_bytes!(self.channel_id()));
                        }
 
                        if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) == 0 {
@@ -2689,7 +2694,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                // channel_reestablish should result in them sending a revoke_and_ack), but we may
                                // have received some updates while we were disconnected. Free the holding cell
                                // now!
-                               match self.free_holding_cell_htlcs() {
+                               match self.free_holding_cell_htlcs(logger) {
                                        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, monitor_update))) => return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), shutdown_msg)),
@@ -2700,9 +2705,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                } else if msg.next_local_commitment_number == our_next_remote_commitment_number - 1 {
                        if required_revoke.is_some() {
-                               log_debug!(self, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", log_bytes!(self.channel_id()));
+                               log_debug!(logger, "Reconnected channel {} with lost outbound RAA and lost remote commitment tx", log_bytes!(self.channel_id()));
                        } else {
-                               log_debug!(self, "Reconnected channel {} with only lost remote commitment tx", log_bytes!(self.channel_id()));
+                               log_debug!(logger, "Reconnected channel {} with only lost remote commitment tx", log_bytes!(self.channel_id()));
                        }
 
                        if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 {
@@ -2710,7 +2715,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                return Ok((resend_funding_locked, None, None, None, self.resend_order.clone(), shutdown_msg));
                        }
 
-                       return Ok((resend_funding_locked, required_revoke, Some(self.get_last_commitment_update()), None, self.resend_order.clone(), shutdown_msg));
+                       return Ok((resend_funding_locked, required_revoke, Some(self.get_last_commitment_update(logger)), None, self.resend_order.clone(), shutdown_msg));
                } else {
                        return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction"));
                }
@@ -3026,7 +3031,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                ChannelValueStat {
                        value_to_self_msat: self.value_to_self_msat,
                        channel_value_msat: self.channel_value_satoshis * 1000,
-                       channel_reserve_msat: self.their_channel_reserve_satoshis * 1000,
+                       channel_reserve_msat: self.local_channel_reserve_satoshis * 1000,
                        pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        holding_cell_outbound_amount_msat: {
@@ -3314,7 +3319,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        push_msat: self.channel_value_satoshis * 1000 - self.value_to_self_msat,
                        dust_limit_satoshis: self.our_dust_limit_satoshis,
                        max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
+                       channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.our_htlc_minimum_msat,
                        feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
                        to_self_delay: self.our_to_self_delay,
@@ -3347,7 +3352,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        temporary_channel_id: self.channel_id,
                        dust_limit_satoshis: self.our_dust_limit_satoshis,
                        max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
+                       channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.our_htlc_minimum_msat,
                        minimum_depth: self.minimum_depth,
                        to_self_delay: self.our_to_self_delay,
@@ -3363,9 +3368,9 @@ 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, ChannelError> {
+       fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
                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;
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).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)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?.0)
        }
@@ -3377,7 +3382,7 @@ 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, ChannelError> {
+       pub fn get_outbound_funding_created<L: Deref>(&mut self, funding_txo: OutPoint, logger: &L) -> Result<msgs::FundingCreated, ChannelError> where L::Target: Logger {
                if !self.channel_outbound {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
@@ -3391,10 +3396,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                self.funding_txo = Some(funding_txo.clone());
-               let our_signature = match self.get_outbound_funding_created_signature() {
+               let our_signature = match self.get_outbound_funding_created_signature(logger) {
                        Ok(res) => res,
                        Err(e) => {
-                               log_error!(self, "Got bad signatures: {:?}!", e);
+                               log_error!(logger, "Got bad signatures: {:?}!", e);
                                self.funding_txo = None;
                                return Err(e);
                        }
@@ -3456,7 +3461,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// May panic if called on a channel that wasn't immediately-previously
        /// self.remove_uncommitted_htlcs_and_mark_paused()'d
-       pub fn get_channel_reestablish(&self) -> msgs::ChannelReestablish {
+       pub fn get_channel_reestablish<L: Deref>(&self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::PeerDisconnected as u32, ChannelState::PeerDisconnected as u32);
                assert_ne!(self.cur_remote_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER);
                // Prior to static_remotekey, my_current_per_commitment_point was critical to claiming
@@ -3469,13 +3474,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let dummy_pubkey = PublicKey::from_slice(&pk).unwrap();
                let data_loss_protect = if self.cur_remote_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER {
                        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));
+                       log_trace!(logger, "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: dummy_pubkey
                        })
                } else {
-                       log_info!(self, "Sending a data_loss_protect with no previous remote per_commitment_secret");
+                       log_info!(logger, "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: dummy_pubkey,
@@ -3551,10 +3556,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
                }
 
-               // Check self.their_channel_reserve_satoshis (the amount we must keep as
-               // reserve for them to have something to claim if we misbehave)
-               if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
-                       return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value"));
+               // Check self.local_channel_reserve_satoshis (the amount we must keep as
+               // reserve for the remote to have something to claim if we misbehave)
+               if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+                       return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
                }
 
                // Now update local state:
@@ -3595,7 +3600,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, ChannelMonitorUpdate), ChannelError> {
+       pub fn send_commitment<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Cannot create commitment tx until channel is fully established");
                }
@@ -3624,10 +3629,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if !have_updates {
                        panic!("Cannot create commitment tx until we have some updates to send");
                }
-               self.send_commitment_no_status_check()
+               self.send_commitment_no_status_check(logger)
        }
        /// Only fails in case of bad keys
-       fn send_commitment_no_status_check(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> {
+       fn send_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
                // 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.
@@ -3648,7 +3653,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                self.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
 
-               let (res, remote_commitment_tx, htlcs) = match self.send_commitment_no_state_update() {
+               let (res, remote_commitment_tx, htlcs) = match self.send_commitment_no_state_update(logger) {
                        Ok((res, (remote_commitment_tx, mut htlcs))) => {
                                // Update state now that we've passed all the can-fail calls...
                                let htlcs_no_ref: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
@@ -3668,14 +3673,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                their_revocation_point: self.their_cur_commitment_point.unwrap()
                        }]
                };
-               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
+               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone(), logger).unwrap();
                self.channel_state |= ChannelState::AwaitingRemoteRevoke as u32;
                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> {
+       fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
                let mut feerate_per_kw = self.feerate_per_kw;
                if let Some(feerate) = self.pending_update_fee {
                        if self.channel_outbound {
@@ -3684,7 +3689,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let remote_keys = self.build_remote_transaction_keys()?;
-               let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true, feerate_per_kw);
+               let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true, feerate_per_kw, logger);
                let (signature, htlc_signatures);
 
                {
@@ -3698,13 +3703,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        signature = res.0;
                        htlc_signatures = res.1;
 
-                       log_trace!(self, "Signed remote commitment tx {} with redeemscript {} -> {}",
+                       log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
                                encode::serialize_hex(&remote_commitment_tx.0),
                                encode::serialize_hex(&self.get_funding_redeemscript()),
                                log_bytes!(signature.serialize_compact()[..]));
 
                        for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
-                               log_trace!(self, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {}",
+                               log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {}",
                                        encode::serialize_hex(&chan_utils::build_htlc_transaction(&remote_commitment_tx.0.txid(), feerate_per_kw, self.our_to_self_delay, htlc, &remote_keys.a_delayed_payment_key, &remote_keys.revocation_key)),
                                        encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &remote_keys)),
                                        log_bytes!(remote_keys.a_htlc_key.serialize()),
@@ -3723,10 +3728,10 @@ 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, ChannelMonitorUpdate)>, ChannelError> {
+       pub fn send_htlc_and_commit<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, logger: &L) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
                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()?;
+                               let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?;
                                Ok(Some((update_add_htlc, commitment_signed, monitor_update)))
                        },
                        None => Ok(None)
@@ -4020,7 +4025,7 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                self.their_dust_limit_satoshis.write(writer)?;
                self.our_dust_limit_satoshis.write(writer)?;
                self.their_max_htlc_value_in_flight_msat.write(writer)?;
-               self.their_channel_reserve_satoshis.write(writer)?;
+               self.local_channel_reserve_satoshis.write(writer)?;
                self.their_htlc_minimum_msat.write(writer)?;
                self.our_htlc_minimum_msat.write(writer)?;
                self.their_to_self_delay.write(writer)?;
@@ -4043,8 +4048,8 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
        }
 }
 
-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> {
+impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
+       fn read<R : ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
                let _ver: u8 = Readable::read(reader)?;
                let min_ver: u8 = Readable::read(reader)?;
                if min_ver > SERIALIZATION_VERSION {
@@ -4176,7 +4181,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                let their_dust_limit_satoshis = Readable::read(reader)?;
                let our_dust_limit_satoshis = Readable::read(reader)?;
                let their_max_htlc_value_in_flight_msat = Readable::read(reader)?;
-               let their_channel_reserve_satoshis = Readable::read(reader)?;
+               let local_channel_reserve_satoshis = Readable::read(reader)?;
                let their_htlc_minimum_msat = Readable::read(reader)?;
                let our_htlc_minimum_msat = Readable::read(reader)?;
                let their_to_self_delay = Readable::read(reader)?;
@@ -4193,7 +4198,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                let their_shutdown_scriptpubkey = Readable::read(reader)?;
                let commitment_secrets = Readable::read(reader)?;
 
-               let (monitor_last_block, channel_monitor) = ReadableArgs::read(reader, logger.clone())?;
+               let (monitor_last_block, channel_monitor) = Readable::read(reader)?;
                // 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
                if monitor_last_block != last_block_connected {
@@ -4255,7 +4260,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                        their_dust_limit_satoshis,
                        our_dust_limit_satoshis,
                        their_max_htlc_value_in_flight_msat,
-                       their_channel_reserve_satoshis,
+                       local_channel_reserve_satoshis,
                        their_htlc_minimum_msat,
                        our_htlc_minimum_msat,
                        their_to_self_delay,
@@ -4275,8 +4280,6 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                        commitment_secrets,
 
                        network_sync: UpdateStatus::Fresh,
-
-                       logger,
                })
        }
 }
@@ -4364,25 +4367,25 @@ mod tests {
        #[test]
        fn channel_reestablish_no_updates() {
                let feeest = TestFeeEstimator{fee_est: 15000};
-               let logger : Arc<Logger> = Arc::new(test_utils::TestLogger::new());
+               let logger = 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>);
+               let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
 
                // 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();
+               let mut node_a_chan = Channel::<EnforcingChannelKeys>::new_outbound(&&feeest, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &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::known(), &open_channel_msg, 7, logger, &config).unwrap();
+               let mut node_b_chan = Channel::<EnforcingChannelKeys>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
 
                // Node B --> Node A: accept channel
                let accept_channel_msg = node_b_chan.get_accept_channel();
@@ -4394,16 +4397,16 @@ mod tests {
                        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();
+               let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap();
+               let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, &&logger).unwrap();
 
                // Node B --> Node A: funding signed
-               let _ = node_a_chan.funding_signed(&funding_signed_msg);
+               let _ = node_a_chan.funding_signed(&funding_signed_msg, &&logger);
 
                // 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 msg = node_b_chan.get_channel_reestablish();
+               node_b_chan.remove_uncommitted_htlcs_and_mark_paused(&&logger);
+               let msg = node_b_chan.get_channel_reestablish(&&logger);
                assert_eq!(msg.next_local_commitment_number, 1); // now called next_commitment_number
                assert_eq!(msg.next_remote_commitment_number, 0); // now called next_revocation_number
                match msg.data_loss_protect {
@@ -4415,8 +4418,8 @@ mod tests {
 
                // Check that the commitment point in Node A's channel_reestablish message
                // is sane.
-               node_a_chan.remove_uncommitted_htlcs_and_mark_paused();
-               let msg = node_a_chan.get_channel_reestablish();
+               node_a_chan.remove_uncommitted_htlcs_and_mark_paused(&&logger);
+               let msg = node_a_chan.get_channel_reestablish(&&logger);
                assert_eq!(msg.next_local_commitment_number, 1); // now called next_commitment_number
                assert_eq!(msg.next_remote_commitment_number, 0); // now called next_revocation_number
                match msg.data_loss_protect {
@@ -4454,7 +4457,7 @@ mod tests {
                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, 10_000_000, 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, &config).unwrap(); // Nothing uses their network key in this test
                chan.their_to_self_delay = 144;
                chan.our_dust_limit_satoshis = 546;
 
@@ -4498,7 +4501,7 @@ mod tests {
                                $( { $htlc_idx: expr, $their_htlc_sig_hex: expr, $our_htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
                        } ) => { {
                                unsigned_tx = {
-                                       let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw);
+                                       let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw, &logger);
                                        let htlcs = res.2.drain(..)
                                                .filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
                                                .collect();