Merge pull request #890 from TheBlueMatt/2021-04-fix-chan-shutdown-crash
[rust-lightning] / lightning / src / ln / channelmanager.rs
index d2ea7dfea798baea181dec19838cf2b55ccaad22..80edbacf573a7a545107a7908698fb0bf371cb13 100644 (file)
@@ -28,7 +28,7 @@ use bitcoin::hashes::hmac::{Hmac, HmacEngine};
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::hashes::cmp::fixed_time_eq;
-use bitcoin::hash_types::BlockHash;
+use bitcoin::hash_types::{BlockHash, Txid};
 
 use bitcoin::secp256k1::key::{SecretKey,PublicKey};
 use bitcoin::secp256k1::Secp256k1;
@@ -36,6 +36,7 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1;
 
 use chain;
+use chain::Confirm;
 use chain::Watch;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
@@ -352,9 +353,6 @@ struct PeerState {
        latest_features: InitFeatures,
 }
 
-#[cfg(not(any(target_pointer_width = "32", target_pointer_width = "64")))]
-const ERR: () = "You need at least 32 bit pointers (well, usize, but we'll assume they're the same) for ChannelManager::latest_block_height";
-
 /// SimpleArcChannelManager is useful when you need a ChannelManager with a static lifetime, e.g.
 /// when you're using lightning-net-tokio (since tokio::spawn requires parameters with static
 /// lifetimes). Other times you can afford a reference, which is more efficient, in which case
@@ -424,10 +422,9 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        tx_broadcaster: T,
 
        #[cfg(test)]
-       pub(super) latest_block_height: AtomicUsize,
+       pub(super) best_block: RwLock<BestBlock>,
        #[cfg(not(test))]
-       latest_block_height: AtomicUsize,
-       last_block_hash: RwLock<BlockHash>,
+       best_block: RwLock<BestBlock>,
        secp_ctx: Secp256k1<secp256k1::All>,
 
        #[cfg(any(test, feature = "_test_utils"))]
@@ -475,13 +472,38 @@ pub struct ChainParameters {
        /// The network for determining the `chain_hash` in Lightning messages.
        pub network: Network,
 
-       /// The hash of the latest block successfully connected.
-       pub latest_hash: BlockHash,
-
-       /// The height of the latest block successfully connected.
+       /// The hash and height of the latest block successfully connected.
        ///
        /// Used to track on-chain channel funding outputs and send payments with reliable timelocks.
-       pub latest_height: usize,
+       pub best_block: BestBlock,
+}
+
+/// The best known block as identified by its hash and height.
+#[derive(Clone, Copy)]
+pub struct BestBlock {
+       block_hash: BlockHash,
+       height: u32,
+}
+
+impl BestBlock {
+       /// Returns the best block from the genesis of the given network.
+       pub fn from_genesis(network: Network) -> Self {
+               BestBlock {
+                       block_hash: genesis_block(network).header.block_hash(),
+                       height: 0,
+               }
+       }
+
+       /// Returns the best block as identified by the given block hash and height.
+       pub fn new(block_hash: BlockHash, height: u32) -> Self {
+               BestBlock { block_hash, height }
+       }
+
+       /// Returns the best block hash.
+       pub fn block_hash(&self) -> BlockHash { self.block_hash }
+
+       /// Returns the best block height.
+       pub fn height(&self) -> u32 { self.height }
 }
 
 /// Whenever we release the `ChannelManager`'s `total_consistency_lock`, from read mode, it is
@@ -822,8 +844,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        chain_monitor,
                        tx_broadcaster,
 
-                       latest_block_height: AtomicUsize::new(params.latest_height),
-                       last_block_hash: RwLock::new(params.latest_hash),
+                       best_block: RwLock::new(params.best_block),
 
                        channel_state: Mutex::new(ChannelHolder{
                                by_id: HashMap::new(),
@@ -851,6 +872,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
+       /// Gets the current configuration applied to all new channels,  as
+       pub fn get_current_default_configuration(&self) -> &UserConfig {
+               &self.default_configuration
+       }
+
        /// Creates a new outbound channel to the given remote node and with the given value.
        ///
        /// user_id will be provided back as user_channel_id in FundingGenerationReady events to allow
@@ -1171,7 +1197,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                // HTLC_FAIL_BACK_BUFFER blocks to go.
                                // Also, ensure that, in the case of an unknown payment hash, our payment logic has enough time to fail the HTLC backward
                                // before our onchain logic triggers a channel closure (see HTLC_FAIL_BACK_BUFFER rational).
-                               if (msg.cltv_expiry as u64) <= self.latest_block_height.load(Ordering::Acquire) as u64 + HTLC_FAIL_BACK_BUFFER as u64 + 1 {
+                               if (msg.cltv_expiry as u64) <= self.best_block.read().unwrap().height() as u64 + HTLC_FAIL_BACK_BUFFER as u64 + 1 {
                                        return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]);
                                }
                                // final_incorrect_htlc_amount
@@ -1294,7 +1320,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + chan.get_cltv_expiry_delta() as u64 { // incorrect_cltv_expiry
                                                break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap())));
                                        }
