From 910484263121c87bdadaf977414269e34723af46 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Tue, 22 Feb 2022 07:10:21 +0800 Subject: [PATCH 01/30] Support generating multiple exec-steps from one geth-step. --- bus-mapping/src/circuit_input_builder.rs | 52 ++++++++++++++-------- bus-mapping/src/evm/opcodes.rs | 18 ++++++-- bus-mapping/src/evm/opcodes/caller.rs | 11 ++++- bus-mapping/src/evm/opcodes/callvalue.rs | 11 ++++- bus-mapping/src/evm/opcodes/dup.rs | 6 ++- bus-mapping/src/evm/opcodes/mload.rs | 9 ++-- bus-mapping/src/evm/opcodes/mstore.rs | 10 +++-- bus-mapping/src/evm/opcodes/number.rs | 3 +- bus-mapping/src/evm/opcodes/sload.rs | 8 ++-- bus-mapping/src/evm/opcodes/stackonlyop.rs | 5 ++- bus-mapping/src/evm/opcodes/stop.rs | 3 +- bus-mapping/src/evm/opcodes/swap.rs | 12 ++--- 12 files changed, 101 insertions(+), 47 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 01498c6056..61d3055abc 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -654,21 +654,34 @@ pub struct CircuitInputStateRef<'a> { pub tx: &'a mut Transaction, /// Transaction Context pub tx_ctx: &'a mut TransactionContext, - /// Step - pub step: &'a mut ExecStep, } impl<'a> CircuitInputStateRef<'a> { + /// + pub fn new_step(&self, geth_step: &GethExecStep) -> ExecStep { + ExecStep::new( + geth_step, + self.tx_ctx.call_index(), + self.block_ctx.rwc, + self.tx_ctx.call_ctx().swc, + ) + } + + /// + pub fn push_step_to_tx(&mut self, step: ExecStep) { + self.tx.steps.push(step); + } + /// Push an [`Operation`] into the [`OperationContainer`] with the next /// [`RWCounter`] and then adds a reference to the stored operation /// ([`OperationRef`]) inside the bus-mapping instance of the current /// [`ExecStep`]. Then increase the block_ctx [`RWCounter`] by one. - pub fn push_op(&mut self, rw: RW, op: T) { + pub fn push_op(&mut self, step: &mut ExecStep, rw: RW, op: T) { let op_ref = self.block .container .insert(Operation::new(self.block_ctx.rwc.inc_pre(), rw, op)); - self.step.bus_mapping_instance.push(op_ref); + step.bus_mapping_instance.push(op_ref); } /// Push an [`Operation`] with reversible to be true into the @@ -679,13 +692,13 @@ impl<'a> CircuitInputStateRef<'a> { /// This method should be used in `Opcode::gen_associated_ops` instead of /// `push_op` when the operation is `RW::WRITE` and it can be reverted (for /// example, a write `StorageOp`). - pub fn push_op_reversible(&mut self, rw: RW, op: T) -> Result<(), Error> { + pub fn push_op_reversible(&mut self, step: &mut ExecStep, rw: RW, op: T) -> Result<(), Error> { let op_ref = self.block.container.insert(Operation::new_reversible( self.block_ctx.rwc.inc_pre(), rw, op, )); - self.step.bus_mapping_instance.push(op_ref); + step.bus_mapping_instance.push(op_ref); // Increase state_write_counter self.call_ctx_mut()?.swc += 1; @@ -710,13 +723,17 @@ impl<'a> CircuitInputStateRef<'a> { /// [`RWCounter`] by one. pub fn push_memory_op( &mut self, + step: &mut ExecStep, rw: RW, address: MemoryAddress, value: u8, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op(rw, MemoryOp::new(call_id, address, value)); - Ok(()) + self.push_op( + step, + rw, + MemoryOp::new(call_id, address, value), + ); } /// Push a [`StackOp`] into the [`OperationContainer`] with the next @@ -726,12 +743,17 @@ impl<'a> CircuitInputStateRef<'a> { /// [`RWCounter`] by one. pub fn push_stack_op( &mut self, + step: &mut ExecStep, rw: RW, address: StackAddress, value: Word, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op(rw, StackOp::new(call_id, address, value)); + self.push_op( + step, + rw, + StackOp::new(call_id, address, value), + ); Ok(()) } @@ -1254,7 +1276,6 @@ impl<'a> CircuitInputBuilder { &'a mut self, tx: &'a mut Transaction, tx_ctx: &'a mut TransactionContext, - step: &'a mut ExecStep, ) -> CircuitInputStateRef { CircuitInputStateRef { sdb: &mut self.sdb, @@ -1263,7 +1284,6 @@ impl<'a> CircuitInputBuilder { block_ctx: &mut self.block_ctx, tx, tx_ctx, - step, } } @@ -1351,18 +1371,13 @@ impl<'a> CircuitInputBuilder { tx.steps.push(step); for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() { - let call_ctx = tx_ctx.call_ctx()?; - let mut step = - ExecStep::new(geth_step, call_ctx.index, self.block_ctx.rwc, call_ctx.swc); - let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx, &mut step); + let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx); gen_associated_ops( &geth_step.op, &mut state_ref, &geth_trace.struct_logs[index..], )?; - - tx.steps.push(step); } // TODO: Move into gen_associated_steps with @@ -1898,8 +1913,7 @@ mod tracer_tests { } fn state_ref(&mut self) -> CircuitInputStateRef { - self.builder - .state_ref(&mut self.tx, &mut self.tx_ctx, &mut self.step) + self.builder.state_ref(&mut self.tx, &mut self.tx_ctx) } } diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index e0102367a4..ab799ddd0e 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -40,9 +40,9 @@ use stop::Stop; use swap::Swap; /// Generic opcode trait which defines the logic of the -/// [`Operation`](crate::operation::Operation) that should be generated for an -/// [`ExecStep`](crate::circuit_input_builder::ExecStep) depending of the -/// [`OpcodeId`] it contains. +/// [`Operation`](crate::operation::Operation) that should be generated for one +/// or multiple [`ExecStep`](crate::circuit_input_builder::ExecStep) depending +/// of the [`OpcodeId`] it contains. pub trait Opcode: Debug { /// Generate the associated [`MemoryOp`](crate::operation::MemoryOp)s, /// [`StackOp`](crate::operation::StackOp)s, and @@ -50,8 +50,20 @@ pub trait Opcode: Debug { /// is implemented for. fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, next_steps: &[GethExecStep], ) -> Result<(), Error>; + + /// + fn gen_associated_ops_multi( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], + ) -> Result<(), Error> { + let mut step = state.new_step(&next_steps[0]); + Self::gen_associated_ops(state, &mut step, next_steps)?; + state.push_step_to_tx(step); + Ok(()) + } } fn dummy_gen_associated_ops( diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index a01ea619dc..9eadd6eab7 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::GethExecStep; @@ -12,6 +12,7 @@ pub(crate) struct Caller; impl Opcode for Caller { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -19,6 +20,7 @@ impl Opcode for Caller { let value = steps[1].stack.last()?; // CallContext read of the caller_address state.push_op( + exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -27,7 +29,12 @@ impl Opcode for Caller { }, ); // Stack write of the caller_address - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; + state.push_stack_op( + exec_step, + RW::WRITE, + step.stack.last_filled().map(|a| a - 1), + value, + )?; Ok(()) } diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index c9901c7fe6..1bc2a59a56 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::GethExecStep; @@ -12,6 +12,7 @@ pub(crate) struct Callvalue; impl Opcode for Callvalue { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -19,6 +20,7 @@ impl Opcode for Callvalue { let value = steps[1].stack.last()?; // CallContext read of the call_value state.push_op( + exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -27,7 +29,12 @@ impl Opcode for Callvalue { }, ); // Stack write of the call_value - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; + state.push_stack_op( + exec_step, + RW::WRITE, + step.stack.last_filled().map(|a| a - 1), + value, + )?; Ok(()) } diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index d06792369d..b6ce4956ab 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -11,15 +11,17 @@ pub(crate) struct Dup; impl Opcode for Dup { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; let stack_value_read = step.stack.nth_last(N - 1)?; let stack_position = step.stack.nth_last_filled(N - 1); - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; state.push_stack_op( + exec_step, RW::WRITE, step.stack.last_filled().map(|a| a - 1), stack_value_read, diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index c4de071d66..f0ac6cadfa 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use core::convert::TryInto; use eth_types::evm_types::MemoryAddress; @@ -16,6 +16,7 @@ pub(crate) struct Mload; impl Opcode for Mload { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -26,7 +27,7 @@ impl Opcode for Mload { let stack_position = step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; // Read the memory let mut mem_read_addr: MemoryAddress = stack_value_read.try_into()?; @@ -40,13 +41,13 @@ impl Opcode for Mload { // // First stack write // - state.push_stack_op(RW::WRITE, stack_position, mem_read_value)?; + state.push_stack_op(exec_step, RW::WRITE, stack_position, mem_read_value)?; // // First mem read -> 32 MemoryOp generated. // for byte in mem_read_value.to_be_bytes() { - state.push_memory_op(RW::READ, mem_read_addr, byte)?; + state.push_memory_op(exec_step, RW::READ, mem_read_addr, byte)?; // Update mem_read_addr to next byte's one mem_read_addr += MemoryAddress::from(1); diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 7d8d7de05b..5ea7bac855 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use core::convert::TryInto; use eth_types::evm_types::MemoryAddress; @@ -14,18 +14,19 @@ pub(crate) struct Mstore; impl Opcode for Mstore { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; // First stack read (offset) let offset = step.stack.nth_last(0)?; let offset_pos = step.stack.nth_last_filled(0); - state.push_stack_op(RW::READ, offset_pos, offset)?; + state.push_stack_op(exec_step, RW::READ, offset_pos, offset)?; // Second stack read (value) let value = step.stack.nth_last(1)?; let value_pos = step.stack.nth_last_filled(1); - state.push_stack_op(RW::READ, value_pos, value)?; + state.push_stack_op(exec_step, RW::READ, value_pos, value)?; // First mem write -> 32 MemoryOp generated. let offset_addr: MemoryAddress = offset.try_into()?; @@ -34,6 +35,7 @@ impl Opcode for Mstore { true => { // stack write operation for mstore8 state.push_memory_op( + exec_step, RW::WRITE, offset_addr, *value.to_le_bytes().first().unwrap(), @@ -43,7 +45,7 @@ impl Opcode for Mstore { // stack write each byte for mstore let bytes = value.to_be_bytes(); for (i, byte) in bytes.iter().enumerate() { - state.push_memory_op(RW::WRITE, offset_addr.map(|a| a + i), *byte)?; + state.push_memory_op(exec_step, RW::WRITE, offset_addr.map(|a| a + i), *byte)?; } } } diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 8351415d72..99640cd2b0 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -38,11 +38,12 @@ mod number_tests { test_builder.block_ctx.rwc, 0, ); - let mut state_ref = test_builder.state_ref(&mut tx, &mut tx_ctx, &mut step); + let mut state_ref = test_builder.state_ref(&mut tx, &mut tx_ctx); // Add the last Stack write let number = block.eth_block.number.unwrap().as_u64(); state_ref.push_stack_op( + &mut step, RW::WRITE, StackAddress::from(1024 - 1), eth_types::U256::from(number), diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 455d199c3a..5158123bd6 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{ operation::{StorageOp, RW}, Error, @@ -15,6 +15,7 @@ pub(crate) struct Sload; impl Opcode for Sload { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -24,11 +25,12 @@ impl Opcode for Sload { let stack_position = step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; // Storage read let storage_value_read = step.storage.get_or_err(&stack_value_read)?; state.push_op( + exec_step, RW::READ, StorageOp::new( state.call()?.address, @@ -41,7 +43,7 @@ impl Opcode for Sload { ); // First stack write - state.push_stack_op(RW::WRITE, stack_position, storage_value_read)?; + state.push_stack_op(exec_step, RW::WRITE, stack_position, storage_value_read)?; Ok(()) } diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index f0b3aeeaf8..d81713b9bf 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -15,6 +15,7 @@ pub(crate) struct StackOnlyOpcode; impl Opcode for StackOnlyOpcode { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -22,6 +23,7 @@ impl Opcode for StackOnlyOpcode Opcode for StackOnlyOpcode Result<(), Error> { state.handle_return()?; diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index fc288983c9..9ff60d379a 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -11,6 +11,7 @@ pub(crate) struct Swap; impl Opcode for Swap { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -18,14 +19,14 @@ impl Opcode for Swap { // Peek b and a let stack_b_value_read = step.stack.nth_last(N)?; let stack_b_position = step.stack.nth_last_filled(N); - state.push_stack_op(RW::READ, stack_b_position, stack_b_value_read)?; + state.push_stack_op(exec_step, RW::READ, stack_b_position, stack_b_value_read)?; let stack_a_value_read = step.stack.last()?; let stack_a_position = step.stack.last_filled(); - state.push_stack_op(RW::READ, stack_a_position, stack_a_value_read)?; + state.push_stack_op(exec_step, RW::READ, stack_a_position, stack_a_value_read)?; // Write a into b_position, write b into a_position - state.push_stack_op(RW::WRITE, stack_b_position, stack_a_value_read)?; - state.push_stack_op(RW::WRITE, stack_a_position, stack_b_value_read)?; + state.push_stack_op(exec_step, RW::WRITE, stack_b_position, stack_a_value_read)?; + state.push_stack_op(exec_step, RW::WRITE, stack_a_position, stack_b_value_read)?; Ok(()) } @@ -74,6 +75,7 @@ mod swap_tests { .filter(|step| step.op.is_swap()) .collect_vec()[i]; + let a_pos = StackAddress(1024 - 6); let b_pos = StackAddress(1024 - 5 + i * 2); let a_val = Word::from(*a); From 27d17c2946b6db1ac6d4220324c2e0a6210c4d72 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 25 Feb 2022 08:49:48 +0800 Subject: [PATCH 02/30] Fix build error. --- bus-mapping/src/evm/opcodes.rs | 2 +- bus-mapping/src/evm/opcodes/calldatasize.rs | 11 +++++++++-- bus-mapping/src/evm/opcodes/selfbalance.rs | 6 +++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index ab799ddd0e..ccfdfbc43b 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -128,7 +128,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { // OpcodeId::DIFFICULTY => {}, // OpcodeId::GASLIMIT => {}, // OpcodeId::CHAINID => {}, - OpcodeId::SELFBALANCE => Selfbalance::gen_associated_ops, + OpcodeId::SELFBALANCE => Selfbalance::gen_associated_ops_multi, // OpcodeId::BASEFEE => {}, OpcodeId::POP => StackOnlyOpcode::<1, 0>::gen_associated_ops, OpcodeId::MLOAD => Mload::gen_associated_ops, diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 32487d0e12..ce79ac7f40 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -1,5 +1,5 @@ use crate::{ - circuit_input_builder::CircuitInputStateRef, + circuit_input_builder::{CircuitInputStateRef, ExecStep}, operation::{CallContextField, CallContextOp, RW}, Error, }; @@ -14,11 +14,13 @@ pub(crate) struct Calldatasize; impl Opcode for Calldatasize { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; let value = steps[1].stack.last()?; state.push_op( + exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -26,7 +28,12 @@ impl Opcode for Calldatasize { value, }, ); - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; + state.push_stack_op( + exec_step, + RW::WRITE, + step.stack.last_filled().map(|a| a - 1), + value, + )?; Ok(()) } } diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index f7769a3f4e..e73e9bb73e 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{AccountField, AccountOp, CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::{GethExecStep, ToWord}; @@ -10,6 +10,7 @@ pub(crate) struct Selfbalance; impl Opcode for Selfbalance { fn gen_associated_ops( state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; @@ -18,6 +19,7 @@ impl Opcode for Selfbalance { // CallContext read of the callee_address state.push_op( + exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -28,6 +30,7 @@ impl Opcode for Selfbalance { // Account read for the balance of the callee_address state.push_op( + exec_step, RW::READ, AccountOp { address: callee_address, @@ -39,6 +42,7 @@ impl Opcode for Selfbalance { // Stack write of self_balance state.push_stack_op( + exec_step, RW::WRITE, step.stack.last_filled().map(|a| a - 1), self_balance, From 24c3806c0bf45dc719c90caba93d06057f842241 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 25 Feb 2022 10:06:17 +0800 Subject: [PATCH 03/30] Fix to use bus-mapping to generate bytecode. --- bus-mapping/src/evm/opcodes.rs | 4 +- .../src/evm_circuit/execution/calldatacopy.rs | 43 ++++++++----------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index ccfdfbc43b..420fb5c4e7 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -15,6 +15,7 @@ use eth_types::{ }; use log::warn; +mod calldatacopy; mod calldatasize; mod caller; mod callvalue; @@ -27,6 +28,7 @@ mod stackonlyop; mod stop; mod swap; +use calldatacopy::Calldatacopy; use calldatasize::Calldatasize; use caller::Caller; use callvalue::Callvalue; @@ -112,7 +114,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { OpcodeId::CALLVALUE => Callvalue::gen_associated_ops, OpcodeId::CALLDATASIZE => Calldatasize::gen_associated_ops, OpcodeId::CALLDATALOAD => StackOnlyOpcode::<1, 1>::gen_associated_ops, - // OpcodeId::CALLDATACOPY => {}, + OpcodeId::CALLDATACOPY => Calldatacopy::gen_associated_ops_multi, // OpcodeId::CODESIZE => {}, // OpcodeId::CODECOPY => {}, // OpcodeId::GASPRICE => {}, diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index bf5ab2f996..91e4eea75c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -243,21 +243,17 @@ mod test { fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { let randomness = Fp::rand(); - let bytecode = Bytecode::new( - [ - vec![OpcodeId::PUSH32.as_u8()], - length.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - data_offset.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - memory_offset.to_be_bytes().to_vec(), - vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], - ] - .concat(), - ); + let mut bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP + }; + let bytecode = Bytecode::new(bytecode.to_vec()); let call_id = 1; let call_data: Vec = rand_bytes(call_data_length); - let mut rws = RwMap( [ ( @@ -386,18 +382,15 @@ mod test { length: Word, ) { let randomness = Fp::rand(); - let bytecode = Bytecode::new( - [ - vec![OpcodeId::PUSH32.as_u8()], - length.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - data_offset.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - memory_offset.to_be_bytes().to_vec(), - vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], - ] - .concat(), - ); + let mut bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP + }; + let bytecode = Bytecode::new(bytecode.to_vec()); let call_id = 1; let call_data = rand_bytes(call_data_length.as_usize()); From 8c26e418d43817d1b657b2513ff8ca77a2857555 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 25 Feb 2022 10:14:53 +0800 Subject: [PATCH 04/30] Update test cases. --- zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 91e4eea75c..4b3a712337 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -235,15 +235,16 @@ mod test { witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; use eth_types::{ + bytecode, evm_types::{GasCost, OpcodeId}, - ToBigEndian, Word, + Word, }; use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { let randomness = Fp::rand(); - let mut bytecode = bytecode! { + let bytecode = bytecode! { PUSH32(length) PUSH32(data_offset) PUSH32(memory_offset) @@ -382,7 +383,7 @@ mod test { length: Word, ) { let randomness = Fp::rand(); - let mut bytecode = bytecode! { + let bytecode = bytecode! { PUSH32(length) PUSH32(data_offset) PUSH32(memory_offset) From 1cdfb99442267b890773893e4f2b760d9ef0738e Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 3 Mar 2022 22:44:04 +0800 Subject: [PATCH 05/30] Add basic of calldatacopy bus-mappinng to just support zero call data length. --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 85 ++++++++++++ .../src/evm_circuit/execution/calldatacopy.rs | 124 +----------------- 2 files changed, 87 insertions(+), 122 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/calldatacopy.rs diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs new file mode 100644 index 0000000000..de77077aa1 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -0,0 +1,85 @@ +use super::Opcode; +use crate::Error; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::operation::RW; +use eth_types::GethExecStep; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct Calldatacopy; + +impl Opcode for Calldatacopy { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, + steps: &[GethExecStep], + ) -> Result<(), Error> { + let step = &steps[0]; + // `CALLDATACOPY` needs three read operation + state.push_stack_op( + exec_step, + RW::READ, + step.stack.nth_last_filled(0), + step.stack.nth_last(0)?, + ); + state.push_stack_op( + exec_step, + RW::READ, + step.stack.nth_last_filled(1), + step.stack.nth_last(1)?, + ); + state.push_stack_op( + exec_step, + RW::READ, + step.stack.nth_last_filled(2), + step.stack.nth_last(2)?, + ); + + Ok(()) + } + + fn gen_associated_ops_multi( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], + ) -> Result<(), Error> { + // Generate an ExecStep of state CALLDATACOPY. + let mut step = state.new_step(&next_steps[0]); + Self::gen_associated_ops(state, &mut step, next_steps)?; + state.push_step_to_tx(step); + + gen_memory_copy_steps(state, next_steps)?; + + Ok(()) + } +} + + +fn gen_memory_copy_steps( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], +) -> Result<(), Error> { + let length = next_steps[0].stack.nth_last(0)?; + if !length.is_zero() { + + /* + + make_memory_copy_steps( + call_id, + &call_data, + 0, + data_offset.as_u64(), + memory_offset.as_u64(), + length.as_usize(), + true, + 100, + 1024, + next_memory_word_size * 32, + &mut rw_counter, + &mut rws, + &mut steps, + ); + + */ + } + + Ok(()) +} diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 4b3a712337..36d073b384 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -234,6 +234,7 @@ mod test { test::{calc_memory_copier_gas_cost, rand_bytes, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; + use crate::test_util::run_test_circuits; use eth_types::{ bytecode, evm_types::{GasCost, OpcodeId}, @@ -243,7 +244,6 @@ mod test { use pairing::bn256::Fr as Fp; fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { - let randomness = Fp::rand(); let bytecode = bytecode! { PUSH32(length) PUSH32(data_offset) @@ -252,127 +252,7 @@ mod test { CALLDATACOPY STOP }; - let bytecode = Bytecode::new(bytecode.to_vec()); - let call_id = 1; - let call_data: Vec = rand_bytes(call_data_length); - let mut rws = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 1, - is_write: false, - call_id, - stack_pointer: 1021, - value: memory_offset, - }, - Rw::Stack { - rw_counter: 2, - is_write: false, - call_id, - stack_pointer: 1022, - value: data_offset, - }, - Rw::Stack { - rw_counter: 3, - is_write: false, - call_id, - stack_pointer: 1023, - value: length, - }, - ], - ), - ( - RwTableTag::CallContext, - vec![Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }], - ), - ] - .into(), - ); - let mut rw_counter = 5; - - let next_memory_word_size = if length.is_zero() { - 0 - } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 - }; - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - - let mut steps = vec![ExecStep { - rw_indices: vec![ - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::Stack, 2), - (RwTableTag::CallContext, 0), - ], - execution_state: ExecutionState::CALLDATACOPY, - rw_counter: 1, - program_counter: 99, - stack_pointer: 1021, - gas_left: gas_cost, - gas_cost, - memory_size: 0, - opcode: Some(OpcodeId::CALLDATACOPY), - ..Default::default() - }]; - - if !length.is_zero() { - make_memory_copy_steps( - call_id, - &call_data, - 0, - data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - true, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - } - - steps.push(ExecStep { - execution_state: ExecutionState::STOP, - rw_counter, - program_counter: 100, - stack_pointer: 1024, - opcode: Some(OpcodeId::STOP), - memory_size: next_memory_word_size * 32, - ..Default::default() - }); - - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - call_data, - call_data_length, - calls: vec![Call { - id: call_id, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - ..Default::default() - }], - steps, - ..Default::default() - }], - rws, - bytecodes: vec![bytecode], - ..Default::default() - }; - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + assert_eq!(run_test_circuits(bytecode), Ok(())); } fn test_ok_internal( From 1c5c57036f2f9ec2064dd6b892977ff2102eb3ad Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 4 Mar 2022 16:48:28 +0800 Subject: [PATCH 06/30] Update bus-mapping calldatacopy. --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 38 +++++++------------ .../src/evm_circuit/execution/calldatacopy.rs | 2 +- zkevm-circuits/src/evm_circuit/witness.rs | 1 + 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index de77077aa1..30b14ec8f5 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -1,7 +1,7 @@ use super::Opcode; -use crate::Error; use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; -use crate::operation::RW; +use crate::operation::{CallContextField, CallContextOp, RW}; +use crate::Error; use eth_types::GethExecStep; #[derive(Clone, Copy, Debug)] @@ -14,7 +14,6 @@ impl Opcode for Calldatacopy { steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; - // `CALLDATACOPY` needs three read operation state.push_stack_op( exec_step, RW::READ, @@ -33,6 +32,15 @@ impl Opcode for Calldatacopy { step.stack.nth_last_filled(2), step.stack.nth_last(2)?, ); + state.push_op( + exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::TxId, + value: state.tx_ctx.id().into(), + }, + ); Ok(()) } @@ -52,33 +60,13 @@ impl Opcode for Calldatacopy { } } - fn gen_memory_copy_steps( - state: &mut CircuitInputStateRef, + _state: &mut CircuitInputStateRef, next_steps: &[GethExecStep], ) -> Result<(), Error> { let length = next_steps[0].stack.nth_last(0)?; if !length.is_zero() { - - /* - - make_memory_copy_steps( - call_id, - &call_data, - 0, - data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - true, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - - */ + // TODO } Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 36d073b384..135daa515e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -243,7 +243,7 @@ mod test { use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; - fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { + fn test_ok_root(_call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { let bytecode = bytecode! { PUSH32(length) PUSH32(data_offset) diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index b5f99c7d6b..44fa188987 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -1121,6 +1121,7 @@ impl From<&bus_mapping::circuit_input_builder::ExecStep> for ExecutionState { OpcodeId::INVALID(_) if [4, 5].contains(&step.bus_mapping_instance.len()) => { ExecutionState::EndTx } + OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, _ => unimplemented!("unimplemented opcode {:?}", step.op), } } From d4d95192207090ea464495eff1334fe2a26206b0 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Tue, 8 Mar 2022 10:10:41 +0800 Subject: [PATCH 07/30] Push op of call_data_length and call data offset. --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 18 ++ .../src/evm_circuit/execution/calldatacopy.rs | 164 +++++++++++++++--- 2 files changed, 161 insertions(+), 21 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 30b14ec8f5..0a9e626be7 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -41,6 +41,24 @@ impl Opcode for Calldatacopy { value: state.tx_ctx.id().into(), }, ); + state.push_op( + exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataLength, + value: state.call().call_data_length.into(), + }, + ); + state.push_op( + exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataOffset, + value: state.call().call_data_offset.into(), + }, + ); Ok(()) } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 135daa515e..91b8022980 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -234,7 +234,6 @@ mod test { test::{calc_memory_copier_gas_cost, rand_bytes, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; - use crate::test_util::run_test_circuits; use eth_types::{ bytecode, evm_types::{GasCost, OpcodeId}, @@ -243,17 +242,139 @@ mod test { use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; - fn test_ok_root(_call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { - let bytecode = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] - CALLDATACOPY - STOP - }; - assert_eq!(run_test_circuits(bytecode), Ok(())); - } + fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { + let randomness = Fp::rand(); + let bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP + }; + let bytecode = Bytecode::new(bytecode.to_vec()); + let call_id = 1; + let call_data: Vec = rand_bytes(call_data_length); + + let mut rws = RwMap( + [ + ( + RwTableTag::Stack, + vec![ + Rw::Stack { + rw_counter: 1, + is_write: false, + call_id, + stack_pointer: 1021, + value: memory_offset, + }, + Rw::Stack { + rw_counter: 2, + is_write: false, + call_id, + stack_pointer: 1022, + value: data_offset, + }, + Rw::Stack { + rw_counter: 3, + is_write: false, + call_id, + stack_pointer: 1023, + value: length, + }, + ], + ), + ( + RwTableTag::CallContext, + vec![Rw::CallContext { + rw_counter: 4, + is_write: false, + call_id, + field_tag: CallContextFieldTag::TxId, + value: Word::one(), + }], + ), + ] + .into(), + ); + let mut rw_counter = 5; + + let next_memory_word_size = if length.is_zero() { + 0 + } else { + (memory_offset.as_u64() + length.as_u64() + 31) / 32 + }; + let gas_cost = GasCost::FASTEST.as_u64() + + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); + + let mut steps = vec![ExecStep { + rw_indices: vec![ + (RwTableTag::Stack, 0), + (RwTableTag::Stack, 1), + (RwTableTag::Stack, 2), + (RwTableTag::CallContext, 0), + ], + execution_state: ExecutionState::CALLDATACOPY, + rw_counter: 1, + program_counter: 99, + stack_pointer: 1021, + gas_left: gas_cost, + gas_cost, + memory_size: 0, + opcode: Some(OpcodeId::CALLDATACOPY), + ..Default::default() + }]; + + if !length.is_zero() { + make_memory_copy_steps( + call_id, + &call_data, + 0, + data_offset.as_u64(), + memory_offset.as_u64(), + length.as_usize(), + true, + 100, + 1024, + next_memory_word_size * 32, + &mut rw_counter, + &mut rws, + &mut steps, + ); + } + + steps.push(ExecStep { + execution_state: ExecutionState::STOP, + rw_counter, + program_counter: 100, + stack_pointer: 1024, + opcode: Some(OpcodeId::STOP), + memory_size: next_memory_word_size * 32, + ..Default::default() + }); + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + call_data, + call_data_length, + calls: vec![Call { + id: call_id, + is_root: true, + is_create: false, + code_source: CodeSource::Account(bytecode.hash), + ..Default::default() + }], + steps, + ..Default::default() + }], + rws, + bytecodes: vec![bytecode], + ..Default::default() + }; + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } fn test_ok_internal( call_data_offset: Word, @@ -262,15 +383,15 @@ mod test { data_offset: Word, length: Word, ) { - let randomness = Fp::rand(); - let bytecode = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] - CALLDATACOPY - STOP - }; + let randomness = Fp::rand(); + let bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP + }; let bytecode = Bytecode::new(bytecode.to_vec()); let call_id = 1; let call_data = rand_bytes(call_data_length.as_usize()); @@ -418,6 +539,7 @@ mod test { bytecodes: vec![bytecode], ..Default::default() }; + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); } From b49fb8c6594d4eeb2fd8cb641f487ba68357d4cd Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Tue, 8 Mar 2022 14:53:26 +0800 Subject: [PATCH 08/30] Add `is_root_call` to BytecodeTestConfig. --- bus-mapping/src/circuit_input_builder.rs | 11 +- bus-mapping/src/evm/opcodes/calldatacopy.rs | 39 ++- bus-mapping/src/evm/opcodes/number.rs | 6 +- .../src/evm_circuit/execution/calldatacopy.rs | 321 ++---------------- zkevm-circuits/src/test_util.rs | 2 + 5 files changed, 66 insertions(+), 313 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 61d3055abc..afc7b23e3a 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -554,6 +554,7 @@ impl Transaction { code_db: &mut CodeDB, eth_tx: ð_types::Transaction, is_success: bool, + is_root: bool, ) -> Result { let (found, _) = sdb.get_account(ð_tx.from); if !found { @@ -1292,6 +1293,7 @@ impl<'a> CircuitInputBuilder { &mut self, eth_tx: ð_types::Transaction, is_success: bool, + is_root: bool, ) -> Result { let call_id = self.block_ctx.rwc.0; @@ -1306,7 +1308,14 @@ impl<'a> CircuitInputBuilder { ), ); - Transaction::new(call_id, &self.sdb, &mut self.code_db, eth_tx, is_success) + Transaction::new( + call_id, + &self.sdb, + &mut self.code_db, + eth_tx, + is_success, + is_root, + ) } /// Iterate over all generated CallContext RwCounterEndOfReversion diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 0a9e626be7..67cccea5e0 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -41,24 +41,27 @@ impl Opcode for Calldatacopy { value: state.tx_ctx.id().into(), }, ); - state.push_op( - exec_step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataLength, - value: state.call().call_data_length.into(), - }, - ); - state.push_op( - exec_step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataOffset, - value: state.call().call_data_offset.into(), - }, - ); + + if !state.call().is_root { + state.push_op( + exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataLength, + value: state.call().call_data_length.into(), + }, + ); + state.push_op( + exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataOffset, + value: state.call().call_data_offset.into(), + }, + ); + } Ok(()) } diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 99640cd2b0..8c12fe51a9 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -23,11 +23,13 @@ mod number_tests { BlockData::new_from_geth_data(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(); + builder + .handle_tx(&block.eth_tx, &block.geth_trace, true) + .unwrap(); let mut test_builder = block.new_circuit_input_builder(); let mut tx = test_builder - .new_tx(&block.eth_tx, !block.geth_trace.failed) + .new_tx(&block.eth_tx, !block.geth_trace.failed, true) .unwrap(); let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 91b8022980..29f02e7390 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -234,6 +234,7 @@ mod test { test::{calc_memory_copier_gas_cost, rand_bytes, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; + use crate::test_util::{run_test_circuits, test_circuits_using_bytecode, BytecodeTestConfig}; use eth_types::{ bytecode, evm_types::{GasCost, OpcodeId}, @@ -242,305 +243,41 @@ mod test { use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; - fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { - let randomness = Fp::rand(); - let bytecode = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] - CALLDATACOPY - STOP - }; - let bytecode = Bytecode::new(bytecode.to_vec()); - let call_id = 1; - let call_data: Vec = rand_bytes(call_data_length); - - let mut rws = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 1, - is_write: false, - call_id, - stack_pointer: 1021, - value: memory_offset, - }, - Rw::Stack { - rw_counter: 2, - is_write: false, - call_id, - stack_pointer: 1022, - value: data_offset, - }, - Rw::Stack { - rw_counter: 3, - is_write: false, - call_id, - stack_pointer: 1023, - value: length, - }, - ], - ), - ( - RwTableTag::CallContext, - vec![Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }], - ), - ] - .into(), - ); - let mut rw_counter = 5; - - let next_memory_word_size = if length.is_zero() { - 0 - } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 - }; - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - - let mut steps = vec![ExecStep { - rw_indices: vec![ - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::Stack, 2), - (RwTableTag::CallContext, 0), - ], - execution_state: ExecutionState::CALLDATACOPY, - rw_counter: 1, - program_counter: 99, - stack_pointer: 1021, - gas_left: gas_cost, - gas_cost, - memory_size: 0, - opcode: Some(OpcodeId::CALLDATACOPY), - ..Default::default() - }]; - - if !length.is_zero() { - make_memory_copy_steps( - call_id, - &call_data, - 0, - data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - true, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - } - - steps.push(ExecStep { - execution_state: ExecutionState::STOP, - rw_counter, - program_counter: 100, - stack_pointer: 1024, - opcode: Some(OpcodeId::STOP), - memory_size: next_memory_word_size * 32, - ..Default::default() - }); - - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - call_data, - call_data_length, - calls: vec![Call { - id: call_id, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - ..Default::default() - }], - steps, - ..Default::default() - }], - rws, - bytecodes: vec![bytecode], - ..Default::default() - }; - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); - } - - fn test_ok_internal( - call_data_offset: Word, - call_data_length: Word, + fn test_ok_root( + _call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word, ) { - let randomness = Fp::rand(); - let bytecode = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] - CALLDATACOPY - STOP - }; - let bytecode = Bytecode::new(bytecode.to_vec()); - let call_id = 1; - let call_data = rand_bytes(call_data_length.as_usize()); - - let mut rws = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 1, - is_write: false, - call_id, - stack_pointer: 1021, - value: memory_offset, - }, - Rw::Stack { - rw_counter: 2, - is_write: false, - call_id, - stack_pointer: 1022, - value: data_offset, - }, - Rw::Stack { - rw_counter: 3, - is_write: false, - call_id, - stack_pointer: 1023, - value: length, - }, - ], - ), - ( - RwTableTag::CallContext, - vec![ - Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }, - Rw::CallContext { - rw_counter: 5, - is_write: false, - call_id, - field_tag: CallContextFieldTag::CallDataLength, - value: call_data_length, - }, - Rw::CallContext { - rw_counter: 6, - is_write: false, - call_id, - field_tag: CallContextFieldTag::CallDataOffset, - value: call_data_offset, - }, - ], - ), - ] - .into(), - ); - let mut rw_counter = 7; - - let curr_memory_word_size = - (call_data_length.as_u64() + call_data_length.as_u64() + 31) / 32; - let next_memory_word_size = if length.is_zero() { - curr_memory_word_size - } else { - std::cmp::max( - curr_memory_word_size, - (memory_offset.as_u64() + length.as_u64() + 31) / 32, - ) + let bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP }; - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost( - curr_memory_word_size, - next_memory_word_size, - length.as_u64(), - ); - let mut steps = vec![ExecStep { - rw_indices: vec![ - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::Stack, 2), - (RwTableTag::CallContext, 0), - (RwTableTag::CallContext, 1), - (RwTableTag::CallContext, 2), - ], - execution_state: ExecutionState::CALLDATACOPY, - rw_counter: 1, - program_counter: 99, - stack_pointer: 1021, - gas_left: gas_cost, - gas_cost, - memory_size: curr_memory_word_size * 32, - opcode: Some(OpcodeId::CALLDATACOPY), - ..Default::default() - }]; - - if !length.is_zero() { - make_memory_copy_steps( - call_id, - &call_data, - call_data_offset.as_u64(), - call_data_offset.as_u64() + data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - false, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - } - - steps.push(ExecStep { - execution_state: ExecutionState::STOP, - rw_counter, - program_counter: 100, - stack_pointer: 1024, - opcode: Some(OpcodeId::STOP), - memory_size: next_memory_word_size * 32, - ..Default::default() - }); + assert_eq!(run_test_circuits(bytecode), Ok(())); + } - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - calls: vec![Call { - id: call_id, - is_root: false, - is_create: false, - call_data_length: call_data_length.as_u64(), - call_data_offset: call_data_offset.as_u64(), - code_source: CodeSource::Account(bytecode.hash), - ..Default::default() - }], - steps, - ..Default::default() - }], - rws, - bytecodes: vec![bytecode], - ..Default::default() + fn test_ok_internal( + _call_data_offset: Word, + _call_data_length: Word, + memory_offset: Word, + data_offset: Word, + length: Word, + ) { + let bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP }; - - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + let mut test_config = BytecodeTestConfig::default(); + test_config.is_root_call = false; + assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); } #[test] diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index ed955b3f03..86c5c0deac 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -32,6 +32,7 @@ pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, pub evm_circuit_lookup_tags: Vec, pub enable_state_circuit_test: bool, + pub is_root_call: bool, pub gas_limit: u64, } @@ -39,6 +40,7 @@ impl Default for BytecodeTestConfig { fn default() -> Self { Self { gas_limit: 1_000_000u64, + is_root_call: true, enable_evm_circuit_test: true, enable_state_circuit_test: true, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), From b02cf91df99c940d3bf572c88c5d353df6441190 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 9 Mar 2022 09:28:16 +0800 Subject: [PATCH 09/30] Replace `OpcodeId` with `ExecState` in bus-mapping ExcStep. --- bus-mapping/src/circuit_input_builder.rs | 19 +++- zkevm-circuits/src/evm_circuit/witness.rs | 107 +++++++++++----------- 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index afc7b23e3a..553aa324b0 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -101,11 +101,24 @@ pub enum ExecError { MaxCodeSizeExceeded, } +/// Execution state +#[derive(Debug)] +pub enum ExecState { + /// EVM Opcode ID + Op(OpcodeId), + /// Virtual step Begin Tx + BeginTx, + /// Virtual step End Tx + EndTx, + /// Virtual step Copy To Memory + CopyToMemory, +} + /// An execution step of the EVM. #[derive(Debug)] pub struct ExecStep { - /// The opcode ID - pub op: OpcodeId, + /// Execution state + pub exec_state: ExecState, /// Program Counter pub pc: ProgramCounter, /// Stack size @@ -140,7 +153,7 @@ impl ExecStep { swc: usize, // State Write Counter ) -> Self { ExecStep { - op: step.op, + exec_state: ExecState::Op(step.op), pc: step.pc, stack_size: step.stack.0.len(), memory_size: step.memory.0.len(), diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 44fa188987..712dfdbdf5 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -1068,61 +1068,61 @@ impl From<&ExecError> for ExecutionState { } } -impl From<&bus_mapping::circuit_input_builder::ExecStep> for ExecutionState { - fn from(step: &bus_mapping::circuit_input_builder::ExecStep) -> Self { +impl From<&circuit_input_builder::ExecStep> for ExecutionState { + fn from(step: &circuit_input_builder::ExecStep) -> Self { if let Some(error) = step.error.as_ref() { return error.into(); } - if step.op.is_dup() { - return ExecutionState::DUP; - } - if step.op.is_push() { - return ExecutionState::PUSH; - } - if step.op.is_swap() { - return ExecutionState::SWAP; - } - match step.op { - OpcodeId::ADD => ExecutionState::ADD, - OpcodeId::MUL => ExecutionState::MUL, - OpcodeId::SUB => ExecutionState::ADD, - OpcodeId::EQ | OpcodeId::LT | OpcodeId::GT => ExecutionState::CMP, - OpcodeId::SLT | OpcodeId::SGT => ExecutionState::SCMP, - OpcodeId::SIGNEXTEND => ExecutionState::SIGNEXTEND, - // TODO: Convert REVERT and RETURN to their own ExecutionState. - OpcodeId::STOP | OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::STOP, - OpcodeId::AND => ExecutionState::BITWISE, - OpcodeId::XOR => ExecutionState::BITWISE, - OpcodeId::OR => ExecutionState::BITWISE, - OpcodeId::POP => ExecutionState::POP, - OpcodeId::PUSH32 => ExecutionState::PUSH, - OpcodeId::BYTE => ExecutionState::BYTE, - OpcodeId::MLOAD => ExecutionState::MEMORY, - OpcodeId::MSTORE => ExecutionState::MEMORY, - OpcodeId::MSTORE8 => ExecutionState::MEMORY, - OpcodeId::JUMPDEST => ExecutionState::JUMPDEST, - OpcodeId::JUMP => ExecutionState::JUMP, - OpcodeId::JUMPI => ExecutionState::JUMPI, - OpcodeId::PC => ExecutionState::PC, - OpcodeId::MSIZE => ExecutionState::MSIZE, - OpcodeId::CALLER => ExecutionState::CALLER, - OpcodeId::CALLVALUE => ExecutionState::CALLVALUE, - OpcodeId::COINBASE => ExecutionState::COINBASE, - OpcodeId::TIMESTAMP => ExecutionState::TIMESTAMP, - OpcodeId::NUMBER => ExecutionState::NUMBER, - OpcodeId::GAS => ExecutionState::GAS, - OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE, - OpcodeId::SLOAD => ExecutionState::SLOAD, - OpcodeId::SSTORE => ExecutionState::SSTORE, - // TODO: Use better way to convert BeginTx and EndTx. - OpcodeId::INVALID(_) if [19, 21].contains(&step.bus_mapping_instance.len()) => { - ExecutionState::BeginTx - } - OpcodeId::INVALID(_) if [4, 5].contains(&step.bus_mapping_instance.len()) => { - ExecutionState::EndTx + match step.exec_state { + circuit_input_builder::ExecState::Op(op) => { + if op.is_dup() { + return ExecutionState::DUP; + } + if op.is_push() { + return ExecutionState::PUSH; + } + if op.is_swap() { + return ExecutionState::SWAP; + } + match op { + OpcodeId::ADD => ExecutionState::ADD, + OpcodeId::MUL => ExecutionState::MUL, + OpcodeId::SUB => ExecutionState::ADD, + OpcodeId::EQ | OpcodeId::LT | OpcodeId::GT => ExecutionState::CMP, + OpcodeId::SLT | OpcodeId::SGT => ExecutionState::SCMP, + OpcodeId::SIGNEXTEND => ExecutionState::SIGNEXTEND, + // TODO: Convert REVERT and RETURN to their own ExecutionState. + OpcodeId::STOP | OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::STOP, + OpcodeId::AND => ExecutionState::BITWISE, + OpcodeId::XOR => ExecutionState::BITWISE, + OpcodeId::OR => ExecutionState::BITWISE, + OpcodeId::POP => ExecutionState::POP, + OpcodeId::PUSH32 => ExecutionState::PUSH, + OpcodeId::BYTE => ExecutionState::BYTE, + OpcodeId::MLOAD => ExecutionState::MEMORY, + OpcodeId::MSTORE => ExecutionState::MEMORY, + OpcodeId::MSTORE8 => ExecutionState::MEMORY, + OpcodeId::JUMPDEST => ExecutionState::JUMPDEST, + OpcodeId::JUMP => ExecutionState::JUMP, + OpcodeId::JUMPI => ExecutionState::JUMPI, + OpcodeId::PC => ExecutionState::PC, + OpcodeId::MSIZE => ExecutionState::MSIZE, + OpcodeId::CALLER => ExecutionState::CALLER, + OpcodeId::CALLVALUE => ExecutionState::CALLVALUE, + OpcodeId::COINBASE => ExecutionState::COINBASE, + OpcodeId::TIMESTAMP => ExecutionState::TIMESTAMP, + OpcodeId::NUMBER => ExecutionState::NUMBER, + OpcodeId::GAS => ExecutionState::GAS, + OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE, + OpcodeId::SLOAD => ExecutionState::SLOAD, + OpcodeId::SSTORE => ExecutionState::SSTORE, + OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, + _ => unimplemented!("unimplemented opcode {:?}", step.op), + } } - OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, - _ => unimplemented!("unimplemented opcode {:?}", step.op), + circuit_input_builder::ExecState::BeginTx => ExecutionState::BeginTx, + circuit_input_builder::ExecState::EndTx => ExecutionState::EndTx, + circuit_input_builder::ExecState::CopyToMemory => ExecutionState::CopyToMemory, } } } @@ -1162,7 +1162,10 @@ fn step_convert(step: &circuit_input_builder::ExecStep) -> ExecStep { stack_pointer: STACK_CAPACITY - step.stack_size, gas_left: step.gas_left.0, gas_cost: step.gas_cost.as_u64(), - opcode: Some(step.op), + opcode: match step.exec_state { + circuit_input_builder::ExecState::Op(op) => Some(op), + _ => None, + }, memory_size: step.memory_size as u64, state_write_counter: step.swc, aux_data: Default::default(), From d851c0aee2098b9fabcaa5dea15613f5234afe19 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 9 Mar 2022 19:18:57 +0800 Subject: [PATCH 10/30] Generate CopyToMemory exection step. --- bus-mapping/src/circuit_input_builder.rs | 23 +++++ bus-mapping/src/evm/opcodes/calldatacopy.rs | 97 +++++++++++++++++++-- zkevm-circuits/src/evm_circuit/witness.rs | 26 +++++- 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 553aa324b0..23610d8050 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -114,6 +114,26 @@ pub enum ExecState { CopyToMemory, } +/// Auxiliary data of Execution step +#[derive(Clone, Debug)] +pub enum StepAuxiliaryData { + /// Auxiliary data of Copy To Memory + CopyToMemory { + /// Source start address + src_addr: u64, + /// Destination address + dst_addr: u64, + /// Bytes left + bytes_left: u64, + /// Source end address + src_addr_end: u64, + /// If from transaction + from_tx: bool, + /// Selectors + selectors: Vec, + }, +} + /// An execution step of the EVM. #[derive(Debug)] pub struct ExecStep { @@ -142,6 +162,8 @@ pub struct ExecStep { pub bus_mapping_instance: Vec, /// Error generated by this step pub error: Option, + /// Step auxiliary data + pub aux_data: Option, } impl ExecStep { @@ -164,6 +186,7 @@ impl ExecStep { swc, bus_mapping_instance: Vec::new(), error: None, + aux_data: None, } } } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 67cccea5e0..8c14588495 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -1,8 +1,15 @@ use super::Opcode; -use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}; use crate::operation::{CallContextField, CallContextOp, RW}; use crate::Error; +use eth_types::evm_types::GasCost; use eth_types::GethExecStep; +// use rand::random; +use std::collections::HashMap; + +// The max number of bytes that can be copied in a step limited by the number +// of cells in a step +const MAX_COPY_BYTES: usize = 71; #[derive(Clone, Copy, Debug)] pub(crate) struct Calldatacopy; @@ -81,13 +88,93 @@ impl Opcode for Calldatacopy { } } +fn gen_memory_copy_step( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], + src_addr: u64, + dst_addr: u64, + src_addr_end: u64, + bytes_left: usize, + from_tx: bool, + bytes_map: &HashMap, +) { + let mut step = state.new_step(&next_steps[0]); + step.exec_state = ExecState::CopyToMemory; + step.gas_cost = GasCost(0); + + let call_id = state.call().call_id; + let mut selectors = vec![0u8; MAX_COPY_BYTES]; + for (idx, selector) in selectors.iter_mut().enumerate() { + if idx < bytes_left { + *selector = 1; + let addr = src_addr + idx as u64; + let byte = if addr < src_addr_end { + debug_assert!(bytes_map.contains_key(&addr)); + if !from_tx { + state.push_memory_op( + &mut step, + RW::READ, + (idx + src_addr as usize).into(), + bytes_map[&addr], + ); + } + bytes_map[&addr] + } else { + 0 + }; + state.push_memory_op(&mut step, RW::WRITE, (idx + dst_addr as usize).into(), byte); + } + } + step.aux_data = Some(StepAuxiliaryData::CopyToMemory { + src_addr, + dst_addr, + bytes_left: bytes_left as u64, + src_addr_end, + from_tx, + selectors, + }); + state.push_step_to_tx(step); +} + fn gen_memory_copy_steps( - _state: &mut CircuitInputStateRef, + state: &mut CircuitInputStateRef, next_steps: &[GethExecStep], ) -> Result<(), Error> { - let length = next_steps[0].stack.nth_last(0)?; - if !length.is_zero() { - // TODO + let memory_offset = next_steps[0].stack.nth_last(0)?.as_u64(); + let data_offset = next_steps[0].stack.nth_last(1)?.as_u64(); + let length = next_steps[0].stack.nth_last(2)?.as_usize(); + + let call_id = state.call().call_id; + let is_root = state.call().is_root; + let (src_addr, buffer_addr, buffer_addr_end) = if is_root { + (data_offset, 0, 0 + state.call().call_data_length) + } else { + ( + data_offset + state.call().call_data_offset, + state.call().call_data_offset, + state.call().call_data_offset + state.call().call_data_length, + ) + }; + // let buffer: Vec = (0..state.call().call_data_length as usize).map(|_| + // random()).collect(); + let buffer: Vec = vec![0; state.call().call_data_length as usize]; + let bytes_map = (buffer_addr..buffer_addr_end) + .zip(buffer.iter().copied()) + .collect(); + + let mut copied = 0; + while copied < length { + gen_memory_copy_step( + state, + next_steps, + src_addr + copied as u64, + memory_offset + copied as u64, + buffer_addr_end, + length - copied, + is_root, + &bytes_map, + ); + copied += MAX_COPY_BYTES; } Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 712dfdbdf5..47f7f1e444 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -283,6 +283,28 @@ pub enum StepAuxiliaryData { }, } +impl From for StepAuxiliaryData { + fn from(data: circuit_input_builder::StepAuxiliaryData) -> Self { + match data { + circuit_input_builder::StepAuxiliaryData::CopyToMemory { + src_addr, + dst_addr, + bytes_left, + src_addr_end, + from_tx, + selectors, + } => StepAuxiliaryData::CopyToMemory { + src_addr, + dst_addr, + bytes_left, + src_addr_end, + from_tx, + selectors, + }, + } + } +} + #[derive(Clone, Debug, Default)] pub struct ExecStep { /// The index in the Transaction calls @@ -1117,7 +1139,7 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { OpcodeId::SLOAD => ExecutionState::SLOAD, OpcodeId::SSTORE => ExecutionState::SSTORE, OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, - _ => unimplemented!("unimplemented opcode {:?}", step.op), + _ => unimplemented!("unimplemented opcode {:?}", op), } } circuit_input_builder::ExecState::BeginTx => ExecutionState::BeginTx, @@ -1168,7 +1190,7 @@ fn step_convert(step: &circuit_input_builder::ExecStep) -> ExecStep { }, memory_size: step.memory_size as u64, state_write_counter: step.swc, - aux_data: Default::default(), + aux_data: step.aux_data.clone().map(Into::into), } } From fc11d4db2c6da964e76f1b91bb26bef2859a5266 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 10 Mar 2022 02:04:42 +0800 Subject: [PATCH 11/30] Add TransactionConfig to bus-mapping handle_tx. --- bus-mapping/src/circuit_input_builder.rs | 39 +++++++++++++++++++----- bus-mapping/src/evm/opcodes/number.rs | 4 +-- zkevm-circuits/src/test_util.rs | 16 ++++++++-- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 23610d8050..1e2629ea66 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -19,7 +19,7 @@ use crate::rpc::GethClient; use ethers_providers::JsonRpcClient; /// Out of Gas errors by opcode -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum OogError { /// Out of Gas for opcodes which have non-zero constant gas cost Constant, @@ -65,7 +65,7 @@ pub enum OogError { } /// EVM Execution Error -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum ExecError { /// Invalid Opcode InvalidOpcode, @@ -102,7 +102,7 @@ pub enum ExecError { } /// Execution state -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum ExecState { /// EVM Opcode ID Op(OpcodeId), @@ -135,7 +135,7 @@ pub enum StepAuxiliaryData { } /// An execution step of the EVM. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct ExecStep { /// Execution state pub exec_state: ExecState, @@ -589,8 +589,8 @@ impl Transaction { sdb: &StateDB, code_db: &mut CodeDB, eth_tx: ð_types::Transaction, + config: TransactionConfig, is_success: bool, - is_root: bool, ) -> Result { let (found, _) = sdb.get_account(ð_tx.from); if !found { @@ -638,6 +638,11 @@ impl Transaction { } }; + let mut input = eth_tx.input.to_vec(); + if input.len() < config.call_data_length { + input.resize(config.call_data_length, 0); + } + Ok(Self { nonce: eth_tx.nonce.as_u64(), gas: eth_tx.gas.as_u64(), @@ -645,7 +650,7 @@ impl Transaction { from: eth_tx.from, to: eth_tx.to.unwrap_or_default(), value: eth_tx.value, - input: eth_tx.input.to_vec(), + input, calls: vec![call], steps: Vec::new(), }) @@ -676,6 +681,24 @@ impl Transaction { } } +#[derive(Debug)] +/// Transaction configuration +pub struct TransactionConfig { + /// If root call + pub is_root_call: bool, + /// Initialized length of call data + pub call_data_length: usize, +} + +impl Default for TransactionConfig { + fn default() -> Self { + Self { + is_root_call: true, + call_data_length: 0, + } + } +} + /// Reference to the internal state of the CircuitInputBuilder in a particular /// [`ExecStep`]. pub struct CircuitInputStateRef<'a> { @@ -1328,8 +1351,8 @@ impl<'a> CircuitInputBuilder { pub fn new_tx( &mut self, eth_tx: ð_types::Transaction, + config: TransactionConfig, is_success: bool, - is_root: bool, ) -> Result { let call_id = self.block_ctx.rwc.0; @@ -1349,8 +1372,8 @@ impl<'a> CircuitInputBuilder { &self.sdb, &mut self.code_db, eth_tx, + config, is_success, - is_root, ) } diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 8c12fe51a9..8f7192da51 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -24,12 +24,12 @@ mod number_tests { let mut builder = block.new_circuit_input_builder(); builder - .handle_tx(&block.eth_tx, &block.geth_trace, true) + .handle_tx(&block.eth_tx, &block.geth_trace, Default::default()) .unwrap(); let mut test_builder = block.new_circuit_input_builder(); let mut tx = test_builder - .new_tx(&block.eth_tx, !block.geth_trace.failed, true) + .new_tx(&block.eth_tx, Default::default(), !block.geth_trace.failed) .unwrap(); let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 86c5c0deac..91314d1ff6 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -30,24 +30,35 @@ pub fn get_fixed_table(conf: FixedTableConfig) -> Vec { pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, - pub evm_circuit_lookup_tags: Vec, pub enable_state_circuit_test: bool, pub is_root_call: bool, + pub call_data_length: usize, pub gas_limit: u64, + pub evm_circuit_lookup_tags: Vec, } impl Default for BytecodeTestConfig { fn default() -> Self { Self { - gas_limit: 1_000_000u64, is_root_call: true, enable_evm_circuit_test: true, enable_state_circuit_test: true, + call_data_length: 0, + gas_limit: 1_000_000u64, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), } } } +impl From<&BytecodeTestConfig> for bus_mapping::circuit_input_builder::TransactionConfig { + fn from(config: &BytecodeTestConfig) -> Self { + Self { + is_root_call: config.is_root_call, + call_data_length: config.call_data_length, + } + } +} + pub fn run_test_circuits(bytecode: eth_types::Bytecode) -> Result<(), Vec> { test_circuits_using_bytecode(bytecode, BytecodeTestConfig::default()) } @@ -67,6 +78,7 @@ pub fn test_circuits_using_bytecode( // build a witness block from trace result let block = crate::evm_circuit::witness::block_convert(&builder.block, &builder.code_db); + // finish required tests according to config using this witness block test_circuits_using_witness_block(block, config) } From 66bd409e223588c402c7c33e0d7d5fc12c609065 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 10 Mar 2022 15:38:29 +0800 Subject: [PATCH 12/30] Update test code. --- bus-mapping/src/circuit_input_builder.rs | 3 + bus-mapping/src/evm/opcodes/calldatacopy.rs | 156 ++++++-- .../src/evm_circuit/execution/calldatacopy.rs | 359 +++++++++++++++++- zkevm-circuits/src/test_util.rs | 5 + 4 files changed, 473 insertions(+), 50 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 1e2629ea66..178572cb46 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -688,6 +688,8 @@ pub struct TransactionConfig { pub is_root_call: bool, /// Initialized length of call data pub call_data_length: usize, + /// Initialized offset of call data + pub call_data_offset: u64, } impl Default for TransactionConfig { @@ -695,6 +697,7 @@ impl Default for TransactionConfig { Self { is_root_call: true, call_data_length: 0, + call_data_offset: 0, } } } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 8c14588495..742ec7bd83 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -1,10 +1,9 @@ use super::Opcode; use crate::circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}; -use crate::operation::{CallContextField, CallContextOp, RW}; +use crate::operation::{CallContextField, CallContextOp, RWCounter, RW}; use crate::Error; -use eth_types::evm_types::GasCost; +use eth_types::evm_types::{Gas, GasCost, ProgramCounter}; use eth_types::GethExecStep; -// use rand::random; use std::collections::HashMap; // The max number of bytes that can be copied in a step limited by the number @@ -21,24 +20,23 @@ impl Opcode for Calldatacopy { steps: &[GethExecStep], ) -> Result<(), Error> { let step = &steps[0]; + let memory_offset = step.stack.nth_last(0)?; + let data_offset = step.stack.nth_last(1)?; + let length = step.stack.nth_last(2)?; + state.push_stack_op( exec_step, RW::READ, step.stack.nth_last_filled(0), - step.stack.nth_last(0)?, + memory_offset, ); state.push_stack_op( exec_step, RW::READ, step.stack.nth_last_filled(1), - step.stack.nth_last(1)?, - ); - state.push_stack_op( - exec_step, - RW::READ, - step.stack.nth_last_filled(2), - step.stack.nth_last(2)?, + data_offset, ); + state.push_stack_op(exec_step, RW::READ, step.stack.nth_last_filled(2), length); state.push_op( exec_step, RW::READ, @@ -49,14 +47,27 @@ impl Opcode for Calldatacopy { }, ); - if !state.call().is_root { + let (memory_word_size, gas_cost) = if state.call().is_root { + let next_memory_word_size = if length.is_zero() { + 0 + } else { + (memory_offset.as_u64() + length.as_u64() + 31) / 32 + }; + + let gas_cost = GasCost::FASTEST.as_u64() + + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); + + (next_memory_word_size, gas_cost) + } else { + let call_data_length = state.call().call_data_length.into(); + let call_data_offset = state.call().call_data_offset.into(); state.push_op( exec_step, RW::READ, CallContextOp { call_id: state.call().call_id, field: CallContextField::CallDataLength, - value: state.call().call_data_length.into(), + value: call_data_length, }, ); state.push_op( @@ -65,10 +76,36 @@ impl Opcode for Calldatacopy { CallContextOp { call_id: state.call().call_id, field: CallContextField::CallDataOffset, - value: state.call().call_data_offset.into(), + value: call_data_offset, }, ); - } + + let curr_memory_word_size = + (call_data_offset.as_u64() + call_data_length.as_u64() + 31) / 32; + let next_memory_word_size = if length.is_zero() { + curr_memory_word_size + } else { + std::cmp::max( + curr_memory_word_size, + (memory_offset.as_u64() + length.as_u64() + 31) / 32, + ) + }; + + let gas_cost = GasCost::FASTEST.as_u64() + + calc_memory_copier_gas_cost( + curr_memory_word_size, + next_memory_word_size, + length.as_u64(), + ); + + (curr_memory_word_size, gas_cost) + }; + + // Should we set `gas_cost` and `memory_size` here? + exec_step.gas_cost = GasCost(gas_cost); + exec_step.memory_size = (memory_word_size * 32) as usize; + + println!("bus-mapping - 2 - {exec_step:?}"); Ok(()) } @@ -78,31 +115,63 @@ impl Opcode for Calldatacopy { next_steps: &[GethExecStep], ) -> Result<(), Error> { // Generate an ExecStep of state CALLDATACOPY. - let mut step = state.new_step(&next_steps[0]); - Self::gen_associated_ops(state, &mut step, next_steps)?; - state.push_step_to_tx(step); + let mut call_data_copy_step = state.new_step(&next_steps[0]); + Self::gen_associated_ops(state, &mut call_data_copy_step, next_steps)?; - gen_memory_copy_steps(state, next_steps)?; + // Generate ExecSteps of virtual state CopyToMemory. + let copy_to_memory_steps = gen_memory_copy_steps(state, &call_data_copy_step, next_steps)?; + + state.push_step_to_tx(call_data_copy_step); + for s in copy_to_memory_steps { + state.push_step_to_tx(s); + } Ok(()) } } +fn calc_memory_expension_gas_cost(curr_memory_word_size: u64, next_memory_word_size: u64) -> u64 { + if next_memory_word_size <= curr_memory_word_size { + 0 + } else { + let total_cost = |mem_word_size| { + mem_word_size * GasCost::MEMORY.as_u64() + mem_word_size * mem_word_size / 512 + }; + total_cost(next_memory_word_size) - total_cost(curr_memory_word_size) + } +} + +fn calc_memory_copier_gas_cost( + curr_memory_word_size: u64, + next_memory_word_size: u64, + num_copy_bytes: u64, +) -> u64 { + let num_words = (num_copy_bytes + 31) / 32; + num_words * GasCost::COPY.as_u64() + + calc_memory_expension_gas_cost(curr_memory_word_size, next_memory_word_size) +} + fn gen_memory_copy_step( state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], + last_step: &ExecStep, src_addr: u64, dst_addr: u64, src_addr_end: u64, bytes_left: usize, + memory_size: usize, from_tx: bool, bytes_map: &HashMap, -) { - let mut step = state.new_step(&next_steps[0]); +) -> ExecStep { + let mut step = last_step.clone(); + step.rwc = RWCounter(step.rwc.0 + step.bus_mapping_instance.len()); + step.bus_mapping_instance = Vec::new(); step.exec_state = ExecState::CopyToMemory; + step.gas_left = Gas(step.gas_left.0 - step.gas_cost.0); step.gas_cost = GasCost(0); + step.pc = ProgramCounter(step.pc.0 + 1); + step.stack_size = 0; + step.memory_size = memory_size; - let call_id = state.call().call_id; let mut selectors = vec![0u8; MAX_COPY_BYTES]; for (idx, selector) in selectors.iter_mut().enumerate() { if idx < bytes_left { @@ -133,49 +202,62 @@ fn gen_memory_copy_step( from_tx, selectors, }); - state.push_step_to_tx(step); + step } fn gen_memory_copy_steps( state: &mut CircuitInputStateRef, + call_data_copy_step: &ExecStep, next_steps: &[GethExecStep], -) -> Result<(), Error> { +) -> Result, Error> { let memory_offset = next_steps[0].stack.nth_last(0)?.as_u64(); let data_offset = next_steps[0].stack.nth_last(1)?.as_u64(); let length = next_steps[0].stack.nth_last(2)?.as_usize(); - let call_id = state.call().call_id; let is_root = state.call().is_root; let (src_addr, buffer_addr, buffer_addr_end) = if is_root { - (data_offset, 0, 0 + state.call().call_data_length) + (data_offset, 0, 0 + state.tx.input.len() as u64) } else { + let call_data_offset = state.call().call_data_offset; + ( - data_offset + state.call().call_data_offset, - state.call().call_data_offset, - state.call().call_data_offset + state.call().call_data_length, + call_data_offset + data_offset, + call_data_offset, + call_data_offset + state.tx.input.len() as u64, ) }; - // let buffer: Vec = (0..state.call().call_data_length as usize).map(|_| - // random()).collect(); - let buffer: Vec = vec![0; state.call().call_data_length as usize]; + + let buffer: Vec = vec![0; (buffer_addr_end - buffer_addr) as usize]; + + let memory_size = if length == 0 { + 0 + } else { + (memory_offset + length as u64 + 31) / 32 * 32 + }; + let bytes_map = (buffer_addr..buffer_addr_end) .zip(buffer.iter().copied()) .collect(); let mut copied = 0; + let mut steps = vec![]; + let mut last_step = call_data_copy_step; + while copied < length { - gen_memory_copy_step( + steps.push(gen_memory_copy_step( state, - next_steps, + last_step, src_addr + copied as u64, memory_offset + copied as u64, buffer_addr_end, length - copied, + memory_size as usize, is_root, &bytes_map, - ); + )); + last_step = steps.last().unwrap(); copied += MAX_COPY_BYTES; } - Ok(()) + Ok(steps) } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 29f02e7390..1cbcc94d3c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -234,21 +234,16 @@ mod test { test::{calc_memory_copier_gas_cost, rand_bytes, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; - use crate::test_util::{run_test_circuits, test_circuits_using_bytecode, BytecodeTestConfig}; + use crate::test_util::{test_circuits_using_bytecode, BytecodeTestConfig}; use eth_types::{ bytecode, evm_types::{GasCost, OpcodeId}, - Word, + ToBigEndian, Word, }; use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; - fn test_ok_root( - _call_data_length: usize, - memory_offset: Word, - data_offset: Word, - length: Word, - ) { + fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { let bytecode = bytecode! { PUSH32(length) PUSH32(data_offset) @@ -257,12 +252,17 @@ mod test { CALLDATACOPY STOP }; - assert_eq!(run_test_circuits(bytecode), Ok(())); + let test_config = BytecodeTestConfig { + is_root_call: true, + call_data_length, + ..Default::default() + }; + assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); } fn test_ok_internal( - _call_data_offset: Word, - _call_data_length: Word, + call_data_offset: Word, + call_data_length: Word, memory_offset: Word, data_offset: Word, length: Word, @@ -275,14 +275,30 @@ mod test { CALLDATACOPY STOP }; - let mut test_config = BytecodeTestConfig::default(); - test_config.is_root_call = false; + let test_config = BytecodeTestConfig { + is_root_call: false, + call_data_length: call_data_length.as_usize(), + call_data_offset: call_data_offset.as_u64(), + ..Default::default() + }; assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); } #[test] fn calldatacopy_gadget_simple() { + /* + test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(10)); + */ + + + test_ok_internal_old( + Word::from(0x40), + Word::from(64), + Word::from(0xA0), + Word::from(16), + Word::from(10), + ); test_ok_internal( Word::from(0x40), Word::from(64), @@ -290,6 +306,8 @@ mod test { Word::from(16), Word::from(10), ); + + // assert!(false); } #[test] @@ -326,4 +344,319 @@ mod test { Word::from(0), ); } + + fn test_ok_root_old( + call_data_length: usize, + memory_offset: Word, + data_offset: Word, + length: Word, + ) { + let randomness = Fp::rand(); + let bytecode = Bytecode::new( + [ + vec![OpcodeId::PUSH32.as_u8()], + length.to_be_bytes().to_vec(), + vec![OpcodeId::PUSH32.as_u8()], + data_offset.to_be_bytes().to_vec(), + vec![OpcodeId::PUSH32.as_u8()], + memory_offset.to_be_bytes().to_vec(), + vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], + ] + .concat(), + ); + let call_id = 1; + let call_data: Vec = rand_bytes(call_data_length); + + let mut rws = RwMap( + [ + ( + RwTableTag::Stack, + vec![ + Rw::Stack { + rw_counter: 1, + is_write: false, + call_id, + stack_pointer: 1021, + value: memory_offset, + }, + Rw::Stack { + rw_counter: 2, + is_write: false, + call_id, + stack_pointer: 1022, + value: data_offset, + }, + Rw::Stack { + rw_counter: 3, + is_write: false, + call_id, + stack_pointer: 1023, + value: length, + }, + ], + ), + ( + RwTableTag::CallContext, + vec![Rw::CallContext { + rw_counter: 4, + is_write: false, + call_id, + field_tag: CallContextFieldTag::TxId, + value: Word::one(), + }], + ), + ] + .into(), + ); + let mut rw_counter = 5; + + let next_memory_word_size = if length.is_zero() { + 0 + } else { + (memory_offset.as_u64() + length.as_u64() + 31) / 32 + }; + let gas_cost = GasCost::FASTEST.as_u64() + + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); + + let mut steps = vec![ExecStep { + rw_indices: vec![ + (RwTableTag::Stack, 0), + (RwTableTag::Stack, 1), + (RwTableTag::Stack, 2), + (RwTableTag::CallContext, 0), + ], + execution_state: ExecutionState::CALLDATACOPY, + rw_counter: 1, + program_counter: 99, + stack_pointer: 1021, + gas_left: gas_cost, + gas_cost, + memory_size: 0, + opcode: Some(OpcodeId::CALLDATACOPY), + ..Default::default() + }]; + + if !length.is_zero() { + make_memory_copy_steps( + call_id, + &call_data, + 0, + data_offset.as_u64(), + memory_offset.as_u64(), + length.as_usize(), + true, + 100, + 1024, + next_memory_word_size * 32, + &mut rw_counter, + &mut rws, + &mut steps, + ); + } + + steps.push(ExecStep { + execution_state: ExecutionState::STOP, + rw_counter, + program_counter: 100, + stack_pointer: 1024, + opcode: Some(OpcodeId::STOP), + memory_size: next_memory_word_size * 32, + ..Default::default() + }); + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + call_data, + call_data_length, + calls: vec![Call { + id: call_id, + is_root: true, + is_create: false, + code_source: CodeSource::Account(bytecode.hash), + ..Default::default() + }], + steps, + ..Default::default() + }], + rws, + bytecodes: vec![bytecode], + ..Default::default() + }; + + println!("zkevm - 2 - {block:?}"); + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } + + fn test_ok_internal_old( + call_data_offset: Word, + call_data_length: Word, + memory_offset: Word, + data_offset: Word, + length: Word, + ) { + let randomness = Fp::rand(); + let bytecode = Bytecode::new( + [ + vec![OpcodeId::PUSH32.as_u8()], + length.to_be_bytes().to_vec(), + vec![OpcodeId::PUSH32.as_u8()], + data_offset.to_be_bytes().to_vec(), + vec![OpcodeId::PUSH32.as_u8()], + memory_offset.to_be_bytes().to_vec(), + vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], + ] + .concat(), + ); + let call_id = 1; + let call_data = rand_bytes(call_data_length.as_usize()); + + let mut rws = RwMap( + [ + ( + RwTableTag::Stack, + vec![ + Rw::Stack { + rw_counter: 1, + is_write: false, + call_id, + stack_pointer: 1021, + value: memory_offset, + }, + Rw::Stack { + rw_counter: 2, + is_write: false, + call_id, + stack_pointer: 1022, + value: data_offset, + }, + Rw::Stack { + rw_counter: 3, + is_write: false, + call_id, + stack_pointer: 1023, + value: length, + }, + ], + ), + ( + RwTableTag::CallContext, + vec![ + Rw::CallContext { + rw_counter: 4, + is_write: false, + call_id, + field_tag: CallContextFieldTag::TxId, + value: Word::one(), + }, + Rw::CallContext { + rw_counter: 5, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataLength, + value: call_data_length, + }, + Rw::CallContext { + rw_counter: 6, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataOffset, + value: call_data_offset, + }, + ], + ), + ] + .into(), + ); + let mut rw_counter = 7; + + let curr_memory_word_size = + (call_data_length.as_u64() + call_data_length.as_u64() + 31) / 32; + let next_memory_word_size = if length.is_zero() { + curr_memory_word_size + } else { + std::cmp::max( + curr_memory_word_size, + (memory_offset.as_u64() + length.as_u64() + 31) / 32, + ) + }; + let gas_cost = GasCost::FASTEST.as_u64() + + calc_memory_copier_gas_cost( + curr_memory_word_size, + next_memory_word_size, + length.as_u64(), + ); + let mut steps = vec![ExecStep { + rw_indices: vec![ + (RwTableTag::Stack, 0), + (RwTableTag::Stack, 1), + (RwTableTag::Stack, 2), + (RwTableTag::CallContext, 0), + (RwTableTag::CallContext, 1), + (RwTableTag::CallContext, 2), + ], + execution_state: ExecutionState::CALLDATACOPY, + rw_counter: 1, + program_counter: 99, + stack_pointer: 1021, + gas_left: gas_cost, + gas_cost, + memory_size: curr_memory_word_size * 32, + opcode: Some(OpcodeId::CALLDATACOPY), + ..Default::default() + }]; + + if !length.is_zero() { + make_memory_copy_steps( + call_id, + &call_data, + call_data_offset.as_u64(), + call_data_offset.as_u64() + data_offset.as_u64(), + memory_offset.as_u64(), + length.as_usize(), + false, + 100, + 1024, + next_memory_word_size * 32, + &mut rw_counter, + &mut rws, + &mut steps, + ); + } + + steps.push(ExecStep { + execution_state: ExecutionState::STOP, + rw_counter, + program_counter: 100, + stack_pointer: 1024, + opcode: Some(OpcodeId::STOP), + memory_size: next_memory_word_size * 32, + ..Default::default() + }); + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + calls: vec![Call { + id: call_id, + is_root: false, + is_create: false, + call_data_length: call_data_length.as_u64(), + call_data_offset: call_data_offset.as_u64(), + code_source: CodeSource::Account(bytecode.hash), + ..Default::default() + }], + steps, + ..Default::default() + }], + rws, + bytecodes: vec![bytecode], + ..Default::default() + }; + + println!("zkevm - internal- 2 - {block:?}"); + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 91314d1ff6..34b27b94a8 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -33,6 +33,7 @@ pub struct BytecodeTestConfig { pub enable_state_circuit_test: bool, pub is_root_call: bool, pub call_data_length: usize, + pub call_data_offset: u64, pub gas_limit: u64, pub evm_circuit_lookup_tags: Vec, } @@ -44,6 +45,7 @@ impl Default for BytecodeTestConfig { enable_evm_circuit_test: true, enable_state_circuit_test: true, call_data_length: 0, + call_data_offset: 0, gas_limit: 1_000_000u64, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), } @@ -55,6 +57,7 @@ impl From<&BytecodeTestConfig> for bus_mapping::circuit_input_builder::Transacti Self { is_root_call: config.is_root_call, call_data_length: config.call_data_length, + call_data_offset: config.call_data_offset, } } } @@ -79,6 +82,8 @@ pub fn test_circuits_using_bytecode( // build a witness block from trace result let block = crate::evm_circuit::witness::block_convert(&builder.block, &builder.code_db); + println!("zkevm - 1 - {block:?}"); + // finish required tests according to config using this witness block test_circuits_using_witness_block(block, config) } From b85ae04bf68a1d49ad709d52e83ee309671ccfbe Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 11 Mar 2022 09:22:38 +0800 Subject: [PATCH 13/30] 1. Remove TransactionConfig and replace with `call_data`. 2. Remove gas calculation and set in `calldatacopy` bus-mapping. 3. Revert `test_ok_internal` since not want to set internal call. --- bus-mapping/src/circuit_input_builder.rs | 14 +- bus-mapping/src/evm/opcodes/calldatacopy.rs | 68 +---- .../src/evm_circuit/execution/calldatacopy.rs | 279 ++++++++---------- zkevm-circuits/src/test_util.rs | 18 +- 4 files changed, 134 insertions(+), 245 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 178572cb46..5bb4016a01 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -589,8 +589,8 @@ impl Transaction { sdb: &StateDB, code_db: &mut CodeDB, eth_tx: ð_types::Transaction, - config: TransactionConfig, is_success: bool, + call_data: Option<&[u8]>, ) -> Result { let (found, _) = sdb.get_account(ð_tx.from); if !found { @@ -638,10 +638,10 @@ impl Transaction { } }; - let mut input = eth_tx.input.to_vec(); - if input.len() < config.call_data_length { - input.resize(config.call_data_length, 0); - } + let input = match call_data { + Some(data) => data.to_vec(), + None => eth_tx.input.to_vec(), + }; Ok(Self { nonce: eth_tx.nonce.as_u64(), @@ -1354,8 +1354,8 @@ impl<'a> CircuitInputBuilder { pub fn new_tx( &mut self, eth_tx: ð_types::Transaction, - config: TransactionConfig, is_success: bool, + call_data: Option<&[u8]>, ) -> Result { let call_id = self.block_ctx.rwc.0; @@ -1375,8 +1375,8 @@ impl<'a> CircuitInputBuilder { &self.sdb, &mut self.code_db, eth_tx, - config, is_success, + call_data, ) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 742ec7bd83..563ca77467 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -2,7 +2,7 @@ use super::Opcode; use crate::circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}; use crate::operation::{CallContextField, CallContextOp, RWCounter, RW}; use crate::Error; -use eth_types::evm_types::{Gas, GasCost, ProgramCounter}; +use eth_types::evm_types::ProgramCounter; use eth_types::GethExecStep; use std::collections::HashMap; @@ -47,27 +47,14 @@ impl Opcode for Calldatacopy { }, ); - let (memory_word_size, gas_cost) = if state.call().is_root { - let next_memory_word_size = if length.is_zero() { - 0 - } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 - }; - - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - - (next_memory_word_size, gas_cost) - } else { - let call_data_length = state.call().call_data_length.into(); - let call_data_offset = state.call().call_data_offset.into(); + if !state.call().is_root { state.push_op( exec_step, RW::READ, CallContextOp { call_id: state.call().call_id, field: CallContextField::CallDataLength, - value: call_data_length, + value: state.call().call_data_length.into(), }, ); state.push_op( @@ -76,35 +63,11 @@ impl Opcode for Calldatacopy { CallContextOp { call_id: state.call().call_id, field: CallContextField::CallDataOffset, - value: call_data_offset, + value: state.call().call_data_offset.into(), }, ); - - let curr_memory_word_size = - (call_data_offset.as_u64() + call_data_length.as_u64() + 31) / 32; - let next_memory_word_size = if length.is_zero() { - curr_memory_word_size - } else { - std::cmp::max( - curr_memory_word_size, - (memory_offset.as_u64() + length.as_u64() + 31) / 32, - ) - }; - - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost( - curr_memory_word_size, - next_memory_word_size, - length.as_u64(), - ); - - (curr_memory_word_size, gas_cost) }; - // Should we set `gas_cost` and `memory_size` here? - exec_step.gas_cost = GasCost(gas_cost); - exec_step.memory_size = (memory_word_size * 32) as usize; - println!("bus-mapping - 2 - {exec_step:?}"); Ok(()) @@ -130,27 +93,6 @@ impl Opcode for Calldatacopy { } } -fn calc_memory_expension_gas_cost(curr_memory_word_size: u64, next_memory_word_size: u64) -> u64 { - if next_memory_word_size <= curr_memory_word_size { - 0 - } else { - let total_cost = |mem_word_size| { - mem_word_size * GasCost::MEMORY.as_u64() + mem_word_size * mem_word_size / 512 - }; - total_cost(next_memory_word_size) - total_cost(curr_memory_word_size) - } -} - -fn calc_memory_copier_gas_cost( - curr_memory_word_size: u64, - next_memory_word_size: u64, - num_copy_bytes: u64, -) -> u64 { - let num_words = (num_copy_bytes + 31) / 32; - num_words * GasCost::COPY.as_u64() - + calc_memory_expension_gas_cost(curr_memory_word_size, next_memory_word_size) -} - fn gen_memory_copy_step( state: &mut CircuitInputStateRef, last_step: &ExecStep, @@ -166,8 +108,6 @@ fn gen_memory_copy_step( step.rwc = RWCounter(step.rwc.0 + step.bus_mapping_instance.len()); step.bus_mapping_instance = Vec::new(); step.exec_state = ExecState::CopyToMemory; - step.gas_left = Gas(step.gas_left.0 - step.gas_cost.0); - step.gas_cost = GasCost(0); step.pc = ProgramCounter(step.pc.0 + 1); step.stack_size = 0; step.memory_size = memory_size; diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 1cbcc94d3c..eaa88c3ff0 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -253,8 +253,7 @@ mod test { STOP }; let test_config = BytecodeTestConfig { - is_root_call: true, - call_data_length, + call_data: Some(rand_bytes(call_data_length)), ..Default::default() }; assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); @@ -266,90 +265,6 @@ mod test { memory_offset: Word, data_offset: Word, length: Word, - ) { - let bytecode = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] - CALLDATACOPY - STOP - }; - let test_config = BytecodeTestConfig { - is_root_call: false, - call_data_length: call_data_length.as_usize(), - call_data_offset: call_data_offset.as_u64(), - ..Default::default() - }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); - } - - #[test] - fn calldatacopy_gadget_simple() { - /* - test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); - test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(10)); - */ - - - test_ok_internal_old( - Word::from(0x40), - Word::from(64), - Word::from(0xA0), - Word::from(16), - Word::from(10), - ); - test_ok_internal( - Word::from(0x40), - Word::from(64), - Word::from(0xA0), - Word::from(16), - Word::from(10), - ); - - // assert!(false); - } - - #[test] - fn calldatacopy_gadget_multi_step() { - test_ok_internal( - Word::from(0x40), - Word::from(128), - Word::from(0x100), - Word::from(16), - Word::from(90), - ); - } - - #[test] - fn calldatacopy_gadget_out_of_bound() { - test_ok_root(64, Word::from(0x40), Word::from(32), Word::from(40)); - test_ok_internal( - Word::from(0x40), - Word::from(32), - Word::from(0xA0), - Word::from(40), - Word::from(10), - ); - } - - #[test] - fn calldatacopy_gadget_zero_length() { - test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(0)); - test_ok_internal( - Word::from(0x40), - Word::from(64), - Word::from(0xA0), - Word::from(16), - Word::from(0), - ); - } - - fn test_ok_root_old( - call_data_length: usize, - memory_offset: Word, - data_offset: Word, - length: Word, ) { let randomness = Fp::rand(); let bytecode = Bytecode::new( @@ -365,7 +280,7 @@ mod test { .concat(), ); let call_id = 1; - let call_data: Vec = rand_bytes(call_data_length); + let call_data = rand_bytes(call_data_length.as_usize()); let mut rws = RwMap( [ @@ -397,33 +312,59 @@ mod test { ), ( RwTableTag::CallContext, - vec![Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }], + vec![ + Rw::CallContext { + rw_counter: 4, + is_write: false, + call_id, + field_tag: CallContextFieldTag::TxId, + value: Word::one(), + }, + Rw::CallContext { + rw_counter: 5, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataLength, + value: call_data_length, + }, + Rw::CallContext { + rw_counter: 6, + is_write: false, + call_id, + field_tag: CallContextFieldTag::CallDataOffset, + value: call_data_offset, + }, + ], ), ] .into(), ); - let mut rw_counter = 5; + let mut rw_counter = 7; + let curr_memory_word_size = + (call_data_length.as_u64() + call_data_length.as_u64() + 31) / 32; let next_memory_word_size = if length.is_zero() { - 0 + curr_memory_word_size } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 + std::cmp::max( + curr_memory_word_size, + (memory_offset.as_u64() + length.as_u64() + 31) / 32, + ) }; let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - + + calc_memory_copier_gas_cost( + curr_memory_word_size, + next_memory_word_size, + length.as_u64(), + ); let mut steps = vec![ExecStep { rw_indices: vec![ (RwTableTag::Stack, 0), (RwTableTag::Stack, 1), (RwTableTag::Stack, 2), (RwTableTag::CallContext, 0), + (RwTableTag::CallContext, 1), + (RwTableTag::CallContext, 2), ], execution_state: ExecutionState::CALLDATACOPY, rw_counter: 1, @@ -431,7 +372,7 @@ mod test { stack_pointer: 1021, gas_left: gas_cost, gas_cost, - memory_size: 0, + memory_size: curr_memory_word_size * 32, opcode: Some(OpcodeId::CALLDATACOPY), ..Default::default() }]; @@ -440,11 +381,11 @@ mod test { make_memory_copy_steps( call_id, &call_data, - 0, - data_offset.as_u64(), + call_data_offset.as_u64(), + call_data_offset.as_u64() + data_offset.as_u64(), memory_offset.as_u64(), length.as_usize(), - true, + false, 100, 1024, next_memory_word_size * 32, @@ -468,12 +409,12 @@ mod test { randomness, txs: vec![Transaction { id: 1, - call_data, - call_data_length, calls: vec![Call { id: call_id, - is_root: true, + is_root: false, is_create: false, + call_data_length: call_data_length.as_u64(), + call_data_offset: call_data_offset.as_u64(), code_source: CodeSource::Account(bytecode.hash), ..Default::default() }], @@ -484,14 +425,62 @@ mod test { bytecodes: vec![bytecode], ..Default::default() }; - - println!("zkevm - 2 - {block:?}"); assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); } - fn test_ok_internal_old( - call_data_offset: Word, - call_data_length: Word, + #[test] + fn calldatacopy_gadget_simple() { + /* gupeng + test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); + test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(10)); + */ + test_ok_internal( + Word::from(0x40), + Word::from(64), + Word::from(0xA0), + Word::from(16), + Word::from(10), + ); + } + + #[test] + fn calldatacopy_gadget_multi_step() { + test_ok_internal( + Word::from(0x40), + Word::from(128), + Word::from(0x100), + Word::from(16), + Word::from(90), + ); + } + + #[test] + fn calldatacopy_gadget_out_of_bound() { + test_ok_root(64, Word::from(0x40), Word::from(32), Word::from(40)); + test_ok_internal( + Word::from(0x40), + Word::from(32), + Word::from(0xA0), + Word::from(40), + Word::from(10), + ); + } + + #[test] + fn calldatacopy_gadget_zero_length() { + test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(0)); + test_ok_internal( + Word::from(0x40), + Word::from(64), + Word::from(0xA0), + Word::from(16), + Word::from(0), + ); + } + + // Need to remove, just for test + fn test_ok_root_old( + call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word, @@ -510,7 +499,7 @@ mod test { .concat(), ); let call_id = 1; - let call_data = rand_bytes(call_data_length.as_usize()); + let call_data: Vec = rand_bytes(call_data_length); let mut rws = RwMap( [ @@ -542,59 +531,33 @@ mod test { ), ( RwTableTag::CallContext, - vec![ - Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }, - Rw::CallContext { - rw_counter: 5, - is_write: false, - call_id, - field_tag: CallContextFieldTag::CallDataLength, - value: call_data_length, - }, - Rw::CallContext { - rw_counter: 6, - is_write: false, - call_id, - field_tag: CallContextFieldTag::CallDataOffset, - value: call_data_offset, - }, - ], + vec![Rw::CallContext { + rw_counter: 4, + is_write: false, + call_id, + field_tag: CallContextFieldTag::TxId, + value: Word::one(), + }], ), ] .into(), ); - let mut rw_counter = 7; + let mut rw_counter = 5; - let curr_memory_word_size = - (call_data_length.as_u64() + call_data_length.as_u64() + 31) / 32; let next_memory_word_size = if length.is_zero() { - curr_memory_word_size + 0 } else { - std::cmp::max( - curr_memory_word_size, - (memory_offset.as_u64() + length.as_u64() + 31) / 32, - ) + (memory_offset.as_u64() + length.as_u64() + 31) / 32 }; let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost( - curr_memory_word_size, - next_memory_word_size, - length.as_u64(), - ); + + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); + let mut steps = vec![ExecStep { rw_indices: vec![ (RwTableTag::Stack, 0), (RwTableTag::Stack, 1), (RwTableTag::Stack, 2), (RwTableTag::CallContext, 0), - (RwTableTag::CallContext, 1), - (RwTableTag::CallContext, 2), ], execution_state: ExecutionState::CALLDATACOPY, rw_counter: 1, @@ -602,7 +565,7 @@ mod test { stack_pointer: 1021, gas_left: gas_cost, gas_cost, - memory_size: curr_memory_word_size * 32, + memory_size: 0, opcode: Some(OpcodeId::CALLDATACOPY), ..Default::default() }]; @@ -611,11 +574,11 @@ mod test { make_memory_copy_steps( call_id, &call_data, - call_data_offset.as_u64(), - call_data_offset.as_u64() + data_offset.as_u64(), + 0, + data_offset.as_u64(), memory_offset.as_u64(), length.as_usize(), - false, + true, 100, 1024, next_memory_word_size * 32, @@ -639,12 +602,12 @@ mod test { randomness, txs: vec![Transaction { id: 1, + call_data, + call_data_length, calls: vec![Call { id: call_id, - is_root: false, + is_root: true, is_create: false, - call_data_length: call_data_length.as_u64(), - call_data_offset: call_data_offset.as_u64(), code_source: CodeSource::Account(bytecode.hash), ..Default::default() }], @@ -656,7 +619,7 @@ mod test { ..Default::default() }; - println!("zkevm - internal- 2 - {block:?}"); + println!("zkevm - 2 - {block:?}"); assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); } } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 34b27b94a8..8538209b3a 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -31,33 +31,19 @@ pub fn get_fixed_table(conf: FixedTableConfig) -> Vec { pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, pub enable_state_circuit_test: bool, - pub is_root_call: bool, - pub call_data_length: usize, - pub call_data_offset: u64, pub gas_limit: u64, pub evm_circuit_lookup_tags: Vec, + pub call_data: Option>, } impl Default for BytecodeTestConfig { fn default() -> Self { Self { - is_root_call: true, enable_evm_circuit_test: true, enable_state_circuit_test: true, - call_data_length: 0, - call_data_offset: 0, gas_limit: 1_000_000u64, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), - } - } -} - -impl From<&BytecodeTestConfig> for bus_mapping::circuit_input_builder::TransactionConfig { - fn from(config: &BytecodeTestConfig) -> Self { - Self { - is_root_call: config.is_root_call, - call_data_length: config.call_data_length, - call_data_offset: config.call_data_offset, + call_data: None, } } } From 4cf808f383b38d622e99f87b5cf4736cadabd28c Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 11 Mar 2022 10:15:07 +0800 Subject: [PATCH 14/30] Update test cases which call `handle_tx` and `new_tx` of circuit input builder. --- bus-mapping/src/evm/opcodes/number.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 8f7192da51..178d87b91e 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -24,12 +24,12 @@ mod number_tests { let mut builder = block.new_circuit_input_builder(); builder - .handle_tx(&block.eth_tx, &block.geth_trace, Default::default()) + .handle_tx(&block.eth_tx, &block.geth_trace, None) .unwrap(); let mut test_builder = block.new_circuit_input_builder(); let mut tx = test_builder - .new_tx(&block.eth_tx, Default::default(), !block.geth_trace.failed) + .new_tx(&block.eth_tx, !block.geth_trace.failed, None) .unwrap(); let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); From db85545088bb33faf08840921e4ad900990ea20f Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sun, 13 Mar 2022 21:59:55 +0800 Subject: [PATCH 15/30] Update constant max address of state circuit. --- zkevm-circuits/src/test_util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 8538209b3a..28796f2148 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -91,7 +91,7 @@ pub fn test_circuits_using_witness_block( // circuit must be same if config.enable_state_circuit_test { let state_circuit = - StateCircuit::::new(block.randomness, &block.rws); + StateCircuit::::new(block.randomness, &block.rws); let prover = MockProver::::run(12, &state_circuit, vec![]).unwrap(); prover.verify()?; } From 20fa747eccc93c9f23652a1a4199c328f7bbb888 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Mon, 14 Mar 2022 10:28:30 +0800 Subject: [PATCH 16/30] Add unit test for calldatacopy bus-mapping. --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 146 +++++++++++++++++--- 1 file changed, 128 insertions(+), 18 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 563ca77467..3a4efe5fbe 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -2,7 +2,7 @@ use super::Opcode; use crate::circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}; use crate::operation::{CallContextField, CallContextOp, RWCounter, RW}; use crate::Error; -use eth_types::evm_types::ProgramCounter; +use eth_types::evm_types::{Gas, GasCost, ProgramCounter}; use eth_types::GethExecStep; use std::collections::HashMap; @@ -68,8 +68,6 @@ impl Opcode for Calldatacopy { ); }; - println!("bus-mapping - 2 - {exec_step:?}"); - Ok(()) } @@ -104,13 +102,20 @@ fn gen_memory_copy_step( from_tx: bool, bytes_map: &HashMap, ) -> ExecStep { - let mut step = last_step.clone(); - step.rwc = RWCounter(step.rwc.0 + step.bus_mapping_instance.len()); - step.bus_mapping_instance = Vec::new(); - step.exec_state = ExecState::CopyToMemory; - step.pc = ProgramCounter(step.pc.0 + 1); - step.stack_size = 0; - step.memory_size = memory_size; + let mut step = ExecStep { + exec_state: ExecState::CopyToMemory, + pc: ProgramCounter(last_step.pc.0 + 1), + stack_size: 0, + memory_size, + gas_left: Gas(last_step.gas_left.0 - last_step.gas_cost.0), + gas_cost: GasCost(0), + call_index: last_step.call_index, + rwc: RWCounter(last_step.rwc.0 + last_step.bus_mapping_instance.len()), + swc: last_step.swc, + bus_mapping_instance: vec![], + error: None, + aux_data: None, + }; let mut selectors = vec![0u8; MAX_COPY_BYTES]; for (idx, selector) in selectors.iter_mut().enumerate() { @@ -134,6 +139,7 @@ fn gen_memory_copy_step( state.push_memory_op(&mut step, RW::WRITE, (idx + dst_addr as usize).into(), byte); } } + step.aux_data = Some(StepAuxiliaryData::CopyToMemory { src_addr, dst_addr, @@ -142,6 +148,7 @@ fn gen_memory_copy_step( from_tx, selectors, }); + step } @@ -155,26 +162,25 @@ fn gen_memory_copy_steps( let length = next_steps[0].stack.nth_last(2)?.as_usize(); let is_root = state.call().is_root; + let call_data_length = state.call().call_data_length; let (src_addr, buffer_addr, buffer_addr_end) = if is_root { - (data_offset, 0, 0 + state.tx.input.len() as u64) + (data_offset, 0, call_data_length) } else { let call_data_offset = state.call().call_data_offset; - ( call_data_offset + data_offset, call_data_offset, - call_data_offset + state.tx.input.len() as u64, + call_data_offset + call_data_length, ) }; - let buffer: Vec = vec![0; (buffer_addr_end - buffer_addr) as usize]; - let memory_size = if length == 0 { 0 } else { (memory_offset + length as u64 + 31) / 32 * 32 }; + let buffer: Vec = vec![0; (buffer_addr_end - buffer_addr) as usize]; let bytes_map = (buffer_addr..buffer_addr_end) .zip(buffer.iter().copied()) .collect(); @@ -182,9 +188,8 @@ fn gen_memory_copy_steps( let mut copied = 0; let mut steps = vec![]; let mut last_step = call_data_copy_step; - while copied < length { - steps.push(gen_memory_copy_step( + let step = gen_memory_copy_step( state, last_step, src_addr + copied as u64, @@ -194,10 +199,115 @@ fn gen_memory_copy_steps( memory_size as usize, is_root, &bytes_map, - )); + ); + steps.push(step); last_step = steps.last().unwrap(); copied += MAX_COPY_BYTES; } Ok(steps) } + +#[cfg(test)] +mod calldatacopy_tests { + use crate::{ + circuit_input_builder::{ExecStep, TransactionContext}, + mock::BlockData, + operation::{CallContextField, CallContextOp, RW}, + Error, + }; + use eth_types::evm_types::StackAddress; + use eth_types::{bytecode, Word}; + use mock::new_single_tx_trace_code_at_start; + use pretty_assertions::assert_eq; + + #[test] + fn calldatacopy_opcode_impl() -> Result<(), Error> { + // Set length to zero for only testing CallDataCopy (without CopyToMemory + // steps). + let length = Word::from(0); + let data_offset = Word::from(0); + let memory_offset = Word::from(0x40); + + let code = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP + }; + + // Get the execution steps from the external tracer + let block = + BlockData::new_from_geth_data(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, None) + .unwrap(); + + let mut test_builder = block.new_circuit_input_builder(); + let mut tx = test_builder + .new_tx(&block.eth_tx, !block.geth_trace.failed, None) + .unwrap(); + let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); + + // Generate step corresponding to CALLDATACOPY + let mut step = ExecStep::new( + &block.geth_trace.struct_logs[0], + 0, + test_builder.block_ctx.rwc, + 0, + ); + let mut state = test_builder.state_ref(&mut tx, &mut tx_ctx); + + state.push_stack_op(&mut step, RW::READ, StackAddress::from(1021), memory_offset); + state.push_stack_op(&mut step, RW::READ, StackAddress::from(1022), data_offset); + state.push_stack_op(&mut step, RW::READ, StackAddress::from(1023), length); + state.push_op( + &mut step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::TxId, + value: state.tx_ctx.id().into(), + }, + ); + + if !state.call().is_root { + state.push_op( + &mut step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataLength, + value: state.call().call_data_length.into(), + }, + ); + state.push_op( + &mut step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataOffset, + value: state.call().call_data_offset.into(), + }, + ); + }; + + 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(()) + } +} From 3c13cbec9fba116bef99d5661417174fe4810cf0 Mon Sep 17 00:00:00 2001 From: Haichen Shen Date: Mon, 14 Mar 2022 17:10:18 -0700 Subject: [PATCH 17/30] change api --- bus-mapping/src/circuit_input_builder.rs | 58 +++----- bus-mapping/src/evm/opcodes.rs | 36 +++-- bus-mapping/src/evm/opcodes/calldatacopy.rs | 136 +++++++++--------- bus-mapping/src/evm/opcodes/calldatasize.rs | 18 +-- bus-mapping/src/evm/opcodes/caller.rs | 18 +-- bus-mapping/src/evm/opcodes/callvalue.rs | 18 +-- bus-mapping/src/evm/opcodes/dup.rs | 20 +-- bus-mapping/src/evm/opcodes/mload.rs | 22 +-- bus-mapping/src/evm/opcodes/mstore.rs | 26 ++-- bus-mapping/src/evm/opcodes/number.rs | 4 +- bus-mapping/src/evm/opcodes/selfbalance.rs | 20 +-- bus-mapping/src/evm/opcodes/sload.rs | 22 +-- bus-mapping/src/evm/opcodes/stackonlyop.rs | 17 ++- bus-mapping/src/evm/opcodes/stop.rs | 5 +- bus-mapping/src/evm/opcodes/swap.rs | 26 ++-- mock/src/lib.rs | 14 +- .../src/evm_circuit/execution/bitwise.rs | 2 +- .../src/evm_circuit/execution/calldatacopy.rs | 24 ++-- .../src/evm_circuit/execution/gas.rs | 2 +- .../src/evm_circuit/execution/memory.rs | 2 +- .../src/evm_circuit/execution/memory_copy.rs | 16 +-- zkevm-circuits/src/test_util.rs | 8 +- 22 files changed, 242 insertions(+), 272 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 5bb4016a01..0d2f2f9258 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -577,8 +577,10 @@ pub struct Transaction { /// Value pub value: Word, /// Input / Call Data - pub input: Vec, // call_data + pub input: Vec, + /// Calls made in the transaction calls: Vec, + /// Execution steps steps: Vec, } @@ -590,7 +592,6 @@ impl Transaction { code_db: &mut CodeDB, eth_tx: ð_types::Transaction, is_success: bool, - call_data: Option<&[u8]>, ) -> Result { let (found, _) = sdb.get_account(ð_tx.from); if !found { @@ -638,11 +639,6 @@ impl Transaction { } }; - let input = match call_data { - Some(data) => data.to_vec(), - None => eth_tx.input.to_vec(), - }; - Ok(Self { nonce: eth_tx.nonce.as_u64(), gas: eth_tx.gas.as_u64(), @@ -650,7 +646,7 @@ impl Transaction { from: eth_tx.from, to: eth_tx.to.unwrap_or_default(), value: eth_tx.value, - input, + input: eth_tx.input.to_vec(), calls: vec![call], steps: Vec::new(), }) @@ -681,27 +677,6 @@ impl Transaction { } } -#[derive(Debug)] -/// Transaction configuration -pub struct TransactionConfig { - /// If root call - pub is_root_call: bool, - /// Initialized length of call data - pub call_data_length: usize, - /// Initialized offset of call data - pub call_data_offset: u64, -} - -impl Default for TransactionConfig { - fn default() -> Self { - Self { - is_root_call: true, - call_data_length: 0, - call_data_offset: 0, - } - } -} - /// Reference to the internal state of the CircuitInputBuilder in a particular /// [`ExecStep`]. pub struct CircuitInputStateRef<'a> { @@ -717,6 +692,8 @@ pub struct CircuitInputStateRef<'a> { pub tx: &'a mut Transaction, /// Transaction Context pub tx_ctx: &'a mut TransactionContext, + // /// Steps + //pub steps: Vec, } impl<'a> CircuitInputStateRef<'a> { @@ -730,11 +707,6 @@ impl<'a> CircuitInputStateRef<'a> { ) } - /// - pub fn push_step_to_tx(&mut self, step: ExecStep) { - self.tx.steps.push(step); - } - /// Push an [`Operation`] into the [`OperationContainer`] with the next /// [`RWCounter`] and then adds a reference to the stored operation /// ([`OperationRef`]) inside the bus-mapping instance of the current @@ -827,6 +799,14 @@ impl<'a> CircuitInputStateRef<'a> { .map(|call_idx| &self.tx.calls[call_idx]) } + // pub fn caller(&self) -> Option<&Call> { + // if self.tx.calls.len() == 1 { + // None + // } else { + // Some(&self.tx.calls[self.tx_ctx.call_index()]) + // } + // } + /// Mutable reference to the current Call pub fn call_mut(&mut self) -> Result<&mut Call, Error> { self.tx_ctx @@ -1355,7 +1335,6 @@ impl<'a> CircuitInputBuilder { &mut self, eth_tx: ð_types::Transaction, is_success: bool, - call_data: Option<&[u8]>, ) -> Result { let call_id = self.block_ctx.rwc.0; @@ -1376,7 +1355,6 @@ impl<'a> CircuitInputBuilder { &mut self.code_db, eth_tx, is_success, - call_data, ) } @@ -1443,12 +1421,12 @@ impl<'a> CircuitInputBuilder { for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() { let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx); - - gen_associated_ops( + let exec_steps = gen_associated_ops( &geth_step.op, &mut state_ref, &geth_trace.struct_logs[index..], )?; + tx.steps.extend(exec_steps); } // TODO: Move into gen_associated_steps with @@ -2055,7 +2033,7 @@ mod tracer_tests { STOP }; let block = - mock::new_single_tx_trace_code_gas(&code, Gas(1_000_000_000_000_000u64)).unwrap(); + mock::new_single_tx_trace_code_gas(&code, Gas(1_000_000_000_000_000u64), None).unwrap(); let struct_logs = &block.geth_traces[0].struct_logs; // get last CALL @@ -2915,7 +2893,7 @@ mod tracer_tests { PUSH1(0x1) PUSH1(0x2) }; - let block = mock::new_single_tx_trace_code_gas(&code, Gas(21004)).unwrap(); + let block = mock::new_single_tx_trace_code_gas(&code, Gas(21004), None).unwrap(); let struct_logs = &block.geth_traces[0].struct_logs; assert_eq!(struct_logs[1].error, Some(GETH_ERR_OUT_OF_GAS.to_string())); diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 420fb5c4e7..15259adcac 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -52,31 +52,29 @@ pub trait Opcode: Debug { /// is implemented for. fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - next_steps: &[GethExecStep], - ) -> Result<(), Error>; + geth_steps: &[GethExecStep], + ) -> Result, Error>; - /// - fn gen_associated_ops_multi( - state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], - ) -> Result<(), Error> { - let mut step = state.new_step(&next_steps[0]); - Self::gen_associated_ops(state, &mut step, next_steps)?; - state.push_step_to_tx(step); - Ok(()) - } + // fn gen_associated_ops( + // state: &mut CircuitInputStateRef, + // next_steps: &[GethExecStep], + // ) -> Result<(), Error> { + // let mut step = state.new_step(&next_steps[0]); + // Self::gen_associated_ops(state, &mut step, next_steps)?; + // state.push_step_to_tx(step); + // Ok(()) + // } } fn dummy_gen_associated_ops( _state: &mut CircuitInputStateRef, _next_steps: &[GethExecStep], -) -> Result<(), Error> { - Ok(()) +) -> Result, Error> { + Ok(vec![]) } type FnGenAssociatedOps = - fn(state: &mut CircuitInputStateRef, next_steps: &[GethExecStep]) -> Result<(), Error>; + fn(state: &mut CircuitInputStateRef, next_steps: &[GethExecStep]) -> Result, Error>; fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { match opcode_id { @@ -114,7 +112,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { OpcodeId::CALLVALUE => Callvalue::gen_associated_ops, OpcodeId::CALLDATASIZE => Calldatasize::gen_associated_ops, OpcodeId::CALLDATALOAD => StackOnlyOpcode::<1, 1>::gen_associated_ops, - OpcodeId::CALLDATACOPY => Calldatacopy::gen_associated_ops_multi, + OpcodeId::CALLDATACOPY => Calldatacopy::gen_associated_ops, // OpcodeId::CODESIZE => {}, // OpcodeId::CODECOPY => {}, // OpcodeId::GASPRICE => {}, @@ -130,7 +128,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { // OpcodeId::DIFFICULTY => {}, // OpcodeId::GASLIMIT => {}, // OpcodeId::CHAINID => {}, - OpcodeId::SELFBALANCE => Selfbalance::gen_associated_ops_multi, + OpcodeId::SELFBALANCE => Selfbalance::gen_associated_ops, // OpcodeId::BASEFEE => {}, OpcodeId::POP => StackOnlyOpcode::<1, 0>::gen_associated_ops, OpcodeId::MLOAD => Mload::gen_associated_ops, @@ -239,7 +237,7 @@ pub fn gen_associated_ops( opcode_id: &OpcodeId, state: &mut CircuitInputStateRef, next_steps: &[GethExecStep], -) -> Result<(), Error> { +) -> Result, Error> { let fn_gen_associated_ops = fn_gen_associated_ops(opcode_id); fn_gen_associated_ops(state, next_steps) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 3a4efe5fbe..0bc1a99a14 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -16,79 +16,71 @@ pub(crate) struct Calldatacopy; impl Opcode for Calldatacopy { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - let memory_offset = step.stack.nth_last(0)?; - let data_offset = step.stack.nth_last(1)?; - let length = step.stack.nth_last(2)?; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_steps = Vec::new(); + let calldatacopy_step = gen_calldatacopy_step(state, geth_step)?; + let memory_copy_steps = gen_memory_copy_steps(state, &calldatacopy_step, geth_steps)?; + exec_steps.push(calldatacopy_step); + exec_steps.extend(memory_copy_steps); + Ok(exec_steps) + } +} - state.push_stack_op( - exec_step, - RW::READ, - step.stack.nth_last_filled(0), - memory_offset, - ); - state.push_stack_op( - exec_step, +fn gen_calldatacopy_step( + state: &mut CircuitInputStateRef, + geth_step: &GethExecStep, +) -> Result { + let mut exec_step = state.new_step(geth_step); + let memory_offset = geth_step.stack.nth_last(0)?; + let data_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; + + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(0), + memory_offset, + ); + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(1), + data_offset, + ); + state.push_stack_op(&mut exec_step, RW::READ, geth_step.stack.nth_last_filled(2), length); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::TxId, + value: state.tx_ctx.id().into(), + }, + ); + + if !state.call().is_root { + state.push_op( + &mut exec_step, RW::READ, - step.stack.nth_last_filled(1), - data_offset, + CallContextOp { + call_id: state.call().call_id, + field: CallContextField::CallDataLength, + value: state.call().call_data_length.into(), + }, ); - state.push_stack_op(exec_step, RW::READ, step.stack.nth_last_filled(2), length); state.push_op( - exec_step, + &mut exec_step, RW::READ, CallContextOp { call_id: state.call().call_id, - field: CallContextField::TxId, - value: state.tx_ctx.id().into(), + field: CallContextField::CallDataOffset, + value: state.call().call_data_offset.into(), }, ); - - if !state.call().is_root { - state.push_op( - exec_step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataLength, - value: state.call().call_data_length.into(), - }, - ); - state.push_op( - exec_step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataOffset, - value: state.call().call_data_offset.into(), - }, - ); - }; - - Ok(()) - } - - fn gen_associated_ops_multi( - state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], - ) -> Result<(), Error> { - // Generate an ExecStep of state CALLDATACOPY. - let mut call_data_copy_step = state.new_step(&next_steps[0]); - Self::gen_associated_ops(state, &mut call_data_copy_step, next_steps)?; - - // Generate ExecSteps of virtual state CopyToMemory. - let copy_to_memory_steps = gen_memory_copy_steps(state, &call_data_copy_step, next_steps)?; - - state.push_step_to_tx(call_data_copy_step); - for s in copy_to_memory_steps { - state.push_step_to_tx(s); - } - - Ok(()) - } + }; + Ok(exec_step) } fn gen_memory_copy_step( @@ -107,7 +99,7 @@ fn gen_memory_copy_step( pc: ProgramCounter(last_step.pc.0 + 1), stack_size: 0, memory_size, - gas_left: Gas(last_step.gas_left.0 - last_step.gas_cost.0), + gas_left: last_step.gas_left, gas_cost: GasCost(0), call_index: last_step.call_index, rwc: RWCounter(last_step.rwc.0 + last_step.bus_mapping_instance.len()), @@ -128,7 +120,7 @@ fn gen_memory_copy_step( state.push_memory_op( &mut step, RW::READ, - (idx + src_addr as usize).into(), + (addr as usize).into(), bytes_map[&addr], ); } @@ -155,11 +147,11 @@ fn gen_memory_copy_step( fn gen_memory_copy_steps( state: &mut CircuitInputStateRef, call_data_copy_step: &ExecStep, - next_steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error> { - let memory_offset = next_steps[0].stack.nth_last(0)?.as_u64(); - let data_offset = next_steps[0].stack.nth_last(1)?.as_u64(); - let length = next_steps[0].stack.nth_last(2)?.as_usize(); + let memory_offset = geth_steps[0].stack.nth_last(0)?.as_u64(); + let data_offset = geth_steps[0].stack.nth_last(1)?.as_u64(); + let length = geth_steps[0].stack.nth_last(2)?.as_usize(); let is_root = state.call().is_root; let call_data_length = state.call().call_data_length; @@ -244,12 +236,12 @@ mod calldatacopy_tests { let mut builder = block.new_circuit_input_builder(); builder - .handle_tx(&block.eth_tx, &block.geth_trace, None) + .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, !block.geth_trace.failed, None) + .new_tx(&block.eth_tx, !block.geth_trace.failed) .unwrap(); let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index ce79ac7f40..c1e47dd8e1 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -14,13 +14,13 @@ pub(crate) struct Calldatasize; impl Opcode for Calldatasize { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - let value = steps[1].stack.last()?; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); + let value = geth_steps[1].stack.last()?; state.push_op( - exec_step, + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -29,12 +29,12 @@ impl Opcode for Calldatasize { }, ); state.push_stack_op( - exec_step, + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), value, )?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index 9eadd6eab7..a27a0f662c 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -12,15 +12,15 @@ pub(crate) struct Caller; impl Opcode for Caller { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // Get caller_address result from next step - let value = steps[1].stack.last()?; + let value = geth_steps[1].stack.last()?; // CallContext read of the caller_address state.push_op( - exec_step, + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -30,13 +30,13 @@ impl Opcode for Caller { ); // Stack write of the caller_address state.push_stack_op( - exec_step, + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), value, )?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index 1bc2a59a56..5c1eb8ab96 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -12,15 +12,15 @@ pub(crate) struct Callvalue; impl Opcode for Callvalue { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // Get call_value result from next step - let value = steps[1].stack.last()?; + let value = geth_steps[1].stack.last()?; // CallContext read of the call_value state.push_op( - exec_step, + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -30,13 +30,13 @@ impl Opcode for Callvalue { ); // Stack write of the call_value state.push_stack_op( - exec_step, + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), value, )?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index b6ce4956ab..b02f361fa9 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -11,23 +11,23 @@ pub(crate) struct Dup; impl Opcode for Dup { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); - let stack_value_read = step.stack.nth_last(N - 1)?; - let stack_position = step.stack.nth_last_filled(N - 1); - state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; + let stack_value_read = geth_step.stack.nth_last(N - 1)?; + let stack_position = geth_step.stack.nth_last_filled(N - 1); + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; state.push_stack_op( - exec_step, + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), stack_value_read, )?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index f0ac6cadfa..e82dce2bb0 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -16,24 +16,24 @@ pub(crate) struct Mload; impl Opcode for Mload { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // // First stack read // - let stack_value_read = step.stack.last()?; - let stack_position = step.stack.last_filled(); + let stack_value_read = geth_step.stack.last()?; + let stack_position = geth_step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; // Read the memory let mut mem_read_addr: MemoryAddress = stack_value_read.try_into()?; // Accesses to memory that hasn't been initialized are valid, and return // 0. - let mem_read_value = steps[1] + let mem_read_value = geth_steps[1] .memory .read_word(mem_read_addr) .unwrap_or_else(|_| Word::zero()); @@ -41,19 +41,19 @@ impl Opcode for Mload { // // First stack write // - state.push_stack_op(exec_step, RW::WRITE, stack_position, mem_read_value)?; + state.push_stack_op(&mut exec_step, RW::WRITE, stack_position, mem_read_value)?; // // First mem read -> 32 MemoryOp generated. // for byte in mem_read_value.to_be_bytes() { - state.push_memory_op(exec_step, RW::READ, mem_read_addr, byte)?; + state.push_memory_op(&mut exec_step, RW::READ, mem_read_addr, byte)?; // Update mem_read_addr to next byte's one mem_read_addr += MemoryAddress::from(1); } - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 5ea7bac855..405e977643 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -14,19 +14,19 @@ pub(crate) struct Mstore; impl Opcode for Mstore { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // First stack read (offset) - let offset = step.stack.nth_last(0)?; - let offset_pos = step.stack.nth_last_filled(0); - state.push_stack_op(exec_step, RW::READ, offset_pos, offset)?; + let offset = geth_step.stack.nth_last(0)?; + let offset_pos = geth_step.stack.nth_last_filled(0); + state.push_stack_op(&mut exec_step, RW::READ, offset_pos, offset)?; // Second stack read (value) - let value = step.stack.nth_last(1)?; - let value_pos = step.stack.nth_last_filled(1); - state.push_stack_op(exec_step, RW::READ, value_pos, value)?; + let value = geth_step.stack.nth_last(1)?; + let value_pos = geth_step.stack.nth_last_filled(1); + state.push_stack_op(&mut exec_step, RW::READ, value_pos, value)?; // First mem write -> 32 MemoryOp generated. let offset_addr: MemoryAddress = offset.try_into()?; @@ -35,7 +35,7 @@ impl Opcode for Mstore { true => { // stack write operation for mstore8 state.push_memory_op( - exec_step, + &mut exec_step, RW::WRITE, offset_addr, *value.to_le_bytes().first().unwrap(), @@ -45,12 +45,12 @@ impl Opcode for Mstore { // stack write each byte for mstore let bytes = value.to_be_bytes(); for (i, byte) in bytes.iter().enumerate() { - state.push_memory_op(exec_step, RW::WRITE, offset_addr.map(|a| a + i), *byte)?; + state.push_memory_op(&mut exec_step, RW::WRITE, offset_addr.map(|a| a + i), *byte)?; } } } - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 178d87b91e..8a8c3dbf27 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -24,12 +24,12 @@ mod number_tests { let mut builder = block.new_circuit_input_builder(); builder - .handle_tx(&block.eth_tx, &block.geth_trace, None) + .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, !block.geth_trace.failed, None) + .new_tx(&block.eth_tx, !block.geth_trace.failed) .unwrap(); let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index e73e9bb73e..86a99ee94c 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -10,16 +10,16 @@ pub(crate) struct Selfbalance; impl Opcode for Selfbalance { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - let self_balance = steps[1].stack.last()?; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); + let self_balance = geth_steps[1].stack.last()?; let callee_address = state.call()?.address; // CallContext read of the callee_address state.push_op( - exec_step, + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -30,7 +30,7 @@ impl Opcode for Selfbalance { // Account read for the balance of the callee_address state.push_op( - exec_step, + &mut exec_step, RW::READ, AccountOp { address: callee_address, @@ -42,13 +42,13 @@ impl Opcode for Selfbalance { // Stack write of self_balance state.push_stack_op( - exec_step, + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), self_balance, )?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 5158123bd6..766b1e253d 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -15,22 +15,22 @@ pub(crate) struct Sload; impl Opcode for Sload { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // First stack read - let stack_value_read = step.stack.last()?; - let stack_position = step.stack.last_filled(); + let stack_value_read = geth_step.stack.last()?; + let stack_position = geth_step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(exec_step, RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; // Storage read - let storage_value_read = step.storage.get_or_err(&stack_value_read)?; + let storage_value_read = geth_step.storage.get_or_err(&stack_value_read)?; state.push_op( - exec_step, + &mut exec_step, RW::READ, StorageOp::new( state.call()?.address, @@ -43,9 +43,9 @@ impl Opcode for Sload { ); // First stack write - state.push_stack_op(exec_step, RW::WRITE, stack_position, storage_value_read)?; + state.push_stack_op(&mut exec_step, RW::WRITE, stack_position, storage_value_read)?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index d81713b9bf..dd4cb9d091 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -15,15 +15,14 @@ pub(crate) struct StackOnlyOpcode; impl Opcode for StackOnlyOpcode { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - - // N_POP stack reads + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); + // N stack reads for i in 0..N_POP { state.push_stack_op( - exec_step, + &mut exec_step, RW::READ, step.stack.nth_last_filled(i), step.stack.nth_last(i)?, @@ -33,14 +32,14 @@ impl Opcode for StackOnlyOpcode Result<(), Error> { + ) -> Result, Error> { state.handle_return()?; - Ok(()) + Ok(vec![]) } } diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index 9ff60d379a..d003d14a9e 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -11,24 +11,24 @@ pub(crate) struct Swap; impl Opcode for Swap { fn gen_associated_ops( state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step); // Peek b and a - let stack_b_value_read = step.stack.nth_last(N)?; - let stack_b_position = step.stack.nth_last_filled(N); - state.push_stack_op(exec_step, RW::READ, stack_b_position, stack_b_value_read)?; - let stack_a_value_read = step.stack.last()?; - let stack_a_position = step.stack.last_filled(); - state.push_stack_op(exec_step, RW::READ, stack_a_position, stack_a_value_read)?; + let stack_b_value_read = geth_step.stack.nth_last(N)?; + let stack_b_position = geth_step.stack.nth_last_filled(N); + state.push_stack_op(&mut exec_step, RW::READ, stack_b_position, stack_b_value_read)?; + let stack_a_value_read = geth_step.stack.last()?; + let stack_a_position = geth_step.stack.last_filled(); + state.push_stack_op(&mut exec_step, RW::READ, stack_a_position, stack_a_value_read)?; // Write a into b_position, write b into a_position - state.push_stack_op(exec_step, RW::WRITE, stack_b_position, stack_a_value_read)?; - state.push_stack_op(exec_step, RW::WRITE, stack_a_position, stack_b_value_read)?; + state.push_stack_op(&mut exec_step, RW::WRITE, stack_b_position, stack_a_value_read)?; + state.push_stack_op(&mut exec_step, RW::WRITE, stack_a_position, stack_b_value_read)?; - Ok(()) + Ok(vec![exec_step]) } } diff --git a/mock/src/lib.rs b/mock/src/lib.rs index 701d362cdd..947a257036 100644 --- a/mock/src/lib.rs +++ b/mock/src/lib.rs @@ -75,7 +75,7 @@ pub fn new_single_tx_trace_accounts_gas( /// The trace will be generated automatically with the external_tracer /// from the accounts code. pub fn new_single_tx_trace_accounts(accounts: Vec) -> Result { - new_single_tx_trace_accounts_gas(accounts, Gas(1_000_000u64)) + new_single_tx_trace_accounts_gas(accounts, Gas(1_000_000u64), None) } /// Create a new block with a single tx that executes the code passed by @@ -89,9 +89,10 @@ pub fn new_single_tx_trace_code(code: &Bytecode) -> Result { /// Create a new block with a single tx with the given gas limit that /// executes the code passed by argument. The trace will be generated /// automatically with the external_tracer from the code. -pub fn new_single_tx_trace_code_gas(code: &Bytecode, gas: Gas) -> Result { +pub fn new_single_tx_trace_code_gas(code: &Bytecode, gas: Gas, input: Option>) +-> Result { let tracer_account = new_tracer_account(code); - new_single_tx_trace_accounts_gas(vec![tracer_account], gas) + new_single_tx_trace_accounts_gas(vec![tracer_account], gas, input) } /// Create a new block with a single tx that executes the code_a passed by @@ -133,7 +134,7 @@ pub fn new_block() -> Block { } /// Generate a new mock transaction with preloaded data, useful for tests. -pub fn new_tx(block: &Block) -> eth_types::Transaction { +pub fn new_tx(block: &Block, input: Option>) -> eth_types::Transaction { eth_types::Transaction { hash: Hash::zero(), nonce: Word::zero(), @@ -145,7 +146,10 @@ pub fn new_tx(block: &Block) -> eth_types::Transaction { value: Word::zero(), gas_price: Some(Word::zero()), gas: Word::from(1_000_000u64), - input: Bytes::default(), + input: match input { + Some(data) => Bytes::from(data), + None => Bytes::default(), + }, v: U64::zero(), r: Word::zero(), s: Word::zero(), diff --git a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs index fc62a757f1..0d5b55efda 100644 --- a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs +++ b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs @@ -128,7 +128,7 @@ mod test { evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Complete), ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); + assert_eq!(test_circuits_using_bytecode(bytecode, test_config, None), Ok(())); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index eaa88c3ff0..f764985638 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -252,11 +252,12 @@ mod test { CALLDATACOPY STOP }; + let call_data = Some(rand_bytes(call_data_length)); + println!("calldata = {:?}", call_data); let test_config = BytecodeTestConfig { - call_data: Some(rand_bytes(call_data_length)), ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); + assert_eq!(test_circuits_using_bytecode(bytecode, test_config, call_data), Ok(())); } fn test_ok_internal( @@ -430,17 +431,16 @@ mod test { #[test] fn calldatacopy_gadget_simple() { - /* gupeng - test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); + //test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(10)); - */ - test_ok_internal( - Word::from(0x40), - Word::from(64), - Word::from(0xA0), - Word::from(16), - Word::from(10), - ); + return + // test_ok_internal( + // Word::from(0x40), + // Word::from(64), + // Word::from(0xA0), + // Word::from(16), + // Word::from(10), + // ); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/gas.rs b/zkevm-circuits/src/evm_circuit/execution/gas.rs index 40be9b035d..9d4bde8454 100644 --- a/zkevm-circuits/src/evm_circuit/execution/gas.rs +++ b/zkevm-circuits/src/evm_circuit/execution/gas.rs @@ -115,7 +115,7 @@ mod test { }; let config = BytecodeTestConfig::default(); let block_trace = BlockData::new_from_geth_data( - new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit)) + new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit), None) .expect("could not build block trace"), ); let mut builder = block_trace.new_circuit_input_builder(); diff --git a/zkevm-circuits/src/evm_circuit/execution/memory.rs b/zkevm-circuits/src/evm_circuit/execution/memory.rs index 31c4050830..d5c1fff506 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory.rs @@ -217,7 +217,7 @@ mod test { enable_state_circuit_test: false, ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); + assert_eq!(test_circuits_using_bytecode(bytecode, test_config, None), Ok(())); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index 30362db32f..f4433f8d33 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -70,14 +70,14 @@ impl ExecutionGadget for CopyToMemoryGadget { ) }); // Read bytes[i] from Tx - cb.condition(from_tx.expr() * read_flag.clone(), |cb| { - cb.tx_context_lookup( - tx_id.expr(), - TxContextFieldTag::CallData, - Some(src_addr.expr() + i.expr()), - buffer_reader.byte(i), - ) - }); + // cb.condition(from_tx.expr() * read_flag.clone(), |cb| { + // cb.tx_context_lookup( + // tx_id.expr(), + // TxContextFieldTag::CallData, + // Some(src_addr.expr() + i.expr()), + // buffer_reader.byte(i), + // ) + // }); // Write bytes[i] to memory when selectors[i] != 0 cb.condition(buffer_reader.has_data(i), |cb| { cb.memory_lookup( diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 28796f2148..3d0c17389f 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -28,12 +28,12 @@ pub fn get_fixed_table(conf: FixedTableConfig) -> Vec { } } +#[derive(Debug, Clone)] pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, pub enable_state_circuit_test: bool, pub gas_limit: u64, pub evm_circuit_lookup_tags: Vec, - pub call_data: Option>, } impl Default for BytecodeTestConfig { @@ -43,22 +43,22 @@ impl Default for BytecodeTestConfig { enable_state_circuit_test: true, gas_limit: 1_000_000u64, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), - call_data: None, } } } pub fn run_test_circuits(bytecode: eth_types::Bytecode) -> Result<(), Vec> { - test_circuits_using_bytecode(bytecode, BytecodeTestConfig::default()) + test_circuits_using_bytecode(bytecode, BytecodeTestConfig::default(), None) } pub fn test_circuits_using_bytecode( bytecode: eth_types::Bytecode, config: BytecodeTestConfig, + call_data: Option>, ) -> Result<(), Vec> { // execute the bytecode and get trace let block_trace = bus_mapping::mock::BlockData::new_from_geth_data( - mock::new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit)).unwrap(), + mock::new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit), call_data).unwrap(), ); let mut builder = block_trace.new_circuit_input_builder(); builder From 3b0a8d37b65cb4479ec4e01b66e50d82d24a29b0 Mon Sep 17 00:00:00 2001 From: Haichen Shen Date: Tue, 15 Mar 2022 14:51:14 -0700 Subject: [PATCH 18/30] fix calldatacopy --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 99 ++++------- bus-mapping/src/evm/opcodes/stop.rs | 6 +- .../src/evm_circuit/execution/calldatacopy.rs | 162 +----------------- zkevm-circuits/src/test_util.rs | 2 - 4 files changed, 42 insertions(+), 227 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 0bc1a99a14..5bf79aa5f9 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -20,9 +20,8 @@ impl Opcode for Calldatacopy { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_steps = Vec::new(); - let calldatacopy_step = gen_calldatacopy_step(state, geth_step)?; - let memory_copy_steps = gen_memory_copy_steps(state, &calldatacopy_step, geth_steps)?; - exec_steps.push(calldatacopy_step); + exec_steps.push(gen_calldatacopy_step(state, geth_step)?); + let memory_copy_steps = gen_memory_copy_steps(state, geth_steps)?; exec_steps.extend(memory_copy_steps); Ok(exec_steps) } @@ -80,120 +79,84 @@ fn gen_calldatacopy_step( }, ); }; + println!("exec_step = {exec_step:?}"); Ok(exec_step) } fn gen_memory_copy_step( state: &mut CircuitInputStateRef, - last_step: &ExecStep, + exec_step: &mut ExecStep, src_addr: u64, dst_addr: u64, src_addr_end: u64, bytes_left: usize, - memory_size: usize, - from_tx: bool, - bytes_map: &HashMap, -) -> ExecStep { - let mut step = ExecStep { - exec_state: ExecState::CopyToMemory, - pc: ProgramCounter(last_step.pc.0 + 1), - stack_size: 0, - memory_size, - gas_left: last_step.gas_left, - gas_cost: GasCost(0), - call_index: last_step.call_index, - rwc: RWCounter(last_step.rwc.0 + last_step.bus_mapping_instance.len()), - swc: last_step.swc, - bus_mapping_instance: vec![], - error: None, - aux_data: None, - }; - +) { let mut selectors = vec![0u8; MAX_COPY_BYTES]; for (idx, selector) in selectors.iter_mut().enumerate() { if idx < bytes_left { *selector = 1; let addr = src_addr + idx as u64; let byte = if addr < src_addr_end { - debug_assert!(bytes_map.contains_key(&addr)); - if !from_tx { - state.push_memory_op( - &mut step, - RW::READ, - (addr as usize).into(), - bytes_map[&addr], - ); + if state.call().is_root { + state.tx.input[addr as usize] + } else { + // TODO, read caller memory + // state.push_memory_op( + // exec_step, + // RW::READ, + // (addr as usize).into(), + // bytes_map[&addr], + // ); + 0u8 } - bytes_map[&addr] } else { 0 }; - state.push_memory_op(&mut step, RW::WRITE, (idx + dst_addr as usize).into(), byte); + println!("calldata[{}] = {}", addr, byte); + state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte); } } - step.aux_data = Some(StepAuxiliaryData::CopyToMemory { + exec_step.aux_data = Some(StepAuxiliaryData::CopyToMemory { src_addr, dst_addr, bytes_left: bytes_left as u64, src_addr_end, - from_tx, + from_tx: state.call().is_root, selectors, }); - - step } fn gen_memory_copy_steps( state: &mut CircuitInputStateRef, - call_data_copy_step: &ExecStep, geth_steps: &[GethExecStep], ) -> Result, Error> { let memory_offset = geth_steps[0].stack.nth_last(0)?.as_u64(); let data_offset = geth_steps[0].stack.nth_last(1)?.as_u64(); let length = geth_steps[0].stack.nth_last(2)?.as_usize(); - let is_root = state.call().is_root; + let call_data_offset = state.call().call_data_offset; let call_data_length = state.call().call_data_length; - let (src_addr, buffer_addr, buffer_addr_end) = if is_root { - (data_offset, 0, call_data_length) - } else { - let call_data_offset = state.call().call_data_offset; - ( - call_data_offset + data_offset, - call_data_offset, - call_data_offset + call_data_length, - ) - }; - - let memory_size = if length == 0 { - 0 - } else { - (memory_offset + length as u64 + 31) / 32 * 32 - }; - - let buffer: Vec = vec![0; (buffer_addr_end - buffer_addr) as usize]; - let bytes_map = (buffer_addr..buffer_addr_end) - .zip(buffer.iter().copied()) - .collect(); + let (src_addr, buffer_addr_end) = ( + call_data_offset + data_offset, + call_data_offset + call_data_length, + ); let mut copied = 0; let mut steps = vec![]; - let mut last_step = call_data_copy_step; while copied < length { - let step = gen_memory_copy_step( + let mut exec_step = state.new_step(&geth_steps[1]); + exec_step.exec_state = ExecState::CopyToMemory; + gen_memory_copy_step( state, - last_step, + &mut exec_step, src_addr + copied as u64, memory_offset + copied as u64, buffer_addr_end, length - copied, - memory_size as usize, - is_root, - &bytes_map, ); - steps.push(step); - last_step = steps.last().unwrap(); + println!("exec_step = {:?}", exec_step); + steps.push(exec_step); copied += MAX_COPY_BYTES; } diff --git a/bus-mapping/src/evm/opcodes/stop.rs b/bus-mapping/src/evm/opcodes/stop.rs index 55ce40fd9a..f4e0f9f466 100644 --- a/bus-mapping/src/evm/opcodes/stop.rs +++ b/bus-mapping/src/evm/opcodes/stop.rs @@ -15,10 +15,10 @@ pub(crate) struct Stop; impl Opcode for Stop { fn gen_associated_ops( state: &mut CircuitInputStateRef, - _steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error> { + let exec_step = state.new_step(&geth_steps[0]); state.handle_return()?; - - Ok(vec![]) + Ok(vec![exec_step]) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index f764985638..00a4c960f6 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -431,20 +431,19 @@ mod test { #[test] fn calldatacopy_gadget_simple() { - //test_ok_root_old(64, Word::from(0x40), Word::from(0), Word::from(10)); test_ok_root(64, Word::from(0x40), Word::from(0), Word::from(10)); - return - // test_ok_internal( - // Word::from(0x40), - // Word::from(64), - // Word::from(0xA0), - // Word::from(16), - // Word::from(10), - // ); + test_ok_internal( + Word::from(0x40), + Word::from(64), + Word::from(0xA0), + Word::from(16), + Word::from(10), + ); } #[test] fn calldatacopy_gadget_multi_step() { + test_ok_root(128, Word::from(0x40), Word::from(16), Word::from(90)); test_ok_internal( Word::from(0x40), Word::from(128), @@ -477,149 +476,4 @@ mod test { Word::from(0), ); } - - // Need to remove, just for test - fn test_ok_root_old( - call_data_length: usize, - memory_offset: Word, - data_offset: Word, - length: Word, - ) { - let randomness = Fp::rand(); - let bytecode = Bytecode::new( - [ - vec![OpcodeId::PUSH32.as_u8()], - length.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - data_offset.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - memory_offset.to_be_bytes().to_vec(), - vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], - ] - .concat(), - ); - let call_id = 1; - let call_data: Vec = rand_bytes(call_data_length); - - let mut rws = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 1, - is_write: false, - call_id, - stack_pointer: 1021, - value: memory_offset, - }, - Rw::Stack { - rw_counter: 2, - is_write: false, - call_id, - stack_pointer: 1022, - value: data_offset, - }, - Rw::Stack { - rw_counter: 3, - is_write: false, - call_id, - stack_pointer: 1023, - value: length, - }, - ], - ), - ( - RwTableTag::CallContext, - vec![Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }], - ), - ] - .into(), - ); - let mut rw_counter = 5; - - let next_memory_word_size = if length.is_zero() { - 0 - } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 - }; - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - - let mut steps = vec![ExecStep { - rw_indices: vec![ - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::Stack, 2), - (RwTableTag::CallContext, 0), - ], - execution_state: ExecutionState::CALLDATACOPY, - rw_counter: 1, - program_counter: 99, - stack_pointer: 1021, - gas_left: gas_cost, - gas_cost, - memory_size: 0, - opcode: Some(OpcodeId::CALLDATACOPY), - ..Default::default() - }]; - - if !length.is_zero() { - make_memory_copy_steps( - call_id, - &call_data, - 0, - data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - true, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - } - - steps.push(ExecStep { - execution_state: ExecutionState::STOP, - rw_counter, - program_counter: 100, - stack_pointer: 1024, - opcode: Some(OpcodeId::STOP), - memory_size: next_memory_word_size * 32, - ..Default::default() - }); - - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - call_data, - call_data_length, - calls: vec![Call { - id: call_id, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - ..Default::default() - }], - steps, - ..Default::default() - }], - rws, - bytecodes: vec![bytecode], - ..Default::default() - }; - - println!("zkevm - 2 - {block:?}"); - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); - } } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 3d0c17389f..9a1f99365b 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -68,8 +68,6 @@ pub fn test_circuits_using_bytecode( // build a witness block from trace result let block = crate::evm_circuit::witness::block_convert(&builder.block, &builder.code_db); - println!("zkevm - 1 - {block:?}"); - // finish required tests according to config using this witness block test_circuits_using_witness_block(block, config) } From 58916b141fefb9352a62bfcbc2777fdca1524350 Mon Sep 17 00:00:00 2001 From: Haichen Shen Date: Tue, 15 Mar 2022 17:15:59 -0700 Subject: [PATCH 19/30] fix rebase --- bus-mapping/src/circuit_input_builder.rs | 87 ++++++++++++--------- bus-mapping/src/evm/opcodes.rs | 52 +++++++++--- bus-mapping/src/evm/opcodes/calldatacopy.rs | 34 ++++---- bus-mapping/src/evm/opcodes/calldatasize.rs | 5 +- bus-mapping/src/evm/opcodes/caller.rs | 5 +- bus-mapping/src/evm/opcodes/callvalue.rs | 5 +- bus-mapping/src/evm/opcodes/dup.rs | 4 +- bus-mapping/src/evm/opcodes/mload.rs | 5 +- bus-mapping/src/evm/opcodes/mstore.rs | 7 +- bus-mapping/src/evm/opcodes/selfbalance.rs | 5 +- bus-mapping/src/evm/opcodes/sload.rs | 5 +- bus-mapping/src/evm/opcodes/stackonlyop.rs | 13 +-- bus-mapping/src/evm/opcodes/stop.rs | 2 +- bus-mapping/src/evm/opcodes/swap.rs | 4 +- mock/src/lib.rs | 3 +- 15 files changed, 148 insertions(+), 88 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 0d2f2f9258..295d69e79b 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -102,7 +102,7 @@ pub enum ExecError { } /// Execution state -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum ExecState { /// EVM Opcode ID Op(OpcodeId), @@ -114,6 +114,35 @@ pub enum ExecState { CopyToMemory, } +impl ExecState { + /// Returns `true` if the `OpcodeId` is a `PUSHn`. + pub fn is_push(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_push() + } else { + false + } + } + + /// Returns `true` if the `OpcodeId` is a `DUPn`. + pub fn is_dup(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_dup() + } else { + false + } + } + + /// Returns `true` if the `OpcodeId` is a `SWAPn`. + pub fn is_swap(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_swap() + } else { + false + } + } +} + /// Auxiliary data of Execution step #[derive(Clone, Debug)] pub enum StepAuxiliaryData { @@ -194,7 +223,7 @@ impl ExecStep { impl Default for ExecStep { fn default() -> Self { Self { - op: OpcodeId::INVALID(0), + exec_state: ExecState::Op(OpcodeId::INVALID(0)), pc: ProgramCounter(0), stack_size: 0, memory_size: 0, @@ -205,6 +234,7 @@ impl Default for ExecStep { swc: 0, bus_mapping_instance: Vec::new(), error: None, + aux_data: None, } } } @@ -404,12 +434,12 @@ impl Call { /// Context of a [`Call`]. #[derive(Debug, Default)] pub struct CallContext { - // Index of call - index: usize, + /// Index of call + pub index: usize, /// State Write Counter tracks the count of state write operations in the /// call. When a subcall in this call succeeds, the `swc` increases by the /// number of successful state writes in the subcall. - swc: usize, + pub swc: usize, } /// A reversion group is the collection of calls and the operations which are @@ -502,6 +532,11 @@ impl TransactionContext { self.is_last_tx } + /// Return the calls in this transaction. + pub fn calls(&self) -> &[CallContext] { + &self.calls + } + /// Return the index of the current call (the last call in the call stack). fn call_index(&self) -> Result { self.calls @@ -692,19 +727,18 @@ pub struct CircuitInputStateRef<'a> { pub tx: &'a mut Transaction, /// Transaction Context pub tx_ctx: &'a mut TransactionContext, - // /// Steps - //pub steps: Vec, } impl<'a> CircuitInputStateRef<'a> { /// - pub fn new_step(&self, geth_step: &GethExecStep) -> ExecStep { - ExecStep::new( + pub fn new_step(&self, geth_step: &GethExecStep) -> Result { + let call_ctx = self.tx_ctx.call_ctx()?; + Ok(ExecStep::new( geth_step, - self.tx_ctx.call_index(), + call_ctx.index, self.block_ctx.rwc, - self.tx_ctx.call_ctx().swc, - ) + call_ctx.swc, + )) } /// Push an [`Operation`] into the [`OperationContainer`] with the next @@ -769,6 +803,7 @@ impl<'a> CircuitInputStateRef<'a> { rw, MemoryOp::new(call_id, address, value), ); + Ok(()) } /// Push a [`StackOp`] into the [`OperationContainer`] with the next @@ -1411,13 +1446,8 @@ impl<'a> CircuitInputBuilder { // - execution_state: BeginTx // - op: None // Generate BeginTx step - let mut step = ExecStep { - gas_left: Gas(tx.gas), - rwc: self.block_ctx.rwc, - ..Default::default() - }; - gen_begin_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx, &mut step))?; - tx.steps.push(step); + let begin_tx_step = gen_begin_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?; + tx.steps.push(begin_tx_step); for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() { let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx); @@ -1433,23 +1463,8 @@ impl<'a> CircuitInputBuilder { // - execution_state: EndTx // - op: None // Generate EndTx step - let step_prev = tx - .steps - .last() - .expect("steps should have at least one BeginTx step"); - let mut step = ExecStep { - gas_left: Gas(step_prev.gas_left.0 - step_prev.gas_cost.0), - rwc: self.block_ctx.rwc, - // For tx without code execution - swc: if let Some(call_ctx) = tx_ctx.calls.last() { - call_ctx.swc - } else { - 0 - }, - ..Default::default() - }; - gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx, &mut step))?; - tx.steps.push(step); + let end_tx_step = gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?; + tx.steps.push(end_tx_step); self.block.txs.push(tx); self.sdb.clear_access_list_and_refund(); diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 15259adcac..5060c7b45f 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,6 +1,6 @@ //! Definition of each opcode of the EVM. use crate::{ - circuit_input_builder::CircuitInputStateRef, + circuit_input_builder::{CircuitInputStateRef, ExecStep}, evm::OpcodeId, operation::{ AccountField, AccountOp, CallContextField, CallContextOp, TxAccessListAccountOp, @@ -10,7 +10,7 @@ use crate::{ }; use core::fmt::Debug; use eth_types::{ - evm_types::{GasCost, MAX_REFUND_QUOTIENT_OF_GAS_USED}, + evm_types::{Gas, GasCost, MAX_REFUND_QUOTIENT_OF_GAS_USED}, GethExecStep, ToWord, }; use log::warn; @@ -242,7 +242,12 @@ pub fn gen_associated_ops( fn_gen_associated_ops(state, next_steps) } -pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { +pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result { + let mut exec_step = ExecStep { + gas_left: Gas(state.tx.gas), + rwc: state.block_ctx.rwc, + ..Default::default() + }; let call = state.call()?.clone(); for (field, value) in [ @@ -257,6 +262,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ), ] { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -269,6 +275,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let caller_address = call.caller_address; let nonce_prev = state.sdb.increase_nonce(&caller_address); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: caller_address, @@ -281,6 +288,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { for address in [call.caller_address, call.address] { state.sdb.add_account_to_access_list(address); state.push_op( + &mut exec_step, RW::WRITE, TxAccessListAccountOp { tx_id: state.tx_ctx.id(), @@ -301,7 +309,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } else { GasCost::TX.as_u64() } + call_data_gas_cost; - state.step.gas_cost = GasCost(intrinsic_gas_cost); + exec_step.gas_cost = GasCost(intrinsic_gas_cost); let (found, caller_account) = state.sdb.get_account_mut(&call.caller_address); if !found { @@ -310,6 +318,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let caller_balance_prev = caller_account.balance; let caller_balance = caller_account.balance - call.value - state.tx.gas_price * state.tx.gas; state.push_op_reversible( + &mut exec_step, RW::WRITE, AccountOp { address: call.caller_address, @@ -327,6 +336,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let callee_balance = callee_account.balance + call.value; let code_hash = callee_account.code_hash; state.push_op_reversible( + &mut exec_step, RW::WRITE, AccountOp { address: call.address, @@ -345,6 +355,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } _ => { state.push_op( + &mut exec_step, RW::READ, AccountOp { address: call.address, @@ -378,6 +389,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { (CallContextField::LastCalleeReturnDataLength, 0.into()), ] { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -387,13 +399,29 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); } - Ok(()) + Ok(exec_step) } -pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { +pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result { + let prev_step = state.tx + .steps() + .last() + .expect("steps should have at least one BeginTx step"); + let mut exec_step = ExecStep { + gas_left: Gas(prev_step.gas_left.0 - prev_step.gas_cost.0), + rwc: state.block_ctx.rwc, + // For tx without code execution + swc: if let Some(call_ctx) = state.tx_ctx.calls().last() { + call_ctx.swc + } else { + 0 + }, + ..Default::default() + }; let call = state.tx.calls()[0].clone(); state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -404,6 +432,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let refund = state.sdb.refund(); state.push_op( + &mut exec_step, RW::READ, TxRefundOp { tx_id: state.tx_ctx.id(), @@ -413,15 +442,16 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); let effective_refund = - refund.min((state.tx.gas - state.step.gas_left.0) / MAX_REFUND_QUOTIENT_OF_GAS_USED as u64); + refund.min((state.tx.gas - exec_step.gas_left.0) / MAX_REFUND_QUOTIENT_OF_GAS_USED as u64); let (found, caller_account) = state.sdb.get_account_mut(&call.caller_address); if !found { return Err(Error::AccountNotFound(call.caller_address)); } let caller_balance_prev = caller_account.balance; let caller_balance = - caller_account.balance + state.tx.gas_price * (state.step.gas_left.0 + effective_refund); + caller_account.balance + state.tx.gas_price * (exec_step.gas_left.0 + effective_refund); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: call.caller_address, @@ -438,8 +468,9 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } let coinbase_balance_prev = coinbase_account.balance; let coinbase_balance = - coinbase_account.balance + effective_tip * (state.tx.gas - state.step.gas_left.0); + coinbase_account.balance + effective_tip * (state.tx.gas - exec_step.gas_left.0); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: state.block.coinbase, @@ -451,6 +482,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { if !state.tx_ctx.is_last_tx() { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.block_ctx.rwc.0 + 1, @@ -460,5 +492,5 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); } - Ok(()) + Ok(exec_step) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 5bf79aa5f9..a75e77db18 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -31,7 +31,7 @@ fn gen_calldatacopy_step( state: &mut CircuitInputStateRef, geth_step: &GethExecStep, ) -> Result { - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; let memory_offset = geth_step.stack.nth_last(0)?; let data_offset = geth_step.stack.nth_last(1)?; let length = geth_step.stack.nth_last(2)?; @@ -41,41 +41,41 @@ fn gen_calldatacopy_step( RW::READ, geth_step.stack.nth_last_filled(0), memory_offset, - ); + )?; state.push_stack_op( &mut exec_step, RW::READ, geth_step.stack.nth_last_filled(1), data_offset, - ); - state.push_stack_op(&mut exec_step, RW::READ, geth_step.stack.nth_last_filled(2), length); + )?; + state.push_stack_op(&mut exec_step, RW::READ, geth_step.stack.nth_last_filled(2), length)?; state.push_op( &mut exec_step, RW::READ, CallContextOp { - call_id: state.call().call_id, + call_id: state.call()?.call_id, field: CallContextField::TxId, value: state.tx_ctx.id().into(), }, ); - if !state.call().is_root { + if !state.call()?.is_root { state.push_op( &mut exec_step, RW::READ, CallContextOp { - call_id: state.call().call_id, + call_id: state.call()?.call_id, field: CallContextField::CallDataLength, - value: state.call().call_data_length.into(), + value: state.call()?.call_data_length.into(), }, ); state.push_op( &mut exec_step, RW::READ, CallContextOp { - call_id: state.call().call_id, + call_id: state.call()?.call_id, field: CallContextField::CallDataOffset, - value: state.call().call_data_offset.into(), + value: state.call()?.call_data_offset.into(), }, ); }; @@ -90,6 +90,7 @@ fn gen_memory_copy_step( dst_addr: u64, src_addr_end: u64, bytes_left: usize, + is_root: bool, ) { let mut selectors = vec![0u8; MAX_COPY_BYTES]; for (idx, selector) in selectors.iter_mut().enumerate() { @@ -97,7 +98,7 @@ fn gen_memory_copy_step( *selector = 1; let addr = src_addr + idx as u64; let byte = if addr < src_addr_end { - if state.call().is_root { + if is_root { state.tx.input[addr as usize] } else { // TODO, read caller memory @@ -122,7 +123,7 @@ fn gen_memory_copy_step( dst_addr, bytes_left: bytes_left as u64, src_addr_end, - from_tx: state.call().is_root, + from_tx: is_root, selectors, }); } @@ -135,8 +136,8 @@ fn gen_memory_copy_steps( let data_offset = geth_steps[0].stack.nth_last(1)?.as_u64(); let length = geth_steps[0].stack.nth_last(2)?.as_usize(); - let call_data_offset = state.call().call_data_offset; - let call_data_length = state.call().call_data_length; + let call_data_offset = state.call()?.call_data_offset; + let call_data_length = state.call()?.call_data_length; let (src_addr, buffer_addr_end) = ( call_data_offset + data_offset, call_data_offset + call_data_length, @@ -145,7 +146,7 @@ fn gen_memory_copy_steps( let mut copied = 0; let mut steps = vec![]; while copied < length { - let mut exec_step = state.new_step(&geth_steps[1]); + let mut exec_step = state.new_step(&geth_steps[1])?; exec_step.exec_state = ExecState::CopyToMemory; gen_memory_copy_step( state, @@ -154,6 +155,7 @@ fn gen_memory_copy_steps( memory_offset + copied as u64, buffer_addr_end, length - copied, + state.call()?.is_root, ); println!("exec_step = {:?}", exec_step); steps.push(exec_step); @@ -163,6 +165,7 @@ fn gen_memory_copy_steps( Ok(steps) } +/* #[cfg(test)] mod calldatacopy_tests { use crate::{ @@ -266,3 +269,4 @@ mod calldatacopy_tests { Ok(()) } } +*/ diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index c1e47dd8e1..1ab6ca9902 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -17,7 +17,7 @@ impl Opcode for Calldatasize { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; let value = geth_steps[1].stack.last()?; state.push_op( &mut exec_step, @@ -40,6 +40,7 @@ impl Opcode for Calldatasize { #[cfg(test)] mod calldatasize_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -64,7 +65,7 @@ mod calldatasize_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLDATASIZE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATASIZE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index a27a0f662c..13922d0ced 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -15,7 +15,7 @@ impl Opcode for Caller { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // Get caller_address result from next step let value = geth_steps[1].stack.last()?; // CallContext read of the caller_address @@ -42,6 +42,7 @@ impl Opcode for Caller { #[cfg(test)] mod caller_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress, ToWord}; use pretty_assertions::assert_eq; @@ -66,7 +67,7 @@ mod caller_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLER) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLER)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index 5c1eb8ab96..a1a43bc2bd 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -15,7 +15,7 @@ impl Opcode for Callvalue { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // Get call_value result from next step let value = geth_steps[1].stack.last()?; // CallContext read of the call_value @@ -42,6 +42,7 @@ impl Opcode for Callvalue { #[cfg(test)] mod callvalue_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -66,7 +67,7 @@ mod callvalue_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLVALUE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLVALUE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index b02f361fa9..3a35fabfd8 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -14,7 +14,7 @@ impl Opcode for Dup { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; let stack_value_read = geth_step.stack.nth_last(N - 1)?; let stack_position = geth_step.stack.nth_last_filled(N - 1); @@ -71,7 +71,7 @@ mod dup_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op.is_dup()) + .filter(|step| step.exec_state.is_dup()) .collect_vec()[i]; assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index e82dce2bb0..5e55aadcda 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -19,7 +19,7 @@ impl Opcode for Mload { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // // First stack read // @@ -60,6 +60,7 @@ impl Opcode for Mload { #[cfg(test)] mod mload_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{MemoryOp, StackOp}; use eth_types::bytecode; use eth_types::evm_types::{OpcodeId, StackAddress}; @@ -90,7 +91,7 @@ mod mload_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::MLOAD) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::MLOAD)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 405e977643..e58bc23726 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -17,7 +17,7 @@ impl Opcode for Mstore { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // First stack read (offset) let offset = geth_step.stack.nth_last(0)?; let offset_pos = geth_step.stack.nth_last_filled(0); @@ -57,6 +57,7 @@ impl Opcode for Mstore { #[cfg(test)] mod mstore_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{MemoryOp, StackOp}; use eth_types::bytecode; use eth_types::evm_types::{MemoryAddress, OpcodeId, StackAddress}; @@ -87,7 +88,7 @@ mod mstore_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op == OpcodeId::MSTORE) + .filter(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE)) .nth(1) .unwrap(); @@ -148,7 +149,7 @@ mod mstore_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::MSTORE8) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE8)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index 86a99ee94c..a590967bff 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -13,7 +13,7 @@ impl Opcode for Selfbalance { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; let self_balance = geth_steps[1].stack.last()?; let callee_address = state.call()?.address; @@ -55,6 +55,7 @@ impl Opcode for Selfbalance { #[cfg(test)] mod selfbalance_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -79,7 +80,7 @@ mod selfbalance_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::SELFBALANCE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::SELFBALANCE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 766b1e253d..b6a0c21b1f 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -18,7 +18,7 @@ impl Opcode for Sload { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // First stack read let stack_value_read = geth_step.stack.last()?; @@ -52,6 +52,7 @@ impl Opcode for Sload { #[cfg(test)] mod sload_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::StackOp; use eth_types::bytecode; use eth_types::evm_types::{OpcodeId, StackAddress}; @@ -85,7 +86,7 @@ mod sload_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::SLOAD) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::SLOAD)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index dd4cb9d091..f081143200 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -18,14 +18,14 @@ impl Opcode for StackOnlyOpcode Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // N stack reads for i in 0..N_POP { state.push_stack_op( &mut exec_step, RW::READ, - step.stack.nth_last_filled(i), - step.stack.nth_last(i)?, + geth_step.stack.nth_last_filled(i), + geth_step.stack.nth_last(i)?, )?; } @@ -34,8 +34,8 @@ impl Opcode for StackOnlyOpcode Opcode for StackOnlyOpcode Result, Error> { - let exec_step = state.new_step(&geth_steps[0]); + let exec_step = state.new_step(&geth_steps[0])?; state.handle_return()?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index d003d14a9e..c60e105e47 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -14,7 +14,7 @@ impl Opcode for Swap { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = state.new_step(geth_step); + let mut exec_step = state.new_step(geth_step)?; // Peek b and a let stack_b_value_read = geth_step.stack.nth_last(N)?; @@ -72,7 +72,7 @@ mod swap_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op.is_swap()) + .filter(|step| step.exec_state.is_swap()) .collect_vec()[i]; diff --git a/mock/src/lib.rs b/mock/src/lib.rs index 947a257036..9c21027534 100644 --- a/mock/src/lib.rs +++ b/mock/src/lib.rs @@ -64,8 +64,9 @@ pub fn new( pub fn new_single_tx_trace_accounts_gas( accounts: Vec, gas: Gas, + input: Option>, ) -> Result { - let mut eth_tx = new_tx(&new_block()); + let mut eth_tx = new_tx(&new_block(), input); eth_tx.gas = Word::from(gas.0); new(accounts, vec![eth_tx]) } From 018866bfc0afee473aca169e827cbf97cd881ef4 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 16 Mar 2022 10:38:04 +0800 Subject: [PATCH 20/30] Set exec_state for BeginTx and EndTx. --- bus-mapping/src/evm/opcodes.rs | 4 +++- bus-mapping/src/evm/opcodes/calldatacopy.rs | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 5060c7b45f..fee984c8dc 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,6 +1,6 @@ //! Definition of each opcode of the EVM. use crate::{ - circuit_input_builder::{CircuitInputStateRef, ExecStep}, + circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep}, evm::OpcodeId, operation::{ AccountField, AccountOp, CallContextField, CallContextOp, TxAccessListAccountOp, @@ -244,6 +244,7 @@ pub fn gen_associated_ops( pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result { let mut exec_step = ExecStep { + exec_state: ExecState::BeginTx, gas_left: Gas(state.tx.gas), rwc: state.block_ctx.rwc, ..Default::default() @@ -408,6 +409,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result Date: Wed, 16 Mar 2022 15:42:37 +0800 Subject: [PATCH 21/30] Fix to return a new exec step in function `dummy_gen_associated_ops`. --- bus-mapping/src/evm/opcodes.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index fee984c8dc..bf51c7545d 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -67,10 +67,10 @@ pub trait Opcode: Debug { } fn dummy_gen_associated_ops( - _state: &mut CircuitInputStateRef, - _next_steps: &[GethExecStep], + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], ) -> Result, Error> { - Ok(vec![]) + Ok(vec![state.new_step(&next_steps[0])?]) } type FnGenAssociatedOps = From ec2ed3399a261b25e7b898a1fb6734f64e1a0270 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 16 Mar 2022 16:49:02 +0800 Subject: [PATCH 22/30] Update bus-mapping calldatacopy unit-test. --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 137 +++++++----------- .../src/evm_circuit/execution/memory_copy.rs | 1 - 2 files changed, 51 insertions(+), 87 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 20f2a57cbd..94c4746600 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -77,7 +77,6 @@ fn gen_calldatacopy_step( }, ); }; - println!("exec_step = {exec_step:?}"); Ok(exec_step) } @@ -89,7 +88,7 @@ fn gen_memory_copy_step( src_addr_end: u64, bytes_left: usize, is_root: bool, -) { +) -> Result<(), Error> { let mut selectors = vec![0u8; MAX_COPY_BYTES]; for (idx, selector) in selectors.iter_mut().enumerate() { if idx < bytes_left { @@ -111,8 +110,7 @@ fn gen_memory_copy_step( } else { 0 }; - println!("calldata[{}] = {}", addr, byte); - state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte); + state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte)?; } } @@ -124,6 +122,8 @@ fn gen_memory_copy_step( from_tx: is_root, selectors, }); + + Ok(()) } fn gen_memory_copy_steps( @@ -154,8 +154,7 @@ fn gen_memory_copy_steps( buffer_addr_end, length - copied, state.call()?.is_root, - ); - println!("exec_step = {:?}", exec_step); + )?; steps.push(exec_step); copied += MAX_COPY_BYTES; } @@ -163,108 +162,74 @@ fn gen_memory_copy_steps( Ok(steps) } -/* #[cfg(test)] mod calldatacopy_tests { - use crate::{ - circuit_input_builder::{ExecStep, TransactionContext}, - mock::BlockData, - operation::{CallContextField, CallContextOp, RW}, - Error, - }; - use eth_types::evm_types::StackAddress; - use eth_types::{bytecode, Word}; - use mock::new_single_tx_trace_code_at_start; + use super::*; + use crate::circuit_input_builder::ExecState; + use crate::operation::StackOp; + use eth_types::{Word, bytecode}; + use eth_types::evm_types::{OpcodeId, StackAddress}; use pretty_assertions::assert_eq; #[test] - fn calldatacopy_opcode_impl() -> Result<(), Error> { - // Set length to zero for only testing CallDataCopy (without CopyToMemory - // steps). - let length = Word::from(0); - let data_offset = Word::from(0); - let memory_offset = Word::from(0x40); - + fn calldatacopy_opcode_impl() { let code = bytecode! { - PUSH32(length) - PUSH32(data_offset) - PUSH32(memory_offset) - #[start] + PUSH32(0) + PUSH32(0) + PUSH32(0x40) CALLDATACOPY STOP }; // Get the execution steps from the external tracer - let block = - BlockData::new_from_geth_data(new_single_tx_trace_code_at_start(&code).unwrap()); + let block = crate::mock::BlockData::new_from_geth_data( + mock::new_single_tx_trace_code(&code).unwrap(), + ); let mut builder = block.new_circuit_input_builder(); builder - .handle_tx(&block.eth_tx, &block.geth_trace) + .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); - let mut test_builder = block.new_circuit_input_builder(); - let mut tx = test_builder - .new_tx(&block.eth_tx, !block.geth_trace.failed) + let step = builder.block.txs()[0] + .steps() + .iter() + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATACOPY)) .unwrap(); - let mut tx_ctx = TransactionContext::new(&block.eth_tx, &block.geth_trace).unwrap(); - // Generate step corresponding to CALLDATACOPY - let mut step = ExecStep::new( - &block.geth_trace.struct_logs[0], - 0, - test_builder.block_ctx.rwc, - 0, + assert_eq!( + [0, 1, 2] + .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|operation| (operation.rw(), operation.op())), + [ + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1021), Word::from(0x40)) + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1022), Word::from(0)) + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1023), Word::from(0)) + ), + ] ); - let mut state = test_builder.state_ref(&mut tx, &mut tx_ctx); - state.push_stack_op(&mut step, RW::READ, StackAddress::from(1021), memory_offset); - state.push_stack_op(&mut step, RW::READ, StackAddress::from(1022), data_offset); - state.push_stack_op(&mut step, RW::READ, StackAddress::from(1023), length); - state.push_op( - &mut step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::TxId, - value: state.tx_ctx.id().into(), + assert_eq!( + { + let operation = &builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()]; + (operation.rw(), operation.op()) }, - ); - - if !state.call().is_root { - state.push_op( - &mut step, - RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataLength, - value: state.call().call_data_length.into(), - }, - ); - state.push_op( - &mut step, + ( RW::READ, - CallContextOp { - call_id: state.call().call_id, - field: CallContextField::CallDataOffset, - value: state.call().call_data_offset.into(), - }, - ); - }; - - 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, + &CallContextOp { + call_id: builder.block.txs()[0].calls()[0].call_id, + field: CallContextField::TxId, + value: Word::from(1), + } + ) ); - - // Compare containers - assert_eq!(builder.block.container, test_builder.block.container); - - Ok(()) } } -*/ diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index f4433f8d33..2555fd113a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -3,7 +3,6 @@ use crate::{ execution::ExecutionGadget, param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, - table::TxContextFieldTag, util::{ constraint_builder::{ConstraintBuilder, StepStateTransition, Transition::Delta}, math_gadget::ComparisonGadget, From 9ec6e77fadd083a395834ae3b281a8a110c5f0a0 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 16 Mar 2022 16:53:25 +0800 Subject: [PATCH 23/30] Update for fmt and clippy. --- bus-mapping/src/circuit_input_builder.rs | 27 ++++++----------- bus-mapping/src/evm/opcodes.rs | 9 ++++-- bus-mapping/src/evm/opcodes/calldatacopy.rs | 15 ++++++---- bus-mapping/src/evm/opcodes/mstore.rs | 7 ++++- bus-mapping/src/evm/opcodes/sload.rs | 7 ++++- bus-mapping/src/evm/opcodes/swap.rs | 29 +++++++++++++++---- mock/src/lib.rs | 7 +++-- .../src/evm_circuit/execution/bitwise.rs | 5 +++- .../src/evm_circuit/execution/calldatacopy.rs | 5 +++- .../src/evm_circuit/execution/memory.rs | 5 +++- zkevm-circuits/src/evm_circuit/witness.rs | 6 ++-- 11 files changed, 81 insertions(+), 41 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 295d69e79b..80d643a5ec 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -761,7 +761,12 @@ impl<'a> CircuitInputStateRef<'a> { /// This method should be used in `Opcode::gen_associated_ops` instead of /// `push_op` when the operation is `RW::WRITE` and it can be reverted (for /// example, a write `StorageOp`). - pub fn push_op_reversible(&mut self, step: &mut ExecStep, rw: RW, op: T) -> Result<(), Error> { + pub fn push_op_reversible( + &mut self, + step: &mut ExecStep, + rw: RW, + op: T, + ) -> Result<(), Error> { let op_ref = self.block.container.insert(Operation::new_reversible( self.block_ctx.rwc.inc_pre(), rw, @@ -798,11 +803,7 @@ impl<'a> CircuitInputStateRef<'a> { value: u8, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op( - step, - rw, - MemoryOp::new(call_id, address, value), - ); + self.push_op(step, rw, MemoryOp::new(call_id, address, value)); Ok(()) } @@ -819,11 +820,7 @@ impl<'a> CircuitInputStateRef<'a> { value: Word, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op( - step, - rw, - StackOp::new(call_id, address, value), - ); + self.push_op(step, rw, StackOp::new(call_id, address, value)); Ok(()) } @@ -1384,13 +1381,7 @@ impl<'a> CircuitInputBuilder { ), ); - Transaction::new( - call_id, - &self.sdb, - &mut self.code_db, - eth_tx, - is_success, - ) + Transaction::new(call_id, &self.sdb, &mut self.code_db, eth_tx, is_success) } /// Iterate over all generated CallContext RwCounterEndOfReversion diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index bf51c7545d..5fc3cf13e5 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -73,8 +73,10 @@ fn dummy_gen_associated_ops( Ok(vec![state.new_step(&next_steps[0])?]) } -type FnGenAssociatedOps = - fn(state: &mut CircuitInputStateRef, next_steps: &[GethExecStep]) -> Result, Error>; +type FnGenAssociatedOps = fn( + state: &mut CircuitInputStateRef, + next_steps: &[GethExecStep], +) -> Result, Error>; fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { match opcode_id { @@ -404,7 +406,8 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result Result { - let prev_step = state.tx + let prev_step = state + .tx .steps() .last() .expect("steps should have at least one BeginTx step"); diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 94c4746600..39209459be 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -17,8 +17,7 @@ impl Opcode for Calldatacopy { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_steps = Vec::new(); - exec_steps.push(gen_calldatacopy_step(state, geth_step)?); + let mut exec_steps = vec![gen_calldatacopy_step(state, geth_step)?]; let memory_copy_steps = gen_memory_copy_steps(state, geth_steps)?; exec_steps.extend(memory_copy_steps); Ok(exec_steps) @@ -46,7 +45,12 @@ fn gen_calldatacopy_step( geth_step.stack.nth_last_filled(1), data_offset, )?; - state.push_stack_op(&mut exec_step, RW::READ, geth_step.stack.nth_last_filled(2), length)?; + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(2), + length, + )?; state.push_op( &mut exec_step, RW::READ, @@ -167,8 +171,8 @@ mod calldatacopy_tests { use super::*; use crate::circuit_input_builder::ExecState; use crate::operation::StackOp; - use eth_types::{Word, bytecode}; use eth_types::evm_types::{OpcodeId, StackAddress}; + use eth_types::{bytecode, Word}; use pretty_assertions::assert_eq; #[test] @@ -219,7 +223,8 @@ mod calldatacopy_tests { assert_eq!( { - let operation = &builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()]; + let operation = + &builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()]; (operation.rw(), operation.op()) }, ( diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index e58bc23726..494107510e 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -45,7 +45,12 @@ impl Opcode for Mstore { // stack write each byte for mstore let bytes = value.to_be_bytes(); for (i, byte) in bytes.iter().enumerate() { - state.push_memory_op(&mut exec_step, RW::WRITE, offset_addr.map(|a| a + i), *byte)?; + state.push_memory_op( + &mut exec_step, + RW::WRITE, + offset_addr.map(|a| a + i), + *byte, + )?; } } } diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index b6a0c21b1f..37ace18d33 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -43,7 +43,12 @@ impl Opcode for Sload { ); // First stack write - state.push_stack_op(&mut exec_step, RW::WRITE, stack_position, storage_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_position, + storage_value_read, + )?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index c60e105e47..188fbbb983 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -19,14 +19,34 @@ impl Opcode for Swap { // Peek b and a let stack_b_value_read = geth_step.stack.nth_last(N)?; let stack_b_position = geth_step.stack.nth_last_filled(N); - state.push_stack_op(&mut exec_step, RW::READ, stack_b_position, stack_b_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::READ, + stack_b_position, + stack_b_value_read, + )?; let stack_a_value_read = geth_step.stack.last()?; let stack_a_position = geth_step.stack.last_filled(); - state.push_stack_op(&mut exec_step, RW::READ, stack_a_position, stack_a_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::READ, + stack_a_position, + stack_a_value_read, + )?; // Write a into b_position, write b into a_position - state.push_stack_op(&mut exec_step, RW::WRITE, stack_b_position, stack_a_value_read)?; - state.push_stack_op(&mut exec_step, RW::WRITE, stack_a_position, stack_b_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_b_position, + stack_a_value_read, + )?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_a_position, + stack_b_value_read, + )?; Ok(vec![exec_step]) } @@ -75,7 +95,6 @@ mod swap_tests { .filter(|step| step.exec_state.is_swap()) .collect_vec()[i]; - let a_pos = StackAddress(1024 - 6); let b_pos = StackAddress(1024 - 5 + i * 2); let a_val = Word::from(*a); diff --git a/mock/src/lib.rs b/mock/src/lib.rs index 9c21027534..098239f846 100644 --- a/mock/src/lib.rs +++ b/mock/src/lib.rs @@ -90,8 +90,11 @@ pub fn new_single_tx_trace_code(code: &Bytecode) -> Result { /// Create a new block with a single tx with the given gas limit that /// executes the code passed by argument. The trace will be generated /// automatically with the external_tracer from the code. -pub fn new_single_tx_trace_code_gas(code: &Bytecode, gas: Gas, input: Option>) --> Result { +pub fn new_single_tx_trace_code_gas( + code: &Bytecode, + gas: Gas, + input: Option>, +) -> Result { let tracer_account = new_tracer_account(code); new_single_tx_trace_accounts_gas(vec![tracer_account], gas, input) } diff --git a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs index 0d5b55efda..0d7b0e4465 100644 --- a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs +++ b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs @@ -128,7 +128,10 @@ mod test { evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Complete), ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config, None), Ok(())); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 00a4c960f6..5cd9a37cf1 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -257,7 +257,10 @@ mod test { let test_config = BytecodeTestConfig { ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config, call_data), Ok(())); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, call_data), + Ok(()) + ); } fn test_ok_internal( diff --git a/zkevm-circuits/src/evm_circuit/execution/memory.rs b/zkevm-circuits/src/evm_circuit/execution/memory.rs index d5c1fff506..d0c0e251ff 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory.rs @@ -217,7 +217,10 @@ mod test { enable_state_circuit_test: false, ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config, None), Ok(())); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 47f7f1e444..a4c70f936e 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -1142,9 +1142,9 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { _ => unimplemented!("unimplemented opcode {:?}", op), } } - circuit_input_builder::ExecState::BeginTx => ExecutionState::BeginTx, - circuit_input_builder::ExecState::EndTx => ExecutionState::EndTx, - circuit_input_builder::ExecState::CopyToMemory => ExecutionState::CopyToMemory, + circuit_input_builder::ExecState::BeginTx => ExecutionState::BeginTx, + circuit_input_builder::ExecState::EndTx => ExecutionState::EndTx, + circuit_input_builder::ExecState::CopyToMemory => ExecutionState::CopyToMemory, } } } From 53d79933709fc1cc973a355476aa96e05f3b6e5c Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 16 Mar 2022 17:25:45 +0800 Subject: [PATCH 24/30] Fix doc test. --- bus-mapping/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus-mapping/src/lib.rs b/bus-mapping/src/lib.rs index d071f4575a..965482b714 100644 --- a/bus-mapping/src/lib.rs +++ b/bus-mapping/src/lib.rs @@ -107,7 +107,7 @@ //! //! // We use some mock data as context for the trace //! let mut eth_block = mock::new_block(); -//! let eth_tx = mock::new_tx(ð_block); +//! let eth_tx = mock::new_tx(ð_block, None); //! eth_block.transactions.push(eth_tx.clone()); //! let mut sdb = StateDB::new(); //! sdb.set_account(ð_tx.from, state_db::Account::zero()); From 3296d5291cdfe0393d0c3dfc7a1d8ff9ff93d7ad Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 17 Mar 2022 07:33:36 +0800 Subject: [PATCH 25/30] Update according to code review. --- bus-mapping/src/circuit_input_builder.rs | 6 +++--- bus-mapping/src/evm/opcodes.rs | 10 ---------- bus-mapping/src/evm/opcodes/calldatacopy.rs | 10 ++-------- bus-mapping/src/evm/opcodes/stackonlyop.rs | 2 +- .../src/evm_circuit/execution/calldatacopy.rs | 9 +++------ 5 files changed, 9 insertions(+), 28 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 80d643a5ec..a031d2c221 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -115,7 +115,7 @@ pub enum ExecState { } impl ExecState { - /// Returns `true` if the `OpcodeId` is a `PUSHn`. + /// Returns `true` if `ExecState` is an opcode and the opcode is a `PUSHn`. pub fn is_push(&self) -> bool { if let ExecState::Op(op) = self { op.is_push() @@ -124,7 +124,7 @@ impl ExecState { } } - /// Returns `true` if the `OpcodeId` is a `DUPn`. + /// Returns `true` if `ExecState` is an opcode and the opcode is a `DUPn`. pub fn is_dup(&self) -> bool { if let ExecState::Op(op) = self { op.is_dup() @@ -133,7 +133,7 @@ impl ExecState { } } - /// Returns `true` if the `OpcodeId` is a `SWAPn`. + /// Returns `true` if `ExecState` is an opcode and the opcode is a `SWAPn`. pub fn is_swap(&self) -> bool { if let ExecState::Op(op) = self { op.is_swap() diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 5fc3cf13e5..5e867cf22a 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -54,16 +54,6 @@ pub trait Opcode: Debug { state: &mut CircuitInputStateRef, geth_steps: &[GethExecStep], ) -> Result, Error>; - - // fn gen_associated_ops( - // state: &mut CircuitInputStateRef, - // next_steps: &[GethExecStep], - // ) -> Result<(), Error> { - // let mut step = state.new_step(&next_steps[0]); - // Self::gen_associated_ops(state, &mut step, next_steps)?; - // state.push_step_to_tx(step); - // Ok(()) - // } } fn dummy_gen_associated_ops( diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 39209459be..482a2fd93f 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -102,14 +102,8 @@ fn gen_memory_copy_step( if is_root { state.tx.input[addr as usize] } else { - // TODO, read caller memory - // state.push_memory_op( - // exec_step, - // RW::READ, - // (addr as usize).into(), - // bytes_map[&addr], - // ); - 0u8 + // TODO: read caller memory + unreachable!() } } else { 0 diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index f081143200..73603f6ce1 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -19,7 +19,7 @@ impl Opcode for StackOnlyOpcode Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - // N stack reads + // N_POP stack reads for i in 0..N_POP { state.push_stack_op( &mut exec_step, diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 5cd9a37cf1..a9d1aaaa88 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -252,13 +252,10 @@ mod test { CALLDATACOPY STOP }; - let call_data = Some(rand_bytes(call_data_length)); - println!("calldata = {:?}", call_data); - let test_config = BytecodeTestConfig { - ..Default::default() - }; + let call_data = rand_bytes(call_data_length); + let test_config = BytecodeTestConfig::default(); assert_eq!( - test_circuits_using_bytecode(bytecode, test_config, call_data), + test_circuits_using_bytecode(bytecode, test_config, Some(call_data)), Ok(()) ); } From 7e8716c3371c2c419bc62f452bbb73093d30de0e Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 17 Mar 2022 07:36:28 +0800 Subject: [PATCH 26/30] Fix a comment. --- bus-mapping/src/circuit_input_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index a031d2c221..8aa2ea4722 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -730,7 +730,7 @@ pub struct CircuitInputStateRef<'a> { } impl<'a> CircuitInputStateRef<'a> { - /// + /// Create a new step from a `GethExecStep` pub fn new_step(&self, geth_step: &GethExecStep) -> Result { let call_ctx = self.tx_ctx.call_ctx()?; Ok(ExecStep::new( From 08fd368f2a224b659be86103e9ebea18a4f8e2f0 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 17 Mar 2022 07:52:50 +0800 Subject: [PATCH 27/30] Fix to directly use StepAuxiliaryData of bus-mapping in zkevm-circuits. --- bus-mapping/src/circuit_input_builder.rs | 8 ----- .../src/evm_circuit/execution/memory_copy.rs | 9 +++-- zkevm-circuits/src/evm_circuit/witness.rs | 36 +------------------ 3 files changed, 5 insertions(+), 48 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 8aa2ea4722..b8bc8f09cd 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -831,14 +831,6 @@ impl<'a> CircuitInputStateRef<'a> { .map(|call_idx| &self.tx.calls[call_idx]) } - // pub fn caller(&self) -> Option<&Call> { - // if self.tx.calls.len() == 1 { - // None - // } else { - // Some(&self.tx.calls[self.tx_ctx.call_index()]) - // } - // } - /// Mutable reference to the current Call pub fn call_mut(&mut self) -> Result<&mut Call, Error> { self.tx_ctx diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index 2555fd113a..ef839ac113 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -9,10 +9,11 @@ use crate::{ memory_gadget::BufferReaderGadget, Cell, }, - witness::{Block, Call, ExecStep, StepAuxiliaryData, Transaction}, + witness::{Block, Call, ExecStep, Transaction}, }, util::Expr, }; +use bus_mapping::circuit_input_builder::StepAuxiliaryData; use eth_types::Field; use halo2_proofs::{circuit::Region, plonk::Error}; @@ -232,11 +233,9 @@ pub mod test { step::ExecutionState, table::RwTableTag, test::{rand_bytes, run_test_circuit_incomplete_fixed_table}, - witness::{ - Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, StepAuxiliaryData, Transaction, - }, + witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; - //use crate::evm_circuit::witness::RwMap; + use bus_mapping::circuit_input_builder::StepAuxiliaryData; use eth_types::evm_types::OpcodeId; use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index a4c70f936e..de71f1d04a 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -7,7 +7,7 @@ use crate::evm_circuit::{ }, util::RandomLinearCombination, }; -use bus_mapping::circuit_input_builder::{self, ExecError, OogError}; +use bus_mapping::circuit_input_builder::{self, ExecError, OogError, StepAuxiliaryData}; use bus_mapping::operation::{self, AccountField, CallContextField}; use eth_types::evm_types::OpcodeId; use eth_types::{Address, Field, ToLittleEndian, ToScalar, ToWord, Word}; @@ -271,40 +271,6 @@ pub struct Call { pub is_static: bool, } -#[derive(Clone, Debug)] -pub enum StepAuxiliaryData { - CopyToMemory { - src_addr: u64, - dst_addr: u64, - bytes_left: u64, - src_addr_end: u64, - from_tx: bool, - selectors: Vec, - }, -} - -impl From for StepAuxiliaryData { - fn from(data: circuit_input_builder::StepAuxiliaryData) -> Self { - match data { - circuit_input_builder::StepAuxiliaryData::CopyToMemory { - src_addr, - dst_addr, - bytes_left, - src_addr_end, - from_tx, - selectors, - } => StepAuxiliaryData::CopyToMemory { - src_addr, - dst_addr, - bytes_left, - src_addr_end, - from_tx, - selectors, - }, - } - } -} - #[derive(Clone, Debug, Default)] pub struct ExecStep { /// The index in the Transaction calls From e89b4725367d5967ebf21d5cb43ac6800edf7c3b Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 17 Mar 2022 08:03:26 +0800 Subject: [PATCH 28/30] Revert a comment and a fix. --- .../src/evm_circuit/execution/memory_copy.rs | 17 +++++++++-------- zkevm-circuits/src/test_util.rs | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index ef839ac113..9529cdf486 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -3,6 +3,7 @@ use crate::{ execution::ExecutionGadget, param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, + table::TxContextFieldTag, util::{ constraint_builder::{ConstraintBuilder, StepStateTransition, Transition::Delta}, math_gadget::ComparisonGadget, @@ -70,14 +71,14 @@ impl ExecutionGadget for CopyToMemoryGadget { ) }); // Read bytes[i] from Tx - // cb.condition(from_tx.expr() * read_flag.clone(), |cb| { - // cb.tx_context_lookup( - // tx_id.expr(), - // TxContextFieldTag::CallData, - // Some(src_addr.expr() + i.expr()), - // buffer_reader.byte(i), - // ) - // }); + cb.condition(from_tx.expr() * read_flag.clone(), |cb| { + cb.tx_context_lookup( + tx_id.expr(), + TxContextFieldTag::CallData, + Some(src_addr.expr() + i.expr()), + buffer_reader.byte(i), + ) + }); // Write bytes[i] to memory when selectors[i] != 0 cb.condition(buffer_reader.has_data(i), |cb| { cb.memory_lookup( diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 9a1f99365b..f435d04770 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -31,9 +31,9 @@ pub fn get_fixed_table(conf: FixedTableConfig) -> Vec { #[derive(Debug, Clone)] pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, + pub evm_circuit_lookup_tags: Vec, pub enable_state_circuit_test: bool, pub gas_limit: u64, - pub evm_circuit_lookup_tags: Vec, } impl Default for BytecodeTestConfig { From 1dc64bbbc6c1c8cb583150533af23fab618c2bad Mon Sep 17 00:00:00 2001 From: Haichen Shen Date: Fri, 18 Mar 2022 13:17:26 -0700 Subject: [PATCH 29/30] address comments --- bus-mapping/src/circuit_input_builder.rs | 37 ++++++++- bus-mapping/src/evm/opcodes.rs | 39 ++------- bus-mapping/src/evm/opcodes/calldatacopy.rs | 29 +++---- bus-mapping/src/evm/opcodes/number.rs | 4 +- .../src/evm_circuit/execution/calldataload.rs | 2 +- .../src/evm_circuit/execution/memory_copy.rs | 83 +++++++++---------- .../src/evm_circuit/util/memory_gadget.rs | 13 +-- 7 files changed, 101 insertions(+), 106 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index b8bc8f09cd..2f57f28c36 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -156,10 +156,8 @@ pub enum StepAuxiliaryData { bytes_left: u64, /// Source end address src_addr_end: u64, - /// If from transaction + /// Indicate if copy from transaction call data from_tx: bool, - /// Selectors - selectors: Vec, }, } @@ -196,7 +194,7 @@ pub struct ExecStep { } impl ExecStep { - /// Create a new Self from a [`GethExecStep`]. + /// Create a new Self from a `GethExecStep`. pub fn new( step: &GethExecStep, call_index: usize, @@ -741,6 +739,37 @@ impl<'a> CircuitInputStateRef<'a> { )) } + /// Create a new BeginTx step + pub fn new_begin_tx_step(&self) -> ExecStep { + ExecStep { + exec_state: ExecState::BeginTx, + gas_left: Gas(self.tx.gas), + rwc: self.block_ctx.rwc, + ..Default::default() + } + } + + /// Create a new EndTx step + pub fn new_end_tx_step(&self) -> ExecStep { + let prev_step = self + .tx + .steps() + .last() + .expect("steps should have at least one BeginTx step"); + ExecStep { + exec_state: ExecState::EndTx, + gas_left: Gas(prev_step.gas_left.0 - prev_step.gas_cost.0), + rwc: self.block_ctx.rwc, + // For tx without code execution + swc: if let Some(call_ctx) = self.tx_ctx.calls().last() { + call_ctx.swc + } else { + 0 + }, + ..Default::default() + } + } + /// Push an [`Operation`] into the [`OperationContainer`] with the next /// [`RWCounter`] and then adds a reference to the stored operation /// ([`OperationRef`]) inside the bus-mapping instance of the current diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 5e867cf22a..e85d58ac3e 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,6 +1,6 @@ //! Definition of each opcode of the EVM. use crate::{ - circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep}, + circuit_input_builder::{CircuitInputStateRef, ExecStep}, evm::OpcodeId, operation::{ AccountField, AccountOp, CallContextField, CallContextOp, TxAccessListAccountOp, @@ -10,7 +10,7 @@ use crate::{ }; use core::fmt::Debug; use eth_types::{ - evm_types::{Gas, GasCost, MAX_REFUND_QUOTIENT_OF_GAS_USED}, + evm_types::{GasCost, MAX_REFUND_QUOTIENT_OF_GAS_USED}, GethExecStep, ToWord, }; use log::warn; @@ -58,14 +58,14 @@ pub trait Opcode: Debug { fn dummy_gen_associated_ops( state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error> { - Ok(vec![state.new_step(&next_steps[0])?]) + Ok(vec![state.new_step(&geth_steps[0])?]) } type FnGenAssociatedOps = fn( state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error>; fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { @@ -228,19 +228,14 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { pub fn gen_associated_ops( opcode_id: &OpcodeId, state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error> { let fn_gen_associated_ops = fn_gen_associated_ops(opcode_id); - fn_gen_associated_ops(state, next_steps) + fn_gen_associated_ops(state, geth_steps) } pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result { - let mut exec_step = ExecStep { - exec_state: ExecState::BeginTx, - gas_left: Gas(state.tx.gas), - rwc: state.block_ctx.rwc, - ..Default::default() - }; + let mut exec_step = state.new_begin_tx_step(); let call = state.call()?.clone(); for (field, value) in [ @@ -396,23 +391,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result Result { - let prev_step = state - .tx - .steps() - .last() - .expect("steps should have at least one BeginTx step"); - let mut exec_step = ExecStep { - exec_state: ExecState::EndTx, - gas_left: Gas(prev_step.gas_left.0 - prev_step.gas_cost.0), - rwc: state.block_ctx.rwc, - // For tx without code execution - swc: if let Some(call_ctx) = state.tx_ctx.calls().last() { - call_ctx.swc - } else { - 0 - }, - ..Default::default() - }; + let mut exec_step = state.new_end_tx_step(); let call = state.tx.calls()[0].clone(); state.push_op( diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 482a2fd93f..46869e4fde 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -93,23 +93,19 @@ fn gen_memory_copy_step( bytes_left: usize, is_root: bool, ) -> Result<(), Error> { - let mut selectors = vec![0u8; MAX_COPY_BYTES]; - for (idx, selector) in selectors.iter_mut().enumerate() { - if idx < bytes_left { - *selector = 1; - let addr = src_addr + idx as u64; - let byte = if addr < src_addr_end { - if is_root { - state.tx.input[addr as usize] - } else { - // TODO: read caller memory - unreachable!() - } + for idx in 0..std::cmp::min(bytes_left, MAX_COPY_BYTES) { + let addr = src_addr + idx as u64; + let byte = if addr < src_addr_end { + if is_root { + state.tx.input[addr as usize] } else { - 0 - }; - state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte)?; - } + // TODO: read caller memory + unimplemented!() + } + } else { + 0 + }; + state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte)?; } exec_step.aux_data = Some(StepAuxiliaryData::CopyToMemory { @@ -118,7 +114,6 @@ fn gen_memory_copy_step( bytes_left: bytes_left as u64, src_addr_end, from_tx: is_root, - selectors, }); Ok(()) diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 8a8c3dbf27..99640cd2b0 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -23,9 +23,7 @@ mod number_tests { BlockData::new_from_geth_data(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(); + builder.handle_tx(&block.eth_tx, &block.geth_trace).unwrap(); let mut test_builder = block.new_circuit_input_builder(); let mut tx = test_builder diff --git a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs index 4e7829d4b5..1d05fbb908 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs @@ -244,7 +244,7 @@ impl ExecutionGadget for CallDataLoadGadget { src_addr as u64, src_addr_end as u64, &calldata_bytes, - &[1u8; N_BYTES_WORD], + &[true; N_BYTES_WORD], )?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index 9529cdf486..aba31b6906 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -173,7 +173,6 @@ impl ExecutionGadget for CopyToMemoryGadget { bytes_left, src_addr_end, from_tx, - selectors, } = step.aux_data.as_ref().unwrap(); self.src_addr @@ -189,16 +188,16 @@ impl ExecutionGadget for CopyToMemoryGadget { self.tx_id .assign(region, offset, Some(F::from(tx.id as u64)))?; - // Retrieve the bytes - assert_eq!(selectors.len(), MAX_COPY_BYTES); + // Fill in selectors and bytes let mut rw_idx = 0; let mut bytes = vec![0u8; MAX_COPY_BYTES]; - for (idx, selector) in selectors.iter().enumerate() { - let addr = *src_addr as usize + idx; - bytes[idx] = if *selector == 1 && addr < *src_addr_end as usize { + let mut selectors = vec![false; MAX_COPY_BYTES]; + for idx in 0..std::cmp::min(*bytes_left as usize, MAX_COPY_BYTES) { + let src_addr = *src_addr as usize + idx; + selectors[idx] = true; + bytes[idx] = if selectors[idx] && src_addr < *src_addr_end as usize { if *from_tx { - assert!(addr < tx.call_data.len()); - tx.call_data[addr] + tx.call_data[src_addr] } else { rw_idx += 1; block.rws[step.rw_indices[rw_idx]].memory_value() @@ -206,16 +205,14 @@ impl ExecutionGadget for CopyToMemoryGadget { } else { 0 }; - if *selector == 1 { - // increase rw_idx for writing back to memory - rw_idx += 1 - } + // increase rw_idx for writing back to memory + rw_idx += 1 } self.buffer_reader - .assign(region, offset, *src_addr, *src_addr_end, &bytes, selectors)?; + .assign(region, offset, *src_addr, *src_addr_end, &bytes, &selectors)?; - let num_bytes_copied = selectors.iter().fold(0, |acc, s| acc + (*s as u64)); + let num_bytes_copied = std::cmp::min(*bytes_left, MAX_COPY_BYTES as u64); self.finish_gadget.assign( region, offset, @@ -257,39 +254,36 @@ pub mod test { rws: &mut RwMap, bytes_map: &HashMap, ) -> (ExecStep, usize) { - let mut selectors = vec![0u8; MAX_COPY_BYTES]; let mut rw_offset: usize = 0; let memory_rws: &mut Vec<_> = rws.0.entry(RwTableTag::Memory).or_insert_with(Vec::new); let rw_idx_start = memory_rws.len(); - for (idx, selector) in selectors.iter_mut().enumerate() { - if idx < bytes_left { - *selector = 1; - let addr = src_addr + idx as u64; - let byte = if addr < src_addr_end { - assert!(bytes_map.contains_key(&addr)); - if !from_tx { - memory_rws.push(Rw::Memory { - rw_counter: rw_counter + rw_offset, - is_write: false, - call_id, - memory_address: src_addr + idx as u64, - byte: bytes_map[&addr], - }); - rw_offset += 1; - } - bytes_map[&addr] - } else { - 0 - }; - memory_rws.push(Rw::Memory { - rw_counter: rw_counter + rw_offset, - is_write: true, - call_id, - memory_address: dst_addr + idx as u64, - byte, - }); - rw_offset += 1; - } + for idx in 0..std::cmp::min(bytes_left, MAX_COPY_BYTES) { + let addr = src_addr + idx as u64; + let byte = if addr < src_addr_end { + assert!(bytes_map.contains_key(&addr)); + if !from_tx { + memory_rws.push(Rw::Memory { + rw_counter: rw_counter + rw_offset, + is_write: false, + call_id, + memory_address: src_addr + idx as u64, + byte: bytes_map[&addr], + }); + rw_offset += 1; + } + bytes_map[&addr] + } else { + 0 + }; + // write to destination memory + memory_rws.push(Rw::Memory { + rw_counter: rw_counter + rw_offset, + is_write: true, + call_id, + memory_address: dst_addr + idx as u64, + byte, + }); + rw_offset += 1; } let rw_idx_end = rws.0[&RwTableTag::Memory].len(); let aux_data = StepAuxiliaryData::CopyToMemory { @@ -298,7 +292,6 @@ pub mod test { bytes_left: bytes_left as u64, src_addr_end, from_tx, - selectors, }; let step = ExecStep { execution_state: ExecutionState::CopyToMemory, diff --git a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs index d510ad3e7e..4c042dc3ee 100644 --- a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs @@ -396,15 +396,16 @@ impl MemoryCopierGasGadget { #[derive(Clone, Debug)] pub(crate) struct BufferReaderGadget { - // The bytes read from buffer + /// The bytes read from buffer bytes: [Cell; MAX_BYTES], - // The selectors that indicate if bytes contain real data + /// The selectors that indicate if the corrsponding byte contains real data + /// or is padded selectors: [Cell; MAX_BYTES], - // bound_dist[i] = max(addr_end - addr_start - i, 0) + /// bound_dist[i] = max(addr_end - addr_start - i, 0) bound_dist: [Cell; MAX_BYTES], - // Check if bound_dist is zero + /// Check if bound_dist is zero bound_dist_is_zero: [IsZeroGadget; MAX_BYTES], - // The min gadget to take the minimum of addr_start and addr_end + /// The min gadget to take the minimum of addr_start and addr_end min_gadget: MinMaxGadget, } @@ -492,7 +493,7 @@ impl addr_start: u64, addr_end: u64, bytes: &[u8], - selectors: &[u8], + selectors: &[bool], ) -> Result<(), Error> { self.min_gadget .assign(region, offset, F::from(addr_start), F::from(addr_end))?; From 103440617cce756eff5300d008bf7d9a7cb0c174 Mon Sep 17 00:00:00 2001 From: Haichen Shen Date: Fri, 18 Mar 2022 15:29:20 -0700 Subject: [PATCH 30/30] fix doc --- zkevm-circuits/src/evm_circuit/util/memory_gadget.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs index 4c042dc3ee..9c1b48ef15 100644 --- a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs @@ -401,7 +401,8 @@ pub(crate) struct BufferReaderGadget; MAX_BYTES], - /// bound_dist[i] = max(addr_end - addr_start - i, 0) + /// `bound_dist` is defined as `max(addr_end - addr_start - i, 0)` for `i` + /// in 0..MAX_BYTES bound_dist: [Cell; MAX_BYTES], /// Check if bound_dist is zero bound_dist_is_zero: [IsZeroGadget; MAX_BYTES],