From 9fb3005924d4da54d96194c98e200c04bccd29ea Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Wed, 19 Jan 2022 19:04:59 +0800 Subject: [PATCH 1/7] feat: initial impl for calldatasize gadget and busmapping --- bus-mapping/src/evm/opcodes.rs | 4 +- bus-mapping/src/evm/opcodes/calldatasize.rs | 91 ++++++++++++++ zkevm-circuits/src/evm_circuit/execution.rs | 7 ++ .../src/evm_circuit/execution/calldatasize.rs | 113 ++++++++++++++++++ .../evm_circuit/util/constraint_builder.rs | 2 +- 5 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/calldatasize.rs create mode 100644 zkevm-circuits/src/evm_circuit/execution/calldatasize.rs diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index a7dba642d9..ed04f32e45 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,4 +1,5 @@ //! Definition of each opcode of the EVM. +mod calldatasize; mod coinbase; mod dup; mod jump; @@ -23,6 +24,7 @@ use eth_types::GethExecStep; use log::warn; use self::push::Push; +use calldatasize::CallDataSize; use coinbase::Coinbase; use dup::Dup; use jump::Jump; @@ -100,7 +102,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { // OpcodeId::CALLER => {}, // OpcodeId::CALLVALUE => {}, // OpcodeId::CALLDATALOAD => {}, - // OpcodeId::CALLDATASIZE => {}, + OpcodeId::CALLDATASIZE => CallDataSize::gen_associated_ops, // OpcodeId::CALLDATACOPY => {}, // OpcodeId::CODESIZE => {}, // OpcodeId::CODECOPY => {}, diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs new file mode 100644 index 0000000000..6ff1409c49 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -0,0 +1,91 @@ +use crate::{ + circuit_input_builder::CircuitInputStateRef, eth_types::GethExecStep, + operation::RW, Error, +}; + +use super::Opcode; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct CallDataSize; + +impl Opcode for CallDataSize { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], + ) -> Result<(), Error> { + let current_step = &next_steps[0]; + let call_data_size = next_steps[1].stack.last()?; + + state.push_stack_op( + RW::WRITE, + current_step.stack.last_filled().map(|a| a - 1), + call_data_size, + ); + + Ok(()) + } +} + +#[cfg(test)] +mod calldatasize_tests { + use super::*; + use crate::{ + bytecode, + circuit_input_builder::{ExecStep, TransactionContext}, + eth_types::ToWord, + evm::StackAddress, + mock, + }; + use pretty_assertions::assert_eq; + + #[test] + fn calldatasize_opcode_impl() -> Result<(), Error> { + let code = bytecode! { + #[start] + CALLDATASIZE + STOP + }; + + // Get the execution steps from the external tracer + let block = + mock::BlockData::new_single_tx_trace_code_at_start(&code).unwrap(); + + let mut builder = block.new_circuit_input_builder(); + builder.handle_tx(&block.eth_tx, &block.geth_trace).unwrap(); + + let mut test_builder = block.new_circuit_input_builder(); + let mut tx = test_builder.new_tx(&block.eth_tx).unwrap(); + let mut tx_ctx = TransactionContext::new(&block.eth_tx); + + // Generate step corresponding to COINBASE + let mut step = ExecStep::new( + &block.geth_trace.struct_logs[0], + 0, + test_builder.block_ctx.rwc, + 0, + ); + let mut state_ref = + test_builder.state_ref(&mut tx, &mut tx_ctx, &mut step); + + // Add the last Stack write + state_ref.push_stack_op( + RW::WRITE, + StackAddress::from(1024 - 1), + block.b_constant.coinbase.to_word(), + ); + + tx.steps_mut().push(step); + test_builder.block.txs_mut().push(tx); + + // Compare first step bus mapping instance + assert_eq!( + builder.block.txs()[0].steps()[0].bus_mapping_instance, + test_builder.block.txs()[0].steps()[0].bus_mapping_instance, + ); + + // Compare containers + assert_eq!(builder.block.container, test_builder.block.container); + + Ok(()) + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 76c4e8a6a8..43ea7a840b 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -20,6 +20,7 @@ mod add; mod begin_tx; mod bitwise; mod byte; +mod calldatasize; mod coinbase; mod comparator; mod dup; @@ -42,6 +43,7 @@ use add::AddGadget; use begin_tx::BeginTxGadget; use bitwise::BitwiseGadget; use byte::ByteGadget; +use calldatasize::CallDataSizeGadget; use coinbase::CoinbaseGadget; use comparator::ComparatorGadget; use dup::DupGadget; @@ -89,6 +91,7 @@ pub(crate) struct ExecutionConfig { bitwise_gadget: BitwiseGadget, begin_tx_gadget: BeginTxGadget, byte_gadget: ByteGadget, + calldatasize_gadget: CallDataSizeGadget, comparator_gadget: ComparatorGadget, dup_gadget: DupGadget, error_oog_pure_memory_gadget: ErrorOOGPureMemoryGadget, @@ -218,6 +221,7 @@ impl ExecutionConfig { bitwise_gadget: configure_gadget!(), begin_tx_gadget: configure_gadget!(), byte_gadget: configure_gadget!(), + calldatasize_gadget: configure_gadget!(), comparator_gadget: configure_gadget!(), dup_gadget: configure_gadget!(), error_oog_pure_memory_gadget: configure_gadget!(), @@ -513,6 +517,9 @@ impl ExecutionConfig { ExecutionState::ErrorOutOfGasPureMemory => { assign_exec_step!(self.error_oog_pure_memory_gadget) } + ExecutionState::CALLDATASIZE => { + assign_exec_step!(self.calldatasize_gadget) + } _ => unimplemented!(), } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs new file mode 100644 index 0000000000..92ce51e63b --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -0,0 +1,113 @@ +use halo2::{arithmetic::FieldExt, circuit::Region, plonk::Error}; + +use crate::{ + evm_circuit::{ + step::ExecutionState, + table::{CallContextFieldTag, TxContextFieldTag}, + util::{ + common_gadget::SameContextGadget, + constraint_builder::{ + ConstraintBuilder, StepStateTransition, Transition, + }, + Cell, + }, + witness::{Block, Call, ExecStep, Transaction}, + }, + util::Expr, +}; + +use super::ExecutionGadget; + +#[derive(Clone, Debug)] +pub(crate) struct CallDataSizeGadget { + same_context: SameContextGadget, + tx_id: Cell, + call_data_size: Cell, +} + +impl ExecutionGadget for CallDataSizeGadget { + const NAME: &'static str = "CALLDATASIZE"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::CALLDATASIZE; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + + // Setting the call_id to `None` looks up the current call id. + let tx_id = cb.call_context(None, CallContextFieldTag::TxId); + + // Calldatasize can be looked up in the above tx_id's context. + let call_data_size = cb.query_cell(); + + // The calldatasize should be compared against tx calldata if the call + // is the root call. If not the root call, it is an internal + // call. + cb.condition(cb.curr.state.is_root.expr(), |cb| { + cb.tx_context_lookup( + tx_id.expr(), + TxContextFieldTag::CallDataLength.expr(), + call_data_size.expr(), + ); + }); + cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { + cb.call_context_lookup( + None, + CallContextFieldTag::CallDataLength, + call_data_size.expr(), + ); + }); + + // The calldatasize should be pushed to the top of the stack. + cb.stack_push(call_data_size.expr()); + + let step_state_transition = StepStateTransition { + rw_counter: Transition::Delta(1.expr()), + program_counter: Transition::Delta(1.expr()), + stack_pointer: Transition::Delta((-1).expr()), + ..Default::default() + }; + + let same_context = SameContextGadget::construct( + cb, + opcode, + step_state_transition, + None, + ); + + Self { + same_context, + tx_id, + call_data_size, + } + } + + fn assign_exec_step( + &self, + region: &mut Region<'_, F>, + offset: usize, + _block: &Block, + transaction: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + self.same_context.assign_exec_step(region, offset, step)?; + + self.tx_id.assign( + region, + offset, + Some(F::from(transaction.id as u64)), + )?; + + self.call_data_size.assign( + region, + offset, + Some(F::from(if call.is_root { + transaction.call_data_length as u64 + } else { + call.call_data_length as u64 + })), + )?; + + Ok(()) + } +} diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 62fbf9698b..9f35f51b1c 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -436,7 +436,7 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { word } - fn tx_context_lookup( + pub(crate) fn tx_context_lookup( &mut self, id: Expression, field_tag: Expression, From da23291dd9d02d884c54f9e14e1dcde8ee66f146 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Fri, 21 Jan 2022 00:25:21 +0800 Subject: [PATCH 2/7] fix: follow specs PR, add tests --- .../src/evm_circuit/execution/calldatasize.rs | 152 +++++++++++++----- 1 file changed, 116 insertions(+), 36 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs index 92ce51e63b..051ffd0177 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -3,7 +3,7 @@ use halo2::{arithmetic::FieldExt, circuit::Region, plonk::Error}; use crate::{ evm_circuit::{ step::ExecutionState, - table::{CallContextFieldTag, TxContextFieldTag}, + table::CallContextFieldTag, util::{ common_gadget::SameContextGadget, constraint_builder::{ @@ -21,7 +21,6 @@ use super::ExecutionGadget; #[derive(Clone, Debug)] pub(crate) struct CallDataSizeGadget { same_context: SameContextGadget, - tx_id: Cell, call_data_size: Cell, } @@ -33,35 +32,21 @@ impl ExecutionGadget for CallDataSizeGadget { fn configure(cb: &mut ConstraintBuilder) -> Self { let opcode = cb.query_cell(); - // Setting the call_id to `None` looks up the current call id. - let tx_id = cb.call_context(None, CallContextFieldTag::TxId); - // Calldatasize can be looked up in the above tx_id's context. let call_data_size = cb.query_cell(); - // The calldatasize should be compared against tx calldata if the call - // is the root call. If not the root call, it is an internal - // call. - cb.condition(cb.curr.state.is_root.expr(), |cb| { - cb.tx_context_lookup( - tx_id.expr(), - TxContextFieldTag::CallDataLength.expr(), - call_data_size.expr(), - ); - }); - cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { - cb.call_context_lookup( - None, - CallContextFieldTag::CallDataLength, - call_data_size.expr(), - ); - }); + // Add lookup constraint in the call context for the calldatasize field. + cb.call_context_lookup( + None, + CallContextFieldTag::CallDataLength, + call_data_size.expr(), + ); // The calldatasize should be pushed to the top of the stack. cb.stack_push(call_data_size.expr()); let step_state_transition = StepStateTransition { - rw_counter: Transition::Delta(1.expr()), + rw_counter: Transition::Delta(2.expr()), program_counter: Transition::Delta(1.expr()), stack_pointer: Transition::Delta((-1).expr()), ..Default::default() @@ -76,7 +61,6 @@ impl ExecutionGadget for CallDataSizeGadget { Self { same_context, - tx_id, call_data_size, } } @@ -86,28 +70,124 @@ impl ExecutionGadget for CallDataSizeGadget { region: &mut Region<'_, F>, offset: usize, _block: &Block, - transaction: &Transaction, + _tx: &Transaction, call: &Call, step: &ExecStep, ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - self.tx_id.assign( - region, - offset, - Some(F::from(transaction.id as u64)), - )?; - self.call_data_size.assign( region, offset, - Some(F::from(if call.is_root { - transaction.call_data_length as u64 - } else { - call.call_data_length as u64 - })), + Some(F::from(call.call_data_length as u64)), )?; Ok(()) } } + +#[cfg(test)] +mod test { + use bus_mapping::{ + bytecode, + eth_types::{ToLittleEndian, Word}, + evm::OpcodeId, + }; + use halo2::arithmetic::BaseExt; + use pairing::bn256::Fr; + + use crate::evm_circuit::{ + step::ExecutionState, + table::CallContextFieldTag, + test::{rand_bytes, run_test_circuit_incomplete_fixed_table}, + util::RandomLinearCombination, + witness::{Block, Bytecode, Call, ExecStep, Rw, Transaction}, + }; + + fn test_ok(call_data_size: usize, is_root: bool) { + let randomness = Fr::rand(); + let bytecode = bytecode! { + #[start] + CALLDATASIZE + STOP + }; + let bytecode = Bytecode::new(bytecode.to_vec()); + let call_id = 1; + let call_data = rand_bytes(call_data_size); + + let rws = vec![ + Rw::CallContext { + rw_counter: 9, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataLength, + value: Word::from(call_data_size), + }, + Rw::Stack { + rw_counter: 10, + is_write: true, + call_id, + stack_pointer: 1023, + value: Word::from(call_data_size), + }, + ]; + + let steps = vec![ + ExecStep { + execution_state: ExecutionState::CALLDATASIZE, + rw_indices: vec![0, 1], + rw_counter: 9, + program_counter: 0, + stack_pointer: 1024, + gas_left: OpcodeId::CALLDATASIZE.constant_gas_cost().as_u64(), + gas_cost: OpcodeId::CALLDATASIZE.constant_gas_cost().as_u64(), + opcode: Some(OpcodeId::CALLDATASIZE), + ..Default::default() + }, + ExecStep { + execution_state: ExecutionState::STOP, + rw_counter: 11, + program_counter: 1, + stack_pointer: 1023, + gas_left: 0, + opcode: Some(OpcodeId::STOP), + ..Default::default() + }, + ]; + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + call_data, + call_data_length: call_data_size, + steps, + calls: vec![Call { + id: call_id, + is_root, + is_create: false, + call_data_length: call_data_size, + opcode_source: + RandomLinearCombination::random_linear_combine( + bytecode.hash.to_le_bytes(), + randomness, + ), + ..Default::default() + }], + ..Default::default() + }], + rws, + bytecodes: vec![bytecode], + ..Default::default() + }; + + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } + + #[test] + fn calldatasize_gadget_root() { + test_ok(32, true); + test_ok(64, true); + test_ok(96, true); + } +} From c2bd2f8476144d89f57a19c9933287cc021decb7 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Sun, 23 Jan 2022 10:45:23 +0800 Subject: [PATCH 3/7] fix: imports after refactoring on main --- bus-mapping/src/evm/opcodes/calldatasize.rs | 8 ++++---- zkevm-circuits/src/evm_circuit/execution/calldatasize.rs | 7 ++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 6ff1409c49..0dc7d287e4 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -1,7 +1,7 @@ use crate::{ - circuit_input_builder::CircuitInputStateRef, eth_types::GethExecStep, - operation::RW, Error, + circuit_input_builder::CircuitInputStateRef, operation::RW, Error, }; +use eth_types::GethExecStep; use super::Opcode; @@ -32,10 +32,10 @@ mod calldatasize_tests { use crate::{ bytecode, circuit_input_builder::{ExecStep, TransactionContext}, - eth_types::ToWord, - evm::StackAddress, mock, }; + use eth_types::evm_types::StackAddress; + use eth_types::ToWord; use pretty_assertions::assert_eq; #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs index 051ffd0177..94216f0d2e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -88,11 +88,8 @@ impl ExecutionGadget for CallDataSizeGadget { #[cfg(test)] mod test { - use bus_mapping::{ - bytecode, - eth_types::{ToLittleEndian, Word}, - evm::OpcodeId, - }; + use bus_mapping::{bytecode, evm::OpcodeId}; + use eth_types::{ToLittleEndian, Word}; use halo2::arithmetic::BaseExt; use pairing::bn256::Fr; From d2471a8033fb498de14e20690ebc58edbf888717 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Wed, 19 Jan 2022 19:04:59 +0800 Subject: [PATCH 4/7] feat: initial impl for calldatasize gadget and busmapping --- bus-mapping/src/evm/opcodes.rs | 4 +- bus-mapping/src/evm/opcodes/calldatasize.rs | 91 ++++++++++++++ zkevm-circuits/src/evm_circuit/execution.rs | 7 ++ .../src/evm_circuit/execution/calldatasize.rs | 113 ++++++++++++++++++ .../evm_circuit/util/constraint_builder.rs | 2 +- 5 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/calldatasize.rs create mode 100644 zkevm-circuits/src/evm_circuit/execution/calldatasize.rs diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index a7dba642d9..ed04f32e45 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,4 +1,5 @@ //! Definition of each opcode of the EVM. +mod calldatasize; mod coinbase; mod dup; mod jump; @@ -23,6 +24,7 @@ use eth_types::GethExecStep; use log::warn; use self::push::Push; +use calldatasize::CallDataSize; use coinbase::Coinbase; use dup::Dup; use jump::Jump; @@ -100,7 +102,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { // OpcodeId::CALLER => {}, // OpcodeId::CALLVALUE => {}, // OpcodeId::CALLDATALOAD => {}, - // OpcodeId::CALLDATASIZE => {}, + OpcodeId::CALLDATASIZE => CallDataSize::gen_associated_ops, // OpcodeId::CALLDATACOPY => {}, // OpcodeId::CODESIZE => {}, // OpcodeId::CODECOPY => {}, diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs new file mode 100644 index 0000000000..6ff1409c49 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -0,0 +1,91 @@ +use crate::{ + circuit_input_builder::CircuitInputStateRef, eth_types::GethExecStep, + operation::RW, Error, +}; + +use super::Opcode; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct CallDataSize; + +impl Opcode for CallDataSize { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], + ) -> Result<(), Error> { + let current_step = &next_steps[0]; + let call_data_size = next_steps[1].stack.last()?; + + state.push_stack_op( + RW::WRITE, + current_step.stack.last_filled().map(|a| a - 1), + call_data_size, + ); + + Ok(()) + } +} + +#[cfg(test)] +mod calldatasize_tests { + use super::*; + use crate::{ + bytecode, + circuit_input_builder::{ExecStep, TransactionContext}, + eth_types::ToWord, + evm::StackAddress, + mock, + }; + use pretty_assertions::assert_eq; + + #[test] + fn calldatasize_opcode_impl() -> Result<(), Error> { + let code = bytecode! { + #[start] + CALLDATASIZE + STOP + }; + + // Get the execution steps from the external tracer + let block = + mock::BlockData::new_single_tx_trace_code_at_start(&code).unwrap(); + + let mut builder = block.new_circuit_input_builder(); + builder.handle_tx(&block.eth_tx, &block.geth_trace).unwrap(); + + let mut test_builder = block.new_circuit_input_builder(); + let mut tx = test_builder.new_tx(&block.eth_tx).unwrap(); + let mut tx_ctx = TransactionContext::new(&block.eth_tx); + + // Generate step corresponding to COINBASE + let mut step = ExecStep::new( + &block.geth_trace.struct_logs[0], + 0, + test_builder.block_ctx.rwc, + 0, + ); + let mut state_ref = + test_builder.state_ref(&mut tx, &mut tx_ctx, &mut step); + + // Add the last Stack write + state_ref.push_stack_op( + RW::WRITE, + StackAddress::from(1024 - 1), + block.b_constant.coinbase.to_word(), + ); + + tx.steps_mut().push(step); + test_builder.block.txs_mut().push(tx); + + // Compare first step bus mapping instance + assert_eq!( + builder.block.txs()[0].steps()[0].bus_mapping_instance, + test_builder.block.txs()[0].steps()[0].bus_mapping_instance, + ); + + // Compare containers + assert_eq!(builder.block.container, test_builder.block.container); + + Ok(()) + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index f0185041ab..7dc19dd91d 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -20,6 +20,7 @@ mod add; mod begin_tx; mod bitwise; mod byte; +mod calldatasize; mod coinbase; mod comparator; mod dup; @@ -42,6 +43,7 @@ use add::AddGadget; use begin_tx::BeginTxGadget; use bitwise::BitwiseGadget; use byte::ByteGadget; +use calldatasize::CallDataSizeGadget; use coinbase::CoinbaseGadget; use comparator::ComparatorGadget; use dup::DupGadget; @@ -89,6 +91,7 @@ pub(crate) struct ExecutionConfig { bitwise_gadget: BitwiseGadget, begin_tx_gadget: BeginTxGadget, byte_gadget: ByteGadget, + calldatasize_gadget: CallDataSizeGadget, comparator_gadget: ComparatorGadget, dup_gadget: DupGadget, error_oog_pure_memory_gadget: ErrorOOGPureMemoryGadget, @@ -218,6 +221,7 @@ impl ExecutionConfig { bitwise_gadget: configure_gadget!(), begin_tx_gadget: configure_gadget!(), byte_gadget: configure_gadget!(), + calldatasize_gadget: configure_gadget!(), comparator_gadget: configure_gadget!(), dup_gadget: configure_gadget!(), error_oog_pure_memory_gadget: configure_gadget!(), @@ -513,6 +517,9 @@ impl ExecutionConfig { ExecutionState::ErrorOutOfGasPureMemory => { assign_exec_step!(self.error_oog_pure_memory_gadget) } + ExecutionState::CALLDATASIZE => { + assign_exec_step!(self.calldatasize_gadget) + } _ => unimplemented!(), } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs new file mode 100644 index 0000000000..92ce51e63b --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -0,0 +1,113 @@ +use halo2::{arithmetic::FieldExt, circuit::Region, plonk::Error}; + +use crate::{ + evm_circuit::{ + step::ExecutionState, + table::{CallContextFieldTag, TxContextFieldTag}, + util::{ + common_gadget::SameContextGadget, + constraint_builder::{ + ConstraintBuilder, StepStateTransition, Transition, + }, + Cell, + }, + witness::{Block, Call, ExecStep, Transaction}, + }, + util::Expr, +}; + +use super::ExecutionGadget; + +#[derive(Clone, Debug)] +pub(crate) struct CallDataSizeGadget { + same_context: SameContextGadget, + tx_id: Cell, + call_data_size: Cell, +} + +impl ExecutionGadget for CallDataSizeGadget { + const NAME: &'static str = "CALLDATASIZE"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::CALLDATASIZE; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + + // Setting the call_id to `None` looks up the current call id. + let tx_id = cb.call_context(None, CallContextFieldTag::TxId); + + // Calldatasize can be looked up in the above tx_id's context. + let call_data_size = cb.query_cell(); + + // The calldatasize should be compared against tx calldata if the call + // is the root call. If not the root call, it is an internal + // call. + cb.condition(cb.curr.state.is_root.expr(), |cb| { + cb.tx_context_lookup( + tx_id.expr(), + TxContextFieldTag::CallDataLength.expr(), + call_data_size.expr(), + ); + }); + cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { + cb.call_context_lookup( + None, + CallContextFieldTag::CallDataLength, + call_data_size.expr(), + ); + }); + + // The calldatasize should be pushed to the top of the stack. + cb.stack_push(call_data_size.expr()); + + let step_state_transition = StepStateTransition { + rw_counter: Transition::Delta(1.expr()), + program_counter: Transition::Delta(1.expr()), + stack_pointer: Transition::Delta((-1).expr()), + ..Default::default() + }; + + let same_context = SameContextGadget::construct( + cb, + opcode, + step_state_transition, + None, + ); + + Self { + same_context, + tx_id, + call_data_size, + } + } + + fn assign_exec_step( + &self, + region: &mut Region<'_, F>, + offset: usize, + _block: &Block, + transaction: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + self.same_context.assign_exec_step(region, offset, step)?; + + self.tx_id.assign( + region, + offset, + Some(F::from(transaction.id as u64)), + )?; + + self.call_data_size.assign( + region, + offset, + Some(F::from(if call.is_root { + transaction.call_data_length as u64 + } else { + call.call_data_length as u64 + })), + )?; + + Ok(()) + } +} diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 95a4b23658..4c36b088a0 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -436,7 +436,7 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { word } - fn tx_context_lookup( + pub(crate) fn tx_context_lookup( &mut self, id: Expression, field_tag: Expression, From a55a3cd4a8f223eee9eff6aac1a9c1c4cad74c42 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Fri, 21 Jan 2022 00:25:21 +0800 Subject: [PATCH 5/7] fix: follow specs PR, add tests --- .../src/evm_circuit/execution/calldatasize.rs | 152 +++++++++++++----- 1 file changed, 116 insertions(+), 36 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs index 92ce51e63b..051ffd0177 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -3,7 +3,7 @@ use halo2::{arithmetic::FieldExt, circuit::Region, plonk::Error}; use crate::{ evm_circuit::{ step::ExecutionState, - table::{CallContextFieldTag, TxContextFieldTag}, + table::CallContextFieldTag, util::{ common_gadget::SameContextGadget, constraint_builder::{ @@ -21,7 +21,6 @@ use super::ExecutionGadget; #[derive(Clone, Debug)] pub(crate) struct CallDataSizeGadget { same_context: SameContextGadget, - tx_id: Cell, call_data_size: Cell, } @@ -33,35 +32,21 @@ impl ExecutionGadget for CallDataSizeGadget { fn configure(cb: &mut ConstraintBuilder) -> Self { let opcode = cb.query_cell(); - // Setting the call_id to `None` looks up the current call id. - let tx_id = cb.call_context(None, CallContextFieldTag::TxId); - // Calldatasize can be looked up in the above tx_id's context. let call_data_size = cb.query_cell(); - // The calldatasize should be compared against tx calldata if the call - // is the root call. If not the root call, it is an internal - // call. - cb.condition(cb.curr.state.is_root.expr(), |cb| { - cb.tx_context_lookup( - tx_id.expr(), - TxContextFieldTag::CallDataLength.expr(), - call_data_size.expr(), - ); - }); - cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { - cb.call_context_lookup( - None, - CallContextFieldTag::CallDataLength, - call_data_size.expr(), - ); - }); + // Add lookup constraint in the call context for the calldatasize field. + cb.call_context_lookup( + None, + CallContextFieldTag::CallDataLength, + call_data_size.expr(), + ); // The calldatasize should be pushed to the top of the stack. cb.stack_push(call_data_size.expr()); let step_state_transition = StepStateTransition { - rw_counter: Transition::Delta(1.expr()), + rw_counter: Transition::Delta(2.expr()), program_counter: Transition::Delta(1.expr()), stack_pointer: Transition::Delta((-1).expr()), ..Default::default() @@ -76,7 +61,6 @@ impl ExecutionGadget for CallDataSizeGadget { Self { same_context, - tx_id, call_data_size, } } @@ -86,28 +70,124 @@ impl ExecutionGadget for CallDataSizeGadget { region: &mut Region<'_, F>, offset: usize, _block: &Block, - transaction: &Transaction, + _tx: &Transaction, call: &Call, step: &ExecStep, ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - self.tx_id.assign( - region, - offset, - Some(F::from(transaction.id as u64)), - )?; - self.call_data_size.assign( region, offset, - Some(F::from(if call.is_root { - transaction.call_data_length as u64 - } else { - call.call_data_length as u64 - })), + Some(F::from(call.call_data_length as u64)), )?; Ok(()) } } + +#[cfg(test)] +mod test { + use bus_mapping::{ + bytecode, + eth_types::{ToLittleEndian, Word}, + evm::OpcodeId, + }; + use halo2::arithmetic::BaseExt; + use pairing::bn256::Fr; + + use crate::evm_circuit::{ + step::ExecutionState, + table::CallContextFieldTag, + test::{rand_bytes, run_test_circuit_incomplete_fixed_table}, + util::RandomLinearCombination, + witness::{Block, Bytecode, Call, ExecStep, Rw, Transaction}, + }; + + fn test_ok(call_data_size: usize, is_root: bool) { + let randomness = Fr::rand(); + let bytecode = bytecode! { + #[start] + CALLDATASIZE + STOP + }; + let bytecode = Bytecode::new(bytecode.to_vec()); + let call_id = 1; + let call_data = rand_bytes(call_data_size); + + let rws = vec![ + Rw::CallContext { + rw_counter: 9, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataLength, + value: Word::from(call_data_size), + }, + Rw::Stack { + rw_counter: 10, + is_write: true, + call_id, + stack_pointer: 1023, + value: Word::from(call_data_size), + }, + ]; + + let steps = vec![ + ExecStep { + execution_state: ExecutionState::CALLDATASIZE, + rw_indices: vec![0, 1], + rw_counter: 9, + program_counter: 0, + stack_pointer: 1024, + gas_left: OpcodeId::CALLDATASIZE.constant_gas_cost().as_u64(), + gas_cost: OpcodeId::CALLDATASIZE.constant_gas_cost().as_u64(), + opcode: Some(OpcodeId::CALLDATASIZE), + ..Default::default() + }, + ExecStep { + execution_state: ExecutionState::STOP, + rw_counter: 11, + program_counter: 1, + stack_pointer: 1023, + gas_left: 0, + opcode: Some(OpcodeId::STOP), + ..Default::default() + }, + ]; + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + call_data, + call_data_length: call_data_size, + steps, + calls: vec![Call { + id: call_id, + is_root, + is_create: false, + call_data_length: call_data_size, + opcode_source: + RandomLinearCombination::random_linear_combine( + bytecode.hash.to_le_bytes(), + randomness, + ), + ..Default::default() + }], + ..Default::default() + }], + rws, + bytecodes: vec![bytecode], + ..Default::default() + }; + + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } + + #[test] + fn calldatasize_gadget_root() { + test_ok(32, true); + test_ok(64, true); + test_ok(96, true); + } +} From 2f952757e1759645b70e0d080ed994cc147716a4 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Sun, 23 Jan 2022 10:45:23 +0800 Subject: [PATCH 6/7] fix: imports after refactoring on main --- bus-mapping/src/evm/opcodes/calldatasize.rs | 8 ++++---- zkevm-circuits/src/evm_circuit/execution/calldatasize.rs | 7 ++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 6ff1409c49..0dc7d287e4 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -1,7 +1,7 @@ use crate::{ - circuit_input_builder::CircuitInputStateRef, eth_types::GethExecStep, - operation::RW, Error, + circuit_input_builder::CircuitInputStateRef, operation::RW, Error, }; +use eth_types::GethExecStep; use super::Opcode; @@ -32,10 +32,10 @@ mod calldatasize_tests { use crate::{ bytecode, circuit_input_builder::{ExecStep, TransactionContext}, - eth_types::ToWord, - evm::StackAddress, mock, }; + use eth_types::evm_types::StackAddress; + use eth_types::ToWord; use pretty_assertions::assert_eq; #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs index 051ffd0177..94216f0d2e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatasize.rs @@ -88,11 +88,8 @@ impl ExecutionGadget for CallDataSizeGadget { #[cfg(test)] mod test { - use bus_mapping::{ - bytecode, - eth_types::{ToLittleEndian, Word}, - evm::OpcodeId, - }; + use bus_mapping::{bytecode, evm::OpcodeId}; + use eth_types::{ToLittleEndian, Word}; use halo2::arithmetic::BaseExt; use pairing::bn256::Fr; From 8750ecc18f846668fd4431750068375ba23866d7 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Tue, 1 Feb 2022 09:38:13 +0800 Subject: [PATCH 7/7] fix: remove unnecessary change --- zkevm-circuits/src/evm_circuit/util/constraint_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 4c36b088a0..95a4b23658 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -436,7 +436,7 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { word } - pub(crate) fn tx_context_lookup( + fn tx_context_lookup( &mut self, id: Expression, field_tag: Expression,