Skip to content

Commit

Permalink
Remove HeaderBackend requirement from AuthorityDiscovery and NetworkW…
Browse files Browse the repository at this point in the history
…orker (paritytech#13730)

* Remove `HeaderBackend` requirement from `NetworkWorker`

* Remove HeaderBackend from authority-discovery
  • Loading branch information
skunert authored Mar 28, 2023
1 parent ae49625 commit 9b43d8f
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 30 deletions.
4 changes: 4 additions & 0 deletions client/authority-discovery/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
3 changes: 2 additions & 1 deletion client/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
//! See [`Worker`] and [`Service`] for more documentation.

pub use crate::{
error::Error,
service::Service,
worker::{AuthorityDiscovery, NetworkProvider, Role, Worker},
};
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 11 additions & 4 deletions client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand All @@ -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`].
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -44,7 +45,6 @@ use std::{
path::{Path, PathBuf},
pin::Pin,
str::{self, FromStr},
sync::Arc,
};

pub use libp2p::{
Expand Down Expand Up @@ -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,

Expand All @@ -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>,
Expand Down
19 changes: 7 additions & 12 deletions client/network/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions client/network/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions client/network/test/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 _,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()),
Expand Down

0 comments on commit 9b43d8f

Please sign in to comment.