-                                       let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+                                       let cur_height = self.best_block.read().unwrap().height() + 1;
                                        // Theoretically, channel counterparty shouldn't send us a HTLC expiring now, but we want to be robust wrt to counterparty
                                        // packet sanitization (see HTLC_FAIL_BACK_BUFFER rational)
                                        if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon
@@ -1511,7 +1537,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        return Err(PaymentSendFailure::PathParameterError(path_errs));
                }
 
-               let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+               let cur_height = self.best_block.read().unwrap().height() + 1;
                let mut results = Vec::new();
                for path in route.paths.iter() {
                        results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height));
@@ -1538,61 +1564,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       /// Call this upon creation of a funding transaction for the given channel.
-       ///
-       /// Returns an [`APIError::APIMisuseError`] if the funding_transaction spent non-SegWit outputs
-       /// or if no output was found which matches the parameters in [`Event::FundingGenerationReady`].
-       ///
-       /// Panics if a funding transaction has already been provided for this channel.
-       ///
-       /// May panic if the output found in the funding transaction is duplicative with some other
-       /// channel (note that this should be trivially prevented by using unique funding transaction
-       /// keys per-channel).
-       ///
-       /// Do NOT broadcast the funding transaction yourself. When we have safely received our
-       /// counterparty's signature the funding transaction will automatically be broadcast via the
-       /// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
-       ///
-       /// Note that this includes RBF or similar transaction replacement strategies - lightning does
-       /// not currently support replacing a funding transaction on an existing channel. Instead,
-       /// create a new channel with a conflicting funding transaction.
-       pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction) -> Result<(), APIError> {
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-
-               for inp in funding_transaction.input.iter() {
-                       if inp.witness.is_empty() {
-                               return Err(APIError::APIMisuseError {
-                                       err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
-                               });
-                       }
-               }
-
+       /// Handles the generation of a funding transaction, optionally (for tests) with a function
+       /// which checks the correctness of the funding transaction given the associated channel.
+       fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<Signer>, &Transaction) -> Result<OutPoint, APIError>>
+                       (&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction, find_funding_output: FundingOutput) -> Result<(), APIError> {
                let (chan, msg) = {
                        let (res, chan) = match self.channel_state.lock().unwrap().by_id.remove(temporary_channel_id) {
                                Some(mut chan) => {
-                                       let mut output_index = None;
-                                       let expected_spk = chan.get_funding_redeemscript().to_v0_p2wsh();
-                                       for (idx, outp) in funding_transaction.output.iter().enumerate() {
-                                               if outp.script_pubkey == expected_spk && outp.value == chan.get_value_satoshis() {
-                                                       if output_index.is_some() {
-                                                               return Err(APIError::APIMisuseError {
-                                                                       err: "Multiple outputs matched the expected script and value".to_owned()
-                                                               });
-                                                       }
-                                                       if idx > u16::max_value() as usize {
-                                                               return Err(APIError::APIMisuseError {
-                                                                       err: "Transaction had more than 2^16 outputs, which is not supported".to_owned()
-                                                               });
-                                                       }
-                                                       output_index = Some(idx as u16);
-                                               }
-                                       }
-                                       if output_index.is_none() {
-                                               return Err(APIError::APIMisuseError {
-                                                       err: "No output matched the script_pubkey and value in the FundingGenerationReady event".to_owned()
-                                               });
-                                       }
-                                       let funding_txo = OutPoint { txid: funding_transaction.txid(), index: output_index.unwrap() };
+                                       let funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
                                        (chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
                                                .map_err(|e| if let ChannelError::Close(msg) = e {
@@ -1628,6 +1607,68 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                Ok(())
        }
 
+       #[cfg(test)]
+       pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
+               self.funding_transaction_generated_intern(temporary_channel_id, funding_transaction, |_, tx| {
+                       Ok(OutPoint { txid: tx.txid(), index: output_index })
+               })
+       }
+
+       /// Call this upon creation of a funding transaction for the given channel.
+       ///
+       /// Returns an [`APIError::APIMisuseError`] if the funding_transaction spent non-SegWit outputs
+       /// or if no output was found which matches the parameters in [`Event::FundingGenerationReady`].
+       ///
+       /// Panics if a funding transaction has already been provided for this channel.
+       ///
+       /// May panic if the output found in the funding transaction is duplicative with some other
+       /// channel (note that this should be trivially prevented by using unique funding transaction
+       /// keys per-channel).
+       ///
+       /// Do NOT broadcast the funding transaction yourself. When we have safely received our
+       /// counterparty's signature the funding transaction will automatically be broadcast via the
+       /// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
+       ///
+       /// Note that this includes RBF or similar transaction replacement strategies - lightning does
+       /// not currently support replacing a funding transaction on an existing channel. Instead,
+       /// create a new channel with a conflicting funding transaction.
+       pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction) -> Result<(), APIError> {
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+
+               for inp in funding_transaction.input.iter() {
+                       if inp.witness.is_empty() {
+                               return Err(APIError::APIMisuseError {
+                                       err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
+                               });
+                       }
+               }
+               self.funding_transaction_generated_intern(temporary_channel_id, funding_transaction, |chan, tx| {
+                       let mut output_index = None;
+                       let expected_spk = chan.get_funding_redeemscript().to_v0_p2wsh();
+                       for (idx, outp) in tx.output.iter().enumerate() {
+                               if outp.script_pubkey == expected_spk && outp.value == chan.get_value_satoshis() {
+                                       if output_index.is_some() {
+                                               return Err(APIError::APIMisuseError {
+                                                       err: "Multiple outputs matched the expected script and value".to_owned()
+                                               });
+                                       }
+                                       if idx > u16::max_value() as usize {
+                                               return Err(APIError::APIMisuseError {
+                                                       err: "Transaction had more than 2^16 outputs, which is not supported".to_owned()
+                                               });
+                                       }
+                                       output_index = Some(idx as u16);
+                               }
+                       }
+                       if output_index.is_none() {
+                               return Err(APIError::APIMisuseError {
+                                       err: "No output matched the script_pubkey and value in the FundingGenerationReady event".to_owned()
+                               });
+                       }
+                       Ok(OutPoint { txid: tx.txid(), index: output_index.unwrap() })
+               })
+       }
+
        fn get_announcement_sigs(&self, chan: &Channel<Signer>) -> Option<msgs::AnnouncementSignatures> {
                if !chan.should_announce() {
                        log_trace!(self.logger, "Can't send announcement_signatures for private channel {}", log_bytes!(chan.channel_id()));
@@ -1905,10 +1946,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                for htlc in htlcs.iter() {
                                                                                        let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                                                                        htlc_msat_height_data.extend_from_slice(
-                                                                                               &byte_utils::be32_to_array(
-                                                                                                       self.latest_block_height.load(Ordering::Acquire)
-                                                                                                               as u32,
-                                                                                               ),
+                                                                                               &byte_utils::be32_to_array(self.best_block.read().unwrap().height()),
                                                                                        );
                                                                                        failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                                        short_channel_id: htlc.prev_hop.short_channel_id,
@@ -2028,8 +2066,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
                                let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
-                                       self.latest_block_height.load(Ordering::Acquire) as u32,
-                               ));
+                                               self.best_block.read().unwrap().height()));
                                self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
                                                HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
                                                HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data });
@@ -2243,8 +2280,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                if (is_mpp && !valid_mpp) || (!is_mpp && (htlc.value < expected_amount || htlc.value > expected_amount * 2)) {
                                        let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
                                        htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
-                                               self.latest_block_height.load(Ordering::Acquire) as u32,
-                                       ));
+                                                       self.best_block.read().unwrap().height()));
                                        self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
                                                                         HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
                                                                         HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data });
