Remove HeaderBackend requirement from AuthorityDiscovery and NetworkWorker (#13730)

* Remove `HeaderBackend` requirement from `NetworkWorker`

* Remove HeaderBackend from authority-discovery

Author: skunert

client/authority-discovery/src/error.rs

@@ -23,6 +23,7 @@ pub type Result<T> = std::result::Result<T, Error>;
 
 /// Error type for the authority discovery module.
 #[derive(Debug, thiserror::Error)]
+#[allow(missing_docs)]
 pub enum Error {
 	#[error("Received dht value found event with records with different keys.")]
 	ReceivingDhtValueFoundEventWithDifferentKeys,
@@ -71,4 +72,7 @@ pub enum Error {
 
 	#[error("Received authority record without a valid signature for the remote peer id.")]
 	MissingPeerIdSignature,
+
+	#[error("Unable to fetch best block.")]
+	BestBlockFetchingError,
 }

client/authority-discovery/src/lib.rs

@@ -28,6 +28,7 @@
 //! See [`Worker`] and [`Service`] for more documentation.
 
 pub use crate::{
+	error::Error,
 	service::Service,
 	worker::{AuthorityDiscovery, NetworkProvider, Role, Worker},
 };
@@ -148,7 +149,7 @@ pub fn new_worker_and_service_with_config<Client, Network, Block, DhtEventStream
 where
 	Block: BlockT + Unpin + 'static,
 	Network: NetworkProvider,
-	Client: AuthorityDiscovery<Block> + HeaderBackend<Block> + 'static,
+	Client: AuthorityDiscovery<Block> + 'static,
 	DhtEventStream: Stream<Item = DhtEvent> + Unpin,
 {
 	let (to_worker, from_service) = mpsc::channel(0);

client/authority-discovery/src/worker.rs

@@ -157,12 +157,15 @@ pub trait AuthorityDiscovery<Block: BlockT> {
 	/// Retrieve authority identifiers of the current and next authority set.
 	async fn authorities(&self, at: Block::Hash)
 		-> std::result::Result<Vec<AuthorityId>, ApiError>;
+
+	/// Retrieve best block hash
+	async fn best_hash(&self) -> std::result::Result<Block::Hash, Error>;
 }
 
 #[async_trait::async_trait]
 impl<Block, T> AuthorityDiscovery<Block> for T
 where
-	T: ProvideRuntimeApi<Block> + Send + Sync,
+	T: ProvideRuntimeApi<Block> + HeaderBackend<Block> + Send + Sync,
 	T::Api: AuthorityDiscoveryApi<Block>,
 	Block: BlockT,
 {
@@ -172,13 +175,17 @@ where
 	) -> std::result::Result<Vec<AuthorityId>, ApiError> {
 		self.runtime_api().authorities(at)
 	}
+
+	async fn best_hash(&self) -> std::result::Result<Block::Hash, Error> {
+		Ok(self.info().best_hash)
+	}
 }
 
 impl<Client, Network, Block, DhtEventStream> Worker<Client, Network, Block, DhtEventStream>
 where
 	Block: BlockT + Unpin + 'static,
 	Network: NetworkProvider,
-	Client: AuthorityDiscovery<Block> + HeaderBackend<Block> + 'static,
+	Client: AuthorityDiscovery<Block> + 'static,
 	DhtEventStream: Stream<Item = DhtEvent> + Unpin,
 {
 	/// Construct a [`Worker`].
@@ -376,7 +383,7 @@ where
 	}
 
 	async fn refill_pending_lookups_queue(&mut self) -> Result<()> {
-		let best_hash = self.client.info().best_hash;
+		let best_hash = self.client.best_hash().await?;
 
 		let local_keys = match &self.role {
 			Role::PublishAndDiscover(key_store) => key_store
@@ -594,7 +601,7 @@ where
 			.into_iter()
 			.collect::<HashSet<_>>();
 
-		let best_hash = client.info().best_hash;
+		let best_hash = client.best_hash().await?;
 		let authorities = client
 			.authorities(best_hash)
 			.await

client/network/src/config.rs

@@ -34,6 +34,7 @@ use prometheus_endpoint::Registry;
 pub use sc_network_common::{role::Role, sync::warp::WarpSyncProvider, ExHashT};
 use zeroize::Zeroize;
 
+use sp_runtime::traits::Block as BlockT;
 use std::{
 	error::Error,
 	fmt, fs,
@@ -44,7 +45,6 @@ use std::{
 	path::{Path, PathBuf},
 	pin::Pin,
 	str::{self, FromStr},
-	sync::Arc,
 };
 
 pub use libp2p::{
@@ -688,7 +688,7 @@ impl NetworkConfiguration {
 }
 
 /// Network initialization parameters.
-pub struct Params<Client> {
+pub struct Params<Block: BlockT> {
 	/// Assigned role for our node (full, light, ...).
 	pub role: Role,
 
@@ -698,12 +698,12 @@ pub struct Params<Client> {
 	/// Network layer configuration.
 	pub network_config: NetworkConfiguration,
 
-	/// Client that contains the blockchain.
-	pub chain: Arc<Client>,
-
 	/// Legacy name of the protocol to use on the wire. Should be different for each chain.
 	pub protocol_id: ProtocolId,
 
+	/// Genesis hash of the chain
+	pub genesis_hash: Block::Hash,
+
 	/// Fork ID to distinguish protocols of different hard forks. Part of the standard protocol
 	/// name on the wire.
 	pub fork_id: Option<String>,

client/network/src/service.rs

@@ -73,8 +73,7 @@ use parking_lot::Mutex;
 use sc_network_common::ExHashT;
 use sc_peerset::PeersetHandle;
 use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender};
-use sp_blockchain::HeaderBackend;
-use sp_runtime::traits::{Block as BlockT, Zero};
+use sp_runtime::traits::Block as BlockT;
 
 use std::{
 	cmp,
@@ -147,9 +146,7 @@ where
 	/// Returns a `NetworkWorker` that implements `Future` and must be regularly polled in order
 	/// for the network processing to advance. From it, you can extract a `NetworkService` using
 	/// `worker.service()`. The `NetworkService` can be shared through the codebase.
-	pub fn new<Client: HeaderBackend<B> + 'static>(
-		mut params: Params<Client>,
-	) -> Result<Self, Error> {
+	pub fn new<Block: BlockT>(mut params: Params<Block>) -> Result<Self, Error> {
 		// Private and public keys configuration.
 		let local_identity = params.network_config.node_key.clone().into_keypair()?;
 		let local_public = local_identity.public();
@@ -277,13 +274,11 @@ where
 				config.discovery_limit(
 					u64::from(params.network_config.default_peers_set.out_peers) + 15,
 				);
-				let genesis_hash = params
-					.chain
-					.hash(Zero::zero())
-					.ok()
-					.flatten()
-					.expect("Genesis block exists; qed");
-				config.with_kademlia(genesis_hash, params.fork_id.as_deref(), &params.protocol_id);
+				config.with_kademlia(
+					params.genesis_hash,
+					params.fork_id.as_deref(),
+					&params.protocol_id,
+				);
 				config.with_dht_random_walk(params.network_config.enable_dht_random_walk);
 				config.allow_non_globals_in_dht(params.network_config.allow_non_globals_in_dht);
 				config.use_kademlia_disjoint_query_paths(

client/network/test/src/lib.rs

@@ -83,7 +83,7 @@ use sp_core::H256;
 use sp_runtime::{
 	codec::{Decode, Encode},
 	generic::BlockId,
-	traits::{Block as BlockT, Header as HeaderT, NumberFor},
+	traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero},
 	Justification, Justifications,
 };
 use substrate_test_runtime_client::AccountKeyring;
@@ -916,13 +916,15 @@ where
 		let sync_service_import_queue = Box::new(sync_service.clone());
 		let sync_service = Arc::new(sync_service.clone());
 
-		let network = NetworkWorker::new(sc_network::config::Params {
+		let genesis_hash =
+			client.hash(Zero::zero()).ok().flatten().expect("Genesis block exists; qed");
+		let network = NetworkWorker::new::<Block>(sc_network::config::Params {
 			role: if config.is_authority { Role::Authority } else { Role::Full },
 			executor: Box::new(|f| {
 				tokio::spawn(f);
 			}),
 			network_config,
-			chain: client.clone(),
+			genesis_hash,
 			protocol_id,
 			fork_id,
 			metrics_registry: None,

client/network/test/src/service.rs

@@ -34,7 +34,8 @@ use sc_network_sync::{
 	service::network::{NetworkServiceHandle, NetworkServiceProvider},
 	state_request_handler::StateRequestHandler,
 };
-use sp_runtime::traits::Block as BlockT;
+use sp_blockchain::HeaderBackend;
+use sp_runtime::traits::{Block as BlockT, Zero};
 use substrate_test_runtime_client::{
 	runtime::{Block as TestBlock, Hash as TestHash},
 	TestClientBuilder, TestClientBuilderExt as _,
@@ -194,17 +195,19 @@ impl TestNetworkBuilder {
 		)
 		.unwrap();
 		let mut link = self.link.unwrap_or(Box::new(chain_sync_service.clone()));
+		let genesis_hash =
+			client.hash(Zero::zero()).ok().flatten().expect("Genesis block exists; qed");
 		let worker = NetworkWorker::<
 			substrate_test_runtime_client::runtime::Block,
 			substrate_test_runtime_client::runtime::Hash,
-		>::new(config::Params {
+		>::new(config::Params::<substrate_test_runtime_client::runtime::Block> {
 			block_announce_config,
 			role: config::Role::Full,
 			executor: Box::new(|f| {
 				tokio::spawn(f);
 			}),
+			genesis_hash,
 			network_config,
-			chain: client.clone(),
 			protocol_id,
 			fork_id,
 			metrics_registry: None,

client/service/src/builder.rs

@@ -850,7 +850,8 @@ where
 		protocol_config
 	}));
 
-	let mut network_params = sc_network::config::Params {
+	let genesis_hash = client.hash(Zero::zero()).ok().flatten().expect("Genesis block exists; qed");
+	let mut network_params = sc_network::config::Params::<TBl> {
 		role: config.role.clone(),
 		executor: {
 			let spawn_handle = Clone::clone(&spawn_handle);
@@ -859,7 +860,7 @@ where
 			})
 		},
 		network_config: config.network.clone(),
-		chain: client.clone(),
+		genesis_hash,
 		protocol_id: protocol_id.clone(),
 		fork_id: config.chain_spec.fork_id().map(ToOwned::to_owned),
 		metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()),