Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(op): storage root isthmus #13214

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions Cargo.lock

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

5 changes: 5 additions & 0 deletions crates/consensus/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ pub enum ConsensusError {
/// The block's timestamp.
timestamp: u64,
},
/// Custom error
// todo: remove in favour of AT Consensus::Error, so OpConsensusError can wrap ConsensusError
emhane marked this conversation as resolved.
Show resolved Hide resolved
// in a variant instead <https://github.com/paradigmxyz/reth/issues/13237>
#[display("custom l2 error (search for it in debug logs)")]
Other,
}

impl ConsensusError {
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/chainspec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ mod tests {
OpHardfork::Fjord.boxed(),
OpHardfork::Granite.boxed(),
OpHardfork::Holocene.boxed(),
// OpHardfork::Isthmus.boxed(),
//OpHardfork::Isthmus.boxed(),
];

assert!(expected_hardforks
Expand Down
4 changes: 4 additions & 0 deletions crates/optimism/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ reth-consensus-common.workspace = true
reth-consensus.workspace = true
reth-primitives.workspace = true
reth-trie-common.workspace = true
reth-storage-api.workspace = true
reth-storage-errors.workspace = true

# op-reth
reth-optimism-forks.workspace = true
Expand All @@ -31,6 +33,8 @@ alloy-primitives.workspace = true
alloy-consensus.workspace = true
alloy-trie.workspace = true

# misc
derive_more = { workspace = true, features = ["display", "error"] }
tracing.workspace = true

[dev-dependencies]
Expand Down
33 changes: 33 additions & 0 deletions crates/optimism/consensus/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//! Optimism consensus errors

use alloy_primitives::B256;
use derive_more::{Display, Error, From};
use reth_storage_errors::ProviderError;

/// Optimism consensus error.
#[derive(Debug, PartialEq, Eq, Clone, Display, Error, From)]
pub enum OpConsensusError {
/// Block body has non-empty withdrawals list.
#[display("non-empty withdrawals list")]
WithdrawalsNonEmpty,
/// Failed to load storage root of
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER).
#[display("failed to load storage root of L2toL1MessagePasser pre-deploy: {_0}")]
#[from]
LoadStorageRootFailed(ProviderError),
/// Storage root of
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER) missing
/// in block (withdrawals root field).
#[display("storage root of L2toL1MessagePasser missing (withdrawals root field empty)")]
StorageRootMissing,
/// Storage root of
/// [`L2toL1MessagePasser`](reth_optimism_primitives::ADDRESS_L2_TO_L1_MESSAGE_PASSER)
/// in block (withdrawals field), doesn't match local storage root.
#[display("L2toL1MessagePasser storage root mismatch, got: {}, expected {expected}", got.map(|hash| hash.to_string()).unwrap_or_else(|| "null".to_string()))]
StorageRootMismatch {
/// Storage root of pre-deploy in block.
got: Option<B256>,
/// Storage root of pre-deploy loaded from local state.
expected: B256,
},
}
84 changes: 73 additions & 11 deletions crates/optimism/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// The `optimism` feature must be enabled to use this crate.
#![cfg(feature = "optimism")]

use core::fmt;

use alloy_consensus::{BlockHeader, Header, EMPTY_OMMER_ROOT_HASH};
use alloy_primitives::{B64, U256};
use reth_chainspec::EthereumHardforks;
Expand All @@ -19,13 +21,18 @@ use reth_consensus_common::validation::{
validate_against_parent_4844, validate_against_parent_eip1559_base_fee,
validate_against_parent_hash_number, validate_against_parent_timestamp,
validate_body_against_header, validate_cancun_gas, validate_header_base_fee,
validate_header_extradata, validate_header_gas, validate_shanghai_withdrawals,
validate_header_extradata, validate_header_gas,
};
use reth_optimism_chainspec::OpChainSpec;
use reth_optimism_forks::OpHardforks;
use reth_optimism_primitives::OpPrimitives;
use reth_optimism_primitives::{predeploys::ADDRESS_L2_TO_L1_MESSAGE_PASSER, OpPrimitives};
use reth_primitives::{BlockBody, BlockWithSenders, GotExpected, SealedBlock, SealedHeader};
use reth_storage_api::{StateProviderFactory, StorageRootProvider};
use std::{sync::Arc, time::SystemTime};
use tracing::debug;

