Fix HTLC-output-in-commitment sorting for duplicate-HTLCs
[rust-lightning] / src / ln / channel.rs
index ec5d83bbd3f305fcc33f71513b789a9c07f2ee25..3d883cd76260b899a9d43ecc71e663829f68def6 100644 (file)
@@ -2,13 +2,14 @@ use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
 use bitcoin::blockdata::opcodes;
-use bitcoin::util::hash::{BitcoinHash, Sha256dHash};
+use bitcoin::util::hash::BitcoinHash;
 use bitcoin::util::bip143;
 use bitcoin::consensus::encode::{self, Encodable, Decodable};
 
 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 secp256k1::key::{PublicKey,SecretKey};
 use secp256k1::{Secp256k1,Signature};
@@ -25,7 +26,7 @@ use chain::transaction::OutPoint;
 use chain::keysinterface::{ChannelKeys, KeysInterface};
 use util::transaction_utils;
 use util::ser::{Readable, ReadableArgs, Writeable, Writer, WriterWriteAdaptor};
-use util::logger::Logger;
+use util::logger::{Logger, LogHolder};
 use util::errors::APIError;
 use util::config::{UserConfig,ChannelConfig};
 
@@ -783,6 +784,8 @@ impl Channel {
                let mut local_htlc_total_msat = 0;
                let mut value_to_self_msat_offset = 0;
 
+               log_trace!(self, "Building commitment transaction number {} for {}, generated by {} with fee {}...", commitment_number, if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
+
                macro_rules! get_htlc_in_commitment {
                        ($htlc: expr, $offered: expr) => {
                                HTLCOutputInCommitment {
@@ -796,25 +799,29 @@ impl Channel {
                }
 
                macro_rules! add_htlc_output {
-                       ($htlc: expr, $outbound: expr, $source: expr) => {
+                       ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => {
                                if $outbound == local { // "offered HTLC output"
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
+                                               log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                txouts.push((TxOut {
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
+                                               log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                } else {
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
+                                               log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                txouts.push((TxOut { // "received HTLC output"
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
+                                               log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                }
@@ -822,18 +829,19 @@ impl Channel {
                }
 
                for ref htlc in self.pending_inbound_htlcs.iter() {
-                       let include = match htlc.state {
-                               InboundHTLCState::RemoteAnnounced(_) => !generated_by_local,
-                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => !generated_by_local,
-                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => true,
-                               InboundHTLCState::Committed => true,
-                               InboundHTLCState::LocalRemoved(_) => !generated_by_local,
+                       let (include, state_name) = match htlc.state {
+                               InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
+                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => (!generated_by_local, "AwaitingRemoteRevokeToAnnounce"),
+                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => (true, "AwaitingAnnouncedRemoteRevoke"),
+                               InboundHTLCState::Committed => (true, "Committed"),
+                               InboundHTLCState::LocalRemoved(_) => (!generated_by_local, "LocalRemoved"),
                        };
 
                        if include {
-                               add_htlc_output!(htlc, false, None);
+                               add_htlc_output!(htlc, false, None, state_name);
                                remote_htlc_total_msat += htlc.amount_msat;
                        } else {
+                               log_trace!(self, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
                                match &htlc.state {
                                        &InboundHTLCState::LocalRemoved(ref reason) => {
                                                if generated_by_local {
@@ -848,18 +856,19 @@ impl Channel {
                }
 
                for ref htlc in self.pending_outbound_htlcs.iter() {
-                       let include = match htlc.state {
-                               OutboundHTLCState::LocalAnnounced(_) => generated_by_local,
-                               OutboundHTLCState::Committed => true,
-                               OutboundHTLCState::RemoteRemoved => generated_by_local,
-                               OutboundHTLCState::AwaitingRemoteRevokeToRemove => generated_by_local,
-                               OutboundHTLCState::AwaitingRemovedRemoteRevoke => false,
+                       let (include, state_name) = match htlc.state {
+                               OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
+                               OutboundHTLCState::Committed => (true, "Committed"),
+                               OutboundHTLCState::RemoteRemoved => (generated_by_local, "RemoteRemoved"),
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove => (generated_by_local, "AwaitingRemoteRevokeToRemove"),
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke => (false, "AwaitingRemovedRemoteRevoke"),
                        };
 
                        if include {
-                               add_htlc_output!(htlc, true, Some(&htlc.source));
+                               add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
                                local_htlc_total_msat += htlc.amount_msat;
                        } else {
+                               log_trace!(self, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
                                match htlc.state {
                                        OutboundHTLCState::AwaitingRemoteRevokeToRemove|OutboundHTLCState::AwaitingRemovedRemoteRevoke => {
                                                if htlc.fail_reason.is_none() {
@@ -925,7 +934,19 @@ impl Channel {
                        }, None));
                }
 
-               transaction_utils::sort_outputs(&mut txouts);
+               transaction_utils::sort_outputs(&mut txouts, |a, b| {
+                       if let &Some(ref a_htlc) = a {
+                               if let &Some(ref b_htlc) = b {
+                                       a_htlc.0.cltv_expiry.cmp(&b_htlc.0.cltv_expiry)
+                                               // Note that due to hash collisions, we have to have a fallback comparison
+                                               // here for fuzztarget mode (otherwise at least chanmon_fail_consistency
+                                               // may fail)!
+                                               .then(a_htlc.0.payment_hash.0.cmp(&b_htlc.0.payment_hash.0))
+                               // For non-HTLC outputs, if they're copying our SPK we don't really care if we
+                               // close the channel due to mismatches - they're doing something dumb:
+                               } else { cmp::Ordering::Equal }
+                       } else { cmp::Ordering::Equal }
+               });
 
                let mut outputs: Vec<TxOut> = Vec::with_capacity(txouts.len());
                let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(txouts.len() + included_dust_htlcs.len());
@@ -1001,7 +1022,7 @@ impl Channel {
                        }, ()));
                }
 
-               transaction_utils::sort_outputs(&mut txouts);
+               transaction_utils::sort_outputs(&mut txouts, |_, _| { cmp::Ordering::Equal }); // Ordering doesnt matter if they used our pubkey...
 
                let mut outputs: Vec<TxOut> = Vec::new();
                for out in txouts.drain(..) {
@@ -1217,6 +1238,7 @@ impl Channel {
                                        _ => {}
                                }
                        }
+                       log_trace!(self, "Adding HTLC claim to holding_cell! Current state: {}", self.channel_state);
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
@@ -1230,6 +1252,7 @@ impl Channel {
                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
                                return Ok((None, Some(self.channel_monitor.clone())));
                        }
+                       log_trace!(self, "Upgrading HTLC {} to LocalRemoved with a Fulfill!", log_bytes!(htlc.payment_hash.0));
                        htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone()));
                }
 
@@ -1712,6 +1735,7 @@ impl Channel {
                };
                let local_commitment_txid = local_commitment_tx.0.txid();
                let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
+               log_trace!(self, "Checking commitment tx signature {} by key {} against tx {} with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey.unwrap().serialize()), encode::serialize_hex(&local_commitment_tx.0), encode::serialize_hex(&funding_script));
                secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid commitment tx signature from peer");
 
                //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
@@ -1737,6 +1761,7 @@ impl Channel {
                        if let Some(_) = htlc.transaction_output_index {
                                let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
+                               log_trace!(self, "Checking HTLC tx signature {} by key {} against tx {} with redeemscript {}", log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(local_keys.b_htlc_key.serialize()), encode::serialize_hex(&htlc_tx), encode::serialize_hex(&htlc_redeemscript));
                                let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
                                secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
                                let htlc_sig = if htlc.offered {
@@ -1982,74 +2007,89 @@ impl Channel {
                        self.monitor_pending_order = None;
                }
 
+               log_trace!(self, "Updating HTLCs on receipt of RAA...");
                let mut to_forward_infos = Vec::new();
                let mut revoked_htlcs = Vec::new();
                let mut update_fail_htlcs = Vec::new();
                let mut update_fail_malformed_htlcs = Vec::new();
                let mut require_commitment = false;
                let mut value_to_self_msat_diff: i64 = 0;
-               // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
-               self.pending_inbound_htlcs.retain(|htlc| {
-                       if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
-                               if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
-                                       value_to_self_msat_diff += htlc.amount_msat as i64;
-                               }
-                               false
-                       } else { true }
-               });
-               self.pending_outbound_htlcs.retain(|htlc| {
-                       if let OutboundHTLCState::AwaitingRemovedRemoteRevoke = htlc.state {
-                               if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
-                                       revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
-                               } else {
-                                       // They fulfilled, so we sent them money
-                                       value_to_self_msat_diff -= htlc.amount_msat as i64;
-                               }
-                               false
-                       } else { true }
-               });
-               for htlc in self.pending_inbound_htlcs.iter_mut() {
-                       let swap = if let &InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) = &htlc.state {
-                               true
-                       } else if let &InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) = &htlc.state {
-                               true
-                       } else { false };
-                       if swap {
-                               let mut state = InboundHTLCState::Committed;
-                               mem::swap(&mut state, &mut htlc.state);
-
-                               if let InboundHTLCState::AwaitingRemoteRevokeToAnnounce(forward_info) = state {
-                                       htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info);
-                                       require_commitment = true;
-                               } else if let InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info) = state {
-                                       match forward_info {
-                                               PendingHTLCStatus::Fail(fail_msg) => {
-                                                       require_commitment = true;
-                                                       match fail_msg {
-                                                               HTLCFailureMsg::Relay(msg) => {
-                                                                       htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
-                                                                       update_fail_htlcs.push(msg)
-                                                               },
-                                                               HTLCFailureMsg::Malformed(msg) => {
-                                                                       htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed((msg.sha256_of_onion, msg.failure_code)));
-                                                                       update_fail_malformed_htlcs.push(msg)
-                                                               },
+
+               {
+                       // Take references explicitly so that we can hold multiple references to self.
+                       let pending_inbound_htlcs: &mut Vec<_> = &mut self.pending_inbound_htlcs;
+                       let pending_outbound_htlcs: &mut Vec<_> = &mut self.pending_outbound_htlcs;
+                       let logger = LogHolder { logger: &self.logger };
+
+                       // We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
+                       pending_inbound_htlcs.retain(|htlc| {
+                               if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
+                                       log_trace!(logger, " ...removing inbound LocalRemoved {}", log_bytes!(htlc.payment_hash.0));
+                                       if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+                                               value_to_self_msat_diff += htlc.amount_msat as i64;
+                                       }
+                                       false
+                               } else { true }
+                       });
+                       pending_outbound_htlcs.retain(|htlc| {
+                               if let OutboundHTLCState::AwaitingRemovedRemoteRevoke = htlc.state {
+                                       log_trace!(logger, " ...removing outbound AwaitingRemovedRemoteRevoke {}", log_bytes!(htlc.payment_hash.0));
+                                       if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
+                                               revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
+                                       } else {
+                                               // They fulfilled, so we sent them money
+                                               value_to_self_msat_diff -= htlc.amount_msat as i64;
+                                       }
+                                       false
+                               } else { true }
+                       });
+                       for htlc in pending_inbound_htlcs.iter_mut() {
+                               let swap = if let &InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) = &htlc.state {
+                                       log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce {} to Committed", log_bytes!(htlc.payment_hash.0));
+                                       true
+                               } else if let &InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) = &htlc.state {
+                                       log_trace!(logger, " ...promoting inbound AwaitingAnnouncedRemoteRevoke {} to Committed", log_bytes!(htlc.payment_hash.0));
+                                       true
+                               } else { false };
+                               if swap {
+                                       let mut state = InboundHTLCState::Committed;
+                                       mem::swap(&mut state, &mut htlc.state);
+
+                                       if let InboundHTLCState::AwaitingRemoteRevokeToAnnounce(forward_info) = state {
+                                               htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info);
+                                               require_commitment = true;
+                                       } else if let InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info) = state {
+                                               match forward_info {
+                                                       PendingHTLCStatus::Fail(fail_msg) => {
+                                                               require_commitment = true;
+                                                               match fail_msg {
+                                                                       HTLCFailureMsg::Relay(msg) => {
+                                                                               htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
+                                                                               update_fail_htlcs.push(msg)
+                                                                       },
+                                                                       HTLCFailureMsg::Malformed(msg) => {
+                                                                               htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed((msg.sha256_of_onion, msg.failure_code)));
+                                                                               update_fail_malformed_htlcs.push(msg)
+                                                                       },
+                                                               }
+                                                       },
+                                                       PendingHTLCStatus::Forward(forward_info) => {
+                                                               to_forward_infos.push((forward_info, htlc.htlc_id));
+                                                               htlc.state = InboundHTLCState::Committed;
                                                        }
-                                               },
-                                               PendingHTLCStatus::Forward(forward_info) => {
-                                                       to_forward_infos.push((forward_info, htlc.htlc_id));
-                                                       htlc.state = InboundHTLCState::Committed;
                                                }
                                        }
                                }
                        }
-               }
-               for htlc in self.pending_outbound_htlcs.iter_mut() {
-                       if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
-                               htlc.state = OutboundHTLCState::Committed;
-                       } else if let OutboundHTLCState::AwaitingRemoteRevokeToRemove = htlc.state {
-                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
-                               require_commitment = true;
+                       for htlc in pending_outbound_htlcs.iter_mut() {
+                               if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
+                                       log_trace!(logger, " ...promoting outbound LocalAnnounced {} to Committed", log_bytes!(htlc.payment_hash.0));
+                                       htlc.state = OutboundHTLCState::Committed;
+                               } else if let OutboundHTLCState::AwaitingRemoteRevokeToRemove = htlc.state {
+                                       log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
+                                       htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
+                                       require_commitment = true;
+                               }
                        }
                }
                self.value_to_self_msat = (self.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
@@ -2346,6 +2386,8 @@ impl Channel {
                        }
                }
 
+               log_trace!(self, "Regenerated latest commitment update with {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds",
+                               update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len());
                msgs::CommitmentUpdate {
                        update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs,
                        update_fee: None, //TODO: We need to support re-generating any update_fees in the last commitment_signed!
@@ -3114,7 +3156,7 @@ impl Channel {
                        excess_data: Vec::new(),
                };
 
-               let msghash = hash_to_message!(&Sha256dHash::from_data(&msg.encode()[..])[..]);
+               let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
                let sig = self.secp_ctx.sign(&msghash, &self.local_keys.funding_key);
 
                Ok((msg, sig))
@@ -3320,6 +3362,7 @@ impl Channel {
                let remote_commitment_txid = remote_commitment_tx.0.txid();
                let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
                let our_sig = self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key);
+               log_trace!(self, "Signing remote commitment tx {} with redeemscript {} with pubkey {} -> {}", encode::serialize_hex(&remote_commitment_tx.0), encode::serialize_hex(&funding_script), log_bytes!(PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key).serialize()), log_bytes!(our_sig.serialize_compact()[..]));
 
                let mut htlc_sigs = Vec::with_capacity(remote_commitment_tx.1);
                for &(ref htlc, _) in remote_commitment_tx.2.iter() {
@@ -3329,6 +3372,7 @@ impl Channel {
                                let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
                                let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
                                htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
+                               log_trace!(self, "Signing remote HTLC tx {} with redeemscript {} with pubkey {} -> {}", encode::serialize_hex(&htlc_tx), encode::serialize_hex(&htlc_redeemscript), log_bytes!(PublicKey::from_secret_key(&self.secp_ctx, &our_htlc_key).serialize()), log_bytes!(htlc_sigs.last().unwrap().serialize_compact()[..]));
                        }
                }
 
@@ -3923,12 +3967,12 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 
 #[cfg(test)]
 mod tests {
-       use bitcoin::util::hash::{Sha256dHash, Hash160};
        use bitcoin::util::bip143;
        use bitcoin::consensus::encode::serialize;
        use bitcoin::blockdata::script::{Script, Builder};
        use bitcoin::blockdata::transaction::Transaction;
        use bitcoin::blockdata::opcodes;
+       use bitcoin_hashes::hex::FromHex;
        use hex;
        use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
        use ln::channel::{Channel,ChannelKeys,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,TxCreationKeys};
@@ -3943,6 +3987,8 @@ mod tests {
        use secp256k1::{Secp256k1,Message,Signature};
        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 std::sync::Arc;
 
@@ -3969,7 +4015,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::from_data(&PublicKey::from_secret_key(&secp_ctx, &channel_monitor_claim_key).serialize());
+                       let our_channel_monitor_claim_key_hash = Hash160::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()
                }