From 8764839aad03b70fab19d7c380bbf44ef8d8bce7 Mon Sep 17 00:00:00 2001 From: Utkarsh Gupta Date: Mon, 1 May 2023 16:33:44 +0530 Subject: [PATCH 01/18] GH-690: use higher timeout for the client inisde the data routing test for different min hops count --- .../src/masq_cores_client.rs | 2 +- .../src/masq_node_client.rs | 4 ++-- .../src/masq_real_node.rs | 5 +++-- .../tests/data_routing_test.rs | 19 ++++++++----------- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/multinode_integration_tests/src/masq_cores_client.rs b/multinode_integration_tests/src/masq_cores_client.rs index 2722b9e24..0c89953cf 100644 --- a/multinode_integration_tests/src/masq_cores_client.rs +++ b/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), } } diff --git a/multinode_integration_tests/src/masq_node_client.rs b/multinode_integration_tests/src/masq_node_client.rs index 1817a1a3a..6df65b68a 100644 --- a/multinode_integration_tests/src/masq_node_client.rs +++ b/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), } } diff --git a/multinode_integration_tests/src/masq_real_node.rs b/multinode_integration_tests/src/masq_real_node.rs index aa3ad7b23..2c02909fe 100644 --- a/multinode_integration_tests/src/masq_real_node.rs +++ b/multinode_integration_tests/src/masq_real_node.rs @@ -37,6 +37,7 @@ 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 { diff --git a/multinode_integration_tests/tests/data_routing_test.rs b/multinode_integration_tests/tests/data_routing_test.rs index 7ce23971c..3b8fe8f06 100644 --- a/multinode_integration_tests/tests/data_routing_test.rs +++ b/multinode_integration_tests/tests/data_routing_test.rs @@ -4,10 +4,7 @@ use itertools::Itertools; use masq_lib::utils::index_of; 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, -}; +use multinode_integration_tests_lib::masq_real_node::{default_consuming_wallet_info, make_consuming_wallet_info, MASQRealNode, NodeStartupConfigBuilder, STANDARD_CLIENT_TIMEOUT_MILLIS}; use native_tls::HandshakeError; use native_tls::TlsConnector; use native_tls::TlsStream; @@ -58,7 +55,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 +90,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(); @@ -150,7 +147,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 +277,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 +309,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 +356,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"); From a4a664a8d193e21a4418c5f9bf1823de74961135 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 2 May 2023 14:52:42 +0530 Subject: [PATCH 02/18] GH-690: improve the help message for the --min-hops count --- masq_lib/src/shared_schema.rs | 39 ++++++++++++++++++++++++++++++----- masq_lib/src/utils.rs | 2 -- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/masq_lib/src/shared_schema.rs b/masq_lib/src/shared_schema.rs index 7a5aa5091..ad3bbeddb 100644 --- a/masq_lib/src/shared_schema.rs +++ b/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 \ @@ -724,10 +739,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 +790,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 \ diff --git a/masq_lib/src/utils.rs b/masq_lib/src/utils.rs index 3556a720e..5196086c5 100644 --- a/masq_lib/src/utils.rs +++ b/masq_lib/src/utils.rs @@ -220,8 +220,6 @@ impl FromStr for NeighborhoodModeLight { } } - - pub fn plus(mut source: Vec, item: T) -> Vec { let mut result = vec![]; result.append(&mut source); From ce1b9a696f9b654544a3228d92e5dbeda8f5b752 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 2 May 2023 15:25:42 +0530 Subject: [PATCH 03/18] GH-690: remove the working fine text and improve the comment --- .../tests/data_routing_test.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/multinode_integration_tests/tests/data_routing_test.rs b/multinode_integration_tests/tests/data_routing_test.rs index 3b8fe8f06..9b0e5a979 100644 --- a/multinode_integration_tests/tests/data_routing_test.rs +++ b/multinode_integration_tests/tests/data_routing_test.rs @@ -4,19 +4,22 @@ use itertools::Itertools; use masq_lib::utils::index_of; 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, STANDARD_CLIENT_TIMEOUT_MILLIS}; +use multinode_integration_tests_lib::masq_real_node::{ + default_consuming_wallet_info, make_consuming_wallet_info, MASQRealNode, + 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() { @@ -104,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] From 7317131b90730f14a92801050a232f3aecbe8f6c Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 2 May 2023 17:06:55 +0530 Subject: [PATCH 04/18] GH-690: refactor out functions from assert_compute_patch --- node/src/neighborhood/gossip_acceptor.rs | 48 +++++++------------ .../src/test_utils/neighborhood_test_utils.rs | 40 +++++++++++++++- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/node/src/neighborhood/gossip_acceptor.rs b/node/src/neighborhood/gossip_acceptor.rs index d8769f723..311d0d620 100644 --- a/node/src/neighborhood/gossip_acceptor.rs +++ b/node/src/neighborhood/gossip_acceptor.rs @@ -1336,7 +1336,11 @@ mod tests { use crate::sub_lib::cryptde_null::CryptDENull; use crate::sub_lib::neighborhood::{ConnectionProgressEvent, ConnectionProgressMessage, Hops}; use crate::sub_lib::utils::time_t_timestamp; - use crate::test_utils::neighborhood_test_utils::{db_from_node, make_meaningless_db, make_node_record, make_node_record_f, MIN_HOPS_COUNT_FOR_TEST}; + use crate::test_utils::neighborhood_test_utils::{ + db_from_node, gossip_about_nodes_from_database, linearly_connect_nodes, + make_meaningless_db, make_node_record, make_node_record_f, make_node_records, + public_keys_from_node_records, MIN_HOPS_COUNT_FOR_TEST, + }; use crate::test_utils::unshared_test_utils::make_cpm_recipient; use crate::test_utils::{assert_contains, main_cryptde, vec_to_set}; use actix::System; @@ -2566,40 +2570,20 @@ mod tests { fn assert_compute_patch(min_hops_count: Hops) { let subject = StandardGossipHandler::new(Logger::new("assert_compute_patch")); - // Create Nodes - let nodes_count = min_hops_count as usize + 2; // one to finish hops and one extra node - let mut nodes = Vec::with_capacity(nodes_count as usize); - for i in 1..=nodes_count { - let nonce = 1000 + i; - let has_ip = if i <= 2 { true } else { false }; - nodes.push(make_node_record(nonce as u16, has_ip)) - } - // Create Database - let root_node = &nodes[0]; - let mut root_node_db = db_from_node(root_node); - for i in 1..nodes_count { - root_node_db.add_node(nodes[i].clone()).unwrap(); - root_node_db - .add_arbitrary_full_neighbor(nodes[i - 1].public_key(), nodes[i].public_key()); - } - // Create Gossip - let mut gossip_builder = GossipBuilder::new(&root_node_db); - for i in 1..nodes_count { - gossip_builder = gossip_builder.node(nodes[i].public_key(), false); - } - let gossip = gossip_builder.build(); + // one node to finish hops and another node that's outside the patch + let nodes_count = min_hops_count as usize + 2; + let nodes = make_node_records(nodes_count as u16); + let db = linearly_connect_nodes(&nodes); + // gossip is intended for the first node (also root), thereby it's excluded + let gossip = gossip_about_nodes_from_database(&db, &nodes[1..]); let agrs: Vec = gossip.try_into().unwrap(); - let result = subject.compute_patch(&agrs, root_node_db.root(), min_hops_count as u8); - - let mut expected_nodes = nodes; - expected_nodes.pop(); - let expected_hashet = expected_nodes - .iter() - .map(|node| node.public_key().clone()) - .collect::>(); + let result = subject.compute_patch(&agrs, db.root(), min_hops_count as u8); - assert_eq!(result, expected_hashet); + // last node is excluded because it is outside the patch + let expected_nodes = &nodes[0..nodes_count - 1]; + let expected_patch = public_keys_from_node_records(&expected_nodes); + assert_eq!(result, expected_patch); } #[test] diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index 3752a1bf7..7e14f7823 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -1,6 +1,6 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::bootstrapper::BootstrapperConfig; -use crate::neighborhood::gossip::GossipNodeRecord; +use crate::neighborhood::gossip::{GossipBuilder, GossipNodeRecord, Gossip_0v1}; use crate::neighborhood::neighborhood_database::NeighborhoodDatabase; use crate::neighborhood::node_record::{NodeRecord, NodeRecordInner_0v1}; use crate::neighborhood::{AccessibleGossipRecord, Neighborhood, DEFAULT_MIN_HOPS_COUNT}; @@ -297,3 +297,41 @@ pub fn make_node(nonce: u8) -> (IpAddr, NodeDescriptor) { (ip_addr, node_descriptor) } + +pub fn make_node_records(count: u16) -> Vec { + (1..=count) + .into_iter() + .map(|i| make_node_record(i, true)) + .collect::>() +} + +pub fn linearly_connect_nodes(nodes: &[NodeRecord]) -> NeighborhoodDatabase { + let root_node = nodes.first().unwrap(); + let mut database = db_from_node(root_node); + let nodes_count = nodes.len(); + for i in 1..nodes_count { + database.add_node(nodes[i].clone()).unwrap(); + database.add_arbitrary_full_neighbor(nodes[i - 1].public_key(), nodes[i].public_key()); + } + + database +} + +pub fn gossip_about_nodes_from_database( + database: &NeighborhoodDatabase, + nodes: &[NodeRecord], +) -> Gossip_0v1 { + nodes + .iter() + .fold(GossipBuilder::new(&database), |builder, node| { + builder.node(node.public_key(), false) + }) + .build() +} + +pub fn public_keys_from_node_records(nodes: &[NodeRecord]) -> HashSet { + nodes + .iter() + .map(|node| node.public_key().clone()) + .collect::>() +} From 35eb8e517a96e0647a72e7e4fbbb8d22720321d2 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 3 May 2023 12:56:11 +0530 Subject: [PATCH 05/18] GH-690: refactor assert_route_query_message() --- node/src/neighborhood/mod.rs | 113 +++++++----------- .../src/test_utils/neighborhood_test_utils.rs | 7 ++ 2 files changed, 47 insertions(+), 73 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index fcad32b57..cfeee1877 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -435,9 +435,7 @@ impl Neighborhood { let neighborhood_mode = &neighborhood_config.mode; let mode: NeighborhoodModeLight = neighborhood_mode.into(); let neighborhood_configs = neighborhood_mode.neighbor_configs(); - if mode == NeighborhoodModeLight::ZeroHop - && !neighborhood_configs.is_empty() - { + if mode == NeighborhoodModeLight::ZeroHop && !neighborhood_configs.is_empty() { panic!( "A zero-hop MASQ Node is not decentralized and cannot have a --neighbors setting" ) @@ -843,9 +841,16 @@ impl Neighborhood { payload_size: 10000, hostname_opt: None, }; - debug!(Logger::new("Multinode"), "Searching for a {}-hops route.", self.min_hops_count as usize); + debug!( + Logger::new("Multinode"), + "Searching for a {}-hops route.", self.min_hops_count as usize + ); if let Some(route_query_response) = self.handle_route_query_message(msg) { - trace!(Logger::new("Multinode"), "Round Trip Route Length: {:?}", route_query_response.route.hops.len()); + trace!( + Logger::new("Multinode"), + "Round Trip Route Length: {:?}", + route_query_response.route.hops.len() + ); debug!( &self.logger, "The connectivity check has found a {}-hops route.", self.min_hops_count as usize @@ -1246,11 +1251,9 @@ impl Neighborhood { hostname_opt, ) .into_iter() - .filter_map(|cr| { - match cr.undesirability <= minimum_undesirability { - true => Some(cr.nodes), - false => None, - } + .filter_map(|cr| match cr.undesirability <= minimum_undesirability { + true => Some(cr.nodes), + false => None, }) .next(); @@ -1638,7 +1641,12 @@ mod tests { use crate::test_utils::assert_contains; use crate::test_utils::make_meaningless_route; use crate::test_utils::make_wallet; - use crate::test_utils::neighborhood_test_utils::{db_from_node, make_global_cryptde_node_record, make_ip, make_node, make_node_descriptor, make_node_record, make_node_record_f, MIN_HOPS_COUNT_FOR_TEST, neighborhood_from_nodes}; + use crate::test_utils::neighborhood_test_utils::{ + cryptdes_from_node_records, db_from_node, linearly_connect_nodes, + make_global_cryptde_node_record, make_ip, make_node, make_node_descriptor, + make_node_record, make_node_record_f, make_node_records, neighborhood_from_nodes, + MIN_HOPS_COUNT_FOR_TEST, + }; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::rate_pack; use crate::test_utils::recorder::make_recorder; @@ -2750,9 +2758,7 @@ mod tests { let addr: Addr = subject.start(); let sub: Recipient = addr.recipient::(); - let future = sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 400, - )); + let future = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 400)); System::current().stop_with_code(0); system.run(); @@ -2768,9 +2774,7 @@ mod tests { let addr: Addr = subject.start(); let sub: Recipient = addr.recipient::(); - let future = sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 430, - )); + let future = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 430)); System::current().stop_with_code(0); system.run(); @@ -2885,7 +2889,7 @@ mod tests { } let addr: Addr = subject.start(); let sub: Recipient = addr.recipient::(); - let msg = RouteQueryMessage::data_indefinite_route_request(None, 10000); + let msg = RouteQueryMessage::data_indefinite_route_request(None, 10000); let future = sub.send(msg); @@ -3019,9 +3023,7 @@ mod tests { let addr: Addr = subject.start(); let sub: Recipient = addr.recipient::(); - let data_route = sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 5000, - )); + let data_route = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 5000)); System::current().stop_with_code(0); system.run(); @@ -3116,12 +3118,8 @@ mod tests { let addr: Addr = subject.start(); let sub: Recipient = addr.recipient::(); - let data_route_0 = sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 2000, - )); - let data_route_1 = sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 3000, - )); + let data_route_0 = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 2000)); + let data_route_1 = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 3000)); System::current().stop_with_code(0); system.run(); @@ -3171,15 +3169,13 @@ mod tests { ) .unwrap(); - let route_request_1 = route_sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 1000, - )); + let route_request_1 = + route_sub.send(RouteQueryMessage::data_indefinite_route_request(None, 1000)); let _ = set_wallet_sub.try_send(SetConsumingWalletMessage { wallet: expected_new_wallet, }); - let route_request_2 = route_sub.send(RouteQueryMessage::data_indefinite_route_request( - None, 2000, - )); + let route_request_2 = + route_sub.send(RouteQueryMessage::data_indefinite_route_request(None, 2000)); System::current().stop(); system.run(); @@ -5321,33 +5317,14 @@ mod tests { fn assert_route_query_message(min_hops_count: Hops) { let hops = min_hops_count as usize; let nodes_count = hops + 1; - // Create Nodes - let nodes = (1..=nodes_count) - .map(|i| { - let nonce = 1000 + i as u16; - let has_ip = if i <= 2 { true } else { false }; - let node = if i == 1 { - make_global_cryptde_node_record(nonce, has_ip) - } else { - make_node_record(nonce, has_ip) - }; - node - }) - .collect::>(); - // Create Database - let root_node = nodes.get(0).unwrap(); + let root_node = make_global_cryptde_node_record(4242, true); + let mut nodes = make_node_records(nodes_count as u16); + nodes[0] = root_node; let neighbor = nodes.get(1).unwrap(); - let mut subject = neighborhood_from_nodes(root_node, Some(neighbor)); + let db = linearly_connect_nodes(&nodes); + let mut subject = neighborhood_from_nodes(db.root(), Some(neighbor)); subject.min_hops_count = min_hops_count; - for i in 1..nodes_count { - subject - .neighborhood_database - .add_node(nodes[i].clone()) - .unwrap(); - subject - .neighborhood_database - .add_arbitrary_full_neighbor(nodes[i - 1].public_key(), nodes[i].public_key()); - } + subject.neighborhood_database = db; let result = subject.make_round_trip_route(RouteQueryMessage { target_key_opt: None, @@ -5364,24 +5341,14 @@ mod tests { } }; let mut route = result.clone().unwrap().route.hops; + let route_length = route.len(); let _accounting = route.pop(); let over_route = &route[..hops]; let back_route = &route[hops..]; - let over_cryptdes = { - let mut over_nodes = nodes.clone(); - over_nodes.pop(); - over_nodes - .iter() - .map(|node| CryptDENull::from(node.public_key(), TEST_DEFAULT_CHAIN)) - .collect::>() - }; - let back_cryptdes = { - nodes - .iter() - .rev() - .map(|node| CryptDENull::from(node.public_key(), TEST_DEFAULT_CHAIN)) - .collect::>() - }; + let over_cryptdes = cryptdes_from_node_records(&nodes[..nodes_count - 1]); + let mut back_cryptdes = cryptdes_from_node_records(&nodes); + back_cryptdes.reverse(); + assert_eq!(route_length, 2 * nodes_count); assert_hops(over_cryptdes, over_route); assert_hops(back_cryptdes, back_route); } diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index 7e14f7823..1d7e62da7 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -335,3 +335,10 @@ pub fn public_keys_from_node_records(nodes: &[NodeRecord]) -> HashSet .map(|node| node.public_key().clone()) .collect::>() } + +pub fn cryptdes_from_node_records(nodes: &[NodeRecord]) -> Vec { + nodes + .iter() + .map(|node| CryptDENull::from(node.public_key(), TEST_DEFAULT_CHAIN)) + .collect::>() +} From 717f29b15f52cbff45ee6bb15eb711cbfc32d4ac Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 3 May 2023 13:47:23 +0530 Subject: [PATCH 06/18] GH-690: refactor make_neighborhood_with_linearly_connected_nodes() --- node/src/neighborhood/mod.rs | 58 ++++--------------- .../src/test_utils/neighborhood_test_utils.rs | 2 +- 2 files changed, 13 insertions(+), 47 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index cfeee1877..1a98e4856 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -3947,7 +3947,7 @@ mod tests { fn neighborhood_starts_accountant_when_first_route_can_be_made() { let (accountant, _, accountant_recording_arc) = make_recorder(); let (ui_gateway, _, _) = make_recorder(); - let mut subject = make_neighborhood_linearly_connected_with_nodes(3); + let mut subject = make_neighborhood_with_linearly_connected_nodes(4); subject.node_to_ui_recipient_opt = Some(ui_gateway.start().recipient()); let peer_actors = peer_actors_builder().accountant(accountant).build(); bind_subject(&mut subject, peer_actors); @@ -4011,8 +4011,9 @@ mod tests { fn assert_connectivity_check(hops: Hops) { init_test_logging(); let test_name = &format!("connectivity_check_for_{}_hops", hops as usize); + let nodes_count = hops as u16 + 1; let mut subject: Neighborhood = - make_neighborhood_linearly_connected_with_nodes(hops as u16); + make_neighborhood_with_linearly_connected_nodes(nodes_count); let (ui_gateway, _, ui_gateway_arc) = make_recorder(); let (accountant, _, _) = make_recorder(); let node_to_ui_recipient = ui_gateway.start().recipient::(); @@ -4069,7 +4070,7 @@ mod tests { fn neighborhood_logs_when_three_hops_route_can_not_be_made() { init_test_logging(); let test_name = "neighborhood_logs_when_three_hops_route_can_not_be_made"; - let mut subject: Neighborhood = make_neighborhood_linearly_connected_with_nodes(2); + let mut subject: Neighborhood = make_neighborhood_with_linearly_connected_nodes(3); let (ui_gateway, _, ui_gateway_arc) = make_recorder(); let (accountant, _, _) = make_recorder(); let node_to_ui_recipient = ui_gateway.start().recipient::(); @@ -5320,9 +5321,8 @@ mod tests { let root_node = make_global_cryptde_node_record(4242, true); let mut nodes = make_node_records(nodes_count as u16); nodes[0] = root_node; - let neighbor = nodes.get(1).unwrap(); let db = linearly_connect_nodes(&nodes); - let mut subject = neighborhood_from_nodes(db.root(), Some(neighbor)); + let mut subject = neighborhood_from_nodes(db.root(), nodes.get(1)); subject.min_hops_count = min_hops_count; subject.neighborhood_database = db; @@ -6066,47 +6066,13 @@ mod tests { message_opt } - fn make_neighborhood_linearly_connected_with_nodes(hops: u16) -> Neighborhood { - let subject_node = make_global_cryptde_node_record(1234, true); - let relay1 = make_node_record(1111, true); - let mut nodes = vec![ - subject_node.public_key().clone(), - relay1.public_key().clone(), - ]; - let mut neighborhood: Neighborhood = neighborhood_from_nodes(&subject_node, Some(&relay1)); - let mut replacement_database = neighborhood.neighborhood_database.clone(); - - fn nonce(x: u16) -> u16 { - x + (10 * x) + (100 * x) + (1000 * x) - } - - for i in 1..=hops { - match i { - 1 => { - replacement_database.add_node(relay1.clone()).unwrap(); - } - i if i < hops => { - let relay_node = make_node_record(nonce(i), false); - replacement_database.add_node(relay_node.clone()).unwrap(); - nodes.push(relay_node.public_key().clone()); - } - i if i == hops => { - let exit_node = make_node_record(nonce(i), false); - replacement_database.add_node(exit_node.clone()).unwrap(); - nodes.push(exit_node.public_key().clone()); - } - _ => panic!("The match statement should be exhaustive."), - } - replacement_database - .add_arbitrary_full_neighbor(&nodes[i as usize - 1], &nodes[i as usize]); - } - - neighborhood.gossip_acceptor = Box::new(DatabaseReplacementGossipAcceptor { - replacement_database, - }); - neighborhood.persistent_config_opt = Some(Box::new( - PersistentConfigurationMock::new().set_past_neighbors_result(Ok(())), - )); + fn make_neighborhood_with_linearly_connected_nodes(nodes_count: u16) -> Neighborhood { + let root_node = make_global_cryptde_node_record(4242, true); + let mut nodes = make_node_records(nodes_count); + nodes[0] = root_node; + let db = linearly_connect_nodes(&nodes); + let mut neighborhood = neighborhood_from_nodes(db.root(), nodes.get(1)); + neighborhood.neighborhood_database = db; neighborhood } diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index 1d7e62da7..e85ccc4cf 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -323,7 +323,7 @@ pub fn gossip_about_nodes_from_database( ) -> Gossip_0v1 { nodes .iter() - .fold(GossipBuilder::new(&database), |builder, node| { + .fold(GossipBuilder::new(database), |builder, node| { builder.node(node.public_key(), false) }) .build() From bc1216e5eecf5117c854a4cc5019b5145fb87ead Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 3 May 2023 15:04:23 +0530 Subject: [PATCH 07/18] GH-690: cleanup in connectivity check --- node/src/neighborhood/mod.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 1a98e4856..f0e7fbc28 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -538,7 +538,6 @@ impl Neighborhood { } else { self.make_round_trip_route(msg) }; - eprintln!("Round Trip Result: {:?}", route_result); match route_result { Ok(response) => { let msg_str = debug_msg_opt.expect("Debug Message wasn't built but expected."); @@ -841,19 +840,10 @@ impl Neighborhood { payload_size: 10000, hostname_opt: None, }; - debug!( - Logger::new("Multinode"), - "Searching for a {}-hops route.", self.min_hops_count as usize - ); - if let Some(route_query_response) = self.handle_route_query_message(msg) { - trace!( - Logger::new("Multinode"), - "Round Trip Route Length: {:?}", - route_query_response.route.hops.len() - ); + if self.handle_route_query_message(msg).is_some() { debug!( &self.logger, - "The connectivity check has found a {}-hops route.", self.min_hops_count as usize + "The connectivity check has found a {}-hop(s) route.", self.min_hops_count as usize ); self.overall_connection_status .update_ocs_stage_and_send_message_to_ui( @@ -4051,7 +4041,7 @@ mod tests { } ); TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: {}: The connectivity check has found a {}-hops route.", + "DEBUG: {}: The connectivity check has found a {}-hop(s) route.", test_name, hops as usize )); } From a5ac7070a9bb65685a9b70da62c901ac9d82775e Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 3 May 2023 18:02:51 +0530 Subject: [PATCH 08/18] GH-690: make --min-hops configurable via cli --- masq_lib/src/shared_schema.rs | 3 +- node/src/daemon/setup_reporter.rs | 28 +++++++++++++++++-- .../unprivileged_parse_args_configuration.rs | 20 ++++++------- node/src/sub_lib/neighborhood.rs | 22 ++++++--------- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/masq_lib/src/shared_schema.rs b/masq_lib/src/shared_schema.rs index ad3bbeddb..a051f87ba 100644 --- a/masq_lib/src/shared_schema.rs +++ b/masq_lib/src/shared_schema.rs @@ -427,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(1) // TODO: does this keep us being able to blank out .max_values(1) .possible_values(&["1", "2", "3", "4", "5", "6"]) .help(MIN_HOPS_HELP), diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index c1ee05c65..ad64c7839 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -43,7 +43,7 @@ use std::net::{IpAddr, Ipv4Addr}; use std::path::{Path, PathBuf}; use std::str::FromStr; -const CONSOLE_DIAGNOSTICS: bool = false; +const CONSOLE_DIAGNOSTICS: bool = true; // TODO: set back to false const ERR_SENSITIVE_BLANKED_OUT_VALUE_PAIRS: &[(&str, &str)] = &[("chain", "data-directory")]; @@ -814,6 +814,13 @@ impl ValueRetriever for MappingProtocol { } } +struct MinHops {} +impl ValueRetriever for MinHops { + fn value_name(&self) -> &'static str { + "min-hops" + } +} + struct NeighborhoodMode {} impl ValueRetriever for NeighborhoodMode { fn value_name(&self) -> &'static str { @@ -1040,6 +1047,7 @@ fn value_retrievers(dirs_wrapper: &dyn DirsWrapper) -> Vec( } }; - let min_hops_arg = value_m!(multi_config, "min-hops", String); - let min_hops_count = match min_hops_arg { - None => DEFAULT_MIN_HOPS_COUNT, - Some(string) => string - .try_into() - .unwrap_or_else(|error| panic!("{}", error)), - }; + let min_hops_count = + value_m!(multi_config, "min-hops", Hops).expect("clap schema specifies no default"); + // let min_hops_count = match min_hops_arg { + // None => DEFAULT_MIN_HOPS_COUNT, + // Some(string) => string + // .try_into() + // .unwrap_or_else(|error| panic!("{}", error)), + // }; match make_neighborhood_mode(multi_config, neighbor_configs, persistent_config) { Ok(mode) => Ok(NeighborhoodConfig { @@ -638,6 +638,7 @@ mod tests { use crate::sub_lib::neighborhood::{Hops, DEFAULT_RATE_PACK}; use crate::sub_lib::utils::make_new_multi_config; use crate::sub_lib::wallet::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::unshared_test_utils::{ configure_default_persistent_config, default_persistent_config_just_accountant_config, @@ -654,7 +655,6 @@ mod tests { use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::time::Duration; - use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST; #[test] fn convert_ci_configs_handles_blockchain_mismatch() { diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index 1d46c8c8e..b9186732c 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -379,11 +379,11 @@ pub enum Hops { SixHops = 6, } -impl TryFrom for Hops { - type Error = String; +impl FromStr for Hops { + type Err = String; - fn try_from(value: String) -> Result { - match value.as_str() { + fn from_str(value: &str) -> Result { + match value { "1" => Ok(Hops::OneHop), "2" => Ok(Hops::TwoHops), "3" => Ok(Hops::ThreeHops), @@ -1038,7 +1038,7 @@ mod tests { #[test] fn data_indefinite_route_request() { - let result = RouteQueryMessage::data_indefinite_route_request(None, 7500); + let result = RouteQueryMessage::data_indefinite_route_request(None, 7500); assert_eq!( result, @@ -1255,19 +1255,15 @@ mod tests { } #[test] - fn min_hops_count_can_be_converted_from_string() { - let min_hops_count = String::from("6"); - - let result: Hops = min_hops_count.try_into().unwrap(); + fn min_hops_count_can_be_converted_from_str() { + let result = Hops::from_str("6").unwrap(); assert_eq!(result, Hops::SixHops); } #[test] - fn min_hops_count_conversion_from_string_panics() { - let min_hops_count = String::from("100"); - - let result: Result = min_hops_count.try_into(); + fn min_hops_count_conversion_from_str_returns_error() { + let result = Hops::from_str("100"); assert_eq!( result, From f6f5991a262b04562e06edf8d3228d04036164f5 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Thu, 4 May 2023 11:45:16 +0530 Subject: [PATCH 09/18] GH-690: fix the bump script for macOS --- ci/bump_version.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/bump_version.sh b/ci/bump_version.sh index 1204b8173..de32eea3c 100755 --- a/ci/bump_version.sh +++ b/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 From 05ae02b881cc75666da7cae091874291786753c7 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Thu, 4 May 2023 17:24:24 +0530 Subject: [PATCH 10/18] GH-690: add todo for the min-hops arg to default properly --- node/src/daemon/setup_reporter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index ad64c7839..f4de41a99 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -1267,7 +1267,7 @@ mod tests { ("ip", "4.3.2.1", Set), ("log-level", "warn", Default), ("mapping-protocol", "", Blank), - ("min-hops", "3", Configured), // TODO: Shouldn't this be default? + ("min-hops", "3", Configured), // TODO: GH-698: This should be changed to Default after this card is played ("neighborhood-mode", "standard", Default), ( "neighbors", From 65db4f2131bd9692b75ddcbcab616d2bb6cdf60a Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Thu, 4 May 2023 17:31:50 +0530 Subject: [PATCH 11/18] GH-690: add todo inisde unpriveleged args --- .../unprivileged_parse_args_configuration.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index 3dbc61873..2b6d15d73 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -215,13 +215,7 @@ pub fn make_neighborhood_config( }; let min_hops_count = - value_m!(multi_config, "min-hops", Hops).expect("clap schema specifies no default"); - // let min_hops_count = match min_hops_arg { - // None => DEFAULT_MIN_HOPS_COUNT, - // Some(string) => string - // .try_into() - // .unwrap_or_else(|error| panic!("{}", error)), - // }; + value_m!(multi_config, "min-hops", Hops).expect("clap schema specifies no default"); // TODO: GH-690: Test Drive this panic match make_neighborhood_mode(multi_config, neighbor_configs, persistent_config) { Ok(mode) => Ok(NeighborhoodConfig { From 0273dcb9199c188819323aef84e33776d84df81c Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 11:24:03 +0530 Subject: [PATCH 12/18] GH-690: change the min_values to 0 for min-hops --- masq_lib/src/shared_schema.rs | 2 +- .../unprivileged_parse_args_configuration.rs | 29 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/masq_lib/src/shared_schema.rs b/masq_lib/src/shared_schema.rs index a051f87ba..4bc795d98 100644 --- a/masq_lib/src/shared_schema.rs +++ b/masq_lib/src/shared_schema.rs @@ -429,7 +429,7 @@ pub fn shared_app(head: App<'static, 'static>) -> App<'static, 'static> { .value_name("MIN_HOPS") .default_value("3") .required(false) - .min_values(1) // TODO: does this keep us being able to blank out + .min_values(0) .max_values(1) .possible_values(&["1", "2", "3", "4", "5", "6"]) .help(MIN_HOPS_HELP), diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index 2b6d15d73..b8e402bf3 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -750,7 +750,34 @@ mod tests { } #[test] - fn make_neighborhood_config_standard_panics_with_undesirable_min_hops_count() { + fn make_neighborhood_config_standard_uses_default_value_when_no_min_hops_count_value_is_provided( + ) { + running_test(); + let args = ArgsBuilder::new() + .param("--neighborhood-mode", "standard") + .param( + "--neighbors", + &format!("masq://{identifier}:QmlsbA@1.2.3.4:1234/2345,masq://{identifier}:VGVk@2.3.4.5:3456/4567",identifier = DEFAULT_CHAIN.rec().literal_identifier), + ) + .param("--fake-public-key", "booga") + .opt("--min-hops"); + let vcl = CommandLineVcl::new(args.into()); + let multi_config = make_new_multi_config(&app_node(), vec![Box::new(vcl)]).unwrap(); + + let result = make_neighborhood_config( + &UnprivilegedParseArgsConfigurationDaoReal {}, + &multi_config, + &mut configure_default_persistent_config(RATE_PACK), + &mut BootstrapperConfig::new(), + ); + + let min_hops_count = result.unwrap().min_hops_count; + assert_eq!(min_hops_count, Hops::ThreeHops); + } + + #[test] + fn make_neighborhood_config_standard_throws_err_when_undesirable_min_hops_count_value_is_provided( + ) { running_test(); let args = ArgsBuilder::new() .param("--neighborhood-mode", "standard") From 1cf77ba3cc65089f0d64c1c17507b3f67cb46cd1 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 12:42:27 +0530 Subject: [PATCH 13/18] GH-690: cleanup in node_configurator --- .../node_configurator_standard.rs | 7 +- .../unprivileged_parse_args_configuration.rs | 176 ++++++++---------- 2 files changed, 78 insertions(+), 105 deletions(-) diff --git a/node/src/node_configurator/node_configurator_standard.rs b/node/src/node_configurator/node_configurator_standard.rs index 0a6503e8b..82bbe40d3 100644 --- a/node/src/node_configurator/node_configurator_standard.rs +++ b/node/src/node_configurator/node_configurator_standard.rs @@ -284,7 +284,9 @@ mod tests { use crate::node_test_utils::DirsWrapperMock; use crate::sub_lib::cryptde::CryptDE; use crate::sub_lib::neighborhood::NeighborhoodMode::ZeroHop; - use crate::sub_lib::neighborhood::{NeighborhoodConfig, NeighborhoodMode, NodeDescriptor}; + use crate::sub_lib::neighborhood::{ + Hops, NeighborhoodConfig, NeighborhoodMode, NodeDescriptor, + }; use crate::sub_lib::wallet::Wallet; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::unshared_test_utils::{ @@ -304,7 +306,6 @@ mod tests { use std::path::PathBuf; use std::sync::{Arc, Mutex}; use std::vec; - use crate::test_utils::neighborhood_test_utils::MIN_HOPS_COUNT_FOR_TEST; #[test] fn node_configurator_standard_unprivileged_uses_parse_args_configurator_dao_real() { @@ -576,7 +577,7 @@ mod tests { config.neighborhood_config, NeighborhoodConfig { mode: NeighborhoodMode::ZeroHop, // not populated on the privileged side - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, + min_hops_count: Hops::ThreeHops, } ); assert_eq!( diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index b8e402bf3..586cbf368 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -632,7 +632,6 @@ mod tests { use crate::sub_lib::neighborhood::{Hops, DEFAULT_RATE_PACK}; use crate::sub_lib::utils::make_new_multi_config; use crate::sub_lib::wallet::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::unshared_test_utils::{ configure_default_persistent_config, default_persistent_config_just_accountant_config, @@ -827,7 +826,7 @@ mod tests { let node_addr = match result { Ok(NeighborhoodConfig { mode: NeighborhoodMode::Standard(node_addr, _, _), - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, + min_hops_count: Hops::ThreeHops, }) => node_addr, x => panic!("Wasn't expecting {:?}", x), }; @@ -860,33 +859,30 @@ mod tests { ); assert_eq!( - result, - Ok(NeighborhoodConfig { - mode: NeighborhoodMode::OriginateOnly( - vec![ - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:QmlsbA@1.2.3.4:1234/2345", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap(), - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:VGVk@2.3.4.5:3456/4567", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap() - ], - DEFAULT_RATE_PACK - ), - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - }) + result.unwrap().mode, + NeighborhoodMode::OriginateOnly( + vec![ + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:QmlsbA@1.2.3.4:1234/2345", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap(), + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:VGVk@2.3.4.5:3456/4567", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap() + ], + DEFAULT_RATE_PACK + ) ); } @@ -939,30 +935,27 @@ mod tests { ); assert_eq!( - result, - Ok(NeighborhoodConfig { - mode: NeighborhoodMode::ConsumeOnly(vec![ - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:QmlsbA@1.2.3.4:1234/2345", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap(), - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:VGVk@2.3.4.5:3456/4567", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap() - ],), - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - }) + result.unwrap().mode, + NeighborhoodMode::ConsumeOnly(vec![ + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:QmlsbA@1.2.3.4:1234/2345", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap(), + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:VGVk@2.3.4.5:3456/4567", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap() + ],), ); } @@ -1021,13 +1014,7 @@ mod tests { &mut BootstrapperConfig::new(), ); - assert_eq!( - result, - Ok(NeighborhoodConfig { - mode: NeighborhoodMode::ZeroHop, - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - }) - ); + assert_eq!(result.unwrap().mode, NeighborhoodMode::ZeroHop); } #[test] @@ -1352,13 +1339,7 @@ mod tests { ) .unwrap(); - assert_eq!( - config.neighborhood_config, - NeighborhoodConfig { - mode: NeighborhoodMode::ZeroHop, - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - } - ); + assert_eq!(config.neighborhood_config.mode, NeighborhoodMode::ZeroHop); let set_past_neighbors_params = set_past_neighbors_params_arc.lock().unwrap(); assert_eq!( *set_past_neighbors_params, @@ -1399,13 +1380,7 @@ mod tests { ) .unwrap(); - assert_eq!( - config.neighborhood_config, - NeighborhoodConfig { - mode: NeighborhoodMode::ZeroHop, - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - } - ); + assert_eq!(config.neighborhood_config.mode, NeighborhoodMode::ZeroHop); let set_past_neighbors_params = set_past_neighbors_params_arc.lock().unwrap(); assert!(set_past_neighbors_params.is_empty()) } @@ -1559,34 +1534,31 @@ mod tests { )), ); assert_eq!( - config.neighborhood_config, - NeighborhoodConfig { - mode: NeighborhoodMode::Standard( - NodeAddr::new(&IpAddr::from_str("34.56.78.90").unwrap(), &[]), - vec![ - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:QmlsbA@1.2.3.4:1234/2345", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap(), - NodeDescriptor::try_from(( - main_cryptde(), - format!( - "masq://{}:VGVk@2.3.4.5:3456/4567", - DEFAULT_CHAIN.rec().literal_identifier - ) - .as_str() - )) - .unwrap(), - ], - DEFAULT_RATE_PACK.clone() - ), - min_hops_count: MIN_HOPS_COUNT_FOR_TEST, - } + config.neighborhood_config.mode, + NeighborhoodMode::Standard( + NodeAddr::new(&IpAddr::from_str("34.56.78.90").unwrap(), &[]), + vec![ + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:QmlsbA@1.2.3.4:1234/2345", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap(), + NodeDescriptor::try_from(( + main_cryptde(), + format!( + "masq://{}:VGVk@2.3.4.5:3456/4567", + DEFAULT_CHAIN.rec().literal_identifier + ) + .as_str() + )) + .unwrap(), + ], + DEFAULT_RATE_PACK.clone() + ) ); assert_eq!(config.db_password_opt, Some(password.to_string())); assert_eq!(config.mapping_protocol_opt, Some(AutomapProtocol::Pcp)); From 91a4c1ab4915adf26680d6ea63cf5499fc33f4be Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 13:19:40 +0530 Subject: [PATCH 14/18] GH-690: cleanup in proxy server --- node/src/proxy_server/mod.rs | 31 +++++-------------------------- node/src/sub_lib/neighborhood.rs | 2 -- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/node/src/proxy_server/mod.rs b/node/src/proxy_server/mod.rs index cfd279e9a..0076b555e 100644 --- a/node/src/proxy_server/mod.rs +++ b/node/src/proxy_server/mod.rs @@ -33,9 +33,7 @@ use crate::sub_lib::peer_actors::BindMessage; use crate::sub_lib::proxy_client::{ClientResponsePayload_0v1, DnsResolveFailure_0v1}; use crate::sub_lib::proxy_server::ClientRequestPayload_0v1; use crate::sub_lib::proxy_server::ProxyServerSubs; -use crate::sub_lib::proxy_server::{ - AddReturnRouteMessage, AddRouteMessage, -}; +use crate::sub_lib::proxy_server::{AddReturnRouteMessage, AddRouteMessage}; use crate::sub_lib::route::Route; use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_handler_pool::TransmitDataMsg; @@ -952,12 +950,6 @@ impl IBCDHelperReal { route_source .send(RouteQueryMessage::data_indefinite_route_request( hostname_opt, - // TODO: GH-690: This edge case hasn't been take care - // if common_args.is_decentralized { - // DEFAULT_MINIMUM_HOP_COUNT - // } else { - // 0 - // }, payload_size, )) .then(move |route_result| { @@ -1291,10 +1283,7 @@ mod tests { let record = recording.get_record::(0); assert_eq!( record, - &RouteQueryMessage::data_indefinite_route_request( - Some("nowhere.com".to_string()), - 47 - ) + &RouteQueryMessage::data_indefinite_route_request(Some("nowhere.com".to_string()), 47) ); let recording = proxy_server_recording_arc.lock().unwrap(); assert_eq!(recording.len(), 0); @@ -1765,7 +1754,6 @@ mod tests { #[test] fn proxy_server_receives_http_request_with_no_consuming_wallet_in_zero_hop_mode_and_handles_normally( ) { - // TODO: GH-690: Initially this test is intended for a RouteQueryMessage with a min_hop_count = 0, but it is working fine irrespective of that init_test_logging(); let main_cryptde = main_cryptde(); let alias_cryptde = alias_cryptde(); @@ -2205,10 +2193,7 @@ mod tests { let record = recording.get_record::(0); assert_eq!( record, - &RouteQueryMessage::data_indefinite_route_request( - Some("nowhere.com".to_string()), - 47 - ) + &RouteQueryMessage::data_indefinite_route_request(Some("nowhere.com".to_string()), 47) ); } @@ -2720,10 +2705,7 @@ mod tests { let record = recording.get_record::(0); assert_eq!( record, - &RouteQueryMessage::data_indefinite_route_request( - Some("nowhere.com".to_string()), - 47 - ) + &RouteQueryMessage::data_indefinite_route_request(Some("nowhere.com".to_string()), 47) ); TestLogHandler::new() .exists_log_containing("ERROR: ProxyServer: Failed to find route to nowhere.com"); @@ -2895,10 +2877,7 @@ mod tests { let record = recording.get_record::(0); assert_eq!( record, - &RouteQueryMessage::data_indefinite_route_request( - Some("nowhere.com".to_string()), - 47 - ) + &RouteQueryMessage::data_indefinite_route_request(Some("nowhere.com".to_string()), 47) ); TestLogHandler::new() .exists_log_containing("ERROR: ProxyServer: Failed to find route to nowhere.com"); diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index b9186732c..6fec349d0 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -482,8 +482,6 @@ impl Message for RouteQueryMessage { } impl RouteQueryMessage { - // Earlier min_hops_count was passed to this function and stored inside RouteQueryMessage - // TODO: GH-690: Make sure the entities using RouteQueryMessage can easily retieve the min_hops_count from Neighborhood pub fn data_indefinite_route_request( hostname_opt: Option, payload_size: usize, From 250614f3a7c3b4201390a449f2bfadacf45bd9b5 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 13:23:37 +0530 Subject: [PATCH 15/18] GH-690: formatting --- multinode_integration_tests/src/masq_real_node.rs | 4 ++-- node/src/actor_system_factory.rs | 2 +- node/src/bootstrapper.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/multinode_integration_tests/src/masq_real_node.rs b/multinode_integration_tests/src/masq_real_node.rs index 2c02909fe..1027c329b 100644 --- a/multinode_integration_tests/src/masq_real_node.rs +++ b/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,7 +35,6 @@ 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; diff --git a/node/src/actor_system_factory.rs b/node/src/actor_system_factory.rs index cef49290a..537a0b3a5 100644 --- a/node/src/actor_system_factory.rs +++ b/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 {} diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index 0f2237f66..31ff25f1c 100644 --- a/node/src/bootstrapper.rs +++ b/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 = Mutex::new(false); From 02d77041e040d7c3463c4b0d0a73096ecac8c66d Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 13:52:49 +0530 Subject: [PATCH 16/18] GH-690: allow clippy warning for from_over_into --- node/src/sub_lib/neighborhood.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index 6fec349d0..be7e742a3 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -87,6 +87,7 @@ impl Display for NeighborhoodMode { } } +#[allow(clippy::from_over_into)] impl Into for &NeighborhoodMode { fn into(self) -> NeighborhoodModeLight { match self { From e9e4b255268e03824074dc1e79222241ef1b8fbd Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 13:55:06 +0530 Subject: [PATCH 17/18] GH-690: disable console diagnostics --- node/src/daemon/setup_reporter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index f4de41a99..8a753bf05 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -43,7 +43,7 @@ use std::net::{IpAddr, Ipv4Addr}; use std::path::{Path, PathBuf}; use std::str::FromStr; -const CONSOLE_DIAGNOSTICS: bool = true; // TODO: set back to false +const CONSOLE_DIAGNOSTICS: bool = false; const ERR_SENSITIVE_BLANKED_OUT_VALUE_PAIRS: &[(&str, &str)] = &[("chain", "data-directory")]; @@ -1110,7 +1110,7 @@ mod tests { #[test] fn constants_have_correct_values() { - assert_eq!(CONSOLE_DIAGNOSTICS, true); + assert_eq!(CONSOLE_DIAGNOSTICS, false); } pub struct DnsInspectorMock { From b73238db26192cdbffc0513578b5a8625f7c82cf Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Fri, 5 May 2023 14:07:09 +0530 Subject: [PATCH 18/18] GH-690: improve expect message if pulling out min_hops_count fails --- .../unprivileged_parse_args_configuration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index 586cbf368..c4c640199 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -214,8 +214,8 @@ pub fn make_neighborhood_config( } }; - let min_hops_count = - value_m!(multi_config, "min-hops", Hops).expect("clap schema specifies no default"); // TODO: GH-690: Test Drive this panic + let min_hops_count = value_m!(multi_config, "min-hops", Hops) + .expect("Clap schema didn't specified the default value."); match make_neighborhood_mode(multi_config, neighbor_configs, persistent_config) { Ok(mode) => Ok(NeighborhoodConfig {