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
2026-04-26 14:37:57 +00:00 · 2020-06-30 12:00:42 +02:00
parent 493d5d8591
commit b832e35c5e
19 changed files with 646 additions and 782 deletions
@@ -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))
 }
@@ -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))
+			}
 		);
 	}

@@ -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)?;
@@ -29,10 +29,16 @@ use node_executor;
 use node_primitives::Block;
 use node_runtime::RuntimeApi;
 use sc_service::{
-	AbstractService, ServiceBuilder, config::Configuration, error::{Error as ServiceError},
+	ServiceBuilder, config::{Role, Configuration}, error::{Error as ServiceError},
+	RpcHandlers, ServiceComponents, TaskManager,
 };
 use sp_inherents::InherentDataProviders;
 use sc_consensus::LongestChain;
+use sc_network::{Event, NetworkService};
+use sp_runtime::traits::Block as BlockT;
+use futures::prelude::*;
+use sc_client_api::ExecutorProvider;
+use sp_core::traits::BareCryptoStorePtr;

 /// Starts a `ServiceBuilder` for a full service.
 ///
@@ -147,183 +153,197 @@ macro_rules! new_full_start {
 	}}
 }

-/// Creates a full service from the configuration.
-///
-/// We need to use a macro because the test suit doesn't work with an opaque service. It expects
-/// concrete types instead.
-macro_rules! new_full {
-	($config:expr, $with_startup_data: expr) => {{
-		use futures::prelude::*;
-		use sc_network::Event;
-		use sc_client_api::ExecutorProvider;
-		use sp_core::traits::BareCryptoStorePtr;
+type FullClient = sc_service::TFullClient<Block, RuntimeApi, node_executor::Executor>;
+type FullBackend = sc_service::TFullBackend<Block>;
+type GrandpaBlockImport = grandpa::GrandpaBlockImport<
+	FullBackend, Block, FullClient, sc_consensus::LongestChain<FullBackend, Block>
+>;
+type BabeBlockImport = sc_consensus_babe::BabeBlockImport<Block, FullClient, GrandpaBlockImport>;

-		let (
-			role,
-			force_authoring,
-			name,
-			disable_grandpa,
-		) = (
-			$config.role.clone(),
-			$config.force_authoring,
-			$config.network.node_name.clone(),
-			$config.disable_grandpa,
+/// Creates a full service from the configuration.
+pub fn new_full_base(
+	config: Configuration,
+	with_startup_data: impl FnOnce(&BabeBlockImport, &sc_consensus_babe::BabeLink<Block>)
+) -> Result<(
+	TaskManager,
+	InherentDataProviders,
+	Arc<FullClient>, Arc<NetworkService<Block, <Block as BlockT>::Hash>>,
+	Arc<sc_transaction_pool::BasicPool<sc_transaction_pool::FullChainApi<FullClient, Block>, Block>>
+), ServiceError> {
+	let (
+		role,
+		force_authoring,
+		name,
+		disable_grandpa,
+	) = (
+		config.role.clone(),
+		config.force_authoring,
+		config.network.node_name.clone(),
+		config.disable_grandpa,
+	);
+
+	let (builder, mut import_setup, inherent_data_providers, mut rpc_setup) =
+		new_full_start!(config);
+
+	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 grandpa::StorageAndProofProvider<_, _>>;
+			Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, provider)) as _)
+		})?
+		.build_full()?;
+
+	let (block_import, grandpa_link, babe_link) = import_setup.take()
+		.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");
+
+	let shared_voter_state = rpc_setup.take()
+		.expect("The SharedVoterState is present for Full Services or setup failed before. qed");
+
+	(with_startup_data)(&block_import, &babe_link);
+
+	if let sc_service::config::Role::Authority { .. } = &role {
+		let proposer = sc_basic_authorship::ProposerFactory::new(
+			client.clone(),
+			transaction_pool.clone(),
+			prometheus_registry.as_ref(),
 		);

-		let (builder, mut import_setup, inherent_data_providers, mut rpc_setup) =
-			new_full_start!($config);
+		let select_chain = select_chain
+			.ok_or(sc_service::Error::SelectChainRequired)?;

-		let service = builder
-			.with_finality_proof_provider(|client, backend| {
-				// GenesisAuthoritySetProvider is implemented for StorageAndProofProvider
-				let provider = client as Arc<dyn grandpa::StorageAndProofProvider<_, _>>;
-				Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, provider)) as _)
-			})?
-			.build_full()?;
+		let can_author_with =
+			sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());

