GH-598: Receivables scheduling hot fix (#750)

Author: bertllll

node/src/accountant/mod.rs

@@ -122,7 +122,10 @@ impl Scanners {
             });
         }
 
-        Self::start_correct_payable_scanner::<ScanForNewPayables>(
+        <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForNewPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut *self.payable,
             wallet,
             timestamp,
@@ -150,7 +153,10 @@ impl Scanners {
             )
         }
 
-        Self::start_correct_payable_scanner::<ScanForRetryPayables>(
+        <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForRetryPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut *self.payable,
             wallet,
             timestamp,
@@ -168,10 +174,14 @@ impl Scanners {
         automatic_scans_enabled: bool,
     ) -> Result<RequestTransactionReceipts, StartScanError> {
         let triggered_manually = response_skeleton_opt.is_some();
-        self.check_general_conditions_for_pending_payable_scan(
-            triggered_manually,
-            automatic_scans_enabled,
-        )?;
+        if triggered_manually && automatic_scans_enabled {
+            return Err(StartScanError::ManualTriggerError(
+                ManulTriggerError::AutomaticScanConflict,
+            ));
+        }
+
+        self.check_pending_payable_existence(triggered_manually)?;
+
         match (
             self.pending_payable.scan_started_at(),
             self.payable.scan_started_at(),
@@ -299,41 +309,14 @@ impl Scanners {
         self.initial_pending_payable_scan = false
     }
 
-    // This is a helper function reducing a boilerplate of complex trait resolving where
-    // the compiler requires to specify which trigger message distinguishes the scan to run.
-    // The payable scanner offers two modes through doubled implementations of StartableScanner
-    // which uses the trigger message type as the only distinction between them.
-    fn start_correct_payable_scanner<'a, TriggerMessage>(
-        scanner: &'a mut (dyn MultistageDualPayableScanner + 'a),
-        wallet: &Wallet,
-        timestamp: SystemTime,
-        response_skeleton_opt: Option<ResponseSkeleton>,
-        logger: &Logger,
-    ) -> Result<InitialTemplatesMessage, StartScanError>
-    where
-        TriggerMessage: Message,
-        (dyn MultistageDualPayableScanner + 'a):
-            StartableScanner<TriggerMessage, InitialTemplatesMessage>,
-    {
-        <(dyn MultistageDualPayableScanner + 'a) as StartableScanner<
-            TriggerMessage,
-            InitialTemplatesMessage,
-        >>::start_scan(scanner, wallet, timestamp, response_skeleton_opt, logger)
-    }
-
-    fn check_general_conditions_for_pending_payable_scan(
+    fn check_pending_payable_existence(
         &mut self,
         triggered_manually: bool,
-        automatic_scans_enabled: bool,
     ) -> Result<(), StartScanError> {
-        if triggered_manually && automatic_scans_enabled {
-            return Err(StartScanError::ManualTriggerError(
-                ManulTriggerError::AutomaticScanConflict,
-            ));
-        }
         if self.initial_pending_payable_scan {
             return Ok(());
         }
+
         if triggered_manually && !self.aware_of_unresolved_pending_payable {
             return Err(StartScanError::ManualTriggerError(
                 ManulTriggerError::UnnecessaryRequest {
@@ -1343,8 +1326,8 @@ mod tests {
 
     #[test]
     #[should_panic(
-        expected = "internal error: entered unreachable code: Automatic pending payable \
-    scan should never start if there are no pending payables to process."
+        expected = "internal error: entered unreachable code: Automatic pending payable scan should \
+        never start if there are no pending payables to process."
     )]
     fn pending_payable_scanner_bumps_into_zero_pending_payable_awareness_in_the_automatic_mode() {
         let consuming_wallet = make_paying_wallet(b"consuming");
@@ -1363,11 +1346,12 @@ mod tests {
     }
 
     #[test]
-    fn check_general_conditions_for_pending_payable_scan_if_it_is_initial_pending_payable_scan() {
+    fn check_pending_payable_existence_for_initial_pending_payable_scan_and_zero_awareness() {
         let mut subject = make_dull_subject();
+        subject.aware_of_unresolved_pending_payable = false;
         subject.initial_pending_payable_scan = true;
 
-        let result = subject.check_general_conditions_for_pending_payable_scan(false, true);
+        let result = subject.check_pending_payable_existence(false);
 
         assert_eq!(result, Ok(()));
         assert_eq!(subject.initial_pending_payable_scan, true);

node/src/accountant/scanners/mod.rs

@@ -35,7 +35,7 @@ impl StartableScanner<ScanForNewPayables, InitialTemplatesMessage> for PayableSc
         match qualified_payables.is_empty() {
             true => {
                 self.mark_as_ended(logger);
-                Err(StartScanError::NothingToProcess)
+                Err(StartScanError::nothing_to_process(response_skeleton_opt))
             }
             false => {
                 info!(
@@ -89,7 +89,7 @@ mod tests {
     use crate::accountant::scanners::payable_scanner::tx_templates::initial::retry::{
         RetryTxTemplate, RetryTxTemplates,
     };
-    use crate::accountant::scanners::Scanners;
+    use crate::accountant::scanners::payable_scanner::MultistageDualPayableScanner;
     use crate::accountant::test_utils::{
         make_payable_account, FailedPayableDaoMock, PayableDaoMock,
     };
@@ -144,7 +144,10 @@ mod tests {
             .payable_dao(payable_dao)
             .build();
 
-        let result = Scanners::start_correct_payable_scanner::<ScanForRetryPayables>(
+        let result = <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForRetryPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut subject,
             &consuming_wallet,
             timestamp,

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

@@ -95,10 +95,12 @@ impl StartableScanner<ScanForPendingPayables, RequestTransactionReceipts>
 
         info!(logger, "Scanning for pending payable");
 
-        let tx_hashes = self.harvest_tables(logger).map_err(|e| {
-            self.mark_as_ended(logger);
-            e
-        })?;
+        let tx_hashes = self
+            .harvest_tables(logger, response_skeleton_opt)
+            .map_err(|e| {
+                self.mark_as_ended(logger);
+                e
+            })?;
 
         Ok(RequestTransactionReceipts {
             tx_hashes,
@@ -162,7 +164,11 @@ impl PendingPayableScanner {
         }
     }
 
-    fn harvest_tables(&mut self, logger: &Logger) -> Result<Vec<TxHashByTable>, StartScanError> {
+    fn harvest_tables(
+        &mut self,
+        logger: &Logger,
+        response_skeleton_opt: Option<ResponseSkeleton>,
+    ) -> Result<Vec<TxHashByTable>, StartScanError> {
         debug!(logger, "Harvesting sent_payable and failed_payable tables");
 
         let pending_tx_hashes_opt = self.harvest_pending_payables();
@@ -172,7 +178,7 @@ impl PendingPayableScanner {
             pending_tx_hashes_opt.as_ref(),
             failure_hashes_opt.as_ref(),
         ) {
-            return Err(StartScanError::NothingToProcess);
+            return Err(StartScanError::nothing_to_process(response_skeleton_opt));
         }
 
         Self::log_records_for_receipt_check(
@@ -878,7 +884,9 @@ mod tests {
     };
     use crate::accountant::scanners::pending_payable_scanner::PendingPayableScanner;
     use crate::accountant::scanners::test_utils::PendingPayableCacheMock;
-    use crate::accountant::scanners::{Scanner, StartScanError, StartableScanner};
+    use crate::accountant::scanners::{
+        AutomaticError, CommonError, Scanner, StartScanError, StartableScanner,
+    };
     use crate::accountant::test_utils::{
         make_transaction_block, FailedPayableDaoMock, PayableDaoMock, PendingPayableScannerBuilder,
         SentPayableDaoMock,
@@ -1230,7 +1238,12 @@ mod tests {
         let result = subject.start_scan(&consuming_wallet, now, None, &Logger::new("test"));
 
         let is_scan_running = subject.scan_started_at().is_some();
-        assert_eq!(result, Err(StartScanError::NothingToProcess));
+        assert_eq!(
+            result,
+            Err(StartScanError::Automatic(AutomaticError::Common(
+                CommonError::NothingToProcess
+            )))
+        );
         assert_eq!(is_scan_running, false);
     }