pub mod error;
pub use error::OpConsensusError;

mod proof;
pub use proof::calculate_receipt_root_no_memo_optimism;
Expand All @@ -37,19 +44,23 @@ pub use validation::validate_block_post_execution;
///
/// Provides basic checks as outlined in the execution specs.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct OpBeaconConsensus {
pub struct OpBeaconConsensus<P> {
/// Configuration
chain_spec: Arc<OpChainSpec>,
provider: P,
}

impl OpBeaconConsensus {
impl<P> OpBeaconConsensus<P> {
/// Create a new instance of [`OpBeaconConsensus`]
pub const fn new(chain_spec: Arc<OpChainSpec>) -> Self {
Self { chain_spec }
pub const fn new(chain_spec: Arc<OpChainSpec>, provider: P) -> Self {
Self { chain_spec, provider }
}
}

impl FullConsensus<OpPrimitives> for OpBeaconConsensus {
impl<P> FullConsensus<OpPrimitives> for OpBeaconConsensus<P>
where
P: StateProviderFactory + fmt::Debug,
emhane marked this conversation as resolved.
Show resolved Hide resolved
{
fn validate_block_post_execution(
&self,
block: &BlockWithSenders,
Expand All @@ -59,7 +70,10 @@ impl FullConsensus<OpPrimitives> for OpBeaconConsensus {
}
}

impl Consensus for OpBeaconConsensus {
impl<P> Consensus for OpBeaconConsensus<P>
where
P: StateProviderFactory + fmt::Debug,
{
fn validate_body_against_header(
&self,
body: &BlockBody,
Expand All @@ -83,19 +97,67 @@ impl Consensus for OpBeaconConsensus {
}

// EIP-4895: Beacon chain push withdrawals as operations
if self.chain_spec.is_shanghai_active_at_timestamp(block.timestamp) {
validate_shanghai_withdrawals(block)?;
if self.chain_spec.is_shanghai_active_at_timestamp(block.timestamp) &&
block.body.withdrawals.as_ref().is_some_and(|withdrawals| !withdrawals.is_empty())
{
debug!(target: "op::consensus",
block_number=block.number,
err=%OpConsensusError::WithdrawalsNonEmpty,
"block failed validation",
);
return Err(ConsensusError::Other)
}

if self.chain_spec.is_cancun_active_at_timestamp(block.timestamp) {
validate_cancun_gas(block)?;
}

if self.chain_spec.is_isthmus_active_at_timestamp(block.timestamp) {
let storage_root_msg_passer = self
Comment on lines +115 to +116
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is problematic in this function,

because we use this when we validate a downloaded block, and accessing the latest state on disk might not exists.

need to figure out how to make this work.

could we in this pr introduce the isthmus check as a standalone function only and figure out where we actually need to check this separately?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.provider
.latest()
.map_err(|err| {
debug!(target: "op::consensus",
block_number=block.number,
err=%OpConsensusError::LoadStorageRootFailed(err),
"failed to load latest state",
);

ConsensusError::Other
})?
.storage_root(ADDRESS_L2_TO_L1_MESSAGE_PASSER, Default::default())
.map_err(|err| {
debug!(target: "op::consensus",
block_number=block.number,
err=%OpConsensusError::LoadStorageRootFailed(err),
"failed to load storage root for L2toL1MessagePasser pre-deploy",
);

ConsensusError::Other
})?;

if block.withdrawals_root.is_none_or(|root| root != storage_root_msg_passer) {
debug!(target: "op::consensus",
block_number=block.number,
err=%OpConsensusError::StorageRootMismatch {
got: block.withdrawals_root,
expected: storage_root_msg_passer
},
"block failed validation",
);

return Err(ConsensusError::Other)
}
}

Ok(())
}
}

impl HeaderValidator for OpBeaconConsensus {
impl<P> HeaderValidator for OpBeaconConsensus<P>
where
P: Send + Sync + fmt::Debug,
{
fn validate_header(&self, header: &SealedHeader) -> Result<(), ConsensusError> {
validate_header_gas(header.header())?;
validate_header_base_fee(header.header(), &self.chain_spec)
Expand Down
12 changes: 9 additions & 3 deletions crates/optimism/node/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::{
OpEngineTypes,
};
use alloy_consensus::Header;
use core::fmt;
use reth_basic_payload_builder::{BasicPayloadJobGenerator, BasicPayloadJobGeneratorConfig};
use reth_chainspec::{EthChainSpec, EthereumHardforks, Hardforks};
use reth_db::transaction::{DbTx, DbTxMut};
Expand Down Expand Up @@ -152,6 +153,7 @@ impl OpNode {
ChainSpec = OpChainSpec,
Primitives = OpPrimitives,
>,
Provider: fmt::Debug,
>,
{
let RollupArgs { disable_txpool_gossip, compute_pending_block, discovery_v4, .. } = args;
Expand All @@ -177,6 +179,7 @@ where
Primitives = OpPrimitives,
Storage = OpStorage,
>,
Provider: fmt::Debug,
>,
{
type ComponentsBuilder = ComponentsBuilder<
Expand Down Expand Up @@ -679,12 +682,15 @@ pub struct OpConsensusBuilder;

impl<Node> ConsensusBuilder<Node> for OpConsensusBuilder
where
Node: FullNodeTypes<Types: NodeTypes<ChainSpec = OpChainSpec, Primitives = OpPrimitives>>,
Node: FullNodeTypes<
Types: NodeTypes<ChainSpec = OpChainSpec, Primitives = OpPrimitives>,
Provider: fmt::Debug,
>,
{
type Consensus = Arc<OpBeaconConsensus>;
type Consensus = Arc<OpBeaconConsensus<Node::Provider>>;

async fn build_consensus(self, ctx: &BuilderContext<Node>) -> eyre::Result<Self::Consensus> {
Ok(Arc::new(OpBeaconConsensus::new(ctx.chain_spec())))
Ok(Arc::new(OpBeaconConsensus::new(ctx.chain_spec(), ctx.provider().clone())))
}
}

Expand Down
3 changes: 3 additions & 0 deletions crates/optimism/node/tests/it/priority.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Node builder test that customizes priority of transactions in the block.
use core::fmt;

use alloy_consensus::TxEip1559;
use alloy_genesis::Genesis;
use alloy_network::TxSignerSync;
Expand Down Expand Up @@ -100,6 +102,7 @@ where
ChainSpec = OpChainSpec,
Primitives = OpPrimitives,
>,
Provider: fmt::Debug,
>,
{
let RollupArgs { disable_txpool_gossip, compute_pending_block, discovery_v4, .. } =
Expand Down
4 changes: 3 additions & 1 deletion crates/optimism/payload/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ reth-optimism-chainspec.workspace = true
reth-optimism-consensus.workspace = true
reth-optimism-evm.workspace = true
reth-optimism-forks.workspace = true
reth-optimism-primitives.workspace = true

# ethereum
revm.workspace = true
Expand All @@ -57,5 +58,6 @@ optimism = [
"reth-optimism-evm/optimism",
"revm/optimism",
"reth-execution-types/optimism",
"reth-optimism-consensus/optimism"
"reth-optimism-consensus/optimism",
"reth-optimism-primitives/optimism",
]
Loading
Loading