Merge branch 'scroll-stable' into goerli-0215

bus-mapping/src/circuit_input_builder.rs

@@ -368,10 +368,7 @@ impl<'a> CircuitInputBuilder {
                 state_ref.call().map(|c| c.call_id).unwrap_or(0),
                 state_ref.call_ctx()?.memory.len(),
                 if geth_step.op.is_push() {
-                    match geth_step.stack.last() {
-                        Ok(w) => format!("{:?}", w),
-                        Err(_) => "".to_string(),
-                    }
+                    format!("{:?}", geth_trace.struct_logs[index + 1].stack.last())
                 } else if geth_step.op.is_call_without_value() {
                     format!(
                         "{:?} {:40x} {:?} {:?} {:?} {:?}",
@@ -427,11 +424,13 @@ impl<'a> CircuitInputBuilder {
         // - execution_state: EndTx
         // - op: None
         // Generate EndTx step
+        log::trace!("gen_end_tx_ops");
         let end_tx_step = gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?;
         tx.steps_mut().push(end_tx_step);
 
         self.sdb.commit_tx();
         self.block.txs.push(tx);
+        log::trace!("handle_tx finished");
 
         Ok(())
     }

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -732,9 +732,10 @@ impl<'a> CircuitInputStateRef<'a> {
                 } else {
                     let (found, account) = self.sdb.get_account(&code_address);
                     if !found {
-                        return Err(Error::AccountNotFound(code_address));
+                        (CodeSource::Address(code_address), H256::from(*EMPTY_HASH))
+                    } else {
+                        (CodeSource::Address(code_address), account.code_hash)
                     }
-                    (CodeSource::Address(code_address), account.code_hash)
                 }
             }
         };
