From 7457f476d0c20801e15634694f5204e8c3b4b3cc Mon Sep 17 00:00:00 2001 From: Thomas Coratger Date: Mon, 22 Jan 2024 15:55:53 +0100 Subject: [PATCH 1/2] add minimum check in check_gas_limit with unit test --- crates/consensus/common/src/validation.rs | 46 ++++++++++++++++++----- crates/interfaces/src/consensus.rs | 13 ++++++- crates/primitives/src/constants/mod.rs | 3 ++ 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index 6ee52bad4c51..35c4d0005c28 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -5,6 +5,7 @@ use reth_primitives::{ constants::{ self, eip4844::{DATA_GAS_PER_BLOB, MAX_DATA_GAS_PER_BLOCK}, + MINIMUM_GAS_LIMIT, }, eip4844::calculate_excess_blob_gas, BlockNumber, ChainSpec, GotExpected, Hardfork, Header, InvalidTransactionError, SealedBlock, @@ -257,36 +258,45 @@ pub fn validate_block_standalone( Ok(()) } -// Check gas limit, max diff between child/parent gas_limit should be max_diff=parent_gas/1024 -// On Optimism, the gas limit can adjust instantly, so we skip this check if the optimism -// flag is enabled in the chain spec. +/// 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; - - // By consensus, gas_limit is multiplied by elasticity (*2) on - // on exact block that hardfork happens. 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, - }) + }); } - } else if parent_gas_limit - child.gas_limit >= parent_gas_limit / 1024 { + } + // 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 allowed limit. + else if child.gas_limit < MINIMUM_GAS_LIMIT { + return Err(ConsensusError::GasLimitInvalidMinimum { child_gas_limit: child.gas_limit }); } Ok(()) @@ -880,6 +890,24 @@ mod tests { 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; diff --git a/crates/interfaces/src/consensus.rs b/crates/interfaces/src/consensus.rs index 0adc63a50d9c..846e70e24e2b 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, B256, U256, + constants::MINIMUM_GAS_LIMIT, BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, + InvalidTransactionError, SealedBlock, SealedHeader, B256, U256, }; use std::fmt::Debug; @@ -170,6 +170,15 @@ pub enum ConsensusError { 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, diff --git a/crates/primitives/src/constants/mod.rs b/crates/primitives/src/constants/mod.rs index 59e14602bcc4..7574321fc513 100644 --- a/crates/primitives/src/constants/mod.rs +++ b/crates/primitives/src/constants/mod.rs @@ -61,6 +61,9 @@ pub const EIP1559_DEFAULT_BASE_FEE_MAX_CHANGE_DENOMINATOR: u64 = 8; /// Elasticity multiplier as defined in [EIP-1559](https://eips.ethereum.org/EIPS/eip-1559) pub const EIP1559_DEFAULT_ELASTICITY_MULTIPLIER: u64 = 2; +/// Minimum gas limit allowed for transactions. +pub const MINIMUM_GAS_LIMIT: u64 = 5000; + /// Base fee max change denominator for Optimism Mainnet as defined in the Optimism /// [transaction costs](https://community.optimism.io/docs/developers/build/differences/#transaction-costs) doc. #[cfg(feature = "optimism")] From 83917b4ae5b69c1567ccdff483957eb685707d5d Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Mon, 22 Jan 2024 17:00:00 +0100 Subject: [PATCH 2/2] Update crates/consensus/common/src/validation.rs Co-authored-by: Matthias Seitz --- crates/consensus/common/src/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index 35c4d0005c28..560d1fb71dd5 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -294,7 +294,7 @@ fn check_gas_limit( child_gas_limit: child.gas_limit, }); } - // Check if the child gas limit is below the minimum allowed 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 }); }