Skip to content

Commit

Permalink
Remove NetworkSpecialization (paritytech#4665)
Browse files Browse the repository at this point in the history
* remove networkspecialization

* Fix most of the fallout

* get all tests compiling

Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
  • Loading branch information
2 people authored and General-Beck committed Mar 4, 2020
1 parent cf3c8a1 commit 39ff1bc
Show file tree
Hide file tree
Showing 20 changed files with 136 additions and 503 deletions.
9 changes: 1 addition & 8 deletions bin/node-template/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use sc_client::LongestChain;
use node_template_runtime::{self, GenesisConfig, opaque::Block, RuntimeApi};
use sc_service::{error::{Error as ServiceError}, AbstractService, Configuration, ServiceBuilder};
use sp_inherents::InherentDataProviders;
use sc_network::{construct_simple_protocol};
use sc_executor::native_executor_instance;
pub use sc_executor::NativeExecutor;
use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
Expand All @@ -19,11 +18,6 @@ native_executor_instance!(
node_template_runtime::native_version,
);

construct_simple_protocol! {
/// Demo protocol attachment for substrate.
pub struct NodeProtocol where Block = Block { }
}

/// Starts a `ServiceBuilder` for a full service.
///
/// Use this macro if you don't actually need the full service, but just the builder in order to
Expand Down Expand Up @@ -95,7 +89,7 @@ pub fn new_full(config: Configuration<GenesisConfig>)
import_setup.take()
.expect("Link Half and Block Import are present for Full Services or setup failed before. qed");

let service = builder.with_network_protocol(|_| Ok(NodeProtocol::new()))?
let service = builder
.with_finality_proof_provider(|client, backend|
Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, client)) as _)
)?
Expand Down Expand Up @@ -228,7 +222,6 @@ pub fn new_light(config: Configuration<GenesisConfig>)

Ok((import_queue, finality_proof_request_builder))
})?
.with_network_protocol(|_| Ok(NodeProtocol::new()))?
.with_finality_proof_provider(|client, backend|
Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, client)) as _)
)?
Expand Down
11 changes: 2 additions & 9 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use sc_service::{
AbstractService, ServiceBuilder, config::Configuration, error::{Error as ServiceError},
};
use sp_inherents::InherentDataProviders;
use sc_network::construct_simple_protocol;

use sc_service::{Service, NetworkStatus};
use sc_client::{Client, LocalCallExecutor};
Expand All @@ -40,11 +39,6 @@ use node_executor::NativeExecutor;
use sc_network::NetworkService;
use sc_offchain::OffchainWorkers;

construct_simple_protocol! {
/// Demo protocol attachment for substrate.
pub struct NodeProtocol where Block = Block { }
}

/// Starts a `ServiceBuilder` for a full service.
///
/// Use this macro if you don't actually need the full service, but just the builder in order to
Expand Down Expand Up @@ -144,7 +138,7 @@ macro_rules! new_full {

let (builder, mut import_setup, inherent_data_providers) = new_full_start!($config);

let service = builder.with_network_protocol(|_| Ok(crate::service::NodeProtocol::new()))?
let service = builder
.with_finality_proof_provider(|client, backend|
Ok(Arc::new(grandpa::FinalityProofProvider::new(backend, client)) as _)
)?
Expand Down Expand Up @@ -283,7 +277,7 @@ pub fn new_full(config: NodeConfiguration)
ConcreteClient,
LongestChain<ConcreteBackend, ConcreteBlock>,
NetworkStatus<ConcreteBlock>,
NetworkService<ConcreteBlock, crate::service::NodeProtocol, <ConcreteBlock as BlockT>::Hash>,
NetworkService<ConcreteBlock, <ConcreteBlock as BlockT>::Hash>,
ConcreteTransactionPool,
OffchainWorkers<
ConcreteClient,
Expand Down Expand Up @@ -348,7 +342,6 @@ pub fn new_light(config: NodeConfiguration)

Ok((import_queue, finality_proof_request_builder))
})?
.with_network_protocol(|_| Ok(NodeProtocol::new()))?
.with_finality_proof_provider(|client, backend|
Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, client)) as _)
)?
Expand Down
4 changes: 1 addition & 3 deletions client/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ use libp2p::Multiaddr;
use log::{debug, error, log_enabled, warn};
use prost::Message;
use sc_client_api::blockchain::HeaderBackend;
use sc_network::specialization::NetworkSpecialization;
use sc_network::{DhtEvent, ExHashT, NetworkStateInfo};
use sp_authority_discovery::{AuthorityDiscoveryApi, AuthorityId, AuthoritySignature, AuthorityPair};
use sp_core::crypto::{key_types, Pair};
Expand Down Expand Up @@ -477,10 +476,9 @@ pub trait NetworkProvider: NetworkStateInfo {
fn get_value(&self, key: &libp2p::kad::record::Key);
}

