Merge branch 'main' into feature/debug-expr-evm1

bus-mapping/src/circuit_input_builder.rs

@@ -34,7 +34,7 @@ pub use execution::{
 pub use input_state_ref::CircuitInputStateRef;
 use itertools::Itertools;
 use log::warn;
-use std::collections::HashMap;
+use std::{collections::HashMap, ops::Deref};
 pub use transaction::{Transaction, TransactionContext};
 
 /// Circuit Setup Parameters
@@ -164,6 +164,7 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder<C> {
     /// Create a new Transaction from a [`eth_types::Transaction`].
     pub fn new_tx(
         &mut self,
+        id: u64,
         eth_tx: &eth_types::Transaction,
         is_success: bool,
     ) -> Result<Transaction, Error> {
@@ -180,7 +181,14 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder<C> {
             ),
         );
 
-        Transaction::new(call_id, &self.sdb, &mut self.code_db, eth_tx, is_success)
+        Transaction::new(
+            id,
+            call_id,
+            &self.sdb,
+            &mut self.code_db,
+            eth_tx,
+            is_success,
+        )
     }
 
     /// Iterate over all generated CallContext RwCounterEndOfReversion
@@ -214,8 +222,9 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder<C> {
         eth_tx: &eth_types::Transaction,
         geth_trace: &GethExecTrace,
         is_last_tx: bool,
+        tx_index: u64,
     ) -> Result<(), Error> {
-        let mut tx = self.new_tx(eth_tx, !geth_trace.failed)?;
+        let mut tx = self.new_tx(tx_index, eth_tx, !geth_trace.failed)?;
         let mut tx_ctx = TransactionContext::new(eth_tx, geth_trace, is_last_tx)?;
 
         // Generate BeginTx step
@@ -286,6 +295,7 @@ impl CircuitInputBuilder<FixedCParams> {
             step.bus_mapping_instance.push(op_ref);
         };
 
+        // rwc index start from 1
         let total_rws = state.block_ctx.rwc.0 - 1;
         // We need at least 1 extra Start row
         #[allow(clippy::int_plus_one)]
@@ -297,13 +307,21 @@ impl CircuitInputBuilder<FixedCParams> {
                 max_rws
             );
         }
-        push_op(&mut end_block_last, RWCounter(1), RW::READ, StartOp {});
+        let (padding_start, padding_end) = (1, max_rws - total_rws); // rw counter start from 1
         push_op(
             &mut end_block_last,
-            RWCounter(max_rws - total_rws),
+            RWCounter(padding_start),
             RW::READ,
             StartOp {},
         );
+        if padding_end != padding_start {
+            push_op(
+                &mut end_block_last,
+                RWCounter(padding_end),
+                RW::READ,
+                StartOp {},
+            );
+        }
 
         self.block.block_steps.end_block_not_last = end_block_not_last;
         self.block.block_steps.end_block_last = end_block_last;
@@ -318,9 +336,16 @@ impl<C: CircuitsParams> CircuitInputBuilder<C> {
         geth_traces: &[eth_types::GethExecTrace],
     ) -> Result<(), Error> {
         // accumulates gas across all txs in the block
-        for (tx_index, tx) in eth_block.transactions.iter().enumerate() {
-            let geth_trace = &geth_traces[tx_index];
-            self.handle_tx(tx, geth_trace, tx_index + 1 == eth_block.transactions.len())?;
+        for (idx, tx) in eth_block.transactions.iter().enumerate() {
+            let geth_trace = &geth_traces[idx];
+            // Transaction index starts from 1
+            let tx_id = idx + 1;
+            self.handle_tx(
+                tx,
+                geth_trace,
+                tx_id == eth_block.transactions.len(),
+                tx_id as u64,
+            )?;
         }
         // set eth_block
         self.block.eth_block = eth_block.clone();
@@ -364,7 +389,12 @@ impl CircuitInputBuilder<DynamicCParams> {
                 .fold(0, |acc, c| acc + c.bytes.len())
                 * 2
                 + 2;
-            let max_rws: usize = self.block_ctx.rwc.into();
+
+            let total_rws_before_padding: usize =
+                <RWCounter as Into<usize>>::into(self.block_ctx.rwc) - 1; // -1 since rwc start from index `1`
+            let max_rws_after_padding = total_rws_before_padding
+                + 1 // fill 1 to have exactly one StartOp padding in below `set_end_block`
+                + if total_rws_before_padding > 0 { 1 /*end_block -> CallContextFieldTag::TxId lookup*/ } else { 0 };
             // Computing the number of rows for the EVM circuit requires the size of ExecStep,
             // which is determined in the code of zkevm-circuits and cannot be imported here.
             // When the evm circuit receives a 0 value it dynamically computes the minimum
@@ -376,7 +406,7 @@ impl CircuitInputBuilder<DynamicCParams> {
             // needed.
             let max_keccak_rows = 0;
             FixedCParams {
-                max_rws: max_rws + 3,
+                max_rws: max_rws_after_padding,
                 max_txs,
                 max_calldata,
                 max_copy_rows,
@@ -404,7 +434,7 @@ impl CircuitInputBuilder<DynamicCParams> {
 pub fn keccak_inputs(block: &Block, code_db: &CodeDB) -> Result<Vec<Vec<u8>>, Error> {
     let mut keccak_inputs = Vec::new();
     // Tx Circuit
-    let txs: Vec<geth_types::Transaction> = block.txs.iter().map(|tx| tx.tx.clone()).collect();
+    let txs: Vec<geth_types::Transaction> = block.txs.iter().map(|tx| tx.deref().clone()).collect();
     keccak_inputs.extend_from_slice(&keccak_inputs_tx_circuit(&txs, block.chain_id.as_u64())?);
     // Bytecode Circuit
     for bytecode in code_db.0.values() {

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -1064,9 +1064,10 @@ impl<'a> CircuitInputStateRef<'a> {
         let geth_step = steps
             .get(0)
             .ok_or(Error::InternalError("invalid index 0"))?;
-        let is_return_revert = geth_step.op == OpcodeId::REVERT || geth_step.op == OpcodeId::RETURN;
+        let is_revert_or_return_call_success = geth_step.op == OpcodeId::REVERT
+            || geth_step.op == OpcodeId::RETURN && exec_step.error.is_none();
 
-        if !is_return_revert && !call.is_success {
+        if !is_revert_or_return_call_success && !call.is_success {
             // add call failure ops for exception cases
             self.call_context_read(
                 exec_step,
@@ -1138,7 +1139,7 @@ impl<'a> CircuitInputStateRef<'a> {
         };
         let gas_refund = geth_step.gas - memory_expansion_gas_cost - code_deposit_cost;
 
-        let caller_gas_left = if is_return_revert || call.is_success {
+        let caller_gas_left = if is_revert_or_return_call_success || call.is_success {
             geth_step_next.gas - gas_refund
         } else {
             geth_step_next.gas

bus-mapping/src/circuit_input_builder/tracer_tests.rs

@@ -10,7 +10,7 @@ use crate::{
 };
 use eth_types::{
     address, bytecode,
-    evm_types::{stack::Stack, OpcodeId},
+    evm_types::{stack::Stack, OpcodeId, INVALID_INIT_CODE_FIRST_BYTE},
     geth_types::GethData,
     word, Bytecode, Hash, ToAddress, ToWord, Word,
 };
@@ -37,7 +37,7 @@ impl CircuitInputBuilderTx {
         let block = crate::mock::BlockData::new_from_geth_data(geth_data.clone());
         let mut builder = block.new_circuit_input_builder();
         let tx = builder
-            .new_tx(&block.eth_block.transactions[0], true)
+            .new_tx(0, &block.eth_block.transactions[0], true)
             .unwrap();
         let tx_ctx = TransactionContext::new(
             &block.eth_block.transactions[0],
@@ -742,11 +742,11 @@ fn check_err_invalid_code(step: &GethExecStep, next_step: Option<&GethExecStep>)
         && result(next_step).is_zero()
         && length > Word::zero()
         && !step.memory.is_empty()
-        && step.memory.0.get(offset.low_u64() as usize) == Some(&0xef)
+        && step.memory.0.get(offset.low_u64() as usize) == Some(&INVALID_INIT_CODE_FIRST_BYTE)
 }
 
 #[test]
-fn tracer_err_invalid_code() {
+fn tracer_err_invalid_code_for_create_opcode() {
     // code_creator outputs byte array that starts with 0xef, which is
     // invalid code.
     let code_creator = bytecode! {
@@ -833,6 +833,61 @@ fn tracer_err_invalid_code() {
     );
 }
 
+// Test ErrInvalidCode in transaction deployment (`tx.to == null`).
+#[test]
+fn tracer_err_invalid_code_for_tx_deployment() {
+    // Code creator outputs a byte array that starts with 0xef, which is the
+    // invalid code.
+    let code_creator = bytecode! {
+        PUSH32(word!("0xef00000000000000000000000000000000000000000000000000000000000000")) // value
+        PUSH1(0x00) // offset
+        MSTORE
+        PUSH1(0x01) // length
+        PUSH1(0x00) // offset
+        RETURN
+    };
+
+    // Get the execution steps from external tracer.
+    let block: GethData = TestContext::<2, 1>::new_with_logger_config(
+        None,
+        |accs| {
+            accs[0].address(address!("0x0000000000000000000000000000000000000000"));
+            accs[1].address(*ADDR_B).balance(Word::from(1u64 << 30));
+        },
+        |mut txs, accs| {
+            txs[0]
+                .from(accs[1].address)
+                .gas(60000u64.into())
+                .input(code_creator.into());
+        },
+        |block, _tx| block.number(0x0264),
+        LoggerConfig::enable_memory(),
+    )
+    .unwrap()
+    .into();
+
+    // Get last RETURN.
+    let (index, step) = block.geth_traces[0]
+        .struct_logs
+        .iter()
+        .enumerate()
+        .rev()
+        .find(|(_, s)| s.op == OpcodeId::RETURN)
+        .unwrap();
+    let next_step = block.geth_traces[0].struct_logs.get(index + 1);
+    assert!(check_err_invalid_code(step, next_step));
+
+    // Setup call context at RETURN.
+    let mut builder = CircuitInputBuilderTx::new(&block, step);
+    builder.tx_ctx.call_is_success.push(false);
+    builder.state_ref().push_call(mock_root_create());
+    builder.state_ref().call_ctx_mut().unwrap().memory = step.memory.clone();
+    assert_eq!(
+        builder.state_ref().get_step_err(step, next_step).unwrap(),
+        Some(ExecError::InvalidCreationCode)
+    );
+}
+
 fn check_err_max_code_size_exceeded(step: &GethExecStep, next_step: Option<&GethExecStep>) -> bool {
     let length = step.stack.nth_last(1).unwrap();
     step.op == OpcodeId::RETURN

bus-mapping/src/circuit_input_builder/transaction.rs

@@ -179,8 +179,10 @@ impl TransactionContext {
 #[derive(Debug, Clone, Default)]
 /// Result of the parsing of an Ethereum Transaction.
 pub struct Transaction {
+    /// The transaction id
+    pub id: u64,
     /// The raw transaction fields
-    pub tx: geth_types::Transaction,
+    tx: geth_types::Transaction,
     /// Calls made in the transaction
     pub(crate) calls: Vec<Call>,
     /// Execution steps
@@ -190,6 +192,7 @@ pub struct Transaction {
 impl Transaction {
     /// Create a new Self.
     pub fn new(
+        id: u64,
         call_id: usize,
         sdb: &StateDB,
         code_db: &mut CodeDB,
@@ -244,17 +247,13 @@ impl Transaction {
         };
 
         Ok(Self {
+            id,
             tx: eth_tx.into(),
             calls: vec![call],
             steps: Vec::new(),
         })
     }
 
-    /// Whether this [`Transaction`] is a create one
-    pub fn is_create(&self) -> bool {
-        self.calls[0].is_create()
-    }
-
     /// Return the list of execution steps of this transaction.
     pub fn steps(&self) -> &[ExecStep] {
         &self.steps
@@ -295,8 +294,19 @@ impl Transaction {
         self.steps.is_empty()
     }
 
-    /// Convinient method for gas limit
-    pub fn gas(&self) -> u64 {
-        self.tx.gas_limit.as_u64()
+    /// Constructor for padding tx in tx circuit
+    pub fn padding_tx(id: usize) -> Self {
+        Self {
+            id: id as u64,
+            ..Default::default()
+        }
+    }
+}
+
+impl std::ops::Deref for Transaction {
+    type Target = geth_types::Transaction;
+
+    fn deref(&self) -> &Self::Target {
+        &self.tx
     }
 }

bus-mapping/src/evm/opcodes.rs

@@ -44,6 +44,7 @@ mod stackonlyop;
 mod stop;
 mod swap;
 
+mod error_invalid_creation_code;
 mod error_invalid_jump;
 mod error_oog_call;
 mod error_oog_exp;
@@ -71,6 +72,7 @@ use codecopy::Codecopy;
 use codesize::Codesize;
 use create::Create;
 use dup::Dup;
+use error_invalid_creation_code::ErrorCreationCode;
 use error_invalid_jump::InvalidJump;
 use error_oog_call::OOGCall;
 use error_oog_exp::OOGExp;
@@ -297,6 +299,7 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociate
         }
         ExecError::WriteProtection => Some(ErrorWriteProtection::gen_associated_ops),
         ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops),
+        ExecError::InvalidCreationCode => Some(ErrorCreationCode::gen_associated_ops),
         // call, callcode, create & create2 can encounter DepthError error,
         ExecError::Depth(DepthError::Call) => Some(CallOpcode::<7>::gen_associated_ops),
         ExecError::Depth(DepthError::Create) => Some(Create::<false>::gen_associated_ops),

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

@@ -73,7 +73,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
 
     let init_code_gas_cost = if state.tx.is_create() {
         // Calculate gas cost of init code for EIP-3860.
-        (state.tx.tx.call_data.len() as u64 + 31) / 32 * eth_types::evm_types::INIT_CODE_WORD_GAS
+        (state.tx.call_data.len() as u64 + 31) / 32 * eth_types::evm_types::INIT_CODE_WORD_GAS
     } else {
         0
     };
@@ -82,7 +82,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
         GasCost::CREATION_TX
     } else {
         GasCost::TX
-    } + state.tx.tx.call_data_gas_cost()
+    } + state.tx.call_data_gas_cost()
         + init_code_gas_cost;
     exec_step.gas_cost = intrinsic_gas_cost;
 
@@ -114,7 +114,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
         callee_exists,
         call.is_create(),
         call.value,
-        Some(state.tx.tx.gas_price * state.tx.gas()),
+        Some(state.tx.gas_price * state.tx.gas()),
     )?;
 
     // In case of contract creation we wish to verify the correctness of the
@@ -168,7 +168,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
                 ),
                 (
                     CallContextField::CallDataLength,
-                    state.tx.tx.call_data.len().into(),
+                    state.tx.call_data.len().into(),
                 ),
                 (CallContextField::Value, call.value),
                 (CallContextField::IsStatic, (call.is_static as usize).into()),
@@ -263,7 +263,7 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
     }
     let caller_balance_prev = caller_account.balance;
     let caller_balance =
-        caller_balance_prev + state.tx.tx.gas_price * (exec_step.gas_left + effective_refund);
+        caller_balance_prev + state.tx.gas_price * (exec_step.gas_left + effective_refund);
     state.account_write(
         &mut exec_step,
         call.caller_address,
@@ -272,7 +272,7 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
         caller_balance_prev,
     )?;
 
-    let effective_tip = state.tx.tx.gas_price - state.block.base_fee;
+    let effective_tip = state.tx.gas_price - state.block.base_fee;
     let (found, coinbase_account) = state.sdb.get_account(&state.block.coinbase);
     if !found {
         return Err(Error::AccountNotFound(state.block.coinbase));