Make channel reserve variable names less confusing.
[rust-lightning] / lightning / src / ln / channel.rs
index 59bdae041d2d739c5075fc1cce3db8fdb8410732..99dfae389d3d9543e6b12b7e035a896273eb4067 100644 (file)
@@ -6,19 +6,18 @@ use bitcoin::util::hash::BitcoinHash;
 use bitcoin::util::bip143;
 use bitcoin::consensus::encode;
 
-use bitcoin_hashes::{Hash, HashEngine};
-use bitcoin_hashes::sha256::Hash as Sha256;
-use bitcoin_hashes::hash160::Hash as Hash160;
-use bitcoin_hashes::sha256d::Hash as Sha256dHash;
+use bitcoin::hashes::{Hash, HashEngine};
+use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
 
-use secp256k1::key::{PublicKey,SecretKey};
-use secp256k1::{Secp256k1,Signature};
-use secp256k1;
+use bitcoin::secp256k1::key::{PublicKey,SecretKey};
+use bitcoin::secp256k1::{Secp256k1,Signature};
+use bitcoin::secp256k1;
 
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep};
+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;
@@ -207,8 +206,8 @@ enum ChannelState {
        /// to drop us, but we store this anyway.
        ShutdownComplete = 4096,
 }
-const BOTH_SIDES_SHUTDOWN_MASK: u32 = (ChannelState::LocalShutdownSent as u32 | ChannelState::RemoteShutdownSent as u32);
-const MULTI_STATE_FLAGS: u32 = (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32);
+const BOTH_SIDES_SHUTDOWN_MASK: u32 = ChannelState::LocalShutdownSent as u32 | ChannelState::RemoteShutdownSent as u32;
+const MULTI_STATE_FLAGS: u32 = BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32;
 
 const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
 
@@ -313,11 +312,11 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full
        /// series of block_connected/block_disconnected calls. Obviously this is not a guarantee as we
        /// could miss the funding_tx_confirmed_in block as well, but it serves as a useful fallback.
-       funding_tx_confirmed_in: Option<Sha256dHash>,
+       funding_tx_confirmed_in: Option<BlockHash>,
        short_channel_id: Option<u64>,
        /// Used to deduplicate block_connected callbacks, also used to verify consistency during
        /// ChannelManager deserialization (hence pub(super))
-       pub(super) last_block_connected: Sha256dHash,
+       pub(super) last_block_connected: BlockHash,
        funding_tx_confirmations: u64,
 
        their_dust_limit_satoshis: u64,
@@ -330,9 +329,9 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        #[cfg(not(test))]
        their_max_htlc_value_in_flight_msat: u64,
        //get_our_max_htlc_value_in_flight_msat(): u64,
-       /// minimum channel reserve for **self** to maintain - set by them.
-       their_channel_reserve_satoshis: u64,
-       //get_our_channel_reserve_satoshis(): u64,
+       /// minimum channel reserve for self to maintain - set by them.
+       local_channel_reserve_satoshis: u64,
+       // get_remote_channel_reserve_satoshis(channel_value_sats: u64): u64
        their_htlc_minimum_msat: u64,
        our_htlc_minimum_msat: u64,
        their_to_self_delay: u16,
@@ -382,7 +381,7 @@ pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
 
 /// Maximmum `funding_satoshis` value, according to the BOLT #2 specification
 /// it's 2^24.
-pub const MAX_FUNDING_SATOSHIS: u64 = (1 << 24);
+pub const MAX_FUNDING_SATOSHIS: u64 = 1 << 24;
 
 /// Used to return a simple Error back to ChannelManager. Will get converted to a
 /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our
@@ -421,10 +420,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                channel_value_satoshis * 1000 / 10 //TODO
        }
 
-       /// Returns a minimum channel reserve value **they** need to maintain
+       /// Returns a minimum channel reserve value the remote needs to maintain,
+       /// required by us.
        ///
        /// Guaranteed to return a value no larger than channel_value_satoshis
-       pub(crate) fn get_our_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
+       pub(crate) fn get_remote_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
                let (q, _) = channel_value_satoshis.overflowing_div(100);
                cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
        }
