GH-611: Self Review Changes

.gitignore

@@ -3,6 +3,7 @@
 /results/
 **/*.rs.bk
 .idea/azure/
+.idea/inspectionProfiles/Project_Default.xml
 
 ### Node
 node_modules

masq_lib/src/logger.rs

@@ -686,10 +686,10 @@ mod tests {
         let one_logger = Logger::new("logger_format_is_correct_one");
         let another_logger = Logger::new("logger_format_is_correct_another");
 
-        let before = SystemTime::now();
+        let before = OffsetDateTime::now_utc();
         error!(one_logger, "one log");
         error!(another_logger, "another log");
-        let after = SystemTime::now();
+        let after = OffsetDateTime::now_utc();
 
         let tlh = TestLogHandler::new();
         let prefix_len = "0000-00-00T00:00:00.000".len();
@@ -702,8 +702,8 @@ mod tests {
             " Thd{}: ERROR: logger_format_is_correct_another: another log",
             thread_id_as_string(thread_id)
         )));
-        let before_str = timestamp_as_string(&before);
-        let after_str = timestamp_as_string(&after);
+        let before_str = timestamp_as_string(before);
+        let after_str = timestamp_as_string(after);
         assert_between(&one_log[..prefix_len], &before_str, &after_str);
         assert_between(&another_log[..prefix_len], &before_str, &after_str);
     }
@@ -872,7 +872,7 @@ mod tests {
         tlh.exists_log_containing("error! 42");
     }
 
-    pub fn timestamp_as_string(timestamp: OffsetDateTime) -> String {
+    fn timestamp_as_string(timestamp: OffsetDateTime) -> String {
         timestamp
             .format(&parse(TIME_FORMATTING_STRING).unwrap())
             .unwrap()

node/src/accountant/mod.rs

@@ -3,7 +3,7 @@ pub mod payable_dao;
 pub mod pending_payable_dao;
 pub mod receivable_dao;
 pub mod scanners;
-pub mod tools;
+pub mod scanners_tools;
 
 #[cfg(test)]
 pub mod test_utils;
@@ -18,7 +18,7 @@ use crate::accountant::payable_dao::{Payable, PayableAccount, PayableDaoError, P
 use crate::accountant::pending_payable_dao::{PendingPayableDao, PendingPayableDaoFactory};
 use crate::accountant::receivable_dao::{ReceivableDaoError, ReceivableDaoFactory};
 use crate::accountant::scanners::scanners::{BeginScanError, NotifyLaterForScanners, Scanners};
-use crate::accountant::tools::common_tools::timestamp_as_string;
+use crate::accountant::scanners_tools::common_tools::timestamp_as_string;
 use crate::banned_dao::BannedDaoFactory;
 use crate::blockchain::blockchain_bridge::{PendingPayableFingerprint, RetrieveTransactions};
 use crate::blockchain::blockchain_interface::{BlockchainError, BlockchainTransaction};
@@ -171,31 +171,11 @@ impl Handler<StartMessage> for Accountant {
     }
 }
 
-impl Handler<SentPayable> for Accountant {
-    type Result = ();
-
-    fn handle(&mut self, msg: SentPayable, _ctx: &mut Self::Context) -> Self::Result {
-        if let Some(node_to_ui_msg) = self.scanners.payable.finish_scan(msg, &self.logger) {
-            self.ui_message_sub
-                .as_ref()
-                .expect("UIGateway is not bound")
-                .try_send(node_to_ui_msg)
-                .expect("UIGateway is dead");
-        }
-    }
-}
-
-#[derive(Debug, PartialEq, Message, Clone)]
-pub struct ReportTransactionReceipts {
-    pub fingerprints_with_receipts: Vec<(Option<TransactionReceipt>, PendingPayableFingerprint)>,
-    pub response_skeleton_opt: Option<ResponseSkeleton>,
-}
-
-impl Handler<ReportTransactionReceipts> for Accountant {
+impl Handler<ReceivedPayments> for Accountant {
     type Result = ();
 
-    fn handle(&mut self, msg: ReportTransactionReceipts, _ctx: &mut Self::Context) -> Self::Result {
-        if let Some(node_to_ui_msg) = self.scanners.pending_payable.finish_scan(msg, &self.logger) {
+    fn handle(&mut self, msg: ReceivedPayments, _ctx: &mut Self::Context) -> Self::Result {
+        if let Some(node_to_ui_msg) = self.scanners.receivable.finish_scan(msg, &self.logger) {
             self.ui_message_sub
                 .as_ref()
                 .expect("UIGateway is not bound")
@@ -205,11 +185,11 @@ impl Handler<ReportTransactionReceipts> for Accountant {
     }
 }
 
-impl Handler<ReceivedPayments> for Accountant {
+impl Handler<SentPayable> for Accountant {
     type Result = ();
 
-    fn handle(&mut self, msg: ReceivedPayments, _ctx: &mut Self::Context) -> Self::Result {
-        if let Some(node_to_ui_msg) = self.scanners.receivable.finish_scan(msg, &self.logger) {
+    fn handle(&mut self, msg: SentPayable, _ctx: &mut Self::Context) -> Self::Result {
+        if let Some(node_to_ui_msg) = self.scanners.payable.finish_scan(msg, &self.logger) {
             self.ui_message_sub
                 .as_ref()
                 .expect("UIGateway is not bound")
@@ -344,6 +324,26 @@ impl SkeletonOptHolder for RequestTransactionReceipts {
     }
 }
 
+#[derive(Debug, PartialEq, Message, Clone)]
+pub struct ReportTransactionReceipts {
+    pub fingerprints_with_receipts: Vec<(Option<TransactionReceipt>, PendingPayableFingerprint)>,
+    pub response_skeleton_opt: Option<ResponseSkeleton>,
+}
+
+impl Handler<ReportTransactionReceipts> for Accountant {
+    type Result = ();
+
+    fn handle(&mut self, msg: ReportTransactionReceipts, _ctx: &mut Self::Context) -> Self::Result {
+        if let Some(node_to_ui_msg) = self.scanners.pending_payable.finish_scan(msg, &self.logger) {
+            self.ui_message_sub
+                .as_ref()
+                .expect("UIGateway is not bound")
+                .try_send(node_to_ui_msg)
+                .expect("UIGateway is dead");
+        }
+    }
+}
+
 impl Handler<PendingPayableFingerprint> for Accountant {
     type Result = ();
     fn handle(&mut self, msg: PendingPayableFingerprint, _ctx: &mut Self::Context) -> Self::Result {
@@ -796,12 +796,10 @@ pub fn unsigned_to_signed(unsigned: u64) -> Result<i64, u64> {
     i64::try_from(unsigned).map_err(|_| unsigned)
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub enum PendingTransactionStatus {
-    StillPending(PendingPayableId),
-    //updates slightly the record, waits an interval and starts a new round
-    Failure(PendingPayableId),
-    //official tx failure
+    StillPending(PendingPayableId), //updates slightly the record, waits an interval and starts a new round
+    Failure(PendingPayableId),      //official tx failure
     Confirmed(PendingPayableFingerprint), //tx was fully processed and successful
 }
 
@@ -934,12 +932,6 @@ mod tests {
 
     #[test]
     fn accountant_have_proper_defaulted_values() {
-        // TODO: Verify Scanners are defaulted properly [write this test once GH-574's recorder's code is merged, or cherry-pick the commit]
-        // When scan() is called, on a scanner, it sends one message to blockchain bridge and a notify_later message, which in turn
-        // schedules another scan, in turn accountant sends another message to blockchain bridge. Make sure a scan() call to a scanner results in two messages
-        // to blockchain bridge (one received directly and other indirectly via notify_later). Make sure 6 messages are received in total.
-        // The second message is received after test defined scan intervals.
-        // Make sure to use a real database instead of using mock utilities. It'll require at least one row for each table of individual scanners.
         let mut bootstrapper_config = make_bc_with_defaults();
         let payable_dao_factory = Box::new(
             PayableDaoFactoryMock::new()

node/src/accountant/scanners.rs

@@ -4,13 +4,13 @@ pub(in crate::accountant) mod scanners {
     use crate::accountant::payable_dao::{Payable, PayableDao, PayableDaoFactory};
     use crate::accountant::pending_payable_dao::{PendingPayableDao, PendingPayableDaoFactory};
     use crate::accountant::receivable_dao::ReceivableDao;
-    use crate::accountant::tools::payable_scanner_tools::{
+    use crate::accountant::scanners_tools::payable_scanner_tools::{
         investigate_debt_extremes, qualified_payables_and_summary, separate_early_errors,
     };
-    use crate::accountant::tools::pending_payable_scanner_tools::{
+    use crate::accountant::scanners_tools::pending_payable_scanner_tools::{
         elapsed_in_ms, handle_none_status, handle_status_with_failure, handle_status_with_success,
     };
-    use crate::accountant::tools::receivable_scanner_tools::balance_and_age;
+    use crate::accountant::scanners_tools::receivable_scanner_tools::balance_and_age;
     use crate::accountant::{
         Accountant, ReceivedPayments, ReportTransactionReceipts, RequestTransactionReceipts,
         ResponseSkeleton, ScanForPayables, ScanForPendingPayables, ScanForReceivables, SentPayable,

node/src/accountant/tools.rs → node/src/accountant/scanners_tools.rs

@@ -294,12 +294,12 @@ pub mod common_tools {
 mod tests {
     use crate::accountant::payable_dao::{Payable, PayableAccount};
     use crate::accountant::receivable_dao::ReceivableAccount;
-    use crate::accountant::tools::payable_scanner_tools::{
+    use crate::accountant::scanners_tools::payable_scanner_tools::{
         calculate_payout_threshold, exceeded_summary, investigate_debt_extremes,
         is_payable_qualified, payable_time_diff, qualified_payables_and_summary,
         separate_early_errors,
     };
-    use crate::accountant::tools::receivable_scanner_tools::balance_and_age;
+    use crate::accountant::scanners_tools::receivable_scanner_tools::balance_and_age;
     use crate::accountant::SentPayable;
     use crate::blockchain::blockchain_interface::BlockchainError;
     use crate::database::dao_utils::{from_time_t, to_time_t};
@@ -534,86 +534,4 @@ mod tests {
         assert_eq!(ok, vec![payable_ok]);
         assert_eq!(err, vec![error])
     }
-
-    // TODO: Either make this test work or write an alternative test in the desired file
-    // #[test]
-    // fn threshold_calculation_depends_on_user_defined_payment_thresholds() {
-    //     let safe_age_params_arc = Arc::new(Mutex::new(vec![]));
-    //     let safe_balance_params_arc = Arc::new(Mutex::new(vec![]));
-    //     let calculate_payable_threshold_params_arc = Arc::new(Mutex::new(vec![]));
-    //     let balance = 5555;
-    //     let how_far_in_past = Duration::from_secs(1111 + 1);
-    //     let last_paid_timestamp = SystemTime::now().sub(how_far_in_past);
-    //     let payable_account = PayableAccount {
-    //         wallet: make_wallet("hi"),
-    //         balance,
-    //         last_paid_timestamp,
-    //         pending_payable_opt: None,
-    //     };
-    //     let custom_payment_thresholds = PaymentThresholds {
-    //         maturity_threshold_sec: 1111,
-    //         payment_grace_period_sec: 2222,
-    //         permanent_debt_allowed_gwei: 3333,
-    //         debt_threshold_gwei: 4444,
-    //         threshold_interval_sec: 5555,
-    //         unban_below_gwei: 3333,
-    //     };
-    //     let mut bootstrapper_config = BootstrapperConfig::default();
-    //     bootstrapper_config.accountant_config_opt = Some(AccountantConfig {
-    //         scan_intervals: Default::default(),
-    //         payment_thresholds: custom_payment_thresholds,
-    //         suppress_initial_scans: false,
-    //         when_pending_too_long_sec: DEFAULT_PENDING_TOO_LONG_SEC,
-    //     });
-    //     let payable_thresholds_tools = PayableThresholdToolsMock::default()
-    //         .is_innocent_age_params(&safe_age_params_arc)
-    //         .is_innocent_age_result(
-    //             how_far_in_past.as_secs()
-    //                 <= custom_payment_thresholds.maturity_threshold_sec as u64,
-    //         )
-    //         .is_innocent_balance_params(&safe_balance_params_arc)
-    //         .is_innocent_balance_result(
-    //             balance <= custom_payment_thresholds.permanent_debt_allowed_gwei,
-    //         )
-    //         .calculate_payout_threshold_params(&calculate_payable_threshold_params_arc)
-    //         .calculate_payout_threshold_result(4567.0); //made up value
-    //     let mut subject = AccountantBuilder::default()
-    //         .bootstrapper_config(bootstrapper_config)
-    //         .build();
-    //     subject.scanners.payables.payable_thresholds_tools = Box::new(payable_thresholds_tools);
-    //
-    //     let result = subject.payable_exceeded_threshold(&payable_account);
-    //
-    //     assert_eq!(result, Some(4567));
-    //     let mut safe_age_params = safe_age_params_arc.lock().unwrap();
-    //     let safe_age_single_params = safe_age_params.remove(0);
-    //     assert_eq!(*safe_age_params, vec![]);
-    //     let (time_elapsed, curve_derived_time) = safe_age_single_params;
-    //     assert!(
-    //         (how_far_in_past.as_secs() - 3) < time_elapsed
-    //             && time_elapsed < (how_far_in_past.as_secs() + 3)
-    //     );
-    //     assert_eq!(
-    //         curve_derived_time,
-    //         custom_payment_thresholds.maturity_threshold_sec as u64
-    //     );
-    //     let safe_balance_params = safe_balance_params_arc.lock().unwrap();
-    //     assert_eq!(
-    //         *safe_balance_params,
-    //         vec![(
-    //             payable_account.balance,
-    //             custom_payment_thresholds.permanent_debt_allowed_gwei
-    //         )]
-    //     );
-    //     let mut calculate_payable_curves_params =
-    //         calculate_payable_threshold_params_arc.lock().unwrap();
-    //     let calculate_payable_curves_single_params = calculate_payable_curves_params.remove(0);
-    //     assert_eq!(*calculate_payable_curves_params, vec![]);
-    //     let (payment_thresholds, time_elapsed) = calculate_payable_curves_single_params;
-    //     assert!(
-    //         (how_far_in_past.as_secs() - 3) < time_elapsed
-    //             && time_elapsed < (how_far_in_past.as_secs() + 3)
-    //     );
-    //     assert_eq!(payment_thresholds, custom_payment_thresholds)
-    // }
 }

node/src/lib.rs

@@ -6,7 +6,6 @@ pub mod sub_lib;
 
 #[macro_use]
 extern crate masq_lib;
-extern crate core;
 
 #[cfg(test)]
 mod node_test_utils;

node/src/sub_lib/accountant.rs

@@ -70,15 +70,6 @@ pub struct ScanIntervals {
     pub receivable_scan_interval: Duration,
 }
 
-// TODO: Remove it once you realise you don't want to know which fields was accountant config composed of
-// #[derive(Clone, PartialEq, Debug)]
-// pub struct AccountantConfig {
-//     pub scan_intervals: ScanIntervals,
-//     pub payment_thresholds: PaymentThresholds,
-//     pub suppress_initial_scans: bool,
-//     pub when_pending_too_long_sec: u64,
-// }
-
 #[derive(Clone)]
 pub struct AccountantSubs {
     pub bind: Recipient<BindMessage>,

node/src/test_utils/recorder.rs

@@ -112,7 +112,6 @@ recorder_message_handler!(AddReturnRouteMessage);
 recorder_message_handler!(AddRouteMessage);
 recorder_message_handler!(AddStreamMsg);
 recorder_message_handler!(BindMessage);
-recorder_message_handler!(ConnectionProgressMessage);
 recorder_message_handler!(CrashNotification);
 recorder_message_handler!(DaemonBindMessage);
 recorder_message_handler!(DispatcherNodeQueryMessage);
@@ -156,6 +155,7 @@ recorder_message_handler!(ReportTransactionReceipts);
 recorder_message_handler!(ReportAccountsPayable);
 recorder_message_handler!(ScanForReceivables);
 recorder_message_handler!(ScanForPayables);
+recorder_message_handler!(ConnectionProgressMessage);
 recorder_message_handler!(ScanForPendingPayables);
 
 impl Handler<NodeQueryMessage> for Recorder {