Remove the service, replacing it with a struct of individual chain components (#6352)

* WIP

* Making progress

* Almost ready

* Get service tests compiling

* Fix node screenshot

* Line widths

* Fix node cli tests

* Fix node cli warning

* ChainComponents -> ServiceComponents, fix tests

* make spawn_handle public

* Remove spawnnamed impl for taskmanager

* Move the keep alive stuff to the task manager

* Move the telemetry, base path, rpc keep_alive to the service builder

* Make the task manager keep alive an internal detail

* Rewrite the browser start_client future

* Remove run_node etc

* Revert my personal changes to browser-demo/build.sh

* use |config|

* Add a runtime_version function to SubstrateCli

* Reexport role and runtime version from sc cli

* Update Cargo.lock

* runtime_version -> native_runtime_version

* Pass chain spec to native_runtime_version for polkadot

* Fix line widths

* Traitify ServiceComponents Client

Author: expenses

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

@@ -18,7 +18,7 @@
 use crate::chain_spec;
 use crate::cli::Cli;
 use crate::service;
-use sc_cli::SubstrateCli;
+use sc_cli::{SubstrateCli, RuntimeVersion, Role, ChainSpec};
 
 impl SubstrateCli for Cli {
 	fn impl_name() -> &'static str {
@@ -58,6 +58,10 @@ impl SubstrateCli for Cli {
 			)?),
 		})
 	}
+
+	fn native_runtime_version(_: &Box<dyn ChainSpec>) -> &'static RuntimeVersion {
+		&node_template_runtime::VERSION
+	}
 }
 
 /// Parse and run command line arguments
@@ -71,11 +75,10 @@ pub fn run() -> sc_cli::Result<()> {
 		}
 		None => {
 			let runner = cli.create_runner(&cli.run)?;
-			runner.run_node(
-				service::new_light,
-				service::new_full,
-				node_template_runtime::VERSION
-			)
+			runner.run_node_until_exit(|config| match config.role {
+				Role::Light => service::new_light(config),
+				_ => service::new_full(config),
+			})
 		}
 	}
 }

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

@@ -5,7 +5,10 @@ use std::time::Duration;
 use sc_client_api::ExecutorProvider;
 use sc_consensus::LongestChain;
 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}, Configuration, ServiceBuilder, ServiceComponents,
+	TaskManager,
+};
 use sp_inherents::InherentDataProviders;
 use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
@@ -93,7 +96,7 @@ macro_rules! new_full_start {
 }
 
 /// Builds a new service for a full client.
-pub fn new_full(config: Configuration) -> Result<impl AbstractService, ServiceError> {
+pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
 	let role = config.role.clone();
 	let force_authoring = config.force_authoring;
 	let name = config.network.node_name.clone();
@@ -105,7 +108,10 @@ pub fn new_full(config: Configuration) -> Result<impl AbstractService, ServiceEr
 		import_setup.take()
 			.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");
 
-	let service = builder
+	let ServiceComponents {
+		client, transaction_pool, task_manager, keystore, network, select_chain,
+		prometheus_registry, telemetry_on_connect_sinks, ..
+	 } = builder
 		.with_finality_proof_provider(|client, backend| {
 			// GenesisAuthoritySetProvider is implemented for StorageAndProofProvider
 			let provider = client as Arc<dyn StorageAndProofProvider<_, _>>;
@@ -115,40 +121,39 @@ pub fn new_full(config: Configuration) -> Result<impl AbstractService, ServiceEr
 
 	if role.is_authority() {
 		let proposer = sc_basic_authorship::ProposerFactory::new(
-			service.client(),
-			service.transaction_pool(),
-			service.prometheus_registry().as_ref(),
+			client.clone(),
+			transaction_pool,
+			prometheus_registry.as_ref(),
 		);
 
-		let client = service.client();
-		let select_chain = service.select_chain()
+		let select_chain = select_chain
 			.ok_or(ServiceError::SelectChainRequired)?;
 
 		let can_author_with =
 			sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());
 
 		let aura = sc_consensus_aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _>(
 			sc_consensus_aura::slot_duration(&*client)?,
-			client,
+			client.clone(),
 			select_chain,
 			block_import,
 			proposer,
-			service.network(),
+			network.clone(),
 			inherent_data_providers.clone(),
 			force_authoring,
-			service.keystore(),
+			keystore.clone(),
 			can_author_with,
 		)?;
 
 		// the AURA authoring task is considered essential, i.e. if it
 		// fails we take down the service with it.
-		service.spawn_essential_task_handle().spawn_blocking("aura", aura);
+		task_manager.spawn_essential_handle().spawn_blocking("aura", aura);
 	}
 
 	// if the node isn't actively participating in consensus then it doesn't
 	// need a keystore, regardless of which protocol we use below.
 	let keystore = if role.is_authority() {
-		Some(service.keystore() as sp_core::traits::BareCryptoStorePtr)
+		Some(keystore.clone() as sp_core::traits::BareCryptoStorePtr)
 	} else {
 		None
 	};
@@ -174,33 +179,33 @@ pub fn new_full(config: Configuration) -> Result<impl AbstractService, ServiceEr
 		let grandpa_config = sc_finality_grandpa::GrandpaParams {
 			config: grandpa_config,
 			link: grandpa_link,
-			network: service.network(),
+			network: network.clone(),
 			inherent_data_providers: inherent_data_providers.clone(),
-			telemetry_on_connect: Some(service.telemetry_on_connect_stream()),
+			telemetry_on_connect: Some(telemetry_on_connect_sinks.on_connect_stream()),
 			voting_rule: sc_finality_grandpa::VotingRulesBuilder::default().build(),
-			prometheus_registry: service.prometheus_registry(),
+			prometheus_registry: prometheus_registry.clone(),
 			shared_voter_state: SharedVoterState::empty(),
 		};
 
 		// the GRANDPA voter task is considered infallible, i.e.
 		// if it fails we take down the service with it.
-		service.spawn_essential_task_handle().spawn_blocking(
+		task_manager.spawn_essential_handle().spawn_blocking(
 			"grandpa-voter",
 			sc_finality_grandpa::run_grandpa_voter(grandpa_config)?
 		);
 	} else {
 		sc_finality_grandpa::setup_disabled_grandpa(
-			service.client(),
+			client,
 			&inherent_data_providers,
-			service.network(),
+			network.clone(),
 		)?;
 	}
 
-	Ok(service)
+	Ok(task_manager)
 }
 
 /// Builds a new service for a light client.
-pub fn new_light(config: Configuration) -> Result<impl AbstractService, ServiceError> {
+pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
 	let inherent_data_providers = InherentDataProviders::new();
 
 	ServiceBuilder::new_light::<Block, RuntimeApi, Executor>(config)?
@@ -265,4 +270,5 @@ pub fn new_light(config: Configuration) -> Result<impl AbstractService, ServiceE
 			Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, provider)) as _)
 		})?
 		.build_light()
