GH-606: Always commit

GH-606: Reduce logging levels and simplify

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,16 @@
 #!/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"
+# All wallets begin with null balances. The only exception is the contract owner wallet whose means are to be
+# redistributed from there to every account that would need it. (Notice the argument --account '<contract owner wallet
+# private key>,<ETH in wei>' that assigns a certain initial balance.) This same principle of initialization needs to be
+# regarded, during the test setup, and applied with both the transaction fee (wei of ETH) and the service fee (MASQ).
+# While on the transaction fee it's a choice done by us, with the latter, there probably isn't any other solution given
+# the mechanism how the deployment of the blockchain smart contract generates the entire token supply only on
+# the account of the contract owner's wallet from where it must be sent out to other wallets if needed.
+
+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/masq_real_node.rs

@@ -1018,7 +1018,7 @@ impl MASQRealNode {
 
         args.push("test_node_image");
         let mut command = Command::new("docker", Command::strings(args));
-        command.stdout_or_stderr()?;
+        println!("{}", command.stdout_or_stderr()?);
         Ok(())
     }
 

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,13 +602,13 @@ 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()
         ])
     }
 
     fn connect(port: u16) -> TcpStream {
-        let deadline = Instant::now().add(Duration::from_secs(1));
+        let deadline = Instant::now().add(Duration::from_secs(5));
         let addr = DockerHostSocketAddr::new(port);
         loop {
             thread::sleep(Duration::from_millis(100));
@@ -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
     );
@@ -435,7 +432,7 @@ fn expire_payables(path: PathBuf) {
     statement.execute([]).unwrap();
 
     let mut config_stmt = conn
-        .prepare("update config set value = '0' where name = 'start_block'")
+        .prepare("update config set value = '0' where name = 'start_block' and (value is null or value < 1)")
         .unwrap();
     config_stmt.execute([]).unwrap();
 }
@@ -450,7 +447,7 @@ fn expire_receivables(path: PathBuf) {
     statement.execute([]).unwrap();
 
     let mut config_stmt = conn
-        .prepare("update config set value = '0' where name = 'start_block'")
+        .prepare("update config set value = '0' where name = 'start_block' and (value is null or value < 1)")
         .unwrap();
     config_stmt.execute([]).unwrap();
 }

node/src/accountant/scanners/mod.rs

@@ -877,7 +877,8 @@ impl Scanner<RetrieveTransactions, ReceivedPayments> for ReceivableScanner {
                         ),
                     }
                 } 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);
+                    debug!(logger, "The new_start_block ({}) is not greater 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);
                 }
             }
         } else {
@@ -938,13 +939,20 @@ impl ReceivableScanner {
                     ),
                 }
                 match txn.commit() {
-                    Ok(_) => {
-                        debug!(logger, "Updated start block to: {}", new_start_block)
-                    }
+                    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);
+                match txn.commit() {
+                    Ok(_) => debug!(logger, "The new_start_block ({}) is not greater than the current_start_block ({}). \
+                    Committed received transactions.", &new_start_block, &current_start_block),
+                    Err(e) => panic!("Commit of received transactions failed: {:?}", e),
+                }
+            }
+        } else {
+            match txn.commit() {
+                Ok(_) => debug!(logger, "Committed received transactions."),
+                Err(e) => panic!("Commit of received transactions failed: {:?}", e),
             }
         }
 

node/src/blockchain/blockchain_bridge.rs

@@ -1425,7 +1425,6 @@ mod tests {
             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);
@@ -1515,7 +1514,6 @@ mod tests {
             Box::new(blockchain_interface),
             Box::new(persistent_config),
             false,
-            Some(make_wallet("consuming")),
         );
         let addr = subject.start();
         let subject_subs = BlockchainBridge::make_subs_from(&addr);
@@ -1763,7 +1761,6 @@ mod tests {
             Box::new(blockchain_interface),
             Box::new(persistent_config),
             false,
-            None, //not needed in this test
         );
         let retrieve_transactions = RetrieveTransactions {
             recipient: make_wallet("somewallet"),

node/src/sub_lib/neighborhood.rs

@@ -433,6 +433,10 @@ impl Debug for NeighborhoodSubs {
     }
 }
 
+// TODO This message doesn't seem to fit exactly to its use. Some fields are required only in some
+// cases, like the NodeAddr and nothing else. We should consider making some or both of the other
+// fields optional. In the bigger image, we might want to design an enum that could store all sorts
+// of responses that could result from a NodeQuery.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct NodeQueryResponseMetadata {
     pub public_key: PublicKey,