Extract syncing protocol from sc-network (#12828)

* Move import queue out of `sc-network`

Add supplementary asynchronous API for the import queue which means
it can be run as an independent task and communicated with through
the `ImportQueueService`.

This commit removes removes block and justification imports from
`sc-network` and provides `ChainSync` with a handle to import queue so
it can import blocks and justifications. Polling of the import queue is
moved complete out of `sc-network` and `sc_consensus::Link` is
implemented for `ChainSyncInterfaceHandled` so the import queue
can still influence the syncing process.

* Move stuff to SyncingEngine

* Move `ChainSync` instanation to `SyncingEngine`

Some of the tests have to be rewritten

* Move peer hashmap to `SyncingEngine`

* Let `SyncingEngine` to implement `ChainSyncInterface`

* Introduce `SyncStatusProvider`

* Move `sync_peer_(connected|disconnected)` to `SyncingEngine`

* Implement `SyncEventStream`

Remove `SyncConnected`/`SyncDisconnected` events from
`NetworkEvenStream` and provide those events through
`ChainSyncInterface` instead.

Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take
`SyncEventStream` object which they listen to for incoming sync peer
events.

* Introduce `ChainSyncInterface`

This interface provides a set of miscellaneous functions that other
subsystems can use to query, for example, the syncing status.

* Move event stream polling to `SyncingEngine`

Subscribe to `NetworkStreamEvent` and poll the incoming notifications
and substream events from `SyncingEngine`.

The code needs refactoring.

* Make `SyncingEngine` into an asynchronous runner

This commits removes the last hard dependency of syncing from
`sc-network` meaning the protocol now lives completely outside of
`sc-network`, ignoring the hardcoded peerset entry which will be
addressed in the future.

Code needs a lot of refactoring.

* Fix warnings

* Code refactoring

* Use `SyncingService` for BEEFY

* Use `SyncingService` for GRANDPA

* Remove call delegation from `NetworkService`

* Remove `ChainSyncService`

* Remove `ChainSync` service tests

They were written for the sole purpose of verifying that `NetworWorker`
continues to function while the calls are being dispatched to
`ChainSync`.

* Refactor code

* Refactor code

* Update client/finality-grandpa/src/communication/tests.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Fix warnings

* Apply review comments

* Fix docs

* Fix test

* cargo-fmt

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

* Add missing docs

* Refactor code

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>

substrate/client/service/src/builder.rs

@@ -38,18 +38,19 @@ use sc_client_db::{Backend, DatabaseSettings};
 use sc_consensus::import_queue::ImportQueue;
 use sc_executor::RuntimeVersionOf;
 use sc_keystore::LocalKeystore;
-use sc_network::{config::SyncMode, NetworkService};
+use sc_network::NetworkService;
 use sc_network_bitswap::BitswapRequestHandler;
 use sc_network_common::{
+	config::SyncMode,
 	protocol::role::Roles,
-	service::{NetworkStateInfo, NetworkStatusProvider},
+	service::{NetworkEventStream, NetworkStateInfo, NetworkStatusProvider},
 	sync::warp::WarpSyncParams,
 };
 use sc_network_light::light_client_requests::handler::LightClientRequestHandler;
 use sc_network_sync::{
-	block_request_handler::BlockRequestHandler, service::network::NetworkServiceProvider,
-	state_request_handler::StateRequestHandler,
-	warp_request_handler::RequestHandler as WarpSyncRequestHandler, ChainSync,
+	block_request_handler::BlockRequestHandler, engine::SyncingEngine,
+	service::network::NetworkServiceProvider, state_request_handler::StateRequestHandler,
+	warp_request_handler::RequestHandler as WarpSyncRequestHandler, SyncingService,
 };
 use sc_rpc::{
 	author::AuthorApiServer,
@@ -349,12 +350,7 @@ where
 
 /// Shared network instance implementing a set of mandatory traits.
 pub trait SpawnTaskNetwork<Block: BlockT>:
-	sc_offchain::NetworkProvider
-	+ NetworkStateInfo
-	+ NetworkStatusProvider<Block>
-	+ Send
-	+ Sync
-	+ 'static
+	sc_offchain::NetworkProvider + NetworkStateInfo + NetworkStatusProvider + Send + Sync + 'static
 {
 }
 
@@ -363,7 +359,7 @@ where
 	Block: BlockT,
 	T: sc_offchain::NetworkProvider
 		+ NetworkStateInfo
-		+ NetworkStatusProvider<Block>
+		+ NetworkStatusProvider
 		+ Send
 		+ Sync
 		+ 'static,
@@ -394,6 +390,8 @@ pub struct SpawnTasksParams<'a, TBl: BlockT, TCl, TExPool, TRpc, Backend> {
 	/// Controller for transactions handlers
 	pub tx_handler_controller:
 		sc_network_transactions::TransactionsHandlerController<<TBl as BlockT>::Hash>,
+	/// Syncing service.
+	pub sync_service: Arc<SyncingService<TBl>>,
 	/// Telemetry instance for this node.
 	pub telemetry: Option<&'a mut Telemetry>,
 }
@@ -471,6 +469,7 @@ where
 		network,
 		system_rpc_tx,
 		tx_handler_controller,
+		sync_service,
 		telemetry,
 	} = params;
 