@@ -453,7 +453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               if Channel::<ChanSigner>::get_our_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
+               if Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
                        return Err(APIError::FeeRateTooHigh{err: format!("Not enough reserve above dust limit can be found at current fee rate({})", background_feerate), feerate: background_feerate});
                }
 
@@ -513,7 +513,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_dust_limit_satoshis: 0,
                        our_dust_limit_satoshis: Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate),
                        their_max_htlc_value_in_flight_msat: 0,
-                       their_channel_reserve_satoshis: 0,
+                       local_channel_reserve_satoshis: 0,
                        their_htlc_minimum_msat: 0,
                        our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: 0,
@@ -639,15 +639,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
 
                let our_dust_limit_satoshis = Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate);
-               let our_channel_reserve_satoshis = Channel::<ChanSigner>::get_our_channel_reserve_satoshis(msg.funding_satoshis);
-               if our_channel_reserve_satoshis < our_dust_limit_satoshis {
+               let remote_channel_reserve_satoshis = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(msg.funding_satoshis);
+               if remote_channel_reserve_satoshis < our_dust_limit_satoshis {
                        return Err(ChannelError::Close("Suitable channel reserve not found. aborting"));
                }
                if msg.channel_reserve_satoshis < our_dust_limit_satoshis {
                        return Err(ChannelError::Close("channel_reserve_satoshis too small"));
                }
-               if our_channel_reserve_satoshis < msg.dust_limit_satoshis {
-                       return Err(ChannelError::Close("Dust limit too high for our channel reserve"));
+               if remote_channel_reserve_satoshis < msg.dust_limit_satoshis {
+                       return Err(ChannelError::Close("Dust limit too high for the channel reserve we require the remote to keep"));
                }
 
                // check if the funder's amount for the initial commitment tx is sufficient
@@ -659,7 +659,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let to_local_msat = msg.push_msat;
                let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT;
-               if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= our_channel_reserve_satoshis * 1000 {
+               if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= remote_channel_reserve_satoshis * 1000 {
                        return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
                }
 
@@ -738,7 +738,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        their_dust_limit_satoshis: msg.dust_limit_satoshis,
                        our_dust_limit_satoshis: our_dust_limit_satoshis,
                        their_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
-                       their_channel_reserve_satoshis: msg.channel_reserve_satoshis,
+                       local_channel_reserve_satoshis: msg.channel_reserve_satoshis,
                        their_htlc_minimum_msat: msg.htlc_minimum_msat,
                        our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        their_to_self_delay: msg.to_self_delay,
@@ -953,9 +953,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        } else {
                                self.max_commitment_tx_output_remote.lock().unwrap()
                        };
-                       debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.their_channel_reserve_satoshis as i64);
+                       debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.local_channel_reserve_satoshis as i64);
                        max_commitment_tx_output.0 = cmp::max(max_commitment_tx_output.0, value_to_self_msat as u64);
-                       debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
+                       debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
                        max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64);
                }
 
@@ -983,7 +983,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        log_trace!(self, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
                        txouts.push((TxOut {
                                script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
-                                                            .push_slice(&Hash160::hash(&keys.b_payment_key.serialize())[..])
+                                                            .push_slice(&WPubkeyHash::hash(&keys.b_payment_key.serialize())[..])
                                                             .into_script(),
                                value: value_to_b as u64
                        }, None));
@@ -1025,7 +1025,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        #[inline]
        fn get_closing_scriptpubkey(&self) -> Script {
-               let our_channel_close_key_hash = Hash160::hash(&self.shutdown_pubkey.serialize());
+               let our_channel_close_key_hash = WPubkeyHash::hash(&self.shutdown_pubkey.serialize());
                Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script()
        }
 
@@ -1133,7 +1133,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: &Sha256dHash, 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: u64) -> 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)
        }
 
@@ -1363,7 +1363,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.channel_reserve_satoshis < self.our_dust_limit_satoshis {
                        return Err(ChannelError::Close("Peer never wants payout outputs?"));
                }
