Skip to content

Commit 1a48450

Browse files
mraszykaterga
mraszyk
and
aterga
authored
chore: StateMachine tests only bump time if strictly necessary (#1892)
This PR makes StateMachine tests only bump time by 1ns before executing a round if strictly necessary, i.e., if time has not increased since the last round. --------- Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org>
1 parent 77bd8fc commit 1a48450

File tree

5 files changed

+47
-30
lines changed

5 files changed

+47
-30
lines changed

‎rs/execution_environment/tests/canister_history.rs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ fn canister_history_tracks_create_install_reinstall() {
140140
};
141141
// check canister history
142142
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
143-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
143+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
144144
0,
145145
CanisterChangeOrigin::from_user(user_id1),
146146
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -170,7 +170,7 @@ fn canister_history_tracks_create_install_reinstall() {
170170
.unwrap();
171171
// check canister history
172172
reference_change_entries.push(CanisterChange::new(
173-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
173+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
174174
1,
175175
CanisterChangeOrigin::from_user(user_id2),
176176
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -208,7 +208,7 @@ fn canister_history_tracks_create_install_reinstall() {
208208
.unwrap();
209209
// check canister history
210210
reference_change_entries.push(CanisterChange::new(
211-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
211+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
212212
2,
213213
CanisterChangeOrigin::from_user(user_id1),
214214
CanisterChangeDetails::code_deployment(Reinstall, UNIVERSAL_CANISTER_WASM_SHA256),
@@ -263,7 +263,7 @@ fn canister_history_tracks_upgrade() {
263263
};
264264
// update reference canister history
265265
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
266-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
266+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
267267
0,
268268
CanisterChangeOrigin::from_user(user_id1),
269269
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -281,7 +281,7 @@ fn canister_history_tracks_upgrade() {
281281
.unwrap();
282282
// update reference canister history
283283
reference_change_entries.push(CanisterChange::new(
284-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
284+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
285285
1,
286286
CanisterChangeOrigin::from_user(user_id2),
287287
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -307,7 +307,7 @@ fn canister_history_tracks_upgrade() {
307307
.unwrap();
308308
// check canister history
309309
reference_change_entries.push(CanisterChange::new(
310-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
310+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
311311
2,
312312
CanisterChangeOrigin::from_user(user_id1),
313313
CanisterChangeDetails::code_deployment(Upgrade, UNIVERSAL_CANISTER_WASM_SHA256),
@@ -362,7 +362,7 @@ fn canister_history_tracks_uninstall() {
362362
};
363363
// update reference canister history
364364
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
365-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
365+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
366366
0,
367367
CanisterChangeOrigin::from_user(user_id1),
368368
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -380,7 +380,7 @@ fn canister_history_tracks_uninstall() {
380380
.unwrap();
381381
// update reference canister history
382382
reference_change_entries.push(CanisterChange::new(
383-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
383+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
384384
1,
385385
CanisterChangeOrigin::from_user(user_id2),
386386
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -399,7 +399,7 @@ fn canister_history_tracks_uninstall() {
399399
.unwrap();
400400
// check canister history
401401
reference_change_entries.push(CanisterChange::new(
402-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
402+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
403403
2,
404404
CanisterChangeOrigin::from_user(user_id1),
405405
CanisterChangeDetails::CanisterCodeUninstall,
@@ -454,7 +454,7 @@ fn canister_history_tracks_controllers_change() {
454454
};
455455
// update reference canister history
456456
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
457-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
457+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
458458
0,
459459
CanisterChangeOrigin::from_user(user_id1),
460460
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -481,7 +481,7 @@ fn canister_history_tracks_controllers_change() {
481481
.unwrap();
482482
// check canister history
483483
reference_change_entries.push(CanisterChange::new(
484-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
484+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
485485
i,
486486
CanisterChangeOrigin::from_user(user_id2),
487487
CanisterChangeDetails::controllers_change(new_controllers),
@@ -538,7 +538,7 @@ fn canister_history_cleared_if_canister_out_of_cycles() {
538538
};
539539
// update reference canister history
540540
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
541-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
541+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
542542
0,
543543
CanisterChangeOrigin::from_user(user_id1),
544544
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -564,7 +564,7 @@ fn canister_history_cleared_if_canister_out_of_cycles() {
564564
.unwrap();
565565
// update reference canister history
566566
reference_change_entries.push(CanisterChange::new(
567-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
567+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
568568
1,
569569
CanisterChangeOrigin::from_user(user_id2),
570570
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -647,7 +647,7 @@ fn canister_history_tracks_changes_from_canister() {
647647
};
648648
// check canister history
649649
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
650-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 2, // the canister is created in the next round after the ingress message is received
650+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1, // the canister is created in the next round after the ingress message is received
651651
0,
652652
CanisterChangeOrigin::from_canister(ucan.into(), Some(2)),
653653
CanisterChangeDetails::canister_creation(vec![ucan.into(), user_id1, user_id2]),
@@ -677,7 +677,7 @@ fn canister_history_tracks_changes_from_canister() {
677677
env.execute_ingress(ucan, "update", ucan_payload).unwrap();
678678
// check canister history
679679
reference_change_entries.push(CanisterChange::new(
680-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 2, // the canister is installed in the next round after the ingress message is received
680+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1, // the canister is installed in the next round after the ingress message is received
681681
1,
682682
CanisterChangeOrigin::from_canister(ucan.into(), None),
683683
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -747,7 +747,7 @@ fn canister_history_fails_with_incorrect_sender_version() {
747747
};
748748
// update reference canister history
749749
let reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
750-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 3, // the universal canister is created in 1st round, installed in 2nd round, this canister is created in 3rd round
750+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 2, // the universal canister is created in 1st round, installed in 2nd round, this canister is created in 3rd round
751751
0,
752752
CanisterChangeOrigin::from_user(user_id1),
753753
CanisterChangeDetails::canister_creation(vec![ucan.into(), user_id1, user_id2]),
@@ -830,7 +830,7 @@ fn canister_info_retrieval() {
830830
};
831831
// update reference canister history
832832
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
833-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
833+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
834834
0,
835835
CanisterChangeOrigin::from_user(user_id1),
836836
CanisterChangeDetails::canister_creation(vec![user_id1, user_id2]),
@@ -848,7 +848,7 @@ fn canister_info_retrieval() {
848848
.unwrap();
849849
// update reference canister history
850850
reference_change_entries.push(CanisterChange::new(
851-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
851+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
852852
1,
853853
CanisterChangeOrigin::from_user(user_id2),
854854
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),
@@ -874,7 +874,7 @@ fn canister_info_retrieval() {
874874
.unwrap();
875875
// update reference canister history
876876
reference_change_entries.push(CanisterChange::new(
877-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
877+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
878878
2,
879879
CanisterChangeOrigin::from_user(user_id1),
880880
CanisterChangeDetails::code_deployment(Upgrade, UNIVERSAL_CANISTER_WASM_SHA256),
@@ -975,7 +975,7 @@ fn canister_info_retrieval() {
975975
.unwrap();
976976
// update reference canister history
977977
reference_change_entries.push(CanisterChange::new(
978-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
978+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
979979
18,
980980
CanisterChangeOrigin::from_user(user_id2),
981981
CanisterChangeDetails::CanisterCodeUninstall,
@@ -1050,7 +1050,7 @@ fn canister_history_load_snapshot_fails_incorrect_sender_version() {
10501050
};
10511051

10521052
let mut reference_change_entries: Vec<CanisterChange> = vec![CanisterChange::new(
1053-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 2, // the canister is created in the next round after the ingress message is received
1053+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1, // the canister is created in the next round after the ingress message is received
10541054
0,
10551055
CanisterChangeOrigin::from_canister(ucan.into(), Some(2)),
10561056
CanisterChangeDetails::canister_creation(vec![ucan.into(), user_id1, user_id2]),
@@ -1072,7 +1072,7 @@ fn canister_history_load_snapshot_fails_incorrect_sender_version() {
10721072
};
10731073
// Check canister history.
10741074
reference_change_entries.push(CanisterChange::new(
1075-
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64 + 1,
1075+
now.duration_since(UNIX_EPOCH).unwrap().as_nanos() as u64,
10761076
1,
10771077
CanisterChangeOrigin::from_user(ucan.into()),
10781078
CanisterChangeDetails::code_deployment(Install, test_canister_sha256),

‎rs/ledger_suite/icp/index/tests/tests.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -975,8 +975,6 @@ fn expected_block_timestamp(rounds: u32, phase: u32, start_time: SystemTime) ->
975975
.expect("checked_add should not overflow")
976976
.checked_add(
977977
SYNC_STEP_SECONDS
978-
.checked_add(Duration::from_nanos(1)) // timestamp increases by 1ns every phase
979-
.expect("checked_mul should not overflow")
980978
.checked_mul(phase)
981979
.expect("checked_mul should not overflow"),
982980
)

‎rs/nervous_system/integration_tests/tests/advance_target_version.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async fn test_get_upgrade_journal() {
3939
assert_eq!(
4040
pocket_ic.get_time().await,
4141
now + Duration::from_secs(sleep_duration_seconds)
42-
+ Duration::from_nanos(TICKS_PER_TASK)
42+
+ Duration::from_nanos(TICKS_PER_TASK.saturating_sub(1))
4343
);
4444
}
4545
};

‎rs/pocket_ic_server/src/pocket_ic.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,9 @@ impl PocketIc {
626626
for subnet in subnets.read().unwrap().values() {
627627
max_time = max(max_time, subnet.get_state_time());
628628
}
629+
// Since calling `StateMachine::set_time` with the maximum time might make the `StateMachine` believe
630+
// that time already progressed, we add one nanosecond to make time strictly monotone on all subnets.
631+
max_time += Duration::from_nanos(1);
629632
for subnet in subnets.read().unwrap().values() {
630633
subnet.set_time(max_time.into());
631634
}

‎rs/state_machine_tests/src/lib.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,13 @@ pub struct StateMachine {
832832
// The atomicity is required for internal mutability and sending across threads.
833833
checkpoint_interval_length: AtomicU64,
834834
nonce: AtomicU64,
835+
// the time used to derive the time of the next round:
836+
// - equal to `time` + 1ns if `time` = `time_of_last_round`;
837+
// - equal to `time` otherwise.
835838
time: AtomicU64,
839+
// the time of the last round
840+
// (equal to `time` when this `StateMachine` is initialized)
841+
time_of_last_round: RwLock<SystemTime>,
836842
idkg_subnet_public_keys: BTreeMap<MasterPublicKeyId, MasterPublicKey>,
837843
idkg_subnet_secret_keys: BTreeMap<MasterPublicKeyId, SignatureSecretKey>,
838844
pub replica_logger: ReplicaLogger,
@@ -1822,6 +1828,7 @@ impl StateMachine {
18221828
checkpoint_interval_length: checkpoint_interval_length.into(),
18231829
nonce: AtomicU64::new(nonce),
18241830
time: AtomicU64::new(time.as_nanos_since_unix_epoch()),
1831+
time_of_last_round: RwLock::new(time.into()),
18251832
idkg_subnet_public_keys,
18261833
idkg_subnet_secret_keys,
18271834
replica_logger: replica_logger.clone(),
@@ -2295,8 +2302,6 @@ impl StateMachine {
22952302
/// Advances time by 1ns (to make sure time is strictly monotone)
22962303
/// and triggers a single round of execution with block payload as an input.
22972304
pub fn execute_payload(&self, payload: PayloadBuilder) -> Height {
2298-
self.advance_time(Self::EXECUTE_ROUND_TIME_INCREMENT);
2299-
23002305
let batch_number = self.message_routing.expected_batch_height();
23012306

23022307
let mut seed = [0u8; 32];
@@ -2333,7 +2338,7 @@ impl StateMachine {
23332338
idkg_subnet_public_keys: self.idkg_subnet_public_keys.clone(),
23342339
idkg_pre_signature_ids: BTreeMap::new(),
23352340
registry_version: self.registry_client.get_latest_version(),
2336-
time: Time::from_nanos_since_unix_epoch(self.time.load(Ordering::Relaxed)),
2341+
time: self.get_time_of_next_round(),
23372342
consensus_responses: payload.consensus_responses,
23382343
blockmaker_metrics: BlockmakerMetrics::new_for_test(),
23392344
};
@@ -2353,6 +2358,10 @@ impl StateMachine {
23532358

23542359
self.check_critical_errors();
23552360

2361+
let time_of_next_round = self.time_of_next_round();
2362+
self.set_time(time_of_next_round);
2363+
*self.time_of_last_round.write().unwrap() = time_of_next_round;
2364+
23562365
batch_number
23572366
}
23582367

@@ -2442,9 +2451,16 @@ impl StateMachine {
24422451
SystemTime::UNIX_EPOCH + Duration::from_nanos(self.time.load(Ordering::Relaxed))
24432452
}
24442453

2445-
/// Returns the state machine time at the beginning of next round.
2454+
/// Returns the state machine time at the beginning of next round
2455+
/// under the assumption that time won't be advanced before that
2456+
/// round is finalized.
24462457
pub fn time_of_next_round(&self) -> SystemTime {
2447-
self.time() + Self::EXECUTE_ROUND_TIME_INCREMENT
2458+
let time = self.time();
2459+
if time == *self.time_of_last_round.read().unwrap() {
2460+
time + Self::EXECUTE_ROUND_TIME_INCREMENT
2461+
} else {
2462+
time
2463+
}
24482464
}
24492465

24502466
/// Returns the current state machine time.

0 commit comments

Comments
 (0)