Skip to content

Commit

Permalink
refactor(consensus, evm): move post-execution validation to consensus (
Browse files Browse the repository at this point in the history
  • Loading branch information
shekhirin authored May 22, 2024
1 parent 9071330 commit f45ca74
Show file tree
Hide file tree
Showing 52 changed files with 424 additions and 346 deletions.
4 changes: 3 additions & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion bin/reth/src/commands/debug_cmd/build_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl Command {

consensus.validate_header_with_total_difficulty(block, U256::MAX)?;
consensus.validate_header(block)?;
consensus.validate_block(block)?;
consensus.validate_block_pre_execution(block)?;

let senders = block.senders().expect("sender recovery failed");
let block_with_senders =
Expand Down
2 changes: 1 addition & 1 deletion bin/reth/src/commands/debug_cmd/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl Command {
)),
PruneModes::none(),
);
executor.execute_one((&sealed_block.clone().unseal(), td).into())?;
executor.execute_and_verify_one((&sealed_block.clone().unseal(), td).into())?;
let BatchBlockExecutionOutput { bundle, receipts, first_block } = executor.finalize();
BundleStateWithReceipts::new(bundle, receipts, first_block).write_to_storage(
provider_rw.tx_ref(),
Expand Down
2 changes: 1 addition & 1 deletion crates/blockchain-tree/src/blockchain_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ where
return Err(e)
}