-               if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) {
+               if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) {
                        return Err(ChannelError::Close("Dust limit is bigger than our channel reverse"));
                }
                if msg.htlc_minimum_msat >= (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000 {
@@ -1425,7 +1425,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
                self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
-               self.their_channel_reserve_satoshis = msg.channel_reserve_satoshis;
+               self.local_channel_reserve_satoshis = msg.channel_reserve_satoshis;
                self.their_htlc_minimum_msat = msg.htlc_minimum_msat;
                self.their_to_self_delay = msg.to_self_delay;
                self.their_max_accepted_htlcs = msg.max_accepted_htlcs;
@@ -1460,7 +1460,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // They sign the "local" commitment transaction...
                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, &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(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
@@ -1515,10 +1515,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                              &their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
                                                                              self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
                                                                              self.get_commitment_transaction_number_obscure_factor(),
+                                                                             local_initial_commitment_tx.clone(),
                                                                              self.logger.clone());
 
                                channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
-                               channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), Vec::new()).unwrap();
                                channel_monitor
                        } }
                }
@@ -1539,22 +1539,25 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
-       pub fn funding_signed(&mut self, msg: &msgs::FundingSigned) -> Result<ChannelMonitorUpdate, (Option<ChannelMonitorUpdate>, ChannelError)> {
+       pub fn funding_signed(&mut self, msg: &msgs::FundingSigned) -> Result<ChannelMonitor<ChanSigner>, ChannelError> {
                if !self.channel_outbound {
-                       return Err((None, ChannelError::Close("Received funding_signed for an inbound channel?")));
+                       return Err(ChannelError::Close("Received funding_signed for an inbound channel?"));
                }
                if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 {
-                       return Err((None, ChannelError::Close("Received funding_signed in strange state!")));
+                       return Err(ChannelError::Close("Received funding_signed in strange state!"));
                }
                if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
-                               self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER - 1 ||
+                               self.cur_remote_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
                                self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
                let funding_script = self.get_funding_redeemscript();
 
-               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number).map_err(|e| (None, e))?;
+               let remote_keys = self.build_remote_transaction_keys()?;
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
+
+               let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
                let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
                let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
@@ -1562,27 +1565,40 @@ 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((None, ChannelError::Close("Invalid funding_signed signature from peer")));
+                       return Err(ChannelError::Close("Invalid funding_signed signature from peer"));
                }
 
-               self.latest_monitor_update_id += 1;
-               let monitor_update = ChannelMonitorUpdate {
-                       update_id: self.latest_monitor_update_id,
-                       updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
-                               commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys, self.feerate_per_kw, Vec::new()),
-                               htlc_outputs: Vec::new(),
-                       }]
-               };
-               self.channel_monitor.as_mut().unwrap().update_monitor_ooo(monitor_update.clone()).unwrap();
-               self.channel_state = ChannelState::FundingSent as u32 | (self.channel_state & (ChannelState::MonitorUpdateFailed as u32));
-               self.cur_local_commitment_transaction_number -= 1;
+               let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
+               let funding_redeemscript = self.get_funding_redeemscript();
+               let funding_txo = self.funding_txo.as_ref().unwrap();
+               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 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()),
+                                                                             &their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
+                                                                             self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
+                                                                             self.get_commitment_transaction_number_obscure_factor(),
+                                                                             local_commitment_tx,
+                                                                             self.logger.clone());
 
-               if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                       Ok(monitor_update)
-               } else {
-                       Err((Some(monitor_update),
-                               ChannelError::Ignore("Previous monitor update failure prevented funding_signed from allowing funding broadcast")))
+                               channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
+
+                               channel_monitor
+                       } }
                }
+
+               self.channel_monitor = Some(create_monitor!());
+               let channel_monitor = create_monitor!();
+
+               assert_eq!(self.channel_state & (ChannelState::MonitorUpdateFailed as u32), 0); // We have no had any monitor(s) yet to fail update!
+               self.channel_state = ChannelState::FundingSent as u32;
+               self.cur_local_commitment_transaction_number -= 1;
+               self.cur_remote_commitment_transaction_number -= 1;
+
+               Ok(channel_monitor)
        }
 
        pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
@@ -1681,7 +1697,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if htlc_inbound_value_msat + msg.amount_msat > Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) {
                        return Err(ChannelError::Close("Remote HTLC add would put them over our max HTLC value"));
                }
-               // Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
+               // Check remote_channel_reserve_satoshis (we're getting paid, so they have to at least meet
                // the reserve_satoshis we told them to always have as direct payment so that they lose
                // something if we punish them for broadcasting an old state).
                // Note that we don't really care about having a small/no to_remote output in our local
