Merge pull request #650 from TheBlueMatt/2020-06-fix-build
[rust-lightning] / lightning / src / ln / channel.rs
index b0f8ef9abee0d60441aaf80563098cf43f880bd9..97c9fa6789b3c4a2045539281c27b1a28cf1d0b4 100644 (file)
@@ -44,6 +44,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 +54,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 +184,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 +197,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 +310,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 +328,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>,
 
@@ -423,8 +448,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:
@@ -533,10 +558,10 @@ 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) {
+               if feerate_per_kw < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) {
                        return Err(ChannelError::Close("Peer's feerate much too low"));
                }
-               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 {
+               if feerate_per_kw as u64 > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64  * 2 {
                        return Err(ChannelError::Close("Peer's feerate much too high"));
                }
                Ok(())
@@ -645,12 +670,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // 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 {
+               if funders_amount_msat < background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT {
                        return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
                }
 
                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"));
                }
@@ -725,7 +750,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,
@@ -766,15 +791,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
                let mut sha = Sha256::engine();
-               let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());
 
                let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
                if self.channel_outbound {
-                       sha.input(&our_payment_point.serialize());
+                       sha.input(&self.local_keys.pubkeys().payment_point.serialize());
                        sha.input(their_payment_point);
                } else {
                        sha.input(their_payment_point);
-                       sha.input(&our_payment_point.serialize());
+                       sha.input(&self.local_keys.pubkeys().payment_point.serialize());
                }
                let res = Sha256::from_engine(sha).into_inner();
 
@@ -804,7 +828,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 = {
@@ -844,7 +868,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(),
@@ -856,7 +880,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(),
@@ -949,7 +973,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 {
@@ -1095,11 +1119,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// 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 delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
+               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"))
        }
 
        #[inline]
@@ -1109,25 +1133,24 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
-               let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
-               let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
+               let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
+               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"))
        }
 
        /// Gets the redeemscript for the funding transaction output (ie the funding transaction output
        /// pays to get_funding_redeemscript().to_v0_p2wsh()).
        /// Panics if called before accept_channel/new_from_req
        pub fn get_funding_redeemscript(&self) -> Script {
-               let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
-               make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
+               make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
        }
 
        /// 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)
        }
 
@@ -1455,7 +1478,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
                secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");
 
-               let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
+               let 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;
@@ -1486,7 +1509,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
-               let funding_txo = OutPoint::new(msg.funding_txid, msg.funding_output_index);
+               let funding_txo = OutPoint{ txid: msg.funding_txid, index: msg.funding_output_index };
                self.funding_txo = Some(funding_txo.clone());
 
                let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature) = match self.funding_created_signature(&msg.signature, logger) {
@@ -1568,7 +1591,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                macro_rules! create_monitor {
                        () => { {
-                               let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
+                               let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
                                let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
                                                                              &self.shutdown_pubkey, self.our_to_self_delay,
                                                                              &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
@@ -1665,8 +1688,83 @@ 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) {
+       // 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"));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1710,9 +1808,56 @@ 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"));
+               }
+
+               // 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"));
+               };
+
+               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"));
+               }
+
+               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"));
+                       }
                }
+
                if self.next_remote_htlc_id != msg.htlc_id {
                        return Err(ChannelError::Close("Remote skipped HTLC ID"));
                }
@@ -1721,7 +1866,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                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");
                        }
                }
@@ -1733,7 +1878,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(())
        }
@@ -1842,7 +1987,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                //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 {
@@ -1899,7 +2044,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let mut monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
                        updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
-                               commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
+                               commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
                                htlc_outputs: htlcs_and_sigs
                        }]
                };
@@ -2322,7 +2467,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");
                }
@@ -2343,11 +2488,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)?;
@@ -2508,7 +2653,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish"));
                }
                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(())
        }
