Cleanup the exports of sc-network (#4983)

* Cleanup the exports of sc-network

* Fix RPC tests

* Revert moving the NetworkStateInfo trait

* Fix GrandPa tests

substrate/client/finality-grandpa/src/finality_proof.rs

@@ -154,7 +154,7 @@ impl<B, Block: BlockT> FinalityProofProvider<B, Block>
 	}
 }
 
-impl<B, Block> sc_network::FinalityProofProvider<Block> for FinalityProofProvider<B, Block>
+impl<B, Block> sc_network::config::FinalityProofProvider<Block> for FinalityProofProvider<B, Block>
 	where
 		Block: BlockT,
 		NumberFor<Block>: BlockNumberOps,

substrate/client/finality-grandpa/src/tests.rs

@@ -171,7 +171,7 @@ impl TestNetFactory for GrandpaTestNet {
 	fn make_finality_proof_provider(
 		&self,
 		client: PeersClient
-	) -> Option<Arc<dyn sc_network::FinalityProofProvider<Block>>> {
+	) -> Option<Arc<dyn sc_network::config::FinalityProofProvider<Block>>> {
 		match client {
 			PeersClient::Full(_, ref backend)  => {
 				let authorities_provider = Arc::new(self.test_config.clone());

substrate/client/network/src/config.rs

@@ -19,12 +19,18 @@
 //! The [`Params`] struct is the struct that must be passed in order to initialize the networking.
 //! See the documentation of [`Params`].
 
-pub use crate::protocol::ProtocolConfig;
+pub use crate::chain::{Client, FinalityProofProvider};
+pub use crate::on_demand_layer::OnDemand;
+pub use crate::service::TransactionPool;
 pub use libp2p::{identity, core::PublicKey, wasm_ext::ExtTransport, build_multiaddr};
 
-use crate::chain::{Client, FinalityProofProvider};
-use crate::on_demand_layer::OnDemand;
-use crate::service::{ExHashT, TransactionPool};
+// Note: this re-export shouldn't be part of the public API of the crate and will be removed in
+// the future.
+#[doc(hidden)]
+pub use crate::protocol::ProtocolConfig;
+
+use crate::service::ExHashT;
+
 use bitflags::bitflags;
 use sp_consensus::{block_validation::BlockAnnounceValidator, import_queue::ImportQueue};
 use sp_runtime::traits::{Block as BlockT};

substrate/client/network/src/lib.rs

@@ -78,12 +78,8 @@
 //! - DNS for addresses of the form `/dns4/example.com/tcp/5` or `/dns4/example.com/tcp/5/ws`. A
 //! node's address can contain a domain name.
 //!
-//! The following encryption protocols are supported:
-//!
-//! - [Secio](https://github.com/libp2p/specs/tree/master/secio). A TLS-1.2-like protocol but
-//! without certificates. Support for secio will likely be deprecated in the far future.
-//! - [Noise](https://noiseprotocol.org/). Support for noise is very experimental. The details are
-//! very blurry and may change at any moment.
+//! On top of the base-layer protocol, the [Noise](https://noiseprotocol.org/) protocol is
+//! negotiated and applied. The exact handshake protocol is experimental and is subject to change.
 //!
 //! The following multiplexing protocols are supported:
 //!
@@ -160,7 +156,8 @@
 //!
 //! After the `NetworkWorker` has been created, the important things to do are:
 //!
-//! - Calling `NetworkWorker::poll` in order to advance the network.
+//! - Calling `NetworkWorker::poll` in order to advance the network. This can be done by
+//! dispatching a background task with the [`NetworkWorker`].
 //! - Calling `on_block_import` whenever a block is added to the client.
 //! - Calling `on_block_finalized` whenever a block is finalized.
 //! - Calling `trigger_repropagate` when a transaction is added to the pool.
@@ -180,34 +177,31 @@ mod utils;
 
 pub mod config;
 pub mod error;
+pub mod network_state;
 
-pub use chain::{Client as ClientHandle, FinalityProofProvider};
-pub use service::{
-	NetworkService, NetworkWorker, TransactionPool, ExHashT, ReportHandle,
-	NetworkStateInfo,
-};
-pub use protocol::{PeerInfo, Context, ProtocolConfig, message, specialization};
+pub use service::{NetworkService, NetworkStateInfo, NetworkWorker, ExHashT, ReportHandle};
+pub use protocol::{PeerInfo, Context, specialization};
 pub use protocol::event::{Event, DhtEvent};
 pub use protocol::sync::SyncState;
 pub use libp2p::{Multiaddr, PeerId};
 #[doc(inline)]
 pub use libp2p::multiaddr;
 
-pub use message::{generic as generic_message, RequestId, Status as StatusMessage};
-pub use on_demand_layer::{OnDemand, RemoteResponse};
+// Note: these re-exports shouldn't be part of the public API of the crate and will be removed in
+// the future.
+#[doc(hidden)]
+pub use protocol::message;
+#[doc(hidden)]
+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;
 
-use libp2p::core::ConnectedPoint;
-use serde::{Deserialize, Serialize};
-use slog_derive::SerdeValue;
-use std::{collections::{HashMap, HashSet}, time::Duration};
-
 /// Extension trait for `NetworkBehaviour` that also accepts discovering nodes.
-pub trait DiscoveryNetBehaviour {
+trait DiscoveryNetBehaviour {
 	/// Notify the protocol that we have learned about the existence of nodes.
 	///
 	/// Can (or most likely will) be called multiple times with the same `PeerId`s.
@@ -216,90 +210,3 @@ pub trait DiscoveryNetBehaviour {
 	/// system, or remove nodes that will fail to reach.
 	fn add_discovered_nodes(&mut self, nodes: impl Iterator<Item = PeerId>);
 }
-
-/// Returns general information about the networking.
-///
-/// Meant for general diagnostic purposes.
-///
-/// **Warning**: This API is not stable.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, SerdeValue)]
-#[serde(rename_all = "camelCase")]
-pub struct NetworkState {
-	/// PeerId of the local node.
-	pub peer_id: String,
-	/// List of addresses the node is currently listening on.
-	pub listened_addresses: HashSet<Multiaddr>,
-	/// List of addresses the node knows it can be reached as.
-	pub external_addresses: HashSet<Multiaddr>,
-	/// List of node we're connected to.
-	pub connected_peers: HashMap<String, NetworkStatePeer>,
-	/// List of node that we know of but that we're not connected to.
-	pub not_connected_peers: HashMap<String, NetworkStateNotConnectedPeer>,
-	/// Downloaded bytes per second averaged over the past few seconds.
-	pub average_download_per_sec: u64,
-	/// Uploaded bytes per second averaged over the past few seconds.
-	pub average_upload_per_sec: u64,
-	/// State of the peerset manager.
-	pub peerset: serde_json::Value,
-}
-
-/// Part of the `NetworkState` struct. Unstable.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct NetworkStatePeer {
-	/// How we are connected to the node.
-	pub endpoint: NetworkStatePeerEndpoint,
-	/// Node information, as provided by the node itself. Can be empty if not known yet.
-	pub version_string: Option<String>,
-	/// Latest ping duration with this node.
-	pub latest_ping_time: Option<Duration>,
-	/// If true, the peer is "enabled", which means that we try to open Substrate-related protocols
-	/// with this peer. If false, we stick to Kademlia and/or other network-only protocols.
-	pub enabled: bool,
-	/// If true, the peer is "open", which means that we have a Substrate-related protocol
-	/// with this peer.
-	pub open: bool,
-	/// List of addresses known for this node.
-	pub known_addresses: HashSet<Multiaddr>,
-}
-
-/// Part of the `NetworkState` struct. Unstable.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct NetworkStateNotConnectedPeer {
-	/// List of addresses known for this node.
-	pub known_addresses: HashSet<Multiaddr>,
-	/// Node information, as provided by the node itself, if we were ever connected to this node.
-	pub version_string: Option<String>,
-	/// Latest ping duration with this node, if we were ever connected to this node.
-	pub latest_ping_time: Option<Duration>,
-}
-
-/// Part of the `NetworkState` struct. Unstable.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub enum NetworkStatePeerEndpoint {
-	/// We are dialing the given address.
-	Dialing(Multiaddr),
-	/// We are listening.
-	Listening {
-		/// Local address of the connection.
-		local_addr: Multiaddr,
-		/// Address data is sent back to.
-		send_back_addr: Multiaddr,
-	},
-}
-
-impl From<ConnectedPoint> for NetworkStatePeerEndpoint {
-	fn from(endpoint: ConnectedPoint) -> Self {
-		match endpoint {
-			ConnectedPoint::Dialer { address } =>
-				NetworkStatePeerEndpoint::Dialing(address),
-			ConnectedPoint::Listener { local_addr, send_back_addr } =>
-				NetworkStatePeerEndpoint::Listening {
-					local_addr,
-					send_back_addr
-				}
-		}
-	}
-}

substrate/client/network/src/network_state.rs

@@ -0,0 +1,111 @@
+// Copyright 2017-2020 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Information about the networking, for diagnostic purposes.
+//!
+//! **Warning**: These APIs are not stable.
+
+use libp2p::{core::ConnectedPoint, Multiaddr};
+use serde::{Deserialize, Serialize};
+use slog_derive::SerdeValue;
+use std::{collections::{HashMap, HashSet}, time::Duration};
+
+/// Returns general information about the networking.
+///
+/// Meant for general diagnostic purposes.
+///
+/// **Warning**: This API is not stable.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, SerdeValue)]
+#[serde(rename_all = "camelCase")]
+pub struct NetworkState {
+	/// PeerId of the local node.
+	pub peer_id: String,
+	/// List of addresses the node is currently listening on.
+	pub listened_addresses: HashSet<Multiaddr>,
+	/// List of addresses the node knows it can be reached as.
+	pub external_addresses: HashSet<Multiaddr>,
+	/// List of node we're connected to.
+	pub connected_peers: HashMap<String, Peer>,
+	/// List of node that we know of but that we're not connected to.
+	pub not_connected_peers: HashMap<String, NotConnectedPeer>,
+	/// Downloaded bytes per second averaged over the past few seconds.
+	pub average_download_per_sec: u64,
+	/// Uploaded bytes per second averaged over the past few seconds.
+	pub average_upload_per_sec: u64,
+	/// State of the peerset manager.
+	pub peerset: serde_json::Value,
+}
+
+/// Part of the `NetworkState` struct. Unstable.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Peer {
+	/// How we are connected to the node.
+	pub endpoint: PeerEndpoint,
+	/// Node information, as provided by the node itself. Can be empty if not known yet.
+	pub version_string: Option<String>,
+	/// Latest ping duration with this node.
+	pub latest_ping_time: Option<Duration>,
+	/// If true, the peer is "enabled", which means that we try to open Substrate-related protocols
+	/// with this peer. If false, we stick to Kademlia and/or other network-only protocols.
+	pub enabled: bool,
+	/// If true, the peer is "open", which means that we have a Substrate-related protocol
+	/// with this peer.
+	pub open: bool,
+	/// List of addresses known for this node.
+	pub known_addresses: HashSet<Multiaddr>,
+}
+
+/// Part of the `NetworkState` struct. Unstable.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct NotConnectedPeer {
+	/// List of addresses known for this node.
+	pub known_addresses: HashSet<Multiaddr>,
+	/// Node information, as provided by the node itself, if we were ever connected to this node.
+	pub version_string: Option<String>,
+	/// Latest ping duration with this node, if we were ever connected to this node.
+	pub latest_ping_time: Option<Duration>,
+}
+
+/// Part of the `NetworkState` struct. Unstable.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub enum PeerEndpoint {
+	/// We are dialing the given address.
+	Dialing(Multiaddr),
+	/// We are listening.
+	Listening {
+		/// Local address of the connection.
+		local_addr: Multiaddr,
+		/// Address data is sent back to.
+		send_back_addr: Multiaddr,
+	},
+}
+
+impl From<ConnectedPoint> for PeerEndpoint {
+	fn from(endpoint: ConnectedPoint) -> Self {
+		match endpoint {
+			ConnectedPoint::Dialer { address } =>
+				PeerEndpoint::Dialing(address),
+			ConnectedPoint::Listener { local_addr, send_back_addr } =>
+				PeerEndpoint::Listening {
+					local_addr,
+					send_back_addr
+				}
+		}
+	}
+}

