-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(levm): add legacy tests to the suite #1870
Conversation
|
Benchmark Results ComparisonPR ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
Main ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
|
|
impl EFTest { | ||
pub fn fork(&self) -> Fork { | ||
match &self.post { | ||
EFTestPost::Prague(_) => Fork::Prague, | ||
EFTestPost::Cancun(_) => Fork::Cancun, | ||
EFTestPost::Shanghai(_) => Fork::Shanghai, | ||
EFTestPost::Homestead(_) => Fork::Homestead, | ||
EFTestPost::Istanbul(_) => Fork::Istanbul, | ||
EFTestPost::London(_) => Fork::London, | ||
EFTestPost::Byzantium(_) => Fork::Byzantium, | ||
EFTestPost::Berlin(_) => Fork::Berlin, | ||
EFTestPost::Constantinople(_) | EFTestPost::ConstantinopleFix(_) => { | ||
Fork::Constantinople | ||
} | ||
EFTestPost::Paris(_) => Fork::Paris, | ||
EFTestPost::Frontier(_) => Fork::Frontier, | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this because we no longer have a single fork for a test. Now a test consists of a single pre state and expected post state for different forks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
crates/vm/levm/Makefile
Outdated
git clone --recurse-submodules $(ETH_TEST_URL) $(TESTS_REPO) | ||
cd $(TESTS_REPO) && git checkout tags/$(ETH_TEST_TAG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to:
git clone --recurse-submodules $(ETH_TEST_URL) $(TESTS_REPO) | |
cd $(TESTS_REPO) && git checkout tags/$(ETH_TEST_TAG) | |
git clone --recurse-submodules -b tags/$(ETH_TEST_TAG) $(ETH_TEST_URL) $(TESTS_REPO) |
I believe that would also cover the following line, but check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I tried that but it didn't work out. I stick to:
git clone --recurse-submodules --depth 1 --branch $(ETH_TEST_TAG) $(ETH_TEST_URL) $(TESTS_REPO)
Done here 16a6725
Motivation
This pull request introduces changes to the Ethereum Foundation (EF) test framework, specifically enhancing the handling of forks within the test reports. The changes include modifications to the deserialization process, test report structure, and test runner logic to support multiple forks.
Description
cmd/ef_tests/levm/report.rs
: RefactoredEFTestReport
to includefork_results
instead of a singlefork
field. IntroducedEFTestReportForkResult
to encapsulate fork-specific results. Updated methods to handle the new structure and ensure correct reporting of test results per fork.cmd/ef_tests/levm/runner/levm_runner.rs
: Updated therun_ef_test
andrun_ef_test_tx
functions to iterate over forks and handle fork-specific test execution. Adjustedprepare_vm_for_tx
andensure_post_state
functions to accept afork
parameter.cmd/ef_tests/levm/runner/mod.rs
: Handled cases where test hashes might be missing to prevent panics.cmd/ef_tests/levm/runner/revm_runner.rs
: Updated imports to includeEFTestReportForkResult
.Closes #1701