@@ -2726,7 +2871,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
@@ -2825,7 +2970,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-               let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
+               let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
                let their_funding_key = self.their_funding_pubkey().serialize();
                if our_funding_key[..] < their_funding_key[..] {
                        tx.input[0].witness.push(our_sig.serialize_der().to_vec());
@@ -2888,7 +3033,7 @@ 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"))?;
@@ -2901,10 +3046,10 @@ 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"));
@@ -2914,7 +3059,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                } 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"));
@@ -2996,7 +3141,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        #[cfg(test)]
-       pub fn get_feerate(&self) -> u64 {
+       pub fn get_feerate(&self) -> u32 {
                self.feerate_per_kw
        }
 
@@ -3038,6 +3183,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,
                }
        }
 
@@ -3067,15 +3213,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
        }
@@ -3154,7 +3300,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 {
@@ -3205,6 +3351,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)) |
@@ -3287,9 +3437,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?");
                }
@@ -3302,6 +3450,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
+               let local_keys = self.local_keys.pubkeys();
 
                msgs::OpenChannel {
                        chain_hash: chain_hash,
@@ -3312,14 +3461,14 @@ 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: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
-                       revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
-                       payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
-                       delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
+                       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: local_keys.htlc_basepoint,
                        first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
                        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() })
@@ -3338,6 +3487,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
+               let local_keys = self.local_keys.pubkeys();
 
                msgs::AcceptChannel {
                        temporary_channel_id: self.channel_id,
@@ -3348,11 +3498,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        minimum_depth: self.minimum_depth,
                        to_self_delay: self.our_to_self_delay,
                        max_accepted_htlcs: OUR_MAX_HTLCS,
-                       funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
-                       revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
-                       payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
-                       delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
-                       htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
+                       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: local_keys.htlc_basepoint,
                        first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
                        shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
                }
@@ -3431,7 +3581,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
-               let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 
                let msg = msgs::UnsignedChannelAnnouncement {
                        features: ChannelFeatures::known(),
@@ -3439,8 +3588,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        short_channel_id: self.get_short_channel_id().unwrap(),
                        node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
                        node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
-                       bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
-                       bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
+                       bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
+                       bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
                        excess_data: Vec::new(),
                };
 
@@ -3547,29 +3696,56 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
                }
 
+               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"));
+                       }
+               }
+
+               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("Cannot send value that would overdraw remaining funds"));
+               }
+
+               // 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("Cannot send value that would not leave enough to pay for fees"));
+               }
+
                // 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 {
+               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("Cannot send value that would put us under local channel reserve value"));
                }
 
                // 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,
                });
@@ -3577,10 +3753,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;
 
@@ -4307,13 +4483,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
                }
        }
@@ -4355,14 +4530,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);
 
@@ -4374,7 +4569,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();
 
@@ -4387,7 +4582,7 @@ mod tests {
                let tx = Transaction { version: 1, lock_time: 0, input: Vec::new(), output: vec![TxOut {
                        value: 10000000, script_pubkey: output_script.clone(),
                }]};
-               let funding_outpoint = OutPoint::new(tx.txid(), 0);
+               let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap();
                let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, &&logger).unwrap();
 
@@ -4442,7 +4637,7 @@ mod tests {
                        (0, 0)
                );
 
-               assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
+               assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
                                hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
                let keys_provider = Keys { chan_keys: chan_keys.clone() };
 
@@ -4453,7 +4648,7 @@ mod tests {
                chan.their_to_self_delay = 144;
                chan.our_dust_limit_satoshis = 546;
 
-               let funding_info = OutPoint::new(Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), 0);
+               let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };
                chan.funding_txo = Some(funding_info);
 
                let their_pubkeys = ChannelPublicKeys {
@@ -4477,11 +4672,11 @@ mod tests {
                // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
                // derived from a commitment_seed, so instead we copy it here and call
                // build_commitment_transaction.
-               let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
+               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);
 
@@ -4512,7 +4707,7 @@ mod tests {
                                })*
                                assert_eq!(unsigned_tx.1.len(), per_htlc.len());
 
-                               localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
+                               localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
                                let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
                                assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);