Do not run unneeded subsystems on collator and its alongside node (#3061

)

Currently, collators and their alongside nodes spin up a full-scale
overseer running a bunch of subsystems that are not needed if the node
is not a validator. That was considered to be harmless; however, we've
got problems with unused subsystems getting stalled for a reason not
currently known, resulting in the overseer exiting and bringing down the
whole node.

This PR aims to only run needed subsystems on such nodes, replacing the
rest with `DummySubsystem`.

It also enables collator-optimized availability recovery subsystem
implementation.

Partially solves #1730.

cumulus/client/relay-chain-inprocess-interface/src/lib.rs

@@ -295,7 +295,7 @@ fn build_polkadot_full_node(
 			workers_path: None,
 			workers_names: None,
 
-			overseer_gen: polkadot_service::RealOverseerGen,
+			overseer_gen: polkadot_service::CollatorOverseerGen,
 			overseer_message_channel_capacity_override: None,
 			malus_finality_delay: None,
 			hwbench,

cumulus/test/service/src/lib.rs

@@ -271,6 +271,7 @@ async fn build_relay_chain_interface(
 				polkadot_service::IsParachainNode::Collator(CollatorPair::generate().0)
 			},
 			None,
+			polkadot_service::CollatorOverseerGen,
 		)
 		.map_err(|e| RelayChainError::Application(Box::new(e) as Box<_>))?,
 		cumulus_client_cli::RelayChainMode::ExternalRpc(rpc_target_urls) =>

polkadot/cli/src/command.rs

