Syncing strategy refactoring (part 3)

cumulus/client/service/src/lib.rs

@@ -41,7 +41,7 @@ use sc_network::{config::SyncMode, service::traits::NetworkService, NetworkBacke
 use sc_network_sync::{service::network::NetworkServiceProvider, SyncingService};
 use sc_network_transactions::TransactionsHandlerController;
 use sc_service::{
-	build_default_block_downloader, build_polkadot_syncing_strategy, Configuration, NetworkStarter,
+	build_default_syncing_engine, Configuration, DefaultSyncingEngineConfig, NetworkStarter,
 	SpawnTaskHandle, TaskManager, WarpSyncConfig,
 };
 use sc_telemetry::{log, TelemetryWorkerHandle};
@@ -499,28 +499,27 @@ where
 		parachain_config.prometheus_config.as_ref().map(|config| &config.registry),
 	);
 
-	let protocol_id = parachain_config.protocol_id();
 	let network_service_provider = NetworkServiceProvider::new();
-	let block_downloader = build_default_block_downloader(
-		&protocol_id,
-		parachain_config.chain_spec.fork_id(),
-		&mut net_config,
-		network_service_provider.handle(),
-		client.clone(),
-		parachain_config.network.default_peers_set.in_peers as usize +
-			parachain_config.network.default_peers_set.out_peers as usize,
-		&spawn_handle,
-	);
-	let syncing_strategy = build_polkadot_syncing_strategy(
-		protocol_id,
-		parachain_config.chain_spec.fork_id(),
-		&mut net_config,
-		warp_sync_config,
-		block_downloader,
-		client.clone(),
-		&spawn_handle,
-		parachain_config.prometheus_config.as_ref().map(|config| &config.registry),
-	)?;
+	let (sync_service, block_announce_config) =
+		build_default_syncing_engine(DefaultSyncingEngineConfig {
+			role: parachain_config.role,
+			protocol_id: parachain_config.protocol_id(),
+			fork_id: None,
+			net_config: &mut net_config,
+			block_announce_validator,
+			network_service_handle: network_service_provider.handle(),
+			warp_sync_config,
+			client: client.clone(),
+			import_queue_service: import_queue.service(),
+			num_peers_hint: parachain_config.network.default_peers_set.in_peers as usize +
+				parachain_config.network.default_peers_set.out_peers as usize,
+			spawn_handle: &spawn_handle,
+			metrics_registry: parachain_config
+				.prometheus_config
+				.as_ref()
+				.map(|config| &config.registry),
+			metrics: metrics.clone(),
+		})?;
 
 	sc_service::build_network(sc_service::BuildNetworkParams {
 		config: parachain_config,
@@ -529,8 +528,8 @@ where
 		transaction_pool,
 		spawn_handle,
 		import_queue,
-		block_announce_validator_builder: Some(Box::new(move |_| block_announce_validator)),
-		syncing_strategy,
+		sync_service,
+		block_announce_config,
 		network_service_provider,
 		metrics,
 	})

polkadot/node/service/src/lib.rs

@@ -84,7 +84,9 @@ use std::{collections::HashMap, path::PathBuf, sync::Arc, time::Duration};
 use prometheus_endpoint::Registry;
 #[cfg(feature = "full-node")]
 use sc_service::KeystoreContainer;
-use sc_service::{build_default_block_downloader, build_polkadot_syncing_strategy, RpcHandlers, SpawnTaskHandle};
+use sc_service::{
+	build_default_syncing_engine, DefaultSyncingEngineConfig, RpcHandlers, SpawnTaskHandle,
+};
 use sc_telemetry::TelemetryWorker;
 #[cfg(feature = "full-node")]
 use sc_telemetry::{Telemetry, TelemetryWorkerHandle};
@@ -118,6 +120,8 @@ pub use {rococo_runtime, rococo_runtime_constants};
 pub use {westend_runtime, westend_runtime_constants};
 
 pub use fake_runtime_api::{GetLastTimestamp, RuntimeApi};
