Relicense as dual Apache-2.0 + MIT
[rust-lightning] / lightning / src / ln / channel.rs
index 0c30a7e96cb53bfec845174a2417e002aedce211..b8f297c45bc075eb413d6ead88c37afc91e79e09 100644 (file)
@@ -1,3 +1,12 @@
+// This file is Copyright its original authors, visible in version control
+// history.
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
 use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
@@ -19,7 +28,7 @@ use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
-use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, PreCalculatedTxCreationKeys};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::transaction::OutPoint;
@@ -34,6 +43,7 @@ use std;
 use std::default::Default;
 use std::{cmp,mem,fmt};
 use std::ops::Deref;
+use bitcoin::hashes::hex::ToHex;
 
 #[cfg(test)]
 pub struct ChannelValueStat {
@@ -44,6 +54,7 @@ pub struct ChannelValueStat {
        pub pending_inbound_htlcs_amount_msat: u64,
        pub holding_cell_outbound_amount_msat: u64,
        pub their_max_htlc_value_in_flight_msat: u64, // outgoing
+       pub their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -53,19 +64,43 @@ enum InboundHTLCRemovalReason {
 }
 
 enum InboundHTLCState {
-       /// Added by remote, to be included in next local commitment tx.
+       /// Offered by remote, to be included in next local commitment tx. I.e., the remote sent an
+       /// update_add_htlc message for this HTLC.
        RemoteAnnounced(PendingHTLCStatus),
-       /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
-       /// the remote side hasn't yet revoked their previous state, which we need them to do before we
-       /// accept this HTLC. Implies AwaitingRemoteRevoke.
-       /// We also have not yet included this HTLC in a commitment_signed message, and are waiting on
-       /// a remote revoke_and_ack on a previous state before we can do so.
+       /// Included in a received commitment_signed message (implying we've
+       /// revoke_and_ack'd it), but the remote hasn't yet revoked their previous
+       /// state (see the example below). We have not yet included this HTLC in a
+       /// commitment_signed message because we are waiting on the remote's
+       /// aforementioned state revocation. One reason this missing remote RAA
+       /// (revoke_and_ack) blocks us from constructing a commitment_signed message
+       /// is because every time we create a new "state", i.e. every time we sign a
+       /// new commitment tx (see [BOLT #2]), we need a new per_commitment_point,
+       /// which are provided one-at-a-time in each RAA. E.g., the last RAA they
+       /// sent provided the per_commitment_point for our current commitment tx.
+       /// The other reason we should not send a commitment_signed without their RAA
+       /// is because their RAA serves to ACK our previous commitment_signed.
+       ///
+       /// Here's an example of how an HTLC could come to be in this state:
+       /// remote --> update_add_htlc(prev_htlc)   --> local
+       /// remote --> commitment_signed(prev_htlc) --> local
+       /// remote <-- revoke_and_ack               <-- local
+       /// remote <-- commitment_signed(prev_htlc) <-- local
+       /// [note that here, the remote does not respond with a RAA]
+       /// remote --> update_add_htlc(this_htlc)   --> local
+       /// remote --> commitment_signed(prev_htlc, this_htlc) --> local
+       /// Now `this_htlc` will be assigned this state. It's unable to be officially
+       /// accepted, i.e. included in a commitment_signed, because we're missing the
+       /// RAA that provides our next per_commitment_point. The per_commitment_point
+       /// is used to derive commitment keys, which are used to construct the
+       /// signatures in a commitment_signed message.
+       /// Implies AwaitingRemoteRevoke.
+       /// [BOLT #2]: https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md
        AwaitingRemoteRevokeToAnnounce(PendingHTLCStatus),
-       /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but
-       /// the remote side hasn't yet revoked their previous state, which we need them to do before we
-       /// accept this HTLC. Implies AwaitingRemoteRevoke.
-       /// We have included this HTLC in our latest commitment_signed and are now just waiting on a
-       /// revoke_and_ack.
+       /// Included in a received commitment_signed message (implying we've revoke_and_ack'd it).
+       /// We have also included this HTLC in our latest commitment_signed and are now just waiting
+       /// on the remote's revoke_and_ack to make this HTLC an irrevocable part of the state of the
+       /// channel (before it can then get forwarded and/or removed).
+       /// Implies AwaitingRemoteRevoke.
        AwaitingAnnouncedRemoteRevoke(PendingHTLCStatus),
        Committed,
        /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we
@@ -159,9 +194,9 @@ enum HTLCUpdateAwaitingACK {
 /// move on to ShutdownComplete, at which point most calls into this channel are disallowed.
 enum ChannelState {
        /// Implies we have (or are prepared to) send our open_channel/accept_channel message
-       OurInitSent = (1 << 0),
+       OurInitSent = 1 << 0,
        /// Implies we have received their open_channel/accept_channel message
-       TheirInitSent = (1 << 1),
+       TheirInitSent = 1 << 1,
        /// We have sent funding_created and are awaiting a funding_signed to advance to FundingSent.
        /// Note that this is nonsense for an inbound channel as we immediately generate funding_signed
        /// upon receipt of funding_created, so simply skip this state.
@@ -172,35 +207,35 @@ enum ChannelState {
        FundingSent = 8,
        /// Flag which can be set on FundingSent to indicate they sent us a funding_locked message.
        /// Once both TheirFundingLocked and OurFundingLocked are set, state moves on to ChannelFunded.
-       TheirFundingLocked = (1 << 4),
+       TheirFundingLocked = 1 << 4,
        /// Flag which can be set on FundingSent to indicate we sent them a funding_locked message.
        /// Once both TheirFundingLocked and OurFundingLocked are set, state moves on to ChannelFunded.
-       OurFundingLocked = (1 << 5),
+       OurFundingLocked = 1 << 5,
        ChannelFunded = 64,
        /// Flag which is set on ChannelFunded and FundingSent indicating remote side is considered
        /// "disconnected" and no updates are allowed until after we've done a channel_reestablish
        /// dance.
-       PeerDisconnected = (1 << 7),
+       PeerDisconnected = 1 << 7,
        /// Flag which is set on ChannelFunded, FundingCreated, and FundingSent indicating the user has
        /// told us they failed to update our ChannelMonitor somewhere and we should pause sending any
        /// outbound messages until they've managed to do so.
-       MonitorUpdateFailed = (1 << 8),
+       MonitorUpdateFailed = 1 << 8,
        /// Flag which implies that we have sent a commitment_signed but are awaiting the responding
        /// revoke_and_ack message. During this time period, we can't generate new commitment_signed
        /// messages as then we will be unable to determine which HTLCs they included in their
        /// revoke_and_ack implicit ACK, so instead we have to hold them away temporarily to be sent
        /// later.
        /// Flag is set on ChannelFunded.
-       AwaitingRemoteRevoke = (1 << 9),
+       AwaitingRemoteRevoke = 1 << 9,
        /// Flag which is set on ChannelFunded or FundingSent after receiving a shutdown message from
        /// the remote end. If set, they may not add any new HTLCs to the channel, and we are expected
        /// to respond with our own shutdown message when possible.
-       RemoteShutdownSent = (1 << 10),
+       RemoteShutdownSent = 1 << 10,
        /// Flag which is set on ChannelFunded or FundingSent after sending a shutdown message. At this
        /// point, we may not add any new HTLCs to the channel.
        /// TODO: Investigate some kind of timeout mechanism by which point the remote end must provide
        /// us their shutdown.
-       LocalShutdownSent = (1 << 11),
+       LocalShutdownSent = 1 << 11,
        /// We've successfully negotiated a closing_signed dance. At this point ChannelManager is about
        /// to drop us, but we store this anyway.
        ShutdownComplete = 4096,
@@ -285,16 +320,16 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        // revoke_and_ack is received and new commitment_signed is generated to be
        // sent to the funder. Otherwise, the pending value is removed when receiving
        // commitment_signed.
-       pending_update_fee: Option<u64>,
+       pending_update_fee: Option<u32>,
        // update_fee() during ChannelState::AwaitingRemoteRevoke is hold in
        // holdina_cell_update_fee then moved to pending_udpate_fee when revoke_and_ack
        // is received. holding_cell_update_fee is updated when there are additional
        // update_fee() during ChannelState::AwaitingRemoteRevoke.
-       holding_cell_update_fee: Option<u64>,
+       holding_cell_update_fee: Option<u32>,
        next_local_htlc_id: u64,
        next_remote_htlc_id: u64,
        update_time_counter: u32,
-       feerate_per_kw: u64,
+       feerate_per_kw: u32,
 
        #[cfg(debug_assertions)]
        /// Max to_local and to_remote outputs in a locally-generated commitment transaction
@@ -303,7 +338,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        /// Max to_local and to_remote outputs in a remote-generated commitment transaction
        max_commitment_tx_output_remote: ::std::sync::Mutex<(u64, u64)>,
 
-       last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig)
+       last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, our_sig)
 
        funding_txo: Option<OutPoint>,
 
@@ -384,17 +419,17 @@ pub const MAX_FUNDING_SATOSHIS: u64 = 1 << 24;
 /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our
 /// channel_id in ChannelManager.
 pub(super) enum ChannelError {
-       Ignore(&'static str),
-       Close(&'static str),
-       CloseDelayBroadcast(&'static str),
+       Ignore(String),
+       Close(String),
+       CloseDelayBroadcast(String),
 }
 
 impl fmt::Debug for ChannelError {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                match self {
-                       &ChannelError::Ignore(e) => write!(f, "Ignore : {}", e),
-                       &ChannelError::Close(e) => write!(f, "Close : {}", e),
-                       &ChannelError::CloseDelayBroadcast(e) => write!(f, "CloseDelayBroadcast : {}", e)
+                       &ChannelError::Ignore(ref e) => write!(f, "Ignore : {}", e),
+                       &ChannelError::Close(ref e) => write!(f, "Close : {}", e),
+                       &ChannelError::CloseDelayBroadcast(ref e) => write!(f, "CloseDelayBroadcast : {}", e)
                }
        }
 }
@@ -423,8 +458,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
        }
 
-       fn derive_our_dust_limit_satoshis(at_open_background_feerate: u64) -> u64 {
-               cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO
+       fn derive_our_dust_limit_satoshis(at_open_background_feerate: u32) -> u64 {
+               cmp::max(at_open_background_feerate as u64 * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO
        }
 
        // Constructors:
@@ -432,20 +467,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
              F::Target: FeeEstimator,
        {
+               let our_to_self_delay = config.own_channel_config.our_to_self_delay;
                let chan_keys = keys_provider.get_channel_keys(false, channel_value_satoshis);
 
                if channel_value_satoshis >= MAX_FUNDING_SATOSHIS {
-                       return Err(APIError::APIMisuseError{err: "funding value > 2^24"});
+                       return Err(APIError::APIMisuseError{err: format!("funding_value must be smaller than {}, it was {}", MAX_FUNDING_SATOSHIS, channel_value_satoshis)});
                }
-
-               if push_msat > channel_value_satoshis * 1000 {
-                       return Err(APIError::APIMisuseError{err: "push value > channel value"});
+               let channel_value_msat = channel_value_satoshis * 1000;
+               if push_msat > channel_value_msat {
+                       return Err(APIError::APIMisuseError { err: format!("Push value ({}) was larger than channel_value ({})", push_msat, channel_value_msat) });
                }
-               if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
-                       return Err(APIError::APIMisuseError{err: "Configured with an unreasonable our_to_self_delay putting user funds at risks"});
+               if our_to_self_delay < BREAKDOWN_TIMEOUT {
+                       return Err(APIError::APIMisuseError {err: format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks", our_to_self_delay)});
                }
-
-
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
                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});
@@ -511,7 +545,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        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,
-                       our_to_self_delay: config.own_channel_config.our_to_self_delay,
+                       our_to_self_delay,
                        their_max_accepted_htlcs: 0,
                        minimum_depth: 0, // Filled in in accept_channel
 
@@ -533,11 +567,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn check_remote_fee<F: Deref>(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError>
                where F::Target: FeeEstimator
        {
-               if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) {
-                       return Err(ChannelError::Close("Peer's feerate much too low"));
+               let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+               if feerate_per_kw < lower_limit {
+                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
                }
-               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 {
-                       return Err(ChannelError::Close("Peer's feerate much too high"));
+               let upper_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64  * 2;
+               if feerate_per_kw as u64 > upper_limit {
+                       return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
                }
                Ok(())
        }
@@ -556,65 +592,68 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        delayed_payment_basepoint: msg.delayed_payment_basepoint,
                        htlc_basepoint: msg.htlc_basepoint
                };
-               chan_keys.set_remote_channel_pubkeys(&their_pubkeys);
+               chan_keys.on_accept(&their_pubkeys, msg.to_self_delay, config.own_channel_config.our_to_self_delay);
                let mut local_config = (*config).channel_options.clone();
 
                if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
-                       return Err(ChannelError::Close("Configured with an unreasonable our_to_self_delay putting user funds at risks"));
+                       return Err(ChannelError::Close(format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks. It must be greater than {}", config.own_channel_config.our_to_self_delay, BREAKDOWN_TIMEOUT)));
                }
 
                // Check sanity of message fields:
                if msg.funding_satoshis >= MAX_FUNDING_SATOSHIS {
-                       return Err(ChannelError::Close("funding value > 2^24"));
+                       return Err(ChannelError::Close(format!("Funding must be smaller than {}. It was {}", MAX_FUNDING_SATOSHIS, msg.funding_satoshis)));
                }
                if msg.channel_reserve_satoshis > msg.funding_satoshis {
-                       return Err(ChannelError::Close("Bogus channel_reserve_satoshis"));
+                       return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must be not greater than funding_satoshis: {}", msg.channel_reserve_satoshis, msg.funding_satoshis)));
                }
-               if msg.push_msat > (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
-                       return Err(ChannelError::Close("push_msat larger than funding value"));
+               let funding_value = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000;
+               if msg.push_msat > funding_value {
+                       return Err(ChannelError::Close(format!("push_msat {} was larger than funding value {}", msg.push_msat, funding_value)));
                }
                if msg.dust_limit_satoshis > msg.funding_satoshis {
-                       return Err(ChannelError::Close("Peer never wants payout outputs?"));
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis {} was larger than funding_satoshis {}. Peer never wants payout outputs?", msg.dust_limit_satoshis, msg.funding_satoshis)));
                }
                if msg.dust_limit_satoshis > msg.channel_reserve_satoshis {
-                       return Err(ChannelError::Close("Bogus; channel reserve is less than dust limit"));
+                       return Err(ChannelError::Close(format!("Bogus; channel reserve ({}) is less than dust limit ({})", msg.channel_reserve_satoshis, msg.dust_limit_satoshis)));
                }
-               if msg.htlc_minimum_msat >= (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
-                       return Err(ChannelError::Close("Minimum htlc value is full channel value"));
+               let full_channel_value_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000;
+               if msg.htlc_minimum_msat >= full_channel_value_msat {
+                       return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat)));
                }
                Channel::<ChanSigner>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
 
-               if msg.to_self_delay > config.peer_channel_config_limits.their_to_self_delay || msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
-                       return Err(ChannelError::Close("They wanted our payments to be delayed by a needlessly long period"));
+               let max_to_self_delay = u16::min(config.peer_channel_config_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
+               if msg.to_self_delay > max_to_self_delay {
+                       return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_to_self_delay, msg.to_self_delay)));
                }
                if msg.max_accepted_htlcs < 1 {
-                       return Err(ChannelError::Close("0 max_accpted_htlcs makes for a useless channel"));
+                       return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
                }
                if msg.max_accepted_htlcs > 483 {
-                       return Err(ChannelError::Close("max_accpted_htlcs > 483"));
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than 483", msg.max_accepted_htlcs)));
                }
 
                // Now check against optional parameters as set by config...
                if msg.funding_satoshis < config.peer_channel_config_limits.min_funding_satoshis {
-                       return Err(ChannelError::Close("funding satoshis is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("Funding satoshis ({}) is less than the user specified limit ({})", msg.funding_satoshis, config.peer_channel_config_limits.min_funding_satoshis)));
                }
                if msg.htlc_minimum_msat > config.peer_channel_config_limits.max_htlc_minimum_msat {
-                       return Err(ChannelError::Close("htlc minimum msat is higher than the user specified limit"));
+                       return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat,  config.peer_channel_config_limits.max_htlc_minimum_msat)));
                }
                if msg.max_htlc_value_in_flight_msat < config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat {
-                       return Err(ChannelError::Close("max htlc value in flight msat is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat)));
                }
                if msg.channel_reserve_satoshis > config.peer_channel_config_limits.max_channel_reserve_satoshis {
-                       return Err(ChannelError::Close("channel reserve satoshis is higher than the user specified limit"));
+                       return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, config.peer_channel_config_limits.max_channel_reserve_satoshis)));
                }
                if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs {
-                       return Err(ChannelError::Close("max accepted htlcs is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs)));
                }
                if msg.dust_limit_satoshis < config.peer_channel_config_limits.min_dust_limit_satoshis {
-                       return Err(ChannelError::Close("dust limit satoshis is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.min_dust_limit_satoshis)));
                }
                if msg.dust_limit_satoshis > config.peer_channel_config_limits.max_dust_limit_satoshis {
-                       return Err(ChannelError::Close("dust limit satoshis is greater than the user specified limit"));
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.max_dust_limit_satoshis)));
                }
 
                // Convert things into internal flags and prep our state:
@@ -622,7 +661,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let their_announce = if (msg.channel_flags & 1) == 1 { true } else { false };
                if config.peer_channel_config_limits.force_announced_channel_preference {
                        if local_config.announced_channel != their_announce {
-                               return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours"));
+                               return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours".to_owned()));
                        }
                }
                // we either accept their preference or the preferences match
@@ -633,26 +672,27 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let our_dust_limit_satoshis = Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate);
                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"));
+                       return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). our_dust_limit_satoshis is ({}).", remote_channel_reserve_satoshis, our_dust_limit_satoshis)));
                }
                if msg.channel_reserve_satoshis < our_dust_limit_satoshis {
-                       return Err(ChannelError::Close("channel_reserve_satoshis too small"));
+                       return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, our_dust_limit_satoshis)));
                }
                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"));
+                       return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.dust_limit_satoshis, remote_channel_reserve_satoshis)));
                }
 
                // check if the funder's amount for the initial commitment tx is sufficient
                // for full fee payment
                let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat;
