diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 22be9a0c4..7b26aa6bd 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -409,11 +409,11 @@ usage! { "--port=[PORT]", "Override the port on which the node should listen.", - ARG arg_min_peers: (u16) = 25u16, or |c: &Config| c.network.as_ref()?.min_peers.clone(), + ARG arg_min_peers: (Option) = None, or |c: &Config| c.network.as_ref()?.min_peers.clone(), "--min-peers=[NUM]", "Try to maintain at least NUM peers.", - ARG arg_max_peers: (u16) = 50u16, or |c: &Config| c.network.as_ref()?.max_peers.clone(), + ARG arg_max_peers: (Option) = None, or |c: &Config| c.network.as_ref()?.max_peers.clone(), "--max-peers=[NUM]", "Allow up to NUM peers.", @@ -1504,8 +1504,8 @@ mod tests { // -- Networking Options flag_no_warp: false, arg_port: 30303u16, - arg_min_peers: 25u16, - arg_max_peers: 50u16, + arg_min_peers: Some(25u16), + arg_max_peers: Some(50u16), arg_max_pending_peers: 64u16, arg_snapshot_peers: 0u16, arg_allow_ips: "all".into(), @@ -1896,4 +1896,18 @@ mod tests { stratum: None, }); } + + #[test] + fn should_not_accept_min_peers_bigger_than_max_peers() { + match Args::parse(&["parity", "--max-peers=39", "--min-peers=40"]) { + Err(ArgsError::PeerConfiguration) => (), + _ => assert_eq!(false, true), + } + } + + #[test] + fn should_accept_max_peers_equal_or_bigger_than_min_peers() { + Args::parse(&["parity", "--max-peers=40", "--min-peers=40"]).unwrap(); + Args::parse(&["parity", "--max-peers=100", "--min-peers=40"]).unwrap(); + } } diff --git a/parity/cli/usage.rs b/parity/cli/usage.rs index 2dabc8378..2bdeaaed1 100644 --- a/parity/cli/usage.rs +++ b/parity/cli/usage.rs @@ -161,6 +161,7 @@ macro_rules! usage { Clap(ClapError), Decode(toml::de::Error), Config(String, io::Error), + PeerConfiguration, } impl ArgsError { @@ -177,6 +178,10 @@ macro_rules! usage { println_stderr!("{}", e); process::exit(2) }, + ArgsError::PeerConfiguration => { + println_stderr!("You have supplied `min_peers` > `max_peers`"); + process::exit(2) + } } } } @@ -312,6 +317,13 @@ macro_rules! usage { pub fn parse>(command: &[S]) -> Result { let raw_args = RawArgs::parse(command)?; + if let (Some(max_peers), Some(min_peers)) = (raw_args.arg_max_peers, raw_args.arg_min_peers) { + // Invalid configuration pattern `mix_peers` > `max_peers` + if min_peers > max_peers { + return Err(ArgsError::PeerConfiguration); + } + } + // Skip loading config file if no_config flag is specified if raw_args.flag_no_config { return Ok(raw_args.into_args(Config::default())); diff --git a/parity/configuration.rs b/parity/configuration.rs index 07245dbbe..db8759a71 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -14,12 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::cmp::{max, min}; use std::time::Duration; use std::io::Read; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::collections::BTreeMap; -use std::cmp::max; use std::str::FromStr; use cli::{Args, ArgsError}; use hash::keccak; @@ -55,6 +55,9 @@ use account::{AccountCmd, NewAccount, ListAccounts, ImportAccounts, ImportFromGe use snapshot::{self, SnapshotCommand}; use network::{IpFilter}; +const DEFAULT_MAX_PEERS: u16 = 50; +const DEFAULT_MIN_PEERS: u16 = 25; + #[derive(Debug, PartialEq)] pub enum Cmd { Run(RunCmd), @@ -469,8 +472,9 @@ impl Configuration { } fn max_peers(&self) -> u32 { - let peers = self.args.arg_max_peers as u32; - max(self.min_peers(), peers) + self.args.arg_max_peers + .or(max(self.args.arg_min_peers, Some(DEFAULT_MAX_PEERS))) + .unwrap_or(DEFAULT_MAX_PEERS) as u32 } fn ip_filter(&self) -> Result { @@ -481,7 +485,9 @@ impl Configuration { } fn min_peers(&self) -> u32 { - self.args.arg_peers.unwrap_or(self.args.arg_min_peers) as u32 + self.args.arg_min_peers + .or(min(self.args.arg_max_peers, Some(DEFAULT_MIN_PEERS))) + .unwrap_or(DEFAULT_MIN_PEERS) as u32 } fn max_pending_peers(&self) -> u32 { @@ -1970,4 +1976,56 @@ mod tests { assert_eq!(std.directories().cache, dir::helpers::replace_home_and_local(&base_path, &local_path, ::dir::CACHE_PATH)); assert_eq!(base.directories().cache, "/test/cache"); } + + #[test] + fn should_respect_only_max_peers_and_default() { + let args = vec!["parity", "--max-peers=50"]; + let conf = Configuration::parse(&args, None).unwrap(); + match conf.into_command().unwrap().cmd { + Cmd::Run(c) => { + assert_eq!(c.net_conf.min_peers, 25); + assert_eq!(c.net_conf.max_peers, 50); + }, + _ => panic!("Should be Cmd::Run"), + } + } + + #[test] + fn should_respect_only_max_peers_less_than_default() { + let args = vec!["parity", "--max-peers=5"]; + let conf = Configuration::parse(&args, None).unwrap(); + match conf.into_command().unwrap().cmd { + Cmd::Run(c) => { + assert_eq!(c.net_conf.min_peers, 5); + assert_eq!(c.net_conf.max_peers, 5); + }, + _ => panic!("Should be Cmd::Run"), + } + } + + #[test] + fn should_respect_only_min_peers_and_default() { + let args = vec!["parity", "--min-peers=5"]; + let conf = Configuration::parse(&args, None).unwrap(); + match conf.into_command().unwrap().cmd { + Cmd::Run(c) => { + assert_eq!(c.net_conf.min_peers, 5); + assert_eq!(c.net_conf.max_peers, 50); + }, + _ => panic!("Should be Cmd::Run"), + } + } + + #[test] + fn should_respect_only_min_peers_and_greater_than_default() { + let args = vec!["parity", "--min-peers=500"]; + let conf = Configuration::parse(&args, None).unwrap(); + match conf.into_command().unwrap().cmd { + Cmd::Run(c) => { + assert_eq!(c.net_conf.min_peers, 500); + assert_eq!(c.net_conf.max_peers, 500); + }, + _ => panic!("Should be Cmd::Run"), + } + } }