Remove superflous parameter overseer_enable_anyways and make parachain node type more explicit (#7617)

* Remove superflous parameter `overseer_enable_anyways` We don't need this flag, as we don't need the overseer enabled when the node isn't a collator or validator. * Rename `IsCollator` to `IsParachainNode` `IsParachainNode` is more expressive and also encapsulates the state of the parachain node being a full node. Some functionality like the overseer needs to run always when the node runs alongside a parachain node. The parachain node needs the overseer to e.g. recover PoVs. Other things like candidate validation or pvf checking are only required for when the node is running as validator. * FMT * Fix CI
2026-06-12 18:11:10 +00:00 · 2023-08-15 10:51:27 +02:00
parent e9c6c72f5a
commit cf3271fea2
16 changed files with 148 additions and 130 deletions
@@ -627,7 +627,7 @@ where

 #[cfg(feature = "full-node")]
 pub struct NewFullParams<OverseerGenerator: OverseerGen> {
-	pub is_collator: IsCollator,
+	pub is_parachain_node: IsParachainNode,
 	pub grandpa_pause: Option<(u32, u32)>,
 	pub jaeger_agent: Option<std::net::SocketAddr>,
 	pub telemetry_worker_handle: Option<TelemetryWorkerHandle>,
@@ -638,7 +638,6 @@ pub struct NewFullParams<OverseerGenerator: OverseerGen> {
 	pub workers_path: Option<std::path::PathBuf>,
 	/// Optional custom names for the prepare and execute workers.
 	pub workers_names: Option<(String, String)>,
-	pub overseer_enable_anyways: bool,
 	pub overseer_gen: OverseerGenerator,
 	pub overseer_message_channel_capacity_override: Option<usize>,
 	#[allow(dead_code)]
@@ -657,32 +656,46 @@ pub struct NewFull {
 	pub backend: Arc<FullBackend>,
 }

-/// Is this node a collator?
+/// Is this node running as in-process node for a parachain node?
 #[cfg(feature = "full-node")]
 #[derive(Clone)]
-pub enum IsCollator {
-	/// This node is a collator.
-	Yes(CollatorPair),
-	/// This node is not a collator.
+pub enum IsParachainNode {
+	/// This node is running as in-process node for a parachain collator.
+	Collator(CollatorPair),
+	/// This node is running as in-process node for a parachain full node.
+	FullNode,
+	/// This node is not running as in-process node for a parachain node, aka a normal relay chain
+	/// node.
 	No,
 }

 #[cfg(feature = "full-node")]
-impl std::fmt::Debug for IsCollator {
+impl std::fmt::Debug for IsParachainNode {
 	fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
 		use sp_core::Pair;
 		match self {
-			IsCollator::Yes(pair) => write!(fmt, "Yes({})", pair.public()),
-			IsCollator::No => write!(fmt, "No"),
+			IsParachainNode::Collator(pair) => write!(fmt, "Collator({})", pair.public()),
+			IsParachainNode::FullNode => write!(fmt, "FullNode"),
+			IsParachainNode::No => write!(fmt, "No"),
 		}
 	}
 }

 #[cfg(feature = "full-node")]
-impl IsCollator {
-	/// Is this a collator?
+impl IsParachainNode {
+	/// Is this running alongside a collator?
 	fn is_collator(&self) -> bool {
-		matches!(self, Self::Yes(_))
+		matches!(self, Self::Collator(_))
+	}
+
+	/// Is this running alongside a full node?
+	fn is_full_node(&self) -> bool {
+		matches!(self, Self::FullNode)
+	}
+
+	/// Is this node running alongside a relay chain node?
+	fn is_running_alongside_parachain_node(&self) -> bool {
+		self.is_collator() || self.is_full_node()
 	}
 }

@@ -696,11 +709,6 @@ pub const AVAILABILITY_CONFIG: AvailabilityConfig = AvailabilityConfig {
 /// This is an advanced feature and not recommended for general use. Generally, `build_full` is
 /// a better choice.
 ///
-/// `overseer_enable_anyways` always enables the overseer, based on the provided
-/// `OverseerGenerator`, regardless of the role the node has. The relay chain selection (longest or
-/// disputes-aware) is still determined based on the role of the node. Likewise for authority
-/// discovery.
-///
 /// `workers_path` is used to get the path to the directory where auxiliary worker binaries reside.
 /// If not specified, the main binary's directory is searched first, then `/usr/lib/polkadot` is
 /// searched. If the path points to an executable rather then directory, that executable is used
@@ -709,14 +717,13 @@ pub const AVAILABILITY_CONFIG: AvailabilityConfig = AvailabilityConfig {
 pub fn new_full<OverseerGenerator: OverseerGen>(
 	mut config: Configuration,
 	NewFullParams {
-		is_collator,
+		is_parachain_node,
 		grandpa_pause,
 		jaeger_agent,
 		telemetry_worker_handle,
 		node_version,
 		workers_path,
 		workers_names,
-		overseer_enable_anyways,
 		overseer_gen,
 		overseer_message_channel_capacity_override,
 		malus_finality_delay: _malus_finality_delay,
@@ -768,8 +775,9 @@ pub fn new_full<OverseerGenerator: OverseerGen>(
 	let chain_spec = config.chain_spec.cloned_box();

 	let keystore = basics.keystore_container.local_keystore();
-	let auth_or_collator = role.is_authority() || is_collator.is_collator();
-	let pvf_checker_enabled = role.is_authority() && !is_collator.is_collator();
+	let auth_or_collator = role.is_authority() || is_parachain_node.is_collator();
+	// We only need to enable the pvf checker when this is a validator.
+	let pvf_checker_enabled = role.is_authority();

 	let select_chain = if auth_or_collator {
 		let metrics =
@@ -832,7 +840,12 @@ pub fn new_full<OverseerGenerator: OverseerGen>(
 	let peerset_protocol_names =
 		PeerSetProtocolNames::new(genesis_hash, config.chain_spec.fork_id());

-	if auth_or_collator || overseer_enable_anyways {
+	// If this is a validator or running alongside a parachain node, we need to enable the
+	// networking protocols.
+	//
+	// Collators and parachain full nodes require the collator and validator networking to send
+	// collations and to be able to recover PoVs.
+	if role.is_authority() || is_parachain_node.is_running_alongside_parachain_node() {
 		use polkadot_network_bridge::{peer_sets_info, IsAuthority};
 		let is_authority = if role.is_authority() { IsAuthority::Yes } else { IsAuthority::No };
 		for config in peer_sets_info(is_authority, &peerset_protocol_names) {
@@ -910,7 +923,7 @@ pub fn new_full<OverseerGenerator: OverseerGen>(
 		slot_duration_millis: slot_duration.as_millis() as u64,
 	};

-	let candidate_validation_config = if role.is_authority() && !is_collator.is_collator() {
+	let candidate_validation_config = if role.is_authority() {
 		let (prep_worker_path, exec_worker_path) =
 			workers::determine_workers_paths(workers_path, workers_names, node_version.clone())?;
 		log::info!("🚀 Using prepare-worker binary at: {:?}", prep_worker_path);
@@ -979,46 +992,50 @@ pub fn new_full<OverseerGenerator: OverseerGen>(
 	let overseer_client = client.clone();
 	let spawner = task_manager.spawn_handle();

-	let authority_discovery_service = if auth_or_collator || overseer_enable_anyways {
-		use futures::StreamExt;
-		use sc_network::{Event, NetworkEventStream};
+	let authority_discovery_service =
+		// We need the authority discovery if this node is either a validator or running alongside a parachain node.
+		// Parachains node require the authority discovery for finding relay chain validators for sending
+		// their PoVs or recovering PoVs.
+		if role.is_authority() || is_parachain_node.is_running_alongside_parachain_node() {
+			use futures::StreamExt;
+			use sc_network::{Event, NetworkEventStream};

-		let authority_discovery_role = if role.is_authority() {
-			sc_authority_discovery::Role::PublishAndDiscover(keystore_container.keystore())
+			let authority_discovery_role = if role.is_authority() {
+				sc_authority_discovery::Role::PublishAndDiscover(keystore_container.keystore())
+			} else {
+				// don't publish our addresses when we're not an authority (collator, cumulus, ..)
+				sc_authority_discovery::Role::Discover
+			};
+			let dht_event_stream =
+				network.event_stream("authority-discovery").filter_map(|e| async move {
+					match e {
+						Event::Dht(e) => Some(e),
+						_ => None,
+					}
+				});
+			let (worker, service) = sc_authority_discovery::new_worker_and_service_with_config(
+				sc_authority_discovery::WorkerConfig {
+					publish_non_global_ips: auth_disc_publish_non_global_ips,
+					// Require that authority discovery records are signed.
+					strict_record_validation: true,
+					..Default::default()
+				},
+				client.clone(),
+				network.clone(),
+				Box::pin(dht_event_stream),
+				authority_discovery_role,
+				prometheus_registry.clone(),
+			);
+
+			task_manager.spawn_handle().spawn(
+				"authority-discovery-worker",
+				Some("authority-discovery"),
+				Box::pin(worker.run()),
+			);
+			Some(service)
 		} else {
-			// don't publish our addresses when we're not an authority (collator, cumulus, ..)
-			sc_authority_discovery::Role::Discover
+			None
 		};
-		let dht_event_stream =
-			network.event_stream("authority-discovery").filter_map(|e| async move {
-				match e {
-					Event::Dht(e) => Some(e),
-					_ => None,
-				}
-			});
-		let (worker, service) = sc_authority_discovery::new_worker_and_service_with_config(
-			sc_authority_discovery::WorkerConfig {
-				publish_non_global_ips: auth_disc_publish_non_global_ips,
-				// Require that authority discovery records are signed.
-				strict_record_validation: true,
-				..Default::default()
-			},
-			client.clone(),
-			network.clone(),
-			Box::pin(dht_event_stream),
-			authority_discovery_role,
-			prometheus_registry.clone(),
-		);
-
-		task_manager.spawn_handle().spawn(
-			"authority-discovery-worker",
-			Some("authority-discovery"),
-			Box::pin(worker.run()),
-		);
-		Some(service)
-	} else {
-		None
-	};

 	let overseer_handle = if let Some(authority_discovery_service) = authority_discovery_service {
 		let (overseer, overseer_handle) = overseer_gen
@@ -1039,7 +1056,7 @@ pub fn new_full<OverseerGenerator: OverseerGen>(
 					dispute_req_receiver,
 					registry: prometheus_registry.as_ref(),
 					spawner,
-					is_collator,
+					is_parachain_node,
 					approval_voting_config,
 					availability_config: AVAILABILITY_CONFIG,
 					candidate_validation_config,
@@ -1332,11 +1349,6 @@ pub fn new_chain_ops(
 ///
 /// The actual "flavor", aka if it will use `Polkadot`, `Rococo` or `Kusama` is determined based on
 /// [`IdentifyVariant`] using the chain spec.
-///
-/// `overseer_enable_anyways` always enables the overseer, based on the provided
-/// `OverseerGenerator`, regardless of the role the node has. The relay chain selection (longest or
-/// disputes-aware) is still determined based on the role of the node. Likewise for authority
-/// discovery.
 #[cfg(feature = "full-node")]
 pub fn build_full<OverseerGenerator: OverseerGen>(
 	config: Configuration,
@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.

-use super::{AuthorityDiscoveryApi, Block, Error, Hash, IsCollator, Registry};
+use super::{AuthorityDiscoveryApi, Block, Error, Hash, IsParachainNode, Registry};
 use polkadot_node_subsystem_types::DefaultSubsystemClient;
 use sc_transaction_pool_api::OffchainTransactionPoolFactory;
 use sp_core::traits::SpawnNamed;
@@ -108,7 +108,7 @@ where
 	/// Task spawner to be used throughout the overseer and the APIs it provides.
 	pub spawner: Spawner,
 	/// Determines the behavior of the collator.
-	pub is_collator: IsCollator,
+	pub is_parachain_node: IsParachainNode,
 	/// Configuration for the approval voting subsystem.
 	pub approval_voting_config: ApprovalVotingConfig,
 	/// Configuration for the availability store subsystem.
@@ -149,7 +149,7 @@ pub fn prepared_overseer_builder<Spawner, RuntimeClient>(
 		dispute_req_receiver,
 		registry,
 		spawner,
-		is_collator,
+		is_parachain_node,
 		approval_voting_config,
 		availability_config,
 		candidate_validation_config,
@@ -266,14 +266,15 @@ where
 		.chain_api(ChainApiSubsystem::new(runtime_client.clone(), Metrics::register(registry)?))
 		.collation_generation(CollationGenerationSubsystem::new(Metrics::register(registry)?))
 		.collator_protocol({
-			let side = match is_collator {
-				IsCollator::Yes(collator_pair) => ProtocolSide::Collator(
+			let side = match is_parachain_node {
+				IsParachainNode::Collator(collator_pair) => ProtocolSide::Collator(
 					network_service.local_peer_id(),
 					collator_pair,
 					collation_req_receiver,
 					Metrics::register(registry)?,
 				),
-				IsCollator::No => ProtocolSide::Validator {
+				IsParachainNode::FullNode => ProtocolSide::None,
+				IsParachainNode::No => ProtocolSide::Validator {
 					keystore: keystore.clone(),
 					eviction_policy: Default::default(),
 					metrics: Metrics::register(registry)?,