@@ -2529,7 +2565,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        fn internal_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> {
                let ((funding_msg, monitor), mut chan) = {
-                       let last_block_hash = *self.last_block_hash.read().unwrap();
+                       let best_block = *self.best_block.read().unwrap();
                        let mut channel_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_lock;
                        match channel_state.by_id.entry(msg.temporary_channel_id.clone()) {
@@ -2537,7 +2573,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
                                        }
-                                       (try_chan_entry!(self, chan.get_mut().funding_created(msg, last_block_hash, &self.logger), channel_state, chan), chan.remove())
+                                       (try_chan_entry!(self, chan.get_mut().funding_created(msg, best_block, &self.logger), channel_state, chan), chan.remove())
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))
                        }
@@ -2586,7 +2622,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        fn internal_funding_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
                let funding_tx = {
-                       let last_block_hash = *self.last_block_hash.read().unwrap();
+                       let best_block = *self.best_block.read().unwrap();
                        let mut channel_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_lock;
                        match channel_state.by_id.entry(msg.channel_id) {
@@ -2594,7 +2630,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                        }
-                                       let (monitor, funding_tx) = match chan.get_mut().funding_signed(&msg, last_block_hash, &self.logger) {
+                                       let (monitor, funding_tx) = match chan.get_mut().funding_signed(&msg, best_block, &self.logger) {
                                                Ok(update) => update,
                                                Err(e) => try_chan_entry!(self, Err(e), channel_state, chan),
                                        };
@@ -3334,38 +3370,119 @@ where
        L::Target: Logger,
 {
        fn block_connected(&self, block: &Block, height: u32) {
-               assert_eq!(*self.last_block_hash.read().unwrap(), block.header.prev_blockhash,
-                       "Blocks must be connected in chain-order - the connected header must build on the last connected header");
-               assert_eq!(self.latest_block_height.load(Ordering::Acquire) as u64, height as u64 - 1,
-                       "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
+               {
+                       let best_block = self.best_block.read().unwrap();
+                       assert_eq!(best_block.block_hash(), block.header.prev_blockhash,
+                               "Blocks must be connected in chain-order - the connected header must build on the last connected header");
+                       assert_eq!(best_block.height(), height - 1,
+                               "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
+               }
+
                let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-               self.transactions_confirmed(&block.header, height, &txdata);
-               self.update_best_block(&block.header, height);
+               self.transactions_confirmed(&block.header, &txdata, height);
+               self.best_block_updated(&block.header, height);
        }
 
        fn block_disconnected(&self, header: &BlockHeader, height: u32) {
-               assert_eq!(*self.last_block_hash.read().unwrap(), header.block_hash(),
-                       "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+               let new_height = height - 1;
+               {
+                       let mut best_block = self.best_block.write().unwrap();
+                       assert_eq!(best_block.block_hash(), header.block_hash(),
+                               "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
+                       assert_eq!(best_block.height(), height,
+                               "Blocks must be disconnected in chain-order - the disconnected block must have the correct height");
+                       *best_block = BestBlock::new(header.prev_blockhash, new_height)
+               }
+
+               self.do_chain_event(Some(new_height), |channel| channel.best_block_updated(new_height, header.time));
+       }
+}
+
+impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Confirm for ChannelManager<Signer, M, T, K, F, L>
+where
+       M::Target: chain::Watch<Signer>,
+       T::Target: BroadcasterInterface,
+       K::Target: KeysInterface<Signer = Signer>,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
+{
+       fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
+               // during initialization prior to the chain_monitor being fully configured in some cases.
+               // See the docs for `ChannelManagerReadArgs` for more.
+
+               let block_hash = header.block_hash();
+               log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
 
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-               let new_height = self.latest_block_height.fetch_sub(1, Ordering::AcqRel) as u32 - 1;
-               assert_eq!(new_height, height - 1,
-                       "Blocks must be disconnected in chain-order - the disconnected block must have the correct height");
-               *self.last_block_hash.write().unwrap() = header.prev_blockhash;
+               self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
+       }
+
+       fn best_block_updated(&self, header: &BlockHeader, height: u32) {
+               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
+               // during initialization prior to the chain_monitor being fully configured in some cases.
+               // See the docs for `ChannelManagerReadArgs` for more.
+
+               let block_hash = header.block_hash();
+               log_trace!(self.logger, "New best block: {} at height {}", block_hash, height);
+
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+
+               *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
+
+               self.do_chain_event(Some(height), |channel| channel.best_block_updated(height, header.time));
+
+               loop {
+                       // Update last_node_announcement_serial to be the max of its current value and the
+                       // block timestamp. This should keep us close to the current time without relying on
+                       // having an explicit local time source.
+                       // Just in case we end up in a race, we loop until we either successfully update
+                       // last_node_announcement_serial or decide we don't need to.
+                       let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
+                       if old_serial >= header.time as usize { break; }
+                       if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
+                               break;
+                       }
+               }
+       }
 
-               self.do_chain_event(new_height, |channel| channel.update_best_block(new_height, header.time));
+       fn get_relevant_txids(&self) -> Vec<Txid> {
+               let channel_state = self.channel_state.lock().unwrap();
+               let mut res = Vec::with_capacity(channel_state.short_to_id.len());
+               for chan in channel_state.by_id.values() {
+                       if let Some(funding_txo) = chan.get_funding_txo() {
+                               res.push(funding_txo.txid);
+                       }
+               }
+               res
+       }
+
+       fn transaction_unconfirmed(&self, txid: &Txid) {
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+               self.do_chain_event(None, |channel| {
+                       if let Some(funding_txo) = channel.get_funding_txo() {
+                               if funding_txo.txid == *txid {
+                                       channel.funding_transaction_unconfirmed().map(|_| (None, Vec::new()))
+                               } else { Ok((None, Vec::new())) }
+                       } else { Ok((None, Vec::new())) }
+               });
        }
 }
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
-        T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
-        F::Target: FeeEstimator,
-        L::Target: Logger,
+where
+       M::Target: chain::Watch<Signer>,
+       T::Target: BroadcasterInterface,
+       K::Target: KeysInterface<Signer = Signer>,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
 {
+       /// Calls a function which handles an on-chain event (blocks dis/connected, transactions
+       /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
+       /// the function.
        fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>>
-                       (&self, height: u32, f: FN) {
+                       (&self, height_opt: Option<u32>, f: FN) {
                // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
                // during initialization prior to the chain_monitor being fully configured in some cases.
                // See the docs for `ChannelManagerReadArgs` for more.
@@ -3424,24 +3541,26 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                true
                        });
 
-                       channel_state.claimable_htlcs.retain(|&(ref payment_hash, _), htlcs| {
-                               htlcs.retain(|htlc| {
-                                       // If height is approaching the number of blocks we think it takes us to get
-                                       // our commitment transaction confirmed before the HTLC expires, plus the
-                                       // number of blocks we generally consider it to take to do a commitment update,
-                                       // just give up on it and fail the HTLC.
-                                       if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
-                                               let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
-                                               htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
-                                               timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason {
-                                                       failure_code: 0x4000 | 15,
-                                                       data: htlc_msat_height_data
-                                               }));
-                                               false
-                                       } else { true }
+                       if let Some(height) = height_opt {
+                               channel_state.claimable_htlcs.retain(|&(ref payment_hash, _), htlcs| {
+                                       htlcs.retain(|htlc| {
+                                               // If height is approaching the number of blocks we think it takes us to get
+                                               // our commitment transaction confirmed before the HTLC expires, plus the
+                                               // number of blocks we generally consider it to take to do a commitment update,
+                                               // just give up on it and fail the HTLC.
+                                               if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
+                                                       let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
+                                                       htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
+                                                       timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason {
+                                                               failure_code: 0x4000 | 15,
+                                                               data: htlc_msat_height_data
+                                                       }));
+                                                       false
+                                               } else { true }
+                                       });
+                                       !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
                                });
-                               !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
-                       });
+                       }
                }
 
                self.handle_init_event_channel_failures(failed_channels);
