diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 22eda3249c..d57b698904 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -20,7 +20,7 @@ use eth_types::{ evm_types::{ gas_utils::memory_expansion_gas_cost, Gas, GasCost, MemoryAddress, OpcodeId, StackAddress, }, - Address, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256, + Address, Bytecode, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256, }; use ethers_core::utils::{get_contract_address, get_create2_address}; use std::cmp::max; @@ -1348,4 +1348,63 @@ impl<'a> CircuitInputStateRef<'a> { } Ok(()) } + + /// Generate copy steps for bytecode. + pub(crate) fn gen_copy_steps_for_bytecode( + &mut self, + exec_step: &mut ExecStep, + bytecode: &Bytecode, + src_addr: u64, + dst_addr: u64, + src_addr_end: u64, + bytes_left: u64, + ) -> Result, Error> { + let mut copy_steps = Vec::with_capacity(bytes_left as usize); + for idx in 0..bytes_left { + let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end); + let step = if addr < src_addr_end { + let code = bytecode.code.get(addr as usize).unwrap(); + (code.value, code.is_code) + } else { + (0, false) + }; + copy_steps.push(step); + self.memory_write(exec_step, (dst_addr + idx).into(), step.0)?; + } + + Ok(copy_steps) + } + + /// Generate copy steps for call data. + pub(crate) fn gen_copy_steps_for_call_data( + &mut self, + exec_step: &mut ExecStep, + src_addr: u64, + dst_addr: u64, + src_addr_end: u64, + bytes_left: u64, + ) -> Result, Error> { + let mut copy_steps = Vec::with_capacity(bytes_left as usize); + for idx in 0..bytes_left { + let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end); + let value = if addr < src_addr_end { + let byte = + self.call_ctx()?.call_data[(addr - self.call()?.call_data_offset) as usize]; + if !self.call()?.is_root { + self.push_op( + exec_step, + RW::READ, + MemoryOp::new(self.call()?.caller_id, addr.into(), byte), + ); + } + byte + } else { + 0 + }; + copy_steps.push((value, false)); + self.memory_write(exec_step, (dst_addr + idx).into(), value)?; + } + + Ok(copy_steps) + } } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 0500131d93..238b17c993 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -3,7 +3,7 @@ use crate::{ circuit_input_builder::{ CircuitInputStateRef, CopyDataType, CopyEvent, ExecStep, NumberOrHash, }, - operation::{CallContextField, MemoryOp, RW}, + operation::CallContextField, Error, }; use eth_types::GethExecStep; @@ -20,13 +20,13 @@ impl Opcode for Calldatacopy { let mut exec_steps = vec![gen_calldatacopy_step(state, geth_step)?]; // reconstruction - let memory_offset = geth_step.stack.nth_last(0)?.as_u64(); - let data_offset = geth_step.stack.nth_last(1)?.as_u64(); - let length = geth_step.stack.nth_last(2)?.as_usize(); + let memory_offset = geth_step.stack.nth_last(0)?; + let data_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - memory.copy_from(memory_offset, &call_ctx.call_data, data_offset, length); + memory.copy_from(memory_offset, data_offset, length, &call_ctx.call_data); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(&mut exec_steps[0], copy_event); @@ -92,64 +92,36 @@ fn gen_calldatacopy_step( Ok(exec_step) } -fn gen_copy_steps( - state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - src_addr: u64, - dst_addr: u64, - src_addr_end: u64, - bytes_left: u64, - is_root: bool, -) -> Result, Error> { - let mut copy_steps = Vec::with_capacity(bytes_left as usize); - for idx in 0..bytes_left { - let addr = src_addr + idx; - let value = if addr < src_addr_end { - let byte = - state.call_ctx()?.call_data[(addr - state.call()?.call_data_offset) as usize]; - if !is_root { - state.push_op( - exec_step, - RW::READ, - MemoryOp::new(state.call()?.caller_id, addr.into(), byte), - ); - } - byte - } else { - 0 - }; - copy_steps.push((value, false)); - state.memory_write(exec_step, (dst_addr + idx).into(), value)?; - } - - Ok(copy_steps) -} - fn gen_copy_event( state: &mut CircuitInputStateRef, geth_step: &GethExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; - let memory_offset = geth_step.stack.nth_last(0)?.as_u64(); - let data_offset = geth_step.stack.nth_last(1)?.as_u64(); + + let memory_offset = geth_step.stack.nth_last(0)?; + let data_offset = geth_step.stack.nth_last(1)?; let length = geth_step.stack.nth_last(2)?.as_u64(); let call_data_offset = state.call()?.call_data_offset; let call_data_length = state.call()?.call_data_length; - let (src_addr, src_addr_end) = ( - call_data_offset + data_offset, - call_data_offset + call_data_length, - ); + + let dst_addr = memory_offset.as_u64(); + let src_addr_end = call_data_offset.checked_add(call_data_length).unwrap(); + + // Reset start offset to end offset if overflow. + let src_addr = u64::try_from(data_offset) + .ok() + .and_then(|offset| offset.checked_add(call_data_offset)) + .unwrap_or(src_addr_end) + .min(src_addr_end); let mut exec_step = state.new_step(geth_step)?; - let copy_steps = gen_copy_steps( - state, + let copy_steps = state.gen_copy_steps_for_call_data( &mut exec_step, src_addr, - memory_offset, + dst_addr, src_addr_end, length, - state.call()?.is_root, )?; let (src_type, src_id) = if state.call()?.is_root { @@ -165,7 +137,7 @@ fn gen_copy_event( src_addr_end, dst_type: CopyDataType::Memory, dst_id: NumberOrHash::Number(state.call()?.call_id), - dst_addr: memory_offset, + dst_addr, log_id: None, rw_counter_start, bytes: copy_steps, diff --git a/bus-mapping/src/evm/opcodes/calldataload.rs b/bus-mapping/src/evm/opcodes/calldataload.rs index c75336b51b..b28ae5d07c 100644 --- a/bus-mapping/src/evm/opcodes/calldataload.rs +++ b/bus-mapping/src/evm/opcodes/calldataload.rs @@ -23,73 +23,79 @@ impl Opcode for Calldataload { let offset = geth_step.stack.nth_last(0)?; state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(0), offset)?; - let is_root = state.call()?.is_root; - if is_root { - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::TxId, - state.tx_ctx.id().into(), + // Check if offset is Uint64 overflow. + let calldata_word = if let Ok(offset) = u64::try_from(offset) { + let is_root = state.call()?.is_root; + let call_id = state.call()?.call_id; + if is_root { + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::TxId, + state.tx_ctx.id().into(), + ); + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::CallDataLength, + state.call()?.call_data_length.into(), + ); + } else { + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::CallerId, + state.call()?.caller_id.into(), + ); + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::CallDataLength, + state.call()?.call_data_length.into(), + ); + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::CallDataOffset, + state.call()?.call_data_offset.into(), + ); + } + + let call_data_offset = state.call()?.call_data_offset; + let call_data_length = state.call()?.call_data_length; + let (src_addr, src_addr_end, caller_id, call_data) = ( + call_data_offset + offset.min(call_data_length), + call_data_offset + call_data_length, + state.call()?.caller_id, + state.call_ctx()?.call_data.to_vec(), ); - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataLength, - state.call()?.call_data_length.into(), - ); - } else { - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallerId, - state.call()?.caller_id.into(), - ); - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataLength, - state.call()?.call_data_length.into(), - ); - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataOffset, - state.call()?.call_data_offset.into(), - ); - } - let call = state.call()?.clone(); - let (src_addr, src_addr_end, caller_id, call_data) = ( - call.call_data_offset as usize + offset.as_usize(), - call.call_data_offset as usize + call.call_data_length as usize, - call.caller_id, - state.call_ctx()?.call_data.to_vec(), - ); - let calldata_word = (0..32) - .map(|idx| { - let addr = src_addr + idx; - if addr < src_addr_end { - let byte = call_data[addr - call.call_data_offset as usize]; - if !is_root { - // caller id as call_id - state.push_op( - &mut exec_step, - RW::READ, - MemoryOp::new(caller_id, (src_addr + idx).into(), byte), - ); + let calldata: Vec<_> = (0..32) + .map(|idx| { + let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end); + if addr < src_addr_end { + let byte = call_data[(addr - call_data_offset) as usize]; + if !is_root { + state.push_op( + &mut exec_step, + RW::READ, + MemoryOp::new(caller_id, (src_addr + idx).into(), byte), + ); + } + byte + } else { + 0 } - byte - } else { - 0 - } - }) - .collect::>(); + }) + .collect(); + + U256::from_big_endian(&calldata) + } else { + // Stack push `0` as result if overflow. + U256::zero() + }; - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled(), - U256::from_big_endian(&calldata_word), - )?; + state.stack_write(&mut exec_step, geth_step.stack.last_filled(), calldata_word)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index cf7364d6db..578f1c733f 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -20,10 +20,9 @@ impl Opcode for Codecopy { let mut exec_steps = vec![gen_codecopy_step(state, geth_step)?]; // reconstruction - - let dest_offset = geth_step.stack.nth_last(0)?.as_u64(); - let code_offset = geth_step.stack.nth_last(1)?.as_u64(); - let length = geth_step.stack.nth_last(2)?.as_u64(); + let dst_offset = geth_step.stack.nth_last(0)?; + let code_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; let code_hash = state.call()?.code_hash; let code = state.code(code_hash)?; @@ -31,7 +30,7 @@ impl Opcode for Codecopy { let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - memory.copy_from(dest_offset, &code, code_offset, length as usize); + memory.copy_from(dst_offset, code_offset, length, &code); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(&mut exec_steps[0], copy_event); @@ -65,56 +64,46 @@ fn gen_codecopy_step( Ok(exec_step) } -fn gen_copy_steps( - state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - src_addr: u64, - dst_addr: u64, - bytes_left: u64, - bytecode: &Bytecode, -) -> Result, Error> { - let mut steps = Vec::with_capacity(bytes_left as usize); - for idx in 0..bytes_left { - let addr = src_addr + idx; - let bytecode_element = bytecode.get(addr as usize).unwrap_or_default(); - steps.push((bytecode_element.value, bytecode_element.is_code)); - state.memory_write(exec_step, (dst_addr + idx).into(), bytecode_element.value)?; - } - Ok(steps) -} - fn gen_copy_event( state: &mut CircuitInputStateRef, geth_step: &GethExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; - let dst_offset = geth_step.stack.nth_last(0)?.as_u64(); - let code_offset = geth_step.stack.nth_last(1)?.as_u64(); + let dst_offset = geth_step.stack.nth_last(0)?; + let code_offset = geth_step.stack.nth_last(1)?; let length = geth_step.stack.nth_last(2)?.as_u64(); let code_hash = state.call()?.code_hash; let bytecode: Bytecode = state.code(code_hash)?.into(); - let src_addr_end = bytecode.to_vec().len() as u64; + let code_size = bytecode.code.len() as u64; + + let dst_addr = dst_offset.as_u64(); + let src_addr_end = code_size; + + // Reset start offset to end offset if overflow. + let src_addr = u64::try_from(code_offset) + .unwrap_or(src_addr_end) + .min(src_addr_end); let mut exec_step = state.new_step(geth_step)?; - let copy_steps = gen_copy_steps( - state, + let copy_steps = state.gen_copy_steps_for_bytecode( &mut exec_step, - code_offset, - dst_offset, - length, &bytecode, + src_addr, + dst_addr, + src_addr_end, + length, )?; Ok(CopyEvent { src_type: CopyDataType::Bytecode, src_id: NumberOrHash::Hash(code_hash), - src_addr: code_offset, + src_addr, src_addr_end, dst_type: CopyDataType::Memory, dst_id: NumberOrHash::Number(state.call()?.call_id), - dst_addr: dst_offset, + dst_addr, log_id: None, rw_counter_start, bytes: copy_steps, diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs index 2beaf34f12..8160bc9455 100644 --- a/bus-mapping/src/evm/opcodes/extcodecopy.rs +++ b/bus-mapping/src/evm/opcodes/extcodecopy.rs @@ -24,9 +24,9 @@ impl Opcode for Extcodecopy { // reconstruction let address = geth_steps[0].stack.nth_last(0)?.to_address(); - let dest_offset = geth_steps[0].stack.nth_last(1)?.as_u64(); - let code_offset = geth_steps[0].stack.nth_last(2)?.as_u64(); - let length = geth_steps[0].stack.nth_last(3)?.as_u64(); + let dst_offset = geth_steps[0].stack.nth_last(1)?; + let code_offset = geth_step.stack.nth_last(2)?; + let length = geth_steps[0].stack.nth_last(3)?; let (_, account) = state.sdb.get_account(&address); let code_hash = account.code_hash; @@ -35,7 +35,7 @@ impl Opcode for Extcodecopy { let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - memory.copy_from(dest_offset, &code, code_offset, length as usize); + memory.copy_from(dst_offset, code_offset, length, &code); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(&mut exec_steps[0], copy_event); @@ -111,39 +111,15 @@ fn gen_extcodecopy_step( Ok(exec_step) } -fn gen_copy_steps( - state: &mut CircuitInputStateRef, - exec_step: &mut ExecStep, - src_addr: u64, - dst_addr: u64, - src_addr_end: u64, - bytes_left: u64, - code: &Bytecode, -) -> Result, Error> { - let mut copy_steps = Vec::with_capacity(bytes_left as usize); - for idx in 0..bytes_left { - let addr = src_addr + idx; - let step = if addr < src_addr_end { - let code = code.code.get(addr as usize).unwrap(); - (code.value, code.is_code) - } else { - (0, false) - }; - copy_steps.push(step); - state.memory_write(exec_step, (dst_addr + idx).into(), step.0)?; - } - - Ok(copy_steps) -} - fn gen_copy_event( state: &mut CircuitInputStateRef, geth_step: &GethExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; + let external_address = geth_step.stack.nth_last(0)?.to_address(); - let memory_offset = geth_step.stack.nth_last(1)?.as_u64(); - let data_offset = geth_step.stack.nth_last(2)?.as_u64(); + let dst_offset = geth_step.stack.nth_last(1)?; + let code_offset = geth_step.stack.nth_last(2)?; let length = geth_step.stack.nth_last(3)?.as_u64(); let account = state.sdb.get_account(&external_address).1; @@ -154,28 +130,37 @@ fn gen_copy_event( H256::zero() }; - let code: Bytecode = if exists { + let bytecode: Bytecode = if exists { state.code(code_hash)?.into() } else { Bytecode::default() }; - let src_addr_end = code.code.len() as u64; + let code_size = bytecode.code.len() as u64; + + let dst_addr = dst_offset.as_u64(); + let src_addr_end = code_size; + + // Reset start offset to end offset if overflow. + let src_addr = u64::try_from(code_offset) + .unwrap_or(src_addr_end) + .min(src_addr_end); + let mut exec_step = state.new_step(geth_step)?; - let copy_steps = gen_copy_steps( - state, + let copy_steps = state.gen_copy_steps_for_bytecode( &mut exec_step, - data_offset, - memory_offset, + &bytecode, + src_addr, + dst_addr, src_addr_end, length, - &code, )?; + Ok(CopyEvent { - src_addr: data_offset, + src_addr, src_addr_end, src_type: CopyDataType::Bytecode, src_id: NumberOrHash::Hash(code_hash), - dst_addr: memory_offset, + dst_addr, dst_type: CopyDataType::Memory, dst_id: NumberOrHash::Number(state.call()?.call_id), log_id: None, diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs index a0e9be1ea1..f1a14a4b16 100644 --- a/bus-mapping/src/evm/opcodes/returndatacopy.rs +++ b/bus-mapping/src/evm/opcodes/returndatacopy.rs @@ -21,17 +21,16 @@ impl Opcode for Returndatacopy { // reconstruction let geth_step = &geth_steps[0]; - let dest_offset = geth_step.stack.nth_last(0)?; - let offset = geth_step.stack.nth_last(1)?; - let size = geth_step.stack.nth_last(2)?; + let dst_offset = geth_step.stack.nth_last(0)?; + let src_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; // can we reduce this clone? let return_data = state.call_ctx()?.return_data.clone(); let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - let length = size.as_usize(); - memory.copy_from(dest_offset.as_u64(), &return_data, offset.as_u64(), length); + memory.copy_from(dst_offset, src_offset, length, &return_data); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(&mut exec_steps[0], copy_event); diff --git a/eth-types/src/evm_types/memory.rs b/eth-types/src/evm_types/memory.rs index e782475de2..ab0e7e5eef 100644 --- a/eth-types/src/evm_types/memory.rs +++ b/eth-types/src/evm_types/memory.rs @@ -329,9 +329,21 @@ impl Memory { } /// Copy source data to memory. Used in (ext)codecopy/calldatacopy. - pub fn copy_from(&mut self, dst_offset: u64, data: &[u8], data_offset: u64, length: usize) { - // https://github.com/ethereum/go-ethereum/blob/df52967ff6080a27243569020ff64cd956fb8362/core/vm/instructions.go#L312 + pub fn copy_from(&mut self, dst_offset: Word, src_offset: Word, length: Word, data: &[u8]) { + // Reference go-ethereum `opCallDataCopy` function for defails. + // https://github.com/ethereum/go-ethereum/blob/bb4ac2d396de254898a5f44b1ea2086bfe5bd193/core/vm/instructions.go#L299 + + // `length` should be checked for overflow during gas cost calculation. + // Otherwise should return an out of gas error previously. + let length = length.as_usize(); if length != 0 { + // `dst_offset` should be within range if length is non-zero. + // https://github.com/ethereum/go-ethereum/blob/bb4ac2d396de254898a5f44b1ea2086bfe5bd193/core/vm/common.go#L37 + let dst_offset = dst_offset.as_u64(); + + // Reset data offset to the maximum value of Uint64 if overflow. + let src_offset = u64::try_from(src_offset).unwrap_or(u64::MAX); + let minimal_length = dst_offset as usize + length; self.extend_at_least(minimal_length); @@ -339,7 +351,7 @@ impl Memory { let mem_ends = mem_starts + length; let dst_slice = &mut self.0[mem_starts..mem_ends]; dst_slice.fill(0); - let data_starts = data_offset as usize; + let data_starts = src_offset as usize; let actual_length = std::cmp::min( length, data.len().checked_sub(data_starts).unwrap_or_default(), diff --git a/zkevm-circuits/src/evm_circuit/execution/blockhash.rs b/zkevm-circuits/src/evm_circuit/execution/blockhash.rs index 97af6df77b..9487951323 100644 --- a/zkevm-circuits/src/evm_circuit/execution/blockhash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/blockhash.rs @@ -4,14 +4,14 @@ use crate::{ param::N_BYTES_U64, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + and, + common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition::Delta, }, - from_bytes, math_gadget::LtGadget, - CachedRegion, Cell, RandomLinearCombination, Word, + CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -26,10 +26,9 @@ use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct BlockHashGadget { same_context: SameContextGadget, - block_number: RandomLinearCombination, + block_number: WordByteCapGadget, current_block_number: Cell, block_hash: Word, - block_lt: LtGadget, diff_lt: LtGadget, } @@ -39,9 +38,6 @@ impl ExecutionGadget for BlockHashGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::BLOCKHASH; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let block_number = cb.query_word_rlc(); - cb.stack_pop(block_number.expr()); - let current_block_number = cb.query_cell(); cb.block_lookup( BlockContextFieldTag::Number.expr(), @@ -49,28 +45,32 @@ impl ExecutionGadget for BlockHashGadget { current_block_number.expr(), ); - let block_lt = LtGadget::construct( - cb, - from_bytes::expr(&block_number.cells), - current_block_number.expr(), - ); + let block_number = WordByteCapGadget::construct(cb, current_block_number.expr()); + cb.stack_pop(block_number.original_word()); + + let block_hash = cb.query_word_rlc(); let diff_lt = LtGadget::construct( cb, current_block_number.expr(), - 257.expr() + from_bytes::expr(&block_number.cells), + 257.expr() + block_number.valid_value(), ); - let block_hash = cb.query_word_rlc(); - cb.condition(block_lt.expr() * diff_lt.expr(), |cb| { + let is_valid = and::expr([block_number.lt_cap(), diff_lt.expr()]); + + cb.condition(is_valid.expr(), |cb| { cb.block_lookup( BlockContextFieldTag::BlockHash.expr(), - Some(from_bytes::expr(&block_number.cells)), + Some(block_number.valid_value()), block_hash.expr(), ); }); - cb.condition(not::expr(block_lt.expr() * diff_lt.expr()), |cb| { - cb.require_zero("invalid range", block_hash.expr()); + + cb.condition(not::expr(is_valid), |cb| { + cb.require_zero( + "Invalid block number for block hash lookup", + block_hash.expr(), + ); }); cb.stack_push(block_hash.expr()); @@ -89,7 +89,6 @@ impl ExecutionGadget for BlockHashGadget { block_number, current_block_number, block_hash, - block_lt, diff_lt, } } @@ -105,29 +104,17 @@ impl ExecutionGadget for BlockHashGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; + let current_block_number = block.context.number; + let current_block_number = current_block_number + .to_scalar() + .expect("unexpected U256 -> Scalar conversion failure"); + let block_number = block.rws[step.rw_indices[0]].stack_value(); - self.block_number.assign( - region, - offset, - Some( - block_number.to_le_bytes()[..N_BYTES_U64] - .try_into() - .unwrap(), - ), - )?; - let block_number: F = block_number.to_scalar().unwrap(); + self.block_number + .assign(region, offset, block_number, current_block_number)?; - let current_block_number = block.context.number; - self.current_block_number.assign( - region, - offset, - Value::known( - current_block_number - .to_scalar() - .expect("unexpected U256 -> Scalar conversion failure"), - ), - )?; - let current_block_number: F = current_block_number.to_scalar().unwrap(); + self.current_block_number + .assign(region, offset, Value::known(current_block_number))?; self.block_hash.assign( region, @@ -135,14 +122,11 @@ impl ExecutionGadget for BlockHashGadget { Some(block.rws[step.rw_indices[1]].stack_value().to_le_bytes()), )?; - self.block_lt - .assign(region, offset, block_number, current_block_number)?; - self.diff_lt.assign( region, offset, current_block_number, - block_number + F::from(257), + F::from(u64::try_from(block_number).unwrap_or(u64::MAX)) + F::from(257), )?; Ok(()) @@ -155,7 +139,7 @@ mod test { use eth_types::{bytecode, U256}; use mock::test_ctx::{helpers::*, TestContext}; - fn test_ok(block_number: usize, current_block_number: u64) { + fn test_ok(block_number: U256, current_block_number: u64) { let code = bytecode! { PUSH32(block_number) BLOCKHASH @@ -185,21 +169,26 @@ mod test { #[test] fn blockhash_gadget_simple() { - test_ok(0, 5); - test_ok(1, 5); - test_ok(2, 5); - test_ok(3, 5); - test_ok(4, 5); - test_ok(5, 5); - test_ok(6, 5); + test_ok(0.into(), 5); + test_ok(1.into(), 5); + test_ok(2.into(), 5); + test_ok(3.into(), 5); + test_ok(4.into(), 5); + test_ok(5.into(), 5); + test_ok(6.into(), 5); } #[test] fn blockhash_gadget_large() { - test_ok(0xcafe - 257, 0xcafeu64); - test_ok(0xcafe - 256, 0xcafeu64); - test_ok(0xcafe - 1, 0xcafeu64); - test_ok(0xcafe, 0xcafeu64); - test_ok(0xcafe + 1, 0xcafeu64); + test_ok((0xcafe - 257).into(), 0xcafeu64); + test_ok((0xcafe - 256).into(), 0xcafeu64); + test_ok((0xcafe - 1).into(), 0xcafeu64); + test_ok(0xcafe.into(), 0xcafeu64); + test_ok((0xcafe + 1).into(), 0xcafeu64); + } + + #[test] + fn blockhash_gadget_block_number_overflow() { + test_ok(U256::MAX, 0xcafeu64); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 6296bb2874..216cfb15d9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -1,17 +1,16 @@ use crate::{ evm_circuit::{ execution::ExecutionGadget, - param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + param::{N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition::{Delta, To}, }, - from_bytes, memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, - not, select, CachedRegion, Cell, MemoryAddress, + not, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -19,7 +18,7 @@ use crate::{ util::Expr, }; use bus_mapping::{circuit_input_builder::CopyDataType, evm::OpcodeId}; -use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar}; +use eth_types::{evm_types::GasCost, Field, ToScalar}; use halo2_proofs::{circuit::Value, plonk::Error}; use std::cmp::min; @@ -28,7 +27,7 @@ use std::cmp::min; pub(crate) struct CallDataCopyGadget { same_context: SameContextGadget, memory_address: MemoryAddressGadget, - data_offset: MemoryAddress, + data_offset: WordByteCapGadget, src_id: Cell, call_data_length: Cell, call_data_offset: Cell, // Only used in the internal call @@ -45,20 +44,19 @@ impl ExecutionGadget for CallDataCopyGadget { fn configure(cb: &mut EVMConstraintBuilder) -> Self { let opcode = cb.query_cell(); - let memory_offset = cb.query_cell_phase2(); - let data_offset = cb.query_word_rlc(); + let src_id = cb.query_cell(); + let call_data_length = cb.query_cell(); + let call_data_offset = cb.query_cell(); + let length = cb.query_word_rlc(); + let memory_offset = cb.query_cell_phase2(); + let data_offset = WordByteCapGadget::construct(cb, call_data_length.expr()); // Pop memory_offset, data_offset, length from stack cb.stack_pop(memory_offset.expr()); - cb.stack_pop(data_offset.expr()); + cb.stack_pop(data_offset.original_word()); cb.stack_pop(length.expr()); - let memory_address = MemoryAddressGadget::construct(cb, memory_offset, length); - let src_id = cb.query_cell(); - let call_data_length = cb.query_cell(); - let call_data_offset = cb.query_cell(); - // Lookup the calldata_length and caller_address in Tx context table or // Call context table cb.condition(cb.curr.state.is_root.expr(), |cb| { @@ -97,6 +95,7 @@ impl ExecutionGadget for CallDataCopyGadget { // Calculate the next memory size and the gas cost for this memory // access + let memory_address = MemoryAddressGadget::construct(cb, memory_offset, length); let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); let memory_copier_gas = MemoryCopierGasGadget::construct( cb, @@ -111,13 +110,23 @@ impl ExecutionGadget for CallDataCopyGadget { CopyDataType::Memory.expr(), ); cb.condition(memory_address.has_length(), |cb| { + // Set source start to the minimun value of data offset and call data length. + let src_addr = call_data_offset.expr() + + select::expr( + data_offset.lt_cap(), + data_offset.valid_value(), + call_data_length.expr(), + ); + + let src_addr_end = call_data_offset.expr() + call_data_length.expr(); + cb.copy_table_lookup( src_id.expr(), src_tag, cb.curr.state.call_id.expr(), CopyDataType::Memory.expr(), - call_data_offset.expr() + from_bytes::expr(&data_offset.cells), - call_data_offset.expr() + call_data_length.expr(), + src_addr, + src_addr_end, memory_address.offset(), memory_address.length(), 0.expr(), // for CALLDATACOPY rlc_acc is 0 @@ -175,15 +184,6 @@ impl ExecutionGadget for CallDataCopyGadget { let memory_address = self .memory_address .assign(region, offset, memory_offset, length)?; - self.data_offset.assign( - region, - offset, - Some( - data_offset.to_le_bytes()[..N_BYTES_MEMORY_ADDRESS] - .try_into() - .unwrap(), - ), - )?; let src_id = if call.is_root { tx.id } else { call.caller_id }; self.src_id.assign( region, @@ -202,6 +202,9 @@ impl ExecutionGadget for CallDataCopyGadget { self.call_data_offset .assign(region, offset, Value::known(F::from(call_data_offset)))?; + self.data_offset + .assign(region, offset, data_offset, F::from(call_data_length))?; + // rw_counter increase from copy lookup is `length` memory writes + a variable // number of memory reads. let copy_rwc_inc = length @@ -212,9 +215,10 @@ impl ExecutionGadget for CallDataCopyGadget { // memory reads when reading from memory of caller is capped by call_data_length // - data_offset. min( - length.low_u64(), - call_data_length - .checked_sub(data_offset.low_u64()) + length.as_u64(), + u64::try_from(data_offset) + .ok() + .and_then(|offset| call_data_length.checked_sub(offset)) .unwrap_or_default(), ) }; @@ -261,8 +265,8 @@ mod test { fn test_root_ok( call_data_length: usize, memory_offset: usize, - data_offset: usize, length: usize, + data_offset: Word, ) { let bytecode = bytecode! { PUSH32(length) @@ -300,14 +304,14 @@ mod test { call_data_offset: usize, call_data_length: usize, dst_offset: usize, - offset: usize, length: usize, + data_offset: Word, ) { let (addr_a, addr_b) = (mock::MOCK_ACCOUNTS[0], mock::MOCK_ACCOUNTS[1]); // code B gets called by code A, so the call is an internal call. let code_b = bytecode! { - .op_calldatacopy(dst_offset, offset, length) + .op_calldatacopy(dst_offset, data_offset, length) STOP }; @@ -340,25 +344,31 @@ mod test { #[test] fn calldatacopy_gadget_simple() { - test_root_ok(0x40, 0x40, 0x00, 10); - test_internal_ok(0x40, 0x40, 0xA0, 0x10, 10); + test_root_ok(0x40, 0x40, 10, 0x00.into()); + test_internal_ok(0x40, 0x40, 0xA0, 10, 0x10.into()); } #[test] fn calldatacopy_gadget_large() { - test_root_ok(0x204, 0x103, 0x102, 0x101); - test_internal_ok(0x30, 0x204, 0x103, 0x102, 0x101); + test_root_ok(0x204, 0x103, 0x101, 0x102.into()); + test_internal_ok(0x30, 0x204, 0x103, 0x101, 0x102.into()); } #[test] fn calldatacopy_gadget_out_of_bound() { - test_root_ok(0x40, 0x40, 0x20, 40); - test_internal_ok(0x40, 0x20, 0xA0, 0x28, 10); + test_root_ok(0x40, 0x40, 40, 0x20.into()); + test_internal_ok(0x40, 0x20, 0xA0, 10, 0x28.into()); } #[test] fn calldatacopy_gadget_zero_length() { - test_root_ok(0x40, 0x40, 0x00, 0); - test_internal_ok(0x40, 0x40, 0xA0, 0x10, 0); + test_root_ok(0x40, 0x40, 0, 0x00.into()); + test_internal_ok(0x40, 0x40, 0xA0, 0, 0x10.into()); + } + + #[test] + fn calldatacopy_gadget_data_offset_overflow() { + test_root_ok(0x40, 0x40, 0, Word::MAX); + test_internal_ok(0x40, 0x40, 0xA0, 0, Word::MAX); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs index 6931647404..f443980188 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs @@ -1,5 +1,5 @@ use bus_mapping::evm::OpcodeId; -use eth_types::{Field, ToLittleEndian}; +use eth_types::Field; use halo2_proofs::{ circuit::Value, plonk::{Error, Expression}, @@ -7,17 +7,17 @@ use halo2_proofs::{ use crate::{ evm_circuit::{ - param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_WORD}, + param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_U64, N_BYTES_WORD}, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + and, + common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition::Delta, }, - from_bytes, memory_gadget::BufferReaderGadget, - not, CachedRegion, Cell, MemoryAddress, + not, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -37,8 +37,6 @@ pub(crate) struct CallDataLoadGadget { /// Source of data, this is transaction ID for a root call and caller ID for /// an internal call. src_id: Cell, - /// The bytes offset in calldata, from which we load a 32-bytes word. - offset: MemoryAddress, /// The size of the call's data (tx input for a root call or calldata length /// of an internal call). call_data_length: Cell, @@ -46,6 +44,9 @@ pub(crate) struct CallDataLoadGadget { /// tx data starts at the first byte, but can be non-zero offset for an /// internal call. call_data_offset: Cell, + /// The bytes offset in calldata, from which we load a 32-bytes word. It + /// is valid if within range of Uint64 and less than call_data_length. + data_offset: WordByteCapGadget, /// Gadget to read from tx calldata, which we validate against the word /// pushed to stack. buffer_reader: BufferReaderGadget, @@ -59,61 +60,84 @@ impl ExecutionGadget for CallDataLoadGadget { fn configure(cb: &mut EVMConstraintBuilder) -> Self { let opcode = cb.query_cell(); - let offset = cb.query_word_rlc(); - - // Pop the offset value from stack. - cb.stack_pop(offset.expr()); - - // Add a lookup constrain for TxId in the RW table. let src_id = cb.query_cell(); let call_data_length = cb.query_cell(); let call_data_offset = cb.query_cell(); - let src_addr = from_bytes::expr(&offset.cells) + call_data_offset.expr(); - let src_addr_end = call_data_length.expr() + call_data_offset.expr(); + let data_offset = WordByteCapGadget::construct(cb, call_data_length.expr()); + cb.stack_pop(data_offset.original_word()); + + cb.condition( + and::expr([data_offset.not_overflow(), cb.curr.state.is_root.expr()]), + |cb| { + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::TxId, + src_id.expr(), + ); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::CallDataLength, + call_data_length.expr(), + ); + cb.require_equal( + "if is_root then call_data_offset == 0", + call_data_offset.expr(), + 0.expr(), + ); + }, + ); - cb.condition(cb.curr.state.is_root.expr(), |cb| { - cb.call_context_lookup(false.expr(), None, CallContextFieldTag::TxId, src_id.expr()); - cb.call_context_lookup( - false.expr(), - None, - CallContextFieldTag::CallDataLength, - call_data_length.expr(), - ); - cb.require_equal( - "if is_root then call_data_offset == 0", - call_data_offset.expr(), - 0.expr(), - ); - }); - cb.condition(not::expr(cb.curr.state.is_root.expr()), |cb| { - cb.call_context_lookup( - false.expr(), - None, - CallContextFieldTag::CallerId, - src_id.expr(), - ); - cb.call_context_lookup( - false.expr(), - None, - CallContextFieldTag::CallDataLength, + cb.condition( + and::expr([ + data_offset.not_overflow(), + not::expr(cb.curr.state.is_root.expr()), + ]), + |cb| { + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::CallerId, + src_id.expr(), + ); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::CallDataLength, + call_data_length.expr(), + ); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::CallDataOffset, + call_data_offset.expr(), + ); + }, + ); + + // Set source start to the minimun value of data offset and call data length. + let src_addr = call_data_offset.expr() + + select::expr( + data_offset.lt_cap(), + data_offset.valid_value(), call_data_length.expr(), ); - cb.call_context_lookup( - false.expr(), - None, - CallContextFieldTag::CallDataOffset, - call_data_offset.expr(), - ); - }); - let buffer_reader = BufferReaderGadget::construct(cb, src_addr.clone(), src_addr_end); + let src_addr_end = call_data_offset.expr() + call_data_length.expr(); + + let buffer_reader = BufferReaderGadget::construct(cb, src_addr.expr(), src_addr_end); - let mut calldata_word = (0..N_BYTES_WORD) + let mut calldata_word: Vec<_> = (0..N_BYTES_WORD) .map(|idx| { - // for a root call, the call data comes from tx's data field. + // For a root call, the call data comes from tx's data field. cb.condition( - cb.curr.state.is_root.expr() * buffer_reader.read_flag(idx), + and::expr([ + data_offset.not_overflow(), + buffer_reader.read_flag(idx), + cb.curr.state.is_root.expr(), + ]), |cb| { cb.tx_context_lookup( src_id.expr(), @@ -123,9 +147,13 @@ impl ExecutionGadget for CallDataLoadGadget { ); }, ); - // for an internal call, the call data comes from memory. + // For an internal call, the call data comes from memory. cb.condition( - (1.expr() - cb.curr.state.is_root.expr()) * buffer_reader.read_flag(idx), + and::expr([ + data_offset.not_overflow(), + buffer_reader.read_flag(idx), + not::expr(cb.curr.state.is_root.expr()), + ]), |cb| { cb.memory_lookup( 0.expr(), @@ -137,7 +165,7 @@ impl ExecutionGadget for CallDataLoadGadget { ); buffer_reader.byte(idx) }) - .collect::>>(); + .collect(); // Since the stack items are in little endian form, we reverse the bytes // here. @@ -146,7 +174,13 @@ impl ExecutionGadget for CallDataLoadGadget { // Add a lookup constraint for the 32-bytes that should have been pushed // to the stack. let calldata_word: [Expression; N_BYTES_WORD] = calldata_word.try_into().unwrap(); - cb.stack_push(cb.word_rlc(calldata_word)); + let calldata_word = cb.word_rlc(calldata_word); + cb.require_zero( + "Stack push result must be 0 if stack pop offset is Uint64 overflow", + data_offset.overflow() * calldata_word.expr(), + ); + + cb.stack_push(calldata_word); let step_state_transition = StepStateTransition { rw_counter: Delta(cb.rw_counter_offset()), @@ -160,10 +194,10 @@ impl ExecutionGadget for CallDataLoadGadget { Self { same_context, - offset, src_id, call_data_length, call_data_offset, + data_offset, buffer_reader, } } @@ -179,62 +213,58 @@ impl ExecutionGadget for CallDataLoadGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - // set the value for bytes offset in calldata. This is where we start - // reading bytes from. - let data_offset = block.rws[step.rw_indices[0]].stack_value(); - - // assign the calldata start and end cells. - self.offset.assign( - region, - offset, - Some( - data_offset.to_le_bytes()[..N_BYTES_MEMORY_ADDRESS] - .try_into() - .unwrap(), - ), - )?; - - // assign to the buffer reader gadget. - let (calldata_length, calldata_offset, src_id) = if call.is_root { - (tx.call_data_length as u64, 0u64, tx.id as u64) + // Assign to the buffer reader gadget. + let (src_id, call_data_offset, call_data_length) = if call.is_root { + (tx.id, 0, tx.call_data_length as u64) } else { - ( - call.call_data_length, - call.call_data_offset, - call.caller_id as u64, - ) + (call.caller_id, call.call_data_offset, call.call_data_length) }; self.src_id - .assign(region, offset, Value::known(F::from(src_id)))?; + .assign(region, offset, Value::known(F::from(src_id as u64)))?; self.call_data_length - .assign(region, offset, Value::known(F::from(calldata_length)))?; + .assign(region, offset, Value::known(F::from(call_data_length)))?; self.call_data_offset - .assign(region, offset, Value::known(F::from(calldata_offset)))?; + .assign(region, offset, Value::known(F::from(call_data_offset)))?; - let mut calldata_bytes = vec![0u8; N_BYTES_WORD]; - let (src_addr, src_addr_end) = ( - data_offset.as_usize() + calldata_offset as usize, - calldata_length as usize + calldata_offset as usize, - ); + let data_offset = block.rws[step.rw_indices[0]].stack_value(); + let offset_not_overflow = + self.data_offset + .assign(region, offset, data_offset, F::from(call_data_length))?; - for (i, byte) in calldata_bytes.iter_mut().enumerate() { - if call.is_root { - // fetch from tx call data - if src_addr + i < tx.call_data_length { - *byte = tx.call_data[src_addr + i]; - } - } else { - // fetch from memory - if src_addr + i < (call.call_data_offset + call.call_data_length) as usize { - *byte = block.rws[step.rw_indices[OFFSET_RW_MEMORY_INDICES + i]].memory_value(); + let data_offset = if offset_not_overflow { + data_offset.as_u64() + } else { + call_data_length + }; + let src_addr_end = call_data_offset + call_data_length; + let src_addr = call_data_offset + .checked_add(data_offset) + .unwrap_or(src_addr_end) + .min(src_addr_end); + + let mut calldata_bytes = vec![0u8; N_BYTES_WORD]; + if offset_not_overflow { + for (i, byte) in calldata_bytes.iter_mut().enumerate() { + if call.is_root { + // Fetch from tx call data. + if src_addr + (i as u64) < tx.call_data_length as u64 { + *byte = tx.call_data[src_addr as usize + i]; + } + } else { + // Fetch from memory. + if src_addr + (i as u64) < call.call_data_offset + call.call_data_length { + *byte = + block.rws[step.rw_indices[OFFSET_RW_MEMORY_INDICES + i]].memory_value(); + } } } } + self.buffer_reader.assign( region, offset, - src_addr as u64, - src_addr_end as u64, + src_addr, + src_addr_end, &calldata_bytes, &[true; N_BYTES_WORD], )?; @@ -249,15 +279,15 @@ mod test { use eth_types::{bytecode, Word}; use mock::{generate_mock_call_bytecode, MockCallBytecodeParams, TestContext}; - fn test_bytecode(offset: usize) -> eth_types::Bytecode { + fn test_bytecode(offset: Word) -> eth_types::Bytecode { bytecode! { - PUSH32(Word::from(offset)) + PUSH32(offset) CALLDATALOAD STOP } } - fn test_root_ok(offset: usize) { + fn test_root_ok(offset: Word) { let bytecode = test_bytecode(offset); CircuitTestBuilder::new_from_test_ctx( @@ -266,7 +296,7 @@ mod test { .run(); } - fn test_internal_ok(call_data_length: usize, call_data_offset: usize, offset: usize) { + fn test_internal_ok(call_data_length: usize, call_data_offset: usize, offset: Word) { let (addr_a, addr_b) = (mock::MOCK_ACCOUNTS[0], mock::MOCK_ACCOUNTS[1]); // code B gets called by code A, so the call is an internal call. @@ -300,17 +330,23 @@ mod test { #[test] fn calldataload_gadget_root() { - test_root_ok(0x00); - test_root_ok(0x08); - test_root_ok(0x10); - test_root_ok(0x2010); + test_root_ok(0x00.into()); + test_root_ok(0x08.into()); + test_root_ok(0x10.into()); + test_root_ok(0x2010.into()); } #[test] fn calldataload_gadget_internal() { - test_internal_ok(0x20, 0x00, 0x00); - test_internal_ok(0x20, 0x10, 0x10); - test_internal_ok(0x40, 0x20, 0x08); - test_internal_ok(0x1010, 0xff, 0x10); + test_internal_ok(0x20, 0x00, 0x00.into()); + test_internal_ok(0x20, 0x10, 0x10.into()); + test_internal_ok(0x40, 0x20, 0x08.into()); + test_internal_ok(0x1010, 0xff, 0x10.into()); + } + + #[test] + fn calldataload_gadget_offset_overflow() { + test_root_ok(Word::MAX); + test_internal_ok(0x1010, 0xff, Word::MAX); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index 17a8258d47..f875c83f48 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -1,19 +1,18 @@ use bus_mapping::{circuit_input_builder::CopyDataType, evm::OpcodeId}; -use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar}; +use eth_types::{evm_types::GasCost, Field, ToScalar}; use halo2_proofs::{circuit::Value, plonk::Error}; use crate::{ evm_circuit::{ - param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + param::{N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition, }, - from_bytes, memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, - not, CachedRegion, Cell, MemoryAddress, + not, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -25,8 +24,9 @@ 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 memory address for the offset in code from where we + /// read. It is valid if within range of Uint64 and less than code_size. + code_offset: WordByteCapGadget, /// Holds the size of the current environment's bytecode. code_size: Cell, /// The code from current environment is copied to memory. To verify this @@ -51,14 +51,15 @@ impl ExecutionGadget for CodeCopyGadget { fn configure(cb: &mut EVMConstraintBuilder) -> Self { let opcode = cb.query_cell(); - // Query elements to be popped from the stack. - let dst_memory_offset = cb.query_cell_phase2(); - let code_offset = cb.query_word_rlc(); + let code_size = cb.query_cell(); + let size = cb.query_word_rlc(); + let dst_memory_offset = cb.query_cell_phase2(); + let code_offset = WordByteCapGadget::construct(cb, code_size.expr()); // Pop items from stack. cb.stack_pop(dst_memory_offset.expr()); - cb.stack_pop(code_offset.expr()); + cb.stack_pop(code_offset.original_word()); cb.stack_pop(size.expr()); // Construct memory address in the destionation (memory) to which we copy code. @@ -68,7 +69,6 @@ impl ExecutionGadget for CodeCopyGadget { let code_hash = cb.curr.state.code_hash.clone(); // Fetch the bytecode length from the bytecode table. - let code_size = cb.query_cell(); cb.bytecode_length(code_hash.expr(), code_size.expr()); // Calculate the next memory size and the gas cost for this memory @@ -83,12 +83,19 @@ impl ExecutionGadget for CodeCopyGadget { let copy_rwc_inc = cb.query_cell(); cb.condition(dst_memory_addr.has_length(), |cb| { + // Set source start to the minimun value of code offset and code size. + let src_addr = select::expr( + code_offset.lt_cap(), + code_offset.valid_value(), + code_size.expr(), + ); + cb.copy_table_lookup( code_hash.expr(), CopyDataType::Bytecode.expr(), cb.curr.state.call_id.expr(), CopyDataType::Memory.expr(), - from_bytes::expr(&code_offset.cells), + src_addr, code_size.expr(), dst_memory_addr.offset(), dst_memory_addr.length(), @@ -147,26 +154,17 @@ impl ExecutionGadget for CodeCopyGadget { 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 code = block + let bytecode = block .bytecodes .get(&call.code_hash) .expect("could not find current environment's bytecode"); - self.code_size.assign( - region, - offset, - Value::known(F::from(code.bytes.len() as u64)), - )?; + + let code_size = bytecode.bytes.len() as u64; + self.code_size + .assign(region, offset, Value::known(F::from(code_size)))?; + + self.code_offset + .assign(region, offset, code_offset, F::from(code_size))?; // assign the destination memory offset. let memory_address = self @@ -202,16 +200,16 @@ mod tests { use eth_types::{bytecode, Word}; use mock::TestContext; - fn test_ok(memory_offset: usize, code_offset: usize, size: usize, large: bool) { + fn test_ok(code_offset: Word, memory_offset: usize, size: usize, large: bool) { let mut code = bytecode! {}; if large { - for _ in 0..0x101 { + for _ in 0..size { code.push(1, Word::from(123)); } } let tail = bytecode! { PUSH32(Word::from(size)) - PUSH32(Word::from(code_offset)) + PUSH32(code_offset) PUSH32(Word::from(memory_offset)) CODECOPY STOP @@ -226,13 +224,18 @@ mod tests { #[test] fn codecopy_gadget_simple() { - test_ok(0x00, 0x00, 0x20, false); - test_ok(0x20, 0x30, 0x30, false); - test_ok(0x10, 0x20, 0x42, false); + test_ok(0x00.into(), 0x00, 0x20, false); + test_ok(0x30.into(), 0x20, 0x30, false); + test_ok(0x20.into(), 0x10, 0x42, false); } #[test] fn codecopy_gadget_large() { - test_ok(0x103, 0x102, 0x101, true); + test_ok(0x102.into(), 0x103, 0x101, true); + } + + #[test] + fn codecopy_gadget_code_offset_overflow() { + test_ok(Word::MAX, 0x103, 0x101, true); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs index 2fc0f95c56..11c95302a8 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs @@ -4,30 +4,26 @@ use crate::{ param::N_BYTES_PROGRAM_COUNTER, step::ExecutionState, util::{ - and, - common_gadget::CommonErrorGadget, + common_gadget::{CommonErrorGadget, WordByteCapGadget}, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, - from_bytes, - math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget}, - select, sum, CachedRegion, Cell, Word, + math_gadget::{IsEqualGadget, IsZeroGadget}, + CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, util::Expr, }; -use eth_types::{evm_types::OpcodeId, Field, ToLittleEndian, U256}; +use eth_types::{evm_types::OpcodeId, Field, U256}; use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct ErrorInvalidJumpGadget { opcode: Cell, - dest_word: Word, + dest: WordByteCapGadget, code_len: Cell, value: Cell, is_code: Cell, - dest_not_overflow: IsZeroGadget, - dest_lt_code_len: LtGadget, is_jump_dest: IsEqualGadget, is_jumpi: IsEqualGadget, phase2_condition: Cell, @@ -41,14 +37,8 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorInvalidJump; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let dest_word = cb.query_word_rlc(); - let dest_not_overflow = - IsZeroGadget::construct(cb, sum::expr(&dest_word.cells[N_BYTES_PROGRAM_COUNTER..])); - let dest = select::expr( - dest_not_overflow.expr(), - from_bytes::expr(&dest_word.cells[..N_BYTES_PROGRAM_COUNTER]), - u64::MAX.expr(), - ); + let code_len = cb.query_cell(); + let dest = WordByteCapGadget::construct(cb, code_len.expr()); let opcode = cb.query_cell(); let value = cb.query_cell(); @@ -71,7 +61,7 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { let is_condition_zero = IsZeroGadget::construct(cb, phase2_condition.expr()); // Pop the value from the stack - cb.stack_pop(dest_word.expr()); + cb.stack_pop(dest.original_word()); cb.condition(is_jumpi.expr(), |cb| { cb.stack_pop(phase2_condition.expr()); @@ -80,39 +70,31 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { }); // Look up bytecode length - let code_len = cb.query_cell(); cb.bytecode_length(cb.curr.state.code_hash.expr(), code_len.expr()); - let dest_lt_code_len = LtGadget::construct(cb, dest.expr(), code_len.expr()); - // If destination is in valid range, lookup for the value. - cb.condition( - and::expr([dest_not_overflow.expr(), dest_lt_code_len.expr()]), - |cb| { - cb.bytecode_lookup( - cb.curr.state.code_hash.expr(), - dest.expr(), - is_code.expr(), - value.expr(), - ); - cb.require_zero( - "is_code is false or not JUMPDEST", - is_code.expr() * is_jump_dest.expr(), - ); - }, - ); + cb.condition(dest.lt_cap(), |cb| { + cb.bytecode_lookup( + cb.curr.state.code_hash.expr(), + dest.valid_value(), + is_code.expr(), + value.expr(), + ); + cb.require_zero( + "is_code is false or not JUMPDEST", + is_code.expr() * is_jump_dest.expr(), + ); + }); let common_error_gadget = CommonErrorGadget::construct(cb, opcode.expr(), 3.expr() + is_jumpi.expr()); Self { opcode, - dest_word, + dest, code_len, value, is_code, - dest_not_overflow, - dest_lt_code_len, is_jump_dest, is_jumpi, phase2_condition, @@ -135,10 +117,6 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; - let dest = block.rws[step.rw_indices[0]].stack_value(); - self.dest_word - .assign(region, offset, Some(dest.to_le_bytes()))?; - let condition = if is_jumpi { block.rws[step.rw_indices[1]].stack_value() } else { @@ -154,19 +132,11 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { self.code_len .assign(region, offset, Value::known(F::from(code_len)))?; - let dest_overflow_hi = dest.to_le_bytes()[N_BYTES_PROGRAM_COUNTER..] - .iter() - .fold(0, |acc, val| acc + u64::from(*val)); - self.dest_not_overflow - .assign(region, offset, F::from(dest_overflow_hi))?; - - let dest = if dest_overflow_hi == 0 { - dest.low_u64() - } else { - u64::MAX - }; + let dest = block.rws[step.rw_indices[0]].stack_value(); + self.dest.assign(region, offset, dest, F::from(code_len))?; // set default value in case can not find value, is_code from bytecode table + let dest = u64::try_from(dest).unwrap_or(code_len); let mut code_pair = [0u8, 0u8]; if dest < code_len { // get real value from bytecode table @@ -184,9 +154,6 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { F::from(OpcodeId::JUMPDEST.as_u64()), )?; - self.dest_lt_code_len - .assign(region, offset, F::from(dest), F::from(code_len))?; - self.is_jumpi.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 01f1ac2a58..376f0f64b7 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -1,16 +1,16 @@ use crate::{ evm_circuit::{ - param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition, }, from_bytes, memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, - not, select, CachedRegion, Cell, MemoryAddress, Word, + not, select, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -28,7 +28,7 @@ pub(crate) struct ExtcodecopyGadget { same_context: SameContextGadget, external_address_word: Word, memory_address: MemoryAddressGadget, - data_offset: MemoryAddress, + code_offset: WordByteCapGadget, tx_id: Cell, reversion_info: ReversionInfo, is_warm: Cell, @@ -51,17 +51,17 @@ impl ExecutionGadget for ExtcodecopyGadget { let external_address = from_bytes::expr(&external_address_word.cells[..N_BYTES_ACCOUNT_ADDRESS]); - let memory_offset = cb.query_cell_phase2(); - let data_offset = cb.query_word_rlc(); + let code_size = cb.query_cell(); + let memory_length = cb.query_word_rlc(); + let memory_offset = cb.query_cell_phase2(); + let code_offset = WordByteCapGadget::construct(cb, code_size.expr()); cb.stack_pop(external_address_word.expr()); cb.stack_pop(memory_offset.expr()); - cb.stack_pop(data_offset.expr()); + cb.stack_pop(code_offset.original_word()); cb.stack_pop(memory_length.expr()); - let memory_address = MemoryAddressGadget::construct(cb, memory_offset, memory_length); - let tx_id = cb.call_context(None, CallContextFieldTag::TxId); let mut reversion_info = cb.reversion_info_read(None); let is_warm = cb.query_bool(); @@ -79,13 +79,13 @@ impl ExecutionGadget for ExtcodecopyGadget { AccountFieldTag::CodeHash, code_hash.expr(), ); - let code_size = cb.query_cell(); // TODO: If external_address doesn't exist, we will get code_hash = 0. With // this value, the bytecode_length lookup will not work, and the copy // from code_hash = 0 will not work. We should use EMPTY_HASH when // code_hash = 0. cb.bytecode_length(code_hash.expr(), code_size.expr()); + let memory_address = MemoryAddressGadget::construct(cb, memory_offset, memory_length); let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); let memory_copier_gas = MemoryCopierGasGadget::construct( cb, @@ -101,12 +101,19 @@ impl ExecutionGadget for ExtcodecopyGadget { let copy_rwc_inc = cb.query_cell(); cb.condition(memory_address.has_length(), |cb| { + // Set source start to the minimun value of code offset and code size. + let src_addr = select::expr( + code_offset.lt_cap(), + code_offset.valid_value(), + code_size.expr(), + ); + cb.copy_table_lookup( code_hash.expr(), CopyDataType::Bytecode.expr(), cb.curr.state.call_id.expr(), CopyDataType::Memory.expr(), - from_bytes::expr(&data_offset.cells), + src_addr, code_size.expr(), memory_address.offset(), memory_address.length(), @@ -136,7 +143,7 @@ impl ExecutionGadget for ExtcodecopyGadget { same_context, external_address_word, memory_address, - data_offset, + code_offset, tx_id, is_warm, reversion_info, @@ -159,22 +166,13 @@ impl ExecutionGadget for ExtcodecopyGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [external_address, memory_offset, data_offset, memory_length] = + let [external_address, memory_offset, code_offset, memory_length] = [0, 1, 2, 3].map(|idx| block.rws[step.rw_indices[idx]].stack_value()); self.external_address_word .assign(region, offset, Some(external_address.to_le_bytes()))?; let memory_address = self.memory_address .assign(region, offset, memory_offset, memory_length)?; - self.data_offset.assign( - region, - offset, - Some( - data_offset.to_le_bytes()[..N_BYTES_MEMORY_ADDRESS] - .try_into() - .unwrap(), - ), - )?; self.tx_id .assign(region, offset, Value::known(F::from(transaction.id as u64)))?; @@ -193,7 +191,7 @@ impl ExecutionGadget for ExtcodecopyGadget { self.code_hash .assign(region, offset, region.word_rlc(code_hash))?; - let bytecode_len = if code_hash.is_zero() { + let code_size = if code_hash.is_zero() { 0 } else { block @@ -201,10 +199,13 @@ impl ExecutionGadget for ExtcodecopyGadget { .get(&code_hash) .expect("could not find external bytecode") .bytes - .len() + .len() as u64 }; self.code_size - .assign(region, offset, Value::known(F::from(bytecode_len as u64)))?; + .assign(region, offset, Value::known(F::from(code_size)))?; + + self.code_offset + .assign(region, offset, code_offset, F::from(code_size))?; self.copy_rwc_inc.assign( region, @@ -250,8 +251,8 @@ mod test { fn test_ok( external_account: Option, + code_offset: Word, memory_offset: usize, - data_offset: usize, length: usize, is_warm: bool, ) { @@ -271,7 +272,7 @@ mod test { } code.append(&bytecode! { PUSH32(length) - PUSH32(data_offset) + PUSH32(code_offset) PUSH32(memory_offset) PUSH20(external_address.to_word()) #[start] @@ -311,8 +312,8 @@ mod test { #[test] fn extcodecopy_empty_account() { - test_ok(None, 0x00, 0x00, 0x36, true); // warm account - test_ok(None, 0x00, 0x00, 0x36, false); // cold account + test_ok(None, 0x00.into(), 0x00, 0x36, true); // warm account + test_ok(None, 0x00.into(), 0x00, 0x36, false); // cold account } #[test] @@ -323,7 +324,7 @@ mod test { code: Bytes::from([10, 40]), ..Default::default() }), - 0x00, + 0x00.into(), 0x00, 0x36, true, @@ -335,7 +336,7 @@ mod test { code: Bytes::from([10, 40]), ..Default::default() }), - 0x00, + 0x00.into(), 0x00, 0x36, false, @@ -350,7 +351,7 @@ mod test { code: Bytes::from(rand_bytes_array::<256>()), ..Default::default() }), - 0x00, + 0x00.into(), 0x00, 0x36, true, @@ -361,7 +362,7 @@ mod test { code: Bytes::from(rand_bytes_array::<256>()), ..Default::default() }), - 0x00, + 0x00.into(), 0x00, 0x36, false, @@ -376,8 +377,8 @@ mod test { code: Bytes::from(rand_bytes_array::<64>()), ..Default::default() }), + 0x20.into(), 0x00, - 0x20, 0x104, true, ); @@ -387,10 +388,36 @@ mod test { code: Bytes::from(rand_bytes_array::<64>()), ..Default::default() }), + 0x20.into(), 0x00, - 0x20, 0x104, false, ); } + + #[test] + fn extcodecopy_code_offset_overflow() { + test_ok( + Some(Account { + address: *EXTERNAL_ADDRESS, + code: Bytes::from(rand_bytes_array::<256>()), + ..Default::default() + }), + Word::MAX, + 0x00, + 0x36, + true, + ); + test_ok( + Some(Account { + address: *EXTERNAL_ADDRESS, + code: Bytes::from(rand_bytes_array::<256>()), + ..Default::default() + }), + Word::MAX, + 0x00, + 0x36, + false, + ); + } } diff --git a/zkevm-circuits/src/evm_circuit/execution/jumpi.rs b/zkevm-circuits/src/evm_circuit/execution/jumpi.rs index 28d873b353..4a463bfd5a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/jumpi.rs +++ b/zkevm-circuits/src/evm_circuit/execution/jumpi.rs @@ -4,26 +4,25 @@ use crate::{ param::N_BYTES_PROGRAM_COUNTER, step::ExecutionState, util::{ - common_gadget::SameContextGadget, + common_gadget::{SameContextGadget, WordByteRangeGadget}, constraint_builder::{ - EVMConstraintBuilder, StepStateTransition, + ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition::{Delta, To}, }, - from_bytes, math_gadget::IsZeroGadget, - select, CachedRegion, Cell, RandomLinearCombination, + select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, util::Expr, }; -use eth_types::{evm_types::OpcodeId, Field, ToLittleEndian}; +use eth_types::{evm_types::OpcodeId, Field}; use halo2_proofs::plonk::Error; #[derive(Clone, Debug)] pub(crate) struct JumpiGadget { same_context: SameContextGadget, - destination: RandomLinearCombination, + dest: WordByteRangeGadget, phase2_condition: Cell, is_condition_zero: IsZeroGadget, } @@ -34,11 +33,11 @@ impl ExecutionGadget for JumpiGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::JUMPI; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let destination = cb.query_word_rlc(); + let dest = WordByteRangeGadget::construct(cb); let phase2_condition = cb.query_cell_phase2(); // Pop the value from the stack - cb.stack_pop(destination.expr()); + cb.stack_pop(dest.original_word()); cb.stack_pop(phase2_condition.expr()); // Determine if the jump condition is met @@ -47,18 +46,20 @@ impl ExecutionGadget for JumpiGadget { // Lookup opcode at destination when should_jump cb.condition(should_jump.clone(), |cb| { - cb.opcode_lookup_at( - from_bytes::expr(&destination.cells), - OpcodeId::JUMPDEST.expr(), + cb.require_equal( + "JUMPI destination must be within range if condition is non-zero", + dest.not_overflow(), 1.expr(), ); + + cb.opcode_lookup_at(dest.valid_value(), OpcodeId::JUMPDEST.expr(), 1.expr()); }); // Transit program_counter to destination when should_jump, otherwise by // delta 1. let next_program_counter = select::expr( should_jump, - from_bytes::expr(&destination.cells), + dest.valid_value(), cb.curr.state.program_counter.expr() + 1.expr(), ); @@ -75,7 +76,7 @@ impl ExecutionGadget for JumpiGadget { Self { same_context, - destination, + dest, phase2_condition, is_condition_zero, } @@ -96,15 +97,7 @@ impl ExecutionGadget for JumpiGadget { [step.rw_indices[0], step.rw_indices[1]].map(|idx| block.rws[idx].stack_value()); let condition = region.word_rlc(condition); - self.destination.assign( - region, - offset, - Some( - destination.to_le_bytes()[..N_BYTES_PROGRAM_COUNTER] - .try_into() - .unwrap(), - ), - )?; + self.dest.assign(region, offset, destination)?; self.phase2_condition.assign(region, offset, condition)?; self.is_condition_zero .assign_value(region, offset, condition)?; @@ -174,4 +167,18 @@ mod test { test_ok(rand_range(1 << 11..0x5fff), Word::zero()); test_ok(rand_range(1 << 11..0x5fff), rand_word()); } + + #[test] + fn jumpi_gadget_with_zero_cond_and_overflow_dest() { + let bytecode = bytecode! { + PUSH32(Word::MAX) + PUSH32(Word::zero()) + JUMPI + }; + + CircuitTestBuilder::new_from_test_ctx( + TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), + ) + .run(); + } } diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index fe2a83d81a..6858c37d56 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -1,7 +1,7 @@ use super::{ constraint_builder::ConstrainBuilderCommon, from_bytes, - math_gadget::{IsEqualGadget, IsZeroGadget}, + math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget}, memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, CachedRegion, }; @@ -1037,3 +1037,121 @@ impl CommonErrorGadget { Ok(1u64) } } + +/// Check if the passed in word is within the specified byte range +/// (not overflow) and less than a maximum cap. +#[derive(Clone, Debug)] +pub(crate) struct WordByteCapGadget { + word: WordByteRangeGadget, + lt_cap: LtGadget, +} + +impl WordByteCapGadget { + pub(crate) fn construct(cb: &mut EVMConstraintBuilder, cap: Expression) -> Self { + let word = WordByteRangeGadget::construct(cb); + let value = select::expr(word.overflow(), cap.expr(), word.valid_value()); + let lt_cap = LtGadget::construct(cb, value, cap); + + Self { word, lt_cap } + } + + /// Return true if within the specified byte range (not overflow), false if + /// overflow. No matter whether it is less than the cap. + pub(crate) fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + original: U256, + cap: F, + ) -> Result { + let not_overflow = self.word.assign(region, offset, original)?; + + let value = if not_overflow { + let mut bytes = [0; 32]; + bytes[0..VALID_BYTES].copy_from_slice(&original.to_le_bytes()[0..VALID_BYTES]); + F::from_repr(bytes).unwrap() + } else { + cap + }; + + self.lt_cap.assign(region, offset, value, cap)?; + + Ok(not_overflow) + } + + pub(crate) fn lt_cap(&self) -> Expression { + self.lt_cap.expr() + } + + pub(crate) fn original_word(&self) -> Expression { + self.word.original_word() + } + + pub(crate) fn overflow(&self) -> Expression { + self.word.overflow() + } + + pub(crate) fn valid_value(&self) -> Expression { + self.word.valid_value() + } + + pub(crate) fn not_overflow(&self) -> Expression { + self.word.not_overflow() + } +} + +/// Check if the passed in word is within the specified byte range (not overflow). +#[derive(Clone, Debug)] +pub(crate) struct WordByteRangeGadget { + original: Word, + not_overflow: IsZeroGadget, +} + +impl WordByteRangeGadget { + pub(crate) fn construct(cb: &mut EVMConstraintBuilder) -> Self { + debug_assert!(VALID_BYTES < 32); + + let original = cb.query_word_rlc(); + let not_overflow = IsZeroGadget::construct(cb, sum::expr(&original.cells[VALID_BYTES..])); + + Self { + original, + not_overflow, + } + } + + /// Return true if within the range, false if overflow. + pub(crate) fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + original: U256, + ) -> Result { + self.original + .assign(region, offset, Some(original.to_le_bytes()))?; + + let overflow_hi = original.to_le_bytes()[VALID_BYTES..] + .iter() + .fold(0, |acc, val| acc + u64::from(*val)); + self.not_overflow + .assign(region, offset, F::from(overflow_hi))?; + + Ok(overflow_hi == 0) + } + + pub(crate) fn original_word(&self) -> Expression { + self.original.expr() + } + + pub(crate) fn overflow(&self) -> Expression { + not::expr(self.not_overflow()) + } + + pub(crate) fn valid_value(&self) -> Expression { + from_bytes::expr(&self.original.cells[..VALID_BYTES]) + } + + pub(crate) fn not_overflow(&self) -> Expression { + self.not_overflow.expr() + } +} diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index b4ccc4f0e1..2baa55690f 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -180,11 +180,11 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { OpcodeId::CODECOPY => ExecutionState::CODECOPY, OpcodeId::CALLDATALOAD => ExecutionState::CALLDATALOAD, OpcodeId::CODESIZE => ExecutionState::CODESIZE, + OpcodeId::EXTCODECOPY => ExecutionState::EXTCODECOPY, OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::RETURN_REVERT, OpcodeId::RETURNDATASIZE => ExecutionState::RETURNDATASIZE, OpcodeId::RETURNDATACOPY => ExecutionState::RETURNDATACOPY, // dummy ops - OpcodeId::EXTCODECOPY => dummy!(ExecutionState::EXTCODECOPY), OpcodeId::CREATE => dummy!(ExecutionState::CREATE), OpcodeId::CREATE2 => dummy!(ExecutionState::CREATE2), OpcodeId::SELFDESTRUCT => dummy!(ExecutionState::SELFDESTRUCT),