GH-606: Apply PR feedback changes

masq/src/commands/set_configuration_command.rs

@@ -7,6 +7,7 @@ use masq_lib::shared_schema::gas_price_arg;
 use masq_lib::shared_schema::min_hops_arg;
 use masq_lib::short_writeln;
 use masq_lib::utils::ExpectValue;
+use std::num::IntErrorKind;
 
 #[derive(Debug, PartialEq, Eq)]
 pub struct SetConfigurationCommand {
@@ -40,7 +41,11 @@ fn validate_start_block(start_block: String) -> Result<(), String> {
     } else {
         match start_block.parse::<u64>() {
             Ok(_) => Ok(()),
-            _ => Err(start_block),
+            Err(e) if e.kind() == &IntErrorKind::PosOverflow => Err(
+                format!("Unable to parse '{}' into a starting block number or provide 'none' or 'latest' for the latest block number: digits exceed {}.",
+                        start_block, u64::MAX),
+            ),
+            Err(e) => Err(format!("Unable to parse '{}' into a starting block number or provide 'none' or 'latest' for the latest block number: {}.", start_block, e))
         }
     }
 }
@@ -126,12 +131,28 @@ mod tests {
         assert!(result.contains("cannot be used with one or more of the other specified arguments"));
     }
 
+    #[test]
+    fn validate_start_block_catches_invalid_values() {
+        assert_eq!(validate_start_block("abc".to_string()), Err("Unable to parse 'abc' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string()));
+        assert_eq!(validate_start_block("918446744073709551615".to_string()), Err("Unable to parse '918446744073709551615' into a starting block number or provide 'none' or 'latest' for the latest block number: digits exceed 18446744073709551615.".to_string()));
+        assert_eq!(validate_start_block("123,456,789".to_string()), Err("Unable to parse '123,456,789' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string()));
+        assert_eq!(validate_start_block("123'456'789".to_string()), Err("Unable to parse '123'456'789' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string()));
+    }
     #[test]
     fn validate_start_block_works() {
-        assert!(validate_start_block("abc".to_string()).is_err());
-        assert!(validate_start_block("1566".to_string()).is_ok());
-        assert!(validate_start_block("latest".to_string()).is_ok());
-        assert!(validate_start_block("none".to_string()).is_ok());
+        assert_eq!(
+            validate_start_block("18446744073709551615".to_string()),
+            Ok(())
+        );
+        assert_eq!(validate_start_block("1566".to_string()), Ok(()));
+        assert_eq!(validate_start_block("none".to_string()), Ok(()));
+        assert_eq!(validate_start_block("None".to_string()), Ok(()));
+        assert_eq!(validate_start_block("NONE".to_string()), Ok(()));
+        assert_eq!(validate_start_block("nOnE".to_string()), Ok(()));
+        assert_eq!(validate_start_block("latest".to_string()), Ok(()));
+        assert_eq!(validate_start_block("LATEST".to_string()), Ok(()));
+        assert_eq!(validate_start_block("LaTeST".to_string()), Ok(()));
+        assert_eq!(validate_start_block("lATEst".to_string()), Ok(()));
     }
 
     #[test]

node/src/blockchain/blockchain_bridge.rs

@@ -309,33 +309,39 @@ impl BlockchainBridge {
             Ok(Some(mbc)) => mbc,
             _ => u64::MAX,
         };
+        let use_unlimited_block_count_range = u64::MAX == max_block_count;
+        let use_latest_block = u64::MAX == start_block_nbr;
         let end_block = match self
             .blockchain_interface
             .lower_interface()
             .get_block_number()
         {
             Ok(eb) => {
-                if u64::MAX == max_block_count || u64::MAX == start_block_nbr {
+                if use_unlimited_block_count_range || use_latest_block {
                     BlockNumber::Number(eb)
                 } else {
                     BlockNumber::Number(eb.as_u64().min(start_block_nbr + max_block_count).into())
                 }
             }
             Err(e) => {
-                if max_block_count == u64::MAX {
-                    info!(
+                if use_unlimited_block_count_range || use_latest_block {
+                    debug!(
                         self.logger,
                         "Using 'latest' block number instead of a literal number. {:?}", e
                     );
                     BlockNumber::Latest
-                } else if u64::MAX == start_block_nbr {
-                    BlockNumber::Latest
                 } else {
+                    debug!(
+                        self.logger,
+                        "Using '{}' ending block number. {:?}",
+                        start_block_nbr + max_block_count,
+                        e
+                    );
                     BlockNumber::Number((start_block_nbr + max_block_count).into())
                 }
             }
         };
-        let start_block = if u64::MAX == start_block_nbr {
+        let start_block = if use_latest_block {
             end_block
         } else {
             BlockNumber::Number(start_block_nbr.into())
@@ -1426,11 +1432,10 @@ mod tests {
                 },
             ],
         };
-        let lower_interface = LowBlockchainIntMock::default().get_block_number_result(
-            LatestBlockNumber::Err(BlockchainError::QueryFailed(
-                "\"Failed to read the latest block number\"".to_string(),
-            )),
-        );
+        let lower_interface =
+            LowBlockchainIntMock::default().get_block_number_result(LatestBlockNumber::Err(
+                BlockchainError::QueryFailed("Failed to read the latest block number".to_string()),
+            ));
         let blockchain_interface_mock = BlockchainInterfaceMock::default()
             .retrieve_transactions_params(&retrieve_transactions_params_arc)
             .retrieve_transactions_result(Ok(expected_transactions.clone()))
@@ -1492,8 +1497,96 @@ mod tests {
                 }),
             }
         );
-        TestLogHandler::new().exists_log_containing(
-            "INFO: BlockchainBridge: Using 'latest' block number instead of a literal number.",
+        TestLogHandler::new().exists_log_containing("DEBUG: BlockchainBridge: Using 'latest' block number instead of a literal number. QueryFailed(\"Failed to read the latest block number\")");
+    }
+
+    #[test]
+    fn handle_retrieve_transactions_when_start_block_number_starts_undefined_in_a_brand_new_database(
+    ) {
+        let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![]));
+        let system = System::new(
+            "handle_retrieve_transactions_when_start_block_number_starts_undefined_in_a_brand_new_database",
+        );
+        let (accountant, _, accountant_recording_arc) = make_recorder();
+        let earning_wallet = make_wallet("somewallet");
+        let amount = 42;
+        let amount2 = 55;
+        let expected_transactions = RetrievedBlockchainTransactions {
+            new_start_block: 8675309u64,
+            transactions: vec![
+                BlockchainTransaction {
+                    block_number: 8675308u64,
+                    from: earning_wallet.clone(),
+                    wei_amount: amount,
+                },
+                BlockchainTransaction {
+                    block_number: 8675309u64,
+                    from: earning_wallet.clone(),
+                    wei_amount: amount2,
+                },
+            ],
+        };
+        let lower_interface = LowBlockchainIntMock::default().get_block_number_result(
+            LatestBlockNumber::Err(BlockchainError::QueryFailed(
+                "\"Failed to read the latest block number\"".to_string(),
+            )),
+        );
+        let blockchain_interface_mock = BlockchainInterfaceMock::default()
+            .retrieve_transactions_params(&retrieve_transactions_params_arc)
+            .retrieve_transactions_result(Ok(expected_transactions.clone()))
+            .lower_interface_results(Box::new(lower_interface));
+        let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
+        let persistent_config = PersistentConfigurationMock::new()
+            .max_block_count_result(Ok(None))
+            .start_block_result(Ok(None))
+            .set_start_block_params(&set_start_block_params_arc)
+            .set_start_block_result(Ok(()));
+        let subject = BlockchainBridge::new(
+            Box::new(blockchain_interface_mock),
+            Box::new(persistent_config),
+            false,
+            Some(make_wallet("consuming")),
+        );
+        let addr = subject.start();
+        let subject_subs = BlockchainBridge::make_subs_from(&addr);
+        let peer_actors = peer_actors_builder().accountant(accountant).build();
+        send_bind_message!(subject_subs, peer_actors);
+        let retrieve_transactions = RetrieveTransactions {
+            recipient: earning_wallet.clone(),
+            response_skeleton_opt: Some(ResponseSkeleton {
+                client_id: 1234,
+                context_id: 4321,
+            }),
+        };
+        let before = SystemTime::now();
+
+        let _ = addr.try_send(retrieve_transactions).unwrap();
+
+        System::current().stop();
+        system.run();
+        let after = SystemTime::now();
+        let set_start_block_params = set_start_block_params_arc.lock().unwrap();
+        assert_eq!(*set_start_block_params, vec![Some(8675309u64)]);
+        let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap();
+        assert_eq!(
+            *retrieve_transactions_params,
+            vec![(BlockNumber::Latest, BlockNumber::Latest, earning_wallet)]
+        );
+        let accountant_received_payment = accountant_recording_arc.lock().unwrap();
+        assert_eq!(accountant_received_payment.len(), 1);
+        let received_payments = accountant_received_payment.get_record::<ReceivedPayments>(0);
+        check_timestamp(before, received_payments.timestamp, after);
+        assert_eq!(
+            received_payments,
+            &ReceivedPayments {
+                timestamp: received_payments.timestamp,
+                payments: expected_transactions.transactions,
+                new_start_block: 8675309u64,
+                response_skeleton_opt: Some(ResponseSkeleton {
+                    client_id: 1234,
+                    context_id: 4321
+                }),
+            }
         );
     }
 

node/src/database/config_dumper.rs

@@ -353,8 +353,7 @@ mod tests {
         );
         assert_value("neighborhoodMode", "zero-hop", &map);
         assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map);
-        assert!(map.contains_key("startBlock"));
-        assert_none("startBlock", &map);
+        assert_null("startBlock", &map);
         assert_value(
             "exampleEncrypted",
             &dao.get("example_encrypted").unwrap().value_opt.unwrap(),
@@ -500,8 +499,7 @@ mod tests {
         assert_value("pastNeighbors", "masq://polygon-mainnet:[email protected]:1234,masq://polygon-mainnet:[email protected]:2345", &map);
         assert_value("neighborhoodMode", "consume-only", &map);
         assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map);
-        assert!(map.contains_key("startBlock"));
-        assert_none("startBlock", &map);
+        assert_null("startBlock", &map);
         let expected_ee_entry = dao.get("example_encrypted").unwrap().value_opt.unwrap();
         let expected_ee_decrypted = Bip39::decrypt_bytes(&expected_ee_entry, "password").unwrap();
         let expected_ee_string = encode_bytes(Some(expected_ee_decrypted)).unwrap().unwrap();
@@ -614,8 +612,7 @@ mod tests {
         );
         assert_value("neighborhoodMode", "standard", &map);
         assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map);
-        assert!(map.contains_key("startBlock"));
-        assert_none("startBlock", &map);
+        assert_null("startBlock", &map);
         assert_value(
             "exampleEncrypted",
             &dao.get("example_encrypted").unwrap().value_opt.unwrap(),
@@ -670,8 +667,14 @@ mod tests {
         assert_eq!(actual_value, expected_value);
     }
 
-    fn assert_none(key: &str, map: &Map<String, Value>) {
-        assert!(!map.get(key).is_none());
+    fn assert_null(key: &str, map: &Map<String, Value>) {
+        assert!(map.contains_key(key));
+        match map
+            .get(key)
+            .unwrap_or_else(|| panic!("record for {} is missing", key))
+        {
+            value => assert!(value.is_null()),
+        }
     }
 
     fn assert_encrypted_value(

node/src/node_configurator/configurator.rs

@@ -2046,10 +2046,6 @@ mod tests {
         assert_eq!(context_id, 4444);
         let check_start_block_params = set_start_block_params_arc.lock().unwrap();
         assert_eq!(*check_start_block_params, vec![Some(166666)]);
-        TestLogHandler::new().exists_log_containing(&format!(
-            "DEBUG: {}: A request from UI received: {:?} from context id: {}",
-            test_name, msg, context_id
-        ));
     }
 
     #[test]
@@ -2088,10 +2084,6 @@ mod tests {
         assert_eq!(context_id, 4444);
         let check_start_block_params = set_start_block_params_arc.lock().unwrap();
         assert_eq!(*check_start_block_params, vec![None]);
-        TestLogHandler::new().exists_log_containing(&format!(
-            "DEBUG: {}: A request from UI received: {:?} from context id: {}",
-            test_name, msg, context_id
-        ));
     }
 
     #[test]

node/src/test_utils/database_utils.rs

@@ -40,27 +40,6 @@ pub fn bring_db_0_back_to_life_and_return_connection(db_path: &Path) -> Connecti
     conn
 }
 
-pub fn bring_db_9_back_to_life_and_return_connection(db_path: &Path) -> Connection {
-    match remove_file(db_path) {
-        Err(e) if e.kind() == std::io::ErrorKind::NotFound => (),
-        Err(e) => panic!("Unexpected but serious error: {}", e),
-        _ => (),
-    }
-    let file_path = current_dir()
-        .unwrap()
-        .join("src")
-        .join("test_utils")
-        .join("database_version_9_sql.txt");
-    let mut file = File::open(file_path).unwrap();
-    let mut buffer = String::new();
-    file.read_to_string(&mut buffer).unwrap();
-    let conn = Connection::open(&db_path).unwrap();
-    buffer.lines().for_each(|stm| {
-        conn.execute(stm, []).unwrap();
-    });
-    conn
-}
-
 #[derive(Default)]
 pub struct DbMigratorMock {
     logger: Option<Logger>,