From 2e2adb4f9fc214aea0c3e812efba606674786a77 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Wed, 23 Feb 2022 23:28:41 +0800 Subject: [PATCH 01/12] feat: codecopy (squashed and rebased with main) --- bus-mapping/src/circuit_input_builder.rs | 13 + zkevm-circuits/src/evm_circuit/execution.rs | 10 + .../src/evm_circuit/execution/codecopy.rs | 481 +++++++++++++++++ .../execution/copy_code_to_memory.rs | 489 ++++++++++++++++++ .../src/evm_circuit/execution/memory_copy.rs | 29 +- zkevm-circuits/src/evm_circuit/step.rs | 2 + zkevm-circuits/src/evm_circuit/table.rs | 1 + .../evm_circuit/util/constraint_builder.rs | 20 + zkevm-circuits/src/evm_circuit/witness.rs | 6 +- 9 files changed, 1042 insertions(+), 9 deletions(-) create mode 100644 zkevm-circuits/src/evm_circuit/execution/codecopy.rs create mode 100644 zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 2f57f28c36..d4e228c7cd 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -159,6 +159,19 @@ pub enum StepAuxiliaryData { /// Indicate if copy from transaction call data from_tx: bool, }, + /// Auxiliary data of Copy Code To Memory + CopyCodeToMemory { + /// Source start address + src_addr: u64, + /// Destination address + dst_addr: u64, + /// Bytes left + bytes_left: u64, + /// Source end address + src_addr_end: u64, + /// Bytecode to be copied + code: eth_types::Bytecode, + }, } /// An execution step of the EVM. diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index f9e78b2503..e5b4be77ae 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -26,8 +26,10 @@ mod calldataload; mod calldatasize; mod caller; mod callvalue; +mod codecopy; mod coinbase; mod comparator; +mod copy_code_to_memory; mod dup; mod end_block; mod end_tx; @@ -62,8 +64,10 @@ use calldataload::CallDataLoadGadget; use calldatasize::CallDataSizeGadget; use caller::CallerGadget; use callvalue::CallValueGadget; +use codecopy::CodeCopyGadget; use coinbase::CoinbaseGadget; use comparator::ComparatorGadget; +use copy_code_to_memory::CopyCodeToMemoryGadget; use dup::DupGadget; use end_block::EndBlockGadget; use end_tx::EndTxGadget; @@ -124,7 +128,9 @@ pub(crate) struct ExecutionConfig { calldatasize_gadget: CallDataSizeGadget, caller_gadget: CallerGadget, call_value_gadget: CallValueGadget, + codecopy_gadget: CodeCopyGadget, comparator_gadget: ComparatorGadget, + copy_code_to_memory_gadget: CopyCodeToMemoryGadget, dup_gadget: DupGadget, end_block_gadget: EndBlockGadget, end_tx_gadget: EndTxGadget, @@ -339,7 +345,9 @@ impl ExecutionConfig { calldatasize_gadget: configure_gadget!(), caller_gadget: configure_gadget!(), call_value_gadget: configure_gadget!(), + codecopy_gadget: configure_gadget!(), comparator_gadget: configure_gadget!(), + copy_code_to_memory_gadget: configure_gadget!(), dup_gadget: configure_gadget!(), end_block_gadget: configure_gadget!(), end_tx_gadget: configure_gadget!(), @@ -649,6 +657,8 @@ impl ExecutionConfig { ExecutionState::CALLDATASIZE => { assign_exec_step!(self.calldatasize_gadget) } + ExecutionState::CODECOPY => assign_exec_step!(self.codecopy_gadget), + ExecutionState::CopyCodeToMemory => assign_exec_step!(self.copy_code_to_memory_gadget), _ => unimplemented!(), } diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs new file mode 100644 index 0000000000..110cf801bd --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -0,0 +1,481 @@ +use std::convert::TryInto; + +use bus_mapping::evm::OpcodeId; +use eth_types::{Field, ToLittleEndian, ToScalar}; +use halo2_proofs::{circuit::Region, plonk::Error}; + +use crate::{ + evm_circuit::{ + param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + step::ExecutionState, + table::{AccountFieldTag, CallContextFieldTag}, + util::{ + common_gadget::SameContextGadget, + constraint_builder::{ConstraintBuilder, StepStateTransition, Transition}, + from_bytes, + memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + Cell, MemoryAddress, + }, + witness::{Block, Call, CodeSource, ExecStep, Transaction}, + }, + util::Expr, +}; + +use super::ExecutionGadget; + +#[derive(Clone, Debug)] +pub(crate) struct CodeCopyGadget { + same_context: SameContextGadget, + /// Holds the memory address for the offset in code from where we read. + code_offset: MemoryAddress, + /// Holds the size of the current environment's bytecode. + code_size: Cell, + /// Holds the current environment's address from where we wish to read code. + account: Cell, + /// The code from current environment is copied to memory. To verify this + /// copy operation we need the MemoryAddressGadget. + dst_memory_addr: MemoryAddressGadget, + /// Opcode CODECOPY has a dynamic gas cost: + /// gas_code = static_gas * minimum_word_size + memory_expansion_cost + memory_expansion: MemoryExpansionGadget, + /// Opcode CODECOPY needs to copy code bytes into memory. We account for + /// the copying costs using the memory copier gas gadget. + memory_copier_gas: MemoryCopierGasGadget, +} + +impl ExecutionGadget for CodeCopyGadget { + const NAME: &'static str = "CODECOPY"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::CODECOPY; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + + // Query elements to be popped from the stack. + let dest_memory_offset = cb.query_cell(); + let code_offset = cb.query_rlc(); + let size = cb.query_rlc(); + + // Pop items from stack. + cb.stack_pop(dest_memory_offset.expr()); + cb.stack_pop(code_offset.expr()); + cb.stack_pop(size.expr()); + + // Construct memory address in the destionation (memory) to which we copy code. + let dst_memory_addr = MemoryAddressGadget::construct(cb, dest_memory_offset, size); + + // Query additional fields for the account's code. + let account = cb.call_context(None, CallContextFieldTag::CalleeAddress); + let code_size = cb.query_cell(); + let code_hash = cb.curr.state.code_source.clone(); + + // Lookup the code hash and code size of the current environment's account. + cb.account_read(account.expr(), AccountFieldTag::CodeSize, code_size.expr()); + cb.account_read(account.expr(), AccountFieldTag::CodeHash, code_hash.expr()); + + // Calculate the next memory size and the gas cost for this memory + // access. This also accounts for the dynamic gas required to copy bytes to + // memory. + let memory_expansion = MemoryExpansionGadget::construct( + cb, + cb.curr.state.memory_word_size.expr(), + [dst_memory_addr.address()], + ); + let memory_copier_gas = MemoryCopierGasGadget::construct( + cb, + dst_memory_addr.length(), + memory_expansion.gas_cost(), + ); + + // Constrain the next step to be the internal `CopyCodeToMemory` step and add + // some preliminary checks on its auxiliary data. + cb.constrain_next_step( + ExecutionState::CopyCodeToMemory, + Some(dst_memory_addr.has_length()), + |cb| { + let next_src_addr = cb.query_cell(); + let next_dst_addr = cb.query_cell(); + let next_bytes_left = cb.query_cell(); + let next_src_addr_end = cb.query_cell(); + let next_code_hash = cb.query_word(); + + cb.require_equal( + "next_src_addr == code_offset", + next_src_addr.expr(), + from_bytes::expr(&code_offset.cells), + ); + cb.require_equal( + "next_dst_addr = memory_offset", + next_dst_addr.expr(), + dst_memory_addr.offset(), + ); + cb.require_equal( + "next_bytes_left = length", + next_bytes_left.expr(), + dst_memory_addr.length(), + ); + cb.require_equal( + "next_src_addr_end == code_size", + next_src_addr_end.expr(), + code_size.expr(), + ); + cb.require_equal( + "next_code_hash == code_hash", + next_code_hash.expr(), + code_hash.expr(), + ); + }, + ); + + // Expected state transition. + let step_state_transition = StepStateTransition { + rw_counter: Transition::Delta(cb.rw_counter_offset()), + program_counter: Transition::Delta(1.expr()), + stack_pointer: Transition::Delta(3.expr()), + memory_word_size: Transition::To(memory_expansion.next_memory_word_size()), + gas_left: Transition::Delta( + -OpcodeId::CODECOPY.constant_gas_cost().expr() - memory_copier_gas.gas_cost(), + ), + ..Default::default() + }; + let same_context = SameContextGadget::construct(cb, opcode, step_state_transition); + + Self { + same_context, + code_offset, + code_size, + account, + dst_memory_addr, + memory_expansion, + memory_copier_gas, + } + } + + fn assign_exec_step( + &self, + region: &mut Region<'_, F>, + offset: usize, + block: &Block, + _tx: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + self.same_context.assign_exec_step(region, offset, step)?; + + // 1. `dest_offset` is the bytes offset in the memory where we start to + // write. + // 2. `code_offset` is the bytes offset in the current + // context's code where we start to read. + // 3. `size` is the number of + // bytes to be read and written (0s to be copied for out of bounds). + let [dest_offset, code_offset, size] = + [0, 1, 2].map(|i| block.rws[step.rw_indices[i]].stack_value()); + + // assign the code offset memory address. + self.code_offset.assign( + region, + offset, + Some( + code_offset.to_le_bytes()[..N_BYTES_MEMORY_ADDRESS] + .try_into() + .unwrap(), + ), + )?; + + let account = if call.is_root { + call.callee_address + } else { + unimplemented!("CODECOPY does not support internal calls yet"); + }; + println!("account = {:?}", account); + self.account.assign(region, offset, account.to_scalar())?; + + let code = block + .bytecodes + .iter() + .find(|b| { + let CodeSource::Account(code_hash) = &call.code_source; + b.hash == *code_hash + }) + .expect("could not find current environment's bytecode"); + self.code_size + .assign(region, offset, Some(F::from(code.bytes.len() as u64)))?; + + // assign the destination memory offset. + let memory_address = + self.dst_memory_addr + .assign(region, offset, dest_offset, size, block.randomness)?; + + // assign to gadgets handling memory expansion cost and copying cost. + let (_, memory_expansion_cost) = self.memory_expansion.assign( + region, + offset, + step.memory_word_size(), + [memory_address], + )?; + self.memory_copier_gas + .assign(region, offset, size.as_u64(), memory_expansion_cost)?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::ops::Sub; + + use bus_mapping::evm::OpcodeId; + use eth_types::{bytecode, Address, ToWord, Word}; + use halo2_proofs::arithmetic::BaseExt; + use num::Zero; + use pairing::bn256::Fr; + + use crate::evm_circuit::{ + execution::copy_code_to_memory::test::make_copy_code_steps, + step::ExecutionState, + table::{AccountFieldTag, CallContextFieldTag, RwTableTag}, + test::{calc_memory_copier_gas_cost, run_test_circuit_incomplete_fixed_table}, + witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, + }; + + fn test_ok(src_addr: u64, dst_addr: u64, size: usize) { + let randomness = Fr::rand(); + let call_id = 1; + let callee_addr = Address::random(); + + let code = bytecode! { + #[start] + PUSH32(Word::from(size)) + PUSH32(Word::from(src_addr)) + PUSH32(Word::from(dst_addr)) + CODECOPY + STOP + }; + let code = Bytecode::new(code.to_vec()); + + let mut rws_map = RwMap( + [ + ( + RwTableTag::Stack, + vec![ + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 1, + is_write: true, + call_id, + stack_pointer: 1023, + value: Word::from(size), + }, + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 2, + is_write: true, + call_id, + stack_pointer: 1022, + value: Word::from(src_addr), + }, + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 3, + is_write: true, + call_id, + stack_pointer: 1021, + value: Word::from(dst_addr), + }, + // First stack item read by CODECOPY. + Rw::Stack { + rw_counter: 4, + is_write: false, + call_id, + stack_pointer: 1021, + value: Word::from(dst_addr), + }, + // Second stack item read by CODECOPY. + Rw::Stack { + rw_counter: 5, + is_write: false, + call_id, + stack_pointer: 1022, + value: Word::from(src_addr), + }, + // Third stack item read by CODECOPY. + Rw::Stack { + rw_counter: 6, + is_write: false, + call_id, + stack_pointer: 1023, + value: Word::from(size), + }, + ], + ), + ( + RwTableTag::CallContext, + vec![ + // Callee address lookup in CODECOPY. + Rw::CallContext { + rw_counter: 7, + call_id, + is_write: false, + field_tag: CallContextFieldTag::CalleeAddress, + value: callee_addr.to_word(), + }, + ], + ), + ( + RwTableTag::Account, + vec![ + // Code size lookup in CODECOPY. + Rw::Account { + rw_counter: 8, + is_write: false, + account_address: callee_addr, + field_tag: AccountFieldTag::CodeSize, + value: Word::from(code.bytes.len()), + value_prev: Word::from(code.bytes.len()), + }, + // Code hash lookup in CODECOPY. + Rw::Account { + rw_counter: 9, + is_write: false, + account_address: callee_addr, + field_tag: AccountFieldTag::CodeHash, + value: code.hash, + value_prev: code.hash, + }, + ], + ), + ] + .into(), + ); + + // After copying bytes from code to memory, we would end up having used this + // much memory. + let next_memory_word_size = if size.is_zero() { + 0 + } else { + (dst_addr + size as u64 + 31) / 32 + }; + let gas_cost_push32 = OpcodeId::PUSH32.constant_gas_cost().as_u64(); + let gas_cost_codecopy = OpcodeId::CODECOPY.constant_gas_cost().as_u64() + + calc_memory_copier_gas_cost(0, next_memory_word_size, size as u64); + let total_gas_cost = (3 * gas_cost_push32) + gas_cost_codecopy; + + let mut steps = vec![ + ExecStep { + rw_indices: vec![(RwTableTag::Stack, 0)], + execution_state: ExecutionState::PUSH, + rw_counter: 1, + program_counter: 0, + stack_pointer: 1024, + gas_left: total_gas_cost, + gas_cost: gas_cost_push32, + opcode: Some(OpcodeId::PUSH32), + ..Default::default() + }, + ExecStep { + rw_indices: vec![(RwTableTag::Stack, 1)], + execution_state: ExecutionState::PUSH, + rw_counter: 2, + program_counter: 33, + stack_pointer: 1023, + gas_left: total_gas_cost.sub(gas_cost_push32), + gas_cost: gas_cost_push32, + opcode: Some(OpcodeId::PUSH32), + ..Default::default() + }, + ExecStep { + rw_indices: vec![(RwTableTag::Stack, 2)], + execution_state: ExecutionState::PUSH, + rw_counter: 3, + program_counter: 66, + stack_pointer: 1022, + gas_left: total_gas_cost.sub(2 * gas_cost_push32), + gas_cost: gas_cost_push32, + opcode: Some(OpcodeId::PUSH32), + ..Default::default() + }, + ExecStep { + rw_indices: vec![ + (RwTableTag::Stack, 3), + (RwTableTag::Stack, 4), + (RwTableTag::Stack, 5), + (RwTableTag::CallContext, 0), + (RwTableTag::Account, 0), + (RwTableTag::Account, 1), + ], + execution_state: ExecutionState::CODECOPY, + rw_counter: 4, + program_counter: 99, + stack_pointer: 1021, + gas_left: gas_cost_codecopy, + gas_cost: gas_cost_codecopy, + opcode: Some(OpcodeId::CODECOPY), + ..Default::default() + }, + ]; + + let program_counter = 100; + let stack_pointer = 1024; + let mut rw_counter = 10; + if !size.is_zero() { + make_copy_code_steps( + call_id, + &code, + src_addr, + dst_addr, + size, + program_counter, + stack_pointer, + next_memory_word_size * 32, + &mut rw_counter, + &mut rws_map, + &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: true, + is_create: false, + code_source: CodeSource::Account(code.hash), + callee_address: callee_addr, + ..Default::default() + }], + steps, + ..Default::default() + }], + rws: rws_map, + bytecodes: vec![code], + ..Default::default() + }; + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } + + #[test] + fn codecopy_gadget_single_step() { + test_ok(0x00, 0x00, 54); + } + + #[test] + fn codecopy_gadget_multi_step() { + test_ok(0x00, 0x40, 123); + } + + #[test] + fn codecopy_gadget_oob() { + test_ok(0x10, 0x20, 200); + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs new file mode 100644 index 0000000000..0d28312fc3 --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs @@ -0,0 +1,489 @@ +use array_init::array_init; +use eth_types::{Field, ToLittleEndian}; +use halo2_proofs::{circuit::Region, plonk::Error}; + +use crate::{ + evm_circuit::{ + param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + step::ExecutionState, + util::{ + constraint_builder::{ConstraintBuilder, StepStateTransition, Transition}, + math_gadget::ComparisonGadget, + memory_gadget::BufferReaderGadget, + Cell, Word, + }, + witness::{Block, Call, ExecStep, StepAuxiliaryData, Transaction}, + }, + util::Expr, +}; + +use super::ExecutionGadget; + +/// Maximum number of bytes that can be copied in one iteration of this gadget. +/// We are bounded because of the limitation on the number of cells we can +/// assign to in one iteration. +const MAX_COPY_BYTES: usize = 54; + +#[derive(Clone, Debug)] +/// This gadget is responsible for copying bytes from an account's code to +/// memory. This is an internal gadget used by the `CodeCopyGadget`. +pub(crate) struct CopyCodeToMemoryGadget { + /// Offset in the source (bytecode) to read from. + src_addr: Cell, + /// Offset in the destination (memory) to write to. + dst_addr: Cell, + /// Number of bytes left to be copied in this iteration. + bytes_left: Cell, + /// Source (bytecode) bytes end here. + src_addr_end: Cell, + /// Keccak-256 hash of the bytecode. + code_hash: Word, + /// Array of booleans to mark whether or not the byte in question is an + /// opcode byte or an argument that follows the opcode. For example, + /// `is_code = true` for `POP`, `is_code = true` for `PUSH32`, but + /// `is_code = false` for the 32 bytes that follow the `PUSH32` opcode. + is_codes: [Cell; MAX_COPY_BYTES], + /// Gadget to assign bytecode to buffer and read byte-by-byte. + buffer_reader: BufferReaderGadget, + /// Comparison gadget to conditionally stop this iterative internal step + /// once all the bytes have been copied. + finish_gadget: ComparisonGadget, +} + +impl ExecutionGadget for CopyCodeToMemoryGadget { + const NAME: &'static str = "COPYCODETOMEMORY"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::CopyCodeToMemory; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + // Query cells for the internal step's auxiliary data and construct the buffer + // reader. + let src_addr = cb.query_cell(); + let dst_addr = cb.query_cell(); + let bytes_left = cb.query_cell(); + let src_addr_end = cb.query_cell(); + let code_hash = cb.query_word(); + let is_codes = array_init(|_| cb.query_bool()); + let buffer_reader = BufferReaderGadget::construct(cb, src_addr.expr(), src_addr_end.expr()); + + // For every byte in the bytecode's span covered in this iteration. + for (idx, is_code) in is_codes.iter().enumerate() { + // Lookup the bytecode table for the byte value read at the appropriate source + // memory address from the buffer. + cb.condition(buffer_reader.read_flag(idx), |cb| { + cb.bytecode_lookup( + code_hash.expr(), + src_addr.expr() + idx.expr(), + buffer_reader.byte(idx), + is_code.expr(), + ); + }); + // Lookup the RW table for a memory write operation at the appropriate + // destination memory address. + cb.condition(buffer_reader.has_data(idx), |cb| { + cb.memory_lookup( + 1.expr(), + dst_addr.expr() + idx.expr(), + buffer_reader.byte(idx), + None, + ); + }); + } + + // Construct the comparison gadget using the number of bytes copied in this + // iteration and the number bytes that were left to be copied before the + // start of this iteration. + let copied_size = buffer_reader.num_bytes(); + let finish_gadget = ComparisonGadget::construct(cb, copied_size.clone(), bytes_left.expr()); + let (lt, finished) = finish_gadget.expr(); + + // We should have continued only until there were no more bytes left to be + // copied. In case the copied size was less than the number of bytes + // left, the iterative process should not be finished. + cb.add_constraint( + "Constrain num_bytes <= bytes_left", + (1.expr() - lt) * (1.expr() - finished.clone()), + ); + + // If the iterative process has not yet finished, we constrain the next step to + // be another `CopyCodeToMemory` while adding some additional + // constraints to the auxiliary data. + cb.constrain_next_step( + ExecutionState::CopyCodeToMemory, + Some(1.expr() - finished), + |cb| { + let next_src_addr = cb.query_cell(); + let next_dst_addr = cb.query_cell(); + let next_bytes_left = cb.query_cell(); + let next_src_addr_end = cb.query_cell(); + let next_code_hash = cb.query_word(); + + cb.require_equal( + "next_src_addr == src_addr + copied_size", + next_src_addr.expr(), + src_addr.expr() + copied_size.clone(), + ); + cb.require_equal( + "dst_addr + copied_size == next_dst_addr", + next_dst_addr.expr(), + dst_addr.expr() + copied_size.clone(), + ); + cb.require_equal( + "next_bytes_left == bytes_left - copied_size", + next_bytes_left.expr(), + bytes_left.expr() - copied_size.clone(), + ); + cb.require_equal( + "next_src_addr_end == src_addr_end", + next_src_addr_end.expr(), + src_addr_end.expr(), + ); + cb.require_equal( + "next_code_hash == code_hash", + next_code_hash.expr(), + code_hash.expr(), + ); + }, + ); + + // Since this is an internal step for `CODECOPY` opcode, we only increment the + // RW counter. The program counter, stack pointer, and other fields do + // not change. + let step_state_transition = StepStateTransition { + rw_counter: Transition::Delta(cb.rw_counter_offset()), + ..Default::default() + }; + cb.require_step_state_transition(step_state_transition); + + Self { + src_addr, + dst_addr, + bytes_left, + src_addr_end, + code_hash, + is_codes, + buffer_reader, + finish_gadget, + } + } + + fn assign_exec_step( + &self, + region: &mut Region<'_, F>, + offset: usize, + block: &Block, + _tx: &Transaction, + _call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + // Read the auxiliary data. + let (src_addr, dst_addr, bytes_left, src_addr_end, code, selectors) = + if let StepAuxiliaryData::CopyCodeToMemory { + src_addr, + dst_addr, + bytes_left, + src_addr_end, + code, + selectors, + } = step + .aux_data + .as_ref() + .expect("could not find aux_data for COPYCODETOMEMORY") + { + ( + src_addr, + dst_addr, + bytes_left, + src_addr_end, + code, + selectors, + ) + } else { + panic!("could not find CopyCodeToMemory aux_data for COPYCODETOMEMORY"); + }; + + // Assign to the appropriate cells. + self.src_addr + .assign(region, offset, Some(F::from(*src_addr)))?; + self.dst_addr + .assign(region, offset, Some(F::from(*dst_addr)))?; + self.bytes_left + .assign(region, offset, Some(F::from(*bytes_left)))?; + self.src_addr_end + .assign(region, offset, Some(F::from(*src_addr_end)))?; + self.code_hash + .assign(region, offset, Some(code.hash.to_le_bytes()))?; + + // Initialise selectors and bytes for the buffer reader. + assert_eq!(selectors.len(), MAX_COPY_BYTES); + let mut bytes = vec![0u8; MAX_COPY_BYTES]; + for (idx, (selector, code_it)) in selectors + .iter() + .zip( + code.table_assignments(block.randomness) + .skip(*src_addr as usize) + .take(MAX_COPY_BYTES), + ) + .enumerate() + { + let addr = *src_addr as usize + idx; + bytes[idx] = if *selector == 1 && addr < *src_addr_end as usize { + assert!(addr < code.bytes.len()); + code.bytes[addr] + } else { + 0 + }; + + // The last entry (index = 3) from the table assignments for bytecode represents + // whether or not it is an opcode byte or an opcode argument. + self.is_codes[idx].assign(region, offset, Some(code_it[3]))?; + } + + // Assign the buffer reader. + self.buffer_reader + .assign(region, offset, *src_addr, *src_addr_end, &bytes, selectors)?; + + // The number of bytes copied here will be the sum of 1s over the selector + // vector. + let num_bytes_copied = selectors.iter().sum::() as u64; + + // Assign the comparison gadget. + self.finish_gadget.assign( + region, + offset, + F::from(num_bytes_copied), + F::from(*bytes_left), + )?; + + Ok(()) + } +} + +#[cfg(test)] +pub(crate) mod test { + use super::MAX_COPY_BYTES; + use std::collections::HashMap; + + use bus_mapping::evm::OpcodeId; + use eth_types::{bytecode, Word}; + use halo2_proofs::arithmetic::BaseExt; + use pairing::bn256::Fr; + + use crate::evm_circuit::{ + step::ExecutionState, + table::RwTableTag, + test::run_test_circuit_incomplete_fixed_table, + witness::{ + Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, StepAuxiliaryData, Transaction, + }, + }; + + #[allow(clippy::too_many_arguments)] + pub(crate) fn make_copy_code_step( + call_id: usize, + src_addr: u64, + dst_addr: u64, + src_addr_end: u64, + bytes_left: usize, + program_counter: u64, + stack_pointer: usize, + memory_size: u64, + rw_counter: usize, + rws: &mut RwMap, + bytes_map: &HashMap, + code: &Bytecode, + ) -> (ExecStep, usize) { + let mut selectors = vec![0u8; MAX_COPY_BYTES]; + let mut rw_offset = 0usize; + 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)); + 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; + } + } + + let rw_idx_end = rws.0[&RwTableTag::Memory].len(); + let aux_data = StepAuxiliaryData::CopyCodeToMemory { + src_addr, + dst_addr, + bytes_left: bytes_left as u64, + src_addr_end, + code: code.clone(), + selectors, + }; + let step = ExecStep { + execution_state: ExecutionState::CopyCodeToMemory, + rw_indices: (rw_idx_start..rw_idx_end) + .map(|idx| (RwTableTag::Memory, idx)) + .collect(), + rw_counter, + program_counter, + stack_pointer, + memory_size, + gas_cost: 0, + aux_data: Some(aux_data), + ..Default::default() + }; + + (step, rw_offset) + } + + #[allow(clippy::too_many_arguments)] + pub(crate) fn make_copy_code_steps( + call_id: usize, + code: &Bytecode, + src_addr: u64, + dst_addr: u64, + length: usize, + program_counter: u64, + stack_pointer: usize, + memory_size: u64, + rw_counter: &mut usize, + rws: &mut RwMap, + steps: &mut Vec, + ) { + let bytes_map = (0..(code.bytes.len() as u64)) + .zip(code.bytes.iter().copied()) + .collect(); + + let mut copied = 0; + while copied < length { + let (step, rw_offset) = make_copy_code_step( + call_id, + src_addr + copied as u64, + dst_addr + copied as u64, + code.bytes.len() as u64, + length - copied, + program_counter, + stack_pointer, + memory_size, + *rw_counter, + rws, + &bytes_map, + code, + ); + steps.push(step); + *rw_counter += rw_offset; + copied += MAX_COPY_BYTES; + } + } + + fn test_ok(src_addr: u64, dst_addr: u64, length: usize) { + let randomness = Fr::rand(); + let call_id = 1; + let mut rws = RwMap::default(); + let mut rw_counter = 1; + let mut steps = Vec::new(); + let memory_size = (dst_addr + length as u64 + 31) / 32 * 32; + + // generate random bytecode longer than `src_addr_end` + let code = bytecode! { + #[start] + PUSH32(Word::from(0x123)) + POP + PUSH32(Word::from(0x213)) + POP + PUSH32(Word::from(0x321)) + POP + PUSH32(Word::from(0x12349AB)) + POP + PUSH32(Word::from(0x1928835)) + POP + }; + let code = Bytecode::new(code.to_vec()); + + let dummy_code = Bytecode::new(vec![OpcodeId::STOP.as_u8()]); + + let program_counter = 0; + let stack_pointer = 1024; + make_copy_code_steps( + call_id, + &code, + src_addr, + dst_addr, + length, + program_counter, + stack_pointer, + memory_size, + &mut rw_counter, + &mut rws, + &mut steps, + ); + + steps.push(ExecStep { + execution_state: ExecutionState::STOP, + rw_counter, + program_counter, + stack_pointer, + memory_size, + opcode: Some(OpcodeId::STOP), + ..Default::default() + }); + + let block = Block { + randomness, + txs: vec![Transaction { + id: 1, + calls: vec![Call { + id: call_id, + is_root: true, + is_create: false, + code_source: CodeSource::Account(dummy_code.hash), + ..Default::default() + }], + steps, + ..Default::default() + }], + rws, + bytecodes: vec![dummy_code, code], + ..Default::default() + }; + assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + } + + #[test] + fn copy_code_to_memory_single_step() { + test_ok( + 0x00, // src_addr + 0x00, // dst_addr + 54, // length + ); + } + + #[test] + fn copy_code_to_memory_multi_step() { + test_ok( + 0x00, // src_addr + 0x40, // dst_addr + 123, // length + ); + } + + #[test] + fn copy_code_to_memory_oob() { + // since the bytecode we construct above is (34 * 5) = 170 bytes long, copying + // 200 bytes means we go out-of-bounds. + test_ok( + 0x10, // src_addr + 0x20, // dst_addr + 200, // length + ); + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index aba31b6906..c4d8f53ec5 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -167,13 +167,28 @@ impl ExecutionGadget for CopyToMemoryGadget { _: &Call, step: &ExecStep, ) -> Result<(), Error> { - let StepAuxiliaryData::CopyToMemory { - src_addr, - dst_addr, - bytes_left, - src_addr_end, - from_tx, - } = step.aux_data.as_ref().unwrap(); + let (src_addr, dst_addr, bytes_left, src_addr_end, from_tx) = + if let StepAuxiliaryData::CopyToMemory { + src_addr, + dst_addr, + bytes_left, + src_addr_end, + from_tx, + } = step + .aux_data + .as_ref() + .expect("could not find aux_data for COPYTOMEMORY") + { + ( + src_addr, + dst_addr, + bytes_left, + src_addr_end, + from_tx, + ) + } else { + panic!("could not find CopyToMemory aux_data for COPYTOMEMORY"); + }; self.src_addr .assign(region, offset, Some(F::from(*src_addr)))?; diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index ebf5343779..c007f5eb05 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -21,6 +21,7 @@ pub enum ExecutionState { BeginTx, EndTx, EndBlock, + CopyCodeToMemory, CopyToMemory, // Opcode successful cases STOP, @@ -140,6 +141,7 @@ impl ExecutionState { Self::BeginTx, Self::EndTx, Self::EndBlock, + Self::CopyCodeToMemory, Self::CopyToMemory, Self::STOP, Self::ADD, diff --git a/zkevm-circuits/src/evm_circuit/table.rs b/zkevm-circuits/src/evm_circuit/table.rs index f067dae2e9..1b47243a74 100644 --- a/zkevm-circuits/src/evm_circuit/table.rs +++ b/zkevm-circuits/src/evm_circuit/table.rs @@ -164,6 +164,7 @@ pub enum AccountFieldTag { Nonce = 1, Balance, CodeHash, + CodeSize, } #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 500b0dcad2..225b963ea5 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -510,6 +510,26 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { ); } + // Bytecode table + + pub(crate) fn bytecode_lookup( + &mut self, + code_hash: Expression, + index: Expression, + value: Expression, + is_code: Expression, + ) { + self.add_lookup( + "Bytecode lookup", + Lookup::Bytecode { + hash: code_hash, + index, + value, + is_code, + }, + ) + } + // Tx context pub(crate) fn tx_context( diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index de71f1d04a..fc97cc9019 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -309,7 +309,7 @@ impl ExecStep { } } -#[derive(Debug, Clone)] +#[derive(Clone, Debug)] pub struct Bytecode { pub hash: Word, pub bytes: Vec, @@ -687,7 +687,9 @@ impl Rw { value_prev, } => { let to_scalar = |value: &Word| match field_tag { - AccountFieldTag::Nonce => value.to_scalar().unwrap(), + AccountFieldTag::Nonce | AccountFieldTag::CodeSize => { + value.to_scalar().unwrap() + } _ => RandomLinearCombination::random_linear_combine( value.to_le_bytes(), randomness, From 6e3e08857ca2038fc37055c6e2b92d94dd91ce51 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Wed, 16 Mar 2022 13:13:22 +0800 Subject: [PATCH 02/12] fix: constraints for aux data --- zkevm-circuits/src/evm_circuit/execution/codecopy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index 110cf801bd..2f3acc4f9d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -62,7 +62,7 @@ impl ExecutionGadget for CodeCopyGadget { cb.stack_pop(size.expr()); // Construct memory address in the destionation (memory) to which we copy code. - let dst_memory_addr = MemoryAddressGadget::construct(cb, dest_memory_offset, size); + let dst_memory_addr = MemoryAddressGadget::construct(cb, dest_memory_offset, size.clone()); // Query additional fields for the account's code. let account = cb.call_context(None, CallContextFieldTag::CalleeAddress); @@ -112,7 +112,7 @@ impl ExecutionGadget for CodeCopyGadget { cb.require_equal( "next_bytes_left = length", next_bytes_left.expr(), - dst_memory_addr.length(), + size.expr(), ); cb.require_equal( "next_src_addr_end == code_size", From cbdabef729719a2e3b4af861c006aa04ec142fe4 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Mon, 21 Mar 2022 12:18:18 +0800 Subject: [PATCH 03/12] feat: update bytecode circuit as per updated spec (failing) --- .../src/bytecode_circuit/bytecode_unroller.rs | 226 +++++++++++++----- zkevm-circuits/src/evm_circuit/table.rs | 7 + zkevm-circuits/src/evm_circuit/witness.rs | 23 +- 3 files changed, 192 insertions(+), 64 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 94483c6728..92901fb428 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -1,6 +1,10 @@ use crate::{ - evm_circuit::util::{ - and, constraint_builder::BaseConstraintBuilder, not, or, select, RandomLinearCombination, + evm_circuit::{ + table::BytecodeFieldTag, + util::{ + and, constraint_builder::BaseConstraintBuilder, not, or, select, + RandomLinearCombination, + }, }, gadget::{ evm_word::encode, @@ -12,7 +16,7 @@ use bus_mapping::evm::OpcodeId; use eth_types::Field; use halo2_proofs::{ circuit::{Layouter, Region}, - plonk::{Advice, Column, ConstraintSystem, Error, Fixed, Selector, VirtualCells}, + plonk::{Advice, Column, ConstraintSystem, Error, Expression, Fixed, Selector, VirtualCells}, poly::Rotation, }; use keccak256::plain::Keccak; @@ -24,9 +28,10 @@ use super::param::{KECCAK_WIDTH, PUSH_TABLE_WIDTH}; #[derive(Clone, Debug, PartialEq)] pub(crate) struct BytecodeRow { hash: F, + tag: F, index: F, is_code: F, - byte: F, + value: F, } /// Unrolled bytecode @@ -44,9 +49,10 @@ pub struct Config { q_first: Column, q_last: Selector, hash: Column, + tag: Column, index: Column, is_code: Column, - byte: Column, + value: Column, push_rindex: Column, hash_rlc: Column, hash_length: Column, @@ -55,6 +61,8 @@ pub struct Config { padding: Column, push_rindex_inv: Column, push_rindex_is_zero: IsZeroConfig, + length_inv: Column, + length_is_zero: IsZeroConfig, push_table: [Column; PUSH_TABLE_WIDTH], keccak_table: [Column; KECCAK_WIDTH], } @@ -65,9 +73,10 @@ impl Config { let q_first = meta.fixed_column(); let q_last = meta.selector(); let hash = meta.advice_column(); + let tag = meta.advice_column(); let index = meta.advice_column(); let is_code = meta.advice_column(); - let byte = meta.advice_column(); + let value = meta.advice_column(); let push_rindex = meta.advice_column(); let hash_rlc = meta.advice_column(); let hash_length = meta.advice_column(); @@ -75,6 +84,7 @@ impl Config { let is_final = meta.advice_column(); let padding = meta.advice_column(); let push_rindex_inv = meta.advice_column(); + let length_inv = meta.advice_column(); let push_table = array_init::array_init(|_| meta.fixed_column()); let keccak_table = array_init::array_init(|_| meta.advice_column()); @@ -92,6 +102,39 @@ impl Config { push_rindex_inv, ); + // Did the previous row have bytecode field tag == Length? + let is_prev_row_tag_length = |meta: &mut VirtualCells| { + and::expr(vec![ + not::expr(meta.query_fixed(q_first, Rotation::cur())), + { + let prev_row_tag = meta.query_advice(tag, Rotation::prev()); + prev_row_tag.expr() * (BytecodeFieldTag::Byte.expr() - prev_row_tag.expr()) + }, + ]) + }; + + // Does the current row have bytecode field tag == Length? + let is_row_tag_length = |meta: &mut VirtualCells| { + let row_tag = meta.query_advice(tag, Rotation::cur()); + row_tag.expr() * (BytecodeFieldTag::Byte.expr() - row_tag.expr()) + }; + + // Does the current row have bytecode field tag == Byte? + let is_row_tag_byte = |meta: &mut VirtualCells| { + let row_tag = meta.query_advice(tag, Rotation::cur()); + row_tag.expr() + * (row_tag.expr() - BytecodeFieldTag::Length.expr()) + * F::from(2).invert().unwrap() + }; + + // For a row tagged Length, is the length (value) zero? + let length_is_zero = IsZeroChip::configure( + meta, + |meta| meta.query_selector(q_enable), + |meta| meta.query_advice(value, Rotation::cur()), + length_inv, + ); + let q_continue = |meta: &mut VirtualCells| { // When // - Not on the first row @@ -105,22 +148,29 @@ impl Config { meta.create_gate("continue", |meta| { let mut cb = BaseConstraintBuilder::default(); cb.require_equal( - "index needs to increase by 1", + "if prev_row_tag == Length: index == 0 else index == index + 1", meta.query_advice(index, Rotation::cur()), - meta.query_advice(index, Rotation::prev()) + 1.expr(), + select::expr( + is_prev_row_tag_length(meta), + 0.expr(), + meta.query_advice(index, Rotation::prev()) + 1.expr(), + ), ); cb.require_equal( "is_code := push_rindex_prev == 0", meta.query_advice(is_code, Rotation::cur()), - push_rindex_is_zero.clone().is_zero_expression, + select::expr( + is_prev_row_tag_length(meta), + 1.expr(), + push_rindex_is_zero.clone().is_zero_expression, + ), ); cb.require_equal( "hash_rlc := hash_rlc_prev * r + byte", meta.query_advice(hash_rlc, Rotation::cur()), meta.query_advice(hash_rlc, Rotation::prev()) * r - + meta.query_advice(byte, Rotation::cur()), + + meta.query_advice(value, Rotation::cur()), ); - cb.require_equal( "hash needs to remain the same", meta.query_advice(hash, Rotation::cur()), @@ -145,26 +195,46 @@ impl Config { ])) }); - meta.create_gate("start", |meta| { + meta.create_gate("start of bytecode", |meta| { let mut cb = BaseConstraintBuilder::default(); - cb.require_zero( - "index needs to start at 0", - meta.query_advice(index, Rotation::cur()), - ); cb.require_equal( - "is_code needs to be 1 (first byte is always an opcode)", - 1.expr(), - meta.query_advice(is_code, Rotation::cur()), + "next row is tagged Length if length of current bytecode == 0", + meta.query_advice(tag, Rotation::next()), + select::expr( + length_is_zero.clone().is_zero_expression, + BytecodeFieldTag::Byte.expr(), + BytecodeFieldTag::Length.expr(), + ), ); - cb.require_equal( - "hash_rlc needs to start at byte", - meta.query_advice(byte, Rotation::cur()), - meta.query_advice(hash_rlc, Rotation::cur()), + cb.condition(length_is_zero.clone().is_zero_expression, |cb| { + cb.require_equal( + "if length == 0: hash == EMPTY_HASH", + meta.query_advice(hash, Rotation::cur()), + Expression::Constant(keccak(&[], r)), + ); + }); + // Conditions: + // - Not Continuing + // - This is the start of a new bytecode + cb.gate(and::expr(vec![ + meta.query_selector(q_enable), + is_row_tag_length(meta), + not::expr(q_continue(meta)), + ])) + }); + + meta.create_gate("start of padding", |meta| { + let mut cb = BaseConstraintBuilder::default(); + cb.require_zero( + "row needs to be marked as padding row", + meta.query_advice(padding, Rotation::cur()), ); // Conditions: - // - Not continuing + // - Not Continuing + // - This is not the start of a new bytecode cb.gate(and::expr(vec![ meta.query_selector(q_enable), + not::expr(is_row_tag_length(meta)), not::expr(q_continue(meta)), ])) }); @@ -196,15 +266,17 @@ impl Config { "padding needs to be boolean", meta.query_advice(padding, Rotation::cur()), ); - cb.require_equal( - "push_rindex := is_code ? byte_push_size : push_rindex_prev - 1", - meta.query_advice(push_rindex, Rotation::cur()), - select::expr( - meta.query_advice(is_code, Rotation::cur()), - meta.query_advice(byte_push_size, Rotation::cur()), - meta.query_advice(push_rindex, Rotation::prev()) - 1.expr(), - ), - ); + cb.condition(is_row_tag_byte(meta), |cb| { + cb.require_equal( + "push_rindex := is_code ? byte_push_size : push_rindex_prev - 1", + meta.query_advice(push_rindex, Rotation::cur()), + select::expr( + meta.query_advice(is_code, Rotation::cur()), + meta.query_advice(byte_push_size, Rotation::cur()), + meta.query_advice(push_rindex, Rotation::prev()) - 1.expr(), + ), + ); + }); // Conditions: Always cb.gate(meta.query_selector(q_enable)) }); @@ -246,9 +318,9 @@ impl Config { // Lookup how many bytes the current opcode pushes // (also indirectly range checks `byte` to be in [0, 255]) meta.lookup_any("Range bytes", |meta| { - // Conditions: Always - let q_enable = meta.query_selector(q_enable); - let lookup_columns = vec![byte, byte_push_size]; + // Conditions: If the current row is is tagged Byte + let q_enable = meta.query_selector(q_enable) * is_row_tag_byte(meta); + let lookup_columns = vec![value, byte_push_size]; let mut constraints = vec![]; for i in 0..PUSH_TABLE_WIDTH { constraints.push(( @@ -286,9 +358,10 @@ impl Config { q_first, q_last, hash, + tag, index, is_code, - byte, + value, push_rindex, hash_rlc, hash_length, @@ -297,6 +370,8 @@ impl Config { padding, push_rindex_inv, push_rindex_is_zero, + length_inv, + length_is_zero, push_table, keccak_table, } @@ -309,6 +384,7 @@ impl Config { witness: &[UnrolledBytecode], ) { let push_rindex_is_zero_chip = IsZeroChip::construct(self.push_rindex_is_zero.clone()); + let length_is_zero_chip = IsZeroChip::construct(self.length_is_zero.clone()); // Subtract the unusable rows from the size let last_row_offset = size - self.minimum_rows + 1; @@ -323,38 +399,41 @@ impl Config { for bytecode in witness.iter() { // Run over all the bytes let mut push_rindex = 0; + let mut byte_push_size = 0; let mut hash_rlc = F::zero(); let hash_length = F::from(bytecode.bytes.len() as u64); - for row in bytecode.rows.iter() { + for (idx, row) in bytecode.rows.iter().enumerate() { // Track which byte is an opcode and which is push // data let is_code = push_rindex == 0; - let byte_push_size = get_push_size(row.byte.get_lower_128() as u8); - push_rindex = if is_code { - byte_push_size - } else { - push_rindex - 1 - }; - - // Add the byte to the accumulator - hash_rlc = hash_rlc * self.r + row.byte; + if idx > 0 { + byte_push_size = get_push_size(row.value.get_lower_128() as u8); + push_rindex = if is_code { + byte_push_size + } else { + push_rindex - 1 + }; + hash_rlc = hash_rlc * self.r + row.value; + } // Set the data for this row self.set_row( &mut region, &push_rindex_is_zero_chip, + &length_is_zero_chip, offset, true, offset == last_row_offset, row.hash, + row.tag, row.index, row.is_code, - row.byte, + row.value, push_rindex, hash_rlc, hash_length, F::from(byte_push_size as u64), - row.index + F::one() == hash_length, + row.index == hash_length, false, F::from(push_rindex_prev), )?; @@ -368,11 +447,13 @@ impl Config { self.set_row( &mut region, &push_rindex_is_zero_chip, + &length_is_zero_chip, idx, idx < size, idx == last_row_offset, F::zero(), F::zero(), + F::zero(), F::one(), F::zero(), 0, @@ -397,13 +478,15 @@ impl Config { &self, region: &mut Region<'_, F>, push_rindex_is_zero_chip: &IsZeroChip, + length_is_zero_chip: &IsZeroChip, offset: usize, enable: bool, last: bool, hash: F, + tag: F, index: F, is_code: F, - byte: F, + value: F, push_rindex: u64, hash_rlc: F, hash_length: F, @@ -433,9 +516,10 @@ impl Config { // Advices for (name, column, value) in &[ ("hash", self.hash, hash), + ("tag", self.tag, tag), ("index", self.index, index), ("is_code", self.is_code, is_code), - ("byte", self.byte, byte), + ("value", self.value, value), ("push_rindex", self.push_rindex, F::from(push_rindex)), ("hash_rlc", self.hash_rlc, hash_rlc), ("hash_length", self.hash_length, hash_length), @@ -454,6 +538,9 @@ impl Config { // push_rindex_is_zero_chip push_rindex_is_zero_chip.assign(region, offset, Some(push_rindex_prev))?; + // length_is_zero chip + length_is_zero_chip.assign(region, offset, Some(hash_length))?; + Ok(()) } @@ -517,7 +604,13 @@ impl Config { fn unroll(bytes: Vec, r: F) -> UnrolledBytecode { let hash = keccak(&bytes[..], r); - let mut rows = vec![]; + let mut rows = vec![BytecodeRow:: { + hash, + tag: F::from(BytecodeFieldTag::Length as u64), + index: F::zero(), + is_code: F::zero(), + value: F::from(bytes.len() as u64), + }]; // Run over all the bytes let mut push_rindex = 0; for (index, byte) in bytes.iter().enumerate() { @@ -531,9 +624,10 @@ fn unroll(bytes: Vec, r: F) -> UnrolledBytecode { rows.push(BytecodeRow:: { hash, + tag: F::from(BytecodeFieldTag::Byte as u64), index: F::from(index as u64), is_code: F::from(is_code as u64), - byte: F::from(*byte as u64), + value: F::from(*byte as u64), }); } UnrolledBytecode { bytes, rows } @@ -653,9 +747,10 @@ mod tests { bytecode.write(byte); rows.push(BytecodeRow { hash: Fr::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), index: Fr::from(rows.len() as u64), is_code: Fr::from(true as u64), - byte: Fr::from(byte as u64), + value: Fr::from(byte as u64), }); } } @@ -665,16 +760,18 @@ mod tests { bytecode.push(n, Word::from_little_endian(&vec![data_byte; n][..])); rows.push(BytecodeRow { hash: Fr::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), index: Fr::from(rows.len() as u64), is_code: Fr::from(true as u64), - byte: Fr::from(OpcodeId::PUSH1.as_u64() + ((n - 1) as u64)), + value: Fr::from(OpcodeId::PUSH1.as_u64() + ((n - 1) as u64)), }); for _ in 0..n { rows.push(BytecodeRow { hash: Fr::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), index: Fr::from(rows.len() as u64), is_code: Fr::from(false as u64), - byte: Fr::from(data_byte as u64), + value: Fr::from(data_byte as u64), }); } } @@ -683,6 +780,16 @@ mod tests { for row in rows.iter_mut() { row.hash = hash; } + rows.insert( + 0, + BytecodeRow { + hash, + tag: Fr::from(BytecodeFieldTag::Length as u64), + index: Fr::zero(), + is_code: Fr::zero(), + value: Fr::from(bytecode.to_vec().len() as u64), + }, + ); // Unroll the bytecode let unrolled = unroll(bytecode.to_vec(), r); // Check if the bytecode was unrolled correctly @@ -693,6 +800,7 @@ mod tests { }, unrolled, ); + println!("reached here"); // Verify the unrolling in the circuit verify::(k, vec![unrolled], true); } @@ -812,19 +920,19 @@ mod tests { // Change the first byte { let mut invalid = unrolled.clone(); - invalid.rows[0].byte = Fr::from(9u64); + invalid.rows[0].value = Fr::from(9u64); verify::(k, vec![invalid], false); } // Change a byte on another position { let mut invalid = unrolled.clone(); - invalid.rows[5].byte = Fr::from(6u64); + invalid.rows[5].value = Fr::from(6u64); verify::(k, vec![invalid], false); } // Set a byte value out of range { let mut invalid = unrolled; - invalid.rows[3].byte = Fr::from(256u64); + invalid.rows[3].value = Fr::from(256u64); verify::(k, vec![invalid], false); } } diff --git a/zkevm-circuits/src/evm_circuit/table.rs b/zkevm-circuits/src/evm_circuit/table.rs index 1b47243a74..c81ae7cc09 100644 --- a/zkevm-circuits/src/evm_circuit/table.rs +++ b/zkevm-circuits/src/evm_circuit/table.rs @@ -167,6 +167,12 @@ pub enum AccountFieldTag { CodeSize, } +#[derive(Clone, Copy, Debug)] +pub enum BytecodeFieldTag { + Length = 1, + Byte, +} + #[derive(Clone, Copy, Debug, PartialEq)] pub enum CallContextFieldTag { RwCounterEndOfReversion = 1, @@ -202,6 +208,7 @@ impl_expr!(FixedTableTag); impl_expr!(TxContextFieldTag); impl_expr!(RwTableTag); impl_expr!(AccountFieldTag); +impl_expr!(BytecodeFieldTag); impl_expr!(CallContextFieldTag); impl_expr!(BlockContextFieldTag); diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index fc97cc9019..195dd3532c 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -3,7 +3,8 @@ use crate::evm_circuit::{ param::{N_BYTES_WORD, STACK_CAPACITY}, step::ExecutionState, table::{ - AccountFieldTag, BlockContextFieldTag, CallContextFieldTag, RwTableTag, TxContextFieldTag, + AccountFieldTag, BlockContextFieldTag, BytecodeFieldTag, CallContextFieldTag, RwTableTag, + TxContextFieldTag, }, util::RandomLinearCombination, }; @@ -324,7 +325,7 @@ impl Bytecode { pub fn table_assignments<'a, F: FieldExt>( &'a self, randomness: F, - ) -> impl Iterator + '_ { + ) -> impl Iterator + '_ { struct BytecodeIterator<'a, F> { idx: usize, push_data_left: usize, @@ -333,10 +334,21 @@ impl Bytecode { } impl<'a, F: FieldExt> Iterator for BytecodeIterator<'a, F> { - type Item = [F; 4]; + type Item = [F; 5]; fn next(&mut self) -> Option { - if self.idx == self.bytes.len() { + if self.idx == 0 { + self.idx += 1; + return Some([ + self.hash, + F::from(BytecodeFieldTag::Length as u64), + F::from(0), + F::from(0), + F::from(self.bytes.len() as u64), + ]); + } + + if self.idx > self.bytes.len() { return None; } @@ -355,9 +367,10 @@ impl Bytecode { Some([ self.hash, + F::from(BytecodeFieldTag::Byte as u64), F::from(idx as u64), - F::from(byte as u64), F::from(is_code as u64), + F::from(byte as u64), ]) } } From b4d2f85c4bc1a3e612b6cea456859033c66df6e0 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Mon, 21 Mar 2022 14:36:02 +0800 Subject: [PATCH 04/12] fix: constraints for updated bytecode circuit --- .../src/bytecode_circuit/bytecode_unroller.rs | 87 ++++++++++++------- zkevm-circuits/src/evm_circuit/witness.rs | 5 +- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 92901fb428..fcdba0755a 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -102,17 +102,6 @@ impl Config { push_rindex_inv, ); - // Did the previous row have bytecode field tag == Length? - let is_prev_row_tag_length = |meta: &mut VirtualCells| { - and::expr(vec![ - not::expr(meta.query_fixed(q_first, Rotation::cur())), - { - let prev_row_tag = meta.query_advice(tag, Rotation::prev()); - prev_row_tag.expr() * (BytecodeFieldTag::Byte.expr() - prev_row_tag.expr()) - }, - ]) - }; - // Does the current row have bytecode field tag == Length? let is_row_tag_length = |meta: &mut VirtualCells| { let row_tag = meta.query_advice(tag, Rotation::cur()); @@ -130,7 +119,7 @@ impl Config { // For a row tagged Length, is the length (value) zero? let length_is_zero = IsZeroChip::configure( meta, - |meta| meta.query_selector(q_enable), + |meta| meta.query_selector(q_enable) * is_row_tag_length(meta), |meta| meta.query_advice(value, Rotation::cur()), length_inv, ); @@ -147,8 +136,20 @@ impl Config { meta.create_gate("continue", |meta| { let mut cb = BaseConstraintBuilder::default(); + + // Did the previous row have bytecode field tag == Length? + let is_prev_row_tag_length = |meta: &mut VirtualCells| { + and::expr(vec![ + not::expr(meta.query_fixed(q_first, Rotation::cur())), + { + let prev_row_tag = meta.query_advice(tag, Rotation::prev()); + prev_row_tag.expr() * (BytecodeFieldTag::Byte.expr() - prev_row_tag.expr()) + }, + ]) + }; + cb.require_equal( - "if prev_row_tag == Length: index == 0 else index == index + 1", + "if prev_row.tag == Length: index == 0 else index == index + 1", meta.query_advice(index, Rotation::cur()), select::expr( is_prev_row_tag_length(meta), @@ -198,17 +199,26 @@ impl Config { meta.create_gate("start of bytecode", |meta| { let mut cb = BaseConstraintBuilder::default(); cb.require_equal( - "next row is tagged Length if length of current bytecode == 0", + "next_row.tag == (tag.Length or tag.Padding) if length == 0 else tag.Byte", meta.query_advice(tag, Rotation::next()), select::expr( length_is_zero.clone().is_zero_expression, + select::expr( + meta.query_advice(padding, Rotation::next()), + 0.expr(), // Padding is 0 + BytecodeFieldTag::Length.expr(), + ), BytecodeFieldTag::Byte.expr(), - BytecodeFieldTag::Length.expr(), ), ); + cb.require_equal( + "if row.tag == tag.Length: value == row.hash_length", + meta.query_advice(value, Rotation::cur()), + meta.query_advice(hash_length, Rotation::cur()), + ); cb.condition(length_is_zero.clone().is_zero_expression, |cb| { cb.require_equal( - "if length == 0: hash == EMPTY_HASH", + "if length == 0: hash == RLC(EMPTY_HASH, randomness)", meta.query_advice(hash, Rotation::cur()), Expression::Constant(keccak(&[], r)), ); @@ -225,9 +235,10 @@ impl Config { meta.create_gate("start of padding", |meta| { let mut cb = BaseConstraintBuilder::default(); - cb.require_zero( - "row needs to be marked as padding row", + cb.require_equal( + "row needs to be marked as padding", meta.query_advice(padding, Rotation::cur()), + 1.expr(), ); // Conditions: // - Not Continuing @@ -241,13 +252,16 @@ impl Config { meta.create_gate("length needs to be correct", |meta| { let mut cb = BaseConstraintBuilder::default(); - cb.require_equal( - "index + 1 needs to equal hash_length", - meta.query_advice(index, Rotation::cur()) + 1.expr(), - meta.query_advice(hash_length, Rotation::cur()), - ); + cb.condition(1.expr() - length_is_zero.clone().is_zero_expression, |cb| { + cb.require_equal( + "index + 1 needs to equal hash_length", + meta.query_advice(index, Rotation::cur()) + 1.expr(), + meta.query_advice(hash_length, Rotation::cur()), + ); + }); // Conditions: // - On the row with the last byte (`is_final == 1`) + // - Of bytecode with length > 0 // - Not padding cb.gate(and::expr(vec![ meta.query_selector(q_enable), @@ -433,7 +447,7 @@ impl Config { hash_rlc, hash_length, F::from(byte_push_size as u64), - row.index == hash_length, + idx == bytecode.bytes.len(), false, F::from(push_rindex_prev), )?; @@ -458,7 +472,7 @@ impl Config { F::zero(), 0, F::zero(), - F::one(), + F::zero(), F::zero(), true, true, @@ -800,7 +814,6 @@ mod tests { }, unrolled, ); - println!("reached here"); // Verify the unrolling in the circuit verify::(k, vec![unrolled], true); } @@ -813,12 +826,24 @@ mod tests { verify::(k, vec![unroll(vec![], r)], true); } + #[test] + fn bytecode_simple() { + let k = 9; + let r = MyCircuit::r(); + let bytecodes = vec![ + unroll(vec![7u8], r), + unroll(vec![6u8], r), + unroll(vec![5u8], r), + ]; + verify::(k, bytecodes, true); + } + /// Tests a fully full circuit #[test] fn bytecode_full() { let k = 9; let r = MyCircuit::r(); - verify::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 6], r)], true); + verify::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7], r)], true); } /// Tests a circuit with incomplete bytecode @@ -920,7 +945,7 @@ mod tests { // Change the first byte { let mut invalid = unrolled.clone(); - invalid.rows[0].value = Fr::from(9u64); + invalid.rows[1].value = Fr::from(9u64); verify::(k, vec![invalid], false); } // Change a byte on another position @@ -956,19 +981,19 @@ mod tests { // Mark the 3rd byte as code (is push data from the first PUSH1) { let mut invalid = unrolled.clone(); - invalid.rows[2].is_code = Fr::one(); + invalid.rows[3].is_code = Fr::one(); verify::(k, vec![invalid], false); } // Mark the 4rd byte as data (is code) { let mut invalid = unrolled.clone(); - invalid.rows[3].is_code = Fr::zero(); + invalid.rows[4].is_code = Fr::zero(); verify::(k, vec![invalid], false); } // Mark the 7th byte as code (is data for the PUSH7) { let mut invalid = unrolled; - invalid.rows[6].is_code = Fr::one(); + invalid.rows[7].is_code = Fr::one(); verify::(k, vec![invalid], false); } } diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 195dd3532c..7255e8b3a5 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -352,19 +352,16 @@ impl Bytecode { return None; } - let idx = self.idx; + let idx = self.idx - 1; let byte = self.bytes[self.idx]; let mut is_code = true; - if self.push_data_left > 0 { is_code = false; self.push_data_left -= 1; } else if (OpcodeId::PUSH1.as_u8()..=OpcodeId::PUSH32.as_u8()).contains(&byte) { self.push_data_left = byte as usize - (OpcodeId::PUSH1.as_u8() - 1) as usize; } - self.idx += 1; - Some([ self.hash, F::from(BytecodeFieldTag::Byte as u64), From 7a89f883930b2dfe147405364cda6e00a848df4e Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Mon, 21 Mar 2022 15:00:03 +0800 Subject: [PATCH 05/12] feat: update cb and codecopy as per bytecode circuit updates --- .../src/evm_circuit/execution/codecopy.rs | 22 ++++---------- .../execution/copy_code_to_memory.rs | 5 ++-- zkevm-circuits/src/evm_circuit/table.rs | 18 +++++++++--- .../evm_circuit/util/constraint_builder.rs | 29 +++++++++++++++---- zkevm-circuits/src/evm_circuit/witness.rs | 2 +- 5 files changed, 47 insertions(+), 29 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index 2f3acc4f9d..d8de423083 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -66,13 +66,14 @@ impl ExecutionGadget for CodeCopyGadget { // Query additional fields for the account's code. let account = cb.call_context(None, CallContextFieldTag::CalleeAddress); - let code_size = cb.query_cell(); let code_hash = cb.curr.state.code_source.clone(); - // Lookup the code hash and code size of the current environment's account. - cb.account_read(account.expr(), AccountFieldTag::CodeSize, code_size.expr()); + // Lookup the code hash of the current environment's account. cb.account_read(account.expr(), AccountFieldTag::CodeHash, code_hash.expr()); + // Fetch the bytecode length from the bytecode table. + let code_size = cb.bytecode_length(code_hash.expr()); + // Calculate the next memory size and the gas cost for this memory // access. This also accounts for the dynamic gas required to copy bytes to // memory. @@ -187,7 +188,6 @@ impl ExecutionGadget for CodeCopyGadget { } else { unimplemented!("CODECOPY does not support internal calls yet"); }; - println!("account = {:?}", account); self.account.assign(region, offset, account.to_scalar())?; let code = block @@ -324,18 +324,9 @@ mod tests { ( RwTableTag::Account, vec![ - // Code size lookup in CODECOPY. - Rw::Account { - rw_counter: 8, - is_write: false, - account_address: callee_addr, - field_tag: AccountFieldTag::CodeSize, - value: Word::from(code.bytes.len()), - value_prev: Word::from(code.bytes.len()), - }, // Code hash lookup in CODECOPY. Rw::Account { - rw_counter: 9, + rw_counter: 8, is_write: false, account_address: callee_addr, field_tag: AccountFieldTag::CodeHash, @@ -401,7 +392,6 @@ mod tests { (RwTableTag::Stack, 5), (RwTableTag::CallContext, 0), (RwTableTag::Account, 0), - (RwTableTag::Account, 1), ], execution_state: ExecutionState::CODECOPY, rw_counter: 4, @@ -416,7 +406,7 @@ mod tests { let program_counter = 100; let stack_pointer = 1024; - let mut rw_counter = 10; + let mut rw_counter = 9; if !size.is_zero() { make_copy_code_steps( call_id, diff --git a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs index 0d28312fc3..3daf4d8eeb 100644 --- a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs @@ -74,8 +74,8 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { cb.bytecode_lookup( code_hash.expr(), src_addr.expr() + idx.expr(), - buffer_reader.byte(idx), is_code.expr(), + buffer_reader.byte(idx), ); }); // Lookup the RW table for a memory write operation at the appropriate @@ -221,7 +221,8 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .iter() .zip( code.table_assignments(block.randomness) - .skip(*src_addr as usize) + // add 1 since the first row is reserved for bytecode length. + .skip(*src_addr as usize + 1) .take(MAX_COPY_BYTES), ) .enumerate() diff --git a/zkevm-circuits/src/evm_circuit/table.rs b/zkevm-circuits/src/evm_circuit/table.rs index c81ae7cc09..ae9005c4f8 100644 --- a/zkevm-circuits/src/evm_circuit/table.rs +++ b/zkevm-circuits/src/evm_circuit/table.rs @@ -262,13 +262,16 @@ pub(crate) enum Lookup { Bytecode { /// Hash to specify which code to read. hash: Expression, + /// Tag to specify whether its the bytecode length or byte value in the + /// bytecode. + tag: Expression, /// Index to specify which byte of bytecode. index: Expression, - /// Value of the index. - value: Expression, /// A boolean value to specify if the value is executable opcode or the /// data portion of PUSH* operations. is_code: Expression, + /// Value of the index. + value: Expression, }, /// Lookup to block table, which contains constants of this block. Block { @@ -321,11 +324,18 @@ impl Lookup { .concat(), Self::Bytecode { hash, + tag, index, - value, is_code, + value, } => { - vec![hash.clone(), index.clone(), value.clone(), is_code.clone()] + vec![ + hash.clone(), + tag.clone(), + index.clone(), + is_code.clone(), + value.clone(), + ] } Self::Block { field_tag, diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 225b963ea5..39081ae7bc 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -3,8 +3,8 @@ use crate::{ param::STACK_CAPACITY, step::{ExecutionState, Preset, Step}, table::{ - AccountFieldTag, CallContextFieldTag, FixedTableTag, Lookup, RwTableTag, - TxContextFieldTag, + AccountFieldTag, BytecodeFieldTag, CallContextFieldTag, FixedTableTag, Lookup, + RwTableTag, TxContextFieldTag, }, util::{Cell, RandomLinearCombination, Word}, }, @@ -502,9 +502,10 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { "Opcode lookup", Lookup::Bytecode { hash: self.curr.state.code_source.expr(), + tag: BytecodeFieldTag::Byte.expr(), index, - value: opcode, is_code, + value: opcode, } .conditional(1.expr() - is_root_create), ); @@ -516,20 +517,36 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { &mut self, code_hash: Expression, index: Expression, - value: Expression, is_code: Expression, + value: Expression, ) { self.add_lookup( - "Bytecode lookup", + "Bytecode (byte) lookup", Lookup::Bytecode { hash: code_hash, + tag: BytecodeFieldTag::Byte.expr(), index, - value, is_code, + value, }, ) } + pub(crate) fn bytecode_length(&mut self, code_hash: Expression) -> Cell { + let cell = self.query_cell(); + self.add_lookup( + "Bytecode (length)", + Lookup::Bytecode { + hash: code_hash, + tag: BytecodeFieldTag::Length.expr(), + index: 0.expr(), + is_code: 0.expr(), + value: cell.expr(), + }, + ); + cell + } + // Tx context pub(crate) fn tx_context( diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 7255e8b3a5..7a31ce68bd 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -353,7 +353,7 @@ impl Bytecode { } let idx = self.idx - 1; - let byte = self.bytes[self.idx]; + let byte = self.bytes[idx]; let mut is_code = true; if self.push_data_left > 0 { is_code = false; From a5ba86e1c0a00a579c2e1075bf048c27c620d0d8 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Tue, 22 Mar 2022 00:36:38 +0800 Subject: [PATCH 06/12] fix: use current instruction's code source --- .../src/evm_circuit/execution/codecopy.rs | 180 +++++++----------- .../execution/copy_code_to_memory.rs | 20 +- 2 files changed, 76 insertions(+), 124 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index d8de423083..f7a2ac49ce 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -1,14 +1,13 @@ use std::convert::TryInto; use bus_mapping::evm::OpcodeId; -use eth_types::{Field, ToLittleEndian, ToScalar}; +use eth_types::{Field, ToLittleEndian}; use halo2_proofs::{circuit::Region, plonk::Error}; use crate::{ evm_circuit::{ param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, - table::{AccountFieldTag, CallContextFieldTag}, util::{ common_gadget::SameContextGadget, constraint_builder::{ConstraintBuilder, StepStateTransition, Transition}, @@ -30,8 +29,6 @@ pub(crate) struct CodeCopyGadget { code_offset: MemoryAddress, /// Holds the size of the current environment's bytecode. code_size: Cell, - /// Holds the current environment's address from where we wish to read code. - account: Cell, /// The code from current environment is copied to memory. To verify this /// copy operation we need the MemoryAddressGadget. dst_memory_addr: MemoryAddressGadget, @@ -64,15 +61,11 @@ impl ExecutionGadget for CodeCopyGadget { // Construct memory address in the destionation (memory) to which we copy code. let dst_memory_addr = MemoryAddressGadget::construct(cb, dest_memory_offset, size.clone()); - // Query additional fields for the account's code. - let account = cb.call_context(None, CallContextFieldTag::CalleeAddress); - let code_hash = cb.curr.state.code_source.clone(); - - // Lookup the code hash of the current environment's account. - cb.account_read(account.expr(), AccountFieldTag::CodeHash, code_hash.expr()); + // Fetch the source code running in current environment. + let code_source = cb.curr.state.code_source.clone(); // Fetch the bytecode length from the bytecode table. - let code_size = cb.bytecode_length(code_hash.expr()); + let code_size = cb.bytecode_length(code_source.expr()); // Calculate the next memory size and the gas cost for this memory // access. This also accounts for the dynamic gas required to copy bytes to @@ -98,7 +91,7 @@ impl ExecutionGadget for CodeCopyGadget { let next_dst_addr = cb.query_cell(); let next_bytes_left = cb.query_cell(); let next_src_addr_end = cb.query_cell(); - let next_code_hash = cb.query_word(); + let next_code_source = cb.query_word(); cb.require_equal( "next_src_addr == code_offset", @@ -121,9 +114,9 @@ impl ExecutionGadget for CodeCopyGadget { code_size.expr(), ); cb.require_equal( - "next_code_hash == code_hash", - next_code_hash.expr(), - code_hash.expr(), + "next_code_source == code_source", + next_code_source.expr(), + code_source.expr(), ); }, ); @@ -145,7 +138,6 @@ impl ExecutionGadget for CodeCopyGadget { same_context, code_offset, code_size, - account, dst_memory_addr, memory_expansion, memory_copier_gas, @@ -183,19 +175,12 @@ impl ExecutionGadget for CodeCopyGadget { ), )?; - let account = if call.is_root { - call.callee_address - } else { - unimplemented!("CODECOPY does not support internal calls yet"); - }; - self.account.assign(region, offset, account.to_scalar())?; - let code = block .bytecodes .iter() .find(|b| { - let CodeSource::Account(code_hash) = &call.code_source; - b.hash == *code_hash + let CodeSource::Account(code_source) = &call.code_source; + b.hash == *code_source }) .expect("could not find current environment's bytecode"); self.code_size @@ -225,7 +210,7 @@ mod tests { use std::ops::Sub; use bus_mapping::evm::OpcodeId; - use eth_types::{bytecode, Address, ToWord, Word}; + use eth_types::{bytecode, Word}; use halo2_proofs::arithmetic::BaseExt; use num::Zero; use pairing::bn256::Fr; @@ -233,7 +218,7 @@ mod tests { use crate::evm_circuit::{ execution::copy_code_to_memory::test::make_copy_code_steps, step::ExecutionState, - table::{AccountFieldTag, CallContextFieldTag, RwTableTag}, + table::RwTableTag, test::{calc_memory_copier_gas_cost, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; @@ -241,7 +226,6 @@ mod tests { fn test_ok(src_addr: u64, dst_addr: u64, size: usize) { let randomness = Fr::rand(); let call_id = 1; - let callee_addr = Address::random(); let code = bytecode! { #[start] @@ -254,88 +238,59 @@ mod tests { let code = Bytecode::new(code.to_vec()); let mut rws_map = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - // Stack item written by PUSH32. - Rw::Stack { - rw_counter: 1, - is_write: true, - call_id, - stack_pointer: 1023, - value: Word::from(size), - }, - // Stack item written by PUSH32. - Rw::Stack { - rw_counter: 2, - is_write: true, - call_id, - stack_pointer: 1022, - value: Word::from(src_addr), - }, - // Stack item written by PUSH32. - Rw::Stack { - rw_counter: 3, - is_write: true, - call_id, - stack_pointer: 1021, - value: Word::from(dst_addr), - }, - // First stack item read by CODECOPY. - Rw::Stack { - rw_counter: 4, - is_write: false, - call_id, - stack_pointer: 1021, - value: Word::from(dst_addr), - }, - // Second stack item read by CODECOPY. - Rw::Stack { - rw_counter: 5, - is_write: false, - call_id, - stack_pointer: 1022, - value: Word::from(src_addr), - }, - // Third stack item read by CODECOPY. - Rw::Stack { - rw_counter: 6, - is_write: false, - call_id, - stack_pointer: 1023, - value: Word::from(size), - }, - ], - ), - ( - RwTableTag::CallContext, - vec![ - // Callee address lookup in CODECOPY. - Rw::CallContext { - rw_counter: 7, - call_id, - is_write: false, - field_tag: CallContextFieldTag::CalleeAddress, - value: callee_addr.to_word(), - }, - ], - ), - ( - RwTableTag::Account, - vec![ - // Code hash lookup in CODECOPY. - Rw::Account { - rw_counter: 8, - is_write: false, - account_address: callee_addr, - field_tag: AccountFieldTag::CodeHash, - value: code.hash, - value_prev: code.hash, - }, - ], - ), - ] + [( + RwTableTag::Stack, + vec![ + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 1, + is_write: true, + call_id, + stack_pointer: 1023, + value: Word::from(size), + }, + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 2, + is_write: true, + call_id, + stack_pointer: 1022, + value: Word::from(src_addr), + }, + // Stack item written by PUSH32. + Rw::Stack { + rw_counter: 3, + is_write: true, + call_id, + stack_pointer: 1021, + value: Word::from(dst_addr), + }, + // First stack item read by CODECOPY. + Rw::Stack { + rw_counter: 4, + is_write: false, + call_id, + stack_pointer: 1021, + value: Word::from(dst_addr), + }, + // Second stack item read by CODECOPY. + Rw::Stack { + rw_counter: 5, + is_write: false, + call_id, + stack_pointer: 1022, + value: Word::from(src_addr), + }, + // Third stack item read by CODECOPY. + Rw::Stack { + rw_counter: 6, + is_write: false, + call_id, + stack_pointer: 1023, + value: Word::from(size), + }, + ], + )] .into(), ); @@ -390,8 +345,6 @@ mod tests { (RwTableTag::Stack, 3), (RwTableTag::Stack, 4), (RwTableTag::Stack, 5), - (RwTableTag::CallContext, 0), - (RwTableTag::Account, 0), ], execution_state: ExecutionState::CODECOPY, rw_counter: 4, @@ -406,7 +359,7 @@ mod tests { let program_counter = 100; let stack_pointer = 1024; - let mut rw_counter = 9; + let mut rw_counter = 7; if !size.is_zero() { make_copy_code_steps( call_id, @@ -441,7 +394,6 @@ mod tests { is_root: true, is_create: false, code_source: CodeSource::Account(code.hash), - callee_address: callee_addr, ..Default::default() }], steps, diff --git a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs index 3daf4d8eeb..6f4cf56333 100644 --- a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs @@ -36,8 +36,8 @@ pub(crate) struct CopyCodeToMemoryGadget { bytes_left: Cell, /// Source (bytecode) bytes end here. src_addr_end: Cell, - /// Keccak-256 hash of the bytecode. - code_hash: Word, + /// Keccak-256 hash of the bytecode source. + code_source: Word, /// Array of booleans to mark whether or not the byte in question is an /// opcode byte or an argument that follows the opcode. For example, /// `is_code = true` for `POP`, `is_code = true` for `PUSH32`, but @@ -62,7 +62,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { let dst_addr = cb.query_cell(); let bytes_left = cb.query_cell(); let src_addr_end = cb.query_cell(); - let code_hash = cb.query_word(); + let code_source = cb.query_word(); let is_codes = array_init(|_| cb.query_bool()); let buffer_reader = BufferReaderGadget::construct(cb, src_addr.expr(), src_addr_end.expr()); @@ -72,7 +72,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { // memory address from the buffer. cb.condition(buffer_reader.read_flag(idx), |cb| { cb.bytecode_lookup( - code_hash.expr(), + code_source.expr(), src_addr.expr() + idx.expr(), is_code.expr(), buffer_reader.byte(idx), @@ -116,7 +116,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { let next_dst_addr = cb.query_cell(); let next_bytes_left = cb.query_cell(); let next_src_addr_end = cb.query_cell(); - let next_code_hash = cb.query_word(); + let next_code_source = cb.query_word(); cb.require_equal( "next_src_addr == src_addr + copied_size", @@ -139,9 +139,9 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { src_addr_end.expr(), ); cb.require_equal( - "next_code_hash == code_hash", - next_code_hash.expr(), - code_hash.expr(), + "next_code_sourcec == code_source", + next_code_source.expr(), + code_source.expr(), ); }, ); @@ -160,7 +160,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { dst_addr, bytes_left, src_addr_end, - code_hash, + code_source, is_codes, buffer_reader, finish_gadget, @@ -211,7 +211,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .assign(region, offset, Some(F::from(*bytes_left)))?; self.src_addr_end .assign(region, offset, Some(F::from(*src_addr_end)))?; - self.code_hash + self.code_source .assign(region, offset, Some(code.hash.to_le_bytes()))?; // Initialise selectors and bytes for the buffer reader. From 1fed95585c79a1846aea3544ee8f50c956a2ac0c Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Tue, 22 Mar 2022 11:24:51 +0800 Subject: [PATCH 07/12] fix: remove selectors field from copycode aux data --- .../execution/copy_code_to_memory.rs | 88 ++++++++----------- 1 file changed, 37 insertions(+), 51 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs index 6f4cf56333..ebe4acad6e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs @@ -177,27 +177,19 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { step: &ExecStep, ) -> Result<(), Error> { // Read the auxiliary data. - let (src_addr, dst_addr, bytes_left, src_addr_end, code, selectors) = + let (src_addr, dst_addr, bytes_left, src_addr_end, code) = if let StepAuxiliaryData::CopyCodeToMemory { src_addr, dst_addr, bytes_left, src_addr_end, code, - selectors, } = step .aux_data .as_ref() .expect("could not find aux_data for COPYCODETOMEMORY") { - ( - src_addr, - dst_addr, - bytes_left, - src_addr_end, - code, - selectors, - ) + (src_addr, dst_addr, bytes_left, src_addr_end, code) } else { panic!("could not find CopyCodeToMemory aux_data for COPYCODETOMEMORY"); }; @@ -215,38 +207,37 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .assign(region, offset, Some(code.hash.to_le_bytes()))?; // Initialise selectors and bytes for the buffer reader. - assert_eq!(selectors.len(), MAX_COPY_BYTES); + let mut new_selectors = vec![0u8; MAX_COPY_BYTES]; let mut bytes = vec![0u8; MAX_COPY_BYTES]; - for (idx, (selector, code_it)) in selectors - .iter() - .zip( - code.table_assignments(block.randomness) - // add 1 since the first row is reserved for bytecode length. - .skip(*src_addr as usize + 1) - .take(MAX_COPY_BYTES), - ) - .enumerate() - { + let is_codes = code + .table_assignments(block.randomness) + .skip(1) + .map(|c| c[3]) + .collect::>(); + for idx in 0..std::cmp::min(*bytes_left as usize, MAX_COPY_BYTES) { + new_selectors[idx] = 1u8; let addr = *src_addr as usize + idx; - bytes[idx] = if *selector == 1 && addr < *src_addr_end as usize { + bytes[idx] = if addr < *src_addr_end as usize { assert!(addr < code.bytes.len()); + self.is_codes[idx].assign(region, offset, Some(is_codes[addr]))?; code.bytes[addr] } else { 0 }; - - // The last entry (index = 3) from the table assignments for bytecode represents - // whether or not it is an opcode byte or an opcode argument. - self.is_codes[idx].assign(region, offset, Some(code_it[3]))?; } - // Assign the buffer reader. - self.buffer_reader - .assign(region, offset, *src_addr, *src_addr_end, &bytes, selectors)?; + self.buffer_reader.assign( + region, + offset, + *src_addr, + *src_addr_end, + &bytes, + &new_selectors, + )?; // The number of bytes copied here will be the sum of 1s over the selector // vector. - let num_bytes_copied = selectors.iter().sum::() as u64; + let num_bytes_copied = std::cmp::min(*bytes_left, MAX_COPY_BYTES as u64); // Assign the comparison gadget. self.finish_gadget.assign( @@ -294,30 +285,26 @@ pub(crate) mod test { bytes_map: &HashMap, code: &Bytecode, ) -> (ExecStep, usize) { - let mut selectors = vec![0u8; MAX_COPY_BYTES]; let mut rw_offset = 0usize; 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)); - 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)); + 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; } let rw_idx_end = rws.0[&RwTableTag::Memory].len(); @@ -327,7 +314,6 @@ pub(crate) mod test { bytes_left: bytes_left as u64, src_addr_end, code: code.clone(), - selectors, }; let step = ExecStep { execution_state: ExecutionState::CopyCodeToMemory, From 197d289f24460e35079d7c6171ef73f93c606768 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Tue, 22 Mar 2022 11:55:18 +0800 Subject: [PATCH 08/12] chore: rebased and updated --- .../src/evm_circuit/execution/codecopy.rs | 2 +- .../execution/copy_code_to_memory.rs | 43 +++++++++---------- .../src/evm_circuit/execution/memory_copy.rs | 8 +--- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index f7a2ac49ce..ea4d1ce0e2 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -235,7 +235,6 @@ mod tests { CODECOPY STOP }; - let code = Bytecode::new(code.to_vec()); let mut rws_map = RwMap( [( @@ -385,6 +384,7 @@ mod tests { ..Default::default() }); + let code = Bytecode::new(code.to_vec()); let block = Block { randomness, txs: vec![Transaction { diff --git a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs index ebe4acad6e..b210d19126 100644 --- a/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/copy_code_to_memory.rs @@ -1,4 +1,5 @@ use array_init::array_init; +use bus_mapping::circuit_input_builder::StepAuxiliaryData; use eth_types::{Field, ToLittleEndian}; use halo2_proofs::{circuit::Region, plonk::Error}; @@ -12,7 +13,7 @@ use crate::{ memory_gadget::BufferReaderGadget, Cell, Word, }, - witness::{Block, Call, ExecStep, StepAuxiliaryData, Transaction}, + witness::{Block, Bytecode, Call, ExecStep, Transaction}, }, util::Expr, }; @@ -189,7 +190,13 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .as_ref() .expect("could not find aux_data for COPYCODETOMEMORY") { - (src_addr, dst_addr, bytes_left, src_addr_end, code) + ( + src_addr, + dst_addr, + bytes_left, + src_addr_end, + Bytecode::new(code.to_vec()), + ) } else { panic!("could not find CopyCodeToMemory aux_data for COPYCODETOMEMORY"); }; @@ -207,7 +214,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .assign(region, offset, Some(code.hash.to_le_bytes()))?; // Initialise selectors and bytes for the buffer reader. - let mut new_selectors = vec![0u8; MAX_COPY_BYTES]; + let mut selectors = vec![false; MAX_COPY_BYTES]; let mut bytes = vec![0u8; MAX_COPY_BYTES]; let is_codes = code .table_assignments(block.randomness) @@ -215,7 +222,7 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { .map(|c| c[3]) .collect::>(); for idx in 0..std::cmp::min(*bytes_left as usize, MAX_COPY_BYTES) { - new_selectors[idx] = 1u8; + selectors[idx] = true; let addr = *src_addr as usize + idx; bytes[idx] = if addr < *src_addr_end as usize { assert!(addr < code.bytes.len()); @@ -226,14 +233,8 @@ impl ExecutionGadget for CopyCodeToMemoryGadget { }; } - self.buffer_reader.assign( - region, - offset, - *src_addr, - *src_addr_end, - &bytes, - &new_selectors, - )?; + self.buffer_reader + .assign(region, offset, *src_addr, *src_addr_end, &bytes, &selectors)?; // The number of bytes copied here will be the sum of 1s over the selector // vector. @@ -256,7 +257,7 @@ pub(crate) mod test { use super::MAX_COPY_BYTES; use std::collections::HashMap; - use bus_mapping::evm::OpcodeId; + use bus_mapping::{circuit_input_builder::StepAuxiliaryData, evm::OpcodeId}; use eth_types::{bytecode, Word}; use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr; @@ -265,9 +266,7 @@ pub(crate) mod test { step::ExecutionState, table::RwTableTag, test::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}, }; #[allow(clippy::too_many_arguments)] @@ -283,7 +282,7 @@ pub(crate) mod test { rw_counter: usize, rws: &mut RwMap, bytes_map: &HashMap, - code: &Bytecode, + code: ð_types::Bytecode, ) -> (ExecStep, usize) { let mut rw_offset = 0usize; let memory_rws: &mut Vec<_> = rws.0.entry(RwTableTag::Memory).or_insert_with(Vec::new); @@ -335,7 +334,7 @@ pub(crate) mod test { #[allow(clippy::too_many_arguments)] pub(crate) fn make_copy_code_steps( call_id: usize, - code: &Bytecode, + code: ð_types::Bytecode, src_addr: u64, dst_addr: u64, length: usize, @@ -346,8 +345,8 @@ pub(crate) mod test { rws: &mut RwMap, steps: &mut Vec, ) { - let bytes_map = (0..(code.bytes.len() as u64)) - .zip(code.bytes.iter().copied()) + let bytes_map = (0..(code.code().len() as u64)) + .zip(code.code().iter().copied()) .collect(); let mut copied = 0; @@ -356,7 +355,7 @@ pub(crate) mod test { call_id, src_addr + copied as u64, dst_addr + copied as u64, - code.bytes.len() as u64, + code.code().len() as u64, length - copied, program_counter, stack_pointer, @@ -394,7 +393,6 @@ pub(crate) mod test { PUSH32(Word::from(0x1928835)) POP }; - let code = Bytecode::new(code.to_vec()); let dummy_code = Bytecode::new(vec![OpcodeId::STOP.as_u8()]); @@ -424,6 +422,7 @@ pub(crate) mod test { ..Default::default() }); + let code = Bytecode::new(code.to_vec()); let block = Block { randomness, txs: vec![Transaction { diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index c4d8f53ec5..cc17f59101 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -179,13 +179,7 @@ impl ExecutionGadget for CopyToMemoryGadget { .as_ref() .expect("could not find aux_data for COPYTOMEMORY") { - ( - src_addr, - dst_addr, - bytes_left, - src_addr_end, - from_tx, - ) + (src_addr, dst_addr, bytes_left, src_addr_end, from_tx) } else { panic!("could not find CopyToMemory aux_data for COPYTOMEMORY"); }; From 6856dfc7690f58c645bf8c9c7d8278f62d5e41fe Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Thu, 24 Mar 2022 11:04:44 +0800 Subject: [PATCH 09/12] feat(eth-types): try opcodeId from u8 --- eth-types/src/evm_types/opcode_ids.rs | 153 ++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/eth-types/src/evm_types/opcode_ids.rs b/eth-types/src/evm_types/opcode_ids.rs index 9c85f0d1b0..4e1c4cbcfe 100644 --- a/eth-types/src/evm_types/opcode_ids.rs +++ b/eth-types/src/evm_types/opcode_ids.rs @@ -635,6 +635,159 @@ impl OpcodeId { } } +impl TryFrom for OpcodeId { + type Error = Error; + + fn try_from(value: u8) -> Result { + Ok(match value { + 0x00u8 => OpcodeId::STOP, + 0x01u8 => OpcodeId::ADD, + 0x02u8 => OpcodeId::MUL, + 0x03u8 => OpcodeId::SUB, + 0x04u8 => OpcodeId::DIV, + 0x05u8 => OpcodeId::SDIV, + 0x06u8 => OpcodeId::MOD, + 0x07u8 => OpcodeId::SMOD, + 0x08u8 => OpcodeId::ADDMOD, + 0x09u8 => OpcodeId::MULMOD, + 0x0au8 => OpcodeId::EXP, + 0x0bu8 => OpcodeId::SIGNEXTEND, + 0x10u8 => OpcodeId::LT, + 0x11u8 => OpcodeId::GT, + 0x12u8 => OpcodeId::SLT, + 0x13u8 => OpcodeId::SGT, + 0x14u8 => OpcodeId::EQ, + 0x15u8 => OpcodeId::ISZERO, + 0x16u8 => OpcodeId::AND, + 0x17u8 => OpcodeId::OR, + 0x18u8 => OpcodeId::XOR, + 0x19u8 => OpcodeId::NOT, + 0x1au8 => OpcodeId::BYTE, + 0x35u8 => OpcodeId::CALLDATALOAD, + 0x36u8 => OpcodeId::CALLDATASIZE, + 0x37u8 => OpcodeId::CALLDATACOPY, + 0x38u8 => OpcodeId::CODESIZE, + 0x39u8 => OpcodeId::CODECOPY, + 0x1bu8 => OpcodeId::SHL, + 0x1cu8 => OpcodeId::SHR, + 0x1du8 => OpcodeId::SAR, + 0x50u8 => OpcodeId::POP, + 0x51u8 => OpcodeId::MLOAD, + 0x52u8 => OpcodeId::MSTORE, + 0x53u8 => OpcodeId::MSTORE8, + 0x56u8 => OpcodeId::JUMP, + 0x57u8 => OpcodeId::JUMPI, + 0x58u8 => OpcodeId::PC, + 0x59u8 => OpcodeId::MSIZE, + 0x5bu8 => OpcodeId::JUMPDEST, + 0x60u8 => OpcodeId::PUSH1, + 0x61u8 => OpcodeId::PUSH2, + 0x62u8 => OpcodeId::PUSH3, + 0x63u8 => OpcodeId::PUSH4, + 0x64u8 => OpcodeId::PUSH5, + 0x65u8 => OpcodeId::PUSH6, + 0x66u8 => OpcodeId::PUSH7, + 0x67u8 => OpcodeId::PUSH8, + 0x68u8 => OpcodeId::PUSH9, + 0x69u8 => OpcodeId::PUSH10, + 0x6au8 => OpcodeId::PUSH11, + 0x6bu8 => OpcodeId::PUSH12, + 0x6cu8 => OpcodeId::PUSH13, + 0x6du8 => OpcodeId::PUSH14, + 0x6eu8 => OpcodeId::PUSH15, + 0x6fu8 => OpcodeId::PUSH16, + 0x70u8 => OpcodeId::PUSH17, + 0x71u8 => OpcodeId::PUSH18, + 0x72u8 => OpcodeId::PUSH19, + 0x73u8 => OpcodeId::PUSH20, + 0x74u8 => OpcodeId::PUSH21, + 0x75u8 => OpcodeId::PUSH22, + 0x76u8 => OpcodeId::PUSH23, + 0x77u8 => OpcodeId::PUSH24, + 0x78u8 => OpcodeId::PUSH25, + 0x79u8 => OpcodeId::PUSH26, + 0x7au8 => OpcodeId::PUSH27, + 0x7bu8 => OpcodeId::PUSH28, + 0x7cu8 => OpcodeId::PUSH29, + 0x7du8 => OpcodeId::PUSH30, + 0x7eu8 => OpcodeId::PUSH31, + 0x7fu8 => OpcodeId::PUSH32, + 0x80u8 => OpcodeId::DUP1, + 0x81u8 => OpcodeId::DUP2, + 0x82u8 => OpcodeId::DUP3, + 0x83u8 => OpcodeId::DUP4, + 0x84u8 => OpcodeId::DUP5, + 0x85u8 => OpcodeId::DUP6, + 0x86u8 => OpcodeId::DUP7, + 0x87u8 => OpcodeId::DUP8, + 0x88u8 => OpcodeId::DUP9, + 0x89u8 => OpcodeId::DUP10, + 0x8au8 => OpcodeId::DUP11, + 0x8bu8 => OpcodeId::DUP12, + 0x8cu8 => OpcodeId::DUP13, + 0x8du8 => OpcodeId::DUP14, + 0x8eu8 => OpcodeId::DUP15, + 0x8fu8 => OpcodeId::DUP16, + 0x90u8 => OpcodeId::SWAP1, + 0x91u8 => OpcodeId::SWAP2, + 0x92u8 => OpcodeId::SWAP3, + 0x93u8 => OpcodeId::SWAP4, + 0x94u8 => OpcodeId::SWAP5, + 0x95u8 => OpcodeId::SWAP6, + 0x96u8 => OpcodeId::SWAP7, + 0x97u8 => OpcodeId::SWAP8, + 0x98u8 => OpcodeId::SWAP9, + 0x99u8 => OpcodeId::SWAP10, + 0x9au8 => OpcodeId::SWAP11, + 0x9bu8 => OpcodeId::SWAP12, + 0x9cu8 => OpcodeId::SWAP13, + 0x9du8 => OpcodeId::SWAP14, + 0x9eu8 => OpcodeId::SWAP15, + 0x9fu8 => OpcodeId::SWAP16, + 0xf3u8 => OpcodeId::RETURN, + 0xfdu8 => OpcodeId::REVERT, + 0xfeu8 => OpcodeId::INVALID(value), + 0x20u8 => OpcodeId::SHA3, + 0x30u8 => OpcodeId::ADDRESS, + 0x31u8 => OpcodeId::BALANCE, + 0x32u8 => OpcodeId::ORIGIN, + 0x33u8 => OpcodeId::CALLER, + 0x34u8 => OpcodeId::CALLVALUE, + 0x3au8 => OpcodeId::GASPRICE, + 0x3bu8 => OpcodeId::EXTCODESIZE, + 0x3cu8 => OpcodeId::EXTCODECOPY, + 0x3fu8 => OpcodeId::EXTCODEHASH, + 0x3du8 => OpcodeId::RETURNDATASIZE, + 0x3eu8 => OpcodeId::RETURNDATACOPY, + 0x40u8 => OpcodeId::BLOCKHASH, + 0x41u8 => OpcodeId::COINBASE, + 0x42u8 => OpcodeId::TIMESTAMP, + 0x43u8 => OpcodeId::NUMBER, + 0x44u8 => OpcodeId::DIFFICULTY, + 0x45u8 => OpcodeId::GASLIMIT, + 0x46u8 => OpcodeId::CHAINID, + 0x47u8 => OpcodeId::SELFBALANCE, + 0x48u8 => OpcodeId::BASEFEE, + 0x54u8 => OpcodeId::SLOAD, + 0x55u8 => OpcodeId::SSTORE, + 0x5au8 => OpcodeId::GAS, + 0xa0u8 => OpcodeId::LOG0, + 0xa1u8 => OpcodeId::LOG1, + 0xa2u8 => OpcodeId::LOG2, + 0xa3u8 => OpcodeId::LOG3, + 0xa4u8 => OpcodeId::LOG4, + 0xf0u8 => OpcodeId::CREATE, + 0xf5u8 => OpcodeId::CREATE2, + 0xf1u8 => OpcodeId::CALL, + 0xf2u8 => OpcodeId::CALLCODE, + 0xf4u8 => OpcodeId::DELEGATECALL, + 0xfau8 => OpcodeId::STATICCALL, + 0xffu8 => OpcodeId::SELFDESTRUCT, + _ => return Err(Error::OpcodeParsing), + }) + } +} + impl FromStr for OpcodeId { type Err = Error; From 2cb9c5458fe5041557d78236ac6d6bdd2175a32e Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Thu, 24 Mar 2022 11:05:16 +0800 Subject: [PATCH 10/12] feat(eth-types): try bytecode from vec --- eth-types/src/bytecode.rs | 65 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/eth-types/src/bytecode.rs b/eth-types/src/bytecode.rs index 7d06d1cb8d..1481a0150d 100644 --- a/eth-types/src/bytecode.rs +++ b/eth-types/src/bytecode.rs @@ -5,13 +5,22 @@ use crate::Word; use std::collections::HashMap; /// EVM Bytecode -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, Clone, PartialEq)] pub struct Bytecode { code: Vec, num_opcodes: usize, markers: HashMap, } +/// Error while constructing `Bytecode` from raw bytes. +#[derive(Debug)] +pub enum BytecodeError { + /// Invalid byte that is not reserved for any known opcode. + InvalidByte(u8), + /// Insufficient number of bytes following a PUSH instruction. + InsufficientPush, +} + impl Bytecode { /// Get a reference to the generated code pub fn code(&self) -> &[u8] { @@ -128,6 +137,37 @@ impl Bytecode { } } +impl TryFrom> for Bytecode { + type Error = BytecodeError; + + fn try_from(input: Vec) -> Result { + let mut code = Bytecode::default(); + + let mut input_iter = input.iter(); + while let Some(byte) = input_iter.next() { + if let Ok(op) = OpcodeId::try_from(*byte) { + if op.is_push() { + let n = (op.as_u8() - OpcodeId::PUSH1.as_u8() + 1) as usize; + let mut value = vec![0u8; n]; + for value_byte in value.iter_mut() { + *value_byte = input_iter + .next() + .cloned() + .ok_or(BytecodeError::InsufficientPush)?; + } + code.push(n, Word::from(value.as_slice())); + } else { + code.write_op(op); + } + } else { + return Err(BytecodeError::InvalidByte(*byte)); + } + } + + Ok(code) + } +} + /// EVM code macro #[macro_export] macro_rules! bytecode { @@ -169,3 +209,26 @@ macro_rules! bytecode_internal { $crate::bytecode_internal!($code, $($rest)*); }}; } + +#[cfg(test)] +mod tests { + use crate::Bytecode; + + #[test] + fn test_bytecode_roundtrip() { + let code = bytecode! { + PUSH8(0x123) + POP + PUSH24(0x321) + PUSH32(0x432) + MUL + CALLVALUE + CALLER + POP + POP + POP + STOP + }; + assert_eq!(Bytecode::try_from(code.to_vec()).unwrap(), code); + } +} From d56d581fcf38e7ae0849f7fc972a7d487b156b53 Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Thu, 24 Mar 2022 11:07:10 +0800 Subject: [PATCH 11/12] feat(bus-mapping): op codecopy and tests --- bus-mapping/src/evm/opcodes.rs | 4 +- bus-mapping/src/evm/opcodes/codecopy.rs | 210 ++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 bus-mapping/src/evm/opcodes/codecopy.rs diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index e85d58ac3e..287d57123a 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -19,6 +19,7 @@ mod calldatacopy; mod calldatasize; mod caller; mod callvalue; +mod codecopy; mod dup; mod mload; mod mstore; @@ -32,6 +33,7 @@ use calldatacopy::Calldatacopy; use calldatasize::Calldatasize; use caller::Caller; use callvalue::Callvalue; +use codecopy::Codecopy; use dup::Dup; use mload::Mload; use mstore::Mstore; @@ -106,7 +108,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { OpcodeId::CALLDATALOAD => StackOnlyOpcode::<1, 1>::gen_associated_ops, OpcodeId::CALLDATACOPY => Calldatacopy::gen_associated_ops, // OpcodeId::CODESIZE => {}, - // OpcodeId::CODECOPY => {}, + OpcodeId::CODECOPY => Codecopy::gen_associated_ops, // OpcodeId::GASPRICE => {}, // OpcodeId::EXTCODESIZE => {}, // OpcodeId::EXTCODECOPY => {}, diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs new file mode 100644 index 0000000000..439d21f412 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -0,0 +1,210 @@ +use crate::{ + circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}, + operation::RW, + Error, +}; +use eth_types::{Bytecode, GethExecStep}; + +use super::Opcode; + +const MAX_COPY_BYTES: usize = 54; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct Codecopy; + +impl Opcode for Codecopy { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_steps = vec![gen_codecopy_step(state, geth_step)?]; + let memory_copy_steps = gen_memory_copy_steps(state, geth_steps)?; + exec_steps.extend(memory_copy_steps); + Ok(exec_steps) + } +} + +fn gen_codecopy_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 code_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; + + // stack reads + 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), + code_offset, + )?; + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(2), + length, + )?; + Ok(exec_step) +} + +fn gen_memory_copy_step( + state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, + src_addr: usize, + dst_addr: usize, + src_addr_end: usize, + bytes_left: usize, + code: eth_types::Bytecode, +) -> Result<(), Error> { + for idx in 0..std::cmp::min(bytes_left, MAX_COPY_BYTES) { + let addr = src_addr + idx; + let byte = if addr < src_addr_end { + code.code()[addr] + } else { + 0 + }; + state.push_memory_op(exec_step, RW::WRITE, (dst_addr + idx).into(), byte)?; + } + + exec_step.aux_data = Some(StepAuxiliaryData::CopyCodeToMemory { + src_addr: src_addr as u64, + dst_addr: dst_addr as u64, + bytes_left: bytes_left as u64, + src_addr_end: src_addr_end as u64, + code, + }); + + Ok(()) +} + +fn gen_memory_copy_steps( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], +) -> Result, Error> { + let memory_offset = geth_steps[0].stack.nth_last(0)?.as_usize(); + let code_offset = geth_steps[0].stack.nth_last(1)?.as_usize(); + let length = geth_steps[0].stack.nth_last(2)?.as_usize(); + + let code_hash = state.call()?.code_hash; + let code = state.code_db.0.get(&code_hash).unwrap(); + let code = Bytecode::try_from(code.clone()).unwrap(); + let src_addr_end = code.code().len(); + + let mut copied = 0; + let mut steps = vec![]; + while copied < length { + let mut exec_step = state.new_step(&geth_steps[1])?; + exec_step.exec_state = ExecState::CopyToMemory; + gen_memory_copy_step( + state, + &mut exec_step, + code_offset + copied, + memory_offset + copied, + src_addr_end, + length - copied, + code.clone(), + )?; + steps.push(exec_step); + copied += MAX_COPY_BYTES; + } + + Ok(steps) +} + +#[cfg(test)] +mod codecopy_tests { + use eth_types::{ + bytecode, + evm_types::{MemoryAddress, OpcodeId, StackAddress}, + Word, + }; + use mock::new_single_tx_trace_code; + + use crate::{ + mock::BlockData, + operation::{MemoryOp, StackOp}, + }; + + use super::*; + + #[test] + fn codecopy_opcode_impl() { + test_ok(0x00, 0x00, 0x40); + test_ok(0x20, 0x40, 0xA0); + } + + fn test_ok(memory_offset: usize, code_offset: usize, size: usize) { + let code = bytecode! { + PUSH32(size) + PUSH32(code_offset) + PUSH32(memory_offset) + CODECOPY + STOP + }; + + let block = BlockData::new_from_geth_data(new_single_tx_trace_code(&code).unwrap()); + + let mut builder = block.new_circuit_input_builder(); + builder + .handle_block(&block.eth_block, &block.geth_traces) + .unwrap(); + + let step = builder.block.txs()[0] + .steps() + .iter() + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CODECOPY)) + .unwrap(); + + assert_eq!( + [0, 1, 2] + .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|op| (op.rw(), op.op())), + [ + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1021), Word::from(memory_offset)), + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1022), Word::from(code_offset)), + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1023), Word::from(size)), + ), + ] + ); + assert_eq!( + (0..size) + .map(|idx| &builder.block.container.memory[idx]) + .map(|op| (op.rw(), op.op().clone())) + .collect::>(), + (0..size) + .map(|idx| { + ( + RW::WRITE, + MemoryOp::new( + 1, + MemoryAddress::from(memory_offset + idx), + if code_offset + idx < code.code().len() { + code.code()[code_offset + idx] + } else { + 0 + }, + ), + ) + }) + .collect::>(), + ); + } +} From 978b8d0b4d3b5e495d78b0475b7acff02c192b5b Mon Sep 17 00:00:00 2001 From: Rohit Narurkar Date: Thu, 24 Mar 2022 11:23:19 +0800 Subject: [PATCH 12/12] tests(circuits): generate witness data through busmapping for codecopy --- bus-mapping/src/circuit_input_builder.rs | 2 ++ bus-mapping/src/evm/opcodes/codecopy.rs | 2 +- .../src/evm_circuit/execution/codecopy.rs | 33 +++++++++++++++---- zkevm-circuits/src/evm_circuit/witness.rs | 2 ++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index d4e228c7cd..e641c8fd09 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -112,6 +112,8 @@ pub enum ExecState { EndTx, /// Virtual step Copy To Memory CopyToMemory, + /// Virtal step Copy Code To Memory + CopyCodeToMemory, } impl ExecState { diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index 439d21f412..b7e610d7c3 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -104,7 +104,7 @@ fn gen_memory_copy_steps( let mut steps = vec![]; while copied < length { let mut exec_step = state.new_step(&geth_steps[1])?; - exec_step.exec_state = ExecState::CopyToMemory; + exec_step.exec_state = ExecState::CopyCodeToMemory; gen_memory_copy_step( state, &mut exec_step, diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index ea4d1ce0e2..1c94d06491 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -215,12 +215,15 @@ mod tests { use num::Zero; use pairing::bn256::Fr; - use crate::evm_circuit::{ - execution::copy_code_to_memory::test::make_copy_code_steps, - step::ExecutionState, - table::RwTableTag, - test::{calc_memory_copier_gas_cost, run_test_circuit_incomplete_fixed_table}, - witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, + use crate::{ + evm_circuit::{ + execution::copy_code_to_memory::test::make_copy_code_steps, + step::ExecutionState, + table::RwTableTag, + test::{calc_memory_copier_gas_cost, run_test_circuit_incomplete_fixed_table}, + witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, + }, + test_util::run_test_circuits, }; fn test_ok(src_addr: u64, dst_addr: u64, size: usize) { @@ -420,4 +423,22 @@ mod tests { fn codecopy_gadget_oob() { test_ok(0x10, 0x20, 200); } + + fn test_busmapping(memory_offset: usize, code_offset: usize, size: usize) { + let code = bytecode! { + PUSH32(Word::from(size)) + PUSH32(Word::from(code_offset)) + PUSH32(Word::from(memory_offset)) + CODECOPY + STOP + }; + assert!(run_test_circuits(code).is_ok()); + } + + #[test] + fn codecopy_gadget_busmapping() { + test_busmapping(0x00, 0x00, 54); + test_busmapping(0x20, 0x30, 0x20); + test_busmapping(0x10, 0x20, 0x40); + } } diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index 7a31ce68bd..9c7fe5895f 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -1117,12 +1117,14 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { OpcodeId::SLOAD => ExecutionState::SLOAD, OpcodeId::SSTORE => ExecutionState::SSTORE, OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, + OpcodeId::CODECOPY => ExecutionState::CODECOPY, _ => 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::CopyCodeToMemory => ExecutionState::CopyCodeToMemory, } } }