substrate/client/network/src/protocol/light_dispatch.rs

@@ -30,7 +30,7 @@ use sp_blockchain::Error as ClientError;
 use sc_client_api::{FetchChecker, RemoteHeaderRequest,
 	RemoteCallRequest, RemoteReadRequest, RemoteChangesRequest, ChangesProof,
 	RemoteReadChildRequest, RemoteBodyRequest, StorageProof};
-use crate::message::{self, BlockAttributes, Direction, FromBlock, RequestId};
+use crate::protocol::message::{self, BlockAttributes, Direction, FromBlock, RequestId};
 use libp2p::PeerId;
 use crate::config::Roles;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};

substrate/client/network/src/protocol/specialization.rs

@@ -28,7 +28,7 @@ pub trait NetworkSpecialization<B: BlockT>: Send + Sync + 'static {
 	fn status(&self) -> Vec<u8>;
 
 	/// Called when a peer successfully handshakes.
-	fn on_connect(&mut self, ctx: &mut dyn Context<B>, who: PeerId, status: crate::message::Status<B>);
+	fn on_connect(&mut self, ctx: &mut dyn Context<B>, who: PeerId, status: crate::protocol::message::Status<B>);
 
 	/// Called when a peer is disconnected. If the peer ID is unknown, it should be ignored.
 	fn on_disconnect(&mut self, ctx: &mut dyn Context<B>, who: PeerId);