-               if funders_amount_msat < background_feerate * COMMITMENT_TX_BASE_WEIGHT {
-                       return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
+               let lower_limit = background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT;
+               if funders_amount_msat < lower_limit {
+                       return Err(ChannelError::Close(format!("Insufficient funding amount ({}) for initial commitment. Must be at least {}", funders_amount_msat, lower_limit)));
                }
 
                let to_local_msat = msg.push_msat;
-               let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT;
+               let to_remote_msat = funders_amount_msat - background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT;
                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"));
+                       return Err(ChannelError::Close("Insufficient funding amount for initial commitment".to_owned()));
                }
 
                let their_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
@@ -666,12 +706,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                None
                                        // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
                                        } else {
-                                               return Err(ChannelError::Close("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format"));
+                                               return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
                                        }
                                },
                                // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
                                &OptionalField::Absent => {
-                                       return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out"));
+                                       return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out".to_owned()));
                                }
                        }
                } else { None };
@@ -725,7 +765,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        last_block_connected: Default::default(),
                        funding_tx_confirmations: 0,
 
-                       feerate_per_kw: msg.feerate_per_kw as u64,
+                       feerate_per_kw: msg.feerate_per_kw,
                        channel_value_satoshis: msg.funding_satoshis,
                        their_dust_limit_satoshis: msg.dust_limit_satoshis,
                        our_dust_limit_satoshis: our_dust_limit_satoshis,