@@ -299,7 +299,7 @@ pub fn run() -> Result<()> {
 	match &cli.subcommand {
 		None => run_node_inner(
 			cli,
-			service::RealOverseerGen,
+			service::ValidatorOverseerGen,
 			None,
 			polkadot_node_metrics::logger_hook(),
 		),

polkadot/cli/src/lib.rs

@@ -29,7 +29,7 @@ mod error;
 pub use service::{self, Block, CoreApi, IdentifyVariant, ProvideRuntimeApi, TFullClient};
 
 #[cfg(feature = "malus")]
-pub use service::overseer::prepared_overseer_builder;
+pub use service::overseer::validator_overseer_builder;
 
 #[cfg(feature = "cli")]
 pub use cli::*;

polkadot/node/core/pvf-checker/src/lib.rs

@@ -49,29 +49,24 @@ use self::{
 
 /// PVF pre-checking subsystem.
 pub struct PvfCheckerSubsystem {
-	enabled: bool,
 	keystore: KeystorePtr,
 	metrics: Metrics,
 }
 
 impl PvfCheckerSubsystem {
-	pub fn new(enabled: bool, keystore: KeystorePtr, metrics: Metrics) -> Self {
-		PvfCheckerSubsystem { enabled, keystore, metrics }
+	pub fn new(keystore: KeystorePtr, metrics: Metrics) -> Self {
+		PvfCheckerSubsystem { keystore, metrics }
 	}
 }
 
 #[overseer::subsystem(PvfChecker, error=SubsystemError, prefix = self::overseer)]
 impl<Context> PvfCheckerSubsystem {
 	fn start(self, ctx: Context) -> SpawnedSubsystem {
-		if self.enabled {
-			let future = run(ctx, self.keystore, self.metrics)
-				.map_err(|e| SubsystemError::with_origin("pvf-checker", e))
-				.boxed();
-
-			SpawnedSubsystem { name: "pvf-checker-subsystem", future }
-		} else {
-			polkadot_overseer::DummySubsystem.start(ctx)
-		}
+		let future = run(ctx, self.keystore, self.metrics)
+			.map_err(|e| SubsystemError::with_origin("pvf-checker", e))
+			.boxed();
+
+		SpawnedSubsystem { name: "pvf-checker-subsystem", future }
 	}
 }
 

polkadot/node/malus/src/variants/back_garbage_candidate.rs

@@ -19,13 +19,12 @@
 //! candidates.
 
 use polkadot_cli::{
-	prepared_overseer_builder,
 	service::{
-		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
-		ProvideRuntimeApi,
+		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, ExtendedOverseerGenArgs,
+		HeaderBackend, Overseer, OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle,
+		ParachainHost, ProvideRuntimeApi,
 	},
-	Cli,
+	validator_overseer_builder, Cli,
 };
 use polkadot_node_subsystem::SpawnGlue;
 use polkadot_node_subsystem_types::DefaultSubsystemClient;
@@ -63,6 +62,7 @@ impl OverseerGen for BackGarbageCandidates {
 		&self,
 		connector: OverseerConnector,
 		args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
+		ext_args: Option<ExtendedOverseerGenArgs>,
 	) -> Result<
 		(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
 		Error,
@@ -80,11 +80,14 @@ impl OverseerGen for BackGarbageCandidates {
 			SpawnGlue(spawner),
 		);
 
-		prepared_overseer_builder(args)?
-			.replace_candidate_validation(move |cv_subsystem| {
-				InterceptedSubsystem::new(cv_subsystem, validation_filter)
-			})
-			.build_with_connector(connector)
-			.map_err(|e| e.into())
+		validator_overseer_builder(
+			args,
+			ext_args.expect("Extended arguments required to build validator overseer are provided"),
+		)?
+		.replace_candidate_validation(move |cv_subsystem| {
+			InterceptedSubsystem::new(cv_subsystem, validation_filter)
+		})
+		.build_with_connector(connector)
+		.map_err(|e| e.into())
 	}
 }

polkadot/node/malus/src/variants/dispute_finalized_candidates.rs

@@ -33,13 +33,12 @@
 
 use futures::channel::oneshot;
 use polkadot_cli::{
-	prepared_overseer_builder,
 	service::{
-		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
-		ProvideRuntimeApi,
+		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, ExtendedOverseerGenArgs,
+		HeaderBackend, Overseer, OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle,
+		ParachainHost, ProvideRuntimeApi,
 	},
-	Cli,
+	validator_overseer_builder, Cli,
 };
 use polkadot_node_subsystem::{messages::ApprovalVotingMessage, SpawnGlue};
 use polkadot_node_subsystem_types::{DefaultSubsystemClient, OverseerSignal};
@@ -237,6 +236,7 @@ impl OverseerGen for DisputeFinalizedCandidates {
 		&self,
 		connector: OverseerConnector,
 		args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
+		ext_args: Option<ExtendedOverseerGenArgs>,
 	) -> Result<
 		(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
 		Error,
@@ -257,9 +257,12 @@ impl OverseerGen for DisputeFinalizedCandidates {
 			dispute_offset: self.dispute_offset,
 		};
 
-		prepared_overseer_builder(args)?
-			.replace_approval_voting(move |cb| InterceptedSubsystem::new(cb, ancestor_disputer))
-			.build_with_connector(connector)
-			.map_err(|e| e.into())
+		validator_overseer_builder(
+			args,
+			ext_args.expect("Extended arguments required to build validator overseer are provided"),
+		)?
+		.replace_approval_voting(move |cb| InterceptedSubsystem::new(cb, ancestor_disputer))
+		.build_with_connector(connector)
+		.map_err(|e| e.into())
 	}
 }

polkadot/node/malus/src/variants/dispute_valid_candidates.rs

@@ -23,13 +23,12 @@
 #![allow(missing_docs)]
 
 use polkadot_cli::{
-	prepared_overseer_builder,
 	service::{
-		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
-		ProvideRuntimeApi,
+		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, ExtendedOverseerGenArgs,
+		HeaderBackend, Overseer, OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle,
+		ParachainHost, ProvideRuntimeApi,
 	},
-	Cli,
+	validator_overseer_builder, Cli,
 };
 use polkadot_node_subsystem::SpawnGlue;
 use polkadot_node_subsystem_types::DefaultSubsystemClient;
@@ -80,6 +79,7 @@ impl OverseerGen for DisputeValidCandidates {
 		&self,
 		connector: OverseerConnector,
 		args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
+		ext_args: Option<ExtendedOverseerGenArgs>,
 	) -> Result<
 		(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
 		Error,
@@ -97,11 +97,14 @@ impl OverseerGen for DisputeValidCandidates {
 			SpawnGlue(spawner.clone()),
 		);
 
-		prepared_overseer_builder(args)?
-			.replace_candidate_validation(move |cv_subsystem| {
-				InterceptedSubsystem::new(cv_subsystem, validation_filter)
-			})
-			.build_with_connector(connector)
-			.map_err(|e| e.into())
+		validator_overseer_builder(
+			args,
+			ext_args.expect("Extended arguments required to build validator overseer are provided"),
+		)?
+		.replace_candidate_validation(move |cv_subsystem| {
+			InterceptedSubsystem::new(cv_subsystem, validation_filter)
+		})
+		.build_with_connector(connector)
+		.map_err(|e| e.into())
 	}
 }

polkadot/node/malus/src/variants/suggest_garbage_candidate.rs

@@ -23,13 +23,12 @@
 #![allow(missing_docs)]
 
 use polkadot_cli::{
-	prepared_overseer_builder,
 	service::{
-		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
-		ProvideRuntimeApi,
+		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, ExtendedOverseerGenArgs,
+		HeaderBackend, Overseer, OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle,
+		ParachainHost, ProvideRuntimeApi,
 	},
-	Cli,
+	validator_overseer_builder, Cli,
 };
 use polkadot_node_core_candidate_validation::find_validation_data;
 use polkadot_node_primitives::{AvailableData, BlockData, PoV};
@@ -266,6 +265,7 @@ impl OverseerGen for SuggestGarbageCandidates {
 		&self,
 		connector: OverseerConnector,
 		args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
+		ext_args: Option<ExtendedOverseerGenArgs>,
 	) -> Result<
 		(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
 		Error,
@@ -293,12 +293,13 @@ impl OverseerGen for SuggestGarbageCandidates {
 			SpawnGlue(args.spawner.clone()),
 		);
 
-		prepared_overseer_builder(args)?
-			.replace_candidate_backing(move |cb| InterceptedSubsystem::new(cb, note_candidate))
-			.replace_candidate_validation(move |cb| {
-				InterceptedSubsystem::new(cb, validation_filter)
-			})
-			.build_with_connector(connector)
-			.map_err(|e| e.into())
+		validator_overseer_builder(
+			args,
+			ext_args.expect("Extended arguments required to build validator overseer are provided"),
+		)?
+		.replace_candidate_backing(move |cb| InterceptedSubsystem::new(cb, note_candidate))
+		.replace_candidate_validation(move |cb| InterceptedSubsystem::new(cb, validation_filter))
+		.build_with_connector(connector)
+		.map_err(|e| e.into())
 	}
 }