@@ -1274,19 +1275,13 @@ impl<'a> CircuitInputStateRef<'a> {
                 step.op,
                 OpcodeId::CALL | OpcodeId::CALLCODE | OpcodeId::DELEGATECALL | OpcodeId::STATICCALL
             ) {
-                let caller = self.call()?;
-                let callee_address = match CallKind::try_from(step.op)? {
-                    CallKind::Call | CallKind::StaticCall => step.stack.nth_last(1)?.to_address(),
-                    CallKind::CallCode | CallKind::DelegateCall => caller.address,
-                    _ => unreachable!(),
-                };
-
-                if is_precompiled(&callee_address) {
+                let code_address = step.stack.nth_last(1)?.to_address();
+                if is_precompiled(&code_address) {
                     // Log the precompile address and gas left. Since this failure is mainly caused
                     // by out of gas.
                     log::trace!(
-                        "Precompile failed: callee_address = {}, step.gas = {}",
-                        callee_address,
+                        "Precompile failed: code_address = {}, step.gas = {}",
+                        code_address,
                         step.gas.0,
                     );
                     return Ok(Some(ExecError::PrecompileFailed));

bus-mapping/src/evm/opcodes.rs

@@ -62,6 +62,7 @@ mod error_invalid_opcode;
 mod error_oog_call;
 mod error_oog_log;
 mod error_oog_sload_sstore;
+mod error_precompile_failed;
 mod error_stack_oog_constant;
 
 #[cfg(test)]
@@ -86,6 +87,7 @@ use error_invalid_opcode::InvalidOpcode;
 use error_oog_call::OOGCall;
 use error_oog_log::ErrorOOGLog;
 use error_oog_sload_sstore::OOGSloadSstore;
+use error_precompile_failed::PrecompileFailed;
 use error_stack_oog_constant::ErrorStackOogConstant;
 use exp::Exponentiation;
 use extcodecopy::Extcodecopy;
@@ -274,6 +276,7 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociate
         ExecError::StackUnderflow => Some(ErrorStackOogConstant::gen_associated_ops),
         // call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode
         ExecError::InsufficientBalance => Some(CallOpcode::<7>::gen_associated_ops),
+        ExecError::PrecompileFailed => Some(PrecompileFailed::gen_associated_ops),
 
         // more future errors place here
         _ => {
@@ -637,6 +640,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
     let effective_tip = state.tx.gas_price - block_info.base_fee;
     let (found, coinbase_account) = state.sdb.get_account_mut(&block_info.coinbase);
     if !found {
+        log::error!("coinbase account not found: {}", block_info.coinbase);
         return Err(Error::AccountNotFound(block_info.coinbase));
     }
     let coinbase_balance_prev = coinbase_account.balance;

bus-mapping/src/evm/opcodes/create.rs

@@ -21,6 +21,7 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
         geth_steps: &[GethExecStep],
     ) -> Result<Vec<ExecStep>, Error> {
         let geth_step = &geth_steps[0];
+        let mut exec_step = state.new_step(geth_step)?;
 
         let offset = geth_step.stack.nth_last(1)?.as_usize();
         let length = geth_step.stack.nth_last(2)?.as_usize();
@@ -34,8 +35,6 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
         }
         let next_memory_word_size = state.call_ctx()?.memory_word_size();
 
-        let mut exec_step = state.new_step(geth_step)?;
-
         let callee = state.parse_call(geth_step)?;
 
         let n_pop = if IS_CREATE2 { 4 } else { 3 };

bus-mapping/src/evm/opcodes/error_precompile_failed.rs

@@ -0,0 +1,43 @@
+use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep};
+use crate::error::ExecError;
+use crate::evm::Opcode;
+use crate::Error;
+use eth_types::evm_types::OpcodeId;
+use eth_types::GethExecStep;
+
+#[derive(Debug, Copy, Clone)]
+pub(crate) struct PrecompileFailed;
+
+impl Opcode for PrecompileFailed {
+    fn gen_associated_ops(
+        state: &mut CircuitInputStateRef,
+        geth_steps: &[GethExecStep],
+    ) -> Result<Vec<ExecStep>, Error> {
+        let geth_step = &geth_steps[0];
+        let stack_input_num = match geth_step.op {
+            OpcodeId::CALL | OpcodeId::CALLCODE => 7,
+            OpcodeId::DELEGATECALL | OpcodeId::STATICCALL => 6,
+            op => unreachable!("{op} should not happen in PrecompileFailed"),
+        };
+
+        let mut exec_step = state.new_step(geth_step)?;
+        exec_step.error = Some(ExecError::PrecompileFailed);
+
+        for i in 0..stack_input_num {
+            state.stack_read(
+                &mut exec_step,
+                geth_step.stack.nth_last_filled(i),
+                geth_step.stack.nth_last(i)?,
+            )?;
+        }
+
+        state.stack_write(
+            &mut exec_step,
+            geth_step.stack.nth_last_filled(stack_input_num - 1),
+            // Must fail.
+            (0_u64).into(),
+        )?;
+
+        Ok(vec![exec_step])
+    }
+}

bus-mapping/src/evm/opcodes/extcodehash.rs

@@ -61,14 +61,16 @@ impl Opcode for Extcodehash {
         } else {
             H256::zero()
         };
+        //log::trace!("extcodehash addr {:?} acc {:?} exists {:?} codehash {:?}",
+        // external_address, account, exists, code_hash);
         state.account_read(
             &mut exec_step,
             external_address,
             AccountField::CodeHash,
             code_hash.to_word(),
             code_hash.to_word(),
         )?;
-
+        debug_assert_eq!(steps[1].stack.last()?, code_hash.to_word());
         // Stack write of the result of EXTCODEHASH.
         state.stack_write(&mut exec_step, stack_address, steps[1].stack.last()?)?;
 

bus-mapping/src/mock.rs

@@ -67,6 +67,11 @@ impl BlockData {
 
         for account in geth_data.accounts {
             let code_hash = code_db.insert(account.code.to_vec());
+            log::trace!(
+                "trace code {:?} {:?}",
+                code_hash,
+                hex::encode(account.code.to_vec())
+            );
             sdb.set_account(
                 &account.address,
                 state_db::Account {

bus-mapping/src/state_db.rs

@@ -114,10 +114,7 @@ impl Account {
 
     /// Return if account is empty or not.
     pub fn is_empty(&self) -> bool {
-        self.nonce.is_zero()
-            && self.balance.is_zero()
-            && self.storage.is_empty()
-            && self.code_hash.eq(&CODE_HASH_ZERO)
+        self.nonce.is_zero() && self.balance.is_zero() && self.code_hash.eq(&CODE_HASH_ZERO)
     }
 }
 

bus-mapping/src/util.rs

@@ -8,5 +8,5 @@ pub fn read_env_var<T: Clone + FromStr>(var_name: &'static str, default: T) -> T
         .map(|s| s.parse::<T>().unwrap_or_else(|_| default.clone()))
         .unwrap_or(default)
 }
-pub(crate) static CHECK_MEM_STRICT: Lazy<bool> =
-    Lazy::new(|| read_env_var("CHECK_MEM_STRICT", false));
+/// ..
+pub static CHECK_MEM_STRICT: Lazy<bool> = Lazy::new(|| read_env_var("CHECK_MEM_STRICT", false));

external-tracer/src/lib.rs

@@ -71,6 +71,8 @@ pub fn trace(config: &TraceConfig) -> Result<Vec<GethExecTrace>, Error> {
         },
     )?;
 
+    //println!("trace {}",trace_string);
+
     let trace = serde_json::from_str(&trace_string).map_err(Error::SerdeError)?;
     Ok(trace)
 }

testool/src/statetest/executor.rs

@@ -8,7 +8,7 @@ use ethers_core::types::transaction::eip2718::TypedTransaction;
 use ethers_core::types::TransactionRequest;
 use ethers_core::utils::keccak256;
 use ethers_signers::{LocalWallet, Signer};
-use external_tracer::TraceConfig;
+use external_tracer::{LoggerConfig, TraceConfig};
 use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr};
 use std::{collections::HashMap, str::FromStr};
 use thiserror::Error;
@@ -157,7 +157,10 @@ fn into_traceconfig(st: StateTest) -> (String, TraceConfig, StateTestResult) {
                 hash: tx_hash.into(),
             }],
             accounts: st.pre,
-            ..Default::default()
+            logger_config: LoggerConfig {
+                enable_memory: *bus_mapping::util::CHECK_MEM_STRICT,
+                ..Default::default()
+            },
         },
         st.result,
     )