@@ -3451,77 +3570,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       /// Updates channel state to take note of transactions which were confirmed in the given block
-       /// at the given height.
-       ///
-       /// Note that you must still call (or have called) [`update_best_block`] with the block
-       /// information which is included here.
-       ///
-       /// This method may be called before or after [`update_best_block`] for a given block's
-       /// transaction data and may be called multiple times with additional transaction data for a
-       /// given block.
-       ///
-       /// This method may be called for a previous block after an [`update_best_block`] call has
-       /// been made for a later block, however it must *not* be called with transaction data from a
-       /// block which is no longer in the best chain (ie where [`update_best_block`] has already
-       /// been informed about a blockchain reorganization which no longer includes the block which
-       /// corresponds to `header`).
-       ///
-       /// [`update_best_block`]: `Self::update_best_block`
-       pub fn transactions_confirmed(&self, header: &BlockHeader, height: u32, txdata: &TransactionData) {
-               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
-               // during initialization prior to the chain_monitor being fully configured in some cases.
-               // See the docs for `ChannelManagerReadArgs` for more.
-
-               let block_hash = header.block_hash();
-               log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
-
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-               self.do_chain_event(height, |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
-       }
-
-       /// Updates channel state with the current best blockchain tip. You should attempt to call this
-       /// quickly after a new block becomes available, however if multiple new blocks become
-       /// available at the same time, only a single `update_best_block()` call needs to be made.
-       ///
-       /// This method should also be called immediately after any block disconnections, once at the
-       /// reorganization fork point, and once with the new chain tip. Calling this method at the
-       /// blockchain reorganization fork point ensures we learn when a funding transaction which was
-       /// previously confirmed is reorganized out of the blockchain, ensuring we do not continue to
-       /// accept payments which cannot be enforced on-chain.
-       ///
-       /// In both the block-connection and block-disconnection case, this method may be called either
-       /// once per block connected or disconnected, or simply at the fork point and new tip(s),
-       /// skipping any intermediary blocks.
-       pub fn update_best_block(&self, header: &BlockHeader, height: u32) {
-               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
-               // during initialization prior to the chain_monitor being fully configured in some cases.
-               // See the docs for `ChannelManagerReadArgs` for more.
-
-               let block_hash = header.block_hash();
-               log_trace!(self.logger, "New best block: {} at height {}", block_hash, height);
-
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-
-               self.latest_block_height.store(height as usize, Ordering::Release);
-               *self.last_block_hash.write().unwrap() = block_hash;
-
-               self.do_chain_event(height, |channel| channel.update_best_block(height, header.time));
-
-               loop {
-                       // Update last_node_announcement_serial to be the max of its current value and the
-                       // block timestamp. This should keep us close to the current time without relying on
-                       // having an explicit local time source.
-                       // Just in case we end up in a race, we loop until we either successfully update
-                       // last_node_announcement_serial or decide we don't need to.
-                       let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
-                       if old_serial >= header.time as usize { break; }
-                       if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
-                               break;
-                       }
-               }
-       }
-
        /// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool
        /// indicating whether persistence is necessary. Only one listener on
        /// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken
