GH-690: Self Review

ci/bump_version.sh

@@ -31,7 +31,7 @@ bump_version() {
   find_pattern='^version\s*=.*[^,]\s*$'
   replace_pattern='s/'$find_pattern'/version = "'"$version"'"/'
 
-  grep -q "$find_pattern" "$file" && sed -i "$replace_pattern" "$file"
+  grep -q "$find_pattern" "$file" && sed -i "" "$replace_pattern" "$file"
   exit_code="$?"
   if [[ "$exit_code" != "0" ]]; then
     final_exit_code=1

masq/src/commands/connection_status_command.rs

@@ -25,8 +25,7 @@ const CONNECTION_STATUS_ABOUT: &str =
 const NOT_CONNECTED_MSG: &str = "NotConnected: No external neighbor is connected to us.";
 const CONNECTED_TO_NEIGHBOR_MSG: &str =
     "ConnectedToNeighbor: External neighbor(s) are connected to us.";
-const THREE_HOPS_ROUTE_FOUND_MSG: &str =
-    "ThreeHopsRouteFound: You can relay data over the network.";
+const ROUTE_FOUND_MSG: &str = "RouteFound: You can relay data over the network.";
 
 pub fn connection_status_subcommand() -> App<'static, 'static> {
     SubCommand::with_name("connection-status").about(CONNECTION_STATUS_ABOUT)
@@ -42,7 +41,7 @@ impl Command for ConnectionStatusCommand {
                 let stdout_msg = match response.stage {
                     UiConnectionStage::NotConnected => NOT_CONNECTED_MSG,
                     UiConnectionStage::ConnectedToNeighbor => CONNECTED_TO_NEIGHBOR_MSG,
-                    UiConnectionStage::ThreeHopsRouteFound => THREE_HOPS_ROUTE_FOUND_MSG,
+                    UiConnectionStage::RouteFound => ROUTE_FOUND_MSG,
                 };
                 short_writeln!(context.stdout(), "\n{}\n", stdout_msg);
                 Ok(())
@@ -109,8 +108,8 @@ mod tests {
             "ConnectedToNeighbor: External neighbor(s) are connected to us."
         );
         assert_eq!(
-            THREE_HOPS_ROUTE_FOUND_MSG,
-            "ThreeHopsRouteFound: You can relay data over the network."
+            ROUTE_FOUND_MSG,
+            "RouteFound: You can relay data over the network."
         )
     }
 
@@ -164,11 +163,8 @@ mod tests {
     #[test]
     fn connection_status_command_happy_path_for_three_hops_route_found() {
         assert_on_connection_status_response(
-            UiConnectionStage::ThreeHopsRouteFound,
-            (
-                "\nThreeHopsRouteFound: You can relay data over the network.\n\n",
-                "",
-            ),
+            UiConnectionStage::RouteFound,
+            ("\nRouteFound: You can relay data over the network.\n\n", ""),
         );
     }
 

masq/src/notifications/connection_change_notification.rs

@@ -21,7 +21,7 @@ impl ConnectionChangeNotification {
             UiConnectionStage::ConnectedToNeighbor => {
                 "\nConnectedToNeighbor: Established neighborship with an external node.\n"
             }
-            UiConnectionStage::ThreeHopsRouteFound => {
+            UiConnectionStage::RouteFound => {
                 "\nThreeHopsRouteFound: You can now relay data over the network.\n"
             }
         };
@@ -63,7 +63,7 @@ mod tests {
         let mut stdout = ByteArrayWriter::new();
         let stderr = ByteArrayWriter::new();
         let msg = UiConnectionChangeBroadcast {
-            stage: UiConnectionStage::ThreeHopsRouteFound,
+            stage: UiConnectionStage::RouteFound,
         };
         let term_interface = TerminalWrapper::new(Arc::new(TerminalPassiveMock::new()));
 

masq_lib/src/messages.rs

@@ -550,7 +550,7 @@ pub struct UiPaymentThresholds {
 pub enum UiConnectionStage {
     NotConnected,
     ConnectedToNeighbor,
-    ThreeHopsRouteFound,
+    RouteFound,
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]

masq_lib/src/multi_config.rs

@@ -88,6 +88,13 @@ impl<'a> MultiConfig<'a> {
             let message = format!("Invalid value: {}", &captures[2]);
             return ConfiguratorError::required(name, &message);
         }
+        let another_invalid_value_regex =
+            Regex::new("error: (.*) isn't a valid value for '--(.*?) <.*>'").expect("Bad regex"); // TODO: is this good enough?
+        if let Some(captures) = another_invalid_value_regex.captures(&e.message) {
+            let name = &captures[2];
+            let message = format!("Invalid value: {}", &captures[1]);
+            return ConfiguratorError::required(name, &message);
+        }
         if e.message
             .contains("The following required arguments were not provided:")
         {

masq_lib/src/shared_schema.rs

@@ -105,6 +105,22 @@ pub const MAPPING_PROTOCOL_HELP: &str =
     public IP address with the --ip parameter. If the Node communicates successfully with your router, \
     it will remember the protocol it used, and on its next run it will try that protocol first, unless \
     you specify a different protocol on the command line.";
+pub const MIN_HOPS_HELP: &str =
+    "The Node is a system that routes data through multiple Nodes to enhance security and privacy. \
+    However, the level of anonymity and security provided depends on the number of hops specified \
+    by the user. By default, the system allows the user to customize the number of hops within a \
+    range of 1 to 6.\n\n\
+    It's important to note that if the user selects less than 3 hops, the anonymity of their data \
+    cannot be guaranteed. Here's a breakdown of the different hop counts and their implications:\n\n\
+    1. A 1-hop route allows Exit Nodes to see your requests.\n\
+    2. A 2-hop route makes it harder to associate your requests with your IP address, but it's \
+    not a foolproof guarantee.\n\
+    3. The minimum number of hops required to guarantee anonymity is 3.\n\
+    4. Increasing the number of hops to 4, 5, or 6 can enhance security, but it will also \
+    increase the cost and latency of the route.\n\
+    If you want to specify a minimum hops count, you can do so by entering a number after the \
+    '--min-hops' command. For example, '--min-hops 4' would require at least 4 hops. If you fail \
+    to provide this argument, the system will default to a minimum hops count of 3.";
 pub const REAL_USER_HELP: &str =
     "The user whose identity Node will assume when dropping privileges after bootstrapping. Since Node refuses to \
      run with root privilege after bootstrapping, you might want to use this if you start the Node as root, or if \
@@ -407,6 +423,17 @@ pub fn shared_app(head: App<'static, 'static>) -> App<'static, 'static> {
             .case_insensitive(true)
             .help(MAPPING_PROTOCOL_HELP),
     )
+    .arg(
+        Arg::with_name("min-hops")
+            .long("min-hops")
+            .value_name("MIN_HOPS")
+            .default_value("3")
+            .required(false)
+            .min_values(0)
+            .max_values(1)
+            .possible_values(&["1", "2", "3", "4", "5", "6"])
+            .help(MIN_HOPS_HELP),
+    )
     .arg(
         Arg::with_name("neighborhood-mode")
             .long("neighborhood-mode")
@@ -764,6 +791,24 @@ mod tests {
              it will remember the protocol it used, and on its next run it will try that protocol first, unless \
              you specify a different protocol on the command line."
         );
+        assert_eq!(
+            MIN_HOPS_HELP,
+            "The Node is a system that routes data through multiple Nodes to enhance security and privacy. \
+             However, the level of anonymity and security provided depends on the number of hops specified \
+             by the user. By default, the system allows the user to customize the number of hops within a \
+             range of 1 to 6.\n\n\
+             It's important to note that if the user selects less than 3 hops, the anonymity of their data \
+             cannot be guaranteed. Here's a breakdown of the different hop counts and their implications:\n\n\
+             1. A 1-hop route allows Exit Nodes to see your requests.\n\
+             2. A 2-hop route makes it harder to associate your requests with your IP address, but it's \
+             not a foolproof guarantee.\n\
+             3. The minimum number of hops required to guarantee anonymity is 3.\n\
+             4. Increasing the number of hops to 4, 5, or 6 can enhance security, but it will also \
+             increase the cost and latency of the route.\n\
+             If you want to specify a minimum hops count, you can do so by entering a number after the \
+             '--min-hops' command. For example, '--min-hops 4' would require at least 4 hops. If you fail \
+             to provide this argument, the system will default to a minimum hops count of 3."
+        );
         assert_eq!(
             REAL_USER_HELP,
             "The user whose identity Node will assume when dropping privileges after bootstrapping. Since Node refuses to \

multinode_integration_tests/src/masq_cores_client.rs

@@ -18,7 +18,7 @@ impl<'a> MASQCoresClient<'a> {
     pub fn new(socket_addr: SocketAddr, cryptde: &'a dyn CryptDE) -> MASQCoresClient<'a> {
         MASQCoresClient {
             cryptde,
-            delegate: MASQNodeClient::new(socket_addr),
+            delegate: MASQNodeClient::new(socket_addr, 1000),
         }
     }
 

multinode_integration_tests/src/masq_node_client.rs

@@ -11,7 +11,7 @@ pub struct MASQNodeClient {
 }
 
 impl MASQNodeClient {
-    pub fn new(socket_addr: SocketAddr) -> MASQNodeClient {
+    pub fn new(socket_addr: SocketAddr, timeout_millis: u64) -> MASQNodeClient {
         let stream = TcpStream::connect(&socket_addr)
             .unwrap_or_else(|_| panic!("Connecting to {}", socket_addr));
         stream
@@ -20,7 +20,7 @@ impl MASQNodeClient {
 
         MASQNodeClient {
             stream,
-            timeout: Duration::from_secs(1),
+            timeout: Duration::from_millis(timeout_millis),
         }
     }
 

multinode_integration_tests/src/masq_real_node.rs

@@ -15,12 +15,13 @@ use masq_lib::test_utils::utils::TEST_DEFAULT_MULTINODE_CHAIN;
 use masq_lib::utils::localhost;
 use masq_lib::utils::{DEFAULT_CONSUMING_DERIVATION_PATH, DEFAULT_EARNING_DERIVATION_PATH};
 use node_lib::blockchain::bip32::Bip32ECKeyProvider;
+use node_lib::neighborhood::DEFAULT_MIN_HOPS_COUNT;
 use node_lib::sub_lib::accountant::{
     PaymentThresholds, DEFAULT_EARNING_WALLET, DEFAULT_PAYMENT_THRESHOLDS,
 };
 use node_lib::sub_lib::cryptde::{CryptDE, PublicKey};
 use node_lib::sub_lib::cryptde_null::CryptDENull;
-use node_lib::sub_lib::neighborhood::{RatePack, DEFAULT_RATE_PACK, ZERO_RATE_PACK};
+use node_lib::sub_lib::neighborhood::{Hops, RatePack, DEFAULT_RATE_PACK, ZERO_RATE_PACK};
 use node_lib::sub_lib::node_addr::NodeAddr;
 use node_lib::sub_lib::wallet::Wallet;
 use regex::Regex;
@@ -36,6 +37,7 @@ use std::thread;
 use std::time::Duration;
 
 pub const DATA_DIRECTORY: &str = "/node_root/home";
+pub const STANDARD_CLIENT_TIMEOUT_MILLIS: u64 = 1000;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Firewall {
@@ -112,6 +114,7 @@ pub fn make_consuming_wallet_info(token: &str) -> ConsumingWalletInfo {
 #[derive(PartialEq, Eq, Clone, Debug)]
 pub struct NodeStartupConfig {
     pub neighborhood_mode: String,
+    pub min_hops_count: Hops,
     pub ip_info: LocalIpInfo,
     pub dns_servers_opt: Option<Vec<IpAddr>>,
     pub neighbors: Vec<NodeReference>,
@@ -143,6 +146,7 @@ impl NodeStartupConfig {
     pub fn new() -> NodeStartupConfig {
         NodeStartupConfig {
             neighborhood_mode: "standard".to_string(),
+            min_hops_count: DEFAULT_MIN_HOPS_COUNT,
             ip_info: LocalIpInfo::ZeroHop,
             dns_servers_opt: None,
             neighbors: Vec::new(),
@@ -174,6 +178,8 @@ impl NodeStartupConfig {
         let mut args = vec![];
         args.push("--neighborhood-mode".to_string());
         args.push(self.neighborhood_mode.clone());
+        args.push("--min-hops".to_string());
+        args.push(format!("{}", self.min_hops_count as usize));
         if let LocalIpInfo::DistributedKnown(ip_addr) = self.ip_info {
             args.push("--ip".to_string());
             args.push(ip_addr.to_string());
@@ -403,6 +409,7 @@ impl NodeStartupConfig {
 
 pub struct NodeStartupConfigBuilder {
     neighborhood_mode: String,
+    min_hops_count: Hops,
     ip_info: LocalIpInfo,
     dns_servers_opt: Option<Vec<IpAddr>>,
     neighbors: Vec<NodeReference>,
@@ -458,6 +465,7 @@ impl NodeStartupConfigBuilder {
     pub fn standard() -> Self {
         Self {
             neighborhood_mode: "standard".to_string(),
+            min_hops_count: DEFAULT_MIN_HOPS_COUNT,
             ip_info: LocalIpInfo::DistributedUnknown,
             dns_servers_opt: None,
             neighbors: vec![],
@@ -483,6 +491,7 @@ impl NodeStartupConfigBuilder {
     pub fn copy(config: &NodeStartupConfig) -> Self {
         Self {
             neighborhood_mode: config.neighborhood_mode.clone(),
+            min_hops_count: config.min_hops_count,
             ip_info: config.ip_info,
             dns_servers_opt: config.dns_servers_opt.clone(),
             neighbors: config.neighbors.clone(),
@@ -521,6 +530,11 @@ impl NodeStartupConfigBuilder {
         }
     }
 
+    pub fn min_hops_count(mut self, value: Hops) -> Self {
+        self.min_hops_count = value;
+        self
+    }
+
     pub fn ip(mut self, value: IpAddr) -> Self {
         self.ip_info = LocalIpInfo::DistributedKnown(value);
         self
@@ -634,6 +648,7 @@ impl NodeStartupConfigBuilder {
     pub fn build(self) -> NodeStartupConfig {
         NodeStartupConfig {
             neighborhood_mode: self.neighborhood_mode,
+            min_hops_count: self.min_hops_count,
             ip_info: self.ip_info,
             dns_servers_opt: self.dns_servers_opt,
             neighbors: self.neighbors,
@@ -926,9 +941,9 @@ impl MASQRealNode {
         }
     }
 
-    pub fn make_client(&self, port: u16) -> MASQNodeClient {
+    pub fn make_client(&self, port: u16, timeout_millis: u64) -> MASQNodeClient {
         let socket_addr = SocketAddr::new(self.ip_address(), port);
-        MASQNodeClient::new(socket_addr)
+        MASQNodeClient::new(socket_addr, timeout_millis)
     }
 
     pub fn make_server(&self, port: u16) -> MASQNodeServer {
@@ -1226,6 +1241,7 @@ mod tests {
     use masq_lib::constants::{HTTP_PORT, TLS_PORT};
     use masq_lib::test_utils::utils::TEST_DEFAULT_MULTINODE_CHAIN;
     use masq_lib::utils::localhost;
+    use node_lib::sub_lib::neighborhood::Hops::TwoHops;
 
     #[test]
     fn node_startup_config_builder_zero_hop() {
@@ -1289,6 +1305,7 @@ mod tests {
 
     #[test]
     fn node_startup_config_builder_settings() {
+        let min_hops_count = Hops::SixHops;
         let ip_addr = IpAddr::from_str("1.2.3.4").unwrap();
         let one_neighbor_key = PublicKey::new(&[1, 2, 3, 4]);
         let one_neighbor_ip_addr = IpAddr::from_str("4.5.6.7").unwrap();
@@ -1317,6 +1334,7 @@ mod tests {
         let dns_target = IpAddr::from_str("8.9.10.11").unwrap();
 
         let result = NodeStartupConfigBuilder::standard()
+            .min_hops_count(min_hops_count)
             .ip(ip_addr)
             .dns_servers(dns_servers.clone())
             .neighbor(neighbors[0].clone())
@@ -1325,6 +1343,7 @@ mod tests {
             .dns_port(35)
             .build();
 
+        assert_eq!(result.min_hops_count, min_hops_count);
         assert_eq!(result.ip_info, LocalIpInfo::DistributedKnown(ip_addr));
         assert_eq!(result.dns_servers_opt, Some(dns_servers));
         assert_eq!(result.neighbors, neighbors);
@@ -1337,6 +1356,7 @@ mod tests {
     fn node_startup_config_builder_copy() {
         let original = NodeStartupConfig {
             neighborhood_mode: "consume-only".to_string(),
+            min_hops_count: TwoHops,
             ip_info: LocalIpInfo::DistributedUnknown,
             dns_servers_opt: Some(vec![IpAddr::from_str("255.255.255.255").unwrap()]),
             neighbors: vec![NodeReference::new(
@@ -1415,6 +1435,7 @@ mod tests {
             .build();
 
         assert_eq!(result.neighborhood_mode, neighborhood_mode);
+        assert_eq!(result.min_hops_count, Hops::TwoHops);
         assert_eq!(result.ip_info, LocalIpInfo::DistributedKnown(ip_addr));
         assert_eq!(result.dns_servers_opt, Some(dns_servers));
         assert_eq!(result.neighbors, neighbors);
@@ -1481,6 +1502,7 @@ mod tests {
 
         let subject = NodeStartupConfigBuilder::standard()
             .neighborhood_mode("consume-only")
+            .min_hops_count(Hops::SixHops)
             .ip(IpAddr::from_str("1.3.5.7").unwrap())
             .neighbor(one_neighbor.clone())
             .neighbor(another_neighbor.clone())
@@ -1496,6 +1518,8 @@ mod tests {
             Command::strings(vec!(
                 "--neighborhood-mode",
                 "consume-only",
+                "--min-hops",
+                "6",
                 "--ip",
                 "1.3.5.7",
                 "--neighbors",

multinode_integration_tests/tests/connection_progress_test.rs

@@ -56,9 +56,9 @@ fn connection_progress_is_properly_broadcast() {
         let message_body =
             ui_client.wait_for_specific_broadcast(vec!["connectionChange"], Duration::from_secs(5));
         let (ccb, _) = UiConnectionChangeBroadcast::fmb(message_body).unwrap();
-        assert_eq!(ccb.stage, UiConnectionStage::ThreeHopsRouteFound);
+        assert_eq!(ccb.stage, UiConnectionStage::RouteFound);
     } else {
-        assert_eq!(ccb.stage, UiConnectionStage::ThreeHopsRouteFound);
+        assert_eq!(ccb.stage, UiConnectionStage::RouteFound);
     }
 }