Update and fix access-list issues with testool

bus-mapping/src/circuit_input_builder.rs

@@ -35,9 +35,9 @@ use eth_types::{
 };
 use ethers_providers::JsonRpcClient;
 pub use execution::{
-    BigModExp, CopyBytes, CopyDataType, CopyEvent, CopyEventStepsBuilder, CopyStep, EcAddOp,
-    EcMulOp, EcPairingOp, EcPairingPair, ExecState, ExecStep, ExpEvent, ExpStep, NumberOrHash,
-    PrecompileEvent, PrecompileEvents, N_BYTES_PER_PAIR, N_PAIRING_PER_OP, SHA256,
+    BigModExp, CopyAccessList, CopyBytes, CopyDataType, CopyEvent, CopyEventStepsBuilder, CopyStep,
+    EcAddOp, EcMulOp, EcPairingOp, EcPairingPair, ExecState, ExecStep, ExpEvent, ExpStep,
+    NumberOrHash, PrecompileEvent, PrecompileEvents, N_BYTES_PER_PAIR, N_PAIRING_PER_OP, SHA256,
 };
 use hex::decode_to_slice;
 
@@ -571,15 +571,6 @@ impl<'a> CircuitInputBuilder {
         debug_tx.rlp_bytes.clear();
         debug_tx.rlp_unsigned_bytes.clear();
         log::trace!("handle_tx tx {:?}", debug_tx);
-        if let Some(al) = &eth_tx.access_list {
-            for item in &al.0 {
-                self.sdb.add_account_to_access_list(item.address);
-                for k in &item.storage_keys {
-                    self.sdb
-                        .add_account_storage_to_access_list((item.address, (*k).to_word()));
-                }
-            }
-        }
 
         // Generate BeginTx step
         let begin_tx_steps = gen_associated_steps(

bus-mapping/src/circuit_input_builder/execution.rs

@@ -404,6 +404,31 @@ impl CopyBytes {
     }
 }
 
+/// Transaction access list for copy event
+#[derive(Clone, Debug)]
+pub struct CopyAccessList {
+    /// Access list address
+    pub address: Address,
+    /// If copy data type is address it's always zero, if copy data type is
+    /// storage key, it saves the storage key.
+    pub storage_key: Word,
+    /// If copy data type is address it's always zero, if copy data type is
+    /// storage key, it saves the internal index of storage key, which starts
+    /// from zero for each address list.
+    pub storage_key_index: u64,
+}
+
+impl CopyAccessList {
+    /// Create a copy access list.
+    pub fn new(address: Address, storage_key: Word, storage_key_index: u64) -> Self {
+        Self {
+            address,
+            storage_key,
+            storage_key_index,
+        }
+    }
+}
+
 /// Defines a copy event associated with EVM opcodes such as CALLDATACOPY,
 /// CODECOPY, CREATE, etc. More information:
 /// <https://github.com/privacy-scaling-explorations/zkevm-specs/blob/master/specs/copy-proof.md>.
@@ -431,11 +456,8 @@ 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)>,
+    /// Represents transaction access list
+    pub access_list: Vec<CopyAccessList>,
 }
 
 pub type CopyEventSteps = Vec<(u8, bool, bool)>;

bus-mapping/src/evm/opcodes/begin_end_tx.rs

