fix(zkevm-circuits/begin_tx): add missing constraints (privacy-scaling-explorations#1776)

### Description

This PR aims to fix privacy-scaling-explorations#1475 by adding missing constraints.

### Issue Link

privacy-scaling-explorations#1475

### Type of change

- [X] Bug fix (non-breaking change which fixes an issue)

### Questions / Need Help

1. Meaning of `value_prev` argument in
`account_access_list_write_unchecked`. Why is it sometimes set to 0, and
sometimes to bool values such as `is_coinbase_warm` or
`is_caller_callee_equal`. What is `is_caller_callee_equal` for?
2. There's expression:
https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/main/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs#L163
Why do we have summation - if `is_empty_code_hash.expr()` is enough (as
well as `callee_not_exists` is enough)?
3. What's
[caller_nonce_hash_bytes](https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/main/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs#L186)?
Is it Keccak(sender, nonce) and we have to constrain this exact value?

---------

Co-authored-by: Eduard S <[email protected]>

Author: curryrasul

bus-mapping/src/circuit_input_builder/block.rs

@@ -84,7 +84,8 @@ pub struct Block {
     pub block_steps: BlockSteps,
     /// Copy events in this block.
     pub copy_events: Vec<CopyEvent>,
-    /// Inputs to the SHA3 opcode
+    /// Inputs to the SHA3 opcode as well as data hashed during the EVM execution like init code
+    /// and address creation inputs.
     pub sha3_inputs: Vec<Vec<u8>>,
     /// Exponentiation events in the block.
     pub exp_events: Vec<ExpEvent>,

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

@@ -1,6 +1,8 @@
 use super::TxExecSteps;
 use crate::{
-    circuit_input_builder::{Call, CircuitInputStateRef, ExecState, ExecStep},
+    circuit_input_builder::{
+        Call, CircuitInputStateRef, CopyDataType, CopyEvent, ExecState, ExecStep, NumberOrHash,
+    },
     operation::{AccountField, AccountOp, CallContextField, TxReceiptField, TxRefundOp, RW},
     state_db::CodeDB,
     Error,
@@ -148,6 +150,28 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
             stream.append(&nonce_prev);
             stream.out().to_vec()
         });
+        // We also hash the call_data as it will be used as init code, and the
+        // call_context.code_hash needs to be checked against the hash of this call_data.
+        state.block.sha3_inputs.push(state.tx.call_data.to_vec());
+
+        // Append the copy for the CopyCircuit to calculate RLC(call_data) for the keccack lookup
+        if state.tx.call_data.len() > 0 {
+            state.push_copy(
+                &mut exec_step,
+                CopyEvent {
+                    src_addr: 0,
+                    src_addr_end: state.tx.call_data.len() as u64,
+                    src_type: CopyDataType::TxCalldata,
+                    src_id: NumberOrHash::Number(state.tx.id as usize),
+                    dst_addr: 0,
+                    dst_type: CopyDataType::RlcAcc,
+                    dst_id: NumberOrHash::Number(0),
+                    log_id: None,
+                    rw_counter_start: state.block_ctx.rwc,
+                    bytes: state.tx.call_data.iter().map(|b| (*b, false)).collect(),
+                },
+            );
+        }
     }
 
     // There are 4 branches from here.

zkevm-circuits/src/evm_circuit/execution/begin_tx.rs

@@ -12,21 +12,22 @@ use crate::{
             },
             is_precompiled,
             math_gadget::{
-                ContractCreateGadget, IsEqualWordGadget, IsZeroWordGadget, RangeCheckGadget,
+                ContractCreateGadget, IsEqualWordGadget, IsZeroGadget, IsZeroWordGadget,
+                RangeCheckGadget,
             },
-            not,
+            not, rlc,
             tx::{BeginTxHelperGadget, TxDataGadget},
             AccountAddress, CachedRegion, Cell, StepRws,
         },
         witness::{Block, Call, ExecStep, Transaction},
     },
-    table::{AccountFieldTag, BlockContextFieldTag, CallContextFieldTag},
+    table::{AccountFieldTag, BlockContextFieldTag, CallContextFieldTag, TxContextFieldTag},
     util::{
         word::{Word32Cell, WordExpr, WordLoHi, WordLoHiCell},
         Expr,
     },
 };
