diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 31f3033b5..85bd9e017 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -30,7 +30,7 @@ use crate::accountant::scanners::pending_payable_scanner::utils::{ PendingPayableScanResult, TxHashByTable, }; use crate::accountant::scanners::scan_schedulers::{ - PayableSequenceScanner, ScanReschedulingAfterEarlyStop, ScanSchedulers, + ScanReschedulingAfterEarlyStop, ScanSchedulers, UnableToStartScanner, }; use crate::accountant::scanners::{Scanners, StartScanError}; use crate::blockchain::blockchain_bridge::{ @@ -312,11 +312,25 @@ impl Handler for Accountant { impl Handler for Accountant { type Result = (); - fn handle(&mut self, msg: ScanForReceivables, ctx: &mut Self::Context) -> Self::Result { + fn handle(&mut self, msg: ScanForReceivables, _ctx: &mut Self::Context) -> Self::Result { // By now we know it is an automatic scan. The ReceivableScanner is independent of other // scanners and rescheduled regularly, just here. - self.handle_request_of_scan_for_receivable(msg.response_skeleton_opt); - self.scan_schedulers.receivable.schedule(ctx, &self.logger); + let scheduling_hint = self.handle_request_of_scan_for_receivable(msg.response_skeleton_opt); + + match scheduling_hint { + ScanReschedulingAfterEarlyStop::Schedule(other_scan_type) => unreachable!( + "Early stopped receivable scan was suggested to be followed up by the scan \ + for {:?}, which is not supported though", + other_scan_type + ), + ScanReschedulingAfterEarlyStop::DoNotSchedule => { + trace!( + self.logger, + "No early rescheduling, as the receivable scan did find results, or this \ + is the NullScanner" + ) + } + } } } @@ -401,13 +415,19 @@ impl Handler for Accountant { impl Handler 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.finish_receivable_scan(msg, &self.logger) { - self.ui_message_sub_opt - .as_ref() - .expect("UIGateway is not bound") - .try_send(node_to_ui_msg) - .expect("UIGateway is dead"); + fn handle(&mut self, msg: ReceivedPayments, ctx: &mut Self::Context) -> Self::Result { + match self.scanners.finish_receivable_scan(msg, &self.logger) { + None => self.scan_schedulers.receivable.schedule(ctx, &self.logger), + Some(node_to_ui_msg) => { + self.ui_message_sub_opt + .as_ref() + .expect("UIGateway is not bound") + .try_send(node_to_ui_msg) + .expect("UIGateway is dead"); + // Externally triggered scans are not allowed to provoke an unwinding scan sequence + // with intervals. The only exception is the PendingPayableScanner that is always + // followed by the retry-payable scanner in a tight tandem. + } } } } @@ -993,8 +1013,8 @@ impl Accountant { .expect("BlockchainBridge is dead"); ScanReschedulingAfterEarlyStop::DoNotSchedule } - Err(e) => self.handle_start_scan_error_and_prevent_scan_stall_point( - PayableSequenceScanner::NewPayables, + Err(e) => self.handle_start_scan_error( + UnableToStartScanner::NewPayables, e, response_skeleton_opt, ), @@ -1025,10 +1045,9 @@ impl Accountant { .expect("BlockchainBridge is dead"); } Err(e) => { - // It is thrown away and there is no rescheduling downstream because every error - // happening here on the start resolves into a panic by the current design - let _ = self.handle_start_scan_error_and_prevent_scan_stall_point( - PayableSequenceScanner::RetryPayables, + // Any error here panics by design, so the return value is unreachable/ignored. + let _ = self.handle_start_scan_error( + UnableToStartScanner::RetryPayables, e, response_skeleton_opt, ); @@ -1063,8 +1082,8 @@ impl Accountant { } Err(e) => { let initial_pending_payable_scan = self.scanners.initial_pending_payable_scan(); - self.handle_start_scan_error_and_prevent_scan_stall_point( - PayableSequenceScanner::PendingPayables { + self.handle_start_scan_error( + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, }, e, @@ -1080,9 +1099,9 @@ impl Accountant { hint } - fn handle_start_scan_error_and_prevent_scan_stall_point( + fn handle_start_scan_error( &self, - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, e: StartScanError, response_skeleton_opt: Option, ) -> ScanReschedulingAfterEarlyStop { @@ -1109,7 +1128,7 @@ impl Accountant { fn handle_request_of_scan_for_receivable( &mut self, response_skeleton_opt: Option, - ) { + ) -> ScanReschedulingAfterEarlyStop { let result: Result = self.scanners.start_receivable_scan_guarded( &self.earning_wallet, @@ -1120,29 +1139,22 @@ impl Accountant { ); match result { - Ok(scan_message) => self - .retrieve_transactions_sub_opt - .as_ref() - .expect("BlockchainBridge is unbound") - .try_send(scan_message) - .expect("BlockchainBridge is dead"), - Err(e) => { - e.log_error( - &self.logger, - ScanType::Receivables, - response_skeleton_opt.is_some(), - ); - - if let Some(skeleton) = response_skeleton_opt { - self.ui_message_sub_opt - .as_ref() - .expect("UiGateway is unbound") - .try_send(NodeToUiMessage { - target: MessageTarget::ClientId(skeleton.client_id), - body: UiScanResponse {}.tmb(skeleton.context_id), - }) - .expect("UiGateway is dead"); - }; + Ok(scan_message) => { + self.retrieve_transactions_sub_opt + .as_ref() + .expect("BlockchainBridge is unbound") + .try_send(scan_message) + .expect("BlockchainBridge is dead"); + ScanReschedulingAfterEarlyStop::DoNotSchedule + } + Err(e) => + // Any error here panics by design, so the return value is unreachable/ignored. + { + self.handle_start_scan_error( + UnableToStartScanner::Receivables, + e, + response_skeleton_opt, + ) } } } @@ -2289,7 +2301,7 @@ mod tests { let (peer_actors, peer_addresses) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) .ui_gateway(ui_gateway) - .build_and_provide_addresses(); + .build_with_addresses(); let subject_addr = subject.start(); let system = System::new("test"); let response_skeleton_opt = Some(ResponseSkeleton { @@ -2723,6 +2735,40 @@ mod tests { ); } + #[test] + #[should_panic( + expected = "internal error: entered unreachable code: Early stopped receivable scan \ + was suggested to be followed up by the scan for PendingPayables, which is not supported though" + )] + fn start_scan_error_in_receivables_and_unexpected_instruction_from_early_stop_scan_rescheduling( + ) { + let mut subject = AccountantBuilder::default().build(); + let reschedule_on_error_resolver = RescheduleScanOnErrorResolverMock::default() + .resolve_rescheduling_on_error_result(ScanReschedulingAfterEarlyStop::Schedule( + ScanType::PendingPayables, + )); + let receivable_scanner = ScannerMock::default() + .scan_started_at_result(None) + .start_scan_result(Err(StartScanError::NoConsumingWalletFound)); + subject + .scanners + .replace_scanner(ScannerReplacement::Receivable(ReplacementType::Mock( + receivable_scanner, + ))); + subject.scan_schedulers.reschedule_on_error_resolver = + Box::new(reschedule_on_error_resolver); + let system = System::new("test"); + let subject_addr = subject.start(); + + subject_addr + .try_send(ScanForReceivables { + response_skeleton_opt: None, + }) + .unwrap(); + + system.run(); + } + #[test] fn received_payments_with_response_skeleton_sends_response_to_ui_gateway() { let mut config = bc_from_earning_wallet(make_wallet("earning_wallet")); @@ -2732,7 +2778,7 @@ mod tests { receivable_scan_interval: Duration::from_millis(10_000), }); config.automatic_scans_enabled = false; - let subject = AccountantBuilder::default() + let mut subject = AccountantBuilder::default() .bootstrapper_config(config) .config_dao( ConfigDaoMock::new() @@ -2740,6 +2786,9 @@ mod tests { .set_result(Ok(())), ) .build(); + // Another scan must not be scheduled + subject.scan_schedulers.receivable.handle = + Box::new(NotifyLaterHandleMock::default().panic_on_schedule_attempt()); let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder(); let subject_addr = subject.start(); let system = System::new("test"); @@ -2846,11 +2895,13 @@ mod tests { let test_name = "accountant_scans_after_startup_and_does_not_detect_any_pending_payables"; let scan_params = ScanParams::default(); let notify_and_notify_later_params = NotifyAndNotifyLaterParams::default(); - let time_until_next_scan_params_arc = Arc::new(Mutex::new(vec![])); + let compute_time_to_next_scan_params_arc = Arc::new(Mutex::new(vec![])); let earning_wallet = make_wallet("earning"); let consuming_wallet = make_wallet("consuming"); let system = System::new(test_name); - let _ = SystemKillerActor::new(Duration::from_secs(10)).start(); + let (blockchain_bridge, _, _) = make_recorder(); + let blockchain_bridge = blockchain_bridge + .system_stop_conditions(match_lazily_every_type_id!(RetrieveTransactions)); let config = bc_from_wallets(consuming_wallet.clone(), earning_wallet.clone()); let payable_scanner = ScannerMock::new() .scan_started_at_result(None) @@ -2863,25 +2914,31 @@ mod tests { let receivable_scanner = ScannerMock::new() .scan_started_at_result(None) .start_scan_params(&scan_params.receivable_start_scan) - .start_scan_result(Err(StartScanError::NothingToProcess)); - let (subject, new_payable_expected_computed_interval, receivable_scan_interval) = + .start_scan_result(Ok(RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: None, + })); + let (subject, new_payable_expected_computed_interval) = configure_accountant_for_startup_with_preexisting_pending_payables( test_name, ¬ify_and_notify_later_params, - &time_until_next_scan_params_arc, + &compute_time_to_next_scan_params_arc, config, pending_payable_scanner, receivable_scanner, payable_scanner, ); - let peer_actors = peer_actors_builder().build(); + let peer_actors = peer_actors_builder() + .blockchain_bridge(blockchain_bridge) + .build(); let subject_addr: Addr = subject.start(); let subject_subs = Accountant::make_subs_from(&subject_addr); send_bind_message!(subject_subs, peer_actors); send_start_message!(subject_subs); - // The system is stopped by the NotifyLaterHandleMock for the Receivable scanner + // The system is stopped by the time the RetrieveTransactions msg arrives at the mocked + // BlockchainBridge. let before = SystemTime::now(); system.run(); let after = SystemTime::now(); @@ -2896,15 +2953,13 @@ mod tests { assert_payable_scanner_for_no_pending_payable_found( &scan_params.payable_start_scan, ¬ify_and_notify_later_params, - time_until_next_scan_params_arc, + compute_time_to_next_scan_params_arc, new_payable_expected_computed_interval, ); assert_receivable_scanner( test_name, earning_wallet, &scan_params.receivable_start_scan, - ¬ify_and_notify_later_params.receivables_notify_later, - receivable_scan_interval, ); // The test lays down evidences that the NewPayableScanner couldn't run before // the PendingPayableScanner, which is an intention. @@ -2962,8 +3017,11 @@ mod tests { let receivable_scanner = ScannerMock::new() .scan_started_at_result(None) .start_scan_params(&scan_params.receivable_start_scan) - .start_scan_result(Err(StartScanError::NothingToProcess)); - let (subject, expected_pending_payable_notify_later_interval, receivable_scan_interval) = + .start_scan_result(Ok(RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: None, + })); + let (subject, expected_pending_payable_notify_later_interval) = configure_accountant_for_startup_with_no_preexisting_pending_payables( test_name, ¬ify_and_notify_later_params, @@ -2972,7 +3030,7 @@ mod tests { pending_payable_scanner, receivable_scanner, ); - let (peer_actors, addresses) = peer_actors_builder().build_and_provide_addresses(); + let (peer_actors, addresses) = peer_actors_builder().build_with_addresses(); let subject_addr: Addr = subject.start(); let subject_subs = Accountant::make_subs_from(&subject_addr); let expected_tx_receipts_msg = TxReceiptsMessage { @@ -3039,8 +3097,6 @@ mod tests { test_name, earning_wallet, &scan_params.receivable_start_scan, - ¬ify_and_notify_later_params.receivables_notify_later, - receivable_scan_interval, ); // Since the assertions proved that the pending payable scanner had run multiple times // before the new payable scanner started or was scheduled, the front position definitely @@ -3065,13 +3121,12 @@ mod tests { new_payables_notify: Arc>>, retry_payables_notify_later: Arc>>, pending_payables_notify_later: Arc>>, - receivables_notify_later: Arc>>, } fn configure_accountant_for_startup_with_preexisting_pending_payables( test_name: &str, notify_and_notify_later_params: &NotifyAndNotifyLaterParams, - time_until_next_scan_params_arc: &Arc>>, + compute_time_to_next_scan_params_arc: &Arc>>, config: BootstrapperConfig, pending_payable_scanner: ScannerMock< RequestTransactionReceipts, @@ -3084,7 +3139,7 @@ mod tests { Option, >, payable_scanner: ScannerMock, - ) -> (Accountant, Duration, Duration) { + ) -> (Accountant, Duration) { let mut subject = make_subject_and_inject_scanners( test_name, config, @@ -3094,7 +3149,6 @@ mod tests { ); let new_payable_expected_computed_interval = Duration::from_secs(3600); // Important that this is made short because the test relies on it with the system stop. - let receivable_scan_interval = Duration::from_millis(50); subject.scan_schedulers.pending_payable.handle = Box::new( NotifyLaterHandleMock::default() .notify_later_params(¬ify_and_notify_later_params.pending_payables_notify_later), @@ -3111,22 +3165,13 @@ mod tests { NotifyHandleMock::default() .notify_params(¬ify_and_notify_later_params.new_payables_notify), ); - let receivable_notify_later_handle_mock = NotifyLaterHandleMock::default() - .notify_later_params(¬ify_and_notify_later_params.receivables_notify_later) - .stop_system_on_count_received(1); - subject.scan_schedulers.receivable.handle = Box::new(receivable_notify_later_handle_mock); - subject.scan_schedulers.receivable.interval = receivable_scan_interval; let interval_computer = NewPayableScanIntervalComputerMock::default() - .time_until_next_scan_params(&time_until_next_scan_params_arc) - .time_until_next_scan_result(ScanTiming::WaitFor( + .compute_time_to_next_scan_params(&compute_time_to_next_scan_params_arc) + .compute_time_to_next_scan_result(ScanTiming::WaitFor( new_payable_expected_computed_interval, )); subject.scan_schedulers.payable.interval_computer = Box::new(interval_computer); - ( - subject, - new_payable_expected_computed_interval, - receivable_scan_interval, - ) + (subject, new_payable_expected_computed_interval) } fn configure_accountant_for_startup_with_no_preexisting_pending_payables( @@ -3144,7 +3189,7 @@ mod tests { ReceivedPayments, Option, >, - ) -> (Accountant, Duration, Duration) { + ) -> (Accountant, Duration) { let mut subject = make_subject_and_inject_scanners( test_name, config, @@ -3152,16 +3197,15 @@ mod tests { receivable_scanner, payable_scanner, ); - let retry_payble_scan_interval = Duration::from_millis(1); + let retry_payable_scan_interval = Duration::from_millis(1); let pending_payable_scan_interval = Duration::from_secs(3600); - let receivable_scan_interval = Duration::from_secs(3600); let pending_payable_notify_later_handle_mock = NotifyLaterHandleMock::default() .notify_later_params(¬ify_and_notify_later_params.pending_payables_notify_later) // This should stop the system .stop_system_on_count_received(1); subject.scan_schedulers.pending_payable.handle = Box::new(pending_payable_notify_later_handle_mock); - subject.scan_schedulers.payable.retry_payable_scan_interval = retry_payble_scan_interval; + subject.scan_schedulers.payable.retry_payable_scan_interval = retry_payable_scan_interval; subject.scan_schedulers.pending_payable.interval = pending_payable_scan_interval; subject.scan_schedulers.payable.new_payable_notify_later = Box::new( NotifyLaterHandleMock::default() @@ -3176,15 +3220,7 @@ mod tests { NotifyHandleMock::default() .notify_params(¬ify_and_notify_later_params.new_payables_notify), ); - let receivable_notify_later_handle_mock = NotifyLaterHandleMock::default() - .notify_later_params(¬ify_and_notify_later_params.receivables_notify_later); - subject.scan_schedulers.receivable.interval = receivable_scan_interval; - subject.scan_schedulers.receivable.handle = Box::new(receivable_notify_later_handle_mock); - ( - subject, - pending_payable_scan_interval, - receivable_scan_interval, - ) + (subject, pending_payable_scan_interval) } fn make_subject_and_inject_scanners( @@ -3485,16 +3521,8 @@ mod tests { receivable_start_scan_params_arc: &Arc< Mutex, Logger, String)>>, >, - scan_for_receivables_notify_later_params_arc: &Arc< - Mutex>, - >, - receivable_scan_interval: Duration, ) { assert_receivable_scan_ran(test_name, receivable_start_scan_params_arc, earning_wallet); - assert_another_receivable_scan_scheduled( - scan_for_receivables_notify_later_params_arc, - receivable_scan_interval, - ) } fn assert_receivable_scan_ran( @@ -3522,25 +3550,6 @@ mod tests { assert_using_the_same_logger(&r_logger, test_name, Some("r")); } - fn assert_another_receivable_scan_scheduled( - scan_for_receivables_notify_later_params_arc: &Arc< - Mutex>, - >, - receivable_scan_interval: Duration, - ) { - let scan_for_receivables_notify_later_params = - scan_for_receivables_notify_later_params_arc.lock().unwrap(); - assert_eq!( - *scan_for_receivables_notify_later_params, - vec![( - ScanForReceivables { - response_skeleton_opt: None - }, - receivable_scan_interval - )] - ); - } - #[test] fn initial_pending_payable_scan_if_some_payables_found() { let sent_payable_dao = @@ -3637,23 +3646,31 @@ mod tests { let start_scan_params_arc = Arc::new(Mutex::new(vec![])); let notify_later_receivable_params_arc = Arc::new(Mutex::new(vec![])); let system = System::new(test_name); + let (blockchain_bridge, _, blockchain_bridge_recording_arc) = make_recorder(); + let blockchain_bridge = blockchain_bridge.system_stop_conditions( + match_lazily_every_type_id!(RetrieveTransactions, RetrieveTransactions), + ); SystemKillerActor::new(Duration::from_secs(10)).start(); // a safety net for GitHub Actions + let recipient = make_wallet("some_recipient"); let receivable_scanner = ScannerMock::new() .scan_started_at_result(None) .scan_started_at_result(None) .start_scan_params(&start_scan_params_arc) - .start_scan_result(Err(StartScanError::NothingToProcess)) .start_scan_result(Ok(RetrieveTransactions { - recipient: make_wallet("some_recipient"), + recipient: recipient.clone(), response_skeleton_opt: None, })) - .stop_the_system_after_last_msg(); + .start_scan_result(Ok(RetrieveTransactions { + recipient: recipient.clone(), + response_skeleton_opt: None, + })) + .finish_scan_result(None); let earning_wallet = make_wallet("earning"); let mut config = bc_from_earning_wallet(earning_wallet.clone()); config.scan_intervals_opt = Some(ScanIntervals { payable_scan_interval: Duration::from_secs(100), pending_payable_scan_interval: Duration::from_secs(10), - receivable_scan_interval: Duration::from_millis(99), + receivable_scan_interval: Duration::from_millis(15), }); let mut subject = AccountantBuilder::default() .bootstrapper_config(config) @@ -3671,8 +3688,25 @@ mod tests { ); let subject_addr = subject.start(); let subject_subs = Accountant::make_subs_from(&subject_addr); - let peer_actors = peer_actors_builder().build(); + let (peer_actors, peer_actors_addrs) = peer_actors_builder() + .blockchain_bridge(blockchain_bridge) + .build_with_addresses(); send_bind_message!(subject_subs, peer_actors); + let counter_msg = ReceivedPayments { + timestamp: SystemTime::now(), + new_start_block: BlockMarker::Value(1234), + transactions: vec![], + response_skeleton_opt: None, + }; + let counter_msg_setup = setup_for_counter_msg_triggered_via_type_id!( + RetrieveTransactions, + counter_msg, + subject_addr + ); + peer_actors_addrs + .blockchain_bridge_addr + .try_send(SetUpCounterMsgs::new(vec![counter_msg_setup])) + .unwrap(); subject_addr .try_send(ScanForReceivables { @@ -3710,37 +3744,37 @@ mod tests { assert!(start_scan_params.is_empty()); debug!( first_attempt_logger, - "first attempt verifying receivable scanner" + "first attempt receivable scanner logger verification" ); debug!( second_attempt_logger, - "second attempt verifying receivable scanner" + "second attempt receivable scanner logger verification" ); + let blockchain_bridge_recording = blockchain_bridge_recording_arc.lock().unwrap(); + let first_msg_towards_bb = + blockchain_bridge_recording.get_record::(0); + let expected_msg = RetrieveTransactions { + recipient, + response_skeleton_opt: None, + }; + assert_eq!(first_msg_towards_bb, &expected_msg); + let second_msg_towards_bb = + blockchain_bridge_recording.get_record::(1); + assert_eq!(second_msg_towards_bb, &expected_msg); assert_eq!( *notify_later_receivable_params, - vec![ - ( - ScanForReceivables { - response_skeleton_opt: None - }, - Duration::from_millis(99) - ), - ( - ScanForReceivables { - response_skeleton_opt: None - }, - Duration::from_millis(99) - ), - ] + vec![( + ScanForReceivables { + response_skeleton_opt: None + }, + Duration::from_millis(15) + ),] ); tlh.exists_log_containing(&format!( - "DEBUG: {test_name}: There was nothing to process during Receivables scan." + "DEBUG: {test_name}: first attempt receivable scanner logger verification", )); tlh.exists_log_containing(&format!( - "DEBUG: {test_name}: first attempt verifying receivable scanner", - )); - tlh.exists_log_containing(&format!( - "DEBUG: {test_name}: second attempt verifying receivable scanner", + "DEBUG: {test_name}: second attempt receivable scanner logger verification", )); } @@ -4215,7 +4249,8 @@ mod tests { expected = "internal error: entered unreachable code: Early stopped new payable scan \ was suggested to be followed up by the scan for Receivables, which is not supported though" )] - fn start_scan_error_in_new_payables_and_unexpected_reaction_by_receivable_scan_scheduling() { + fn start_scan_error_in_new_payables_and_unexpected_instruction_from_early_stop_scan_rescheduling( + ) { let mut subject = AccountantBuilder::default().build(); let reschedule_on_error_resolver = RescheduleScanOnErrorResolverMock::default() .resolve_rescheduling_on_error_result(ScanReschedulingAfterEarlyStop::Schedule( @@ -4435,32 +4470,6 @@ mod tests { ); } - #[test] - #[should_panic( - expected = "internal error: entered unreachable code: Early stopped pending payable scan \ - was suggested to be followed up by the scan for Receivables, which is not supported though" - )] - fn start_scan_error_in_pending_payables_and_unexpected_reaction_by_receivable_scan_scheduling() - { - let mut subject = AccountantBuilder::default().build(); - let reschedule_on_error_resolver = RescheduleScanOnErrorResolverMock::default() - .resolve_rescheduling_on_error_result(ScanReschedulingAfterEarlyStop::Schedule( - ScanType::Receivables, - )); - subject.scan_schedulers.reschedule_on_error_resolver = - Box::new(reschedule_on_error_resolver); - let system = System::new("test"); - let subject_addr = subject.start(); - - subject_addr - .try_send(ScanForPendingPayables { - response_skeleton_opt: None, - }) - .unwrap(); - - system.run(); - } - #[test] fn report_routing_service_provided_message_is_received() { init_test_logging(); @@ -5501,7 +5510,7 @@ mod tests { let test_name = "accountant_confirms_all_pending_txs_and_schedules_new_payable_scanner_timely"; let finish_scan_params_arc = Arc::new(Mutex::new(vec![])); - let time_until_next_scan_params_arc = Arc::new(Mutex::new(vec![])); + let compute_time_to_next_scan_params_arc = Arc::new(Mutex::new(vec![])); let new_payable_notify_later_arc = Arc::new(Mutex::new(vec![])); let new_payable_notify_arc = Arc::new(Mutex::new(vec![])); let system = System::new("new_payable_scanner_timely"); @@ -5518,9 +5527,9 @@ mod tests { ))); let expected_computed_interval = Duration::from_secs(3); let interval_computer = NewPayableScanIntervalComputerMock::default() - .time_until_next_scan_params(&time_until_next_scan_params_arc) + .compute_time_to_next_scan_params(&compute_time_to_next_scan_params_arc) // This determines the test - .time_until_next_scan_result(ScanTiming::WaitFor(expected_computed_interval)); + .compute_time_to_next_scan_result(ScanTiming::WaitFor(expected_computed_interval)); subject.scan_schedulers.payable.interval_computer = Box::new(interval_computer); subject.scan_schedulers.payable.new_payable_notify_later = Box::new( NotifyLaterHandleMock::default().notify_later_params(&new_payable_notify_later_arc), @@ -5578,7 +5587,7 @@ mod tests { let test_name = "accountant_confirms_payable_txs_and_schedules_the_delayed_new_payable_scanner_asap"; let finish_scan_params_arc = Arc::new(Mutex::new(vec![])); - let time_until_next_scan_params_arc = Arc::new(Mutex::new(vec![])); + let compute_time_to_next_scan_params_arc = Arc::new(Mutex::new(vec![])); let new_payable_notify_later_arc = Arc::new(Mutex::new(vec![])); let new_payable_notify_arc = Arc::new(Mutex::new(vec![])); let mut subject = AccountantBuilder::default() @@ -5593,9 +5602,9 @@ mod tests { pending_payable_scanner, ))); let interval_computer = NewPayableScanIntervalComputerMock::default() - .time_until_next_scan_params(&time_until_next_scan_params_arc) + .compute_time_to_next_scan_params(&compute_time_to_next_scan_params_arc) // This determines the test - .time_until_next_scan_result(ScanTiming::ReadyNow); + .compute_time_to_next_scan_result(ScanTiming::ReadyNow); subject.scan_schedulers.payable.interval_computer = Box::new(interval_computer); subject.scan_schedulers.payable.new_payable_notify_later = Box::new( NotifyLaterHandleMock::default().notify_later_params(&new_payable_notify_later_arc), @@ -5630,8 +5639,8 @@ mod tests { "Should be empty but {:?}", finish_scan_params ); - let time_until_next_scan_params = time_until_next_scan_params_arc.lock().unwrap(); - assert_eq!(*time_until_next_scan_params, vec![()]); + let compute_time_to_next_scan_params = compute_time_to_next_scan_params_arc.lock().unwrap(); + assert_eq!(*compute_time_to_next_scan_params, vec![()]); let new_payable_notify_later = new_payable_notify_later_arc.lock().unwrap(); assert!( new_payable_notify_later.is_empty(), @@ -5682,7 +5691,7 @@ mod tests { }), }]); let left_side_bound = if let ScanTiming::WaitFor(interval) = - assertion_interval_computer.time_until_next_scan() + assertion_interval_computer.compute_time_to_next_scan() { interval } else { @@ -5696,7 +5705,7 @@ mod tests { let new_payable_notify_later = new_payable_notify_later_arc.lock().unwrap(); let (_, actual_interval) = new_payable_notify_later[0]; let right_side_bound = if let ScanTiming::WaitFor(interval) = - assertion_interval_computer.time_until_next_scan() + assertion_interval_computer.compute_time_to_next_scan() { interval } else { @@ -5874,8 +5883,9 @@ mod tests { // Setup let notify_later_params_arc = Arc::new(Mutex::new(vec![])); scan_schedulers.payable.interval_computer = Box::new( - NewPayableScanIntervalComputerMock::default() - .time_until_next_scan_result(ScanTiming::WaitFor(Duration::from_secs(152))), + NewPayableScanIntervalComputerMock::default().compute_time_to_next_scan_result( + ScanTiming::WaitFor(Duration::from_secs(152)), + ), ); scan_schedulers.payable.new_payable_notify_later = Box::new( NotifyLaterHandleMock::default().notify_later_params(¬ify_later_params_arc), diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index cbc337a8d..d14814a5b 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -122,7 +122,10 @@ impl Scanners { }); } - Self::start_correct_payable_scanner::( + <(dyn MultistageDualPayableScanner) as StartableScanner< + ScanForNewPayables, + InitialTemplatesMessage, + >>::start_scan( &mut *self.payable, wallet, timestamp, @@ -150,7 +153,10 @@ impl Scanners { ) } - Self::start_correct_payable_scanner::( + <(dyn MultistageDualPayableScanner) as StartableScanner< + ScanForRetryPayables, + InitialTemplatesMessage, + >>::start_scan( &mut *self.payable, wallet, timestamp, @@ -168,10 +174,14 @@ impl Scanners { automatic_scans_enabled: bool, ) -> Result { 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, - logger: &Logger, - ) -> Result - where - TriggerMessage: Message, - (dyn MultistageDualPayableScanner + 'a): - StartableScanner, - { - <(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); diff --git a/node/src/accountant/scanners/payable_scanner/start_scan.rs b/node/src/accountant/scanners/payable_scanner/start_scan.rs index 457ab73ee..654580c73 100644 --- a/node/src/accountant/scanners/payable_scanner/start_scan.rs +++ b/node/src/accountant/scanners/payable_scanner/start_scan.rs @@ -35,7 +35,7 @@ impl StartableScanner 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::( + let result = <(dyn MultistageDualPayableScanner) as StartableScanner< + ScanForRetryPayables, + InitialTemplatesMessage, + >>::start_scan( &mut subject, &consuming_wallet, timestamp, diff --git a/node/src/accountant/scanners/pending_payable_scanner/mod.rs b/node/src/accountant/scanners/pending_payable_scanner/mod.rs index 4fe12add1..d3fe5a29d 100644 --- a/node/src/accountant/scanners/pending_payable_scanner/mod.rs +++ b/node/src/accountant/scanners/pending_payable_scanner/mod.rs @@ -95,10 +95,12 @@ impl StartableScanner 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, StartScanError> { + fn harvest_tables( + &mut self, + logger: &Logger, + response_skeleton_opt: Option, + ) -> Result, 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); } diff --git a/node/src/accountant/scanners/scan_schedulers.rs b/node/src/accountant/scanners/scan_schedulers.rs index 97a3edd62..5de151c73 100644 --- a/node/src/accountant/scanners/scan_schedulers.rs +++ b/node/src/accountant/scanners/scan_schedulers.rs @@ -51,28 +51,31 @@ pub enum ScanReschedulingAfterEarlyStop { } #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum PayableSequenceScanner { +pub enum UnableToStartScanner { NewPayables, RetryPayables, PendingPayables { initial_pending_payable_scan: bool }, + Receivables, } -impl Display for PayableSequenceScanner { +impl Display for UnableToStartScanner { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - PayableSequenceScanner::NewPayables => write!(f, "NewPayables"), - PayableSequenceScanner::RetryPayables => write!(f, "RetryPayables"), - PayableSequenceScanner::PendingPayables { .. } => write!(f, "PendingPayables"), + UnableToStartScanner::NewPayables => write!(f, "NewPayables"), + UnableToStartScanner::RetryPayables => write!(f, "RetryPayables"), + UnableToStartScanner::PendingPayables { .. } => write!(f, "PendingPayables"), + UnableToStartScanner::Receivables => write!(f, "Receivables"), } } } -impl From for ScanType { - fn from(scanner: PayableSequenceScanner) -> Self { +impl From for ScanType { + fn from(scanner: UnableToStartScanner) -> Self { match scanner { - PayableSequenceScanner::NewPayables => ScanType::Payables, - PayableSequenceScanner::RetryPayables => ScanType::Payables, - PayableSequenceScanner::PendingPayables { .. } => ScanType::PendingPayables, + UnableToStartScanner::NewPayables => ScanType::Payables, + UnableToStartScanner::RetryPayables => ScanType::Payables, + UnableToStartScanner::PendingPayables { .. } => ScanType::PendingPayables, + UnableToStartScanner::Receivables => ScanType::Receivables, } } } @@ -99,7 +102,7 @@ impl PayableScanScheduler { } pub fn schedule_new_payable_scan(&self, ctx: &mut Context, logger: &Logger) { - if let ScanTiming::WaitFor(interval) = self.interval_computer.time_until_next_scan() { + if let ScanTiming::WaitFor(interval) = self.interval_computer.compute_time_to_next_scan() { debug!( logger, "Scheduling a new-payable scan in {}ms", @@ -153,7 +156,7 @@ impl PayableScanScheduler { } pub trait NewPayableScanIntervalComputer { - fn time_until_next_scan(&self) -> ScanTiming; + fn compute_time_to_next_scan(&self) -> ScanTiming; fn reset_last_scan_timestamp(&mut self); @@ -167,7 +170,7 @@ pub struct NewPayableScanIntervalComputerReal { } impl NewPayableScanIntervalComputer for NewPayableScanIntervalComputerReal { - fn time_until_next_scan(&self) -> ScanTiming { + fn compute_time_to_next_scan(&self) -> ScanTiming { let current_time = self.clock.now(); let time_since_last_scan = current_time .duration_since(self.last_scan_timestamp) @@ -250,7 +253,7 @@ where pub trait RescheduleScanOnErrorResolver { fn resolve_rescheduling_on_error( &self, - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, @@ -263,25 +266,28 @@ pub struct RescheduleScanOnErrorResolverReal {} impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverReal { fn resolve_rescheduling_on_error( &self, - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop { let reschedule_hint = match scanner { - PayableSequenceScanner::NewPayables => { + UnableToStartScanner::NewPayables => { Self::resolve_new_payables(error, is_externally_triggered) } - PayableSequenceScanner::RetryPayables => { + UnableToStartScanner::RetryPayables => { Self::resolve_retry_payables(error, is_externally_triggered) } - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, } => Self::resolve_pending_payables( error, initial_pending_payable_scan, is_externally_triggered, ), + UnableToStartScanner::Receivables => { + Self::resolve_receivables(error, is_externally_triggered) + } }; Self::log_rescheduling(scanner, is_externally_triggered, logger, &reschedule_hint); @@ -296,14 +302,18 @@ impl RescheduleScanOnErrorResolverReal { is_externally_triggered: bool, ) -> ScanReschedulingAfterEarlyStop { if is_externally_triggered { - ScanReschedulingAfterEarlyStop::DoNotSchedule - } else if matches!(err, StartScanError::ScanAlreadyRunning { .. }) { - unreachable!( - "an automatic scan of NewPayableScanner should never interfere with itself {:?}", - err - ) - } else { - ScanReschedulingAfterEarlyStop::Schedule(ScanType::Payables) + return ScanReschedulingAfterEarlyStop::DoNotSchedule; + } + + match err { + StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, + StartScanError::ScanAlreadyRunning { .. } => { + unreachable!( + "an automatic scan of NewPayableScanner should never interfere with itself {:?}", + err + ) + } + _ => ScanReschedulingAfterEarlyStop::Schedule(ScanType::Payables), } } @@ -330,18 +340,26 @@ impl RescheduleScanOnErrorResolverReal { is_externally_triggered: bool, ) -> ScanReschedulingAfterEarlyStop { if is_externally_triggered { - ScanReschedulingAfterEarlyStop::DoNotSchedule - } else if err == &StartScanError::NothingToProcess { - if initial_pending_payable_scan { + return ScanReschedulingAfterEarlyStop::DoNotSchedule; + } + + match err { + StartScanError::NothingToProcess => { + if !initial_pending_payable_scan { + unreachable!( + "the automatic pending payable scan should always be requested only in need, \ + which contradicts the current StartScanError::NothingToProcess" + ) + } ScanReschedulingAfterEarlyStop::Schedule(ScanType::Payables) - } else { - unreachable!( - "the automatic pending payable scan should always be requested only in need, \ - which contradicts the current StartScanError::NothingToProcess" - ) } - } else if err == &StartScanError::NoConsumingWalletFound { - if initial_pending_payable_scan { + StartScanError::NoConsumingWalletFound => { + if !initial_pending_payable_scan { + unreachable!( + "PendingPayableScanner called later than the initial attempt, but \ + the consuming wallet is still missing; this should not be possible" + ) + } // Cannot deduce there are strayed pending payables from the previous Node's run // (StartScanError::NoConsumingWalletFound is thrown before // StartScanError::NothingToProcess can be evaluated); but may be cautious and @@ -350,22 +368,34 @@ impl RescheduleScanOnErrorResolverReal { // TODO Correctly, a check-point during the bootstrap, not allowing to come // this far, should be the solution. Part of the issue mentioned in GH-799 ScanReschedulingAfterEarlyStop::Schedule(ScanType::PendingPayables) - } else { - unreachable!( - "PendingPayableScanner called later than the initial attempt, but \ - the consuming wallet is still missing; this should not be possible" - ) } - } else { - unreachable!( + StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, + _ => unreachable!( "{:?} should be impossible with PendingPayableScanner in automatic mode", err - ) + ), + } + } + + fn resolve_receivables( + err: &StartScanError, + is_externally_triggered: bool, + ) -> ScanReschedulingAfterEarlyStop { + if is_externally_triggered { + return ScanReschedulingAfterEarlyStop::DoNotSchedule; + } + + match err { + StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, + _ => unreachable!( + "{:?} should be impossible with ReceivableScanner in automatic mode", + err + ), } } fn log_rescheduling( - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, is_externally_triggered: bool, logger: &Logger, reschedule_hint: &ScanReschedulingAfterEarlyStop, @@ -389,8 +419,8 @@ impl RescheduleScanOnErrorResolverReal { #[cfg(test)] mod tests { use crate::accountant::scanners::scan_schedulers::{ - NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, PayableSequenceScanner, - ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, + NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, + ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, UnableToStartScanner, }; use crate::accountant::scanners::test_utils::NewPayableScanIntervalComputerMock; use crate::accountant::scanners::{ManulTriggerError, StartScanError}; @@ -469,7 +499,7 @@ mod tests { subject.scan_interval = standard_interval; subject.last_scan_timestamp = past_instant; - let result = subject.time_until_next_scan(); + let result = subject.compute_time_to_next_scan(); assert_eq!( result, @@ -504,7 +534,7 @@ mod tests { subject.scan_interval = standard_interval; subject.last_scan_timestamp = past_instant; - let result = subject.time_until_next_scan(); + let result = subject.compute_time_to_next_scan(); assert_eq!( result, @@ -524,7 +554,7 @@ mod tests { subject.scan_interval = Duration::from_secs(180); subject.clock = Box::new(SimpleClockMock::default().now_result(now)); - let result = subject.time_until_next_scan(); + let result = subject.compute_time_to_next_scan(); assert_eq!( result, @@ -570,7 +600,7 @@ mod tests { subject.clock = Box::new(SimpleClockMock::default().now_result(now)); subject.last_scan_timestamp = now.checked_add(Duration::from_secs(1)).unwrap(); - let _ = subject.time_until_next_scan(); + let _ = subject.compute_time_to_next_scan(); } #[test] @@ -648,7 +678,6 @@ mod tests { StartScanError::CalledFromNullScanner ]; - let mut check_vec = candidates .iter() .fold(vec![],|mut acc, current|{ @@ -709,14 +738,14 @@ mod tests { test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = false)", test_name), &subject, - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }, ); test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = true)", test_name), &subject, - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }, ); @@ -725,7 +754,7 @@ mod tests { fn test_what_if_externally_triggered( test_name: &str, subject: &ScanSchedulers, - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, ) { init_test_logging(); let logger = Logger::new(test_name); @@ -755,17 +784,16 @@ mod tests { } #[test] - fn resolve_error_for_pending_payables_if_nothing_to_process_and_initial_pending_payable_scan_true( - ) { + fn resolve_error_if_nothing_to_process_and_initial_pending_payable_scan_true() { init_test_logging(); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); - let test_name = "resolve_error_for_pending_payables_if_nothing_to_process_and_initial_pending_payable_scan_true"; + let test_name = "resolve_error_if_nothing_to_process_and_initial_pending_payable_scan_true"; let logger = Logger::new(test_name); let result = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }, &StartScanError::NothingToProcess, @@ -798,7 +826,7 @@ mod tests { let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }, &StartScanError::NothingToProcess, @@ -808,12 +836,13 @@ mod tests { } #[test] - fn resolve_error_for_pending_p_if_no_consuming_wallet_found_in_initial_pending_payable_scan() { + fn resolve_error_if_no_consuming_wallet_found_in_initial_pending_payable_scan() { init_test_logging(); - let test_name = "resolve_error_for_pending_p_if_no_consuming_wallet_found_in_initial_pending_payable_scan"; + let test_name = + "resolve_error_if_no_consuming_wallet_found_in_initial_pending_payable_scan"; let logger = Logger::new(test_name); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); - let scanner = PayableSequenceScanner::PendingPayables { + let scanner = UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }; @@ -845,9 +874,10 @@ mod tests { than the initial attempt, but the consuming wallet is still missing; this should not be \ possible" )] - fn pending_p_scan_attempt_if_no_consuming_wallet_found_mustnt_happen_if_not_initial_scan() { + fn pending_payable_scan_attempt_if_no_consuming_wallet_found_mustnt_happen_if_not_initial_scan() + { let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); - let scanner = PayableSequenceScanner::PendingPayables { + let scanner = UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }; @@ -861,6 +891,20 @@ mod tests { ); } + #[test] + fn resolve_error_for_pending_payables_if_null_scanner_is_used_in_automatic_mode() { + test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + UnableToStartScanner::PendingPayables { + initial_pending_payable_scan: true, + }, + ); + test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + UnableToStartScanner::PendingPayables { + initial_pending_payable_scan: false, + }, + ); + } + #[test] fn resolve_error_for_pending_payables_forbidden_states() { fn test_forbidden_states( @@ -873,7 +917,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, }, *error, @@ -900,6 +944,7 @@ mod tests { let inputs = ListOfStartScanErrors::default().eliminate_already_tested_variants(vec![ StartScanError::NothingToProcess, StartScanError::NoConsumingWalletFound, + StartScanError::CalledFromNullScanner, ]); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); @@ -916,7 +961,7 @@ mod tests { test_what_if_externally_triggered( test_name, &subject, - PayableSequenceScanner::RetryPayables {}, + UnableToStartScanner::RetryPayables {}, ); } @@ -929,7 +974,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::RetryPayables, + UnableToStartScanner::RetryPayables, error, false, &Logger::new("test"), @@ -952,15 +997,14 @@ mod tests { } #[test] - fn resolve_rescheduling_on_error_works_for_new_payables_if_externally_triggered() { - let test_name = - "resolve_rescheduling_on_error_works_for_new_payables_if_externally_triggered"; + fn resolve_rescheduling_on_error_for_new_payables_if_externally_triggered() { + let test_name = "resolve_rescheduling_on_error_for_new_payables_if_externally_triggered"; let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); test_what_if_externally_triggered( test_name, &subject, - PayableSequenceScanner::NewPayables {}, + UnableToStartScanner::NewPayables {}, ); } @@ -969,13 +1013,13 @@ mod tests { expected = "internal error: entered unreachable code: an automatic scan of NewPayableScanner \ should never interfere with itself ScanAlreadyRunning { cross_scan_cause_opt: None, started_at:" )] - fn resolve_hint_for_new_payables_if_scan_is_already_running_error_and_is_automatic_scan() { + fn resolve_hint_for_new_payables_error_if_scan_is_already_running_in_automatic_scan() { let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::NewPayables, + UnableToStartScanner::NewPayables, &StartScanError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: SystemTime::now(), @@ -985,10 +1029,18 @@ mod tests { ); } + #[test] + fn resolve_error_for_new_payables_if_null_scanner_is_used_in_automatic_mode() { + test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + UnableToStartScanner::NewPayables, + ); + } + #[test] fn resolve_new_payables_with_error_cases_resulting_in_future_rescheduling() { let test_name = "resolve_new_payables_with_error_cases_resulting_in_future_rescheduling"; let inputs = ListOfStartScanErrors::default().eliminate_already_tested_variants(vec![ + StartScanError::CalledFromNullScanner, StartScanError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: SystemTime::now(), @@ -1002,7 +1054,7 @@ mod tests { let result = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::NewPayables, + UnableToStartScanner::NewPayables, *error, false, &logger, @@ -1021,27 +1073,100 @@ mod tests { }) } + #[test] + fn resolve_rescheduling_on_error_for_receivables_if_externally_triggered() { + let test_name = "resolve_rescheduling_on_error_for_receivables_if_externally_triggered"; + let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); + + test_what_if_externally_triggered(test_name, &subject, UnableToStartScanner::Receivables); + } + + #[test] + fn resolve_error_for_receivables_if_null_scanner_is_used_in_automatic_mode() { + test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + UnableToStartScanner::Receivables, + ); + } + + #[test] + fn resolve_error_for_receivables_all_fatal_cases_in_automatic_mode() { + let inputs = ListOfStartScanErrors::default() + .eliminate_already_tested_variants(vec![StartScanError::CalledFromNullScanner]); + let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); + + inputs.errors.iter().for_each(|error| { + let panic = catch_unwind(AssertUnwindSafe(|| { + subject + .reschedule_on_error_resolver + .resolve_rescheduling_on_error( + UnableToStartScanner::Receivables, + *error, + false, + &Logger::new("test"), + ) + })) + .unwrap_err(); + + let panic_msg = panic.downcast_ref::().unwrap(); + let expected_msg = format!( + "internal error: entered unreachable code: {:?} should be impossible with \ + ReceivableScanner in automatic mode", + error + ); + assert_eq!( + panic_msg, &expected_msg, + "We expected '{}' but got '{}'", + expected_msg, panic_msg + ) + }) + } + #[test] fn conversion_between_hintable_scanner_and_scan_type_works() { assert_eq!( - ScanType::from(PayableSequenceScanner::NewPayables), + ScanType::from(UnableToStartScanner::NewPayables), ScanType::Payables ); assert_eq!( - ScanType::from(PayableSequenceScanner::RetryPayables), + ScanType::from(UnableToStartScanner::RetryPayables), ScanType::Payables ); assert_eq!( - ScanType::from(PayableSequenceScanner::PendingPayables { + ScanType::from(UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(PayableSequenceScanner::PendingPayables { + ScanType::from(UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true }), ScanType::PendingPayables ); + assert_eq!( + ScanType::from(UnableToStartScanner::Receivables), + ScanType::Receivables + ) + } + + fn test_resolve_error_if_null_scanner_is_used_in_automatic_mode(scanner: UnableToStartScanner) { + let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); + + let result = subject + .reschedule_on_error_resolver + .resolve_rescheduling_on_error( + scanner, + &StartScanError::CalledFromNullScanner, + false, + &Logger::new("test"), + ); + + assert_eq!( + result, + ScanReschedulingAfterEarlyStop::DoNotSchedule, + "We expected DoNotSchedule but got {:?} for {:?}", + result, + scanner + ); } } diff --git a/node/src/accountant/scanners/test_utils.rs b/node/src/accountant/scanners/test_utils.rs index 08325dedc..031e1bb7d 100644 --- a/node/src/accountant/scanners/test_utils.rs +++ b/node/src/accountant/scanners/test_utils.rs @@ -18,8 +18,8 @@ use crate::accountant::scanners::pending_payable_scanner::{ CachesEmptiableScanner, ExtendedPendingPayablePrivateScanner, }; use crate::accountant::scanners::scan_schedulers::{ - NewPayableScanIntervalComputer, PayableSequenceScanner, RescheduleScanOnErrorResolver, - ScanReschedulingAfterEarlyStop, ScanTiming, + NewPayableScanIntervalComputer, RescheduleScanOnErrorResolver, ScanReschedulingAfterEarlyStop, + ScanTiming, UnableToStartScanner, }; use crate::accountant::scanners::{ PendingPayableScanner, PrivateScanner, RealScannerMarker, ReceivableScanner, Scanner, @@ -336,15 +336,20 @@ impl ScannerMockMarker for ScannerMock>>, - time_until_next_scan_results: RefCell>, + compute_time_to_next_scan_params: Arc>>, + compute_time_to_next_scan_results: RefCell>, reset_last_scan_timestamp_params: Arc>>, } impl NewPayableScanIntervalComputer for NewPayableScanIntervalComputerMock { - fn time_until_next_scan(&self) -> ScanTiming { - self.time_until_next_scan_params.lock().unwrap().push(()); - self.time_until_next_scan_results.borrow_mut().remove(0) + fn compute_time_to_next_scan(&self) -> ScanTiming { + self.compute_time_to_next_scan_params + .lock() + .unwrap() + .push(()); + self.compute_time_to_next_scan_results + .borrow_mut() + .remove(0) } fn reset_last_scan_timestamp(&mut self) { @@ -358,13 +363,15 @@ impl NewPayableScanIntervalComputer for NewPayableScanIntervalComputerMock { } impl NewPayableScanIntervalComputerMock { - pub fn time_until_next_scan_params(mut self, params: &Arc>>) -> Self { - self.time_until_next_scan_params = params.clone(); + pub fn compute_time_to_next_scan_params(mut self, params: &Arc>>) -> Self { + self.compute_time_to_next_scan_params = params.clone(); self } - pub fn time_until_next_scan_result(self, result: ScanTiming) -> Self { - self.time_until_next_scan_results.borrow_mut().push(result); + pub fn compute_time_to_next_scan_result(self, result: ScanTiming) -> Self { + self.compute_time_to_next_scan_results + .borrow_mut() + .push(result); self } @@ -470,14 +477,14 @@ pub fn assert_timestamps_from_str(examined_str: &str, expected_timestamps: Vec>>, + Arc>>, resolve_rescheduling_on_error_results: RefCell>, } impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverMock { fn resolve_rescheduling_on_error( &self, - scanner: PayableSequenceScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, @@ -500,7 +507,7 @@ impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverMock { impl RescheduleScanOnErrorResolverMock { pub fn resolve_rescheduling_on_error_params( mut self, - params: &Arc>>, + params: &Arc>>, ) -> Self { self.resolve_rescheduling_on_error_params = params.clone(); self diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index f52b1a0c8..569a5f804 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -641,8 +641,9 @@ impl PeerActorsBuilder { } // This must be called after System.new and before System.run. - // These addresses may be helpful for setting up the Counter Messages. - pub fn build_and_provide_addresses(self) -> (PeerActors, PeerActorAddrs) { + // + // The addresses may be helpful for setting up the Counter Messages. + pub fn build_with_addresses(self) -> (PeerActors, PeerActorAddrs) { let proxy_server_addr = self.proxy_server.start(); let dispatcher_addr = self.dispatcher.start(); let hopper_addr = self.hopper.start(); @@ -683,7 +684,7 @@ impl PeerActorsBuilder { // This must be called after System.new and before System.run pub fn build(self) -> PeerActors { - let (peer_actors, _) = self.build_and_provide_addresses(); + let (peer_actors, _) = self.build_with_addresses(); peer_actors } }