From 5d2710864357c1fc98c410bdfa1a9db74f0cc7be Mon Sep 17 00:00:00 2001 From: lightsing Date: Wed, 16 Oct 2024 15:53:04 +0800 Subject: [PATCH 1/3] fix --- Cargo.lock | 22 ++++++++---- crates/bin/src/commands/run_file.rs | 54 +++++++++++++++++++++-------- crates/core/src/database.rs | 13 +++++-- crates/core/src/error.rs | 3 ++ crates/core/src/executor/mod.rs | 6 ++++ crates/utils/src/macros.rs | 10 +++--- 6 files changed, 79 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b56661a..d11bcc5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -839,12 +839,13 @@ dependencies = [ [[package]] name = "bn254" version = "0.1.0" -source = "git+https://github.com/scroll-tech/bn254#3653980f712ad44d4e079fa8f0199c5678104ef4" +source = "git+https://github.com/scroll-tech/bn254.git?branch=master#db67681f5e9ae1736565a48ec294a6d0dacb4e0d" dependencies = [ "ff", "getrandom", "rand", "rand_core", + "sp1-intrinsics", "subtle", ] @@ -2468,10 +2469,11 @@ checksum = "953ec861398dccce10c670dfeaf3ec4911ca479e9c02154b3a215178c5f566f2" [[package]] name = "poseidon-bn254" version = "0.1.0" -source = "git+https://github.com/scroll-tech/poseidon-bn254?branch=master#526a64a81419bcab6bd8280a8a3f9808189e0373" +source = "git+https://github.com/scroll-tech/poseidon-bn254?branch=master#a96ba028dd00987551451d8f4bd65b7ea5469a44" dependencies = [ "bn254", "itertools 0.13.0", + "sp1-intrinsics", ] [[package]] @@ -2812,7 +2814,7 @@ dependencies = [ [[package]] name = "revm" version = "14.0.0" -source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#f7f0745a46b16097a274c2def9d1334cbdd9da26" +source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#df2fc95185c1f24c5d3ebabb0067a864d8cfc754" dependencies = [ "auto_impl", "cfg-if", @@ -2826,7 +2828,7 @@ dependencies = [ [[package]] name = "revm-interpreter" version = "10.0.0" -source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#f7f0745a46b16097a274c2def9d1334cbdd9da26" +source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#df2fc95185c1f24c5d3ebabb0067a864d8cfc754" dependencies = [ "cfg-if", "revm-primitives", @@ -2836,7 +2838,7 @@ dependencies = [ [[package]] name = "revm-precompile" version = "11.0.0" -source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#f7f0745a46b16097a274c2def9d1334cbdd9da26" +source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#df2fc95185c1f24c5d3ebabb0067a864d8cfc754" dependencies = [ "aurora-engine-modexp", "c-kzg", @@ -2853,7 +2855,7 @@ dependencies = [ [[package]] name = "revm-primitives" version = "9.0.0" -source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#f7f0745a46b16097a274c2def9d1334cbdd9da26" +source = "git+https://github.com/scroll-tech/revm?branch=scroll-evm-executor/v42#df2fc95185c1f24c5d3ebabb0067a864d8cfc754" dependencies = [ "alloy-eips", "alloy-primitives", @@ -3405,6 +3407,14 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "sp1-intrinsics" +version = "0.0.0" +source = "git+https://github.com/scroll-tech/sp1-intrinsics.git?branch=master#7e038e60db0b2e847f6d8f49e148ccac8c6fc394" +dependencies = [ + "cfg-if", +] + [[package]] name = "spin" version = "0.9.8" diff --git a/crates/bin/src/commands/run_file.rs b/crates/bin/src/commands/run_file.rs index 7ea23a1..114ac2f 100644 --- a/crates/bin/src/commands/run_file.rs +++ b/crates/bin/src/commands/run_file.rs @@ -1,10 +1,12 @@ use crate::utils; -use anyhow::bail; +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}, }; +use std::panic::catch_unwind; use std::{cell::RefCell, path::PathBuf}; use tiny_keccak::{Hasher, Keccak}; use tokio::task::JoinSet; @@ -43,7 +45,7 @@ impl RunFileCommand { while let Some(task) = tasks.join_next().await { if let Err(err) = task? { - bail!("{:?}", err); + dev_error!("{:?}", err); } } @@ -114,19 +116,21 @@ async fn read_block_trace(path: &PathBuf) -> anyhow::Result { } fn deserialize_block_trace(trace: &str) -> anyhow::Result { + // Try to deserialize `BlockTrace` from JSON. In case of failure, try to + // deserialize `BlockTrace` from a JSON-RPC response that has the actual block + // trace nested in the value of the key "result". Ok( - // Try to deserialize `BlockTrace` from JSON. In case of failure, try to - // deserialize `BlockTrace` from a JSON-RPC response that has the actual block - // trace nested in the value of the key "result". - serde_json::from_str::(trace).or_else(|_| { - #[derive(serde::Deserialize, Default, Debug, Clone)] - pub struct BlockTraceJsonRpcResult { - pub result: BlockTrace, - } - Ok::<_, serde_json::Error>( - serde_json::from_str::(trace)?.result, - ) - })?, + serde_json::from_str::>(trace) + .or_else(|_| { + #[derive(serde::Deserialize, Default, Debug, Clone)] + pub struct BlockTraceJsonRpcResult { + pub result: BlockTrace, + } + Ok::<_, serde_json::Error>( + serde_json::from_str::(trace)?.result, + ) + })? + .into(), ) } @@ -136,6 +140,26 @@ async fn run_trace( ) -> anyhow::Result<()> { let trace = read_block_trace(&path).await?; let fork_config = fork_config(trace.chain_id()); - tokio::task::spawn_blocking(move || utils::verify(&trace, &fork_config)).await??; + if let Err(e) = + tokio::task::spawn_blocking(move || catch_unwind(|| utils::verify(&trace, &fork_config))) + .await? + .map_err(|e| { + e.downcast_ref::<&str>() + .map(|s| anyhow!("task panics with: {s}")) + .or_else(|| { + e.downcast_ref::() + .map(|s| anyhow!("task panics with: {s}")) + }) + .unwrap_or_else(|| anyhow!("task panics")) + }) + .and_then(|r| r.map_err(anyhow::Error::from)) + { + dev_error!( + "Error occurs when verifying block ({}): {:?}", + path.display(), + e + ); + return Err(e); + } Ok(()) } diff --git a/crates/core/src/database.rs b/crates/core/src/database.rs index 7256fa7..32cce45 100644 --- a/crates/core/src/database.rs +++ b/crates/core/src/database.rs @@ -26,7 +26,7 @@ pub struct EvmDatabase<'a, CodeDb, ZkDb> { storage_root_caches: RefCell>, /// Storage trie cache, avoid re-creating trie for the same account. /// Need to invalidate before `update`, otherwise the trie root may be outdated - storage_trie_caches: RefCell>>, + storage_trie_caches: RefCell>>>, /// Current uncommitted zkTrie root based on the block trace. committed_zktrie_root: B256, /// The underlying zkTrie database. @@ -97,7 +97,7 @@ impl<'a, CodeDb: KVDatabase, ZkDb: KVDatabase + 'static> EvmDatabase<'a, CodeDb, storage_trie_caches.remove(&old); } - storage_trie_caches.insert(new_root, storage_root); + storage_trie_caches.insert(new_root, Some(storage_root)); } /// Get the committed zkTrie root. @@ -207,8 +207,15 @@ impl DatabaseRef for EvmDatabase dev_debug!("storage root of {:?} is {:?}", address, storage_root); ZkTrie::new_with_root(self.zktrie_db, NoCacheHasher, *storage_root) - .expect("storage trie associated with account not found") + .inspect_err(|e| { + dev_warn!("storage trie associated with account({address}) not found: {e}") + }) + .ok() }); + if trie.is_none() { + return Err(DatabaseError::NotIncluded); + } + let trie = trie.as_mut().unwrap(); #[cfg(debug_assertions)] { diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index 174f260..7406a35 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -10,6 +10,9 @@ pub enum DatabaseError { /// Error encountered from zkTrie. #[error("error encountered from zkTrie: {0}")] ZkTrie(Box), + /// Error when try to load un-included account/storage. + #[error("account/storage not included in zkTrie")] + NotIncluded, } /// Error variants encountered during verification of transactions in a L2 block. diff --git a/crates/core/src/executor/mod.rs b/crates/core/src/executor/mod.rs index 562fb37..70a8a2e 100644 --- a/crates/core/src/executor/mod.rs +++ b/crates/core/src/executor/mod.rs @@ -262,6 +262,11 @@ impl EvmExecutor<'_, '_, CodeDb, #[cfg(feature = "debug-storage")] debug_recorder.record_storage_root(*addr, storage_root); + } else if db_acc.account_state.is_storage_cleared() { + // previous storage root might not be zero, and evm cleared the storage + // e.g. create storage collision + // not something could happen in real world + storage_root = B256::ZERO; } if !info.is_empty() { // if account not exist, all fields will be zero. @@ -285,6 +290,7 @@ impl EvmExecutor<'_, '_, CodeDb, debug_recorder.record_account(*addr, info.clone(), storage_root); let acc_data = Account::from_revm_account_with_storage_root(info, storage_root); + dev_trace!("committing account {addr}: {acc_data:?}"); measure_duration_micros!( zktrie_update_duration_microseconds, cycle_track!( diff --git a/crates/utils/src/macros.rs b/crates/utils/src/macros.rs index f8f1af1..c3b21d2 100644 --- a/crates/utils/src/macros.rs +++ b/crates/utils/src/macros.rs @@ -39,8 +39,8 @@ macro_rules! cycle_tracker_end { #[macro_export] macro_rules! dev_trace { ($($arg:tt)*) => { - #[cfg(any(feature = "dev", test))] { + #[cfg(any(feature = "dev", test))] $crate::tracing::trace!($($arg)*); } }; @@ -50,8 +50,8 @@ macro_rules! dev_trace { #[macro_export] macro_rules! dev_info { ($($arg:tt)*) => { - #[cfg(any(feature = "dev", test))] { + #[cfg(any(feature = "dev", test))] $crate::tracing::info!($($arg)*); } }; @@ -61,8 +61,8 @@ macro_rules! dev_info { #[macro_export] macro_rules! dev_error { ($($arg:tt)*) => { - #[cfg(any(feature = "dev", test))] { + #[cfg(any(feature = "dev", test))] $crate::tracing::error!($($arg)*); } }; @@ -72,8 +72,8 @@ macro_rules! dev_error { #[macro_export] macro_rules! dev_debug { ($($arg:tt)*) => { - #[cfg(any(feature = "dev", test))] { + #[cfg(any(feature = "dev", test))] $crate::tracing::debug!($($arg)*); } }; @@ -83,8 +83,8 @@ macro_rules! dev_debug { #[macro_export] macro_rules! dev_warn { ($($arg:tt)*) => { - #[cfg(any(feature = "dev", test))] { + #[cfg(any(feature = "dev", test))] $crate::tracing::warn!($($arg)*); } }; From 1d3c0f6ba2536589f1471ae89fc3afdea9004c3a Mon Sep 17 00:00:00 2001 From: lightsing Date: Thu, 17 Oct 2024 12:42:06 +0800 Subject: [PATCH 2/3] bump MSRV --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a5839db..6998911 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ resolver = "2" [workspace.package] version = "2.0.0" edition = "2021" -rust-version = "1.75" +rust-version = "1.81" authors = ["Scroll developers"] license = "MIT OR Apache-2.0" homepage = "https://github.com/scroll-tech/stateless-block-verifier" From 5ed973acb096365d6c81ec5458e4be048e066440 Mon Sep 17 00:00:00 2001 From: lightsing Date: Thu, 17 Oct 2024 12:47:54 +0800 Subject: [PATCH 3/3] remove clear storage fix --- crates/core/src/executor/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/core/src/executor/mod.rs b/crates/core/src/executor/mod.rs index 70a8a2e..838fb4f 100644 --- a/crates/core/src/executor/mod.rs +++ b/crates/core/src/executor/mod.rs @@ -262,11 +262,6 @@ impl EvmExecutor<'_, '_, CodeDb, #[cfg(feature = "debug-storage")] debug_recorder.record_storage_root(*addr, storage_root); - } else if db_acc.account_state.is_storage_cleared() { - // previous storage root might not be zero, and evm cleared the storage - // e.g. create storage collision - // not something could happen in real world - storage_root = B256::ZERO; } if !info.is_empty() { // if account not exist, all fields will be zero.