impl<B, S, H> NetworkProvider for sc_network::NetworkService<B, S, H>
impl<B, H> NetworkProvider for sc_network::NetworkService<B, H>
where
B: BlockT + 'static,
S: NetworkSpecialization<B>,
H: ExHashT,
{
fn set_priority_group(
Expand Down
9 changes: 4 additions & 5 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,10 @@ mod tests {
const SLOT_DURATION: u64 = 1000;

pub struct AuraTestNet {
peers: Vec<Peer<(), DummySpecialization>>,
peers: Vec<Peer<()>>,
}

impl TestNetFactory for AuraTestNet {
type Specialization = DummySpecialization;
type Verifier = AuraVerifier<PeersFullClient, AuthorityPair, ()>;
type PeerData = ();

Expand Down Expand Up @@ -935,15 +934,15 @@ mod tests {
}
}

fn peer(&mut self, i: usize) -> &mut Peer<Self::PeerData, DummySpecialization> {
fn peer(&mut self, i: usize) -> &mut Peer<Self::PeerData> {
&mut self.peers[i]
}

fn peers(&self) -> &Vec<Peer<Self::PeerData, DummySpecialization>> {
fn peers(&self) -> &Vec<Peer<Self::PeerData>> {
&self.peers
}

fn mut_peers<F: FnOnce(&mut Vec<Peer<Self::PeerData, DummySpecialization>>)>(&mut self, closure: F) {
fn mut_peers<F: FnOnce(&mut Vec<Peer<Self::PeerData>>)>(&mut self, closure: F) {
closure(&mut self.peers);
}
}
Expand Down
9 changes: 4 additions & 5 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<B: BlockImport<TestBlock>> BlockImport<TestBlock> for PanickingBlockImport<
}

pub struct BabeTestNet {
peers: Vec<Peer<Option<PeerData>, DummySpecialization>>,
peers: Vec<Peer<Option<PeerData>>>,
}

type TestHeader = <TestBlock as BlockT>::Header;
Expand Down Expand Up @@ -236,7 +236,6 @@ pub struct PeerData {
}

impl TestNetFactory for BabeTestNet {
type Specialization = DummySpecialization;
type Verifier = TestVerifier;
type PeerData = Option<PeerData>;

Expand Down Expand Up @@ -307,17 +306,17 @@ impl TestNetFactory for BabeTestNet {
}
}

fn peer(&mut self, i: usize) -> &mut Peer<Self::PeerData, DummySpecialization> {
fn peer(&mut self, i: usize) -> &mut Peer<Self::PeerData> {
trace!(target: "babe", "Retrieving a peer");
&mut self.peers[i]
}

fn peers(&self) -> &Vec<Peer<Self::PeerData, DummySpecialization>> {
fn peers(&self) -> &Vec<Peer<Self::PeerData>> {
trace!(target: "babe", "Retrieving peers");
&self.peers
}

fn mut_peers<F: FnOnce(&mut Vec<Peer<Self::PeerData, DummySpecialization>>)>(
fn mut_peers<F: FnOnce(&mut Vec<Peer<Self::PeerData>>)>(
&mut self,
closure: F,
) {
Expand Down
3 changes: 1 addition & 2 deletions client/finality-grandpa/src/communication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,8 @@ pub trait Network<Block: BlockT>: GossipNetwork<Block> + Clone + Send + 'static
fn set_sync_fork_request(&self, peers: Vec<sc_network::PeerId>, hash: Block::Hash, number: NumberFor<Block>);
}

impl<B, S, H> Network<B> for Arc<NetworkService<B, S, H>> where
impl<B, H> Network<B> for Arc<NetworkService<B, H>> where
B: BlockT,
S: sc_network::specialization::NetworkSpecialization<B>,
H: sc_network::ExHashT,
{
fn set_sync_fork_request(&self, peers: Vec<sc_network::PeerId>, hash: B::Hash, number: NumberFor<B>) {
Expand Down
5 changes: 2 additions & 3 deletions client/finality-grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::*;
use environment::HasVoted;
use sc_network_test::{
Block, DummySpecialization, Hash, TestNetFactory, BlockImportAdapter, Peer,
Block, Hash, TestNetFactory, BlockImportAdapter, Peer,
PeersClient, PassThroughVerifier,
};
use sc_network::config::{ProtocolConfig, Roles, BoxFinalityProofRequestBuilder};
Expand Down Expand Up @@ -68,7 +68,7 @@ type PeerData =
>
>
>;
type GrandpaPeer = Peer<PeerData, DummySpecialization>;
type GrandpaPeer = Peer<PeerData>;

struct GrandpaTestNet {
peers: Vec<GrandpaPeer>,
Expand All @@ -90,7 +90,6 @@ impl GrandpaTestNet {
}

impl TestNetFactory for GrandpaTestNet {
type Specialization = DummySpecialization;
type Verifier = PassThroughVerifier;
type PeerData = PeerData;

Expand Down
4 changes: 2 additions & 2 deletions client/network-gossip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub use self::state_machine::TopicNotification;
pub use self::validator::{DiscardAll, MessageIntent, Validator, ValidatorContext, ValidationResult};

use futures::prelude::*;
use sc_network::{specialization::NetworkSpecialization, Event, ExHashT, NetworkService, PeerId, ReputationChange};
use sc_network::{Event, ExHashT, NetworkService, PeerId, ReputationChange};
use sp_runtime::{traits::Block as BlockT, ConsensusEngineId};
use std::{borrow::Cow, pin::Pin, sync::Arc};

Expand Down Expand Up @@ -97,7 +97,7 @@ pub trait Network<B: BlockT> {
fn announce(&self, block: B::Hash, associated_data: Vec<u8>);
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Network<B> for Arc<NetworkService<B, S, H>> {
impl<B: BlockT, H: ExHashT> Network<B> for Arc<NetworkService<B, H>> {
fn event_stream(&self) -> Pin<Box<dyn Stream<Item = Event> + Send>> {
Box::pin(NetworkService::event_stream(self))
}
Expand Down
33 changes: 16 additions & 17 deletions client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@

use crate::{
debug_info, discovery::DiscoveryBehaviour, discovery::DiscoveryOut, DiscoveryNetBehaviour,
Event, protocol::event::DhtEvent
Event, protocol::event::DhtEvent, ExHashT,
};
use crate::{ExHashT, specialization::NetworkSpecialization};
use crate::protocol::{self, light_client_handler, CustomMessageOutcome, Protocol};
use libp2p::NetworkBehaviour;
use libp2p::core::{Multiaddr, PeerId, PublicKey};
Expand All @@ -33,9 +32,9 @@ use void;
/// General behaviour of the network. Combines all protocols together.
#[derive(NetworkBehaviour)]
#[behaviour(out_event = "BehaviourOut<B>", poll_method = "poll")]
pub struct Behaviour<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> {
pub struct Behaviour<B: BlockT, H: ExHashT> {
/// All the substrate-specific protocols.
substrate: Protocol<B, S, H>,
substrate: Protocol<B, H>,
/// Periodically pings and identifies the nodes we are connected to, and store information in a
/// cache.
debug_info: debug_info::DebugInfoBehaviour,
Expand All @@ -58,10 +57,10 @@ pub enum BehaviourOut<B: BlockT> {
Event(Event),
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> Behaviour<B, H> {
/// Builds a new `Behaviour`.
pub async fn new(
substrate: Protocol<B, S, H>,
substrate: Protocol<B, H>,
user_agent: String,
local_public_key: PublicKey,
known_addresses: Vec<(PeerId, Multiaddr)>,
Expand Down Expand Up @@ -107,12 +106,12 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
}

/// Returns a shared reference to the user protocol.
pub fn user_protocol(&self) -> &Protocol<B, S, H> {
pub fn user_protocol(&self) -> &Protocol<B, H> {
&self.substrate
}

/// Returns a mutable reference to the user protocol.
pub fn user_protocol_mut(&mut self) -> &mut Protocol<B, S, H> {
pub fn user_protocol_mut(&mut self) -> &mut Protocol<B, H> {
&mut self.substrate
}

Expand All @@ -133,15 +132,15 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
}
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventProcess<void::Void> for
Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<void::Void> for
Behaviour<B, H> {
fn inject_event(&mut self, event: void::Void) {
void::unreachable(event)
}
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventProcess<CustomMessageOutcome<B>> for
Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<CustomMessageOutcome<B>> for
Behaviour<B, H> {
fn inject_event(&mut self, event: CustomMessageOutcome<B>) {
match event {
CustomMessageOutcome::BlockImport(origin, blocks) =>
Expand Down Expand Up @@ -174,8 +173,8 @@ Behaviour<B, S, H> {
}
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventProcess<debug_info::DebugInfoEvent>
for Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<debug_info::DebugInfoEvent>
for Behaviour<B, H> {
fn inject_event(&mut self, event: debug_info::DebugInfoEvent) {
let debug_info::DebugInfoEvent::Identified { peer_id, mut info } = event;
if info.listen_addrs.len() > 30 {
Expand All @@ -192,8 +191,8 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventPr
}
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventProcess<DiscoveryOut>
for Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<DiscoveryOut>
for Behaviour<B, H> {
fn inject_event(&mut self, out: DiscoveryOut) {
match out {
DiscoveryOut::UnroutablePeer(_peer_id) => {
Expand Down Expand Up @@ -221,7 +220,7 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> NetworkBehaviourEventPr
}
}

impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Behaviour<B, S, H> {
impl<B: BlockT, H: ExHashT> Behaviour<B, H> {
fn poll<TEv>(&mut self, _: &mut Context, _: &mut impl PollParameters) -> Poll<NetworkBehaviourAction<TEv, BehaviourOut<B>>> {
if !self.events.is_empty() {
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(self.events.remove(0)))
Expand Down
5 changes: 1 addition & 4 deletions client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use std::{error::Error, fs, io::{self, Write}, net::Ipv4Addr, path::{Path, PathB
use zeroize::Zeroize;

/// Network initialization parameters.
pub struct Params<B: BlockT, S, H: ExHashT> {
pub struct Params<B: BlockT, H: ExHashT> {
/// Assigned roles for our node (full, light, ...).
pub roles: Roles,

Expand Down Expand Up @@ -88,9 +88,6 @@ pub struct Params<B: BlockT, S, H: ExHashT> {
/// valid.
pub import_queue: Box<dyn ImportQueue<B>>,

/// Customization of the network. Use this to plug additional networking capabilities.
pub specialization: S,

/// Type to check incoming block announcements.
pub block_announce_validator: Box<dyn BlockAnnounceValidator<B> + Send>,
}
Expand Down
10 changes: 1 addition & 9 deletions client/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,6 @@
//! - Light-client requests. When a light client requires information, a random node we have a
//! substream open with is chosen, and the information is requested from it.
//! - Gossiping. Used for example by grandpa.
//! - Network specialization. The network protocol can be specialized through a template parameter
//! of the network service. This specialization is free to send and receive messages with the
//! remote. This is meant to be used by the chain that is being built on top of Substrate
//! (eg. Polkadot).
//!
//! It is intended that in the future each of these components gets more isolated, so that they
//! are free to open and close their own substreams, and so that syncing and light client requests
Expand Down Expand Up @@ -180,7 +176,7 @@ pub mod error;
pub mod network_state;

pub use service::{NetworkService, NetworkStateInfo, NetworkWorker, ExHashT, ReportHandle};
pub use protocol::{PeerInfo, Context, specialization};
pub use protocol::PeerInfo;
pub use protocol::event::{Event, DhtEvent};
pub use protocol::sync::SyncState;
pub use libp2p::{Multiaddr, PeerId};
Expand All @@ -196,10 +192,6 @@ pub use protocol::message::Status as StatusMessage;

pub use sc_peerset::ReputationChange;

// Used by the `construct_simple_protocol!` macro.
#[doc(hidden)]
pub use sp_runtime::traits::Block as BlockT;

/// Extension trait for `NetworkBehaviour` that also accepts discovering nodes.
trait DiscoveryNetBehaviour {
/// Notify the protocol that we have learned about the existence of nodes.
Expand Down
Loading

0 comments on commit 39ff1bc

Please sign in to comment.