Replace Arc<SyncingService<Block>> with SyncingService<Block>

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

@@ -364,7 +364,7 @@ pub fn build_inprocess_relay_chain(
 	let relay_chain_interface = Arc::new(RelayChainInProcessInterface::new(
 		full_node.client,
 		full_node.backend,
-		full_node.sync_service,
+		Arc::new(full_node.sync_service),
 		full_node.overseer_handle.clone().ok_or(RelayChainError::GenericError(
 			"Overseer not running in full node.".to_string(),
 		))?,

cumulus/client/service/src/lib.rs

@@ -91,7 +91,7 @@ pub struct StartCollatorParams<'a, Block: BlockT, BS, Client, RCInterface, Spawn
 	pub collator_key: CollatorPair,
 	pub relay_chain_slot_duration: Duration,
 	pub recovery_handle: Box<dyn RecoveryHandle>,
-	pub sync_service: Arc<SyncingService<Block>>,
+	pub sync_service: SyncingService<Block>,
 }
 
 /// Parameters given to [`start_relay_chain_tasks`].
@@ -105,7 +105,7 @@ pub struct StartRelayChainTasksParams<'a, Block: BlockT, Client, RCInterface> {
 	pub import_queue: Box<dyn ImportQueueService<Block>>,
 	pub relay_chain_slot_duration: Duration,
 	pub recovery_handle: Box<dyn RecoveryHandle>,
-	pub sync_service: Arc<SyncingService<Block>>,
+	pub sync_service: SyncingService<Block>,
 }
 
 /// Parameters given to [`start_full_node`].
@@ -118,7 +118,7 @@ pub struct StartFullNodeParams<'a, Block: BlockT, Client, RCInterface> {
 	pub relay_chain_slot_duration: Duration,
 	pub import_queue: Box<dyn ImportQueueService<Block>>,
 	pub recovery_handle: Box<dyn RecoveryHandle>,
-	pub sync_service: Arc<SyncingService<Block>>,
+	pub sync_service: SyncingService<Block>,
 }
 
 /// Start a collator node for a parachain.
@@ -275,7 +275,7 @@ where
 		relay_chain_interface.clone(),
 		para_id,
 		recovery_chan_rx,
-		sync_service,
+		Arc::new(sync_service),
 	);
 
 	task_manager
@@ -439,7 +439,7 @@ pub async fn build_network<'a, Block, Client, RCInterface, IQ, Network>(
 	TracingUnboundedSender<sc_rpc::system::Request<Block>>,
 	TransactionsHandlerController<Block::Hash>,
 	NetworkStarter,
-	Arc<SyncingService<Block>>,
+	SyncingService<Block>,
 )>
 where
 	Block: BlockT,

cumulus/test/service/src/lib.rs

