From 396abd43fbded414b3399eab020a286d4909864b Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov <ditegulov@gmail.com> 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 | 49 ++++++++++++++++++++------------------- 4 files changed, 56 insertions(+), 55 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<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> Configurat } fn config_get_current_timestamp(&self) -> Result<u64> { - Ok(self.time.last_timestamp()) + Ok(self.time.current_timestamp()) } fn config_set_show_calls(&self, value: String) -> Result<String> { 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<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> { /// 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<T: TimeRead, ST: ReadStorage>( + pub fn create_l1_batch_env<T: ReadTime, ST: ReadStorage>( &self, time: &T, storage: StoragePtr<ST>, @@ -347,7 +347,7 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> { 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<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> { /// # Returns /// /// A `Result` with a `Fee` representing the estimated gas related data. - pub fn estimate_gas_impl<T: TimeRead>( + pub fn estimate_gas_impl<T: ReadTime>( &self, time: &T, req: zksync_types::transaction_request::CallRequest, @@ -837,7 +837,7 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> { Ok(()) } - fn apply_block<T: TimeExclusive>( + fn apply_block<T: AdvanceTime>( &mut self, time: &mut T, block: Block<TransactionVariant>, @@ -853,7 +853,7 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> { } 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<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> { // 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<T: TimeExclusive>( + pub fn seal_block<T: AdvanceTime>( &self, time: &mut T, txs: Vec<L2Tx>, @@ -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<T: TimeRead>(&self, time: &T) -> BlockContext { + pub fn new_block<T: ReadTime>(&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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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::<HttpForkSource>::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..93410921 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. @@ -61,7 +62,7 @@ impl TimestampManager { } /// 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<()> { @@ -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<Item = u64>>( &'a self, offsets: I, - ) -> impl TimeExclusive + 'a + ) -> impl AdvanceTime + 'a where <I as IntoIterator>::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 { @@ -161,8 +162,8 @@ impl TimestampManagerInternal { } } -impl TimeRead for TimestampManagerInternal { - fn last_timestamp(&self) -> u64 { +impl ReadTime for TimestampManagerInternal { + fn current_timestamp(&self) -> u64 { self.last_timestamp } @@ -172,8 +173,8 @@ impl TimeRead for TimestampManagerInternal { } } -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()), @@ -193,9 +194,9 @@ struct TimeLockWithOffsets<'a> { offsets: VecDeque<u64>, } -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 +207,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 +218,7 @@ impl TimeExclusive for TimeLockWithOffsets<'_> { timestamp } - None => self.guard.next_timestamp(), + None => self.guard.advance_timestamp(), } } }