Correctly detect missing HTLCs when a local commitment tx was broadcast
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 42c3e13a34feeff236a1b19712470808f0b19f69..b5f6bce228a3886ee17d337ddcac2f103ba32cd7 100644 (file)
@@ -53,9 +53,9 @@ use util::events::Event;
 
 use prelude::*;
 use core::{cmp, mem};
-use std::io::Error;
+use io::{self, Error};
 use core::ops::Deref;
-use std::sync::Mutex;
+use sync::Mutex;
 
 /// An update generated by the underlying Channel itself which contains some new information the
 /// ChannelMonitor should be made aware of.
@@ -88,7 +88,7 @@ pub struct ChannelMonitorUpdate {
 pub const CLOSED_CHANNEL_UPDATE_ID: u64 = core::u64::MAX;
 
 impl Writeable for ChannelMonitorUpdate {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
                write_ver_prefix!(w, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
                self.update_id.write(w)?;
                (self.updates.len() as u64).write(w)?;
@@ -100,7 +100,7 @@ impl Writeable for ChannelMonitorUpdate {
        }
 }
 impl Readable for ChannelMonitorUpdate {
-       fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
                let _ver = read_ver_prefix!(r, SERIALIZATION_VERSION);
                let update_id: u64 = Readable::read(r)?;
                let len: u64 = Readable::read(r)?;
@@ -199,10 +199,12 @@ pub enum MonitorEvent {
 pub struct HTLCUpdate {
        pub(crate) payment_hash: PaymentHash,
        pub(crate) payment_preimage: Option<PaymentPreimage>,
-       pub(crate) source: HTLCSource
+       pub(crate) source: HTLCSource,
+       pub(crate) onchain_value_satoshis: Option<u64>,
 }
 impl_writeable_tlv_based!(HTLCUpdate, {
        (0, payment_hash, required),
+       (1, onchain_value_satoshis, option),
        (2, source, required),
        (4, payment_preimage, option),
 });
@@ -293,7 +295,7 @@ struct CounterpartyCommitmentTransaction {
 }
 
 impl Writeable for CounterpartyCommitmentTransaction {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
                w.write_all(&byte_utils::be64_to_array(self.per_htlc.len() as u64))?;
                for (ref txid, ref htlcs) in self.per_htlc.iter() {
                        w.write_all(&txid[..])?;
@@ -311,7 +313,7 @@ impl Writeable for CounterpartyCommitmentTransaction {
        }
 }
 impl Readable for CounterpartyCommitmentTransaction {
-       fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
                let counterparty_commitment_transaction = {
                        let per_htlc_len: u64 = Readable::read(r)?;
                        let mut per_htlc = HashMap::with_capacity(cmp::min(per_htlc_len as usize, MAX_ALLOC_SIZE / 64));
@@ -385,6 +387,7 @@ enum OnchainEvent {
        HTLCUpdate {
                source: HTLCSource,
                payment_hash: PaymentHash,
+               onchain_value_satoshis: Option<u64>,
        },
        MaturingOutput {
                descriptor: SpendableOutputDescriptor,
@@ -400,6 +403,7 @@ impl_writeable_tlv_based!(OnchainEventEntry, {
 impl_writeable_tlv_based_enum!(OnchainEvent,
        (0, HTLCUpdate) => {
                (0, source, required),
+               (1, onchain_value_satoshis, option),
                (2, payment_hash, required),
        },
        (1, MaturingOutput) => {
@@ -506,6 +510,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        on_holder_tx_csv: u16,
 
        commitment_secrets: CounterpartyCommitmentSecrets,
+       /// The set of outpoints in each counterparty commitment transaction. We always need at least
+       /// the payment hash from `HTLCOutputInCommitment` to claim even a revoked commitment
+       /// transaction broadcast as we need to be able to construct the witness script in all cases.
        counterparty_claimable_outpoints: HashMap<Txid, Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>,
        /// We cannot identify HTLC-Success or HTLC-Timeout transactions by themselves on the chain.
        /// Nor can we figure out their commitment numbers without the commitment transaction they are
@@ -1197,6 +1204,81 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        }
 }
 
+/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
+/// failing any HTLCs which didn't make it into the broadcasted commitment transaction back
+/// after ANTI_REORG_DELAY blocks.
+///
+/// We always compare against the set of HTLCs in counterparty commitment transactions, as those
+/// are the commitment transactions which are generated by us. The off-chain state machine in
+/// `Channel` will automatically resolve any HTLCs which were never included in a commitment
+/// transaction when it detects channel closure, but it is up to us to ensure any HTLCs which were
+/// included in a remote commitment transaction are failed back if they are not present in the
+/// broadcasted commitment transaction.
+///
+/// Specifically, the removal process for HTLCs in `Channel` is always based on the counterparty
+/// sending a `revoke_and_ack`, which causes us to clear `prev_counterparty_commitment_txid`. Thus,
+/// as long as we examine both the current counterparty commitment transaction and, if it hasn't
+/// been revoked yet, the previous one, we we will never "forget" to resolve an HTLC.
+macro_rules! fail_unbroadcast_htlcs {
+       ($self: expr, $commitment_tx_type: expr, $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { {
+               macro_rules! check_htlc_fails {
+                       ($txid: expr, $commitment_tx: expr) => {
+                               if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) {
+                                       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
+                                                       // broadcast, but not if it was below the dust limit, which we should
+                                                       // fail backwards immediately as there is no way for us to learn the
+                                                       // payment_preimage.
+                                                       // Note that if the dust limit were allowed to change between
+                                                       // commitment transactions we'd want to be check whether *any*
+                                                       // broadcastable commitment transaction has the HTLC in it, but it
+                                                       // cannot currently change after channel initialization, so we don't
+                                                       // need to here.
+                                                       let confirmed_htlcs_iter: &mut Iterator<Item = (&HTLCOutputInCommitment, Option<&HTLCSource>)> = &mut $confirmed_htlcs_list;
+                                                       let mut matched_htlc = false;
+                                                       for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter {
+                                                               if broadcast_htlc.transaction_output_index.is_some() && Some(&**source) == *broadcast_source {
+                                                                       matched_htlc = true;
+                                                                       break;
+                                                               }
+                                                       }
+                                                       if matched_htlc { continue; }
+                                                       $self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
+                                                               if entry.height != $commitment_tx_conf_height { return true; }
+                                                               match entry.event {
+                                                                       OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
+                                                                               *update_source != **source
+                                                                       },
+                                                                       _ => true,
+                                                               }
+                                                       });
+                                                       let entry = OnchainEventEntry {
+                                                               txid: *$txid,
+                                                               height: $commitment_tx_conf_height,
+                                                               event: OnchainEvent::HTLCUpdate {
+                                                                       source: (**source).clone(),
+                                                                       payment_hash: htlc.payment_hash.clone(),
+                                                                       onchain_value_satoshis: Some(htlc.amount_msat / 1000),
+                                                               },
+                                                       };
+                                                       log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction, waiting for confirmation (at height {})",
+                                                               log_bytes!(htlc.payment_hash.0), $commitment_tx, $commitment_tx_type, entry.confirmation_threshold());
+                                                       $self.onchain_events_awaiting_threshold_conf.push(entry);
+                                               }
+                                       }
+                               }
+                       }
+               }
+               if let Some(ref txid) = $self.current_counterparty_commitment_txid {
+                       check_htlc_fails!(txid, "current");
+               }
+               if let Some(ref txid) = $self.prev_counterparty_commitment_txid {
+                       check_htlc_fails!(txid, "previous");
+               }
+       } }
+}
+
 impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
        /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
@@ -1554,42 +1636,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                }
                                self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
-                               macro_rules! check_htlc_fails {
-                                       ($txid: expr, $commitment_tx: expr) => {
-                                               if let Some(ref outpoints) = self.counterparty_claimable_outpoints.get($txid) {
-                                                       for &(ref htlc, ref source_option) in outpoints.iter() {
-                                                               if let &Some(ref source) = source_option {
-                                                                       self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
-                                                                               if entry.height != height { return true; }
-                                                                               match entry.event {
-                                                                                       OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
-                                                                                               *update_source != **source
-                                                                                       },
-                                                                                       _ => true,
-                                                                               }
-                                                                       });
-                                                                       let entry = OnchainEventEntry {
-                                                                               txid: *$txid,
-                                                                               height,
-                                                                               event: OnchainEvent::HTLCUpdate {
-                                                                                       source: (**source).clone(),
-                                                                                       payment_hash: htlc.payment_hash.clone(),
-                                                                               },
-                                                                       };
-                                                                       log_info!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of revoked counterparty commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, entry.confirmation_threshold());
-                                                                       self.onchain_events_awaiting_threshold_conf.push(entry);
-                                                               }
-                                                       }
-                                               }
-                                       }
-                               }
-                               if let Some(ref txid) = self.current_counterparty_commitment_txid {
-                                       check_htlc_fails!(txid, "current");
-                               }
-                               if let Some(ref txid) = self.prev_counterparty_commitment_txid {
-                                       check_htlc_fails!(txid, "counterparty");
-                               }
-                               // No need to check holder commitment txn, symmetric HTLCSource must be present as per-htlc data on counterparty commitment tx
+                               fail_unbroadcast_htlcs!(self, "revoked counterparty", height, [].iter().map(|a| *a), logger);
                        }
                } else if let Some(per_commitment_data) = per_commitment_option {
                        // While this isn't useful yet, there is a potential race where if a counterparty
@@ -1605,55 +1652,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
                        log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
-
-                       macro_rules! check_htlc_fails {
-                               ($txid: expr, $commitment_tx: expr, $id: tt) => {
-                                       if let Some(ref latest_outpoints) = self.counterparty_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
-                                                               // broadcast, but not if it was below the dust limit, which we should
-                                                               // fail backwards immediately as there is no way for us to learn the
-                                                               // payment_preimage.
-                                                               // Note that if the dust limit were allowed to change between
-                                                               // commitment transactions we'd want to be check whether *any*
-                                                               // broadcastable commitment transaction has the HTLC in it, but it
-                                                               // cannot currently change after channel initialization, so we don't
-                                                               // need to here.
-                                                               for &(ref broadcast_htlc, ref broadcast_source) in per_commitment_data.iter() {
-                                                                       if broadcast_htlc.transaction_output_index.is_some() && Some(source) == broadcast_source.as_ref() {
-                                                                               continue $id;
-                                                                       }
-                                                               }
-                                                               log_trace!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of counterparty commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
-                                                               self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
-                                                                       if entry.height != height { return true; }
-                                                                       match entry.event {
-                                                                               OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
-                                                                                       *update_source != **source
-                                                                               },
-                                                                               _ => true,
-                                                                       }
-                                                               });
-                                                               self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
-                                                                       txid: *$txid,
-                                                                       height,
-                                                                       event: OnchainEvent::HTLCUpdate {
-                                                                               source: (**source).clone(),
-                                                                               payment_hash: htlc.payment_hash.clone(),
-                                                                       },
-                                                               });
-                                                       }
-                                               }
-                                       }
-                               }
-                       }
-                       if let Some(ref txid) = self.current_counterparty_commitment_txid {
-                               check_htlc_fails!(txid, "current", 'current_loop);
-                       }
-                       if let Some(ref txid) = self.prev_counterparty_commitment_txid {
-                               check_htlc_fails!(txid, "previous", 'prev_loop);
-                       }
+                       fail_unbroadcast_htlcs!(self, "counterparty", height, per_commitment_data.iter().map(|(a, b)| (a, b.as_ref().map(|b| b.as_ref()))), logger);
 
                        let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx));
                        for req in htlc_claim_reqs {
@@ -1779,27 +1778,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                let mut claim_requests = Vec::new();
                let mut watch_outputs = Vec::new();
 
-               macro_rules! wait_threshold_conf {
-                       ($source: expr, $commitment_tx: expr, $payment_hash: expr) => {
-                               self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
-                                       if entry.height != height { return true; }
-                                       match entry.event {
-                                               OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
-                                                       *update_source != $source
-                                               },
-                                               _ => true,
-                                       }
-                               });
-                               let entry = OnchainEventEntry {
-                                       txid: commitment_txid,
-                                       height,
-                                       event: OnchainEvent::HTLCUpdate { source: $source, payment_hash: $payment_hash },
-                               };
-                               log_trace!(logger, "Failing HTLC with payment_hash {} from {} holder commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, entry.confirmation_threshold());
-                               self.onchain_events_awaiting_threshold_conf.push(entry);
-                       }
-               }
-
                macro_rules! append_onchain_update {
                        ($updates: expr, $to_watch: expr) => {
                                claim_requests = $updates.0;
@@ -1817,6 +1795,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
                        let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx);
                        append_onchain_update!(res, to_watch);
+                       fail_unbroadcast_htlcs!(self, "latest holder", height, self.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
                } else if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
                        if holder_tx.txid == commitment_txid {
                                is_holder_tx = true;
@@ -1824,26 +1803,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                let res = self.get_broadcasted_holder_claims(holder_tx, height);
                                let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx);
                                append_onchain_update!(res, to_watch);
-                       }
-               }
-
-               macro_rules! fail_dust_htlcs_after_threshold_conf {
-                       ($holder_tx: expr) => {
-                               for &(ref htlc, _, ref source) in &$holder_tx.htlc_outputs {
-                                       if htlc.transaction_output_index.is_none() {
-                                               if let &Some(ref source) = source {
-                                                       wait_threshold_conf!(source.clone(), "lastest", htlc.payment_hash.clone());
-                                               }
-                                       }
-                               }
+                               fail_unbroadcast_htlcs!(self, "previous holder", height, holder_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
                        }
                }
 
                if is_holder_tx {
-                       fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx);
-                       if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
-                               fail_dust_htlcs_after_threshold_conf!(holder_tx);
-                       }
                }
 
                (claim_requests, (commitment_txid, watch_outputs))
