Move client consensus parts out of primitives and into client/consensus/api (#9319)

* moved client code out of primitives

* bump ci

* Fixup from merge.

* Removed unused deps thanks to review feedback

* Removing unneeded deps

* updating lock file

* note about rustfmt

* fixed typo to bump ci

* Move lonely CacheKeyId to parent

* cargo fmt

* updating import style

* Update docs/STYLE_GUIDE.md

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

substrate/client/service/src/builder.rs

@@ -36,6 +36,7 @@ use sc_client_api::{
 	ForkBlocks, StorageProvider, UsageProvider,
 };
 use sc_client_db::{Backend, DatabaseSettings};
+use sc_consensus::import_queue::ImportQueue;
 use sc_executor::{NativeExecutionDispatch, NativeExecutor, RuntimeInfo};
 use sc_keystore::LocalKeystore;
 use sc_network::{
@@ -49,9 +50,8 @@ use sc_telemetry::{telemetry, ConnectionMessage, Telemetry, TelemetryHandle, SUB
 use sc_transaction_pool_api::MaintainedTransactionPool;
 use sp_api::{CallApiAt, ProvideRuntimeApi};
 use sp_blockchain::{HeaderBackend, HeaderMetadata};
-use sp_consensus::{
-	block_validation::{BlockAnnounceValidator, Chain, DefaultBlockAnnounceValidator},
-	import_queue::ImportQueue,
+use sp_consensus::block_validation::{
+	BlockAnnounceValidator, Chain, DefaultBlockAnnounceValidator,
 };
 use sp_core::traits::{CodeExecutor, SpawnNamed};
 use sp_keystore::{CryptoStore, SyncCryptoStore, SyncCryptoStorePtr};

substrate/client/service/src/chain_ops/check_block.rs

@@ -20,7 +20,7 @@ use crate::error::Error;
 use codec::Encode;
 use futures::{future, prelude::*};
 use sc_client_api::{BlockBackend, UsageProvider};
-use sp_consensus::import_queue::ImportQueue;
+use sc_consensus::import_queue::ImportQueue;
 use sp_runtime::{generic::BlockId, traits::Block as BlockT};
 
 use crate::chain_ops::import_blocks;

substrate/client/service/src/chain_ops/import_blocks.rs

@@ -19,21 +19,21 @@
 use crate::{error, error::Error};
 use codec::{Decode, IoReader as CodecIoReader};
 use futures::{future, prelude::*};
+use futures_timer::Delay;
 use log::{info, warn};
 use sc_chain_spec::ChainSpec;
-use sp_consensus::{
-	import_queue::{BlockImportError, BlockImportResult, ImportQueue, IncomingBlock, Link},
-	BlockOrigin,
+use sc_client_api::UsageProvider;
+use sc_consensus::import_queue::{
+	BlockImportError, BlockImportStatus, ImportQueue, IncomingBlock, Link,
 };
+use serde_json::{de::IoRead as JsonIoRead, Deserializer, StreamDeserializer};
+use sp_consensus::BlockOrigin;
 use sp_runtime::{
 	generic::SignedBlock,
-	traits::{Block as BlockT, Header, MaybeSerializeDeserialize, NumberFor, Zero},
+	traits::{
+		Block as BlockT, CheckedDiv, Header, MaybeSerializeDeserialize, NumberFor, Saturating, Zero,
+	},
 };
-
-use futures_timer::Delay;
-use sc_client_api::UsageProvider;
-use serde_json::{de::IoRead as JsonIoRead, Deserializer, StreamDeserializer};
-use sp_runtime::traits::{CheckedDiv, Saturating};
 use std::{
 	convert::{TryFrom, TryInto},
 	io::{Read, Seek},
@@ -316,7 +316,7 @@ where
 			&mut self,
 			imported: usize,
 			_num_expected_blocks: usize,
-			results: Vec<(Result<BlockImportResult<NumberFor<B>>, BlockImportError>, B::Hash)>,
+			results: Vec<(Result<BlockImportStatus<NumberFor<B>>, BlockImportError>, B::Hash)>,
 		) {
 			self.imported_blocks += imported as u64;
 

substrate/client/service/src/client/client.rs

@@ -45,6 +45,9 @@ use sc_client_api::{
 	notifications::{StorageEventStream, StorageNotifications},
 	CallExecutor, ExecutorProvider, KeyIterator, ProofProvider, UsageProvider,
 };
+use sc_consensus::{
+	BlockCheckParams, BlockImportParams, ForkChoiceStrategy, ImportResult, StateAction,
+};
 use sc_executor::RuntimeVersion;
 use sc_light::fetcher::ChangesProof;
 use sc_telemetry::{telemetry, TelemetryHandle, SUBSTRATE_INFO};
@@ -56,10 +59,8 @@ use sp_blockchain::{
 	self as blockchain, well_known_cache_keys::Id as CacheKeyId, Backend as ChainBackend, Cache,
 	CachedHeaderMetadata, Error, HeaderBackend as ChainHeaderBackend, HeaderMetadata, ProvideCache,
 };
-use sp_consensus::{
-	BlockCheckParams, BlockImportParams, BlockOrigin, BlockStatus, Error as ConsensusError,
-	ForkChoiceStrategy, ImportResult, StateAction,
-};
+use sp_consensus::{BlockOrigin, BlockStatus, Error as ConsensusError};
+
 use sp_core::{
 	convert_hash,
 	storage::{well_known_keys, ChildInfo, PrefixedStorageKey, StorageData, StorageKey},
@@ -120,17 +121,18 @@ where
 	_phantom: PhantomData<RA>,
 }
 
-// used in importing a block, where additional changes are made after the runtime
-// executed.
+/// Used in importing a block, where additional changes are made after the runtime
+/// executed.
 enum PrePostHeader<H> {
-	// they are the same: no post-runtime digest items.
+	/// they are the same: no post-runtime digest items.
 	Same(H),
-	// different headers (pre, post).
+	/// different headers (pre, post).
 	Different(H, H),
 }
 
 impl<H> PrePostHeader<H> {
-	// get a reference to the "post-header" -- the header as it should be after all changes are applied.
+	/// get a reference to the "post-header" -- the header as it should be
+	/// after all changes are applied.
 	fn post(&self) -> &H {
 		match *self {
 			PrePostHeader::Same(ref h) => h,
@@ -138,7 +140,8 @@ impl<H> PrePostHeader<H> {
 		}
 	}
 
-	// convert to the "post-header" -- the header as it should be after all changes are applied.
+	/// convert to the "post-header" -- the header as it should be after
+	/// all changes are applied.
 	fn into_post(self) -> H {
 		match self {
 			PrePostHeader::Same(h) => h,
@@ -149,7 +152,7 @@ impl<H> PrePostHeader<H> {
 
 enum PrepareStorageChangesResult<B: backend::Backend<Block>, Block: BlockT> {
 	Discard(ImportResult),
-	Import(Option<sp_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>>),
+	Import(Option<sc_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>>),
 }
 
 /// Create an instance of in-memory client.
@@ -577,7 +580,8 @@ where
 		Ok(StorageProof::merge(proofs))
 	}
 
-	/// Generates CHT-based proof for roots of changes tries at given blocks (that are part of single CHT).
+	/// Generates CHT-based proof for roots of changes tries at given blocks
+	/// (that are part of single CHT).
 	fn changes_trie_roots_proof_at_cht(
 		&self,
 		cht_size: NumberFor<Block>,
@@ -603,11 +607,12 @@ where
 		Ok(proof)
 	}
 
-	/// Returns changes trie storage and all configurations that have been active in the range [first; last].
+	/// Returns changes trie storage and all configurations that have been active
+	/// in the range [first; last].
 	///
 	/// Configurations are returned in descending order (and obviously never overlap).
-	/// If fail_if_disabled is false, returns maximal consequent configurations ranges, starting from last and
-	/// stopping on either first, or when CT have been disabled.
+	/// If fail_if_disabled is false, returns maximal consequent configurations ranges,
+	/// starting from last and stopping on either first, or when CT have been disabled.
 	/// If fail_if_disabled is true, fails when there's a subrange where CT have been disabled
 	/// inside first..last blocks range.
 	fn require_changes_trie(
@@ -656,7 +661,7 @@ where
 		import_block: BlockImportParams<Block, backend::TransactionFor<B, Block>>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 		storage_changes: Option<
-			sp_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>,
+			sc_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>,
 		>,
 	) -> sp_blockchain::Result<ImportResult>
 	where
@@ -749,7 +754,7 @@ where
 		body: Option<Vec<Block::Extrinsic>>,
 		indexed_body: Option<Vec<Vec<u8>>>,
 		storage_changes: Option<
-			sp_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>,
+			sc_consensus::StorageChanges<Block, backend::TransactionFor<B, Block>>,
 		>,
 		new_cache: HashMap<CacheKeyId, Vec<u8>>,
 		finalized: bool,
@@ -793,7 +798,7 @@ where
 		let storage_changes = match storage_changes {
 			Some(storage_changes) => {
 				let storage_changes = match storage_changes {
-					sp_consensus::StorageChanges::Changes(storage_changes) => {
+					sc_consensus::StorageChanges::Changes(storage_changes) => {
 						self.backend
 							.begin_state_operation(&mut operation.op, BlockId::Hash(parent_hash))?;
 						let (main_sc, child_sc, offchain_sc, tx, _, changes_trie_tx, tx_index) =
@@ -813,7 +818,7 @@ where
 
 						Some((main_sc, child_sc))
 					},
-					sp_consensus::StorageChanges::Import(changes) => {
+					sc_consensus::StorageChanges::Import(changes) => {
 						let storage = sp_storage::Storage {
 							top: changes.state.into_iter().collect(),
 							children_default: Default::default(),
@@ -889,7 +894,8 @@ where
 
 		operation.op.insert_aux(aux)?;
 
-		// we only notify when we are already synced to the tip of the chain or if this import triggers a re-org
+		// we only notify when we are already synced to the tip of the chain
+		// or if this import triggers a re-org
 		if make_notifications || tree_route.is_some() {
 			if finalized {
 				operation.notify_finalized.push(hash);
@@ -933,7 +939,7 @@ where
 			(_, StateAction::Skip) => (false, None),
 			(
 				BlockStatus::InChainPruned,
-				StateAction::ApplyChanges(sp_consensus::StorageChanges::Changes(_)),
+				StateAction::ApplyChanges(sc_consensus::StorageChanges::Changes(_)),
 			) => return Ok(PrepareStorageChangesResult::Discard(ImportResult::MissingState)),
 			(BlockStatus::InChainPruned, StateAction::Execute) =>
 				return Ok(PrepareStorageChangesResult::Discard(ImportResult::MissingState)),
@@ -975,7 +981,7 @@ where
 				{
 					return Err(Error::InvalidStateRoot)
 				}
-				Some(sp_consensus::StorageChanges::Changes(gen_storage_changes))
+				Some(sc_consensus::StorageChanges::Changes(gen_storage_changes))
 			},
 			// No block body, no storage changes
 			(true, None, None) => None,
@@ -1852,7 +1858,7 @@ where
 /// objects. Otherwise, importing blocks directly into the client would be bypassing
 /// important verification work.
 #[async_trait::async_trait]
-impl<B, E, Block, RA> sp_consensus::BlockImport<Block> for &Client<B, E, Block, RA>
+impl<B, E, Block, RA> sc_consensus::BlockImport<Block> for &Client<B, E, Block, RA>
 where
 	B: backend::Backend<Block>,
 	E: CallExecutor<Block> + Send + Sync,
@@ -1960,7 +1966,7 @@ where
 }
 
 #[async_trait::async_trait]
-impl<B, E, Block, RA> sp_consensus::BlockImport<Block> for Client<B, E, Block, RA>
+impl<B, E, Block, RA> sc_consensus::BlockImport<Block> for Client<B, E, Block, RA>
 where
 	B: backend::Backend<Block>,
 	E: CallExecutor<Block> + Send + Sync,

substrate/client/service/src/lib.rs

@@ -67,6 +67,7 @@ pub use sc_chain_spec::{
 	Properties, RuntimeGenesis,
 };
 use sc_client_api::{blockchain::HeaderBackend, BlockchainEvents};
+pub use sc_consensus::ImportQueue;
 pub use sc_executor::NativeExecutionDispatch;
 #[doc(hidden)]
 pub use sc_network::config::{OnDemand, TransactionImport, TransactionImportFuture};
@@ -74,7 +75,6 @@ pub use sc_rpc::Metadata as RpcMetadata;
 pub use sc_tracing::TracingReceiver;
 pub use sc_transaction_pool::Options as TransactionPoolOptions;
 pub use sc_transaction_pool_api::{error::IntoPoolError, InPoolTransaction, TransactionPool};
-pub use sp_consensus::import_queue::ImportQueue;
 #[doc(hidden)]
 pub use std::{ops::Deref, result::Result, sync::Arc};
 pub use task_manager::{SpawnTaskHandle, TaskManager};