diff --git a/CHANGELOG.md b/CHANGELOG.md index b86f4793dd4..e1b62716ce0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ This changelog covers Firehose-specific changes only. For upstream reth changes, The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## reth-v2.7.0-fh3.1-2 + +### Fixed + +- Staged sync no longer rejects Amsterdam blocks with `BlockAccessListHashMissing`. The Firehose + block executor now rebuilds each block's access list on both its traced and untraced paths and + hands it to the execution stage for validation. + +### Changed + +- `reth_firehose::run_wrapped_block` leaves the rebuilt block access list in the state. Call the + new `reth_firehose::take_traced_block_access_list` afterwards to check its hash and record its + RLP on the traced block; `FirehoseBlockExecutor` already does. + ## reth-v2.7.0-fh3.1-1 ### Fixed diff --git a/crates/firehose-tests/Cargo.toml b/crates/firehose-tests/Cargo.toml index 05d5f784a64..cf700bc6480 100644 --- a/crates/firehose-tests/Cargo.toml +++ b/crates/firehose-tests/Cargo.toml @@ -48,3 +48,7 @@ path = "tests/prestate.rs" [[test]] name = "batch_executor" path = "tests/batch_executor.rs" + +[[test]] +name = "staged_sync_bal" +path = "tests/staged_sync_bal.rs" diff --git a/crates/firehose-tests/src/lib.rs b/crates/firehose-tests/src/lib.rs index 5dbdb744eac..a1742b8d477 100644 --- a/crates/firehose-tests/src/lib.rs +++ b/crates/firehose-tests/src/lib.rs @@ -15,6 +15,7 @@ pub mod prestate; pub use prestate::{ - assert_block_equals_golden, build_account_info, decode_hex, parse_fire_block_for, run_prestate, - seed_cache_db, Prestate, RunOutcome, TraceContext, + assert_block_equals_golden, build_account_info, decode_hex, load_prestate, + parse_fire_block_for, run_prestate, seed_cache_db, LoadedPrestate, Prestate, RunOutcome, + TraceContext, }; diff --git a/crates/firehose-tests/src/prestate.rs b/crates/firehose-tests/src/prestate.rs index ee2759c1e9b..7c8f1728ee1 100644 --- a/crates/firehose-tests/src/prestate.rs +++ b/crates/firehose-tests/src/prestate.rs @@ -25,7 +25,10 @@ use prost::Message; use reth_chainspec::ChainSpec; use reth_ethereum_primitives::{Block, BlockBody, EthPrimitives, TransactionSigned}; use reth_evm_ethereum::EthEvmConfig; -use reth_firehose::{run_wrapped_block, FirehoseBlockTracer, NoPostTxExtras, NoPreTxAdjust}; +use reth_firehose::{ + run_wrapped_block, take_traced_block_access_list, FirehoseBlockTracer, NoPostTxExtras, + NoPreTxAdjust, +}; use reth_primitives_traits::{Block as _, RecoveredBlock}; use reth_revm::State; use revm::{ @@ -90,6 +93,19 @@ pub struct TraceContext { pub block_access_list_hash: Option, } +/// A test case's `prestate.json`, turned into what executing its block needs. +#[derive(Debug)] +pub struct LoadedPrestate { + /// The parsed fixture. + pub prestate: Prestate, + /// Chain spec built from the fixture's genesis. + pub chain_spec: Arc, + /// The block to execute, carrying the fixture's single transaction. + pub block: RecoveredBlock, + /// State database seeded with the genesis allocation. + pub db: CacheDB, +} + /// Run the prestate-driven Firehose harness against `case_folder` and return the captured Block. /// /// `case_folder` must contain `prestate.json` (Geth-style genesis + block context + RLP-encoded @@ -97,33 +113,7 @@ pub struct TraceContext { /// `FIRE BLOCK` line emitted for the executed block; assert against a `.binpb` golden via /// [`assert_block_equals_golden`]. pub fn run_prestate(case_folder: &Path) -> eyre::Result { - let prestate_path = case_folder.join("prestate.json"); - let prestate: Prestate = serde_json::from_slice(&std::fs::read(&prestate_path)?) - .with_context(|| format!("reading prestate.json at {}", prestate_path.display()))?; - - let chain_spec = Arc::new(ChainSpec::from(prestate.genesis.clone())); - let parent_hash = chain_spec.genesis_hash(); - - let tx_bytes = decode_hex(&prestate.input).context("decoding prestate.input hex")?; - let signed_tx = TransactionSigned::network_decode(&mut tx_bytes.as_slice()) - .context("RLP-decoding prestate.input as a signed transaction")?; - - let transactions = vec![signed_tx]; - let header = build_header(&prestate.context, parent_hash, &transactions); - - // Match Geth's `extblock` RLP shape on Shanghai+ chains: 4-element list - // [header, txs, uncles, withdrawals] where the withdrawals slot is always present (empty - // list when the block has no withdrawals). Reth's encoder elides the slot entirely when - // `withdrawals: None`, producing a 3-element list and a 1-byte-shorter `block.size`. - let block = Block { - header, - body: BlockBody { transactions, ommers: vec![], withdrawals: Some(Withdrawals::default()) }, - }; - let recovered: RecoveredBlock = - block.try_into_recovered().map_err(|_| eyre::eyre!("recovering tx senders"))?; - - let mut db = CacheDB::new(EmptyDB::default()); - seed_cache_db(&mut db, &prestate.genesis)?; + let LoadedPrestate { prestate, chain_spec, block: recovered, db } = load_prestate(case_folder)?; let mut state = State::builder().with_database(db).with_bundle_update().build(); let evm_config = EthEvmConfig::new(chain_spec.clone()); @@ -164,7 +154,11 @@ pub fn run_prestate(case_folder: &Path) -> eyre::Result { &mut block_tracer, NoPreTxAdjust, NoPostTxExtras, - ); + ) + .and_then(|result| { + take_traced_block_access_list(&mut state, &recovered, &mut block_tracer)?; + Ok(result) + }); match exec_result { Ok(_) => block_tracer.mark_verified(), @@ -182,6 +176,39 @@ pub fn run_prestate(case_folder: &Path) -> eyre::Result { Ok(RunOutcome { block, raw }) } +/// Loads `case_folder`'s `prestate.json` into the block it describes and a state database seeded +/// with its genesis allocation. +pub fn load_prestate(case_folder: &Path) -> eyre::Result { + let prestate_path = case_folder.join("prestate.json"); + let prestate: Prestate = serde_json::from_slice(&std::fs::read(&prestate_path)?) + .with_context(|| format!("reading prestate.json at {}", prestate_path.display()))?; + + let chain_spec = Arc::new(ChainSpec::from(prestate.genesis.clone())); + let parent_hash = chain_spec.genesis_hash(); + + let tx_bytes = decode_hex(&prestate.input).context("decoding prestate.input hex")?; + let signed_tx = TransactionSigned::network_decode(&mut tx_bytes.as_slice()) + .context("RLP-decoding prestate.input as a signed transaction")?; + + let transactions = vec![signed_tx]; + let header = build_header(&prestate.context, parent_hash, &transactions); + + // Match Geth's `extblock` RLP shape on Shanghai+ chains: 4-element list + // [header, txs, uncles, withdrawals] where the withdrawals slot is always present (empty + // list when the block has no withdrawals). Reth's encoder elides the slot entirely when + // `withdrawals: None`, producing a 3-element list and a 1-byte-shorter `block.size`. + let block = Block { + header, + body: BlockBody { transactions, ommers: vec![], withdrawals: Some(Withdrawals::default()) }, + }; + let recovered: RecoveredBlock = + block.try_into_recovered().map_err(|_| eyre::eyre!("recovering tx senders"))?; + + let mut db = CacheDB::new(EmptyDB::default()); + seed_cache_db(&mut db, &prestate.genesis)?; + Ok(LoadedPrestate { prestate, chain_spec, block: recovered, db }) +} + /// Compare a captured Firehose `Block` against the binary protobuf golden at `golden_path`. /// /// On mismatch, returns an error containing pretty-printed `{:#?}` debug dumps of both the diff --git a/crates/firehose-tests/tests/prestate.rs b/crates/firehose-tests/tests/prestate.rs index a541cdf4105..cc48cec25e6 100644 --- a/crates/firehose-tests/tests/prestate.rs +++ b/crates/firehose-tests/tests/prestate.rs @@ -39,8 +39,9 @@ fn amsterdam_slot_number() { } /// EIP-7928: `run_wrapped_block` (this repo's pipeline/backfill path) has no payload sidecar to -/// source the block access list from, so it must reconstruct it via re-execution and only surface -/// it once the reconstructed hash matches the header's declared `block_access_list_hash`. +/// source the block access list from, so it must reconstruct it via re-execution, and +/// `take_traced_block_access_list` only surfaces it once the reconstructed hash matches the +/// header's declared `block_access_list_hash`. /// /// Because of that check the fixture's `context.blockAccessListHash` is not free-standing: it has /// to be regenerated from the reconstructed value whenever anything about the block changes, diff --git a/crates/firehose-tests/tests/staged_sync_bal.rs b/crates/firehose-tests/tests/staged_sync_bal.rs new file mode 100644 index 00000000000..065770b164a --- /dev/null +++ b/crates/firehose-tests/tests/staged_sync_bal.rs @@ -0,0 +1,56 @@ +//! Regression test: staged sync gets a block access list for Amsterdam blocks. +//! +//! The execution stage hashes the access list it takes from the executor with +//! [`Executor::take_bal`] and rejects an Amsterdam block when there is none +//! (`BlockAccessListHashMissing`). [`FirehoseBlockExecutor`] must therefore rebuild it on both its +//! traced and untraced paths, and record it on the traced Firehose block. +//! +//! It lives in its own integration-test binary because it installs the process-wide tracer. + +use std::path::PathBuf; + +use alloy_consensus::BlockHeader; +use alloy_primitives::keccak256; +use reth_evm::execute::Executor; +use reth_evm_ethereum::EthEvmConfig; +use reth_firehose::FirehoseBlockExecutor; +use reth_firehose_tests::{load_prestate, parse_fire_block_for, LoadedPrestate}; + +#[test] +fn staged_sync_rebuilds_the_block_access_list() { + let case = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("cases") + .join("amsterdam_block_access_list"); + let LoadedPrestate { prestate, chain_spec, block, db } = + load_prestate(&case).expect("loading the amsterdam_block_access_list prestate"); + let expected_hash = block + .header() + .block_access_list_hash() + .expect("the Amsterdam fixture declares a block access list hash"); + let evm_config = EthEvmConfig::new(chain_spec); + + let mut executor = FirehoseBlockExecutor::new(evm_config.clone(), db.clone()); + executor.execute_one(&block).expect("execute_one succeeds"); + let bal = executor.take_bal().expect("execute_one rebuilds the block access list"); + assert_eq!(keccak256(alloy_rlp::encode(&bal)), expected_hash); + + let config = &prestate.genesis.config; + let buffer = reth_firehose::init_tracer_with_buffer( + config.chain_id, + config.shanghai_time, + config.cancun_time, + config.prague_time, + ); + let mut executor = FirehoseBlockExecutor::new(evm_config, db); + executor.execute_and_trace_one(&block).expect("execute_and_trace_one succeeds"); + let bal = executor.take_bal().expect("execute_and_trace_one rebuilds the block access list"); + assert_eq!(keccak256(alloy_rlp::encode(&bal)), expected_hash); + + // Reaching `into_state` confirms the last block passed validation, which flushes it. + let _ = executor.into_state(); + let fire_block = parse_fire_block_for(&buffer.get_bytes(), block.header().number()) + .expect("the traced block is emitted"); + let header = fire_block.header.expect("the Firehose block has a header"); + assert_eq!(header.block_access_list_rlp, Some(alloy_rlp::encode(&bal))); +} diff --git a/crates/firehose/src/executor.rs b/crates/firehose/src/executor.rs index 2949e47f863..bb735cc0e25 100644 --- a/crates/firehose/src/executor.rs +++ b/crates/firehose/src/executor.rs @@ -40,6 +40,7 @@ use crate::{ mapper::SignatureFields, }; use alloy_consensus::{transaction::TxHashRef, BlockHeader, Transaction, TxReceipt}; +use alloy_eip7928::BlockAccessList; use alloy_eips::eip4895::Withdrawals; use alloy_evm::{ block::{BlockExecutor, CommitChanges, ExecutableTx, GasOutput}, @@ -57,7 +58,7 @@ use reth_provider::EvmStateProviderBox; use reth_revm::{ database::StateProviderDatabase, db::states::bundle_state::BundleRetention, - revm::{context::Block as RevmBlock, context_interface::Cfg}, + revm::{context::Block as RevmBlock, context_interface::Cfg, state::bal::Bal}, Database as _, State, }; @@ -698,6 +699,9 @@ pub struct FirehoseBlockExecutor { pending_tracer: Option, /// Chain-specific per-block wrapping strategy. hooks: H, + /// Block access list rebuilt by the most recent `execute_and_trace_one`, handed out by + /// `take_bal` for post-execution validation. + built_bal: Option, } impl Debug for FirehoseBlockExecutor<(), ()> { @@ -717,7 +721,7 @@ impl FirehoseBlockExecutor { /// Creates a new `FirehoseBlockExecutor` with a caller-supplied [`ChainHooks`]. pub fn new_with_chain_hooks(strategy_factory: F, db: DB, hooks: H) -> Self { let db = State::builder().with_database(db).with_bundle_update().build(); - Self { strategy_factory, db, pending_tracer: None, hooks } + Self { strategy_factory, db, pending_tracer: None, hooks, built_bal: None } } } @@ -741,11 +745,30 @@ where block: &RecoveredBlock<::Block>, ) -> Result::Receipt>, Self::Error> { - let result = self + self.built_bal = None; + let mut executor = self .strategy_factory .executor_for_block(&mut self.db, block) - .map_err(BlockExecutionError::other)? - .execute_block(block.transactions_recovered())?; + .map_err(BlockExecutionError::other)?; + + // Same EIP-7928 index tracking as `BasicBlockExecutor::execute_one`: index 0 is + // pre-execution, each transaction gets its own index, the last one is post-execution. + let has_bal = block.header().block_access_list_hash().is_some(); + executor.evm_mut().db_mut().bal_state.bal_builder = has_bal.then(Bal::new); + + executor.apply_pre_execution_changes()?; + if has_bal { + executor.evm_mut().db_mut().bump_bal_index(); + } + + for tx in block.transactions_recovered() { + executor.execute_transaction(tx)?; + if has_bal { + executor.evm_mut().db_mut().bump_bal_index(); + } + } + + let result = executor.apply_post_execution_changes()?; self.db.merge_transitions(BundleRetention::Reverts); Ok(result) } @@ -779,16 +802,18 @@ where let mut tracer = FirehoseBlockTracer::start::(block.sealed_block(), finalized); - let block_result = - self.hooks.execute_one_traced(&self.strategy_factory, &mut self.db, block, &mut tracer); + self.built_bal = None; + let block_result = self + .hooks + .execute_one_traced(&self.strategy_factory, &mut self.db, block, &mut tracer) + .and_then(|result| { + self.built_bal = take_traced_block_access_list(&mut self.db, block, &mut tracer)?; + Ok(result) + }); match block_result { Ok(result) => { self.db.merge_transitions(BundleRetention::Reverts); - // EIP-7928: `block_access_list_rlp` is left unset on this path (pipeline/backfill - // replay) — there is no payload sidecar to source it from here, unlike the live - // engine path (see `payload_validator.rs`), and this executor does not (yet) - // reconstruct it by tracking BAL during re-execution. self.pending_tracer = Some(tracer); Ok(result) } @@ -835,8 +860,8 @@ where self.db.bundle_state.size_hint() } - fn take_bal(&mut self) -> Option> { - None + fn take_bal(&mut self) -> Option { + self.built_bal.take().or_else(|| self.db.take_built_alloy_bal()) } } @@ -856,6 +881,9 @@ where /// Does **not** emit `on_block_start` / `on_block_end` — those are owned by the /// [`FirehoseBlockTracer`] lifecycle (see [`FirehoseBlockTracer::start`] and /// [`FirehoseBlockTracer::mark_verified`]). +/// +/// When the header declares a block access list, the one rebuilt by this execution is left in +/// `db`; collect it with [`take_traced_block_access_list`]. pub fn run_wrapped_block( evm_config: &F, db: &mut State, @@ -899,9 +927,8 @@ where // re-execution via revm's BAL-index tracking, the same mechanism // `BasicBlockExecutor::execute_one` uses, whenever the header declares a hash. let has_bal = block.header().block_access_list_hash().is_some(); - if has_bal { - db.bal_state.bal_builder = Some(reth_revm::revm::state::bal::Bal::new()); - } + db.bal_state.bal_builder = has_bal.then(Bal::new); + db.reset_bal_index(); let inspector = tracer_guard.inspector(); let evm = evm_config.evm_with_env_and_inspector(&mut *db, evm_env, inspector); @@ -911,28 +938,42 @@ where let wrapped = FirehoseWrappedExecutor::with_hooks(inner, withdrawals, adjust, extras) .with_bal_tracking(has_bal); - let result = wrapped.execute_block(block.transactions_recovered())?; - - if has_bal && let Some(bal) = db.take_built_alloy_bal() { - let computed_hash = alloy_eip7928::compute_block_access_list_hash(bal.as_slice()); - // Hard error rather than warn-and-continue: this is the first re-execution-based BAL - // reconstruction shipped, and a mismatch means the reconstruction is wrong somewhere, not - // that the block is invalid. Revisit once this has proven itself on testnets — the live - // path (payload-sourced, not reconstructed) is unaffected either way. - if Some(computed_hash) != block.header().block_access_list_hash() { - return Err(BlockExecutionError::msg(format!( - "reconstructed block access list hash mismatch for block {}: expected {}, computed {computed_hash}", - block.header().number(), - block.header().block_access_list_hash().unwrap_or_default(), - ))); - } - if let Some(header) = tracer_guard.tracer_mut().block_mut().and_then(|b| b.header.as_mut()) - { - header.block_access_list_rlp = Some(alloy_rlp::encode(&bal)); - } + wrapped.execute_block(block.transactions_recovered()) +} + +/// Takes the block access list [`run_wrapped_block`] rebuilt for `block` out of `db`, checks it +/// against the header's `block_access_list_hash` and records its RLP on the traced block. +/// +/// Returns `None` when the header declares no block access list. +pub fn take_traced_block_access_list( + db: &mut State, + block: &RecoveredBlock, + tracer_guard: &mut FirehoseBlockTracer, +) -> Result, BlockExecutionError> +where + B: BlockTrait, + DB: reth_evm::Database, + G: std::ops::DerefMut, +{ + let Some(expected_hash) = block.header().block_access_list_hash() else { return Ok(None) }; + let Some(bal) = db.take_built_alloy_bal() else { return Ok(None) }; + + let computed_hash = alloy_eip7928::compute_block_access_list_hash(bal.as_slice()); + // Hard error rather than warn-and-continue: this is the first re-execution-based BAL + // reconstruction shipped, and a mismatch means the reconstruction is wrong somewhere, not + // that the block is invalid. Revisit once this has proven itself on testnets — the live + // path (payload-sourced, not reconstructed) is unaffected either way. + if computed_hash != expected_hash { + return Err(BlockExecutionError::msg(format!( + "reconstructed block access list hash mismatch for block {}: expected {expected_hash}, computed {computed_hash}", + block.header().number(), + ))); + } + if let Some(header) = tracer_guard.tracer_mut().block_mut().and_then(|b| b.header.as_mut()) { + header.block_access_list_rlp = Some(alloy_rlp::encode(&bal)); } - Ok(result) + Ok(Some(bal)) } // --------------------------------------------------------------------------- diff --git a/crates/firehose/src/lib.rs b/crates/firehose/src/lib.rs index e92e900da28..e4eab924213 100644 --- a/crates/firehose/src/lib.rs +++ b/crates/firehose/src/lib.rs @@ -19,9 +19,9 @@ pub mod prelude; pub use block_tracer::{FirehoseBlockTracer, GlobalTracerGuard}; pub use executor::{ - run_wrapped_block, ChainHooks, FirehoseBlockExecutor, FirehoseEvmConfig, FirehoseLiveHooks, - FirehoseWrappedExecutor, LiveTracedEvm, NoChainHooks, NoPostTxExtras, NoPreTxAdjust, - PostTxExtras, PreTxAdjust, + run_wrapped_block, take_traced_block_access_list, ChainHooks, FirehoseBlockExecutor, + FirehoseEvmConfig, FirehoseLiveHooks, FirehoseWrappedExecutor, LiveTracedEvm, NoChainHooks, + NoPostTxExtras, NoPreTxAdjust, PostTxExtras, PreTxAdjust, }; pub use finality::finalized_ref_for_block; pub use firehose_tracer::types::FinalizedBlockRef;