GH-606: Apply PR feedback changes

Author: masqrauder

node/src/accountant/mod.rs

@@ -123,7 +123,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: u64,
+    pub new_start_block: Option<u64>,
     pub response_skeleton_opt: Option<ResponseSkeleton>,
 }
 
@@ -1013,6 +1013,7 @@ mod tests {
     use crate::blockchain::test_utils::{make_tx_hash, BlockchainInterfaceMock};
     use crate::database::rusqlite_wrappers::TransactionSafeWrapper;
     use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder;
+    use crate::db_config::config_dao::ConfigDaoRecord;
     use crate::db_config::mocks::ConfigDaoMock;
     use crate::match_every_type_id;
     use crate::sub_lib::accountant::{
@@ -1266,7 +1267,11 @@ mod tests {
         config.suppress_initial_scans = true;
         let subject = AccountantBuilder::default()
             .bootstrapper_config(config)
-            .config_dao(ConfigDaoMock::new().set_result(Ok(())))
+            .config_dao(
+                ConfigDaoMock::new()
+                    .get_result(Ok(ConfigDaoRecord::new("start_block", None, false)))
+                    .set_result(Ok(())),
+            )
             .build();
         let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder();
         let subject_addr = subject.start();
@@ -1276,7 +1281,7 @@ mod tests {
         let received_payments = ReceivedPayments {
             timestamp: SystemTime::now(),
             payments: vec![],
-            new_start_block: 1234567,
+            new_start_block: Some(1234567),
             response_skeleton_opt: Some(ResponseSkeleton {
                 client_id: 1234,
                 context_id: 4321,
@@ -1876,6 +1881,7 @@ mod tests {
     ) {
         let more_money_received_params_arc = Arc::new(Mutex::new(vec![]));
         let commit_params_arc = Arc::new(Mutex::new(vec![]));
+        let get_params_arc = Arc::new(Mutex::new(vec![]));
         let set_by_guest_transaction_params_arc = Arc::new(Mutex::new(vec![]));
         let now = SystemTime::now();
         let earning_wallet = make_wallet("earner3000");
@@ -1899,6 +1905,8 @@ mod tests {
             .more_money_received_params(&more_money_received_params_arc)
             .more_money_received_result(wrapped_transaction);
         let config_dao = ConfigDaoMock::new()
+            .get_params(&get_params_arc)
+            .get_result(Ok(ConfigDaoRecord::new("start_block", None, false)))
             .set_by_guest_transaction_params(&set_by_guest_transaction_params_arc)
             .set_by_guest_transaction_result(Ok(()));
         let accountant = AccountantBuilder::default()
@@ -1913,7 +1921,7 @@ mod tests {
             .try_send(ReceivedPayments {
                 timestamp: now,
                 payments: vec![expected_receivable_1.clone(), expected_receivable_2.clone()],
-                new_start_block: 123456789,
+                new_start_block: Some(123456789u64),
                 response_skeleton_opt: None,
             })
             .expect("unexpected actix error");

node/src/accountant/scanners/mod.rs

@@ -859,15 +859,23 @@ impl Scanner<RetrieveTransactions, ReceivedPayments> for ReceivableScanner {
                 "No newly received payments were detected during the scanning process."
             );
 
-            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
-                ),
+            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 {
             self.handle_new_received_payments(&msg, logger)
@@ -912,23 +920,29 @@ impl ReceivableScanner {
             .as_mut()
             .more_money_received(msg.timestamp, &msg.payments);
 
-        let new_start_block = msg.new_start_block;
-        match self
-            .persistent_configuration
-            .set_start_block_from_txn(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)
+        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),
+                }
             }
-            Err(e) => panic!("Commit of received transactions failed: {:?}", e),
         }
 
         let total_newly_paid_receivable = msg
@@ -1623,7 +1637,7 @@ mod tests {
             .iter()
             .map(|ppayable| ppayable.hash)
             .collect::<HashSet<H256>>();
-        // Not in an ascending order
+        // Not in ascending order
         let rowids_and_hashes_from_fingerprints = vec![(hash_1, 3), (hash_3, 5), (hash_2, 6)]
             .iter()
             .map(|(hash, _id)| *hash)
@@ -1875,7 +1889,7 @@ mod tests {
         00000000000000000000000000000000000000000315 failed due to RecordDeletion(\"Gosh, I overslept \
         without an alarm set\")");
         let log_handler = TestLogHandler::new();
-        // There is a possible situation when we stumble over missing fingerprints and so we log it.
+        // There is a possible situation when we stumble over missing fingerprints, so we log it.
         // Here we don't and so any ERROR log shouldn't turn up
         log_handler.exists_no_log_containing(&format!("ERROR: {}", test_name))
     }
@@ -2075,9 +2089,7 @@ 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 as u64,
-            )
+            .is_innocent_age_result(debt_age_s <= custom_payment_thresholds.maturity_threshold_sec)
             .is_innocent_balance_params(&is_innocent_balance_params_arc)
             .is_innocent_balance_result(
                 balance <= gwei_to_wei(custom_payment_thresholds.permanent_debt_allowed_gwei),
@@ -2098,7 +2110,7 @@ mod tests {
         assert_eq!(debt_age_returned_innocent, debt_age_s);
         assert_eq!(
             curve_derived_time,
-            custom_payment_thresholds.maturity_threshold_sec as u64
+            custom_payment_thresholds.maturity_threshold_sec
         );
         let is_innocent_balance_params = is_innocent_balance_params_arc.lock().unwrap();
         assert_eq!(
@@ -3043,8 +3055,9 @@ 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 = 4321;
+        let new_start_block = Some(4321);
         let persistent_config = PersistentConfigurationMock::new()
+            .start_block_result(Ok(None))
             .set_start_block_params(&set_start_block_params_arc)
             .set_start_block_result(Ok(()));
         let mut subject = ReceivableScannerBuilder::new()
@@ -3074,16 +3087,21 @@ mod tests {
         init_test_logging();
         let test_name = "no_transactions_received_but_start_block_setting_fails";
         let now = SystemTime::now();
-        let persistent_config = PersistentConfigurationMock::new().set_start_block_result(Err(
-            PersistentConfigError::UninterpretableValue("Illiterate database manager".to_string()),
-        ));
+        let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
+        let new_start_block = Some(6709u64);
+        let persistent_config = PersistentConfigurationMock::new()
+            .start_block_result(Ok(None))
+            .set_start_block_params(&set_start_block_params_arc)
+            .set_start_block_result(Err(PersistentConfigError::UninterpretableValue(
+                "Illiterate database manager".to_string(),
+            )));
         let mut subject = ReceivableScannerBuilder::new()
             .persistent_configuration(persistent_config)
             .build();
         let msg = ReceivedPayments {
             timestamp: now,
             payments: vec![],
-            new_start_block: 6709,
+            new_start_block,
             response_skeleton_opt: None,
         };
         // Not necessary, rather for preciseness
@@ -3107,6 +3125,7 @@ mod tests {
             .set_arbitrary_id_stamp(transaction_id);
         let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
         let persistent_config = PersistentConfigurationMock::new()
+            .start_block_result(Ok(None))
             .set_start_block_from_txn_params(&set_start_block_from_txn_params_arc)
             .set_start_block_from_txn_result(Ok(()));
         let receivable_dao = ReceivableDaoMock::new()
@@ -3134,7 +3153,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables.clone(),
-            new_start_block: 7890123,
+            new_start_block: Some(7890123),
             response_skeleton_opt: None,
         };
         subject.mark_as_started(SystemTime::now());
@@ -3153,7 +3172,7 @@ mod tests {
         let set_by_guest_transaction_params = set_start_block_from_txn_params_arc.lock().unwrap();
         assert_eq!(
             *set_by_guest_transaction_params,
-            vec![(7890123, transaction_id)]
+            vec![(Some(7890123u64), transaction_id)]
         );
         let commit_params = commit_params_arc.lock().unwrap();
         assert_eq!(*commit_params, vec![()]);
@@ -3171,9 +3190,11 @@ mod tests {
         let now = SystemTime::now();
         let txn_inner_builder = TransactionInnerWrapperMockBuilder::default();
         let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
-        let persistent_config = PersistentConfigurationMock::new().set_start_block_from_txn_result(
-            Err(PersistentConfigError::DatabaseError("Fatigue".to_string())),
-        );
+        let persistent_config = PersistentConfigurationMock::new()
+            .start_block_result(Ok(None))
+            .set_start_block_from_txn_result(Err(PersistentConfigError::DatabaseError(
+                "Fatigue".to_string(),
+            )));
         let receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction);
         let mut subject = ReceivableScannerBuilder::new()
             .receivable_dao(receivable_dao)
@@ -3187,7 +3208,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables,
-            new_start_block: 7890123,
+            new_start_block: Some(7890123),
             response_skeleton_opt: None,
         };
         // Not necessary, rather for preciseness
@@ -3215,8 +3236,9 @@ mod tests {
         let txn_inner_builder =
             TransactionInnerWrapperMockBuilder::default().commit_result(commit_err);
         let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
-        let persistent_config =
-            PersistentConfigurationMock::new().set_start_block_from_txn_result(Ok(()));
+        let persistent_config = PersistentConfigurationMock::new()
+            .start_block_result(Ok(None))
+            .set_start_block_from_txn_result(Ok(()));
         let receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction);
         let mut subject = ReceivableScannerBuilder::new()
             .receivable_dao(receivable_dao)
@@ -3230,7 +3252,7 @@ mod tests {
         let msg = ReceivedPayments {
             timestamp: now,
             payments: receivables,
-            new_start_block: 7890123,
+            new_start_block: Some(7890123),
             response_skeleton_opt: None,
         };
         // Not necessary, rather for preciseness