From 95965ae6f105e8b23a629f6a60df66c5d048b1f9 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:11:34 +0200 Subject: [PATCH 1/4] move fill_block_env to ConfigureEvmEnv --- crates/evm/src/lib.rs | 47 ++++++++++++++++++++- crates/primitives/src/revm/env.rs | 40 +----------------- crates/rpc/rpc-eth-api/src/helpers/state.rs | 15 ++++--- 3 files changed, 56 insertions(+), 46 deletions(-) diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index b57969de8e61..7f3f7d08451c 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -16,7 +16,7 @@ use core::ops::Deref; use reth_chainspec::ChainSpec; use reth_primitives::{ - revm::env::fill_block_env, Address, Header, TransactionSigned, TransactionSignedEcRecovered, + revm::env::block_coinbase, Address, Header, TransactionSigned, TransactionSignedEcRecovered, U256, }; use revm::{inspector_handle_register, Database, Evm, EvmBuilder, GetInspector}; @@ -126,6 +126,44 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { total_difficulty: U256, ); + /// Fill [`BlockEnv`] field according to the chain spec and given header + fn fill_block_env( + &self, + block_env: &mut BlockEnv, + chain_spec: &ChainSpec, + header: &Header, + after_merge: bool, + ) { + let coinbase = block_coinbase(chain_spec, header, after_merge); + Self::fill_block_env_with_coinbase(block_env, header, after_merge, coinbase); + } + + /// Fill block environment with coinbase. + fn fill_block_env_with_coinbase( + block_env: &mut BlockEnv, + header: &Header, + after_merge: bool, + coinbase: Address, + ) { + block_env.number = U256::from(header.number); + block_env.coinbase = coinbase; + block_env.timestamp = U256::from(header.timestamp); + if after_merge { + block_env.prevrandao = Some(header.mix_hash); + block_env.difficulty = U256::ZERO; + } else { + block_env.difficulty = header.difficulty; + block_env.prevrandao = None; + } + block_env.basefee = U256::from(header.base_fee_per_gas.unwrap_or_default()); + block_env.gas_limit = U256::from(header.gas_limit); + + // EIP-4844 excess blob gas of this block, introduced in Cancun + if let Some(excess_blob_gas) = header.excess_blob_gas { + block_env.set_blob_excess_gas_and_price(excess_blob_gas); + } + } + /// Convenience function to call both [`fill_cfg_env`](ConfigureEvmEnv::fill_cfg_env) and /// [`fill_block_env`]. fn fill_cfg_and_block_env( @@ -137,6 +175,11 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { ) { Self::fill_cfg_env(cfg, chain_spec, header, total_difficulty); let after_merge = cfg.handler_cfg.spec_id >= SpecId::MERGE; - fill_block_env(block_env, chain_spec, header, after_merge); + Self::fill_block_env_with_coinbase( + block_env, + header, + after_merge, + block_coinbase(chain_spec, header, after_merge), + ); } } diff --git a/crates/primitives/src/revm/env.rs b/crates/primitives/src/revm/env.rs index 6cc81128c2a0..b60a493fd1b2 100644 --- a/crates/primitives/src/revm/env.rs +++ b/crates/primitives/src/revm/env.rs @@ -1,6 +1,6 @@ use crate::{ recover_signer_unchecked, - revm_primitives::{BlockEnv, Env, TxEnv}, + revm_primitives::{Env, TxEnv}, Address, Bytes, Header, TxKind, B256, U256, }; use reth_chainspec::{Chain, ChainSpec}; @@ -12,44 +12,6 @@ use revm_primitives::OptimismFields; #[cfg(not(feature = "std"))] use alloc::vec::Vec; -/// Fill block environment from Block. -pub fn fill_block_env( - block_env: &mut BlockEnv, - chain_spec: &ChainSpec, - header: &Header, - after_merge: bool, -) { - let coinbase = block_coinbase(chain_spec, header, after_merge); - fill_block_env_with_coinbase(block_env, header, after_merge, coinbase); -} - -/// Fill block environment with coinbase. -#[inline] -pub fn fill_block_env_with_coinbase( - block_env: &mut BlockEnv, - header: &Header, - after_merge: bool, - coinbase: Address, -) { - block_env.number = U256::from(header.number); - block_env.coinbase = coinbase; - block_env.timestamp = U256::from(header.timestamp); - if after_merge { - block_env.prevrandao = Some(header.mix_hash); - block_env.difficulty = U256::ZERO; - } else { - block_env.difficulty = header.difficulty; - block_env.prevrandao = None; - } - block_env.basefee = U256::from(header.base_fee_per_gas.unwrap_or_default()); - block_env.gas_limit = U256::from(header.gas_limit); - - // EIP-4844 excess blob gas of this block, introduced in Cancun - if let Some(excess_blob_gas) = header.excess_blob_gas { - block_env.set_blob_excess_gas_and_price(excess_blob_gas); - } -} - /// Return the coinbase address for the given header and chain spec. pub fn block_coinbase(chain_spec: &ChainSpec, header: &Header, after_merge: bool) -> Address { // Clique consensus fills the EXTRA_SEAL (last 65 bytes) of the extra data with the diff --git a/crates/rpc/rpc-eth-api/src/helpers/state.rs b/crates/rpc/rpc-eth-api/src/helpers/state.rs index 80859b929b7b..a17447249606 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/state.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/state.rs @@ -2,11 +2,11 @@ //! RPC methods. use futures::Future; -use reth_primitives::{ - revm::env::fill_block_env_with_coinbase, Address, BlockId, BlockNumberOrTag, Bytes, Header, - B256, U256, +use reth_evm::ConfigureEvmEnv; +use reth_primitives::{Address, BlockId, BlockNumberOrTag, Bytes, Header, B256, U256}; +use reth_provider::{ + BlockIdReader, ChainSpecProvider, StateProvider, StateProviderBox, StateProviderFactory, }; -use reth_provider::{BlockIdReader, StateProvider, StateProviderBox, StateProviderFactory}; use reth_rpc_eth_types::{ EthApiError, EthResult, EthStateCache, PendingBlockEnv, RpcInvalidTransactionError, }; @@ -209,7 +209,12 @@ pub trait LoadState { let (cfg, mut block_env, _) = self.evm_env_at(header.parent_hash.into()).await?; let after_merge = cfg.handler_cfg.spec_id >= SpecId::MERGE; - fill_block_env_with_coinbase(&mut block_env, header, after_merge, header.beneficiary); + self.evm_config().fill_block_env( + &mut block_env, + &LoadPendingBlock::provider(self).chain_spec(), + &header, + after_merge, + ); Ok((cfg, block_env)) } From f537f6d32767a4d9aa015bf4966907f814d30dab Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:14:35 +0200 Subject: [PATCH 2/4] move block_coinbase --- crates/evm/src/lib.rs | 2 +- crates/primitives/src/header.rs | 69 +++++++++++++++++++++++++++++ crates/primitives/src/revm/env.rs | 72 ------------------------------- 3 files changed, 70 insertions(+), 73 deletions(-) diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index 7f3f7d08451c..fa9a9ec5c139 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -16,7 +16,7 @@ use core::ops::Deref; use reth_chainspec::ChainSpec; use reth_primitives::{ - revm::env::block_coinbase, Address, Header, TransactionSigned, TransactionSignedEcRecovered, + header::block_coinbase, Address, Header, TransactionSigned, TransactionSignedEcRecovered, U256, }; use revm::{inspector_handle_register, Database, Evm, EvmBuilder, GetInspector}; diff --git a/crates/primitives/src/header.rs b/crates/primitives/src/header.rs index ea80328e7528..b3d9095ff07b 100644 --- a/crates/primitives/src/header.rs +++ b/crates/primitives/src/header.rs @@ -1,12 +1,71 @@ //! Header types. +use crate::{recover_signer_unchecked, Address, Bytes}; use alloy_rlp::{Decodable, Encodable}; use bytes::BufMut; +use reth_chainspec::{Chain, ChainSpec}; use reth_codecs::derive_arbitrary; use serde::{Deserialize, Serialize}; pub use reth_primitives_traits::{Header, HeaderError, SealedHeader}; +/// Return the coinbase address for the given header and chain spec. +pub fn block_coinbase(chain_spec: &ChainSpec, header: &Header, after_merge: bool) -> Address { + // Clique consensus fills the EXTRA_SEAL (last 65 bytes) of the extra data with the + // signer's signature. + // + // On the genesis block, the extra data is filled with zeros, so we should not attempt to + // recover the signer on the genesis block. + // + // From EIP-225: + // + // * `EXTRA_SEAL`: Fixed number of extra-data suffix bytes reserved for signer seal. + // * 65 bytes fixed as signatures are based on the standard `secp256k1` curve. + // * Filled with zeros on genesis block. + if chain_spec.chain == Chain::goerli() && !after_merge && header.number > 0 { + recover_header_signer(header).unwrap_or_else(|err| { + panic!( + "Failed to recover goerli Clique Consensus signer from header ({}, {}) using extradata {}: {:?}", + header.number, header.hash_slow(), header.extra_data, err + ) + }) + } else { + header.beneficiary + } +} + +/// Error type for recovering Clique signer from a header. +#[derive(Debug, thiserror_no_std::Error)] +pub enum CliqueSignerRecoveryError { + /// Header extradata is too short. + #[error("Invalid extra data length")] + InvalidExtraData, + /// Recovery failed. + #[error("Invalid signature: {0}")] + InvalidSignature(#[from] secp256k1::Error), +} + +/// Recover the account from signed header per clique consensus rules. +pub fn recover_header_signer(header: &Header) -> Result { + let extra_data_len = header.extra_data.len(); + // Fixed number of extra-data suffix bytes reserved for signer signature. + // 65 bytes fixed as signatures are based on the standard secp256k1 curve. + // Filled with zeros on genesis block. + let signature_start_byte = extra_data_len - 65; + let signature: [u8; 65] = header.extra_data[signature_start_byte..] + .try_into() + .map_err(|_| CliqueSignerRecoveryError::InvalidExtraData)?; + let seal_hash = { + let mut header_to_seal = header.clone(); + header_to_seal.extra_data = Bytes::from(header.extra_data[..signature_start_byte].to_vec()); + header_to_seal.hash_slow() + }; + + // TODO: this is currently unchecked recovery, does this need to be checked w.r.t EIP-2? + recover_signer_unchecked(&signature, &seal_hash.0) + .map_err(CliqueSignerRecoveryError::InvalidSignature) +} + /// Represents the direction for a headers request depending on the `reverse` field of the request. /// > The response must contain a number of block headers, of rising number when reverse is 0, /// > falling when 1 @@ -87,6 +146,7 @@ impl From for bool { #[cfg(test)] mod tests { + use super::*; use crate::{ address, b256, bloom, bytes, hex, Address, Bytes, Header, HeadersDirection, B256, U256, }; @@ -353,4 +413,13 @@ mod tests { Header::decode(&mut data.as_slice()) .expect_err("blob_gas_used size should make this header decoding fail"); } + + #[test] + fn test_recover_genesis_goerli_signer() { + // just ensures that `block_coinbase` does not panic on the genesis block + let chain_spec = reth_chainspec::GOERLI.clone(); + let header = chain_spec.genesis_header(); + let block_coinbase = block_coinbase(&chain_spec, &header, false); + assert_eq!(block_coinbase, header.beneficiary); + } } diff --git a/crates/primitives/src/revm/env.rs b/crates/primitives/src/revm/env.rs index b60a493fd1b2..bd9840df5206 100644 --- a/crates/primitives/src/revm/env.rs +++ b/crates/primitives/src/revm/env.rs @@ -12,63 +12,6 @@ use revm_primitives::OptimismFields; #[cfg(not(feature = "std"))] use alloc::vec::Vec; -/// Return the coinbase address for the given header and chain spec. -pub fn block_coinbase(chain_spec: &ChainSpec, header: &Header, after_merge: bool) -> Address { - // Clique consensus fills the EXTRA_SEAL (last 65 bytes) of the extra data with the - // signer's signature. - // - // On the genesis block, the extra data is filled with zeros, so we should not attempt to - // recover the signer on the genesis block. - // - // From EIP-225: - // - // * `EXTRA_SEAL`: Fixed number of extra-data suffix bytes reserved for signer seal. - // * 65 bytes fixed as signatures are based on the standard `secp256k1` curve. - // * Filled with zeros on genesis block. - if chain_spec.chain == Chain::goerli() && !after_merge && header.number > 0 { - recover_header_signer(header).unwrap_or_else(|err| { - panic!( - "Failed to recover goerli Clique Consensus signer from header ({}, {}) using extradata {}: {:?}", - header.number, header.hash_slow(), header.extra_data, err - ) - }) - } else { - header.beneficiary - } -} - -/// Error type for recovering Clique signer from a header. -#[derive(Debug, thiserror_no_std::Error)] -pub enum CliqueSignerRecoveryError { - /// Header extradata is too short. - #[error("Invalid extra data length")] - InvalidExtraData, - /// Recovery failed. - #[error("Invalid signature: {0}")] - InvalidSignature(#[from] secp256k1::Error), -} - -/// Recover the account from signed header per clique consensus rules. -pub fn recover_header_signer(header: &Header) -> Result { - let extra_data_len = header.extra_data.len(); - // Fixed number of extra-data suffix bytes reserved for signer signature. - // 65 bytes fixed as signatures are based on the standard secp256k1 curve. - // Filled with zeros on genesis block. - let signature_start_byte = extra_data_len - 65; - let signature: [u8; 65] = header.extra_data[signature_start_byte..] - .try_into() - .map_err(|_| CliqueSignerRecoveryError::InvalidExtraData)?; - let seal_hash = { - let mut header_to_seal = header.clone(); - header_to_seal.extra_data = Bytes::from(header.extra_data[..signature_start_byte].to_vec()); - header_to_seal.hash_slow() - }; - - // TODO: this is currently unchecked recovery, does this need to be checked w.r.t EIP-2? - recover_signer_unchecked(&signature, &seal_hash.0) - .map_err(CliqueSignerRecoveryError::InvalidSignature) -} - /// Fill transaction environment with the EIP-4788 system contract message data. /// /// This requirements for the beacon root contract call defined by @@ -157,18 +100,3 @@ fn fill_tx_env_with_system_contract_call( // disable the base fee check for this call by setting the base fee to zero env.block.basefee = U256::ZERO; } - -#[cfg(test)] -mod tests { - use super::*; - use reth_chainspec::GOERLI; - - #[test] - fn test_recover_genesis_goerli_signer() { - // just ensures that `block_coinbase` does not panic on the genesis block - let chain_spec = GOERLI.clone(); - let header = chain_spec.genesis_header(); - let block_coinbase = block_coinbase(&chain_spec, &header, false); - assert_eq!(block_coinbase, header.beneficiary); - } -} From a3441cd91dbf05ee0a619ba66997af9d5e918723 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:24:04 +0200 Subject: [PATCH 3/4] clippy --- crates/evm/src/lib.rs | 3 +-- crates/primitives/src/revm/env.rs | 4 +--- crates/rpc/rpc-eth-api/src/helpers/state.rs | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index fa9a9ec5c139..9eb2a5f3c7d4 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -16,8 +16,7 @@ use core::ops::Deref; use reth_chainspec::ChainSpec; use reth_primitives::{ - header::block_coinbase, Address, Header, TransactionSigned, TransactionSignedEcRecovered, - U256, + header::block_coinbase, Address, Header, TransactionSigned, TransactionSignedEcRecovered, U256, }; use revm::{inspector_handle_register, Database, Evm, EvmBuilder, GetInspector}; use revm_primitives::{BlockEnv, CfgEnvWithHandlerCfg, EnvWithHandlerCfg, SpecId, TxEnv}; diff --git a/crates/primitives/src/revm/env.rs b/crates/primitives/src/revm/env.rs index bd9840df5206..5c7ae7b0d5de 100644 --- a/crates/primitives/src/revm/env.rs +++ b/crates/primitives/src/revm/env.rs @@ -1,9 +1,7 @@ use crate::{ - recover_signer_unchecked, revm_primitives::{Env, TxEnv}, - Address, Bytes, Header, TxKind, B256, U256, + Address, Bytes, TxKind, B256, U256, }; -use reth_chainspec::{Chain, ChainSpec}; use alloy_eips::{eip4788::BEACON_ROOTS_ADDRESS, eip7002::WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS}; #[cfg(feature = "optimism")] diff --git a/crates/rpc/rpc-eth-api/src/helpers/state.rs b/crates/rpc/rpc-eth-api/src/helpers/state.rs index a17447249606..34424e94fcdf 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/state.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/state.rs @@ -212,7 +212,7 @@ pub trait LoadState { self.evm_config().fill_block_env( &mut block_env, &LoadPendingBlock::provider(self).chain_spec(), - &header, + header, after_merge, ); From e3c0ae27b4ff2030e59f3cdaaeb82a9425b6e731 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Tue, 2 Jul 2024 12:19:11 +0200 Subject: [PATCH 4/4] fix cargo docs --- crates/evm/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index 9eb2a5f3c7d4..e5e9dbdf7122 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -164,7 +164,7 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { } /// Convenience function to call both [`fill_cfg_env`](ConfigureEvmEnv::fill_cfg_env) and - /// [`fill_block_env`]. + /// [`ConfigureEvmEnv::fill_block_env`]. fn fill_cfg_and_block_env( cfg: &mut CfgEnvWithHandlerCfg, block_env: &mut BlockEnv,