From 4996eb6c11fb4eb4bcde7eff55827ecf6cfaaf03 Mon Sep 17 00:00:00 2001 From: Magamedrasul Ibragimov Date: Tue, 12 Mar 2024 12:02:51 +0400 Subject: [PATCH] fix(zkevm-circuits/begin_tx): add missing constraints (#1776) ### Description This PR aims to fix #1475 by adding missing constraints. ### Issue Link #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 --- .../src/circuit_input_builder/block.rs | 3 +- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 26 +++++- .../src/evm_circuit/execution/begin_tx.rs | 80 ++++++++++++++++--- 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/block.rs b/bus-mapping/src/circuit_input_builder/block.rs index fa0abc6f41..219a844ce4 100644 --- a/bus-mapping/src/circuit_input_builder/block.rs +++ b/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, - /// 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>, /// Exponentiation events in the block. pub exp_events: Vec, diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index f9ee29d714..69ad7432f0 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/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 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. diff --git a/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs b/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs index 817b7dbff7..307d2198f0 100644 --- a/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs +++ b/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 { code_hash: WordLoHiCell, is_empty_code_hash: IsEqualWordGadget>, WordLoHi>>, caller_nonce_hash_bytes: Word32Cell, + calldata_length: Cell, + calldata_length_is_zero: IsZeroGadget, + calldata_rlc: Cell, create: ContractCreateGadget, callee_not_exists: IsZeroWordGadget>, is_caller_callee_equal: Cell, @@ -183,12 +187,23 @@ impl ExecutionGadget for BeginTxGadget { ); 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 ExecutionGadget for BeginTxGadget { ) .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 ExecutionGadget for BeginTxGadget { 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 ExecutionGadget for BeginTxGadget { 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,