-		let (block_import, grandpa_link, babe_link) = import_setup.take()
-			.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");
+		let babe_config = sc_consensus_babe::BabeParams {
+			keystore: keystore.clone(),
+			client: client.clone(),
+			select_chain,
+			env: proposer,
+			block_import,
+			sync_oracle: network.clone(),
+			inherent_data_providers: inherent_data_providers.clone(),
+			force_authoring,
+			babe_link,
+			can_author_with,
+		};

-		let shared_voter_state = rpc_setup.take()
-			.expect("The SharedVoterState is present for Full Services or setup failed before. qed");
+		let babe = sc_consensus_babe::start_babe(babe_config)?;
+		task_manager.spawn_essential_handle().spawn_blocking("babe-proposer", babe);
+	}

-		($with_startup_data)(&block_import, &babe_link);
-
-		if let sc_service::config::Role::Authority { .. } = &role {
-			let proposer = sc_basic_authorship::ProposerFactory::new(
-				service.client(),
-				service.transaction_pool(),
-				service.prometheus_registry().as_ref(),
-			);
-
-			let client = service.client();
-			let select_chain = service.select_chain()
-				.ok_or(sc_service::Error::SelectChainRequired)?;
-
-			let can_author_with =
-				sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());
-
-			let babe_config = sc_consensus_babe::BabeParams {
-				keystore: service.keystore(),
-				client,
-				select_chain,
-				env: proposer,
-				block_import,
-				sync_oracle: service.network(),
-				inherent_data_providers: inherent_data_providers.clone(),
-				force_authoring,
-				babe_link,
-				can_author_with,
-			};
-
-			let babe = sc_consensus_babe::start_babe(babe_config)?;
-			service.spawn_essential_task_handle().spawn_blocking("babe-proposer", babe);
-		}
-
-		// Spawn authority discovery module.
-		if matches!(role, sc_service::config::Role::Authority{..} | sc_service::config::Role::Sentry {..}) {
-			let (sentries, authority_discovery_role) = match role {
-				sc_service::config::Role::Authority { ref sentry_nodes } => (
-					sentry_nodes.clone(),
-					sc_authority_discovery::Role::Authority (
-						service.keystore(),
-					),
+	// Spawn authority discovery module.
+	if matches!(role, Role::Authority{..} | Role::Sentry {..}) {
+		let (sentries, authority_discovery_role) = match role {
+			sc_service::config::Role::Authority { ref sentry_nodes } => (
+				sentry_nodes.clone(),
+				sc_authority_discovery::Role::Authority (
+					keystore.clone(),
 				),
-				sc_service::config::Role::Sentry {..} => (
-					vec![],
-					sc_authority_discovery::Role::Sentry,
-				),
-				_ => unreachable!("Due to outer matches! constraint; qed.")
-			};
+			),
+			sc_service::config::Role::Sentry {..} => (
+				vec![],
+				sc_authority_discovery::Role::Sentry,
+			),
+			_ => unreachable!("Due to outer matches! constraint; qed.")
+		};

-			let network = service.network();
-			let dht_event_stream = network.event_stream("authority-discovery").filter_map(|e| async move { match e {
+		let dht_event_stream = network.event_stream("authority-discovery")
+			.filter_map(|e| async move { match e {
 				Event::Dht(e) => Some(e),
 				_ => None,
 			}}).boxed();
-			let authority_discovery = sc_authority_discovery::AuthorityDiscovery::new(
-				service.client(),
-				network,
-				sentries,
-				dht_event_stream,
-				authority_discovery_role,
-				service.prometheus_registry(),
-			);
+		let authority_discovery = sc_authority_discovery::AuthorityDiscovery::new(
+			client.clone(),
+			network.clone(),
+			sentries,
+			dht_event_stream,
+			authority_discovery_role,
+			prometheus_registry.clone(),
+		);

-			service.spawn_task_handle().spawn("authority-discovery", authority_discovery);
-		}
+		task_manager.spawn_handle().spawn("authority-discovery", authority_discovery);
+	}

-		// 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 BareCryptoStorePtr)
-		} else {
-			None
+	// 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(keystore.clone() as BareCryptoStorePtr)
+	} else {
+		None
+	};
+
+	let config = grandpa::Config {
+		// FIXME #1578 make this available through chainspec
+		gossip_duration: std::time::Duration::from_millis(333),
+		justification_period: 512,
+		name: Some(name),
+		observer_enabled: false,
+		keystore,
+		is_authority: role.is_network_authority(),
+	};
+
+	let enable_grandpa = !disable_grandpa;
+	if enable_grandpa {
+		// start the full GRANDPA voter
+		// NOTE: non-authorities could run the GRANDPA observer protocol, but at
+		// this point the full voter should provide better guarantees of block
+		// and vote data availability than the observer. The observer has not
+		// been tested extensively yet and having most nodes in a network run it
+		// could lead to finality stalls.
+		let grandpa_config = grandpa::GrandpaParams {
+			config,
+			link: grandpa_link,
+			network: network.clone(),
+			inherent_data_providers: inherent_data_providers.clone(),
+			telemetry_on_connect: Some(telemetry_on_connect_sinks.on_connect_stream()),
+			voting_rule: grandpa::VotingRulesBuilder::default().build(),
+			prometheus_registry: prometheus_registry.clone(),
+			shared_voter_state,
 		};

-		let config = grandpa::Config {
-			// FIXME #1578 make this available through chainspec
-			gossip_duration: std::time::Duration::from_millis(333),
-			justification_period: 512,
-			name: Some(name),
-			observer_enabled: false,
-			keystore,
-			is_authority: role.is_network_authority(),
-		};
+		// the GRANDPA voter task is considered infallible, i.e.
+		// if it fails we take down the service with it.
+		task_manager.spawn_essential_handle().spawn_blocking(
+			"grandpa-voter",
+			grandpa::run_grandpa_voter(grandpa_config)?
+		);
+	} else {
+		grandpa::setup_disabled_grandpa(
+			client.clone(),
+			&inherent_data_providers,
+			network.clone(),
+		)?;
+	}

-		let enable_grandpa = !disable_grandpa;
-		if enable_grandpa {
-			// start the full GRANDPA voter
-			// NOTE: non-authorities could run the GRANDPA observer protocol, but at
-			// this point the full voter should provide better guarantees of block
-			// and vote data availability than the observer. The observer has not
-			// been tested extensively yet and having most nodes in a network run it
-			// could lead to finality stalls.
-			let grandpa_config = grandpa::GrandpaParams {
-				config,
-				link: grandpa_link,
-				network: service.network(),
-				inherent_data_providers: inherent_data_providers.clone(),
-				telemetry_on_connect: Some(service.telemetry_on_connect_stream()),
-				voting_rule: grandpa::VotingRulesBuilder::default().build(),
-				prometheus_registry: service.prometheus_registry(),
-				shared_voter_state,
-			};
-
-			// 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(
-				"grandpa-voter",
-				grandpa::run_grandpa_voter(grandpa_config)?
-			);
-		} else {
-			grandpa::setup_disabled_grandpa(
-				service.client(),
-				&inherent_data_providers,
-				service.network(),
-			)?;
-		}
-
-		Ok((service, inherent_data_providers))
-	}};
-	($config:expr) => {{
-		new_full!($config, |_, _| {})
-	}}
+	Ok((task_manager, inherent_data_providers, client, network, transaction_pool))
 }

 /// Builds a new service for a full client.
 pub fn new_full(config: Configuration)
