removes use of sc_client::Client from sc_basic_authorship (#5050)

* removes use of sc-client from sc-basic-authorship

* refactor use of ProposerFactory

* correct dep path

substrate/Cargo.lock

@@ -5615,7 +5615,6 @@ dependencies = [
  "parity-scale-codec",
  "parking_lot 0.10.0",
  "sc-block-builder",
- "sc-client",
  "sc-client-api",
  "sc-telemetry",
  "sc-transaction-pool",
@@ -6112,6 +6111,7 @@ dependencies = [
  "parking_lot 0.10.0",
  "pin-project",
  "rand 0.7.3",
+ "sc-block-builder",
  "sc-client",
  "sc-client-api",
  "sc-keystore",
@@ -6944,6 +6944,7 @@ dependencies = [
  "criterion 0.3.1",
  "parity-scale-codec",
  "rustversion",
+ "sc-block-builder",
  "sp-api",
  "sp-blockchain",
  "sp-consensus",

substrate/bin/node-template/node/src/service.rs

@@ -93,10 +93,10 @@ pub fn new_full(config: Configuration<GenesisConfig>)
 		.build()?;
 
 	if participates_in_consensus {
-		let proposer = sc_basic_authorship::ProposerFactory {
+		let proposer = sc_basic_authorship::ProposerFactory::new(
-			client: service.client(),
+			service.client(),
-			transaction_pool: service.transaction_pool(),
+			service.transaction_pool()
-		};
+		);
 
 		let client = service.client();
 		let select_chain = service.select_chain()

substrate/bin/node/cli/src/service.rs

@@ -150,10 +150,10 @@ macro_rules! new_full {
 		($with_startup_data)(&block_import, &babe_link);
 
 		if participates_in_consensus {
-			let proposer = sc_basic_authorship::ProposerFactory {
+			let proposer = sc_basic_authorship::ProposerFactory::new(
-				client: service.client(),
+				service.client(),
-				transaction_pool: service.transaction_pool(),
+				service.transaction_pool()
-			};
+			);
 
 			let client = service.client();
 			let select_chain = service.select_chain()
@@ -501,10 +501,10 @@ mod tests {
 				let parent_header = service.client().header(&parent_id).unwrap().unwrap();
 				let parent_hash = parent_header.hash();
 				let parent_number = *parent_header.number();
-				let mut proposer_factory = sc_basic_authorship::ProposerFactory {
+				let mut proposer_factory = sc_basic_authorship::ProposerFactory::new(
-					client: service.client(),
+					service.client(),
-					transaction_pool: service.transaction_pool(),
+					service.transaction_pool()
-				};
+				);
 
 				let epoch = babe_link.epoch_changes().lock().epoch_for_child_of(
 					descendent_query(&*service.client()),

substrate/client/basic-authorship/Cargo.toml

@@ -16,7 +16,6 @@ sp-api = { version = "2.0.0-alpha.2", path = "../../primitives/api" }
 sp-runtime = { version = "2.0.0-alpha.2", path = "../../primitives/runtime" }
 sp-core = { version = "2.0.0-alpha.2", path = "../../primitives/core" }
 sp-blockchain = { version = "2.0.0-alpha.2", path = "../../primitives/blockchain" }
-sc-client = { version = "0.8.0-alpha.2", path = "../" }
 sc-client-api = { version = "2.0.0-alpha.2", path = "../api" }
 sp-consensus = { version = "0.8.0-alpha.2", path = "../../primitives/consensus/common" }
 sp-inherents = { version = "2.0.0-alpha.2", path = "../../primitives/inherents" }

substrate/client/basic-authorship/src/basic_authorship.rs

@@ -19,8 +19,7 @@
 // FIXME #1021 move this into sp-consensus
 
 use std::{time, sync::Arc};
-use sc_client_api::{CallExecutor, backend};
+use sc_client_api::backend;
-use sc_client::Client as SubstrateClient;
 use codec::Decode;
 use sp_consensus::{evaluation, Proposal, RecordProof};
 use sp_inherents::InherentData;
@@ -32,35 +31,47 @@ use sp_runtime::{
 };
 use sp_transaction_pool::{TransactionPool, InPoolTransaction};
 use sc_telemetry::{telemetry, CONSENSUS_INFO};
-use sc_block_builder::BlockBuilderApi;
+use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
 use sp_api::{ProvideRuntimeApi, ApiExt};
 use futures::prelude::*;
+use sp_blockchain::HeaderBackend;
+use std::marker::PhantomData;
 
 /// Proposer factory.
-pub struct ProposerFactory<C, A> where A: TransactionPool {
+pub struct ProposerFactory<A, B, C> {
 	/// The client instance.
-	pub client: Arc<C>,
+	client: Arc<C>,
 	/// The transaction pool.
-	pub transaction_pool: Arc<A>,
+	transaction_pool: Arc<A>,
+	/// phantom member to pin the `Backend` type.
+	_phantom: PhantomData<B>,
 }
 
-impl<B, E, Block, RA, A> ProposerFactory<SubstrateClient<B, E, Block, RA>, A>
+impl<A, B, C> ProposerFactory<A, B, C> {
+	pub fn new(client: Arc<C>, transaction_pool: Arc<A>) -> Self {
+		ProposerFactory {
+			client,
+			transaction_pool,
+			_phantom: PhantomData,
+		}
+	}
+}
+
+impl<B, Block, C, A> ProposerFactory<A, B, C>
 	where
 		A: TransactionPool<Block = Block> + 'static,
 		B: backend::Backend<Block> + Send + Sync + 'static,
-		E: CallExecutor<Block> + Send + Sync + Clone + 'static,
 		Block: BlockT,
-		RA: Send + Sync + 'static,
+		C: BlockBuilderProvider<B, Block, C> + HeaderBackend<Block> + ProvideRuntimeApi<Block>
-		SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi<Block>,
+			+ Send + Sync + 'static,
-		<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi<Block>>::Api:
+		C::Api: ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
-			BlockBuilderApi<Block, Error = sp_blockchain::Error> +
+			+ BlockBuilderApi<Block, Error = sp_blockchain::Error>,
-			ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>,
 {
 	pub fn init_with_now(
 		&mut self,
 		parent_header: &<Block as BlockT>::Header,
 		now: Box<dyn Fn() -> time::Instant + Send + Sync>,
-	) -> Proposer<Block, SubstrateClient<B, E, Block, RA>, A> {
+	) -> Proposer<B, Block, C, A> {
 		let parent_hash = parent_header.hash();
 
 		let id = BlockId::hash(parent_hash);
@@ -75,6 +86,7 @@ impl<B, E, Block, RA, A> ProposerFactory<SubstrateClient<B, E, Block, RA>, A>
 				parent_number: *parent_header.number(),
 				transaction_pool: self.transaction_pool.clone(),
 				now,
+				_phantom: PhantomData,
 			}),
 		};
 
@@ -82,21 +94,19 @@ impl<B, E, Block, RA, A> ProposerFactory<SubstrateClient<B, E, Block, RA>, A>
 	}
 }
 
-impl<B, E, Block, RA, A> sp_consensus::Environment<Block> for
+impl<A, B, Block, C> sp_consensus::Environment<Block> for
-	ProposerFactory<SubstrateClient<B, E, Block, RA>, A>
+	ProposerFactory<A, B, C>
 		where
 			A: TransactionPool<Block = Block> + 'static,
 			B: backend::Backend<Block> + Send + Sync + 'static,
-			E: CallExecutor<Block> + Send + Sync + Clone + 'static,
 			Block: BlockT,
-			RA: Send + Sync + 'static,
+			C: BlockBuilderProvider<B, Block, C> + HeaderBackend<Block> + ProvideRuntimeApi<Block>
-			SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi<Block>,
+				+ Send + Sync + 'static,
-			<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi<Block>>::Api:
+			C::Api: ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
-				BlockBuilderApi<Block, Error = sp_blockchain::Error> +
+				+ BlockBuilderApi<Block, Error = sp_blockchain::Error>,
-				ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>,
 {
 	type CreateProposer = future::Ready<Result<Self::Proposer, Self::Error>>;
-	type Proposer = Proposer<Block, SubstrateClient<B, E, Block, RA>, A>;
+	type Proposer = Proposer<B, Block, C, A>;
 	type Error = sp_blockchain::Error;
 
 	fn init(
@@ -108,32 +118,31 @@ impl<B, E, Block, RA, A> sp_consensus::Environment<Block> for
 }
 
 /// The proposer logic.
-pub struct Proposer<Block: BlockT, C, A: TransactionPool> {
+pub struct Proposer<B, Block: BlockT, C, A: TransactionPool> {
-	inner: Arc<ProposerInner<Block, C, A>>,
+	inner: Arc<ProposerInner<B, Block, C, A>>,
 }
 
 /// Proposer inner, to wrap parameters under Arc.
-struct ProposerInner<Block: BlockT, C, A: TransactionPool> {
+struct ProposerInner<B, Block: BlockT, C, A: TransactionPool> {
 	client: Arc<C>,
 	parent_hash: <Block as BlockT>::Hash,
 	parent_id: BlockId<Block>,
 	parent_number: <<Block as BlockT>::Header as HeaderT>::Number,
 	transaction_pool: Arc<A>,
 	now: Box<dyn Fn() -> time::Instant + Send + Sync>,
+	_phantom: PhantomData<B>,
 }
 
-impl<B, E, Block, RA, A> sp_consensus::Proposer<Block> for
+impl<A, B, Block, C> sp_consensus::Proposer<Block> for
-	Proposer<Block, SubstrateClient<B, E, Block, RA>, A>
+	Proposer<B, Block, C, A>
 		where
 			A: TransactionPool<Block = Block> + 'static,
 			B: backend::Backend<Block> + Send + Sync + 'static,
-			E: CallExecutor<Block> + Send + Sync + Clone + 'static,
 			Block: BlockT,
-			RA: Send + Sync + 'static,
+			C: BlockBuilderProvider<B, Block, C> + HeaderBackend<Block> + ProvideRuntimeApi<Block>
-			SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi<Block>,
+				+ Send + Sync + 'static,
-			<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi<Block>>::Api:
+			C::Api: ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
-				BlockBuilderApi<Block, Error = sp_blockchain::Error> +
+				+ BlockBuilderApi<Block, Error = sp_blockchain::Error>,
-				ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>,
 {
 	type Transaction = backend::TransactionFor<B, Block>;
 	type Proposal = tokio_executor::blocking::Blocking<
@@ -157,16 +166,15 @@ impl<B, E, Block, RA, A> sp_consensus::Proposer<Block> for
 	}
 }
 
-impl<Block, B, E, RA, A> ProposerInner<Block, SubstrateClient<B, E, Block, RA>, A>	where
+impl<A, B, Block, C> ProposerInner<B, Block, C, A>
-	A: TransactionPool<Block = Block>,
+	where
-	B: sc_client_api::backend::Backend<Block> + Send + Sync + 'static,
+		A: TransactionPool<Block = Block>,
-	E: CallExecutor<Block> + Send + Sync + Clone + 'static,
+		B: backend::Backend<Block> + Send + Sync + 'static,
-	Block: BlockT,
+		Block: BlockT,
-	RA: Send + Sync + 'static,
+		C: BlockBuilderProvider<B, Block, C> + HeaderBackend<Block> + ProvideRuntimeApi<Block>
-	SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi<Block>,
+			+ Send + Sync + 'static,
-	<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi<Block>>::Api:
+		C::Api: ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
-		BlockBuilderApi<Block, Error = sp_blockchain::Error> +
+			+ BlockBuilderApi<Block, Error = sp_blockchain::Error>,
-		ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>,
 {
 	fn propose_with(
 		&self,
@@ -315,10 +323,7 @@ mod tests {
 			txpool.submit_at(&BlockId::number(0), vec![extrinsic(0), extrinsic(1)])
 		).unwrap();
 
-		let mut proposer_factory = ProposerFactory {
+		let mut proposer_factory = ProposerFactory::new(client.clone(), txpool.clone());
-			client: client.clone(),
-			transaction_pool: txpool.clone(),
-		};
 
 		let cell = Mutex::new(time::Instant::now());
 		let mut proposer = proposer_factory.init_with_now(
@@ -359,10 +364,7 @@ mod tests {
 			txpool.submit_at(&BlockId::number(0), vec![extrinsic(0)]),
 		).unwrap();
 
-		let mut proposer_factory = ProposerFactory {
+		let mut proposer_factory = ProposerFactory::new(client.clone(), txpool.clone());
-			client: client.clone(),
-			transaction_pool: txpool.clone(),
-		};
 
 		let mut proposer = proposer_factory.init_with_now(
 			&client.header(&block_id).unwrap().unwrap(),

substrate/client/basic-authorship/src/lib.rs

@@ -28,10 +28,7 @@
 //! # let client = Arc::new(substrate_test_runtime_client::new());
 //! # let txpool = Arc::new(BasicPool::new(Default::default(), Arc::new(FullChainApi::new(client.clone()))).0);
 //! // The first step is to create a `ProposerFactory`.
-//! let mut proposer_factory = ProposerFactory {
+//! let mut proposer_factory = ProposerFactory::new(client.clone(), txpool.clone());
-//! 	client: client.clone(),
-//! 	transaction_pool: txpool.clone(),
-//! };
 //!
 //! // From this factory, we create a `Proposer`.
 //! let proposer = proposer_factory.init(

substrate/client/block-builder/src/lib.rs

@@ -63,6 +63,27 @@ impl<Block: BlockT, StateBackend: backend::StateBackend<HasherFor<Block>>> Built
 	}
 }
 
+/// Block builder provider
+pub trait BlockBuilderProvider<B, Block, RA>
+	where
+		Block: BlockT,
+		B: backend::Backend<Block>,
+		Self: Sized,
+		RA: ProvideRuntimeApi<Block>,
+{
+	/// Create a new block, built on top of `parent`.
+	///
+	/// When proof recording is enabled, all accessed trie nodes are saved.
+	/// These recorded trie nodes can be used by a third party to proof the
+	/// output of this block builder without having access to the full storage.
+	fn new_block_at<R: Into<RecordProof>>(
+		&self,
+		parent: &BlockId<Block>,
+		inherent_digests: DigestFor<Block>,
+		record_proof: R,
+	) -> sp_blockchain::Result<BlockBuilder<Block, RA, B>>;
+}
+
 /// Utility for building new (valid) blocks from a stream of extrinsics.
 pub struct BlockBuilder<'a, Block: BlockT, A: ProvideRuntimeApi<Block>, B> {
 	extrinsics: Vec<Block::Extrinsic>,

substrate/client/consensus/babe/src/tests.rs

@@ -23,7 +23,7 @@ use super::*;
 use authorship::claim_slot;
 
 use sp_consensus_babe::{AuthorityPair, SlotNumber};
-use sc_block_builder::BlockBuilder;
+use sc_block_builder::{BlockBuilder, BlockBuilderProvider};
 use sp_consensus::{
 	NoNetwork as DummyOracle, Proposal, RecordProof,
 	import_queue::{BoxBlockImport, BoxJustificationImport, BoxFinalityProofImport},

substrate/client/consensus/manual-seal/src/lib.rs

@@ -244,10 +244,10 @@ mod tests {
 		let select_chain = LongestChain::new(backend.clone());
 		let inherent_data_providers = InherentDataProviders::new();
 		let pool = Arc::new(BasicPool::new(Options::default(), api()).0);
-		let env = ProposerFactory {
+		let env = ProposerFactory::new(
-			transaction_pool: pool.clone(),
+			client.clone(),
-			client: client.clone(),
+			pool.clone()
-		};
+		);
 		// this test checks that blocks are created as soon as transactions are imported into the pool.
 		let (sender, receiver) = futures::channel::oneshot::channel();
 		let mut sender = Arc::new(Some(sender));
@@ -309,10 +309,10 @@ mod tests {
 		let select_chain = LongestChain::new(backend.clone());
 		let inherent_data_providers = InherentDataProviders::new();
 		let pool = Arc::new(BasicPool::new(Options::default(), api()).0);
-		let env = ProposerFactory {
+		let env = ProposerFactory::new(
-			transaction_pool: pool.clone(),
+			client.clone(),
-			client: client.clone(),
+			pool.clone()
-		};
+		);
 		// this test checks that blocks are created as soon as an engine command is sent over the stream.
 		let (mut sink, stream) = futures::channel::mpsc::channel(1024);
 		let future = run_manual_seal(
@@ -378,10 +378,10 @@ mod tests {
 		let inherent_data_providers = InherentDataProviders::new();
 		let pool_api = api();
 		let pool = Arc::new(BasicPool::new(Options::default(), pool_api.clone()).0);
-		let env = ProposerFactory {
+		let env = ProposerFactory::new(
-			transaction_pool: pool.clone(),
+			client.clone(),
-			client: client.clone(),
+			pool.clone(),
-		};
+		);
 		// this test checks that blocks are created as soon as an engine command is sent over the stream.
 		let (mut sink, stream) = futures::channel::mpsc::channel(1024);
 		let future = run_manual_seal(

substrate/client/finality-grandpa/Cargo.toml

@@ -35,6 +35,7 @@ sc-network = { version = "0.8.0-alpha.2", path = "../network" }
 sc-network-gossip = { version = "0.8.0-alpha.2", path = "../network-gossip" }
 sp-finality-tracker = { version = "2.0.0-alpha.2", path = "../../primitives/finality-tracker" }
 sp-finality-grandpa = { version = "2.0.0-alpha.2", path = "../../primitives/finality-grandpa" }
+sc-block-builder = { version = "0.8.0-alpha.2", path = "../block-builder" }
 finality-grandpa = { version = "0.11.1", features = ["derive-codec"] }
 pin-project = "0.4.6"
 

substrate/client/finality-grandpa/src/tests.rs

@@ -55,6 +55,7 @@ use finality_proof::{
 	FinalityProofProvider, AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker,
 };
 use consensus_changes::ConsensusChanges;
+use sc_block_builder::BlockBuilderProvider;
 
 type PeerData =
 	Mutex<

substrate/client/network/test/src/lib.rs

@@ -35,7 +35,7 @@ use sc_client_api::{
 	FinalityNotification,
 	backend::{TransactionFor, AuxStore, Backend, Finalizer},
 };
-use sc_block_builder::BlockBuilder;
+use sc_block_builder::{BlockBuilder, BlockBuilderProvider};
 use sc_client::LongestChain;
 use sc_network::config::Roles;
 use sp_consensus::block_validation::DefaultBlockAnnounceValidator;

substrate/client/src/client.rs

@@ -60,7 +60,7 @@ use sp_api::{
 	CallApiAt, ConstructRuntimeApi, Core as CoreApi, ApiExt, ApiRef, ProvideRuntimeApi,
 	CallApiAtParams,
 };
-use sc_block_builder::BlockBuilderApi;
+use sc_block_builder::{BlockBuilderApi, BlockBuilderProvider};
 
 pub use sc_client_api::{
 	backend::{
@@ -863,33 +863,6 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		)
 	}
 
-	/// Create a new block, built on top of `parent`.
-	///
-	/// When proof recording is enabled, all accessed trie nodes are saved.
-	/// These recorded trie nodes can be used by a third party to proof the
-	/// output of this block builder without having access to the full storage.
-	pub fn new_block_at<R: Into<RecordProof>>(
-		&self,
-		parent: &BlockId<Block>,
-		inherent_digests: DigestFor<Block>,
-		record_proof: R,
-	) -> sp_blockchain::Result<sc_block_builder::BlockBuilder<Block, Self, B>> where
-		E: Clone + Send + Sync,
-		RA: Send + Sync,
-		Self: ProvideRuntimeApi<Block>,
-		<Self as ProvideRuntimeApi<Block>>::Api: BlockBuilderApi<Block, Error = Error> +
-			ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
-	{
-		sc_block_builder::BlockBuilder::new(
-			self,
-			self.expect_block_hash_from_id(parent)?,
-			self.expect_block_number_from_id(parent)?,
-			record_proof.into(),
-			inherent_digests,
-			&self.backend
-		)
-	}
-
 	/// Apply a checked and validated block to an operation. If a justification is provided
 	/// then `finalized` *must* be true.
 	fn apply_block(
@@ -1421,6 +1394,32 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	}
 }
 
+impl<B, E, Block, RA> BlockBuilderProvider<B, Block, Self> for Client<B, E, Block, RA>
+	where
+		B: backend::Backend<Block> + Send + Sync + 'static,
+		E: CallExecutor<Block> + Send + Sync + 'static,
+		Block: BlockT,
+		Self: ChainHeaderBackend<Block> + ProvideRuntimeApi<Block>,
+		<Self as ProvideRuntimeApi<Block>>::Api: ApiExt<Block, StateBackend = backend::StateBackendFor<B, Block>>
+			+ BlockBuilderApi<Block, Error = Error>,
+{
+	fn new_block_at<R: Into<RecordProof>>(
+		&self,
+		parent: &BlockId<Block>,
+		inherent_digests: DigestFor<Block>,
+		record_proof: R,
+	) -> sp_blockchain::Result<sc_block_builder::BlockBuilder<Block, Self, B>> {
+		sc_block_builder::BlockBuilder::new(
+			self,
+			self.expect_block_hash_from_id(parent)?,
+			self.expect_block_number_from_id(parent)?,
+			record_proof.into(),
+			inherent_digests,
+			&self.backend
+		)
+	}
+}
+
 impl<B, E, Block, RA> HeaderMetadata<Block> for Client<B, E, Block, RA> where
 	B: backend::Backend<Block>,
 	E: CallExecutor<Block>,

substrate/primitives/api/test/Cargo.toml

@@ -15,6 +15,7 @@ sp-version = { version = "2.0.0-alpha.2", path = "../../version" }
 sp-runtime = { version = "2.0.0-alpha.2", path = "../../runtime" }
 sp-blockchain = { version = "2.0.0-alpha.2", path = "../../blockchain" }
 sp-consensus = { version = "0.8.0-alpha.2", path = "../../../primitives/consensus/common" }
+sc-block-builder = { version = "0.8.0-alpha.2", path = "../../../client/block-builder" }
 codec = { package = "parity-scale-codec", version = "1.0.0" }
 sp-state-machine = { version = "0.8.0-alpha.2", path = "../../../primitives/state-machine" }
 trybuild = "1.0.17"

substrate/primitives/api/test/tests/runtime_calls.rs

@@ -28,6 +28,7 @@ use sp_state_machine::{
 
 use sp_consensus::SelectChain;
 use codec::Encode;
+use sc_block_builder::BlockBuilderProvider;
 
 fn calling_function_with_strat(strat: ExecutionStrategy) {
 	let client = TestClientBuilder::new().set_execution_strategy(strat).build();

substrate/test-utils/runtime/client/src/trait_tests.rs

@@ -30,6 +30,7 @@ use substrate_test_client::sp_consensus::BlockOrigin;
 use substrate_test_runtime::{self, Transfer};
 use sp_runtime::generic::BlockId;
 use sp_runtime::traits::{Block as BlockT, HasherFor};
+use sc_block_builder::BlockBuilderProvider;
 
 /// helper to test the `leaves` implementation for various backends
 pub fn test_leaves_for_backend<B: 'static>(backend: Arc<B>) where