GH-606: Squashing commits

- Save start_block_nbr if no msg but send as non-Option
- Always commit
- Reduce logging levels and simplify
- Follow the Option naming pattern

multinode_integration_tests/docker/blockchain/Dockerfile

@@ -1,8 +1,8 @@
 # Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.
-FROM trufflesuite/ganache-cli:v6.7.0
+FROM trufflesuite/ganache-cli:v6.12.2
 
 ADD ./entrypoint.sh /app/
 
 EXPOSE 18545
 
-ENTRYPOINT /app/entrypoint.sh
+ENTRYPOINT ["/app/entrypoint.sh"]

multinode_integration_tests/docker/blockchain/entrypoint.sh

@@ -1,3 +1,8 @@
 #!/bin/sh
 
-node /app/ganache-core.docker.cli.js -p 18545 --networkId 2 --verbose --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold"
+node /app/ganache-core.docker.cli.js \
+    -h 0.0.0.0 \
+    -p 18545 \
+    --networkId 2 \
+    --verbose \
+    --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold"

multinode_integration_tests/src/main.rs

@@ -1,11 +1,10 @@
 // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.
 
-use self::sub_lib::utils::indicates_dead_stream;
 use masq_lib::command::{Command, StdStreams};
 use masq_lib::constants::{HIGHEST_USABLE_PORT, LOWEST_USABLE_INSECURE_PORT};
-use node_lib::sub_lib;
 use node_lib::sub_lib::framer::Framer;
 use node_lib::sub_lib::node_addr::NodeAddr;
+use node_lib::sub_lib::utils::indicates_dead_stream;
 use node_lib::test_utils::data_hunk::DataHunk;
 use node_lib::test_utils::data_hunk_framer::DataHunkFramer;
 use std::borrow::BorrowMut;
@@ -14,10 +13,9 @@ use std::env;
 use std::io;
 use std::io::Read;
 use std::io::Write;
-use std::net::Shutdown;
-use std::net::SocketAddr;
 use std::net::TcpListener;
 use std::net::TcpStream;
+use std::net::{Shutdown, SocketAddr};
 use std::process;
 use std::str::FromStr;
 use std::sync::{Arc, Mutex, MutexGuard};
@@ -223,10 +221,10 @@ impl MockNode {
     }
 
     fn usage(stderr: &mut dyn Write) -> u8 {
-        writeln! (stderr, "Usage: MockNode <IP address>:<port>/<port>/... where <IP address> is the address MockNode is running on and <port> is between {} and {}",
-            LOWEST_USABLE_INSECURE_PORT,
-            HIGHEST_USABLE_PORT,
-        ).unwrap ();
+        writeln!(stderr, "Usage: MockNode <IP address>:<port>/<port>/... where <IP address> is the address MockNode is running on and <port> is between {} and {}",
+                 LOWEST_USABLE_INSECURE_PORT,
+                 HIGHEST_USABLE_PORT,
+        ).unwrap();
         1
     }
 
@@ -369,7 +367,7 @@ mod tests {
 
         assert_eq!(result, 1);
         let stderr = holder.stderr;
-        assert_eq! (stderr.get_string (), String::from ("Usage: MockNode <IP address>:<port>/<port>/... where <IP address> is the address MockNode is running on and <port> is between 1025 and 65535\n\n"));
+        assert_eq!(stderr.get_string(), String::from("Usage: MockNode <IP address>:<port>/<port>/... where <IP address> is the address MockNode is running on and <port> is between 1025 and 65535\n\n"));
     }
 
     #[test]

multinode_integration_tests/src/mock_blockchain_client_server.rs

@@ -241,8 +241,10 @@ impl MockBlockchainClientServer {
                         let mut requests = requests_arc.lock().unwrap();
                         requests.push(body);
                     }
-                    let response = responses.remove(0);
-                    Self::send_body(conn_state, response);
+                    if !responses.is_empty() {
+                        let response = responses.remove(0);
+                        Self::send_body(conn_state, response);
+                    }
                     let _ = notifier_tx.send(()); // receiver doesn't exist if test didn't set it up
                 }
                 None => (),