-> Result<impl AbstractService, ServiceError>
-{
-	new_full!(config).map(|(service, _)| service)
+-> Result<TaskManager, ServiceError> {
+	new_full_base(config, |_, _| ()).map(|(task_manager, _, _, _, _)| {
+		task_manager
+	})
 }

-/// Builds a new service for a light client.
-pub fn new_light(config: Configuration)
-> Result<impl AbstractService, ServiceError> {
+type LightClient = sc_service::TLightClient<Block, RuntimeApi, node_executor::Executor>;
+type LightFetcher = sc_network::config::OnDemand<Block>;
+
+pub fn new_light_base(config: Configuration) -> Result<(
+	TaskManager, Arc<RpcHandlers>, Arc<LightClient>,
+	Arc<NetworkService<Block, <Block as BlockT>::Hash>>,
+	Arc<sc_transaction_pool::BasicPool<
+		sc_transaction_pool::LightChainApi<LightClient, LightFetcher, Block>, Block
+	>>
+), ServiceError> {
 	let inherent_data_providers = InherentDataProviders::new();

-	let service = ServiceBuilder::new_light::<Block, RuntimeApi, node_executor::Executor>(config)?
+	let ServiceComponents {
+		task_manager, rpc_handlers, client, network, transaction_pool, ..
+	} = ServiceBuilder::new_light::<Block, RuntimeApi, node_executor::Executor>(config)?
 		.with_select_chain(|_config, backend| {
 			Ok(LongestChain::new(backend.clone()))
 		})?
@@ -406,16 +426,21 @@ pub fn new_light(config: Configuration)
 			Ok(node_rpc::create_light(light_deps))
 		})?
 		.build_light()?;
+	
+	Ok((task_manager, rpc_handlers, client, network, transaction_pool))
+}