@@ -533,7 +532,12 @@ where
 	spawn_handle.spawn(
 		"telemetry-periodic-send",
 		None,
-		metrics_service.run(client.clone(), transaction_pool.clone(), network.clone()),
+		metrics_service.run(
+			client.clone(),
+			transaction_pool.clone(),
+			network.clone(),
+			sync_service.clone(),
+		),
 	);
 
 	let rpc_id_provider = config.rpc_id_provider.take();
@@ -560,7 +564,12 @@ where
 	spawn_handle.spawn(
 		"informant",
 		None,
-		sc_informant::build(client.clone(), network, config.informant_output_format),
+		sc_informant::build(
+			client.clone(),
+			network,
+			sync_service.clone(),
+			config.informant_output_format,
+		),
 	);
 
 	task_manager.keep_alive((config.base_path, rpc));
@@ -771,6 +780,7 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>(
 		TracingUnboundedSender<sc_rpc::system::Request<TBl>>,
 		sc_network_transactions::TransactionsHandlerController<<TBl as BlockT>::Hash>,
 		NetworkStarter,
+		Arc<SyncingService<TBl>>,
 	),
 	Error,
 >
@@ -876,27 +886,23 @@ where
 	};
 
 	let (chain_sync_network_provider, chain_sync_network_handle) = NetworkServiceProvider::new();
