From adbdceab687034ebb09963529dd1477528ef2769 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Thu, 28 Nov 2024 16:30:49 +1100 Subject: [PATCH] rename time traits and methods --- src/node/config_api.rs | 4 +- src/node/in_memory.rs | 16 ++++---- src/node/in_memory_ext.rs | 42 ++++++++++---------- src/node/time.rs | 84 ++++++++++++++++++++------------------- 4 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/node/config_api.rs b/src/node/config_api.rs index 2181b48d..4536db88 100644 --- a/src/node/config_api.rs +++ b/src/node/config_api.rs @@ -1,6 +1,6 @@ use zksync_web3_decl::error::Web3Error; -use crate::node::time::TimeRead; +use crate::node::time::ReadTime; use crate::{ config::show_details::{ShowCalls, ShowGasDetails, ShowStorageLogs, ShowVMDetails}, fork::ForkSource, @@ -38,7 +38,7 @@ impl Configurat } fn config_get_current_timestamp(&self) -> Result { - Ok(self.time.last_timestamp()) + Ok(self.time.current_timestamp()) } fn config_set_show_calls(&self, value: String) -> Result { diff --git a/src/node/in_memory.rs b/src/node/in_memory.rs index 6cfeb2c7..05250f94 100644 --- a/src/node/in_memory.rs +++ b/src/node/in_memory.rs @@ -47,7 +47,7 @@ use zksync_utils::{bytecode::hash_bytecode, h256_to_account_address, h256_to_u25 use zksync_web3_decl::error::Web3Error; use crate::node::impersonate::{ImpersonationManager, ImpersonationState}; -use crate::node::time::{TimeExclusive, TimeRead, TimestampManager}; +use crate::node::time::{AdvanceTime, ReadTime, TimestampManager}; use crate::node::TxPool; use crate::{ bootloader_debug::{BootloaderDebug, BootloaderDebugTracer}, @@ -334,7 +334,7 @@ impl InMemoryNodeInner { /// We compute l1/l2 block details from storage to support fork testing, where the storage /// can be updated mid execution and no longer matches with the initial node's state. /// The L1 & L2 timestamps are also compared with node's timestamp to ensure it always increases monotonically. - pub fn create_l1_batch_env( + pub fn create_l1_batch_env( &self, time: &T, storage: StoragePtr, @@ -347,7 +347,7 @@ impl InMemoryNodeInner { let last_l2_block = load_last_l2_block(&storage).unwrap_or_else(|| L2Block { number: self.current_miniblock as u32, hash: L2BlockHasher::legacy_hash(L2BlockNumber(self.current_miniblock as u32)), - timestamp: time.last_timestamp(), + timestamp: time.current_timestamp(), }); let block_ctx = BlockContext { @@ -427,7 +427,7 @@ impl InMemoryNodeInner { /// # Returns /// /// A `Result` with a `Fee` representing the estimated gas related data. - pub fn estimate_gas_impl( + pub fn estimate_gas_impl( &self, time: &T, req: zksync_types::transaction_request::CallRequest, @@ -837,7 +837,7 @@ impl InMemoryNodeInner { Ok(()) } - fn apply_block( + fn apply_block( &mut self, time: &mut T, block: Block, @@ -853,7 +853,7 @@ impl InMemoryNodeInner { } self.current_miniblock = self.current_miniblock.saturating_add(1); - let expected_timestamp = time.next_timestamp(); + let expected_timestamp = time.advance_timestamp(); let actual_l1_batch_number = block .l1_batch_number @@ -1684,7 +1684,7 @@ impl InMemoryNode { // Requirement for `TimeExclusive` ensures that we have exclusive writeable access to time // manager. Meaning we can construct blocks and apply them without worrying about TOCTOU with // timestamps. - pub fn seal_block( + pub fn seal_block( &self, time: &mut T, txs: Vec, @@ -1839,7 +1839,7 @@ pub struct BlockContext { impl BlockContext { /// Create the next batch instance that uses the same batch number, and has all other parameters incremented by `1`. - pub fn new_block(&self, time: &T) -> BlockContext { + pub fn new_block(&self, time: &T) -> BlockContext { Self { hash: H256::zero(), batch: self.batch, diff --git a/src/node/in_memory_ext.rs b/src/node/in_memory_ext.rs index fc5733eb..ec5c8503 100644 --- a/src/node/in_memory_ext.rs +++ b/src/node/in_memory_ext.rs @@ -367,7 +367,7 @@ mod tests { use super::*; use crate::fork::ForkStorage; use crate::namespaces::EthNamespaceT; - use crate::node::time::{TimeRead, TimestampManager}; + use crate::node::time::{ReadTime, TimestampManager}; use crate::node::{ImpersonationManager, InMemoryNodeInner, Snapshot, TxPool}; use crate::{http_fork_source::HttpForkSource, node::InMemoryNode}; use std::str::FromStr; @@ -524,7 +524,7 @@ mod tests { assert_eq!(node.snapshots.read().unwrap().len(), 0); let inner = node.inner.read().unwrap(); - assert_eq!(node.time.last_timestamp(), 1000); + assert_eq!(node.time.current_timestamp(), 1000); assert_eq!(inner.current_batch, 0); assert_eq!(inner.current_miniblock, 0); assert_ne!(inner.current_miniblock_hash, H256::random()); @@ -658,13 +658,13 @@ mod tests { let node = InMemoryNode::::default(); let increase_value_seconds = 0u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); let expected_response = increase_value_seconds; let actual_response = node .increase_time(increase_value_seconds.into()) .expect("failed increasing timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!( @@ -679,14 +679,14 @@ mod tests { let node = InMemoryNode::::default(); let increase_value_seconds = u64::MAX; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_ne!(0, timestamp_before, "initial timestamp must be non zero",); let expected_response = increase_value_seconds; let actual_response = node .increase_time(increase_value_seconds.into()) .expect("failed increasing timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!( @@ -701,13 +701,13 @@ mod tests { let node = InMemoryNode::::default(); let increase_value_seconds = 100u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); let expected_response = increase_value_seconds; let actual_response = node .increase_time(increase_value_seconds.into()) .expect("failed increasing timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!( @@ -722,7 +722,7 @@ mod tests { let node = InMemoryNode::::default(); let new_timestamp = 10_000u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_ne!( timestamp_before, new_timestamp, "timestamps must be different" @@ -731,7 +731,7 @@ mod tests { node.set_next_block_timestamp(new_timestamp.into()) .expect("failed setting timestamp"); node.mine_block().expect("failed to mine a block"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!( new_timestamp, timestamp_after, @@ -743,7 +743,7 @@ mod tests { async fn test_set_next_block_timestamp_past_fails() { let node = InMemoryNode::::default(); - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); let new_timestamp = timestamp_before + 500; node.set_next_block_timestamp(new_timestamp.into()) @@ -761,13 +761,13 @@ mod tests { let node = InMemoryNode::::default(); let new_timestamp = 1000u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_eq!(timestamp_before, new_timestamp, "timestamps must be same"); let response = node.set_next_block_timestamp(new_timestamp.into()); assert!(response.is_err()); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!( timestamp_before, timestamp_after, "timestamp must not change", @@ -779,14 +779,14 @@ mod tests { let node = InMemoryNode::::default(); let new_time = 10_000u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_ne!(timestamp_before, new_time, "timestamps must be different"); let expected_response = 9000; let actual_response = node .set_time(new_time.into()) .expect("failed setting timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!(new_time, timestamp_after, "timestamp was not set correctly",); @@ -797,14 +797,14 @@ mod tests { let node = InMemoryNode::::default(); let new_time = 10u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_ne!(timestamp_before, new_time, "timestamps must be different"); let expected_response = -990; let actual_response = node .set_time(new_time.into()) .expect("failed setting timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!(new_time, timestamp_after, "timestamp was not set correctly",); @@ -815,14 +815,14 @@ mod tests { let node = InMemoryNode::::default(); let new_time = 1000u64; - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_eq!(timestamp_before, new_time, "timestamps must be same"); let expected_response = 0; let actual_response = node .set_time(new_time.into()) .expect("failed setting timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!(expected_response, actual_response, "erroneous response"); assert_eq!( @@ -836,7 +836,7 @@ mod tests { let node = InMemoryNode::::default(); for new_time in [0, u64::MAX] { - let timestamp_before = node.time.last_timestamp(); + let timestamp_before = node.time.current_timestamp(); assert_ne!( timestamp_before, new_time, "case {new_time}: timestamps must be different" @@ -846,7 +846,7 @@ mod tests { let actual_response = node .set_time(new_time.into()) .expect("failed setting timestamp"); - let timestamp_after = node.time.last_timestamp(); + let timestamp_after = node.time.current_timestamp(); assert_eq!( expected_response, actual_response, diff --git a/src/node/time.rs b/src/node/time.rs index 71717785..379a6e0b 100644 --- a/src/node/time.rs +++ b/src/node/time.rs @@ -3,21 +3,22 @@ use std::collections::VecDeque; use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}; /// Shared readable view on time. -pub trait TimeRead { - /// Returns the last timestamp (in seconds) that has already been used. - fn last_timestamp(&self) -> u64; +pub trait ReadTime { + /// Returns timestamp (in seconds) that the clock is currently on. + fn current_timestamp(&self) -> u64; - /// Peek at what the next call to `next_timestamp` will return. + /// Peek at what the next call to `advance_timestamp` will return. fn peek_next_timestamp(&self) -> u64; } /// Writeable view on time management. The owner of this view should be able to treat it as /// exclusive access to the underlying clock. -pub trait TimeExclusive: TimeRead { - /// Returns the next unique timestamp in seconds. +pub trait AdvanceTime: ReadTime { + /// Advances clock to the next timestamp and returns that timestamp in seconds. /// - /// Subsequent calls to this method return monotonically increasing values. - fn next_timestamp(&mut self) -> u64; + /// Subsequent calls to this method return monotonically increasing values. Time difference + /// between calls is implementation-specific. + fn advance_timestamp(&mut self) -> u64; } /// Manages timestamps (in seconds) across the system. @@ -29,10 +30,10 @@ pub struct TimestampManager { } impl TimestampManager { - pub fn new(last_timestamp: u64) -> TimestampManager { + pub fn new(current_timestamp: u64) -> TimestampManager { TimestampManager { internal: Arc::new(RwLock::new(TimestampManagerInternal { - last_timestamp, + current_timestamp, next_timestamp: None, interval: None, })), @@ -53,24 +54,24 @@ impl TimestampManager { /// Sets last used timestamp (in seconds) to the provided value and returns the difference /// between new value and old value (represented as a signed number of seconds). - pub fn set_last_timestamp_unchecked(&self, timestamp: u64) -> i128 { + pub fn set_current_timestamp_unchecked(&self, timestamp: u64) -> i128 { let mut this = self.get_mut(); - let diff = (timestamp as i128).saturating_sub(this.last_timestamp as i128); + let diff = (timestamp as i128).saturating_sub(this.current_timestamp as i128); this.reset_to(timestamp); diff } /// Forces clock to return provided value as the next timestamp. Time skip will not be performed - /// before the next invocation of `next_timestamp`. + /// before the next invocation of `advance_timestamp`. /// /// Expects provided timestamp to be in the future, returns error otherwise. pub fn enforce_next_timestamp(&self, timestamp: u64) -> anyhow::Result<()> { let mut this = self.get_mut(); - if timestamp <= this.last_timestamp { + if timestamp <= this.current_timestamp { Err(anyhow!( "timestamp ({}) must be greater than the last used timestamp ({})", timestamp, - this.last_timestamp + this.current_timestamp )) } else { this.next_timestamp.replace(timestamp); @@ -81,7 +82,7 @@ impl TimestampManager { /// Fast-forwards time by the given amount of seconds. pub fn increase_time(&self, seconds: u64) -> u64 { let mut this = self.get_mut(); - let next = this.last_timestamp.saturating_add(seconds); + let next = this.current_timestamp.saturating_add(seconds); this.reset_to(next); next } @@ -103,7 +104,7 @@ impl TimestampManager { /// /// Use this method when you need to ensure that no one else can access [`TimeManager`] during /// this view's lifetime. - pub fn lock(&self) -> impl TimeExclusive + '_ { + pub fn lock(&self) -> impl AdvanceTime + '_ { self.lock_with_offsets([]) } @@ -116,7 +117,7 @@ impl TimestampManager { pub fn lock_with_offsets<'a, I: IntoIterator>( &'a self, offsets: I, - ) -> impl TimeExclusive + 'a + ) -> impl AdvanceTime + 'a where ::IntoIter: 'a, { @@ -129,9 +130,9 @@ impl TimestampManager { } } -impl TimeRead for TimestampManager { - fn last_timestamp(&self) -> u64 { - (*self.get()).last_timestamp() +impl ReadTime for TimestampManager { + fn current_timestamp(&self) -> u64 { + (*self.get()).current_timestamp() } fn peek_next_timestamp(&self) -> u64 { @@ -141,19 +142,20 @@ impl TimeRead for TimestampManager { #[derive(Debug, Default)] struct TimestampManagerInternal { - /// The latest timestamp (in seconds) that has already been used. - last_timestamp: u64, - /// The next timestamp (in seconds) that the clock will be forced to return. + /// The current timestamp (in seconds). This timestamp is considered to be used already: there + /// might be a logical event that already happened on that timestamp (e.g. a block was sealed + /// with this timestamp). + current_timestamp: u64, + /// The next timestamp (in seconds) that the clock will be forced to advance to. next_timestamp: Option, - /// The interval to use when determining the next block's timestamp (i.e. the difference in - /// seconds between two consecutive blocks). + /// The interval to use when determining the next timestamp to advance to. interval: Option, } impl TimestampManagerInternal { fn reset_to(&mut self, timestamp: u64) { self.next_timestamp.take(); - self.last_timestamp = timestamp; + self.current_timestamp = timestamp; } fn interval(&self) -> u64 { @@ -161,25 +163,25 @@ impl TimestampManagerInternal { } } -impl TimeRead for TimestampManagerInternal { - fn last_timestamp(&self) -> u64 { - self.last_timestamp +impl ReadTime for TimestampManagerInternal { + fn current_timestamp(&self) -> u64 { + self.current_timestamp } fn peek_next_timestamp(&self) -> u64 { self.next_timestamp - .unwrap_or_else(|| self.last_timestamp.saturating_add(self.interval())) + .unwrap_or_else(|| self.current_timestamp.saturating_add(self.interval())) } } -impl TimeExclusive for TimestampManagerInternal { - fn next_timestamp(&mut self) -> u64 { +impl AdvanceTime for TimestampManagerInternal { + fn advance_timestamp(&mut self) -> u64 { let next_timestamp = match self.next_timestamp.take() { Some(next_timestamp) => next_timestamp, - None => self.last_timestamp.saturating_add(self.interval()), + None => self.current_timestamp.saturating_add(self.interval()), }; - self.last_timestamp = next_timestamp; + self.current_timestamp = next_timestamp; next_timestamp } } @@ -193,9 +195,9 @@ struct TimeLockWithOffsets<'a> { offsets: VecDeque, } -impl TimeRead for TimeLockWithOffsets<'_> { - fn last_timestamp(&self) -> u64 { - self.guard.last_timestamp() +impl ReadTime for TimeLockWithOffsets<'_> { + fn current_timestamp(&self) -> u64 { + self.guard.current_timestamp() } fn peek_next_timestamp(&self) -> u64 { @@ -206,8 +208,8 @@ impl TimeRead for TimeLockWithOffsets<'_> { } } -impl TimeExclusive for TimeLockWithOffsets<'_> { - fn next_timestamp(&mut self) -> u64 { +impl AdvanceTime for TimeLockWithOffsets<'_> { + fn advance_timestamp(&mut self) -> u64 { match self.offsets.pop_front() { Some(offset) => { let timestamp = self.start_timestamp.saturating_add(offset); @@ -217,7 +219,7 @@ impl TimeExclusive for TimeLockWithOffsets<'_> { timestamp } - None => self.guard.next_timestamp(), + None => self.guard.advance_timestamp(), } } }