-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(anvil): support millisecond mining with correct block.timestamp #8010
Changes from all commits
0fe3836
16ab682
9ca08e5
6bc5d6a
b7c8eb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1845,12 +1845,17 @@ impl EthApi { | |
Ok(true) | ||
} | ||
|
||
/// Sets an interval for the block timestamp | ||
/// Sets an interval for the block timestamp in milliseconds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is incorrect and should be seconds |
||
/// | ||
/// Handler for RPC call: `anvil_setBlockTimestampInterval` | ||
pub fn evm_set_block_timestamp_interval(&self, seconds: u64) -> Result<()> { | ||
pub fn evm_set_block_timestamp_interval(&self, seconds: f64) -> Result<()> { | ||
node_info!("anvil_setBlockTimestampInterval"); | ||
self.backend.time().set_block_timestamp_interval(seconds); | ||
if seconds == 0.0 { | ||
return Err(BlockchainError::Message("interval cannot be zero".to_string())); | ||
} | ||
let dur = Duration::try_from_secs_f64(seconds) | ||
.map_err(|_| BlockchainError::Message("Cannot convert f64 to Duration".to_string()))?; | ||
self.backend.time().set_block_timestamp_interval(dur.as_millis() as u64); | ||
Ok(()) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,8 +244,10 @@ impl Backend { | |
precompile_factory, | ||
}; | ||
|
||
if let Some(interval_block_time) = automine_block_time { | ||
backend.update_interval_mine_block_time(interval_block_time); | ||
if let Some(interval) = automine_block_time { | ||
// Set interval in TimeManager as milliseconds. | ||
backend.time.set_block_timestamp_interval((interval).as_millis().try_into().unwrap()); | ||
Comment on lines
+248
to
+249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this additional call now required? |
||
backend.update_interval_mine_block_time(interval); | ||
} | ||
|
||
// Note: this can only fail in forking mode, in which case we can't recover | ||
|
@@ -944,7 +946,7 @@ impl Backend { | |
env.block.number = env.block.number.saturating_add(U256::from(1)); | ||
env.block.basefee = U256::from(current_base_fee); | ||
env.block.blob_excess_gas_and_price = current_excess_blob_gas_and_price; | ||
env.block.timestamp = U256::from(self.time.next_timestamp()); | ||
env.block.timestamp = U256::from(self.time.next_timestamp_as_secs()); | ||
|
||
let best_hash = self.blockchain.storage.read().best_hash; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,14 +15,25 @@ pub fn utc_from_secs(secs: u64) -> DateTime<Utc> { | |
pub struct TimeManager { | ||
/// tracks the overall applied timestamp offset | ||
offset: Arc<RwLock<i128>>, | ||
/// The timestamp of the last block header | ||
/// The timestamp of the last block header. | ||
/// | ||
/// Interpreted as seconds | ||
last_timestamp: Arc<RwLock<u64>>, | ||
/// Contains the next timestamp to use | ||
/// if this is set then the next time `[TimeManager::current_timestamp()]` is called this value | ||
/// will be taken and returned. After which the `offset` will be updated accordingly | ||
/// | ||
/// Interpreted as seconds | ||
next_exact_timestamp: Arc<RwLock<Option<u64>>>, | ||
/// The interval to use when determining the next block's timestamp | ||
/// | ||
/// Interpreted as milliseconds | ||
interval: Arc<RwLock<Option<u64>>>, | ||
/// The wall clock timestamp with precision upto milliseconds | ||
/// | ||
/// This keeps track of the current timestamp in milliseconds, which is helpful for interval | ||
/// mining with < 1000ms block times. | ||
wall_clock_timestamp: Arc<RwLock<Option<u64>>>, | ||
} | ||
|
||
// === impl TimeManager === | ||
|
@@ -34,6 +45,7 @@ impl TimeManager { | |
offset: Default::default(), | ||
next_exact_timestamp: Default::default(), | ||
interval: Default::default(), | ||
wall_clock_timestamp: Default::default(), | ||
}; | ||
time_manager.reset(start_timestamp); | ||
time_manager | ||
|
@@ -46,6 +58,7 @@ impl TimeManager { | |
*self.last_timestamp.write() = start_timestamp; | ||
*self.offset.write() = (start_timestamp as i128) - current; | ||
self.next_exact_timestamp.write().take(); | ||
*self.wall_clock_timestamp.write() = Some(start_timestamp.saturating_mul(1000)); // Since, wall_clock_timestamp is in milliseconds | ||
} | ||
|
||
pub fn offset(&self) -> i128 { | ||
|
@@ -68,11 +81,11 @@ impl TimeManager { | |
self.add_offset(seconds as i128) | ||
} | ||
|
||
/// Sets the exact timestamp to use in the next block | ||
/// Sets the exact timestamp (`next_exact_timestamp`) to use in the next block | ||
/// Fails if it's before (or at the same time) the last timestamp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is no longer accurate |
||
pub fn set_next_block_timestamp(&self, timestamp: u64) -> Result<(), BlockchainError> { | ||
trace!(target: "time", "override next timestamp {}", timestamp); | ||
if timestamp <= *self.last_timestamp.read() { | ||
if timestamp < *self.last_timestamp.read() { | ||
return Err(BlockchainError::TimestampError(format!( | ||
"{timestamp} is lower than or equal to previous block's timestamp" | ||
))) | ||
|
@@ -81,7 +94,7 @@ impl TimeManager { | |
Ok(()) | ||
} | ||
|
||
/// Sets an interval to use when computing the next timestamp | ||
/// Sets an interval (in milliseconds) to use when computing the next timestamp | ||
/// | ||
/// If an interval already exists, this will update the interval, otherwise a new interval will | ||
/// be set starting with the current timestamp. | ||
|
@@ -100,30 +113,58 @@ impl TimeManager { | |
} | ||
} | ||
|
||
/// Updates `wall_clock_timestamp` by `interval_ms` | ||
pub fn update_wall_clock_timestamp_by_interval(&self, interval_ms: u64) { | ||
let current_wall_timestamp = self.wall_clock_timestamp.read().unwrap(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no unwrap here please |
||
self.update_wall_clock_timestamp(current_wall_timestamp.saturating_add(interval_ms)); | ||
} | ||
|
||
/// Updates `wall_clock_timestamp` to the given timestamp (milliseconds precision) | ||
pub fn update_wall_clock_timestamp(&self, timestamp: u64) { | ||
*self.wall_clock_timestamp.write() = Some(timestamp); | ||
} | ||
|
||
/// Computes the next timestamp without updating internals | ||
fn compute_next_timestamp(&self) -> (u64, Option<i128>) { | ||
let current = duration_since_unix_epoch().as_secs() as i128; | ||
fn compute_next_timestamp(&self, update_wall: bool) -> (u64, Option<i128>) { | ||
let current = duration_since_unix_epoch().as_secs() as i128; // TODO(yash): Getting current time here as seconds. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's left todo here? |
||
let last_timestamp = *self.last_timestamp.read(); | ||
|
||
let interval = *self.interval.read(); | ||
let (mut next_timestamp, update_offset) = | ||
if let Some(next) = *self.next_exact_timestamp.read() { | ||
if update_wall { | ||
self.update_wall_clock_timestamp(next * 1000); // Needs to be in ms | ||
} | ||
(next, true) | ||
} else if let Some(interval) = *self.interval.read() { | ||
(last_timestamp.saturating_add(interval), false) | ||
} else if let Some(interval) = interval { | ||
let wall_clock_timestamp = if update_wall { | ||
self.update_wall_clock_timestamp_by_interval(interval); | ||
self.wall_clock_timestamp.read().unwrap() | ||
} else { | ||
let current_wall = self.wall_clock_timestamp.read().unwrap(); | ||
current_wall.saturating_add(interval) | ||
}; | ||
let next_timestamp = (wall_clock_timestamp as f64 / 1000.0).floor() as u64; | ||
|
||
(next_timestamp, false) | ||
} else { | ||
(current.saturating_add(self.offset()) as u64, false) | ||
let next = current.saturating_add(self.offset()) as u64; | ||
if update_wall { | ||
self.update_wall_clock_timestamp(next * 1000); // Needs to be in ms | ||
} | ||
(next, false) | ||
}; | ||
// Ensures that the timestamp is always increasing | ||
if next_timestamp <= last_timestamp { | ||
if next_timestamp < last_timestamp { | ||
next_timestamp = last_timestamp + 1; | ||
} | ||
let next_offset = update_offset.then_some((next_timestamp as i128) - current); | ||
(next_timestamp, next_offset) | ||
} | ||
|
||
/// Returns the current timestamp and updates the underlying offset and interval accordingly | ||
pub fn next_timestamp(&self) -> u64 { | ||
let (next_timestamp, next_offset) = self.compute_next_timestamp(); | ||
pub fn next_timestamp_as_secs(&self) -> u64 { | ||
let (next_timestamp, next_offset) = self.compute_next_timestamp(true); | ||
// Make sure we reset the `next_exact_timestamp` | ||
self.next_exact_timestamp.write().take(); | ||
if let Some(next_offset) = next_offset { | ||
|
@@ -135,7 +176,7 @@ impl TimeManager { | |
|
||
/// Returns the current timestamp for a call that does _not_ update the value | ||
pub fn current_call_timestamp(&self) -> u64 { | ||
let (next_timestamp, _) = self.compute_next_timestamp(); | ||
let (next_timestamp, _) = self.compute_next_timestamp(false); | ||
next_timestamp | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -292,6 +292,9 @@ async fn test_set_next_timestamp() { | |
assert_eq!(block.header.number.unwrap(), 1); | ||
assert_eq!(block.header.timestamp, next_timestamp.as_secs()); | ||
|
||
// Sleep for 1s | ||
tokio::time::sleep(Duration::from_secs(1)).await; | ||
|
||
api.evm_mine(None).await.unwrap(); | ||
|
||
let next = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
|
@@ -318,6 +321,11 @@ async fn test_evm_set_time() { | |
|
||
assert!(block.header.timestamp >= timestamp.as_secs()); | ||
|
||
// Sleep for 1s | ||
// This needs to be added as we removed the if condition in TimeManager wherein | ||
// next_timestamp==last_timesatmp => next_timestamp + 1; | ||
tokio::time::sleep(Duration::from_secs(1)).await; | ||
|
||
api.evm_mine(None).await.unwrap(); | ||
let next = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
|
||
|
@@ -350,7 +358,7 @@ async fn test_timestamp_interval() { | |
let provider = handle.http_provider(); | ||
|
||
api.evm_mine(None).await.unwrap(); | ||
let interval = 10; | ||
let interval: f64 = 1.0; // 1s | ||
|
||
for _ in 0..5 { | ||
let block = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
|
@@ -361,7 +369,7 @@ async fn test_timestamp_interval() { | |
|
||
let new_block = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
|
||
assert_eq!(new_block.header.timestamp, block.header.timestamp + interval); | ||
assert_eq!(new_block.header.timestamp, block.header.timestamp + interval as u64); | ||
} | ||
|
||
let block = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
|
@@ -377,7 +385,7 @@ async fn test_timestamp_interval() { | |
|
||
let block = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
// interval also works after setting the next timestamp manually | ||
assert_eq!(block.header.timestamp, next_timestamp + interval); | ||
assert_eq!(block.header.timestamp, next_timestamp + interval as u64); | ||
|
||
assert!(api.evm_remove_block_timestamp_interval().unwrap()); | ||
|
||
|
@@ -390,7 +398,7 @@ async fn test_timestamp_interval() { | |
api.evm_mine(None).await.unwrap(); | ||
let another_block = provider.get_block(BlockId::default(), false).await.unwrap().unwrap(); | ||
// check interval is disabled | ||
assert!(another_block.header.timestamp - new_block.header.timestamp < interval); | ||
assert!(another_block.header.timestamp - new_block.header.timestamp <= interval as u64); | ||
} | ||
|
||
// <https://github.com/foundry-rs/foundry/issues/2341> | ||
|
@@ -582,6 +590,9 @@ async fn test_fork_revert_next_block_timestamp() { | |
let block = api.block_by_number(BlockNumberOrTag::Latest).await.unwrap().unwrap(); | ||
assert_eq!(block, latest_block); | ||
|
||
// Sleep for 1s | ||
tokio::time::sleep(Duration::from_secs(1)).await; | ||
Comment on lines
+593
to
+594
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need to sleep now? |
||
|
||
api.mine_one().await; | ||
let block = api.block_by_number(BlockNumberOrTag::Latest).await.unwrap().unwrap(); | ||
assert!(block.header.timestamp > latest_block.header.timestamp); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is okay due to how json numbers work and the existing test should cover this as well.