@@ -755,13 +795,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(chan)
        }
 
-       // Utilities to derive keys:
-
-       fn build_local_commitment_secret(&self, idx: u64) -> SecretKey {
-               let res = chan_utils::build_commitment_secret(self.local_keys.commitment_seed(), idx);
-               SecretKey::from_slice(&res).unwrap()
-       }
-
        // Utilities to build transactions:
 
        fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
@@ -803,7 +836,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<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 {
+       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u32, 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 = {
@@ -843,7 +876,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => {
                                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) {
+                                       if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
                                                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(),
@@ -855,7 +888,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        }
                                } 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) {
+                                       if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) {
                                                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(),
@@ -948,7 +981,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64);
                }
 
-               let total_fee: u64 = feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
+               let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
                let (value_to_self, value_to_remote) = if self.channel_outbound {
                        (value_to_self_msat / 1000 - total_fee as i64, value_to_remote_msat / 1000)
                } else {
@@ -1093,12 +1126,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// The result is a transaction which we can revoke ownership of (ie a "local" transaction)
        /// TODO Some magic rust shit to compile-time check this?
        fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
-               let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
+               let per_commitment_point = self.local_keys.get_per_commitment_point(commitment_number, &self.secp_ctx);
                let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
 
-               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
+               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys".to_owned()))
        }
 
        #[inline]
@@ -1109,10 +1142,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
                let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
 
-               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
+               Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys".to_owned()))
        }
 
        /// Gets the redeemscript for the funding transaction output (ie the funding transaction output
@@ -1125,7 +1158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
        /// @local is used only to convert relevant internal structures which refer to remote vs local
        /// to decide value of outputs and direction of HTLCs.
-       fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u64) -> Transaction {
+       fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction {
                chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.their_to_self_delay } else { self.our_to_self_delay }, htlc, &keys.a_delayed_payment_key, &keys.revocation_key)
        }
 
@@ -1175,7 +1208,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
                if pending_idx == std::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
+                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
                }
 
                // Now update local state:
@@ -1285,14 +1318,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        },
                                        _ => {
                                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
-                                               return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
+                                               return Err(ChannelError::Ignore(format!("Unable to find a pending HTLC which matched the given HTLC ID ({})", htlc.htlc_id)));
                                        }
                                }
                                pending_idx = idx;
                        }
                }
                if pending_idx == std::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
+                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
                }
 
                // Now update local state:
@@ -1302,13 +1335,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
                                                        debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
-                                                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
+                                                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
                                                        debug_assert!(false, "Tried to fail an HTLC that was already failed");
-                                                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
+                                                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
                                                }
                                        },
                                        _ => {}
@@ -1338,60 +1371,63 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> {
                // Check sanity of message fields:
                if !self.channel_outbound {
-                       return Err(ChannelError::Close("Got an accept_channel message from an inbound peer"));
+                       return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned()));
                }
                if self.channel_state != ChannelState::OurInitSent as u32 {
-                       return Err(ChannelError::Close("Got an accept_channel message at a strange time"));
+                       return Err(ChannelError::Close("Got an accept_channel message at a strange time".to_owned()));
                }
                if msg.dust_limit_satoshis > 21000000 * 100000000 {
-                       return Err(ChannelError::Close("Peer never wants payout outputs?"));
+                       return Err(ChannelError::Close(format!("Peer never wants payout outputs? dust_limit_satoshis was {}", msg.dust_limit_satoshis)));
                }
                if msg.channel_reserve_satoshis > self.channel_value_satoshis {
-                       return Err(ChannelError::Close("Bogus channel_reserve_satoshis"));
+                       return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must not be greater than ({})", msg.channel_reserve_satoshis, self.channel_value_satoshis)));
                }
                if msg.dust_limit_satoshis > msg.channel_reserve_satoshis {
-                       return Err(ChannelError::Close("Bogus channel_reserve and dust_limit"));
+                       return Err(ChannelError::Close(format!("Bogus channel_reserve ({}) and dust_limit ({})", msg.channel_reserve_satoshis, msg.dust_limit_satoshis)));
                }
                if msg.channel_reserve_satoshis < self.our_dust_limit_satoshis {
-                       return Err(ChannelError::Close("Peer never wants payout outputs?"));
+                       return Err(ChannelError::Close(format!("Peer never wants payout outputs? channel_reserve_satoshis was ({}). our_dust_limit is ({})", msg.channel_reserve_satoshis, self.our_dust_limit_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"));
+               let remote_reserve = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+               if msg.dust_limit_satoshis > remote_reserve {
+                       return Err(ChannelError::Close(format!("Dust limit ({}) is bigger than our channel reserve ({})", msg.dust_limit_satoshis, remote_reserve)));
                }
-               if msg.htlc_minimum_msat >= (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000 {
-                       return Err(ChannelError::Close("Minimum htlc value is full channel value"));
+               let full_channel_value_msat = (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000;
+               if msg.htlc_minimum_msat >= full_channel_value_msat {
+                       return Err(ChannelError::Close(format!("Minimum htlc value ({}) is full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat)));
                }
-               if msg.to_self_delay > config.peer_channel_config_limits.their_to_self_delay || msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
-                       return Err(ChannelError::Close("They wanted our payments to be delayed by a needlessly long period"));
+               let max_delay_acceptable = u16::min(config.peer_channel_config_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
+               if msg.to_self_delay > max_delay_acceptable {
+                       return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_delay_acceptable, msg.to_self_delay)));
                }
                if msg.max_accepted_htlcs < 1 {
-                       return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel"));
+                       return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
                }
                if msg.max_accepted_htlcs > 483 {
-                       return Err(ChannelError::Close("max_accepted_htlcs > 483"));
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than 483", msg.max_accepted_htlcs)));
                }
 
                // Now check against optional parameters as set by config...
                if msg.htlc_minimum_msat > config.peer_channel_config_limits.max_htlc_minimum_msat {
-                       return Err(ChannelError::Close("htlc minimum msat is higher than the user specified limit"));
+                       return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat, config.peer_channel_config_limits.max_htlc_minimum_msat)));
                }
                if msg.max_htlc_value_in_flight_msat < config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat {
-                       return Err(ChannelError::Close("max htlc value in flight msat is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, config.peer_channel_config_limits.min_max_htlc_value_in_flight_msat)));
                }
                if msg.channel_reserve_satoshis > config.peer_channel_config_limits.max_channel_reserve_satoshis {
-                       return Err(ChannelError::Close("channel reserve satoshis is higher than the user specified limit"));
+                       return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, config.peer_channel_config_limits.max_channel_reserve_satoshis)));
                }
                if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs {
-                       return Err(ChannelError::Close("max accepted htlcs is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs)));
                }
                if msg.dust_limit_satoshis < config.peer_channel_config_limits.min_dust_limit_satoshis {
-                       return Err(ChannelError::Close("dust limit satoshis is less than the user specified limit"));
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.min_dust_limit_satoshis)));
                }
                if msg.dust_limit_satoshis > config.peer_channel_config_limits.max_dust_limit_satoshis {
-                       return Err(ChannelError::Close("dust limit satoshis is greater than the user specified limit"));
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.max_dust_limit_satoshis)));
                }
                if msg.minimum_depth > config.peer_channel_config_limits.max_minimum_depth {
-                       return Err(ChannelError::Close("We consider the minimum depth to be unreasonably large"));
+                       return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth)));
                }
 
                let their_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
@@ -1405,12 +1441,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                None
                                        // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
                                        } else {
-                                               return Err(ChannelError::Close("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format"));
+                                               return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. scriptpubkey: ({})", script.to_bytes().to_hex())));
                                        }
                                },
                                // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
                                &OptionalField::Absent => {
-                                       return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out"));
+                                       return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out".to_owned()));
                                }
                        }
                } else { None };
@@ -1431,7 +1467,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        htlc_basepoint: msg.htlc_basepoint
                };
 
-               self.local_keys.set_remote_channel_pubkeys(&their_pubkeys);
+               self.local_keys.on_accept(&their_pubkeys, msg.to_self_delay, self.our_to_self_delay);
                self.their_pubkeys = Some(their_pubkeys);
 
                self.their_cur_commitment_point = Some(msg.first_per_commitment_point);
@@ -1451,14 +1487,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // They sign the "local" commitment transaction...
                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");
+               secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
 
                let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, 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, 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;
+               let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+               let remote_signature = self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
+                               .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
 
                // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
                Ok((remote_initial_commitment_tx, localtx, remote_signature))
@@ -1470,13 +1507,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        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?"));
+                       return Err(ChannelError::Close("Received funding_created for an outbound channel?".to_owned()));
                }
                if self.channel_state != (ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32) {
                        // BOLT 2 says that if we disconnect before we send funding_signed we SHOULD NOT
                        // remember the channel, so it's safe to just send an error_message here and drop the
                        // channel.
-                       return Err(ChannelError::Close("Received funding_created after we got the channel!"));
+                       return Err(ChannelError::Close("Received funding_created after we got the channel!".to_owned()));
                }
                if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
@@ -1533,10 +1570,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// If this call is successful, broadcast the funding transaction (and not before!)
        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?"));
+                       return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
                }
                if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 {
-                       return Err(ChannelError::Close("Received funding_signed in strange state!"));
+                       return Err(ChannelError::Close("Received funding_signed in strange state!".to_owned()));
                }
                if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
@@ -1557,7 +1594,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
                if let Err(_) = self.secp_ctx.verify(&local_sighash, &msg.signature, their_funding_pubkey) {
-                       return Err(ChannelError::Close("Invalid funding_signed signature from peer"));
+                       return Err(ChannelError::Close("Invalid funding_signed signature from peer".to_owned()));
                }
 
                let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
