Skip to content
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.

Sign up for GitHub

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

Encapsulate validate_header_regarding_parent inside SealedHeader #6404

Merged
merged 5 commits into from
Feb 6, 2024
+384 −322
Merged

Encapsulate validate_header_regarding_parent inside SealedHeader #6404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa8ca14
Encapsulate validate_header_regarding_parent inside SealedHeader
tcoratger Feb 5, 2024
f25fb8b
fix
tcoratger Feb 5, 2024
5f47626
Merge branch 'main' into validate_regarding_parent
tcoratger Feb 5, 2024
be3128b
fix namings
tcoratger Feb 6, 2024
581f526
Merge branch 'validate_regarding_parent' of https://github.com/tcorat…
tcoratger Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/consensus/beacon-core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_against_parent(parent, &self.chain_spec).map_err(ConsensusError::from)?;
Ok(())
}

Expand Down
257 changes: 1 addition & 256 deletions crates/consensus/common/src/validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -397,41 +276,6 @@ pub fn validate_block_regarding_chain<PROV: HeaderProvider + WithdrawalsProvider
Ok(parent.seal(block.parent_hash))
}

/// 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_with_parent(
parent: &SealedHeader,
child: &SealedHeader,
) -> 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
Expand Down Expand Up @@ -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,
})
);
}
}
65 changes: 5 additions & 60 deletions crates/interfaces/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use reth_primitives::{
constants::MINIMUM_GAS_LIMIT, BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header,
BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, HeaderValidationError,
InvalidTransactionError, SealedBlock, SealedHeader, B256, U256,
};
use std::fmt::Debug;
Expand Down Expand Up @@ -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<B256>),

/// 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 {
Expand All @@ -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<u64>),

/// Error when there is a transaction signer recovery error.
#[error("transaction signer recovery error")]
TransactionSignerRecoveryError,
Expand Down Expand Up @@ -270,22 +226,11 @@ pub enum ConsensusError {
#[error("blob gas used mismatch: {0}")]
BlobGasUsedDiff(GotExpected<u64>),

/// 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<u64>,
/// 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 HeaderValidationError.
#[error(transparent)]
HeaderValidationError(#[from] HeaderValidationError),
}
Loading
Loading