From 190ff417e592ccf61ba4f1b443b7d2fb7aeb79e3 Mon Sep 17 00:00:00 2001 From: Chih Cheng Liang Date: Fri, 29 Sep 2023 06:06:29 +0800 Subject: [PATCH] Address various testool issues (#1630) ### Description - [x] `SkipTestDifficulty` and `SkipTestBalanceOverflow` should not be ignored. - [x] `creationTxInitCodeSizeLimit_d1(invalid)_g0_v0` should be parsed as exception and return ok. - Address this error`Exception(expected:false, found:"TracingError(\"Failed to run Trace, err: Failed to apply config.Transactions[0]: max initcode size exceeded: code size 49153 limit 49152\")")` - [x] skip precompile tests - [x] Use shanghai fork - [x] Fix "circuit was not satisfied." We report the first failure and the total number of failures. ### Issue Link #1622 ### Type of change Bug fix (non-breaking change which fixes an issue) --------- Co-authored-by: Eduard S. Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com> --- testool/Config.toml | 7 ++++- testool/src/statetest/executor.rs | 45 ++++++++++++++++--------------- testool/src/utils.rs | 2 +- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/testool/Config.toml b/testool/Config.toml index 9352ea0589..07943c9243 100644 --- a/testool/Config.toml +++ b/testool/Config.toml @@ -16,7 +16,12 @@ paths = [ "EIP1559", "EIP2930", "stPreCompiledContracts", - "stZeroKnowledge" + "stZeroKnowledge", + "DelegatecallToPrecompile", + "RevertPrecompiledTouchExactOOG", + "StaticcallToPrecompileFrom", + "create_callprecompile_returndatasize_d0_g0_v0", + "extCodeHashPrecompiles", ] [[skip_paths]] diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index 524f0280d2..92a143cf01 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -4,7 +4,7 @@ use bus_mapping::{ circuit_input_builder::{CircuitInputBuilder, FixedCParams}, mock::BlockData, }; -use eth_types::{geth_types, Address, Bytes, GethExecTrace, ToBigEndian, U256, U64}; +use eth_types::{geth_types, Address, Bytes, Error, GethExecTrace, U256, U64}; use ethers_core::{ k256::ecdsa::SigningKey, types::{transaction::eip2718::TypedTransaction, TransactionRequest}, @@ -32,6 +32,11 @@ pub enum StateTestError { expected: U256, found: U256, }, + #[error("CircuitUnsatisfied(num_failure: {num_failure:?} first: {first_failure:?}")] + CircuitUnsatisfied { + num_failure: usize, + first_failure: String, + }, #[error("SkipTestMaxGasLimit({0})")] SkipTestMaxGasLimit(u64), #[error("SkipTestMaxSteps({0})")] @@ -39,7 +44,6 @@ pub enum StateTestError { #[error("SkipTestSelfDestruct")] SkipTestSelfDestruct, #[error("SkipTestDifficulty")] - // scroll evm always returns 0 for "difficulty" opcode SkipTestDifficulty, #[error("SkipTestBalanceOverflow")] SkipTestBalanceOverflow, @@ -49,17 +53,16 @@ pub enum StateTestError { impl StateTestError { pub fn is_skip(&self) -> bool { - // Avoid lint `variant is never constructed` if no feature skip-self-destruct. - let _ = StateTestError::SkipTestSelfDestruct; + // Avoid lint `variant is never constructed` + // We plan to add runtime-feature set in the future to include these skipping options let _ = StateTestError::SkipTestDifficulty; + let _ = StateTestError::SkipTestBalanceOverflow; matches!( self, StateTestError::SkipTestMaxSteps(_) | StateTestError::SkipTestMaxGasLimit(_) | StateTestError::SkipTestSelfDestruct - | StateTestError::SkipTestBalanceOverflow - | StateTestError::SkipTestDifficulty ) } } @@ -208,14 +211,6 @@ fn check_geth_traces( return Err(StateTestError::SkipTestSelfDestruct); } - if geth_traces.iter().any(|gt| { - gt.struct_logs - .iter() - .any(|sl| sl.op == eth_types::evm_types::OpcodeId::DIFFICULTY) - }) { - return Err(StateTestError::SkipTestDifficulty); - } - if geth_traces[0].struct_logs.len() as u64 > suite.max_steps { return Err(StateTestError::SkipTestMaxSteps( geth_traces[0].struct_logs.len(), @@ -242,11 +237,6 @@ pub fn run_test( let (_, trace_config, post) = into_traceconfig(st.clone()); - for acc in trace_config.accounts.values() { - if acc.balance.to_be_bytes()[0] != 0u8 { - return Err(StateTestError::SkipTestBalanceOverflow); - } - } let geth_traces = external_tracer::trace(&trace_config); let geth_traces = match (geth_traces, st.exception) { @@ -259,10 +249,18 @@ pub fn run_test( } (Err(_), true) => return Ok(()), (Err(err), false) => { + if let Error::TracingError(ref err) = err { + if err.contains("max initcode size exceeded") { + return Err(StateTestError::Exception { + expected: true, + found: err.to_string(), + }); + } + } return Err(StateTestError::Exception { expected: false, found: err.to_string(), - }) + }); } }; @@ -350,7 +348,12 @@ pub fn run_test( builder = _builder; let prover = MockProver::run(k, &circuit, instance).unwrap(); - prover.assert_satisfied_par(); + prover + .verify_par() + .map_err(|err| StateTestError::CircuitUnsatisfied { + num_failure: err.len(), + first_failure: err[0].to_string(), + })?; }; check_post(&builder, &post)?; diff --git a/testool/src/utils.rs b/testool/src/utils.rs index 35a4d312ba..e4e4d647ba 100644 --- a/testool/src/utils.rs +++ b/testool/src/utils.rs @@ -25,7 +25,7 @@ pub enum MainnetFork { Frontier = 1, } -pub const TEST_FORK: MainnetFork = MainnetFork::Merge; +pub const TEST_FORK: MainnetFork = MainnetFork::Shanghai; impl FromStr for MainnetFork { type Err = anyhow::Error;