@@ -1594,7 +1631,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent funding_locked when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent funding_locked when we needed a channel_reestablish".to_owned()));
                }
 
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
@@ -1612,12 +1649,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                (self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) ==
                                                      (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32)) {
                        if self.their_cur_commitment_point != Some(msg.next_per_commitment_point) {
-                               return Err(ChannelError::Close("Peer sent a reconnect funding_locked with a different point"));
+                               return Err(ChannelError::Close("Peer sent a reconnect funding_locked with a different point".to_owned()));
                        }
                        // They probably disconnected/reconnected and re-sent the funding_locked, which is required
                        return Ok(());
                } else {
-                       return Err(ChannelError::Close("Peer sent a funding_locked at a strange time"));
+                       return Err(ChannelError::Close("Peer sent a funding_locked at a strange time".to_owned()));
                }
 
                self.their_prev_commitment_point = self.their_cur_commitment_point;
@@ -1663,30 +1700,105 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
        }
 
-       pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError> {
-               if (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
+       // Get the fee cost of a commitment tx with a given number of HTLC outputs.
+       // Note that num_htlcs should not include dust HTLCs.
+       fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+               // Note that we need to divide before multiplying to round properly,
+               // since the lowest denomination of bitcoin on-chain is the satoshi.
+               (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw as u64 / 1000 * 1000
+       }
+
+       // Get the commitment tx fee for the local (i.e our) next commitment transaction
+       // based on the number of pending HTLCs that are on track to be in our next
+       // commitment tx. `addl_htcs` is an optional parameter allowing the caller
+       // to add a number of additional HTLCs to the calculation. Note that dust
+       // HTLCs are excluded.
+       fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+               assert!(self.channel_outbound);
+
+               let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+               for ref htlc in self.pending_outbound_htlcs.iter() {
+                       if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+                               continue
+                       }
+                       match htlc.state {
+                               OutboundHTLCState::Committed => their_acked_htlcs += 1,
+                               OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+                               OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+                               _ => {},
+                       }
+               }
+
+               for htlc in self.holding_cell_htlc_updates.iter() {
+                       match htlc {
+                               &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+                               _ => {},
+                       }
+               }
+
+               self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+       }
+
+       // Get the commitment tx fee for the remote's next commitment transaction
+       // based on the number of pending HTLCs that are on track to be in their
+       // next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+       // to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+       // are excluded.
+       fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+               assert!(!self.channel_outbound);
+
+               // When calculating the set of HTLCs which will be included in their next
+               // commitment_signed, all inbound HTLCs are included (as all states imply it will be
+               // included) and only committed outbound HTLCs, see below.
+               let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+               for ref htlc in self.pending_outbound_htlcs.iter() {
+                       if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+                               continue
+                       }
+                       // We only include outbound HTLCs if it will not be included in their next
+                       // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+                       match htlc.state {
+                               OutboundHTLCState::Committed => their_acked_htlcs += 1,
+                               OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+                               _ => {},
+                       }
+               }
+
+               self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+       }
+
+       pub fn update_add_htlc<F, L: Deref>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F, logger: &L) -> Result<(), ChannelError>
+       where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus, L::Target: Logger {
+               // We can't accept HTLCs sent after we've sent a shutdown.
+               let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+               if local_sent_shutdown {
+                       pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+               }
+               // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+               let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+               if remote_sent_shutdown {
+                       return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent update_add_htlc when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent update_add_htlc when we needed a channel_reestablish".to_owned()));
                }
                if msg.amount_msat > self.channel_value_satoshis * 1000 {
-                       return Err(ChannelError::Close("Remote side tried to send more than the total value of the channel"));
+                       return Err(ChannelError::Close("Remote side tried to send more than the total value of the channel".to_owned()));
                }
                if msg.amount_msat == 0 {
-                       return Err(ChannelError::Close("Remote side tried to send a 0-msat HTLC"));
+                       return Err(ChannelError::Close("Remote side tried to send a 0-msat HTLC".to_owned()));
                }
                if msg.amount_msat < self.our_htlc_minimum_msat {
-                       return Err(ChannelError::Close("Remote side tried to send less than our minimum HTLC value"));
+                       return Err(ChannelError::Close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.our_htlc_minimum_msat, msg.amount_msat)));
                }
 
                let (inbound_htlc_count, htlc_inbound_value_msat) = self.get_inbound_pending_htlc_stats();
                if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 {
-                       return Err(ChannelError::Close("Remote tried to push more than our max accepted HTLCs"));
+                       return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", OUR_MAX_HTLCS)));
                }
-               // Check our_max_htlc_value_in_flight_msat
-               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"));
+               let our_max_htlc_value_in_flight_msat = Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis);
+               if htlc_inbound_value_msat + msg.amount_msat > our_max_htlc_value_in_flight_msat {
+                       return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", our_max_htlc_value_in_flight_msat)));
                }
                // 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
@@ -1708,18 +1820,65 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                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_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"));
+
+               let pending_value_to_self_msat =
+                       self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+               let pending_remote_value_msat =
+                       self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+               if pending_remote_value_msat < msg.amount_msat {
+                       return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds".to_owned()));
+               }
+
+               // Check that the remote can afford to pay for this HTLC on-chain at the current
+               // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+               let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+                       // +1 for this HTLC.
+                       self.next_remote_commit_tx_fee_msat(1)
+               };
+               if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+                       return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
+               };
+
+               let chan_reserve_msat =
+                       Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+               if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+                       return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
+               }
+
+               if !self.channel_outbound {
+                       // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+                       // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+                       // only on the sender's.
+                       // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+                       // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+                       // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+                       // being sensitive to fee spikes.
+                       let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+                       if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+                               // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+                               // the HTLC, i.e. its status is already set to failing.
+                               log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation");
+                               pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+                       }
+               } else {
+                       // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+                       // +1 for this HTLC.
+                       let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+                       if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+                               return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value".to_owned()));
+                       }
                }
+
                if self.next_remote_htlc_id != msg.htlc_id {
-                       return Err(ChannelError::Close("Remote skipped HTLC ID"));
+                       return Err(ChannelError::Close(format!("Remote skipped HTLC ID (skipped ID: {})", self.next_remote_htlc_id)));
                }
                if msg.cltv_expiry >= 500000000 {
-                       return Err(ChannelError::Close("Remote provided CLTV expiry in seconds instead of block height"));
+                       return Err(ChannelError::Close("Remote provided CLTV expiry in seconds instead of block height".to_owned()));
                }
 
                if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
-                       if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+                       if let PendingHTLCStatus::Forward(_) = pending_forward_status {
                                panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
                        }
                }
@@ -1731,7 +1890,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        amount_msat: msg.amount_msat,
                        payment_hash: msg.payment_hash,
                        cltv_expiry: msg.cltv_expiry,
-                       state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+                       state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
                });
                Ok(())
        }
@@ -1745,30 +1904,30 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        None => {},
                                        Some(payment_hash) =>
                                                if payment_hash != htlc.payment_hash {
-                                                       return Err(ChannelError::Close("Remote tried to fulfill HTLC with an incorrect preimage"));
+                                                       return Err(ChannelError::Close(format!("Remote tried to fulfill HTLC ({}) with an incorrect preimage", htlc_id)));
                                                }
                                };
                                match htlc.state {
                                        OutboundHTLCState::LocalAnnounced(_) =>
-                                               return Err(ChannelError::Close("Remote tried to fulfill/fail HTLC before it had been committed")),
+                                               return Err(ChannelError::Close(format!("Remote tried to fulfill/fail HTLC ({}) before it had been committed", htlc_id))),
                                        OutboundHTLCState::Committed => {
                                                htlc.state = OutboundHTLCState::RemoteRemoved(fail_reason);
                                        },
                                        OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) | OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) | OutboundHTLCState::RemoteRemoved(_) =>
-                                               return Err(ChannelError::Close("Remote tried to fulfill/fail HTLC that they'd already fulfilled/failed")),
+                                               return Err(ChannelError::Close(format!("Remote tried to fulfill/fail HTLC ({}) that they'd already fulfilled/failed", htlc_id))),
                                }
                                return Ok(&htlc.source);
                        }
                }
-               Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find"))
+               Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned()))
        }
 
        pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<HTLCSource, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state"));
+                       return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent update_fulfill_htlc when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent update_fulfill_htlc when we needed a channel_reestablish".to_owned()));
                }
 
                let payment_hash = PaymentHash(Sha256::hash(&msg.payment_preimage.0[..]).into_inner());
@@ -1777,10 +1936,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(ChannelError::Close("Got fail HTLC message when channel was not in an operational state"));
+                       return Err(ChannelError::Close("Got fail HTLC message when channel was not in an operational state".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent update_fail_htlc when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent update_fail_htlc when we needed a channel_reestablish".to_owned()));
                }
 
                self.mark_outbound_htlc_removed(msg.htlc_id, None, Some(fail_reason))?;
@@ -1789,10 +1948,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        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"));
+                       return Err(ChannelError::Close("Got fail malformed HTLC message when channel was not in an operational state".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent update_fail_malformed_htlc when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent update_fail_malformed_htlc when we needed a channel_reestablish".to_owned()));
                }
 
                self.mark_outbound_htlc_removed(msg.htlc_id, None, Some(fail_reason))?;
@@ -1804,13 +1963,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                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")));
+                       return Err((None, ChannelError::Close("Got commitment signed message when channel was not in an operational state".to_owned())));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err((None, ChannelError::Close("Peer sent commitment_signed when we needed a channel_reestablish")));
+                       return Err((None, ChannelError::Close("Peer sent commitment_signed when we needed a channel_reestablish".to_owned())));
                }
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK == BOTH_SIDES_SHUTDOWN_MASK && self.last_sent_closing_fee.is_some() {
-                       return Err((None, ChannelError::Close("Peer sent commitment_signed after we'd started exchanging closing_signeds")));
+                       return Err((None, ChannelError::Close("Peer sent commitment_signed after we'd started exchanging closing_signeds".to_owned())));
                }
 
                let funding_script = self.get_funding_redeemscript();
@@ -1834,22 +1993,22 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                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!(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")));
+                       return Err((None, ChannelError::Close("Invalid commitment tx signature from peer".to_owned())));
                }
 
                //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
                if update_fee {
                        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;
+                       let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
 
                        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")));
+                               return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned())));
                        }
                }
 
                if msg.htlc_signatures.len() != local_commitment_tx.1 {
-                       return Err((None, ChannelError::Close("Got wrong number of HTLC signatures from remote")));
+                       return Err((None, ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), local_commitment_tx.1))));
                }
 
                // TODO: Merge these two, sadly they are currently both required to be passed separately to