@@ -2090,7 +2054,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                // Produce actionable events from on-chain events having reached their threshold.
                for entry in onchain_events_reaching_threshold_conf.drain(..) {
                        match entry.event {
-                               OnchainEvent::HTLCUpdate { ref source, payment_hash } => {
+                               OnchainEvent::HTLCUpdate { ref source, payment_hash, onchain_value_satoshis } => {
                                        // Check for duplicate HTLC resolutions.
                                        #[cfg(debug_assertions)]
                                        {
@@ -2109,9 +2073,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
                                        log_debug!(logger, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!(payment_hash.0));
                                        self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
-                                               payment_hash: payment_hash,
+                                               payment_hash,
                                                payment_preimage: None,
                                                source: source.clone(),
+                                               onchain_value_satoshis,
                                        }));
                                },
                                OnchainEvent::MaturingOutput { descriptor } => {
@@ -2328,7 +2293,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        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 counterparty commitment tx", false, pending_htlc, true);
-                                                                       payment_data = Some(((**source).clone(), $htlc_output.payment_hash));
+                                                                       payment_data = Some(((**source).clone(), $htlc_output.payment_hash, $htlc_output.amount_msat));
                                                                        break;
                                                                }
                                                        }
@@ -2348,7 +2313,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                                // transaction. This implies we either learned a preimage, the HTLC
                                                                // 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));