@@ -1701,8 +1717,8 @@ 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_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
-                       return Err(ChannelError::Close("Remote HTLC add would put them over their reserve value"));
+               if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
+                       return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
                }
                if self.next_remote_htlc_id != msg.htlc_id {
                        return Err(ChannelError::Close("Remote skipped HTLC ID"));
@@ -1711,8 +1727,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Remote provided CLTV expiry in seconds instead of block height"));
                }
 
-               //TODO: Check msg.cltv_expiry further? Do this in channel manager?
-
                if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
                        if let PendingHTLCStatus::Forward(_) = pending_forward_state {
                                panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
@@ -1834,7 +1848,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        let num_htlcs = local_commitment_tx.1;
                        let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
 
-                       if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
+                       let remote_reserve_we_require = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+                       if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + remote_reserve_we_require {
                                return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee")));
                        }
                }
@@ -1888,7 +1903,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, &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(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
                                htlc_outputs: htlcs_and_sigs
                        }]
                };
@@ -2951,7 +2966,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// May only be called after funding has been initiated (ie is_funding_initiated() is true)
        pub fn channel_monitor(&mut self) -> &mut ChannelMonitor<ChanSigner> {
-               if self.channel_state < ChannelState::FundingCreated as u32 {
+               if self.channel_state < ChannelState::FundingSent as u32 {
                        panic!("Can't get a channel monitor until funding has been created");
                }
                self.channel_monitor.as_mut().unwrap()
@@ -3020,7 +3035,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                ChannelValueStat {
                        value_to_self_msat: self.value_to_self_msat,
                        channel_value_msat: self.channel_value_satoshis * 1000,
-                       channel_reserve_msat: self.their_channel_reserve_satoshis * 1000,
+                       channel_reserve_msat: self.local_channel_reserve_satoshis * 1000,
                        pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        holding_cell_outbound_amount_msat: {
@@ -3105,7 +3120,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Returns true if funding_created was sent/received.
        pub fn is_funding_initiated(&self) -> bool {
-               self.channel_state >= ChannelState::FundingCreated as u32
+               self.channel_state >= ChannelState::FundingSent as u32
        }
 
        /// Returns true if this channel is fully shut down. True here implies that no further actions
@@ -3138,13 +3153,33 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.network_sync == UpdateStatus::DisabledMarked
        }
 
-       /// Called by channelmanager based on chain blocks being connected.
-       /// Note that we only need to use this to detect funding_signed, anything else is handled by
-       /// the channel_monitor.
-       /// In case of Err, the channel may have been closed, at which point the standard requirements
-       /// apply - no calls may be made except those explicitly stated to be allowed post-shutdown.
+       /// When we receive a new block, we (a) check whether the block contains the funding
+       /// transaction (which would start us counting blocks until we send the funding_signed), and
+       /// (b) check the height of the block against outbound holding cell HTLCs in case we need to
+       /// give up on them prematurely and time them out. Everything else (e.g. commitment
+       /// transaction broadcasts, channel closure detection, HTLC transaction broadcasting, etc) is
+       /// handled by the ChannelMonitor.
+       ///
+       /// If we return Err, the channel may have been closed, at which point the standard
+       /// requirements apply - no calls may be made except those explicitly stated to be allowed
+       /// post-shutdown.
        /// Only returns an ErrorAction of DisconnectPeer, if Err.
-       pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> {
+       ///
+       /// 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> {
+               let mut timed_out_htlcs = Vec::new();
+               self.holding_cell_htlc_updates.retain(|htlc_update| {
+                       match htlc_update {
+                               &HTLCUpdateAwaitingACK::AddHTLC { ref payment_hash, ref source, ref cltv_expiry, .. } => {
+                                       if *cltv_expiry <= height + HTLC_FAIL_BACK_BUFFER {
+                                               timed_out_htlcs.push((source.clone(), payment_hash.clone()));
+                                               false
+                                       } else { true }
+                               },
+                               _ => true
+                       }
+               });
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
                if header.bitcoin_hash() != self.last_block_connected {
                        if self.funding_tx_confirmations > 0 {
@@ -3227,19 +3262,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                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);
-                                                       return Ok(Some(msgs::FundingLocked {
+                                                       return Ok((Some(msgs::FundingLocked {
                                                                channel_id: self.channel_id,
                                                                next_per_commitment_point: next_per_commitment_point,
-                                                       }));
+                                                       }), timed_out_htlcs));
                                                } else {
                                                        self.monitor_pending_funding_locked = true;
-                                                       return Ok(None);
+                                                       return Ok((None, timed_out_htlcs));
                                                }
                                        }
                                }
                        }
                }
