diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 4cb636385..153158dd4 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -732,9 +732,11 @@ impl Accountant { &mut self, msg: ReportRoutingServiceProvidedMessage, ) { - debug!( + trace!( self.logger, - "Charging routing of {} bytes to wallet {}", msg.payload_size, msg.paying_wallet + "Charging routing of {} bytes to wallet {}", + msg.payload_size, + msg.paying_wallet ); self.record_service_provided( msg.service_rate, @@ -749,7 +751,7 @@ impl Accountant { &mut self, msg: ReportExitServiceProvidedMessage, ) { - debug!( + trace!( self.logger, "Charging exit service for {} bytes to wallet {} at {} per service and {} per byte", msg.payload_size, @@ -973,9 +975,7 @@ impl Accountant { None => Err(StartScanError::NoConsumingWalletFound), }; - self.scan_schedulers - .payable - .update_last_new_payable_scan_timestamp(); + self.scan_schedulers.payable.reset_scan_timer(); match result { Ok(scan_message) => { @@ -1311,11 +1311,11 @@ mod tests { use crate::accountant::scanners::payable_scanner::utils::PayableScanResult; use crate::accountant::scanners::pending_payable_scanner::utils::TxByTable; use crate::accountant::scanners::scan_schedulers::{ - NewPayableScanDynIntervalComputer, NewPayableScanDynIntervalComputerReal, + NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, ScanTiming, }; use crate::accountant::scanners::test_utils::{ - MarkScanner, NewPayableScanDynIntervalComputerMock, PendingPayableCacheMock, - ReplacementType, RescheduleScanOnErrorResolverMock, ScannerMock, ScannerReplacement, + MarkScanner, NewPayableScanIntervalComputerMock, PendingPayableCacheMock, ReplacementType, + RescheduleScanOnErrorResolverMock, ScannerMock, ScannerReplacement, }; use crate::accountant::scanners::StartScanError; use crate::accountant::test_utils::DaoWithDestination::{ @@ -1524,9 +1524,9 @@ mod tests { result .scan_schedulers .payable - .dyn_interval_computer + .interval_computer .as_any() - .downcast_ref::() + .downcast_ref::() .unwrap(); assert_eq!( result.scan_schedulers.pending_payable.interval, @@ -2374,13 +2374,13 @@ mod tests { fn accountant_sends_qualified_payable_msg_when_qualified_payable_found( act_msg: ActorMessage, initial_templates: Either, - zero_out_params_expected: Vec<()>, + reset_last_scan_timestamp_params_expected: Vec<()>, ) where ActorMessage: Message + Send + 'static, ActorMessage::Result: Send, Accountant: Handler, { - let zero_out_params_arc = Arc::new(Mutex::new(vec![])); + let reset_last_scan_timestamp_params_arc = Arc::new(Mutex::new(vec![])); let (blockchain_bridge, _, blockchain_bridge_recording_arc) = make_recorder(); let system = System::new("accountant_sends_qualified_payable_msg_when_qualified_payable_found"); @@ -2408,8 +2408,9 @@ mod tests { subject .scanners .replace_scanner(ScannerReplacement::Receivable(ReplacementType::Null)); - subject.scan_schedulers.payable.dyn_interval_computer = Box::new( - NewPayableScanDynIntervalComputerMock::default().zero_out_params(&zero_out_params_arc), + subject.scan_schedulers.payable.interval_computer = Box::new( + NewPayableScanIntervalComputerMock::default() + .reset_last_scan_timestamp_params(&reset_last_scan_timestamp_params_arc), ); let accountant_addr = subject.start(); let accountant_subs = Accountant::make_subs_from(&accountant_addr); @@ -2426,8 +2427,11 @@ mod tests { assert_eq!(blockchain_bridge_recorder.len(), 1); let message = blockchain_bridge_recorder.get_record::(0); assert_eq!(message, &initial_template_msg); - let zero_out_params = zero_out_params_arc.lock().unwrap(); - assert_eq!(*zero_out_params, zero_out_params_expected) + let reset_last_scan_timestamp_params = reset_last_scan_timestamp_params_arc.lock().unwrap(); + assert_eq!( + *reset_last_scan_timestamp_params, + reset_last_scan_timestamp_params_expected + ) } #[test] @@ -2480,21 +2484,24 @@ mod tests { response_skeleton_opt: None } ); - let default_scan_intervals = ScanIntervals::compute_default(TEST_DEFAULT_CHAIN); - // The previous last_new_payable_scan_timestamp is UNIX_EPOCH, if the interval was derived - // from that timestamp, it would result in an immediate-scan command. This implies that - // the last_new_payable_scan_timestamp was reset to zero, which is how it is meant to be. - let left_bound = default_scan_intervals - .payable_scan_interval - .checked_sub(Duration::from_secs(5)) - .unwrap(); - let right_bound = default_scan_intervals - .payable_scan_interval - .checked_add(Duration::from_secs(5)) - .unwrap(); + // The initial last_new_payable_scan_timestamp is UNIX_EPOCH by this design. Such a value + // would've driven an immediate scan without an interval. Therefore, the performed interval + // implies that the last_new_payable_scan_timestamp must have been updated to the current + // time. (As the result of running into StartScanError::NothingToProcess) + let default_interval = + ScanIntervals::compute_default(TEST_DEFAULT_CHAIN).payable_scan_interval; + let tolerance = Duration::from_secs(5); + let min_interval = default_interval.checked_sub(tolerance).unwrap(); + let max_interval = default_interval.checked_add(tolerance).unwrap(); // The divergence should be only a few milliseconds, definitely not seconds; the tested // interval should be safe for slower machines too. - assert!(left_bound <= actual_interval && actual_interval <= right_bound); + assert!( + min_interval <= actual_interval && actual_interval <= max_interval, + "Expected interval between {:?} and {:?}, got {:?}", + min_interval, + max_interval, + actual_interval + ); assert_eq!(notify_later_params.len(), 0); // Accountant is unbound; therefore, it is guaranteed that sending a message to // the BlockchainBridge wasn't attempted. It would've panicked otherwise. @@ -2831,7 +2838,7 @@ 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 compute_interval_params_arc = Arc::new(Mutex::new(vec![])); + let time_until_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); @@ -2850,10 +2857,10 @@ mod tests { .start_scan_params(&scan_params.receivable_start_scan) .start_scan_result(Err(StartScanError::NothingToProcess)); let (subject, new_payable_expected_computed_interval, receivable_scan_interval) = - set_up_subject_for_no_p_p_found_startup_test( + configure_accountant_for_startup_with_preexisting_pending_payables( test_name, ¬ify_and_notify_later_params, - &compute_interval_params_arc, + &time_until_next_scan_params_arc, config, pending_payable_scanner, receivable_scanner, @@ -2870,7 +2877,7 @@ mod tests { let before = SystemTime::now(); system.run(); let after = SystemTime::now(); - assert_pending_payable_scanner_for_no_p_p_found( + assert_pending_payable_scanner_for_no_pending_payable_found( test_name, consuming_wallet, &scan_params.pending_payable_start_scan, @@ -2878,10 +2885,10 @@ mod tests { before, after, ); - assert_payable_scanner_for_no_p_p_found( + assert_payable_scanner_for_no_pending_payable_found( &scan_params.payable_start_scan, ¬ify_and_notify_later_params, - compute_interval_params_arc, + time_until_next_scan_params_arc, new_payable_expected_computed_interval, ); assert_receivable_scanner( @@ -2949,7 +2956,7 @@ mod tests { .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) = - set_up_subject_for_some_p_p_found_startup_test( + configure_accountant_for_startup_with_no_preexisting_pending_payables( test_name, ¬ify_and_notify_later_params, config, @@ -3003,7 +3010,7 @@ mod tests { let before = SystemTime::now(); system.run(); let after = SystemTime::now(); - assert_pending_payable_scanner_for_some_p_p_found( + assert_pending_payable_scanner_for_some_pending_payable_found( test_name, consuming_wallet.clone(), &scan_params, @@ -3013,7 +3020,7 @@ mod tests { before, after, ); - assert_payable_scanner_for_some_p_p_found( + assert_payable_scanner_for_some_pending_payable_found( test_name, consuming_wallet, &scan_params, @@ -3053,10 +3060,10 @@ mod tests { receivables_notify_later: Arc>>, } - fn set_up_subject_for_no_p_p_found_startup_test( + fn configure_accountant_for_startup_with_preexisting_pending_payables( test_name: &str, notify_and_notify_later_params: &NotifyAndNotifyLaterParams, - compute_interval_params_arc: &Arc>>, + time_until_next_scan_params_arc: &Arc>>, config: BootstrapperConfig, pending_payable_scanner: ScannerMock< RequestTransactionReceipts, @@ -3101,10 +3108,12 @@ mod tests { .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 dyn_interval_computer = NewPayableScanDynIntervalComputerMock::default() - .compute_interval_params(&compute_interval_params_arc) - .compute_interval_result(Some(new_payable_expected_computed_interval)); - subject.scan_schedulers.payable.dyn_interval_computer = Box::new(dyn_interval_computer); + let interval_computer = NewPayableScanIntervalComputerMock::default() + .time_until_next_scan_params(&time_until_next_scan_params_arc) + .time_until_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, @@ -3112,7 +3121,7 @@ mod tests { ) } - fn set_up_subject_for_some_p_p_found_startup_test( + fn configure_accountant_for_startup_with_no_preexisting_pending_payables( test_name: &str, notify_and_notify_later_params: &NotifyAndNotifyLaterParams, config: BootstrapperConfig, @@ -3205,7 +3214,7 @@ mod tests { subject } - fn assert_pending_payable_scanner_for_no_p_p_found( + fn assert_pending_payable_scanner_for_no_pending_payable_found( test_name: &str, consuming_wallet: Wallet, pending_payable_start_scan_params_arc: &Arc< @@ -3238,7 +3247,7 @@ mod tests { assert_using_the_same_logger(&pp_logger, test_name, Some("pp")); } - fn assert_pending_payable_scanner_for_some_p_p_found( + fn assert_pending_payable_scanner_for_some_pending_payable_found( test_name: &str, consuming_wallet: Wallet, scan_params: &ScanParams, @@ -3319,12 +3328,12 @@ mod tests { pp_logger } - fn assert_payable_scanner_for_no_p_p_found( + fn assert_payable_scanner_for_no_pending_payable_found( payable_scanner_start_scan_arc: &Arc< Mutex, Logger, String)>>, >, notify_and_notify_later_params: &NotifyAndNotifyLaterParams, - compute_interval_until_next_new_payable_scan_params_arc: Arc>>, + time_until_next_scan_until_next_new_payable_scan_params_arc: Arc>>, new_payable_expected_computed_interval: Duration, ) { // Note that there is no functionality from the payable scanner actually running. @@ -3342,12 +3351,12 @@ mod tests { new_payable_expected_computed_interval )] ); - let compute_interval_until_next_new_payable_scan_params = - compute_interval_until_next_new_payable_scan_params_arc + let time_until_next_scan_until_next_new_payable_scan_params = + time_until_next_scan_until_next_new_payable_scan_params_arc .lock() .unwrap(); assert_eq!( - *compute_interval_until_next_new_payable_scan_params, + *time_until_next_scan_until_next_new_payable_scan_params, vec![()] ); let payable_scanner_start_scan = payable_scanner_start_scan_arc.lock().unwrap(); @@ -3375,23 +3384,23 @@ mod tests { ); } - fn assert_payable_scanner_for_some_p_p_found( + fn assert_payable_scanner_for_some_pending_payable_found( test_name: &str, consuming_wallet: Wallet, scan_params: &ScanParams, notify_and_notify_later_params: &NotifyAndNotifyLaterParams, expected_sent_payables: SentPayables, ) { - assert_payable_scanner_ran_for_some_p_p_found( + assert_payable_scanner_ran_for_some_pending_payable_found( test_name, consuming_wallet, scan_params, expected_sent_payables, ); - assert_scan_scheduling_for_some_p_p_found(notify_and_notify_later_params); + assert_scan_scheduling_for_some_pending_payable_found(notify_and_notify_later_params); } - fn assert_payable_scanner_ran_for_some_p_p_found( + fn assert_payable_scanner_ran_for_some_pending_payable_found( test_name: &str, consuming_wallet: Wallet, scan_params: &ScanParams, @@ -3429,7 +3438,7 @@ mod tests { ); } - fn assert_scan_scheduling_for_some_p_p_found( + fn assert_scan_scheduling_for_some_pending_payable_found( notify_and_notify_later_params: &NotifyAndNotifyLaterParams, ) { let scan_for_new_payables_notify_later_params = notify_and_notify_later_params @@ -4485,10 +4494,6 @@ mod tests { more_money_receivable_parameters[0], (now, make_wallet("booga"), (1 * 42) + (1234 * 24)) ); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: Accountant: Charging routing of 1234 bytes to wallet {}", - paying_wallet - )); } #[test] @@ -4622,10 +4627,6 @@ mod tests { more_money_receivable_parameters[0], (now, make_wallet("booga"), (1 * 42) + (1234 * 24)) ); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: Accountant: Charging exit service for 1234 bytes to wallet {}", - paying_wallet - )); } #[test] @@ -5480,7 +5481,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 compute_interval_params_arc = Arc::new(Mutex::new(vec![])); + let time_until_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"); @@ -5496,11 +5497,11 @@ mod tests { pending_payable_scanner, ))); let expected_computed_interval = Duration::from_secs(3); - let dyn_interval_computer = NewPayableScanDynIntervalComputerMock::default() - .compute_interval_params(&compute_interval_params_arc) + let interval_computer = NewPayableScanIntervalComputerMock::default() + .time_until_next_scan_params(&time_until_next_scan_params_arc) // This determines the test - .compute_interval_result(Some(expected_computed_interval)); - subject.scan_schedulers.payable.dyn_interval_computer = Box::new(dyn_interval_computer); + .time_until_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), ); @@ -5557,7 +5558,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 compute_interval_params_arc = Arc::new(Mutex::new(vec![])); + let time_until_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() @@ -5571,11 +5572,11 @@ mod tests { .replace_scanner(ScannerReplacement::PendingPayable(ReplacementType::Mock( pending_payable_scanner, ))); - let dyn_interval_computer = NewPayableScanDynIntervalComputerMock::default() - .compute_interval_params(&compute_interval_params_arc) + let interval_computer = NewPayableScanIntervalComputerMock::default() + .time_until_next_scan_params(&time_until_next_scan_params_arc) // This determines the test - .compute_interval_result(None); - subject.scan_schedulers.payable.dyn_interval_computer = Box::new(dyn_interval_computer); + .time_until_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), ); @@ -5609,8 +5610,8 @@ mod tests { "Should be empty but {:?}", finish_scan_params ); - let compute_interval_params = compute_interval_params_arc.lock().unwrap(); - assert_eq!(*compute_interval_params, vec![()]); + let time_until_next_scan_params = time_until_next_scan_params_arc.lock().unwrap(); + assert_eq!(*time_until_next_scan_params, vec![()]); let new_payable_notify_later = new_payable_notify_later_arc.lock().unwrap(); assert!( new_payable_notify_later.is_empty(), @@ -5641,16 +5642,15 @@ mod tests { NotifyLaterHandleMock::default().notify_later_params(&new_payable_notify_later_arc), ); let default_scan_intervals = ScanIntervals::compute_default(TEST_DEFAULT_CHAIN); - let mut assertion_interval_computer = NewPayableScanDynIntervalComputerReal::new( - default_scan_intervals.payable_scan_interval, - ); + let mut assertion_interval_computer = + NewPayableScanIntervalComputerReal::new(default_scan_intervals.payable_scan_interval); { subject .scan_schedulers .payable - .dyn_interval_computer - .zero_out(); - assertion_interval_computer.zero_out(); + .interval_computer + .reset_last_scan_timestamp(); + assertion_interval_computer.reset_last_scan_timestamp(); } let system = System::new(test_name); let subject_addr = subject.start(); @@ -5661,7 +5661,13 @@ mod tests { block_number: U64::from(100), }), }]); - let left_side_bound = assertion_interval_computer.compute_interval().unwrap(); + let left_side_bound = if let ScanTiming::WaitFor(interval) = + assertion_interval_computer.time_until_next_scan() + { + interval + } else { + panic!("expected an interval") + }; subject_addr.try_send(msg).unwrap(); @@ -5669,7 +5675,13 @@ mod tests { system.run(); let new_payable_notify_later = new_payable_notify_later_arc.lock().unwrap(); let (_, actual_interval) = new_payable_notify_later[0]; - let right_side_bound = assertion_interval_computer.compute_interval().unwrap(); + let right_side_bound = if let ScanTiming::WaitFor(interval) = + assertion_interval_computer.time_until_next_scan() + { + interval + } else { + panic!("expected an interval") + }; assert!( left_side_bound >= actual_interval && actual_interval >= right_side_bound, "expected actual {:?} to be between {:?} and {:?}", @@ -5841,9 +5853,9 @@ mod tests { |_scanners: &mut Scanners, scan_schedulers: &mut ScanSchedulers| { // Setup let notify_later_params_arc = Arc::new(Mutex::new(vec![])); - scan_schedulers.payable.dyn_interval_computer = Box::new( - NewPayableScanDynIntervalComputerMock::default() - .compute_interval_result(Some(Duration::from_secs(152))), + scan_schedulers.payable.interval_computer = Box::new( + NewPayableScanIntervalComputerMock::default() + .time_until_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 6c9b27a2c..dedfee1e5 100644 --- a/node/src/accountant/scanners/scan_schedulers.rs +++ b/node/src/accountant/scanners/scan_schedulers.rs @@ -78,9 +78,7 @@ impl From for ScanType { pub struct PayableScanScheduler { pub new_payable_notify_later: Box>, - pub dyn_interval_computer: Box, - // pub inner: Arc>, - // pub new_payable_interval: Duration, + pub interval_computer: Box, pub new_payable_notify: Box>, pub retry_payable_notify: Box>, } @@ -89,18 +87,16 @@ impl PayableScanScheduler { fn new(new_payable_interval: Duration) -> Self { Self { new_payable_notify_later: Box::new(NotifyLaterHandleReal::default()), - dyn_interval_computer: Box::new(NewPayableScanDynIntervalComputerReal::new( + interval_computer: Box::new(NewPayableScanIntervalComputerReal::new( new_payable_interval, )), - // inner: Arc::new(Mutex::new(PayableScanSchedulerInner::default())), - // new_payable_interval, new_payable_notify: Box::new(NotifyHandleReal::default()), retry_payable_notify: Box::new(NotifyHandleReal::default()), } } pub fn schedule_new_payable_scan(&self, ctx: &mut Context, logger: &Logger) { - if let Some(interval) = self.dyn_interval_computer.compute_interval() { + if let ScanTiming::WaitFor(interval) = self.interval_computer.time_until_next_scan() { debug!( logger, "Scheduling a new-payable scan in {}ms", @@ -126,8 +122,8 @@ impl PayableScanScheduler { } } - pub fn update_last_new_payable_scan_timestamp(&mut self) { - self.dyn_interval_computer.zero_out(); + pub fn reset_scan_timer(&mut self) { + self.interval_computer.reset_last_scan_timestamp(); } // This message ships into the Accountant's mailbox with no delay. @@ -150,46 +146,47 @@ impl PayableScanScheduler { } } -pub trait NewPayableScanDynIntervalComputer { - fn compute_interval(&self) -> Option; +pub trait NewPayableScanIntervalComputer { + fn time_until_next_scan(&self) -> ScanTiming; - fn zero_out(&mut self); + fn reset_last_scan_timestamp(&mut self); as_any_ref_in_trait!(); } -pub struct NewPayableScanDynIntervalComputerReal { +pub struct NewPayableScanIntervalComputerReal { scan_interval: Duration, last_scan_timestamp: SystemTime, clock: Box, } -impl NewPayableScanDynIntervalComputer for NewPayableScanDynIntervalComputerReal { - fn compute_interval(&self) -> Option { - let now = self.clock.now(); - let elapsed = now +impl NewPayableScanIntervalComputer for NewPayableScanIntervalComputerReal { + fn time_until_next_scan(&self) -> ScanTiming { + let current_time = self.clock.now(); + let time_since_last_scan = current_time .duration_since(self.last_scan_timestamp) .unwrap_or_else(|_| { panic!( - "Now ({:?}) earlier than past timestamp ({:?})", - now, self.last_scan_timestamp + "Current time ({:?}) is earlier than last scan timestamp ({:?})", + current_time, self.last_scan_timestamp ) }); - if elapsed >= self.scan_interval { - None + + if time_since_last_scan >= self.scan_interval { + ScanTiming::ReadyNow } else { - Some(self.scan_interval - elapsed) + ScanTiming::WaitFor(self.scan_interval - time_since_last_scan) } } - fn zero_out(&mut self) { + fn reset_last_scan_timestamp(&mut self) { self.last_scan_timestamp = SystemTime::now(); } as_any_ref_in_trait_impl!(); } -impl NewPayableScanDynIntervalComputerReal { +impl NewPayableScanIntervalComputerReal { pub fn new(scan_interval: Duration) -> Self { Self { scan_interval, @@ -199,6 +196,12 @@ impl NewPayableScanDynIntervalComputerReal { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum ScanTiming { + ReadyNow, + WaitFor(Duration), +} + pub struct SimplePeriodicalScanScheduler { pub handle: Box>, pub interval: Duration, @@ -231,13 +234,13 @@ where } } -// Scanners that take part in a scan sequence composed of different scanners must handle -// StartScanErrors delicately to maintain the continuity and periodicity of this process. Where -// possible, either the same, some other, but traditional, or even a totally unrelated scan chosen -// just in the event of emergency, may be scheduled. The intention is to prevent a full panic while -// ensuring no harmful, toxic issues are left behind for the future scans. Following that philosophy, -// panic is justified only if the error was thought to be impossible by design and contextual -// things but still happened. +// In a scan sequence incorporating different scanners, one makes another dependent on the previous +// one. Such scanners must be handling StartScanErrors delicately with the regard to ensuring +// further continuity and periodicity of this process. Where possible, either the same one, some +// tightly related, or even a totally unrelated scan, just for the event of emergency, should be +// scheduled. The intention is to prevent panics while not creating any harmful conditions for +// the scans running in the future. Following this philosophy, panics should be restricted just +// to so-believed unreachable conditions (by the intended design). pub trait RescheduleScanOnErrorResolver { fn resolve_rescheduling_on_error( &self, @@ -380,10 +383,10 @@ impl RescheduleScanOnErrorResolverReal { #[cfg(test)] mod tests { use crate::accountant::scanners::scan_schedulers::{ - NewPayableScanDynIntervalComputer, NewPayableScanDynIntervalComputerReal, - PayableSequenceScanner, ScanReschedulingAfterEarlyStop, ScanSchedulers, + NewPayableScanIntervalComputer, NewPayableScanIntervalComputerReal, PayableSequenceScanner, + ScanReschedulingAfterEarlyStop, ScanSchedulers, ScanTiming, }; - use crate::accountant::scanners::test_utils::NewPayableScanDynIntervalComputerMock; + use crate::accountant::scanners::test_utils::NewPayableScanIntervalComputerMock; use crate::accountant::scanners::{ManulTriggerError, StartScanError}; use crate::sub_lib::accountant::ScanIntervals; use crate::test_utils::unshared_test_utils::TEST_SCAN_INTERVALS; @@ -411,9 +414,9 @@ mod tests { let payable_interval_computer = schedulers .payable - .dyn_interval_computer + .interval_computer .as_any() - .downcast_ref::() + .downcast_ref::() .unwrap(); assert_eq!( payable_interval_computer.scan_interval, @@ -432,8 +435,8 @@ mod tests { } #[test] - fn scan_dyn_interval_computer_computes_remaining_time_to_standard_interval_correctly() { - let (clock, now) = fill_simple_clock_mock_and_return_now(); + fn scan_interval_computer_computes_remaining_time_to_standard_interval_correctly() { + let (clock, now) = set_up_mocked_clock(); let inputs = vec![ ( now.checked_sub(Duration::from_secs(32)).unwrap(), @@ -451,7 +454,7 @@ mod tests { Duration::from_secs(4), ), ]; - let mut subject = make_subject(); + let mut subject = initialize_scan_interval_computer(); subject.clock = Box::new(clock); inputs @@ -460,21 +463,21 @@ mod tests { subject.scan_interval = standard_interval; subject.last_scan_timestamp = past_instant; - let result = subject.compute_interval(); + let result = subject.time_until_next_scan(); assert_eq!( result, - Some(expected_result), + ScanTiming::WaitFor(expected_result), "We expected Some({}) ms, but got {:?} ms", expected_result.as_millis(), - result.map(|duration| duration.as_millis()) + from_scan_timing_to_millis(result) ) }) } #[test] - fn scan_dyn_interval_computer_realizes_the_standard_interval_has_been_exceeded() { - let (clock, now) = fill_simple_clock_mock_and_return_now(); + fn scan_interval_computer_realizes_the_standard_interval_has_been_exceeded() { + let (clock, now) = set_up_mocked_clock(); let inputs = vec![ ( now.checked_sub(Duration::from_millis(32001)).unwrap(), @@ -485,7 +488,7 @@ mod tests { Duration::from_secs(123), ), ]; - let mut subject = make_subject(); + let mut subject = initialize_scan_interval_computer(); subject.clock = Box::new(clock); inputs @@ -495,74 +498,82 @@ mod tests { subject.scan_interval = standard_interval; subject.last_scan_timestamp = past_instant; - let result = subject.compute_interval(); + let result = subject.time_until_next_scan(); assert_eq!( result, - None, + ScanTiming::ReadyNow, "We expected None ms, but got {:?} ms at idx {}", - result.map(|duration| duration.as_millis()), + from_scan_timing_to_millis(result), idx ) }) } #[test] - fn scan_dyn_interval_computer_realizes_standard_interval_just_met() { + fn scan_interval_computer_realizes_standard_interval_just_met() { let now = SystemTime::now(); - let mut subject = make_subject(); + let mut subject = initialize_scan_interval_computer(); subject.last_scan_timestamp = now.checked_sub(Duration::from_secs(180)).unwrap(); subject.scan_interval = Duration::from_secs(180); subject.clock = Box::new(SimpleClockMock::default().now_result(now)); - let result = subject.compute_interval(); + let result = subject.time_until_next_scan(); assert_eq!( result, - None, + ScanTiming::ReadyNow, "We expected None ms, but got {:?} ms", - result.map(|duration| duration.as_millis()) + from_scan_timing_to_millis(result) ) } + fn from_scan_timing_to_millis(scan_timing: ScanTiming) -> u128 { + if let ScanTiming::WaitFor(interval) = scan_timing { + interval.as_millis() + } else { + panic!("expected an interval") + } + } + #[test] #[cfg(windows)] #[should_panic( - expected = "Now (SystemTime { intervals: 116454736000000000 }) earlier than past timestamp \ - (SystemTime { intervals: 116454736010000000 })" + expected = "Current time (SystemTime { intervals: 116454736000000000 }) is earlier than last \ + scan timestamp (SystemTime { intervals: 116454736010000000 })" )] - fn scan_dyn_interval_computer_panics() { - test_scan_dyn_interval_computer_panics() + fn scan_interval_computer_panics() { + test_scan_interval_computer_panics() } #[test] #[cfg(not(windows))] #[should_panic( - expected = "Now (SystemTime { tv_sec: 1000000, tv_nsec: 0 }) earlier than past timestamp \ - (SystemTime { tv_sec: 1000001, tv_nsec: 0 })" + expected = "Current time (SystemTime { tv_sec: 1000000, tv_nsec: 0 }) is earlier than last \ + scan timestamp (SystemTime { tv_sec: 1000001, tv_nsec: 0 })" )] - fn scan_dyn_interval_computer_panics() { - test_scan_dyn_interval_computer_panics() + fn scan_interval_computer_panics() { + test_scan_interval_computer_panics() } - fn test_scan_dyn_interval_computer_panics() { + fn test_scan_interval_computer_panics() { let now = UNIX_EPOCH .checked_add(Duration::from_secs(1_000_000)) .unwrap(); - let mut subject = make_subject(); + let mut subject = initialize_scan_interval_computer(); subject.clock = Box::new(SimpleClockMock::default().now_result(now)); subject.last_scan_timestamp = now.checked_add(Duration::from_secs(1)).unwrap(); - let _ = subject.compute_interval(); + let _ = subject.time_until_next_scan(); } #[test] - fn zero_out_works_for_default_subject() { - let mut subject = make_subject(); + fn reset_last_scan_timestamp_works_for_default_subject() { + let mut subject = initialize_scan_interval_computer(); let last_scan_timestamp_before = subject.last_scan_timestamp; let before_act = SystemTime::now(); - subject.zero_out(); + subject.reset_last_scan_timestamp(); let after_act = SystemTime::now(); let last_scan_timestamp_after = subject.last_scan_timestamp; @@ -574,14 +585,14 @@ mod tests { } #[test] - fn zero_out_works_for_general_subject() { - let mut subject = make_subject(); + fn reset_last_scan_timestamp_works_for_general_subject() { + let mut subject = initialize_scan_interval_computer(); subject.last_scan_timestamp = SystemTime::now() .checked_sub(Duration::from_secs(100)) .unwrap(); let before_act = SystemTime::now(); - subject.zero_out(); + subject.reset_last_scan_timestamp(); let after_act = SystemTime::now(); let last_scan_timestamp_after = subject.last_scan_timestamp; @@ -592,26 +603,27 @@ mod tests { } #[test] - fn update_last_new_payable_scan_timestamp_works() { - let zero_out_params_arc = Arc::new(Mutex::new(vec![])); + fn reset_scan_timer_works() { + let reset_last_scan_timestamp_params_arc = Arc::new(Mutex::new(vec![])); let scan_intervals = ScanIntervals::compute_default(TEST_DEFAULT_CHAIN); let mut subject = ScanSchedulers::new(scan_intervals, true); - subject.payable.dyn_interval_computer = Box::new( - NewPayableScanDynIntervalComputerMock::default().zero_out_params(&zero_out_params_arc), + subject.payable.interval_computer = Box::new( + NewPayableScanIntervalComputerMock::default() + .reset_last_scan_timestamp_params(&reset_last_scan_timestamp_params_arc), ); - subject.payable.update_last_new_payable_scan_timestamp(); + subject.payable.reset_scan_timer(); - let zero_out_params = zero_out_params_arc.lock().unwrap(); - assert_eq!(*zero_out_params, vec![()]) + let reset_last_scan_timestamp_params = reset_last_scan_timestamp_params_arc.lock().unwrap(); + assert_eq!(*reset_last_scan_timestamp_params, vec![()]) } - fn make_subject() -> NewPayableScanDynIntervalComputerReal { + fn initialize_scan_interval_computer() -> NewPayableScanIntervalComputerReal { // The interval is just a garbage value, we reset it in the tests by injection if needed - NewPayableScanDynIntervalComputerReal::new(Duration::from_secs(100)) + NewPayableScanIntervalComputerReal::new(Duration::from_secs(100)) } - fn fill_simple_clock_mock_and_return_now() -> (SimpleClockMock, SystemTime) { + fn set_up_mocked_clock() -> (SimpleClockMock, SystemTime) { let now = SystemTime::now(); ( (0..3).fold(SimpleClockMock::default(), |clock, _| clock.now_result(now)), diff --git a/node/src/accountant/scanners/test_utils.rs b/node/src/accountant/scanners/test_utils.rs index 4b11abee2..08325dedc 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::{ - NewPayableScanDynIntervalComputer, PayableSequenceScanner, RescheduleScanOnErrorResolver, - ScanReschedulingAfterEarlyStop, + NewPayableScanIntervalComputer, PayableSequenceScanner, RescheduleScanOnErrorResolver, + ScanReschedulingAfterEarlyStop, ScanTiming, }; use crate::accountant::scanners::{ PendingPayableScanner, PrivateScanner, RealScannerMarker, ReceivableScanner, Scanner, @@ -335,38 +335,41 @@ pub trait ScannerMockMarker {} impl ScannerMockMarker for ScannerMock {} #[derive(Default)] -pub struct NewPayableScanDynIntervalComputerMock { - compute_interval_params: Arc>>, - compute_interval_results: RefCell>>, - zero_out_params: Arc>>, +pub struct NewPayableScanIntervalComputerMock { + time_until_next_scan_params: Arc>>, + time_until_next_scan_results: RefCell>, + reset_last_scan_timestamp_params: Arc>>, } -impl NewPayableScanDynIntervalComputer for NewPayableScanDynIntervalComputerMock { - fn compute_interval(&self) -> Option { - self.compute_interval_params.lock().unwrap().push(()); - self.compute_interval_results.borrow_mut().remove(0) +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 zero_out(&mut self) { - self.zero_out_params.lock().unwrap().push(()); + fn reset_last_scan_timestamp(&mut self) { + self.reset_last_scan_timestamp_params + .lock() + .unwrap() + .push(()); } as_any_ref_in_trait_impl!(); } -impl NewPayableScanDynIntervalComputerMock { - pub fn compute_interval_params(mut self, params: &Arc>>) -> Self { - self.compute_interval_params = params.clone(); +impl NewPayableScanIntervalComputerMock { + pub fn time_until_next_scan_params(mut self, params: &Arc>>) -> Self { + self.time_until_next_scan_params = params.clone(); self } - pub fn compute_interval_result(self, result: Option) -> Self { - self.compute_interval_results.borrow_mut().push(result); + pub fn time_until_next_scan_result(self, result: ScanTiming) -> Self { + self.time_until_next_scan_results.borrow_mut().push(result); self } - pub fn zero_out_params(mut self, params: &Arc>>) -> Self { - self.zero_out_params = params.clone(); + pub fn reset_last_scan_timestamp_params(mut self, params: &Arc>>) -> Self { + self.reset_last_scan_timestamp_params = params.clone(); self } } diff --git a/node/src/blockchain/errors/validation_status.rs b/node/src/blockchain/errors/validation_status.rs index 6346548a6..5fe46dc9a 100644 --- a/node/src/blockchain/errors/validation_status.rs +++ b/node/src/blockchain/errors/validation_status.rs @@ -243,45 +243,6 @@ mod tests { assert_eq!(other_error_stats, None); } - // #[test] - // fn previous_attempts_hash_works_correctly() { - // let now = SystemTime::now(); - // let clock = SimpleClockMock::default() - // .now_result(now) - // .now_result(now) - // .now_result(now + Duration::from_secs(2)); - // let attempts1 = PreviousAttempts::new( - // BlockchainErrorKind::AppRpc(AppRpcErrorKind::Local(LocalErrorKind::Decoder)), - // &clock, - // ); - // let attempts2 = PreviousAttempts::new( - // BlockchainErrorKind::AppRpc(AppRpcErrorKind::Local(LocalErrorKind::Decoder)), - // &clock, - // ); - // let attempts3 = PreviousAttempts::new( - // BlockchainErrorKind::AppRpc(AppRpcErrorKind::Local(LocalErrorKind::Io)), - // &clock, - // ); - // let hash1 = { - // let mut hasher = DefaultHasher::new(); - // attempts1.hash(&mut hasher); - // hasher.finish() - // }; - // let hash2 = { - // let mut hasher = DefaultHasher::new(); - // attempts2.hash(&mut hasher); - // hasher.finish() - // }; - // let hash3 = { - // let mut hasher = DefaultHasher::new(); - // attempts3.hash(&mut hasher); - // hasher.finish() - // }; - // - // assert_eq!(hash1, hash2); - // assert_ne!(hash1, hash3); - // } - #[test] fn previous_attempts_ordering_works_correctly_with_mock() { let now = SystemTime::now();