GH-691: Integrate --min-hops with PersistentConfiguration (MASQ-P…

…roject#294)

* GH-691: database migration for 7 to 8

* GH-691: update CURRENT_SCHEMA_VERSION to 8

* GH-691: add test to set the min_hops_count inside PersistentConfiguration

* GH-691: fix tests in actor_system_factory.rs

* GH-691: add the ability to retrieve value of min_hops_count from the database

* GH-691: add tests for validating min hops count

* GH-691: test drive code for validating the min_hops_count value from the persistent configuration

* GH-691: minor code improvements

* GH-691: remove second declaration of CURRENT_SCHEMA_VERSION

* GH-691: remove unnecessary comments

* GH-691: improve validate_database_min_hops_count()

* GH-691: fixed test cleanup_after_deceased_clients_integration

* GH-691: add todos to where we should set the min_hops_count

* GH-691: set min_hops_count value inside configure_database()

* GH-691: remove code for setting min_hops_count inside actor_system_factory.rs

* GH-691: set min_hops_count default value while initializing database

* GH-691: change how min_hops_count is handled inside make_neighborhood_config()

* GH-691: instead of defaulting min_hops_count inside clap, default it in db

* GH-691: feed min_hops_count value inside ConfigDaoNull

* GH-691: add test for checking whether the node panics when the min_hops_count value is not found

* GH-691: remove clippy warnings

* GH-691: remove redundant code

* GH-691: more improvements to  validate_database_min_hops_count()

* GH-691: Review 1 (MASQ-Project#300)

* GH-691: minor changes in persistent_configuration.rs

* GH-691: Display for Hops can be simplified

* feat: panic when value of min_hops_count is not present in db

* GH-591: remove setting a result for set_min_hops_count inside actor_system_factory.rs

* GH-691: improvements in migration_7_to_8.rs

* fix: the todo has been changed to a comment after discussion in the dev team

* GH-691: Review 2 (MASQ-Project#302)

* GH-691: rename min_hops_count to min_hops everywhere

* GH-691: implement the computed_default()

* GH-691: implement the computed_default() for min hops

* GH-691: replace min hops value to the the one in db

* GH-691: rename DEFAULT_MIN_HOPS_COUNT and MIN_HOPS_COUNT_FOR_TEST to DEFAULT_MIN_HOPS and MIN_HOPS_FOR_TEST respectively

* GH-691: remove unnecessary test

* GH-691: Review 3 (MASQ-Project#304)

* GH-691: convert panic into error log inside the setup_reporter.rs

* GH-691: use Default instead of Configured if the default min hops value is found in db

* GH-691: change debug to info in the validation

* GH-691: use AssertionsMessage for asserting when the validation does not need to replace the value

* GH-691: remove use of min_hops_params from the tests in Neighborhood

* GH-691: remove min_hops_params

* GH-691: fix typo in test name

* GH-691: improve the log statement

* GH-691: fix the remaining renames (MASQ-Project#312)

masq_lib/src/constants.rs

@@ -5,7 +5,7 @@ use crate::data_version::DataVersion;
 use const_format::concatcp;
 
 pub const DEFAULT_CHAIN: Chain = Chain::PolyMainnet;
-pub const CURRENT_SCHEMA_VERSION: usize = 7;
+pub const CURRENT_SCHEMA_VERSION: usize = 8;
 
 pub const HIGHEST_RANDOM_CLANDESTINE_PORT: u16 = 9999;
 pub const HTTP_PORT: u16 = 80;

masq_lib/src/logger.rs

@@ -1,10 +1,9 @@
 use std::fmt::{Debug, Formatter};
 // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.
-use crate::constants::CURRENT_SCHEMA_VERSION;
 use crate::constants::{
     CLIENT_REQUEST_PAYLOAD_CURRENT_VERSION, CLIENT_RESPONSE_PAYLOAD_CURRENT_VERSION,
-    DNS_RESOLVER_FAILURE_CURRENT_VERSION, GOSSIP_CURRENT_VERSION, GOSSIP_FAILURE_CURRENT_VERSION,
-    NODE_RECORD_INNER_CURRENT_VERSION,
+    CURRENT_SCHEMA_VERSION, DNS_RESOLVER_FAILURE_CURRENT_VERSION, GOSSIP_CURRENT_VERSION,
+    GOSSIP_FAILURE_CURRENT_VERSION, NODE_RECORD_INNER_CURRENT_VERSION,
 };
 use crate::data_version::DataVersion;
 use crate::messages::SerializableLogLevel;

masq_lib/src/shared_schema.rs

@@ -431,7 +431,6 @@ 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(0)
             .max_values(1)

multinode_integration_tests/src/masq_real_node.rs

@@ -15,7 +15,7 @@ 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::neighborhood::DEFAULT_MIN_HOPS;
 use node_lib::sub_lib::accountant::{
     PaymentThresholds, DEFAULT_EARNING_WALLET, DEFAULT_PAYMENT_THRESHOLDS,
 };
@@ -114,7 +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 min_hops: Hops,
     pub ip_info: LocalIpInfo,
     pub dns_servers_opt: Option<Vec<IpAddr>>,
     pub neighbors: Vec<NodeReference>,
@@ -146,7 +146,7 @@ impl NodeStartupConfig {
     pub fn new() -> NodeStartupConfig {
         NodeStartupConfig {
             neighborhood_mode: "standard".to_string(),
-            min_hops_count: DEFAULT_MIN_HOPS_COUNT,
+            min_hops: DEFAULT_MIN_HOPS,
             ip_info: LocalIpInfo::ZeroHop,
             dns_servers_opt: None,
             neighbors: Vec::new(),
@@ -179,7 +179,7 @@ impl NodeStartupConfig {
         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));
+        args.push(format!("{}", self.min_hops as usize));
         if let LocalIpInfo::DistributedKnown(ip_addr) = self.ip_info {
             args.push("--ip".to_string());
             args.push(ip_addr.to_string());
@@ -409,7 +409,7 @@ impl NodeStartupConfig {
 
 pub struct NodeStartupConfigBuilder {
     neighborhood_mode: String,
-    min_hops_count: Hops,
+    min_hops: Hops,
     ip_info: LocalIpInfo,
     dns_servers_opt: Option<Vec<IpAddr>>,
     neighbors: Vec<NodeReference>,
@@ -465,7 +465,7 @@ impl NodeStartupConfigBuilder {
     pub fn standard() -> Self {
         Self {
             neighborhood_mode: "standard".to_string(),
-            min_hops_count: DEFAULT_MIN_HOPS_COUNT,
+            min_hops: DEFAULT_MIN_HOPS,
             ip_info: LocalIpInfo::DistributedUnknown,
             dns_servers_opt: None,
             neighbors: vec![],
@@ -491,7 +491,7 @@ impl NodeStartupConfigBuilder {
     pub fn copy(config: &NodeStartupConfig) -> Self {
         Self {
             neighborhood_mode: config.neighborhood_mode.clone(),
-            min_hops_count: config.min_hops_count,
+            min_hops: config.min_hops,
             ip_info: config.ip_info,
             dns_servers_opt: config.dns_servers_opt.clone(),
             neighbors: config.neighbors.clone(),
@@ -530,8 +530,8 @@ impl NodeStartupConfigBuilder {
         }
     }
 
-    pub fn min_hops_count(mut self, value: Hops) -> Self {
-        self.min_hops_count = value;
+    pub fn min_hops(mut self, value: Hops) -> Self {
+        self.min_hops = value;
         self
     }
 
@@ -648,7 +648,7 @@ impl NodeStartupConfigBuilder {
     pub fn build(self) -> NodeStartupConfig {
         NodeStartupConfig {
             neighborhood_mode: self.neighborhood_mode,
-            min_hops_count: self.min_hops_count,
+            min_hops: self.min_hops,
             ip_info: self.ip_info,
             dns_servers_opt: self.dns_servers_opt,
             neighbors: self.neighbors,
@@ -1304,7 +1304,7 @@ mod tests {
 
     #[test]
     fn node_startup_config_builder_settings() {
-        let min_hops_count = Hops::SixHops;
+        let min_hops = 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();
@@ -1333,7 +1333,7 @@ mod tests {
         let dns_target = IpAddr::from_str("8.9.10.11").unwrap();
 
         let result = NodeStartupConfigBuilder::standard()
-            .min_hops_count(min_hops_count)
+            .min_hops(min_hops)
             .ip(ip_addr)
             .dns_servers(dns_servers.clone())
             .neighbor(neighbors[0].clone())
@@ -1342,7 +1342,7 @@ mod tests {
             .dns_port(35)
             .build();
 
-        assert_eq!(result.min_hops_count, min_hops_count);
+        assert_eq!(result.min_hops, min_hops);
         assert_eq!(result.ip_info, LocalIpInfo::DistributedKnown(ip_addr));
         assert_eq!(result.dns_servers_opt, Some(dns_servers));
         assert_eq!(result.neighbors, neighbors);
@@ -1355,7 +1355,7 @@ mod tests {
     fn node_startup_config_builder_copy() {
         let original = NodeStartupConfig {
             neighborhood_mode: "consume-only".to_string(),
-            min_hops_count: Hops::TwoHops,
+            min_hops: Hops::TwoHops,
             ip_info: LocalIpInfo::DistributedUnknown,
             dns_servers_opt: Some(vec![IpAddr::from_str("255.255.255.255").unwrap()]),
             neighbors: vec![NodeReference::new(
@@ -1434,7 +1434,7 @@ mod tests {
             .build();
 
         assert_eq!(result.neighborhood_mode, neighborhood_mode);
-        assert_eq!(result.min_hops_count, Hops::TwoHops);
+        assert_eq!(result.min_hops, 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);
@@ -1501,7 +1501,7 @@ mod tests {
 
         let subject = NodeStartupConfigBuilder::standard()
             .neighborhood_mode("consume-only")
-            .min_hops_count(Hops::SixHops)
+            .min_hops(Hops::SixHops)
             .ip(IpAddr::from_str("1.3.5.7").unwrap())
             .neighbor(one_neighbor.clone())
             .neighbor(another_neighbor.clone())

multinode_integration_tests/tests/data_routing_test.rs

@@ -70,16 +70,16 @@ fn http_end_to_end_routing_test() {
     );
 }
 
-fn assert_http_end_to_end_routing_test(min_hops_count: Hops) {
+fn assert_http_end_to_end_routing_test(min_hops: Hops) {
     let mut cluster = MASQNodeCluster::start().unwrap();
     let config = NodeStartupConfigBuilder::standard()
-        .min_hops_count(min_hops_count)
+        .min_hops(min_hops)
         .chain(cluster.chain)
         .consuming_wallet_info(make_consuming_wallet_info("first_node"))
         .build();
     let first_node = cluster.start_real_node(config);
 
-    let nodes_count = 2 * (min_hops_count as usize) + 1;
+    let nodes_count = 2 * (min_hops as usize) + 1;
     let nodes = (0..nodes_count)
         .map(|_| {
             cluster.start_real_node(
@@ -106,7 +106,7 @@ 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() {
+fn http_end_to_end_routing_test_with_different_min_hops() {
     // 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);

node/src/actor_system_factory.rs

@@ -642,7 +642,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::neighborhood_test_utils::MIN_HOPS_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,
@@ -1084,13 +1084,14 @@ mod tests {
                     vec![],
                     rate_pack(100),
                 ),
-                min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+                min_hops: MIN_HOPS_FOR_TEST,
             },
             payment_thresholds_opt: Some(PaymentThresholds::default()),
             when_pending_too_long_sec: DEFAULT_PENDING_TOO_LONG_SEC,
         };
-        let persistent_config =
-            PersistentConfigurationMock::default().chain_name_result("eth-ropsten".to_string());
+        let persistent_config = PersistentConfigurationMock::default()
+            .chain_name_result("eth-ropsten".to_string())
+            .set_min_hops_result(Ok(()));
         Bootstrapper::pub_initialize_cryptdes_for_testing(
             &Some(main_cryptde()),
             &Some(alias_cryptde()),
@@ -1158,7 +1159,7 @@ mod tests {
                     vec![],
                     rate_pack(100),
                 ),
-                min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+                min_hops: MIN_HOPS_FOR_TEST,
             },
             payment_thresholds_opt: Default::default(),
             when_pending_too_long_sec: DEFAULT_PENDING_TOO_LONG_SEC
@@ -1276,6 +1277,7 @@ mod tests {
         let mut config = BootstrapperConfig::default();
         config.neighborhood_config = NeighborhoodConfig {
             mode: NeighborhoodMode::ConsumeOnly(vec![]),
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let subject = ActorSystemFactoryToolsReal::new();
         let state_before = INITIALIZATION_COUNTER.lock().unwrap().0;
@@ -1302,7 +1304,7 @@ mod tests {
                 vec![],
                 rate_pack(100),
             ),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let make_params_arc = Arc::new(Mutex::new(vec![]));
         let mut subject = make_subject_with_null_setter();
@@ -1456,7 +1458,7 @@ mod tests {
             real_user: RealUser::null(),
             neighborhood_config: NeighborhoodConfig {
                 mode: NeighborhoodMode::ConsumeOnly(vec![]),
-                min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+                min_hops: MIN_HOPS_FOR_TEST,
             },
             payment_thresholds_opt: Default::default(),
             when_pending_too_long_sec: DEFAULT_PENDING_TOO_LONG_SEC
@@ -1468,7 +1470,7 @@ mod tests {
         let _ = subject.prepare_initial_messages(
             make_cryptde_pair(),
             config.clone(),
-            Box::new(PersistentConfigurationMock::new()),
+            Box::new(PersistentConfigurationMock::new().set_min_hops_result(Ok(()))),
             Box::new(actor_factory),
         );
 
@@ -1645,7 +1647,7 @@ mod tests {
                     vec![],
                     rate_pack(100),
                 ),
-                min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+                min_hops: MIN_HOPS_FOR_TEST,
             },
             node_descriptor: Default::default(),
             payment_thresholds_opt: Default::default(),
@@ -1657,7 +1659,7 @@ mod tests {
         let _ = subject.prepare_initial_messages(
             make_cryptde_pair(),
             config.clone(),
-            Box::new(PersistentConfigurationMock::new()),
+            Box::new(PersistentConfigurationMock::new().set_min_hops_result(Ok(()))),
             Box::new(actor_factory),
         );
 

node/src/bootstrapper.rs

@@ -15,7 +15,7 @@ use crate::json_discriminator_factory::JsonDiscriminatorFactory;
 use crate::listener_handler::ListenerHandler;
 use crate::listener_handler::ListenerHandlerFactory;
 use crate::listener_handler::ListenerHandlerFactoryReal;
-use crate::neighborhood::DEFAULT_MIN_HOPS_COUNT;
+use crate::neighborhood::DEFAULT_MIN_HOPS;
 use crate::node_configurator::node_configurator_standard::{
     NodeConfiguratorStandardPrivileged, NodeConfiguratorStandardUnprivileged,
 };
@@ -398,7 +398,7 @@ impl BootstrapperConfig {
             consuming_wallet_opt: None,
             neighborhood_config: NeighborhoodConfig {
                 mode: NeighborhoodMode::ZeroHop,
-                min_hops_count: DEFAULT_MIN_HOPS_COUNT,
+                min_hops: DEFAULT_MIN_HOPS,
             },
             when_pending_too_long_sec: DEFAULT_PENDING_TOO_LONG_SEC,
         }
@@ -741,7 +741,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::neighborhood_test_utils::MIN_HOPS_FOR_TEST;
     use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock;
     use crate::test_utils::recorder::make_recorder;
     use crate::test_utils::recorder::RecordAwaiter;
@@ -1234,7 +1234,7 @@ mod tests {
         let clandestine_port_opt = Some(44444);
         let neighborhood_config = NeighborhoodConfig {
             mode: NeighborhoodMode::OriginateOnly(vec![], rate_pack(9)),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let earning_wallet = make_wallet("earning wallet");
         let consuming_wallet_opt = Some(make_wallet("consuming wallet"));
@@ -1853,7 +1853,7 @@ mod tests {
                 ))],
                 rate_pack(100),
             ),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         config.data_directory = data_dir.clone();
         config.clandestine_port_opt = Some(port);
@@ -1923,7 +1923,7 @@ mod tests {
                 ))],
                 rate_pack(100),
             ),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         config.data_directory = data_dir.clone();
         config.clandestine_port_opt = None;
@@ -1972,7 +1972,7 @@ mod tests {
                 ))],
                 rate_pack(100),
             ),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let listener_handler = ListenerHandlerNull::new(vec![]);
         let mut subject = BootstrapperBuilder::new()
@@ -2009,7 +2009,7 @@ mod tests {
                 Chain::EthRopsten,
                 cryptde,
             ))]),
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let listener_handler = ListenerHandlerNull::new(vec![]);
         let mut subject = BootstrapperBuilder::new()
@@ -2039,7 +2039,7 @@ mod tests {
         config.clandestine_port_opt = None;
         config.neighborhood_config = NeighborhoodConfig {
             mode: NeighborhoodMode::ZeroHop,
-            min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+            min_hops: MIN_HOPS_FOR_TEST,
         };
         let listener_handler = ListenerHandlerNull::new(vec![]);
         let mut subject = BootstrapperBuilder::new()
@@ -2070,7 +2070,7 @@ mod tests {
             config.data_directory = data_dir.to_path_buf();
             config.neighborhood_config = NeighborhoodConfig {
                 mode: NeighborhoodMode::Standard(NodeAddr::default(), vec![], DEFAULT_RATE_PACK),
-                min_hops_count: MIN_HOPS_COUNT_FOR_TEST,
+                min_hops: MIN_HOPS_FOR_TEST,
             };
             let mut subject = BootstrapperBuilder::new().config(config).build();
             subject.set_up_clandestine_port();