Add unit test for get_witnesses_weight
[rust-lightning] / src / ln / channelmonitor.rs
index 546e4a68451cc076c440ca9b2ac0902233d2e8ef..15db1426735b66643dde33477a43502be4ead2a1 100644 (file)
@@ -17,14 +17,15 @@ use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
 use bitcoin::consensus::encode::{self, Decodable, Encodable};
-use bitcoin::util::hash::{BitcoinHash,Sha256dHash};
+use bitcoin::util::hash::BitcoinHash;
 use bitcoin::util::bip143;
 
 use bitcoin_hashes::Hash;
 use bitcoin_hashes::sha256::Hash as Sha256;
 use bitcoin_hashes::hash160::Hash as Hash160;
+use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 
-use secp256k1::{Secp256k1,Message,Signature};
+use secp256k1::{Secp256k1,Signature};
 use secp256k1::key::{SecretKey,PublicKey};
 use secp256k1;
 
@@ -33,7 +34,7 @@ use ln::chan_utils;
 use ln::chan_utils::HTLCOutputInCommitment;
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
-use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface};
+use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator, ConfirmationTarget};
 use chain::transaction::OutPoint;
 use chain::keysinterface::SpendableOutputDescriptor;
 use util::logger::Logger;
@@ -47,8 +48,8 @@ use std::{hash,cmp, mem};
 /// An error enum representing a failure to persist a channel monitor update.
 #[derive(Clone)]
 pub enum ChannelMonitorUpdateErr {
-       /// Used to indicate a temporary failure (eg connection to a watchtower failed, but is expected
-       /// to succeed at some point in the future).
+       /// Used to indicate a temporary failure (eg connection to a watchtower or remote backup of
+       /// our state failed, but is expected to succeed at some point in the future).
        ///
        /// Such a failure will "freeze" a channel, preventing us from revoking old states or
        /// submitting new commitment transactions to the remote party.
@@ -70,6 +71,10 @@ pub enum ChannelMonitorUpdateErr {
        /// Note that even if updates made after TemporaryFailure succeed you must still call
        /// test_restore_channel_monitor to ensure you have the latest monitor and re-enable normal
        /// channel operation.
+       ///
+       /// For deployments where a copy of ChannelMonitors and other local state are backed up in a
+       /// remote location (with local copies persisted immediately), it is anticipated that all
+       /// updates will return TemporaryFailure until the remote copies could be updated.
        TemporaryFailure,
        /// Used to indicate no further channel monitor updates will be allowed (eg we've moved on to a
        /// different watchtower and cannot update with all watchtowers that were previously informed
@@ -138,6 +143,7 @@ pub struct SimpleManyChannelMonitor<Key> {
        pending_events: Mutex<Vec<events::Event>>,
        pending_htlc_updated: Mutex<HashMap<PaymentHash, Vec<(HTLCSource, Option<PaymentPreimage>)>>>,
        logger: Arc<Logger>,
+       fee_estimator: Arc<FeeEstimator>
 }
 
 impl<Key : Send + cmp::Eq + hash::Hash> ChainListener for SimpleManyChannelMonitor<Key> {
@@ -148,7 +154,7 @@ impl<Key : Send + cmp::Eq + hash::Hash> ChainListener for SimpleManyChannelMonit
                {
                        let mut monitors = self.monitors.lock().unwrap();
                        for monitor in monitors.values_mut() {
-                               let (txn_outputs, spendable_outputs, mut htlc_updated) = monitor.block_connected(txn_matched, height, &block_hash, &*self.broadcaster);
+                               let (txn_outputs, spendable_outputs, mut htlc_updated) = monitor.block_connected(txn_matched, height, &block_hash, &*self.broadcaster, &*self.fee_estimator);
                                if spendable_outputs.len() > 0 {
                                        new_events.push(events::Event::SpendableOutputs {
                                                outputs: spendable_outputs,
@@ -205,7 +211,7 @@ impl<Key : Send + cmp::Eq + hash::Hash> ChainListener for SimpleManyChannelMonit
 impl<Key : Send + cmp::Eq + hash::Hash + 'static> SimpleManyChannelMonitor<Key> {
        /// Creates a new object which can be used to monitor several channels given the chain
        /// interface with which to register to receive notifications.
-       pub fn new(chain_monitor: Arc<ChainWatchInterface>, broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>) -> Arc<SimpleManyChannelMonitor<Key>> {
+       pub fn new(chain_monitor: Arc<ChainWatchInterface>, broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>, feeest: Arc<FeeEstimator>) -> Arc<SimpleManyChannelMonitor<Key>> {
                let res = Arc::new(SimpleManyChannelMonitor {
                        monitors: Mutex::new(HashMap::new()),
                        chain_monitor,
@@ -213,13 +219,14 @@ impl<Key : Send + cmp::Eq + hash::Hash + 'static> SimpleManyChannelMonitor<Key>
                        pending_events: Mutex::new(Vec::new()),
                        pending_htlc_updated: Mutex::new(HashMap::new()),
                        logger,
+                       fee_estimator: feeest,
                });
                let weak_res = Arc::downgrade(&res);
                res.chain_monitor.register_listener(weak_res);
                res
        }
 
-       /// Adds or udpates the monitor which monitors the channel referred to by the given key.
+       /// Adds or updates the monitor which monitors the channel referred to by the given key.
        pub fn add_update_monitor_by_key(&self, key: Key, monitor: ChannelMonitor) -> Result<(), MonitorUpdateError> {
                let mut monitors = self.monitors.lock().unwrap();
                match monitors.get_mut(&key) {
@@ -299,7 +306,7 @@ pub(crate) const HTLC_FAIL_TIMEOUT_BLOCKS: u32 = 3;
 /// Number of blocks we wait on seeing a confirmed HTLC-Timeout or previous revoked commitment
 /// transaction before we fail corresponding inbound HTLCs. This prevents us from failing backwards
 /// and then getting a reorg resulting in us losing money.
-//TODO: We currently dont actually use this...we should
+//TODO: We currently don't actually use this...we should
 pub(crate) const HTLC_FAIL_ANTI_REORG_DELAY: u32 = 6;
 
 #[derive(Clone, PartialEq)]
@@ -335,6 +342,15 @@ struct LocalSignedTx {
        htlc_outputs: Vec<(HTLCOutputInCommitment, Option<(Signature, Signature)>, Option<HTLCSource>)>,
 }
 
+#[derive(PartialEq)]
+enum InputDescriptors {
+       RevokedOfferedHTLC,
+       RevokedReceivedHTLC,
+       OfferedHTLC,
+       ReceivedHTLC,
+       RevokedOutput, // either a revoked to_local output on commitment tx, a revoked HTLC-Timeout output or a revoked HTLC-Success output
+}
+
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
@@ -470,6 +486,36 @@ impl ChannelMonitor {
                }
        }
 
+       fn get_witnesses_weight(inputs: &[InputDescriptors]) -> u64 {
+               let mut tx_weight = 2; // count segwit flags
+               for inp in inputs {
+                       // We use expected weight (and not actual) as signatures and time lock delays may vary
+                       tx_weight +=  match inp {
+                               // number_of_witness_elements + sig_length + revocation_sig + pubkey_length + revocationpubkey + witness_script_length + witness_script
+                               &InputDescriptors::RevokedOfferedHTLC => {
+                                       1 + 1 + 73 + 1 + 33 + 1 + 133
+                               },
+                               // number_of_witness_elements + sig_length + revocation_sig + pubkey_length + revocationpubkey + witness_script_length + witness_script
+                               &InputDescriptors::RevokedReceivedHTLC => {
+                                       1 + 1 + 73 + 1 + 33 + 1 + 139
+                               },
+                               // number_of_witness_elements + sig_length + remotehtlc_sig  + preimage_length + preimage + witness_script_length + witness_script
+                               &InputDescriptors::OfferedHTLC => {
+                                       1 + 1 + 73 + 1 + 32 + 1 + 133
+                               },
+                               // number_of_witness_elements + sig_length + revocation_sig + pubkey_length + revocationpubkey + witness_script_length + witness_script
+                               &InputDescriptors::ReceivedHTLC => {
+                                       1 + 1 + 73 + 1 + 1 + 1 + 139
+                               },
+                               // number_of_witness_elements + sig_length + revocation_sig + true_length + op_true + witness_script_length + witness_script
+                               &InputDescriptors::RevokedOutput => {
+                                       1 + 1 + 73 + 1 + 1 + 1 + 77
+                               },
+                       };
+               }
+               tx_weight
+       }
+
        #[inline]
        fn place_secret(idx: u64) -> u8 {
                for i in 0..48 {
@@ -642,7 +688,7 @@ impl ChannelMonitor {
                                let our_funding_info = funding_info;
                                if let Storage::Local { ref funding_info, .. } = other.key_storage {
                                        if funding_info.is_none() { return Err(MonitorUpdateError("Try to combine a Local monitor without funding_info")); }
-                                       // We should be able to compare the entire funding_txo, but in fuzztarget its trivially
+                                       // We should be able to compare the entire funding_txo, but in fuzztarget it's trivially
                                        // easy to collide the funding_txo hash and have a different scriptPubKey.
                                        if funding_info.as_ref().unwrap().0 != our_funding_info.as_ref().unwrap().0 {
                                                return Err(MonitorUpdateError("Funding transaction outputs are not identical!"));
@@ -913,8 +959,8 @@ impl ChannelMonitor {
                                        serialize_htlc_in_commitment!(htlc_output);
                                        if let &Some((ref their_sig, ref our_sig)) = sigs {
                                                1u8.write(writer)?;
-                                               writer.write_all(&their_sig.serialize_compact(&self.secp_ctx))?;
-                                               writer.write_all(&our_sig.serialize_compact(&self.secp_ctx))?;
+                                               writer.write_all(&their_sig.serialize_compact())?;
+                                               writer.write_all(&our_sig.serialize_compact())?;
                                        } else {
                                                0u8.write(writer)?;
                                        }
@@ -1014,7 +1060,7 @@ impl ChannelMonitor {
        /// HTLC-Success/HTLC-Timeout transactions.
        /// Return updates for HTLC pending in the channel and failed automatically by the broadcast of
        /// revoked remote commitment tx
-       fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec<Transaction>, (Sha256dHash, Vec<TxOut>), Vec<SpendableOutputDescriptor>, Vec<(HTLCSource, Option<PaymentPreimage>, PaymentHash)>)  {
+       fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32, fee_estimator: &FeeEstimator) -> (Vec<Transaction>, (Sha256dHash, Vec<TxOut>), Vec<SpendableOutputDescriptor>, Vec<(HTLCSource, Option<PaymentPreimage>, PaymentHash)>)  {
                // Most secp and related errors trying to create keys means we have no hope of constructing
                // a spend transaction...so we return no transactions to broadcast
                let mut txn_to_broadcast = Vec::new();
@@ -1037,7 +1083,7 @@ impl ChannelMonitor {
                let commitment_number = 0xffffffffffff - ((((tx.input[0].sequence as u64 & 0xffffff) << 3*8) | (tx.lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
                if commitment_number >= self.get_min_seen_secret() {
                        let secret = self.get_secret(commitment_number).unwrap();
-                       let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret));
+                       let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
                        let (revocation_pubkey, b_htlc_key, local_payment_key) = match self.key_storage {
                                Storage::Local { ref revocation_base_key, ref htlc_base_key, ref payment_base_key, .. } => {
                                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
@@ -1065,13 +1111,14 @@ impl ChannelMonitor {
                                // Note that the Network here is ignored as we immediately drop the address for the
                                // script_pubkey version.
                                let payment_hash160 = Hash160::hash(&PublicKey::from_secret_key(&self.secp_ctx, &payment_key).serialize());
-                               Some(Builder::new().push_opcode(opcodes::All::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script())
+                               Some(Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script())
                        } else { None };
 
                        let mut total_value = 0;
                        let mut values = Vec::new();
                        let mut inputs = Vec::new();
                        let mut htlc_idxs = Vec::new();
+                       let mut input_descriptors = Vec::new();
 
                        for (idx, outp) in tx.output.iter().enumerate() {
                                if outp.script_pubkey == revokeable_p2wsh {
@@ -1087,6 +1134,7 @@ impl ChannelMonitor {
                                        htlc_idxs.push(None);
                                        values.push(outp.value);
                                        total_value += outp.value;
+                                       input_descriptors.push(InputDescriptors::RevokedOutput);
                                } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
                                        spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH {
                                                outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
@@ -1105,7 +1153,7 @@ impl ChannelMonitor {
                                                                        let htlc = &per_commitment_option.unwrap()[$htlc_idx.unwrap()].0;
                                                                        chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey)
                                                                };
-                                                               let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
+                                                               let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
                                                                let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
                                                                (self.secp_ctx.sign(&sighash, &revocation_key), redeemscript)
                                                        },
@@ -1113,7 +1161,7 @@ impl ChannelMonitor {
                                                                unimplemented!();
                                                        }
                                                };
-                                               $input.witness.push(sig.serialize_der(&self.secp_ctx).to_vec());
+                                               $input.witness.push(sig.serialize_der().to_vec());
                                                $input.witness[0].push(SigHashType::All as u8);
                                                if $htlc_idx.is_none() {
                                                        $input.witness.push(vec!(1));
@@ -1150,6 +1198,7 @@ impl ChannelMonitor {
                                                        htlc_idxs.push(Some(idx));
                                                        values.push(tx.output[transaction_output_index as usize].value);
                                                        total_value += htlc.amount_msat / 1000;
+                                                       input_descriptors.push(if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC });
                                                } else {
                                                        let mut single_htlc_tx = Transaction {
                                                                version: 2,
@@ -1157,9 +1206,10 @@ impl ChannelMonitor {
                                                                input: vec![input],
                                                                output: vec!(TxOut {
                                                                        script_pubkey: self.destination_script.clone(),
-                                                                       value: htlc.amount_msat / 1000, //TODO: - fee
+                                                                       value: htlc.amount_msat / 1000,
                                                                }),
                                                        };
+                                                       single_htlc_tx.output[0].value -= fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * (single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }])) / 1000;
                                                        let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
                                                        sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
                                                        txn_to_broadcast.push(single_htlc_tx);
@@ -1179,7 +1229,7 @@ impl ChannelMonitor {
                                // on-chain claims, so we can do that at the same time.
                                macro_rules! check_htlc_fails {
                                        ($txid: expr, $commitment_tx: expr) => {
-                                               if let Some(ref outpoints) = self.remote_claimable_outpoints.get(&$txid) {
+                                               if let Some(ref outpoints) = self.remote_claimable_outpoints.get($txid) {
                                                        for &(ref htlc, ref source_option) in outpoints.iter() {
                                                                if let &Some(ref source) = source_option {
                                                                        log_trace!(self, "Failing HTLC with payment_hash {} from {} remote commitment tx due to broadcast of revoked remote commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
@@ -1203,7 +1253,7 @@ impl ChannelMonitor {
 
                        let outputs = vec!(TxOut {
                                script_pubkey: self.destination_script.clone(),
-                               value: total_value, //TODO: - fee
+                               value: total_value,
                        });
                        let mut spend_tx = Transaction {
                                version: 2,
@@ -1211,6 +1261,7 @@ impl ChannelMonitor {
                                input: inputs,
                                output: outputs,
                        };
+                       spend_tx.output[0].value -= fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * (spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..])) / 1000;
 
                        let mut values_drain = values.drain(..);
                        let sighash_parts = bip143::SighashComponents::new(&spend_tx);
@@ -1243,7 +1294,7 @@ impl ChannelMonitor {
                        // on-chain claims, so we can do that at the same time.
                        macro_rules! check_htlc_fails {
                                ($txid: expr, $commitment_tx: expr, $id: tt) => {
-                                       if let Some(ref latest_outpoints) = self.remote_claimable_outpoints.get(&$txid) {
+                                       if let Some(ref latest_outpoints) = self.remote_claimable_outpoints.get($txid) {
                                                $id: for &(ref htlc, ref source_option) in latest_outpoints.iter() {
                                                        if let &Some(ref source) = source_option {
                                                                // Check if the HTLC is present in the commitment transaction that was
@@ -1319,6 +1370,7 @@ impl ChannelMonitor {
                                        let mut total_value = 0;
                                        let mut values = Vec::new();
                                        let mut inputs = Vec::new();
+                                       let mut input_descriptors = Vec::new();
 
                                        macro_rules! sign_input {
                                                ($sighash_parts: expr, $input: expr, $amount: expr, $preimage: expr) => {
@@ -1327,7 +1379,7 @@ impl ChannelMonitor {
                                                                        Storage::Local { ref htlc_base_key, .. } => {
                                                                                let htlc = &per_commitment_option.unwrap()[$input.sequence as usize].0;
                                                                                let redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
-                                                                               let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
+                                                                               let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
                                                                                let htlc_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key));
                                                                                (self.secp_ctx.sign(&sighash, &htlc_key), redeemscript)
                                                                        },
@@ -1335,7 +1387,7 @@ impl ChannelMonitor {
                                                                                unimplemented!();
                                                                        }
                                                                };
-                                                               $input.witness.push(sig.serialize_der(&self.secp_ctx).to_vec());
+                                                               $input.witness.push(sig.serialize_der().to_vec());
                                                                $input.witness[0].push(SigHashType::All as u8);
                                                                $input.witness.push($preimage);
                                                                $input.witness.push(redeemscript.into_bytes());
@@ -1365,6 +1417,7 @@ impl ChannelMonitor {
                                                                        inputs.push(input);
                                                                        values.push((tx.output[transaction_output_index as usize].value, payment_preimage));
                                                                        total_value += htlc.amount_msat / 1000;
+                                                                       input_descriptors.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC });
                                                                } else {
                                                                        let mut single_htlc_tx = Transaction {
                                                                                version: 2,
@@ -1372,9 +1425,10 @@ impl ChannelMonitor {
                                                                                input: vec![input],
                                                                                output: vec!(TxOut {
                                                                                        script_pubkey: self.destination_script.clone(),
-                                                                                       value: htlc.amount_msat / 1000, //TODO: - fee
+                                                                                       value: htlc.amount_msat / 1000,
                                                                                }),
                                                                        };
+                                                                       single_htlc_tx.output[0].value -= fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * (single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }])) / 1000;
                                                                        let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
                                                                        sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
                                                                        spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
@@ -1416,7 +1470,7 @@ impl ChannelMonitor {
 
                                        let outputs = vec!(TxOut {
                                                script_pubkey: self.destination_script.clone(),
-                                               value: total_value, //TODO: - fee
+                                               value: total_value
                                        });
                                        let mut spend_tx = Transaction {
                                                version: 2,
@@ -1424,6 +1478,7 @@ impl ChannelMonitor {
                                                input: inputs,
                                                output: outputs,
                                        };
+                                       spend_tx.output[0].value -= fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * (spend_tx.get_weight() + Self::get_witnesses_weight(&input_descriptors[..])) / 1000;
 
                                        let mut values_drain = values.drain(..);
                                        let sighash_parts = bip143::SighashComponents::new(&spend_tx);
@@ -1445,8 +1500,8 @@ impl ChannelMonitor {
                (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated)
        }
 
-       /// Attempst to claim a remote HTLC-Success/HTLC-Timeout s outputs using the revocation key
-       fn check_spend_remote_htlc(&self, tx: &Transaction, commitment_number: u64) -> (Option<Transaction>, Option<SpendableOutputDescriptor>) {
+       /// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key
+       fn check_spend_remote_htlc(&self, tx: &Transaction, commitment_number: u64, fee_estimator: &FeeEstimator) -> (Option<Transaction>, Option<SpendableOutputDescriptor>) {
                if tx.input.len() != 1 || tx.output.len() != 1 {
                        return (None, None)
                }
@@ -1461,7 +1516,7 @@ impl ChannelMonitor {
                }
 
                let secret = if let Some(secret) = self.get_secret(commitment_number) { secret } else { return (None, None); };
-               let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret));
+               let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
                let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
                let revocation_pubkey = match self.key_storage {
                        Storage::Local { ref revocation_base_key, .. } => {
@@ -1498,7 +1553,7 @@ impl ChannelMonitor {
                if !inputs.is_empty() {
                        let outputs = vec!(TxOut {
                                script_pubkey: self.destination_script.clone(),
-                               value: amount, //TODO: - fee
+                               value: amount
                        });
 
                        let mut spend_tx = Transaction {
@@ -1507,12 +1562,13 @@ impl ChannelMonitor {
                                input: inputs,
                                output: outputs,
                        };
+                       spend_tx.output[0].value -= fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * (spend_tx.get_weight() + Self::get_witnesses_weight(&vec![InputDescriptors::RevokedOutput])) / 1000;
 
                        let sighash_parts = bip143::SighashComponents::new(&spend_tx);
 
                        let sig = match self.key_storage {
                                Storage::Local { ref revocation_base_key, .. } => {
-                                       let sighash = ignore_error!(Message::from_slice(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]));
+                                       let sighash = hash_to_message!(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]);
                                        let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
                                        self.secp_ctx.sign(&sighash, &revocation_key)
                                }
@@ -1520,7 +1576,7 @@ impl ChannelMonitor {
                                        unimplemented!();
                                }
                        };
-                       spend_tx.input[0].witness.push(sig.serialize_der(&self.secp_ctx).to_vec());
+                       spend_tx.input[0].witness.push(sig.serialize_der().to_vec());
                        spend_tx.input[0].witness[0].push(SigHashType::All as u8);
                        spend_tx.input[0].witness.push(vec!(1));
                        spend_tx.input[0].witness.push(redeemscript.into_bytes());
@@ -1568,13 +1624,14 @@ impl ChannelMonitor {
                        if let Some(transaction_output_index) = htlc.transaction_output_index {
                                if let &Some((ref their_sig, ref our_sig)) = sigs {
                                        if htlc.offered {
+                                               log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions");
                                                let mut htlc_timeout_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
 
                                                htlc_timeout_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-                                               htlc_timeout_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
+                                               htlc_timeout_tx.input[0].witness.push(their_sig.serialize_der().to_vec());
                                                htlc_timeout_tx.input[0].witness[1].push(SigHashType::All as u8);
-                                               htlc_timeout_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
+                                               htlc_timeout_tx.input[0].witness.push(our_sig.serialize_der().to_vec());
                                                htlc_timeout_tx.input[0].witness[2].push(SigHashType::All as u8);
 
                                                htlc_timeout_tx.input[0].witness.push(Vec::new());
@@ -1584,13 +1641,14 @@ impl ChannelMonitor {
                                                res.push(htlc_timeout_tx);
                                        } else {
                                                if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
+                                                       log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions");
                                                        let mut htlc_success_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
 
                                                        htlc_success_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-                                                       htlc_success_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
+                                                       htlc_success_tx.input[0].witness.push(their_sig.serialize_der().to_vec());
                                                        htlc_success_tx.input[0].witness[1].push(SigHashType::All as u8);
-                                                       htlc_success_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
+                                                       htlc_success_tx.input[0].witness.push(our_sig.serialize_der().to_vec());
                                                        htlc_success_tx.input[0].witness[2].push(SigHashType::All as u8);
 
                                                        htlc_success_tx.input[0].witness.push(payment_preimage.0.to_vec());
@@ -1613,11 +1671,12 @@ impl ChannelMonitor {
        /// Should not be used if check_spend_revoked_transaction succeeds.
        fn check_spend_local_transaction(&self, tx: &Transaction, _height: u32) -> (Vec<Transaction>, Vec<SpendableOutputDescriptor>, (Sha256dHash, Vec<TxOut>)) {
                let commitment_txid = tx.txid();
-               // TODO: If we find a match here we need to fail back HTLCs that were't included in the
+               // TODO: If we find a match here we need to fail back HTLCs that weren't included in the
                // broadcast commitment transaction, either because they didn't meet dust or because they
                // weren't yet included in our commitment transaction(s).
                if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx {
                        if local_tx.txid == commitment_txid {
+                               log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim");
                                match self.key_storage {
                                        Storage::Local { ref delayed_payment_base_key, ref latest_per_commitment_point, .. } => {
                                                let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, latest_per_commitment_point, &Some(*delayed_payment_base_key));
@@ -1632,6 +1691,7 @@ impl ChannelMonitor {
                }
                if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx {
                        if local_tx.txid == commitment_txid {
+                               log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim");
                                match self.key_storage {
                                        Storage::Local { ref delayed_payment_base_key, ref prev_latest_per_commitment_point, .. } => {
                                                let (local_txn, spendable_outputs, watch_outputs) = self.broadcast_by_local_state(local_tx, prev_latest_per_commitment_point, &Some(*delayed_payment_base_key));
@@ -1653,7 +1713,7 @@ impl ChannelMonitor {
                        match self.key_storage {
                                Storage::Local { ref shutdown_pubkey, .. } =>  {
                                        let our_channel_close_key_hash = Hash160::hash(&shutdown_pubkey.serialize());
-                                       let shutdown_script = Builder::new().push_opcode(opcodes::All::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script();
+                                       let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script();
                                        for (idx, output) in tx.output.iter().enumerate() {
                                                if shutdown_script == output.script_pubkey {
                                                        return Some(SpendableOutputDescriptor::StaticOutput {
@@ -1665,7 +1725,7 @@ impl ChannelMonitor {
                                }
                                Storage::Watchtower { .. } => {
                                        //TODO: we need to ensure an offline client will generate the event when it
-                                       // cames back online after only the watchtower saw the transaction
+                                       // comes back online after only the watchtower saw the transaction
                                }
                        }
                }
@@ -1689,7 +1749,7 @@ impl ChannelMonitor {
                }
        }
 
-       fn block_connected(&mut self, txn_matched: &[&Transaction], height: u32, block_hash: &Sha256dHash, broadcaster: &BroadcasterInterface)-> (Vec<(Sha256dHash, Vec<TxOut>)>, Vec<SpendableOutputDescriptor>, Vec<(HTLCSource, Option<PaymentPreimage>, PaymentHash)>) {
+       fn block_connected(&mut self, txn_matched: &[&Transaction], height: u32, block_hash: &Sha256dHash, broadcaster: &BroadcasterInterface, fee_estimator: &FeeEstimator)-> (Vec<(Sha256dHash, Vec<TxOut>)>, Vec<SpendableOutputDescriptor>, Vec<(HTLCSource, Option<PaymentPreimage>, PaymentHash)>) {
                let mut watch_outputs = Vec::new();
                let mut spendable_outputs = Vec::new();
                let mut htlc_updated = Vec::new();
@@ -1710,7 +1770,7 @@ impl ChannelMonitor {
                                        }
                                };
                                if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) {
-                                       let (remote_txn, new_outputs, mut spendable_output, mut updated) = self.check_spend_remote_transaction(tx, height);
+                                       let (remote_txn, new_outputs, mut spendable_output, mut updated) = self.check_spend_remote_transaction(tx, height, fee_estimator);
                                        txn = remote_txn;
                                        spendable_outputs.append(&mut spendable_output);
                                        if !new_outputs.1.is_empty() {
@@ -1734,7 +1794,7 @@ impl ChannelMonitor {
                                        }
                                } else {
                                        if let Some(&(commitment_number, _)) = self.remote_commitment_txn_on_chain.get(&prevout.txid) {
-                                               let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number);
+                                               let (tx, spendable_output) = self.check_spend_remote_htlc(tx, commitment_number, fee_estimator);
                                                if let Some(tx) = tx {
                                                        txn.push(tx);
                                                }
@@ -1787,44 +1847,69 @@ impl ChannelMonitor {
        }
 
        pub(super) fn would_broadcast_at_height(&self, height: u32) -> bool {
-               // TODO: We need to consider HTLCs which weren't included in latest local commitment
-               // transaction (or in any of the latest two local commitment transactions). This probably
-               // needs to use the same logic as the revoked-tx-announe logic - checking the last two
-               // remote commitment transactions. This probably has implications for what data we need to
-               // store in local commitment transactions.
+               // We need to consider all HTLCs which are:
+               //  * in any unrevoked remote commitment transaction, as they could broadcast said
+               //    transactions and we'd end up in a race, or
+               //  * are in our latest local commitment transaction, as this is the thing we will
+               //    broadcast if we go on-chain.
                // Note that we consider HTLCs which were below dust threshold here - while they don't
                // strictly imply that we need to fail the channel, we need to go ahead and fail them back
                // to the source, and if we don't fail the channel we will have to ensure that the next
                // updates that peer sends us are update_fails, failing the channel if not. It's probably
                // easier to just fail the channel as this case should be rare enough anyway.
+               macro_rules! scan_commitment {
+                       ($htlcs: expr, $local_tx: expr) => {
+                               for ref htlc in $htlcs {
+                                       // For inbound HTLCs which we know the preimage for, we have to ensure we hit the
+                                       // chain with enough room to claim the HTLC without our counterparty being able to
+                                       // time out the HTLC first.
+                                       // For outbound HTLCs which our counterparty hasn't failed/claimed, our primary
+                                       // concern is being able to claim the corresponding inbound HTLC (on another
+                                       // channel) before it expires. In fact, we don't even really care if our
+                                       // counterparty here claims such an outbound HTLC after it expired as long as we
+                                       // can still claim the corresponding HTLC. Thus, to avoid needlessly hitting the
+                                       // chain when our counterparty is waiting for expiration to off-chain fail an HTLC
+                                       // we give ourselves a few blocks of headroom after expiration before going
+                                       // on-chain for an expired HTLC.
+                                       // Note that, to avoid a potential attack whereby a node delays claiming an HTLC
+                                       // from us until we've reached the point where we go on-chain with the
+                                       // corresponding inbound HTLC, we must ensure that outbound HTLCs go on chain at
+                                       // least CLTV_CLAIM_BUFFER blocks prior to the inbound HTLC.
+                                       //  aka outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS == height - CLTV_CLAIM_BUFFER
+                                       //      inbound_cltv == height + CLTV_CLAIM_BUFFER
+                                       //      outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS + CLTV_CLAIM_BUFFER <= inbound_cltv - CLTV_CLAIM_BUFFER
+                                       //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFFER <= inbound_cltv - outbound_cltv
+                                       //      CLTV_EXPIRY_DELTA <= inbound_cltv - outbound_cltv (by check in ChannelManager::decode_update_add_htlc_onion)
+                                       //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFFER <= CLTV_EXPIRY_DELTA
+                                       //  The final, above, condition is checked for statically in channelmanager
+                                       //  with CHECK_CLTV_EXPIRY_SANITY_2.
+                                       let htlc_outbound = $local_tx == htlc.offered;
+                                       if ( htlc_outbound && htlc.cltv_expiry + HTLC_FAIL_TIMEOUT_BLOCKS <= height) ||
+                                          (!htlc_outbound && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) {
+                                               log_info!(self, "Force-closing channel due to {} HTLC timeout, HTLC expiry is {}", if htlc_outbound { "outbound" } else { "inbound "}, htlc.cltv_expiry);
+                                               return true;
+                                       }
+                               }
+                       }
+               }
+
                if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
-                       for &(ref htlc, _, _) in cur_local_tx.htlc_outputs.iter() {
-                               // For inbound HTLCs which we know the preimage for, we have to ensure we hit the
-                               // chain with enough room to claim the HTLC without our counterparty being able to
-                               // time out the HTLC first.
-                               // For outbound HTLCs which our counterparty hasn't failed/claimed, our primary
-                               // concern is being able to claim the corresponding inbound HTLC (on another
-                               // channel) before it expires. In fact, we don't even really care if our
-                               // counterparty here claims such an outbound HTLC after it expired as long as we
-                               // can still claim the corresponding HTLC. Thus, to avoid needlessly hitting the
-                               // chain when our counterparty is waiting for expiration to off-chain fail an HTLC
-                               // we give ourselves a few blocks of headroom after expiration before going
-                               // on-chain for an expired HTLC.
-                               // Note that, to avoid a potential attack whereby a node delays claiming an HTLC
-                               // from us until we've reached the point where we go on-chain with the
-                               // corresponding inbound HTLC, we must ensure that outbound HTLCs go on chain at
-                               // least CLTV_CLAIM_BUFFER blocks prior to the inbound HTLC.
-                               //  aka outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS == height - CLTV_CLAIM_BUFFER
-                               //      inbound_cltv == height + CLTV_CLAIM_BUFFER
-                               //      outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS + CLTV_CLAIM_BUFER <= inbound_cltv - CLTV_CLAIM_BUFFER
-                               //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFER <= inbound_cltv - outbound_cltv
-                               //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFER <= CLTV_EXPIRY_DELTA
-                               if ( htlc.offered && htlc.cltv_expiry + HTLC_FAIL_TIMEOUT_BLOCKS <= height) ||
-                                  (!htlc.offered && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) {
-                                       return true;
+                       scan_commitment!(cur_local_tx.htlc_outputs.iter().map(|&(ref a, _, _)| a), true);
+               }
+
+               if let Storage::Local { ref current_remote_commitment_txid, ref prev_remote_commitment_txid, .. } = self.key_storage {
+                       if let &Some(ref txid) = current_remote_commitment_txid {
+                               if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
+                                       scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false);
+                               }
+                       }
+                       if let &Some(ref txid) = prev_remote_commitment_txid {
+                               if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
+                                       scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false);
                                }
                        }
                }
+
                false
        }
 
@@ -1843,8 +1928,9 @@ impl ChannelMonitor {
                        macro_rules! log_claim {
                                ($tx_info: expr, $local_tx: expr, $htlc: expr, $source_avail: expr) => {
                                        // We found the output in question, but aren't failing it backwards
-                                       // as we have no corresponding source. This implies either it is an
-                                       // inbound HTLC or an outbound HTLC on a revoked transaction.
+                                       // as we have no corresponding source and no valid remote commitment txid
+                                       // to try a weak source binding with same-hash, same-value still-valid offered HTLC.
+                                       // This implies either it is an inbound HTLC or an outbound HTLC on a revoked transaction.
                                        let outbound_htlc = $local_tx == $htlc.offered;
                                        if ($local_tx && revocation_sig_claim) ||
                                                        (outbound_htlc && !$source_avail && (accepted_preimage_claim || offered_preimage_claim)) {
@@ -1861,6 +1947,22 @@ impl ChannelMonitor {
                                }
                        }
 
+                       macro_rules! check_htlc_valid_remote {
+                               ($remote_txid: expr, $htlc_output: expr) => {
+                                       if let &Some(txid) = $remote_txid {
+                                               for &(ref pending_htlc, ref pending_source) in self.remote_claimable_outpoints.get(&txid).unwrap() {
+                                                       if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat {
+                                                               if let &Some(ref source) = pending_source {
+                                                                       log_claim!("revoked remote commitment tx", false, pending_htlc, true);
+                                                                       payment_data = Some(((**source).clone(), $htlc_output.payment_hash));
+                                                                       break;
+                                                               }
+                                                       }
+                                               }
+                                       }
+                               }
+                       }
+
                        macro_rules! scan_commitment {
                                ($htlcs: expr, $tx_info: expr, $local_tx: expr) => {
                                        for (ref htlc_output, source_option) in $htlcs {
@@ -1873,7 +1975,17 @@ impl ChannelMonitor {
                                                                // has timed out, or we screwed up. In any case, we should now
                                                                // resolve the source HTLC with the original sender.
                                                                payment_data = Some(((*source).clone(), htlc_output.payment_hash));
-                                                       } else {
+                                                       } else if !$local_tx {
+                                                               if let Storage::Local { ref current_remote_commitment_txid, .. } = self.key_storage {
+                                                                       check_htlc_valid_remote!(current_remote_commitment_txid, htlc_output);
+                                                               }
+                                                               if payment_data.is_none() {
+                                                                       if let Storage::Local { ref prev_remote_commitment_txid, .. } = self.key_storage {
+                                                                               check_htlc_valid_remote!(prev_remote_commitment_txid, htlc_output);
+                                                                       }
+                                                               }
+                                                       }
+                                                       if payment_data.is_none() {
                                                                log_claim!($tx_info, $local_tx, htlc_output, false);
                                                                continue 'outer_loop;
                                                        }
@@ -1931,13 +2043,6 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                }
                        }
                }
-               macro_rules! read_option { () => {
-                       match <u8 as Readable<R>>::read(reader)? {
-                               0 => None,
-                               1 => Some(Readable::read(reader)?),
-                               _ => return Err(DecodeError::InvalidValue),
-                       }
-               } }
 
                let _ver: u8 = Readable::read(reader)?;
                let min_ver: u8 = Readable::read(reader)?;
@@ -1954,16 +2059,8 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                let delayed_payment_base_key = Readable::read(reader)?;
                                let payment_base_key = Readable::read(reader)?;
                                let shutdown_pubkey = Readable::read(reader)?;
-                               let prev_latest_per_commitment_point = match <u8 as Readable<R>>::read(reader)? {
-                                       0 => None,
-                                       1 => Some(Readable::read(reader)?),
-                                       _ => return Err(DecodeError::InvalidValue),
-                               };
-                               let latest_per_commitment_point = match <u8 as Readable<R>>::read(reader)? {
-                                       0 => None,
-                                       1 => Some(Readable::read(reader)?),
-                                       _ => return Err(DecodeError::InvalidValue),
-                               };
+                               let prev_latest_per_commitment_point = Readable::read(reader)?;
+                               let latest_per_commitment_point = Readable::read(reader)?;
                                // Technically this can fail and serialize fail a round-trip, but only for serialization of
                                // barely-init'd ChannelMonitors that we can't do anything with.
                                let outpoint = OutPoint {
@@ -1971,8 +2068,8 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                        index: Readable::read(reader)?,
                                };
                                let funding_info = Some((outpoint, Readable::read(reader)?));
-                               let current_remote_commitment_txid = read_option!();
-                               let prev_remote_commitment_txid = read_option!();
+                               let current_remote_commitment_txid = Readable::read(reader)?;
+                               let prev_remote_commitment_txid = Readable::read(reader)?;
                                Storage::Local {
                                        revocation_base_key,
                                        htlc_base_key,
@@ -2002,7 +2099,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                if second_point_slice[0..32] == [0; 32] && second_point_slice[32] == 0 {
                                        Some((first_idx, first_point, None))
                                } else {
-                                       Some((first_idx, first_point, Some(unwrap_obj!(PublicKey::from_slice(&secp_ctx, &second_point_slice)))))
+                                       Some((first_idx, first_point, Some(unwrap_obj!(PublicKey::from_slice(&second_point_slice)))))
                                }
                        }
                };
@@ -2023,7 +2120,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                        let amount_msat: u64 = Readable::read(reader)?;
                                        let cltv_expiry: u32 = Readable::read(reader)?;
                                        let payment_hash: PaymentHash = Readable::read(reader)?;
-                                       let transaction_output_index: Option<u32> = read_option!();
+                                       let transaction_output_index: Option<u32> = Readable::read(reader)?;
 
                                        HTLCOutputInCommitment {
                                                offered, amount_msat, cltv_expiry, payment_hash, transaction_output_index
@@ -2039,7 +2136,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                        let htlcs_count: u64 = Readable::read(reader)?;
                        let mut htlcs = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / 32));
                        for _ in 0..htlcs_count {
-                               htlcs.push((read_htlc_in_commitment!(), read_option!().map(|o: HTLCSource| Box::new(o))));
+                               htlcs.push((read_htlc_in_commitment!(), <Option<HTLCSource> as Readable<R>>::read(reader)?.map(|o: HTLCSource| Box::new(o))));
                        }
                        if let Some(_) = remote_claimable_outpoints.insert(txid, htlcs) {
                                return Err(DecodeError::InvalidValue);
@@ -2102,7 +2199,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                                        1 => Some((Readable::read(reader)?, Readable::read(reader)?)),
                                                        _ => return Err(DecodeError::InvalidValue),
                                                };
-                                               htlcs.push((htlc, sigs, read_option!()));
+                                               htlcs.push((htlc, sigs, Readable::read(reader)?));
                                        }
 
                                        LocalSignedTx {
@@ -2178,13 +2275,19 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
 
 #[cfg(test)]
 mod tests {
-       use bitcoin::blockdata::script::Script;
-       use bitcoin::blockdata::transaction::Transaction;
+       use bitcoin::blockdata::script::{Script, Builder};
+       use bitcoin::blockdata::opcodes;
+       use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType};
+       use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
+       use bitcoin::util::bip143;
        use bitcoin_hashes::Hash;
        use bitcoin_hashes::sha256::Hash as Sha256;
+       use bitcoin_hashes::sha256d::Hash as Sha256dHash;
+       use bitcoin_hashes::hex::FromHex;
        use hex;
        use ln::channelmanager::{PaymentPreimage, PaymentHash};
-       use ln::channelmonitor::ChannelMonitor;
+       use ln::channelmonitor::{ChannelMonitor, InputDescriptors};
+       use ln::chan_utils;
        use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys};
        use util::test_utils::TestLogger;
        use secp256k1::key::{SecretKey,PublicKey};
@@ -2214,7 +2317,7 @@ mod tests {
 
                {
                        // insert_secret correct sequence
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2260,7 +2363,7 @@ mod tests {
 
                {
                        // insert_secret #1 incorrect
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2276,7 +2379,7 @@ mod tests {
 
                {
                        // insert_secret #2 incorrect (#1 derived from incorrect)
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2302,7 +2405,7 @@ mod tests {
 
                {
                        // insert_secret #3 incorrect
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2328,7 +2431,7 @@ mod tests {
 
                {
                        // insert_secret #4 incorrect (1,2,3 derived from incorrect)
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2374,7 +2477,7 @@ mod tests {
 
                {
                        // insert_secret #5 incorrect
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2410,7 +2513,7 @@ mod tests {
 
                {
                        // insert_secret #6 incorrect (5 derived from incorrect)
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2456,7 +2559,7 @@ mod tests {
 
                {
                        // insert_secret #7 incorrect
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2502,7 +2605,7 @@ mod tests {
 
                {
                        // insert_secret #8 incorrect
-                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+                       monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                        secrets.clear();
 
                        secrets.push([0; 32]);
@@ -2552,7 +2655,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(TestLogger::new());
 
-               let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap());
+               let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                macro_rules! dummy_keys {
                        () => {
                                {
@@ -2617,7 +2720,7 @@ mod tests {
 
                // Prune with one old state and a local commitment tx holding a few overlaps with the
                // old state.
-               let mut monitor = ChannelMonitor::new(&SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[43; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &SecretKey::from_slice(&secp_ctx, &[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[45; 32]).unwrap()), 0, Script::new(), logger.clone());
+               let mut monitor = ChannelMonitor::new(&SecretKey::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[43; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &SecretKey::from_slice(&[44; 32]).unwrap(), &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()), 0, Script::new(), logger.clone());
                monitor.set_their_to_self_delay(10);
 
                monitor.provide_latest_local_commitment_tx_info(dummy_tx.clone(), dummy_keys!(), 0, preimages_to_local_htlcs!(preimages[0..10]));
@@ -2661,5 +2764,117 @@ mod tests {
                test_preimages_exist!(&preimages[0..5], monitor);
        }
 
+       #[test]
+       fn test_claim_txn_weight_computation() {
+               // We test Claim txn weight, knowing that we want expected weigth and
+               // not actual case to avoid sigs and time-lock delays hell variances.
+
+               let secp_ctx = Secp256k1::new();
+               let privkey = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
+               let pubkey = PublicKey::from_secret_key(&secp_ctx, &privkey);
+               let mut sum_actual_sigs: u64 = 0;
+
+               macro_rules! sign_input {
+                       ($sighash_parts: expr, $input: expr, $idx: expr, $amount: expr, $input_type: expr, $sum_actual_sigs: expr) => {
+                               let htlc = HTLCOutputInCommitment {
+                                       offered: if *$input_type == InputDescriptors::RevokedOfferedHTLC || *$input_type == InputDescriptors::OfferedHTLC { true } else { false },
+                                       amount_msat: 0,
+                                       cltv_expiry: 2 << 16,
+                                       payment_hash: PaymentHash([1; 32]),
+                                       transaction_output_index: Some($idx),
+                               };
+                               let redeem_script = if *$input_type == InputDescriptors::RevokedOutput { chan_utils::get_revokeable_redeemscript(&pubkey, 256, &pubkey) } else { chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &pubkey, &pubkey, &pubkey) };
+                               let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeem_script, $amount)[..]);
+                               let sig = secp_ctx.sign(&sighash, &privkey);
+                               $input.witness.push(sig.serialize_der().to_vec());
+                               $input.witness[0].push(SigHashType::All as u8);
+                               sum_actual_sigs += $input.witness[0].len() as u64;
+                               if *$input_type == InputDescriptors::RevokedOutput {
+                                       $input.witness.push(vec!(1));
+                               } else if *$input_type == InputDescriptors::RevokedOfferedHTLC || *$input_type == InputDescriptors::RevokedReceivedHTLC {
+                                       $input.witness.push(pubkey.clone().serialize().to_vec());
+                               } else if *$input_type == InputDescriptors::ReceivedHTLC {
+                                       $input.witness.push(vec![0]);
+                               } else {
+                                       $input.witness.push(PaymentPreimage([1; 32]).0.to_vec());
+                               }
+                               $input.witness.push(redeem_script.into_bytes());
+                               println!("witness[0] {}", $input.witness[0].len());
+                               println!("witness[1] {}", $input.witness[1].len());
+                               println!("witness[2] {}", $input.witness[2].len());
+                       }
+               }
+
+               let script_pubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script();
+               let txid = Sha256dHash::from_hex("56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d").unwrap();
+
+               // Justice tx with 1 to_local, 2 revoked offered HTLCs, 1 revoked received HTLCs
+               let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
+               for i in 0..4 {
+                       claim_tx.input.push(TxIn {
+                               previous_output: BitcoinOutPoint {
+                                       txid,
+                                       vout: i,
+                               },
+                               script_sig: Script::new(),
+                               sequence: 0xfffffffd,
+                               witness: Vec::new(),
+                       });
+               }
+               claim_tx.output.push(TxOut {
+                       script_pubkey: script_pubkey.clone(),
+                       value: 0,
+               });
+               let base_weight = claim_tx.get_weight();
+               let sighash_parts = bip143::SighashComponents::new(&claim_tx);
+               let inputs_des = vec![InputDescriptors::RevokedOutput, InputDescriptors::RevokedOfferedHTLC, InputDescriptors::RevokedOfferedHTLC, InputDescriptors::RevokedReceivedHTLC];
+               for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() {
+                       sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs);
+               }
+               assert_eq!(base_weight + ChannelMonitor::get_witnesses_weight(&inputs_des),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() as u64 - sum_actual_sigs));
+
+               // Claim tx with 1 offered HTLCs, 3 received HTLCs
+               claim_tx.input.clear();
+               sum_actual_sigs = 0;
+               for i in 0..4 {
+                       claim_tx.input.push(TxIn {
+                               previous_output: BitcoinOutPoint {
+                                       txid,
+                                       vout: i,
+                               },
+                               script_sig: Script::new(),
+                               sequence: 0xfffffffd,
+                               witness: Vec::new(),
+                       });
+               }
+               let base_weight = claim_tx.get_weight();
+               let sighash_parts = bip143::SighashComponents::new(&claim_tx);
+               let inputs_des = vec![InputDescriptors::OfferedHTLC, InputDescriptors::ReceivedHTLC, InputDescriptors::ReceivedHTLC, InputDescriptors::ReceivedHTLC];
+               for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() {
+                       sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs);
+               }
+               assert_eq!(base_weight + ChannelMonitor::get_witnesses_weight(&inputs_des),  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() as u64 - sum_actual_sigs));
+
+               // Justice tx with 1 revoked HTLC-Success tx output
+               claim_tx.input.clear();
+               sum_actual_sigs = 0;
+               claim_tx.input.push(TxIn {
+                       previous_output: BitcoinOutPoint {
+                               txid,
+                               vout: 0,
+                       },
+                       script_sig: Script::new(),
+                       sequence: 0xfffffffd,
+                       witness: Vec::new(),
+               });
+               let base_weight = claim_tx.get_weight();
+               let sighash_parts = bip143::SighashComponents::new(&claim_tx);
+               let inputs_des = vec![InputDescriptors::RevokedOutput];
+               for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() {
+                       sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs);
+               }
+               assert_eq!(base_weight + ChannelMonitor::get_witnesses_weight(&inputs_des), claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_des.len() as u64 - sum_actual_sigs));
+       }
+
        // Further testing is done in the ChannelManager integration tests.
 }