Gh 690: Self Review 2

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_lib/src/shared_schema.rs

@@ -105,7 +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 = "Enter the count as an argument. 3-hops is required for anonymity."; // TODO: rewrite this
+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 \
@@ -412,8 +427,9 @@ pub fn shared_app(head: App<'static, 'static>) -> App<'static, 'static> {
         Arg::with_name("min-hops")
             .long("min-hops")
             .value_name("MIN_HOPS")
+            .default_value("3")
             .required(false)
-            .min_values(1)
+            .min_values(0)
             .max_values(1)
             .possible_values(&["1", "2", "3", "4", "5", "6"])
             .help(MIN_HOPS_HELP),
@@ -724,10 +740,6 @@ mod tests {
              generates a lot of log traffic. This will both consume your disk space and degrade your Node's performance. \
              You should probably not specify a level higher than the default unless you have security concerns about \
              persistent logs being kept on your computer: if your Node crashes, it's good to know why.");
-        assert_eq!(
-            MIN_HOPS_HELP,
-            "Enter the count as an argument. 3-hops is required for anonymity."
-        );
         assert_eq!(
             NEIGHBORS_HELP,
             "One or more Node descriptors for running Nodes in the MASQ \
@@ -779,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 \

masq_lib/src/utils.rs

@@ -220,8 +220,6 @@ impl FromStr for NeighborhoodModeLight {
     }
 }
 
-
-
 pub fn plus<T>(mut source: Vec<T>, item: T) -> Vec<T> {
     let mut result = vec![];
     result.append(&mut source);

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(2),
+            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, Hops};
+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;
@@ -34,9 +35,9 @@ use std::str::FromStr;
 use std::string::ToString;
 use std::thread;
 use std::time::Duration;
-use node_lib::neighborhood::DEFAULT_MIN_HOPS_COUNT;
 
 pub const DATA_DIRECTORY: &str = "/node_root/home";
+pub const STANDARD_CLIENT_TIMEOUT_MILLIS: u64 = 1000;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Firewall {
@@ -940,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 {

multinode_integration_tests/tests/data_routing_test.rs

@@ -6,20 +6,20 @@ use multinode_integration_tests_lib::masq_node::MASQNode;
 use multinode_integration_tests_lib::masq_node_cluster::MASQNodeCluster;
 use multinode_integration_tests_lib::masq_real_node::{
     default_consuming_wallet_info, make_consuming_wallet_info, MASQRealNode,
-    NodeStartupConfigBuilder,
+    NodeStartupConfigBuilder, STANDARD_CLIENT_TIMEOUT_MILLIS,
 };
 use native_tls::HandshakeError;
 use native_tls::TlsConnector;
 use native_tls::TlsStream;
 use node_lib::proxy_server::protocol_pack::ServerImpersonator;
 use node_lib::proxy_server::server_impersonator_http::ServerImpersonatorHttp;
+use node_lib::sub_lib::neighborhood::Hops;
 use node_lib::test_utils::{handle_connection_error, read_until_timeout};
 use std::io::Write;
 use std::net::{IpAddr, SocketAddr, TcpStream};
 use std::str::FromStr;
 use std::thread;
 use std::time::Duration;
-use node_lib::sub_lib::neighborhood::Hops;
 
 #[test]
 fn http_end_to_end_routing_test() {
@@ -58,7 +58,7 @@ fn http_end_to_end_routing_test() {
 
     thread::sleep(Duration::from_millis(500));
 
-    let mut client = last_node.make_client(8080);
+    let mut client = last_node.make_client(8080, STANDARD_CLIENT_TIMEOUT_MILLIS);
     client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
     let response = client.wait_for_chunk();
 
@@ -93,7 +93,7 @@ fn assert_http_end_to_end_routing_test(min_hops_count: Hops) {
 
     thread::sleep(Duration::from_millis(500 * (nodes.len() as u64)));
 
-    let mut client = first_node.make_client(8080);
+    let mut client = first_node.make_client(8080, 5000);
     client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
     let response = client.wait_for_chunk();
 
@@ -107,13 +107,11 @@ fn assert_http_end_to_end_routing_test(min_hops_count: Hops) {
 
 #[test]
 fn http_end_to_end_routing_test_with_different_min_hops_count() {
-    // TODO: This test fails sometimes due to a timeout: Couldn't read chunk: Kind(TimedOut)
-    assert_http_end_to_end_routing_test(Hops::OneHop); // Working fine
-    assert_http_end_to_end_routing_test(Hops::TwoHops); // Working fine
-    // assert_http_end_to_end_routing_test(Hops::ThreeHops); // Working fine
-    // assert_http_end_to_end_routing_test(Hops::FourHops); // Working fine
-    // assert_http_end_to_end_routing_test(Hops::FiveHops); // Working fine
-    assert_http_end_to_end_routing_test(Hops::SixHops); // Working fine
+    // This test fails sometimes due to a timeout: "Couldn't read chunk: Kind(TimedOut)"
+    // You may fix it by increasing the timeout for the client.
+    assert_http_end_to_end_routing_test(Hops::OneHop);
+    assert_http_end_to_end_routing_test(Hops::TwoHops);
+    assert_http_end_to_end_routing_test(Hops::SixHops);
 }
 
 #[test]
@@ -150,7 +148,7 @@ fn http_end_to_end_routing_test_with_consume_and_originate_only_nodes() {
 
     thread::sleep(Duration::from_millis(1000));
 
-    let mut client = originating_node.make_client(8080);
+    let mut client = originating_node.make_client(8080, STANDARD_CLIENT_TIMEOUT_MILLIS);
     client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
     let response = client.wait_for_chunk();
 
@@ -280,7 +278,7 @@ fn http_routing_failure_produces_internal_error_response() {
     );
     thread::sleep(Duration::from_millis(1000));
 
-    let mut client = originating_node.make_client(8080);
+    let mut client = originating_node.make_client(8080, STANDARD_CLIENT_TIMEOUT_MILLIS);
 
     client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
     let response = client.wait_for_chunk();
@@ -312,7 +310,7 @@ fn tls_routing_failure_produces_internal_error_response() {
             .chain(cluster.chain)
             .build(),
     );
-    let mut client = originating_node.make_client(8443);
+    let mut client = originating_node.make_client(8443, STANDARD_CLIENT_TIMEOUT_MILLIS);
     let client_hello = vec![
         0x16, // content_type: Handshake
         0x03, 0x03, // TLS 1.2
@@ -359,8 +357,8 @@ fn multiple_stream_zero_hop_test() {
             .chain(cluster.chain)
             .build(),
     );
-    let mut one_client = zero_hop_node.make_client(8080);
-    let mut another_client = zero_hop_node.make_client(8080);
+    let mut one_client = zero_hop_node.make_client(8080, STANDARD_CLIENT_TIMEOUT_MILLIS);
+    let mut another_client = zero_hop_node.make_client(8080, STANDARD_CLIENT_TIMEOUT_MILLIS);
 
     one_client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
     another_client.send_chunk(b"GET /online/ HTTP/1.1\r\nHost: whatever.neverssl.com\r\n\r\n");

node/src/actor_system_factory.rs

@@ -630,6 +630,7 @@ mod tests {
     use crate::sub_lib::ui_gateway::UiGatewayConfig;
     use crate::test_utils::automap_mocks::{AutomapControlFactoryMock, AutomapControlMock};
     use crate::test_utils::make_wallet;
+    use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST;
     use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock;
     use crate::test_utils::recorder::{
         make_accountant_subs_from_recorder, make_blockchain_bridge_subs_from,
@@ -678,7 +679,6 @@ mod tests {
     use std::sync::{Arc, Mutex};
     use std::thread;
     use std::time::Duration;
-    use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST;
 
     struct LogRecipientSetterNull {}
 

node/src/bootstrapper.rs

@@ -732,6 +732,7 @@ mod tests {
     use crate::sub_lib::node_addr::NodeAddr;
     use crate::sub_lib::socket_server::ConfiguredByPrivilege;
     use crate::sub_lib::stream_connector::ConnectionInfo;
+    use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST;
     use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock;
     use crate::test_utils::recorder::make_recorder;
     use crate::test_utils::recorder::RecordAwaiter;
@@ -770,7 +771,6 @@ mod tests {
     use tokio::executor::current_thread::CurrentThread;
     use tokio::prelude::stream::FuturesUnordered;
     use tokio::prelude::Async;
-    use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST;
 
     lazy_static! {
         pub static ref INITIALIZATION: Mutex<bool> = Mutex::new(false);