diff --git a/substrate/core/client/src/blockchain.rs b/substrate/core/client/src/blockchain.rs index a6edb8505a..7cf510faf9 100644 --- a/substrate/core/client/src/blockchain.rs +++ b/substrate/core/client/src/blockchain.rs @@ -21,6 +21,8 @@ use std::sync::Arc; use sr_primitives::traits::{Block as BlockT, Header as HeaderT, NumberFor}; use sr_primitives::generic::BlockId; use sr_primitives::Justification; +use log::warn; +use parking_lot::Mutex; use crate::error::{Error, Result}; @@ -89,6 +91,123 @@ pub trait Backend: HeaderBackend { /// Return hashes of all blocks that are children of the block with `parent_hash`. fn children(&self, parent_hash: Block::Hash) -> Result>; + + /// Get the most recent block hash of the best (longest) chains + /// that contain block with the given `target_hash`. + /// + /// The search space is always limited to blocks which are in the finalized + /// chain or descendents of it. + /// + /// If `maybe_max_block_number` is `Some(max_block_number)` + /// the search is limited to block `numbers <= max_block_number`. + /// in other words as if there were no blocks greater `max_block_number`. + /// Returns `Ok(None)` if `target_hash` is not found in search space. + /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) + fn best_containing( + &self, + target_hash: Block::Hash, + maybe_max_number: Option>, + import_lock: &Mutex<()>, + ) -> Result> { + let target_header = { + match self.header(BlockId::Hash(target_hash))? { + Some(x) => x, + // target not in blockchain + None => { return Ok(None); }, + } + }; + + if let Some(max_number) = maybe_max_number { + // target outside search range + if target_header.number() > &max_number { + return Ok(None); + } + } + + let leaves = { + // ensure no blocks are imported during this code block. + // an import could trigger a reorg which could change the canonical chain. + // we depend on the canonical chain staying the same during this code block. + let _import_guard = import_lock.lock(); + + let info = self.info(); + + // this can be `None` if the best chain is shorter than the target header. + let maybe_canon_hash = self.hash(*target_header.number())?; + + if maybe_canon_hash.as_ref() == Some(&target_hash) { + // if a `max_number` is given we try to fetch the block at the + // given depth, if it doesn't exist or `max_number` is not + // provided, we continue to search from all leaves below. + if let Some(max_number) = maybe_max_number { + if let Some(header) = self.hash(max_number)? { + return Ok(Some(header)); + } + } + } else if info.finalized_number >= *target_header.number() { + // header is on a dead fork. + return Ok(None); + } + + self.leaves()? + }; + + // for each chain. longest chain first. shortest last + for leaf_hash in leaves { + // start at the leaf + let mut current_hash = leaf_hash; + + // if search is not restricted then the leaf is the best + let mut best_hash = leaf_hash; + + // go backwards entering the search space + // waiting until we are <= max_number + if let Some(max_number) = maybe_max_number { + loop { + let current_header = self.header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| Error::from(format!("failed to get header for hash {}", current_hash)))?; + + if current_header.number() <= &max_number { + best_hash = current_header.hash(); + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // go backwards through the chain (via parent links) + loop { + // until we find target + if current_hash == target_hash { + return Ok(Some(best_hash)); + } + + let current_header = self.header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| Error::from(format!("failed to get header for hash {}", current_hash)))?; + + // stop search in this chain once we go below the target's block number + if current_header.number() < target_header.number() { + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // header may be on a dead fork -- the only leaves that are considered are + // those which can still be finalized. + // + // FIXME #1558 only issue this warning when not on a dead fork + warn!( + "Block {:?} exists in chain but not found when following all \ + leaves backwards. Number limit = {:?}", + target_hash, + maybe_max_number, + ); + + Ok(None) + } } /// Provides access to the optional cache. diff --git a/substrate/core/client/src/client.rs b/substrate/core/client/src/client.rs index 5203c72303..32af79a4bf 100644 --- a/substrate/core/client/src/client.rs +++ b/substrate/core/client/src/client.rs @@ -1668,129 +1668,14 @@ where fn best_block_header(&self) -> error::Result<::Header> { let info = self.backend.blockchain().info(); - let best_hash = self.best_containing(info.best_hash, None)? + let import_lock = self.backend.get_import_lock(); + let best_hash = self.backend.blockchain().best_containing(info.best_hash, None, import_lock)? .unwrap_or(info.best_hash); Ok(self.backend.blockchain().header(BlockId::Hash(best_hash))? .expect("given block hash was fetched from block in db; qed")) } - /// Get the most recent block hash of the best (longest) chains - /// that contain block with the given `target_hash`. - /// - /// The search space is always limited to blocks which are in the finalized - /// chain or descendents of it. - /// - /// If `maybe_max_block_number` is `Some(max_block_number)` - /// the search is limited to block `numbers <= max_block_number`. - /// in other words as if there were no blocks greater `max_block_number`. - /// Returns `Ok(None)` if `target_hash` is not found in search space. - /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) - fn best_containing( - &self, - target_hash: Block::Hash, - maybe_max_number: Option> - ) -> error::Result> { - let target_header = { - match self.backend.blockchain().header(BlockId::Hash(target_hash))? { - Some(x) => x, - // target not in blockchain - None => { return Ok(None); }, - } - }; - - if let Some(max_number) = maybe_max_number { - // target outside search range - if target_header.number() > &max_number { - return Ok(None); - } - } - - let leaves = { - // ensure no blocks are imported during this code block. - // an import could trigger a reorg which could change the canonical chain. - // we depend on the canonical chain staying the same during this code block. - let _import_lock = self.backend.get_import_lock().lock(); - - let info = self.backend.blockchain().info(); - - // this can be `None` if the best chain is shorter than the target header. - let maybe_canon_hash = self.backend.blockchain().hash(*target_header.number())?; - - if maybe_canon_hash.as_ref() == Some(&target_hash) { - // if a `max_number` is given we try to fetch the block at the - // given depth, if it doesn't exist or `max_number` is not - // provided, we continue to search from all leaves below. - if let Some(max_number) = maybe_max_number { - if let Some(header) = self.backend.blockchain().hash(max_number)? { - return Ok(Some(header)); - } - } - } else if info.finalized_number >= *target_header.number() { - // header is on a dead fork. - return Ok(None); - } - - self.backend.blockchain().leaves()? - }; - - // for each chain. longest chain first. shortest last - for leaf_hash in leaves { - // start at the leaf - let mut current_hash = leaf_hash; - - // if search is not restricted then the leaf is the best - let mut best_hash = leaf_hash; - - // go backwards entering the search space - // waiting until we are <= max_number - if let Some(max_number) = maybe_max_number { - loop { - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - if current_header.number() <= &max_number { - best_hash = current_header.hash(); - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // go backwards through the chain (via parent links) - loop { - // until we find target - if current_hash == target_hash { - return Ok(Some(best_hash)); - } - - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - // stop search in this chain once we go below the target's block number - if current_header.number() < target_header.number() { - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // header may be on a dead fork -- the only leaves that are considered are - // those which can still be finalized. - // - // FIXME #1558 only issue this warning when not on a dead fork - warn!( - "Block {:?} exists in chain but not found when following all \ - leaves backwards. Number limit = {:?}", - target_hash, - maybe_max_number, - ); - - Ok(None) - } - fn leaves(&self) -> Result::Hash>, error::Error> { self.backend.blockchain().leaves() } @@ -1819,7 +1704,8 @@ where target_hash: Block::Hash, maybe_max_number: Option> ) -> Result, ConsensusError> { - LongestChain::best_containing(self, target_hash, maybe_max_number) + let import_lock = self.backend.get_import_lock(); + self.backend.blockchain().best_containing(target_hash, maybe_max_number, import_lock) .map_err(|e| ConsensusError::ChainLookup(e.to_string()).into()) } }