Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Remove NetworkService::config() #8653

Merged
merged 1 commit into from
Jun 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 32 additions & 17 deletions ethcore/sync/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just remove struct NetworkConfiguration entirely?

/// 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

fn num_peers_range(&self) -> Range<u32>;
/// Get network context for protocol.
fn with_proto_context(&self, proto: ProtocolId, f: &mut FnMut(&NetworkContext));
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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),
}
},
_ => {},
}

Expand All @@ -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)) {
Expand All @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the range exclusive and not inclusive? This is a breaking change. If someone have min: 10 and max: 10 specified in the config, the new min and max will be 10 and 9.

min: peers_range.start as usize,
}
}

Expand Down
5 changes: 3 additions & 2 deletions parity/informant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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(),
}))
}
Expand Down
5 changes: 3 additions & 2 deletions rpc/src/v1/impls/parity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down
5 changes: 3 additions & 2 deletions rpc/src/v1/tests/mocked/manage_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) { }
}
28 changes: 20 additions & 8 deletions util/network-devp2p/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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(())
Expand Down
2 changes: 1 addition & 1 deletion whisper/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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,
Expand Down