@@ -4,7 +4,8 @@ use super::{
 };
 use crate::{
     circuit_input_builder::{
-        CircuitInputStateRef, CopyBytes, CopyDataType, CopyEvent, ExecStep, NumberOrHash,
+        CircuitInputStateRef, CopyAccessList, CopyBytes, CopyDataType, CopyEvent, ExecStep,
+        NumberOrHash,
     },
     l2_predeployed::l1_gas_price_oracle,
     operation::{
@@ -44,8 +45,9 @@ use ethers_core::utils::get_contract_address;
 pub fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<Vec<ExecStep>, Error> {
     let mut exec_step = state.new_begin_tx_step();
 
-    // Add two copy-events for tx access-list addresses and storage keys if EIP-2930.
-    gen_tx_eip2930_ops(state, &mut exec_step)?;
+    // Add two copy-events for tx access-list addresses and storage keys for
+    // EIP-1559 and EIP-2930.
+    gen_tx_access_list_ops(state, &mut exec_step)?;
 
     let call = state.call()?.clone();
     let caller_address = call.caller_address;
@@ -744,12 +746,13 @@ fn gen_tx_l1_fee_ops(
     Ok(())
 }
 
-// Add two copy-events for tx access-list addresses and storage keys if EIP-2930.
-fn gen_tx_eip2930_ops(
+// Add two copy-events for tx access-list addresses and storage keys for
+// EIP-1559 and EIP-2930.
+fn gen_tx_access_list_ops(
     state: &mut CircuitInputStateRef,
     exec_step: &mut ExecStep,
 ) -> Result<(), Error> {
-    if !state.tx.tx_type.is_eip2930() {
+    if !(state.tx.tx_type.is_eip1559() || state.tx.tx_type.is_eip2930()) {
         return Ok(());
     }
 
@@ -772,12 +775,25 @@ fn add_access_list_address_copy_event(
             .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()))
+                let is_warm_prev = !state.sdb.add_account_to_access_list(item.address);
+                state.tx_access_list_account_write(
+                    exec_step,
+                    tx_id,
+                    item.address,
+                    true,
+                    is_warm_prev,
+                )?;
+
+                Ok(CopyAccessList::new(item.address, Word::zero(), 0))
             })
-            .collect::<Result<Vec<(_, _)>, Error>>()
+            .collect::<Result<Vec<_>, Error>>()
     })?;
 
+    // Unnecessary to add copy event if no access-list address.
+    if access_list.is_empty() {
+        return Ok(());
+    }
+
     let tx_id = NumberOrHash::Number(tx_id);
 
     // Use placeholder bytes for copy steps.
@@ -789,7 +805,8 @@ fn add_access_list_address_copy_event(
         dst_type: CopyDataType::AccessListAddresses,
         src_id: tx_id.clone(),
         dst_id: tx_id,
-        src_addr: 1, // index starts from 1.
+        // Access list address index starts from 1 in tx-table.
+        src_addr: 1,
         src_addr_end: access_list.len() as u64 + 1,
         dst_addr: 1,
         rw_counter_start,
@@ -820,21 +837,25 @@ fn add_access_list_storage_key_copy_event(
                 .map(|item| {
                     item.storage_keys
                         .iter()
-                        .map(|&sk| {
+                        .enumerate()
+                        .map(|(idx, &sk)| {
                             let sk = sk.to_word();
 
                             // Add RW write operations for access list address storage keys
                             // (will lookup in copy circuit).
+                            let is_warm_prev = !state
+                                .sdb
+                                .add_account_storage_to_access_list((item.address, sk));
                             state.tx_access_list_storage_key_write(
                                 exec_step,
                                 tx_id,
                                 item.address,
                                 sk,
                                 true,
-                                false,
+                                is_warm_prev,
                             )?;
 
-                            Ok((item.address, sk))
+                            Ok(CopyAccessList::new(item.address, sk, idx as u64))
                         })
                         .collect::<Result<Vec<_>, _>>()
                 })
@@ -844,6 +865,11 @@ fn add_access_list_storage_key_copy_event(
         .flatten()
         .collect::<Vec<_>>();
 
+    // Unnecessary to add copy event if no access-list storage key.
+    if access_list.is_empty() {
+        return Ok(());
+    }
+
     let tx_id = NumberOrHash::Number(state.tx_ctx.id());
 
     // Use placeholder bytes for copy steps.
@@ -855,9 +881,10 @@ fn add_access_list_storage_key_copy_event(
         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_addr: 1,
+        // Access list storage key index starts from 0 in tx-table.
+        src_addr: 0,
+        src_addr_end: access_list.len() as u64,
+        dst_addr: 0,
         rw_counter_start,
         copy_bytes,
         access_list,

testool/src/statetest/json.rs

@@ -137,9 +137,14 @@ impl<'a> JsonStateTestBuilder<'a> {
                 .max_fee_per_gas
                 .map_or(Ok(None), |s| parse::parse_u256(&s).map(Some))?;
 
-            // Gas price is replaced with maxFeePerGas for EIP-1559 transaction.
-            let gas_price = parse::parse_u256(&test.transaction.gas_price)
-                .unwrap_or_else(|_| max_fee_per_gas.unwrap());
+            // Set gas price to `min(max_priority_fee_per_gas + base_fee, max_fee_per_gas)` for
+            // EIP-1559 transaction.
+            // <https://github.com/ethereum/go-ethereum/blob/1485814f89d8206bb4a1c8e10a4a2893920f683a/core/state_transition.go#L167>
+            let gas_price = parse::parse_u256(&test.transaction.gas_price).unwrap_or_else(|_| {
+                max_fee_per_gas
+                    .unwrap()
+                    .min(max_priority_fee_per_gas.unwrap() + env.current_base_fee)
+            });
 
             let access_list = &test.transaction.access_list;
 

testool/src/statetest/yaml.rs

@@ -106,9 +106,14 @@ impl<'a> YamlStateTestBuilder<'a> {
                 Self::parse_u256(&yaml_transaction["maxPriorityFeePerGas"]).ok();
             let max_fee_per_gas = Self::parse_u256(&yaml_transaction["maxFeePerGas"]).ok();
 
-            // Gas price is replaced with maxFeePerGas for EIP-1559 transaction.
-            let gas_price = Self::parse_u256(&yaml_transaction["gasPrice"])
-                .unwrap_or_else(|_| max_fee_per_gas.unwrap());
+            // Set gas price to `min(max_priority_fee_per_gas + base_fee, max_fee_per_gas)` for
+            // EIP-1559 transaction.
+            // <https://github.com/ethereum/go-ethereum/blob/1485814f89d8206bb4a1c8e10a4a2893920f683a/core/state_transition.go#L167>
+            let gas_price = Self::parse_u256(&yaml_transaction["gasPrice"]).unwrap_or_else(|_| {
+                max_fee_per_gas
+                    .unwrap()
+                    .min(max_priority_fee_per_gas.unwrap() + env.current_base_fee)
+            });
 
             let nonce = Self::parse_u256(&yaml_transaction["nonce"])?;
             let to = Self::parse_to_address(&yaml_transaction["to"])?;

zkevm-circuits/src/copy_circuit.rs

@@ -451,37 +451,33 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
             .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("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_word_rlc, 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);
+            let address = meta.query_advice(value_word_rlc, CURRENT);
 
             vec![
                 1.expr(),
@@ -503,39 +499,34 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
             .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("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(value, CURRENT);
+            let storage_key = meta.query_advice(value_word_rlc_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);
+            let address = meta.query_advice(value_word_rlc, CURRENT);
+            let storage_key = meta.query_advice(value_word_rlc_prev, CURRENT);
 
             vec![
                 1.expr(),