+		.map(|ServiceComponents { task_manager, .. }| task_manager)
 }

bin/node/cli/src/browser.rs

@@ -52,8 +52,10 @@ async fn start_inner(chain_spec: Option<String>, log_level: String) -> Result<Cl
 	info!("👤 Role: {:?}", config.role);
 
 	// Create the service. This is the most heavy initialization step.
-	let service = crate::service::new_light(config)
-		.map_err(|e| format!("{:?}", e))?;
+	let (task_manager, rpc_handlers) =
+		crate::service::new_light_base(config)
+			.map(|(components, rpc_handlers, _, _, _)| (components, rpc_handlers))
+			.map_err(|e| format!("{:?}", e))?;
 
-	Ok(browser_utils::start_client(service))
+	Ok(browser_utils::start_client(task_manager, rpc_handlers))
 }

bin/node/cli/src/chain_spec.rs

@@ -380,7 +380,7 @@ pub fn local_testnet_config() -> ChainSpec {
 #[cfg(test)]
 pub(crate) mod tests {
 	use super::*;
-	use crate::service::{new_full, new_light};
+	use crate::service::{new_full_base, new_light_base};
 	use sc_service_test;
 	use sp_runtime::BuildStorage;
 
@@ -430,8 +430,14 @@ pub(crate) mod tests {
 	fn test_connectivity() {
 		sc_service_test::connectivity(
 			integration_test_config_with_two_authorities(),
-			|config| new_full(config),
-			|config| new_light(config),
+			|config| {
+				let (keep_alive, _, client, network, transaction_pool) = new_full_base(config,|_, _| ())?;
+				Ok(sc_service_test::TestNetComponents::new(keep_alive, client, network, transaction_pool))
+			},
+			|config| {
+				let (keep_alive, _, client, network, transaction_pool) = new_light_base(config)?;
+				Ok(sc_service_test::TestNetComponents::new(keep_alive, client, network, transaction_pool))
+			}
 		);
 	}
 

bin/node/cli/src/command.rs

@@ -19,7 +19,7 @@
 use crate::{chain_spec, service, Cli, Subcommand};
 use node_executor::Executor;
 use node_runtime::{Block, RuntimeApi};
-use sc_cli::{Result, SubstrateCli};
+use sc_cli::{Result, SubstrateCli, RuntimeVersion, Role, ChainSpec};
 
 impl SubstrateCli for Cli {
 	fn impl_name() -> &'static str {
@@ -61,6 +61,10 @@ impl SubstrateCli for Cli {
 			)?),
 		})
 	}
+
+	fn native_runtime_version(_: &Box<dyn ChainSpec>) -> &'static RuntimeVersion {
+		&node_runtime::VERSION
+	}
 }
 
 /// Parse command line arguments into service configuration.
@@ -70,11 +74,10 @@ pub fn run() -> Result<()> {
 	match &cli.subcommand {
 		None => {
 			let runner = cli.create_runner(&cli.run)?;
-			runner.run_node(
-				service::new_light,
-				service::new_full,
-				node_runtime::VERSION
-			)
+			runner.run_node_until_exit(|config| match config.role {
+				Role::Light => service::new_light(config),
+				_ => service::new_full(config),
+			})
 		}
 		Some(Subcommand::Inspect(cmd)) => {
 			let runner = cli.create_runner(cmd)?;