@@ -1863,7 +2022,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                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!(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")));
+                                       return Err((None, ChannelError::Close("Invalid HTLC tx signature from peer".to_owned())));
                                }
                                htlcs_without_source.push((htlc.clone(), Some(msg.htlc_signatures[idx])));
                                htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source));
@@ -1873,8 +2032,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
-               let per_commitment_secret = chan_utils::build_commitment_secret(self.local_keys.commitment_seed(), self.cur_local_commitment_transaction_number + 1);
+               let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number - 1, &self.secp_ctx);
+               let per_commitment_secret = self.local_keys.release_commitment_secret(self.cur_local_commitment_transaction_number + 1);
 
                // Update state now that we've passed all the can-fail calls...
                let mut need_our_commitment = false;
@@ -1943,7 +2102,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                        // TODO: Call maybe_propose_first_closing_signed on restoration (or call it here and
                        // re-send the message on restoration)
-                       return Err((Some(monitor_update), ChannelError::Ignore("Previous monitor update failure prevented generation of RAA")));
+                       return Err((Some(monitor_update), ChannelError::Ignore("Previous monitor update failure prevented generation of RAA".to_owned())));
                }
 
                let (our_commitment_signed, closing_signed) = if need_our_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 {
@@ -1969,7 +2128,7 @@ 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<L: Deref>(&mut self, logger: &L) -> Result<Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
+       fn free_holding_cell_htlcs<L: Deref>(&mut self, logger: &L) -> Result<(Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>), 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!(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 { "" });
@@ -1984,110 +2143,94 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        let mut update_add_htlcs = Vec::with_capacity(htlc_updates.len());
                        let mut update_fulfill_htlcs = Vec::with_capacity(htlc_updates.len());
                        let mut update_fail_htlcs = Vec::with_capacity(htlc_updates.len());
-                       let mut err = None;
+                       let mut htlcs_to_fail = Vec::new();
                        for htlc_update in htlc_updates.drain(..) {
                                // Note that this *can* fail, though it should be due to rather-rare conditions on
                                // fee races with adding too many outputs which push our total payments just over
                                // the limit. In case it's less rare than I anticipate, we may want to revisit
                                // handling this case better and maybe fulfilling some of the HTLCs while attempting
                                // to rebalance channels.
-                               if err.is_some() { // We're back to AwaitingRemoteRevoke (or are about to fail the channel)
-                                       self.holding_cell_htlc_updates.push(htlc_update);
-                               } else {
-                                       match &htlc_update {
-                                               &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, ..} => {
-                                                       match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone()) {
-                                                               Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
-                                                               Err(e) => {
-                                                                       match e {
-                                                                               ChannelError::Ignore(ref msg) => {
-                                                                                       log_info!(logger, "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 {} resulting in a channel closure during holding_cell freeing", log_bytes!(payment_hash.0));
-                                                                               },
-                                                                       }
-                                                                       err = Some(e);
+                               match &htlc_update {
+                                       &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, ..} => {
+                                               match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone()) {
+                                                       Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
+                                                       Err(e) => {
+                                                               match e {
+                                                                       ChannelError::Ignore(ref msg) => {
+                                                                               log_info!(logger, "Failed to send HTLC with payment_hash {} due to {}", log_bytes!(payment_hash.0), msg);
+                                                                               // If we fail to send here, then this HTLC should
+                                                                               // be failed backwards. Failing to send here
+                                                                               // indicates that this HTLC may keep being put back
+                                                                               // into the holding cell without ever being
+                                                                               // successfully forwarded/failed/fulfilled, causing
+                                                                               // our counterparty to eventually close on us.
+                                                                               htlcs_to_fail.push((source.clone(), *payment_hash));
+                                                                       },
+                                                                       _ => {
+                                                                               panic!("Got a non-IgnoreError action trying to send holding cell HTLC");
+                                                                       },
                                                                }
                                                        }
-                                               },
-                                               &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
-                                                       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 {
-                                                                               monitor_update.updates.append(&mut additional_monitor_update.updates);
-                                                                       }
-                                                               },
-                                                               Err(e) => {
-                                                                       if let ChannelError::Ignore(_) = e {}
-                                                                       else {
-                                                                               panic!("Got a non-IgnoreError action trying to fulfill holding cell HTLC");
-                                                                       }
+                                               }
+                                       },
+                                       &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
+                                               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 {
+                                                                       monitor_update.updates.append(&mut additional_monitor_update.updates);
+                                                               }
+                                                       },
+                                                       Err(e) => {
+                                                               if let ChannelError::Ignore(_) = e {}
+                                                               else {
+                                                                       panic!("Got a non-IgnoreError action trying to fulfill holding cell HTLC");
                                                                }
                                                        }
-                                               },
-                                               &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
-                                                       match self.get_update_fail_htlc(htlc_id, err_packet.clone()) {
-                                                               Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()),
-                                                               Err(e) => {
-                                                                       if let ChannelError::Ignore(_) = e {}
-                                                                       else {
-                                                                               panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
-                                                                       }
+                                               }
+                                       },
+                                       &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
+                                               match self.get_update_fail_htlc(htlc_id, err_packet.clone()) {
+                                                       Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()),
+                                                       Err(e) => {
+                                                               if let ChannelError::Ignore(_) = e {}
+                                                               else {
+                                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
                                                                }
                                                        }
-                                               },
-                                       }
-                                       if err.is_some() {
-                                               self.holding_cell_htlc_updates.push(htlc_update);
-                                               if let Some(ChannelError::Ignore(_)) = err {
-                                                       // If we failed to add the HTLC, but got an Ignore error, we should
-                                                       // still send the new commitment_signed, so reset the err to None.
-                                                       err = None;
                                                }
-                                       }
+                                       },
                                }
                        }
-                       //TODO: Need to examine the type of err - if it's a fee issue or similar we may want to
-                       //fail it back the route, if it's a temporary issue we can ignore it...
-                       match err {
-                               None => {
-                                       if update_add_htlcs.is_empty() && update_fulfill_htlcs.is_empty() && update_fail_htlcs.is_empty() && self.holding_cell_update_fee.is_none() {
-                                               // This should never actually happen and indicates we got some Errs back
-                                               // from update_fulfill_htlc/update_fail_htlc, but we handle it anyway in
-                                               // case there is some strange way to hit duplicate HTLC removes.
-                                               return Ok(None);
-                                       }
-                                       let update_fee = if let Some(feerate) = self.holding_cell_update_fee {
-                                                       self.pending_update_fee = self.holding_cell_update_fee.take();
-                                                       Some(msgs::UpdateFee {
-                                                               channel_id: self.channel_id,
-                                                               feerate_per_kw: feerate as u32,
-                                                       })
-                                               } else {
-                                                       None
-                                               };
+                       if update_add_htlcs.is_empty() && update_fulfill_htlcs.is_empty() && update_fail_htlcs.is_empty() && self.holding_cell_update_fee.is_none() {
+                               return Ok((None, htlcs_to_fail));
+                       }
+                       let update_fee = if let Some(feerate) = self.holding_cell_update_fee {
+                               self.pending_update_fee = self.holding_cell_update_fee.take();
+                               Some(msgs::UpdateFee {
+                                       channel_id: self.channel_id,
+                                       feerate_per_kw: feerate as u32,
+                               })
+                       } else {
+                               None
+                       };
 
-                                       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;
-                                       monitor_update.updates.append(&mut additional_update.updates);
+                       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;
+                       monitor_update.updates.append(&mut additional_update.updates);
 
-                                       Ok(Some((msgs::CommitmentUpdate {
-                                               update_add_htlcs,
-                                               update_fulfill_htlcs,
-                                               update_fail_htlcs,
-                                               update_fail_malformed_htlcs: Vec::new(),
-                                               update_fee: update_fee,
-                                               commitment_signed,
-                                       }, monitor_update)))
-                               },
-                               Some(e) => Err(e)
-                       }
+                       Ok((Some((msgs::CommitmentUpdate {
+                               update_add_htlcs,
+                               update_fulfill_htlcs,
+                               update_fail_htlcs,
+                               update_fail_malformed_htlcs: Vec::new(),
+                               update_fee: update_fee,
+                               commitment_signed,
+                       }, monitor_update)), htlcs_to_fail))
                } else {
-                       Ok(None)
+                       Ok((None, Vec::new()))
                }
        }
 
@@ -2096,23 +2239,23 @@ 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, 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>
+       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, Vec<(HTLCSource, PaymentHash)>), 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"));
+                       return Err(ChannelError::Close("Got revoke/ACK message when channel was not in an operational state".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent revoke_and_ack when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent revoke_and_ack when we needed a channel_reestablish".to_owned()));
                }
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK == BOTH_SIDES_SHUTDOWN_MASK && self.last_sent_closing_fee.is_some() {
-                       return Err(ChannelError::Close("Peer sent revoke_and_ack after we'd started exchanging closing_signeds"));
+                       return Err(ChannelError::Close("Peer sent revoke_and_ack after we'd started exchanging closing_signeds".to_owned()));
                }
 
                if let Some(their_prev_commitment_point) = self.their_prev_commitment_point {
-                       if PublicKey::from_secret_key(&self.secp_ctx, &secp_check!(SecretKey::from_slice(&msg.per_commitment_secret), "Peer provided an invalid per_commitment_secret")) != their_prev_commitment_point {
-                               return Err(ChannelError::Close("Got a revoke commitment secret which didn't correspond to their current pubkey"));
+                       if PublicKey::from_secret_key(&self.secp_ctx, &secp_check!(SecretKey::from_slice(&msg.per_commitment_secret), "Peer provided an invalid per_commitment_secret".to_owned())) != their_prev_commitment_point {
+                               return Err(ChannelError::Close("Got a revoke commitment secret which didn't correspond to their current pubkey".to_owned()));
                        }
                }
 
@@ -2124,11 +2267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // lot of work, and there's some chance this is all a misunderstanding anyway.
                        // We have to do *something*, though, since our signer may get mad at us for otherwise
                        // jumping a remote commitment number, so best to just force-close and move on.
-                       return Err(ChannelError::Close("Received an unexpected revoke_and_ack"));
+                       return Err(ChannelError::Close("Received an unexpected revoke_and_ack".to_owned()));
                }
 
                self.commitment_secrets.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret)
