Refactor feature testing for spec tests (#6737)

* Refactor spec testing for features and simplify usage.

* Fix `SszStatic` tests for PeerDAS: exclude eip7594 test vectors when testing Electra types.

* Merge branch 'unstable' into refactor-ef-tests-features
This commit is contained in:
Jimmy Chen
2025-01-13 11:08:51 +11:00
committed by GitHub
parent 348fbdb838
commit c9747fb77f
13 changed files with 103 additions and 56 deletions

View File

@@ -7,9 +7,6 @@ use std::marker::PhantomData;
use std::path::PathBuf;
use types::{BeaconState, EthSpec, ForkName};
const EIP7594_FORK: ForkName = ForkName::Deneb;
const EIP7594_TESTS: [&str; 4] = ["ssz_static", "merkle_proof", "networking", "kzg"];
pub trait Handler {
type Case: Case + LoadCase;
@@ -39,13 +36,12 @@ pub trait Handler {
for fork_name in ForkName::list_all() {
if !self.disabled_forks().contains(&fork_name) && self.is_enabled_for_fork(fork_name) {
self.run_for_fork(fork_name);
}
}
if fork_name == EIP7594_FORK
&& EIP7594_TESTS.contains(&Self::runner_name())
&& self.is_enabled_for_feature(FeatureName::Eip7594)
{
self.run_for_feature(EIP7594_FORK, FeatureName::Eip7594);
}
for feature_name in FeatureName::list_all() {
if self.is_enabled_for_feature(feature_name) {
self.run_for_feature(feature_name);
}
}
}
@@ -96,8 +92,9 @@ pub trait Handler {
crate::results::assert_tests_pass(&name, &handler_path, &results);
}
fn run_for_feature(&self, fork_name: ForkName, feature_name: FeatureName) {
fn run_for_feature(&self, feature_name: FeatureName) {
let feature_name_str = feature_name.to_string();
let fork_name = feature_name.fork_name();
let handler_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("consensus-spec-tests")
@@ -352,6 +349,22 @@ where
fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool {
self.supported_forks.contains(&fork_name)
}
fn is_enabled_for_feature(&self, feature_name: FeatureName) -> bool {
// This ensures we only run the tests **once** for `Eip7594`, using the types matching the
// correct fork, e.g. `Eip7594` uses SSZ types from `Deneb` as of spec test version
// `v1.5.0-alpha.8`, therefore the `Eip7594` tests should get included when testing Deneb types.
//
// e.g. Eip7594 test vectors are executed in the first line below, but excluded in the 2nd
// line when testing the type `AttestationElectra`:
//
// ```
// SszStaticHandler::<AttestationBase<MainnetEthSpec>, MainnetEthSpec>::pre_electra().run();
// SszStaticHandler::<AttestationElectra<MainnetEthSpec>, MainnetEthSpec>::electra_only().run();
// ```
feature_name == FeatureName::Eip7594
&& self.supported_forks.contains(&feature_name.fork_name())
}
}
impl<E> Handler for SszStaticTHCHandler<BeaconState<E>, E>
@@ -371,6 +384,10 @@ where
fn handler_name(&self) -> String {
BeaconState::<E>::name().into()
}
fn is_enabled_for_feature(&self, feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
}
impl<T, E> Handler for SszStaticWithSpecHandler<T, E>
@@ -392,6 +409,10 @@ where
fn handler_name(&self) -> String {
T::name().into()
}
fn is_enabled_for_feature(&self, feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
}
#[derive(Derivative)]
@@ -971,9 +992,12 @@ impl<E: EthSpec + TypeName> Handler for KzgInclusionMerkleProofValidityHandler<E
}
fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool {
// Enabled in Deneb
fork_name.deneb_enabled()
}
fn is_enabled_for_feature(&self, feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
}
#[derive(Derivative)]