-               Ok(None)
+               Ok((None, timed_out_htlcs))
        }
 
        /// Called by channelmanager based on chain blocks being disconnected.
@@ -3265,7 +3300,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: Sha256dHash, fee_estimator: &F) -> msgs::OpenChannel
+       pub fn get_open_channel<F: Deref>(&self, chain_hash: BlockHash, fee_estimator: &F) -> msgs::OpenChannel
                where F::Target: FeeEstimator
        {
                if !self.channel_outbound {
@@ -3288,7 +3323,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        push_msat: self.channel_value_satoshis * 1000 - self.value_to_self_msat,
                        dust_limit_satoshis: self.our_dust_limit_satoshis,
                        max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
+                       channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.our_htlc_minimum_msat,
                        feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
                        to_self_delay: self.our_to_self_delay,
@@ -3321,7 +3356,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        temporary_channel_id: self.channel_id,
                        dust_limit_satoshis: self.our_dust_limit_satoshis,
                        max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
+                       channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.our_htlc_minimum_msat,
                        minimum_depth: self.minimum_depth,
                        to_self_delay: self.our_to_self_delay,
@@ -3337,11 +3372,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
-       fn get_outbound_funding_created_signature(&mut self) -> Result<(Signature, Transaction), ChannelError> {
+       fn get_outbound_funding_created_signature(&mut self) -> Result<Signature, ChannelError> {
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
-               Ok((self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx)
-                               .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?.0, remote_initial_commitment_tx))
+               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)
        }
 
        /// Updates channel state with knowledge of the funding transaction's txid/index, and generates
@@ -3351,7 +3386,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Note that channel_id changes during this call!
        /// Do NOT broadcast the funding transaction until after a successful funding_signed call!
        /// If an Err is returned, it is a ChannelError::Close.
-       pub fn get_outbound_funding_created(&mut self, funding_txo: OutPoint) -> Result<(msgs::FundingCreated, ChannelMonitor<ChanSigner>), ChannelError> {
+       pub fn get_outbound_funding_created(&mut self, funding_txo: OutPoint) -> Result<msgs::FundingCreated, ChannelError> {
                if !self.channel_outbound {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
@@ -3365,7 +3400,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                self.funding_txo = Some(funding_txo.clone());
-               let (our_signature, commitment_tx) = match self.get_outbound_funding_created_signature() {
+               let our_signature = match self.get_outbound_funding_created_signature() {
                        Ok(res) => res,
                        Err(e) => {
                                log_error!(self, "Got bad signatures: {:?}!", e);
@@ -3378,37 +3413,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // Now that we're past error-generating stuff, update our local state:
 
-               let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
-               let funding_redeemscript = self.get_funding_redeemscript();
-               let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
-               macro_rules! create_monitor {
-                       () => { {
-                               let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
-                                                                             &self.shutdown_pubkey, self.our_to_self_delay,
-                                                                             &self.destination_script, (funding_txo, funding_txo_script.clone()),
-                                                                             &their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
-                                                                             self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
-                                                                             self.get_commitment_transaction_number_obscure_factor(),
-                                                                             self.logger.clone());
-
-                               channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
-                               channel_monitor
-                       } }
-               }
-
-               self.channel_monitor = Some(create_monitor!());
-               let channel_monitor = create_monitor!();
-
                self.channel_state = ChannelState::FundingCreated as u32;
                self.channel_id = funding_txo.to_channel_id();
-               self.cur_remote_commitment_transaction_number -= 1;
 
-               Ok((msgs::FundingCreated {
-                       temporary_channel_id: temporary_channel_id,
+               Ok(msgs::FundingCreated {
+                       temporary_channel_id,
                        funding_txid: funding_txo.txid,
                        funding_output_index: funding_txo.index,
                        signature: our_signature
-               }, channel_monitor))
+               })
        }
 
        /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our
@@ -3419,7 +3432,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// closing).
        /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
        /// https://github.com/lightningnetwork/lightning-rfc/issues/468
-       pub fn get_channel_announcement(&self, our_node_id: PublicKey, chain_hash: Sha256dHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
+       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"));
                }
@@ -3434,7 +3447,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 
                let msg = msgs::UnsignedChannelAnnouncement {
-                       features: ChannelFeatures::supported(),
+                       features: ChannelFeatures::known(),
                        chain_hash: chain_hash,
                        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() },
@@ -3539,14 +3552,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
                }
 
-               // Check self.their_channel_reserve_satoshis (the amount we must keep as
-               // reserve for them to have something to claim if we misbehave)
-               if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
-                       return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value"));
+               // Check self.local_channel_reserve_satoshis (the amount we must keep as
+               // reserve for the remote to have something to claim if we misbehave)
+               if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+                       return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
                }
 
-               //TODO: Check cltv_expiry? Do this in channel manager?
-
                // Now update local state:
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
@@ -4010,7 +4021,7 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                self.their_dust_limit_satoshis.write(writer)?;
                self.our_dust_limit_satoshis.write(writer)?;
                self.their_max_htlc_value_in_flight_msat.write(writer)?;
-               self.their_channel_reserve_satoshis.write(writer)?;
+               self.local_channel_reserve_satoshis.write(writer)?;
                self.their_htlc_minimum_msat.write(writer)?;
                self.our_htlc_minimum_msat.write(writer)?;
                self.their_to_self_delay.write(writer)?;
@@ -4166,7 +4177,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                let their_dust_limit_satoshis = Readable::read(reader)?;
                let our_dust_limit_satoshis = Readable::read(reader)?;
                let their_max_htlc_value_in_flight_msat = Readable::read(reader)?;
-               let their_channel_reserve_satoshis = Readable::read(reader)?;
+               let local_channel_reserve_satoshis = Readable::read(reader)?;
                let their_htlc_minimum_msat = Readable::read(reader)?;
                let our_htlc_minimum_msat = Readable::read(reader)?;
                let their_to_self_delay = Readable::read(reader)?;
@@ -4245,7 +4256,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
                        their_dust_limit_satoshis,
                        our_dust_limit_satoshis,
                        their_max_htlc_value_in_flight_msat,
-                       their_channel_reserve_satoshis,
+                       local_channel_reserve_satoshis,
                        their_htlc_minimum_msat,
                        our_htlc_minimum_msat,
                        their_to_self_delay,
@@ -4281,7 +4292,7 @@ mod tests {
        use bitcoin::blockdata::constants::genesis_block;
        use bitcoin::blockdata::opcodes;
        use bitcoin::network::constants::Network;
-       use bitcoin_hashes::hex::FromHex;
+       use bitcoin::hashes::hex::FromHex;
        use hex;
        use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
        use ln::channel::{Channel,ChannelKeys,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,TxCreationKeys};
@@ -4297,12 +4308,11 @@ mod tests {
        use util::enforcing_trait_impls::EnforcingChannelKeys;
        use util::test_utils;
        use util::logger::Logger;
-       use secp256k1::{Secp256k1, Message, Signature, All};
-       use secp256k1::key::{SecretKey,PublicKey};
-       use bitcoin_hashes::sha256::Hash as Sha256;
-       use bitcoin_hashes::sha256d::Hash as Sha256dHash;
-       use bitcoin_hashes::hash160::Hash as Hash160;
-       use bitcoin_hashes::Hash;
+       use bitcoin::secp256k1::{Secp256k1, Message, Signature, All};
+       use bitcoin::secp256k1::key::{SecretKey,PublicKey};
+       use bitcoin::hashes::sha256::Hash as Sha256;
+       use bitcoin::hashes::Hash;
+       use bitcoin::hash_types::{Txid, WPubkeyHash};
        use std::sync::Arc;
        use rand::{thread_rng,Rng};
 
@@ -4331,7 +4341,7 @@ mod tests {
                fn get_destination_script(&self) -> Script {
                        let secp_ctx = Secp256k1::signing_only();
                        let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
-                       let our_channel_monitor_claim_key_hash = Hash160::hash(&PublicKey::from_secret_key(&secp_ctx, &channel_monitor_claim_key).serialize());
+                       let our_channel_monitor_claim_key_hash = WPubkeyHash::hash(&PublicKey::from_secret_key(&secp_ctx, &channel_monitor_claim_key).serialize());
                        Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_monitor_claim_key_hash[..]).into_script()
                }
 
@@ -4373,11 +4383,11 @@ mod tests {
                // Create Node B's channel by receiving Node A's open_channel message
                let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash(), &&feeest);
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-               let mut node_b_chan = Channel::<EnforcingChannelKeys>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::supported(), &open_channel_msg, 7, logger, &config).unwrap();
+               let mut node_b_chan = Channel::<EnforcingChannelKeys>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, logger, &config).unwrap();
 
                // Node B --> Node A: accept channel
                let accept_channel_msg = node_b_chan.get_accept_channel();
-               node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::supported()).unwrap();
+               node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap();
 
                // Node A --> Node B: funding created
                let output_script = node_a_chan.get_funding_redeemscript();
@@ -4385,7 +4395,7 @@ mod tests {
                        value: 10000000, script_pubkey: output_script.clone(),
                }]};
                let funding_outpoint = OutPoint::new(tx.txid(), 0);