-                       .map_err(|_| ChannelError::Close("Previous secrets did not match new one"))?;
+                       .map_err(|_| ChannelError::Close("Previous secrets did not match new one".to_owned()))?;
                self.latest_monitor_update_id += 1;
                let mut monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
@@ -2271,11 +2414,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                        self.monitor_pending_forwards.append(&mut to_forward_infos);
                        self.monitor_pending_failures.append(&mut revoked_htlcs);
-                       return Ok((None, Vec::new(), Vec::new(), None, monitor_update))
+                       return Ok((None, Vec::new(), Vec::new(), None, monitor_update, Vec::new()))
                }
 
                match self.free_holding_cell_htlcs(logger)? {
-                       Some((mut commitment_update, mut additional_update)) => {
+                       (Some((mut commitment_update, mut additional_update)), htlcs_to_fail) => {
                                commitment_update.update_fail_htlcs.reserve(update_fail_htlcs.len());
                                for fail_msg in update_fail_htlcs.drain(..) {
                                        commitment_update.update_fail_htlcs.push(fail_msg);
@@ -2290,9 +2433,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                self.latest_monitor_update_id = monitor_update.update_id;
                                monitor_update.updates.append(&mut additional_update.updates);
 
-                               Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, None, monitor_update))
+                               Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, None, monitor_update, htlcs_to_fail))
                        },
-                       None => {
+                       (None, htlcs_to_fail) => {
                                if require_commitment {
                                        let (commitment_signed, mut additional_update) = self.send_commitment_no_status_check(logger)?;
 
@@ -2308,9 +2451,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                update_fail_malformed_htlcs,
                                                update_fee: None,
                                                commitment_signed
-                                       }), to_forward_infos, revoked_htlcs, None, monitor_update))
+                                       }), to_forward_infos, revoked_htlcs, None, monitor_update, htlcs_to_fail))
                                } else {
-                                       Ok((None, to_forward_infos, revoked_htlcs, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update))
+                                       Ok((None, to_forward_infos, revoked_htlcs, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, htlcs_to_fail))
                                }
                        }
                }
@@ -2320,7 +2463,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Adds a pending update to this channel. See the doc for send_htlc for
        /// further details on the optionness of the return value.
        /// You MUST call send_commitment prior to any other calls on this Channel
-       fn send_update_fee(&mut self, feerate_per_kw: u64) -> Option<msgs::UpdateFee> {
+       fn send_update_fee(&mut self, feerate_per_kw: u32) -> Option<msgs::UpdateFee> {
                if !self.channel_outbound {
                        panic!("Cannot send fee from inbound channel");
                }
@@ -2341,11 +2484,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                Some(msgs::UpdateFee {
                        channel_id: self.channel_id,
-                       feerate_per_kw: feerate_per_kw as u32,
+                       feerate_per_kw: feerate_per_kw,
                })
        }
 
-       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 {
+       pub fn send_update_fee_and_commit<L: Deref>(&mut self, feerate_per_kw: u32, 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(logger)?;
@@ -2412,6 +2555,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
+                               // Note that currently on channel reestablish we assert that there are
+                               // no holding cell HTLC update_adds, so if in the future we stop
+                               // dropping added HTLCs here and failing them backwards, then there will
+                               // need to be corresponding changes made in the Channel's re-establish
+                               // logic.
                                &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, .. } => {
                                        outbound_drops.push((source.clone(), payment_hash.clone()));
                                        false
@@ -2459,8 +2607,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let funding_locked = if self.monitor_pending_funding_locked {
                        assert!(!self.channel_outbound, "Funding transaction broadcast without FundingBroadcastSafe!");
                        self.monitor_pending_funding_locked = false;
-                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
-                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
+                       let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
@@ -2500,20 +2647,20 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                where F::Target: FeeEstimator
        {
                if self.channel_outbound {
-                       return Err(ChannelError::Close("Non-funding remote tried to update channel fee"));
+                       return Err(ChannelError::Close("Non-funding remote tried to update channel fee".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned()));
                }
                Channel::<ChanSigner>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
-               self.pending_update_fee = Some(msg.feerate_per_kw as u64);
+               self.pending_update_fee = Some(msg.feerate_per_kw);
                self.update_time_counter += 1;
                Ok(())
        }
 
        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);
+               let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
+               let per_commitment_secret = self.local_keys.release_commitment_secret(self.cur_local_commitment_transaction_number + 2);
                msgs::RevokeAndACK {
                        channel_id: self.channel_id,
                        per_commitment_secret,
@@ -2585,23 +2732,26 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // 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
                        // just close here instead of trying to recover.
-                       return Err(ChannelError::Close("Peer sent a loose channel_reestablish not after reconnect"));
+                       return Err(ChannelError::Close("Peer sent a loose channel_reestablish not after reconnect".to_owned()));
                }
 
                if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER ||
                        msg.next_local_commitment_number == 0 {
-                       return Err(ChannelError::Close("Peer sent a garbage channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent a garbage channel_reestablish".to_owned()));
                }
 
                if msg.next_remote_commitment_number > 0 {
                        match msg.data_loss_protect {
                                OptionalField::Present(ref data_loss) => {
-                                       if chan_utils::build_commitment_secret(self.local_keys.commitment_seed(), INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1) != data_loss.your_last_per_commitment_secret {
-                                               return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided"));
+                                       let expected_point = self.local_keys.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.secp_ctx);
+                                       let given_secret = SecretKey::from_slice(&data_loss.your_last_per_commitment_secret)
+                                               .map_err(|_| ChannelError::Close("Peer sent a garbage channel_reestablish with unparseable secret key".to_owned()))?;
+                                       if expected_point != PublicKey::from_secret_key(&self.secp_ctx, &given_secret) {
+                                               return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided".to_owned()));
                                        }
                                        if msg.next_remote_commitment_number > INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
                                                return Err(ChannelError::CloseDelayBroadcast(
-                                                       "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting"
+                                                       "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting".to_owned()
                                                ));
                                        }
                                },
@@ -2625,15 +2775,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        if self.channel_state & (ChannelState::OurFundingLocked as u32) == 0 ||
                                        self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 {
                                if msg.next_remote_commitment_number != 0 {
-                                       return Err(ChannelError::Close("Peer claimed they saw a revoke_and_ack but we haven't sent funding_locked yet"));
+                                       return Err(ChannelError::Close("Peer claimed they saw a revoke_and_ack but we haven't sent funding_locked yet".to_owned()));
                                }
                                // Short circuit the whole handler as there is nothing we can resend them
                                return Ok((None, None, None, None, RAACommitmentOrder::CommitmentFirst, shutdown_msg));
                        }
 
                        // We have OurFundingLocked set!
-                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
-                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
+                       let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        return Ok((Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
@@ -2652,7 +2801,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                Some(self.get_last_revoke_and_ack())
                        }
                } else {
-                       return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old local commitment transaction"));
+                       return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old local commitment transaction".to_owned()));
                };
 
                // We increment cur_remote_commitment_transaction_number only upon receipt of
@@ -2663,8 +2812,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let resend_funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number == 1 {
                        // We should never have to worry about MonitorUpdateFailed resending FundingLocked
-                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
-                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
+                       let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
                                channel_id: self.channel_id(),
                                next_per_commitment_point: next_per_commitment_point,
@@ -2679,6 +2827,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
 
                        if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) == 0 {
+                               // Note that if in the future we no longer drop holding cell update_adds on peer
+                               // disconnect, this logic will need to be updated.
+                               for htlc_update in self.holding_cell_htlc_updates.iter() {
+                                       if let &HTLCUpdateAwaitingACK::AddHTLC { .. } = htlc_update {
+                                               debug_assert!(false, "There shouldn't be any add-HTLCs in the holding cell now because they should have been dropped on peer disconnect. Panic here because said HTLCs won't be handled correctly.");
+                                       }
+                               }
+
                                // We're up-to-date and not waiting on a remote revoke (if we are our
                                // 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
@@ -2686,8 +2842,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                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)),
-                                       Ok(None) => return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), shutdown_msg)),
+                                       Ok((Some((commitment_update, monitor_update)), htlcs_to_fail)) => {
+                                               // If in the future we no longer drop holding cell update_adds on peer
+                                               // disconnect, we may be handed some HTLCs to fail backwards here.
+                                               assert!(htlcs_to_fail.is_empty());
+                                               return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), shutdown_msg));
+                                       },
+                                       Ok((None, htlcs_to_fail)) => {
+                                               // If in the future we no longer drop holding cell update_adds on peer
+                                               // disconnect, we may be handed some HTLCs to fail backwards here.
+                                               assert!(htlcs_to_fail.is_empty());
+                                               return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), shutdown_msg));
+                                       },
                                }
                        } else {
                                return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), shutdown_msg));
@@ -2706,7 +2872,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        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"));
+                       return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction".to_owned()));
                }
        }
 
@@ -2724,7 +2890,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        proposed_feerate = self.feerate_per_kw;
                }
                let tx_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
-               let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000;
+               let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
 
                let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                let our_sig = self.local_keys