@@ -437,7 +439,7 @@ mod tests {
             .response("Thank you and good night", 40)
             .start();
         let mut client = connect(port);
-        client.write (b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap();
+        client.write(b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap();
 
         let (_, body) = receive_response(&mut client);
         assert_eq!(
@@ -567,7 +569,7 @@ mod tests {
         assert_eq!(notified.try_recv().is_err(), true);
 
         let requests = subject.requests();
-        assert_eq! (requests, vec! [
+        assert_eq!(requests, vec![
             "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 82\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"first\", \"params\": [\"biddle\", \"de\", \"bee\"], \"id\": 40}".to_string(),
             "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 48\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"second\", \"id\": 42}".to_string(),
             "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 47\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"third\", \"id\": 42}".to_string(),
@@ -600,7 +602,7 @@ mod tests {
             r#"{"jsonrpc": "2.0", "result": {"name":"Billy","age":15}, "id": 42}"#
         );
         let requests = subject.requests();
-        assert_eq! (requests, vec! [
+        assert_eq!(requests, vec![
             "POST / HTTP/1.1\r\ncontent-type: application/json\r\nuser-agent: web3.rs\r\nhost: 172.18.0.1:32768\r\ncontent-length: 308\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{\"address\":\"0x59882e4a8f5d24643d4dda422922a870f1b3e664\",\"fromBlock\":\"0x3e8\",\"toBlock\":\"latest\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x00000000000000000000000027d9a2ac83b493f88ce9b4532edcf74e95b9788d\"]}],\"id\":0}".to_string()
         ])
     }
@@ -704,6 +706,6 @@ mod tests {
             body.len(),
             body
         )
-        .into_bytes()
+            .into_bytes()
     }
 }

multinode_integration_tests/tests/verify_bill_payment.rs

@@ -38,10 +38,7 @@ use web3::Web3;
 
 #[test]
 fn verify_bill_payment() {
-    let mut cluster = match MASQNodeCluster::start() {
-        Ok(cluster) => cluster,
-        Err(e) => panic!("{}", e),
-    };
+    let mut cluster = MASQNodeCluster::start().unwrap();
     let blockchain_server = BlockchainServer {
         name: "ganache-cli",
     };
@@ -64,7 +61,7 @@ fn verify_bill_payment() {
     assert_balances(
         &contract_owner_wallet,
         &blockchain_interface,
-        "99998043204000000000",
+        "99998381140000000000",
         "472000000000000000000000000",
     );
     let payment_thresholds = PaymentThresholds {
@@ -189,7 +186,7 @@ fn verify_bill_payment() {
     assert_balances(
         &contract_owner_wallet,
         &blockchain_interface,
-        "99998043204000000000",
+        "99998381140000000000",
         "472000000000000000000000000",
     );
 
@@ -235,7 +232,7 @@ fn verify_bill_payment() {
     assert_balances(
         &contract_owner_wallet,
         &blockchain_interface,
-        "99997886466000000000",
+        "99998223682000000000",
         "471999999700000000000000000",
     );
 
@@ -330,7 +327,7 @@ fn assert_balances(
     assert_eq!(
         format!("{}", eth_balance),
         String::from(expected_eth_balance),
-        "Actual EthBalance {} doesn't much with expected {}",
+        "Actual EthBalance {} doesn't match with expected {}",
         eth_balance,
         expected_eth_balance
     );

node/src/accountant/mod.rs

@@ -124,7 +124,7 @@ pub struct ReceivedPayments {
     // a problem? Do we want to correct the timestamp? Discuss.
     pub timestamp: SystemTime,
     pub payments: Vec<BlockchainTransaction>,
-    pub new_start_block: Option<u64>,
+    pub new_start_block: u64,
     pub response_skeleton_opt: Option<ResponseSkeleton>,
 }
 
@@ -1388,7 +1388,7 @@ mod tests {
         let received_payments = ReceivedPayments {
             timestamp: SystemTime::now(),
             payments: vec![],
-            new_start_block: Some(1234567),
+            new_start_block: 1234567,
             response_skeleton_opt: Some(ResponseSkeleton {
                 client_id: 1234,
                 context_id: 4321,
@@ -2036,7 +2036,7 @@ mod tests {
             .try_send(ReceivedPayments {
                 timestamp: now,
                 payments: vec![expected_receivable_1.clone(), expected_receivable_2.clone()],
-                new_start_block: Some(123456789u64),
+                new_start_block: 123456789u64,
                 response_skeleton_opt: None,
             })
             .expect("unexpected actix error");
@@ -4782,6 +4782,7 @@ mod tests {
             let factory = Accountant::dao_factory(data_dir);
             factory.make();
         };
+
         assert_on_initialization_with_panic_on_migration(&data_dir, &act);
     }
 }

node/src/accountant/scanners/mod.rs

@@ -860,25 +860,15 @@ impl Scanner<RetrieveTransactions, ReceivedPayments> for ReceivableScanner {
                 "No newly received payments were detected during the scanning process."
             );
 
-            if let Some(new_start_block) = msg.new_start_block {
-                let current_start_block = match self.persistent_configuration.start_block() {
-                    Ok(Some(current_start_block)) => current_start_block,
-                    _ => 0u64,
-                };
-                if new_start_block > current_start_block {
-                    match self
-                        .persistent_configuration
-                        .set_start_block(msg.new_start_block)
-                    {
-                        Ok(()) => debug!(logger, "Start block updated to {}", &new_start_block),
-                        Err(e) => panic!(
-                            "Attempt to set new start block to {} failed due to: {:?}",
-                            &new_start_block, e
-                        ),
-                    }
-                } else {
-                    warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, &current_start_block);
-                }
+            match self
+                .persistent_configuration
+                .set_start_block(Some(msg.new_start_block))
+            {
+                Ok(()) => debug!(logger, "Start block updated to {}", msg.new_start_block),
+                Err(e) => panic!(
+                    "Attempt to set new start block to {} failed due to: {:?}",
+                    msg.new_start_block, e
+                ),
             }
         } else {
             self.handle_new_received_payments(&msg, logger)
@@ -921,31 +911,23 @@ impl ReceivableScanner {
             .as_mut()
             .more_money_received(msg.timestamp, &msg.payments);
 
-        if let Some(new_start_block) = msg.new_start_block {
-            let current_start_block = match self.persistent_configuration.start_block() {
-                Ok(Some(start_block)) => start_block,
-                _ => 0u64,
-            };
-            if new_start_block > current_start_block {
-                match self
-                    .persistent_configuration
-                    .set_start_block_from_txn(msg.new_start_block, &mut txn)
-                {
-                    Ok(()) => (),
-                    Err(e) => panic!(
-                        "Attempt to set new start block to {} failed due to: {:?}",
-                        new_start_block, e
-                    ),
-                }
-                match txn.commit() {
-                    Ok(_) => {
-                        debug!(logger, "Updated start block to: {}", new_start_block)
-                    }
-                    Err(e) => panic!("Commit of received transactions failed: {:?}", e),
-                }
-            } else {
-                warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, &current_start_block);
+        let new_start_block = msg.new_start_block;
+        match self
+            .persistent_configuration
+            .set_start_block_from_txn(Some(new_start_block), &mut txn)
+        {
+            Ok(()) => (),
+            Err(e) => panic!(
+                "Attempt to set new start block to {} failed due to: {:?}",
+                new_start_block, e
+            ),
+        }
+
+        match txn.commit() {
+            Ok(_) => {
+                debug!(logger, "Updated start block to: {}", new_start_block)
             }
+            Err(e) => panic!("Commit of received transactions failed: {:?}", e),
         }
 
         let total_newly_paid_receivable = msg
@@ -2101,7 +2083,9 @@ mod tests {
         };
         let payable_thresholds_gauge = PayableThresholdsGaugeMock::default()
             .is_innocent_age_params(&is_innocent_age_params_arc)
-            .is_innocent_age_result(debt_age_s <= custom_payment_thresholds.maturity_threshold_sec)
+            .is_innocent_age_result(
+                debt_age_s <= custom_payment_thresholds.maturity_threshold_sec as u64,
+            )
             .is_innocent_balance_params(&is_innocent_balance_params_arc)
             .is_innocent_balance_result(
                 balance <= gwei_to_wei(custom_payment_thresholds.permanent_debt_allowed_gwei),
@@ -2122,7 +2106,7 @@ mod tests {
         assert_eq!(debt_age_returned_innocent, debt_age_s);
         assert_eq!(
             curve_derived_time,
-            custom_payment_thresholds.maturity_threshold_sec
+            custom_payment_thresholds.maturity_threshold_sec as u64
         );
         let is_innocent_balance_params = is_innocent_balance_params_arc.lock().unwrap();
         assert_eq!(
@@ -3084,7 +3068,7 @@ mod tests {
         init_test_logging();
         let test_name = "receivable_scanner_aborts_scan_if_no_payments_were_supplied";
         let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
-        let new_start_block = Some(4321);
+        let new_start_block = 4321;
         let persistent_config = PersistentConfigurationMock::new()
             .start_block_result(Ok(None))
             .set_start_block_params(&set_start_block_params_arc)
@@ -3117,7 +3101,7 @@ mod tests {
         let test_name = "no_transactions_received_but_start_block_setting_fails";
         let now = SystemTime::now();
         let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
-        let new_start_block = Some(6709u64);
+        let new_start_block = 6709u64;
         let persistent_config = PersistentConfigurationMock::new()
             .start_block_result(Ok(None))
             .set_start_block_params(&set_start_block_params_arc)
@@ -3182,7 +3166,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables.clone(),
-            new_start_block: Some(7890123),
+            new_start_block: 7890123,
             response_skeleton_opt: None,
         };
         subject.mark_as_started(SystemTime::now());
@@ -3237,7 +3221,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables,
-            new_start_block: Some(7890123),
+            new_start_block: 7890123,
             response_skeleton_opt: None,
         };
         // Not necessary, rather for preciseness
@@ -3281,7 +3265,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables,
-            new_start_block: Some(7890123),
+            new_start_block: 7890123,
             response_skeleton_opt: None,
         };
         // Not necessary, rather for preciseness