Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove HeaderBackend requirement from AuthorityDiscovery and NetworkWorker #13730

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Can this be generic over Hash only?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaakkk 🤣 Late again. Just a silly nit anyway

/// 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