From dab967ace8ad1f4e02c597efda8da8eac874206a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 1 Jun 2018 09:49:46 +0200 Subject: [PATCH] Remove NetworkService::config() (#8653) --- ethcore/sync/src/api.rs | 49 +++++++++++++++-------- parity/informant.rs | 5 ++- rpc/src/v1/impls/parity.rs | 5 ++- rpc/src/v1/tests/mocked/manage_network.rs | 5 ++- util/network-devp2p/src/service.rs | 28 +++++++++---- whisper/cli/src/main.rs | 2 +- 6 files changed, 62 insertions(+), 32 deletions(-) diff --git a/ethcore/sync/src/api.rs b/ethcore/sync/src/api.rs index 2f90e410a..9e6cdafa6 100644 --- a/ethcore/sync/src/api.rs +++ b/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; /// 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 { + 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 { + 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, } } diff --git a/parity/informant.rs b/parity/informant.rs index e2f3960b6..43788bc9d 100644 --- a/parity/informant.rs +++ b/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(), })) } diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index 08d514720..3fa9cb991 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/rpc/src/v1/impls/parity.rs @@ -212,13 +212,14 @@ impl Parity for ParityClient where fn net_peers(&self) -> Result { 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 }) } diff --git a/rpc/src/v1/tests/mocked/manage_network.rs b/rpc/src/v1/tests/mocked/manage_network.rs index 3a901c8e9..da4f1aa51 100644 --- a/rpc/src/v1/tests/mocked/manage_network.rs +++ b/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 . -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 { 25 .. 51 } fn with_proto_context(&self, _: ProtocolId, _: &mut FnMut(&NetworkContext)) { } } diff --git a/util/network-devp2p/src/service.rs b/util/network-devp2p/src/service.rs index d8105f649..f90c66067 100644 --- a/util/network-devp2p/src/service.rs +++ b/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 { + 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)> { 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(()) diff --git a/whisper/cli/src/main.rs b/whisper/cli/src/main.rs index d76c216be..6f3aec859 100644 --- a/whisper/cli/src/main.rs +++ b/whisper/cli/src/main.rs @@ -218,7 +218,7 @@ fn execute(command: I) -> Result<(), Error> where I: IntoIterator, 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,