Service builder refactoring

bin/node-template/node/src/command.rs

@@ -67,7 +67,14 @@ pub fn run() -> sc_cli::Result<()> {
 	match &cli.subcommand {
 		Some(subcommand) => {
 			let runner = cli.create_runner(subcommand)?;
-			runner.run_subcommand(subcommand, |config| Ok(new_full_start!(config).0))
+			runner.run_subcommand(subcommand, |config| {
+				let (
+					client, _import_setup, _inherent_data_providers, _backend, _tasks_builder,
+					_keystore, import_queue, _select_chain, _transaction_pool, _background_tasks,
+				) = new_full_start!(config);
+
+				Ok((client, import_queue))
+			})
 		}
 		None => {
 			let runner = cli.create_runner(&cli.run)?;

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

@@ -5,7 +5,7 @@ use std::time::Duration;
 use sc_client::LongestChain;
 use sc_client_api::ExecutorProvider;
 use node_template_runtime::{self, opaque::Block, RuntimeApi};
-use sc_service::{error::{Error as ServiceError}, AbstractService, Configuration, ServiceBuilder};
+use sc_service::{error::{Error as ServiceError}, AbstractService, Configuration};
 use sp_inherents::InherentDataProviders;
 use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
@@ -26,45 +26,48 @@ native_executor_instance!(
 macro_rules! new_full_start {
 	($config:expr) => {{
 		use std::sync::Arc;
-		let mut import_setup = None;
 		let inherent_data_providers = sp_inherents::InherentDataProviders::new();
 
-		let builder = sc_service::ServiceBuilder::new_full::<
+		let (client, backend, keystore, tasks_builder) = sc_service::new_full_parts::<
 			node_template_runtime::opaque::Block, node_template_runtime::RuntimeApi, crate::service::Executor
-		>($config)?
-			.with_select_chain(|_config, backend| {
-				Ok(sc_client::LongestChain::new(backend.clone()))
-			})?
-			.with_transaction_pool(|config, client, _fetcher| {
-				let pool_api = sc_transaction_pool::FullChainApi::new(client.clone());
-				Ok(sc_transaction_pool::BasicPool::new(config, std::sync::Arc::new(pool_api)))
-			})?
-			.with_import_queue(|_config, client, mut select_chain, _transaction_pool| {
-				let select_chain = select_chain.take()
-					.ok_or_else(|| sc_service::Error::SelectChainRequired)?;
-
-				let (grandpa_block_import, grandpa_link) =
-					sc_finality_grandpa::block_import(client.clone(), &(client.clone() as Arc<_>), select_chain)?;
-
-				let aura_block_import = sc_consensus_aura::AuraBlockImport::<_, _, _, AuraPair>::new(
-					grandpa_block_import.clone(), client.clone(),
-				);
-
-				let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair>(
-					sc_consensus_aura::slot_duration(&*client)?,
-					aura_block_import,
-					Some(Box::new(grandpa_block_import.clone())),
-					None,
-					client,
-					inherent_data_providers.clone(),
-				)?;
-
-				import_setup = Some((grandpa_block_import, grandpa_link));
-
-				Ok(import_queue)
-			})?;
-
-		(builder, import_setup, inherent_data_providers)
+		>(&$config)?;
+		let client = Arc::new(client);
+		let select_chain = sc_client::LongestChain::new(backend.clone());
+		let pool_api = sc_transaction_pool::FullChainApi::new(Arc::clone(&client));
+		let (transaction_pool, background_task_one) = sc_transaction_pool::BasicPool::new($config.transaction_pool.clone(), std::sync::Arc::new(pool_api));
+		let transaction_pool = Arc::new(transaction_pool);
+		let mut background_tasks = Vec::new();
+
+		if let Some(bg_t) = background_task_one {
+			background_tasks.push(("txpool-background", bg_t));
+		}
+
+		let (import_queue, import_setup) = {
+			let (grandpa_block_import, grandpa_link) =
+				sc_finality_grandpa::block_import(Arc::clone(&client), &(Arc::clone(&client) as Arc<_>), select_chain.clone())?;
+
+			let aura_block_import = sc_consensus_aura::AuraBlockImport::<_, _, _, AuraPair>::new(
+				grandpa_block_import.clone(), Arc::clone(&client),
+			);
+
+			let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair>(
+				sc_consensus_aura::slot_duration(&*client)?,
+				aura_block_import,
+				Some(Box::new(grandpa_block_import.clone())),
+				None,
+				client.clone(),
+				inherent_data_providers.clone(),
+			)?;
+
+			let import_setup = Some((grandpa_block_import, grandpa_link));
+
+			(import_queue, import_setup)
+		};
+
+		(
+			client, import_setup, inherent_data_providers, backend, tasks_builder, keystore,
+			import_queue, select_chain, transaction_pool, background_tasks,
+		)
 	}}
 }
 
@@ -77,19 +80,33 @@ pub fn new_full(config: Configuration)
 	let name = config.network.node_name.clone();
 	let disable_grandpa = config.disable_grandpa;
 
-	let (builder, mut import_setup, inherent_data_providers) = new_full_start!(config);
+	let (
+		client, mut import_setup, inherent_data_providers, backend, tasks_builder, keystore,
+		import_queue, select_chain, transaction_pool, background_tasks,
+	) = new_full_start!(config);
 
 	let (block_import, grandpa_link) =
 		import_setup.take()
 			.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");
 
-	let service = builder
-		.with_finality_proof_provider(|client, backend| {
-			// GenesisAuthoritySetProvider is implemented for StorageAndProofProvider
-			let provider = client as Arc<dyn StorageAndProofProvider<_, _>>;
-			Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, provider)) as _)
-		})?
-		.build()?;
+	let provider = client.clone() as Arc<dyn StorageAndProofProvider<_, _>>;
+	let finality_proof_provider = Arc::new(GrandpaFinalityProofProvider::new(backend.clone(), provider));
+	let service = sc_service::build(
+		config,
+		client,
+		backend,
+		tasks_builder,
+		keystore,
+		None,
+		Some(select_chain),
+		import_queue,
+		None,
+		Some(finality_proof_provider),
+		transaction_pool,
+		(),
+		None,
+		background_tasks,
+	)?;
 
 	if role.is_authority() {
 		let proposer =
@@ -179,49 +196,57 @@ pub fn new_light(config: Configuration)
 {
 	let inherent_data_providers = InherentDataProviders::new();
 
-	ServiceBuilder::new_light::<Block, RuntimeApi, Executor>(config)?
-		.with_select_chain(|_config, backend| {
-			Ok(LongestChain::new(backend.clone()))
-		})?
-		.with_transaction_pool(|config, client, fetcher| {
-			let fetcher = fetcher
-				.ok_or_else(|| "Trying to start light transaction pool without active fetcher")?;
-
-			let pool_api = sc_transaction_pool::LightChainApi::new(client.clone(), fetcher.clone());
-			let pool = sc_transaction_pool::BasicPool::with_revalidation_type(
-				config, Arc::new(pool_api), sc_transaction_pool::RevalidationType::Light,
-			);
-			Ok(pool)
-		})?
-		.with_import_queue_and_fprb(|_config, client, backend, fetcher, _select_chain, _tx_pool| {
-			let fetch_checker = fetcher
-				.map(|fetcher| fetcher.checker().clone())
-				.ok_or_else(|| "Trying to start light import queue without active fetch checker")?;
-			let grandpa_block_import = sc_finality_grandpa::light_block_import(
-				client.clone(),
-				backend,
-				&(client.clone() as Arc<_>),
-				Arc::new(fetch_checker),
-			)?;
-			let finality_proof_import = grandpa_block_import.clone();
-			let finality_proof_request_builder =
-				finality_proof_import.create_finality_proof_request_builder();
-
-			let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair>(
-				sc_consensus_aura::slot_duration(&*client)?,
-				grandpa_block_import,
-				None,
-				Some(Box::new(finality_proof_import)),
-				client,
-				inherent_data_providers.clone(),
-			)?;
-
-			Ok((import_queue, finality_proof_request_builder))
-		})?
-		.with_finality_proof_provider(|client, backend| {
-			// GenesisAuthoritySetProvider is implemented for StorageAndProofProvider
-			let provider = client as Arc<dyn StorageAndProofProvider<_, _>>;
-			Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, provider)) as _)
-		})?
-		.build()
+	let ((client, backend, keystore, tasks_builder), fetcher, remote_blockchain) = sc_service::new_light_parts::<
+		Block, RuntimeApi, Executor
+	>(&config)?;
+	let client = Arc::new(client);
+	let select_chain = LongestChain::new(backend.clone());
+	let pool_api = sc_transaction_pool::LightChainApi::new(client.clone(), fetcher.clone());
+	let (transaction_pool, background_task_one) = sc_transaction_pool::BasicPool::with_revalidation_type(
+		config.transaction_pool.clone(), Arc::new(pool_api), sc_transaction_pool::RevalidationType::Light,
+	);
+	let transaction_pool = Arc::new(transaction_pool);
+	let mut background_tasks = Vec::new();
+
+	if let Some(bg_t) = background_task_one {
+		background_tasks.push(("txpool-background", bg_t));
+	}
+	let fetch_checker = fetcher.checker().clone();
+	let grandpa_block_import = sc_finality_grandpa::light_block_import(
+		client.clone(),
+		backend.clone(),
+		&(client.clone() as Arc<_>),
+		Arc::new(fetch_checker),
+	)?;
+	let finality_proof_import = grandpa_block_import.clone();
+	let finality_proof_request_builder =
+		finality_proof_import.create_finality_proof_request_builder();
+
+	let import_queue = sc_consensus_aura::import_queue::<_, _, _, AuraPair>(
+		sc_consensus_aura::slot_duration(&*client)?,
+		grandpa_block_import,
+		None,
+		Some(Box::new(finality_proof_import)),
+		client.clone(),
+		inherent_data_providers.clone(),
+	)?;
+	// GenesisAuthoritySetProvider is implemented for StorageAndProofProvider
+	let provider = client.clone() as Arc<dyn StorageAndProofProvider<_, _>>;
+	let finality_proof_provider = Arc::new(GrandpaFinalityProofProvider::new(backend.clone(), provider));
+	sc_service::build(
+		config,
+		client,
+		backend,
+		tasks_builder,
+		keystore,
+		None,
+		Some(select_chain),
+		import_queue,
+		Some(finality_proof_request_builder),
+		Some(finality_proof_provider),
+		transaction_pool,
+		(),
+		Some(remote_blockchain),
+		background_tasks,
+	)
 }

