Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Commit

Permalink
fix(zkevm-circuits/begin_tx): add missing constraints (#1776)
Browse files Browse the repository at this point in the history 
### 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 <[email protected]>
  • Loading branch information
@curryrasul @ed255
curryrasul and ed255 authored Mar 12, 2024
1 parent b82ea41 commit 4996eb6
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 15 deletions.
3 changes: 2 additions & 1 deletion bus-mapping/src/circuit_input_builder/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
26 changes: 25 additions & 1 deletion bus-mapping/src/evm/opcodes/begin_end_tx.rs
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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.
Expand Down
80 changes: 67 additions & 13 deletions zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>,
Expand Down Expand Up @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4996eb6

Please sign in to comment.