Refactor spec testing for features and simplify usage.

This commit is contained in:
Jimmy Chen
2024-12-20 12:08:54 +11:00
parent d74b2d96f5
commit aa593cf35c
13 changed files with 91 additions and 56 deletions

View File

@@ -74,11 +74,37 @@ pub use ssz_generic::*;
pub use ssz_static::*;
pub use transition::TransitionTest;
#[derive(Debug, PartialEq)]
/// Used for running feature tests for future forks that have not yet been added to `ForkName`.
/// This runs tests in the directory named by the feature instead of the fork name. This has been
/// the pattern used in the `consensus-spec-tests` repository:
/// `consensus-spec-tests/tests/general/[feature_name]/[runner_name].`
/// e.g. consensus-spec-tests/tests/general/peerdas/ssz_static
///
/// The feature tests can be run with one of the following methods:
/// 1. `handler.run_for_feature(feature_name)` for new tests that are not on existing fork, i.e. a
/// new handler. This will be temporary and the test will need to be updated to use
/// `handle.run()` once the feature is incorporated into a fork.
/// 2. `handler.run()` for tests that are already on existing forks, but with new test vectors for
/// the feature. In this case the `handler.is_enabled_for_feature` will need to be implemented
/// to return `true` for the feature in order for the feature test vector to be tested.
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum FeatureName {
Eip7594,
}
impl FeatureName {
pub fn list_all() -> Vec<FeatureName> {
vec![FeatureName::Eip7594]
}
/// `ForkName` to use when running the feature tests.
pub fn fork_name(&self) -> ForkName {
match self {
FeatureName::Eip7594 => ForkName::Deneb,
}
}
}
impl Display for FeatureName {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
@@ -107,11 +133,13 @@ pub trait Case: Debug + Sync {
true
}
/// Whether or not this test exists for the given `feature_name`.
/// Whether or not this test exists for the given `feature_name`. This is intended to be used
/// for features that have not been added to a fork yet, and there is usually a separate folder
/// for the feature in the `consensus-spec-tests` repository.
///
/// Returns `true` by default.
/// Returns `false` by default.
fn is_enabled_for_feature(_feature_name: FeatureName) -> bool {
true
false
}
/// Execute a test and return the result.

View File

@@ -21,6 +21,14 @@ impl<E: EthSpec> LoadCase for GetCustodyColumns<E> {
}
impl<E: EthSpec> Case for GetCustodyColumns<E> {
fn is_enabled_for_fork(_fork_name: ForkName) -> bool {
false
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let spec = E::default_spec();
let node_id = U256::from_str_radix(&self.node_id, 10)
@@ -33,6 +41,7 @@ impl<E: EthSpec> Case for GetCustodyColumns<E> {
)
.expect("should compute custody columns")
.collect::<Vec<_>>();
let expected = &self.result;
if computed == *expected {
Ok(())

View File

@@ -31,10 +31,6 @@ impl<E: EthSpec> Case for KZGBlobToKZGCommitment<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let kzg = get_kzg();
let commitment = parse_blob::<E>(&self.input.blob).and_then(|blob| {

View File

@@ -32,10 +32,6 @@ impl<E: EthSpec> Case for KZGComputeBlobKZGProof<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let parse_input = |input: &KZGComputeBlobKZGProofInput| -> Result<_, Error> {
let blob = parse_blob::<E>(&input.blob)?;

View File

@@ -26,8 +26,12 @@ impl<E: EthSpec> LoadCase for KZGComputeCellsAndKZGProofs<E> {
}
impl<E: EthSpec> Case for KZGComputeCellsAndKZGProofs<E> {
fn is_enabled_for_fork(fork_name: ForkName) -> bool {
fork_name == ForkName::Deneb
fn is_enabled_for_fork(_fork_name: ForkName) -> bool {
false
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {

View File

@@ -39,10 +39,6 @@ impl<E: EthSpec> Case for KZGComputeKZGProof<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let parse_input = |input: &KZGComputeKZGProofInput| -> Result<_, Error> {
let blob = parse_blob::<E>(&input.blob)?;

View File

@@ -27,8 +27,12 @@ impl<E: EthSpec> LoadCase for KZGRecoverCellsAndKZGProofs<E> {
}
impl<E: EthSpec> Case for KZGRecoverCellsAndKZGProofs<E> {
fn is_enabled_for_fork(fork_name: ForkName) -> bool {
fork_name == ForkName::Deneb
fn is_enabled_for_fork(_fork_name: ForkName) -> bool {
false
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {

View File

@@ -116,10 +116,6 @@ impl<E: EthSpec> Case for KZGVerifyBlobKZGProof<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let parse_input = |input: &KZGVerifyBlobKZGProofInput| -> Result<(Blob<E>, KzgCommitment, KzgProof), Error> {
let blob = parse_blob::<E>(&input.blob)?;

View File

@@ -33,10 +33,6 @@ impl<E: EthSpec> Case for KZGVerifyBlobKZGProofBatch<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let parse_input = |input: &KZGVerifyBlobKZGProofBatchInput| -> Result<_, Error> {
let blobs = input

View File

@@ -29,8 +29,12 @@ impl<E: EthSpec> LoadCase for KZGVerifyCellKZGProofBatch<E> {
}
impl<E: EthSpec> Case for KZGVerifyCellKZGProofBatch<E> {
fn is_enabled_for_fork(fork_name: ForkName) -> bool {
fork_name == ForkName::Deneb
fn is_enabled_for_fork(_fork_name: ForkName) -> bool {
false
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name == FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {

View File

@@ -33,10 +33,6 @@ impl<E: EthSpec> Case for KZGVerifyKZGProof<E> {
fork_name == ForkName::Deneb
}
fn is_enabled_for_feature(feature_name: FeatureName) -> bool {
feature_name != FeatureName::Eip7594
}
fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> {
let parse_input = |input: &KZGVerifyKZGProofInput| -> Result<_, Error> {
let commitment = parse_commitment(&input.commitment)?;

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")
@@ -344,6 +341,10 @@ 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 {
feature_name == FeatureName::Eip7594
}
}
impl<E> Handler for SszStaticTHCHandler<BeaconState<E>, E>
@@ -363,6 +364,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>
@@ -384,6 +389,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)]
@@ -963,9 +972,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)]

View File

@@ -627,17 +627,17 @@ mod ssz_static {
#[test]
fn data_column_sidecar() {
SszStaticHandler::<DataColumnSidecar<MinimalEthSpec>, MinimalEthSpec>::deneb_only()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
SszStaticHandler::<DataColumnSidecar<MainnetEthSpec>, MainnetEthSpec>::deneb_only()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
}
#[test]
fn data_column_identifier() {
SszStaticHandler::<DataColumnIdentifier, MinimalEthSpec>::deneb_only()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
SszStaticHandler::<DataColumnIdentifier, MainnetEthSpec>::deneb_only()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
}
#[test]
@@ -902,19 +902,19 @@ fn kzg_verify_kzg_proof() {
#[test]
fn kzg_compute_cells_and_proofs() {
KZGComputeCellsAndKZGProofHandler::<MainnetEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
}
#[test]
fn kzg_verify_cell_proof_batch() {
KZGVerifyCellKZGProofBatchHandler::<MainnetEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
}
#[test]
fn kzg_recover_cells_and_proofs() {
KZGRecoverCellsAndKZGProofHandler::<MainnetEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
.run_for_feature(FeatureName::Eip7594);
}
#[test]
@@ -949,8 +949,6 @@ fn rewards() {
#[test]
fn get_custody_columns() {
GetCustodyColumnsHandler::<MainnetEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
GetCustodyColumnsHandler::<MinimalEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
GetCustodyColumnsHandler::<MainnetEthSpec>::default().run_for_feature(FeatureName::Eip7594);
GetCustodyColumnsHandler::<MinimalEthSpec>::default().run_for_feature(FeatureName::Eip7594);
}