+                                                               payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat));
                                                        } else if !$holder_tx {
                                                                        check_htlc_valid_counterparty!(self.current_counterparty_commitment_txid, htlc_output);
                                                                if payment_data.is_none() {
@@ -2381,7 +2346,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
                        // Check that scan_commitment, above, decided there is some source worth relaying an
                        // HTLC resolution backwards to and figure out whether we learned a preimage from it.
-                       if let Some((source, payment_hash)) = payment_data {
+                       if let Some((source, payment_hash, amount_msat)) = payment_data {
                                let mut payment_preimage = PaymentPreimage([0; 32]);
                                if accepted_preimage_claim {
                                        if !self.pending_monitor_events.iter().any(
@@ -2390,7 +2355,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
                                                        source,
                                                        payment_preimage: Some(payment_preimage),
-                                                       payment_hash
+                                                       payment_hash,
+                                                       onchain_value_satoshis: Some(amount_msat / 1000),
                                                }));
                                        }
                                } else if offered_preimage_claim {
@@ -2402,7 +2368,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
                                                        source,
                                                        payment_preimage: Some(payment_preimage),
-                                                       payment_hash
+                                                       payment_hash,
+                                                       onchain_value_satoshis: Some(amount_msat / 1000),
                                                }));
                                        }
                                } else {
@@ -2418,7 +2385,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        let entry = OnchainEventEntry {
                                                txid: tx.txid(),
                                                height,
-                                               event: OnchainEvent::HTLCUpdate { source: source, payment_hash: payment_hash },
+                                               event: OnchainEvent::HTLCUpdate {
+                                                       source, payment_hash,
+                                                       onchain_value_satoshis: Some(amount_msat / 1000),
+                                               },
                                        };
                                        log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height {})", log_bytes!(payment_hash.0), entry.confirmation_threshold());
                                        self.onchain_events_awaiting_threshold_conf.push(entry);