@@ -2744,17 +2910,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                where F::Target: FeeEstimator
        {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned()));
                }
                if self.channel_state < ChannelState::FundingSent as u32 {
                        // Spec says we should fail the connection, not the channel, but that's nonsense, there
                        // are plenty of reasons you may want to fail a channel pre-funding, and spec says you
                        // can do that via error message without getting a connection fail anyway...
-                       return Err(ChannelError::Close("Peer sent shutdown pre-funding generation"));
+                       return Err(ChannelError::Close("Peer sent shutdown pre-funding generation".to_owned()));
                }
                for htlc in self.pending_inbound_htlcs.iter() {
                        if let InboundHTLCState::RemoteAnnounced(_) = htlc.state {
-                               return Err(ChannelError::Close("Got shutdown with remote pending HTLCs"));
+                               return Err(ChannelError::Close("Got shutdown with remote pending HTLCs".to_owned()));
                        }
                }
                assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
@@ -2762,17 +2928,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // BOLT 2 says we must only send a scriptpubkey of certain standard forms, which are up to
                // 34 bytes in length, so don't let the remote peer feed us some super fee-heavy script.
                if self.channel_outbound && msg.scriptpubkey.len() > 34 {
-                       return Err(ChannelError::Close("Got shutdown_scriptpubkey of absurd length from remote peer"));
+                       return Err(ChannelError::Close(format!("Got shutdown_scriptpubkey ({}) of absurd length from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
                }
 
                //Check shutdown_scriptpubkey form as BOLT says we must
                if !msg.scriptpubkey.is_p2pkh() && !msg.scriptpubkey.is_p2sh() && !msg.scriptpubkey.is_v0_p2wpkh() && !msg.scriptpubkey.is_v0_p2wsh() {
-                       return Err(ChannelError::Close("Got a nonstandard scriptpubkey from remote peer"));
+                       return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
                }
 
                if self.their_shutdown_scriptpubkey.is_some() {
                        if Some(&msg.scriptpubkey) != self.their_shutdown_scriptpubkey.as_ref() {
-                               return Err(ChannelError::Close("Got shutdown request with a scriptpubkey which did not match their previous scriptpubkey"));
+                               return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", msg.scriptpubkey.to_bytes().to_hex())));
                        }
                } else {
                        self.their_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone());
@@ -2842,22 +3008,22 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                where F::Target: FeeEstimator
        {
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK {
-                       return Err(ChannelError::Close("Remote end sent us a closing_signed before both sides provided a shutdown"));
+                       return Err(ChannelError::Close("Remote end sent us a closing_signed before both sides provided a shutdown".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
-                       return Err(ChannelError::Close("Peer sent closing_signed when we needed a channel_reestablish"));
+                       return Err(ChannelError::Close("Peer sent closing_signed when we needed a channel_reestablish".to_owned()));
                }
                if !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() {
-                       return Err(ChannelError::Close("Remote end sent us a closing_signed while there were still pending HTLCs"));
+                       return Err(ChannelError::Close("Remote end sent us a closing_signed while there were still pending HTLCs".to_owned()));
                }
                if msg.fee_satoshis > 21000000 * 10000000 { //this is required to stop potential overflow in build_closing_transaction
-                       return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee"));
+                       return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned()));
                }
 
                let funding_redeemscript = self.get_funding_redeemscript();
                let (mut closing_tx, used_total_fee) = self.build_closing_transaction(msg.fee_satoshis, false);
                if used_total_fee != msg.fee_satoshis {
-                       return Err(ChannelError::Close("Remote sent us a closing_signed with a fee greater than the value they can claim"));
+                       return Err(ChannelError::Close(format!("Remote sent us a closing_signed with a fee greater than the value they can claim. Fee in message: {}", msg.fee_satoshis)));
                }
                let mut sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
 
@@ -2870,7 +3036,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                // limits, so check for that case by re-checking the signature here.
                                closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0;
                                sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
-                               secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, self.their_funding_pubkey()), "Invalid closing tx signature from peer");
+                               secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, self.their_funding_pubkey()), "Invalid closing tx signature from peer".to_owned());
                        },
                };
 
@@ -2886,10 +3052,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                macro_rules! propose_new_feerate {
                        ($new_feerate: expr) => {
                                let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
-                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false);
+                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * closing_tx_max_weight / 1000, false);
                                let our_sig = self.local_keys
                                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
-                                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?;
+                                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
                                self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, our_sig.clone()));
                                return Ok((Some(msgs::ClosingSigned {
                                        channel_id: self.channel_id,
@@ -2899,23 +3065,23 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight() as u64;
+               let proposed_sat_per_kw = msg.fee_satoshis  * 1000 / closing_tx.get_weight() as u64;
                if self.channel_outbound {
                        let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
-                       if proposed_sat_per_kw > our_max_feerate {
+                       if (proposed_sat_per_kw as u32) > our_max_feerate {
                                if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
                                        if our_max_feerate <= last_feerate {
-                                               return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate"));
+                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something higher ({}) than our Normal feerate ({})", last_feerate, our_max_feerate)));
                                        }
                                }
                                propose_new_feerate!(our_max_feerate);
                        }
                } else {
                        let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-                       if proposed_sat_per_kw < our_min_feerate {
+                       if (proposed_sat_per_kw as u32) < our_min_feerate {
                                if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
                                        if our_min_feerate >= last_feerate {
-                                               return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate"));
+                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something lower ({}) than our Background feerate ({}).", last_feerate, our_min_feerate)));
                                        }
                                }
                                propose_new_feerate!(our_min_feerate);
@@ -2924,7 +3090,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let our_sig = self.local_keys
                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
-                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?;
+                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
                self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
 
                self.channel_state = ChannelState::ShutdownComplete as u32;
@@ -2980,6 +3146,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.our_htlc_minimum_msat
        }
 
+       /// Allowed in any state (including after shutdown)
+       pub fn get_announced_htlc_max_msat(&self) -> u64 {
+               return cmp::min(
+                       // Upper bound by capacity. We make it a bit less than full capacity to prevent attempts
+                       // to use full capacity. This is an effort to reduce routing failures, because in many cases
+                       // channel might have been used to route very small values (either by honest users or as DoS).
+                       self.channel_value_satoshis * 9 / 10,
+
+                       Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis)
+               );
+       }
+
        /// Allowed in any state (including after shutdown)
        pub fn get_their_htlc_minimum_msat(&self) -> u64 {
                self.our_htlc_minimum_msat
@@ -2994,7 +3172,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        #[cfg(test)]
-       pub fn get_feerate(&self) -> u64 {
+       pub fn get_feerate(&self) -> u32 {
                self.feerate_per_kw
        }
 
@@ -3036,6 +3214,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                res
                        },
                        their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+                       their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
                }
        }
 
@@ -3065,15 +3244,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // output value back into a transaction with the regular channel output:
 
                // the fee cost of the HTLC-Success/HTLC-Timeout transaction:
-               let mut res = self.feerate_per_kw * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000;
+               let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000;
 
                if self.channel_outbound {
                        // + the marginal fee increase cost to us in the commitment transaction:
-                       res += self.feerate_per_kw * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000;
+                       res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000;
                }
 
                // + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output:
-               res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000;
+               res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000;
 
                res as u32
        }
@@ -3152,7 +3331,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
+       pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
                let mut timed_out_htlcs = Vec::new();
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
@@ -3203,6 +3382,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                }
                                                        }
                                                }
+                                               if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff {
+                                                       panic!("Block was bogus - either height 16 million or had > 16 million transactions");
+                                               }
+                                               assert!(txo_idx <= 0xffff); // txo_idx is a (u16 as usize), so this is just listed here for completeness
                                                self.funding_tx_confirmations = 1;
                                                self.short_channel_id = Some(((height as u64)          << (5*8)) |
                                                                             ((*index_in_block as u64) << (2*8)) |
@@ -3245,8 +3428,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        //a protocol oversight, but I assume I'm just missing something.
                                        if need_commitment_update {
                                                if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                                                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
-                                                       let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
+                                                       let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                                                        return Ok((Some(msgs::FundingLocked {
                                                                channel_id: self.channel_id,
                                                                next_per_commitment_point: next_per_commitment_point,
@@ -3285,9 +3467,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        // Methods to get unprompted messages to send to the remote end (or where we already returned
        // something in the handler for the message that prompted this message):
 
-       pub fn get_open_channel<F: Deref>(&self, chain_hash: BlockHash, fee_estimator: &F) -> msgs::OpenChannel
-               where F::Target: FeeEstimator
-       {
+       pub fn get_open_channel(&self, chain_hash: BlockHash) -> msgs::OpenChannel {
                if !self.channel_outbound {
                        panic!("Tried to open a channel for an inbound channel?");
                }
@@ -3299,7 +3479,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        panic!("Tried to send an open_channel for a channel that has already advanced");
                }
 
-               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
+               let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                let local_keys = self.local_keys.pubkeys();
 
                msgs::OpenChannel {
@@ -3311,15 +3491,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        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_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,
+                       feerate_per_kw: self.feerate_per_kw as u32,
                        to_self_delay: self.our_to_self_delay,
                        max_accepted_htlcs: OUR_MAX_HTLCS,
                        funding_pubkey: local_keys.funding_pubkey,
                        revocation_basepoint: local_keys.revocation_basepoint,
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
-                       first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
+                       htlc_basepoint: local_keys.htlc_basepoint,
+                       first_per_commitment_point,
                        channel_flags: if self.config.announced_channel {1} else {0},
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
@@ -3336,7 +3516,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        panic!("Tried to send an accept_channel for a channel that has already advanced");
                }
 
-               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
+               let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
                let local_keys = self.local_keys.pubkeys();
 
                msgs::AcceptChannel {
@@ -3352,8 +3532,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        revocation_basepoint: local_keys.revocation_basepoint,
                        payment_point: local_keys.payment_point,
                        delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
-                       first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
+                       htlc_basepoint: local_keys.htlc_basepoint,
+                       first_per_commitment_point,
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
        }
@@ -3362,8 +3542,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        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, 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)
+               let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+               Ok(self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
+                               .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
        }
 
        /// Updates channel state with knowledge of the funding transaction's txid/index, and generates
@@ -3421,13 +3602,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// https://github.com/lightningnetwork/lightning-rfc/issues/468
        pub fn get_channel_announcement(&self, our_node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
                if !self.config.announced_channel {
-                       return Err(ChannelError::Ignore("Channel is not available for public announcements"));
+                       return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
                }
                if self.channel_state & (ChannelState::ChannelFunded as u32) == 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement until the channel funding has been locked"));
+                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement until the channel funding has been locked".to_owned()));
                }
                if (self.channel_state & (ChannelState::LocalShutdownSent as u32 | ChannelState::ShutdownComplete as u32)) != 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement once the channel is closing"));
+                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement once the channel is closing".to_owned()));
                }
 
                let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
@@ -3444,7 +3625,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                };
 
                let sig = self.local_keys.sign_channel_announcement(&msg, &self.secp_ctx)
-                       .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement"))?;
+                       .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?;
 
                Ok((msg, sig))
        }
@@ -3512,19 +3693,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// If an Err is returned, it's a ChannelError::Ignore!
        pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down"));
+                       return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned()));
                }
