The network configuration now makes more sense (#2706)

* The network configuration now makes more sense

* Reexport ProtocolConfig

substrate/core/network/src/config.rs

@@ -16,9 +16,11 @@
 
 //! Configuration for the networking layer of Substrate.
 
-pub use network_libp2p::{NonReservedPeerMode, NetworkConfiguration, NodeKeyConfig, Secret};
+pub use crate::protocol::ProtocolConfig;
+pub use network_libp2p::{NonReservedPeerMode, NetworkConfiguration, NodeKeyConfig, ProtocolId, Secret};
 
 use bitflags::bitflags;
+use consensus::import_queue::ImportQueue;
 use crate::chain::{Client, FinalityProofProvider};
 use parity_codec;
 use crate::on_demand_layer::OnDemand;
@@ -28,8 +30,8 @@ use std::sync::Arc;
 
 /// Service initialization parameters.
 pub struct Params<B: BlockT, S, H: ExHashT> {
-	/// Configuration.
-	pub config: ProtocolConfig,
+	/// Assigned roles for our node.
+	pub roles: Roles,
 	/// Network layer configuration.
 	pub network_config: NetworkConfiguration,
 	/// Substrate relay chain access point.
@@ -40,25 +42,14 @@ pub struct Params<B: BlockT, S, H: ExHashT> {
 	pub on_demand: Option<Arc<OnDemand<B>>>,
 	/// Transaction pool.
 	pub transaction_pool: Arc<dyn TransactionPool<H, B>>,
+	/// Name of the protocol to use on the wire. Should be different for each chain.
+	pub protocol_id: ProtocolId,
+	/// Import queue to use.
+	pub import_queue: Box<ImportQueue<B>>,
 	/// Protocol specialization.
 	pub specialization: S,
 }
 
-/// Configuration for the Substrate-specific part of the networking layer.
-#[derive(Clone)]
-pub struct ProtocolConfig {
-	/// Assigned roles.
-	pub roles: Roles,
-}
-
-impl Default for ProtocolConfig {
-	fn default() -> ProtocolConfig {
-		ProtocolConfig {
-			roles: Roles::FULL,
-		}
-	}
-}
-
 bitflags! {
 	/// Bitmask of the roles that a node fulfills.
 	pub struct Roles: u8 {

substrate/core/network/src/protocol.rs

@@ -34,7 +34,7 @@ use crate::on_demand::{OnDemandCore, OnDemandNetwork, RequestData};
 use crate::specialization::NetworkSpecialization;
 use crate::sync::{ChainSync, Context as SyncContext, Status as SyncStatus, SyncState};
 use crate::service::{TransactionPool, ExHashT};
-use crate::config::{ProtocolConfig, Roles};
+use crate::config::Roles;
 use rustc_hex::ToHex;
 use std::collections::{BTreeMap, HashMap};
 use std::sync::Arc;
@@ -273,6 +273,21 @@ struct ContextData<B: BlockT, H: ExHashT> {
 	pub chain: Arc<Client<B>>,
 }
 
+/// Configuration for the Substrate-specific part of the networking layer.
+#[derive(Clone)]
+pub struct ProtocolConfig {
+	/// Assigned roles.
+	pub roles: Roles,
+}
+
+impl Default for ProtocolConfig {
+	fn default() -> ProtocolConfig {
+		ProtocolConfig {
+			roles: Roles::FULL,
+		}
+	}
+}
+
 impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> {
 	/// Create a new instance.
 	pub fn new(

substrate/core/network/src/service.rs

@@ -22,9 +22,8 @@ use std::{io, thread, time::Duration};
 use log::{warn, debug, error, info};
 use futures::{Async, Future, Stream, sync::oneshot, sync::mpsc};
 use parking_lot::{Mutex, RwLock};
-use network_libp2p::{ProtocolId, NetworkConfiguration};
 use network_libp2p::{start_service, parse_str_addr, Service as NetworkService, ServiceEvent as NetworkServiceEvent};
-use network_libp2p::{RegisteredProtocol, NetworkState};
+use network_libp2p::{NetworkConfiguration, RegisteredProtocol, NetworkState};
 use peerset::PeersetHandle;
 use consensus::import_queue::{ImportQueue, Link, SharedFinalityProofRequestBuilder};
 use runtime_primitives::{traits::{Block as BlockT, NumberFor}, ConsensusEngineId};
@@ -205,8 +204,6 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>> Service<B, S> {
 	/// Creates and register protocol with the network service
 	pub fn new<H: ExHashT>(
 		params: Params<B, S, H>,
-		protocol_id: ProtocolId,
-		import_queue: Box<ImportQueue<B>>,
 	) -> Result<Arc<Service<B, S>>, Error> {
 		let (network_chan, network_port) = mpsc::unbounded();
 		let (protocol_sender, protocol_rx) = mpsc::unbounded();
@@ -217,27 +214,27 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>> Service<B, S> {
 			protocol_sender: protocol_sender.clone(),
 			network_sender: network_chan.clone(),
 		};
-		import_queue.start(Box::new(link))?;
+		params.import_queue.start(Box::new(link))?;
 
 		// Start in off-line mode, since we're not connected to any nodes yet.
 		let is_offline = Arc::new(AtomicBool::new(true));
 		let is_major_syncing = Arc::new(AtomicBool::new(false));
 		let peers: Arc<RwLock<HashMap<PeerId, ConnectedPeer<B>>>> = Arc::new(Default::default());
 		let protocol = Protocol::new(
-			params.config,
+			protocol::ProtocolConfig { roles: params.roles },
 			params.chain,
 			params.on_demand.as_ref().map(|od| od.checker().clone())
 				.unwrap_or(Arc::new(AlwaysBadChecker)),
 			params.specialization,
 		)?;
 		let versions: Vec<_> = ((protocol::MIN_VERSION as u8)..=(protocol::CURRENT_VERSION as u8)).collect();
-		let registered = RegisteredProtocol::new(protocol_id, &versions);
+		let registered = RegisteredProtocol::new(params.protocol_id, &versions);
 		let (thread, network, peerset) = start_thread(
 			is_offline.clone(),
 			is_major_syncing.clone(),
 			protocol,
 			peers.clone(),
-			import_queue,
+			params.import_queue,
 			params.transaction_pool,
 			params.finality_proof_provider,
 			network_port,
@@ -248,7 +245,7 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>> Service<B, S> {
 			params.on_demand.and_then(|od| od.extract_receiver()),
 		)?;
 
-		let service = Arc::new(Service {
+		Ok(Arc::new(Service {
 			status_sinks,
 			is_offline,
 			is_major_syncing,
@@ -258,9 +255,7 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>> Service<B, S> {
 			network,
 			protocol_sender,
 			bg_thread: Some(thread),
-		});
-
-		Ok(service)
+		}))
 	}
 
 	/// Returns the downloaded bytes per second averaged over the past few seconds.

substrate/core/network/src/test/mod.rs

@@ -31,7 +31,7 @@ use client::{self, ClientInfo, BlockchainEvents, FinalityNotifications};
 use client::{in_mem::Backend as InMemoryBackend, error::Result as ClientResult};
 use client::block_builder::BlockBuilder;
 use client::backend::AuxStore;
-use crate::config::{ProtocolConfig, Roles};
+use crate::config::Roles;
 use consensus::import_queue::{BasicQueue, ImportQueue, IncomingBlock};
 use consensus::import_queue::{
 	Link, SharedBlockImport, SharedJustificationImport, Verifier, SharedFinalityProofImport,
@@ -45,7 +45,7 @@ use crate::message::Message;
 use network_libp2p::PeerId;
 use parking_lot::{Mutex, RwLock};
 use primitives::{H256, sr25519::Public as AuthorityId, Blake2Hasher};
-use crate::protocol::{Context, Protocol, ProtocolStatus, CustomMessageOutcome, NetworkOut};
+use crate::protocol::{Context, Protocol, ProtocolConfig, ProtocolStatus, CustomMessageOutcome, NetworkOut};
 use runtime_primitives::generic::BlockId;
 use runtime_primitives::traits::{AuthorityIdFor, Block as BlockT, Digest, DigestItem, Header, NumberFor};
 use runtime_primitives::{Justification, ConsensusEngineId};