Optimize tree route to sync faster (#3665)

* Introduce HeaderMetadata and use it for tree_route. Add lowest_common_ancestor.
* Add tests.

substrate/core/client/src/blockchain.rs

@@ -24,6 +24,8 @@ use sr_primitives::Justification;
 use log::warn;
 use parking_lot::Mutex;
 
+use header_metadata::HeaderMetadata;
+
 use crate::error::{Error, Result};
 
 /// Blockchain database header backend. Does not perform any validation.
@@ -74,7 +76,7 @@ pub trait HeaderBackend<Block: BlockT>: Send + Sync {
 }
 
 /// Blockchain database backend. Does not perform any validation.
-pub trait Backend<Block: BlockT>: HeaderBackend<Block> {
+pub trait Backend<Block: BlockT>: HeaderBackend<Block> + HeaderMetadata<Block, Error=Error> {
 	/// Get block body. Returns `None` if block is not found.
 	fn body(&self, id: BlockId<Block>) -> Result<Option<Vec<<Block as BlockT>::Extrinsic>>>;
 	/// Get block justification. Returns `None` if justification does not exist.
@@ -257,122 +259,6 @@ pub enum BlockStatus {
 	Unknown,
 }
 
-/// An entry in a tree route.
-#[derive(Debug)]
-pub struct RouteEntry<Block: BlockT> {
-	/// The number of the block.
-	pub number: <Block::Header as HeaderT>::Number,
-	/// The hash of the block.
-	pub hash: Block::Hash,
-}
-
-/// A tree-route from one block to another in the chain.
-///
-/// All blocks prior to the pivot in the deque is the reverse-order unique ancestry
-/// of the first block, the block at the pivot index is the common ancestor,
-/// and all blocks after the pivot is the ancestry of the second block, in
-/// order.
-///
-/// The ancestry sets will include the given blocks, and thus the tree-route is
-/// never empty.
-///
-/// ```text
-/// Tree route from R1 to E2. Retracted is [R1, R2, R3], Common is C, enacted [E1, E2]
-///   <- R3 <- R2 <- R1
-///  /
-/// C
-///  \-> E1 -> E2
-/// ```
-///
-/// ```text
-/// Tree route from C to E2. Retracted empty. Common is C, enacted [E1, E2]
-/// C -> E1 -> E2
-/// ```
-#[derive(Debug)]
-pub struct TreeRoute<Block: BlockT> {
-	route: Vec<RouteEntry<Block>>,
-	pivot: usize,
-}
-
-impl<Block: BlockT> TreeRoute<Block> {
-	/// Get a slice of all retracted blocks in reverse order (towards common ancestor)
-	pub fn retracted(&self) -> &[RouteEntry<Block>] {
-		&self.route[..self.pivot]
-	}
-
-	/// Get the common ancestor block. This might be one of the two blocks of the
-	/// route.
-	pub fn common_block(&self) -> &RouteEntry<Block> {
-		self.route.get(self.pivot).expect("tree-routes are computed between blocks; \
-			which are included in the route; \
-			thus it is never empty; qed")
-	}
-
-	/// Get a slice of enacted blocks (descendents of the common ancestor)
-	pub fn enacted(&self) -> &[RouteEntry<Block>] {
-		&self.route[self.pivot + 1 ..]
-	}
-}
-
-/// Compute a tree-route between two blocks. See tree-route docs for more details.
-pub fn tree_route<Block: BlockT, F: Fn(BlockId<Block>) -> Result<<Block as BlockT>::Header>>(
-	load_header: F,
-	from: BlockId<Block>,
-	to: BlockId<Block>,
-) -> Result<TreeRoute<Block>> {
-	let mut from = load_header(from)?;
-	let mut to = load_header(to)?;
-
-	let mut from_branch = Vec::new();
-	let mut to_branch = Vec::new();
-
-	while to.number() > from.number() {
-		to_branch.push(RouteEntry {
-			number: to.number().clone(),
-			hash: to.hash(),
-		});
-
-		to = load_header(BlockId::Hash(*to.parent_hash()))?;
-	}
-
-	while from.number() > to.number() {
-		from_branch.push(RouteEntry {
-			number: from.number().clone(),
-			hash: from.hash(),
-		});
-		from = load_header(BlockId::Hash(*from.parent_hash()))?;
-	}
-
-	// numbers are equal now. walk backwards until the block is the same
-
-	while to != from {
-		to_branch.push(RouteEntry {
-			number: to.number().clone(),
-			hash: to.hash(),
-		});
-		to = load_header(BlockId::Hash(*to.parent_hash()))?;
-
-		from_branch.push(RouteEntry {
-			number: from.number().clone(),
-			hash: from.hash(),
-		});
-		from = load_header(BlockId::Hash(*from.parent_hash()))?;
-	}
-
-	// add the pivot block. and append the reversed to-branch (note that it's reverse order originalls)
-	let pivot = from_branch.len();
-	from_branch.push(RouteEntry {
-		number: to.number().clone(),
-		hash: to.hash(),
-	});
-	from_branch.extend(to_branch.into_iter().rev());
-
-	Ok(TreeRoute {
-		route: from_branch,
-		pivot,
-	})
-}
-
 /// A list of all well known keys in the blockchain cache.
 pub mod well_known_cache_keys {
 	/// The type representing cache keys.

substrate/core/client/src/client.rs

@@ -51,6 +51,7 @@ use consensus::{
 	ImportResult, BlockOrigin, ForkChoiceStrategy,
 	SelectChain, self,
 };
+use header_metadata::{HeaderMetadata, CachedHeaderMetadata};
 
 use crate::{
 	runtime_api::{
@@ -966,10 +967,10 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		};
 
 		let retracted = if is_new_best {
-			let route_from_best = crate::blockchain::tree_route(
-				|id| self.header(&id)?.ok_or_else(|| Error::UnknownBlock(format!("{:?}", id))),
-				BlockId::Hash(info.best_hash),
-				BlockId::Hash(parent_hash),
+			let route_from_best = header_metadata::tree_route(
+				self.backend.blockchain(),
+				info.best_hash,
+				parent_hash,
 			)?;
 			route_from_best.retracted().iter().rev().map(|e| e.hash.clone()).collect()
 		} else {
@@ -1100,11 +1101,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			return Ok(());
 		}
 
-		let route_from_finalized = crate::blockchain::tree_route(
-			|id| self.header(&id)?.ok_or_else(|| Error::UnknownBlock(format!("{:?}", id))),
-			BlockId::Hash(last_finalized),
-			BlockId::Hash(block),
-		)?;
+		let route_from_finalized = header_metadata::tree_route(self.backend.blockchain(), last_finalized, block)?;
 
 		if let Some(retracted) = route_from_finalized.retracted().get(0) {
 			warn!("Safety violation: attempted to revert finalized block {:?} which is not in the \
@@ -1113,11 +1110,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			return Err(error::Error::NotInFinalizedChain);
 		}
 
-		let route_from_best = crate::blockchain::tree_route(
-			|id| self.header(&id)?.ok_or_else(|| Error::UnknownBlock(format!("{:?}", id))),
-			BlockId::Hash(best_block),
-			BlockId::Hash(block),
-		)?;
+		let route_from_best = header_metadata::tree_route(self.backend.blockchain(), best_block, block)?;
 
 		// if the block is not a direct ancestor of the current best chain,
 		// then some other block is the common ancestor.
@@ -1321,6 +1314,26 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	}
 }
 
+impl<B, E, Block, RA> HeaderMetadata<Block> for Client<B, E, Block, RA> where
+	B: backend::Backend<Block, Blake2Hasher>,
+	E: CallExecutor<Block, Blake2Hasher>,
+	Block: BlockT<Hash=H256>,
+{
+	type Error = error::Error;
+
+	fn header_metadata(&self, hash: Block::Hash) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
+		self.backend.blockchain().header_metadata(hash)
+	}
+
+	fn insert_header_metadata(&self, hash: Block::Hash, metadata: CachedHeaderMetadata<Block>) {
+		self.backend.blockchain().insert_header_metadata(hash, metadata)
+	}
+
+	fn remove_header_metadata(&self, hash: Block::Hash) {
+		self.backend.blockchain().remove_header_metadata(hash)
+	}
+}
+
 impl<B, E, Block, RA> ProvideUncles<Block> for Client<B, E, Block, RA> where
 	B: backend::Backend<Block, Blake2Hasher>,
 	E: CallExecutor<Block, Blake2Hasher>,
@@ -1798,7 +1811,7 @@ where
 /// Utility methods for the client.
 pub mod utils {
 	use super::*;
-	use crate::{blockchain, error};
+	use crate::error;
 	use primitives::H256;
 	use std::borrow::Borrow;
 
@@ -1812,7 +1825,7 @@ pub mod utils {
 		client: &'a T,
 		current: Option<(H, H)>,
 	) -> impl Fn(&H256, &H256) -> Result<bool, error::Error> + 'a
-		where T: ChainHeaderBackend<Block>,
+		where T: ChainHeaderBackend<Block> + HeaderMetadata<Block, Error=error::Error>,
 	{
 		move |base, hash| {
 			if base == hash { return Ok(false); }
@@ -1831,13 +1844,9 @@ pub mod utils {
 				}
 			}
 
-			let tree_route = blockchain::tree_route(
-				|id| client.header(id)?.ok_or_else(|| Error::UnknownBlock(format!("{:?}", id))),
-				BlockId::Hash(*hash),
-				BlockId::Hash(*base),
-			)?;
+			let ancestor = header_metadata::lowest_common_ancestor(client, *hash, *base)?;
 
-			Ok(tree_route.common_block().hash == *base)
+			Ok(ancestor.hash == *base)
 		}
 	}
 }

substrate/core/client/src/in_mem.rs

@@ -27,12 +27,15 @@ use state_machine::backend::{Backend as StateBackend, InMemory};
 use state_machine::{self, InMemoryChangesTrieStorage, ChangesTrieAnchorBlockId, ChangesTrieTransaction};
 use hash_db::{Hasher, Prefix};
 use trie::MemoryDB;
+use header_metadata::{CachedHeaderMetadata, HeaderMetadata};
 
 use crate::error;
 use crate::backend::{self, NewBlockState, StorageCollection, ChildStorageCollection};
 use crate::light;
 use crate::leaves::LeafSet;
-use crate::blockchain::{self, BlockStatus, HeaderBackend, well_known_cache_keys::Id as CacheKeyId};
+use crate::blockchain::{
+	self, BlockStatus, HeaderBackend, well_known_cache_keys::Id as CacheKeyId
+};
 
 struct PendingBlock<B: BlockT> {
 	block: StoredBlock<B>,
@@ -221,11 +224,7 @@ impl<Block: BlockT> Blockchain<Block> {
 			if &best_hash == header.parent_hash() {
 				None
 			} else {
-				let route = crate::blockchain::tree_route(
-					|id| self.header(id)?.ok_or_else(|| error::Error::UnknownBlock(format!("{:?}", id))),
-					BlockId::Hash(best_hash),
-					BlockId::Hash(*header.parent_hash()),
-				)?;
+				let route = header_metadata::tree_route(self, best_hash, *header.parent_hash())?;
 				Some(route)
 			}
 		};
@@ -320,6 +319,21 @@ impl<Block: BlockT> HeaderBackend<Block> for Blockchain<Block> {
 	}
 }
 
+impl<Block: BlockT> HeaderMetadata<Block> for Blockchain<Block> {
+	type Error = error::Error;
+
+	fn header_metadata(&self, hash: Block::Hash) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
+		self.header(BlockId::hash(hash))?.map(|header| CachedHeaderMetadata::from(&header))
+			.ok_or(error::Error::UnknownBlock("header not found".to_owned()))
+	}
+
+	fn insert_header_metadata(&self, _hash: Block::Hash, _metadata: CachedHeaderMetadata<Block>) {
+		// No need to implement.
+	}
+	fn remove_header_metadata(&self, _hash: Block::Hash) {
+		// No need to implement.
+	}
+}
 
 impl<Block: BlockT> blockchain::Backend<Block> for Blockchain<Block> {
 	fn body(&self, id: BlockId<Block>) -> error::Result<Option<Vec<<Block as BlockT>::Extrinsic>>> {

substrate/core/client/src/light/blockchain.rs

@@ -23,18 +23,20 @@ use std::{sync::Arc, collections::HashMap};
 use sr_primitives::{Justification, generic::BlockId};
 use sr_primitives::traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero};
 
+use header_metadata::{HeaderMetadata, CachedHeaderMetadata};
+
 use crate::backend::{AuxStore, NewBlockState};
 use crate::blockchain::{
 	Backend as BlockchainBackend, BlockStatus, Cache as BlockchainCache,
 	HeaderBackend as BlockchainHeaderBackend, Info as BlockchainInfo, ProvideCache,
-	well_known_cache_keys,
+	well_known_cache_keys
 };
 use crate::cht;
 use crate::error::{Error as ClientError, Result as ClientResult};
 use crate::light::fetcher::{Fetcher, RemoteHeaderRequest};
 
 /// Light client blockchain storage.
-pub trait Storage<Block: BlockT>: AuxStore + BlockchainHeaderBackend<Block> {
+pub trait Storage<Block: BlockT>: AuxStore + BlockchainHeaderBackend<Block> + HeaderMetadata<Block, Error=ClientError> {
 	/// Store new header. Should refuse to revert any finalized blocks.
 	///
 	/// Takes new authorities, the leaf state of the new block, and
@@ -140,6 +142,22 @@ impl<S, Block> BlockchainHeaderBackend<Block> for Blockchain<S> where Block: Blo
 	}
 }
 
+impl<S, Block> HeaderMetadata<Block> for Blockchain<S> where Block: BlockT, S: Storage<Block> {
+	type Error = ClientError;
+
+	fn header_metadata(&self, hash: Block::Hash) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
+		self.storage.header_metadata(hash)
+	}
+
+	fn insert_header_metadata(&self, hash: Block::Hash, metadata: CachedHeaderMetadata<Block>) {
+		self.storage.insert_header_metadata(hash, metadata)
+	}
+
+	fn remove_header_metadata(&self, hash: Block::Hash) {
+		self.storage.remove_header_metadata(hash)
+	}
+}
+
 impl<S, Block> BlockchainBackend<Block> for Blockchain<S> where Block: BlockT, S: Storage<Block> {
 	fn body(&self, _id: BlockId<Block>) -> ClientResult<Option<Vec<Block::Extrinsic>>> {
 		Err(ClientError::NotAvailableOnLightClient)
@@ -281,6 +299,17 @@ pub mod tests {
 		}
 	}
 
+	impl HeaderMetadata<Block> for DummyStorage {
+		type Error = ClientError;
+
+		fn header_metadata(&self, hash: Hash) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
+			self.header(BlockId::hash(hash))?.map(|header| CachedHeaderMetadata::from(&header))
+				.ok_or(ClientError::UnknownBlock("header not found".to_owned()))
+		}
+		fn insert_header_metadata(&self, _hash: Hash, _metadata: CachedHeaderMetadata<Block>) {}
+		fn remove_header_metadata(&self, _hash: Hash) {}
+	}
+
 	impl AuxStore for DummyStorage {
 		fn insert_aux<
 			'a,