@@ -228,6 +231,7 @@ pub fn run_test(
             s: tx.s,
             v: U64::from(tx.v),
             block_number: Some(U64::from(trace_config.block_constants.number.as_u64())),
+            chain_id: Some(trace_config.chain_id),
             ..eth_types::Transaction::default()
         })
         .collect();
@@ -245,7 +249,10 @@ pub fn run_test(
 
     let wallet: LocalWallet = SigningKey::from_bytes(&st.secret_key).unwrap().into();
     let mut wallets = HashMap::new();
-    wallets.insert(wallet.address(), wallet.with_chain_id(1u64));
+    wallets.insert(
+        wallet.address(),
+        wallet.with_chain_id(trace_config.chain_id.as_u64()),
+    );
 
     // process the transaction
     let mut geth_data = eth_types::geth_types::GethData {

zkevm-circuits/Cargo.toml

@@ -62,3 +62,4 @@ enable-sign-verify = []
 codehash = []
 reject-eip2718 = []
 poseidon-codehash = []
+scroll = []

zkevm-circuits/src/evm_circuit.rs

@@ -303,9 +303,13 @@ pub mod test {
     use crate::{
         evm_circuit::{witness::Block, EvmCircuitConfig},
         table::{BlockTable, BytecodeTable, CopyTable, ExpTable, KeccakTable, RwTable, TxTable},
-        util::Challenges,
     };
 
+    #[cfg(not(feature = "onephase"))]
+    use crate::util::Challenges;
+    #[cfg(feature = "onephase")]
+    use crate::util::MockChallenges as Challenges;
+
     use eth_types::{Field, Word};
 
     use halo2_proofs::{
@@ -346,6 +350,8 @@ pub mod test {
         }
 
         fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config {
+            let challenges = Challenges::construct(meta);
+            let challenges_expr = challenges.exprs(meta);
             let rw_table = RwTable::construct(meta);
             let tx_table = TxTable::construct(meta);
             let bytecode_table = BytecodeTable::construct(meta);
@@ -354,8 +360,6 @@ pub mod test {
             let copy_table = CopyTable::construct(meta, q_copy_table);
             let keccak_table = KeccakTable::construct(meta);
             let exp_table = ExpTable::construct(meta);
-            let challenges = Challenges::construct(meta);
-            let challenges_expr = challenges.exprs(meta);
             (
                 EvmCircuitConfig::new(
                     meta,

zkevm-circuits/src/evm_circuit/execution.rs

@@ -84,6 +84,7 @@ mod error_oog_constant;
 mod error_oog_log;
 mod error_oog_sload_sstore;
 mod error_oog_static_memory;
+mod error_precompile_failed;
 mod error_stack;
 mod exp;
 mod extcodecopy;
@@ -152,6 +153,7 @@ use error_oog_call::ErrorOOGCallGadget;
 use error_oog_constant::ErrorOOGConstantGadget;
 use error_oog_log::ErrorOOGLogGadget;
 use error_oog_sload_sstore::ErrorOOGSloadSstoreGadget;
+use error_precompile_failed::ErrorPrecompileFailedGadget;
 use error_stack::ErrorStackGadget;
 use exp::ExponentiationGadget;
 use extcodecopy::ExtcodecopyGadget;
@@ -317,7 +319,7 @@ pub(crate) struct ExecutionConfig<F> {
     error_invalid_creation_code: DummyGadget<F, 0, 0, { ExecutionState::ErrorInvalidCreationCode }>,
     error_return_data_out_of_bound:
         DummyGadget<F, 0, 0, { ExecutionState::ErrorReturnDataOutOfBound }>,
-    error_precompile_failed: DummyGadget<F, 0, 0, { ExecutionState::ErrorPrecompileFailed }>,
+    error_precompile_failed: ErrorPrecompileFailedGadget<F>,
 }
 
 impl<F: Field> ExecutionConfig<F> {
@@ -1137,7 +1139,7 @@ impl<F: Field> ExecutionConfig<F> {
             1,
             offset_begin,
         );
-        self.assign_exec_step_int(region, offset_begin, block, transaction, call, step)?;
+        self.assign_exec_step_int(region, offset_begin, block, transaction, call, step, false)?;
 
         region.replicate_assignment_for_range(
             || format!("repeat {:?} rows", step.execution_state),
@@ -1194,12 +1196,14 @@ impl<F: Field> ExecutionConfig<F> {
                 transaction_next,
                 call_next,
                 step_next,
+                false,
             )?;
         }
 
-        self.assign_exec_step_int(region, offset, block, transaction, call, step)
+        self.assign_exec_step_int(region, offset, block, transaction, call, step, true)
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn assign_exec_step_int(
         &self,
         region: &mut CachedRegion<'_, '_, F>,
@@ -1208,6 +1212,7 @@ impl<F: Field> ExecutionConfig<F> {
         transaction: &Transaction,
         call: &Call,
         step: &ExecStep,
+        check_rw: bool,
     ) -> Result<(), Error> {
         self.step
             .assign_exec_step(region, offset, block, transaction, call, step)?;
@@ -1367,7 +1372,7 @@ impl<F: Field> ExecutionConfig<F> {
         let assigned_stored_expressions = self.assign_stored_expressions(region, offset, step)?;
 
         // enable with `CHECK_RW_LOOKUP=true`
-        if *CHECK_RW_LOOKUP {
+        if *CHECK_RW_LOOKUP && check_rw {
             let is_padding_step = matches!(step.execution_state, ExecutionState::EndBlock)
                 && step.rw_indices.is_empty();
             if !is_padding_step {