GH-605: Review 3

node/src/accountant/db_access_objects/test_utils.rs

@@ -6,6 +6,7 @@ use crate::accountant::db_access_objects::failed_payable_dao::{
 };
 use crate::accountant::db_access_objects::sent_payable_dao::{Tx, TxStatus};
 use crate::accountant::db_access_objects::utils::{current_unix_timestamp, TxHash};
+use crate::accountant::scanners::payable_scanner::tx_templates::signable::SignableTxTemplate;
 use crate::blockchain::test_utils::make_tx_hash;
 use crate::database::db_initializer::{
     DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE,
@@ -51,6 +52,14 @@ impl TxBuilder {
         self
     }
 
+    pub fn template(mut self, signable_tx_template: SignableTxTemplate) -> Self {
+        self.receiver_address_opt = Some(signable_tx_template.receiver_address);
+        self.amount_opt = Some(signable_tx_template.amount_in_wei);
+        self.gas_price_wei_opt = Some(signable_tx_template.gas_price_wei);
+        self.nonce_opt = Some(signable_tx_template.nonce);
+        self
+    }
+
     pub fn status(mut self, status: TxStatus) -> Self {
         self.status_opt = Some(status);
         self
@@ -123,6 +132,14 @@ impl FailedTxBuilder {
         self
     }
 
+    pub fn template(mut self, signable_tx_template: SignableTxTemplate) -> Self {
+        self.receiver_address_opt = Some(signable_tx_template.receiver_address);
+        self.amount_opt = Some(signable_tx_template.amount_in_wei);
+        self.gas_price_wei_opt = Some(signable_tx_template.gas_price_wei);
+        self.nonce_opt = Some(signable_tx_template.nonce);
+        self
+    }
+
     pub fn status(mut self, failure_status: FailureStatus) -> Self {
         self.status_opt = Some(failure_status);
         self

node/src/accountant/mod.rs

@@ -139,7 +139,7 @@ pub struct ReportTransactionReceipts {
     pub response_skeleton_opt: Option<ResponseSkeleton>,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Copy, Clone)]
 pub enum PayableScanType {
     New,
     Retry,
@@ -4866,14 +4866,11 @@ mod tests {
 
     #[test]
     fn accountant_processes_sent_payables_and_schedules_pending_payable_scanner() {
-        let mark_pending_payables_rowids_params_arc = Arc::new(Mutex::new(vec![]));
         let pending_payable_notify_later_params_arc = Arc::new(Mutex::new(vec![]));
         let inserted_new_records_params_arc = Arc::new(Mutex::new(vec![]));
         let expected_wallet = make_wallet("paying_you");
         let expected_hash = H256::from("transaction_hash".keccak256());
-        let payable_dao = PayableDaoMock::new()
-            .mark_pending_payables_rowids_params(&mark_pending_payables_rowids_params_arc)
-            .mark_pending_payables_rowids_result(Ok(()));
+        let payable_dao = PayableDaoMock::new();
         let sent_payable_dao = SentPayableDaoMock::new()
             .insert_new_records_params(&inserted_new_records_params_arc)
             .insert_new_records_result(Ok(()));
@@ -4890,6 +4887,12 @@ mod tests {
             NotifyLaterHandleMock::default()
                 .notify_later_params(&pending_payable_notify_later_params_arc),
         );
+        subject.scan_schedulers.payable.new_payable_notify =
+            Box::new(NotifyHandleMock::default().panic_on_schedule_attempt());
+        subject.scan_schedulers.payable.new_payable_notify_later =
+            Box::new(NotifyLaterHandleMock::default().panic_on_schedule_attempt());
+        subject.scan_schedulers.payable.retry_payable_notify =
+            Box::new(NotifyHandleMock::default().panic_on_schedule_attempt());
         let expected_tx = TxBuilder::default().hash(expected_hash.clone()).build();
         let sent_payable = SentPayables {
             payment_procedure_result: Ok(BatchResults {
@@ -4916,21 +4919,15 @@ mod tests {
             *pending_payable_notify_later_params,
             vec![(ScanForPendingPayables::default(), pending_payable_interval)]
         );
-        // The accountant is unbound here. We don't use the bind message. It means we can prove
-        // none of those other scan requests could have been sent (especially ScanForNewPayables,
-        // ScanForRetryPayables)
     }
 
     #[test]
     fn accountant_finishes_processing_of_retry_payables_and_schedules_pending_payable_scanner() {
-        let mark_pending_payables_rowids_params_arc = Arc::new(Mutex::new(vec![]));
         let pending_payable_notify_later_params_arc = Arc::new(Mutex::new(vec![]));
         let inserted_new_records_params_arc = Arc::new(Mutex::new(vec![]));
         let expected_wallet = make_wallet("paying_you");
         let expected_hash = H256::from("transaction_hash".keccak256());
-        let payable_dao = PayableDaoMock::new()
-            .mark_pending_payables_rowids_params(&mark_pending_payables_rowids_params_arc)
-            .mark_pending_payables_rowids_result(Ok(()));
+        let payable_dao = PayableDaoMock::new();
         let sent_payable_dao = SentPayableDaoMock::new()
             .insert_new_records_params(&inserted_new_records_params_arc)
             .insert_new_records_result(Ok(()));
@@ -4950,6 +4947,12 @@ mod tests {
             NotifyLaterHandleMock::default()
                 .notify_later_params(&pending_payable_notify_later_params_arc),
         );
+        subject.scan_schedulers.payable.new_payable_notify =
+            Box::new(NotifyHandleMock::default().panic_on_schedule_attempt());
+        subject.scan_schedulers.payable.new_payable_notify_later =
+            Box::new(NotifyLaterHandleMock::default().panic_on_schedule_attempt());
+        subject.scan_schedulers.payable.retry_payable_notify =
+            Box::new(NotifyHandleMock::default().panic_on_schedule_attempt());
         let expected_tx = TxBuilder::default().hash(expected_hash.clone()).build();
         let sent_payable = SentPayables {
             payment_procedure_result: Ok(BatchResults {
@@ -4976,9 +4979,6 @@ mod tests {
             *pending_payable_notify_later_params,
             vec![(ScanForPendingPayables::default(), pending_payable_interval)]
         );
-        // The accountant is unbound here. We don't use the bind message. It means we can prove
-        // none of those other scan requests could have been sent (especially ScanForNewPayables,
-        // ScanForRetryPayables)
     }
 
     #[test]

node/src/accountant/scanners/mod.rs

@@ -1077,12 +1077,22 @@ mod tests {
 
     #[test]
     fn finish_payable_scan_keeps_the_aware_of_unresolved_pending_payable_flag_as_false_in_case_of_err(
+    ) {
+        assert_finish_payable_scan_keeps_aware_flag_false_on_error(PayableScanType::New);
+        assert_finish_payable_scan_keeps_aware_flag_false_on_error(PayableScanType::Retry);
+    }
+
+    fn assert_finish_payable_scan_keeps_aware_flag_false_on_error(
+        payable_scan_type: PayableScanType,
     ) {
         init_test_logging();
-        let test_name = "finish_payable_scan_keeps_the_aware_of_unresolved_pending_payable_flag_as_false_in_case_of_err";
+        let test_name = match payable_scan_type {
+            PayableScanType::New => "finish_payable_scan_keeps_the_aware_of_unresolved_pending_payable_flag_as_false_in_case_of_err_for_new_scan",
+            PayableScanType::Retry => "finish_payable_scan_keeps_the_aware_of_unresolved_pending_payable_flag_as_false_in_case_of_err_for_retry_scan",
+        };
         let sent_payable = SentPayables {
             payment_procedure_result: Err("Some error".to_string()),
-            payable_scan_type: PayableScanType::New,
+            payable_scan_type,
             response_skeleton_opt: None,
         };
         let logger = Logger::new(test_name);

node/src/accountant/scanners/payable_scanner/mod.rs

@@ -21,7 +21,7 @@ use crate::accountant::payment_adjuster::PaymentAdjuster;
 use crate::accountant::scanners::payable_scanner::msgs::InitialTemplatesMessage;
 use crate::accountant::scanners::payable_scanner::payment_adjuster_integration::SolvencySensitivePaymentInstructor;
 use crate::accountant::scanners::payable_scanner::utils::{
-    batch_stats, calculate_lengths, filter_receiver_addresses_from_txs, generate_status_updates,
+    batch_stats, calculate_occurences, filter_receiver_addresses_from_txs, generate_status_updates,
     payables_debug_summary, NextScanToRun, PayableScanResult, PayableThresholdsGauge,
     PayableThresholdsGaugeReal,
 };
@@ -163,15 +163,19 @@ impl PayableScanner {
     fn process_message(&self, msg: &SentPayables, logger: &Logger) {
         match &msg.payment_procedure_result {
             Ok(batch_results) => match msg.payable_scan_type {
-                PayableScanType::New => self.handle_new(batch_results, logger),
-                PayableScanType::Retry => self.handle_retry(batch_results, logger),
+                PayableScanType::New => {
+                    self.handle_batch_results_for_new_scan(batch_results, logger)
+                }
+                PayableScanType::Retry => {
+                    self.handle_batch_results_for_retry_scan(batch_results, logger)
+                }
             },
             Err(local_error) => Self::log_local_error(local_error, logger),
         }
     }
 
-    fn handle_new(&self, batch_results: &BatchResults, logger: &Logger) {
-        let (sent, failed) = calculate_lengths(&batch_results);
+    fn handle_batch_results_for_new_scan(&self, batch_results: &BatchResults, logger: &Logger) {
+        let (sent, failed) = calculate_occurences(&batch_results);
         debug!(
             logger,
             "Processed new txs while sending to RPC: {}",
@@ -185,8 +189,8 @@ impl PayableScanner {
         }
     }
 
-    fn handle_retry(&self, batch_results: &BatchResults, logger: &Logger) {
-        let (sent, failed) = calculate_lengths(&batch_results);
+    fn handle_batch_results_for_retry_scan(&self, batch_results: &BatchResults, logger: &Logger) {
+        let (sent, failed) = calculate_occurences(&batch_results);
         debug!(
             logger,
             "Processed retried txs while sending to RPC: {}",
@@ -572,11 +576,14 @@ mod tests {
     }
 
     #[test]
-    fn handle_new_does_not_perform_any_operation_when_sent_txs_is_empty() {
-        let insert_new_records_params_sent = Arc::new(Mutex::new(vec![]));
+    fn handle_batch_results_for_new_scan_does_not_perform_any_operation_when_sent_txs_is_empty() {
+        let insert_new_records_sent_tx_params_arc = Arc::new(Mutex::new(vec![]));
+        let insert_new_records_failed_tx_params_arc = Arc::new(Mutex::new(vec![]));
         let sent_payable_dao = SentPayableDaoMock::default()
-            .insert_new_records_params(&insert_new_records_params_sent);
-        let failed_payable_dao = FailedPayableDaoMock::default().insert_new_records_result(Ok(()));
+            .insert_new_records_params(&insert_new_records_sent_tx_params_arc);
+        let failed_payable_dao = FailedPayableDaoMock::default()
+            .insert_new_records_params(&insert_new_records_failed_tx_params_arc)
+            .insert_new_records_result(Ok(()));
         let subject = PayableScannerBuilder::new()
             .sent_payable_dao(sent_payable_dao)
             .failed_payable_dao(failed_payable_dao)
@@ -586,13 +593,23 @@ mod tests {
             failed_txs: vec![make_failed_tx(1)],
         };
 
-        subject.handle_new(&batch_results, &Logger::new("test"));
+        subject.handle_batch_results_for_new_scan(&batch_results, &Logger::new("test"));
 
-        assert!(insert_new_records_params_sent.lock().unwrap().is_empty());
+        assert_eq!(
+            insert_new_records_failed_tx_params_arc
+                .lock()
+                .unwrap()
+                .len(),
+            1
+        );
+        assert!(insert_new_records_sent_tx_params_arc
+            .lock()
+            .unwrap()
+            .is_empty());
     }
 
     #[test]
-    fn handle_new_does_not_perform_any_operation_when_failed_txs_is_empty() {
+    fn handle_batch_results_for_new_scan_does_not_perform_any_operation_when_failed_txs_is_empty() {
         let insert_new_records_params_failed = Arc::new(Mutex::new(vec![]));
         let sent_payable_dao = SentPayableDaoMock::default().insert_new_records_result(Ok(()));
         let failed_payable_dao = FailedPayableDaoMock::default()
@@ -606,18 +623,18 @@ mod tests {
             failed_txs: vec![],
         };
 
-        subject.handle_new(&batch_results, &Logger::new("test"));
+        subject.handle_batch_results_for_new_scan(&batch_results, &Logger::new("test"));
 
         assert!(insert_new_records_params_failed.lock().unwrap().is_empty());
     }
 
     #[test]
-    fn handle_retry_does_not_perform_any_operation_when_sent_txs_is_empty() {
-        let insert_new_records_params_sent = Arc::new(Mutex::new(vec![]));
+    fn handle_batch_results_for_retry_scan_does_not_perform_any_operation_when_sent_txs_is_empty() {
+        let insert_new_records_sent_tx_params_arc = Arc::new(Mutex::new(vec![]));
         let retrieve_txs_params = Arc::new(Mutex::new(vec![]));
         let update_statuses_params = Arc::new(Mutex::new(vec![]));
         let sent_payable_dao = SentPayableDaoMock::default()
-            .insert_new_records_params(&insert_new_records_params_sent);
+            .insert_new_records_params(&insert_new_records_sent_tx_params_arc);
         let failed_payable_dao = FailedPayableDaoMock::default()
             .retrieve_txs_params(&retrieve_txs_params)
             .update_statuses_params(&update_statuses_params);
@@ -630,9 +647,12 @@ mod tests {
             failed_txs: vec![make_failed_tx(1)],
         };
 
-        subject.handle_retry(&batch_results, &Logger::new("test"));
+        subject.handle_batch_results_for_retry_scan(&batch_results, &Logger::new("test"));
 
-        assert!(insert_new_records_params_sent.lock().unwrap().is_empty());
+        assert!(insert_new_records_sent_tx_params_arc
+            .lock()
+            .unwrap()
+            .is_empty());
         assert!(retrieve_txs_params.lock().unwrap().is_empty());
         assert!(update_statuses_params.lock().unwrap().is_empty());
     }
@@ -655,7 +675,7 @@ mod tests {
             failed_txs: vec![],
         };
 
-        subject.handle_retry(&batch_results, &Logger::new(test_name));
+        subject.handle_batch_results_for_retry_scan(&batch_results, &Logger::new(test_name));
 
         let tlh = TestLogHandler::new();
         tlh.exists_no_log_containing(&format!("WARN: {test_name}"));

node/src/accountant/scanners/payable_scanner/tx_templates/priced/new.rs

@@ -86,7 +86,7 @@ impl PricedNewTxTemplates {
         ceil: u128,
     ) -> String {
         format!(
-            "The computed gas price {} wei is above the ceil value of {} wei set by the Node.\n\
+            "The computed gas price {} wei is above the ceil value of {} wei computed by this Node.\n\
              Transaction(s) to following receivers are affected:\n\
              {}",
             computed_gas_price_wei.separate_with_commas(),

node/src/accountant/scanners/payable_scanner/tx_templates/priced/retry.rs

@@ -150,7 +150,7 @@ impl RetryLogBuilder {
         } else {
             Some(format!(
                 "The computed gas price(s) in wei is \
-                 above the ceil value of {} wei set by the Node.\n\
+                 above the ceil value of {} wei computed by this Node.\n\
                  Transaction(s) to following receivers are affected:\n\
                  {}",
                 self.ceil.separate_with_commas(),