Skip to content
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

Fix Shanghai tests parsing #63

Merged
merged 5 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion common/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
pub const GENERATION_INPUTS_DEFAULT_OUTPUT_DIR: &str = "generation_inputs";
pub const MAIN_TEST_DIR: &str = "BlockchainTests";
/// The source directory to look for tests to parse.
/// We use the `BlockchainTests` subdirectory of the `Cancun` folder
/// as it contains all hardfork variants up to this one.
pub const MAIN_TEST_DIR: &str = "Cancun/BlockchainTests";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name may be confusing, but it's implying all tests until Cancun included.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment to explain what MAIN_TEST_DIR corresponds to then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it might be worth a comment just explaining that a sub-group is now the main sub-group, but I'm good either way.

pub const MATIC_CHAIN_ID: u64 = 137;
pub const ETHEREUM_CHAIN_ID: u64 = 1;
34 changes: 31 additions & 3 deletions eth_test_parser/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,34 @@
pub(crate) const ETH_TESTS_REPO_URL: &str = "https://github.com/ethereum/tests.git";
use common::config::MAIN_TEST_DIR;

// The PR <https://github.com/ethereum/tests/pull/1380> moved all test versions prior Cancun HF
// to the `LegacyTests` folder.
pub(crate) const ETH_TESTS_REPO_URL: &str = "https://github.com/ethereum/legacytests.git";
pub(crate) const ETH_TESTS_REPO_LOCAL_PATH: &str = "eth_tests";
pub(crate) const GENERAL_GROUP: &str = "BlockchainTests";
pub(crate) const GENERAL_GROUP: &str = MAIN_TEST_DIR;
pub(crate) const TEST_GROUPS: [&str; 1] = ["GeneralStateTests"];
// The following subgroups contain subfolders unlike the other test folders.
pub(crate) const SPECIAL_TEST_SUBGROUPS: [&str; 3] = ["Cancun", "Shanghai", "VMTests"];
pub(crate) const SPECIAL_TEST_SUBGROUPS: [&str; 2] = ["Shanghai", "VMTests"];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are actually no _Shanghai tests in the Cancun folder so it can be simply removed.

/// These test variants are used for stress testing. As such, they have
/// unrealistic scenarios that go beyond the provable bounds of the zkEVM.
/// Witness generation for these variants is still possible, but takes too
/// much time to be useful and usable in testing occuring regularly.
pub(crate) const UNPROVABLE_VARIANTS: [&str; 17] = [
"CALLBlake2f_d9g0v0_Shanghai",
"CALLCODEBlake2f_d9g0v0_Shanghai",
"Call50000_d0g1v0_Shanghai",
"Callcode50000_d0g1v0_Shanghai",
"static_Call50000_d1g0v0_Shanghai",
"static_Call50000_ecrec_d0g0v0_Shanghai",
"static_Call50000_ecrec_d1g0v0_Shanghai",
"static_Call50000_identity2_d0g0v0_Shanghai",
"static_Call50000_identity2_d1g0v0_Shanghai",
"static_Call50000_identity_d0g0v0_Shanghai",
"static_Call50000_identity_d1g0v0_Shanghai",
"static_Call50000_rip160_d0g0v0_Shanghai",
"static_Call50000_sha256_d0g0v0_Shanghai",
"static_Call50000_sha256_d1g0v0_Shanghai",
"static_Return50000_2_d0g0v0_Shanghai",
"Return50000_d0g1v0_Shanghai",
"Return50000_2_d0g1v0_Shanghai",
];
6 changes: 5 additions & 1 deletion eth_test_parser/src/deserialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use serde::{
};
use serde_with::serde_as;

use crate::config::UNPROVABLE_VARIANTS;

#[derive(Deserialize, Debug, Clone)]
// "self" just points to this module.
pub(crate) struct ByteString(#[serde(with = "self")] pub(crate) Vec<u8>);
Expand Down Expand Up @@ -339,7 +341,9 @@ impl<'de> Deserialize<'de> for TestFile {
// While we are parsing many values, we only care about the ones containing
// `Shanghai` in their key name.
while let Some((key, value)) = access.next_entry::<String, ValueJson>()? {
if key.contains("Shanghai") {
if key.contains("Shanghai")
&& !UNPROVABLE_VARIANTS.iter().any(|v| key.contains(v))
{
if value.blocks[0].transaction_sequence.is_none() {
let test_body = TestBody::from_parsed_json(&value, key.clone());

Expand Down
Loading