simplify logic, new ParseError & add tests

parity/cli/mod.rs

@@ -1855,4 +1855,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();
+	}
 }

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,9 +317,16 @@ macro_rules! usage {
 			pub fn parse<S: AsRef<str>>(command: &[S]) -> Result<Self, ArgsError> {
 				let raw_args = RawArgs::parse(command)?;
 
+				// Invalid configuration pattern `mix_peers` > `max_peers`
+				if let (Some(max_peers), Some(min_peers)) = (raw_args.arg_max_peers, raw_args.arg_min_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()));
+				else if raw_args.flag_no_config {
+					return Ok(raw_args.into_args(Config::default()))
 				}
 
 				let config_file = raw_args.arg_config.clone().unwrap_or_else(|| raw_args.clone().into_args(Config::default()).arg_config);

parity/configuration.rs

@@ -19,7 +19,6 @@ use std::io::Read;
 use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
 use std::collections::BTreeMap;
-use std::cmp::{min, max};
 use std::str::FromStr;
 use cli::{Args, ArgsError};
 use hash::keccak;
@@ -458,8 +457,13 @@ impl Configuration {
 	}
 
 	fn max_peers(&self) -> u32 {
-		max(self.args.arg_max_peers.unwrap_or(DEFAULT_MAX_PEERS) as u32,
-			self.args.arg_min_peers.unwrap_or(DEFAULT_MIN_PEERS) as u32)
+		match (self.args.arg_max_peers, self.args.arg_min_peers) {
+			// Only `min_peers` specified by the user then set `max_peers` to that value
+			(None, Some(min)) => min as u32,
+
+			// Both or none specified by the user and max ensured to be bigger than min
+			(_, _) => self.args.arg_max_peers.unwrap_or(DEFAULT_MAX_PEERS) as u32,
+		}
 	}
 
 	fn ip_filter(&self) -> Result<IpFilter, String> {
@@ -471,17 +475,11 @@ impl Configuration {
 
 	fn min_peers(&self) -> u32 {
 		match (self.args.arg_max_peers, self.args.arg_min_peers) {
-			// Only `max peers``specified and respect that by configuring `min peers` that value
+			// Only `max_peers` specified by the user then set `min_peers` to that value
 			(Some(max), None) => max as u32,
 
-			// Only `min_peers` specified pick that value
-			(None, Some(min)) => min as u32,
-
-			// Both or none specified, pick the smallest value
-			(_, _) => {
-				min(self.args.arg_max_peers.unwrap_or(DEFAULT_MAX_PEERS) as u32,
-				    self.args.arg_min_peers.unwrap_or(DEFAULT_MIN_PEERS) as u32)
-			}
+			// Both or none specified by the user and max ensured to be bigger than min
+			(_, _) => self.args.arg_min_peers.unwrap_or(DEFAULT_MIN_PEERS) as u32,
 		}
 	}
 
@@ -1921,4 +1919,30 @@ 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() {
+		let args = vec!["parity", "--max-peers=5"];
+		let conf = Configuration::parse(&args, None).unwrap();
+		match conf.into_command().unwrap().cmd {
+			Cmd::Run(c) => {
+				assert!(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() {
+		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!(c.net_conf.max_peers >= 500);
+			},
+			_ => panic!("Should be Cmd::Run"),
+		}
+	}
 }