@@ -267,7 +267,7 @@ async fn build_relay_chain_interface(
 	task_manager: &mut TaskManager,
 ) -> RelayChainResult<Arc<dyn RelayChainInterface + 'static>> {
 	let relay_chain_full_node = match collator_options.relay_chain_mode {
-		cumulus_client_cli::RelayChainMode::Embedded => polkadot_test_service::new_full(
+		RelayChainMode::Embedded => polkadot_test_service::new_full(
 			relay_chain_config,
 			if let Some(ref key) = collator_key {
 				polkadot_service::IsParachainNode::Collator(key.clone())
@@ -278,15 +278,15 @@ async fn build_relay_chain_interface(
 			polkadot_service::CollatorOverseerGen,
 		)
 		.map_err(|e| RelayChainError::Application(Box::new(e) as Box<_>))?,
-		cumulus_client_cli::RelayChainMode::ExternalRpc(rpc_target_urls) =>
+		RelayChainMode::ExternalRpc(rpc_target_urls) =>
 			return build_minimal_relay_chain_node_with_rpc(
 				relay_chain_config,
 				task_manager,
 				rpc_target_urls,
 			)
 			.await
 			.map(|r| r.0),
-		cumulus_client_cli::RelayChainMode::LightClient =>
+		RelayChainMode::LightClient =>
 			return build_minimal_relay_chain_node_light_client(relay_chain_config, task_manager)
 				.await
 				.map(|r| r.0),
@@ -295,9 +295,9 @@ async fn build_relay_chain_interface(
 	task_manager.add_child(relay_chain_full_node.task_manager);
 	tracing::info!("Using inprocess node.");
 	Ok(Arc::new(RelayChainInProcessInterface::new(
-		relay_chain_full_node.client.clone(),
-		relay_chain_full_node.backend.clone(),
-		relay_chain_full_node.sync_service.clone(),
+		relay_chain_full_node.client,
+		relay_chain_full_node.backend,
+		Arc::new(relay_chain_full_node.sync_service),
 		relay_chain_full_node.overseer_handle.ok_or(RelayChainError::GenericError(
 			"Overseer should be running in full node.".to_string(),
 		))?,

polkadot/node/service/src/lib.rs

@@ -675,7 +675,7 @@ pub struct NewFull {
 	pub client: Arc<FullClient>,
 	pub overseer_handle: Option<Handle>,
 	pub network: Arc<dyn sc_network::service::traits::NetworkService>,
-	pub sync_service: Arc<sc_network_sync::SyncingService<Block>>,
+	pub sync_service: sc_network_sync::SyncingService<Block>,
 	pub rpc_handlers: RpcHandlers,
 	pub backend: Arc<FullBackend>,
 }
@@ -1166,7 +1166,7 @@ pub fn new_full<
 				OverseerGenArgs {
 					runtime_client,
 					network_service: network.clone(),
-					sync_service: sync_service.clone(),
+					sync_service: Arc::new(sync_service.clone()),
 					authority_discovery_service,
 					collation_req_v1_receiver,
 					collation_req_v2_receiver,

prdoc/pr_5454.prdoc

@@ -0,0 +1,24 @@
+title: Replace `Arc<SyncingService<Block>>` with `SyncingService<Block>`
+
+doc:
+  - audience: Node Dev
+    description: |
+      `SyncingService<Block>` is already cloneable and doesn't need to be wrapped in `Arc` by default
+
+crates:
+  - name: sc-consensus-beefy
+    bump: major
+  - name: sc-mixnet
+    bump: major
+  - name: sc-service
+    bump: major
+  - name: sc-service-test
+    bump: major
+  - name: sc-network-test
+    bump: major
+  - name: polkadot-service
+    bump: major
+  - name: cumulus-relay-chain-inprocess-interface
+    bump: major
+  - name: cumulus-client-service
+    bump: major

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

@@ -384,7 +384,7 @@ pub struct NewFullBase {
 	/// The networking service of the node.
 	pub network: Arc<dyn NetworkService>,
 	/// The syncing service of the node.
-	pub sync: Arc<SyncingService<Block>>,
+	pub sync: SyncingService<Block>,
 	/// The transaction pool of the node.
 	pub transaction_pool: Arc<TransactionPool>,
 	/// The rpc handlers of the node.

substrate/client/consensus/beefy/src/lib.rs

@@ -196,7 +196,7 @@ pub struct BeefyNetworkParams<B: Block, N, S> {
 	/// Network implementing gossip, requests and sync-oracle.
 	pub network: Arc<N>,
 	/// Syncing service implementing a sync oracle and an event stream for peers.
-	pub sync: Arc<S>,
+	pub sync: S,
 	/// Handle for receiving notification events.
 	pub notification_service: Box<dyn NotificationService>,
 	/// Chain specific BEEFY gossip protocol name. See
@@ -310,7 +310,7 @@ where
 	pub fn build<P, S, N>(
 		self,
 		payload_provider: P,
-		sync: Arc<S>,
+		sync: S,
 		comms: BeefyComms<B, N, AuthorityId>,
 		links: BeefyVoterLinks<B, AuthorityId>,
 		pending_justifications: BTreeMap<NumberFor<B>, BeefyVersionedFinalityProof<B, AuthorityId>>,
@@ -527,7 +527,7 @@ pub async fn start_beefy_gadget<B, BE, C, N, P, R, S, AuthorityId>(
 	R: ProvideRuntimeApi<B>,
 	R::Api: BeefyApi<B, AuthorityId>,
 	N: GossipNetwork<B> + NetworkRequest + Send + Sync + 'static,
-	S: GossipSyncing<B> + SyncOracle + 'static,
+	S: GossipSyncing<B> + SyncOracle + Clone + 'static,
 	AuthorityId: AuthorityIdBound,
 {
 	let BeefyParams {

substrate/client/consensus/beefy/src/worker.rs

@@ -386,7 +386,7 @@ pub(crate) struct BeefyWorker<B: Block, BE, P, RuntimeApi, S, N, AuthorityId: Au
 	pub runtime: Arc<RuntimeApi>,
 	pub key_store: Arc<BeefyKeystore<AuthorityId>>,
 	pub payload_provider: P,
-	pub sync: Arc<S>,
+	pub sync: S,
 	pub fisherman: Arc<Fisherman<B, BE, RuntimeApi, AuthorityId>>,
 
 	// communication (created once, but returned and reused if worker is restarted/reinitialized)
@@ -1079,7 +1079,7 @@ pub(crate) mod tests {
 		Backend,
 		MmrRootProvider<Block, TestApi>,
 		TestApi,
-		Arc<SyncingService<Block>>,
+		SyncingService<Block>,
 		TestNetwork,
 		ecdsa_crypto::AuthorityId,
 	> {
@@ -1154,7 +1154,7 @@ pub(crate) mod tests {
 			key_store: key_store.clone(),
 			metrics,
 			payload_provider,
-			sync: Arc::new(sync),
+			sync,
 			fisherman: Arc::new(Fisherman::new(backend, api, key_store)),
 			links,
 			comms,

substrate/client/mixnet/src/run.rs

@@ -150,7 +150,7 @@ pub async fn run<B, C, S, P>(
 	config: Config,
 	mut api_backend: ApiBackend,
 	client: Arc<C>,
-	sync: Arc<S>,
+	sync: S,
 	network: Arc<dyn NetworkService>,
 	protocol_name: ProtocolName,
 	transaction_pool: Arc<P>,

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

@@ -236,7 +236,7 @@ pub struct Peer<D, BlockImport> {
 	select_chain: Option<LongestChain<substrate_test_runtime_client::Backend, Block>>,
 	backend: Option<Arc<substrate_test_runtime_client::Backend>>,
 	network: NetworkWorker<Block, <Block as BlockT>::Hash>,
-	sync_service: Arc<SyncingService<Block>>,
+	sync_service: SyncingService<Block>,
 	imported_blocks_stream: Pin<Box<dyn Stream<Item = BlockImportNotification<Block>> + Send>>,
 	finality_notification_stream: Pin<Box<dyn Stream<Item = FinalityNotification<Block>> + Send>>,
 	listen_addr: Multiaddr,
@@ -510,7 +510,7 @@ where
 	}
 
 	/// Get `SyncingService`.
-	pub fn sync_service(&self) -> &Arc<SyncingService<Block>> {
+	pub fn sync_service(&self) -> &SyncingService<Block> {
 		&self.sync_service
 	}
 
@@ -925,7 +925,6 @@ pub trait TestNetFactory: Default + Sized + Send {
 			)
 			.unwrap();
 		let sync_service_import_queue = Box::new(sync_service.clone());
-		let sync_service = Arc::new(sync_service.clone());
 
 		for config in config.request_response_protocols {
 			full_net_config.add_request_response_protocol(config);

substrate/client/service/src/builder.rs

@@ -371,7 +371,7 @@ pub struct SpawnTasksParams<'a, TBl: BlockT, TCl, TExPool, TRpc, Backend> {
 	pub tx_handler_controller:
 		sc_network_transactions::TransactionsHandlerController<<TBl as BlockT>::Hash>,
 	/// Syncing service.
-	pub sync_service: Arc<SyncingService<TBl>>,
+	pub sync_service: SyncingService<TBl>,
 	/// Telemetry instance for this node.
 	pub telemetry: Option<&'a mut Telemetry>,
 }
@@ -774,7 +774,7 @@ pub fn build_network<TBl, TNet, TExPool, TImpQu, TCl>(
 		TracingUnboundedSender<sc_rpc::system::Request<TBl>>,
 		sc_network_transactions::TransactionsHandlerController<<TBl as BlockT>::Hash>,
 		NetworkStarter,
-		Arc<SyncingService<TBl>>,
+		SyncingService<TBl>,
 	),
 	Error,
 >
@@ -948,7 +948,6 @@ where
 		Arc::clone(&peer_store_handle),
 	)?;
 	let sync_service_import_queue = sync_service.clone();
-	let sync_service = Arc::new(sync_service);
 
 	let genesis_hash = client.hash(Zero::zero()).ok().flatten().expect("Genesis block exists; qed");
 	let network_params = sc_network::config::Params::<TBl, <TBl as BlockT>::Hash, TNet> {

substrate/client/service/src/lib.rs

@@ -171,7 +171,7 @@ async fn build_network_future<
 >(
 	network: N,
 	client: Arc<C>,
-	sync_service: Arc<SyncingService<B>>,
+	sync_service: SyncingService<B>,
 	announce_imported_blocks: bool,
 ) {
 	let mut imported_blocks_stream = client.import_notification_stream().fuse();
@@ -237,7 +237,7 @@ pub async fn build_system_rpc_future<
 >(
 	role: Role,
 	network_service: Arc<dyn NetworkService>,
-	sync_service: Arc<SyncingService<B>>,
+	sync_service: SyncingService<B>,
 	client: Arc<C>,
 	mut rpc_rx: TracingUnboundedReceiver<sc_rpc::system::Request<B>>,
 	should_have_peers: bool,
@@ -267,7 +267,7 @@ pub async fn build_system_rpc_future<
 				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()

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

@@ -74,7 +74,7 @@ pub trait TestNetNode: Clone + Future<Output = Result<(), Error>> + Send + 'stat
 	fn client(&self) -> Arc<Client<Self::Backend, Self::Executor, Self::Block, Self::RuntimeApi>>;
 	fn transaction_pool(&self) -> Arc<Self::TransactionPool>;
 	fn network(&self) -> Arc<dyn sc_network::service::traits::NetworkService>;
-	fn sync(&self) -> &Arc<SyncingService<Self::Block>>;
+	fn sync(&self) -> &SyncingService<Self::Block>;
 	fn spawn_handle(&self) -> SpawnTaskHandle;
 }
 
@@ -83,7 +83,7 @@ pub struct TestNetComponents<TBl: BlockT, TBackend, TExec, TRtApi, TExPool> {
 	client: Arc<Client<TBackend, TExec, TBl, TRtApi>>,
 	transaction_pool: Arc<TExPool>,
 	network: Arc<dyn sc_network::service::traits::NetworkService>,
-	sync: Arc<SyncingService<TBl>>,
+	sync: SyncingService<TBl>,
 }
 
 impl<TBl: BlockT, TBackend, TExec, TRtApi, TExPool>
@@ -93,7 +93,7 @@ impl<TBl: BlockT, TBackend, TExec, TRtApi, TExPool>
 		task_manager: TaskManager,
 		client: Arc<Client<TBackend, TExec, TBl, TRtApi>>,
 		network: Arc<dyn sc_network::service::traits::NetworkService>,
-		sync: Arc<SyncingService<TBl>>,
+		sync: SyncingService<TBl>,
 		transaction_pool: Arc<TExPool>,
 	) -> Self {
 		Self {
@@ -154,7 +154,7 @@ where
 	fn network(&self) -> Arc<dyn sc_network::service::traits::NetworkService> {
 		self.network.clone()
 	}
-	fn sync(&self) -> &Arc<SyncingService<Self::Block>> {
+	fn sync(&self) -> &SyncingService<Self::Block> {
 		&self.sync
 	}
 	fn spawn_handle(&self) -> SpawnTaskHandle {