From aa8ca14b95c075a24727fff53e5247144126c3aa Mon Sep 17 00:00:00 2001 From: Thomas Coratger Date: Mon, 5 Feb 2024 15:34:17 +0100 Subject: [PATCH 1/3] Encapsulate validate_header_regarding_parent inside SealedHeader --- Cargo.lock | 1 + crates/consensus/beacon-core/src/lib.rs | 2 +- crates/consensus/common/src/validation.rs | 257 +-------------- crates/interfaces/src/consensus.rs | 67 +--- crates/primitives/Cargo.toml | 1 + crates/primitives/src/header.rs | 380 +++++++++++++++++++++- crates/primitives/src/lib.rs | 2 +- 7 files changed, 386 insertions(+), 324 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 37aa6516bf17..335e8278d117 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6563,6 +6563,7 @@ dependencies = [ "byteorder", "bytes", "c-kzg", + "cfg-if", "clap", "criterion", "derive_more", diff --git a/crates/consensus/beacon-core/src/lib.rs b/crates/consensus/beacon-core/src/lib.rs index ad04672f1473..0042780f2a9f 100644 --- a/crates/consensus/beacon-core/src/lib.rs +++ b/crates/consensus/beacon-core/src/lib.rs @@ -43,7 +43,7 @@ impl Consensus for BeaconConsensus { header: &SealedHeader, parent: &SealedHeader, ) -> Result<(), ConsensusError> { - validation::validate_header_regarding_parent(parent, header, &self.chain_spec)?; + header.validate_regarding_parent(parent, &self.chain_spec).map_err(ConsensusError::from)?; Ok(()) } diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index 0fd66017618c..6dd50f99ec40 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -2,12 +2,7 @@ use reth_interfaces::{consensus::ConsensusError, RethResult}; use reth_primitives::{ - constants::{ - self, - eip4844::{DATA_GAS_PER_BLOB, MAX_DATA_GAS_PER_BLOCK}, - MINIMUM_GAS_LIMIT, - }, - eip4844::calculate_excess_blob_gas, + constants::eip4844::{DATA_GAS_PER_BLOB, MAX_DATA_GAS_PER_BLOCK}, BlockNumber, ChainSpec, GotExpected, Hardfork, Header, InvalidTransactionError, SealedBlock, SealedHeader, Transaction, TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxEip4844, TxLegacy, @@ -254,122 +249,6 @@ pub fn validate_block_standalone( Ok(()) } -/// Checks the gas limit for consistency between parent and child headers. -/// -/// The maximum allowable difference between child and parent gas limits is determined by the -/// parent's gas limit divided by the elasticity multiplier (1024). -/// -/// This check is skipped if the Optimism flag is enabled in the chain spec, as gas limits on -/// Optimism can adjust instantly. -#[inline(always)] -fn check_gas_limit( - parent: &SealedHeader, - child: &SealedHeader, - chain_spec: &ChainSpec, -) -> Result<(), ConsensusError> { - // Determine the parent gas limit, considering elasticity multiplier on the London fork. - let mut parent_gas_limit = parent.gas_limit; - if chain_spec.fork(Hardfork::London).transitions_at_block(child.number) { - parent_gas_limit = - parent.gas_limit * chain_spec.base_fee_params(child.timestamp).elasticity_multiplier; - } - - // Check for an increase in gas limit beyond the allowed threshold. - if child.gas_limit > parent_gas_limit { - if child.gas_limit - parent_gas_limit >= parent_gas_limit / 1024 { - return Err(ConsensusError::GasLimitInvalidIncrease { - parent_gas_limit, - child_gas_limit: child.gas_limit, - }); - } - } - // Check for a decrease in gas limit beyond the allowed threshold. - else if parent_gas_limit - child.gas_limit >= parent_gas_limit / 1024 { - return Err(ConsensusError::GasLimitInvalidDecrease { - parent_gas_limit, - child_gas_limit: child.gas_limit, - }); - } - // Check if the child gas limit is below the minimum required limit. - else if child.gas_limit < MINIMUM_GAS_LIMIT { - return Err(ConsensusError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }); - } - - Ok(()) -} - -/// Validate block in regards to parent -pub fn validate_header_regarding_parent( - parent: &SealedHeader, - child: &SealedHeader, - chain_spec: &ChainSpec, -) -> Result<(), ConsensusError> { - // Parent number is consistent. - if parent.number + 1 != child.number { - return Err(ConsensusError::ParentBlockNumberMismatch { - parent_block_number: parent.number, - block_number: child.number, - }) - } - - if parent.hash != child.parent_hash { - return Err(ConsensusError::ParentHashMismatch( - GotExpected { got: child.parent_hash, expected: parent.hash }.into(), - )) - } - - // timestamp in past check - if child.header.is_timestamp_in_past(parent.timestamp) { - return Err(ConsensusError::TimestampIsInPast { - parent_timestamp: parent.timestamp, - timestamp: child.timestamp, - }) - } - - // TODO Check difficulty increment between parent and child - // Ace age did increment it by some formula that we need to follow. - - cfg_if::cfg_if! { - if #[cfg(feature = "optimism")] { - // On Optimism, the gas limit can adjust instantly, so we skip this check - // if the optimism feature is enabled in the chain spec. - if !chain_spec.is_optimism() { - check_gas_limit(parent, child, chain_spec)?; - } - } else { - check_gas_limit(parent, child, chain_spec)?; - } - } - - // EIP-1559 check base fee - if chain_spec.fork(Hardfork::London).active_at_block(child.number) { - let base_fee = child.base_fee_per_gas.ok_or(ConsensusError::BaseFeeMissing)?; - - let expected_base_fee = - if chain_spec.fork(Hardfork::London).transitions_at_block(child.number) { - constants::EIP1559_INITIAL_BASE_FEE - } else { - // This BaseFeeMissing will not happen as previous blocks are checked to have them. - parent - .next_block_base_fee(chain_spec.base_fee_params(child.timestamp)) - .ok_or(ConsensusError::BaseFeeMissing)? - }; - if expected_base_fee != base_fee { - return Err(ConsensusError::BaseFeeDiff(GotExpected { - expected: expected_base_fee, - got: base_fee, - })) - } - } - - // ensure that the blob gas fields for this block - if chain_spec.fork(Hardfork::Cancun).active_at_timestamp(child.timestamp) { - validate_4844_header_with_parent(parent, child)?; - } - - Ok(()) -} - /// Validate block in regards to chain (parent) /// /// Checks: @@ -397,41 +276,6 @@ pub fn validate_block_regarding_chain Result<(), ConsensusError> { - // From [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844#header-extension): - // - // > For the first post-fork block, both parent.blob_gas_used and parent.excess_blob_gas - // > are evaluated as 0. - // - // This means in the first post-fork block, calculate_excess_blob_gas will return 0. - let parent_blob_gas_used = parent.blob_gas_used.unwrap_or(0); - let parent_excess_blob_gas = parent.excess_blob_gas.unwrap_or(0); - - if child.blob_gas_used.is_none() { - return Err(ConsensusError::BlobGasUsedMissing) - } - let excess_blob_gas = child.excess_blob_gas.ok_or(ConsensusError::ExcessBlobGasMissing)?; - - let expected_excess_blob_gas = - calculate_excess_blob_gas(parent_excess_blob_gas, parent_blob_gas_used); - if expected_excess_blob_gas != excess_blob_gas { - return Err(ConsensusError::ExcessBlobGasDiff { - diff: GotExpected { got: excess_blob_gas, expected: expected_excess_blob_gas }, - parent_excess_blob_gas, - parent_blob_gas_used, - }) - } - - Ok(()) -} - /// Validates that the EIP-4844 header fields exist and conform to the spec. This ensures that: /// /// * `blob_gas_used` exists as a header field @@ -825,103 +669,4 @@ mod tests { })) ); } - - #[test] - fn test_valid_gas_limit_increase() { - let parent = SealedHeader { - header: Header { gas_limit: 1024 * 10, ..Default::default() }, - ..Default::default() - }; - let child = SealedHeader { - header: Header { gas_limit: parent.header.gas_limit + 5, ..Default::default() }, - ..Default::default() - }; - let chain_spec = ChainSpec::default(); - - assert_eq!(check_gas_limit(&parent, &child, &chain_spec), Ok(())); - } - - #[test] - fn test_gas_limit_below_minimum() { - let parent = SealedHeader { - header: Header { gas_limit: MINIMUM_GAS_LIMIT, ..Default::default() }, - ..Default::default() - }; - let child = SealedHeader { - header: Header { gas_limit: MINIMUM_GAS_LIMIT - 1, ..Default::default() }, - ..Default::default() - }; - let chain_spec = ChainSpec::default(); - - assert_eq!( - check_gas_limit(&parent, &child, &chain_spec), - Err(ConsensusError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }) - ); - } - - #[test] - fn test_invalid_gas_limit_increase_exceeding_limit() { - let gas_limit = 1024 * 10; - let parent = SealedHeader { - header: Header { gas_limit, ..Default::default() }, - ..Default::default() - }; - let child = SealedHeader { - header: Header { - gas_limit: parent.header.gas_limit + parent.header.gas_limit / 1024 + 1, - ..Default::default() - }, - ..Default::default() - }; - let chain_spec = ChainSpec::default(); - - assert_eq!( - check_gas_limit(&parent, &child, &chain_spec), - Err(ConsensusError::GasLimitInvalidIncrease { - parent_gas_limit: parent.header.gas_limit, - child_gas_limit: child.header.gas_limit, - }) - ); - } - - #[test] - fn test_valid_gas_limit_decrease_within_limit() { - let gas_limit = 1024 * 10; - let parent = SealedHeader { - header: Header { gas_limit, ..Default::default() }, - ..Default::default() - }; - let child = SealedHeader { - header: Header { gas_limit: parent.header.gas_limit - 5, ..Default::default() }, - ..Default::default() - }; - let chain_spec = ChainSpec::default(); - - assert_eq!(check_gas_limit(&parent, &child, &chain_spec), Ok(())); - } - - #[test] - fn test_invalid_gas_limit_decrease_exceeding_limit() { - let gas_limit = 1024 * 10; - let parent = SealedHeader { - header: Header { gas_limit, ..Default::default() }, - ..Default::default() - }; - let child = SealedHeader { - header: Header { - gas_limit: parent.header.gas_limit - parent.header.gas_limit / 1024 - 1, - ..Default::default() - }, - ..Default::default() - }; - let chain_spec = ChainSpec::default(); - - assert_eq!( - check_gas_limit(&parent, &child, &chain_spec), - Err(ConsensusError::GasLimitInvalidDecrease { - parent_gas_limit: parent.header.gas_limit, - child_gas_limit: child.header.gas_limit, - }) - ); - } } diff --git a/crates/interfaces/src/consensus.rs b/crates/interfaces/src/consensus.rs index 846e70e24e2b..574754cb06ed 100644 --- a/crates/interfaces/src/consensus.rs +++ b/crates/interfaces/src/consensus.rs @@ -1,6 +1,6 @@ use reth_primitives::{ - constants::MINIMUM_GAS_LIMIT, BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, - InvalidTransactionError, SealedBlock, SealedHeader, B256, U256, + BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, InvalidTransactionError, + SealedBlock, SealedHeader, SealedHeaderError, B256, U256, }; use std::fmt::Debug; @@ -130,19 +130,6 @@ pub enum ConsensusError { block_number: BlockNumber, }, - /// Error when the parent hash does not match the expected parent hash. - #[error("mismatched parent hash: {0}")] - ParentHashMismatch(GotExpectedBoxed), - - /// Error when the block timestamp is in the past compared to the parent timestamp. - #[error("block timestamp {timestamp} is in the past compared to the parent timestamp {parent_timestamp}")] - TimestampIsInPast { - /// The parent block's timestamp. - parent_timestamp: u64, - /// The block's timestamp. - timestamp: u64, - }, - /// Error when the block timestamp is in the future compared to our clock time. #[error("block timestamp {timestamp} is in the future compared to our clock time {present_timestamp}")] TimestampIsInFuture { @@ -152,41 +139,10 @@ pub enum ConsensusError { present_timestamp: u64, }, - /// Error when the child gas limit exceeds the maximum allowed increase. - #[error("child gas_limit {child_gas_limit} max increase is {parent_gas_limit}/1024")] - GasLimitInvalidIncrease { - /// The parent gas limit. - parent_gas_limit: u64, - /// The child gas limit. - child_gas_limit: u64, - }, - - /// Error when the child gas limit exceeds the maximum allowed decrease. - #[error("child gas_limit {child_gas_limit} max decrease is {parent_gas_limit}/1024")] - GasLimitInvalidDecrease { - /// The parent gas limit. - parent_gas_limit: u64, - /// The child gas limit. - child_gas_limit: u64, - }, - - /// Error indicating that the child gas limit is below the minimum allowed limit. - /// - /// This error occurs when the child gas limit is less than the specified minimum gas limit. - #[error("child gas limit {child_gas_limit} is below the minimum allowed limit ({MINIMUM_GAS_LIMIT})")] - GasLimitInvalidMinimum { - /// The child gas limit. - child_gas_limit: u64, - }, - /// Error when the base fee is missing. #[error("base fee missing")] BaseFeeMissing, - /// Error when the block's base fee is different from the expected base fee. - #[error("block base fee mismatch: {0}")] - BaseFeeDiff(GotExpected), - /// Error when there is a transaction signer recovery error. #[error("transaction signer recovery error")] TransactionSignerRecoveryError, @@ -270,22 +226,11 @@ pub enum ConsensusError { #[error("blob gas used mismatch: {0}")] BlobGasUsedDiff(GotExpected), - /// Error when there is an invalid excess blob gas. - #[error( - "invalid excess blob gas: {diff}; \ - parent excess blob gas: {parent_excess_blob_gas}, \ - parent blob gas used: {parent_blob_gas_used}" - )] - ExcessBlobGasDiff { - /// The excess blob gas diff. - diff: GotExpected, - /// The parent excess blob gas. - parent_excess_blob_gas: u64, - /// The parent blob gas used. - parent_blob_gas_used: u64, - }, - /// Error for a transaction that violates consensus. #[error(transparent)] InvalidTransaction(#[from] InvalidTransactionError), + + /// Error type transparently wrapping SealedHeaderError. + #[error(transparent)] + SealedHeaderError(#[from] SealedHeaderError), } diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 280b151fba3e..6fe49310474c 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -55,6 +55,7 @@ tempfile.workspace = true thiserror.workspace = true zstd = { version = "0.12", features = ["experimental"] } ahash.workspace = true +cfg-if = "1.0.0" # `test-utils` feature hash-db = { version = "~0.15", optional = true } diff --git a/crates/primitives/src/header.rs b/crates/primitives/src/header.rs index bb195e0dd553..6f69ea75d6c5 100644 --- a/crates/primitives/src/header.rs +++ b/crates/primitives/src/header.rs @@ -1,9 +1,13 @@ use crate::{ basefee::calculate_next_block_base_fee, - constants::{ALLOWED_FUTURE_BLOCK_TIME_SECONDS, EMPTY_OMMER_ROOT_HASH, EMPTY_ROOT_HASH}, + constants, + constants::{ + ALLOWED_FUTURE_BLOCK_TIME_SECONDS, EMPTY_OMMER_ROOT_HASH, EMPTY_ROOT_HASH, + MINIMUM_GAS_LIMIT, + }, eip4844::{calc_blob_gasprice, calculate_excess_blob_gas}, - keccak256, Address, BaseFeeParams, BlockHash, BlockNumHash, BlockNumber, Bloom, Bytes, B256, - B64, U256, + keccak256, Address, BaseFeeParams, BlockHash, BlockNumHash, BlockNumber, Bloom, Bytes, + ChainSpec, GotExpected, GotExpectedBoxed, Hardfork, B256, B64, U256, }; use alloy_rlp::{length_of_length, Decodable, Encodable, EMPTY_LIST_CODE, EMPTY_STRING_CODE}; use bytes::{Buf, BufMut, BytesMut}; @@ -574,11 +578,97 @@ impl Decodable for Header { } } +/// Errors that can occur during header sanity checks. +#[derive(thiserror::Error, Debug, PartialEq, Eq, Clone)] +pub enum SealedHeaderError { + /// Error when the block number does not match the parent block number. + #[error( + "block number {block_number} does not match parent block number {parent_block_number}" + )] + ParentBlockNumberMismatch { + /// The parent block number. + parent_block_number: BlockNumber, + /// The block number. + block_number: BlockNumber, + }, + + /// Error when the parent hash does not match the expected parent hash. + #[error("mismatched parent hash: {0}")] + ParentHashMismatch(GotExpectedBoxed), + + /// Error when the block timestamp is in the past compared to the parent timestamp. + #[error("block timestamp {timestamp} is in the past compared to the parent timestamp {parent_timestamp}")] + TimestampIsInPast { + /// The parent block's timestamp. + parent_timestamp: u64, + /// The block's timestamp. + timestamp: u64, + }, + + /// Error when the base fee is missing. + #[error("base fee missing")] + BaseFeeMissing, + + /// Error when the block's base fee is different from the expected base fee. + #[error("block base fee mismatch: {0}")] + BaseFeeDiff(GotExpected), + + /// Error when the child gas limit exceeds the maximum allowed decrease. + #[error("child gas_limit {child_gas_limit} max decrease is {parent_gas_limit}/1024")] + GasLimitInvalidDecrease { + /// The parent gas limit. + parent_gas_limit: u64, + /// The child gas limit. + child_gas_limit: u64, + }, + + /// Error when the child gas limit exceeds the maximum allowed increase. + #[error("child gas_limit {child_gas_limit} max increase is {parent_gas_limit}/1024")] + GasLimitInvalidIncrease { + /// The parent gas limit. + parent_gas_limit: u64, + /// The child gas limit. + child_gas_limit: u64, + }, + + /// Error indicating that the child gas limit is below the minimum allowed limit. + /// + /// This error occurs when the child gas limit is less than the specified minimum gas limit. + #[error("child gas limit {child_gas_limit} is below the minimum allowed limit ({MINIMUM_GAS_LIMIT})")] + GasLimitInvalidMinimum { + /// The child gas limit. + child_gas_limit: u64, + }, + + /// Error when blob gas used is missing. + #[error("missing blob gas used")] + BlobGasUsedMissing, + + /// Error when excess blob gas is missing. + #[error("missing excess blob gas")] + ExcessBlobGasMissing, + + /// Error when there is an invalid excess blob gas. + #[error( + "invalid excess blob gas: {diff}; \ + parent excess blob gas: {parent_excess_blob_gas}, \ + parent blob gas used: {parent_blob_gas_used}" + )] + ExcessBlobGasDiff { + /// The excess blob gas diff. + diff: GotExpected, + /// The parent excess blob gas. + parent_excess_blob_gas: u64, + /// The parent blob gas used. + parent_blob_gas_used: u64, + }, +} + /// A [`Header`] that is sealed at a precalculated hash, use [`SealedHeader::unseal()`] if you want /// to modify header. #[add_arbitrary_tests(rlp)] #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct SealedHeader { +pub struct v { /// Locked Header fields. pub header: Header, /// Locked Header hash. @@ -586,6 +676,184 @@ pub struct SealedHeader { } impl SealedHeader { + /// Checks the gas limit for consistency between parent and self headers. + /// + /// The maximum allowable difference between self and parent gas limits is determined by the + /// parent's gas limit divided by the elasticity multiplier (1024). + /// + /// This check is skipped if the Optimism flag is enabled in the chain spec, as gas limits on + /// Optimism can adjust instantly. + #[inline(always)] + fn validate_gas_limit( + &self, + parent: &SealedHeader, + chain_spec: &ChainSpec, + ) -> Result<(), SealedHeaderError> { + // Determine the parent gas limit, considering elasticity multiplier on the London fork. + let mut parent_gas_limit = parent.gas_limit; + if chain_spec.fork(Hardfork::London).transitions_at_block(self.number) { + parent_gas_limit = + parent.gas_limit * chain_spec.base_fee_params(self.timestamp).elasticity_multiplier; + } + + // Check for an increase in gas limit beyond the allowed threshold. + if self.gas_limit > parent_gas_limit { + if self.gas_limit - parent_gas_limit >= parent_gas_limit / 1024 { + return Err(SealedHeaderError::GasLimitInvalidIncrease { + parent_gas_limit, + child_gas_limit: self.gas_limit, + }); + } + } + // Check for a decrease in gas limit beyond the allowed threshold. + else if parent_gas_limit - self.gas_limit >= parent_gas_limit / 1024 { + return Err(SealedHeaderError::GasLimitInvalidDecrease { + parent_gas_limit, + child_gas_limit: self.gas_limit, + }); + } + // Check if the self gas limit is below the minimum required limit. + else if self.gas_limit < MINIMUM_GAS_LIMIT { + return Err(SealedHeaderError::GasLimitInvalidMinimum { + child_gas_limit: self.gas_limit, + }); + } + + Ok(()) + } + + /// Validates the integrity and consistency of a sealed block header in relation to its parent + /// header. + /// + /// This function checks various properties of the sealed header against its parent header and + /// the chain specification. It ensures that the block forms a valid and secure continuation + /// of the blockchain. + /// + /// ## Arguments + /// + /// * `parent` - The sealed header of the parent block. + /// * `chain_spec` - The chain specification providing configuration parameters for the + /// blockchain. + /// + /// ## Errors + /// + /// Returns a [`SealedHeaderError`] if any validation check fails, indicating specific issues + /// with the sealed header. The possible errors include mismatched block numbers, parent + /// hash mismatches, timestamp inconsistencies, gas limit violations, base fee discrepancies + /// (for EIP-1559), and errors related to the blob gas fields (EIP-4844). + /// + /// ## Note + /// + /// Some checks, such as gas limit validation, are conditionally skipped based on the presence + /// of certain features (e.g., Optimism feature) or the activation of specific hardforks. + pub fn validate_regarding_parent( + &self, + parent: &SealedHeader, + chain_spec: &ChainSpec, + ) -> Result<(), SealedHeaderError> { + // Parent number is consistent. + if parent.number + 1 != self.number { + return Err(SealedHeaderError::ParentBlockNumberMismatch { + parent_block_number: parent.number, + block_number: self.number, + }) + } + + if parent.hash != self.parent_hash { + return Err(SealedHeaderError::ParentHashMismatch( + GotExpected { got: self.parent_hash, expected: parent.hash }.into(), + )) + } + + // timestamp in past check + if self.header.is_timestamp_in_past(parent.timestamp) { + return Err(SealedHeaderError::TimestampIsInPast { + parent_timestamp: parent.timestamp, + timestamp: self.timestamp, + }) + } + + // TODO Check difficulty increment between parent and self + // Ace age did increment it by some formula that we need to follow. + + cfg_if::cfg_if! { + if #[cfg(feature = "optimism")] { + // On Optimism, the gas limit can adjust instantly, so we skip this check + // if the optimism feature is enabled in the chain spec. + if !chain_spec.is_optimism() { + self.validate_gas_limit(parent, chain_spec)?; + } + } else { + self.validate_gas_limit(parent, chain_spec)?; + } + } + + // EIP-1559 check base fee + if chain_spec.fork(Hardfork::London).active_at_block(self.number) { + let base_fee = self.base_fee_per_gas.ok_or(SealedHeaderError::BaseFeeMissing)?; + + let expected_base_fee = + if chain_spec.fork(Hardfork::London).transitions_at_block(self.number) { + constants::EIP1559_INITIAL_BASE_FEE + } else { + // This BaseFeeMissing will not happen as previous blocks are checked to have + // them. + parent + .next_block_base_fee(chain_spec.base_fee_params(self.timestamp)) + .ok_or(SealedHeaderError::BaseFeeMissing)? + }; + if expected_base_fee != base_fee { + return Err(SealedHeaderError::BaseFeeDiff(GotExpected { + expected: expected_base_fee, + got: base_fee, + })) + } + } + + // ensure that the blob gas fields for this block + if chain_spec.fork(Hardfork::Cancun).active_at_timestamp(self.timestamp) { + self.validate_4844_header_regarding_parent(parent)?; + } + + Ok(()) + } + + /// Validates that the EIP-4844 header fields are correct with respect to the parent block. This + /// ensures that the `blob_gas_used` and `excess_blob_gas` fields exist in the child header, and + /// that the `excess_blob_gas` field matches the expected `excess_blob_gas` calculated from the + /// parent header fields. + pub fn validate_4844_header_regarding_parent( + &self, + parent: &SealedHeader, + ) -> Result<(), SealedHeaderError> { + // From [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844#header-extension): + // + // > For the first post-fork block, both parent.blob_gas_used and parent.excess_blob_gas + // > are evaluated as 0. + // + // This means in the first post-fork block, calculate_excess_blob_gas will return 0. + let parent_blob_gas_used = parent.blob_gas_used.unwrap_or(0); + let parent_excess_blob_gas = parent.excess_blob_gas.unwrap_or(0); + + if self.blob_gas_used.is_none() { + return Err(SealedHeaderError::BlobGasUsedMissing) + } + let excess_blob_gas = + self.excess_blob_gas.ok_or(SealedHeaderError::ExcessBlobGasMissing)?; + + let expected_excess_blob_gas = + calculate_excess_blob_gas(parent_excess_blob_gas, parent_blob_gas_used); + if expected_excess_blob_gas != excess_blob_gas { + return Err(SealedHeaderError::ExcessBlobGasDiff { + diff: GotExpected { got: excess_blob_gas, expected: expected_excess_blob_gas }, + parent_excess_blob_gas, + parent_blob_gas_used, + }) + } + + Ok(()) + } + /// Extract raw header that can be modified. pub fn unseal(self) -> Header { self.header @@ -807,7 +1075,10 @@ mod ethers_compat { #[cfg(test)] mod tests { use super::{Bytes, Decodable, Encodable, Header, B256}; - use crate::{address, b256, bloom, bytes, hex, Address, HeadersDirection, U256}; + use crate::{ + address, b256, bloom, bytes, header::MINIMUM_GAS_LIMIT, hex, Address, ChainSpec, + HeadersDirection, SealedHeader, SealedHeaderError, U256, + }; use std::str::FromStr; // Test vector from: https://eips.ethereum.org/EIPS/eip-2481 @@ -1067,4 +1338,103 @@ mod tests { Header::decode(&mut data.as_slice()) .expect_err("blob_gas_used size should make this header decoding fail"); } + + #[test] + fn test_valid_gas_limit_increase() { + let parent = SealedHeader { + header: Header { gas_limit: 1024 * 10, ..Default::default() }, + ..Default::default() + }; + let child = SealedHeader { + header: Header { gas_limit: parent.header.gas_limit + 5, ..Default::default() }, + ..Default::default() + }; + let chain_spec = ChainSpec::default(); + + assert_eq!(child.validate_gas_limit(&parent, &chain_spec), Ok(())); + } + + #[test] + fn test_gas_limit_below_minimum() { + let parent = SealedHeader { + header: Header { gas_limit: MINIMUM_GAS_LIMIT, ..Default::default() }, + ..Default::default() + }; + let child = SealedHeader { + header: Header { gas_limit: MINIMUM_GAS_LIMIT - 1, ..Default::default() }, + ..Default::default() + }; + let chain_spec = ChainSpec::default(); + + assert_eq!( + child.validate_gas_limit(&parent, &chain_spec), + Err(SealedHeaderError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }) + ); + } + + #[test] + fn test_invalid_gas_limit_increase_exceeding_limit() { + let gas_limit = 1024 * 10; + let parent = SealedHeader { + header: Header { gas_limit, ..Default::default() }, + ..Default::default() + }; + let child = SealedHeader { + header: Header { + gas_limit: parent.header.gas_limit + parent.header.gas_limit / 1024 + 1, + ..Default::default() + }, + ..Default::default() + }; + let chain_spec = ChainSpec::default(); + + assert_eq!( + child.validate_gas_limit(&parent, &chain_spec), + Err(SealedHeaderError::GasLimitInvalidIncrease { + parent_gas_limit: parent.header.gas_limit, + child_gas_limit: child.header.gas_limit, + }) + ); + } + + #[test] + fn test_valid_gas_limit_decrease_within_limit() { + let gas_limit = 1024 * 10; + let parent = SealedHeader { + header: Header { gas_limit, ..Default::default() }, + ..Default::default() + }; + let child = SealedHeader { + header: Header { gas_limit: parent.header.gas_limit - 5, ..Default::default() }, + ..Default::default() + }; + let chain_spec = ChainSpec::default(); + + assert_eq!(child.validate_gas_limit(&parent, &chain_spec), Ok(())); + } + + #[test] + fn test_invalid_gas_limit_decrease_exceeding_limit() { + let gas_limit = 1024 * 10; + let parent = SealedHeader { + header: Header { gas_limit, ..Default::default() }, + ..Default::default() + }; + let child = SealedHeader { + header: Header { + gas_limit: parent.header.gas_limit - parent.header.gas_limit / 1024 - 1, + ..Default::default() + }, + ..Default::default() + }; + let chain_spec = ChainSpec::default(); + + assert_eq!( + child.validate_gas_limit(&parent, &chain_spec), + Err(SealedHeaderError::GasLimitInvalidDecrease { + parent_gas_limit: parent.header.gas_limit, + child_gas_limit: child.header.gas_limit, + }) + ); + } } diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index cfa7ad364fdb..eb28a074dc2e 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -60,7 +60,7 @@ pub use constants::{ }; pub use error::{GotExpected, GotExpectedBoxed}; pub use genesis::{ChainConfig, Genesis, GenesisAccount}; -pub use header::{Header, HeadersDirection, SealedHeader}; +pub use header::{Header, HeadersDirection, SealedHeader, SealedHeaderError}; pub use integer_list::IntegerList; pub use log::{logs_bloom, Log}; pub use net::{ From f25fb8b10b9771684054e8a01b9ed9a77bac78cc Mon Sep 17 00:00:00 2001 From: Thomas Coratger Date: Mon, 5 Feb 2024 15:44:12 +0100 Subject: [PATCH 2/3] fix --- crates/primitives/src/header.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives/src/header.rs b/crates/primitives/src/header.rs index 6f69ea75d6c5..ce6b39737cb5 100644 --- a/crates/primitives/src/header.rs +++ b/crates/primitives/src/header.rs @@ -668,7 +668,7 @@ pub enum SealedHeaderError { /// to modify header. #[add_arbitrary_tests(rlp)] #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct v { +pub struct SealedHeader { /// Locked Header fields. pub header: Header, /// Locked Header hash. From be3128b49bf759db30461634959bb07b04b2889f Mon Sep 17 00:00:00 2001 From: Thomas Coratger Date: Tue, 6 Feb 2024 16:10:22 +0100 Subject: [PATCH 3/3] fix namings --- crates/consensus/beacon-core/src/lib.rs | 2 +- crates/interfaces/src/consensus.rs | 8 ++-- crates/primitives/src/header.rs | 54 ++++++++++++------------- crates/primitives/src/lib.rs | 2 +- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/crates/consensus/beacon-core/src/lib.rs b/crates/consensus/beacon-core/src/lib.rs index 0042780f2a9f..3837872ab2ed 100644 --- a/crates/consensus/beacon-core/src/lib.rs +++ b/crates/consensus/beacon-core/src/lib.rs @@ -43,7 +43,7 @@ impl Consensus for BeaconConsensus { header: &SealedHeader, parent: &SealedHeader, ) -> Result<(), ConsensusError> { - header.validate_regarding_parent(parent, &self.chain_spec).map_err(ConsensusError::from)?; + header.validate_against_parent(parent, &self.chain_spec).map_err(ConsensusError::from)?; Ok(()) } diff --git a/crates/interfaces/src/consensus.rs b/crates/interfaces/src/consensus.rs index 574754cb06ed..2bad200c6041 100644 --- a/crates/interfaces/src/consensus.rs +++ b/crates/interfaces/src/consensus.rs @@ -1,6 +1,6 @@ use reth_primitives::{ - BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, InvalidTransactionError, - SealedBlock, SealedHeader, SealedHeaderError, B256, U256, + BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, HeaderValidationError, + InvalidTransactionError, SealedBlock, SealedHeader, B256, U256, }; use std::fmt::Debug; @@ -230,7 +230,7 @@ pub enum ConsensusError { #[error(transparent)] InvalidTransaction(#[from] InvalidTransactionError), - /// Error type transparently wrapping SealedHeaderError. + /// Error type transparently wrapping HeaderValidationError. #[error(transparent)] - SealedHeaderError(#[from] SealedHeaderError), + HeaderValidationError(#[from] HeaderValidationError), } diff --git a/crates/primitives/src/header.rs b/crates/primitives/src/header.rs index ce6b39737cb5..efee4a1410c3 100644 --- a/crates/primitives/src/header.rs +++ b/crates/primitives/src/header.rs @@ -580,7 +580,7 @@ impl Decodable for Header { /// Errors that can occur during header sanity checks. #[derive(thiserror::Error, Debug, PartialEq, Eq, Clone)] -pub enum SealedHeaderError { +pub enum HeaderValidationError { /// Error when the block number does not match the parent block number. #[error( "block number {block_number} does not match parent block number {parent_block_number}" @@ -688,7 +688,7 @@ impl SealedHeader { &self, parent: &SealedHeader, chain_spec: &ChainSpec, - ) -> Result<(), SealedHeaderError> { + ) -> Result<(), HeaderValidationError> { // Determine the parent gas limit, considering elasticity multiplier on the London fork. let mut parent_gas_limit = parent.gas_limit; if chain_spec.fork(Hardfork::London).transitions_at_block(self.number) { @@ -699,7 +699,7 @@ impl SealedHeader { // Check for an increase in gas limit beyond the allowed threshold. if self.gas_limit > parent_gas_limit { if self.gas_limit - parent_gas_limit >= parent_gas_limit / 1024 { - return Err(SealedHeaderError::GasLimitInvalidIncrease { + return Err(HeaderValidationError::GasLimitInvalidIncrease { parent_gas_limit, child_gas_limit: self.gas_limit, }); @@ -707,14 +707,14 @@ impl SealedHeader { } // Check for a decrease in gas limit beyond the allowed threshold. else if parent_gas_limit - self.gas_limit >= parent_gas_limit / 1024 { - return Err(SealedHeaderError::GasLimitInvalidDecrease { + return Err(HeaderValidationError::GasLimitInvalidDecrease { parent_gas_limit, child_gas_limit: self.gas_limit, }); } // Check if the self gas limit is below the minimum required limit. else if self.gas_limit < MINIMUM_GAS_LIMIT { - return Err(SealedHeaderError::GasLimitInvalidMinimum { + return Err(HeaderValidationError::GasLimitInvalidMinimum { child_gas_limit: self.gas_limit, }); } @@ -737,37 +737,37 @@ impl SealedHeader { /// /// ## Errors /// - /// Returns a [`SealedHeaderError`] if any validation check fails, indicating specific issues - /// with the sealed header. The possible errors include mismatched block numbers, parent - /// hash mismatches, timestamp inconsistencies, gas limit violations, base fee discrepancies - /// (for EIP-1559), and errors related to the blob gas fields (EIP-4844). + /// Returns a [`HeaderValidationError`] if any validation check fails, indicating specific + /// issues with the sealed header. The possible errors include mismatched block numbers, + /// parent hash mismatches, timestamp inconsistencies, gas limit violations, base fee + /// discrepancies (for EIP-1559), and errors related to the blob gas fields (EIP-4844). /// /// ## Note /// /// Some checks, such as gas limit validation, are conditionally skipped based on the presence /// of certain features (e.g., Optimism feature) or the activation of specific hardforks. - pub fn validate_regarding_parent( + pub fn validate_against_parent( &self, parent: &SealedHeader, chain_spec: &ChainSpec, - ) -> Result<(), SealedHeaderError> { + ) -> Result<(), HeaderValidationError> { // Parent number is consistent. if parent.number + 1 != self.number { - return Err(SealedHeaderError::ParentBlockNumberMismatch { + return Err(HeaderValidationError::ParentBlockNumberMismatch { parent_block_number: parent.number, block_number: self.number, }) } if parent.hash != self.parent_hash { - return Err(SealedHeaderError::ParentHashMismatch( + return Err(HeaderValidationError::ParentHashMismatch( GotExpected { got: self.parent_hash, expected: parent.hash }.into(), )) } // timestamp in past check if self.header.is_timestamp_in_past(parent.timestamp) { - return Err(SealedHeaderError::TimestampIsInPast { + return Err(HeaderValidationError::TimestampIsInPast { parent_timestamp: parent.timestamp, timestamp: self.timestamp, }) @@ -790,7 +790,7 @@ impl SealedHeader { // EIP-1559 check base fee if chain_spec.fork(Hardfork::London).active_at_block(self.number) { - let base_fee = self.base_fee_per_gas.ok_or(SealedHeaderError::BaseFeeMissing)?; + let base_fee = self.base_fee_per_gas.ok_or(HeaderValidationError::BaseFeeMissing)?; let expected_base_fee = if chain_spec.fork(Hardfork::London).transitions_at_block(self.number) { @@ -800,10 +800,10 @@ impl SealedHeader { // them. parent .next_block_base_fee(chain_spec.base_fee_params(self.timestamp)) - .ok_or(SealedHeaderError::BaseFeeMissing)? + .ok_or(HeaderValidationError::BaseFeeMissing)? }; if expected_base_fee != base_fee { - return Err(SealedHeaderError::BaseFeeDiff(GotExpected { + return Err(HeaderValidationError::BaseFeeDiff(GotExpected { expected: expected_base_fee, got: base_fee, })) @@ -812,7 +812,7 @@ impl SealedHeader { // ensure that the blob gas fields for this block if chain_spec.fork(Hardfork::Cancun).active_at_timestamp(self.timestamp) { - self.validate_4844_header_regarding_parent(parent)?; + self.validate_4844_header_against_parent(parent)?; } Ok(()) @@ -822,10 +822,10 @@ impl SealedHeader { /// ensures that the `blob_gas_used` and `excess_blob_gas` fields exist in the child header, and /// that the `excess_blob_gas` field matches the expected `excess_blob_gas` calculated from the /// parent header fields. - pub fn validate_4844_header_regarding_parent( + pub fn validate_4844_header_against_parent( &self, parent: &SealedHeader, - ) -> Result<(), SealedHeaderError> { + ) -> Result<(), HeaderValidationError> { // From [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844#header-extension): // // > For the first post-fork block, both parent.blob_gas_used and parent.excess_blob_gas @@ -836,15 +836,15 @@ impl SealedHeader { let parent_excess_blob_gas = parent.excess_blob_gas.unwrap_or(0); if self.blob_gas_used.is_none() { - return Err(SealedHeaderError::BlobGasUsedMissing) + return Err(HeaderValidationError::BlobGasUsedMissing) } let excess_blob_gas = - self.excess_blob_gas.ok_or(SealedHeaderError::ExcessBlobGasMissing)?; + self.excess_blob_gas.ok_or(HeaderValidationError::ExcessBlobGasMissing)?; let expected_excess_blob_gas = calculate_excess_blob_gas(parent_excess_blob_gas, parent_blob_gas_used); if expected_excess_blob_gas != excess_blob_gas { - return Err(SealedHeaderError::ExcessBlobGasDiff { + return Err(HeaderValidationError::ExcessBlobGasDiff { diff: GotExpected { got: excess_blob_gas, expected: expected_excess_blob_gas }, parent_excess_blob_gas, parent_blob_gas_used, @@ -1077,7 +1077,7 @@ mod tests { use super::{Bytes, Decodable, Encodable, Header, B256}; use crate::{ address, b256, bloom, bytes, header::MINIMUM_GAS_LIMIT, hex, Address, ChainSpec, - HeadersDirection, SealedHeader, SealedHeaderError, U256, + HeaderValidationError, HeadersDirection, SealedHeader, U256, }; use std::str::FromStr; @@ -1368,7 +1368,7 @@ mod tests { assert_eq!( child.validate_gas_limit(&parent, &chain_spec), - Err(SealedHeaderError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }) + Err(HeaderValidationError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }) ); } @@ -1390,7 +1390,7 @@ mod tests { assert_eq!( child.validate_gas_limit(&parent, &chain_spec), - Err(SealedHeaderError::GasLimitInvalidIncrease { + Err(HeaderValidationError::GasLimitInvalidIncrease { parent_gas_limit: parent.header.gas_limit, child_gas_limit: child.header.gas_limit, }) @@ -1431,7 +1431,7 @@ mod tests { assert_eq!( child.validate_gas_limit(&parent, &chain_spec), - Err(SealedHeaderError::GasLimitInvalidDecrease { + Err(HeaderValidationError::GasLimitInvalidDecrease { parent_gas_limit: parent.header.gas_limit, child_gas_limit: child.header.gas_limit, }) diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index eb28a074dc2e..9b0476a828c6 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -60,7 +60,7 @@ pub use constants::{ }; pub use error::{GotExpected, GotExpectedBoxed}; pub use genesis::{ChainConfig, Genesis, GenesisAccount}; -pub use header::{Header, HeadersDirection, SealedHeader, SealedHeaderError}; +pub use header::{Header, HeaderValidationError, HeadersDirection, SealedHeader}; pub use integer_list::IntegerList; pub use log::{logs_bloom, Log}; pub use net::{