From e2ca8075412a876ea2f7ab8af61892b95088e4ca Mon Sep 17 00:00:00 2001 From: Bert Date: Sat, 29 Nov 2025 23:12:10 +0100 Subject: [PATCH 1/4] GH-598-receivables-scheduling-hot-fix: finished --- node/src/accountant/mod.rs | 347 +++++++++--------- .../accountant/scanners/scan_schedulers.rs | 289 +++++++++++---- node/src/accountant/scanners/test_utils.rs | 35 +- node/src/test_utils/recorder.rs | 7 +- 4 files changed, 410 insertions(+), 268 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 31f3033b5..4b3acdf9d 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, StartScanFallibleScanner, }; 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,7 +415,7 @@ impl Handler for Accountant { impl Handler for Accountant { type Result = (); - fn handle(&mut self, msg: ReceivedPayments, _ctx: &mut Self::Context) -> Self::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() @@ -409,6 +423,8 @@ impl Handler for Accountant { .try_send(node_to_ui_msg) .expect("UIGateway is dead"); } + + self.scan_schedulers.receivable.schedule(ctx, &self.logger); } } @@ -994,7 +1010,7 @@ impl Accountant { ScanReschedulingAfterEarlyStop::DoNotSchedule } Err(e) => self.handle_start_scan_error_and_prevent_scan_stall_point( - PayableSequenceScanner::NewPayables, + StartScanFallibleScanner::NewPayables, e, response_skeleton_opt, ), @@ -1025,10 +1041,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 + // Any error here panics by design, so the return value is unreachable/ignored. let _ = self.handle_start_scan_error_and_prevent_scan_stall_point( - PayableSequenceScanner::RetryPayables, + StartScanFallibleScanner::RetryPayables, e, response_skeleton_opt, ); @@ -1064,7 +1079,7 @@ 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 { + StartScanFallibleScanner::PendingPayables { initial_pending_payable_scan, }, e, @@ -1082,7 +1097,7 @@ impl Accountant { fn handle_start_scan_error_and_prevent_scan_stall_point( &self, - scanner: PayableSequenceScanner, + scanner: StartScanFallibleScanner, e: StartScanError, response_skeleton_opt: Option, ) -> ScanReschedulingAfterEarlyStop { @@ -1109,7 +1124,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 +1135,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_and_prevent_scan_stall_point( + StartScanFallibleScanner::Receivables, + e, + response_skeleton_opt, + ) } } } @@ -2289,7 +2297,7 @@ mod tests { let (peer_actors, peer_addresses) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) .ui_gateway(ui_gateway) - .build_and_provide_addresses(); + .build_providing_addresses(); let subject_addr = subject.start(); let system = System::new("test"); let response_skeleton_opt = Some(ResponseSkeleton { @@ -2723,6 +2731,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")); @@ -2846,11 +2888,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 +2907,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 +2946,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 +3010,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 +3023,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_providing_addresses(); let subject_addr: Addr = subject.start(); let subject_subs = Accountant::make_subs_from(&subject_addr); let expected_tx_receipts_msg = TxReceiptsMessage { @@ -3039,8 +3090,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 +3114,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 +3132,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 +3142,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 +3158,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 +3182,7 @@ mod tests { ReceivedPayments, Option, >, - ) -> (Accountant, Duration, Duration) { + ) -> (Accountant, Duration) { let mut subject = make_subject_and_inject_scanners( test_name, config, @@ -3152,16 +3190,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 +3213,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 +3514,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 +3543,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 +3639,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 +3681,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_providing_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 +3737,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 +4242,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 +4463,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 +5503,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 +5520,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 +5580,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 +5595,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 +5632,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 +5684,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 +5698,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 +5876,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/scan_schedulers.rs b/node/src/accountant/scanners/scan_schedulers.rs index 97a3edd62..e5ed95042 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 StartScanFallibleScanner { NewPayables, RetryPayables, PendingPayables { initial_pending_payable_scan: bool }, + Receivables, } -impl Display for PayableSequenceScanner { +impl Display for StartScanFallibleScanner { 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"), + StartScanFallibleScanner::NewPayables => write!(f, "NewPayables"), + StartScanFallibleScanner::RetryPayables => write!(f, "RetryPayables"), + StartScanFallibleScanner::PendingPayables { .. } => write!(f, "PendingPayables"), + StartScanFallibleScanner::Receivables => write!(f, "Receivables"), } } } -impl From for ScanType { - fn from(scanner: PayableSequenceScanner) -> Self { +impl From for ScanType { + fn from(scanner: StartScanFallibleScanner) -> Self { match scanner { - PayableSequenceScanner::NewPayables => ScanType::Payables, - PayableSequenceScanner::RetryPayables => ScanType::Payables, - PayableSequenceScanner::PendingPayables { .. } => ScanType::PendingPayables, + StartScanFallibleScanner::NewPayables => ScanType::Payables, + StartScanFallibleScanner::RetryPayables => ScanType::Payables, + StartScanFallibleScanner::PendingPayables { .. } => ScanType::PendingPayables, + StartScanFallibleScanner::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: StartScanFallibleScanner, 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: StartScanFallibleScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop { let reschedule_hint = match scanner { - PayableSequenceScanner::NewPayables => { + StartScanFallibleScanner::NewPayables => { Self::resolve_new_payables(error, is_externally_triggered) } - PayableSequenceScanner::RetryPayables => { + StartScanFallibleScanner::RetryPayables => { Self::resolve_retry_payables(error, is_externally_triggered) } - PayableSequenceScanner::PendingPayables { + StartScanFallibleScanner::PendingPayables { initial_pending_payable_scan, } => Self::resolve_pending_payables( error, initial_pending_payable_scan, is_externally_triggered, ), + StartScanFallibleScanner::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: StartScanFallibleScanner, 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, StartScanFallibleScanner, }; 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 { + StartScanFallibleScanner::PendingPayables { initial_pending_payable_scan: false, }, ); test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = true)", test_name), &subject, - PayableSequenceScanner::PendingPayables { + StartScanFallibleScanner::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: StartScanFallibleScanner, ) { 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 { + StartScanFallibleScanner::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 { + StartScanFallibleScanner::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 = StartScanFallibleScanner::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 = StartScanFallibleScanner::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( + StartScanFallibleScanner::PendingPayables { + initial_pending_payable_scan: true, + }, + ); + test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + StartScanFallibleScanner::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 { + StartScanFallibleScanner::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 {}, + StartScanFallibleScanner::RetryPayables {}, ); } @@ -929,7 +974,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - PayableSequenceScanner::RetryPayables, + StartScanFallibleScanner::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 {}, + StartScanFallibleScanner::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, + StartScanFallibleScanner::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( + StartScanFallibleScanner::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, + StartScanFallibleScanner::NewPayables, *error, false, &logger, @@ -1021,27 +1073,106 @@ 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, + StartScanFallibleScanner::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( + StartScanFallibleScanner::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( + StartScanFallibleScanner::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(StartScanFallibleScanner::NewPayables), ScanType::Payables ); assert_eq!( - ScanType::from(PayableSequenceScanner::RetryPayables), + ScanType::from(StartScanFallibleScanner::RetryPayables), ScanType::Payables ); assert_eq!( - ScanType::from(PayableSequenceScanner::PendingPayables { + ScanType::from(StartScanFallibleScanner::PendingPayables { initial_pending_payable_scan: false }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(PayableSequenceScanner::PendingPayables { + ScanType::from(StartScanFallibleScanner::PendingPayables { initial_pending_payable_scan: true }), ScanType::PendingPayables ); + assert_eq!( + ScanType::from(StartScanFallibleScanner::Receivables), + ScanType::Receivables + ) + } + + fn test_resolve_error_if_null_scanner_is_used_in_automatic_mode( + scanner: StartScanFallibleScanner, + ) { + 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..8a2315d56 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, StartScanFallibleScanner, }; 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: StartScanFallibleScanner, 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..0f6f15bc1 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_providing_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_providing_addresses(); peer_actors } } From 6c418e6a625133f16b1d72e94359d5db1be01c33 Mon Sep 17 00:00:00 2001 From: Bert Date: Sat, 13 Dec 2025 23:43:46 +0100 Subject: [PATCH 2/4] GH-598-receivables-scheduling-hot-fix: save point before changing the perspective of ScanAlreadyRunning error --- node/src/accountant/mod.rs | 66 +- node/src/accountant/scanners/mod.rs | 574 ++++++++++++------ .../scanners/payable_scanner/start_scan.rs | 2 +- .../scanners/pending_payable_scanner/mod.rs | 29 +- .../accountant/scanners/scan_schedulers.rs | 349 ++++------- node/src/accountant/scanners/test_utils.rs | 76 ++- node/src/test_utils/recorder.rs | 4 +- 7 files changed, 639 insertions(+), 461 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 4b3acdf9d..90c01e1c7 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::{ - ScanReschedulingAfterEarlyStop, ScanSchedulers, StartScanFallibleScanner, + ScanReschedulingAfterEarlyStop, ScanSchedulers, StartFallibleScanner, }; use crate::accountant::scanners::{Scanners, StartScanError}; use crate::blockchain::blockchain_bridge::{ @@ -995,7 +995,9 @@ impl Accountant { &self.logger, self.scan_schedulers.automatic_scans_enabled, ), - None => Err(StartScanError::NoConsumingWalletFound), + None => Err(StartScanError::no_consuming_wallet_found( + response_skeleton_opt, + )), }; self.scan_schedulers.payable.reset_scan_timer(&self.logger); @@ -1009,8 +1011,8 @@ impl Accountant { .expect("BlockchainBridge is dead"); ScanReschedulingAfterEarlyStop::DoNotSchedule } - Err(e) => self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::NewPayables, + Err(e) => self.handle_start_scan_error( + StartFallibleScanner::NewPayables, e, response_skeleton_opt, ), @@ -1029,7 +1031,9 @@ impl Accountant { response_skeleton_opt, &self.logger, ), - None => Err(StartScanError::NoConsumingWalletFound), + None => Err(StartScanError::no_consuming_wallet_found( + response_skeleton_opt, + )), }; match result { @@ -1042,8 +1046,8 @@ impl Accountant { } Err(e) => { // Any error here panics by design, so the return value is unreachable/ignored. - let _ = self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::RetryPayables, + let _ = self.handle_start_scan_error( + StartFallibleScanner::RetryPayables, e, response_skeleton_opt, ); @@ -1064,7 +1068,9 @@ impl Accountant { &self.logger, self.scan_schedulers.automatic_scans_enabled, ), - None => Err(StartScanError::NoConsumingWalletFound), + None => Err(StartScanError::no_consuming_wallet_found( + response_skeleton_opt, + )), }; let hint: ScanReschedulingAfterEarlyStop = match result { @@ -1078,8 +1084,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( - StartScanFallibleScanner::PendingPayables { + self.handle_start_scan_error( + StartFallibleScanner::PendingPayables { initial_pending_payable_scan, }, e, @@ -1095,15 +1101,13 @@ impl Accountant { hint } - fn handle_start_scan_error_and_prevent_scan_stall_point( + fn handle_start_scan_error( &self, - scanner: StartScanFallibleScanner, + scanner: StartFallibleScanner, e: StartScanError, response_skeleton_opt: Option, ) -> ScanReschedulingAfterEarlyStop { - let is_externally_triggered = response_skeleton_opt.is_some(); - - e.log_error(&self.logger, scanner.into(), is_externally_triggered); + e.log_error(&self.logger, scanner.into()); if let Some(skeleton) = response_skeleton_opt { self.ui_message_sub_opt @@ -1118,7 +1122,7 @@ impl Accountant { self.scan_schedulers .reschedule_on_error_resolver - .resolve_rescheduling_on_error(scanner, &e, is_externally_triggered, &self.logger) + .resolve_rescheduling_on_error(scanner, &e, &self.logger) } fn handle_request_of_scan_for_receivable( @@ -1146,8 +1150,8 @@ impl Accountant { Err(e) => // Any error here panics by design, so the return value is unreachable/ignored. { - self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::Receivables, + self.handle_start_scan_error( + StartFallibleScanner::Receivables, e, response_skeleton_opt, ) @@ -1332,7 +1336,7 @@ mod tests { MarkScanner, NewPayableScanIntervalComputerMock, PendingPayableCacheMock, ReplacementType, RescheduleScanOnErrorResolverMock, ScannerMock, ScannerReplacement, }; - use crate::accountant::scanners::StartScanError; + use crate::accountant::scanners::{AutomaticError, CommonError, StartScanError}; use crate::accountant::test_utils::DaoWithDestination::{ ForAccountantBody, ForPayableScanner, ForPendingPayableScanner, ForReceivableScanner, }; @@ -2237,7 +2241,7 @@ mod tests { let test_name = "externally_triggered_scan_for_pending_payables_is_prevented_if_all_payments_already_complete"; let expected_log_msg = format!( "INFO: {test_name}: User requested PendingPayables scan was denied expecting zero \ - findings. Run the Payable scanner first." + findings. Run Payable scanner first." ); test_externally_triggered_scan_is_prevented_if( @@ -2297,7 +2301,7 @@ mod tests { let (peer_actors, peer_addresses) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) .ui_gateway(ui_gateway) - .build_providing_addresses(); + .build_with_addresses(); let subject_addr = subject.start(); let system = System::new("test"); let response_skeleton_opt = Some(ResponseSkeleton { @@ -2469,7 +2473,9 @@ mod tests { let payable_scanner = ScannerMock::default() .scan_started_at_result(None) .scan_started_at_result(None) - .start_scan_result(Err(StartScanError::NothingToProcess)); + .start_scan_result(Err(StartScanError::Automatic(AutomaticError::Common( + CommonError::NothingToProcess, + )))); subject .scanners .replace_scanner(ScannerReplacement::Payable(ReplacementType::Mock( @@ -2745,7 +2751,9 @@ mod tests { )); let receivable_scanner = ScannerMock::default() .scan_started_at_result(None) - .start_scan_result(Err(StartScanError::NoConsumingWalletFound)); + .start_scan_result(Err(StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound, + )))); subject .scanners .replace_scanner(ScannerReplacement::Receivable(ReplacementType::Mock( @@ -2899,11 +2907,15 @@ mod tests { let payable_scanner = ScannerMock::new() .scan_started_at_result(None) .start_scan_params(&scan_params.payable_start_scan) - .start_scan_result(Err(StartScanError::NothingToProcess)); + .start_scan_result(Err(StartScanError::Automatic(AutomaticError::Common( + CommonError::NothingToProcess, + )))); let pending_payable_scanner = ScannerMock::new() .scan_started_at_result(None) .start_scan_params(&scan_params.pending_payable_start_scan) - .start_scan_result(Err(StartScanError::NothingToProcess)); + .start_scan_result(Err(StartScanError::Automatic(AutomaticError::Common( + CommonError::NothingToProcess, + )))); let receivable_scanner = ScannerMock::new() .scan_started_at_result(None) .start_scan_params(&scan_params.receivable_start_scan) @@ -3023,7 +3035,7 @@ mod tests { pending_payable_scanner, receivable_scanner, ); - let (peer_actors, addresses) = peer_actors_builder().build_providing_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 { @@ -3683,7 +3695,7 @@ mod tests { let subject_subs = Accountant::make_subs_from(&subject_addr); let (peer_actors, peer_actors_addrs) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) - .build_providing_addresses(); + .build_with_addresses(); send_bind_message!(subject_subs, peer_actors); let counter_msg = ReceivedPayments { timestamp: SystemTime::now(), diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index cbc337a8d..f31849b3b 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -41,7 +41,6 @@ use std::rc::Rc; use std::time::SystemTime; use time::format_description::parse; use time::OffsetDateTime; -use variant_count::VariantCount; // Leave the individual scanner objects private! pub struct Scanners { @@ -111,15 +110,13 @@ impl Scanners { ) -> Result { let triggered_manually = response_skeleton_opt.is_some(); if triggered_manually && automatic_scans_enabled { - return Err(StartScanError::ManualTriggerError( - ManulTriggerError::AutomaticScanConflict, - )); + return Err(StartScanError::Manual(ManualError::AutomaticScanConflict)); } if let Some(started_at) = self.payable.scan_started_at() { - return Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at, - }); + return Err(StartScanError::scan_already_running(response_skeleton_opt, + None, + started_at) + ); } Self::start_correct_payable_scanner::( @@ -168,7 +165,7 @@ impl Scanners { automatic_scans_enabled: bool, ) -> Result { let triggered_manually = response_skeleton_opt.is_some(); - self.check_general_conditions_for_pending_payable_scan( + self.check_initial_conditions_for_pending_payable_scan( triggered_manually, automatic_scans_enabled, )?; @@ -188,16 +185,20 @@ impl Scanners { ) } (Some(started_at), None) => { - return Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at, - }) + return Err(StartScanError::Automatic( + AutomaticError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at, + }, + )) } (None, Some(started_at)) => { - return Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: Some(ScanType::Payables), - started_at, - }) + return Err(StartScanError::Automatic( + AutomaticError::ScanAlreadyRunning { + cross_scan_cause_opt: Some(ScanType::Payables), + started_at, + }, + )) } (None, None) => (), } @@ -216,15 +217,15 @@ impl Scanners { ) -> Result { let triggered_manually = response_skeleton_opt.is_some(); if triggered_manually && automatic_scans_enabled { - return Err(StartScanError::ManualTriggerError( - ManulTriggerError::AutomaticScanConflict, - )); + return Err(StartScanError::Manual(ManualError::AutomaticScanConflict)); } if let Some(started_at) = self.receivable.scan_started_at() { - return Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at, - }); + return Err(StartScanError::Automatic( + AutomaticError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at, + }, + )); } self.receivable @@ -321,29 +322,25 @@ impl Scanners { >>::start_scan(scanner, wallet, timestamp, response_skeleton_opt, logger) } - fn check_general_conditions_for_pending_payable_scan( + fn check_initial_conditions_for_pending_payable_scan( &mut self, triggered_manually: bool, automatic_scans_enabled: bool, ) -> Result<(), StartScanError> { if triggered_manually && automatic_scans_enabled { - return Err(StartScanError::ManualTriggerError( - ManulTriggerError::AutomaticScanConflict, - )); + return Err(StartScanError::Manual(ManualError::AutomaticScanConflict)); } if self.initial_pending_payable_scan { return Ok(()); } if triggered_manually && !self.aware_of_unresolved_pending_payable { - return Err(StartScanError::ManualTriggerError( - ManulTriggerError::UnnecessaryRequest { - hint_opt: Some("Run the Payable scanner first.".to_string()), - }, - )); + return Err(StartScanError::Manual(ManualError::UnnecessaryRequest { + hint_opt: Some("Run Payable scanner first.".to_string()), + })); } if !self.aware_of_unresolved_pending_payable { unreachable!( - "Automatic pending payable scan should never start if there are no pending \ + "The automatic scan for pending payables should only run when there are pending \ payables to process." ) } @@ -448,69 +445,118 @@ macro_rules! time_marking_methods { }; } -#[derive(Debug, PartialEq, Eq, Clone, VariantCount)] +#[derive(Debug, PartialEq, Eq, Clone)] pub enum StartScanError { - NothingToProcess, - NoConsumingWalletFound, + Automatic(AutomaticError), + Manual(ManualError), + Test, +} + +impl StartScanError { + pub fn is_manual_error(&self) -> bool { + match self { + StartScanError::Automatic(_) => false, + StartScanError::Test => false, + StartScanError::Manual(_) => true, + } + } + pub fn no_consuming_wallet_found(response_skeleton_opt: Option) -> Self { + match response_skeleton_opt { + Some(_) => StartScanError::Manual(ManualError::Common(CommonError::NoConsumingWalletFound)), + None => StartScanError::Automatic(AutomaticError::Common(CommonError::NoConsumingWalletFound)) + } + } + + pub fn nothing_to_process(response_skeleton_opt: Option) -> Self { + match response_skeleton_opt { + Some(_) => StartScanError::Manual(ManualError::Common(CommonError::NothingToProcess)), + None => StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)) + } + } +} + +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum AutomaticError { + Common(CommonError), +} + +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum ManualError { + AutomaticScanConflict, + UnnecessaryRequest { hint_opt: Option }, ScanAlreadyRunning { cross_scan_cause_opt: Option, started_at: SystemTime, }, - CalledFromNullScanner, // Exclusive for tests - ManualTriggerError(ManulTriggerError), + Common(CommonError), +} + +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum CommonError { + NothingToProcess, + NoConsumingWalletFound, } impl StartScanError { - pub fn log_error(&self, logger: &Logger, scan_type: ScanType, is_externally_triggered: bool) { + pub fn log_error(&self, logger: &Logger, scan_type: ScanType) { enum ErrorType { - Temporary(String), + Temporary { + msg: String, + externally_triggered: bool, + }, Permanent(String), } - let log_message = match self { - StartScanError::NothingToProcess => ErrorType::Temporary(format!( - "There was nothing to process during {:?} scan.", - scan_type - )), - StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt, - started_at, - } => ErrorType::Temporary(Self::scan_already_running_msg( - scan_type, - *cross_scan_cause_opt, - *started_at, - )), - StartScanError::NoConsumingWalletFound => ErrorType::Permanent(format!( - "Cannot initiate {:?} scan because no consuming wallet was found.", - scan_type - )), - StartScanError::CalledFromNullScanner => match cfg!(test) { - true => ErrorType::Permanent(format!( - "Called from NullScanner, not the {:?} scanner.", + StartScanError::Automatic(AutomaticError::Common(e)) | StartScanError::Manual(ManualError::Common(e))=> match e { + CommonError::NoConsumingWalletFound => ErrorType::Permanent(format!( + "Cannot initiate {:?} scan because no consuming wallet was found.", scan_type )), - false => panic!("Null Scanner shouldn't be running inside production code."), + CommonError::NothingToProcess => ErrorType::Temporary { + msg: format!("There was nothing to process during {:?} scan.", scan_type), + externally_triggered: self.is_manual_error(), + }, }, - StartScanError::ManualTriggerError(e) => match e { - ManulTriggerError::AutomaticScanConflict => ErrorType::Permanent(format!( + StartScanError::Manual(ManualError::AutomaticScanConflict) => { + ErrorType::Permanent(format!( "User requested {:?} scan was denied. Automatic mode prevents manual triggers.", scan_type - )), - ManulTriggerError::UnnecessaryRequest { hint_opt } => { - ErrorType::Temporary(format!( + )) + } + StartScanError::Manual(ManualError::UnnecessaryRequest { hint_opt }) => { + ErrorType::Temporary { + msg: format!( "User requested {:?} scan was denied expecting zero findings.{}", scan_type, match hint_opt { Some(hint) => format!(" {}", hint), None => "".to_string(), } - )) + ), + externally_triggered: true, } + } + StartScanError::Manual(ManualError::ScanAlreadyRunning { + cross_scan_cause_opt, + started_at, + }) => ErrorType::Temporary { + msg: Self::scan_already_running_msg(scan_type, *cross_scan_cause_opt, *started_at), + externally_triggered: false, + }, + StartScanError::Test => match cfg!(test) { + true => ErrorType::Permanent(format!( + "Called from NullScanner, not the {:?} scanner.", + scan_type + )), + false => panic!("Null Scanner shouldn't be running inside production code."), }, }; match log_message { - ErrorType::Temporary(msg) => match is_externally_triggered { + ErrorType::Temporary { + msg, + externally_triggered, + } => match externally_triggered { true => info!(logger, "{}", msg), false => debug!(logger, "{}", msg), }, @@ -549,12 +595,6 @@ impl StartScanError { } } -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum ManulTriggerError { - AutomaticScanConflict, - UnnecessaryRequest { hint_opt: Option }, -} - pub trait RealScannerMarker {} macro_rules! impl_real_scanner_marker { @@ -586,13 +626,10 @@ mod tests { }; use crate::accountant::scanners::pending_payable_scanner::PendingPayableScanner; use crate::accountant::scanners::receivable_scanner::ReceivableScanner; - use crate::accountant::scanners::test_utils::{ - assert_timestamps_from_str, parse_system_time_from_str, - trim_expected_timestamp_to_three_digits_nanos, MarkScanner, NullScanner, - PendingPayableCacheMock, ReplacementType, ScannerReplacement, - }; + use crate::accountant::scanners::test_utils::{assert_timestamps_from_str, parse_system_time_from_str, trim_expected_timestamp_to_three_digits_nanos, ListOfStartScanErrors, MarkScanner, NullScanner, PendingPayableCacheMock, ReplacementType, ScannerReplacement}; use crate::accountant::scanners::{ - ManulTriggerError, Scanner, ScannerCommon, Scanners, StartScanError, StartableScanner, + AutomaticError, CommonError, ManualError, Scanner, ScannerCommon, Scanners, StartScanError, + StartableScanner, }; use crate::accountant::test_utils::{ make_custom_payment_thresholds, make_qualified_and_unqualified_payables, @@ -876,7 +913,7 @@ mod tests { } #[test] - fn new_payable_scanner_cannot_be_initiated_if_it_is_already_running() { + fn new_payable_scanner_cannot_be_initiated_if_it_is_already_running_in_manual_mode(){ let consuming_wallet = make_paying_wallet(b"consuming wallet"); let (_, _, retrieved_payables) = make_qualified_and_unqualified_payables( SystemTime::now(), @@ -894,7 +931,7 @@ mod tests { previous_scan_started_at, None, &Logger::new("test"), - true, + false, ); let result = subject.start_new_payable_scan_guarded( @@ -902,17 +939,51 @@ mod tests { SystemTime::now(), None, &Logger::new("test"), - true, + false, ); let is_scan_running = subject.payable.scan_started_at().is_some(); assert_eq!(is_scan_running, true); assert_eq!( result, - Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at: previous_scan_started_at - }) + Err(StartScanError::Manual( + ManualError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at: previous_scan_started_at + }) + ) + ); + } + + #[test] + #[should_panic(expected = "bluhhh")] + fn new_payable_scanner_panics_if_it_is_already_running_in_automatic_mode() { + let consuming_wallet = make_paying_wallet(b"consuming wallet"); + let (_, _, retrieved_payables) = make_qualified_and_unqualified_payables( + SystemTime::now(), + &PaymentThresholds::default(), + ); + let payable_dao = PayableDaoMock::new().retrieve_payables_result(retrieved_payables); + let mut subject = make_dull_subject(); + let payable_scanner = PayableScannerBuilder::new() + .payable_dao(payable_dao) + .build(); + subject.payable = Box::new(payable_scanner); + let previous_scan_started_at = SystemTime::now(); + let _ = subject.start_new_payable_scan_guarded( + &consuming_wallet, + previous_scan_started_at, + None, + &Logger::new("test"), + true, + ); + + let result = subject.start_new_payable_scan_guarded( + &consuming_wallet, + SystemTime::now(), + None, + &Logger::new("test"), + true, ); } @@ -941,7 +1012,12 @@ mod tests { let is_scan_running = subject.scan_started_at(ScanType::Payables).is_some(); assert_eq!(is_scan_running, false); - assert_eq!(result, Err(StartScanError::NothingToProcess)); + assert_eq!( + result, + Err(StartScanError::Automatic(AutomaticError::Common( + CommonError::NothingToProcess + ))) + ); } #[test] @@ -1207,7 +1283,7 @@ mod tests { } #[test] - fn pending_payable_scanner_cannot_be_initiated_if_it_itself_is_already_running() { + fn pending_payable_scanner_cannot_be_initiated_if_it_itself_is_already_running_in_manual_mode() { let now = SystemTime::now(); let consuming_wallet = make_paying_wallet(b"consuming"); let mut subject = make_dull_subject(); @@ -1242,14 +1318,51 @@ mod tests { assert_eq!(is_scan_running, true); assert_eq!( result, - Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at: now - }) + Err( StartScanError::Manual( + ManualError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at: now + } + )) + ); + } + + #[test] + #[should_panic(expected = "bluhh")] + fn pending_payable_scanner_panics_if_it_itself_is_already_running_in_automatic_mode() { + let now = SystemTime::now(); + let consuming_wallet = make_paying_wallet(b"consuming"); + let mut subject = make_dull_subject(); + let sent_payable_dao = + SentPayableDaoMock::new().retrieve_txs_result(btreeset![make_sent_tx(123)]); + let failed_payable_dao = + FailedPayableDaoMock::new().retrieve_txs_result(BTreeSet::from([make_failed_tx(456)])); + let pending_payable_scanner = PendingPayableScannerBuilder::new() + .sent_payable_dao(sent_payable_dao) + .failed_payable_dao(failed_payable_dao) + .sent_payable_cache(Box::new(CurrentPendingPayables::default())) + .failed_payable_cache(Box::new(RecheckRequiringFailures::default())) + .build(); + // Important + subject.aware_of_unresolved_pending_payable = true; + subject.pending_payable = Box::new(pending_payable_scanner); + let payable_scanner = PayableScannerBuilder::new().build(); + subject.payable = Box::new(payable_scanner); + let logger = Logger::new("test"); + let _ = + subject.start_pending_payable_scan_guarded(&consuming_wallet, now, None, &logger, true); + + let result = subject.start_pending_payable_scan_guarded( + &consuming_wallet, + SystemTime::now(), + None, + &logger, + true, ); } #[test] + #[should_panic(expected = "bluhhh")] fn pending_payable_scanner_cannot_be_initiated_if_payable_scanner_is_still_running() { let consuming_wallet = make_paying_wallet(b"consuming"); let mut subject = make_dull_subject(); @@ -1270,16 +1383,18 @@ mod tests { &logger, true, ); - - let is_scan_running = subject.pending_payable.scan_started_at().is_some(); - assert_eq!(is_scan_running, false); - assert_eq!( - result, - Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: Some(ScanType::Payables), - started_at: previous_scan_started_at - }) - ); + // + // let is_scan_running = subject.pending_payable.scan_started_at().is_some(); + // assert_eq!(is_scan_running, false); + // assert_eq!( + // result, + // Err(StartScanError::Automatic( + // AutomaticError::ScanAlreadyRunning { + // cross_scan_cause_opt: Some(ScanType::Payables), + // started_at: previous_scan_started_at + // } + // )) + // ); } #[test] @@ -1343,8 +1458,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: The automatic scan for pending \ + payables should only run when there are 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 +1478,11 @@ mod tests { } #[test] - fn check_general_conditions_for_pending_payable_scan_if_it_is_initial_pending_payable_scan() { + fn check_initial_conditions_for_pending_payable_scan_if_it_is_initial_pending_payable_scan() { let mut subject = make_dull_subject(); subject.initial_pending_payable_scan = true; - let result = subject.check_general_conditions_for_pending_payable_scan(false, true); + let result = subject.check_initial_conditions_for_pending_payable_scan(false, true); assert_eq!(result, Ok(())); assert_eq!(subject.initial_pending_payable_scan, true); @@ -1596,7 +1711,7 @@ mod tests { } #[test] - fn receivable_scanner_throws_error_in_case_scan_is_already_running() { + fn receivable_scanner_throws_error_if_it_is_already_running_in_manual_mode() { let now = SystemTime::now(); let receivable_dao = ReceivableDaoMock::new() .new_delinquencies_result(vec![]) @@ -1612,7 +1727,7 @@ mod tests { now, None, &Logger::new("test"), - true, + false, ); let result = subject.start_receivable_scan_guarded( @@ -1620,17 +1735,49 @@ mod tests { SystemTime::now(), None, &Logger::new("test"), - true, + false, ); let is_scan_running = subject.receivable.scan_started_at().is_some(); assert_eq!(is_scan_running, true); assert_eq!( result, - Err(StartScanError::ScanAlreadyRunning { - cross_scan_cause_opt: None, - started_at: now - }) + Err(StartScanError::Manual( + ManualError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at: now + }) + ) + ); + } + + #[test] + #[should_panic(expected = "bluh")] + fn receivable_scanner_panics_if_it_is_already_running_in_automatic_mode() { + let now = SystemTime::now(); + let receivable_dao = ReceivableDaoMock::new() + .new_delinquencies_result(vec![]) + .paid_delinquencies_result(vec![]); + let earning_wallet = make_wallet("earning"); + let mut subject = make_dull_subject(); + let receivable_scanner = ReceivableScannerBuilder::new() + .receivable_dao(receivable_dao) + .build(); + subject.receivable = Box::new(receivable_scanner); + let _ = subject.start_receivable_scan_guarded( + &earning_wallet, + now, + None, + &Logger::new("test"), + true, + ); + + let _ = subject.start_receivable_scan_guarded( + &earning_wallet, + SystemTime::now(), + None, + &Logger::new("test"), + true, ); } @@ -2178,103 +2325,156 @@ mod tests { } #[test] - fn log_error_works_fine() { + fn logging_scan_error_works_for_automatic_errors() { init_test_logging(); - let test_name = "log_error_works_fine"; + let test_name = "logging_scan_error_works_for_automatic_errors"; + let input: Vec<(StartScanError, String)> = vec![ + ( + StartScanError::Automatic(AutomaticError::Common(CommonError::NoConsumingWalletFound)), + format!( + "WARN: {test_name}: Cannot initiate Payables scan because no consuming wallet was found.", + ) + ), + ( + StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)), + format!( + "DEBUG: {test_name}: There was nothing to process during Payables scan.", + ) + ), + ]; + let logger = Logger::new(test_name); + let test_log_handler = TestLogHandler::new(); + + input.into_iter().for_each(|(err, expected_log_msg)| { + err.log_error(&logger, ScanType::Payables); + + test_log_handler.exists_log_containing(&expected_log_msg); + }); + } + + #[test] + fn logging_scan_errors_works_for_manual_errors() { + init_test_logging(); + let test_name = "logging_scan_errors_works_for_manual_errors"; let now = SystemTime::now(); - let input: Vec<(StartScanError, Box String>, &str, &str)> = vec![ + let input: Vec<(StartScanError, String)> = vec![ ( - StartScanError::ScanAlreadyRunning { + StartScanError::Manual(ManualError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: now, - }, - Box::new(|sev| { - format!( - "{sev}: {test_name}: Payables scan was already initiated at {}", - StartScanError::timestamp_as_string(now) - ) }), - "INFO", - "DEBUG", + format!( + "DEBUG: {test_name}: Payables scan was already initiated at {}", + StartScanError::timestamp_as_string(now) + ) ), ( - StartScanError::ManualTriggerError(ManulTriggerError::AutomaticScanConflict), - Box::new(|sev| { - format!("{sev}: {test_name}: User requested Payables scan was denied. Automatic mode prevents manual triggers.") - }), - "WARN", - "WARN", + StartScanError::Manual(ManualError::AutomaticScanConflict), + format!("WARN: {test_name}: User requested Payables scan was denied. Automatic mode prevents manual triggers.") ), ( - StartScanError::ManualTriggerError(ManulTriggerError::UnnecessaryRequest { + StartScanError::Manual(ManualError::UnnecessaryRequest { hint_opt: Some("Wise words".to_string()), }), - Box::new(|sev| { - format!("{sev}: {test_name}: User requested Payables scan was denied expecting zero findings. Wise words") - }), - "INFO", - "DEBUG", + format!("INFO: {test_name}: User requested Payables scan was denied expecting zero findings. Wise words") ), ( - StartScanError::ManualTriggerError(ManulTriggerError::UnnecessaryRequest { + StartScanError::Manual(ManualError::UnnecessaryRequest { hint_opt: None, }), - Box::new(|sev| { - format!("{sev}: {test_name}: User requested Payables scan was denied expecting zero findings.") - }), - "INFO", - "DEBUG", - ), - ( - StartScanError::CalledFromNullScanner, - Box::new(|sev| { - format!( - "{sev}: {test_name}: Called from NullScanner, not the Payables scanner." - ) - }), - "WARN", - "WARN", + format!("INFO: {test_name}: User requested Payables scan was denied expecting zero findings.") ), ( - StartScanError::NoConsumingWalletFound, - Box::new(|sev| { - format!("{sev}: {test_name}: Cannot initiate Payables scan because no consuming wallet was found.") - }), - "WARN", - "WARN", + StartScanError::Manual(ManualError::Common(CommonError::NoConsumingWalletFound)), + format!("WARN: {test_name}: Cannot initiate Payables scan because no consuming wallet was found.") ), ( - StartScanError::NothingToProcess, - Box::new(|sev| { - format!( - "{sev}: {test_name}: There was nothing to process during Payables scan." - ) - }), - "INFO", - "DEBUG", + StartScanError::Manual(ManualError::Common(CommonError::NothingToProcess)), + format!( + "INFO: {test_name}: There was nothing to process during Payables scan." + ), ), ]; let logger = Logger::new(test_name); let test_log_handler = TestLogHandler::new(); - input.into_iter().for_each( - |( - err, - form_expected_log_msg, - log_severity_for_externally_triggered_scans, - log_severity_for_automatic_scans, - )| { - let test_log_error_by_mode = - |is_externally_triggered: bool, expected_severity: &str| { - err.log_error(&logger, ScanType::Payables, is_externally_triggered); - let expected_log_msg = form_expected_log_msg(expected_severity); - test_log_handler.exists_log_containing(&expected_log_msg); - }; - - test_log_error_by_mode(true, log_severity_for_externally_triggered_scans); - - test_log_error_by_mode(false, log_severity_for_automatic_scans); - }, + input.into_iter().for_each(|(err, expected_log_msg)| { + err.log_error(&logger, ScanType::Payables); + + test_log_handler.exists_log_containing(&expected_log_msg); + }); + } + + #[test] + fn logging_scan_errors_works_for_null_scanner_error() { + init_test_logging(); + let test_name = "logging_scan_errors_works_for_manual_errors"; + let logger = Logger::new(test_name); + let error = StartScanError::Test; + + error.log_error(&logger, ScanType::Payables); + + let test_log_handler = TestLogHandler::new(); + test_log_handler.exists_log_containing(&format!( + "WARN: {test_name}: Called from NullScanner, not the Payables scanner." + )); + } + + #[test] + fn start_scan_error_is_externally_triggered_works_for_manual_errors() { + let errs = ListOfStartScanErrors::default().exclude_variants(|err|matches!(err, StartScanError::Automatic(_) | StartScanError::Test)); + + errs.errors.iter().for_each(|err| { + assert!(err.is_manual_error(), "Expected {:?} to be externally triggered", err); + }) + } + + #[test] + fn start_scan_error_is_externally_triggered_works_for_automatic_errors() { + let errs = ListOfStartScanErrors::default().exclude_variants(|err|matches!(err, StartScanError::Manual(_) | StartScanError::Test)); + + errs.errors.iter().for_each(|err| { + assert!(!err.is_manual_error(), "Expected {:?} to not be externally triggered", err); + }) + } + + #[test] + fn start_scan_error_is_externally_triggered_works_for_null_scanner_err() { + let err = StartScanError::Test; + assert!(!err.is_manual_error(), "Expected {:?} to not be externally triggered", err); + } + + #[test] + fn start_scan_error_no_consuming_wallet_found_constructor_works( + ) { + assert_eq!( + StartScanError::no_consuming_wallet_found(None), + StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound + )) + ); + assert_eq!( + StartScanError::no_consuming_wallet_found(Some(ResponseSkeleton { + client_id: 123, + context_id: 456, + })), + StartScanError::Manual(ManualError::Common(CommonError::NoConsumingWalletFound)) + ); + } + + #[test] + fn start_scan_error_nothing_to_process_constructor_works( + ) { + assert_eq!( + StartScanError::nothing_to_process(None), + StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)) + ); + assert_eq!( + StartScanError::nothing_to_process(Some(ResponseSkeleton { + client_id: 123, + context_id: 456, + })), + StartScanError::Manual(ManualError::Common(CommonError::NothingToProcess)) ); } diff --git a/node/src/accountant/scanners/payable_scanner/start_scan.rs b/node/src/accountant/scanners/payable_scanner/start_scan.rs index 457ab73ee..df2fd714f 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!( 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 e5ed95042..f2e88f319 100644 --- a/node/src/accountant/scanners/scan_schedulers.rs +++ b/node/src/accountant/scanners/scan_schedulers.rs @@ -1,6 +1,6 @@ // Copyright (c) 2025, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use crate::accountant::scanners::StartScanError; +use crate::accountant::scanners::{AutomaticError, CommonError, StartScanError}; use crate::accountant::{ Accountant, ResponseSkeleton, ScanForNewPayables, ScanForPendingPayables, ScanForReceivables, ScanForRetryPayables, @@ -51,31 +51,31 @@ pub enum ScanReschedulingAfterEarlyStop { } #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum StartScanFallibleScanner { +pub enum StartFallibleScanner { NewPayables, RetryPayables, PendingPayables { initial_pending_payable_scan: bool }, Receivables, } -impl Display for StartScanFallibleScanner { +impl Display for StartFallibleScanner { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - StartScanFallibleScanner::NewPayables => write!(f, "NewPayables"), - StartScanFallibleScanner::RetryPayables => write!(f, "RetryPayables"), - StartScanFallibleScanner::PendingPayables { .. } => write!(f, "PendingPayables"), - StartScanFallibleScanner::Receivables => write!(f, "Receivables"), + StartFallibleScanner::NewPayables => write!(f, "NewPayables"), + StartFallibleScanner::RetryPayables => write!(f, "RetryPayables"), + StartFallibleScanner::PendingPayables { .. } => write!(f, "PendingPayables"), + StartFallibleScanner::Receivables => write!(f, "Receivables"), } } } -impl From for ScanType { - fn from(scanner: StartScanFallibleScanner) -> Self { +impl From for ScanType { + fn from(scanner: StartFallibleScanner) -> Self { match scanner { - StartScanFallibleScanner::NewPayables => ScanType::Payables, - StartScanFallibleScanner::RetryPayables => ScanType::Payables, - StartScanFallibleScanner::PendingPayables { .. } => ScanType::PendingPayables, - StartScanFallibleScanner::Receivables => ScanType::Receivables, + StartFallibleScanner::NewPayables => ScanType::Payables, + StartFallibleScanner::RetryPayables => ScanType::Payables, + StartFallibleScanner::PendingPayables { .. } => ScanType::PendingPayables, + StartFallibleScanner::Receivables => ScanType::Receivables, } } } @@ -253,9 +253,8 @@ where pub trait RescheduleScanOnErrorResolver { fn resolve_rescheduling_on_error( &self, - scanner: StartScanFallibleScanner, + scanner: StartFallibleScanner, error: &StartScanError, - is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop; } @@ -266,28 +265,18 @@ pub struct RescheduleScanOnErrorResolverReal {} impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverReal { fn resolve_rescheduling_on_error( &self, - scanner: StartScanFallibleScanner, + scanner: StartFallibleScanner, error: &StartScanError, - is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop { + let is_externally_triggered = error.is_manual_error(); let reschedule_hint = match scanner { - StartScanFallibleScanner::NewPayables => { - Self::resolve_new_payables(error, is_externally_triggered) - } - StartScanFallibleScanner::RetryPayables => { - Self::resolve_retry_payables(error, is_externally_triggered) - } - StartScanFallibleScanner::PendingPayables { - initial_pending_payable_scan, - } => Self::resolve_pending_payables( - error, + StartFallibleScanner::NewPayables => Self::resolve_new_payables(error), + StartFallibleScanner::RetryPayables => Self::resolve_retry_payables(error), + StartFallibleScanner::PendingPayables { initial_pending_payable_scan, - is_externally_triggered, - ), - StartScanFallibleScanner::Receivables => { - Self::resolve_receivables(error, is_externally_triggered) - } + } => Self::resolve_pending_payables(error, initial_pending_payable_scan), + StartFallibleScanner::Receivables => Self::resolve_receivables(error), }; Self::log_rescheduling(scanner, is_externally_triggered, logger, &reschedule_hint); @@ -297,19 +286,16 @@ impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverReal { } impl RescheduleScanOnErrorResolverReal { - fn resolve_new_payables( - err: &StartScanError, - is_externally_triggered: bool, - ) -> ScanReschedulingAfterEarlyStop { - if is_externally_triggered { + fn resolve_new_payables(err: &StartScanError) -> ScanReschedulingAfterEarlyStop { + if err.is_manual_error() { return ScanReschedulingAfterEarlyStop::DoNotSchedule; } match err { - StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, - StartScanError::ScanAlreadyRunning { .. } => { + StartScanError::Test => ScanReschedulingAfterEarlyStop::DoNotSchedule, + StartScanError::Automatic(AutomaticError::ScanAlreadyRunning { .. }) => { unreachable!( - "an automatic scan of NewPayableScanner should never interfere with itself {:?}", + "an automatic scan of NewPayableScanner should never interfere with itself: {:?}", err ) } @@ -320,11 +306,8 @@ impl RescheduleScanOnErrorResolverReal { // Paradoxical at first, but this scanner is meant to be shielded by the scanner right before // it. That should ensure this scanner will not be requested if there was already something // fishy. We can impose strictness. - fn resolve_retry_payables( - err: &StartScanError, - is_externally_triggered: bool, - ) -> ScanReschedulingAfterEarlyStop { - if is_externally_triggered { + fn resolve_retry_payables(err: &StartScanError) -> ScanReschedulingAfterEarlyStop { + if err.is_manual_error() { ScanReschedulingAfterEarlyStop::DoNotSchedule } else { unreachable!( @@ -337,14 +320,13 @@ impl RescheduleScanOnErrorResolverReal { fn resolve_pending_payables( err: &StartScanError, initial_pending_payable_scan: bool, - is_externally_triggered: bool, ) -> ScanReschedulingAfterEarlyStop { - if is_externally_triggered { + if err.is_manual_error() { return ScanReschedulingAfterEarlyStop::DoNotSchedule; } match err { - StartScanError::NothingToProcess => { + StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)) => { if !initial_pending_payable_scan { unreachable!( "the automatic pending payable scan should always be requested only in need, \ @@ -353,7 +335,9 @@ impl RescheduleScanOnErrorResolverReal { } ScanReschedulingAfterEarlyStop::Schedule(ScanType::Payables) } - StartScanError::NoConsumingWalletFound => { + StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound, + )) => { if !initial_pending_payable_scan { unreachable!( "PendingPayableScanner called later than the initial attempt, but \ @@ -369,7 +353,7 @@ impl RescheduleScanOnErrorResolverReal { // this far, should be the solution. Part of the issue mentioned in GH-799 ScanReschedulingAfterEarlyStop::Schedule(ScanType::PendingPayables) } - StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, + StartScanError::Test => ScanReschedulingAfterEarlyStop::DoNotSchedule, _ => unreachable!( "{:?} should be impossible with PendingPayableScanner in automatic mode", err @@ -377,16 +361,13 @@ impl RescheduleScanOnErrorResolverReal { } } - fn resolve_receivables( - err: &StartScanError, - is_externally_triggered: bool, - ) -> ScanReschedulingAfterEarlyStop { - if is_externally_triggered { + fn resolve_receivables(err: &StartScanError) -> ScanReschedulingAfterEarlyStop { + if err.is_manual_error() { return ScanReschedulingAfterEarlyStop::DoNotSchedule; } match err { - StartScanError::CalledFromNullScanner => ScanReschedulingAfterEarlyStop::DoNotSchedule, + StartScanError::Test => ScanReschedulingAfterEarlyStop::DoNotSchedule, _ => unreachable!( "{:?} should be impossible with ReceivableScanner in automatic mode", err @@ -395,7 +376,7 @@ impl RescheduleScanOnErrorResolverReal { } fn log_rescheduling( - scanner: StartScanFallibleScanner, + scanner: StartFallibleScanner, is_externally_triggered: bool, logger: &Logger, reschedule_hint: &ScanReschedulingAfterEarlyStop, @@ -420,14 +401,12 @@ impl RescheduleScanOnErrorResolverReal { mod tests { use crate::accountant::scanners::scan_schedulers::{ NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, - ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, StartScanFallibleScanner, + ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, StartFallibleScanner, }; - use crate::accountant::scanners::test_utils::NewPayableScanIntervalComputerMock; - use crate::accountant::scanners::{ManulTriggerError, StartScanError}; + use crate::accountant::scanners::test_utils::{ListOfStartScanErrors, NewPayableScanIntervalComputerMock}; + use crate::accountant::scanners::{AutomaticError, CommonError, StartScanError}; use crate::sub_lib::accountant::ScanIntervals; use crate::test_utils::unshared_test_utils::TEST_SCAN_INTERVALS; - use itertools::Itertools; - use lazy_static::lazy_static; use masq_lib::logger::Logger; use masq_lib::messages::ScanType; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; @@ -667,68 +646,6 @@ mod tests { ) } - lazy_static! { - static ref ALL_START_SCAN_ERRORS: Vec = { - - let candidates = vec![ - StartScanError::NothingToProcess, - StartScanError::NoConsumingWalletFound, - StartScanError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: SystemTime::now()}, - StartScanError::ManualTriggerError(ManulTriggerError::AutomaticScanConflict), - StartScanError::CalledFromNullScanner - ]; - - let mut check_vec = candidates - .iter() - .fold(vec![],|mut acc, current|{ - acc.push(ListOfStartScanErrors::number_variant(current)); - acc - }); - // Making sure we didn't count in one variant multiple times - check_vec.dedup(); - assert_eq!(check_vec.len(), StartScanError::VARIANT_COUNT, "The check on variant \ - exhaustiveness failed."); - candidates - }; - } - - struct ListOfStartScanErrors<'a> { - errors: Vec<&'a StartScanError>, - } - - impl<'a> Default for ListOfStartScanErrors<'a> { - fn default() -> Self { - Self { - errors: ALL_START_SCAN_ERRORS.iter().collect_vec(), - } - } - } - - impl<'a> ListOfStartScanErrors<'a> { - fn eliminate_already_tested_variants( - mut self, - errors_to_eliminate: Vec, - ) -> Self { - let error_variants_to_remove: Vec<_> = errors_to_eliminate - .iter() - .map(Self::number_variant) - .collect(); - self.errors - .retain(|err| !error_variants_to_remove.contains(&Self::number_variant(*err))); - self - } - - fn number_variant(error: &StartScanError) -> usize { - match error { - StartScanError::NothingToProcess => 1, - StartScanError::NoConsumingWalletFound => 2, - StartScanError::ScanAlreadyRunning { .. } => 3, - StartScanError::CalledFromNullScanner => 4, - StartScanError::ManualTriggerError(..) => 5, - } - } - } - #[test] fn resolve_rescheduling_on_error_works_for_pending_payables_if_externally_triggered() { let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); @@ -738,14 +655,14 @@ mod tests { test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = false)", test_name), &subject, - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: false, }, ); test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = true)", test_name), &subject, - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: true, }, ); @@ -754,18 +671,20 @@ mod tests { fn test_what_if_externally_triggered( test_name: &str, subject: &ScanSchedulers, - scanner: StartScanFallibleScanner, + scanner: StartFallibleScanner, ) { init_test_logging(); let logger = Logger::new(test_name); let test_log_handler = TestLogHandler::new(); - ALL_START_SCAN_ERRORS + ListOfStartScanErrors::default() + .exclude_variants(|err| matches!(err, StartScanError::Automatic(_) | StartScanError::Test )) + .errors .iter() .enumerate() .for_each(|(idx, error)| { let result = subject .reschedule_on_error_resolver - .resolve_rescheduling_on_error(scanner, error, true, &logger); + .resolve_rescheduling_on_error(scanner, error, &logger); assert_eq!( result, @@ -793,11 +712,10 @@ mod tests { let result = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: true, }, - &StartScanError::NothingToProcess, - false, + &StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)), &logger, ); @@ -826,11 +744,10 @@ mod tests { let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: false, }, - &StartScanError::NothingToProcess, - false, + &StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)), &Logger::new("test"), ); } @@ -842,7 +759,7 @@ mod tests { "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 = StartScanFallibleScanner::PendingPayables { + let scanner = StartFallibleScanner::PendingPayables { initial_pending_payable_scan: true, }; @@ -850,8 +767,9 @@ mod tests { .reschedule_on_error_resolver .resolve_rescheduling_on_error( scanner, - &StartScanError::NoConsumingWalletFound, - false, + &StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound, + )), &logger, ); @@ -877,7 +795,7 @@ mod tests { 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 = StartScanFallibleScanner::PendingPayables { + let scanner = StartFallibleScanner::PendingPayables { initial_pending_payable_scan: false, }; @@ -885,8 +803,9 @@ mod tests { .reschedule_on_error_resolver .resolve_rescheduling_on_error( scanner, - &StartScanError::NoConsumingWalletFound, - false, + &StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound, + )), &Logger::new("test"), ); } @@ -894,12 +813,12 @@ 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( - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: true, }, ); test_resolve_error_if_null_scanner_is_used_in_automatic_mode( - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan: false, }, ); @@ -917,11 +836,10 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + StartFallibleScanner::PendingPayables { initial_pending_payable_scan, }, *error, - false, &Logger::new("test"), ) })) @@ -941,11 +859,19 @@ mod tests { }) } - let inputs = ListOfStartScanErrors::default().eliminate_already_tested_variants(vec![ - StartScanError::NothingToProcess, - StartScanError::NoConsumingWalletFound, - StartScanError::CalledFromNullScanner, - ]); + let inputs = ListOfStartScanErrors::default().exclude_variants(|err| { + matches!( + err, + StartScanError::Manual(_) + | StartScanError::Automatic(AutomaticError::Common( + CommonError::NothingToProcess + )) + | StartScanError::Automatic(AutomaticError::Common( + CommonError::NoConsumingWalletFound + )) + | StartScanError::Test + ) + }); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); test_forbidden_states(&subject, &inputs, false); @@ -961,7 +887,7 @@ mod tests { test_what_if_externally_triggered( test_name, &subject, - StartScanFallibleScanner::RetryPayables {}, + StartFallibleScanner::RetryPayables {}, ); } @@ -969,31 +895,34 @@ mod tests { fn any_automatic_scan_with_start_scan_error_is_fatal_for_retry_payables() { let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); - ALL_START_SCAN_ERRORS.iter().for_each(|error| { - let panic = catch_unwind(AssertUnwindSafe(|| { - subject - .reschedule_on_error_resolver - .resolve_rescheduling_on_error( - StartScanFallibleScanner::RetryPayables, - error, - false, - &Logger::new("test"), - ) - })) - .unwrap_err(); + ListOfStartScanErrors::default() + .exclude_variants(|err| matches!(err, StartScanError::Manual(_))) + .errors + .iter() + .for_each(|error| { + let panic = catch_unwind(AssertUnwindSafe(|| { + subject + .reschedule_on_error_resolver + .resolve_rescheduling_on_error( + StartFallibleScanner::RetryPayables, + error, + &Logger::new("test"), + ) + })) + .unwrap_err(); - let panic_msg = panic.downcast_ref::().unwrap(); - let expected_msg = format!( - "internal error: entered unreachable code: {:?} should be impossible \ + let panic_msg = panic.downcast_ref::().unwrap(); + let expected_msg = format!( + "internal error: entered unreachable code: {:?} should be impossible \ with RetryPayableScanner in automatic mode", - error - ); - assert_eq!( - panic_msg, &expected_msg, - "We expected '{}' but got '{}'", - expected_msg, panic_msg, - ) - }) + error + ); + assert_eq!( + panic_msg, &expected_msg, + "We expected '{}' but got '{}'", + expected_msg, panic_msg, + ) + }) } #[test] @@ -1004,14 +933,15 @@ mod tests { test_what_if_externally_triggered( test_name, &subject, - StartScanFallibleScanner::NewPayables {}, + StartFallibleScanner::NewPayables {}, ); } #[test] #[should_panic( expected = "internal error: entered unreachable code: an automatic scan of NewPayableScanner \ - should never interfere with itself ScanAlreadyRunning { cross_scan_cause_opt: None, started_at:" + should never interfere with itself: Automatic(ScanAlreadyRunning { cross_scan_cause_opt: \ + None, started_at:" )] fn resolve_hint_for_new_payables_error_if_scan_is_already_running_in_automatic_scan() { let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); @@ -1019,12 +949,11 @@ mod tests { let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::NewPayables, - &StartScanError::ScanAlreadyRunning { + StartFallibleScanner::NewPayables, + &StartScanError::Automatic(AutomaticError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: SystemTime::now(), - }, - false, + }), &Logger::new("test"), ); } @@ -1032,20 +961,20 @@ 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( - StartScanFallibleScanner::NewPayables, + StartFallibleScanner::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(), - }, - ]); + let inputs = ListOfStartScanErrors::default().exclude_variants(|err| { + matches!( + err, + StartScanError::Test + | StartScanError::Automatic(AutomaticError::ScanAlreadyRunning { .. }) | StartScanError::Manual(_) + ) + }); let logger = Logger::new(test_name); let test_log_handler = TestLogHandler::new(); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); @@ -1053,12 +982,7 @@ mod tests { inputs.errors.iter().for_each(|error| { let result = subject .reschedule_on_error_resolver - .resolve_rescheduling_on_error( - StartScanFallibleScanner::NewPayables, - *error, - false, - &logger, - ); + .resolve_rescheduling_on_error(StartFallibleScanner::NewPayables, *error, &logger); assert_eq!( result, @@ -1078,24 +1002,21 @@ mod tests { 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, - StartScanFallibleScanner::Receivables, - ); + test_what_if_externally_triggered(test_name, &subject, StartFallibleScanner::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( - StartScanFallibleScanner::Receivables, + StartFallibleScanner::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 inputs = ListOfStartScanErrors::default().exclude_variants(|err| { + matches!(err, StartScanError::Test | StartScanError::Manual(_)) + }); let subject = ScanSchedulers::new(*TEST_SCAN_INTERVALS, true); inputs.errors.iter().for_each(|error| { @@ -1103,9 +1024,8 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::Receivables, + StartFallibleScanner::Receivables, *error, - false, &Logger::new("test"), ) })) @@ -1128,44 +1048,37 @@ mod tests { #[test] fn conversion_between_hintable_scanner_and_scan_type_works() { assert_eq!( - ScanType::from(StartScanFallibleScanner::NewPayables), + ScanType::from(StartFallibleScanner::NewPayables), ScanType::Payables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::RetryPayables), + ScanType::from(StartFallibleScanner::RetryPayables), ScanType::Payables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::PendingPayables { + ScanType::from(StartFallibleScanner::PendingPayables { initial_pending_payable_scan: false }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::PendingPayables { + ScanType::from(StartFallibleScanner::PendingPayables { initial_pending_payable_scan: true }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::Receivables), + ScanType::from(StartFallibleScanner::Receivables), ScanType::Receivables ) } - fn test_resolve_error_if_null_scanner_is_used_in_automatic_mode( - scanner: StartScanFallibleScanner, - ) { + fn test_resolve_error_if_null_scanner_is_used_in_automatic_mode(scanner: StartFallibleScanner) { 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"), - ); + .resolve_rescheduling_on_error(scanner, &StartScanError::Test, &Logger::new("test")); assert_eq!( result, diff --git a/node/src/accountant/scanners/test_utils.rs b/node/src/accountant/scanners/test_utils.rs index 8a2315d56..04f71fcae 100644 --- a/node/src/accountant/scanners/test_utils.rs +++ b/node/src/accountant/scanners/test_utils.rs @@ -19,12 +19,9 @@ use crate::accountant::scanners::pending_payable_scanner::{ }; use crate::accountant::scanners::scan_schedulers::{ NewPayableScanIntervalComputer, RescheduleScanOnErrorResolver, ScanReschedulingAfterEarlyStop, - ScanTiming, StartScanFallibleScanner, -}; -use crate::accountant::scanners::{ - PendingPayableScanner, PrivateScanner, RealScannerMarker, ReceivableScanner, Scanner, - StartScanError, StartableScanner, + ScanTiming, StartFallibleScanner, }; +use crate::accountant::scanners::{AutomaticError, CommonError, ManualError, PendingPayableScanner, PrivateScanner, RealScannerMarker, ReceivableScanner, Scanner, StartScanError, StartableScanner}; use crate::accountant::{ ReceivedPayments, RequestTransactionReceipts, ResponseSkeleton, SentPayables, TxReceiptsMessage, }; @@ -32,7 +29,7 @@ use crate::blockchain::blockchain_bridge::RetrieveTransactions; use crate::sub_lib::blockchain_bridge::{ConsumingWalletBalances, OutboundPaymentsInstructions}; use crate::sub_lib::wallet::Wallet; use actix::{Message, System}; -use itertools::Either; +use itertools::{Either, Itertools}; use masq_lib::logger::{Logger, TIME_FORMATTING_STRING}; use masq_lib::ui_gateway::NodeToUiMessage; use regex::Regex; @@ -41,7 +38,9 @@ use std::cell::RefCell; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use lazy_static::lazy_static; use time::{format_description, PrimitiveDateTime}; +use masq_lib::messages::ScanType; pub struct NullScanner {} @@ -63,10 +62,12 @@ where &mut self, _wallet: &Wallet, _timestamp: SystemTime, - _response_skeleton_opt: Option, + response_skeleton_opt: Option, _logger: &Logger, ) -> Result { - Err(StartScanError::CalledFromNullScanner) + Err(StartScanError::no_consuming_wallet_found( + response_skeleton_opt, + )) } } @@ -477,27 +478,21 @@ 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: StartScanFallibleScanner, + scanner: StartFallibleScanner, error: &StartScanError, - is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop { self.resolve_rescheduling_on_error_params .lock() .unwrap() - .push(( - scanner, - error.clone(), - is_externally_triggered, - logger.clone(), - )); + .push((scanner, error.clone(), logger.clone())); self.resolve_rescheduling_on_error_results .borrow_mut() .remove(0) @@ -507,7 +502,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 @@ -593,3 +588,48 @@ impl PendingPayableCacheMock { self } } + + +lazy_static! { + static ref ALL_START_SCAN_ERRORS: Vec = vec![ + StartScanError::Automatic(AutomaticError::ScanAlreadyRunning { + cross_scan_cause_opt: None, + started_at: SystemTime::now() + }), + StartScanError::Automatic(AutomaticError::ScanAlreadyRunning { + cross_scan_cause_opt: Some(ScanType::PendingPayables), + started_at: SystemTime::now() + }), + StartScanError::Automatic(AutomaticError::Common(CommonError::NoConsumingWalletFound)), + StartScanError::Automatic(AutomaticError::Common(CommonError::NothingToProcess)), + StartScanError::Manual(ManualError::AutomaticScanConflict), + StartScanError::Manual(ManualError::UnnecessaryRequest { hint_opt: None }), + StartScanError::Manual(ManualError::UnnecessaryRequest { + hint_opt: Some("bluh".to_string()) + }), + StartScanError::Manual(ManualError::Common(CommonError::NoConsumingWalletFound)), + StartScanError::Manual(ManualError::Common(CommonError::NothingToProcess)), + StartScanError::Test, + ]; + } + +pub struct ListOfStartScanErrors<'a> { + pub errors: Vec<&'a StartScanError>, +} + +impl<'a> Default for ListOfStartScanErrors<'a> { + fn default() -> Self { + Self { + errors: ALL_START_SCAN_ERRORS.iter().collect_vec(), + } + } +} + +type MatchesErrorVersion = fn(&StartScanError) -> bool; + +impl<'a> ListOfStartScanErrors<'a> { + pub fn exclude_variants(mut self, eliminate_errors_conditions: MatchesErrorVersion) -> Self { + self.errors.retain(|err| !eliminate_errors_conditions(*err)); + self + } +} diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index 0f6f15bc1..569a5f804 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -643,7 +643,7 @@ impl PeerActorsBuilder { // This must be called after System.new and before System.run. // // The addresses may be helpful for setting up the Counter Messages. - pub fn build_providing_addresses(self) -> (PeerActors, PeerActorAddrs) { + 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(); @@ -684,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_providing_addresses(); + let (peer_actors, _) = self.build_with_addresses(); peer_actors } } From d8e533c0cbdfc4336f2d977a799e4d03ee45349a Mon Sep 17 00:00:00 2001 From: Bert Date: Sun, 14 Dec 2025 16:32:08 +0100 Subject: [PATCH 3/4] GH-598-receivables-scheduling-hot-fix-review-two: the requested part done --- node/src/accountant/mod.rs | 53 +++++----- node/src/accountant/scanners/mod.rs | 32 ++----- .../scanners/payable_scanner/start_scan.rs | 7 +- .../accountant/scanners/scan_schedulers.rs | 96 +++++++++---------- node/src/accountant/scanners/test_utils.rs | 8 +- node/src/test_utils/recorder.rs | 4 +- 6 files changed, 94 insertions(+), 106 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 4b3acdf9d..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::{ - ScanReschedulingAfterEarlyStop, ScanSchedulers, StartScanFallibleScanner, + ScanReschedulingAfterEarlyStop, ScanSchedulers, UnableToStartScanner, }; use crate::accountant::scanners::{Scanners, StartScanError}; use crate::blockchain::blockchain_bridge::{ @@ -416,15 +416,19 @@ 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"); + 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. + } } - - self.scan_schedulers.receivable.schedule(ctx, &self.logger); } } @@ -1009,8 +1013,8 @@ impl Accountant { .expect("BlockchainBridge is dead"); ScanReschedulingAfterEarlyStop::DoNotSchedule } - Err(e) => self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::NewPayables, + Err(e) => self.handle_start_scan_error( + UnableToStartScanner::NewPayables, e, response_skeleton_opt, ), @@ -1042,8 +1046,8 @@ impl Accountant { } Err(e) => { // Any error here panics by design, so the return value is unreachable/ignored. - let _ = self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::RetryPayables, + let _ = self.handle_start_scan_error( + UnableToStartScanner::RetryPayables, e, response_skeleton_opt, ); @@ -1078,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( - StartScanFallibleScanner::PendingPayables { + self.handle_start_scan_error( + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, }, e, @@ -1095,9 +1099,9 @@ impl Accountant { hint } - fn handle_start_scan_error_and_prevent_scan_stall_point( + fn handle_start_scan_error( &self, - scanner: StartScanFallibleScanner, + scanner: UnableToStartScanner, e: StartScanError, response_skeleton_opt: Option, ) -> ScanReschedulingAfterEarlyStop { @@ -1146,8 +1150,8 @@ impl Accountant { Err(e) => // Any error here panics by design, so the return value is unreachable/ignored. { - self.handle_start_scan_error_and_prevent_scan_stall_point( - StartScanFallibleScanner::Receivables, + self.handle_start_scan_error( + UnableToStartScanner::Receivables, e, response_skeleton_opt, ) @@ -2297,7 +2301,7 @@ mod tests { let (peer_actors, peer_addresses) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) .ui_gateway(ui_gateway) - .build_providing_addresses(); + .build_with_addresses(); let subject_addr = subject.start(); let system = System::new("test"); let response_skeleton_opt = Some(ResponseSkeleton { @@ -2774,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() @@ -2782,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"); @@ -3023,7 +3030,7 @@ mod tests { pending_payable_scanner, receivable_scanner, ); - let (peer_actors, addresses) = peer_actors_builder().build_providing_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 { @@ -3683,7 +3690,7 @@ mod tests { let subject_subs = Accountant::make_subs_from(&subject_addr); let (peer_actors, peer_actors_addrs) = peer_actors_builder() .blockchain_bridge(blockchain_bridge) - .build_providing_addresses(); + .build_with_addresses(); send_bind_message!(subject_subs, peer_actors); let counter_msg = ReceivedPayments { timestamp: SystemTime::now(), diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index cbc337a8d..aeeac9ecf 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, @@ -299,28 +305,6 @@ 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( &mut self, triggered_manually: bool, diff --git a/node/src/accountant/scanners/payable_scanner/start_scan.rs b/node/src/accountant/scanners/payable_scanner/start_scan.rs index 457ab73ee..2684407f7 100644 --- a/node/src/accountant/scanners/payable_scanner/start_scan.rs +++ b/node/src/accountant/scanners/payable_scanner/start_scan.rs @@ -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/scan_schedulers.rs b/node/src/accountant/scanners/scan_schedulers.rs index e5ed95042..5de151c73 100644 --- a/node/src/accountant/scanners/scan_schedulers.rs +++ b/node/src/accountant/scanners/scan_schedulers.rs @@ -51,31 +51,31 @@ pub enum ScanReschedulingAfterEarlyStop { } #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum StartScanFallibleScanner { +pub enum UnableToStartScanner { NewPayables, RetryPayables, PendingPayables { initial_pending_payable_scan: bool }, Receivables, } -impl Display for StartScanFallibleScanner { +impl Display for UnableToStartScanner { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - StartScanFallibleScanner::NewPayables => write!(f, "NewPayables"), - StartScanFallibleScanner::RetryPayables => write!(f, "RetryPayables"), - StartScanFallibleScanner::PendingPayables { .. } => write!(f, "PendingPayables"), - StartScanFallibleScanner::Receivables => write!(f, "Receivables"), + 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: StartScanFallibleScanner) -> Self { +impl From for ScanType { + fn from(scanner: UnableToStartScanner) -> Self { match scanner { - StartScanFallibleScanner::NewPayables => ScanType::Payables, - StartScanFallibleScanner::RetryPayables => ScanType::Payables, - StartScanFallibleScanner::PendingPayables { .. } => ScanType::PendingPayables, - StartScanFallibleScanner::Receivables => ScanType::Receivables, + UnableToStartScanner::NewPayables => ScanType::Payables, + UnableToStartScanner::RetryPayables => ScanType::Payables, + UnableToStartScanner::PendingPayables { .. } => ScanType::PendingPayables, + UnableToStartScanner::Receivables => ScanType::Receivables, } } } @@ -253,7 +253,7 @@ where pub trait RescheduleScanOnErrorResolver { fn resolve_rescheduling_on_error( &self, - scanner: StartScanFallibleScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, @@ -266,26 +266,26 @@ pub struct RescheduleScanOnErrorResolverReal {} impl RescheduleScanOnErrorResolver for RescheduleScanOnErrorResolverReal { fn resolve_rescheduling_on_error( &self, - scanner: StartScanFallibleScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, ) -> ScanReschedulingAfterEarlyStop { let reschedule_hint = match scanner { - StartScanFallibleScanner::NewPayables => { + UnableToStartScanner::NewPayables => { Self::resolve_new_payables(error, is_externally_triggered) } - StartScanFallibleScanner::RetryPayables => { + UnableToStartScanner::RetryPayables => { Self::resolve_retry_payables(error, is_externally_triggered) } - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, } => Self::resolve_pending_payables( error, initial_pending_payable_scan, is_externally_triggered, ), - StartScanFallibleScanner::Receivables => { + UnableToStartScanner::Receivables => { Self::resolve_receivables(error, is_externally_triggered) } }; @@ -395,7 +395,7 @@ impl RescheduleScanOnErrorResolverReal { } fn log_rescheduling( - scanner: StartScanFallibleScanner, + scanner: UnableToStartScanner, is_externally_triggered: bool, logger: &Logger, reschedule_hint: &ScanReschedulingAfterEarlyStop, @@ -420,7 +420,7 @@ impl RescheduleScanOnErrorResolverReal { mod tests { use crate::accountant::scanners::scan_schedulers::{ NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, - ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, StartScanFallibleScanner, + ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, UnableToStartScanner, }; use crate::accountant::scanners::test_utils::NewPayableScanIntervalComputerMock; use crate::accountant::scanners::{ManulTriggerError, StartScanError}; @@ -738,14 +738,14 @@ mod tests { test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = false)", test_name), &subject, - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }, ); test_what_if_externally_triggered( &format!("{}(initial_pending_payable_scan = true)", test_name), &subject, - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }, ); @@ -754,7 +754,7 @@ mod tests { fn test_what_if_externally_triggered( test_name: &str, subject: &ScanSchedulers, - scanner: StartScanFallibleScanner, + scanner: UnableToStartScanner, ) { init_test_logging(); let logger = Logger::new(test_name); @@ -793,7 +793,7 @@ mod tests { let result = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }, &StartScanError::NothingToProcess, @@ -826,7 +826,7 @@ mod tests { let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }, &StartScanError::NothingToProcess, @@ -842,7 +842,7 @@ mod tests { "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 = StartScanFallibleScanner::PendingPayables { + let scanner = UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }; @@ -877,7 +877,7 @@ mod tests { 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 = StartScanFallibleScanner::PendingPayables { + let scanner = UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }; @@ -894,12 +894,12 @@ 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( - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true, }, ); test_resolve_error_if_null_scanner_is_used_in_automatic_mode( - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false, }, ); @@ -917,7 +917,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::PendingPayables { + UnableToStartScanner::PendingPayables { initial_pending_payable_scan, }, *error, @@ -961,7 +961,7 @@ mod tests { test_what_if_externally_triggered( test_name, &subject, - StartScanFallibleScanner::RetryPayables {}, + UnableToStartScanner::RetryPayables {}, ); } @@ -974,7 +974,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::RetryPayables, + UnableToStartScanner::RetryPayables, error, false, &Logger::new("test"), @@ -1004,7 +1004,7 @@ mod tests { test_what_if_externally_triggered( test_name, &subject, - StartScanFallibleScanner::NewPayables {}, + UnableToStartScanner::NewPayables {}, ); } @@ -1019,7 +1019,7 @@ mod tests { let _ = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::NewPayables, + UnableToStartScanner::NewPayables, &StartScanError::ScanAlreadyRunning { cross_scan_cause_opt: None, started_at: SystemTime::now(), @@ -1032,7 +1032,7 @@ 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( - StartScanFallibleScanner::NewPayables, + UnableToStartScanner::NewPayables, ); } @@ -1054,7 +1054,7 @@ mod tests { let result = subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::NewPayables, + UnableToStartScanner::NewPayables, *error, false, &logger, @@ -1078,17 +1078,13 @@ mod tests { 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, - StartScanFallibleScanner::Receivables, - ); + 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( - StartScanFallibleScanner::Receivables, + UnableToStartScanner::Receivables, ); } @@ -1103,7 +1099,7 @@ mod tests { subject .reschedule_on_error_resolver .resolve_rescheduling_on_error( - StartScanFallibleScanner::Receivables, + UnableToStartScanner::Receivables, *error, false, &Logger::new("test"), @@ -1128,34 +1124,32 @@ mod tests { #[test] fn conversion_between_hintable_scanner_and_scan_type_works() { assert_eq!( - ScanType::from(StartScanFallibleScanner::NewPayables), + ScanType::from(UnableToStartScanner::NewPayables), ScanType::Payables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::RetryPayables), + ScanType::from(UnableToStartScanner::RetryPayables), ScanType::Payables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::PendingPayables { + ScanType::from(UnableToStartScanner::PendingPayables { initial_pending_payable_scan: false }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::PendingPayables { + ScanType::from(UnableToStartScanner::PendingPayables { initial_pending_payable_scan: true }), ScanType::PendingPayables ); assert_eq!( - ScanType::from(StartScanFallibleScanner::Receivables), + ScanType::from(UnableToStartScanner::Receivables), ScanType::Receivables ) } - fn test_resolve_error_if_null_scanner_is_used_in_automatic_mode( - scanner: StartScanFallibleScanner, - ) { + 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 diff --git a/node/src/accountant/scanners/test_utils.rs b/node/src/accountant/scanners/test_utils.rs index 8a2315d56..031e1bb7d 100644 --- a/node/src/accountant/scanners/test_utils.rs +++ b/node/src/accountant/scanners/test_utils.rs @@ -19,7 +19,7 @@ use crate::accountant::scanners::pending_payable_scanner::{ }; use crate::accountant::scanners::scan_schedulers::{ NewPayableScanIntervalComputer, RescheduleScanOnErrorResolver, ScanReschedulingAfterEarlyStop, - ScanTiming, StartScanFallibleScanner, + ScanTiming, UnableToStartScanner, }; use crate::accountant::scanners::{ PendingPayableScanner, PrivateScanner, RealScannerMarker, ReceivableScanner, Scanner, @@ -477,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: StartScanFallibleScanner, + scanner: UnableToStartScanner, error: &StartScanError, is_externally_triggered: bool, logger: &Logger, @@ -507,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 0f6f15bc1..569a5f804 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -643,7 +643,7 @@ impl PeerActorsBuilder { // This must be called after System.new and before System.run. // // The addresses may be helpful for setting up the Counter Messages. - pub fn build_providing_addresses(self) -> (PeerActors, PeerActorAddrs) { + 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(); @@ -684,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_providing_addresses(); + let (peer_actors, _) = self.build_with_addresses(); peer_actors } } From 9e9181faed6b003da0d6e5567a69b4897f682d8e Mon Sep 17 00:00:00 2001 From: Bert Date: Sun, 14 Dec 2025 19:03:55 +0100 Subject: [PATCH 4/4] GH-598-receivables-scheduling-hot-fix-review-two: additional minor stuff --- node/src/accountant/scanners/mod.rs | 30 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index aeeac9ecf..d14814a5b 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -174,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(), @@ -305,19 +309,14 @@ impl Scanners { self.initial_pending_payable_scan = false } - 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 { @@ -1327,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"); @@ -1347,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);