From d298fb1b81b8300c794bd14e9aa47a23c081f866 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 4 Dec 2024 15:27:49 +0100 Subject: [PATCH] fix(op): add missing op consensus validation check (#13122) --- crates/optimism/bin/Cargo.toml | 2 +- crates/optimism/chainspec/src/lib.rs | 39 ++++++++++++------- crates/optimism/cli/Cargo.toml | 2 +- crates/optimism/consensus/Cargo.toml | 5 ++- crates/optimism/consensus/src/lib.rs | 31 ++++++++++++--- crates/optimism/evm/Cargo.toml | 2 +- crates/optimism/node/Cargo.toml | 2 +- crates/optimism/primitives/Cargo.toml | 5 ++- crates/optimism/primitives/src/lib.rs | 2 +- .../primitives/src/transaction/signed.rs | 24 ++++++------ crates/optimism/rpc/Cargo.toml | 2 +- crates/storage/provider/Cargo.toml | 2 +- 12 files changed, 75 insertions(+), 43 deletions(-) diff --git a/crates/optimism/bin/Cargo.toml b/crates/optimism/bin/Cargo.toml index 60fde90f1914..b182a4f278a3 100644 --- a/crates/optimism/bin/Cargo.toml +++ b/crates/optimism/bin/Cargo.toml @@ -45,7 +45,7 @@ optimism = [ "reth-optimism-payload-builder/optimism", "reth-optimism-rpc/optimism", "reth-provider/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", ] dev = [ diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index a3dab80705e5..0ee86bc7d24b 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -21,7 +21,7 @@ use alloc::{boxed::Box, vec, vec::Vec}; use alloy_chains::Chain; use alloy_consensus::Header; use alloy_genesis::Genesis; -use alloy_primitives::{Bytes, B256, U256}; +use alloy_primitives::{B256, U256}; pub use base::BASE_MAINNET; pub use base_sepolia::BASE_SEPOLIA; use derive_more::{Constructor, Deref, Display, From, Into}; @@ -185,6 +185,28 @@ pub struct OpChainSpec { } impl OpChainSpec { + /// Extracts the Holcene 1599 parameters from the encoded extradata from the parent header. + /// + /// Caution: Caller must ensure that holocene is active in the parent header. + /// + /// See also [Base fee computation](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/holocene/exec-engine.md#base-fee-computation) + pub fn decode_holocene_base_fee( + &self, + parent: &Header, + timestamp: u64, + ) -> Result { + let (denominator, elasticity) = decode_holocene_1559_params(&parent.extra_data)?; + let base_fee = if elasticity == 0 && denominator == 0 { + parent + .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) + .unwrap_or_default() + } else { + let base_fee_params = BaseFeeParams::new(denominator as u128, elasticity as u128); + parent.next_block_base_fee(base_fee_params).unwrap_or_default() + }; + Ok(base_fee) + } + /// Read from parent to determine the base fee for the next block /// /// See also [Base fee computation](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/holocene/exec-engine.md#base-fee-computation) @@ -204,16 +226,7 @@ impl OpChainSpec { // from the parent block's extra data. // Else, use the base fee params (default values) from chainspec if is_holocene_activated { - 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())) + Ok(U256::from(self.decode_holocene_base_fee(parent, timestamp)?)) } else { Ok(U256::from( parent @@ -247,7 +260,7 @@ impl core::error::Error for DecodeError { /// Extracts the Holcene 1599 parameters from the encoded form: /// -pub fn decode_holocene_1559_params(extra_data: Bytes) -> Result<(u32, u32), DecodeError> { +pub fn decode_holocene_1559_params(extra_data: &[u8]) -> Result<(u32, u32), DecodeError> { if extra_data.len() < 9 { return Err(DecodeError::InsufficientData); } @@ -492,7 +505,7 @@ mod tests { use std::sync::Arc; use alloy_genesis::{ChainConfig, Genesis}; - use alloy_primitives::b256; + use alloy_primitives::{b256, Bytes}; use reth_chainspec::{test_fork_ids, BaseFeeParams, BaseFeeParamsKind}; use reth_ethereum_forks::{EthereumHardfork, ForkCondition, ForkHash, ForkId, Head}; use reth_optimism_forks::{OpHardfork, OpHardforks}; diff --git a/crates/optimism/cli/Cargo.toml b/crates/optimism/cli/Cargo.toml index 48ea2d07decc..4e18b51160e5 100644 --- a/crates/optimism/cli/Cargo.toml +++ b/crates/optimism/cli/Cargo.toml @@ -95,7 +95,7 @@ optimism = [ "reth-execution-types/optimism", "reth-db/optimism", "reth-db-api/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", "reth-downloaders/optimism" ] asm-keccak = [ diff --git a/crates/optimism/consensus/Cargo.toml b/crates/optimism/consensus/Cargo.toml index 30f16e4eb228..faece6eacf87 100644 --- a/crates/optimism/consensus/Cargo.toml +++ b/crates/optimism/consensus/Cargo.toml @@ -22,7 +22,8 @@ reth-trie-common.workspace = true # op-reth reth-optimism-forks.workspace = true reth-optimism-chainspec.workspace = true -reth-optimism-primitives.workspace = true +# TODO: remove this after feature cleanup +reth-optimism-primitives = { workspace = true, features = ["serde"] } # ethereum alloy-primitives.workspace = true @@ -36,4 +37,4 @@ alloy-primitives.workspace = true reth-optimism-chainspec.workspace = true [features] -optimism = ["reth-primitives/optimism"] +optimism = ["reth-primitives/optimism", "reth-optimism-primitives/optimism"] diff --git a/crates/optimism/consensus/src/lib.rs b/crates/optimism/consensus/src/lib.rs index b50efd5f6f26..6d457f42c901 100644 --- a/crates/optimism/consensus/src/lib.rs +++ b/crates/optimism/consensus/src/lib.rs @@ -9,7 +9,7 @@ // The `optimism` feature must be enabled to use this crate. #![cfg(feature = "optimism")] -use alloy_consensus::{Header, EMPTY_OMMER_ROOT_HASH}; +use alloy_consensus::{BlockHeader, Header, EMPTY_OMMER_ROOT_HASH}; use alloy_primitives::{B64, U256}; use reth_chainspec::EthereumHardforks; use reth_consensus::{ @@ -112,11 +112,30 @@ impl HeaderValidator for OpBeaconConsensus { validate_against_parent_timestamp(header.header(), parent.header())?; } - validate_against_parent_eip1559_base_fee( - header.header(), - parent.header(), - &self.chain_spec, - )?; + // EIP1559 base fee validation + // + // > if Holocene is active in parent_header.timestamp, then the parameters from + // > parent_header.extraData are used. + if self.chain_spec.is_holocene_active_at_timestamp(parent.timestamp) { + let header_base_fee = + header.base_fee_per_gas().ok_or(ConsensusError::BaseFeeMissing)?; + let expected_base_fee = self + .chain_spec + .decode_holocene_base_fee(parent, header.timestamp) + .map_err(|_| ConsensusError::BaseFeeMissing)?; + if expected_base_fee != header_base_fee { + return Err(ConsensusError::BaseFeeDiff(GotExpected { + expected: expected_base_fee, + got: header_base_fee, + })) + } + } else { + validate_against_parent_eip1559_base_fee( + header.header(), + parent.header(), + &self.chain_spec, + )?; + } // ensure that the blob gas fields for this block if self.chain_spec.is_cancun_active_at_timestamp(header.timestamp) { diff --git a/crates/optimism/evm/Cargo.toml b/crates/optimism/evm/Cargo.toml index 309ddc1cb4e1..ab22e3e3e813 100644 --- a/crates/optimism/evm/Cargo.toml +++ b/crates/optimism/evm/Cargo.toml @@ -75,5 +75,5 @@ optimism = [ "reth-optimism-consensus/optimism", "revm/optimism", "revm-primitives/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", ] diff --git a/crates/optimism/node/Cargo.toml b/crates/optimism/node/Cargo.toml index b0b7065f3363..b833342282a6 100644 --- a/crates/optimism/node/Cargo.toml +++ b/crates/optimism/node/Cargo.toml @@ -97,7 +97,7 @@ optimism = [ "reth-db/optimism", "reth-optimism-node/optimism", "reth-node-core/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", ] asm-keccak = [ "reth-primitives/asm-keccak", diff --git a/crates/optimism/primitives/Cargo.toml b/crates/optimism/primitives/Cargo.toml index 38f76aa6256a..ed8e9686fa7d 100644 --- a/crates/optimism/primitives/Cargo.toml +++ b/crates/optimism/primitives/Cargo.toml @@ -101,6 +101,7 @@ arbitrary = [ "revm-primitives/arbitrary", "rand", ] -op = [ - "revm-primitives/optimism", +optimism = [ + "revm-primitives/optimism", + "reth-primitives/optimism" ] diff --git a/crates/optimism/primitives/src/lib.rs b/crates/optimism/primitives/src/lib.rs index df5042110217..b1f029d20bc2 100644 --- a/crates/optimism/primitives/src/lib.rs +++ b/crates/optimism/primitives/src/lib.rs @@ -7,7 +7,7 @@ )] #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] // The `optimism` feature must be enabled to use this crate. -#![cfg(feature = "op")] +#![cfg(feature = "optimism")] #![cfg_attr(not(test), warn(unused_crate_dependencies))] #![cfg_attr(not(feature = "std"), no_std)] diff --git a/crates/optimism/primitives/src/transaction/signed.rs b/crates/optimism/primitives/src/transaction/signed.rs index 2dc72026e7cb..26581214e67b 100644 --- a/crates/optimism/primitives/src/transaction/signed.rs +++ b/crates/optimism/primitives/src/transaction/signed.rs @@ -1,13 +1,7 @@ //! A signed Optimism transaction. +use crate::{OpTransaction, OpTxType}; use alloc::vec::Vec; -use core::{ - hash::{Hash, Hasher}, - mem, -}; -#[cfg(feature = "std")] -use std::sync::OnceLock; - use alloy_consensus::{ transaction::RlpEcdsaTx, SignableTransaction, Transaction, TxEip1559, TxEip2930, TxEip7702, }; @@ -20,6 +14,10 @@ use alloy_primitives::{ keccak256, Address, Bytes, PrimitiveSignature as Signature, TxHash, TxKind, Uint, B256, U256, }; use alloy_rlp::Header; +use core::{ + hash::{Hash, Hasher}, + mem, +}; use derive_more::{AsRef, Deref}; #[cfg(not(feature = "std"))] use once_cell::sync::OnceCell as OnceLock; @@ -32,8 +30,8 @@ use reth_primitives::{ }; use reth_primitives_traits::{FillTxEnv, InMemorySize, SignedTransaction}; use revm_primitives::{AuthorizationList, OptimismFields, TxEnv}; - -use crate::{OpTransaction, OpTxType}; +#[cfg(feature = "std")] +use std::sync::OnceLock; /// Signed transaction. #[cfg_attr(any(test, feature = "reth-codec"), reth_codecs::add_arbitrary_tests(rlp))] @@ -105,10 +103,6 @@ impl SignedTransaction for OpTransactionSigned { recover_signer_unchecked(signature, signature_hash) } - fn recalculate_hash(&self) -> B256 { - keccak256(self.encoded_2718()) - } - fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. @@ -119,6 +113,10 @@ impl SignedTransaction for OpTransactionSigned { let signature_hash = keccak256(buf); recover_signer_unchecked(&self.signature, signature_hash) } + + fn recalculate_hash(&self) -> B256 { + keccak256(self.encoded_2718()) + } } impl FillTxEnv for OpTransactionSigned { diff --git a/crates/optimism/rpc/Cargo.toml b/crates/optimism/rpc/Cargo.toml index 9894dd8a3dba..968beaf9e833 100644 --- a/crates/optimism/rpc/Cargo.toml +++ b/crates/optimism/rpc/Cargo.toml @@ -73,5 +73,5 @@ optimism = [ "revm/optimism", "reth-optimism-consensus/optimism", "reth-optimism-payload-builder/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", ] diff --git a/crates/storage/provider/Cargo.toml b/crates/storage/provider/Cargo.toml index f6d577aadbec..84808ed7c381 100644 --- a/crates/storage/provider/Cargo.toml +++ b/crates/storage/provider/Cargo.toml @@ -96,7 +96,7 @@ optimism = [ "reth-db/optimism", "reth-db-api/optimism", "revm/optimism", - "reth-optimism-primitives/op", + "reth-optimism-primitives/optimism", ] serde = [ "dashmap/serde",