Remove NetworkService::config() (#8653)

Author: tomaka

ethcore/sync/src/api.rs

@@ -17,6 +17,7 @@
 use std::sync::Arc;
 use std::collections::{HashMap, BTreeMap};
 use std::io;
+use std::ops::Range;
 use std::time::Duration;
 use bytes::Bytes;
 use devp2p::NetworkService;
@@ -452,11 +453,18 @@ impl ChainNotify for EthSync {
 	}
 
 	fn start(&self) {
-		match self.network.start().map_err(Into::into) {
-			Err(ErrorKind::Io(ref e)) if e.kind() == io::ErrorKind::AddrInUse => warn!("Network port {:?} is already in use, make sure that another instance of an Ethereum client is not running or change the port using the --port option.", self.network.config().listen_address.expect("Listen address is not set.")),
-			Err(err) => warn!("Error starting network: {}", err),
+		match self.network.start() {
+			Err((err, listen_address)) => {
+				match err.into() {
+					ErrorKind::Io(ref e) if e.kind() == io::ErrorKind::AddrInUse => {
+						warn!("Network port {:?} is already in use, make sure that another instance of an Ethereum client is not running or change the port using the --port option.", listen_address.expect("Listen address is not set."))
+					},
+					err => warn!("Error starting network: {}", err),
+				}
+			},
 			_ => {},
 		}
+
 		self.network.register_protocol(self.eth_handler.clone(), self.subprotocol_name, &[ETH_PROTOCOL_VERSION_62, ETH_PROTOCOL_VERSION_63])
 			.unwrap_or_else(|e| warn!("Error registering ethereum protocol: {:?}", e));
 		// register the warp sync subprotocol
@@ -520,8 +528,10 @@ pub trait ManageNetwork : Send + Sync {
 	fn start_network(&self);
 	/// Stop network
 	fn stop_network(&self);
-	/// Query the current configuration of the network
-	fn network_config(&self) -> NetworkConfiguration;
+	/// Returns the minimum and maximum peers.
+	/// Note that `range.end` is *exclusive*.
+	// TODO: Range should be changed to RangeInclusive once stable (https://github.com/rust-lang/rust/pull/50758)
+	fn num_peers_range(&self) -> Range<u32>;
 	/// Get network context for protocol.
 	fn with_proto_context(&self, proto: ProtocolId, f: &mut FnMut(&NetworkContext));
 }
@@ -561,8 +571,8 @@ impl ManageNetwork for EthSync {
 		self.stop();
 	}
 
-	fn network_config(&self) -> NetworkConfiguration {
-		NetworkConfiguration::from(self.network.config().clone())
+	fn num_peers_range(&self) -> Range<u32> {
+		self.network.num_peers_range()
 	}
 
 	fn with_proto_context(&self, proto: ProtocolId, f: &mut FnMut(&NetworkContext)) {
@@ -815,11 +825,15 @@ impl ManageNetwork for LightSync {
 	}
 
 	fn start_network(&self) {
-		match self.network.start().map_err(Into::into) {
-			Err(ErrorKind::Io(ref e)) if e.kind() == io::ErrorKind::AddrInUse => {
-				warn!("Network port {:?} is already in use, make sure that another instance of an Ethereum client is not running or change the port using the --port option.", self.network.config().listen_address.expect("Listen address is not set."))
-			}
-			Err(err) => warn!("Error starting network: {}", err),
+		match self.network.start() {
+			Err((err, listen_address)) => {
+				match err.into() {
+					ErrorKind::Io(ref e) if e.kind() == io::ErrorKind::AddrInUse => {
+						warn!("Network port {:?} is already in use, make sure that another instance of an Ethereum client is not running or change the port using the --port option.", listen_address.expect("Listen address is not set."))
+					},
+					err => warn!("Error starting network: {}", err),
+				}
+			},
 			_ => {},
 		}
 
@@ -836,8 +850,8 @@ impl ManageNetwork for LightSync {
 		self.network.stop();
 	}
 
-	fn network_config(&self) -> NetworkConfiguration {
-		NetworkConfiguration::from(self.network.config().clone())
+	fn num_peers_range(&self) -> Range<u32> {
+		self.network.num_peers_range()
 	}
 
 	fn with_proto_context(&self, proto: ProtocolId, f: &mut FnMut(&NetworkContext)) {
@@ -848,12 +862,13 @@ impl ManageNetwork for LightSync {
 impl LightSyncProvider for LightSync {
 	fn peer_numbers(&self) -> PeerNumbers {
 		let (connected, active) = self.proto.peer_count();
-		let config = self.network_config();
+		let peers_range = self.num_peers_range();
+		debug_assert!(peers_range.end > peers_range.start);
 		PeerNumbers {
 			connected: connected,
 			active: active,
-			max: config.max_peers as usize,
-			min: config.min_peers as usize,
+			max: peers_range.end as usize - 1,
+			min: peers_range.start as usize,
 		}
 	}
 

parity/informant.rs

@@ -145,7 +145,8 @@ impl InformantData for FullNodeInformantData {
 		let (importing, sync_info) = match (self.sync.as_ref(), self.net.as_ref()) {
 			(Some(sync), Some(net)) => {
 				let status = sync.status();
-				let net_config = net.network_config();
+				let num_peers_range = net.num_peers_range();
+				debug_assert!(num_peers_range.end > num_peers_range.start);
 
 				cache_sizes.insert("sync", status.mem_used);
 
@@ -154,7 +155,7 @@ impl InformantData for FullNodeInformantData {
 					last_imported_block_number: status.last_imported_block_number.unwrap_or(chain_info.best_block_number),
 					last_imported_old_block_number: status.last_imported_old_block_number,
 					num_peers: status.num_peers,
-					max_peers: status.current_max_peers(net_config.min_peers, net_config.max_peers),
+					max_peers: status.current_max_peers(num_peers_range.start, num_peers_range.end - 1),
 					snapshot_sync: status.is_snapshot_syncing(),
 				}))
 			}

rpc/src/v1/impls/parity.rs

@@ -212,13 +212,14 @@ impl<C, M, U, S> Parity for ParityClient<C, M, U> where
 
 	fn net_peers(&self) -> Result<Peers> {
 		let sync_status = self.sync.status();
-		let net_config = self.net.network_config();
+		let num_peers_range = self.net.num_peers_range();
+		debug_assert!(num_peers_range.end > num_peers_range.start);
 		let peers = self.sync.peers().into_iter().map(Into::into).collect();
 
 		Ok(Peers {
 			active: sync_status.num_active_peers,
 			connected: sync_status.num_peers,
-			max: sync_status.current_max_peers(net_config.min_peers, net_config.max_peers),
+			max: sync_status.current_max_peers(num_peers_range.start, num_peers_range.end - 1),
 			peers: peers
 		})
 	}

rpc/src/v1/tests/mocked/manage_network.rs

@@ -14,7 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use sync::{ManageNetwork, NetworkConfiguration};
+use std::ops::Range;
+use sync::ManageNetwork;
 use self::ethcore_network::{ProtocolId, NetworkContext};
 
 extern crate ethcore_network;
@@ -29,6 +30,6 @@ impl ManageNetwork for TestManageNetwork {
 	fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { Ok(()) }
 	fn start_network(&self) {}
 	fn stop_network(&self) {}
-	fn network_config(&self) -> NetworkConfiguration { NetworkConfiguration::new_local() }
+	fn num_peers_range(&self) -> Range<u32> { 25 .. 51 }
 	fn with_proto_context(&self, _: ProtocolId, _: &mut FnMut(&NetworkContext)) { }
 }

util/network-devp2p/src/service.rs

@@ -19,6 +19,8 @@ use network::{NetworkContext, PeerId, ProtocolId, NetworkIoMessage};
 use host::Host;
 use io::*;
 use parking_lot::RwLock;
+use std::net::SocketAddr;
+use std::ops::Range;
 use std::sync::Arc;
 use ansi_term::Colour;
 use network::ConnectionFilter;
@@ -92,9 +94,13 @@ impl NetworkService {
 		&self.io_service
 	}
 
-	/// Returns network configuration.
-	pub fn config(&self) -> &NetworkConfiguration {
-		&self.config
+	/// Returns the number of peers allowed.
+	///
+	/// Keep in mind that `range.end` is *exclusive*.
+	pub fn num_peers_range(&self) -> Range<u32> {
+		let start = self.config.min_peers;
+		let end = self.config.max_peers + 1;
+		start .. end
 	}
 
 	/// Returns external url if available.
@@ -109,17 +115,23 @@ impl NetworkService {
 		host.as_ref().map(|h| h.local_url())
 	}
 
-	/// Start network IO
-	pub fn start(&self) -> Result<(), Error> {
+	/// Start network IO.
+	///
+	/// In case of error, also returns the listening address for better error reporting.
+	pub fn start(&self) -> Result<(), (Error, Option<SocketAddr>)> {
 		let mut host = self.host.write();
+		let listen_addr = self.config.listen_address.clone();
 		if host.is_none() {
-			let h = Arc::new(Host::new(self.config.clone(), self.filter.clone())?);
-			self.io_service.register_handler(h.clone())?;
+			let h = Arc::new(Host::new(self.config.clone(), self.filter.clone())
+				.map_err(|err| (err.into(), listen_addr))?);
+			self.io_service.register_handler(h.clone())
+				.map_err(|err| (err.into(), listen_addr))?;
 			*host = Some(h);
 		}
 
 		if self.host_handler.public_url.read().is_none() {
-			self.io_service.register_handler(self.host_handler.clone())?;
+			self.io_service.register_handler(self.host_handler.clone())
+				.map_err(|err| (err.into(), listen_addr))?;
 		}
 
 		Ok(())

whisper/cli/src/main.rs

@@ -218,7 +218,7 @@ fn execute<S, I>(command: I) -> Result<(), Error> where I: IntoIterator<Item=S>,
 	let network = devp2p::NetworkService::new(net::NetworkConfiguration::new_local(), None)?;
 
 	// Start network service
-	network.start()?;
+	network.start().map_err(|(err, _)| err)?;
 
 	// Attach whisper protocol to the network service
 	network.register_protocol(whisper_network_handler.clone(), whisper::net::PROTOCOL_ID,