@@ -2451,7 +2421,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        output: outp.clone(),
                                });
                                break;
-                       } else if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
+                       }
+                       if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
                                if broadcasted_holder_revokable_script.0 == outp.script_pubkey {
                                        spendable_output =  Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor {
                                                outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
@@ -2464,7 +2435,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        }));
                                        break;
                                }
-                       } else if self.counterparty_payment_script == outp.script_pubkey {
+                       }
+                       if self.counterparty_payment_script == outp.script_pubkey {
                                spendable_output = Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor {
                                        outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
                                        output: outp.clone(),
@@ -2472,11 +2444,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        channel_value_satoshis: self.channel_value_satoshis,
                                }));
                                break;
-                       } else if outp.script_pubkey == self.shutdown_script {
+                       }
+                       if outp.script_pubkey == self.shutdown_script {
                                spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
                                        outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
                                        output: outp.clone(),
                                });
+                               break;
                        }
                }
                if let Some(spendable_output) = spendable_output {
@@ -2581,7 +2555,7 @@ const MAX_ALLOC_SIZE: usize = 64*1024;
 
 impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                for (BlockHash, ChannelMonitor<Signer>) {
-       fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
+       fn read<R: io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
                macro_rules! unwrap_obj {
                        ($key: expr) => {
                                match $key {
@@ -2843,7 +2817,7 @@ mod tests {
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
        use bitcoin::secp256k1::key::{SecretKey,PublicKey};
        use bitcoin::secp256k1::Secp256k1;
-       use std::sync::{Arc, Mutex};
+       use sync::{Arc, Mutex};
        use chain::keysinterface::InMemorySigner;
        use prelude::*;
 
@@ -2852,7 +2826,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(TestLogger::new());
                let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
-               let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 });
+               let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) });
 
                let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };