Skip to content

Commit

Permalink
fix(op): add missing op consensus validation check (paradigmxyz#13122)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse authored Dec 4, 2024
1 parent 24af0a8 commit d298fb1
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 43 deletions.
2 changes: 1 addition & 1 deletion crates/optimism/bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
39 changes: 26 additions & 13 deletions crates/optimism/chainspec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<u64, DecodeError> {
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)
Expand All @@ -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
Expand Down Expand Up @@ -247,7 +260,7 @@ impl core::error::Error for DecodeError {

/// Extracts the Holcene 1599 parameters from the encoded form:
/// <https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/holocene/exec-engine.md#eip1559params-encoding>
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);
}
Expand Down Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
5 changes: 3 additions & 2 deletions crates/optimism/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"]
31 changes: 25 additions & 6 deletions crates/optimism/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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
// <https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/holocene/exec-engine.md#base-fee-computation>
// > 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) {
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@ optimism = [
"reth-optimism-consensus/optimism",
"revm/optimism",
"revm-primitives/optimism",
"reth-optimism-primitives/op",
"reth-optimism-primitives/optimism",
]
2 changes: 1 addition & 1 deletion crates/optimism/node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions crates/optimism/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ arbitrary = [
"revm-primitives/arbitrary",
"rand",
]
op = [
"revm-primitives/optimism",
optimism = [
"revm-primitives/optimism",
"reth-primitives/optimism"
]
2 changes: 1 addition & 1 deletion crates/optimism/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down
24 changes: 11 additions & 13 deletions crates/optimism/primitives/src/transaction/signed.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand All @@ -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;
Expand All @@ -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))]
Expand Down Expand Up @@ -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<u8>) -> Option<Address> {
// Optimism's Deposit transaction does not have a signature. Directly return the
// `from` address.
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ optimism = [
"revm/optimism",
"reth-optimism-consensus/optimism",
"reth-optimism-payload-builder/optimism",
"reth-optimism-primitives/op",
"reth-optimism-primitives/optimism",
]
2 changes: 1 addition & 1 deletion crates/storage/provider/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit d298fb1

Please sign in to comment.