-use bus_mapping::state_db::CodeDB;
+use bus_mapping::{circuit_input_builder::CopyDataType, state_db::CodeDB};
 use eth_types::{evm_types::PRECOMPILE_COUNT, keccak256, Field, OpsIdentity, ToWord, U256};
 use halo2_proofs::{
     circuit::Value,
@@ -45,6 +46,9 @@ pub(crate) struct BeginTxGadget<F> {
     code_hash: WordLoHiCell<F>,
     is_empty_code_hash: IsEqualWordGadget<F, WordLoHi<Expression<F>>, WordLoHi<Expression<F>>>,
     caller_nonce_hash_bytes: Word32Cell<F>,
+    calldata_length: Cell<F>,
+    calldata_length_is_zero: IsZeroGadget<F>,
+    calldata_rlc: Cell<F>,
     create: ContractCreateGadget<F, false>,
     callee_not_exists: IsZeroWordGadget<F, WordLoHiCell<F>>,
     is_caller_callee_equal: Cell<F>,
@@ -183,12 +187,23 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
         );
 
         let caller_nonce_hash_bytes = cb.query_word32();
+        let calldata_length = cb.query_cell();
+        let calldata_length_is_zero = cb.is_zero(calldata_length.expr());
+        let calldata_rlc = cb.query_cell_phase2();
         let create = ContractCreateGadget::construct(cb);
         cb.require_equal_word(
             "tx caller address equivalence",
             tx.caller_address.to_word(),
             create.caller_address(),
         );
+
+        cb.require_equal(
+            "tx nonce equivalence",
+            tx.nonce.expr(),
+            create.caller_nonce(),
+        );
+
+        // 1. Handle contract creation transaction.
         cb.condition(tx.is_create.expr(), |cb| {
             cb.require_equal_word(
                 "call callee address equivalence",
@@ -201,21 +216,45 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
                 )
                 .to_word(),
             );
-        });
-        cb.require_equal(
-            "tx nonce equivalence",
-            tx.nonce.expr(),
-            create.caller_nonce(),
-        );
-
-        // 1. Handle contract creation transaction.
-        cb.condition(tx.is_create.expr(), |cb| {
             cb.keccak_table_lookup(
                 create.input_rlc(cb),
                 create.input_length(),
                 caller_nonce_hash_bytes.to_word(),
             );
 
+            cb.tx_context_lookup(
+                tx_id.expr(),
+                TxContextFieldTag::CallDataLength,
+                None,
+                WordLoHi::from_lo_unchecked(calldata_length.expr()),
+            );
+            // If calldata_length > 0 we use the copy circuit to calculate the calldata_rlc for the
+            // keccack input.
+            cb.condition(not::expr(calldata_length_is_zero.expr()), |cb| {
+                cb.copy_table_lookup(
+                    WordLoHi::from_lo_unchecked(tx_id.expr()),
+                    CopyDataType::TxCalldata.expr(),
+                    WordLoHi::zero(),
+                    CopyDataType::RlcAcc.expr(),
+                    0.expr(),
+                    calldata_length.expr(),
+                    0.expr(),
+                    calldata_length.expr(),
+                    calldata_rlc.expr(),
+                    0.expr(),
+                )
+            });
+            // If calldata_length == 0, the copy circuit will not contain any entry, so we skip the
+            // lookup and instead force calldata_rlc to be 0 for the keccack input.
+            cb.condition(calldata_length_is_zero.expr(), |cb| {
+                cb.require_equal("calldata_rlc = 0", calldata_rlc.expr(), 0.expr());
+            });
+            cb.keccak_table_lookup(
+                calldata_rlc.expr(),
+                calldata_length.expr(),
+                cb.curr.state.code_hash.to_word(),
+            );
+
             cb.account_write(
                 call_callee_address.to_word(),
                 AccountFieldTag::Nonce,
@@ -434,6 +473,9 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
             code_hash,
             is_empty_code_hash,
             caller_nonce_hash_bytes,
+            calldata_length,
+            calldata_length_is_zero,
+            calldata_rlc,
             create,
             callee_not_exists,
             is_caller_callee_equal,
@@ -522,6 +564,18 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
             offset,
             U256::from_big_endian(&untrimmed_contract_addr),
         )?;
+        self.calldata_length.assign(
+            region,
+            offset,
+            Value::known(F::from(tx.call_data.len() as u64)),
+        )?;
+        self.calldata_length_is_zero
+            .assign(region, offset, F::from(tx.call_data.len() as u64))?;
+        let calldata_rlc = region
+            .challenges()
+            .keccak_input()
+            .map(|randomness| rlc::value(tx.call_data.iter().rev(), randomness));
+        self.calldata_rlc.assign(region, offset, calldata_rlc)?;
         self.create.assign(
             region,
             offset,