+use sc_consensus::ImportQueue;
+use sp_consensus::block_validation::DefaultBlockAnnounceValidator;
 
 #[cfg(feature = "full-node")]
 pub type FullBackend = sc_service::TFullBackend<Block>;
@@ -1029,29 +1033,24 @@ pub fn new_full<
 		})
 	};
 
-	let protocol_id = config.protocol_id();
-	let spawn_handle = task_manager.spawn_handle();
 	let network_service_provider = NetworkServiceProvider::new();
-	let block_downloader = build_default_block_downloader(
-		&protocol_id,
-		config.chain_spec.fork_id(),
-		&mut net_config,
-		network_service_provider.handle(),
-		client.clone(),
-		config.network.default_peers_set.in_peers as usize +
-			config.network.default_peers_set.out_peers as usize,
-		&spawn_handle,
-	);
-	let syncing_strategy = build_polkadot_syncing_strategy(
-		protocol_id,
-		config.chain_spec.fork_id(),
-		&mut net_config,
-		Some(WarpSyncConfig::WithProvider(warp_sync)),
-		block_downloader,
-		client.clone(),
-		&spawn_handle,
-		config.prometheus_config.as_ref().map(|config| &config.registry),
-	)?;
+	let (sync_service, block_announce_config) =
+		build_default_syncing_engine(DefaultSyncingEngineConfig {
+			role: config.role,
+			protocol_id: config.protocol_id(),
+			fork_id: None,
+			net_config: &mut net_config,
+			block_announce_validator: Box::new(DefaultBlockAnnounceValidator),
+			network_service_handle: network_service_provider.handle(),
+			warp_sync_config: Some(WarpSyncConfig::WithProvider(warp_sync)),
+			client: client.clone(),
+			import_queue_service: import_queue.service(),
+			num_peers_hint: config.network.default_peers_set.in_peers as usize +
+				config.network.default_peers_set.out_peers as usize,
+			spawn_handle: &task_manager.spawn_handle(),
+			metrics_registry: config.prometheus_config.as_ref().map(|config| &config.registry),
+			metrics: metrics.clone(),
+		})?;
 
 	let (network, system_rpc_tx, tx_handler_controller, network_starter, sync_service) =
 		sc_service::build_network(sc_service::BuildNetworkParams {
@@ -1061,8 +1060,8 @@ pub fn new_full<
 			transaction_pool: transaction_pool.clone(),
 			spawn_handle: task_manager.spawn_handle(),
 			import_queue,
-			block_announce_validator_builder: None,
-			syncing_strategy,
+			sync_service,
+			block_announce_config,
 			network_service_provider,
 			metrics,
 		})?;

substrate/bin/node/cli/src/service.rs

@@ -34,7 +34,8 @@ use kitchensink_runtime::RuntimeApi;
 use node_primitives::Block;
 use polkadot_sdk::{
 	sc_network_sync::service::network::NetworkServiceProvider,
-	sc_service::{build_default_block_downloader, build_polkadot_syncing_strategy},
+	sc_service::{build_default_syncing_engine, DefaultSyncingEngineConfig, ImportQueue},
+	sp_consensus::block_validation::DefaultBlockAnnounceValidator,
 };
 use sc_client_api::{Backend, BlockBackend};
 use sc_consensus_babe::{self, SlotProportion};
@@ -510,29 +511,24 @@ pub fn new_full_base<N: NetworkBackend<Block, <Block as BlockT>::Hash>>(
 		Vec::default(),
 	));
 
-	let protocol_id = config.protocol_id();
-	let spawn_handle = task_manager.spawn_handle();
 	let network_service_provider = NetworkServiceProvider::new();
