From e2f0aca89a7b050cbe5e7bc1235db8d7f5397b10 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 24 Jul 2023 09:38:59 -0400 Subject: [PATCH 1/4] fix: do not perform future timestamp checks post-merge --- .../consensus/beacon/src/beacon_consensus.rs | 26 ++++++++++++++++--- crates/consensus/common/src/validation.rs | 10 ------- crates/primitives/src/constants.rs | 9 +++++++ 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/crates/consensus/beacon/src/beacon_consensus.rs b/crates/consensus/beacon/src/beacon_consensus.rs index 449d46845c76..f413de2b527f 100644 --- a/crates/consensus/beacon/src/beacon_consensus.rs +++ b/crates/consensus/beacon/src/beacon_consensus.rs @@ -2,10 +2,10 @@ use reth_consensus_common::validation; use reth_interfaces::consensus::{Consensus, ConsensusError}; use reth_primitives::{ - constants::MAXIMUM_EXTRA_DATA_SIZE, Chain, ChainSpec, Hardfork, Header, SealedBlock, - SealedHeader, EMPTY_OMMER_ROOT, U256, + constants::{ALLOWED_FUTURE_BLOCK_TIME_SECONDS, MAXIMUM_EXTRA_DATA_SIZE}, + Chain, ChainSpec, Hardfork, Header, SealedBlock, SealedHeader, EMPTY_OMMER_ROOT, U256, }; -use std::sync::Arc; +use std::{sync::Arc, time::SystemTime}; /// Ethereum beacon consensus /// @@ -59,6 +59,14 @@ impl Consensus for BeaconConsensus { return Err(ConsensusError::TheMergeOmmerRootIsNotEmpty) } + // Post-merge, the consensus layer is expected to perform checks such that the block + // timestamp is a function of the slot. This is different from pre-merge, where blocks + // are only allowed to be in the future (compared to the system's block) by a certain + // threshold. + // + // Block validation with respect to the parent should ensure that the block timestamp + // is greater than its parent timestamp. + // validate header extradata for all networks post merge validate_header_extradata(header)?; @@ -69,6 +77,18 @@ impl Consensus for BeaconConsensus { // * difficulty, mix_hash & nonce aka PoW stuff // low priority as syncing is done in reverse order + // Check if timestamp is in future. Clock can drift but this can be consensus issue. + let present_timestamp = + SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs() + + ALLOWED_FUTURE_BLOCK_TIME_SECONDS; + + if header.timestamp > present_timestamp { + return Err(ConsensusError::TimestampIsInFuture { + timestamp: header.timestamp, + present_timestamp, + }) + } + // Goerli exception: // * If the network is goerli pre-merge, ignore the extradata check, since we do not // support clique. diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index f61e160f0390..e7dea5556fe7 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -23,16 +23,6 @@ pub fn validate_header_standalone( }) } - // Check if timestamp is in future. Clock can drift but this can be consensus issue. - let present_timestamp = - SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs(); - if header.timestamp > present_timestamp { - return Err(ConsensusError::TimestampIsInFuture { - timestamp: header.timestamp, - present_timestamp, - }) - } - // Check if base fee is set. if chain_spec.fork(Hardfork::London).active_at_block(header.number) && header.base_fee_per_gas.is_none() diff --git a/crates/primitives/src/constants.rs b/crates/primitives/src/constants.rs index 8530a2bc67c8..048a8801c4b3 100644 --- a/crates/primitives/src/constants.rs +++ b/crates/primitives/src/constants.rs @@ -108,6 +108,15 @@ pub const EMPTY_WITHDRAWALS: H256 = EMPTY_SET_HASH; /// the database. pub const BEACON_CONSENSUS_REORG_UNWIND_DEPTH: u64 = 3; +/// Max seconds from current time allowed for blocks, before they're considered future blocks. +/// +/// This is only used when checking whether or not the timestamp for pre-merge blocks is in the +/// future. +/// +/// See: +/// +pub const ALLOWED_FUTURE_BLOCK_TIME_SECONDS: u64 = 15; + #[cfg(test)] mod tests { use super::*; From 5ac4f192a591e69658cf5eeebe1f63d24e5f0720 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 24 Jul 2023 09:49:45 -0400 Subject: [PATCH 2/4] make clippy happy --- crates/consensus/common/src/validation.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index e7dea5556fe7..485d6146737b 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -5,10 +5,7 @@ use reth_primitives::{ SealedHeader, Transaction, TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxLegacy, }; use reth_provider::{AccountReader, HeaderProvider, WithdrawalsProvider}; -use std::{ - collections::{hash_map::Entry, HashMap}, - time::SystemTime, -}; +use std::collections::{hash_map::Entry, HashMap}; /// Validate header standalone pub fn validate_header_standalone( From 0c69e1b493a6df49742ba0b6d4fb8061c964db38 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 24 Jul 2023 09:55:13 -0400 Subject: [PATCH 3/4] add const in condition instead of in variable --- crates/consensus/beacon/src/beacon_consensus.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/consensus/beacon/src/beacon_consensus.rs b/crates/consensus/beacon/src/beacon_consensus.rs index f413de2b527f..bb604f4cd9a2 100644 --- a/crates/consensus/beacon/src/beacon_consensus.rs +++ b/crates/consensus/beacon/src/beacon_consensus.rs @@ -79,10 +79,9 @@ impl Consensus for BeaconConsensus { // Check if timestamp is in future. Clock can drift but this can be consensus issue. let present_timestamp = - SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs() + - ALLOWED_FUTURE_BLOCK_TIME_SECONDS; + SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs(); - if header.timestamp > present_timestamp { + if header.timestamp > present_timestamp + ALLOWED_FUTURE_BLOCK_TIME_SECONDS { return Err(ConsensusError::TimestampIsInFuture { timestamp: header.timestamp, present_timestamp, From e4a1d32019b36b7d50ebe2dccabc8f33af7e25fa Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 24 Jul 2023 10:12:17 -0400 Subject: [PATCH 4/4] s/block/clock --- crates/consensus/beacon/src/beacon_consensus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/consensus/beacon/src/beacon_consensus.rs b/crates/consensus/beacon/src/beacon_consensus.rs index bb604f4cd9a2..897bc25f4a1f 100644 --- a/crates/consensus/beacon/src/beacon_consensus.rs +++ b/crates/consensus/beacon/src/beacon_consensus.rs @@ -61,7 +61,7 @@ impl Consensus for BeaconConsensus { // Post-merge, the consensus layer is expected to perform checks such that the block // timestamp is a function of the slot. This is different from pre-merge, where blocks - // are only allowed to be in the future (compared to the system's block) by a certain + // are only allowed to be in the future (compared to the system's clock) by a certain // threshold. // // Block validation with respect to the parent should ensure that the block timestamp