-	Ok(service)
+/// Builds a new service for a light client.
+pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {	
+	new_light_base(config).map(|(task_manager, _, _, _, _)| {
+		task_manager
+	})
 }

 #[cfg(test)]
 mod tests {
 	use std::{sync::Arc, borrow::Cow, any::Any};
-	use sc_consensus_babe::{
-		CompatibleDigestItem, BabeIntermediate, INTERMEDIATE_KEY
-	};
+	use sc_consensus_babe::{CompatibleDigestItem, BabeIntermediate, INTERMEDIATE_KEY};
 	use sc_consensus_epochs::descendent_query;
 	use sp_consensus::{
 		Environment, Proposer, BlockImportParams, BlockOrigin, ForkChoiceStrategy, BlockImport,
@@ -434,10 +459,11 @@ mod tests {
 	use sp_timestamp;
 	use sp_finality_tracker;
 	use sp_keyring::AccountKeyring;
-	use sc_service::AbstractService;
-	use crate::service::{new_full, new_light};
+	use sc_service_test::TestNetNode;
+	use crate::service::{new_full_base, new_light_base};
 	use sp_runtime::traits::IdentifyAccount;
 	use sp_transaction_pool::{MaintainedTransactionPool, ChainEvent};
+	use sc_client_api::BlockBackend;

 	type AccountPublic = <Signature as Verify>::Signer;

@@ -466,14 +492,25 @@ mod tests {
 			chain_spec,
 			|config| {
 				let mut setup_handles = None;
-				new_full!(config, |
-					block_import: &sc_consensus_babe::BabeBlockImport<Block, _, _>,
-					babe_link: &sc_consensus_babe::BabeLink<Block>,
-				| {
-					setup_handles = Some((block_import.clone(), babe_link.clone()));
-				}).map(move |(node, x)| (node, (x, setup_handles.unwrap())))
+				let (keep_alive, inherent_data_providers, client, network, transaction_pool) =
+					new_full_base(config,
+						|
+							block_import: &sc_consensus_babe::BabeBlockImport<Block, _, _>,
+							babe_link: &sc_consensus_babe::BabeLink<Block>,
+						| {
+							setup_handles = Some((block_import.clone(), babe_link.clone()));
+						}
+					)?;
+				
+				let node = sc_service_test::TestNetComponents::new(
+					keep_alive, client, network, transaction_pool
+				);
+				Ok((node, (inherent_data_providers, setup_handles.unwrap())))
+			},
+			|config| {
+				let (keep_alive, _, client, network, transaction_pool) = new_light_base(config)?;
+				Ok(sc_service_test::TestNetComponents::new(keep_alive, client, network, transaction_pool))
 			},
-			|config| new_light(config),
 			|service, &mut (ref inherent_data_providers, (ref mut block_import, ref babe_link))| {
 				let mut inherent_data = inherent_data_providers
 					.create_inherent_data()
@@ -620,8 +657,14 @@ mod tests {
 	fn test_consensus() {
 		sc_service_test::consensus(
 			crate::chain_spec::tests::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))
+			},
 			vec![
 				"//Alice".into(),
 				"//Bob".into(),