-	let block_downloader = build_default_block_downloader(
-		&protocol_id,
-		config.chain_spec.fork_id(),
-		&mut net_config,
-		network_service_provider.handle(),
-		client.clone(),
-		config.network.default_peers_set.in_peers as usize +
-			config.network.default_peers_set.out_peers as usize,
-		&spawn_handle,
-	);
-	let syncing_strategy = build_polkadot_syncing_strategy(
-		protocol_id,
-		config.chain_spec.fork_id(),
-		&mut net_config,
-		Some(WarpSyncConfig::WithProvider(warp_sync)),
-		block_downloader,
-		client.clone(),
-		&spawn_handle,
-		config.prometheus_config.as_ref().map(|config| &config.registry),
-	)?;
+	let (sync_service, block_announce_config) =
+		build_default_syncing_engine(DefaultSyncingEngineConfig {
+			role: config.role,
+			protocol_id: config.protocol_id(),
+			fork_id: None,
+			net_config: &mut net_config,
+			block_announce_validator: Box::new(DefaultBlockAnnounceValidator),
+			network_service_handle: network_service_provider.handle(),
+			warp_sync_config: Some(WarpSyncConfig::WithProvider(warp_sync)),
+			client: client.clone(),
+			import_queue_service: import_queue.service(),
+			num_peers_hint: config.network.default_peers_set.in_peers as usize +
+				config.network.default_peers_set.out_peers as usize,
+			spawn_handle: &task_manager.spawn_handle(),
+			metrics_registry: config.prometheus_config.as_ref().map(|config| &config.registry),
+			metrics: metrics.clone(),
+		})?;
 
 	let (network, system_rpc_tx, tx_handler_controller, network_starter, sync_service) =
 		sc_service::build_network(sc_service::BuildNetworkParams {
@@ -542,8 +538,8 @@ pub fn new_full_base<N: NetworkBackend<Block, <Block as BlockT>::Hash>>(
 			transaction_pool: transaction_pool.clone(),
 			spawn_handle: task_manager.spawn_handle(),
 			import_queue,
-			block_announce_validator_builder: None,
-			syncing_strategy,
+			sync_service,
+			block_announce_config,
 			network_service_provider,
 			metrics,
 		})?;

substrate/client/network/sync/src/engine.rs

@@ -279,7 +279,7 @@ where
 		network_metrics: NotificationMetrics,
 		net_config: &FullNetworkConfiguration<B, <B as BlockT>::Hash, N>,
 		protocol_id: ProtocolId,
-		fork_id: &Option<String>,
+		fork_id: Option<&str>,
 		block_announce_validator: Box<dyn BlockAnnounceValidator<B> + Send>,
 		syncing_strategy: Box<dyn SyncingStrategy<B>>,
 		network_service: service::network::NetworkServiceHandle,
@@ -1039,7 +1039,7 @@ where
 	/// Get config for the block announcement protocol
 	fn get_block_announce_proto_config<N: NetworkBackend<B, <B as BlockT>::Hash>>(
 		protocol_id: ProtocolId,
-		fork_id: &Option<String>,
+		fork_id: Option<&str>,
 		roles: Roles,
 		best_number: NumberFor<B>,
 		best_hash: B::Hash,
@@ -1050,7 +1050,7 @@ where
 	) -> (N::NotificationProtocolConfig, Box<dyn NotificationService>) {
 		let block_announces_protocol = {
 			let genesis_hash = genesis_hash.as_ref();
-			if let Some(ref fork_id) = fork_id {
+			if let Some(fork_id) = fork_id {
 				format!(
 					"/{}/{}/block-announces/1",
 					array_bytes::bytes2hex("", genesis_hash),

substrate/client/network/test/src/lib.rs

@@ -936,7 +936,7 @@ pub trait TestNetFactory: Default + Sized + Send {
 				metrics,
 				&full_net_config,
 				protocol_id.clone(),
-				&fork_id,
+				fork_id.as_deref(),
 				block_announce_validator,
 				syncing_strategy,
 				chain_sync_network_handle,

substrate/client/network/test/src/service.rs

@@ -224,7 +224,7 @@ impl TestNetworkBuilder {
 			NotificationMetrics::new(None),
 			&full_net_config,
 			protocol_id.clone(),
-			&None,
+			None,
 			Box::new(sp_consensus::block_validation::DefaultBlockAnnounceValidator),
 			syncing_strategy,
 			chain_sync_network_handle,