diff --git a/crates/consensus/consensus/src/lib.rs b/crates/consensus/consensus/src/lib.rs index e31db6df82..7bf171d479 100644 --- a/crates/consensus/consensus/src/lib.rs +++ b/crates/consensus/consensus/src/lib.rs @@ -13,7 +13,7 @@ extern crate alloc; use alloc::{fmt::Debug, string::String, vec::Vec}; use alloy_consensus::Header; -use alloy_primitives::{BlockHash, BlockNumber, Bloom, B256, U256}; +use alloy_primitives::{BlockHash, BlockNumber, Bloom, B256}; use reth_execution_types::BlockExecutionResult; use reth_primitives_traits::{ constants::{MAXIMUM_GAS_LIMIT_BLOCK, MINIMUM_GAS_LIMIT}, @@ -121,18 +121,6 @@ pub trait HeaderValidator: Debug + Send + Sync { } Ok(()) } - - /// Validate if the header is correct and follows the consensus specification, including - /// computed properties (like total difficulty). - /// - /// Some consensus engines may want to do additional checks here. - /// - /// Note: validating headers with TD does not include other HeaderValidator validation. - fn validate_header_with_total_difficulty( - &self, - header: &H, - total_difficulty: U256, - ) -> Result<(), ConsensusError>; } /// Consensus Errors diff --git a/crates/consensus/consensus/src/noop.rs b/crates/consensus/consensus/src/noop.rs index ecfb35d20d..259fae27d6 100644 --- a/crates/consensus/consensus/src/noop.rs +++ b/crates/consensus/consensus/src/noop.rs @@ -1,6 +1,5 @@ use crate::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; use alloc::sync::Arc; -use alloy_primitives::U256; use reth_execution_types::BlockExecutionResult; use reth_primitives_traits::{Block, NodePrimitives, RecoveredBlock, SealedBlock, SealedHeader}; @@ -28,14 +27,6 @@ impl HeaderValidator for NoopConsensus { ) -> Result<(), ConsensusError> { Ok(()) } - - fn validate_header_with_total_difficulty( - &self, - _header: &H, - _total_difficulty: U256, - ) -> Result<(), ConsensusError> { - Ok(()) - } } impl Consensus for NoopConsensus { diff --git a/crates/consensus/consensus/src/test_utils.rs b/crates/consensus/consensus/src/test_utils.rs index a7ca0f72f3..ad881cc9a7 100644 --- a/crates/consensus/consensus/src/test_utils.rs +++ b/crates/consensus/consensus/src/test_utils.rs @@ -1,5 +1,4 @@ use crate::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; -use alloy_primitives::U256; use core::sync::atomic::{AtomicBool, Ordering}; use reth_execution_types::BlockExecutionResult; use reth_primitives_traits::{Block, NodePrimitives, RecoveredBlock, SealedBlock, SealedHeader}; @@ -105,16 +104,4 @@ impl HeaderValidator for TestConsensus { Ok(()) } } - - fn validate_header_with_total_difficulty( - &self, - _header: &H, - _total_difficulty: U256, - ) -> Result<(), ConsensusError> { - if self.fail_validation() { - Err(ConsensusError::BaseFeeMissing) - } else { - Ok(()) - } - } } diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 0e2f0918a5..56d69d3373 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -11,7 +11,7 @@ use alloy_consensus::BlockHeader; use alloy_eips::{merge::EPOCH_SLOTS, BlockNumHash, NumHash}; use alloy_primitives::{ map::{HashMap, HashSet}, - BlockNumber, B256, U256, + BlockNumber, B256, }; use alloy_rpc_types_engine::{ ForkchoiceState, PayloadStatus, PayloadStatusEnum, PayloadValidationError, @@ -1906,18 +1906,6 @@ where /// Validate if block is correct and satisfies all the consensus rules that concern the header /// and block body itself. fn validate_block(&self, block: &RecoveredBlock) -> Result<(), ConsensusError> { - if let Err(e) = - self.consensus.validate_header_with_total_difficulty(block.header(), U256::MAX) - { - error!( - target: "engine::tree", - ?block, - "Failed to validate total difficulty for block {}: {e}", - block.hash() - ); - return Err(e) - } - if let Err(e) = self.consensus.validate_header(block.sealed_header()) { error!(target: "engine::tree", ?block, "Failed to validate header {}: {e}", block.hash()); return Err(e) diff --git a/crates/ethereum/cli/src/debug_cmd/build_block.rs b/crates/ethereum/cli/src/debug_cmd/build_block.rs index 20f4ec91d2..8942dd27ac 100644 --- a/crates/ethereum/cli/src/debug_cmd/build_block.rs +++ b/crates/ethereum/cli/src/debug_cmd/build_block.rs @@ -4,7 +4,7 @@ use alloy_eips::{ eip2718::Encodable2718, eip4844::{env_settings::EnvKzgSettings, BlobTransactionSidecar}, }; -use alloy_primitives::{Address, Bytes, B256, U256}; +use alloy_primitives::{Address, Bytes, B256}; use alloy_rlp::Decodable; use alloy_rpc_types::engine::{BlobsBundleV1, PayloadAttributes}; use clap::Parser; @@ -218,7 +218,6 @@ impl> Command { let block = payload.block(); debug!(target: "reth::cli", ?block, "Built new payload"); - consensus.validate_header_with_total_difficulty(block, U256::MAX)?; consensus.validate_header(block.sealed_header())?; consensus.validate_block_pre_execution(block)?; diff --git a/crates/ethereum/consensus/src/lib.rs b/crates/ethereum/consensus/src/lib.rs index 8dce0efe53..89ce9f72e0 100644 --- a/crates/ethereum/consensus/src/lib.rs +++ b/crates/ethereum/consensus/src/lib.rs @@ -14,7 +14,6 @@ extern crate alloc; use alloc::{fmt::Debug, sync::Arc}; use alloy_consensus::EMPTY_OMMER_ROOT_HASH; use alloy_eips::eip7840::BlobParams; -use alloy_primitives::U256; use reth_chainspec::{EthChainSpec, EthereumHardforks}; use reth_consensus::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; use reth_consensus_common::validation::{ @@ -137,8 +136,42 @@ where H: BlockHeader, { fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError> { - validate_header_gas(header.header())?; - validate_header_base_fee(header.header(), &self.chain_spec)?; + let header = header.header(); + let is_post_merge = self.chain_spec.is_paris_active_at_block(header.number()); + + if is_post_merge { + if !header.difficulty().is_zero() { + return Err(ConsensusError::TheMergeDifficultyIsNotZero); + } + + if !header.nonce().is_some_and(|nonce| nonce.is_zero()) { + return Err(ConsensusError::TheMergeNonceIsNotZero); + } + + if header.ommers_hash() != EMPTY_OMMER_ROOT_HASH { + return Err(ConsensusError::TheMergeOmmerRootIsNotEmpty); + } + } else { + #[cfg(feature = "std")] + { + let present_timestamp = std::time::SystemTime::now() + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + + if header.timestamp() > + present_timestamp + alloy_eips::merge::ALLOWED_FUTURE_BLOCK_TIME_SECONDS + { + return Err(ConsensusError::TimestampIsInFuture { + timestamp: header.timestamp(), + present_timestamp, + }); + } + } + } + validate_header_extra_data(header)?; + validate_header_gas(header)?; + validate_header_base_fee(header, &self.chain_spec)?; // EIP-4895: Beacon chain push withdrawals as operations if self.chain_spec.is_shanghai_active_at_timestamp(header.timestamp()) && @@ -154,7 +187,7 @@ where // Ensures that EIP-4844 fields are valid once cancun is active. if self.chain_spec.is_cancun_active_at_timestamp(header.timestamp()) { validate_4844_header_standalone( - header.header(), + header, self.chain_spec .blob_params_at_timestamp(header.timestamp()) .unwrap_or_else(BlobParams::cancun), @@ -204,67 +237,6 @@ where Ok(()) } - - fn validate_header_with_total_difficulty( - &self, - header: &H, - _total_difficulty: U256, - ) -> Result<(), ConsensusError> { - let is_post_merge = self.chain_spec.is_paris_active_at_block(header.number()); - - if is_post_merge { - if !header.difficulty().is_zero() { - return Err(ConsensusError::TheMergeDifficultyIsNotZero) - } - - if !header.nonce().is_some_and(|nonce| nonce.is_zero()) { - return Err(ConsensusError::TheMergeNonceIsNotZero) - } - - if header.ommers_hash() != EMPTY_OMMER_ROOT_HASH { - 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 clock) 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 extra data for all networks post merge - validate_header_extra_data(header)?; - - // mixHash is used instead of difficulty inside EVM - // https://eips.ethereum.org/EIPS/eip-4399#using-mixhash-field-instead-of-difficulty - } else { - // Note: This does not perform any pre merge checks for difficulty, mix_hash & nonce - // because those are deprecated and the expectation is that a reth node syncs in reverse - // order, making those checks obsolete. - - // Check if timestamp is in the future. Clock can drift but this can be consensus issue. - #[cfg(feature = "std")] - { - let present_timestamp = std::time::SystemTime::now() - .duration_since(std::time::SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs(); - if header.timestamp() > - present_timestamp + alloy_eips::merge::ALLOWED_FUTURE_BLOCK_TIME_SECONDS - { - return Err(ConsensusError::TimestampIsInFuture { - timestamp: header.timestamp(), - present_timestamp, - }) - } - } - - validate_header_extra_data(header)?; - } - - Ok(()) - } } #[cfg(test)] diff --git a/crates/optimism/consensus/src/lib.rs b/crates/optimism/consensus/src/lib.rs index 808e3cdcd8..0d3a6f155b 100644 --- a/crates/optimism/consensus/src/lib.rs +++ b/crates/optimism/consensus/src/lib.rs @@ -13,7 +13,7 @@ extern crate alloc; use alloc::{format, sync::Arc}; use alloy_consensus::{BlockHeader as _, EMPTY_OMMER_ROOT_HASH}; -use alloy_primitives::{B64, U256}; +use alloy_primitives::B64; use core::fmt::Debug; use reth_chainspec::{EthChainSpec, EthereumHardforks}; use reth_consensus::{Consensus, ConsensusError, FullConsensus, HeaderValidator}; @@ -134,8 +134,33 @@ impl HeaderValidator for OpBeaconConsensus { fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError> { - validate_header_gas(header.header())?; - validate_header_base_fee(header.header(), &self.chain_spec) + let header = header.header(); + // with OP-stack Bedrock activation number determines when TTD (eth Merge) has been reached. + debug_assert!( + self.chain_spec.is_bedrock_active_at_block(header.number()), + "manually import OVM blocks" + ); + + if header.nonce() != Some(B64::ZERO) { + return Err(ConsensusError::TheMergeNonceIsNotZero) + } + + if header.ommers_hash() != EMPTY_OMMER_ROOT_HASH { + 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 clock) 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 extra data for all networks post merge + validate_header_extra_data(header)?; + validate_header_gas(header)?; + validate_header_base_fee(header, &self.chain_spec) } fn validate_header_against_parent( @@ -180,40 +205,4 @@ impl HeaderValidator Ok(()) } - - fn validate_header_with_total_difficulty( - &self, - header: &H, - _total_difficulty: U256, - ) -> Result<(), ConsensusError> { - // with OP-stack Bedrock activation number determines when TTD (eth Merge) has been reached. - debug_assert!( - self.chain_spec.is_bedrock_active_at_block(header.number()), - "manually import OVM blocks" - ); - - if header.nonce() != Some(B64::ZERO) { - return Err(ConsensusError::TheMergeNonceIsNotZero) - } - - if header.ommers_hash() != EMPTY_OMMER_ROOT_HASH { - 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 clock) 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 extra data for all networks post merge - validate_header_extra_data(header)?; - - // mixHash is used instead of difficulty inside EVM - // https://eips.ethereum.org/EIPS/eip-4399#using-mixhash-field-instead-of-difficulty - - Ok(()) - } } diff --git a/crates/rpc/rpc/src/validation.rs b/crates/rpc/rpc/src/validation.rs index 73578a6cd9..7f3d3ee6a0 100644 --- a/crates/rpc/rpc/src/validation.rs +++ b/crates/rpc/rpc/src/validation.rs @@ -118,7 +118,6 @@ where ) -> Result<(), ValidationApiError> { self.validate_message_against_header(block.sealed_header(), &message)?; - self.consensus.validate_header_with_total_difficulty(block.sealed_header(), U256::MAX)?; self.consensus.validate_header(block.sealed_header())?; self.consensus.validate_block_pre_execution(block.sealed_block())?; diff --git a/crates/stages/stages/src/stages/headers.rs b/crates/stages/stages/src/stages/headers.rs index 32bf79eb1a..434527e97b 100644 --- a/crates/stages/stages/src/stages/headers.rs +++ b/crates/stages/stages/src/stages/headers.rs @@ -136,7 +136,7 @@ where .map_err(|err| StageError::Fatal(Box::new(err)))? .into(); - let (header, header_hash) = sealed_header.split(); + let (header, header_hash) = sealed_header.split_ref(); if header.number() == 0 { continue } @@ -145,19 +145,16 @@ where // Increase total difficulty td += header.difficulty(); - // Header validation - self.consensus.validate_header_with_total_difficulty(&header, td).map_err(|error| { - StageError::Block { - block: Box::new(BlockWithParent::new( - header.parent_hash(), - NumHash::new(header.number(), header_hash), - )), - error: BlockErrorKind::Validation(error), - } + self.consensus.validate_header(&sealed_header).map_err(|error| StageError::Block { + block: Box::new(BlockWithParent::new( + sealed_header.parent_hash(), + NumHash::new(sealed_header.number(), sealed_header.hash()), + )), + error: BlockErrorKind::Validation(error), })?; // Append to Headers segment - writer.append_header(&header, td, &header_hash)?; + writer.append_header(header, td, header_hash)?; } info!(target: "sync::stages::headers", total = total_headers, "Writing headers hash index"); diff --git a/testing/ef-tests/src/cases/blockchain_test.rs b/testing/ef-tests/src/cases/blockchain_test.rs index 1b57b4c4ec..a690dadb66 100644 --- a/testing/ef-tests/src/cases/blockchain_test.rs +++ b/testing/ef-tests/src/cases/blockchain_test.rs @@ -315,7 +315,6 @@ fn pre_execution_checks( let consensus: EthBeaconConsensus = EthBeaconConsensus::new(chain_spec); let sealed_header = block.sealed_header(); - let header = block.header(); as Consensus>::validate_body_against_header( &consensus, @@ -323,7 +322,6 @@ fn pre_execution_checks( sealed_header, )?; consensus.validate_header_against_parent(sealed_header, parent.sealed_header())?; - consensus.validate_header_with_total_difficulty(header, block.difficulty)?; consensus.validate_header(sealed_header)?; consensus.validate_block_pre_execution(block)?;