polkadot/node/malus/src/variants/support_disabled.rs

@@ -18,13 +18,12 @@
 //! to always return an empty set of disabled validators.
 
 use polkadot_cli::{
-	prepared_overseer_builder,
 	service::{
-		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
-		ProvideRuntimeApi,
+		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, ExtendedOverseerGenArgs,
+		HeaderBackend, Overseer, OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle,
+		ParachainHost, ProvideRuntimeApi,
 	},
-	Cli,
+	validator_overseer_builder, Cli,
 };
 use polkadot_node_subsystem::SpawnGlue;
 use polkadot_node_subsystem_types::DefaultSubsystemClient;
@@ -50,6 +49,7 @@ impl OverseerGen for SupportDisabled {
 		&self,
 		connector: OverseerConnector,
 		args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
+		ext_args: Option<ExtendedOverseerGenArgs>,
 	) -> Result<
 		(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
 		Error,
@@ -59,12 +59,15 @@ impl OverseerGen for SupportDisabled {
 		RuntimeClient::Api: ParachainHost<Block> + BabeApi<Block> + AuthorityDiscoveryApi<Block>,
 		Spawner: 'static + SpawnNamed + Clone + Unpin,
 	{
-		prepared_overseer_builder(args)?
-			.replace_runtime_api(move |ra_subsystem| {
-				InterceptedSubsystem::new(ra_subsystem, IgnoreDisabled)
-			})
-			.build_with_connector(connector)
-			.map_err(|e| e.into())
+		validator_overseer_builder(
+			args,
+			ext_args.expect("Extended arguments required to build validator overseer are provided"),
+		)?
+		.replace_runtime_api(move |ra_subsystem| {
+			InterceptedSubsystem::new(ra_subsystem, IgnoreDisabled)
+		})
+		.build_with_connector(connector)
+		.map_err(|e| e.into())
 	}
 }