Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Commit

Permalink
Address various testool issues (#1630)
Browse files Browse the repository at this point in the history
### 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. <eduardsanou@posteo.net>
Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 28, 2023
1 parent c036246 commit 190ff41
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
7 changes: 6 additions & 1 deletion testool/Config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ paths = [
"EIP1559",
"EIP2930",
"stPreCompiledContracts",
"stZeroKnowledge"
"stZeroKnowledge",
"DelegatecallToPrecompile",
"RevertPrecompiledTouchExactOOG",
"StaticcallToPrecompileFrom",
"create_callprecompile_returndatasize_d0_g0_v0",
"extCodeHashPrecompiles",
]

[[skip_paths]]
Expand Down
45 changes: 24 additions & 21 deletions testool/src/statetest/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -32,14 +32,18 @@ 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})")]
SkipTestMaxSteps(usize),
#[error("SkipTestSelfDestruct")]
SkipTestSelfDestruct,
#[error("SkipTestDifficulty")]
// scroll evm always returns 0 for "difficulty" opcode
SkipTestDifficulty,
#[error("SkipTestBalanceOverflow")]
SkipTestBalanceOverflow,
Expand All @@ -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
)
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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) {
Expand All @@ -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(),
})
});
}
};

Expand Down Expand Up @@ -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)?;

Expand Down
2 changes: 1 addition & 1 deletion testool/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 190ff41

Please sign in to comment.