From cea86d9e6c38ac66fe72779884b6e52db296f7b2 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Fri, 18 Oct 2024 20:05:48 -0400 Subject: [PATCH 01/19] eip1559 params in extradata --- .../ethereum/engine-primitives/src/payload.rs | 14 ++ crates/optimism/chainspec/src/lib.rs | 144 +++++++++++++++++- crates/optimism/evm/src/config.rs | 8 +- crates/optimism/evm/src/lib.rs | 10 +- crates/optimism/hardforks/src/hardfork.rs | 4 + crates/optimism/node/src/engine.rs | 141 ++++++++++++++++- crates/optimism/node/tests/e2e/utils.rs | 1 + crates/optimism/payload/src/builder.rs | 87 ++++++++++- crates/optimism/payload/src/payload.rs | 11 +- 9 files changed, 400 insertions(+), 20 deletions(-) diff --git a/crates/ethereum/engine-primitives/src/payload.rs b/crates/ethereum/engine-primitives/src/payload.rs index ae370fdb9d7b..344727128d21 100644 --- a/crates/ethereum/engine-primitives/src/payload.rs +++ b/crates/ethereum/engine-primitives/src/payload.rs @@ -272,6 +272,20 @@ pub(crate) fn payload_id(parent: &B256, attributes: &PayloadAttributes) -> Paylo PayloadId::new(out.as_slice()[..8].try_into().expect("sufficient length")) } +impl Default for EthPayloadBuilderAttributes { + fn default() -> Self { + Self { + id: PayloadId::new([0; 8]), + parent: B256::default(), + timestamp: 0, + suggested_fee_recipient: Address::ZERO, + prev_randao: B256::default(), + withdrawals: Withdrawals::default(), + parent_beacon_block_root: None, + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 98c6589d1ceb..603d29a0af3d 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -20,7 +20,7 @@ mod op_sepolia; use alloc::{vec, vec::Vec}; use alloy_chains::Chain; use alloy_genesis::Genesis; -use alloy_primitives::{Parity, Signature, B256, U256}; +use alloy_primitives::{Bytes, Parity, Signature, B256, U256}; pub use base::BASE_MAINNET; pub use base_sepolia::BASE_SEPOLIA; use core::fmt::Display; @@ -160,6 +160,16 @@ impl OpChainSpecBuilder { self } + /// Enable Holocene at genesis + pub fn holocene_activated(mut self) -> Self { + self = self.granite_activated(); + self.inner = self.inner.with_fork( + reth_optimism_forks::OptimismHardfork::Holocene, + ForkCondition::Timestamp(0), + ); + self + } + /// Build the resulting [`OpChainSpec`]. /// /// # Panics @@ -178,6 +188,51 @@ pub struct OpChainSpec { pub inner: ChainSpec, } +/// Fee trait for OP chain specs. +pub trait Fee { + /// Read from parent to determine the base fee for the next block + fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256; +} + +/// Extracts the Holcene 1599 parameters from the encoded form: +/// +pub fn decode_holocene_1559_params(extra_data: Bytes) -> (u32, u32) { + let denominator = extra_data[1..5].try_into().unwrap(); + let elasticity = extra_data[5..9].try_into().unwrap(); + (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) +} + +impl Fee for OpChainSpec { + fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256 { + let is_holocene = self.inner.is_fork_active_at_timestamp( + reth_optimism_forks::OptimismHardfork::Holocene, + timestamp, + ); + // If we are in the Holocene, we need to use the base fee params from the parent block's + // nonce Else, use the base fee params from chainspec + if is_holocene { + // First 4 bytes of the nonce are the base fee denominator, the last 4 bytes are the + // elasticity + let (elasticity, denominator) = decode_holocene_1559_params(parent.extra_data.clone()); + if elasticity == 0 && denominator == 0 { + return U256::from( + parent + .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) + .unwrap_or_default(), + ); + } + let base_fee_params = BaseFeeParams::new(denominator as u128, elasticity as u128); + U256::from(parent.next_block_base_fee(base_fee_params).unwrap_or_default()) + } else { + U256::from( + parent + .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) + .unwrap_or_default(), + ) + } + } +} + /// Returns the signature for the optimism deposit transactions, which don't include a /// signature. pub fn optimism_deposit_tx_signature() -> Signature { @@ -412,8 +467,10 @@ impl OptimismGenesisInfo { #[cfg(test)] mod tests { + use std::sync::Arc; + use alloy_genesis::{ChainConfig, Genesis}; - use alloy_primitives::b256; + use alloy_primitives::{b256, hex::FromHex}; use reth_chainspec::{test_fork_ids, BaseFeeParams, BaseFeeParamsKind}; use reth_ethereum_forks::{EthereumHardfork, ForkCondition, ForkHash, ForkId, Head}; use reth_optimism_forks::{OptimismHardfork, OptimismHardforks}; @@ -926,4 +983,87 @@ mod tests { .all(|(expected, actual)| &**expected == *actual)); assert_eq!(expected_hardforks.len(), hardforks.len()); } + + #[test] + fn test_get_base_fee_pre_holocene() { + let op_chain_spec = &BASE_SEPOLIA; + let parent = Header { + base_fee_per_gas: Some(1), + gas_used: 15763614, + gas_limit: 144000000, + ..Default::default() + }; + let base_fee = op_chain_spec.next_block_base_fee(&parent, 0); + assert_eq!( + base_fee, + U256::from( + parent + .next_block_base_fee(op_chain_spec.base_fee_params_at_timestamp(0)) + .unwrap_or_default() + ) + ); + } + + fn holocene_chainspec() -> Arc { + let mut hardforks = OptimismHardfork::base_sepolia(); + hardforks.insert(OptimismHardfork::Holocene.boxed(), ForkCondition::Timestamp(1800000000)); + Arc::new(OpChainSpec { + inner: ChainSpec { + chain: BASE_SEPOLIA.inner.chain, + genesis: BASE_SEPOLIA.inner.genesis.clone(), + genesis_hash: BASE_SEPOLIA.inner.genesis_hash.clone(), + paris_block_and_final_difficulty: Some((0, U256::from(0))), + hardforks, + base_fee_params: BASE_SEPOLIA.inner.base_fee_params.clone(), + max_gas_limit: crate::constants::BASE_SEPOLIA_MAX_GAS_LIMIT, + prune_delete_limit: 10000, + ..Default::default() + }, + }) + } + + #[test] + fn test_get_base_fee_holocene_nonce_not_set() { + let op_chain_spec = holocene_chainspec(); + let parent = Header { + base_fee_per_gas: Some(1), + gas_used: 15763614, + gas_limit: 144000000, + timestamp: 1800000003, + extra_data: Bytes::from_static(&[0, 0, 0, 0, 0, 0, 0, 0, 0]), + ..Default::default() + }; + let base_fee = op_chain_spec.next_block_base_fee(&parent, 1800000005); + assert_eq!( + base_fee, + U256::from( + parent + .next_block_base_fee(op_chain_spec.base_fee_params_at_timestamp(0)) + .unwrap_or_default() + ) + ); + } + + #[test] + fn test_get_base_fee_holocene_nonce_set() { + let op_chain_spec = holocene_chainspec(); + let parent = Header { + base_fee_per_gas: Some(1), + gas_used: 15763614, + gas_limit: 144000000, + extra_data: Bytes::from_static(&[0, 0, 0, 0, 8, 0, 0, 0, 8]), + timestamp: 1800000003, + ..Default::default() + }; + + let base_fee = op_chain_spec.next_block_base_fee(&parent, 1800000005); + assert_eq!( + base_fee, + U256::from( + parent + .next_block_base_fee(BaseFeeParams::new(0x00000008, 0x00000008)) + .unwrap_or_default() + ) + ); + } } diff --git a/crates/optimism/evm/src/config.rs b/crates/optimism/evm/src/config.rs index 668fcba4ddc4..b00341ff6776 100644 --- a/crates/optimism/evm/src/config.rs +++ b/crates/optimism/evm/src/config.rs @@ -12,7 +12,9 @@ pub fn revm_spec_by_timestamp_after_bedrock( chain_spec: &OpChainSpec, timestamp: u64, ) -> revm_primitives::SpecId { - if chain_spec.fork(OptimismHardfork::Granite).active_at_timestamp(timestamp) { + if chain_spec.fork(OptimismHardfork::Holocene).active_at_timestamp(timestamp) { + revm_primitives::HOLOCENE + } else if chain_spec.fork(OptimismHardfork::Granite).active_at_timestamp(timestamp) { revm_primitives::GRANITE } else if chain_spec.fork(OptimismHardfork::Fjord).active_at_timestamp(timestamp) { revm_primitives::FJORD @@ -29,7 +31,9 @@ pub fn revm_spec_by_timestamp_after_bedrock( /// Map the latest active hardfork at the given block to a revm [`SpecId`](revm_primitives::SpecId). pub fn revm_spec(chain_spec: &OpChainSpec, block: &Head) -> revm_primitives::SpecId { - if chain_spec.fork(OptimismHardfork::Granite).active_at_head(block) { + if chain_spec.fork(OptimismHardfork::Holocene).active_at_head(block) { + revm_primitives::HOLOCENE + } else if chain_spec.fork(OptimismHardfork::Granite).active_at_head(block) { revm_primitives::GRANITE } else if chain_spec.fork(OptimismHardfork::Fjord).active_at_head(block) { revm_primitives::FJORD diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 4d0f9d89ff41..cab4e8aa6167 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -11,7 +11,7 @@ use alloy_primitives::{Address, U256}; use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; -use reth_optimism_chainspec::OpChainSpec; +use reth_optimism_chainspec::{Fee, OpChainSpec}; use reth_primitives::{ revm_primitives::{AnalysisKind, CfgEnvWithHandlerCfg, TxEnv}, transaction::FillTxEnv, @@ -155,13 +155,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { prevrandao: Some(attributes.prev_randao), gas_limit: U256::from(parent.gas_limit), // calculate basefee based on parent block's gas usage - basefee: U256::from( - parent - .next_block_base_fee( - self.chain_spec.base_fee_params_at_timestamp(attributes.timestamp), - ) - .unwrap_or_default(), - ), + basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp), // calculate excess gas based on parent block's blob gas usage blob_excess_gas_and_price, }; diff --git a/crates/optimism/hardforks/src/hardfork.rs b/crates/optimism/hardforks/src/hardfork.rs index 011c4ae72fdd..440314e3711f 100644 --- a/crates/optimism/hardforks/src/hardfork.rs +++ b/crates/optimism/hardforks/src/hardfork.rs @@ -31,6 +31,8 @@ hardfork!( Fjord, /// Granite: Granite, + /// Holocene: + Holocene, } ); @@ -156,6 +158,7 @@ impl OptimismHardfork { Self::Ecotone => Some(1708534800), Self::Fjord => Some(1716998400), Self::Granite => Some(1723478400), + Self::Holocene => None, }, ) } @@ -190,6 +193,7 @@ impl OptimismHardfork { Self::Ecotone => Some(1710374401), Self::Fjord => Some(1720627201), Self::Granite => Some(1726070401), + Self::Holocene => None, }, ) } diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index a83f4c696a1c..00c971907276 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -15,7 +15,9 @@ use reth_node_api::{ }; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_forks::OptimismHardfork; -use reth_optimism_payload_builder::{OptimismBuiltPayload, OptimismPayloadBuilderAttributes}; +use reth_optimism_payload_builder::{ + builder::decode_eip_1559_params, OptimismBuiltPayload, OptimismPayloadBuilderAttributes, +}; /// The types used in the optimism beacon consensus engine. #[derive(Debug, Default, Clone, serde::Deserialize, serde::Serialize)] @@ -147,6 +149,143 @@ where )) } + if self.chain_spec.is_fork_active_at_timestamp( + OptimismHardfork::Holocene, + attributes.payload_attributes.timestamp, + ) { + if attributes.eip_1559_params.is_none() { + return Err(EngineObjectValidationError::InvalidParams( + "MissingEip1559ParamsInPayloadAttributes".to_string().into(), + )) + } + + let (elasticity, denominator) = decode_eip_1559_params( + attributes.eip_1559_params.expect("eip_1559_params is none"), + ); + + if elasticity != 0 && denominator == 0 { + return Err(EngineObjectValidationError::InvalidParams( + "Eip1559ParamsDenominatorZero".to_string().into(), + )) + } + } + Ok(()) } } + +#[cfg(test)] +mod test { + + use crate::engine; + use alloy_primitives::{b64, Address, B256, B64}; + use alloy_rpc_types_engine::PayloadAttributes; + use reth_chainspec::ForkCondition; + use reth_optimism_chainspec::BASE_SEPOLIA; + + use super::*; + + fn get_chainspec(is_holocene: bool) -> Arc { + let mut hardforks = OptimismHardfork::base_sepolia(); + if is_holocene { + hardforks + .insert(OptimismHardfork::Holocene.boxed(), ForkCondition::Timestamp(1800000000)); + } + Arc::new(OpChainSpec { + inner: ChainSpec { + chain: BASE_SEPOLIA.inner.chain, + genesis: BASE_SEPOLIA.inner.genesis.clone(), + genesis_hash: BASE_SEPOLIA.inner.genesis_hash.clone(), + paris_block_and_final_difficulty: BASE_SEPOLIA + .inner + .paris_block_and_final_difficulty, + hardforks, + base_fee_params: BASE_SEPOLIA.inner.base_fee_params.clone(), + max_gas_limit: BASE_SEPOLIA.inner.max_gas_limit, + prune_delete_limit: 10000, + ..Default::default() + }, + }) + } + + const fn get_attributes(eip_1559_params: Option, timestamp: u64) -> OpPayloadAttributes { + OpPayloadAttributes { + gas_limit: Some(1000), + eip_1559_params, + transactions: None, + no_tx_pool: None, + payload_attributes: PayloadAttributes { + timestamp, + prev_randao: B256::ZERO, + suggested_fee_recipient: Address::ZERO, + withdrawals: Some(vec![]), + parent_beacon_block_root: Some(B256::ZERO), + }, + } + } + + #[test] + fn test_well_formed_attributes_pre_holocene() { + let validator = OptimismEngineValidator::new(get_chainspec(false)); + let attributes = get_attributes(None, 1799999999); + + let result = >::ensure_well_formed_attributes( + &validator, EngineApiMessageVersion::V3, &attributes + ); + assert!(result.is_ok()); + } + + #[test] + fn test_well_formed_attributes_holocene_no_eip1559_params() { + let validator = OptimismEngineValidator::new(get_chainspec(true)); + let attributes = get_attributes(None, 1800000000); + + let result = >::ensure_well_formed_attributes( + &validator, EngineApiMessageVersion::V3, &attributes + ); + assert!(matches!(result, Err(EngineObjectValidationError::InvalidParams(_)))); + } + + #[test] + fn test_well_formed_attributes_holocene_eip1559_params_zero_denominator() { + let validator = OptimismEngineValidator::new(get_chainspec(true)); + let attributes = get_attributes(Some(b64!("0000000000000008")), 1800000000); + + let result = >::ensure_well_formed_attributes( + &validator, EngineApiMessageVersion::V3, &attributes + ); + assert!(matches!(result, Err(EngineObjectValidationError::InvalidParams(_)))); + } + + #[test] + fn test_well_formed_attributes_holocene_valid() { + let validator = OptimismEngineValidator::new(get_chainspec(true)); + let attributes = get_attributes(Some(b64!("0000000800000008")), 1800000000); + + let result = >::ensure_well_formed_attributes( + &validator, EngineApiMessageVersion::V3, &attributes + ); + assert!(result.is_ok()); + } + + #[test] + fn test_well_formed_attributes_holocene_valid_all_zero() { + let validator = OptimismEngineValidator::new(get_chainspec(true)); + let attributes = get_attributes(Some(b64!("0000000000000000")), 1800000000); + + let result = >::ensure_well_formed_attributes( + &validator, EngineApiMessageVersion::V3, &attributes + ); + assert!(result.is_ok()); + } +} diff --git a/crates/optimism/node/tests/e2e/utils.rs b/crates/optimism/node/tests/e2e/utils.rs index 8ea8df380b0b..005d747be96e 100644 --- a/crates/optimism/node/tests/e2e/utils.rs +++ b/crates/optimism/node/tests/e2e/utils.rs @@ -66,5 +66,6 @@ pub(crate) fn optimism_payload_attributes(timestamp: u64) -> OptimismPayloadBuil transactions: vec![], no_tx_pool: false, gas_limit: Some(30_000_000), + eip_1559_params: None, } } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 0a8dcdb12449..85c558ab3e68 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -1,11 +1,10 @@ //! Optimism payload builder implementation. - use std::sync::Arc; -use alloy_primitives::U256; +use alloy_primitives::{b64, B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; -use reth_chainspec::{ChainSpecProvider, EthereumHardforks}; +use reth_chainspec::{BaseFeeParams, ChainSpecProvider, EthereumHardforks}; use reth_evm::{system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; use reth_execution_types::ExecutionOutcome; use reth_optimism_chainspec::OpChainSpec; @@ -29,7 +28,7 @@ use revm::{ primitives::{EVMError, EnvWithHandlerCfg, InvalidTransaction, ResultAndState}, DatabaseCommit, }; -use revm_primitives::calc_excess_blob_gas; +use revm_primitives::{calc_excess_blob_gas, Bytes}; use tracing::{debug, trace, warn}; use crate::{ @@ -173,7 +172,7 @@ where let state = StateProviderDatabase::new(state_provider); let mut db = State::builder().with_database_ref(cached_reads.as_db(state)).with_bundle_update().build(); - let PayloadConfig { parent_block, attributes, extra_data } = config; + let PayloadConfig { parent_block, attributes, mut extra_data } = config; debug!(target: "payload_builder", id=%attributes.payload_attributes.payload_id(), parent_hash = ?parent_block.hash(), parent_number = parent_block.number, "building new payload"); @@ -485,6 +484,18 @@ where blob_gas_used = Some(0); } + let is_holocene = chain_spec.is_fork_active_at_timestamp( + OptimismHardfork::Holocene, + attributes.payload_attributes.timestamp, + ); + + if is_holocene { + extra_data = get_holocene_extra_data( + &attributes, + chain_spec.base_fee_params_at_timestamp(attributes.payload_attributes.timestamp), + ); + } + let header = Header { parent_hash: parent_block.hash(), ommers_hash: EMPTY_OMMER_ROOT_HASH, @@ -541,3 +552,69 @@ where Ok(BuildOutcome::Better { payload, cached_reads }) } + +/// Extracts the Holcene 1599 parameters from the encoded form: +/// +pub fn decode_eip_1559_params(eip_1559_params: B64) -> (u32, u32) { + let elasticity: [u8; 4] = eip_1559_params.0[..4].try_into().unwrap(); + let denominator: [u8; 4] = eip_1559_params.0[4..].try_into().unwrap(); + println!("denominator: {:?}", denominator); + println!("elasticity: {:?}", elasticity); + (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) +} + +fn get_holocene_extra_data( + attributes: &OptimismPayloadBuilderAttributes, + default_base_fee_params: BaseFeeParams, +) -> Bytes { + let mut extra_data = [0u8; 9]; + // If eip 1559 params are set, use them, otherwise use the canyon base fee param constants + // The eip 1559 params should exist here since there was a check previously + if attributes.eip_1559_params.unwrap() == B64::ZERO { + let mut default_params = [0u8; 8]; + default_params[..4].copy_from_slice( + &(default_base_fee_params.max_change_denominator as u32).to_be_bytes(), + ); + default_params[4..] + .copy_from_slice(&(default_base_fee_params.elasticity_multiplier as u32).to_be_bytes()); + extra_data[1..].copy_from_slice(&default_params); + Bytes::copy_from_slice(&extra_data) + } else { + let (elasticity, denominator) = + decode_eip_1559_params(attributes.eip_1559_params.expect("eip_1559_params is none")); + let mut eip_1559_params = [0u8; 8]; + println!("denominator: {}", denominator); + println!("elasticity: {}", elasticity); + eip_1559_params[..4].copy_from_slice(&(denominator as u32).to_be_bytes()); + eip_1559_params[4..].copy_from_slice(&(elasticity as u32).to_be_bytes()); + extra_data[1..].copy_from_slice(&eip_1559_params); + Bytes::copy_from_slice(&extra_data) + } +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use super::*; + + #[test] + fn test_get_extra_data_post_holocene() { + let attributes = OptimismPayloadBuilderAttributes { + eip_1559_params: Some(B64::from_str("0x0000000800000008").unwrap()), + ..Default::default() + }; + let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); + assert_eq!(extra_data, Bytes::copy_from_slice(&[0, 0, 0, 0, 8, 0, 0, 0, 8])); + } + + #[test] + fn test_get_extra_data_post_holocene_default() { + let attributes = OptimismPayloadBuilderAttributes { + eip_1559_params: Some(B64::ZERO), + ..Default::default() + }; + let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); + assert_eq!(extra_data, Bytes::copy_from_slice(&[0, 0, 0, 0, 80, 0, 0, 0, 60])); + } +} diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index 122c2fde5269..718b34261a86 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -3,7 +3,7 @@ //! Optimism builder support use alloy_eips::eip2718::Decodable2718; -use alloy_primitives::{Address, B256, U256}; +use alloy_primitives::{Address, B256, B64, U256}; use alloy_rlp::Encodable; use alloy_rpc_types_engine::{ExecutionPayloadEnvelopeV2, ExecutionPayloadV1, PayloadId}; /// Re-export for use in downstream arguments. @@ -24,7 +24,7 @@ use reth_rpc_types_compat::engine::payload::{ use std::sync::Arc; /// Optimism Payload Builder Attributes -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct OptimismPayloadBuilderAttributes { /// Inner ethereum payload builder attributes pub payload_attributes: EthPayloadBuilderAttributes, @@ -35,6 +35,8 @@ pub struct OptimismPayloadBuilderAttributes { pub transactions: Vec>, /// The gas limit for the generated payload pub gas_limit: Option, + /// EIP-1559 parameters for the generated payload + pub eip_1559_params: Option, } impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { @@ -79,6 +81,7 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { no_tx_pool: attributes.no_tx_pool.unwrap_or_default(), transactions, gas_limit: attributes.gas_limit, + eip_1559_params: attributes.eip_1559_params, }) } @@ -296,6 +299,10 @@ pub(crate) fn payload_id_optimism(parent: &B256, attributes: &OpPayloadAttribute hasher.update(gas_limit.to_be_bytes()); } + if let Some(eip_1559_params) = &attributes.eip_1559_params { + hasher.update(eip_1559_params.0); + } + let out = hasher.finalize(); PayloadId::new(out.as_slice()[..8].try_into().expect("sufficient length")) } From e34fb0e496a881e748d091cd6301205d53c7acc2 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Mon, 21 Oct 2024 19:10:11 -0400 Subject: [PATCH 02/19] remove fee traits; fix unwrap() usage --- crates/optimism/chainspec/src/lib.rs | 61 ++++++++++++++------------ crates/optimism/evm/src/lib.rs | 2 +- crates/optimism/node/src/engine.rs | 9 ++-- crates/optimism/payload/src/builder.rs | 30 ++++++------- 4 files changed, 52 insertions(+), 50 deletions(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 603d29a0af3d..83f57272b6e6 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -188,22 +188,9 @@ pub struct OpChainSpec { pub inner: ChainSpec, } -/// Fee trait for OP chain specs. -pub trait Fee { +impl OpChainSpec { /// Read from parent to determine the base fee for the next block - fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256; -} - -/// Extracts the Holcene 1599 parameters from the encoded form: -/// -pub fn decode_holocene_1559_params(extra_data: Bytes) -> (u32, u32) { - let denominator = extra_data[1..5].try_into().unwrap(); - let elasticity = extra_data[5..9].try_into().unwrap(); - (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) -} - -impl Fee for OpChainSpec { - fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256 { + pub fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256 { let is_holocene = self.inner.is_fork_active_at_timestamp( reth_optimism_forks::OptimismHardfork::Holocene, timestamp, @@ -211,18 +198,25 @@ impl Fee for OpChainSpec { // If we are in the Holocene, we need to use the base fee params from the parent block's // nonce Else, use the base fee params from chainspec if is_holocene { - // First 4 bytes of the nonce are the base fee denominator, the last 4 bytes are the - // elasticity - let (elasticity, denominator) = decode_holocene_1559_params(parent.extra_data.clone()); - if elasticity == 0 && denominator == 0 { - return U256::from( - parent - .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) - .unwrap_or_default(), - ); + match decode_holocene_1559_params(parent.extra_data.clone()) { + Ok((denominator, elasticity)) => { + if elasticity == 0 && denominator == 0 { + return U256::from( + parent + .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) + .unwrap_or_default(), + ); + } + let base_fee_params = + BaseFeeParams::new(denominator as u128, elasticity as u128); + return U256::from( + parent.next_block_base_fee(base_fee_params).unwrap_or_default(), + ) + } + Err(e) => { + panic!("Error occurred: {}", e) + } } - let base_fee_params = BaseFeeParams::new(denominator as u128, elasticity as u128); - U256::from(parent.next_block_base_fee(base_fee_params).unwrap_or_default()) } else { U256::from( parent @@ -233,6 +227,19 @@ impl Fee for OpChainSpec { } } +/// Extracts the Holcene 1599 parameters from the encoded form: +/// +pub fn decode_holocene_1559_params(extra_data: Bytes) -> Result<(u32, u32), String> { + if extra_data.len() < 9 { + return Err("Insufficient data".to_string()); + } + let denominator: [u8; 4] = + extra_data[1..5].try_into().map_err(|_| "Failed to extract denominator".to_string())?; + let elasticity: [u8; 4] = + extra_data[5..9].try_into().map_err(|_| "Failed to extract elasticity".to_string())?; + Ok((u32::from_be_bytes(denominator), u32::from_be_bytes(elasticity))) +} + /// Returns the signature for the optimism deposit transactions, which don't include a /// signature. pub fn optimism_deposit_tx_signature() -> Signature { @@ -470,7 +477,7 @@ mod tests { use std::sync::Arc; use alloy_genesis::{ChainConfig, Genesis}; - use alloy_primitives::{b256, hex::FromHex}; + use alloy_primitives::{b256}; use reth_chainspec::{test_fork_ids, BaseFeeParams, BaseFeeParamsKind}; use reth_ethereum_forks::{EthereumHardfork, ForkCondition, ForkHash, ForkId, Head}; use reth_optimism_forks::{OptimismHardfork, OptimismHardforks}; diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index cab4e8aa6167..d2e58ceaabda 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -11,7 +11,7 @@ use alloy_primitives::{Address, U256}; use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; -use reth_optimism_chainspec::{Fee, OpChainSpec}; +use reth_optimism_chainspec::OpChainSpec; use reth_primitives::{ revm_primitives::{AnalysisKind, CfgEnvWithHandlerCfg, TxEnv}, transaction::FillTxEnv, diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 00c971907276..caaf3f593513 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -153,15 +153,12 @@ where OptimismHardfork::Holocene, attributes.payload_attributes.timestamp, ) { - if attributes.eip_1559_params.is_none() { + let Some(eip_1559_params) = attributes.eip_1559_params else { return Err(EngineObjectValidationError::InvalidParams( "MissingEip1559ParamsInPayloadAttributes".to_string().into(), )) - } - - let (elasticity, denominator) = decode_eip_1559_params( - attributes.eip_1559_params.expect("eip_1559_params is none"), - ); + }; + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); if elasticity != 0 && denominator == 0 { return Err(EngineObjectValidationError::InvalidParams( diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 85c558ab3e68..e87dd46d570c 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -1,7 +1,7 @@ //! Optimism payload builder implementation. use std::sync::Arc; -use alloy_primitives::{b64, B64, U256}; +use alloy_primitives::{B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; use reth_chainspec::{BaseFeeParams, ChainSpecProvider, EthereumHardforks}; @@ -558,8 +558,6 @@ where pub fn decode_eip_1559_params(eip_1559_params: B64) -> (u32, u32) { let elasticity: [u8; 4] = eip_1559_params.0[..4].try_into().unwrap(); let denominator: [u8; 4] = eip_1559_params.0[4..].try_into().unwrap(); - println!("denominator: {:?}", denominator); - println!("elasticity: {:?}", elasticity); (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) } @@ -570,24 +568,24 @@ fn get_holocene_extra_data( let mut extra_data = [0u8; 9]; // If eip 1559 params are set, use them, otherwise use the canyon base fee param constants // The eip 1559 params should exist here since there was a check previously - if attributes.eip_1559_params.unwrap() == B64::ZERO { - let mut default_params = [0u8; 8]; - default_params[..4].copy_from_slice( + let Some(eip_1559_params) = attributes.eip_1559_params else { + panic!("eip_1559_params is none") + }; + + if eip_1559_params == B64::ZERO { + extra_data[1..5].copy_from_slice( &(default_base_fee_params.max_change_denominator as u32).to_be_bytes(), ); - default_params[4..] + extra_data[5..9] .copy_from_slice(&(default_base_fee_params.elasticity_multiplier as u32).to_be_bytes()); - extra_data[1..].copy_from_slice(&default_params); Bytes::copy_from_slice(&extra_data) } else { - let (elasticity, denominator) = - decode_eip_1559_params(attributes.eip_1559_params.expect("eip_1559_params is none")); - let mut eip_1559_params = [0u8; 8]; - println!("denominator: {}", denominator); - println!("elasticity: {}", elasticity); - eip_1559_params[..4].copy_from_slice(&(denominator as u32).to_be_bytes()); - eip_1559_params[4..].copy_from_slice(&(elasticity as u32).to_be_bytes()); - extra_data[1..].copy_from_slice(&eip_1559_params); + let Some(eip_1559_params) = attributes.eip_1559_params else { + panic!("eip_1559_params is none") + }; + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); + extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); Bytes::copy_from_slice(&extra_data) } } From 3e053c04f3f71bc5f4e81c685f7370a2d270348c Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Tue, 22 Oct 2024 10:15:02 -0400 Subject: [PATCH 03/19] get rid of unwrap() --- crates/optimism/chainspec/src/lib.rs | 2 +- crates/optimism/node/src/engine.rs | 16 +++++++++----- crates/optimism/payload/src/builder.rs | 29 +++++++++++++++++++------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 83f57272b6e6..44bd038d65e9 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -477,7 +477,7 @@ mod tests { use std::sync::Arc; use alloy_genesis::{ChainConfig, Genesis}; - use alloy_primitives::{b256}; + use alloy_primitives::b256; use reth_chainspec::{test_fork_ids, BaseFeeParams, BaseFeeParamsKind}; use reth_ethereum_forks::{EthereumHardfork, ForkCondition, ForkHash, ForkId, Head}; use reth_optimism_forks::{OptimismHardfork, OptimismHardforks}; diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index caaf3f593513..01d325c68c58 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -158,12 +158,18 @@ where "MissingEip1559ParamsInPayloadAttributes".to_string().into(), )) }; - let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); - if elasticity != 0 && denominator == 0 { - return Err(EngineObjectValidationError::InvalidParams( - "Eip1559ParamsDenominatorZero".to_string().into(), - )) + match decode_eip_1559_params(eip_1559_params) { + Ok((elasticity, denominator)) => { + if elasticity != 0 && denominator == 0 { + return Err(EngineObjectValidationError::InvalidParams( + "Eip1559ParamsDenominatorZero".to_string().into(), + )) + } + } + Err(e) => { + return Err(EngineObjectValidationError::InvalidParams(e.to_string().into())) + } } } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index e87dd46d570c..4923350700f5 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -555,10 +555,16 @@ where /// Extracts the Holcene 1599 parameters from the encoded form: /// -pub fn decode_eip_1559_params(eip_1559_params: B64) -> (u32, u32) { - let elasticity: [u8; 4] = eip_1559_params.0[..4].try_into().unwrap(); - let denominator: [u8; 4] = eip_1559_params.0[4..].try_into().unwrap(); - (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) +pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), String> { + let elasticity: [u8; 4] = eip_1559_params.0[..4] + .try_into() + .map_err(|_| "Failed to extract elasticity".to_string())?; + + let denominator: [u8; 4] = eip_1559_params.0[4..8] + .try_into() + .map_err(|_| "Failed to extract denominator".to_string())?; + + Ok((u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator))) } fn get_holocene_extra_data( @@ -583,10 +589,17 @@ fn get_holocene_extra_data( let Some(eip_1559_params) = attributes.eip_1559_params else { panic!("eip_1559_params is none") }; - let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); - extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); - extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); - Bytes::copy_from_slice(&extra_data) + + match decode_eip_1559_params(eip_1559_params) { + Ok((elasticity, denominator)) => { + extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); + return Bytes::copy_from_slice(&extra_data); + } + Err(e) => { + panic!("Error occurred: {}", e) + } + } } } From 83231edf4e35b7fd44baf8612a6abc88912c6c52 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Tue, 22 Oct 2024 10:25:38 -0400 Subject: [PATCH 04/19] fmt + default --- crates/ethereum/engine-primitives/src/payload.rs | 16 +--------------- crates/optimism/payload/src/builder.rs | 2 +- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/crates/ethereum/engine-primitives/src/payload.rs b/crates/ethereum/engine-primitives/src/payload.rs index 5d7863444355..722099d07a83 100644 --- a/crates/ethereum/engine-primitives/src/payload.rs +++ b/crates/ethereum/engine-primitives/src/payload.rs @@ -169,7 +169,7 @@ impl From for ExecutionPayloadEnvelopeV4 { } /// Container type for all components required to build a payload. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct EthPayloadBuilderAttributes { /// Id of the payload pub id: PayloadId, @@ -279,20 +279,6 @@ pub(crate) fn payload_id(parent: &B256, attributes: &PayloadAttributes) -> Paylo PayloadId::new(out.as_slice()[..8].try_into().expect("sufficient length")) } -impl Default for EthPayloadBuilderAttributes { - fn default() -> Self { - Self { - id: PayloadId::new([0; 8]), - parent: B256::default(), - timestamp: 0, - suggested_fee_recipient: Address::ZERO, - prev_randao: B256::default(), - withdrawals: Withdrawals::default(), - parent_beacon_block_root: None, - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 2bdc6f7d45f4..d8e9be572c04 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -1,8 +1,8 @@ //! Optimism payload builder implementation. use std::sync::Arc; -use alloy_primitives::{B64, U256}; use alloy_consensus::EMPTY_OMMER_ROOT_HASH; +use alloy_primitives::{B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; use reth_chainspec::{BaseFeeParams, ChainSpecProvider, EthereumHardforks}; From b45a754bb4626e9c6e66589bb2fc46ca83b4d2e2 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Tue, 22 Oct 2024 18:33:54 -0400 Subject: [PATCH 05/19] add custom errors and remove panic --- crates/optimism/chainspec/src/lib.rs | 47 +++++++++++++-------- crates/optimism/evm/src/lib.rs | 3 +- crates/optimism/node/src/engine.rs | 3 +- crates/optimism/payload/src/builder.rs | 57 ++++++++++++++------------ 4 files changed, 65 insertions(+), 45 deletions(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 44bd038d65e9..e9f77ce62c28 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -190,7 +190,11 @@ pub struct OpChainSpec { impl OpChainSpec { /// Read from parent to determine the base fee for the next block - pub fn next_block_base_fee(&self, parent: &Header, timestamp: u64) -> U256 { + pub fn next_block_base_fee( + &self, + parent: &Header, + timestamp: u64, + ) -> Result { let is_holocene = self.inner.is_fork_active_at_timestamp( reth_optimism_forks::OptimismHardfork::Holocene, timestamp, @@ -201,42 +205,51 @@ impl OpChainSpec { match decode_holocene_1559_params(parent.extra_data.clone()) { Ok((denominator, elasticity)) => { if elasticity == 0 && denominator == 0 { - return U256::from( + return Ok(U256::from( parent .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) .unwrap_or_default(), - ); + )); } let base_fee_params = BaseFeeParams::new(denominator as u128, elasticity as u128); - return U256::from( + return Ok(U256::from( parent.next_block_base_fee(base_fee_params).unwrap_or_default(), - ) - } - Err(e) => { - panic!("Error occurred: {}", e) + )) } + Err(e) => Err(e), } } else { - U256::from( + Ok(U256::from( parent .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) .unwrap_or_default(), - ) + )) } } } +#[derive(Debug)] +/// Error type for decoding Holocene 1559 parameters +pub enum DecodeError { + /// Insufficient data to decode + InsufficientData, + /// Invalid denominator parameter + InvalidDenominator, + /// Invalid elasticity parameter + InvalidElasticity, +} + /// Extracts the Holcene 1599 parameters from the encoded form: /// -pub fn decode_holocene_1559_params(extra_data: Bytes) -> Result<(u32, u32), String> { +pub fn decode_holocene_1559_params(extra_data: Bytes) -> Result<(u32, u32), DecodeError> { if extra_data.len() < 9 { - return Err("Insufficient data".to_string()); + return Err(DecodeError::InsufficientData); } let denominator: [u8; 4] = - extra_data[1..5].try_into().map_err(|_| "Failed to extract denominator".to_string())?; + extra_data[1..5].try_into().map_err(|_| DecodeError::InvalidDenominator)?; let elasticity: [u8; 4] = - extra_data[5..9].try_into().map_err(|_| "Failed to extract elasticity".to_string())?; + extra_data[5..9].try_into().map_err(|_| DecodeError::InvalidElasticity)?; Ok((u32::from_be_bytes(denominator), u32::from_be_bytes(elasticity))) } @@ -1002,7 +1015,7 @@ mod tests { }; let base_fee = op_chain_spec.next_block_base_fee(&parent, 0); assert_eq!( - base_fee, + base_fee.unwrap(), U256::from( parent .next_block_base_fee(op_chain_spec.base_fee_params_at_timestamp(0)) @@ -1042,7 +1055,7 @@ mod tests { }; let base_fee = op_chain_spec.next_block_base_fee(&parent, 1800000005); assert_eq!( - base_fee, + base_fee.unwrap(), U256::from( parent .next_block_base_fee(op_chain_spec.base_fee_params_at_timestamp(0)) @@ -1065,7 +1078,7 @@ mod tests { let base_fee = op_chain_spec.next_block_base_fee(&parent, 1800000005); assert_eq!( - base_fee, + base_fee.unwrap(), U256::from( parent .next_block_base_fee(BaseFeeParams::new(0x00000008, 0x00000008)) diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 6a3d45de40b1..8dfaa1e20a91 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -156,7 +156,8 @@ impl ConfigureEvmEnv for OptimismEvmConfig { prevrandao: Some(attributes.prev_randao), gas_limit: U256::from(parent.gas_limit), // calculate basefee based on parent block's gas usage - basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp), + // basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp), + basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp).unwrap(), // calculate excess gas based on parent block's blob gas usage blob_excess_gas_and_price, }; diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 7c1ea841edce..7b24a312d633 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -16,7 +16,8 @@ use reth_node_api::{ use reth_optimism_chainspec::OpChainSpec; use reth_optimism_forks::OptimismHardfork; use reth_optimism_payload_builder::{ - builder::decode_eip_1559_params, OptimismBuiltPayload, OptimismPayloadBuilderAttributes, + builder::{decode_eip_1559_params, EIP1559ParamError}, + OptimismBuiltPayload, OptimismPayloadBuilderAttributes, }; /// The types used in the optimism beacon consensus engine. diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index d8e9be572c04..fa4e4864d6bf 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -494,7 +494,8 @@ where extra_data = get_holocene_extra_data( &attributes, chain_spec.base_fee_params_at_timestamp(attributes.payload_attributes.timestamp), - ); + ) + .map_err(|err| PayloadBuilderError::other(err))?; } let header = Header { @@ -554,16 +555,28 @@ where Ok(BuildOutcome::Better { payload, cached_reads }) } +#[derive(Debug, thiserror::Error)] +// Error type for EIP-1559 parameters +pub enum EIP1559ParamError { + #[error("No EIP-1559 parameters provided")] + /// No EIP-1559 parameters provided + NoEIP1559Params, + #[error("Invalid elasticity parameter")] + /// Invalid denominator parameter + InvalidDenominator, + #[error("Invalid denominator parameter")] + /// Invalid elasticity parameter + InvalidElasticity, +} + /// Extracts the Holcene 1599 parameters from the encoded form: /// -pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), String> { - let elasticity: [u8; 4] = eip_1559_params.0[..4] - .try_into() - .map_err(|_| "Failed to extract elasticity".to_string())?; +pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), EIP1559ParamError> { + let elasticity: [u8; 4] = + eip_1559_params.0[..4].try_into().map_err(|_| EIP1559ParamError::InvalidElasticity)?; - let denominator: [u8; 4] = eip_1559_params.0[4..8] - .try_into() - .map_err(|_| "Failed to extract denominator".to_string())?; + let denominator: [u8; 4] = + eip_1559_params.0[4..8].try_into().map_err(|_| EIP1559ParamError::InvalidDenominator)?; Ok((u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator))) } @@ -571,35 +584,27 @@ pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), String fn get_holocene_extra_data( attributes: &OptimismPayloadBuilderAttributes, default_base_fee_params: BaseFeeParams, -) -> Bytes { - let mut extra_data = [0u8; 9]; - // If eip 1559 params are set, use them, otherwise use the canyon base fee param constants - // The eip 1559 params should exist here since there was a check previously - let Some(eip_1559_params) = attributes.eip_1559_params else { - panic!("eip_1559_params is none") - }; +) -> Result { + let eip_1559_params = attributes.eip_1559_params.ok_or(EIP1559ParamError::NoEIP1559Params)?; + let mut extra_data = [0u8; 9]; + // If eip 1559 params aren't set, use the canyon base fee param constants + // otherwise use them if eip_1559_params == B64::ZERO { extra_data[1..5].copy_from_slice( &(default_base_fee_params.max_change_denominator as u32).to_be_bytes(), ); extra_data[5..9] .copy_from_slice(&(default_base_fee_params.elasticity_multiplier as u32).to_be_bytes()); - Bytes::copy_from_slice(&extra_data) + Ok(Bytes::copy_from_slice(&extra_data)) } else { - let Some(eip_1559_params) = attributes.eip_1559_params else { - panic!("eip_1559_params is none") - }; - match decode_eip_1559_params(eip_1559_params) { Ok((elasticity, denominator)) => { extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); - return Bytes::copy_from_slice(&extra_data); - } - Err(e) => { - panic!("Error occurred: {}", e) + Ok(Bytes::copy_from_slice(&extra_data)) } + Err(e) => return Err(EIP1559ParamError::InvalidElasticity), } } } @@ -617,7 +622,7 @@ mod tests { ..Default::default() }; let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); - assert_eq!(extra_data, Bytes::copy_from_slice(&[0, 0, 0, 0, 8, 0, 0, 0, 8])); + assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 8, 0, 0, 0, 8])); } #[test] @@ -627,6 +632,6 @@ mod tests { ..Default::default() }; let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); - assert_eq!(extra_data, Bytes::copy_from_slice(&[0, 0, 0, 0, 80, 0, 0, 0, 60])); + assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 80, 0, 0, 0, 60])); } } From db98d0b0daa5210e2e271d395669c7be9c0a9f8f Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Wed, 23 Oct 2024 17:04:22 -0400 Subject: [PATCH 06/19] use ? instead of match --- crates/optimism/chainspec/src/lib.rs | 30 ++++++++------------ crates/optimism/node/src/engine.rs | 20 +++++-------- crates/optimism/payload/src/builder.rs | 39 +++++++++++++++++--------- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index e9f77ce62c28..9ac2266467e3 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -199,26 +199,20 @@ impl OpChainSpec { reth_optimism_forks::OptimismHardfork::Holocene, timestamp, ); - // If we are in the Holocene, we need to use the base fee params from the parent block's - // nonce Else, use the base fee params from chainspec + // If we are in the Holocene, we need to use the base fee params + // from the parent block's extra data. + // Else, use the base fee params (default values) from chainspec if is_holocene { - match decode_holocene_1559_params(parent.extra_data.clone()) { - Ok((denominator, elasticity)) => { - if elasticity == 0 && denominator == 0 { - return Ok(U256::from( - parent - .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) - .unwrap_or_default(), - )); - } - let base_fee_params = - BaseFeeParams::new(denominator as u128, elasticity as u128); - return Ok(U256::from( - parent.next_block_base_fee(base_fee_params).unwrap_or_default(), - )) - } - Err(e) => Err(e), + let (denominator, elasticity) = decode_holocene_1559_params(parent.extra_data.clone())?; + if elasticity == 0 && denominator == 0 { + return Ok(U256::from( + parent + .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) + .unwrap_or_default(), + )); } + let base_fee_params = BaseFeeParams::new(denominator as u128, elasticity as u128); + Ok(U256::from(parent.next_block_base_fee(base_fee_params).unwrap_or_default())) } else { Ok(U256::from( parent diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 7b24a312d633..a49c7b0c3ab3 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -16,8 +16,7 @@ use reth_node_api::{ use reth_optimism_chainspec::OpChainSpec; use reth_optimism_forks::OptimismHardfork; use reth_optimism_payload_builder::{ - builder::{decode_eip_1559_params, EIP1559ParamError}, - OptimismBuiltPayload, OptimismPayloadBuilderAttributes, + builder::decode_eip_1559_params, OptimismBuiltPayload, OptimismPayloadBuilderAttributes, }; /// The types used in the optimism beacon consensus engine. @@ -160,17 +159,12 @@ where )) }; - match decode_eip_1559_params(eip_1559_params) { - Ok((elasticity, denominator)) => { - if elasticity != 0 && denominator == 0 { - return Err(EngineObjectValidationError::InvalidParams( - "Eip1559ParamsDenominatorZero".to_string().into(), - )) - } - } - Err(e) => { - return Err(EngineObjectValidationError::InvalidParams(e.to_string().into())) - } + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params) + .map_err(|e| EngineObjectValidationError::InvalidParams(e.to_string().into()))?; + if elasticity != 0 && denominator == 0 { + return Err(EngineObjectValidationError::InvalidParams( + "Eip1559ParamsDenominatorZero".to_string().into(), + )) } } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index fa4e4864d6bf..ec95ce839d6b 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -495,7 +495,7 @@ where &attributes, chain_spec.base_fee_params_at_timestamp(attributes.payload_attributes.timestamp), ) - .map_err(|err| PayloadBuilderError::other(err))?; + .map_err(PayloadBuilderError::other)?; } let header = Header { @@ -567,6 +567,12 @@ pub enum EIP1559ParamError { #[error("Invalid denominator parameter")] /// Invalid elasticity parameter InvalidElasticity, + #[error("Denominator overflow")] + /// Denominator overflow + DenominatorOverflow, + #[error("Elasticity overflow")] + /// Elasticity overflow + ElasticityOverflow, } /// Extracts the Holcene 1599 parameters from the encoded form: @@ -591,21 +597,26 @@ fn get_holocene_extra_data( // If eip 1559 params aren't set, use the canyon base fee param constants // otherwise use them if eip_1559_params == B64::ZERO { - extra_data[1..5].copy_from_slice( - &(default_base_fee_params.max_change_denominator as u32).to_be_bytes(), - ); - extra_data[5..9] - .copy_from_slice(&(default_base_fee_params.elasticity_multiplier as u32).to_be_bytes()); + // Try casting max_change_denominator to u32 + let max_change_denominator: u32 = (default_base_fee_params.max_change_denominator) + .try_into() + .map_err(|_| EIP1559ParamError::DenominatorOverflow)?; + + // Try casting elasticity_multiplier to u32 + let elasticity_multiplier: u32 = (default_base_fee_params.elasticity_multiplier) + .try_into() + .map_err(|_| EIP1559ParamError::ElasticityOverflow)?; + + // Copy the values safely + extra_data[1..5].copy_from_slice(&max_change_denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity_multiplier.to_be_bytes()); Ok(Bytes::copy_from_slice(&extra_data)) } else { - match decode_eip_1559_params(eip_1559_params) { - Ok((elasticity, denominator)) => { - extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); - extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); - Ok(Bytes::copy_from_slice(&extra_data)) - } - Err(e) => return Err(EIP1559ParamError::InvalidElasticity), - } + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params) + .map_err(|_| EIP1559ParamError::InvalidElasticity)?; + extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); + Ok(Bytes::copy_from_slice(&extra_data)) } } From dcd22cd983707ffd2df147014ed6a1a0e3b3bbcf Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Thu, 24 Oct 2024 00:41:06 -0400 Subject: [PATCH 07/19] remove all unwrap and panic and use Result --- Cargo.lock | 5 +++++ crates/ethereum/evm/src/lib.rs | 6 +++--- crates/ethereum/payload/src/lib.rs | 12 ++++++++---- crates/evm/Cargo.toml | 3 +++ crates/evm/src/lib.rs | 10 +++++++++- crates/optimism/chainspec/Cargo.toml | 1 + crates/optimism/chainspec/src/lib.rs | 5 ++++- crates/optimism/evm/Cargo.toml | 1 + crates/optimism/evm/src/lib.rs | 14 +++++++++----- crates/optimism/payload/src/builder.rs | 16 +++++++++++----- examples/custom-evm/Cargo.toml | 1 + examples/custom-evm/src/main.rs | 3 ++- examples/stateful-precompile/Cargo.toml | 1 + examples/stateful-precompile/src/main.rs | 3 ++- 14 files changed, 60 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59c582fec3f2..f633475137e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2849,6 +2849,7 @@ dependencies = [ "eyre", "reth", "reth-chainspec", + "reth-evm", "reth-evm-ethereum", "reth-node-api", "reth-node-core", @@ -3033,6 +3034,7 @@ dependencies = [ "parking_lot", "reth", "reth-chainspec", + "reth-evm", "reth-node-api", "reth-node-core", "reth-node-ethereum", @@ -7429,6 +7431,7 @@ dependencies = [ "reth-storage-errors", "revm", "revm-primitives", + "thiserror", ] [[package]] @@ -8093,6 +8096,7 @@ dependencies = [ "reth-optimism-forks", "reth-primitives-traits", "serde_json", + "thiserror", ] [[package]] @@ -8179,6 +8183,7 @@ dependencies = [ "reth-revm", "revm", "revm-primitives", + "thiserror", "tracing", ] diff --git a/crates/ethereum/evm/src/lib.rs b/crates/ethereum/evm/src/lib.rs index 9abb11976363..b8129c3e34fa 100644 --- a/crates/ethereum/evm/src/lib.rs +++ b/crates/ethereum/evm/src/lib.rs @@ -20,7 +20,7 @@ extern crate alloc; use alloc::{sync::Arc, vec::Vec}; use alloy_primitives::{Address, Bytes, TxKind, U256}; use reth_chainspec::{ChainSpec, Head}; -use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; +use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError}; use reth_primitives::{transaction::FillTxEnv, Header, TransactionSigned}; use revm_primitives::{ AnalysisKind, BlobExcessGasAndPrice, BlockEnv, CfgEnv, CfgEnvWithHandlerCfg, Env, SpecId, TxEnv, @@ -131,7 +131,7 @@ impl ConfigureEvmEnv for EthEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { // configure evm env based on parent block let cfg = CfgEnv::default().with_chain_id(self.chain_spec.chain().id()); @@ -179,7 +179,7 @@ impl ConfigureEvmEnv for EthEvmConfig { blob_excess_gas_and_price, }; - (CfgEnvWithHandlerCfg::new_with_spec_id(cfg, spec_id), block_env) + Ok((CfgEnvWithHandlerCfg::new_with_spec_id(cfg, spec_id), block_env)) } } diff --git a/crates/ethereum/payload/src/lib.rs b/crates/ethereum/payload/src/lib.rs index 7f94acf723cf..6850aefe1453 100644 --- a/crates/ethereum/payload/src/lib.rs +++ b/crates/ethereum/payload/src/lib.rs @@ -19,7 +19,7 @@ use reth_basic_payload_builder::{ use reth_chain_state::ExecutedBlock; use reth_chainspec::ChainSpec; use reth_errors::RethError; -use reth_evm::{system_calls::SystemCaller, ConfigureEvm, NextBlockEnvAttributes}; +use reth_evm::{system_calls::SystemCaller, ConfigureEvm, NextBlockEnvAttributes, NextCfgError}; use reth_evm_ethereum::{eip6110::parse_deposits_from_receipts, EthEvmConfig}; use reth_execution_types::ExecutionOutcome; use reth_payload_builder::{EthBuiltPayload, EthPayloadBuilderAttributes}; @@ -69,7 +69,7 @@ where &self, config: &PayloadConfig, parent: &Header, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { let next_attributes = NextBlockEnvAttributes { timestamp: config.attributes.timestamp(), suggested_fee_recipient: config.attributes.suggested_fee_recipient(), @@ -93,7 +93,9 @@ where &self, args: BuildArguments, ) -> Result, PayloadBuilderError> { - let (cfg_env, block_env) = self.cfg_and_block_env(&args.config, &args.config.parent_block); + let (cfg_env, block_env) = self + .cfg_and_block_env(&args.config, &args.config.parent_block) + .map_err(PayloadBuilderError::other)?; default_ethereum_payload(self.evm_config.clone(), args, cfg_env, block_env) } @@ -111,7 +113,9 @@ where cancel: Default::default(), best_payload: None, }; - let (cfg_env, block_env) = self.cfg_and_block_env(&args.config, &args.config.parent_block); + let (cfg_env, block_env) = self + .cfg_and_block_env(&args.config, &args.config.parent_block) + .map_err(PayloadBuilderError::other)?; default_ethereum_payload(self.evm_config.clone(), args, cfg_env, block_env)? .into_payload() .ok_or_else(|| PayloadBuilderError::MissingPayload) diff --git a/crates/evm/Cargo.toml b/crates/evm/Cargo.toml index d97a57864193..5ef247be1ffd 100644 --- a/crates/evm/Cargo.toml +++ b/crates/evm/Cargo.toml @@ -36,6 +36,9 @@ futures-util.workspace = true metrics = { workspace = true, optional = true } parking_lot = { workspace = true, optional = true } +# misc +thiserror.workspace = true + [dev-dependencies] parking_lot.workspace = true reth-ethereum-forks.workspace = true diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index b75feea83a15..8a212030ecbc 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -107,6 +107,14 @@ pub trait ConfigureEvm: ConfigureEvmEnv { fn default_external_context<'a>(&self) -> Self::DefaultExternalContext<'a>; } +#[derive(thiserror::Error, Debug)] +/// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. +pub enum NextCfgError { + /// Another type of error that is not covered by the above variants. + #[error("Invalid params: {0}")] + InvalidConfigError(#[from] Box), +} + /// This represents the set of methods used to configure the EVM's environment before block /// execution. /// @@ -192,7 +200,7 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> (CfgEnvWithHandlerCfg, BlockEnv); + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError>; } /// Represents additional attributes required to configure the next block. diff --git a/crates/optimism/chainspec/Cargo.toml b/crates/optimism/chainspec/Cargo.toml index 6b068dabbf0c..c41e2148787c 100644 --- a/crates/optimism/chainspec/Cargo.toml +++ b/crates/optimism/chainspec/Cargo.toml @@ -36,6 +36,7 @@ serde_json.workspace = true # misc derive_more.workspace = true once_cell.workspace = true +thiserror.workspace = true [dev-dependencies] reth-chainspec = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 9ac2266467e3..9a8e571094ea 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -223,13 +223,16 @@ impl OpChainSpec { } } -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] /// Error type for decoding Holocene 1559 parameters pub enum DecodeError { + #[error("Insufficient data to decode")] /// Insufficient data to decode InsufficientData, + #[error("Invalid denominator parameter")] /// Invalid denominator parameter InvalidDenominator, + #[error("Invalid elasticity parameter")] /// Invalid elasticity parameter InvalidElasticity, } diff --git a/crates/optimism/evm/Cargo.toml b/crates/optimism/evm/Cargo.toml index f6b22ad14c8d..fd4ebd5f74e5 100644 --- a/crates/optimism/evm/Cargo.toml +++ b/crates/optimism/evm/Cargo.toml @@ -40,6 +40,7 @@ revm-primitives.workspace = true # misc derive_more.workspace = true tracing.workspace = true +thiserror.workspace = true [dev-dependencies] reth-evm = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 8dfaa1e20a91..f654b2472e45 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -14,7 +14,7 @@ extern crate alloc; use alloc::{sync::Arc, vec::Vec}; use alloy_primitives::{Address, U256}; -use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; +use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError}; use reth_optimism_chainspec::OpChainSpec; use reth_primitives::{ revm_primitives::{AnalysisKind, CfgEnvWithHandlerCfg, TxEnv}, @@ -134,7 +134,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { // configure evm env based on parent block let cfg = CfgEnv::default().with_chain_id(self.chain_spec.chain().id()); @@ -148,6 +148,11 @@ impl ConfigureEvmEnv for OptimismEvmConfig { .or_else(|| (spec_id.is_enabled_in(SpecId::CANCUN)).then_some(0)) .map(BlobExcessGasAndPrice::new); + let base_fee = self + .chain_spec + .next_block_base_fee(parent, attributes.timestamp) + .map_err(|e| NextCfgError::InvalidConfigError(e.to_string().into()))?; + let block_env = BlockEnv { number: U256::from(parent.number + 1), coinbase: attributes.suggested_fee_recipient, @@ -156,8 +161,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { prevrandao: Some(attributes.prev_randao), gas_limit: U256::from(parent.gas_limit), // calculate basefee based on parent block's gas usage - // basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp), - basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp).unwrap(), + basefee: base_fee, // calculate excess gas based on parent block's blob gas usage blob_excess_gas_and_price, }; @@ -170,7 +174,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { }; } - (cfg_with_handler_cfg, block_env) + Ok((cfg_with_handler_cfg, block_env)) } } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index ec95ce839d6b..a3d373fc1db7 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -6,7 +6,9 @@ use alloy_primitives::{B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; use reth_chainspec::{BaseFeeParams, ChainSpecProvider, EthereumHardforks}; -use reth_evm::{system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; +use reth_evm::{ + system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError, +}; use reth_execution_types::ExecutionOutcome; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism; @@ -79,7 +81,7 @@ where &self, config: &PayloadConfig, parent: &Header, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { let next_attributes = NextBlockEnvAttributes { timestamp: config.attributes.timestamp(), suggested_fee_recipient: config.attributes.suggested_fee_recipient(), @@ -103,7 +105,9 @@ where &self, args: BuildArguments, ) -> Result, PayloadBuilderError> { - let (cfg_env, block_env) = self.cfg_and_block_env(&args.config, &args.config.parent_block); + let (cfg_env, block_env) = self + .cfg_and_block_env(&args.config, &args.config.parent_block) + .map_err(PayloadBuilderError::other)?; optimism_payload( self.evm_config.clone(), args, @@ -138,7 +142,9 @@ where cancel: Default::default(), best_payload: None, }; - let (cfg_env, block_env) = self.cfg_and_block_env(&args.config, &args.config.parent_block); + let (cfg_env, block_env) = self + .cfg_and_block_env(&args.config, &args.config.parent_block) + .map_err(PayloadBuilderError::other)?; optimism_payload(self.evm_config.clone(), args, cfg_env, block_env, false)? .into_payload() .ok_or_else(|| PayloadBuilderError::MissingPayload) @@ -556,7 +562,7 @@ where } #[derive(Debug, thiserror::Error)] -// Error type for EIP-1559 parameters +/// Error type for EIP-1559 parameters pub enum EIP1559ParamError { #[error("No EIP-1559 parameters provided")] /// No EIP-1559 parameters provided diff --git a/examples/custom-evm/Cargo.toml b/examples/custom-evm/Cargo.toml index 53563ab9575b..f4a19d5a54f1 100644 --- a/examples/custom-evm/Cargo.toml +++ b/examples/custom-evm/Cargo.toml @@ -16,6 +16,7 @@ reth-node-ethereum = { workspace = true, features = ["test-utils"] } reth-tracing.workspace = true alloy-genesis.workspace = true alloy-primitives.workspace = true +reth-evm.workspace = true eyre.workspace = true tokio.workspace = true diff --git a/examples/custom-evm/src/main.rs b/examples/custom-evm/src/main.rs index 55063fc9bbcb..43a9e98108e5 100644 --- a/examples/custom-evm/src/main.rs +++ b/examples/custom-evm/src/main.rs @@ -23,6 +23,7 @@ use reth::{ transaction_pool::TransactionPool, }; use reth_chainspec::{Chain, ChainSpec}; +use reth_evm::NextCfgError; use reth_evm_ethereum::EthEvmConfig; use reth_node_api::{ ConfigureEvm, ConfigureEvmEnv, FullNodeTypes, NextBlockEnvAttributes, NodeTypes, @@ -115,7 +116,7 @@ impl ConfigureEvmEnv for MyEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { self.inner.next_cfg_and_block_env(parent, attributes) } } diff --git a/examples/stateful-precompile/Cargo.toml b/examples/stateful-precompile/Cargo.toml index 47a784c36e14..154390e0711a 100644 --- a/examples/stateful-precompile/Cargo.toml +++ b/examples/stateful-precompile/Cargo.toml @@ -15,6 +15,7 @@ reth-node-ethereum = { workspace = true, features = ["test-utils"] } reth-tracing.workspace = true alloy-genesis.workspace = true alloy-primitives.workspace = true +reth-evm.workspace = true eyre.workspace = true parking_lot.workspace = true diff --git a/examples/stateful-precompile/src/main.rs b/examples/stateful-precompile/src/main.rs index b0165e4de26c..679cd1f4bbb2 100644 --- a/examples/stateful-precompile/src/main.rs +++ b/examples/stateful-precompile/src/main.rs @@ -18,6 +18,7 @@ use reth::{ tasks::TaskManager, }; use reth_chainspec::{Chain, ChainSpec}; +use reth_evm::NextCfgError; use reth_node_api::{ConfigureEvm, ConfigureEvmEnv, FullNodeTypes, NodeTypes}; use reth_node_core::{args::RpcServerArgs, node_config::NodeConfig}; use reth_node_ethereum::{ @@ -175,7 +176,7 @@ impl ConfigureEvmEnv for MyEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> (CfgEnvWithHandlerCfg, BlockEnv) { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { self.inner.next_cfg_and_block_env(parent, attributes) } } From afa45b4b02f2832ec4f69cd65ed84471fe7a2fba Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Thu, 24 Oct 2024 01:11:58 -0400 Subject: [PATCH 08/19] add documentation --- 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 67b11d3ac994..52716ca68734 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -111,7 +111,7 @@ pub trait ConfigureEvm: ConfigureEvmEnv { /// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. pub enum NextCfgError { #[error("Invalid config: {0}")] - // This is a generic error type that can be used to wrap any error type that implements + /// This is a generic error type that can be used to wrap any error type that implements InvalidConfigError(#[from] Box), } From 4dc1a9ce866f28f6d05cfb4864d8a08b95159984 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Thu, 24 Oct 2024 01:47:44 -0400 Subject: [PATCH 09/19] optimize code --- crates/optimism/payload/src/builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 49558f0c34c8..1bb4efe8a5c3 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -616,14 +616,13 @@ fn get_holocene_extra_data( // Copy the values safely extra_data[1..5].copy_from_slice(&max_change_denominator.to_be_bytes()); extra_data[5..9].copy_from_slice(&elasticity_multiplier.to_be_bytes()); - Ok(Bytes::copy_from_slice(&extra_data)) } else { let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params) .map_err(|_| EIP1559ParamError::InvalidElasticity)?; extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); - Ok(Bytes::copy_from_slice(&extra_data)) } + Ok(Bytes::copy_from_slice(&extra_data)) } #[cfg(test)] From a9fd0549d76ceaf6c904a4e305e969e8f4893409 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Thu, 24 Oct 2024 18:19:22 -0400 Subject: [PATCH 10/19] fix test --- crates/optimism/payload/src/builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 84d3b6e0f017..78258d9d8cfc 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -578,10 +578,10 @@ pub enum EIP1559ParamError { /// Extracts the Holcene 1599 parameters from the encoded form: /// pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), EIP1559ParamError> { - let elasticity: [u8; 4] = + let denominator: [u8; 4] = eip_1559_params.0[..4].try_into().map_err(|_| EIP1559ParamError::InvalidElasticity)?; - let denominator: [u8; 4] = + let elasticity: [u8; 4] = eip_1559_params.0[4..8].try_into().map_err(|_| EIP1559ParamError::InvalidDenominator)?; Ok((u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator))) From 6fd2ffe1567cb87c2e45e38a72f947c880fb5bd3 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Mon, 28 Oct 2024 14:14:53 -0400 Subject: [PATCH 11/19] use this error no std --- Cargo.lock | 4 ++-- crates/evm/Cargo.toml | 2 +- crates/evm/src/lib.rs | 3 ++- crates/optimism/chainspec/Cargo.toml | 2 +- crates/optimism/chainspec/src/lib.rs | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 783764329aa3..ab6cc8f7f5d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7437,7 +7437,7 @@ dependencies = [ "reth-storage-errors", "revm", "revm-primitives", - "thiserror", + "thiserror-no-std", ] [[package]] @@ -8107,7 +8107,7 @@ dependencies = [ "reth-optimism-forks", "reth-primitives-traits", "serde_json", - "thiserror", + "thiserror-no-std", ] [[package]] diff --git a/crates/evm/Cargo.toml b/crates/evm/Cargo.toml index efd113c64b67..f7e15380fd1f 100644 --- a/crates/evm/Cargo.toml +++ b/crates/evm/Cargo.toml @@ -37,7 +37,7 @@ metrics = { workspace = true, optional = true } parking_lot = { workspace = true, optional = true } # misc -thiserror.workspace = true +thiserror-no-std.workspace = true [dev-dependencies] parking_lot.workspace = true diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index 52716ca68734..81dea3128647 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -18,6 +18,7 @@ extern crate alloc; use crate::builder::RethEvmBuilder; +use alloc::boxed::Box; use alloy_primitives::{Address, Bytes, B256, U256}; use reth_primitives::TransactionSigned; use reth_primitives_traits::BlockHeader; @@ -107,7 +108,7 @@ pub trait ConfigureEvm: ConfigureEvmEnv { fn default_external_context<'a>(&self) -> Self::DefaultExternalContext<'a>; } -#[derive(thiserror::Error, Debug)] +#[derive(Debug, thiserror_no_std::Error)] /// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. pub enum NextCfgError { #[error("Invalid config: {0}")] diff --git a/crates/optimism/chainspec/Cargo.toml b/crates/optimism/chainspec/Cargo.toml index 067c2f7235ea..2de12c8ad0f5 100644 --- a/crates/optimism/chainspec/Cargo.toml +++ b/crates/optimism/chainspec/Cargo.toml @@ -37,7 +37,7 @@ serde_json.workspace = true # misc derive_more.workspace = true once_cell.workspace = true -thiserror.workspace = true +thiserror-no-std.workspace = true [dev-dependencies] reth-chainspec = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 554062605593..83126e835e84 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -222,7 +222,7 @@ impl OpChainSpec { } } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, thiserror_no_std::Error)] /// Error type for decoding Holocene 1559 parameters pub enum DecodeError { #[error("Insufficient data to decode")] From 69aed697dd996eceecdcdaa2056c0371d26a867d Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Mon, 28 Oct 2024 14:21:46 -0400 Subject: [PATCH 12/19] revert back to thiserror --- Cargo.lock | 4 ++-- crates/evm/Cargo.toml | 2 +- crates/evm/src/lib.rs | 2 +- crates/optimism/chainspec/Cargo.toml | 2 +- crates/optimism/chainspec/src/lib.rs | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab6cc8f7f5d7..783764329aa3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7437,7 +7437,7 @@ dependencies = [ "reth-storage-errors", "revm", "revm-primitives", - "thiserror-no-std", + "thiserror", ] [[package]] @@ -8107,7 +8107,7 @@ dependencies = [ "reth-optimism-forks", "reth-primitives-traits", "serde_json", - "thiserror-no-std", + "thiserror", ] [[package]] diff --git a/crates/evm/Cargo.toml b/crates/evm/Cargo.toml index f7e15380fd1f..efd113c64b67 100644 --- a/crates/evm/Cargo.toml +++ b/crates/evm/Cargo.toml @@ -37,7 +37,7 @@ metrics = { workspace = true, optional = true } parking_lot = { workspace = true, optional = true } # misc -thiserror-no-std.workspace = true +thiserror.workspace = true [dev-dependencies] parking_lot.workspace = true diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index 81dea3128647..67f3251e434a 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -108,7 +108,7 @@ pub trait ConfigureEvm: ConfigureEvmEnv { fn default_external_context<'a>(&self) -> Self::DefaultExternalContext<'a>; } -#[derive(Debug, thiserror_no_std::Error)] +#[derive(Debug, thiserror::Error)] /// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. pub enum NextCfgError { #[error("Invalid config: {0}")] diff --git a/crates/optimism/chainspec/Cargo.toml b/crates/optimism/chainspec/Cargo.toml index 2de12c8ad0f5..067c2f7235ea 100644 --- a/crates/optimism/chainspec/Cargo.toml +++ b/crates/optimism/chainspec/Cargo.toml @@ -37,7 +37,7 @@ serde_json.workspace = true # misc derive_more.workspace = true once_cell.workspace = true -thiserror-no-std.workspace = true +thiserror.workspace = true [dev-dependencies] reth-chainspec = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 83126e835e84..85b22f1edd56 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -23,6 +23,7 @@ use alloy_genesis::Genesis; use alloy_primitives::{Bytes, Parity, Signature, B256, U256}; pub use base::BASE_MAINNET; pub use base_sepolia::BASE_SEPOLIA; +use core::error; use derive_more::{Constructor, Deref, From, Into}; pub use dev::OP_DEV; #[cfg(not(feature = "std"))] @@ -222,7 +223,7 @@ impl OpChainSpec { } } -#[derive(Debug, thiserror_no_std::Error)] +#[derive(Debug, thiserror::Error)] /// Error type for decoding Holocene 1559 parameters pub enum DecodeError { #[error("Insufficient data to decode")] From 8e1d67f94669ad6dd1cc086e23f96caa6b7b2175 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Mon, 28 Oct 2024 14:38:53 -0400 Subject: [PATCH 13/19] remove core error --- crates/optimism/chainspec/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 85b22f1edd56..554062605593 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -23,7 +23,6 @@ use alloy_genesis::Genesis; use alloy_primitives::{Bytes, Parity, Signature, B256, U256}; pub use base::BASE_MAINNET; pub use base_sepolia::BASE_SEPOLIA; -use core::error; use derive_more::{Constructor, Deref, From, Into}; pub use dev::OP_DEV; #[cfg(not(feature = "std"))] From 1f24f5fcab9d98bd58a718c14734a6d4e7aef7d5 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Mon, 28 Oct 2024 16:15:51 -0400 Subject: [PATCH 14/19] not use thisiserror for no std --- Cargo.lock | 3 +-- crates/evm/Cargo.toml | 2 +- crates/evm/src/lib.rs | 5 +++-- crates/optimism/chainspec/Cargo.toml | 1 - crates/optimism/chainspec/src/lib.rs | 10 +++++----- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 783764329aa3..9dcd551df69f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7420,6 +7420,7 @@ dependencies = [ "alloy-eips", "alloy-primitives", "auto_impl", + "derive_more 1.0.0", "futures-util", "metrics", "parking_lot", @@ -7437,7 +7438,6 @@ dependencies = [ "reth-storage-errors", "revm", "revm-primitives", - "thiserror", ] [[package]] @@ -8107,7 +8107,6 @@ dependencies = [ "reth-optimism-forks", "reth-primitives-traits", "serde_json", - "thiserror", ] [[package]] diff --git a/crates/evm/Cargo.toml b/crates/evm/Cargo.toml index efd113c64b67..79afc6dcdcd7 100644 --- a/crates/evm/Cargo.toml +++ b/crates/evm/Cargo.toml @@ -37,7 +37,7 @@ metrics = { workspace = true, optional = true } parking_lot = { workspace = true, optional = true } # misc -thiserror.workspace = true +derive_more.workspace = true [dev-dependencies] parking_lot.workspace = true diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index 67f3251e434a..f6971e2aa42e 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -20,6 +20,7 @@ extern crate alloc; use crate::builder::RethEvmBuilder; use alloc::boxed::Box; use alloy_primitives::{Address, Bytes, B256, U256}; +use derive_more::{Display, From}; use reth_primitives::TransactionSigned; use reth_primitives_traits::BlockHeader; use revm::{Database, Evm, GetInspector}; @@ -108,10 +109,10 @@ pub trait ConfigureEvm: ConfigureEvmEnv { fn default_external_context<'a>(&self) -> Self::DefaultExternalContext<'a>; } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, From, Display)] /// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. pub enum NextCfgError { - #[error("Invalid config: {0}")] + #[display("Invalid config error: {_0}")] /// This is a generic error type that can be used to wrap any error type that implements InvalidConfigError(#[from] Box), } diff --git a/crates/optimism/chainspec/Cargo.toml b/crates/optimism/chainspec/Cargo.toml index 067c2f7235ea..4e573ce29946 100644 --- a/crates/optimism/chainspec/Cargo.toml +++ b/crates/optimism/chainspec/Cargo.toml @@ -37,7 +37,6 @@ serde_json.workspace = true # misc derive_more.workspace = true once_cell.workspace = true -thiserror.workspace = true [dev-dependencies] reth-chainspec = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 554062605593..afa91d8fda5f 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -23,7 +23,7 @@ use alloy_genesis::Genesis; use alloy_primitives::{Bytes, Parity, Signature, B256, U256}; pub use base::BASE_MAINNET; pub use base_sepolia::BASE_SEPOLIA; -use derive_more::{Constructor, Deref, From, Into}; +use derive_more::{Constructor, Deref, Display, From, Into}; pub use dev::OP_DEV; #[cfg(not(feature = "std"))] pub(crate) use once_cell::sync::Lazy as LazyLock; @@ -222,16 +222,16 @@ impl OpChainSpec { } } -#[derive(Debug, thiserror::Error)] +#[derive(Clone, Debug, Display, Eq, PartialEq)] /// Error type for decoding Holocene 1559 parameters pub enum DecodeError { - #[error("Insufficient data to decode")] + #[display("Insufficient data to decode")] /// Insufficient data to decode InsufficientData, - #[error("Invalid denominator parameter")] + #[display("Invalid denominator parameter")] /// Invalid denominator parameter InvalidDenominator, - #[error("Invalid elasticity parameter")] + #[display("Invalid elasticity parameter")] /// Invalid elasticity parameter InvalidElasticity, } From fc7302d3db9c30620ff83308446707f4c523b38e Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:50:43 -0400 Subject: [PATCH 15/19] fix: use associated error type, remove trait --- Cargo.lock | 3 --- crates/ethereum/evm/src/lib.rs | 7 +++++-- crates/ethereum/payload/src/lib.rs | 4 ++-- crates/evm/Cargo.toml | 19 ++++++++----------- crates/evm/src/lib.rs | 15 ++++----------- crates/optimism/chainspec/src/lib.rs | 7 +++++++ crates/optimism/evm/src/lib.rs | 9 +++++---- crates/optimism/payload/src/builder.rs | 6 ++---- examples/custom-evm/Cargo.toml | 1 - examples/custom-evm/src/main.rs | 6 +++--- examples/stateful-precompile/Cargo.toml | 1 - examples/stateful-precompile/src/main.rs | 6 +++--- 12 files changed, 39 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9dcd551df69f..27b757eb0ec9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2870,7 +2870,6 @@ dependencies = [ "eyre", "reth", "reth-chainspec", - "reth-evm", "reth-evm-ethereum", "reth-node-api", "reth-node-core", @@ -3055,7 +3054,6 @@ dependencies = [ "parking_lot", "reth", "reth-chainspec", - "reth-evm", "reth-node-api", "reth-node-core", "reth-node-ethereum", @@ -7420,7 +7418,6 @@ dependencies = [ "alloy-eips", "alloy-primitives", "auto_impl", - "derive_more 1.0.0", "futures-util", "metrics", "parking_lot", diff --git a/crates/ethereum/evm/src/lib.rs b/crates/ethereum/evm/src/lib.rs index 482dd4ac9f74..1c340c0927ba 100644 --- a/crates/ethereum/evm/src/lib.rs +++ b/crates/ethereum/evm/src/lib.rs @@ -17,10 +17,12 @@ extern crate alloc; +use core::convert::Infallible; + use alloc::{sync::Arc, vec::Vec}; use alloy_primitives::{Address, Bytes, TxKind, U256}; use reth_chainspec::{ChainSpec, Head}; -use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError}; +use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; use reth_primitives::{transaction::FillTxEnv, Header, TransactionSigned}; use revm_primitives::{ AnalysisKind, BlobExcessGasAndPrice, BlockEnv, CfgEnv, CfgEnvWithHandlerCfg, Env, SpecId, TxEnv, @@ -59,6 +61,7 @@ impl EthEvmConfig { impl ConfigureEvmEnv for EthEvmConfig { type Header = Header; + type Error = Infallible; fn fill_tx_env(&self, tx_env: &mut TxEnv, transaction: &TransactionSigned, sender: Address) { transaction.fill_tx_env(tx_env, sender); @@ -131,7 +134,7 @@ impl ConfigureEvmEnv for EthEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), Self::Error> { // configure evm env based on parent block let cfg = CfgEnv::default().with_chain_id(self.chain_spec.chain().id()); diff --git a/crates/ethereum/payload/src/lib.rs b/crates/ethereum/payload/src/lib.rs index f6022b36b990..293f90c3bcc5 100644 --- a/crates/ethereum/payload/src/lib.rs +++ b/crates/ethereum/payload/src/lib.rs @@ -19,7 +19,7 @@ use reth_basic_payload_builder::{ use reth_chain_state::ExecutedBlock; use reth_chainspec::ChainSpec; use reth_errors::RethError; -use reth_evm::{system_calls::SystemCaller, ConfigureEvm, NextBlockEnvAttributes, NextCfgError}; +use reth_evm::{system_calls::SystemCaller, ConfigureEvm, NextBlockEnvAttributes}; use reth_evm_ethereum::{eip6110::parse_deposits_from_receipts, EthEvmConfig}; use reth_execution_types::ExecutionOutcome; use reth_payload_builder::{EthBuiltPayload, EthPayloadBuilderAttributes}; @@ -73,7 +73,7 @@ where &self, config: &PayloadConfig, parent: &Header, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), EvmConfig::Error> { let next_attributes = NextBlockEnvAttributes { timestamp: config.attributes.timestamp(), suggested_fee_recipient: config.attributes.suggested_fee_recipient(), diff --git a/crates/evm/Cargo.toml b/crates/evm/Cargo.toml index 79afc6dcdcd7..a4ce3c3893ba 100644 --- a/crates/evm/Cargo.toml +++ b/crates/evm/Cargo.toml @@ -36,9 +36,6 @@ futures-util.workspace = true metrics = { workspace = true, optional = true } parking_lot = { workspace = true, optional = true } -# misc -derive_more.workspace = true - [dev-dependencies] parking_lot.workspace = true reth-ethereum-forks.workspace = true @@ -60,12 +57,12 @@ std = [ "revm/std", ] test-utils = [ - "dep:parking_lot", - "reth-chainspec/test-utils", - "reth-consensus/test-utils", - "reth-primitives/test-utils", - "reth-primitives-traits/test-utils", - "reth-revm/test-utils", - "revm/test-utils", - "reth-prune-types/test-utils" + "dep:parking_lot", + "reth-chainspec/test-utils", + "reth-consensus/test-utils", + "reth-primitives/test-utils", + "reth-primitives-traits/test-utils", + "reth-revm/test-utils", + "revm/test-utils", + "reth-prune-types/test-utils" ] diff --git a/crates/evm/src/lib.rs b/crates/evm/src/lib.rs index f6971e2aa42e..e30ff9b1a7ad 100644 --- a/crates/evm/src/lib.rs +++ b/crates/evm/src/lib.rs @@ -18,9 +18,7 @@ extern crate alloc; use crate::builder::RethEvmBuilder; -use alloc::boxed::Box; use alloy_primitives::{Address, Bytes, B256, U256}; -use derive_more::{Display, From}; use reth_primitives::TransactionSigned; use reth_primitives_traits::BlockHeader; use revm::{Database, Evm, GetInspector}; @@ -109,14 +107,6 @@ pub trait ConfigureEvm: ConfigureEvmEnv { fn default_external_context<'a>(&self) -> Self::DefaultExternalContext<'a>; } -#[derive(Debug, From, Display)] -/// Error type for [`ConfigureEvmEnv::next_cfg_and_block_env`]. -pub enum NextCfgError { - #[display("Invalid config error: {_0}")] - /// This is a generic error type that can be used to wrap any error type that implements - InvalidConfigError(#[from] Box), -} - /// This represents the set of methods used to configure the EVM's environment before block /// execution. /// @@ -126,6 +116,9 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { /// The header type used by the EVM. type Header: BlockHeader; + /// The error type that is returned by [`Self::next_cfg_and_block_env`]. + type Error: core::error::Error + Send + Sync; + /// Returns a [`TxEnv`] from a [`TransactionSigned`] and [`Address`]. fn tx_env(&self, transaction: &TransactionSigned, signer: Address) -> TxEnv { let mut tx_env = TxEnv::default(); @@ -202,7 +195,7 @@ pub trait ConfigureEvmEnv: Send + Sync + Unpin + Clone + 'static { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError>; + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), Self::Error>; } /// Represents additional attributes required to configure the next block. diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index afa91d8fda5f..75186d73a268 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -236,6 +236,13 @@ pub enum DecodeError { InvalidElasticity, } +impl core::error::Error for DecodeError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + // None of the errors have sub-errors + None + } +} + /// Extracts the Holcene 1599 parameters from the encoded form: /// pub fn decode_holocene_1559_params(extra_data: Bytes) -> Result<(u32, u32), DecodeError> { diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index fd812837376e..92fc32ae93e1 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -14,8 +14,8 @@ extern crate alloc; use alloc::{sync::Arc, vec::Vec}; use alloy_primitives::{Address, U256}; -use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError}; -use reth_optimism_chainspec::OpChainSpec; +use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; +use reth_optimism_chainspec::{DecodeError, OpChainSpec}; use reth_primitives::{ revm_primitives::{AnalysisKind, CfgEnvWithHandlerCfg, TxEnv}, transaction::FillTxEnv, @@ -56,6 +56,7 @@ impl OptimismEvmConfig { impl ConfigureEvmEnv for OptimismEvmConfig { type Header = Header; + type Error = DecodeError; fn fill_tx_env(&self, tx_env: &mut TxEnv, transaction: &TransactionSigned, sender: Address) { transaction.fill_tx_env(tx_env, sender); @@ -134,7 +135,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), Self::Error> { // configure evm env based on parent block let cfg = CfgEnv::default().with_chain_id(self.chain_spec.chain().id()); @@ -159,7 +160,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { basefee: self .chain_spec .next_block_base_fee(parent, attributes.timestamp) - .map_err(|e| NextCfgError::InvalidConfigError(e.to_string().into()))?, + .map_err(|e| e.into())?, // calculate excess gas based on parent block's blob gas usage blob_excess_gas_and_price, }; diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index eaabc3a63728..28d8b3c5ed78 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -7,9 +7,7 @@ use alloy_primitives::{Bytes, B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; use reth_chainspec::{BaseFeeParams, ChainSpecProvider}; -use reth_evm::{ - system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes, NextCfgError, -}; +use reth_evm::{system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; use reth_execution_types::ExecutionOutcome; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism; @@ -80,7 +78,7 @@ where &self, config: &PayloadConfig, parent: &Header, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), EvmConfig::Error> { let next_attributes = NextBlockEnvAttributes { timestamp: config.attributes.timestamp(), suggested_fee_recipient: config.attributes.suggested_fee_recipient(), diff --git a/examples/custom-evm/Cargo.toml b/examples/custom-evm/Cargo.toml index f4a19d5a54f1..53563ab9575b 100644 --- a/examples/custom-evm/Cargo.toml +++ b/examples/custom-evm/Cargo.toml @@ -16,7 +16,6 @@ reth-node-ethereum = { workspace = true, features = ["test-utils"] } reth-tracing.workspace = true alloy-genesis.workspace = true alloy-primitives.workspace = true -reth-evm.workspace = true eyre.workspace = true tokio.workspace = true diff --git a/examples/custom-evm/src/main.rs b/examples/custom-evm/src/main.rs index 43a9e98108e5..16aad63c0932 100644 --- a/examples/custom-evm/src/main.rs +++ b/examples/custom-evm/src/main.rs @@ -23,7 +23,6 @@ use reth::{ transaction_pool::TransactionPool, }; use reth_chainspec::{Chain, ChainSpec}; -use reth_evm::NextCfgError; use reth_evm_ethereum::EthEvmConfig; use reth_node_api::{ ConfigureEvm, ConfigureEvmEnv, FullNodeTypes, NextBlockEnvAttributes, NodeTypes, @@ -39,7 +38,7 @@ use reth_primitives::{ Header, TransactionSigned, }; use reth_tracing::{RethTracer, Tracer}; -use std::sync::Arc; +use std::{convert::Infallible, sync::Arc}; /// Custom EVM configuration #[derive(Debug, Clone)] @@ -88,6 +87,7 @@ impl MyEvmConfig { impl ConfigureEvmEnv for MyEvmConfig { type Header = Header; + type Error = Infallible; fn fill_tx_env(&self, tx_env: &mut TxEnv, transaction: &TransactionSigned, sender: Address) { self.inner.fill_tx_env(tx_env, transaction, sender); @@ -116,7 +116,7 @@ impl ConfigureEvmEnv for MyEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), Self::Error> { self.inner.next_cfg_and_block_env(parent, attributes) } } diff --git a/examples/stateful-precompile/Cargo.toml b/examples/stateful-precompile/Cargo.toml index 154390e0711a..47a784c36e14 100644 --- a/examples/stateful-precompile/Cargo.toml +++ b/examples/stateful-precompile/Cargo.toml @@ -15,7 +15,6 @@ reth-node-ethereum = { workspace = true, features = ["test-utils"] } reth-tracing.workspace = true alloy-genesis.workspace = true alloy-primitives.workspace = true -reth-evm.workspace = true eyre.workspace = true parking_lot.workspace = true diff --git a/examples/stateful-precompile/src/main.rs b/examples/stateful-precompile/src/main.rs index 679cd1f4bbb2..371fbf4f78bc 100644 --- a/examples/stateful-precompile/src/main.rs +++ b/examples/stateful-precompile/src/main.rs @@ -18,7 +18,6 @@ use reth::{ tasks::TaskManager, }; use reth_chainspec::{Chain, ChainSpec}; -use reth_evm::NextCfgError; use reth_node_api::{ConfigureEvm, ConfigureEvmEnv, FullNodeTypes, NodeTypes}; use reth_node_core::{args::RpcServerArgs, node_config::NodeConfig}; use reth_node_ethereum::{ @@ -31,7 +30,7 @@ use reth_primitives::{ }; use reth_tracing::{RethTracer, Tracer}; use schnellru::{ByLength, LruMap}; -use std::{collections::HashMap, sync::Arc}; +use std::{collections::HashMap, convert::Infallible, sync::Arc}; /// Type alias for the LRU cache used within the [`PrecompileCache`]. type PrecompileLRUCache = LruMap<(Bytes, u64), PrecompileResult>; @@ -148,6 +147,7 @@ impl StatefulPrecompileMut for WrappedPrecompile { impl ConfigureEvmEnv for MyEvmConfig { type Header = Header; + type Error = Infallible; fn fill_tx_env(&self, tx_env: &mut TxEnv, transaction: &TransactionSigned, sender: Address) { self.inner.fill_tx_env(tx_env, transaction, sender) @@ -176,7 +176,7 @@ impl ConfigureEvmEnv for MyEvmConfig { &self, parent: &Self::Header, attributes: NextBlockEnvAttributes, - ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), NextCfgError> { + ) -> Result<(CfgEnvWithHandlerCfg, BlockEnv), Self::Error> { self.inner.next_cfg_and_block_env(parent, attributes) } } From a2524cadf7a42a087d05ae45e61943b047199b86 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:03:19 -0400 Subject: [PATCH 16/19] merge with main --- crates/optimism/payload/src/payload.rs | 76 ++++++++++++++++--- crates/optimism/rpc/src/eth/mod.rs | 9 +-- crates/optimism/rpc/src/eth/receipt.rs | 10 +-- crates/optimism/rpc/src/eth/transaction.rs | 6 +- crates/rpc/rpc-eth-api/src/helpers/block.rs | 3 +- crates/rpc/rpc-eth-api/src/helpers/receipt.rs | 10 +-- crates/rpc/rpc-eth-api/src/helpers/state.rs | 11 +-- .../rpc-eth-api/src/helpers/transaction.rs | 14 ++-- crates/rpc/rpc/src/eth/helpers/receipt.rs | 11 +-- crates/rpc/rpc/src/eth/helpers/state.rs | 10 +-- crates/rpc/rpc/src/eth/helpers/transaction.rs | 5 -- 11 files changed, 90 insertions(+), 75 deletions(-) diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index 5c0f2f29a753..b60ba555fa98 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -3,7 +3,7 @@ //! Optimism builder support use alloy_eips::{eip2718::Decodable2718, eip7685::Requests}; -use alloy_primitives::{Address, B256, B64, U256}; +use alloy_primitives::{keccak256, Address, B256, B64, U256}; use alloy_rlp::Encodable; use alloy_rpc_types_engine::{ExecutionPayloadEnvelopeV2, ExecutionPayloadV1, PayloadId}; /// Re-export for use in downstream arguments. @@ -48,9 +48,9 @@ impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { fn try_new( parent: B256, attributes: OpPayloadAttributes, - _version: u8, + version: u8, ) -> Result { - let id = payload_id_optimism(&parent, &attributes); + let id = payload_id_optimism(&parent, &attributes, version); let transactions = attributes .transactions @@ -284,7 +284,11 @@ impl From for OpExecutionPayloadEnvelopeV4 { /// Generates the payload id for the configured payload from the [`OpPayloadAttributes`]. /// /// Returns an 8-byte identifier by hashing the payload components with sha256 hash. -pub(crate) fn payload_id_optimism(parent: &B256, attributes: &OpPayloadAttributes) -> PayloadId { +pub(crate) fn payload_id_optimism( + parent: &B256, + attributes: &OpPayloadAttributes, + payload_version: u8, +) -> PayloadId { use sha2::Digest; let mut hasher = sha2::Sha256::new(); hasher.update(parent.as_slice()); @@ -302,19 +306,71 @@ pub(crate) fn payload_id_optimism(parent: &B256, attributes: &OpPayloadAttribute } let no_tx_pool = attributes.no_tx_pool.unwrap_or_default(); - hasher.update([no_tx_pool as u8]); - if let Some(txs) = &attributes.transactions { - txs.iter().for_each(|tx| hasher.update(tx)); + if no_tx_pool || attributes.transactions.as_ref().is_some_and(|txs| !txs.is_empty()) { + hasher.update([no_tx_pool as u8]); + let txs_len = attributes.transactions.as_ref().map(|txs| txs.len()).unwrap_or_default(); + hasher.update(&txs_len.to_be_bytes()[..]); + if let Some(txs) = &attributes.transactions { + for tx in txs { + // we have to just hash the bytes here because otherwise we would need to decode + // the transactions here which really isn't ideal + let tx_hash = keccak256(tx); + // maybe we can try just taking the hash and not decoding + hasher.update(tx_hash) + } + } } if let Some(gas_limit) = attributes.gas_limit { hasher.update(gas_limit.to_be_bytes()); } - if let Some(eip_1559_params) = &attributes.eip_1559_params { - hasher.update(eip_1559_params.0); + if let Some(eip_1559_params) = attributes.eip_1559_params { + hasher.update(eip_1559_params.as_slice()); } - let out = hasher.finalize(); + let mut out = hasher.finalize(); + out[0] = payload_version; PayloadId::new(out.as_slice()[..8].try_into().expect("sufficient length")) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::OpPayloadAttributes; + use alloy_primitives::{address, b256, bytes, FixedBytes}; + use alloy_rpc_types_engine::PayloadAttributes; + use reth_payload_primitives::EngineApiMessageVersion; + use std::str::FromStr; + + #[test] + fn test_payload_id_parity_op_geth() { + // INFO rollup_boost::server:received fork_choice_updated_v3 from builder and l2_client + // payload_id_builder="0x6ef26ca02318dcf9" payload_id_l2="0x03d2dae446d2a86a" + let expected = + PayloadId::new(FixedBytes::<8>::from_str("0x03d2dae446d2a86a").unwrap().into()); + let attrs = OpPayloadAttributes { + payload_attributes: PayloadAttributes { + timestamp: 1728933301, + prev_randao: b256!("9158595abbdab2c90635087619aa7042bbebe47642dfab3c9bfb934f6b082765"), + suggested_fee_recipient: address!("4200000000000000000000000000000000000011"), + withdrawals: Some([].into()), + parent_beacon_block_root: b256!("8fe0193b9bf83cb7e5a08538e494fecc23046aab9a497af3704f4afdae3250ff").into() + }, + transactions: Some([bytes!("7ef8f8a0dc19cfa777d90980e4875d0a548a881baaa3f83f14d1bc0d3038bc329350e54194deaddeaddeaddeaddeaddeaddeaddeaddead00019442000000000000000000000000000000000000158080830f424080b8a4440a5e20000f424000000000000000000000000300000000670d6d890000000000000125000000000000000000000000000000000000000000000000000000000000000700000000000000000000000000000000000000000000000000000000000000014bf9181db6e381d4384bbf69c48b0ee0eed23c6ca26143c6d2544f9d39997a590000000000000000000000007f83d659683caf2767fd3c720981d51f5bc365bc")].into()), + no_tx_pool: None, + gas_limit: Some(30000000), + eip_1559_params: None, + }; + + // Reth's `PayloadId` should match op-geth's `PayloadId`. This fails + assert_eq!( + expected, + payload_id_optimism( + &b256!("3533bf30edaf9505d0810bf475cbe4e5f4b9889904b9845e83efdeab4e92eb1e"), + &attrs, + EngineApiMessageVersion::V3 as u8 + ) + ); + } +} diff --git a/crates/optimism/rpc/src/eth/mod.rs b/crates/optimism/rpc/src/eth/mod.rs index 52acf4240352..7b427467b2c5 100644 --- a/crates/optimism/rpc/src/eth/mod.rs +++ b/crates/optimism/rpc/src/eth/mod.rs @@ -215,17 +215,12 @@ where } } -impl LoadState for OpEthApi -where +impl LoadState for OpEthApi where N: RpcNodeCore< Provider: StateProviderFactory + ChainSpecProvider, Pool: TransactionPool, - >, + > { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } } impl EthState for OpEthApi diff --git a/crates/optimism/rpc/src/eth/receipt.rs b/crates/optimism/rpc/src/eth/receipt.rs index f2f09cdc7ff4..3f2a81573e2f 100644 --- a/crates/optimism/rpc/src/eth/receipt.rs +++ b/crates/optimism/rpc/src/eth/receipt.rs @@ -11,7 +11,7 @@ use reth_optimism_forks::OptimismHardforks; use reth_primitives::{Receipt, TransactionMeta, TransactionSigned, TxType}; use reth_provider::ChainSpecProvider; use reth_rpc_eth_api::{helpers::LoadReceipt, FromEthApiError, RpcReceipt}; -use reth_rpc_eth_types::{EthApiError, EthStateCache, ReceiptBuilder}; +use reth_rpc_eth_types::{EthApiError, ReceiptBuilder}; use crate::{OpEthApi, OpEthApiError}; @@ -20,18 +20,14 @@ where Self: Send + Sync, N: FullNodeComponents>, { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } - async fn build_transaction_receipt( &self, tx: TransactionSigned, meta: TransactionMeta, receipt: Receipt, ) -> Result, Self::Error> { - let (block, receipts) = LoadReceipt::cache(self) + let (block, receipts) = self + .cache() .get_block_and_receipts(meta.block_hash) .await .map_err(Self::Error::from_eth_err)? diff --git a/crates/optimism/rpc/src/eth/transaction.rs b/crates/optimism/rpc/src/eth/transaction.rs index 5135b13a2ded..3345ac5d4521 100644 --- a/crates/optimism/rpc/src/eth/transaction.rs +++ b/crates/optimism/rpc/src/eth/transaction.rs @@ -12,7 +12,7 @@ use reth_rpc_eth_api::{ helpers::{EthSigner, EthTransactions, LoadTransaction, SpawnBlocking}, FromEthApiError, FullEthApiTypes, RpcNodeCore, TransactionCompat, }; -use reth_rpc_eth_types::{utils::recover_raw_transaction, EthStateCache}; +use reth_rpc_eth_types::utils::recover_raw_transaction; use reth_transaction_pool::{PoolTransaction, TransactionOrigin, TransactionPool}; use crate::{OpEthApi, SequencerClient}; @@ -59,10 +59,6 @@ where Self: SpawnBlocking + FullEthApiTypes, N: RpcNodeCore, { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } } impl OpEthApi diff --git a/crates/rpc/rpc-eth-api/src/helpers/block.rs b/crates/rpc/rpc-eth-api/src/helpers/block.rs index c777c64d420d..fa397db35e02 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/block.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/block.rs @@ -145,7 +145,8 @@ pub trait EthBlocks: LoadBlock { if let Some(block_hash) = self.provider().block_hash_for_id(block_id).map_err(Self::Error::from_eth_err)? { - return LoadReceipt::cache(self) + return self + .cache() .get_block_and_receipts(block_hash) .await .map_err(Self::Error::from_eth_err) diff --git a/crates/rpc/rpc-eth-api/src/helpers/receipt.rs b/crates/rpc/rpc-eth-api/src/helpers/receipt.rs index eae99bbe45d8..48394f1cd6bb 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/receipt.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/receipt.rs @@ -3,19 +3,13 @@ use futures::Future; use reth_primitives::{Receipt, TransactionMeta, TransactionSigned}; -use reth_rpc_eth_types::EthStateCache; -use crate::{EthApiTypes, RpcReceipt}; +use crate::{EthApiTypes, RpcNodeCoreExt, RpcReceipt}; /// Assembles transaction receipt data w.r.t to network. /// /// Behaviour shared by several `eth_` RPC methods, not exclusive to `eth_` receipts RPC methods. -pub trait LoadReceipt: EthApiTypes + Send + Sync { - /// Returns a handle for reading data from memory. - /// - /// Data access in default (L1) trait method implementations. - fn cache(&self) -> &EthStateCache; - +pub trait LoadReceipt: EthApiTypes + RpcNodeCoreExt + Send + Sync { /// Helper method for `eth_getBlockReceipts` and `eth_getTransactionReceipt`. fn build_transaction_receipt( &self, diff --git a/crates/rpc/rpc-eth-api/src/helpers/state.rs b/crates/rpc/rpc-eth-api/src/helpers/state.rs index 87e66cb74817..97c94b949323 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/state.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/state.rs @@ -13,12 +13,12 @@ use reth_provider::{ BlockIdReader, BlockNumReader, ChainSpecProvider, StateProvider, StateProviderBox, StateProviderFactory, }; -use reth_rpc_eth_types::{EthApiError, EthStateCache, PendingBlockEnv, RpcInvalidTransactionError}; +use reth_rpc_eth_types::{EthApiError, PendingBlockEnv, RpcInvalidTransactionError}; use reth_rpc_types_compat::proof::from_primitive_account_proof; use reth_transaction_pool::TransactionPool; use revm_primitives::{BlockEnv, CfgEnvWithHandlerCfg, SpecId}; -use crate::{EthApiTypes, FromEthApiError, RpcNodeCore}; +use crate::{EthApiTypes, FromEthApiError, RpcNodeCore, RpcNodeCoreExt}; use super::{EthApiSpec, LoadPendingBlock, SpawnBlocking}; @@ -170,17 +170,12 @@ pub trait EthState: LoadState + SpawnBlocking { /// Behaviour shared by several `eth_` RPC methods, not exclusive to `eth_` state RPC methods. pub trait LoadState: EthApiTypes - + RpcNodeCore< + + RpcNodeCoreExt< Provider: StateProviderFactory + ChainSpecProvider, Pool: TransactionPool, > { - /// Returns a handle for reading data from memory. - /// - /// Data access in default (L1) trait method implementations. - fn cache(&self) -> &EthStateCache; - /// Returns the state at the given block number fn state_at_hash(&self, block_hash: B256) -> Result { self.provider().history_by_block_hash(block_hash).map_err(Self::Error::from_eth_err) diff --git a/crates/rpc/rpc-eth-api/src/helpers/transaction.rs b/crates/rpc/rpc-eth-api/src/helpers/transaction.rs index af647fedf2c4..3c526cbb0253 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/transaction.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/transaction.rs @@ -15,14 +15,15 @@ use reth_primitives::{ use reth_provider::{BlockNumReader, BlockReaderIdExt, ReceiptProvider, TransactionsProvider}; use reth_rpc_eth_types::{ utils::{binary_search, recover_raw_transaction}, - EthApiError, EthStateCache, SignError, TransactionSource, + EthApiError, SignError, TransactionSource, }; use reth_rpc_types_compat::transaction::{from_recovered, from_recovered_with_block_context}; use reth_transaction_pool::{PoolTransaction, TransactionOrigin, TransactionPool}; use std::sync::Arc; use crate::{ - FromEthApiError, FullEthApiTypes, IntoEthApiError, RpcNodeCore, RpcReceipt, RpcTransaction, + FromEthApiError, FullEthApiTypes, IntoEthApiError, RpcNodeCore, RpcNodeCoreExt, RpcReceipt, + RpcTransaction, }; use super::{ @@ -461,13 +462,10 @@ pub trait EthTransactions: LoadTransaction { /// Behaviour shared by several `eth_` RPC methods, not exclusive to `eth_` transactions RPC /// methods. pub trait LoadTransaction: - SpawnBlocking + FullEthApiTypes + RpcNodeCore + SpawnBlocking + + FullEthApiTypes + + RpcNodeCoreExt { - /// Returns a handle for reading data from memory. - /// - /// Data access in default (L1) trait method implementations. - fn cache(&self) -> &EthStateCache; - /// Returns the transaction by hash. /// /// Checks the pool and state. diff --git a/crates/rpc/rpc/src/eth/helpers/receipt.rs b/crates/rpc/rpc/src/eth/helpers/receipt.rs index 570ec4fa3c0f..d0cb5867eac4 100644 --- a/crates/rpc/rpc/src/eth/helpers/receipt.rs +++ b/crates/rpc/rpc/src/eth/helpers/receipt.rs @@ -2,20 +2,15 @@ use alloy_serde::WithOtherFields; use reth_primitives::{Receipt, TransactionMeta, TransactionSigned}; -use reth_rpc_eth_api::{helpers::LoadReceipt, FromEthApiError, RpcReceipt}; -use reth_rpc_eth_types::{EthApiError, EthStateCache, ReceiptBuilder}; +use reth_rpc_eth_api::{helpers::LoadReceipt, FromEthApiError, RpcNodeCoreExt, RpcReceipt}; +use reth_rpc_eth_types::{EthApiError, ReceiptBuilder}; use crate::EthApi; impl LoadReceipt for EthApi where - Self: Send + Sync, + Self: RpcNodeCoreExt, { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } - async fn build_transaction_receipt( &self, tx: TransactionSigned, diff --git a/crates/rpc/rpc/src/eth/helpers/state.rs b/crates/rpc/rpc/src/eth/helpers/state.rs index 8c958ea2ae2d..a3e909cf6f6f 100644 --- a/crates/rpc/rpc/src/eth/helpers/state.rs +++ b/crates/rpc/rpc/src/eth/helpers/state.rs @@ -8,7 +8,6 @@ use reth_rpc_eth_api::{ helpers::{EthState, LoadState, SpawnBlocking}, RpcNodeCore, }; -use reth_rpc_eth_types::EthStateCache; use crate::EthApi; @@ -21,17 +20,12 @@ where } } -impl LoadState for EthApi -where +impl LoadState for EthApi where Self: RpcNodeCore< Provider: StateProviderFactory + ChainSpecProvider, Pool: TransactionPool, - >, + > { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } } #[cfg(test)] diff --git a/crates/rpc/rpc/src/eth/helpers/transaction.rs b/crates/rpc/rpc/src/eth/helpers/transaction.rs index 623db35e5ade..8ac0785b2620 100644 --- a/crates/rpc/rpc/src/eth/helpers/transaction.rs +++ b/crates/rpc/rpc/src/eth/helpers/transaction.rs @@ -5,7 +5,6 @@ use reth_rpc_eth_api::{ helpers::{EthSigner, EthTransactions, LoadTransaction, SpawnBlocking}, FullEthApiTypes, RpcNodeCore, }; -use reth_rpc_eth_types::EthStateCache; use reth_transaction_pool::TransactionPool; use crate::EthApi; @@ -28,10 +27,6 @@ where + FullEthApiTypes + RpcNodeCore, { - #[inline] - fn cache(&self) -> &EthStateCache { - self.inner.cache() - } } #[cfg(test)] From 878c88829d1a5ae6c53580beeb888c2cc592f392 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:29:59 -0400 Subject: [PATCH 17/19] make clippy happy --- Cargo.lock | 1 - crates/optimism/evm/Cargo.toml | 1 - crates/optimism/evm/src/lib.rs | 5 +---- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27b757eb0ec9..c7a47e8f81ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8192,7 +8192,6 @@ dependencies = [ "reth-revm", "revm", "revm-primitives", - "thiserror", "tracing", ] diff --git a/crates/optimism/evm/Cargo.toml b/crates/optimism/evm/Cargo.toml index fd4ebd5f74e5..f6b22ad14c8d 100644 --- a/crates/optimism/evm/Cargo.toml +++ b/crates/optimism/evm/Cargo.toml @@ -40,7 +40,6 @@ revm-primitives.workspace = true # misc derive_more.workspace = true tracing.workspace = true -thiserror.workspace = true [dev-dependencies] reth-evm = { workspace = true, features = ["test-utils"] } diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 92fc32ae93e1..03aecf2c83e8 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -157,10 +157,7 @@ impl ConfigureEvmEnv for OptimismEvmConfig { prevrandao: Some(attributes.prev_randao), gas_limit: U256::from(parent.gas_limit), // calculate basefee based on parent block's gas usage - basefee: self - .chain_spec - .next_block_base_fee(parent, attributes.timestamp) - .map_err(|e| e.into())?, + basefee: self.chain_spec.next_block_base_fee(parent, attributes.timestamp)?, // calculate excess gas based on parent block's blob gas usage blob_excess_gas_and_price, }; From 8fe8aa124155f691a6a57d753f0f20d267ba3b71 Mon Sep 17 00:00:00 2001 From: Cody Wang Date: Tue, 29 Oct 2024 12:12:20 -0400 Subject: [PATCH 18/19] nit fixes --- crates/optimism/chainspec/src/lib.rs | 4 ++-- crates/optimism/payload/src/builder.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index 75186d73a268..70adf2272cfb 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -194,14 +194,14 @@ impl OpChainSpec { parent: &Header, timestamp: u64, ) -> Result { - let is_holocene = self.inner.is_fork_active_at_timestamp( + let is_holocene_activated = self.inner.is_fork_active_at_timestamp( reth_optimism_forks::OptimismHardfork::Holocene, timestamp, ); // If we are in the Holocene, we need to use the base fee params // from the parent block's extra data. // Else, use the base fee params (default values) from chainspec - if is_holocene { + if is_holocene_activated { let (denominator, elasticity) = decode_holocene_1559_params(parent.extra_data.clone())?; if elasticity == 0 && denominator == 0 { return Ok(U256::from( diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 28d8b3c5ed78..38165617cc43 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -548,8 +548,8 @@ where } } -#[derive(Debug, thiserror::Error)] /// Error type for EIP-1559 parameters +#[derive(Debug, thiserror::Error)] pub enum EIP1559ParamError { #[error("No EIP-1559 parameters provided")] /// No EIP-1559 parameters provided From b22d02e7aede272ed59b68b4917b0d82e9ff7a34 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 30 Oct 2024 13:46:04 +0100 Subject: [PATCH 19/19] chore: touchups --- crates/optimism/node/src/engine.rs | 3 +- crates/optimism/payload/src/builder.rs | 106 +++---------------------- crates/optimism/payload/src/error.rs | 14 ++++ crates/optimism/payload/src/payload.rs | 59 +++++++++++++- 4 files changed, 84 insertions(+), 98 deletions(-) diff --git a/crates/optimism/node/src/engine.rs b/crates/optimism/node/src/engine.rs index 76ec3aa13f5e..966d87279c5a 100644 --- a/crates/optimism/node/src/engine.rs +++ b/crates/optimism/node/src/engine.rs @@ -158,8 +158,7 @@ where "MissingEip1559ParamsInPayloadAttributes".to_string().into(), )) }; - let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params) - .map_err(|e| EngineObjectValidationError::InvalidParams(e.to_string().into()))?; + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); if elasticity != 0 && denominator == 0 { return Err(EngineObjectValidationError::InvalidParams( "Eip1559ParamsDenominatorZero".to_string().into(), diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 2f2adc4db8f1..e9b7e2c76f80 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -3,10 +3,10 @@ use std::sync::Arc; use alloy_consensus::EMPTY_OMMER_ROOT_HASH; use alloy_eips::merge::BEACON_NONCE; -use alloy_primitives::{Bytes, B64, U256}; +use alloy_primitives::{B64, U256}; use reth_basic_payload_builder::*; use reth_chain_state::ExecutedBlock; -use reth_chainspec::{BaseFeeParams, ChainSpecProvider}; +use reth_chainspec::ChainSpecProvider; use reth_evm::{system_calls::SystemCaller, ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; use reth_execution_types::ExecutionOutcome; use reth_optimism_chainspec::OpChainSpec; @@ -479,11 +479,11 @@ where ); if is_holocene { - extra_data = get_holocene_extra_data( - &attributes, - chain_spec.base_fee_params_at_timestamp(attributes.payload_attributes.timestamp), - ) - .map_err(PayloadBuilderError::other)?; + extra_data = attributes + .get_holocene_extra_data( + chain_spec.base_fee_params_at_timestamp(attributes.payload_attributes.timestamp), + ) + .map_err(PayloadBuilderError::other)?; } let header = Header { @@ -549,93 +549,11 @@ where } } -/// Error type for EIP-1559 parameters -#[derive(Debug, thiserror::Error)] -pub enum EIP1559ParamError { - #[error("No EIP-1559 parameters provided")] - /// No EIP-1559 parameters provided - NoEIP1559Params, - #[error("Invalid elasticity parameter")] - /// Invalid denominator parameter - InvalidDenominator, - #[error("Invalid denominator parameter")] - /// Invalid elasticity parameter - InvalidElasticity, - #[error("Denominator overflow")] - /// Denominator overflow - DenominatorOverflow, - #[error("Elasticity overflow")] - /// Elasticity overflow - ElasticityOverflow, -} - -/// Extracts the Holcene 1599 parameters from the encoded form: +/// Extracts the Holocene 1599 parameters from the encoded form: /// -pub fn decode_eip_1559_params(eip_1559_params: B64) -> Result<(u32, u32), EIP1559ParamError> { - let denominator: [u8; 4] = - eip_1559_params.0[..4].try_into().map_err(|_| EIP1559ParamError::InvalidElasticity)?; - - let elasticity: [u8; 4] = - eip_1559_params.0[4..8].try_into().map_err(|_| EIP1559ParamError::InvalidDenominator)?; - - Ok((u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator))) -} - -fn get_holocene_extra_data( - attributes: &OptimismPayloadBuilderAttributes, - default_base_fee_params: BaseFeeParams, -) -> Result { - let eip_1559_params = attributes.eip_1559_params.ok_or(EIP1559ParamError::NoEIP1559Params)?; - - let mut extra_data = [0u8; 9]; - // If eip 1559 params aren't set, use the canyon base fee param constants - // otherwise use them - if eip_1559_params == B64::ZERO { - // Try casting max_change_denominator to u32 - let max_change_denominator: u32 = (default_base_fee_params.max_change_denominator) - .try_into() - .map_err(|_| EIP1559ParamError::DenominatorOverflow)?; - - // Try casting elasticity_multiplier to u32 - let elasticity_multiplier: u32 = (default_base_fee_params.elasticity_multiplier) - .try_into() - .map_err(|_| EIP1559ParamError::ElasticityOverflow)?; - - // Copy the values safely - extra_data[1..5].copy_from_slice(&max_change_denominator.to_be_bytes()); - extra_data[5..9].copy_from_slice(&elasticity_multiplier.to_be_bytes()); - } else { - let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params) - .map_err(|_| EIP1559ParamError::InvalidElasticity)?; - extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); - extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); - } - Ok(Bytes::copy_from_slice(&extra_data)) -} - -#[cfg(test)] -mod tests { - use std::str::FromStr; +pub fn decode_eip_1559_params(eip_1559_params: B64) -> (u32, u32) { + let denominator: [u8; 4] = eip_1559_params.0[..4].try_into().expect("sufficient length"); + let elasticity: [u8; 4] = eip_1559_params.0[4..8].try_into().expect("sufficient length"); - use super::*; - - #[test] - fn test_get_extra_data_post_holocene() { - let attributes = OptimismPayloadBuilderAttributes { - eip_1559_params: Some(B64::from_str("0x0000000800000008").unwrap()), - ..Default::default() - }; - let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); - assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 8, 0, 0, 0, 8])); - } - - #[test] - fn test_get_extra_data_post_holocene_default() { - let attributes = OptimismPayloadBuilderAttributes { - eip_1559_params: Some(B64::ZERO), - ..Default::default() - }; - let extra_data = get_holocene_extra_data(&attributes, BaseFeeParams::new(80, 60)); - assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 80, 0, 0, 0, 60])); - } + (u32::from_be_bytes(elasticity), u32::from_be_bytes(denominator)) } diff --git a/crates/optimism/payload/src/error.rs b/crates/optimism/payload/src/error.rs index 2016fdc6dd93..ce5f584a1cec 100644 --- a/crates/optimism/payload/src/error.rs +++ b/crates/optimism/payload/src/error.rs @@ -21,3 +21,17 @@ pub enum OptimismPayloadBuilderError { #[error("blob transaction included in sequencer block")] BlobTransactionRejected, } + +/// Error type for EIP-1559 parameters +#[derive(Debug, thiserror::Error)] +pub enum EIP1559ParamError { + /// No EIP-1559 parameters provided + #[error("No EIP-1559 parameters provided")] + NoEIP1559Params, + /// Denominator overflow + #[error("Denominator overflow")] + DenominatorOverflow, + /// Elasticity overflow + #[error("Elasticity overflow")] + ElasticityOverflow, +} diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index b60ba555fa98..056edfe7b638 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -2,8 +2,9 @@ //! Optimism builder support -use alloy_eips::{eip2718::Decodable2718, eip7685::Requests}; -use alloy_primitives::{keccak256, Address, B256, B64, U256}; +use crate::{builder::decode_eip_1559_params, error::EIP1559ParamError}; +use alloy_eips::{eip1559::BaseFeeParams, eip2718::Decodable2718, eip7685::Requests}; +use alloy_primitives::{keccak256, Address, Bytes, B256, B64, U256}; use alloy_rlp::Encodable; use alloy_rpc_types_engine::{ExecutionPayloadEnvelopeV2, ExecutionPayloadV1, PayloadId}; /// Re-export for use in downstream arguments. @@ -38,6 +39,40 @@ pub struct OptimismPayloadBuilderAttributes { pub eip_1559_params: Option, } +impl OptimismPayloadBuilderAttributes { + /// Extracts the `eip1559` parameters for the payload. + pub fn get_holocene_extra_data( + &self, + default_base_fee_params: BaseFeeParams, + ) -> Result { + let eip_1559_params = self.eip_1559_params.ok_or(EIP1559ParamError::NoEIP1559Params)?; + + let mut extra_data = [0u8; 9]; + // If eip 1559 params aren't set, use the canyon base fee param constants + // otherwise use them + if eip_1559_params.is_zero() { + // Try casting max_change_denominator to u32 + let max_change_denominator: u32 = (default_base_fee_params.max_change_denominator) + .try_into() + .map_err(|_| EIP1559ParamError::DenominatorOverflow)?; + + // Try casting elasticity_multiplier to u32 + let elasticity_multiplier: u32 = (default_base_fee_params.elasticity_multiplier) + .try_into() + .map_err(|_| EIP1559ParamError::ElasticityOverflow)?; + + // Copy the values safely + extra_data[1..5].copy_from_slice(&max_change_denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity_multiplier.to_be_bytes()); + } else { + let (elasticity, denominator) = decode_eip_1559_params(eip_1559_params); + extra_data[1..5].copy_from_slice(&denominator.to_be_bytes()); + extra_data[5..9].copy_from_slice(&elasticity.to_be_bytes()); + } + Ok(Bytes::copy_from_slice(&extra_data)) + } +} + impl PayloadBuilderAttributes for OptimismPayloadBuilderAttributes { type RpcPayloadAttributes = OpPayloadAttributes; type Error = alloy_rlp::Error; @@ -373,4 +408,24 @@ mod tests { ) ); } + + #[test] + fn test_get_extra_data_post_holocene() { + let attributes = OptimismPayloadBuilderAttributes { + eip_1559_params: Some(B64::from_str("0x0000000800000008").unwrap()), + ..Default::default() + }; + let extra_data = attributes.get_holocene_extra_data(BaseFeeParams::new(80, 60)); + assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 8, 0, 0, 0, 8])); + } + + #[test] + fn test_get_extra_data_post_holocene_default() { + let attributes = OptimismPayloadBuilderAttributes { + eip_1559_params: Some(B64::ZERO), + ..Default::default() + }; + let extra_data = attributes.get_holocene_extra_data(BaseFeeParams::new(80, 60)); + assert_eq!(extra_data.unwrap(), Bytes::copy_from_slice(&[0, 0, 0, 0, 80, 0, 0, 0, 60])); + } }