Skip to content

Commit

Permalink
Fix remaining testool failed cases for EIP-1559 and EIP-2930 (#1088)
Browse files Browse the repository at this point in the history
* Fix testool case `storageCosts_d35(declaredKeyWrite)_g0_v0`.

* Fix to set `GasFeeCap` and `GasTipCap` if not exist (for `tipTooHigh_d0(declaredKeyWrite)_g0_v0` and `transactionIntinsicBug_d0(declaredKeyWrite)_g0_v0`).

* Parse wrong account addresses in yaml result.

* Fix `max_fee_per_gas` and `max_priority_fee_per_gas` to optional.
  • Loading branch information
silathdiir authored Jan 17, 2024
1 parent 0d3563c commit 5252666
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 32 deletions.
4 changes: 2 additions & 2 deletions bus-mapping/src/circuit_input_builder/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ impl From<&Transaction> for geth_types::Transaction {
v: tx.signature.v,
r: tx.signature.r,
s: tx.signature.s,
gas_fee_cap: tx.gas_fee_cap,
gas_tip_cap: tx.gas_tip_cap,
gas_fee_cap: Some(tx.gas_fee_cap),
gas_tip_cap: Some(tx.gas_tip_cap),
rlp_unsigned_bytes: tx.rlp_unsigned_bytes.clone(),
rlp_bytes: tx.rlp_bytes.clone(),
tx_type: tx.tx_type,
Expand Down
12 changes: 6 additions & 6 deletions eth-types/src/geth_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,9 @@ pub struct Transaction {
/// Gas Price
pub gas_price: Option<Word>,
/// Gas fee cap
pub gas_fee_cap: Word,
pub gas_fee_cap: Option<Word>,
/// Gas tip cap
pub gas_tip_cap: Word,
pub gas_tip_cap: Option<Word>,
/// The compiled code of a contract OR the first 4 bytes of the hash of the
/// invoked method signature and encoded parameters. For details see
/// Ethereum Contract ABI
Expand Down Expand Up @@ -301,8 +301,8 @@ impl From<&Transaction> for crate::Transaction {
gas: tx.gas_limit,
value: tx.value,
gas_price: tx.gas_price,
max_priority_fee_per_gas: Some(tx.gas_tip_cap),
max_fee_per_gas: Some(tx.gas_fee_cap),
max_priority_fee_per_gas: tx.gas_tip_cap,
max_fee_per_gas: tx.gas_fee_cap,
input: tx.call_data.clone(),
access_list: tx.access_list.clone(),
v: tx.v.into(),
Expand All @@ -324,8 +324,8 @@ impl From<&crate::Transaction> for Transaction {
gas_limit: tx.gas,
value: tx.value,
gas_price: tx.gas_price,
gas_tip_cap: tx.max_priority_fee_per_gas.unwrap_or_default(),
gas_fee_cap: tx.max_fee_per_gas.unwrap_or_default(),
gas_tip_cap: tx.max_priority_fee_per_gas,
gas_fee_cap: tx.max_fee_per_gas,
call_data: tx.input.clone(),
access_list: tx.access_list.clone(),
v: tx.v.as_u64(),
Expand Down
10 changes: 7 additions & 3 deletions geth-utils/l1geth/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,14 @@ func Trace(config TraceConfig) ([]*ExecutionResult, error) {
blockGasLimit := toBigInt(config.Block.GasLimit).Uint64()
messages := make([]core.Message, len(config.Transactions))
for i, tx := range config.Transactions {
// If gas price is specified directly, the tx is treated as legacy type.
if tx.GasPrice != nil {
tx.GasFeeCap = tx.GasPrice
tx.GasTipCap = tx.GasPrice
// Set GasFeeCap and GasTipCap to GasPrice if not exist.
if tx.GasFeeCap == nil {
tx.GasFeeCap = tx.GasPrice
}
if tx.GasTipCap == nil {
tx.GasTipCap = tx.GasPrice
}
}

txAccessList := make(types.AccessList, len(tx.AccessList))
Expand Down
10 changes: 7 additions & 3 deletions geth-utils/l2geth/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,14 @@ func transferTxs(txs []Transaction) types.Transactions {
}
t_txs = append(t_txs, types.NewTx(legacyTx))
default:
// If gas price is specified directly, the tx is treated as legacy type.
// if tx.GasPrice != nil {
// tx.GasFeeCap = tx.GasPrice
// tx.GasTipCap = tx.GasPrice
// // Set GasFeeCap and GasTipCap to GasPrice if not exist.
// if tx.GasFeeCap == nil {
// tx.GasFeeCap = tx.GasPrice
// }
// if tx.GasTipCap == nil {
// tx.GasTipCap = tx.GasPrice
// }
// }

// txAccessList := make(types.AccessList, len(tx.AccessList))
Expand Down
8 changes: 4 additions & 4 deletions testool/src/statetest/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ fn into_traceconfig(st: StateTest) -> (String, TraceConfig, StateTestResult) {
value: st.value,
gas_limit: U256::from(st.gas_limit),
gas_price: Some(st.gas_price),
gas_fee_cap: st.max_fee_per_gas.unwrap_or_default(),
gas_tip_cap: st.max_priority_fee_per_gas.unwrap_or_default(),
gas_fee_cap: st.max_fee_per_gas,
gas_tip_cap: st.max_priority_fee_per_gas,
call_data: st.data,
access_list: st.access_list,
v,
Expand Down Expand Up @@ -359,8 +359,8 @@ fn trace_config_to_witness_block_l1(
to: tx.to,
value: tx.value,
input: tx.call_data,
max_priority_fee_per_gas: Some(tx.gas_tip_cap),
max_fee_per_gas: Some(tx.gas_fee_cap),
max_priority_fee_per_gas: tx.gas_tip_cap,
max_fee_per_gas: tx.gas_fee_cap,
gas_price: tx.gas_price,
access_list: tx.access_list,
nonce: tx.nonce,
Expand Down
39 changes: 30 additions & 9 deletions testool/src/statetest/yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use anyhow::{anyhow, bail, Context, Result};
use eth_types::{geth_types::Account, Address, Bytes, H256, U256};
use ethers_core::{k256::ecdsa::SigningKey, utils::secret_key_to_address};
use std::{
collections::{BTreeMap, HashMap},
collections::{BTreeMap, HashMap, HashSet},
convert::TryInto,
str::FromStr,
};
Expand Down Expand Up @@ -74,7 +74,7 @@ impl<'a> YamlStateTestBuilder<'a> {

// parse pre (account states before executing the transaction)
let pre: BTreeMap<Address, Account> = self
.parse_accounts(&yaml_test["pre"])?
.parse_accounts(&yaml_test["pre"], None)?
.into_iter()
.map(|(addr, account)| (addr, account.try_into().expect("unable to parse account")))
.collect();
Expand Down Expand Up @@ -148,7 +148,10 @@ impl<'a> YamlStateTestBuilder<'a> {
let data_refs = Self::parse_refs(&expect["indexes"]["data"])?;
let gas_refs = Self::parse_refs(&expect["indexes"]["gas"])?;
let value_refs = Self::parse_refs(&expect["indexes"]["value"])?;
let result = self.parse_accounts(&expect["result"])?;

// Pass the account addresses before transaction as expected for result.
let expected_addresses = pre.keys().collect();
let result = self.parse_accounts(&expect["result"], Some(&expected_addresses))?;

if MainnetFork::in_network_range(&networks)? {
expects.push((exception, data_refs, gas_refs, value_refs, result));
Expand Down Expand Up @@ -218,7 +221,7 @@ impl<'a> YamlStateTestBuilder<'a> {
Ok(Env {
current_base_fee: Self::parse_u256(&yaml["currentBaseFee"])
.unwrap_or_else(|_| U256::from(DEFAULT_BASE_FEE)),
current_coinbase: Self::parse_address(&yaml["currentCoinbase"])?,
current_coinbase: Self::parse_address(&yaml["currentCoinbase"], None)?,
current_difficulty: Self::parse_u256(&yaml["currentDifficulty"])?,
current_gas_limit: Self::parse_u64(&yaml["currentGasLimit"])?,
current_number: Self::parse_u64(&yaml["currentNumber"])?,
Expand All @@ -228,7 +231,11 @@ impl<'a> YamlStateTestBuilder<'a> {
}

/// parse a vector of address=>(storage,balance,code,nonce) entry
fn parse_accounts(&mut self, yaml: &Yaml) -> Result<HashMap<Address, AccountMatch>> {
fn parse_accounts(
&mut self,
yaml: &Yaml,
expected_addresses: Option<&HashSet<&Address>>,
) -> Result<HashMap<Address, AccountMatch>> {
let mut accounts = HashMap::new();
for (address, account) in yaml.as_hash().context("parse_hash")?.iter() {
let acc_storage = &account["storage"];
Expand All @@ -243,7 +250,7 @@ impl<'a> YamlStateTestBuilder<'a> {
}
}

let address = Self::parse_address(address)?;
let address = Self::parse_address(address, expected_addresses)?;
let account = AccountMatch {
address,
balance: if acc_balance.is_badvalue() {
Expand Down Expand Up @@ -297,12 +304,26 @@ impl<'a> YamlStateTestBuilder<'a> {
tags
}
/// returns the element as an address
fn parse_address(yaml: &Yaml) -> Result<Address> {
fn parse_address(
yaml: &Yaml,
expected_addresses: Option<&HashSet<&Address>>,
) -> Result<Address> {
if let Some(as_str) = yaml.as_str() {
parse::parse_address(as_str)
} else if let Some(as_i64) = yaml.as_i64() {
let hex = format!("{as_i64:0>40}");
Ok(Address::from_slice(&hex::decode(hex)?))
// Try to convert the integer to hex if has expected addresses for
// accounts after transaction (result).
// e.g. 10 -> 0xa
if let Some(expected_addresses) = expected_addresses {
let address = Address::from_slice(&hex::decode(format!("{as_i64:040x}"))?);
if expected_addresses.contains(&address) {
return Ok(address);
}
}

// Format as a hex string directly for accounts before transaction (pre).
// e.g. 10 -> 0x10
Ok(Address::from_slice(&hex::decode(format!("{as_i64:0>40}"))?))
} else if let Yaml::Real(as_real) = yaml {
Ok(Address::from_str(as_real)?)
} else {
Expand Down
10 changes: 5 additions & 5 deletions zkevm-circuits/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1905,11 +1905,11 @@ impl CopyTable {

if is_access_list {
let access_list = &copy_event.access_list[step_idx / 2];
[thread.word_rlc, thread.word_rlc_prev] = [
access_list.address.to_scalar(),
access_list.storage_key.to_scalar(),
]
.map(|val| Value::known(val.unwrap()));

thread.word_rlc = Value::known(access_list.address.to_scalar().unwrap());
thread.word_rlc_prev = challenges
.evm_word()
.map(|challenge| rlc::value(&access_list.storage_key.to_le_bytes(), challenge));
}

let word_index = (step_idx as u64 / 2) % 32;
Expand Down

0 comments on commit 5252666

Please sign in to comment.