From 3d7b60cdc7d3c0bfeb10cc6086af7b247c7ce706 Mon Sep 17 00:00:00 2001 From: Akase Haruka Date: Mon, 18 Nov 2024 10:35:38 +0800 Subject: [PATCH] refactor: remove hooks (#69) The hooks pattern seems to be outdated since we no longer doing post tx checking. The only reason to have hook is to reuse the rlp bytes, which we can do it in a trivial way -- returning it after handling the block. This pr removes the ExecutionHook and changes the way we reusing the rlp bytes. --- crates/bin/src/commands/run_file.rs | 26 ++++++----- crates/core/src/chunk.rs | 18 ++++---- crates/core/src/executor/builder.rs | 25 +++-------- crates/core/src/executor/hooks.rs | 70 ----------------------------- crates/core/src/executor/mod.rs | 36 ++++++++++----- crates/core/src/lib.rs | 2 +- crates/primitives/src/lib.rs | 7 +++ crates/primitives/src/types/mod.rs | 12 +++++ 8 files changed, 71 insertions(+), 125 deletions(-) delete mode 100644 crates/core/src/executor/hooks.rs diff --git a/crates/bin/src/commands/run_file.rs b/crates/bin/src/commands/run_file.rs index 3e57abd6..15208e21 100644 --- a/crates/bin/src/commands/run_file.rs +++ b/crates/bin/src/commands/run_file.rs @@ -1,14 +1,17 @@ use crate::utils; use anyhow::{anyhow, bail}; use clap::Args; -use sbv::primitives::types::LegacyStorageTrace; use sbv::{ - core::{ChunkInfo, EvmExecutorBuilder, HardforkConfig}, - primitives::{types::BlockTrace, zk_trie::db::kv::HashMapDb, Block, B256}, + core::{BlockExecutionResult, ChunkInfo, EvmExecutorBuilder, HardforkConfig}, + primitives::{ + types::{BlockTrace, LegacyStorageTrace}, + zk_trie::db::kv::HashMapDb, + Block, B256, + }, }; use serde::Deserialize; use std::panic::catch_unwind; -use std::{cell::RefCell, path::PathBuf}; +use std::path::PathBuf; use tiny_keccak::{Hasher, Keccak}; use tokio::task::JoinSet; @@ -80,22 +83,21 @@ impl RunFileCommand { let (chunk_info, mut zktrie_db) = ChunkInfo::from_block_traces(&traces); let mut code_db = HashMapDb::default(); - let tx_bytes_hasher = RefCell::new(Keccak::v256()); + let mut tx_bytes_hasher = Keccak::v256(); let mut executor = EvmExecutorBuilder::new(&mut code_db, &mut zktrie_db) .hardfork_config(fork_config) .chain_id(traces[0].chain_id()) - .with_hooks(traces[0].root_before(), |hooks| { - hooks.add_tx_rlp_handler(|_, rlp| { - tx_bytes_hasher.borrow_mut().update(rlp); - }); - })?; + .build(traces[0].root_before())?; for trace in traces.iter() { executor.insert_codes(trace)?; } for trace in traces.iter() { - executor.handle_block(trace)?; + let BlockExecutionResult { tx_rlps, .. } = executor.handle_block(trace)?; + for tx_rlp in tx_rlps { + tx_bytes_hasher.update(&tx_rlp); + } } let post_state_root = executor.commit_changes()?; @@ -105,7 +107,7 @@ impl RunFileCommand { drop(executor); let mut tx_bytes_hash = B256::ZERO; - tx_bytes_hasher.into_inner().finalize(&mut tx_bytes_hash.0); + tx_bytes_hasher.finalize(&mut tx_bytes_hash.0); let _public_input_hash = chunk_info.public_input_hash(&tx_bytes_hash); dev_info!("[chunk mode] public input hash: {:?}", _public_input_hash); diff --git a/crates/core/src/chunk.rs b/crates/core/src/chunk.rs index 663ce353..3847200e 100644 --- a/crates/core/src/chunk.rs +++ b/crates/core/src/chunk.rs @@ -117,10 +117,9 @@ impl ChunkInfo { #[cfg(test)] mod tests { use super::*; - use crate::{EvmExecutorBuilder, HardforkConfig}; + use crate::{BlockExecutionResult, EvmExecutorBuilder, HardforkConfig}; use revm::primitives::b256; use sbv_primitives::types::BlockTrace; - use std::cell::RefCell; const TRACES_STR: [&str; 4] = [ include_str!("../../../testdata/mainnet_blocks/8370400.json"), @@ -145,23 +144,22 @@ mod tests { let (chunk_info, mut zktrie_db) = ChunkInfo::from_block_traces(&traces); let mut code_db = HashMapDb::default(); - let tx_bytes_hasher = RefCell::new(Keccak::v256()); + let mut tx_bytes_hasher = Keccak::v256(); let mut executor = EvmExecutorBuilder::new(&mut code_db, &mut zktrie_db) .hardfork_config(fork_config) .chain_id(traces[0].chain_id()) - .with_hooks(traces[0].root_before(), |hooks| { - hooks.add_tx_rlp_handler(|_, rlp| { - tx_bytes_hasher.borrow_mut().update(rlp); - }); - }) + .build(traces[0].root_before()) .unwrap(); for trace in traces.iter() { executor.insert_codes(trace).unwrap(); } for trace in traces.iter() { - executor.handle_block(trace).unwrap(); + let BlockExecutionResult { tx_rlps, .. } = executor.handle_block(trace).unwrap(); + for tx_rlp in tx_rlps { + tx_bytes_hasher.update(&tx_rlp); + } } let post_state_root = executor.commit_changes().unwrap(); @@ -169,7 +167,7 @@ mod tests { drop(executor); // drop executor to release Rc let mut tx_bytes_hash = B256::ZERO; - tx_bytes_hasher.into_inner().finalize(&mut tx_bytes_hash.0); + tx_bytes_hasher.finalize(&mut tx_bytes_hash.0); let public_input_hash = chunk_info.public_input_hash(&tx_bytes_hash); assert_eq!( public_input_hash, diff --git a/crates/core/src/executor/builder.rs b/crates/core/src/executor/builder.rs index 99bfc2ef..3214634f 100644 --- a/crates/core/src/executor/builder.rs +++ b/crates/core/src/executor/builder.rs @@ -1,10 +1,8 @@ -use crate::error::DatabaseError; -use crate::{executor::hooks::ExecuteHooks, EvmDatabase, EvmExecutor, HardforkConfig}; +use crate::{error::DatabaseError, EvmDatabase, EvmExecutor, HardforkConfig}; use revm::db::CacheDB; -use sbv_primitives::alloy_primitives::ChainId; -use sbv_primitives::zk_trie::db::kv::KVDatabase; -use sbv_primitives::zk_trie::db::NodeDb; -use sbv_primitives::B256; +use sbv_primitives::{ + alloy_primitives::ChainId, zk_trie::db::kv::KVDatabase, zk_trie::db::NodeDb, B256, +}; use std::fmt::{self, Debug}; /// Builder for EVM executor. @@ -93,14 +91,7 @@ impl<'a, CodeDb: KVDatabase, ZkDb: KVDatabase + 'static> EvmExecutorBuilder<'a, HardforkConfig, ChainId, CodeDb, ZkDb> { /// Initialize an EVM executor from a block trace as the initial state. - pub fn with_hooks<'h, F: FnOnce(&mut ExecuteHooks<'h, CodeDb, ZkDb>)>( - self, - root: B256, - with_execute_hooks: F, - ) -> Result, DatabaseError> { - let mut execute_hooks = ExecuteHooks::new(); - with_execute_hooks(&mut execute_hooks); - + pub fn build(self, root: B256) -> Result, DatabaseError> { let db = cycle_track!( CacheDB::new(EvmDatabase::new_from_root( root, @@ -114,12 +105,6 @@ impl<'a, CodeDb: KVDatabase, ZkDb: KVDatabase + 'static> hardfork_config: self.hardfork_config, chain_id: self.chain_id, db, - hooks: execute_hooks, }) } - - /// Initialize an EVM executor from a block trace as the initial state. - pub fn build<'e>(self, root: B256) -> Result, DatabaseError> { - self.with_hooks(root, |_| {}) - } } diff --git a/crates/core/src/executor/hooks.rs b/crates/core/src/executor/hooks.rs deleted file mode 100644 index 1e17d7c9..00000000 --- a/crates/core/src/executor/hooks.rs +++ /dev/null @@ -1,70 +0,0 @@ -use crate::EvmExecutor; -use std::fmt::{Debug, Formatter}; - -/// Transaction RLP handler. -pub type TxRLPHandler<'a, CodeDb, ZkDb> = dyn Fn(&EvmExecutor, &[u8]) + 'a; -/// Post transaction execution handler. -pub type PostTxExecutionHandler<'a, CodeDb, ZkDb> = dyn Fn(&EvmExecutor, usize) + 'a; - -/// Hooks for the EVM executor. -pub struct ExecuteHooks<'a, CodeDb, ZkDb> { - tx_rlp_handlers: Vec>>, - post_tx_execution_handlers: Vec>>, -} - -impl<'a, CodeDb, ZkDb> Default for ExecuteHooks<'a, CodeDb, ZkDb> { - fn default() -> Self { - Self { - tx_rlp_handlers: Vec::new(), - post_tx_execution_handlers: Vec::new(), - } - } -} - -impl<'a, CodeDb, ZkDb> ExecuteHooks<'a, CodeDb, ZkDb> { - /// Create a new hooks. - pub fn new() -> Self { - Self::default() - } - - /// Add a transaction RLP handler. - pub fn add_tx_rlp_handler(&mut self, handler: F) - where - F: Fn(&EvmExecutor, &[u8]) + 'a, - { - self.tx_rlp_handlers.push(Box::new(handler)); - } - - /// Add a post transaction execution handler. - pub fn add_post_tx_execution_handler(&mut self, handler: F) - where - F: Fn(&EvmExecutor, usize) + 'a, - { - self.post_tx_execution_handlers.push(Box::new(handler)); - } - - /// Execute transaction RLP handlers. - pub(crate) fn tx_rlp(&self, executor: &EvmExecutor, rlp: &[u8]) { - for handler in &self.tx_rlp_handlers { - handler(executor, rlp); - } - } - - pub(crate) fn post_tx_execution(&self, executor: &EvmExecutor, tx_index: usize) { - for handler in &self.post_tx_execution_handlers { - handler(executor, tx_index); - } - } -} - -impl Debug for ExecuteHooks<'_, CodeDb, ZkDb> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.debug_struct("ExecuteHooks") - .field("tx_rlp_handlers", &self.tx_rlp_handlers.len()) - .field( - "post_tx_execution_handlers", - &self.post_tx_execution_handlers.len(), - ) - .finish() - } -} diff --git a/crates/core/src/executor/mod.rs b/crates/core/src/executor/mod.rs index a4e719dc..23d5ef80 100644 --- a/crates/core/src/executor/mod.rs +++ b/crates/core/src/executor/mod.rs @@ -9,6 +9,7 @@ use revm::{ primitives::{AccountInfo, Env, B256, KECCAK_EMPTY, POSEIDON_EMPTY}, }; use sbv_primitives::{ + alloy_primitives::Bytes, zk_trie::{ db::kv::KVDatabase, hash::{key_hasher::NoCacheHasher, poseidon::Poseidon}, @@ -22,18 +23,23 @@ use std::fmt::Debug; mod builder; pub use builder::EvmExecutorBuilder; -/// Execute hooks -pub mod hooks; - /// EVM executor that handles the block. -pub struct EvmExecutor<'db, 'h, CodeDb, ZkDb> { +pub struct EvmExecutor<'db, CodeDb, ZkDb> { chain_id: ChainId, hardfork_config: HardforkConfig, db: CacheDB>, - hooks: hooks::ExecuteHooks<'h, CodeDb, ZkDb>, } -impl EvmExecutor<'_, '_, CodeDb, ZkDb> { +/// Block execution result +#[derive(Debug, Clone)] +pub struct BlockExecutionResult { + /// Gas used in this block + pub gas_used: u64, + /// RLP bytes of transactions + pub tx_rlps: Vec, +} + +impl EvmExecutor<'_, CodeDb, ZkDb> { /// Get reference to the DB pub fn db(&self) -> &CacheDB> { &self.db @@ -45,7 +51,10 @@ impl EvmExecutor<'_, '_, CodeDb, } /// Handle a block. - pub fn handle_block(&mut self, l2_trace: &T) -> Result { + pub fn handle_block( + &mut self, + l2_trace: &T, + ) -> Result { #[allow(clippy::let_and_return)] let gas_used = measure_duration_millis!( handle_block_duration_milliseconds, @@ -59,7 +68,10 @@ impl EvmExecutor<'_, '_, CodeDb, } #[inline(always)] - fn handle_block_inner(&mut self, l2_trace: &T) -> Result { + fn handle_block_inner( + &mut self, + l2_trace: &T, + ) -> Result { let spec_id = self.hardfork_config.get_spec_id(l2_trace.number()); dev_trace!("use spec id {spec_id:?}",); self.hardfork_config @@ -81,6 +93,7 @@ impl EvmExecutor<'_, '_, CodeDb, }; let mut gas_used = 0; + let mut tx_rlps = Vec::with_capacity(l2_trace.num_txs()); for (idx, tx) in l2_trace.transactions().enumerate() { cycle_tracker_start!("handle tx"); @@ -129,7 +142,7 @@ impl EvmExecutor<'_, '_, CodeDb, } env.tx.scroll.is_l1_msg = tx.is_l1_msg(); let rlp_bytes = tx.rlp(); - self.hooks.tx_rlp(self, &rlp_bytes); + tx_rlps.push(rlp_bytes.clone()); env.tx.scroll.rlp_bytes = Some(rlp_bytes); dev_trace!("{env:#?}"); @@ -161,12 +174,11 @@ impl EvmExecutor<'_, '_, CodeDb, dev_trace!("{result:#?}"); } - self.hooks.post_tx_execution(self, idx); dev_debug!("handle {idx}th tx done"); cycle_tracker_end!("handle tx"); } - Ok(gas_used) + Ok(BlockExecutionResult { gas_used, tx_rlps }) } /// Commit pending changes in cache db to zktrie @@ -345,7 +357,7 @@ impl EvmExecutor<'_, '_, CodeDb, } } -impl Debug for EvmExecutor<'_, '_, CodeDb, ZkDb> { +impl Debug for EvmExecutor<'_, CodeDb, ZkDb> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("EvmExecutor").field("db", &self.db).finish() } diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 8c16077e..b1ee5bcd 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -14,7 +14,7 @@ mod error; pub use error::{DatabaseError, VerificationError}; mod executor; -pub use executor::{hooks, EvmExecutor, EvmExecutorBuilder}; +pub use executor::{BlockExecutionResult, EvmExecutor, EvmExecutorBuilder}; mod genesis; pub use genesis::GenesisConfig; diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index 73def97f..69ba30a2 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -69,6 +69,9 @@ pub trait Block: Debug { /// transactions fn transactions(&self) -> impl Iterator; + /// Number of l1 transactions + fn num_txs(&self) -> usize; + /// root before fn root_before(&self) -> B256; /// root after @@ -314,6 +317,10 @@ impl Block for &T { (*self).transactions() } + fn num_txs(&self) -> usize { + (*self).num_txs() + } + fn root_before(&self) -> B256 { (*self).root_before() } diff --git a/crates/primitives/src/types/mod.rs b/crates/primitives/src/types/mod.rs index 408588fd..11b74627 100644 --- a/crates/primitives/src/types/mod.rs +++ b/crates/primitives/src/types/mod.rs @@ -380,6 +380,10 @@ where self.transactions.iter() } + fn num_txs(&self) -> usize { + self.transactions.len() + } + fn root_before(&self) -> B256 { self.storage_trace.root_before() } @@ -459,6 +463,10 @@ where self.transactions.iter() } + fn num_txs(&self) -> usize { + self.transactions.len() + } + fn root_before(&self) -> B256 { self.storage_trace.root_before() } @@ -672,6 +680,10 @@ impl Block for alloy::rpc::types::Block usize { + self.transactions.len() + } + fn root_before(&self) -> B256 { unimplemented!() }