From 86adab374429c4f9c5adbb4871498fbb1fe2ed7f Mon Sep 17 00:00:00 2001 From: chad Date: Thu, 29 Feb 2024 22:22:13 -0500 Subject: [PATCH 1/8] refactor: use StepRws in evm circuits to replace rw indices (#1586) --- .../src/evm_circuit/execution/end_tx.rs | 25 ++++++-------- .../evm_circuit/execution/error_oog_call.rs | 34 +++++++++++-------- .../execution/error_oog_memory_copy.rs | 18 ++++++---- .../execution/error_oog_sload_sstore.rs | 15 +++++--- .../execution/error_return_data_oo_bound.rs | 12 ++++--- .../execution/error_write_protection.rs | 7 ++-- .../src/evm_circuit/execution/extcodecopy.rs | 12 ++++--- .../src/evm_circuit/execution/extcodehash.rs | 10 +++--- .../src/evm_circuit/execution/extcodesize.rs | 14 +++++--- .../src/evm_circuit/execution/logs.rs | 11 +++--- .../evm_circuit/execution/return_revert.rs | 10 +++--- .../evm_circuit/execution/returndatacopy.rs | 9 ++--- .../src/evm_circuit/execution/sha3.rs | 9 ++--- .../src/evm_circuit/execution/sload.rs | 19 ++++++++--- .../src/evm_circuit/execution/sstore.rs | 16 ++++++--- zkevm-circuits/src/evm_circuit/util.rs | 8 +++++ 16 files changed, 143 insertions(+), 86 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index 5f650f693a..80f5f9fd7d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -11,7 +11,7 @@ use crate::{ MulWordByU64Gadget, }, tx::EndTxHelperGadget, - CachedRegion, Cell, + CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -152,9 +152,12 @@ impl ExecutionGadget for EndTxGadget { step: &ExecStep, ) -> Result<(), Error> { let gas_used = tx.gas() - step.gas_left; - let (refund, _) = block.get_rws(step, 2).tx_refund_value_pair(); - let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_balance_pair(); - let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair(); + let mut rws = StepRws::new(block, step); + rws.offset_add(2); + + let (refund, _) = rws.next().tx_refund_value_pair(); + let (caller_balance, caller_balance_prev) = rws.next().account_balance_pair(); + let (coinbase_code_hash_prev, _) = rws.next().account_codehash_pair(); self.tx_id .assign(region, offset, Value::known(F::from(tx.id)))?; @@ -209,16 +212,10 @@ impl ExecutionGadget for EndTxGadget { self.coinbase_code_hash_is_zero .assign_u256(region, offset, coinbase_code_hash_prev)?; if !coinbase_reward.is_zero() { - let coinbase_balance_pair = block - .get_rws( - step, - if coinbase_code_hash_prev.is_zero() { - 6 - } else { - 5 - }, - ) - .account_balance_pair(); + if coinbase_code_hash_prev.is_zero() { + rws.next(); + } + let coinbase_balance_pair = rws.next().account_balance_pair(); self.coinbase_reward.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index 53b61484aa..2f37310b77 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{IsZeroGadget, LtGadget}, memory_gadget::MemoryExpandedAddressGadget, - or, CachedRegion, Cell, + or, CachedRegion, Cell, StepRws, }, }, table::CallContextFieldTag, @@ -126,26 +126,32 @@ impl ExecutionGadget for ErrorOOGCallGadget { let opcode = step.opcode().unwrap(); let is_call_or_callcode = usize::from([OpcodeId::CALL, OpcodeId::CALLCODE].contains(&opcode)); - let [tx_id, is_static] = - [0, 1].map(|index| block.get_rws(step, index).call_context_value()); - let [gas, callee_address] = [2, 3].map(|index| block.get_rws(step, index).stack_value()); + + let mut rws = StepRws::new(block, step); + + let tx_id = rws.next().call_context_value(); + let is_static = rws.next().call_context_value(); + + let gas = rws.next().stack_value(); + let callee_address = rws.next().stack_value(); + let value = if is_call_or_callcode == 1 { - block.get_rws(step, 4).stack_value() + rws.next().stack_value() } else { U256::zero() }; - let [cd_offset, cd_length, rd_offset, rd_length] = - [4, 5, 6, 7].map(|i| block.get_rws(step, is_call_or_callcode + i).stack_value()); - let callee_code_hash = block - .get_rws(step, 9 + is_call_or_callcode) - .account_codehash_pair() - .0; + let cd_offset = rws.next().stack_value(); + let cd_length = rws.next().stack_value(); + + let rd_offset = rws.next().stack_value(); + let rd_length = rws.next().stack_value(); + + let callee_code_hash = rws.next().account_codehash_pair().0; + let callee_exists = !callee_code_hash.is_zero(); - let (is_warm, is_warm_prev) = block - .get_rws(step, 10 + is_call_or_callcode) - .tx_access_list_value_pair(); + let (is_warm, is_warm_prev) = rws.next().tx_access_list_value_pair(); let memory_expansion_gas_cost = self.call.assign( region, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 697279a626..7a98b41a78 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -11,7 +11,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryCopierGasGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, }, - or, select, AccountAddress, CachedRegion, Cell, + or, select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -156,6 +156,8 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let opcode = step.opcode().unwrap(); let is_extcodecopy = opcode == OpcodeId::EXTCODECOPY; + let mut rws = StepRws::new(block, step); + log::debug!( "ErrorOutOfGasMemoryCopy: opcode = {}, gas_left = {}, gas_cost = {}", opcode, @@ -165,16 +167,20 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let (is_warm, external_address) = if is_extcodecopy { ( - block.get_rws(step, 1).tx_access_list_value_pair().0, - block.get_rws(step, 2).stack_value(), + rws.next().tx_access_list_value_pair().0, + rws.next().stack_value(), ) } else { (false, U256::zero()) }; - let rw_offset = if is_extcodecopy { 3 } else { 0 }; - let [dst_offset, src_offset, copy_size] = [rw_offset, rw_offset + 1, rw_offset + 2] - .map(|index| block.get_rws(step, index).stack_value()); + if is_extcodecopy { + rws.next(); // Skip external address + } + + let dst_offset = rws.next().stack_value(); + let src_offset = rws.next().stack_value(); + let copy_size = rws.next().stack_value(); self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs index b0b8bc97dc..5e8a2ab48e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs @@ -11,7 +11,7 @@ use crate::{ }, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{LtGadget, PairSelectGadget}, - or, select, CachedRegion, Cell, + or, select, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -161,12 +161,17 @@ impl ExecutionGadget for ErrorOOGSloadSstoreGadget { ) -> Result<(), Error> { let opcode = step.opcode().unwrap(); let is_sstore = opcode == OpcodeId::SSTORE; - let key = block.get_rws(step, 3).stack_value(); - let (is_warm, _) = block.get_rws(step, 4).tx_access_list_value_pair(); + + let mut rws = StepRws::new(block, step); + + rws.offset_add(3); + + let key = rws.next().stack_value(); + let (is_warm, _) = rws.next().tx_access_list_value_pair(); let (value, value_prev, original_value, gas_cost) = if is_sstore { - let value = block.get_rws(step, 5).stack_value(); - let (_, value_prev, _, original_value) = block.get_rws(step, 6).storage_value_aux(); + let value = rws.next().stack_value(); + let (_, value_prev, _, original_value) = rws.next().storage_value_aux(); let gas_cost = cal_sstore_gas_cost_for_assignment(value, value_prev, original_value, is_warm); (value, value_prev, original_value, gas_cost) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs b/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs index 52a9839239..25ee698337 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, from_bytes, math_gadget::{AddWordsGadget, IsZeroGadget, LtGadget}, - not, or, sum, CachedRegion, Cell, U64Cell, + not, or, sum, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -130,8 +130,11 @@ impl ExecutionGadget for ErrorReturnDataOutOfBoundGadget { self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; - let [dest_offset, data_offset, size] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let dest_offset = rws.next().stack_value(); + let data_offset = rws.next().stack_value(); + let size = rws.next().stack_value(); self.memory_offset .assign(region, offset, Some(dest_offset.as_u64().to_le_bytes()))?; @@ -140,7 +143,8 @@ impl ExecutionGadget for ErrorReturnDataOutOfBoundGadget { self.sum .assign(region, offset, [data_offset, size], remainder_end)?; - let return_data_length = block.get_rws(step, 3).call_context_value(); + let return_data_length = rws.next().call_context_value(); + self.return_data_length.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs index 531bdc9813..a7d557b301 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs @@ -6,7 +6,7 @@ use crate::{ common_gadget::CommonErrorGadget, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{IsEqualGadget, IsZeroWordGadget}, - AccountAddress, CachedRegion, Cell, + AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -109,9 +109,10 @@ impl ExecutionGadget for ErrorWriteProtectionGadget { .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; let [mut gas, mut code_address, mut value] = [U256::zero(), U256::zero(), U256::zero()]; + let mut rws = StepRws::new(block, step); + if is_call { - [gas, code_address, value] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + [gas, code_address, value] = [0, 1, 2].map(|_| rws.next().stack_value()); } self.gas.assign_u256(region, offset, gas)?; diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 5cda8007d8..3e07a65739 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -13,7 +13,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - not, select, AccountAddress, CachedRegion, Cell, + not, select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -182,8 +182,10 @@ impl ExecutionGadget for ExtcodecopyGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; + let mut rws = StepRws::new(block, step); + let [external_address, memory_offset, code_offset, memory_length] = - [0, 1, 2, 3].map(|idx| block.get_rws(step, idx).stack_value()); + [0, 1, 2, 3].map(|_| rws.next().stack_value()); self.external_address_word .assign_u256(region, offset, external_address)?; let memory_address = @@ -199,11 +201,13 @@ impl ExecutionGadget for ExtcodecopyGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 7).tx_access_list_value_pair(); + rws.offset_add(3); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 8).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; self.not_exists.assign_u256(region, offset, code_hash)?; diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs index 99cd8b5b6a..98bc1ee0f4 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - select, AccountAddress, CachedRegion, Cell, + select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -106,7 +106,9 @@ impl ExecutionGadget for ExtcodehashGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let address = block.get_rws(step, 0).stack_value(); + let mut rws = StepRws::new(block, step); + + let address = rws.next().stack_value(); self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -118,11 +120,11 @@ impl ExecutionGadget for ExtcodehashGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 5).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs b/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs index 908b1e2b7b..443990c42d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs @@ -10,7 +10,7 @@ use crate::{ Transition::Delta, }, math_gadget::IsZeroWordGadget, - not, select, AccountAddress, CachedRegion, Cell, U64Cell, + not, select, AccountAddress, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -123,7 +123,9 @@ impl ExecutionGadget for ExtcodesizeGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let address = block.get_rws(step, 0).stack_value(); + let mut rws = StepRws::new(block, step); + + let address = rws.next().stack_value(); self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -136,16 +138,18 @@ impl ExecutionGadget for ExtcodesizeGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); + rws.offset_add(3); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 5).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; self.not_exists .assign(region, offset, WordLoHi::from(code_hash))?; - let code_size = block.get_rws(step, 6).stack_value().as_u64(); + let code_size = rws.next().stack_value().as_u64(); self.code_size .assign(region, offset, Some(code_size.to_le_bytes()))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/logs.rs b/zkevm-circuits/src/evm_circuit/execution/logs.rs index 327cae451f..55d3c67d4f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/logs.rs +++ b/zkevm-circuits/src/evm_circuit/execution/logs.rs @@ -12,7 +12,7 @@ use crate::{ memory_gadget::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, }, - not, sum, CachedRegion, Cell, + not, sum, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -201,7 +201,9 @@ impl ExecutionGadget for LogGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [memory_start, msize] = [0, 1].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_start, msize] = [0, 1].map(|_| rws.next().stack_value()); let memory_address = self .memory_address .assign(region, offset, memory_start, msize)?; @@ -221,9 +223,8 @@ impl ExecutionGadget for LogGadget { // It takes 6 + is_persistent reads or writes to reach the topic stack write section. // Each topic takes at least 1 stack read. They take an additional tx log write if the // call is persistent. - block - .get_rws(step, 6 + is_persistent + topic * (1 + is_persistent)) - .stack_value() + rws.offset_add(6 + is_persistent + topic * (1 + is_persistent)); + rws.next().stack_value() }); for i in 0..4 { let topic = topics.next(); diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index 02a1a5854a..b6f720510a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -13,7 +13,7 @@ use crate::{ memory_gadget::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, }, - not, CachedRegion, Cell, + not, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -292,7 +292,9 @@ impl ExecutionGadget for ReturnRevertGadget { Value::known(F::from(step.opcode().unwrap().as_u64())), )?; - let [memory_offset, length] = [0, 1].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_offset, length] = [0, 1].map(|_| rws.next().stack_value()); let range = self.range.assign(region, offset, memory_offset, length)?; self.memory_expansion .assign(region, offset, step.memory_word_size(), [range])?; @@ -321,7 +323,7 @@ impl ExecutionGadget for ReturnRevertGadget { if call.is_create() && call.is_success { let values: Vec<_> = (4..4 + length.as_usize()) - .map(|index| block.get_rws(step, index).memory_value()) + .map(|_| rws.next().memory_value()) .collect(); self.deployed_code_rlc.assign( region, @@ -349,7 +351,7 @@ impl ExecutionGadget for ReturnRevertGadget { let is_contract_deployment = call.is_create() && call.is_success && !length.is_zero(); let init_code_first_byte = if is_contract_deployment { - block.get_rws(step, 3).memory_value() + rws.next().memory_value() } else { 0 } diff --git a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs index 7e1a8b57aa..01537fd613 100644 --- a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs @@ -14,7 +14,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - CachedRegion, Cell, MemoryAddress, + CachedRegion, Cell, MemoryAddress, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -176,8 +176,9 @@ impl ExecutionGadget for ReturnDataCopyGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [dest_offset, data_offset, size] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [dest_offset, data_offset, size] = [0, 1, 2].map(|_| rws.next().stack_value()); self.data_offset.assign_u256(region, offset, data_offset)?; @@ -187,7 +188,7 @@ impl ExecutionGadget for ReturnDataCopyGadget { (5, CallContextFieldTag::LastCalleeReturnDataLength), ] .map(|(i, tag)| { - let rw = block.get_rws(step, i); + let rw = rws.next(); assert_eq!(rw.field_tag(), Some(tag as u64)); rw.call_context_value() }); diff --git a/zkevm-circuits/src/evm_circuit/execution/sha3.rs b/zkevm-circuits/src/evm_circuit/execution/sha3.rs index 12cbac2ee4..1d7125e60f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sha3.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sha3.rs @@ -16,7 +16,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - rlc, CachedRegion, Cell, + rlc, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -123,8 +123,9 @@ impl ExecutionGadget for Sha3Gadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [memory_offset, size, sha3_output] = - [0, 1, 2].map(|idx| block.get_rws(step, idx).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_offset, size, sha3_output] = [0, 1, 2].map(|_| rws.next().stack_value()); let memory_address = self .memory_address .assign(region, offset, memory_offset, size)?; @@ -140,7 +141,7 @@ impl ExecutionGadget for Sha3Gadget { )?; let values: Vec = (3..3 + (size.low_u64() as usize)) - .map(|i| block.get_rws(step, i).memory_value()) + .map(|_| rws.next().memory_value()) .collect(); let rlc_acc = region diff --git a/zkevm-circuits/src/evm_circuit/execution/sload.rs b/zkevm-circuits/src/evm_circuit/execution/sload.rs index 7a222c9b44..aceb8ddb3c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sload.rs @@ -7,7 +7,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - CachedRegion, Cell, + CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -120,16 +120,25 @@ impl ExecutionGadget for SloadGadget { self.callee_address .assign_h160(region, offset, call.address)?; - let key = block.get_rws(step, 4).stack_value(); - let value = block.get_rws(step, 6).stack_value(); + let mut rws = StepRws::new(block, step); + + rws.offset_add(4); + + let key = rws.next().stack_value(); + + let (_, committed_value) = rws.next().aux_pair(); + + let value = rws.next().stack_value(); + self.key.assign_u256(region, offset, key)?; self.value.assign_u256(region, offset, value)?; - let (_, committed_value) = block.get_rws(step, 5).aux_pair(); self.committed_value .assign_u256(region, offset, committed_value)?; - let (_, is_warm) = block.get_rws(step, 8).tx_access_list_value_pair(); + rws.next(); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index ebcb7b52db..f49d74b143 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -10,7 +10,7 @@ use crate::{ Transition::Delta, }, math_gadget::{IsEqualWordGadget, IsZeroWordGadget, LtGadget}, - not, CachedRegion, Cell, U64Cell, + not, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -186,17 +186,23 @@ impl ExecutionGadget for SstoreGadget { self.callee_address .assign_h160(region, offset, call.address)?; - let key = block.get_rws(step, 5).stack_value(); - let value = block.get_rws(step, 6).stack_value(); + let mut rws = StepRws::new(block, step); + + rws.offset_add(5); + + let key = rws.next().stack_value(); + let value = rws.next().stack_value(); self.key.assign_u256(region, offset, key)?; self.value.assign_u256(region, offset, value)?; - let (_, value_prev, _, original_value) = block.get_rws(step, 7).storage_value_aux(); + let (_, value_prev, _, original_value) = rws.next().storage_value_aux(); self.value_prev.assign_u256(region, offset, value_prev)?; self.original_value .assign_u256(region, offset, original_value)?; - let (_, is_warm) = block.get_rws(step, 9).tx_access_list_value_pair(); + rws.next(); // skip the storage write + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index 1b58790914..8c24158390 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -475,6 +475,14 @@ impl<'a> StepRws<'a> { self.offset += 1; rw } + /// Return the next rw operations over the next `n` steps. + pub(crate) fn next_n(&mut self, n: usize) -> Vec { + let mut rws = Vec::with_capacity(n); + for _ in 0..n { + rws.push(self.next()); + } + rws + } } #[cfg(test)] From abf581e6b5dc985c9aaab100ebb5011eccae6e19 Mon Sep 17 00:00:00 2001 From: chad Date: Sun, 3 Mar 2024 17:38:30 -0500 Subject: [PATCH 2/8] fix: resolve issue with return_revert offset --- .../src/evm_circuit/execution/end_tx.rs | 25 ++++++-------- .../evm_circuit/execution/error_oog_call.rs | 34 +++++++++++-------- .../execution/error_oog_memory_copy.rs | 18 ++++++---- .../execution/error_oog_sload_sstore.rs | 15 +++++--- .../execution/error_return_data_oo_bound.rs | 12 ++++--- .../execution/error_write_protection.rs | 7 ++-- .../src/evm_circuit/execution/extcodecopy.rs | 12 ++++--- .../src/evm_circuit/execution/extcodehash.rs | 10 +++--- .../src/evm_circuit/execution/extcodesize.rs | 14 +++++--- .../src/evm_circuit/execution/logs.rs | 11 +++--- .../evm_circuit/execution/return_revert.rs | 11 ++++-- .../evm_circuit/execution/returndatacopy.rs | 9 ++--- .../src/evm_circuit/execution/sha3.rs | 9 ++--- .../src/evm_circuit/execution/sload.rs | 19 ++++++++--- .../src/evm_circuit/execution/sstore.rs | 16 ++++++--- 15 files changed, 137 insertions(+), 85 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index 5f650f693a..80f5f9fd7d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -11,7 +11,7 @@ use crate::{ MulWordByU64Gadget, }, tx::EndTxHelperGadget, - CachedRegion, Cell, + CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -152,9 +152,12 @@ impl ExecutionGadget for EndTxGadget { step: &ExecStep, ) -> Result<(), Error> { let gas_used = tx.gas() - step.gas_left; - let (refund, _) = block.get_rws(step, 2).tx_refund_value_pair(); - let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_balance_pair(); - let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair(); + let mut rws = StepRws::new(block, step); + rws.offset_add(2); + + let (refund, _) = rws.next().tx_refund_value_pair(); + let (caller_balance, caller_balance_prev) = rws.next().account_balance_pair(); + let (coinbase_code_hash_prev, _) = rws.next().account_codehash_pair(); self.tx_id .assign(region, offset, Value::known(F::from(tx.id)))?; @@ -209,16 +212,10 @@ impl ExecutionGadget for EndTxGadget { self.coinbase_code_hash_is_zero .assign_u256(region, offset, coinbase_code_hash_prev)?; if !coinbase_reward.is_zero() { - let coinbase_balance_pair = block - .get_rws( - step, - if coinbase_code_hash_prev.is_zero() { - 6 - } else { - 5 - }, - ) - .account_balance_pair(); + if coinbase_code_hash_prev.is_zero() { + rws.next(); + } + let coinbase_balance_pair = rws.next().account_balance_pair(); self.coinbase_reward.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index 53b61484aa..2f37310b77 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{IsZeroGadget, LtGadget}, memory_gadget::MemoryExpandedAddressGadget, - or, CachedRegion, Cell, + or, CachedRegion, Cell, StepRws, }, }, table::CallContextFieldTag, @@ -126,26 +126,32 @@ impl ExecutionGadget for ErrorOOGCallGadget { let opcode = step.opcode().unwrap(); let is_call_or_callcode = usize::from([OpcodeId::CALL, OpcodeId::CALLCODE].contains(&opcode)); - let [tx_id, is_static] = - [0, 1].map(|index| block.get_rws(step, index).call_context_value()); - let [gas, callee_address] = [2, 3].map(|index| block.get_rws(step, index).stack_value()); + + let mut rws = StepRws::new(block, step); + + let tx_id = rws.next().call_context_value(); + let is_static = rws.next().call_context_value(); + + let gas = rws.next().stack_value(); + let callee_address = rws.next().stack_value(); + let value = if is_call_or_callcode == 1 { - block.get_rws(step, 4).stack_value() + rws.next().stack_value() } else { U256::zero() }; - let [cd_offset, cd_length, rd_offset, rd_length] = - [4, 5, 6, 7].map(|i| block.get_rws(step, is_call_or_callcode + i).stack_value()); - let callee_code_hash = block - .get_rws(step, 9 + is_call_or_callcode) - .account_codehash_pair() - .0; + let cd_offset = rws.next().stack_value(); + let cd_length = rws.next().stack_value(); + + let rd_offset = rws.next().stack_value(); + let rd_length = rws.next().stack_value(); + + let callee_code_hash = rws.next().account_codehash_pair().0; + let callee_exists = !callee_code_hash.is_zero(); - let (is_warm, is_warm_prev) = block - .get_rws(step, 10 + is_call_or_callcode) - .tx_access_list_value_pair(); + let (is_warm, is_warm_prev) = rws.next().tx_access_list_value_pair(); let memory_expansion_gas_cost = self.call.assign( region, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 697279a626..7a98b41a78 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -11,7 +11,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryCopierGasGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, }, - or, select, AccountAddress, CachedRegion, Cell, + or, select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -156,6 +156,8 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let opcode = step.opcode().unwrap(); let is_extcodecopy = opcode == OpcodeId::EXTCODECOPY; + let mut rws = StepRws::new(block, step); + log::debug!( "ErrorOutOfGasMemoryCopy: opcode = {}, gas_left = {}, gas_cost = {}", opcode, @@ -165,16 +167,20 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let (is_warm, external_address) = if is_extcodecopy { ( - block.get_rws(step, 1).tx_access_list_value_pair().0, - block.get_rws(step, 2).stack_value(), + rws.next().tx_access_list_value_pair().0, + rws.next().stack_value(), ) } else { (false, U256::zero()) }; - let rw_offset = if is_extcodecopy { 3 } else { 0 }; - let [dst_offset, src_offset, copy_size] = [rw_offset, rw_offset + 1, rw_offset + 2] - .map(|index| block.get_rws(step, index).stack_value()); + if is_extcodecopy { + rws.next(); // Skip external address + } + + let dst_offset = rws.next().stack_value(); + let src_offset = rws.next().stack_value(); + let copy_size = rws.next().stack_value(); self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs index b0b8bc97dc..5e8a2ab48e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_sload_sstore.rs @@ -11,7 +11,7 @@ use crate::{ }, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{LtGadget, PairSelectGadget}, - or, select, CachedRegion, Cell, + or, select, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -161,12 +161,17 @@ impl ExecutionGadget for ErrorOOGSloadSstoreGadget { ) -> Result<(), Error> { let opcode = step.opcode().unwrap(); let is_sstore = opcode == OpcodeId::SSTORE; - let key = block.get_rws(step, 3).stack_value(); - let (is_warm, _) = block.get_rws(step, 4).tx_access_list_value_pair(); + + let mut rws = StepRws::new(block, step); + + rws.offset_add(3); + + let key = rws.next().stack_value(); + let (is_warm, _) = rws.next().tx_access_list_value_pair(); let (value, value_prev, original_value, gas_cost) = if is_sstore { - let value = block.get_rws(step, 5).stack_value(); - let (_, value_prev, _, original_value) = block.get_rws(step, 6).storage_value_aux(); + let value = rws.next().stack_value(); + let (_, value_prev, _, original_value) = rws.next().storage_value_aux(); let gas_cost = cal_sstore_gas_cost_for_assignment(value, value_prev, original_value, is_warm); (value, value_prev, original_value, gas_cost) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs b/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs index 52a9839239..25ee698337 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_return_data_oo_bound.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, from_bytes, math_gadget::{AddWordsGadget, IsZeroGadget, LtGadget}, - not, or, sum, CachedRegion, Cell, U64Cell, + not, or, sum, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -130,8 +130,11 @@ impl ExecutionGadget for ErrorReturnDataOutOfBoundGadget { self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; - let [dest_offset, data_offset, size] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let dest_offset = rws.next().stack_value(); + let data_offset = rws.next().stack_value(); + let size = rws.next().stack_value(); self.memory_offset .assign(region, offset, Some(dest_offset.as_u64().to_le_bytes()))?; @@ -140,7 +143,8 @@ impl ExecutionGadget for ErrorReturnDataOutOfBoundGadget { self.sum .assign(region, offset, [data_offset, size], remainder_end)?; - let return_data_length = block.get_rws(step, 3).call_context_value(); + let return_data_length = rws.next().call_context_value(); + self.return_data_length.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs index 531bdc9813..a7d557b301 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs @@ -6,7 +6,7 @@ use crate::{ common_gadget::CommonErrorGadget, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{IsEqualGadget, IsZeroWordGadget}, - AccountAddress, CachedRegion, Cell, + AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -109,9 +109,10 @@ impl ExecutionGadget for ErrorWriteProtectionGadget { .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; let [mut gas, mut code_address, mut value] = [U256::zero(), U256::zero(), U256::zero()]; + let mut rws = StepRws::new(block, step); + if is_call { - [gas, code_address, value] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + [gas, code_address, value] = [0, 1, 2].map(|_| rws.next().stack_value()); } self.gas.assign_u256(region, offset, gas)?; diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 5cda8007d8..3e07a65739 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -13,7 +13,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - not, select, AccountAddress, CachedRegion, Cell, + not, select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -182,8 +182,10 @@ impl ExecutionGadget for ExtcodecopyGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; + let mut rws = StepRws::new(block, step); + let [external_address, memory_offset, code_offset, memory_length] = - [0, 1, 2, 3].map(|idx| block.get_rws(step, idx).stack_value()); + [0, 1, 2, 3].map(|_| rws.next().stack_value()); self.external_address_word .assign_u256(region, offset, external_address)?; let memory_address = @@ -199,11 +201,13 @@ impl ExecutionGadget for ExtcodecopyGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 7).tx_access_list_value_pair(); + rws.offset_add(3); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 8).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; self.not_exists.assign_u256(region, offset, code_hash)?; diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs index 99cd8b5b6a..98bc1ee0f4 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - select, AccountAddress, CachedRegion, Cell, + select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -106,7 +106,9 @@ impl ExecutionGadget for ExtcodehashGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let address = block.get_rws(step, 0).stack_value(); + let mut rws = StepRws::new(block, step); + + let address = rws.next().stack_value(); self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -118,11 +120,11 @@ impl ExecutionGadget for ExtcodehashGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 5).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs b/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs index 908b1e2b7b..443990c42d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodesize.rs @@ -10,7 +10,7 @@ use crate::{ Transition::Delta, }, math_gadget::IsZeroWordGadget, - not, select, AccountAddress, CachedRegion, Cell, U64Cell, + not, select, AccountAddress, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -123,7 +123,9 @@ impl ExecutionGadget for ExtcodesizeGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let address = block.get_rws(step, 0).stack_value(); + let mut rws = StepRws::new(block, step); + + let address = rws.next().stack_value(); self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -136,16 +138,18 @@ impl ExecutionGadget for ExtcodesizeGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); + rws.offset_add(3); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 5).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; self.not_exists .assign(region, offset, WordLoHi::from(code_hash))?; - let code_size = block.get_rws(step, 6).stack_value().as_u64(); + let code_size = rws.next().stack_value().as_u64(); self.code_size .assign(region, offset, Some(code_size.to_le_bytes()))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/logs.rs b/zkevm-circuits/src/evm_circuit/execution/logs.rs index 327cae451f..55d3c67d4f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/logs.rs +++ b/zkevm-circuits/src/evm_circuit/execution/logs.rs @@ -12,7 +12,7 @@ use crate::{ memory_gadget::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, }, - not, sum, CachedRegion, Cell, + not, sum, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -201,7 +201,9 @@ impl ExecutionGadget for LogGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [memory_start, msize] = [0, 1].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_start, msize] = [0, 1].map(|_| rws.next().stack_value()); let memory_address = self .memory_address .assign(region, offset, memory_start, msize)?; @@ -221,9 +223,8 @@ impl ExecutionGadget for LogGadget { // It takes 6 + is_persistent reads or writes to reach the topic stack write section. // Each topic takes at least 1 stack read. They take an additional tx log write if the // call is persistent. - block - .get_rws(step, 6 + is_persistent + topic * (1 + is_persistent)) - .stack_value() + rws.offset_add(6 + is_persistent + topic * (1 + is_persistent)); + rws.next().stack_value() }); for i in 0..4 { let topic = topics.next(); diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index 02a1a5854a..3d8b4a1d5f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -13,7 +13,7 @@ use crate::{ memory_gadget::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, }, - not, CachedRegion, Cell, + not, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -292,7 +292,10 @@ impl ExecutionGadget for ReturnRevertGadget { Value::known(F::from(step.opcode().unwrap().as_u64())), )?; - let [memory_offset, length] = [0, 1].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_offset, length] = [0, 1].map(|_| rws.next().stack_value()); + let range = self.range.assign(region, offset, memory_offset, length)?; self.memory_expansion .assign(region, offset, step.memory_word_size(), [range])?; @@ -348,8 +351,10 @@ impl ExecutionGadget for ReturnRevertGadget { let is_contract_deployment = call.is_create() && call.is_success && !length.is_zero(); + rws.next(); + let init_code_first_byte = if is_contract_deployment { - block.get_rws(step, 3).memory_value() + rws.next().memory_value() } else { 0 } diff --git a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs index 7e1a8b57aa..01537fd613 100644 --- a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs @@ -14,7 +14,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - CachedRegion, Cell, MemoryAddress, + CachedRegion, Cell, MemoryAddress, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -176,8 +176,9 @@ impl ExecutionGadget for ReturnDataCopyGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [dest_offset, data_offset, size] = - [0, 1, 2].map(|index| block.get_rws(step, index).stack_value()); + let mut rws = StepRws::new(block, step); + + let [dest_offset, data_offset, size] = [0, 1, 2].map(|_| rws.next().stack_value()); self.data_offset.assign_u256(region, offset, data_offset)?; @@ -187,7 +188,7 @@ impl ExecutionGadget for ReturnDataCopyGadget { (5, CallContextFieldTag::LastCalleeReturnDataLength), ] .map(|(i, tag)| { - let rw = block.get_rws(step, i); + let rw = rws.next(); assert_eq!(rw.field_tag(), Some(tag as u64)); rw.call_context_value() }); diff --git a/zkevm-circuits/src/evm_circuit/execution/sha3.rs b/zkevm-circuits/src/evm_circuit/execution/sha3.rs index 12cbac2ee4..1d7125e60f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sha3.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sha3.rs @@ -16,7 +16,7 @@ use crate::{ CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget, }, - rlc, CachedRegion, Cell, + rlc, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -123,8 +123,9 @@ impl ExecutionGadget for Sha3Gadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let [memory_offset, size, sha3_output] = - [0, 1, 2].map(|idx| block.get_rws(step, idx).stack_value()); + let mut rws = StepRws::new(block, step); + + let [memory_offset, size, sha3_output] = [0, 1, 2].map(|_| rws.next().stack_value()); let memory_address = self .memory_address .assign(region, offset, memory_offset, size)?; @@ -140,7 +141,7 @@ impl ExecutionGadget for Sha3Gadget { )?; let values: Vec = (3..3 + (size.low_u64() as usize)) - .map(|i| block.get_rws(step, i).memory_value()) + .map(|_| rws.next().memory_value()) .collect(); let rlc_acc = region diff --git a/zkevm-circuits/src/evm_circuit/execution/sload.rs b/zkevm-circuits/src/evm_circuit/execution/sload.rs index 7a222c9b44..aceb8ddb3c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sload.rs @@ -7,7 +7,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - CachedRegion, Cell, + CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -120,16 +120,25 @@ impl ExecutionGadget for SloadGadget { self.callee_address .assign_h160(region, offset, call.address)?; - let key = block.get_rws(step, 4).stack_value(); - let value = block.get_rws(step, 6).stack_value(); + let mut rws = StepRws::new(block, step); + + rws.offset_add(4); + + let key = rws.next().stack_value(); + + let (_, committed_value) = rws.next().aux_pair(); + + let value = rws.next().stack_value(); + self.key.assign_u256(region, offset, key)?; self.value.assign_u256(region, offset, value)?; - let (_, committed_value) = block.get_rws(step, 5).aux_pair(); self.committed_value .assign_u256(region, offset, committed_value)?; - let (_, is_warm) = block.get_rws(step, 8).tx_access_list_value_pair(); + rws.next(); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index ebcb7b52db..f49d74b143 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -10,7 +10,7 @@ use crate::{ Transition::Delta, }, math_gadget::{IsEqualWordGadget, IsZeroWordGadget, LtGadget}, - not, CachedRegion, Cell, U64Cell, + not, CachedRegion, Cell, StepRws, U64Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -186,17 +186,23 @@ impl ExecutionGadget for SstoreGadget { self.callee_address .assign_h160(region, offset, call.address)?; - let key = block.get_rws(step, 5).stack_value(); - let value = block.get_rws(step, 6).stack_value(); + let mut rws = StepRws::new(block, step); + + rws.offset_add(5); + + let key = rws.next().stack_value(); + let value = rws.next().stack_value(); self.key.assign_u256(region, offset, key)?; self.value.assign_u256(region, offset, value)?; - let (_, value_prev, _, original_value) = block.get_rws(step, 7).storage_value_aux(); + let (_, value_prev, _, original_value) = rws.next().storage_value_aux(); self.value_prev.assign_u256(region, offset, value_prev)?; self.original_value .assign_u256(region, offset, original_value)?; - let (_, is_warm) = block.get_rws(step, 9).tx_access_list_value_pair(); + rws.next(); // skip the storage write + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; From 0b4282f4015d25616b140827e0fb18d2782b6c35 Mon Sep 17 00:00:00 2001 From: chad Date: Sun, 3 Mar 2024 17:40:14 -0500 Subject: [PATCH 3/8] refactor: remove unused method --- zkevm-circuits/src/evm_circuit/util.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index 8c24158390..1b58790914 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -475,14 +475,6 @@ impl<'a> StepRws<'a> { self.offset += 1; rw } - /// Return the next rw operations over the next `n` steps. - pub(crate) fn next_n(&mut self, n: usize) -> Vec { - let mut rws = Vec::with_capacity(n); - for _ in 0..n { - rws.push(self.next()); - } - rws - } } #[cfg(test)] From 6defa55026a10c0da42ad4a4b142d2561674e040 Mon Sep 17 00:00:00 2001 From: chad Date: Sun, 3 Mar 2024 18:41:57 -0500 Subject: [PATCH 4/8] fix lint issue --- zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs index 01537fd613..89d8f7f42f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs @@ -187,7 +187,7 @@ impl ExecutionGadget for ReturnDataCopyGadget { (4, CallContextFieldTag::LastCalleeReturnDataOffset), (5, CallContextFieldTag::LastCalleeReturnDataLength), ] - .map(|(i, tag)| { + .map(|(_, tag)| { let rw = rws.next(); assert_eq!(rw.field_tag(), Some(tag as u64)); rw.call_context_value() From 4a6de58d2efda68842d62e833c439dbd9de9de32 Mon Sep 17 00:00:00 2001 From: chad Date: Sun, 17 Mar 2024 15:15:01 -0500 Subject: [PATCH 5/8] fix: update rws steps due to broken tests --- .../src/evm_circuit/execution/end_tx.rs | 25 +++++++++++-------- .../evm_circuit/execution/error_oog_call.rs | 9 ++++++- .../execution/error_oog_memory_copy.rs | 9 ++++--- .../src/evm_circuit/execution/extcodecopy.rs | 9 ++++--- .../src/evm_circuit/execution/extcodehash.rs | 10 +++----- .../evm_circuit/execution/return_revert.rs | 5 ++-- zkevm-circuits/src/evm_circuit/util.rs | 8 ++++++ 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index 80f5f9fd7d..5f650f693a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -11,7 +11,7 @@ use crate::{ MulWordByU64Gadget, }, tx::EndTxHelperGadget, - CachedRegion, Cell, StepRws, + CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -152,12 +152,9 @@ impl ExecutionGadget for EndTxGadget { step: &ExecStep, ) -> Result<(), Error> { let gas_used = tx.gas() - step.gas_left; - let mut rws = StepRws::new(block, step); - rws.offset_add(2); - - let (refund, _) = rws.next().tx_refund_value_pair(); - let (caller_balance, caller_balance_prev) = rws.next().account_balance_pair(); - let (coinbase_code_hash_prev, _) = rws.next().account_codehash_pair(); + let (refund, _) = block.get_rws(step, 2).tx_refund_value_pair(); + let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_balance_pair(); + let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair(); self.tx_id .assign(region, offset, Value::known(F::from(tx.id)))?; @@ -212,10 +209,16 @@ impl ExecutionGadget for EndTxGadget { self.coinbase_code_hash_is_zero .assign_u256(region, offset, coinbase_code_hash_prev)?; if !coinbase_reward.is_zero() { - if coinbase_code_hash_prev.is_zero() { - rws.next(); - } - let coinbase_balance_pair = rws.next().account_balance_pair(); + let coinbase_balance_pair = block + .get_rws( + step, + if coinbase_code_hash_prev.is_zero() { + 6 + } else { + 5 + }, + ) + .account_balance_pair(); self.coinbase_reward.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index 2f37310b77..14d4def2a9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -141,12 +141,19 @@ impl ExecutionGadget for ErrorOOGCallGadget { U256::zero() }; + if is_call_or_callcode == 1 { + rws.offset_sub(1); + } + + rws.offset_add(is_call_or_callcode); + let cd_offset = rws.next().stack_value(); let cd_length = rws.next().stack_value(); - let rd_offset = rws.next().stack_value(); let rd_length = rws.next().stack_value(); + rws.offset_add(1); + let callee_code_hash = rws.next().account_codehash_pair().0; let callee_exists = !callee_code_hash.is_zero(); diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 7a98b41a78..96029ee37a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -166,6 +166,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { ); let (is_warm, external_address) = if is_extcodecopy { + rws.next(); ( rws.next().tx_access_list_value_pair().0, rws.next().stack_value(), @@ -174,12 +175,14 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { (false, U256::zero()) }; - if is_extcodecopy { - rws.next(); // Skip external address - } + if !is_extcodecopy { + rws.offset_reset(); + }; let dst_offset = rws.next().stack_value(); + let src_offset = rws.next().stack_value(); + let copy_size = rws.next().stack_value(); self.opcode diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 3e07a65739..6a5e4568d7 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -184,8 +184,11 @@ impl ExecutionGadget for ExtcodecopyGadget { let mut rws = StepRws::new(block, step); - let [external_address, memory_offset, code_offset, memory_length] = - [0, 1, 2, 3].map(|_| rws.next().stack_value()); + let external_address = rws.next().stack_value(); + let memory_offset = rws.next().stack_value(); + let code_offset = rws.next().stack_value(); + let memory_length = rws.next().stack_value(); + self.external_address_word .assign_u256(region, offset, external_address)?; let memory_address = @@ -201,7 +204,7 @@ impl ExecutionGadget for ExtcodecopyGadget { call.is_persistent, )?; - rws.offset_add(3); + rws.offset_add(4); let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs index 98bc1ee0f4..99cd8b5b6a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - select, AccountAddress, CachedRegion, Cell, StepRws, + select, AccountAddress, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -106,9 +106,7 @@ impl ExecutionGadget for ExtcodehashGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let mut rws = StepRws::new(block, step); - - let address = rws.next().stack_value(); + let address = block.get_rws(step, 0).stack_value(); self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -120,11 +118,11 @@ impl ExecutionGadget for ExtcodehashGadget { call.is_persistent, )?; - let (_, is_warm) = rws.next().tx_access_list_value_pair(); + let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = rws.next().account_codehash_pair().0; + let code_hash = block.get_rws(step, 5).account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index aab0ff6b85..7043642b6c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -294,7 +294,8 @@ impl ExecutionGadget for ReturnRevertGadget { let mut rws = StepRws::new(block, step); - let [memory_offset, length] = [0, 1].map(|_| rws.next().stack_value()); + let memory_offset = rws.next().stack_value(); + let length = rws.next().stack_value(); let range = self.range.assign(region, offset, memory_offset, length)?; self.memory_expansion @@ -324,7 +325,7 @@ impl ExecutionGadget for ReturnRevertGadget { if call.is_create() && call.is_success { let values: Vec<_> = (4..4 + length.as_usize()) - .map(|_| rws.next().memory_value()) + .map(|index| block.get_rws(step, index).memory_value()) .collect(); self.deployed_code_rlc.assign( region, diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index 1b58790914..e6fde220d5 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -469,6 +469,14 @@ impl<'a> StepRws<'a> { pub(crate) fn offset_add(&mut self, inc: usize) { self.offset += inc } + /// Decrement the step rw operation offset by `dec`. + pub(crate) fn offset_sub(&mut self, dec: usize) { + self.offset -= dec + } + /// Reset the step rw operation offset to 0. + pub(crate) fn offset_reset(&mut self) { + self.offset = 0 + } /// Return the next rw operation from the step. pub(crate) fn next(&mut self) -> Rw { let rw = self.rws[self.step.rw_index(self.offset)]; From 8dca7b445b2c76c6ea73deee8b7e1ebe60fac721 Mon Sep 17 00:00:00 2001 From: chad Date: Tue, 19 Mar 2024 17:14:44 -0400 Subject: [PATCH 6/8] updated broken indices --- .../src/evm_circuit/execution/end_tx.rs | 27 +++++++++---------- .../src/evm_circuit/execution/extcodecopy.rs | 2 +- .../src/evm_circuit/execution/extcodehash.rs | 13 ++++++--- .../src/evm_circuit/execution/logs.rs | 9 ++++--- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index 5f650f693a..cb35197d8a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -11,7 +11,7 @@ use crate::{ MulWordByU64Gadget, }, tx::EndTxHelperGadget, - CachedRegion, Cell, + CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -152,9 +152,14 @@ impl ExecutionGadget for EndTxGadget { step: &ExecStep, ) -> Result<(), Error> { let gas_used = tx.gas() - step.gas_left; - let (refund, _) = block.get_rws(step, 2).tx_refund_value_pair(); - let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_balance_pair(); - let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair(); + + let mut rws = StepRws::new(block, step); + + rws.offset_add(2); + + let (refund, _) = rws.next().tx_refund_value_pair(); + let (caller_balance, caller_balance_prev) = rws.next().account_balance_pair(); + let (coinbase_code_hash_prev, _) = rws.next().account_codehash_pair(); self.tx_id .assign(region, offset, Value::known(F::from(tx.id)))?; @@ -209,16 +214,10 @@ impl ExecutionGadget for EndTxGadget { self.coinbase_code_hash_is_zero .assign_u256(region, offset, coinbase_code_hash_prev)?; if !coinbase_reward.is_zero() { - let coinbase_balance_pair = block - .get_rws( - step, - if coinbase_code_hash_prev.is_zero() { - 6 - } else { - 5 - }, - ) - .account_balance_pair(); + if coinbase_code_hash_prev.is_zero() { + rws.offset_add(1) + } + let coinbase_balance_pair = rws.next().account_balance_pair(); self.coinbase_reward.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 6a5e4568d7..8a1373c392 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -204,7 +204,7 @@ impl ExecutionGadget for ExtcodecopyGadget { call.is_persistent, )?; - rws.offset_add(4); + rws.offset_add(3); let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs index 99cd8b5b6a..a7340ca6f8 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs @@ -8,7 +8,7 @@ use crate::{ constraint_builder::{ EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::Delta, }, - select, AccountAddress, CachedRegion, Cell, + select, AccountAddress, CachedRegion, Cell, StepRws, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -106,7 +106,10 @@ impl ExecutionGadget for ExtcodehashGadget { ) -> Result<(), Error> { self.same_context.assign_exec_step(region, offset, step)?; - let address = block.get_rws(step, 0).stack_value(); + let mut rws = StepRws::new(block, step); + + let address = rws.next().stack_value(); + self.address_word.assign_u256(region, offset, address)?; self.tx_id @@ -118,11 +121,13 @@ impl ExecutionGadget for ExtcodehashGadget { call.is_persistent, )?; - let (_, is_warm) = block.get_rws(step, 4).tx_access_list_value_pair(); + rws.offset_add(3); + + let (_, is_warm) = rws.next().tx_access_list_value_pair(); self.is_warm .assign(region, offset, Value::known(F::from(is_warm as u64)))?; - let code_hash = block.get_rws(step, 5).account_codehash_pair().0; + let code_hash = rws.next().account_codehash_pair().0; self.code_hash.assign_u256(region, offset, code_hash)?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/logs.rs b/zkevm-circuits/src/evm_circuit/execution/logs.rs index 55d3c67d4f..0a248ec31e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/logs.rs +++ b/zkevm-circuits/src/evm_circuit/execution/logs.rs @@ -203,7 +203,9 @@ impl ExecutionGadget for LogGadget { let mut rws = StepRws::new(block, step); - let [memory_start, msize] = [0, 1].map(|_| rws.next().stack_value()); + let memory_start = rws.next().stack_value(); + let msize = rws.next().stack_value(); + let memory_address = self .memory_address .assign(region, offset, memory_start, msize)?; @@ -223,8 +225,9 @@ impl ExecutionGadget for LogGadget { // It takes 6 + is_persistent reads or writes to reach the topic stack write section. // Each topic takes at least 1 stack read. They take an additional tx log write if the // call is persistent. - rws.offset_add(6 + is_persistent + topic * (1 + is_persistent)); - rws.next().stack_value() + block + .get_rws(step, 6 + is_persistent + topic * (1 + is_persistent)) + .stack_value() }); for i in 0..4 { let topic = topics.next(); From 036e36653ba286fadd1645a37d0a6ebf95141290 Mon Sep 17 00:00:00 2001 From: chad Date: Mon, 25 Mar 2024 11:02:45 -0500 Subject: [PATCH 7/8] lint: code cleanup --- .../src/evm_circuit/execution/error_oog_call.rs | 9 --------- .../src/evm_circuit/execution/error_oog_memory_copy.rs | 2 -- zkevm-circuits/src/evm_circuit/execution/extcodehash.rs | 1 - zkevm-circuits/src/evm_circuit/execution/sload.rs | 2 -- 4 files changed, 14 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index 0e00faed82..dafd542d5a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -132,7 +132,6 @@ impl ExecutionGadget for ErrorOOGCallGadget { let tx_id = rws.next().call_context_value(); let is_static = rws.next().call_context_value(); - let gas = rws.next().stack_value(); let callee_address = rws.next().stack_value(); @@ -142,12 +141,6 @@ impl ExecutionGadget for ErrorOOGCallGadget { U256::zero() }; - if is_call_or_callcode == 1 { - rws.offset_sub(1); - } - - rws.offset_add(is_call_or_callcode); - let cd_offset = rws.next().stack_value(); let cd_length = rws.next().stack_value(); let rd_offset = rws.next().stack_value(); @@ -156,9 +149,7 @@ impl ExecutionGadget for ErrorOOGCallGadget { rws.offset_add(1); let callee_code_hash = rws.next().account_codehash_pair().0; - let callee_exists = !callee_code_hash.is_zero(); - let (is_warm, is_warm_prev) = rws.next().tx_access_list_value_pair(); let memory_expansion_gas_cost = self.call.assign( diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 44b438f8de..e4d9b57ccc 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -181,9 +181,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { }; let dst_offset = rws.next().stack_value(); - let src_offset = rws.next().stack_value(); - let copy_size = rws.next().stack_value(); self.opcode diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs index 9af6b00ada..e7c90402ce 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodehash.rs @@ -110,7 +110,6 @@ impl ExecutionGadget for ExtcodehashGadget { let mut rws = StepRws::new(block, step); let address = rws.next().stack_value(); - self.address_word.assign_u256(region, offset, address)?; self.tx_id diff --git a/zkevm-circuits/src/evm_circuit/execution/sload.rs b/zkevm-circuits/src/evm_circuit/execution/sload.rs index 9de3ebaf37..5b3c7ced0b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sload.rs @@ -126,9 +126,7 @@ impl ExecutionGadget for SloadGadget { rws.offset_add(4); let key = rws.next().stack_value(); - let (_, committed_value) = rws.next().aux_pair(); - let value = rws.next().stack_value(); self.key.assign_u256(region, offset, key)?; From bcfd4f7c61dba0c29562006011d6e8b3dcff961e Mon Sep 17 00:00:00 2001 From: chad Date: Tue, 26 Mar 2024 20:24:54 -0500 Subject: [PATCH 8/8] fix: PR adjustments --- .../evm_circuit/execution/error_oog_memory_copy.rs | 4 ---- .../src/evm_circuit/execution/returndatacopy.rs | 14 ++++---------- zkevm-circuits/src/evm_circuit/util.rs | 8 -------- 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index e4d9b57ccc..bdbf1f5115 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -176,10 +176,6 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { (false, U256::zero()) }; - if !is_extcodecopy { - rws.offset_reset(); - }; - let dst_offset = rws.next().stack_value(); let src_offset = rws.next().stack_value(); let copy_size = rws.next().stack_value(); diff --git a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs index acf32643b8..8fa7a5be1a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs @@ -183,16 +183,10 @@ impl ExecutionGadget for ReturnDataCopyGadget { self.data_offset.assign_u256(region, offset, data_offset)?; - let [last_callee_id, return_data_offset, return_data_size] = [ - (3, CallContextFieldTag::LastCalleeId), - (4, CallContextFieldTag::LastCalleeReturnDataOffset), - (5, CallContextFieldTag::LastCalleeReturnDataLength), - ] - .map(|(_, tag)| { - let rw = rws.next(); - assert_eq!(rw.field_tag(), Some(tag as u64)); - rw.call_context_value() - }); + let last_callee_id = rws.next().call_context_value(); + let return_data_offset = rws.next().call_context_value(); + let return_data_size = rws.next().call_context_value(); + self.last_callee_id.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index e6fde220d5..1b58790914 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -469,14 +469,6 @@ impl<'a> StepRws<'a> { pub(crate) fn offset_add(&mut self, inc: usize) { self.offset += inc } - /// Decrement the step rw operation offset by `dec`. - pub(crate) fn offset_sub(&mut self, dec: usize) { - self.offset -= dec - } - /// Reset the step rw operation offset to 0. - pub(crate) fn offset_reset(&mut self) { - self.offset = 0 - } /// Return the next rw operation from the step. pub(crate) fn next(&mut self) -> Rw { let rw = self.rws[self.step.rw_index(self.offset)];