-               let (funding_created_msg, _) = node_a_chan.get_outbound_funding_created(funding_outpoint).unwrap();
+               let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint).unwrap();
                let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg).unwrap();
 
                // Node B --> Node A: funding signed
@@ -4447,7 +4457,7 @@ mod tests {
                chan.their_to_self_delay = 144;
                chan.our_dust_limit_satoshis = 546;
 
-               let funding_info = OutPoint::new(Sha256dHash::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), 0);
+               let funding_info = OutPoint::new(Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), 0);
                chan.funding_txo = Some(funding_info);
 
                let their_pubkeys = ChannelPublicKeys {
@@ -4485,7 +4495,7 @@ mod tests {
                macro_rules! test_commitment {
                        ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr, {
                                $( { $htlc_idx: expr, $their_htlc_sig_hex: expr, $our_htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
-                       } ) => {
+                       } ) => { {
                                unsigned_tx = {
                                        let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw);
                                        let htlcs = res.2.drain(..)
@@ -4506,12 +4516,15 @@ mod tests {
                                })*
                                assert_eq!(unsigned_tx.1.len(), per_htlc.len());
 
-                               localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
-                               chan_keys.sign_local_commitment(&mut localtx, &chan.secp_ctx);
+                               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);
+                               let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 
-                               assert_eq!(serialize(localtx.with_valid_witness())[..],
+                               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 mut htlc_sig_iter = localtx.per_htlc.iter().zip(htlc_sigs.iter().enumerate());
+
                                $({
                                        let remote_signature = Signature::from_der(&hex::decode($their_htlc_sig_hex).unwrap()[..]).unwrap();
 
@@ -4533,12 +4546,19 @@ mod tests {
                                                assert!(preimage.is_some());
                                        }
 
-                                       chan_keys.sign_htlc_transaction(&mut localtx, $htlc_idx, preimage, chan.their_to_self_delay, &chan.secp_ctx);
+                                       let mut htlc_sig = htlc_sig_iter.next().unwrap();
+                                       while (htlc_sig.1).1.is_none() { htlc_sig = htlc_sig_iter.next().unwrap(); }
+                                       assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx));
 
-                                       assert_eq!(serialize(localtx.htlc_with_valid_witness($htlc_idx).as_ref().unwrap())[..],
+                                       assert_eq!(serialize(&localtx.get_signed_htlc_tx((htlc_sig.1).0, &(htlc_sig.1).1.unwrap(), &preimage, chan.their_to_self_delay))[..],
                                                        hex::decode($htlc_tx_hex).unwrap()[..]);
                                })*
-                       }
+                               loop {
+                                       let htlc_sig = htlc_sig_iter.next();
+                                       if htlc_sig.is_none() { break; }
+                                       assert!((htlc_sig.unwrap().1).1.is_none());
+                               }
+                       } }
                }
 
                {