@@ -4082,8 +4130,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
 
                self.genesis_hash.write(writer)?;
-               (self.latest_block_height.load(Ordering::Acquire) as u32).write(writer)?;
-               self.last_block_hash.read().unwrap().write(writer)?;
+               {
+                       let best_block = self.best_block.read().unwrap();
+                       best_block.height().write(writer)?;
+                       best_block.block_hash().write(writer)?;
+               }
 
                let channel_state = self.channel_state.lock().unwrap();
                let mut unfunded_channels = 0;
@@ -4275,8 +4326,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                }
 
                let genesis_hash: BlockHash = Readable::read(reader)?;
-               let latest_block_height: u32 = Readable::read(reader)?;
-               let last_block_hash: BlockHash = Readable::read(reader)?;
+               let best_block_height: u32 = Readable::read(reader)?;
+               let best_block_hash: BlockHash = Readable::read(reader)?;
 
                let mut failed_htlcs = Vec::new();
 
@@ -4384,8 +4435,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                        chain_monitor: args.chain_monitor,
                        tx_broadcaster: args.tx_broadcaster,
 
-                       latest_block_height: AtomicUsize::new(latest_block_height as usize),
-                       last_block_hash: RwLock::new(last_block_hash),
+                       best_block: RwLock::new(BestBlock::new(best_block_hash, best_block_height)),
 
                        channel_state: Mutex::new(ChannelHolder {
                                by_id,
@@ -4419,7 +4469,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                //TODO: Broadcast channel update for closed channels, but only after we've made a
                //connection or two.
 
-               Ok((last_block_hash.clone(), channel_manager))
+               Ok((best_block_hash.clone(), channel_manager))
        }
 }
 
@@ -4480,7 +4530,7 @@ pub mod bench {
        use chain::chainmonitor::ChainMonitor;
        use chain::channelmonitor::Persist;
        use chain::keysinterface::{KeysManager, InMemorySigner};
-       use ln::channelmanager::{ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
+       use ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
        use ln::features::InitFeatures;
        use ln::functional_test_utils::*;
        use ln::msgs::ChannelMessageHandler;
@@ -4532,8 +4582,7 @@ pub mod bench {
                let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
                let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &logger_a, &keys_manager_a, config.clone(), ChainParameters {
                        network,
-                       latest_hash: genesis_hash,
-                       latest_height: 0,
+                       best_block: BestBlock::from_genesis(network),
                });
                let node_a_holder = NodeHolder { node: &node_a };
 
@@ -4543,8 +4592,7 @@ pub mod bench {
                let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
                let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &logger_b, &keys_manager_b, config.clone(), ChainParameters {
                        network,
-                       latest_hash: genesis_hash,
-                       latest_height: 0,
+                       best_block: BestBlock::from_genesis(network),
                });
                let node_b_holder = NodeHolder { node: &node_b };