Skip to content

Commit

Permalink
rename time traits and methods
Browse files Browse the repository at this point in the history
  • Loading branch information
itegulov committed Nov 28, 2024
1 parent e434f90 commit 396abd4
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 55 deletions.
4 changes: 2 additions & 2 deletions src/node/config_api.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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> {
Expand Down
16 changes: 8 additions & 8 deletions src/node/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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>,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand All @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 21 additions & 21 deletions src/node/in_memory_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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!(
Expand All @@ -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!(
Expand All @@ -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!(
Expand All @@ -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"
Expand All @@ -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,
Expand All @@ -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())
Expand All @@ -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",
Expand All @@ -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",);
Expand All @@ -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",);
Expand All @@ -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!(
Expand All @@ -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"
Expand All @@ -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,
Expand Down
49 changes: 25 additions & 24 deletions src/node/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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([])
}

Expand All @@ -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,
{
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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()),
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -217,7 +218,7 @@ impl TimeExclusive for TimeLockWithOffsets<'_> {

timestamp
}
None => self.guard.next_timestamp(),
None => self.guard.advance_timestamp(),
}
}
}

0 comments on commit 396abd4

Please sign in to comment.