client/cli/src/commands/check_block_cmd.rs

@@ -18,12 +18,13 @@ use crate::error;
 use crate::params::ImportParams;
 use crate::params::SharedParams;
 use crate::CliConfiguration;
-use sc_service::{Configuration, ServiceBuilderCommand};
+//use sc_service::{Configuration, ServiceBuilderCommand};
 use sp_runtime::generic::BlockId;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use std::fmt::Debug;
 use std::str::FromStr;
 use structopt::StructOpt;
+use sc_service::check_block;
 
 /// The `check-block` command used to validate blocks.
 #[derive(Debug, StructOpt, Clone)]
@@ -49,17 +50,18 @@ pub struct CheckBlockCmd {
 
 impl CheckBlockCmd {
 	/// Run the check-block command
-	pub async fn run<B, BC, BB>(
+	pub async fn run<B, BA, CE, IQ, RA>(
 		&self,
-		config: Configuration,
-		builder: B,
+		client: std::sync::Arc<sc_service::Client<BA, CE, B, RA>>,
+		import_queue: IQ,
 	) -> error::Result<()>
 	where
-		B: FnOnce(Configuration) -> Result<BC, sc_service::error::Error>,
-		BC: ServiceBuilderCommand<Block = BB> + Unpin,
-		BB: sp_runtime::traits::Block + Debug,
-		<<<BB as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug,
-		<BB as BlockT>::Hash: std::str::FromStr,
+		B: BlockT,
+		BA: sc_client_api::backend::Backend<B> + 'static,
+		CE: sc_client_api::call_executor::CallExecutor<B> + Send + Sync + 'static,
+		IQ: sc_service::ImportQueue<B> + 'static,
+		RA: Send + Sync + 'static,
+		<B as sp_runtime::traits::Block>::Hash: std::str::FromStr,
 	{
 		let input = if self.input.starts_with("0x") {
 			&self.input[2..]
@@ -79,7 +81,7 @@ impl CheckBlockCmd {
 		};
 
 		let start = std::time::Instant::now();
-		builder(config)?.check_block(block_id).await?;
+		check_block(client, import_queue, block_id).await?;
 		println!("Completed in {} ms.", start.elapsed().as_millis());
 
 		Ok(())

client/cli/src/commands/export_blocks_cmd.rs

@@ -19,14 +19,15 @@ use crate::params::{BlockNumber, PruningParams, SharedParams};
 use crate::CliConfiguration;
 use log::info;
 use sc_service::{
-	config::DatabaseConfig, Configuration, ServiceBuilderCommand,
+	config::DatabaseConfig, //Configuration, ServiceBuilderCommand,
 };
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use std::fmt::Debug;
 use std::fs;
 use std::io;
 use std::path::PathBuf;
 use structopt::StructOpt;
+use sc_service::export_blocks;
 
 /// The `export-blocks` command used to export blocks.
 #[derive(Debug, StructOpt, Clone)]
@@ -62,21 +63,23 @@ pub struct ExportBlocksCmd {
 
 impl ExportBlocksCmd {
 	/// Run the export-blocks command
-	pub async fn run<B, BC, BB>(
+	pub async fn run<B, BA, CE, RA>(
 		&self,
-		config: Configuration,
-		builder: B,
+		client: std::sync::Arc<sc_service::Client<BA, CE, B, RA>>,
 	) -> error::Result<()>
 	where
-		B: FnOnce(Configuration) -> Result<BC, sc_service::error::Error>,
-		BC: ServiceBuilderCommand<Block = BB> + Unpin,
-		BB: sp_runtime::traits::Block + Debug,
-		<<<BB as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug,
-		<BB as BlockT>::Hash: std::str::FromStr,
+		B: BlockT,
+		BA: sc_client_api::backend::Backend<B> + 'static,
+		CE: sc_client_api::call_executor::CallExecutor<B> + Send + Sync + 'static,
+		RA: Send + Sync + 'static,
+		<<<B as sp_runtime::traits::Block>::Header as sp_runtime::traits::Header>::Number as std::str::FromStr>::Err: std::fmt::Debug,
 	{
+		// TODO: should probably not be here
+		/*
 		if let DatabaseConfig::Path { ref path, .. } = &config.database {
 			info!("DB path: {}", path.display());
 		}
+		*/
 
 		let from = self.from.as_ref().and_then(|f| f.parse().ok()).unwrap_or(1);
 		let to = self.to.as_ref().and_then(|t| t.parse().ok());
@@ -88,8 +91,7 @@ impl ExportBlocksCmd {
 			None => Box::new(io::stdout()),
 		};
 
-		builder(config)?
-			.export_blocks(file, from.into(), to, binary)
+		export_blocks(client, file, from.into(), to, binary)
 			.await
 			.map_err(Into::into)
 	}