-
-               if amount_msat > self.channel_value_satoshis * 1000 {
-                       return Err(ChannelError::Ignore("Cannot send more than the total value of the channel"));
+               let channel_total_msat = self.channel_value_satoshis * 1000;
+               if amount_msat > channel_total_msat {
+                       return Err(ChannelError::Ignore(format!("Cannot send amount {}, because it is more than the total value of the channel {}", amount_msat, channel_total_msat)));
                }
 
                if amount_msat == 0 {
-                       return Err(ChannelError::Ignore("Cannot send 0-msat HTLC"));
+                       return Err(ChannelError::Ignore("Cannot send 0-msat HTLC".to_owned()));
                }
 
                if amount_msat < self.their_htlc_minimum_msat {
-                       return Err(ChannelError::Ignore("Cannot send less than their minimum HTLC value"));
+                       return Err(ChannelError::Ignore(format!("Cannot send less than their minimum HTLC value ({})", self.their_htlc_minimum_msat)));
                }
 
                if (self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 {
@@ -3534,41 +3715,68 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        // disconnected during the time the previous hop was doing the commitment dance we may
                        // end up getting here after the forwarding delay. In any case, returning an
                        // IgnoreError will get ChannelManager to do the right thing and fail backwards now.
-                       return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected/frozen for channel monitor update"));
+                       return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected/frozen for channel monitor update".to_owned()));
                }
 
                let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats();
                if outbound_htlc_count + 1 > self.their_max_accepted_htlcs as u32 {
-                       return Err(ChannelError::Ignore("Cannot push more than their max accepted HTLCs"));
+                       return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.their_max_accepted_htlcs)));
                }
                // Check their_max_htlc_value_in_flight_msat
                if htlc_outbound_value_msat + amount_msat > self.their_max_htlc_value_in_flight_msat {
-                       return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
+                       return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.their_max_htlc_value_in_flight_msat)));
+               }
+
+               if !self.channel_outbound {
+                       // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+                       let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+                       let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+                       // 1 additional HTLC corresponding to this HTLC.
+                       let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+                       if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+                               return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value".to_owned()));
+                       }
+               }
+
+               let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+               if pending_value_to_self_msat < amount_msat {
+                       return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, pending_value_to_self_msat)));
+               }
+
+               // The `+1` is for the HTLC currently being added to the commitment tx and
+               // the `2 *` and `+1` are for the fee spike buffer.
+               let local_commit_tx_fee_msat = if self.channel_outbound {
+                       2 * self.next_local_commit_tx_fee_msat(1 + 1)
+               } else { 0 };
+               if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+                       return Err(ChannelError::Ignore(format!("Cannot send value that would not leave enough to pay for fees. Pending value to self: {}. local_commit_tx_fee {}", pending_value_to_self_msat, local_commit_tx_fee_msat)));
                }
 
                // 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"));
+               let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+               if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
+                       return Err(ChannelError::Ignore(format!("Cannot send value that would put us under local channel reserve value ({})", chan_reserve_msat)));
                }
 
                // Now update local state:
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
-                               amount_msat: amount_msat,
-                               payment_hash: payment_hash,
-                               cltv_expiry: cltv_expiry,
+                               amount_msat,
+                               payment_hash,
+                               cltv_expiry,
                                source,
-                               onion_routing_packet: onion_routing_packet,
+                               onion_routing_packet,
                        });
                        return Ok(None);
                }
 
                self.pending_outbound_htlcs.push(OutboundHTLCOutput {
                        htlc_id: self.next_local_htlc_id,
-                       amount_msat: amount_msat,
+                       amount_msat,
                        payment_hash: payment_hash.clone(),
-                       cltv_expiry: cltv_expiry,
+                       cltv_expiry,
                        state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
                        source,
                });
@@ -3576,10 +3784,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let res = msgs::UpdateAddHTLC {
                        channel_id: self.channel_id,
                        htlc_id: self.next_local_htlc_id,
-                       amount_msat: amount_msat,
-                       payment_hash: payment_hash,
-                       cltv_expiry: cltv_expiry,
-                       onion_routing_packet: onion_routing_packet,
+                       amount_msat,
+                       payment_hash,
+                       cltv_expiry,
+                       onion_routing_packet,
                };
                self.next_local_htlc_id += 1;
 
@@ -3688,10 +3896,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                htlcs.push(htlc);
                        }
 
-                       let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &remote_keys, &htlcs, self.our_to_self_delay, &self.secp_ctx)
-                               .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?;
+                       let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+                       let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &pre_remote_keys, &htlcs, &self.secp_ctx)
+                               .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?;
                        signature = res.0;
                        htlc_signatures = res.1;
+                       let remote_keys = pre_remote_keys.trust_key_derivation();
 
                        log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
                                encode::serialize_hex(&remote_commitment_tx.0),
@@ -3701,7 +3911,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
                                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)),
+                                       encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, remote_keys)),
                                        log_bytes!(remote_keys.a_htlc_key.serialize()),
                                        log_bytes!(htlc_sig.serialize_compact()[..]));
                        }
@@ -3733,20 +3943,20 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> {
                for htlc in self.pending_outbound_htlcs.iter() {
                        if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
-                               return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first"});
+                               return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()});
                        }
                }
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != 0 {
                        if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 {
-                               return Err(APIError::APIMisuseError{err: "Shutdown already in progress"});
+                               return Err(APIError::APIMisuseError{err: "Shutdown already in progress".to_owned()});
                        }
                        else if (self.channel_state & ChannelState::RemoteShutdownSent as u32) == ChannelState::RemoteShutdownSent as u32 {
-                               return Err(APIError::ChannelUnavailable{err: "Shutdown already in progress by remote"});
+                               return Err(APIError::ChannelUnavailable{err: "Shutdown already in progress by remote".to_owned()});
                        }
                }
                assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
                if self.channel_state & (ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) != 0 {
-                       return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?"});
+                       return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()});
                }
 
                let our_closing_script = self.get_closing_scriptpubkey();
@@ -4306,13 +4516,12 @@ mod tests {
        use bitcoin::hashes::Hash;
        use bitcoin::hash_types::{Txid, WPubkeyHash};
        use std::sync::Arc;
-       use rand::{thread_rng,Rng};
 
        struct TestFeeEstimator {
-               fee_est: u64
+               fee_est: u32
        }
        impl FeeEstimator for TestFeeEstimator {
-               fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 {
+               fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 {
                        self.fee_est
                }
        }
@@ -4354,14 +4563,34 @@ mod tests {
                PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap())
        }
 
+       // Check that, during channel creation, we use the same feerate in the open channel message
+       // as we do in the Channel object creation itself.
+       #[test]
+       fn test_open_channel_msg_fee() {
+               let original_fee = 253;
+               let mut fee_est = TestFeeEstimator{fee_est: original_fee };
+               let secp_ctx = Secp256k1::new();
+               let seed = [42; 32];
+               let network = Network::Testnet;
+               let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
+
+               let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
+               let config = UserConfig::default();
+               let node_a_chan = Channel::<EnforcingChannelKeys>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &config).unwrap();
+
+               // Now change the fee so we can check that the fee in the open_channel message is the
+               // same as the old fee.
+               fee_est.fee_est = 500;
+               let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash());
+               assert_eq!(open_channel_msg.feerate_per_kw, original_fee);
+       }
+
        #[test]
        fn channel_reestablish_no_updates() {
                let feeest = TestFeeEstimator{fee_est: 15000};
                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 seed = [42; 32];
                let network = Network::Testnet;
                let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
 
@@ -4373,7 +4602,7 @@ mod tests {
                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 open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash());
                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, &config).unwrap();
 
@@ -4462,7 +4691,7 @@ mod tests {
                        delayed_payment_basepoint: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
                        htlc_basepoint: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444")
                };
-               chan_keys.set_remote_channel_pubkeys(&their_pubkeys);
+               chan_keys.on_accept(&their_pubkeys, chan.their_to_self_delay, chan.our_to_self_delay);
 
                assert_eq!(their_pubkeys.payment_point.serialize()[..],
                           hex::decode("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]);
@@ -4479,8 +4708,8 @@ mod tests {
                let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
                let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
                let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
-               let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
-               let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
+               let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
+               let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 
                chan.their_pubkeys = Some(their_pubkeys);
 
@@ -4518,7 +4747,7 @@ mod tests {
                                assert_eq!(serialize(&localtx.add_local_sig(&redeemscript, local_sig))[..],
                                                hex::decode($tx_hex).unwrap()[..]);
 
-                               let htlc_sigs = chan_keys.sign_local_commitment_htlc_transactions(&localtx, chan.their_to_self_delay, &chan.secp_ctx).unwrap();
+                               let htlc_sigs = chan_keys.sign_local_commitment_htlc_transactions(&localtx, &chan.secp_ctx).unwrap();
                                let mut htlc_sig_iter = localtx.per_htlc.iter().zip(htlc_sigs.iter().enumerate());
 
                                $({