From 4ebbcd131a4734558ddf4fbf6858573daabd158c Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Mon, 27 Nov 2023 17:59:03 +0800 Subject: [PATCH 01/12] Implement access list address and storage key lookups in copy circuit. --- .../src/circuit_input_builder/execution.rs | 18 ++- .../circuit_input_builder/input_state_ref.rs | 29 +++- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 142 ++++++++++++----- bus-mapping/src/evm/opcodes/calldatacopy.rs | 2 + bus-mapping/src/evm/opcodes/callop.rs | 3 + bus-mapping/src/evm/opcodes/codecopy.rs | 1 + bus-mapping/src/evm/opcodes/create.rs | 1 + bus-mapping/src/evm/opcodes/extcodecopy.rs | 1 + bus-mapping/src/evm/opcodes/logs.rs | 1 + bus-mapping/src/evm/opcodes/return_revert.rs | 2 + bus-mapping/src/evm/opcodes/returndatacopy.rs | 1 + bus-mapping/src/evm/opcodes/sha3.rs | 1 + zkevm-circuits/src/copy_circuit.rs | 146 ++++++++++++++++-- .../src/copy_circuit/copy_gadgets.rs | 10 ++ zkevm-circuits/src/copy_circuit/test.rs | 2 + zkevm-circuits/src/table.rs | 22 ++- zkevm-circuits/src/tx_circuit.rs | 2 +- 17 files changed, 324 insertions(+), 60 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 12c377f0ed..7505691bf0 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -15,7 +15,7 @@ use crate::{ use eth_types::{ evm_types::{memory::MemoryWordRange, Gas, GasCost, MemoryAddress, OpcodeId, ProgramCounter}, sign_types::SignData, - Field, GethExecStep, ToLittleEndian, Word, H256, U256, + Address, Field, GethExecStep, ToLittleEndian, Word, H256, U256, }; use ethers_core::k256::elliptic_curve::subtle::CtOption; use gadgets::impl_expr; @@ -228,6 +228,7 @@ pub enum CopyDataType { /// tx-table and destination is rw-table. AccessListStorageKeys, } + impl CopyDataType { /// How many bits are necessary to represent a copy data type. pub const N_BITS: usize = 3usize; @@ -247,6 +248,8 @@ impl CopyDataTypeIter { 3usize => Some(CopyDataType::TxCalldata), 4usize => Some(CopyDataType::TxLog), 5usize => Some(CopyDataType::RlcAcc), + 6usize => Some(CopyDataType::AccessListAddresses), + 7usize => Some(CopyDataType::AccessListStorageKeys), _ => None, } } @@ -363,6 +366,12 @@ pub enum NumberOrHash { Hash(H256), } +impl Default for NumberOrHash { + fn default() -> Self { + Self::Number(0) + } +} + /// Represents all bytes related in one copy event. /// /// - When the source is memory, `bytes` is the memory content, including masked areas. The @@ -404,7 +413,7 @@ impl CopyBytes { /// Defines a copy event associated with EVM opcodes such as CALLDATACOPY, /// CODECOPY, CREATE, etc. More information: /// . -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct CopyEvent { /// Represents the start address at the source of the copy event. pub src_addr: u64, @@ -428,6 +437,11 @@ pub struct CopyEvent { pub rw_counter_start: RWCounter, /// Represents the list of bytes related during this copy event pub copy_bytes: CopyBytes, + /// Represents transaction access list (EIP-2930), if copy data type is + /// address, the first item is access list address and second is zero, if + /// copy data type is storage key, the first item is access list address and + /// second is access list storage key. + pub access_list: Vec<(Address, Word)>, } pub type CopyEventSteps = Vec<(u8, bool, bool)>; 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 a5615573da..e0d6616eb3 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -16,8 +16,8 @@ use crate::{ exec_trace::OperationRef, operation::{ AccountField, AccountOp, CallContextField, CallContextOp, MemoryOp, Op, OpEnum, Operation, - StackOp, Target, TxAccessListAccountOp, TxLogField, TxLogOp, TxReceiptField, TxReceiptOp, - RW, + StackOp, Target, TxAccessListAccountOp, TxAccessListAccountStorageOp, TxLogField, TxLogOp, + TxReceiptField, TxReceiptOp, RW, }, precompile::{is_precompiled, PrecompileCalls}, state_db::{CodeDB, StateDB}, @@ -591,7 +591,7 @@ impl<'a> CircuitInputStateRef<'a> { /// adds a reference to the stored operation ([`OperationRef`]) inside /// the bus-mapping instance of the current [`ExecStep`]. Then increase /// the `block_ctx` [`RWCounter`](crate::operation::RWCounter) by one. - pub fn tx_accesslist_account_write( + pub fn tx_access_list_account_write( &mut self, step: &mut ExecStep, tx_id: usize, @@ -611,6 +611,29 @@ impl<'a> CircuitInputStateRef<'a> { ) } + /// Add address storage key to access list for the current transaction. + pub fn tx_access_list_storage_key_write( + &mut self, + step: &mut ExecStep, + tx_id: usize, + address: Address, + key: Word, + is_warm: bool, + is_warm_prev: bool, + ) -> Result<(), Error> { + self.push_op( + step, + RW::WRITE, + TxAccessListAccountStorageOp { + tx_id, + address, + key, + is_warm, + is_warm_prev, + }, + ) + } + /// Push 2 reversible [`AccountOp`] to update `sender` and `receiver`'s /// balance by `value`. If `fee` is existing (not None), also need to push 1 /// non-reversible [`AccountOp`] to update `sender` balance by `fee`. diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index 0689b03411..79190db894 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -1,3 +1,4 @@ +use super::TxExecSteps; use crate::{ circuit_input_builder::{ CircuitInputStateRef, CopyBytes, CopyDataType, CopyEvent, ExecState, ExecStep, NumberOrHash, @@ -16,13 +17,10 @@ use eth_types::{ gas_utils::{tx_access_list_gas_cost, tx_data_gas_cost}, GasCost, MAX_REFUND_QUOTIENT_OF_GAS_USED, }, - geth_types::access_list_size, Bytecode, ToWord, Word, }; use ethers_core::utils::get_contract_address; -use super::TxExecSteps; - #[derive(Clone, Copy, Debug)] pub(crate) struct BeginEndTx; @@ -146,7 +144,7 @@ pub fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result Result Result Result<(), Error> { + let tx_id = state.tx_ctx.id(); - // Add copy event for access-list addresses. + // Build copy access list including addresses. + let access_list = if let Some(access_list) = state.tx.access_list.clone() { + let mut addresses = Vec::with_capacity(access_list.0.len()); + + for item in access_list.0.iter() { + // Add RW write operations for access list addresses + // (will lookup in copy circuit). + state.tx_access_list_account_write(exec_step, tx_id, item.address, true, false)?; + + addresses.push((item.address, 0.into())); + } + + addresses + } else { + vec![] + }; + + let tx_id = NumberOrHash::Number(tx_id); let rw_counter_start = state.block_ctx.rwc; - state.push_copy( - exec_step, - CopyEvent { - src_addr: 0, - src_addr_end: address_size, - src_type: CopyDataType::AccessListAddresses, - src_id: tx_id.clone(), - dst_addr: 0, - dst_type: CopyDataType::AccessListAddresses, - dst_id: tx_id.clone(), - log_id: None, - rw_counter_start, - // TODO - copy_bytes: CopyBytes::new(vec![], None, None), - }, - ); + let copy_bytes = CopyBytes::new(vec![(0, false, false); access_list.len()], None, None); + + // Add copy event to copy table. + let copy_event = CopyEvent { + src_type: CopyDataType::AccessListAddresses, + src_id: tx_id, + src_addr: 1, // index starts from 1. + src_addr_end: access_list.len() as u64 + 1, + rw_counter_start, + copy_bytes, + access_list, + ..Default::default() + }; + + state.push_copy(exec_step, copy_event); + + Ok(()) +} + +fn add_access_list_storage_key_copy_event( + state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, +) -> Result<(), Error> { + let tx_id = state.tx_ctx.id(); + + // Build copy access list including addresses and storage keys. + let access_list = if let Some(access_list) = state.tx.access_list.clone() { + let mut address_storage_keys = vec![]; + + for item in access_list.0.iter() { + for storage_key in item.storage_keys.iter() { + let storage_key = storage_key.to_word(); + + // Add RW write operations for access list address storage keys + // (will lookup in copy circuit). + state.tx_access_list_storage_key_write( + exec_step, + tx_id, + item.address, + storage_key, + true, + false, + )?; + + address_storage_keys.push((item.address, storage_key)); + } + } + + address_storage_keys + } else { + vec![] + }; - // Add copy event for access-list storage keys. let rw_counter_start = state.block_ctx.rwc; - state.push_copy( - exec_step, - CopyEvent { - src_addr: 0, - src_addr_end: storage_key_size, - src_type: CopyDataType::AccessListStorageKeys, - src_id: tx_id.clone(), - dst_addr: 0, - dst_type: CopyDataType::AccessListStorageKeys, - dst_id: tx_id, - log_id: None, - rw_counter_start, - // TODO - copy_bytes: CopyBytes::new(vec![], None, None), - }, - ); + let tx_id = NumberOrHash::Number(state.tx_ctx.id()); + let copy_bytes = CopyBytes::new(vec![(0, false, false); access_list.len()], None, None); + + // Add copy event to copy table. + let copy_event = CopyEvent { + src_type: CopyDataType::AccessListStorageKeys, + src_id: tx_id, + src_addr: 1, // index starts from 1 in tx-table. + src_addr_end: access_list.len() as u64 + 1, + rw_counter_start, + copy_bytes, + access_list, + ..Default::default() + }; + + state.push_copy(exec_step, copy_event); Ok(()) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 5c77f46d37..784d0748df 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -120,6 +120,7 @@ fn gen_copy_event( log_id: None, rw_counter_start, copy_bytes, + ..Default::default() }) } else { let (read_steps, write_steps, prev_bytes) = @@ -137,6 +138,7 @@ fn gen_copy_event( rw_counter_start, //fetch pre read and write bytes of CopyBytes copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(prev_bytes)), + ..Default::default() }) } } diff --git a/bus-mapping/src/evm/opcodes/callop.rs b/bus-mapping/src/evm/opcodes/callop.rs index 61927d4092..5fb89dd7f0 100644 --- a/bus-mapping/src/evm/opcodes/callop.rs +++ b/bus-mapping/src/evm/opcodes/callop.rs @@ -382,6 +382,7 @@ impl Opcode for CallOpcode { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, None), + ..Default::default() }, ); Some(input_bytes) @@ -412,6 +413,7 @@ impl Opcode for CallOpcode { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), + ..Default::default() }, ); Some(output_bytes) @@ -451,6 +453,7 @@ impl Opcode for CallOpcode { Some(write_steps), Some(prev_bytes), ), + ..Default::default() }, ); Some(returned_bytes) diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index e890096057..cdf8a6ba72 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -87,6 +87,7 @@ fn gen_copy_event( rw_counter_start, //fetch pre write bytes of CopyBytes copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), + ..Default::default() }) } diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs index 7167dad7c7..4b1bd68dec 100644 --- a/bus-mapping/src/evm/opcodes/create.rs +++ b/bus-mapping/src/evm/opcodes/create.rs @@ -386,6 +386,7 @@ fn handle_copy( dst_addr: 0, log_id: None, copy_bytes: CopyBytes::new(copy_steps, None, None), + ..Default::default() }, ); diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs index 876888c652..78904c9aad 100644 --- a/bus-mapping/src/evm/opcodes/extcodecopy.rs +++ b/bus-mapping/src/evm/opcodes/extcodecopy.rs @@ -144,6 +144,7 @@ fn gen_copy_event( log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), + ..Default::default() }) } diff --git a/bus-mapping/src/evm/opcodes/logs.rs b/bus-mapping/src/evm/opcodes/logs.rs index 00b9d9a2bf..5463440557 100644 --- a/bus-mapping/src/evm/opcodes/logs.rs +++ b/bus-mapping/src/evm/opcodes/logs.rs @@ -150,6 +150,7 @@ fn gen_copy_event( log_id: Some(state.tx_ctx.log_id as u64 + 1), rw_counter_start, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), None), + ..Default::default() }) } diff --git a/bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return_revert.rs index e1c03a0e37..cb41d543b7 100644 --- a/bus-mapping/src/evm/opcodes/return_revert.rs +++ b/bus-mapping/src/evm/opcodes/return_revert.rs @@ -282,6 +282,7 @@ fn handle_copy( dst_addr: destination.offset.try_into().unwrap(), log_id: None, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(dst_data_prev)), + ..Default::default() }, ); @@ -348,6 +349,7 @@ fn handle_create( dst_addr: 0, log_id: None, copy_bytes: CopyBytes::new(copy_steps, None, None), + ..Default::default() }, ); diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs index 350826ace8..e60407c608 100644 --- a/bus-mapping/src/evm/opcodes/returndatacopy.rs +++ b/bus-mapping/src/evm/opcodes/returndatacopy.rs @@ -111,6 +111,7 @@ fn gen_copy_event( log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(prev_bytes)), + ..Default::default() }) } diff --git a/bus-mapping/src/evm/opcodes/sha3.rs b/bus-mapping/src/evm/opcodes/sha3.rs index 96a042f90d..d62d063a32 100644 --- a/bus-mapping/src/evm/opcodes/sha3.rs +++ b/bus-mapping/src/evm/opcodes/sha3.rs @@ -96,6 +96,7 @@ impl Opcode for Sha3 { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, None), + ..Default::default() }, ); diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 340d47033b..20eb27c581 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -11,9 +11,9 @@ mod test; #[cfg(any(feature = "test", test, feature = "test-circuits"))] pub use dev::CopyCircuit as TestCopyCircuit; +use array_init::array_init; use bus_mapping::circuit_input_builder::{CopyDataType, CopyEvent}; use eth_types::{Field, Word}; - use gadgets::{ binary_number::BinaryNumberChip, is_equal::{IsEqualChip, IsEqualConfig, IsEqualInstruction}, @@ -97,6 +97,12 @@ pub struct CopyCircuitConfig { pub is_memory: Column, /// Booleans to indicate what copy data type exists at the current row. pub is_tx_log: Column, + /// Booleans to indicate if `CopyDataType::AccessListAddresses` exists at + /// the current row. + pub is_access_list_address: Column, + /// Booleans to indicate if `CopyDataType::AccessListStorageKeys` exists at + /// the current row. + pub is_access_list_storage_key: Column, /// Whether the row is enabled or not. pub q_enable: Column, /// The Copy Table contains the columns that are exposed via the lookup @@ -159,13 +165,8 @@ impl SubCircuitConfig for CopyCircuitConfig { let value_word_rlc_prev = meta.advice_column_in(SecondPhase); let value_acc = meta.advice_column_in(SecondPhase); - let (is_tx_calldata, is_bytecode, is_memory, is_tx_log) = ( - meta.advice_column(), - meta.advice_column(), - meta.advice_column(), - meta.advice_column(), - ); - let is_pad = meta.advice_column(); + let [is_pad, is_tx_calldata, is_bytecode, is_memory, is_tx_log, is_access_list_address, is_access_list_storage_key] = + array_init(|_| meta.advice_column()); let is_first = copy_table.is_first; let id = copy_table.id; let addr = copy_table.addr; @@ -210,6 +211,8 @@ impl SubCircuitConfig for CopyCircuitConfig { is_bytecode, is_memory, is_tx_log, + is_access_list_address, + is_access_list_storage_key, ); meta.create_gate("verify copy events", |meta| { @@ -233,6 +236,8 @@ impl SubCircuitConfig for CopyCircuitConfig { // NEXT_STEP. let is_word_end = is_word_end.expr(); let is_tx_log = meta.query_advice(is_tx_log, CURRENT); + let is_access_list_address = meta.query_advice(is_access_list_address, CURRENT); + let is_access_list_storage_key = meta.query_advice(is_access_list_storage_key, CURRENT); constrain_first_last(cb, is_reader.expr(), is_first.expr(), is_last.expr()); @@ -254,8 +259,11 @@ impl SubCircuitConfig for CopyCircuitConfig { let (mask, mask_next, front_mask) = { // The first 31 bytes may be front_mask, but not the last byte of the first word. - // LOG has no front mask at all. - let forbid_front_mask = is_word_end.expr() + is_tx_log.expr(); + // LOG, access-list address and storage-key have no front mask at all. + let forbid_front_mask = is_word_end.expr() + + is_tx_log.expr() + + is_access_list_address.expr() + + is_access_list_storage_key.expr(); constrain_mask( cb, @@ -324,7 +332,10 @@ impl SubCircuitConfig for CopyCircuitConfig { constrain_address(cb, meta, is_continue.expr(), front_mask.expr(), addr); { - let is_rw_type = meta.query_advice(is_memory, CURRENT) + is_tx_log.expr(); + let is_rw_type = meta.query_advice(is_memory, CURRENT) + + is_tx_log.expr() + + is_access_list_address.expr() + + is_access_list_storage_key.expr(); constrain_rw_counter( cb, @@ -432,6 +443,103 @@ impl SubCircuitConfig for CopyCircuitConfig { .collect() }); + meta.lookup_any("Tx access list address lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_address, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let index = meta.query_advice(addr, CURRENT); + let address = meta.query_advice(value, CURRENT); + + vec![ + 1.expr(), + tx_id, + TxContextFieldTag::AccessListAddress.expr(), + index, + address, + ] + .into_iter() + .zip(tx_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + + meta.lookup_any("Rw access list address lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_address, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let address = meta.query_advice(value, CURRENT); + + vec![ + 1.expr(), + meta.query_advice(rw_counter, CURRENT), + 1.expr(), + RwTableTag::TxAccessListAccount.expr(), + tx_id, + address, // access list address + 0.expr(), + 0.expr(), + 1.expr(), // is_warm + 0.expr(), // is_warm_prev + 0.expr(), + 0.expr(), + ] + .into_iter() + .zip(rw_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + + meta.lookup_any("Tx access list storage key lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_storage_key, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let index = meta.query_advice(addr, CURRENT); + let storage_key = meta.query_advice(value_prev, CURRENT); + + vec![ + 1.expr(), + tx_id, + TxContextFieldTag::AccessListStorageKey.expr(), + index, + storage_key, + ] + .into_iter() + .zip(tx_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + + meta.lookup_any("Rw access list storage key lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_storage_key, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let address = meta.query_advice(value, CURRENT); + let storage_key = meta.query_advice(value_prev, CURRENT); + + vec![ + 1.expr(), + meta.query_advice(rw_counter, CURRENT), + 1.expr(), + RwTableTag::TxAccessListAccountStorage.expr(), + tx_id, + address, // access list address + 0.expr(), + storage_key, // access list storage key + 1.expr(), // is_warm + 0.expr(), // is_warm_prev + 0.expr(), + 0.expr(), + ] + .into_iter() + .zip(rw_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + Self { q_step, is_last, @@ -448,6 +556,8 @@ impl SubCircuitConfig for CopyCircuitConfig { is_bytecode, is_memory, is_tx_log, + is_access_list_address, + is_access_list_storage_key, q_enable, is_src_end, is_word_end, @@ -585,6 +695,18 @@ impl CopyCircuitConfig { *offset, || Value::known(F::from(tag.eq(&CopyDataType::TxLog))), )?; + region.assign_advice( + || format!("is_access_list_address at row: {}", *offset), + self.is_access_list_address, + *offset, + || Value::known(F::from(tag.eq(&CopyDataType::AccessListAddresses))), + )?; + region.assign_advice( + || format!("is_access_list_storage_key at row: {}", *offset), + self.is_access_list_storage_key, + *offset, + || Value::known(F::from(tag.eq(&CopyDataType::AccessListStorageKeys))), + )?; *offset += 1; } @@ -865,6 +987,8 @@ impl CopyCircuitConfig { self.is_bytecode, self.is_memory, self.is_tx_log, + self.is_access_list_address, + self.is_access_list_storage_key, ] { region.assign_advice( || format!("assigning padding row: {}", *offset), diff --git a/zkevm-circuits/src/copy_circuit/copy_gadgets.rs b/zkevm-circuits/src/copy_circuit/copy_gadgets.rs index ff74812cff..5d7077faef 100644 --- a/zkevm-circuits/src/copy_circuit/copy_gadgets.rs +++ b/zkevm-circuits/src/copy_circuit/copy_gadgets.rs @@ -19,6 +19,8 @@ pub fn constrain_tag( is_bytecode: Column, is_memory: Column, is_tx_log: Column, + is_access_list_address: Column, + is_access_list_storage_key: Column, ) { meta.create_gate("decode tag", |meta| { let enabled = meta.query_fixed(q_enable, CURRENT); @@ -26,6 +28,8 @@ pub fn constrain_tag( let is_bytecode = meta.query_advice(is_bytecode, CURRENT); let is_memory = meta.query_advice(is_memory, CURRENT); let is_tx_log = meta.query_advice(is_tx_log, CURRENT); + let is_access_list_address = meta.query_advice(is_access_list_address, CURRENT); + let is_access_list_storage_key = meta.query_advice(is_access_list_storage_key, CURRENT); vec![ // Match boolean indicators to their respective tag values. enabled.expr() @@ -34,6 +38,12 @@ pub fn constrain_tag( * (is_bytecode - tag.value_equals(CopyDataType::Bytecode, CURRENT)(meta)), enabled.expr() * (is_memory - tag.value_equals(CopyDataType::Memory, CURRENT)(meta)), enabled.expr() * (is_tx_log - tag.value_equals(CopyDataType::TxLog, CURRENT)(meta)), + enabled.expr() + * (is_access_list_address + - tag.value_equals(CopyDataType::AccessListAddresses, CURRENT)(meta)), + enabled.expr() + * (is_access_list_storage_key + - tag.value_equals(CopyDataType::AccessListStorageKeys, CURRENT)(meta)), ] }); } diff --git a/zkevm-circuits/src/copy_circuit/test.rs b/zkevm-circuits/src/copy_circuit/test.rs index bbf23b7acd..0349d5a620 100644 --- a/zkevm-circuits/src/copy_circuit/test.rs +++ b/zkevm-circuits/src/copy_circuit/test.rs @@ -259,6 +259,8 @@ fn gen_tx_log_data() -> CircuitInputBuilder { builder } +// TODO: add test for access list data. + fn gen_create_data() -> CircuitInputBuilder { let code = bytecode! { PUSH21(Word::from("6B6020600060003760206000F3600052600C6014F3")) diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index e8208bff03..ee23b87af0 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -174,6 +174,10 @@ pub enum TxFieldTag { TxHash, /// TxType: Type of the transaction TxType, + /// Access list address + AccessListAddress, + /// Access list storage key + AccessListStorageKey, /// Access list address count (EIP-2930) AccessListAddressesLen, /// Access list all storage key count (EIP-2930) @@ -1794,13 +1798,27 @@ impl CopyTable { let is_pad = is_read_step && thread.addr >= thread.addr_end; - let value = Value::known(F::from(copy_step.value as u64)); + let [value, value_prev] = if copy_event.src_type == CopyDataType::AccessListAddresses + || copy_event.src_type == CopyDataType::AccessListStorageKeys + { + let address_pair = copy_event.access_list[step_idx]; + [ + address_pair.0.to_scalar().unwrap(), + address_pair.1.to_scalar().unwrap(), + ] + } else { + [ + F::from(copy_step.value as u64), + F::from(copy_step.prev_value as u64), + ] + } + .map(Value::known); + let value_or_pad = if is_pad { Value::known(F::zero()) } else { value }; - let value_prev = Value::known(F::from(copy_step.prev_value as u64)); if !copy_step.mask { thread.front_mask = false; diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index bd886e5c5c..bc86ff941a 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -80,7 +80,7 @@ use halo2_proofs::plonk::SecondPhase; use itertools::Itertools; /// Number of rows of one tx occupies in the fixed part of tx table -pub const TX_LEN: usize = 26; +pub const TX_LEN: usize = 28; /// Offset of TxHash tag in the tx table pub const TX_HASH_OFFSET: usize = 21; /// Offset of ChainID tag in the tx table From 7b2e0609cf048ced4bee50194642d701115472b5 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 7 Dec 2023 17:46:05 +0800 Subject: [PATCH 02/12] Fix wrong TX length. --- zkevm-circuits/src/tx_circuit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index bc86ff941a..bd886e5c5c 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -80,7 +80,7 @@ use halo2_proofs::plonk::SecondPhase; use itertools::Itertools; /// Number of rows of one tx occupies in the fixed part of tx table -pub const TX_LEN: usize = 28; +pub const TX_LEN: usize = 26; /// Offset of TxHash tag in the tx table pub const TX_HASH_OFFSET: usize = 21; /// Offset of ChainID tag in the tx table From 91868dbd9bb60c2a49862c2252822c3bee2654f1 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 8 Dec 2023 17:02:15 +0800 Subject: [PATCH 03/12] Add valid test case of access list to copy circuit tests. --- eth-types/src/lib.rs | 5 ++- zkevm-circuits/src/copy_circuit/test.rs | 59 ++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/eth-types/src/lib.rs b/eth-types/src/lib.rs index a0675b8ad3..e36841355c 100644 --- a/eth-types/src/lib.rs +++ b/eth-types/src/lib.rs @@ -38,7 +38,10 @@ use ethers_core::types; pub use ethers_core::{ abi::ethereum_types::{BigEndianHash, U512}, types::{ - transaction::{eip2930::AccessList, response::Transaction}, + transaction::{ + eip2930::{AccessList, AccessListItem}, + response::Transaction, + }, Address, Block, Bytes, Signature, H160, H256, H64, U256, U64, }, }; diff --git a/zkevm-circuits/src/copy_circuit/test.rs b/zkevm-circuits/src/copy_circuit/test.rs index 0349d5a620..baa2e5a071 100644 --- a/zkevm-circuits/src/copy_circuit/test.rs +++ b/zkevm-circuits/src/copy_circuit/test.rs @@ -12,12 +12,17 @@ use bus_mapping::{ mock::BlockData, precompile::PrecompileCalls, }; -use eth_types::{bytecode, geth_types::GethData, word, ToWord, Word}; +use eth_types::{ + address, bytecode, geth_types::GethData, word, AccessList, AccessListItem, ToWord, Word, H256, +}; use halo2_proofs::{ dev::{MockProver, VerifyFailure}, halo2curves::bn256::Fr, }; -use mock::{test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS}; +use mock::{ + eth, gwei, test_ctx::helpers::account_0_code_account_1_no_code, MockTransaction, TestContext, + MOCK_ACCOUNTS, +}; const K: u32 = 20; @@ -259,7 +264,46 @@ fn gen_tx_log_data() -> CircuitInputBuilder { builder } -// TODO: add test for access list data. +fn gen_access_list_data() -> CircuitInputBuilder { + let test_access_list = AccessList(vec![ + AccessListItem { + address: address!("0x0000000000000000000000000000000000001111"), + storage_keys: [10, 11].map(H256::from_low_u64_be).to_vec(), + }, + AccessListItem { + address: address!("0x0000000000000000000000000000000000002222"), + storage_keys: [20, 22].map(H256::from_low_u64_be).to_vec(), + }, + AccessListItem { + address: address!("0x0000000000000000000000000000000000003333"), + storage_keys: [30, 33].map(H256::from_low_u64_be).to_vec(), + }, + ]); + let test_ctx = TestContext::<1, 1>::new( + None, + |accs| { + accs[0].address(MOCK_ACCOUNTS[0]).balance(eth(20)); + }, + |mut txs, _accs| { + txs[0] + .from(MOCK_ACCOUNTS[0]) + .to(MOCK_ACCOUNTS[1]) + .gas_price(gwei(2)) + .gas(Word::from(0x10000)) + .value(eth(2)) + .access_list(test_access_list); + }, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap(); + let block: GethData = test_ctx.into(); + let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); + builder + .handle_block(&block.eth_block, &block.geth_traces) + .unwrap(); + + builder +} fn gen_create_data() -> CircuitInputBuilder { let code = bytecode! { @@ -345,6 +389,13 @@ fn copy_circuit_valid_tx_log() { assert_eq!(test_copy_circuit_from_block(block), Ok(())); } +#[test] +fn copy_circuit_valid_access_list() { + let builder = gen_access_list_data(); + let block = block_convert::(&builder.block, &builder.code_db).unwrap(); + assert_eq!(test_copy_circuit_from_block(block), Ok(())); +} + #[test] fn copy_circuit_valid_create() { let builder = gen_create_data(); @@ -451,8 +502,6 @@ fn copy_circuit_invalid_tx_log() { .expect("there should be a lookup error"); } -// todo: add invalid create/return/returndatacopy tests - #[test] fn copy_circuit_precompile_call() { // TODO: as we add support for more precompiles, we should populate those here as well. From a2ec940d05249f5bb5eebe6c807f75d473ac41a0 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Fri, 8 Dec 2023 22:43:20 +0800 Subject: [PATCH 04/12] Update --- zkevm-circuits/src/table.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index ee23b87af0..bad63d28b0 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -1801,7 +1801,7 @@ impl CopyTable { let [value, value_prev] = if copy_event.src_type == CopyDataType::AccessListAddresses || copy_event.src_type == CopyDataType::AccessListStorageKeys { - let address_pair = copy_event.access_list[step_idx]; + let address_pair = copy_event.access_list[step_idx / 2]; [ address_pair.0.to_scalar().unwrap(), address_pair.1.to_scalar().unwrap(), From 2481616522d836297653f0ebe41e24369ca05f2e Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sat, 9 Dec 2023 12:13:08 +0800 Subject: [PATCH 05/12] Fix `NUM_COPY_DATA_TYPES`. --- bus-mapping/src/circuit_input_builder/execution.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 7505691bf0..7ad7b37dcb 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -233,7 +233,7 @@ impl CopyDataType { /// How many bits are necessary to represent a copy data type. pub const N_BITS: usize = 3usize; } -const NUM_COPY_DATA_TYPES: usize = 6usize; +const NUM_COPY_DATA_TYPES: usize = 8usize; pub struct CopyDataTypeIter { idx: usize, back_idx: usize, From 852b446ae8714568ad12a5d8bf389a2a8ee9a44a Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sat, 9 Dec 2023 12:42:32 +0800 Subject: [PATCH 06/12] Update --- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 2 ++ zkevm-circuits/src/copy_circuit.rs | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index 79190db894..9fe622fd37 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -666,6 +666,7 @@ fn add_access_list_address_copy_event( src_id: tx_id, src_addr: 1, // index starts from 1. src_addr_end: access_list.len() as u64 + 1, + dst_type: CopyDataType::AccessListAddresses, rw_counter_start, copy_bytes, access_list, @@ -721,6 +722,7 @@ fn add_access_list_storage_key_copy_event( src_id: tx_id, src_addr: 1, // index starts from 1 in tx-table. src_addr_end: access_list.len() as u64 + 1, + dst_type: CopyDataType::AccessListStorageKeys, rw_counter_start, copy_bytes, access_list, diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 20eb27c581..38a2739af8 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -236,8 +236,8 @@ impl SubCircuitConfig for CopyCircuitConfig { // NEXT_STEP. let is_word_end = is_word_end.expr(); let is_tx_log = meta.query_advice(is_tx_log, CURRENT); - let is_access_list_address = meta.query_advice(is_access_list_address, CURRENT); - let is_access_list_storage_key = meta.query_advice(is_access_list_storage_key, CURRENT); + let is_access_list = meta.query_advice(is_access_list_address, CURRENT) + + meta.query_advice(is_access_list_storage_key, CURRENT); constrain_first_last(cb, is_reader.expr(), is_first.expr(), is_last.expr()); @@ -260,10 +260,8 @@ impl SubCircuitConfig for CopyCircuitConfig { let (mask, mask_next, front_mask) = { // The first 31 bytes may be front_mask, but not the last byte of the first word. // LOG, access-list address and storage-key have no front mask at all. - let forbid_front_mask = is_word_end.expr() - + is_tx_log.expr() - + is_access_list_address.expr() - + is_access_list_storage_key.expr(); + let forbid_front_mask = + is_word_end.expr() + is_tx_log.expr() + is_access_list.expr(); constrain_mask( cb, @@ -302,8 +300,11 @@ impl SubCircuitConfig for CopyCircuitConfig { constrain_word_rlc( cb, meta, - is_first.expr(), - is_continue.expr(), + // Not constrain word rlc for access list, since `value` + // saves access list address and `value_prev` saves storage + // key. + is_first.expr() * (1.expr() - is_access_list.expr()), + is_continue.expr() * (1.expr() - is_access_list.expr()), is_word_end.expr(), word_rlc, value, @@ -334,8 +335,7 @@ impl SubCircuitConfig for CopyCircuitConfig { { let is_rw_type = meta.query_advice(is_memory, CURRENT) + is_tx_log.expr() - + is_access_list_address.expr() - + is_access_list_storage_key.expr(); + + is_access_list.expr(); constrain_rw_counter( cb, From dbd4c93b74707bfba7b3b663df2a59adddfb0d21 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sat, 9 Dec 2023 15:54:50 +0800 Subject: [PATCH 07/12] Update --- bus-mapping/src/circuit_input_builder/execution.rs | 13 ++++++++++++- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 7ad7b37dcb..049a03318d 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -465,7 +465,10 @@ impl CopyEvent { /// Whether the destination performs RW lookups in the state circuit. pub fn is_destination_rw(&self) -> bool { - self.dst_type == CopyDataType::Memory || self.dst_type == CopyDataType::TxLog + self.dst_type == CopyDataType::Memory + || self.dst_type == CopyDataType::TxLog + || self.dst_type == CopyDataType::AccessListAddresses + || self.dst_type == CopyDataType::AccessListStorageKeys } /// Whether the RLC of data must be computed. @@ -483,6 +486,14 @@ impl CopyEvent { /// The number of RW lookups performed by this copy event. pub fn rw_counter_delta(&self) -> u64 { + if self.dst_type == CopyDataType::AccessListAddresses + || self.dst_type == CopyDataType::AccessListStorageKeys + { + // For access list, the placeholder is used for copy bytes which + // value will be replaced by address and storage key. + return self.full_length(); + } + (self.is_source_rw() as u64 + self.is_destination_rw() as u64) * (self.full_length() / 32) } } diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index 9fe622fd37..9938a91000 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -658,6 +658,8 @@ fn add_access_list_address_copy_event( let tx_id = NumberOrHash::Number(tx_id); let rw_counter_start = state.block_ctx.rwc; + + // Use placeholder bytes for copy steps. let copy_bytes = CopyBytes::new(vec![(0, false, false); access_list.len()], None, None); // Add copy event to copy table. @@ -714,6 +716,8 @@ fn add_access_list_storage_key_copy_event( let rw_counter_start = state.block_ctx.rwc; let tx_id = NumberOrHash::Number(state.tx_ctx.id()); + + // Use placeholder bytes for copy steps. let copy_bytes = CopyBytes::new(vec![(0, false, false); access_list.len()], None, None); // Add copy event to copy table. From 6c5c4d7d6c90562182d51378372d8452cff5138b Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sun, 10 Dec 2023 12:17:32 +0800 Subject: [PATCH 08/12] Update copy-circuit. --- .../src/circuit_input_builder/execution.rs | 3 ++- zkevm-circuits/src/copy_circuit.rs | 24 ++++++++++++------- .../src/copy_circuit/copy_gadgets.rs | 18 +++++++++----- zkevm-circuits/src/table.rs | 11 +++++---- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 049a03318d..bc44ef4cf0 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -490,7 +490,8 @@ impl CopyEvent { || self.dst_type == CopyDataType::AccessListStorageKeys { // For access list, the placeholder is used for copy bytes which - // value will be replaced by address and storage key. + // value will be replaced by address and storage key, and no word + // operations. return self.full_length(); } diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 38a2739af8..6fb2bd1730 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -17,7 +17,7 @@ use eth_types::{Field, Word}; use gadgets::{ binary_number::BinaryNumberChip, is_equal::{IsEqualChip, IsEqualConfig, IsEqualInstruction}, - util::{not, Expr}, + util::{not, select, Expr}, }; use halo2_proofs::{ circuit::{Layouter, Region, Value}, @@ -46,8 +46,9 @@ use crate::{ use self::copy_gadgets::{ constrain_address, constrain_bytes_left, constrain_event_rlc_acc, constrain_first_last, constrain_forward_parameters, constrain_is_pad, constrain_mask, constrain_masked_value, - constrain_must_terminate, constrain_non_pad_non_mask, constrain_rw_counter, constrain_tag, - constrain_value_rlc, constrain_word_index, constrain_word_rlc, + constrain_must_terminate, constrain_non_pad_non_mask, constrain_rw_counter, + constrain_rw_word_complete, constrain_tag, constrain_value_rlc, constrain_word_index, + constrain_word_rlc, }; /// The current row. @@ -333,20 +334,27 @@ impl SubCircuitConfig for CopyCircuitConfig { constrain_address(cb, meta, is_continue.expr(), front_mask.expr(), addr); { - let is_rw_type = meta.query_advice(is_memory, CURRENT) - + is_tx_log.expr() - + is_access_list.expr(); + let is_rw_word_type = meta.query_advice(is_memory, CURRENT) + is_tx_log.expr(); + let is_rw_type = is_rw_word_type.expr() + is_access_list.expr(); + + // No word align for access list address and storage key. + let is_row_end = select::expr( + is_access_list.expr(), + not::expr(is_reader), + is_word_end.expr(), + ); constrain_rw_counter( cb, meta, is_last.expr(), - is_last_step.expr(), is_rw_type.expr(), - is_word_end.expr(), + is_row_end.expr(), rw_counter, rwc_inc_left, ); + + constrain_rw_word_complete(cb, is_last_step, is_rw_word_type.expr(), is_word_end); } cb.gate(meta.query_fixed(q_enable, CURRENT)) diff --git a/zkevm-circuits/src/copy_circuit/copy_gadgets.rs b/zkevm-circuits/src/copy_circuit/copy_gadgets.rs index 5d7077faef..f9e3040b7e 100644 --- a/zkevm-circuits/src/copy_circuit/copy_gadgets.rs +++ b/zkevm-circuits/src/copy_circuit/copy_gadgets.rs @@ -494,15 +494,14 @@ pub fn constrain_address( pub fn constrain_rw_counter( cb: &mut BaseConstraintBuilder, meta: &mut VirtualCells<'_, F>, - is_last: Expression, // The last row. - is_last_step: Expression, // Both the last reader and writer rows. + is_last: Expression, // The last row. is_rw_type: Expression, - is_word_end: Expression, + is_row_end: Expression, rw_counter: Column, rwc_inc_left: Column, ) { // Decrement rwc_inc_left for the next row, when an RW operation happens. - let rwc_diff = is_rw_type.expr() * is_word_end.expr(); + let rwc_diff = is_rw_type.expr() * is_row_end.expr(); let new_value = meta.query_advice(rwc_inc_left, CURRENT) - rwc_diff; // At the end, it must reach 0. let update_or_finish = select::expr( @@ -524,13 +523,20 @@ pub fn constrain_rw_counter( meta.query_advice(rw_counter, NEXT_ROW) + meta.query_advice(rwc_inc_left, NEXT_ROW), ); }); +} - // Ensure that the word operation completes. +/// Ensure the word operation completes for RW. +pub fn constrain_rw_word_complete( + cb: &mut BaseConstraintBuilder, + is_last_step: Expression, // Both the last reader and writer rows. + is_rw_word_type: Expression, + is_word_end: Expression, +) { cb.require_zero( "is_last_step requires is_word_end for word-based types", and::expr([ is_last_step.expr(), - is_rw_type.expr(), + is_rw_word_type.expr(), not::expr(is_word_end.expr()), ]), ); diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index bad63d28b0..42c1a3fb30 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -1764,6 +1764,8 @@ impl CopyTable { word_rlc_prev: Value::known(F::zero()), }; + let is_access_list = copy_event.src_type == CopyDataType::AccessListAddresses + || copy_event.src_type == CopyDataType::AccessListStorageKeys; for (step_idx, (is_read_step, mut copy_step)) in copy_steps .flat_map(|(read_step, write_step)| { let read_step = CopyStep { @@ -1798,9 +1800,7 @@ impl CopyTable { let is_pad = is_read_step && thread.addr >= thread.addr_end; - let [value, value_prev] = if copy_event.src_type == CopyDataType::AccessListAddresses - || copy_event.src_type == CopyDataType::AccessListStorageKeys - { + let [value, value_prev] = if is_access_list { let address_pair = copy_event.access_list[step_idx / 2]; [ address_pair.0.to_scalar().unwrap(), @@ -1881,9 +1881,10 @@ impl CopyTable { if !copy_step.mask { thread.bytes_left -= 1; } + // No word operation for access list data types. + let is_row_end = is_access_list || (step_idx / 2) % 32 == 31; // Update the RW counter. - let is_word_end = (step_idx / 2) % 32 == 31; - if is_word_end && thread.is_rw { + if is_row_end && thread.is_rw { rw_counter += 1; rwc_inc_left -= 1; } From f3ca5e7606eec5149afec9051e141a86552d79c9 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sun, 10 Dec 2023 13:47:53 +0800 Subject: [PATCH 09/12] Fix RW lookup. --- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index 9938a91000..6907df1c3c 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -638,6 +638,7 @@ fn add_access_list_address_copy_event( exec_step: &mut ExecStep, ) -> Result<(), Error> { let tx_id = state.tx_ctx.id(); + let rw_counter_start = state.block_ctx.rwc; // Build copy access list including addresses. let access_list = if let Some(access_list) = state.tx.access_list.clone() { @@ -657,7 +658,6 @@ fn add_access_list_address_copy_event( }; let tx_id = NumberOrHash::Number(tx_id); - let rw_counter_start = state.block_ctx.rwc; // Use placeholder bytes for copy steps. let copy_bytes = CopyBytes::new(vec![(0, false, false); access_list.len()], None, None); @@ -665,10 +665,11 @@ fn add_access_list_address_copy_event( // Add copy event to copy table. let copy_event = CopyEvent { src_type: CopyDataType::AccessListAddresses, - src_id: tx_id, + dst_type: CopyDataType::AccessListAddresses, + src_id: tx_id.clone(), + dst_id: tx_id, src_addr: 1, // index starts from 1. src_addr_end: access_list.len() as u64 + 1, - dst_type: CopyDataType::AccessListAddresses, rw_counter_start, copy_bytes, access_list, @@ -723,10 +724,11 @@ fn add_access_list_storage_key_copy_event( // Add copy event to copy table. let copy_event = CopyEvent { src_type: CopyDataType::AccessListStorageKeys, - src_id: tx_id, + dst_type: CopyDataType::AccessListStorageKeys, + src_id: tx_id.clone(), + dst_id: tx_id, src_addr: 1, // index starts from 1 in tx-table. src_addr_end: access_list.len() as u64 + 1, - dst_type: CopyDataType::AccessListStorageKeys, rw_counter_start, copy_bytes, access_list, From 56c432a4d49dd5ebfbcb71f115e99923fe4cfe9b Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sun, 10 Dec 2023 15:01:48 +0800 Subject: [PATCH 10/12] Enable copy-circuit test for access list. --- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 2 +- zkevm-circuits/src/copy_circuit.rs | 89 ++++++++++++--------- zkevm-circuits/src/copy_circuit/test.rs | 1 + 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index 6907df1c3c..b45db5bc66 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -686,6 +686,7 @@ fn add_access_list_storage_key_copy_event( exec_step: &mut ExecStep, ) -> Result<(), Error> { let tx_id = state.tx_ctx.id(); + let rw_counter_start = state.block_ctx.rwc; // Build copy access list including addresses and storage keys. let access_list = if let Some(access_list) = state.tx.access_list.clone() { @@ -715,7 +716,6 @@ fn add_access_list_storage_key_copy_event( vec![] }; - let rw_counter_start = state.block_ctx.rwc; let tx_id = NumberOrHash::Number(state.tx_ctx.id()); // Use placeholder bytes for copy steps. diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 6fb2bd1730..3ffe201d15 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -451,26 +451,30 @@ impl SubCircuitConfig for CopyCircuitConfig { .collect() }); - meta.lookup_any("Tx access list address lookup", |meta| { - let cond = meta.query_fixed(q_enable, CURRENT) - * meta.query_advice(is_access_list_address, CURRENT); - - let tx_id = meta.query_advice(id, CURRENT); - let index = meta.query_advice(addr, CURRENT); - let address = meta.query_advice(value, CURRENT); - - vec![ - 1.expr(), - tx_id, - TxContextFieldTag::AccessListAddress.expr(), - index, - address, - ] - .into_iter() - .zip(tx_table.table_exprs(meta)) - .map(|(arg, table)| (cond.clone() * arg, table)) - .collect() - }); + /* TODO: enable tx lookup for access list after merging EIP-1559 PR with tx-table update. + + meta.lookup_any("Tx access list address lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_address, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let index = meta.query_advice(addr, CURRENT); + let address = meta.query_advice(value, CURRENT); + + vec![ + 1.expr(), + tx_id, + TxContextFieldTag::AccessListAddress.expr(), + index, + address.expr(), + address, + ] + .into_iter() + .zip(tx_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + */ meta.lookup_any("Rw access list address lookup", |meta| { let cond = meta.query_fixed(q_enable, CURRENT) @@ -499,26 +503,31 @@ impl SubCircuitConfig for CopyCircuitConfig { .collect() }); - meta.lookup_any("Tx access list storage key lookup", |meta| { - let cond = meta.query_fixed(q_enable, CURRENT) - * meta.query_advice(is_access_list_storage_key, CURRENT); - - let tx_id = meta.query_advice(id, CURRENT); - let index = meta.query_advice(addr, CURRENT); - let storage_key = meta.query_advice(value_prev, CURRENT); - - vec![ - 1.expr(), - tx_id, - TxContextFieldTag::AccessListStorageKey.expr(), - index, - storage_key, - ] - .into_iter() - .zip(tx_table.table_exprs(meta)) - .map(|(arg, table)| (cond.clone() * arg, table)) - .collect() - }); + /* TODO: enable tx lookup for access list after merging EIP-1559 PR with tx-table update. + + meta.lookup_any("Tx access list storage key lookup", |meta| { + let cond = meta.query_fixed(q_enable, CURRENT) + * meta.query_advice(is_access_list_storage_key, CURRENT); + + let tx_id = meta.query_advice(id, CURRENT); + let index = meta.query_advice(addr, CURRENT); + let address = meta.query_advice(value, CURRENT); + let storage_key = meta.query_advice(value_prev, CURRENT); + + vec![ + 1.expr(), + tx_id, + TxContextFieldTag::AccessListStorageKey.expr(), + index, + storage_key, + address, + ] + .into_iter() + .zip(tx_table.table_exprs(meta)) + .map(|(arg, table)| (cond.clone() * arg, table)) + .collect() + }); + */ meta.lookup_any("Rw access list storage key lookup", |meta| { let cond = meta.query_fixed(q_enable, CURRENT) diff --git a/zkevm-circuits/src/copy_circuit/test.rs b/zkevm-circuits/src/copy_circuit/test.rs index baa2e5a071..4a5281b34b 100644 --- a/zkevm-circuits/src/copy_circuit/test.rs +++ b/zkevm-circuits/src/copy_circuit/test.rs @@ -291,6 +291,7 @@ fn gen_access_list_data() -> CircuitInputBuilder { .gas_price(gwei(2)) .gas(Word::from(0x10000)) .value(eth(2)) + .transaction_type(1) // Set tx type to EIP-2930. .access_list(test_access_list); }, |block, _tx| block.number(0xcafeu64), From f8a3b8a85a0684cf05a2e8baf84438983a906ace Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Sun, 10 Dec 2023 15:42:25 +0800 Subject: [PATCH 11/12] Remove the added `Default` trait from `CopyEvent`, replace `..Default::default()` with `access_list: vec![]` in call places. --- bus-mapping/src/circuit_input_builder/execution.rs | 8 +------- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 8 +++++--- bus-mapping/src/evm/opcodes/calldatacopy.rs | 4 ++-- bus-mapping/src/evm/opcodes/callop.rs | 6 +++--- bus-mapping/src/evm/opcodes/codecopy.rs | 2 +- bus-mapping/src/evm/opcodes/create.rs | 2 +- bus-mapping/src/evm/opcodes/extcodecopy.rs | 2 +- bus-mapping/src/evm/opcodes/logs.rs | 2 +- bus-mapping/src/evm/opcodes/return_revert.rs | 4 ++-- bus-mapping/src/evm/opcodes/returndatacopy.rs | 2 +- bus-mapping/src/evm/opcodes/sha3.rs | 2 +- 11 files changed, 19 insertions(+), 23 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index bc44ef4cf0..bda35a4fbc 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -366,12 +366,6 @@ pub enum NumberOrHash { Hash(H256), } -impl Default for NumberOrHash { - fn default() -> Self { - Self::Number(0) - } -} - /// Represents all bytes related in one copy event. /// /// - When the source is memory, `bytes` is the memory content, including masked areas. The @@ -413,7 +407,7 @@ impl CopyBytes { /// Defines a copy event associated with EVM opcodes such as CALLDATACOPY, /// CODECOPY, CREATE, etc. More information: /// . -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct CopyEvent { /// Represents the start address at the source of the copy event. pub src_addr: u64, diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index b45db5bc66..b3c79dadf4 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -314,7 +314,7 @@ pub fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result Opcode for CallOpcode { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, None), - ..Default::default() + access_list: vec![], }, ); Some(input_bytes) @@ -413,7 +413,7 @@ impl Opcode for CallOpcode { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), - ..Default::default() + access_list: vec![], }, ); Some(output_bytes) @@ -453,7 +453,7 @@ impl Opcode for CallOpcode { Some(write_steps), Some(prev_bytes), ), - ..Default::default() + access_list: vec![], }, ); Some(returned_bytes) diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index cdf8a6ba72..ac03acf886 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -87,7 +87,7 @@ fn gen_copy_event( rw_counter_start, //fetch pre write bytes of CopyBytes copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), - ..Default::default() + access_list: vec![], }) } diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs index 4b1bd68dec..58d65313cd 100644 --- a/bus-mapping/src/evm/opcodes/create.rs +++ b/bus-mapping/src/evm/opcodes/create.rs @@ -386,7 +386,7 @@ fn handle_copy( dst_addr: 0, log_id: None, copy_bytes: CopyBytes::new(copy_steps, None, None), - ..Default::default() + access_list: vec![], }, ); diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs index 78904c9aad..f6444ee5b4 100644 --- a/bus-mapping/src/evm/opcodes/extcodecopy.rs +++ b/bus-mapping/src/evm/opcodes/extcodecopy.rs @@ -144,7 +144,7 @@ fn gen_copy_event( log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)), - ..Default::default() + access_list: vec![], }) } diff --git a/bus-mapping/src/evm/opcodes/logs.rs b/bus-mapping/src/evm/opcodes/logs.rs index 5463440557..a7b074fc42 100644 --- a/bus-mapping/src/evm/opcodes/logs.rs +++ b/bus-mapping/src/evm/opcodes/logs.rs @@ -150,7 +150,7 @@ fn gen_copy_event( log_id: Some(state.tx_ctx.log_id as u64 + 1), rw_counter_start, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), None), - ..Default::default() + access_list: vec![], }) } diff --git a/bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return_revert.rs index cb41d543b7..2702d1d1c4 100644 --- a/bus-mapping/src/evm/opcodes/return_revert.rs +++ b/bus-mapping/src/evm/opcodes/return_revert.rs @@ -282,7 +282,7 @@ fn handle_copy( dst_addr: destination.offset.try_into().unwrap(), log_id: None, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(dst_data_prev)), - ..Default::default() + access_list: vec![], }, ); @@ -349,7 +349,7 @@ fn handle_create( dst_addr: 0, log_id: None, copy_bytes: CopyBytes::new(copy_steps, None, None), - ..Default::default() + access_list: vec![], }, ); diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs index e60407c608..7f12f67458 100644 --- a/bus-mapping/src/evm/opcodes/returndatacopy.rs +++ b/bus-mapping/src/evm/opcodes/returndatacopy.rs @@ -111,7 +111,7 @@ fn gen_copy_event( log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(prev_bytes)), - ..Default::default() + access_list: vec![], }) } diff --git a/bus-mapping/src/evm/opcodes/sha3.rs b/bus-mapping/src/evm/opcodes/sha3.rs index d62d063a32..297a711fbc 100644 --- a/bus-mapping/src/evm/opcodes/sha3.rs +++ b/bus-mapping/src/evm/opcodes/sha3.rs @@ -96,7 +96,7 @@ impl Opcode for Sha3 { log_id: None, rw_counter_start, copy_bytes: CopyBytes::new(copy_steps, None, None), - ..Default::default() + access_list: vec![], }, ); From 2501220abb87abdf5d7126c4aa3881143f8476b0 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Mon, 11 Dec 2023 09:15:43 +0800 Subject: [PATCH 12/12] Refactor to create access list of addresses and storage keys in bus-mapping. --- bus-mapping/src/evm/opcodes/begin_end_tx.rs | 83 +++++++++++---------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs index b3c79dadf4..737b1dd3d9 100644 --- a/bus-mapping/src/evm/opcodes/begin_end_tx.rs +++ b/bus-mapping/src/evm/opcodes/begin_end_tx.rs @@ -641,21 +641,16 @@ fn add_access_list_address_copy_event( let rw_counter_start = state.block_ctx.rwc; // Build copy access list including addresses. - let access_list = if let Some(access_list) = state.tx.access_list.clone() { - let mut addresses = Vec::with_capacity(access_list.0.len()); - - for item in access_list.0.iter() { - // Add RW write operations for access list addresses - // (will lookup in copy circuit). - state.tx_access_list_account_write(exec_step, tx_id, item.address, true, false)?; - - addresses.push((item.address, 0.into())); - } - - addresses - } else { - vec![] - }; + let access_list = state.tx.access_list.clone().map_or(Ok(vec![]), |al| { + al.0.iter() + .map(|item| { + // Add RW write operations for access list addresses + // (will lookup in copy circuit). + state.tx_access_list_account_write(exec_step, tx_id, item.address, true, false)?; + Ok((item.address, Word::zero())) + }) + .collect::, Error>>() + })?; let tx_id = NumberOrHash::Number(tx_id); @@ -690,32 +685,38 @@ fn add_access_list_storage_key_copy_event( let rw_counter_start = state.block_ctx.rwc; // Build copy access list including addresses and storage keys. - let access_list = if let Some(access_list) = state.tx.access_list.clone() { - let mut address_storage_keys = vec![]; - - for item in access_list.0.iter() { - for storage_key in item.storage_keys.iter() { - let storage_key = storage_key.to_word(); - - // Add RW write operations for access list address storage keys - // (will lookup in copy circuit). - state.tx_access_list_storage_key_write( - exec_step, - tx_id, - item.address, - storage_key, - true, - false, - )?; - - address_storage_keys.push((item.address, storage_key)); - } - } - - address_storage_keys - } else { - vec![] - }; + let access_list = state + .tx + .access_list + .clone() + .map_or(Ok(vec![]), |al| { + al.0.iter() + .map(|item| { + item.storage_keys + .iter() + .map(|&sk| { + let sk = sk.to_word(); + + // Add RW write operations for access list address storage keys + // (will lookup in copy circuit). + state.tx_access_list_storage_key_write( + exec_step, + tx_id, + item.address, + sk, + true, + false, + )?; + + Ok((item.address, sk)) + }) + .collect::, _>>() + }) + .collect::, Error>>() + })? + .into_iter() + .flatten() + .collect::>(); let tx_id = NumberOrHash::Number(state.tx_ctx.id());