@@ -62,7 +62,7 @@ impl<B: BlockT> NetworkSpecialization<B> for DummySpecialization {
 		&mut self,
 		_ctx: &mut dyn Context<B>,
 		_peer_id: PeerId,
-		_status: crate::message::Status<B>
+		_status: crate::protocol::message::Status<B>
 	) {}
 
 	fn on_disconnect(&mut self, _ctx: &mut dyn Context<B>, _peer_id: PeerId) {}

substrate/client/network/src/protocol/sync.rs

@@ -35,7 +35,7 @@ use sp_consensus::{BlockOrigin, BlockStatus,
 };
 use crate::{
 	config::{Roles, BoxFinalityProofRequestBuilder},
-	message::{self, generic::FinalityProofRequest, BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse,
+	protocol::message::{self, generic::FinalityProofRequest, BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse,
 	FinalityProofResponse},
 };
 use either::Either;

substrate/client/network/src/protocol/sync/blocks.rs

@@ -22,7 +22,7 @@ use std::collections::hash_map::Entry;
 use log::trace;
 use libp2p::PeerId;
 use sp_runtime::traits::{Block as BlockT, NumberFor, One};
-use crate::message;
+use crate::protocol::message;
 
 /// Block data with origin.
 #[derive(Debug, Clone, PartialEq, Eq)]

substrate/client/network/src/service.rs

@@ -41,10 +41,10 @@ use sc_peerset::PeersetHandle;
 use sp_runtime::{traits::{Block as BlockT, NumberFor}, ConsensusEngineId};
 
 use crate::{behaviour::{Behaviour, BehaviourOut}, config::{parse_str_addr, parse_addr}};
-use crate::{NetworkState, NetworkStateNotConnectedPeer, NetworkStatePeer};
 use crate::{transport, config::NonReservedPeerMode, ReputationChange};
 use crate::config::{Params, TransportConfig};
 use crate::error::Error;
+use crate::network_state::{NetworkState, NotConnectedPeer as NetworkStateNotConnectedPeer, Peer as NetworkStatePeer};
 use crate::protocol::{self, Protocol, Context, PeerInfo};
 use crate::protocol::{event::Event, light_dispatch::{AlwaysBadChecker, RequestData}};
 use crate::protocol::specialization::NetworkSpecialization;

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

@@ -25,7 +25,7 @@ use std::{collections::HashMap, pin::Pin, sync::Arc, marker::PhantomData};
 
 use libp2p::build_multiaddr;
 use log::trace;
-use sc_network::FinalityProofProvider;
+use sc_network::config::FinalityProofProvider;
 use sp_blockchain::{
 	Result as ClientResult, well_known_cache_keys::{self, Id as CacheKeyId}, Info as BlockchainInfo,
 };
@@ -52,11 +52,11 @@ use sc_network::config::{NetworkConfiguration, TransportConfig, BoxFinalityProof
 use libp2p::PeerId;
 use parking_lot::Mutex;
 use sp_core::H256;
-use sc_network::ProtocolConfig;
+use sc_network::config::ProtocolConfig;
 use sp_runtime::generic::{BlockId, OpaqueDigestItemId};
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};
 use sp_runtime::Justification;
-use sc_network::TransactionPool;
+use sc_network::config::TransactionPool;
 use sc_network::specialization::NetworkSpecialization;
 use substrate_test_runtime_client::{self, AccountKeyring};
 

substrate/client/peerset/src/lib.rs

@@ -46,7 +46,7 @@ enum Action {
 	RemoveFromPriorityGroup(String, PeerId),
 }
 
-/// Shared handle to the peer set manager (PSM). Distributed around the code.
+/// Description of a reputation adjustment for a node.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct ReputationChange {
 	/// Reputation delta.

substrate/client/rpc/src/system/tests.rs

@@ -69,7 +69,7 @@ fn api<T: Into<Option<Status>>>(sync: T) -> System<Block> {
 					let _ = sender.send(peers);
 				}
 				Request::NetworkState(sender) => {
-					let _ = sender.send(serde_json::to_value(&sc_network::NetworkState {
+					let _ = sender.send(serde_json::to_value(&sc_network::network_state::NetworkState {
 						peer_id: String::new(),
 						listened_addresses: Default::default(),
 						external_addresses: Default::default(),
@@ -223,8 +223,8 @@ fn system_peers() {
 fn system_network_state() {
 	let res = wait_receiver(api(None).system_network_state());
 	assert_eq!(
-		serde_json::from_value::<sc_network::NetworkState>(res).unwrap(),
-		sc_network::NetworkState {
+		serde_json::from_value::<sc_network::network_state::NetworkState>(res).unwrap(),
+		sc_network::network_state::NetworkState {
 			peer_id: String::new(),
 			listened_addresses: Default::default(),
 			external_addresses: Default::default(),

substrate/client/service/src/builder.rs

@@ -34,8 +34,8 @@ use futures::{
 };
 use sc_keystore::{Store as Keystore};
 use log::{info, warn, error};
-use sc_network::{FinalityProofProvider, OnDemand, NetworkService, NetworkStateInfo};
-use sc_network::{config::BoxFinalityProofRequestBuilder, specialization::NetworkSpecialization};
+use sc_network::config::{FinalityProofProvider, OnDemand, BoxFinalityProofRequestBuilder};
+use sc_network::{NetworkService, NetworkStateInfo, specialization::NetworkSpecialization};
 use parking_lot::{Mutex, RwLock};
 use sp_runtime::generic::BlockId;
 use sp_runtime::traits::{
@@ -369,7 +369,7 @@ where TGen: RuntimeGenesis, TCSExt: Extension {
 				executor.clone(),
 			),
 		);
-		let fetcher = Arc::new(sc_network::OnDemand::new(fetch_checker));
+		let fetcher = Arc::new(sc_network::config::OnDemand::new(fetch_checker));
 		let backend = sc_client::light::new_light_backend(light_blockchain);
 		let remote_blockchain = backend.remote_blockchain();
 		let client = Arc::new(sc_client::light::new_light(

substrate/client/service/src/lib.rs

@@ -46,7 +46,7 @@ use futures::{
 	task::{Spawn, FutureObj, SpawnError},
 };
 use sc_network::{
-	NetworkService, NetworkState, specialization::NetworkSpecialization,
+	NetworkService, network_state::NetworkState, specialization::NetworkSpecialization,
 	PeerId, ReportHandle,
 };
 use log::{log, warn, debug, error, Level};
@@ -71,7 +71,7 @@ pub use sc_executor::NativeExecutionDispatch;
 #[doc(hidden)]
 pub use std::{ops::Deref, result::Result, sync::Arc};
 #[doc(hidden)]
-pub use sc_network::{FinalityProofProvider, OnDemand, config::BoxFinalityProofRequestBuilder};
+pub use sc_network::config::{FinalityProofProvider, OnDemand, BoxFinalityProofRequestBuilder};
 
 const DEFAULT_PROTOCOL_ID: &str = "sup";
 
@@ -634,10 +634,10 @@ where
 		.collect()
 }
 
-impl<B, H, C, Pool, E> sc_network::TransactionPool<H, B> for
+impl<B, H, C, Pool, E> sc_network::config::TransactionPool<H, B> for
 	TransactionPoolAdapter<C, Pool>
 where
-	C: sc_network::ClientHandle<B> + Send + Sync,
+	C: sc_network::config::Client<B> + Send + Sync,
 	Pool: 'static + TransactionPool<Block=B, Hash=H, Error=E>,
 	B: BlockT,
 	H: std::hash::Hash + Eq + sp_runtime::traits::Member + sp_runtime::traits::MaybeSerialize,