-	let (chain_sync, chain_sync_service, block_announce_config) = ChainSync::new(
-		match config.network.sync_mode {
-			SyncMode::Full => sc_network_common::sync::SyncMode::Full,
-			SyncMode::Fast { skip_proofs, storage_chain_mode } =>
-				sc_network_common::sync::SyncMode::LightState { skip_proofs, storage_chain_mode },
-			SyncMode::Warp => sc_network_common::sync::SyncMode::Warp,
-		},
+	let (engine, sync_service, block_announce_config) = SyncingEngine::new(
+		Roles::from(&config.role),
 		client.clone(),
+		config.prometheus_config.as_ref().map(|config| config.registry.clone()).as_ref(),
+		&config.network,
 		protocol_id.clone(),
 		&config.chain_spec.fork_id().map(ToOwned::to_owned),
-		Roles::from(&config.role),
 		block_announce_validator,
-		config.network.max_parallel_downloads,
 		warp_sync_params,
-		config.prometheus_config.as_ref().map(|config| config.registry.clone()).as_ref(),
 		chain_sync_network_handle,
 		import_queue.service(),
 		block_request_protocol_config.name.clone(),
 		state_request_protocol_config.name.clone(),
 		warp_sync_protocol_config.as_ref().map(|config| config.name.clone()),
 	)?;
+	let sync_service_import_queue = sync_service.clone();
+	let sync_service = Arc::new(sync_service);
 
 	request_response_protocol_configs.push(config.network.ipfs_server.then(|| {
 		let (handler, protocol_config) = BitswapRequestHandler::new(client.clone());
@@ -916,8 +922,6 @@ where
 		chain: client.clone(),
 		protocol_id: protocol_id.clone(),
 		fork_id: config.chain_spec.fork_id().map(ToOwned::to_owned),
-		chain_sync: Box::new(chain_sync),
-		chain_sync_service: Box::new(chain_sync_service.clone()),
 		metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()),
 		block_announce_config,
 		request_response_protocol_configs: request_response_protocol_configs
@@ -953,6 +957,7 @@ where
 
 	let (tx_handler, tx_handler_controller) = transactions_handler_proto.build(
 		network.clone(),
+		sync_service.clone(),
 		Arc::new(TransactionPoolAdapter { pool: transaction_pool, client: client.clone() }),
 		config.prometheus_config.as_ref().map(|config| &config.registry),
 	)?;
@@ -963,11 +968,10 @@ where
 		Some("networking"),
 		chain_sync_network_provider.run(network.clone()),
 	);
-	spawn_handle.spawn(
-		"import-queue",
-		None,
-		import_queue.run(Box::new(chain_sync_service.clone())),
-	);
+	spawn_handle.spawn("import-queue", None, import_queue.run(Box::new(sync_service_import_queue)));
+
+	let event_stream = network.event_stream("syncing");
+	spawn_handle.spawn("syncing", None, engine.run(event_stream));
 
 	let (system_rpc_tx, system_rpc_rx) = tracing_unbounded("mpsc_system_rpc", 10_000);
 	spawn_handle.spawn(
@@ -976,7 +980,7 @@ where
 		build_system_rpc_future(
 			config.role.clone(),
 			network_mut.service().clone(),
-			chain_sync_service.clone(),
+			sync_service.clone(),
 			client.clone(),
 			system_rpc_rx,
 			has_bootnodes,
@@ -984,7 +988,7 @@ where
 	);
 
 	let future =
-		build_network_future(network_mut, client, chain_sync_service, config.announce_block);
+		build_network_future(network_mut, client, sync_service.clone(), config.announce_block);
 
 	// TODO: Normally, one is supposed to pass a list of notifications protocols supported by the
 	// node through the `NetworkConfiguration` struct. But because this function doesn't know in
@@ -1022,7 +1026,13 @@ where
 		future.await
 	});
 
-	Ok((network, system_rpc_tx, tx_handler_controller, NetworkStarter(network_start_tx)))
+	Ok((
+		network,
+		system_rpc_tx,
+		tx_handler_controller,
+		NetworkStarter(network_start_tx),
+		sync_service.clone(),
+	))
 }
 
 /// Object used to start the network.

substrate/client/service/src/config.rs