if let Err(e) = self.externals.consensus.validate_block(block) {
if let Err(e) = self.externals.consensus.validate_block_pre_execution(block) {
error!(?block, "Failed to validate block {}: {e}", block.header.hash());
return Err(e)
}
Expand Down
3 changes: 3 additions & 0 deletions crates/blockchain-tree/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,11 @@ impl AppendableChain {
let executor = externals.executor_factory.executor(db);
let block_hash = block.hash();
let block = block.unseal();

let state = executor.execute((&block, U256::MAX).into())?;
let BlockExecutionOutput { state, receipts, .. } = state;
externals.consensus.validate_block_post_execution(&block, &receipts)?;

let bundle_state = BundleStateWithReceipts::new(
state,
Receipts::from_block_receipt(receipts),
Expand Down
2 changes: 1 addition & 1 deletion crates/config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Config {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!("reth config file extension must be '{EXTENSION}'"),
));
))
}
confy::store_path(path, self).map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))
}
Expand Down
39 changes: 14 additions & 25 deletions crates/consensus/auto-seal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ use reth_interfaces::executor::{BlockExecutionError, BlockValidationError};
use reth_primitives::{
constants::{EMPTY_TRANSACTIONS, ETHEREUM_BLOCK_GAS_LIMIT},
eip4844::calculate_excess_blob_gas,
proofs, Block, BlockBody, BlockHash, BlockHashOrNumber, BlockNumber, ChainSpec, Header,
Receipts, SealedBlock, SealedHeader, TransactionSigned, Withdrawals, B256, U256,
proofs, Block, BlockBody, BlockHash, BlockHashOrNumber, BlockNumber, BlockWithSenders,
ChainSpec, Header, Receipt, Receipts, SealedBlock, SealedHeader, TransactionSigned,
Withdrawals, B256, U256,
};
use reth_provider::{
BlockReaderIdExt, BundleStateWithReceipts, CanonStateNotificationSender, StateProviderFactory,
Expand Down Expand Up @@ -84,7 +85,15 @@ impl Consensus for AutoSealConsensus {
Ok(())
}

fn validate_block(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
fn validate_block_pre_execution(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
Ok(())
}

fn validate_block_post_execution(
&self,
_block: &BlockWithSenders,
_receipts: &[Receipt],
) -> Result<(), ConsensusError> {
Ok(())
}
}
Expand Down Expand Up @@ -361,7 +370,7 @@ impl StorageInner {
let header =
self.build_header_template(&transactions, &ommers, withdrawals.as_ref(), chain_spec);

let mut block = Block {
let block = Block {
header,
body: transactions,
ommers: ommers.clone(),
Expand All @@ -376,27 +385,7 @@ impl StorageInner {
provider.latest().map_err(BlockExecutionError::LatestBlock)?,
);

// TODO(mattsse): At this point we don't know certain fields of the header, so we first
// execute it and then update the header this can be improved by changing the executor
// input, for now we intercept the errors and retry
loop {
match executor.executor(&mut db).execute((&block, U256::ZERO).into()) {
Err(BlockExecutionError::Validation(BlockValidationError::BlockGasUsed {
gas,
..
})) => {
block.block.header.gas_used = gas.got;
}
Err(BlockExecutionError::Validation(BlockValidationError::ReceiptRootDiff(
err,
))) => {
block.block.header.receipts_root = err.got;
}
_ => break,
};
}

// now execute the block
// execute the block
let BlockExecutionOutput { state, receipts, .. } =
executor.executor(&mut db).execute((&block, U256::ZERO).into())?;
let bundle_state = BundleStateWithReceipts::new(
Expand Down
10 changes: 5 additions & 5 deletions crates/consensus/common/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn validate_header_standalone(
/// - Compares the transactions root in the block header to the block body
/// - Pre-execution transaction validation
/// - (Optionally) Compares the receipts root in the block header to the block body
pub fn validate_block_standalone(
pub fn validate_block_pre_execution(
block: &SealedBlock,
chain_spec: &ChainSpec,
) -> Result<(), ConsensusError> {
Expand Down Expand Up @@ -366,13 +366,13 @@ mod tests {

// Single withdrawal
let block = create_block_with_withdrawals(&[1]);
assert_eq!(validate_block_standalone(&block, &chain_spec), Ok(()));
assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(()));

// Multiple increasing withdrawals
let block = create_block_with_withdrawals(&[1, 2, 3]);
assert_eq!(validate_block_standalone(&block, &chain_spec), Ok(()));
assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(()));
let block = create_block_with_withdrawals(&[5, 6, 7, 8, 9]);
assert_eq!(validate_block_standalone(&block, &chain_spec), Ok(()));
assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(()));
let (_, parent) = mock_block();

// Withdrawal index should be the last withdrawal index + 1
Expand Down Expand Up @@ -428,7 +428,7 @@ mod tests {

// validate blob, it should fail blob gas used validation
assert_eq!(
validate_block_standalone(&block, &chain_spec),
validate_block_pre_execution(&block, &chain_spec),
Err(ConsensusError::BlobGasUsedDiff(GotExpected {
got: 1,
expected: expected_blob_gas_used
Expand Down
2 changes: 1 addition & 1 deletion crates/consensus/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ auto_impl.workspace = true
thiserror.workspace = true

[features]
test-utils = []
test-utils = []
35 changes: 32 additions & 3 deletions crates/consensus/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

use reth_primitives::{
BlockHash, BlockNumber, GotExpected, GotExpectedBoxed, Header, HeaderValidationError,
InvalidTransactionError, SealedBlock, SealedHeader, B256, U256,
BlockHash, BlockNumber, BlockWithSenders, Bloom, GotExpected, GotExpectedBoxed, Header,
HeaderValidationError, InvalidTransactionError, Receipt, SealedBlock, SealedHeader, B256, U256,
};
use std::fmt::Debug;

Expand Down Expand Up @@ -83,7 +83,19 @@ pub trait Consensus: Debug + Send + Sync {
/// **This should not be called for the genesis block**.
///
/// Note: validating blocks does not include other validations of the Consensus
fn validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError>;
fn validate_block_pre_execution(&self, block: &SealedBlock) -> Result<(), ConsensusError>;

/// Validate a block considering world state, i.e. things that can not be checked before
/// execution.
///
/// See the Yellow Paper sections 4.3.2 "Holistic Validity".
///
/// Note: validating blocks does not include other validations of the Consensus
fn validate_block_post_execution(
&self,
block: &BlockWithSenders,
receipts: &[Receipt],
) -> Result<(), ConsensusError>;
}

/// Consensus Errors
Expand All @@ -98,6 +110,15 @@ pub enum ConsensusError {
gas_limit: u64,
},

/// Error when block gas used doesn't match expected value
#[error("block gas used mismatch: {gas}; gas spent by each transaction: {gas_spent_by_tx:?}")]
BlockGasUsed {
/// The gas diff.
gas: GotExpected<u64>,
/// Gas spent by each transaction
gas_spent_by_tx: Vec<(u64, u64)>,
},

/// Error when the hash of block ommer is different from the expected hash.
#[error("mismatched block ommer hash: {0}")]
BodyOmmersHashDiff(GotExpectedBoxed<B256>),
Expand All @@ -111,6 +132,14 @@ pub enum ConsensusError {
#[error("mismatched block transaction root: {0}")]
BodyTransactionRootDiff(GotExpectedBoxed<B256>),

/// Error when the receipt root in the block is different from the expected receipt root.
#[error("receipt root mismatch: {0}")]
BodyReceiptRootDiff(GotExpectedBoxed<B256>),

/// Error when header bloom filter is different from the expected bloom filter.
#[error("header bloom filter mismatch: {0}")]
BodyBloomLogDiff(GotExpectedBoxed<Bloom>),

/// Error when the withdrawals root in the block is different from the expected withdrawals
/// root.
#[error("mismatched block withdrawals root: {0}")]
Expand Down
16 changes: 14 additions & 2 deletions crates/consensus/consensus/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{Consensus, ConsensusError};
use reth_primitives::{Header, SealedBlock, SealedHeader, U256};
use reth_primitives::{BlockWithSenders, Header, Receipt, SealedBlock, SealedHeader, U256};
use std::sync::atomic::{AtomicBool, Ordering};

/// Consensus engine implementation for testing
Expand Down Expand Up @@ -60,7 +60,19 @@ impl Consensus for TestConsensus {
}
}

fn validate_block(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
fn validate_block_pre_execution(&self, _block: &SealedBlock) -> Result<(), ConsensusError> {
if self.fail_validation() {
Err(ConsensusError::BaseFeeMissing)
} else {
Ok(())
}
}

fn validate_block_post_execution(
&self,
_block: &BlockWithSenders,
_receipts: &[Receipt],
) -> Result<(), ConsensusError> {
if self.fail_validation() {
Err(ConsensusError::BaseFeeMissing)
} else {
Expand Down
4 changes: 2 additions & 2 deletions crates/e2e-test-utils/src/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ impl<E: EngineTypes + 'static> PayloadTestContext<E> {
let payload = self.payload_builder.best_payload(payload_id).await.unwrap().unwrap();
if payload.block().body.is_empty() {
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
continue;
continue
}
break;
break
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ethereum/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ reth-primitives.workspace = true
reth-consensus.workspace = true

[features]
optimism = ["reth-primitives/optimism"]
optimism = ["reth-primitives/optimism"]
28 changes: 21 additions & 7 deletions crates/ethereum/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,18 @@
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

use reth_consensus::{Consensus, ConsensusError};
use reth_consensus_common::validation;
use reth_consensus_common::validation::{
validate_block_pre_execution, validate_header_extradata, validate_header_standalone,
};
use reth_primitives::{
Chain, ChainSpec, Hardfork, Header, SealedBlock, SealedHeader, EMPTY_OMMER_ROOT_HASH, U256,
BlockWithSenders, Chain, ChainSpec, Hardfork, Header, Receipt, SealedBlock, SealedHeader,
EMPTY_OMMER_ROOT_HASH, U256,
};
use std::{sync::Arc, time::SystemTime};

mod validation;
pub use validation::validate_block_post_execution;

/// Ethereum beacon consensus
///
/// This consensus engine does basic checks as outlined in the execution specs.
Expand All @@ -33,7 +39,7 @@ impl EthBeaconConsensus {

impl Consensus for EthBeaconConsensus {
fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError> {
validation::validate_header_standalone(header, &self.chain_spec)?;
validate_header_standalone(header, &self.chain_spec)?;
Ok(())
}

Expand Down Expand Up @@ -87,7 +93,7 @@ impl Consensus for EthBeaconConsensus {
// is greater than its parent timestamp.

// validate header extradata for all networks post merge
validation::validate_header_extradata(header)?;
validate_header_extradata(header)?;

// mixHash is used instead of difficulty inside EVM
// https://eips.ethereum.org/EIPS/eip-4399#using-mixhash-field-instead-of-difficulty
Expand All @@ -111,14 +117,22 @@ impl Consensus for EthBeaconConsensus {
// * If the network is goerli pre-merge, ignore the extradata check, since we do not
// support clique. Same goes for OP blocks below Bedrock.
if self.chain_spec.chain != Chain::goerli() && !self.chain_spec.is_optimism() {
validation::validate_header_extradata(header)?;
validate_header_extradata(header)?;
}
}

Ok(())
}

fn validate_block(&self, block: &SealedBlock) -> Result<(), ConsensusError> {
validation::validate_block_standalone(block, &self.chain_spec)
fn validate_block_pre_execution(&self, block: &SealedBlock) -> Result<(), ConsensusError> {
validate_block_pre_execution(block, &self.chain_spec)
}

fn validate_block_post_execution(
&self,
block: &BlockWithSenders,
receipts: &[Receipt],
) -> Result<(), ConsensusError> {
validate_block_post_execution(block, &self.chain_spec, receipts)
}
}
Loading

0 comments on commit f45ca74

Please sign in to comment.