@@ -22,11 +22,14 @@ pub use sc_client_api::execution_extensions::{ExecutionStrategies, ExecutionStra
 pub use sc_client_db::{BlocksPruning, Database, DatabaseSource, PruningMode};
 pub use sc_executor::{WasmExecutionMethod, WasmtimeInstantiationStrategy};
 pub use sc_network::{
-	config::{NetworkConfiguration, NodeKeyConfig, Role},
+	config::{NetworkConfiguration, Role},
 	Multiaddr,
 };
 pub use sc_network_common::{
-	config::{MultiaddrWithPeerId, NonDefaultSetConfig, ProtocolId, SetConfig, TransportConfig},
+	config::{
+		MultiaddrWithPeerId, NodeKeyConfig, NonDefaultSetConfig, ProtocolId, SetConfig,
+		TransportConfig,
+	},
 	request_responses::{
 		IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig,
 	},
@@ -34,7 +37,7 @@ pub use sc_network_common::{
 
 use prometheus_endpoint::Registry;
 use sc_chain_spec::ChainSpec;
-use sc_network::config::SyncMode;
+use sc_network_common::config::SyncMode;
 pub use sc_telemetry::TelemetryEndpoints;
 pub use sc_transaction_pool::Options as TransactionPoolOptions;
 use sp_core::crypto::SecretString;

substrate/client/service/src/lib.rs

@@ -46,7 +46,7 @@ use sc_network_common::{
 	config::MultiaddrWithPeerId,
 	service::{NetworkBlock, NetworkPeers},
 };
-use sc_network_sync::service::chain_sync::ChainSyncInterfaceHandle;
+use sc_network_sync::SyncingService;
 use sc_utils::mpsc::TracingUnboundedReceiver;
 use sp_blockchain::HeaderMetadata;
 use sp_consensus::SyncOracle;
@@ -158,9 +158,9 @@ async fn build_network_future<
 		+ 'static,
 	H: sc_network_common::ExHashT,
 >(
-	network: sc_network::NetworkWorker<B, H, C>,
+	network: sc_network::NetworkWorker<B, H>,
 	client: Arc<C>,
-	chain_sync_service: ChainSyncInterfaceHandle<B>,
+	sync_service: Arc<SyncingService<B>>,
 	announce_imported_blocks: bool,
 ) {
 	let mut imported_blocks_stream = client.import_notification_stream().fuse();
@@ -168,8 +168,6 @@ async fn build_network_future<
 	// Stream of finalized blocks reported by the client.
 	let mut finality_notification_stream = client.finality_notification_stream().fuse();
 
-	let network_service = network.service().clone();
-
 	let network_run = network.run().fuse();
 	pin_mut!(network_run);
 
@@ -188,11 +186,11 @@ async fn build_network_future<
 				};
 
 				if announce_imported_blocks {
-					network_service.announce_block(notification.hash, None);
+					sync_service.announce_block(notification.hash, None);
 				}
 
 				if notification.is_new_best {
-					network_service.new_best_block_imported(
+					sync_service.new_best_block_imported(
 						notification.hash,
 						*notification.header.number(),
 					);
@@ -201,7 +199,7 @@ async fn build_network_future<
 
 			// List of blocks that the client has finalized.
 			notification = finality_notification_stream.select_next_some() => {
-				chain_sync_service.on_block_finalized(notification.hash, *notification.header.number());
+				sync_service.on_block_finalized(notification.hash, notification.header);
 			}
 
 			// Drive the network. Shut down the network future if `NetworkWorker` has terminated.
@@ -228,7 +226,7 @@ async fn build_system_rpc_future<
 >(
 	role: Role,
 	network_service: Arc<sc_network::NetworkService<B, H>>,
-	chain_sync_service: ChainSyncInterfaceHandle<B>,
+	sync_service: Arc<SyncingService<B>>,
 	client: Arc<C>,
 	mut rpc_rx: TracingUnboundedReceiver<sc_rpc::system::Request<B>>,
 	should_have_peers: bool,
@@ -244,23 +242,21 @@ async fn build_system_rpc_future<
 		};
 
 		match req {
-			sc_rpc::system::Request::Health(sender) => {
-				let peers = network_service.peers_debug_info().await;
-				if let Ok(peers) = peers {
+			sc_rpc::system::Request::Health(sender) => match sync_service.peers_info().await {
+				Ok(info) => {
 					let _ = sender.send(sc_rpc::system::Health {
-						peers: peers.len(),
-						is_syncing: network_service.is_major_syncing(),
+						peers: info.len(),
+						is_syncing: sync_service.is_major_syncing(),
 						should_have_peers,
 					});
-				} else {
-					break
-				}
+				},
+				Err(_) => log::error!("`SyncingEngine` shut down"),
 			},
 			sc_rpc::system::Request::LocalPeerId(sender) => {
 				let _ = sender.send(network_service.local_peer_id().to_base58());
 			},
 			sc_rpc::system::Request::LocalListenAddresses(sender) => {
-				let peer_id = network_service.local_peer_id().into();
+				let peer_id = (network_service.local_peer_id()).into();
 				let p2p_proto_suffix = sc_network::multiaddr::Protocol::P2p(peer_id);
 				let addresses = network_service
 					.listen_addresses()
@@ -269,12 +265,10 @@ async fn build_system_rpc_future<
 					.collect();
 				let _ = sender.send(addresses);
 			},
-			sc_rpc::system::Request::Peers(sender) => {
-				let peers = network_service.peers_debug_info().await;
-				if let Ok(peers) = peers {
+			sc_rpc::system::Request::Peers(sender) => match sync_service.peers_info().await {
+				Ok(info) => {
 					let _ = sender.send(
-						peers
-							.into_iter()
+						info.into_iter()
 							.map(|(peer_id, p)| sc_rpc::system::PeerInfo {
 								peer_id: peer_id.to_base58(),
 								roles: format!("{:?}", p.roles),
@@ -283,9 +277,8 @@ async fn build_system_rpc_future<
 							})
 							.collect(),
 					);
-				} else {
-					break
-				}
+				},
+				Err(_) => log::error!("`SyncingEngine` shut down"),
 			},
 			sc_rpc::system::Request::NetworkState(sender) => {
 				let network_state = network_service.network_state().await;
@@ -339,21 +332,21 @@ async fn build_system_rpc_future<
 			sc_rpc::system::Request::SyncState(sender) => {
 				use sc_rpc::system::SyncState;
 
-				let best_number = client.info().best_number;
-
-				let Ok(status) = chain_sync_service.status().await else {
-					debug!("`ChainSync` has terminated, shutting down the system RPC future.");
-					return
-				};
-
-				let _ = sender.send(SyncState {
-					starting_block,
-					current_block: best_number,
-					highest_block: status.best_seen_block.unwrap_or(best_number),
-				});
+				match sync_service.best_seen_block().await {
+					Ok(best_seen_block) => {
+						let best_number = client.info().best_number;
+						let _ = sender.send(SyncState {
+							starting_block,
+							current_block: best_number,
+							highest_block: best_seen_block.unwrap_or(best_number),
+						});
+					},
+					Err(_) => log::error!("`SyncingEngine` shut down"),
+				}
 			},
 		}
 	}
+
 	debug!("`NetworkWorker` has terminated, shutting down the system RPC future.");
 }
 

substrate/client/service/src/metrics.rs

@@ -23,7 +23,10 @@ use futures_timer::Delay;
 use prometheus_endpoint::{register, Gauge, GaugeVec, Opts, PrometheusError, Registry, U64};
 use sc_client_api::{ClientInfo, UsageProvider};
 use sc_network::config::Role;
-use sc_network_common::service::{NetworkStatus, NetworkStatusProvider};
+use sc_network_common::{
+	service::{NetworkStatus, NetworkStatusProvider},
+	sync::{SyncStatus, SyncStatusProvider},
+};
 use sc_telemetry::{telemetry, TelemetryHandle, SUBSTRATE_INFO};
 use sc_transaction_pool_api::{MaintainedTransactionPool, PoolStatus};
 use sc_utils::metrics::register_globals;
@@ -175,16 +178,18 @@ impl MetricsService {
 	/// Returns a never-ending `Future` that performs the
 	/// metric and telemetry updates with information from
 	/// the given sources.
-	pub async fn run<TBl, TExPool, TCl, TNet>(
+	pub async fn run<TBl, TExPool, TCl, TNet, TSync>(
 		mut self,
 		client: Arc<TCl>,
 		transactions: Arc<TExPool>,
 		network: TNet,
+		syncing: TSync,
 	) where
 		TBl: Block,
 		TCl: ProvideRuntimeApi<TBl> + UsageProvider<TBl>,
 		TExPool: MaintainedTransactionPool<Block = TBl, Hash = <TBl as Block>::Hash>,
-		TNet: NetworkStatusProvider<TBl>,
+		TNet: NetworkStatusProvider,
+		TSync: SyncStatusProvider<TBl>,
 	{
 		let mut timer = Delay::new(Duration::from_secs(0));
 		let timer_interval = Duration::from_secs(5);
@@ -196,8 +201,11 @@ impl MetricsService {
 			// Try to get the latest network information.
 			let net_status = network.status().await.ok();
 
+			// Try to get the latest syncing information.
+			let sync_status = syncing.status().await.ok();
+
 			// Update / Send the metrics.
-			self.update(&client.usage_info(), &transactions.status(), net_status);
+			self.update(&client.usage_info(), &transactions.status(), net_status, sync_status);
 
 			// Schedule next tick.
 			timer.reset(timer_interval);
@@ -208,7 +216,8 @@ impl MetricsService {
 		&mut self,
 		info: &ClientInfo<T>,
 		txpool_status: &PoolStatus,
-		net_status: Option<NetworkStatus<T>>,
+		net_status: Option<NetworkStatus>,
+		sync_status: Option<SyncStatus<T>>,
 	) {
 		let now = Instant::now();
 		let elapsed = (now - self.last_update).as_secs();
@@ -273,10 +282,12 @@ impl MetricsService {
 				"bandwidth_download" => avg_bytes_per_sec_inbound,
 				"bandwidth_upload" => avg_bytes_per_sec_outbound,
 			);
+		}
 
+		if let Some(sync_status) = sync_status {
 			if let Some(metrics) = self.metrics.as_ref() {
 				let best_seen_block: Option<u64> =
-					net_status.best_seen_block.map(|num: NumberFor<T>| {
+					sync_status.best_seen_block.map(|num: NumberFor<T>| {
 						UniqueSaturatedInto::<u64>::unique_saturated_into(num)
 					});