From 22cd855fd0fae7e36057913441baf2bfd4a9e6af Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 11:35:57 +0300 Subject: [PATCH 1/7] Add get_custody_columns spec tests. --- consensus/types/src/data_column_subnet_id.rs | 17 +++-- testing/ef_tests/src/cases.rs | 14 +++- .../ef_tests/src/cases/get_custody_columns.rs | 39 +++++++++++ testing/ef_tests/src/handler.rs | 67 ++++++++++++++++++- testing/ef_tests/src/lib.rs | 8 +-- testing/ef_tests/tests/tests.rs | 6 ++ 6 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 testing/ef_tests/src/cases/get_custody_columns.rs diff --git a/consensus/types/src/data_column_subnet_id.rs b/consensus/types/src/data_column_subnet_id.rs index 94f06317360..79b427ee63f 100644 --- a/consensus/types/src/data_column_subnet_id.rs +++ b/consensus/types/src/data_column_subnet_id.rs @@ -2,6 +2,7 @@ use crate::data_column_sidecar::ColumnIndex; use crate::EthSpec; use ethereum_types::U256; +use itertools::Itertools; use safe_arith::{ArithError, SafeArith}; use serde::{Deserialize, Serialize}; use smallvec::SmallVec; @@ -60,15 +61,15 @@ impl DataColumnSubnetId { node_id: U256, custody_subnet_count: u64, ) -> impl Iterator { - // NOTE: we could perform check on `custody_subnet_count` here to ensure that it is a valid + // TODO(das): we could perform check on `custody_subnet_count` here to ensure that it is a valid // value, but here we assume it is valid. let mut subnets = SmallVec::<[u64; 32]>::new(); - let mut offset = 0; + let mut current_id = node_id; while (subnets.len() as u64) < custody_subnet_count { - let offset_node_id = node_id + U256::from(offset); - let offset_node_id = offset_node_id.low_u64().to_le_bytes(); - let hash: [u8; 32] = ethereum_hashing::hash_fixed(&offset_node_id); + let mut node_id_bytes = [0u8; 32]; + current_id.to_little_endian(&mut node_id_bytes); + let hash = ethereum_hashing::hash_fixed(&node_id_bytes); let hash_prefix = [ hash[0], hash[1], hash[2], hash[3], hash[4], hash[5], hash[6], hash[7], ]; @@ -79,7 +80,10 @@ impl DataColumnSubnetId { subnets.push(subnet); } - offset += 1 + if current_id == U256::MAX { + current_id = U256::zero() + } + current_id += U256::one() } subnets.into_iter().map(DataColumnSubnetId::new) } @@ -90,6 +94,7 @@ impl DataColumnSubnetId { ) -> impl Iterator { Self::compute_custody_subnets::(node_id, custody_subnet_count) .flat_map(|subnet| subnet.columns::()) + .sorted() } } diff --git a/testing/ef_tests/src/cases.rs b/testing/ef_tests/src/cases.rs index f328fa64047..2c0be470f32 100644 --- a/testing/ef_tests/src/cases.rs +++ b/testing/ef_tests/src/cases.rs @@ -1,6 +1,6 @@ use super::*; use rayon::prelude::*; -use std::fmt::Debug; +use std::fmt::{Debug, Display, Formatter}; use std::path::{Path, PathBuf}; use types::ForkName; @@ -18,6 +18,7 @@ mod fork; mod fork_choice; mod genesis_initialization; mod genesis_validity; +mod get_custody_columns; mod kzg_blob_to_kzg_commitment; mod kzg_compute_blob_kzg_proof; mod kzg_compute_kzg_proof; @@ -48,6 +49,7 @@ pub use epoch_processing::*; pub use fork::ForkTest; pub use genesis_initialization::*; pub use genesis_validity::*; +pub use get_custody_columns::*; pub use kzg_blob_to_kzg_commitment::*; pub use kzg_compute_blob_kzg_proof::*; pub use kzg_compute_kzg_proof::*; @@ -64,6 +66,16 @@ pub use ssz_generic::*; pub use ssz_static::*; pub use transition::TransitionTest; +pub enum FeatureName { + Eip7594, +} + +impl Display for FeatureName { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str("eip7594") + } +} + pub trait LoadCase: Sized { /// Load the test case from a test case directory. fn load_from_dir(_path: &Path, _fork_name: ForkName) -> Result; diff --git a/testing/ef_tests/src/cases/get_custody_columns.rs b/testing/ef_tests/src/cases/get_custody_columns.rs new file mode 100644 index 00000000000..608362bbf0d --- /dev/null +++ b/testing/ef_tests/src/cases/get_custody_columns.rs @@ -0,0 +1,39 @@ +use super::*; +use ethereum_types::U256; +use serde::Deserialize; +use std::marker::PhantomData; +use types::DataColumnSubnetId; + +#[derive(Debug, Clone, Deserialize)] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] +pub struct GetCustodyColumns { + pub node_id: String, + pub custody_subnet_count: u64, + pub result: Vec, + #[serde(skip)] + _phantom: PhantomData, +} + +impl LoadCase for GetCustodyColumns { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { + decode::yaml_decode_file(path.join("meta.yaml").as_path()) + } +} + +impl Case for GetCustodyColumns { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { + let node_id = U256::from_dec_str(&self.node_id) + .map_err(|e| Error::FailedToParseTest(format!("{e:?}")))?; + let computed = + DataColumnSubnetId::compute_custody_columns::(node_id, self.custody_subnet_count) + .collect::>(); + let expected = &self.result; + if computed == *expected { + Ok(()) + } else { + Err(Error::NotEqual(format!( + "Got {computed:?}\nExpected {expected:?}" + ))) + } + } +} diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index 2d5ea4149ef..de90cbd0dd8 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -1,6 +1,6 @@ use crate::cases::{self, Case, Cases, EpochTransition, LoadCase, Operation}; -use crate::type_name; use crate::type_name::TypeName; +use crate::{type_name, FeatureName}; use derivative::Derivative; use std::fs::{self, DirEntry}; use std::marker::PhantomData; @@ -82,6 +82,51 @@ pub trait Handler { ); crate::results::assert_tests_pass(&name, &handler_path, &results); } + + fn run_for_feature(&self, feature_name: FeatureName) { + let feature_name_str = feature_name.to_string(); + + let handler_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("consensus-spec-tests") + .join("tests") + .join(Self::config_name()) + .join(&feature_name_str) + .join(Self::runner_name()) + .join(self.handler_name()); + + // fork name not used for features + let dummy_fork_name = ForkName::Deneb; + + // Iterate through test suites + let as_directory = |entry: Result| -> Option { + entry + .ok() + .filter(|e| e.file_type().map(|ty| ty.is_dir()).unwrap()) + }; + + let test_cases = fs::read_dir(&handler_path) + .unwrap_or_else(|e| panic!("handler dir {} exists: {:?}", handler_path.display(), e)) + .filter_map(as_directory) + .flat_map(|suite| fs::read_dir(suite.path()).expect("suite dir exists")) + .filter_map(as_directory) + .map(|test_case_dir| { + let path = test_case_dir.path(); + let case = + Self::Case::load_from_dir(&path, dummy_fork_name).expect("test should load"); + (path, case) + }) + .collect(); + + let results = Cases { test_cases }.test_results(dummy_fork_name, Self::use_rayon()); + + let name = format!( + "{}/{}/{}", + feature_name_str, + Self::runner_name(), + self.handler_name() + ); + crate::results::assert_tests_pass(&name, &handler_path, &results); + } } macro_rules! bls_eth_handler { @@ -771,6 +816,26 @@ impl Handler for KZGVerifyKZGProofHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] +pub struct GetCustodyColumnsHandler(PhantomData); + +impl Handler for GetCustodyColumnsHandler { + type Case = cases::GetCustodyColumns; + + fn config_name() -> &'static str { + E::name() + } + + fn runner_name() -> &'static str { + "networking" + } + + fn handler_name(&self) -> String { + "get_custody_columns".into() + } +} + #[derive(Derivative)] #[derivative(Default(bound = ""))] pub struct MerkleProofValidityHandler(PhantomData); diff --git a/testing/ef_tests/src/lib.rs b/testing/ef_tests/src/lib.rs index 5ab2b4b7b43..430607c16c0 100644 --- a/testing/ef_tests/src/lib.rs +++ b/testing/ef_tests/src/lib.rs @@ -1,10 +1,10 @@ pub use case_result::CaseResult; pub use cases::WithdrawalsPayload; pub use cases::{ - Case, EffectiveBalanceUpdates, Eth1DataReset, HistoricalRootsUpdate, HistoricalSummariesUpdate, - InactivityUpdates, JustificationAndFinalization, ParticipationFlagUpdates, - ParticipationRecordUpdates, RandaoMixesReset, RegistryUpdates, RewardsAndPenalties, Slashings, - SlashingsReset, SyncCommitteeUpdates, + Case, EffectiveBalanceUpdates, Eth1DataReset, FeatureName, HistoricalRootsUpdate, + HistoricalSummariesUpdate, InactivityUpdates, JustificationAndFinalization, + ParticipationFlagUpdates, ParticipationRecordUpdates, RandaoMixesReset, RegistryUpdates, + RewardsAndPenalties, Slashings, SlashingsReset, SyncCommitteeUpdates, }; pub use decode::log_file_access; pub use error::Error; diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 5226c7ac2b0..87e5bfd49d3 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -745,3 +745,9 @@ fn rewards() { RewardsHandler::::new(handler).run(); } } + +#[test] +fn get_custody_columns() { + GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); + GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); +} From 25e1840fab6f3cdb89a57aa12d6e19d00e9bd7a5 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 15:27:23 +0300 Subject: [PATCH 2/7] Add kzg merkle proof spec tests. --- .../src/cases/merkle_proof_validity.rs | 65 ++++++++++++++++--- testing/ef_tests/tests/tests.rs | 9 +++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/testing/ef_tests/src/cases/merkle_proof_validity.rs b/testing/ef_tests/src/cases/merkle_proof_validity.rs index 8d5c0687753..b68bbdc5d39 100644 --- a/testing/ef_tests/src/cases/merkle_proof_validity.rs +++ b/testing/ef_tests/src/cases/merkle_proof_validity.rs @@ -3,7 +3,8 @@ use crate::decode::{ssz_decode_file, ssz_decode_state, yaml_decode_file}; use serde::Deserialize; use tree_hash::Hash256; use types::{ - BeaconBlockBody, BeaconBlockBodyDeneb, BeaconBlockBodyElectra, BeaconState, FullPayload, + BeaconBlockBody, BeaconBlockBodyDeneb, BeaconBlockBodyElectra, BeaconState, FixedVector, + FullPayload, Unsigned, }; #[derive(Debug, Clone, Deserialize)] @@ -81,12 +82,18 @@ impl Case for MerkleProofValidity { } } -#[derive(Debug, Clone, Deserialize)] -#[serde(bound = "E: EthSpec")] +#[derive(Debug, Clone)] pub struct KzgInclusionMerkleProofValidity { pub metadata: Option, pub block: BeaconBlockBody, pub merkle_proof: MerkleProof, + pub proof_type: KzgInclusionProofType, +} + +#[derive(Debug, Clone)] +pub enum KzgInclusionProofType { + Single, + List, } impl LoadCase for KzgInclusionMerkleProofValidity { @@ -115,21 +122,33 @@ impl LoadCase for KzgInclusionMerkleProofValidity { None }; + let file_name = path + .file_name() + .and_then(|file_name| file_name.to_str()) + .ok_or(Error::InternalError( + "failed to read file name from path".to_string(), + ))?; + + let proof_type = if file_name.starts_with("blob_kzg_commitments") { + KzgInclusionProofType::List + } else { + KzgInclusionProofType::Single + }; + Ok(Self { metadata, block, merkle_proof, + proof_type, }) } } -impl Case for KzgInclusionMerkleProofValidity { - fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { - let Ok(proof) = self.block.to_ref().kzg_commitment_merkle_proof(0) else { - return Err(Error::FailedToParseTest( - "Could not retrieve merkle proof".to_string(), - )); - }; +impl KzgInclusionMerkleProofValidity { + fn verify_kzg_inclusion_proof( + &self, + proof: FixedVector, + ) -> Result<(), Error> { let proof_len = proof.len(); let branch_len = self.merkle_proof.branch.len(); if proof_len != branch_len { @@ -153,3 +172,29 @@ impl Case for KzgInclusionMerkleProofValidity { Ok(()) } } +impl Case for KzgInclusionMerkleProofValidity { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { + match self.proof_type { + KzgInclusionProofType::Single => { + let proof = self + .block + .to_ref() + .kzg_commitment_merkle_proof(0) + .map_err(|e| { + Error::FailedToParseTest(format!("Could not retrieve merkle proof: {e:?}")) + })?; + self.verify_kzg_inclusion_proof(proof) + } + KzgInclusionProofType::List => { + let proof = self + .block + .to_ref() + .kzg_commitments_merkle_proof() + .map_err(|e| { + Error::FailedToParseTest(format!("Could not retrieve merkle proof: {e:?}")) + })?; + self.verify_kzg_inclusion_proof(proof) + } + } + } +} diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 87e5bfd49d3..e802452fc2b 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -751,3 +751,12 @@ fn get_custody_columns() { GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); } + +#[test] +#[cfg(feature = "fake_crypto")] +fn kzg_inclusion_merkle_proof_validity_eip7594() { + KzgInclusionMerkleProofValidityHandler::::default() + .run_for_feature(FeatureName::Eip7594); + KzgInclusionMerkleProofValidityHandler::::default() + .run_for_feature(FeatureName::Eip7594); +} From 50a10dd88f6f6fb5aec1ea0c3e0460ce4f98ad60 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 16:24:12 +0300 Subject: [PATCH 3/7] Add SSZ spec tests. --- testing/ef_tests/src/handler.rs | 18 ++++++++++++------ testing/ef_tests/tests/tests.rs | 9 --------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index de90cbd0dd8..cbba73e0880 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -7,6 +7,9 @@ use std::marker::PhantomData; use std::path::PathBuf; use types::{BeaconState, EthSpec, ForkName}; +const EIP7594_FORK: ForkName = ForkName::Deneb; +const EIP7594_TESTS: [&str; 2] = ["ssz_static", "single_merkle_proof"]; + pub trait Handler { type Case: Case + LoadCase; @@ -32,7 +35,11 @@ pub trait Handler { fn run(&self) { 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) + self.run_for_fork(fork_name); + + if fork_name == EIP7594_FORK && EIP7594_TESTS.contains(&Self::runner_name()) { + self.run_for_feature(FeatureName::Eip7594); + } } } } @@ -94,8 +101,8 @@ pub trait Handler { .join(Self::runner_name()) .join(self.handler_name()); - // fork name not used for features - let dummy_fork_name = ForkName::Deneb; + // Feature is currently built on top of Deneb without type changes. + let fork_name = ForkName::Deneb; // Iterate through test suites let as_directory = |entry: Result| -> Option { @@ -111,13 +118,12 @@ pub trait Handler { .filter_map(as_directory) .map(|test_case_dir| { let path = test_case_dir.path(); - let case = - Self::Case::load_from_dir(&path, dummy_fork_name).expect("test should load"); + let case = Self::Case::load_from_dir(&path, fork_name).expect("test should load"); (path, case) }) .collect(); - let results = Cases { test_cases }.test_results(dummy_fork_name, Self::use_rayon()); + let results = Cases { test_cases }.test_results(fork_name, Self::use_rayon()); let name = format!( "{}/{}/{}", diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index e802452fc2b..87e5bfd49d3 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -751,12 +751,3 @@ fn get_custody_columns() { GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); } - -#[test] -#[cfg(feature = "fake_crypto")] -fn kzg_inclusion_merkle_proof_validity_eip7594() { - KzgInclusionMerkleProofValidityHandler::::default() - .run_for_feature(FeatureName::Eip7594); - KzgInclusionMerkleProofValidityHandler::::default() - .run_for_feature(FeatureName::Eip7594); -} From 3703007e2574152c8e9235123d4761f541758093 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 19:01:55 +0300 Subject: [PATCH 4/7] Add remaining KZG tests --- testing/ef_tests/check_all_files_accessed.py | 4 + testing/ef_tests/src/cases.rs | 12 +++ .../src/cases/kzg_blob_to_kzg_commitment.rs | 4 + .../src/cases/kzg_compute_blob_kzg_proof.rs | 4 + .../cases/kzg_compute_cells_and_kzg_proofs.rs | 69 ++++++++++++++++ .../src/cases/kzg_compute_kzg_proof.rs | 4 + .../src/cases/kzg_verify_blob_kzg_proof.rs | 33 +++++++- .../cases/kzg_verify_blob_kzg_proof_batch.rs | 4 + .../cases/kzg_verify_cell_kzg_proof_batch.rs | 78 +++++++++++++++++++ .../src/cases/kzg_verify_kzg_proof.rs | 4 + testing/ef_tests/src/handler.rs | 58 ++++++++++++-- testing/ef_tests/tests/tests.rs | 18 ++++- 12 files changed, 282 insertions(+), 10 deletions(-) create mode 100644 testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs create mode 100644 testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index 7629d61827f..8cfdf988fa1 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -31,6 +31,10 @@ "tests/.*/.*/ssz_static/LightClientStore", # LightClientSnapshot "tests/.*/.*/ssz_static/LightClientSnapshot", + # Unused kzg methods + "tests/.*/.*/kzg/compute_cells", + "tests/.*/.*/kzg/recover_all_cells", + "tests/.*/.*/kzg/verify_cell_kzg_batch", # One of the EF researchers likes to pack the tarballs on a Mac ".*\.DS_Store.*", # More Mac weirdness. diff --git a/testing/ef_tests/src/cases.rs b/testing/ef_tests/src/cases.rs index 2c0be470f32..d89f3a3d302 100644 --- a/testing/ef_tests/src/cases.rs +++ b/testing/ef_tests/src/cases.rs @@ -21,9 +21,11 @@ mod genesis_validity; mod get_custody_columns; mod kzg_blob_to_kzg_commitment; mod kzg_compute_blob_kzg_proof; +mod kzg_compute_cells_and_kzg_proofs; mod kzg_compute_kzg_proof; mod kzg_verify_blob_kzg_proof; mod kzg_verify_blob_kzg_proof_batch; +mod kzg_verify_cell_kzg_proof_batch; mod kzg_verify_kzg_proof; mod merkle_proof_validity; mod operations; @@ -52,9 +54,11 @@ pub use genesis_validity::*; pub use get_custody_columns::*; pub use kzg_blob_to_kzg_commitment::*; pub use kzg_compute_blob_kzg_proof::*; +pub use kzg_compute_cells_and_kzg_proofs::*; pub use kzg_compute_kzg_proof::*; pub use kzg_verify_blob_kzg_proof::*; pub use kzg_verify_blob_kzg_proof_batch::*; +pub use kzg_verify_cell_kzg_proof_batch::*; pub use kzg_verify_kzg_proof::*; pub use merkle_proof_validity::*; pub use operations::*; @@ -66,6 +70,7 @@ pub use ssz_generic::*; pub use ssz_static::*; pub use transition::TransitionTest; +#[derive(Debug, PartialEq)] pub enum FeatureName { Eip7594, } @@ -96,6 +101,13 @@ pub trait Case: Debug + Sync { true } + /// Whether or not this test exists for the given `feature_name`. + /// + /// Returns `true` by default. + fn is_enabled_for_feature(_feature_name: FeatureName) -> bool { + true + } + /// Execute a test and return the result. /// /// `case_index` reports the index of the case in the set of test cases. It is not strictly diff --git a/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs b/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs index aa48c127b20..d1110492fe4 100644 --- a/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs +++ b/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs @@ -31,6 +31,10 @@ impl Case for KZGBlobToKZGCommitment { 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()?; diff --git a/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs index 71e1ff8e23d..61e7248deeb 100644 --- a/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs @@ -32,6 +32,10 @@ impl Case for KZGComputeBlobKZGProof { 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::(&input.blob)?; diff --git a/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs b/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs new file mode 100644 index 00000000000..cb1c7f5b1b9 --- /dev/null +++ b/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs @@ -0,0 +1,69 @@ +use super::*; +use crate::case_result::compare_result; +use kzg::KzgProof; +use kzg::{Blob as KzgBlob, Cell}; +use serde::Deserialize; +use std::marker::PhantomData; + +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct KZGComputeCellsAndKzgProofsInput { + pub blob: String, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] +pub struct KZGComputeCellsAndKZGProofs { + pub input: KZGComputeCellsAndKzgProofsInput, + pub output: Option<(Vec, Vec)>, + #[serde(skip)] + _phantom: PhantomData, +} + +impl LoadCase for KZGComputeCellsAndKZGProofs { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { + decode::yaml_decode_file(path.join("data.yaml").as_path()) + } +} + +impl Case for KZGComputeCellsAndKZGProofs { + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name == ForkName::Deneb + } + + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { + // TODO(das): lazy init Kzg as a static ref + let kzg = get_kzg()?; + let cells_and_proofs = parse_blob::(&self.input.blob).and_then(|blob| { + let blob = KzgBlob::from_bytes(&blob).map_err(|e| { + Error::InternalError(format!("Failed to convert blob to kzg blob: {e:?}")) + })?; + kzg.compute_cells_and_proofs(&blob).map_err(|e| { + Error::InternalError(format!("Failed to compute cells and kzg proofs: {e:?}")) + }) + }); + + let expected = self.output.as_ref().and_then(|(cells, proofs)| { + parse_cells_and_proofs::(cells, proofs) + .map(|(cells, proofs)| { + ( + cells + .try_into() + .map_err(|e| { + Error::FailedToParseTest(format!("Failed to parse cells: {e:?}")) + }) + .unwrap(), + proofs + .try_into() + .map_err(|e| { + Error::FailedToParseTest(format!("Failed to parse proofs: {e:?}")) + }) + .unwrap(), + ) + }) + .ok() + }); + + compare_result::<(Box<[Cell; 128]>, Box<[KzgProof; 128]>), _>(&cells_and_proofs, &expected) + } +} diff --git a/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs index 98bb7492491..ca19882d501 100644 --- a/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs @@ -39,6 +39,10 @@ impl Case for KZGComputeKZGProof { 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::(&input.blob)?; diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs index f68f0fd7ed0..58fe84c7dcd 100644 --- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs @@ -2,7 +2,7 @@ use super::*; use crate::case_result::compare_result; use beacon_chain::kzg_utils::validate_blob; use eth2_network_config::TRUSTED_SETUP_BYTES; -use kzg::{Error as KzgError, Kzg, KzgCommitment, KzgProof, TrustedSetup}; +use kzg::{Cell, Error as KzgError, Kzg, KzgCommitment, KzgProof, TrustedSetup}; use serde::Deserialize; use std::marker::PhantomData; use types::Blob; @@ -14,6 +14,33 @@ pub fn get_kzg() -> Result { .map_err(|e| Error::InternalError(format!("Failed to initialize kzg: {:?}", e))) } +// TODO(das) `EthSpec` can be removed once KZG lib exposes the `BytesPerCell` constant. +pub fn parse_cells_and_proofs( + cells: &Vec, + proofs: &Vec, +) -> Result<(Vec, Vec), Error> { + let cells = cells + .iter() + .map(|s| parse_cell::(s.as_str())) + .collect::, Error>>()?; + + let proofs = proofs + .iter() + .map(|s| parse_proof(s.as_str())) + .collect::, Error>>()?; + + Ok((cells, proofs)) +} + +pub fn parse_cell(cell: &str) -> Result { + hex::decode(strip_0x(cell)?) + .map_err(|e| Error::FailedToParseTest(format!("Failed to parse cell: {:?}", e))) + .and_then(|bytes| { + Cell::from_bytes(bytes.as_ref()) + .map_err(|e| Error::FailedToParseTest(format!("Failed to parse proof: {:?}", e))) + }) +} + pub fn parse_proof(proof: &str) -> Result { hex::decode(strip_0x(proof)?) .map_err(|e| Error::FailedToParseTest(format!("Failed to parse proof: {:?}", e))) @@ -80,6 +107,10 @@ impl Case for KZGVerifyBlobKZGProof { 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, KzgCommitment, KzgProof), Error> { let blob = parse_blob::(&input.blob)?; diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs index ae5caedf069..2566cb7ea19 100644 --- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs +++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs @@ -33,6 +33,10 @@ impl Case for KZGVerifyBlobKZGProofBatch { 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 diff --git a/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs b/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs new file mode 100644 index 00000000000..24829be577d --- /dev/null +++ b/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs @@ -0,0 +1,78 @@ +use super::*; +use crate::case_result::compare_result; +use kzg::{Bytes48, Error as KzgError}; +use serde::Deserialize; +use std::marker::PhantomData; + +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct KZGVerifyCellKZGProofBatchInput { + pub row_commitments: Vec, + pub row_indices: Vec, + pub column_indices: Vec, + pub cells: Vec, + pub proofs: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] +pub struct KZGVerifyCellKZGProofBatch { + pub input: KZGVerifyCellKZGProofBatchInput, + pub output: Option, + #[serde(skip)] + _phantom: PhantomData, +} + +impl LoadCase for KZGVerifyCellKZGProofBatch { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { + decode::yaml_decode_file(path.join("data.yaml").as_path()) + } +} + +impl Case for KZGVerifyCellKZGProofBatch { + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name == ForkName::Deneb + } + + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { + let parse_input = |input: &KZGVerifyCellKZGProofBatchInput| -> Result<_, Error> { + let (cells, proofs) = parse_cells_and_proofs::(&input.cells, &input.proofs)?; + let row_commitments = input + .row_commitments + .iter() + .map(|s| parse_commitment(s)) + .collect::, _>>()?; + let coordinates = input + .row_indices + .iter() + .zip(&input.column_indices) + .map(|(&row, &col)| (row as u64, col as u64)) + .collect::>(); + + Ok((cells, proofs, coordinates, row_commitments)) + }; + + let kzg = get_kzg()?; + + let result = + parse_input(&self.input).and_then(|(cells, proofs, coordinates, commitments)| { + let proofs: Vec = proofs.iter().map(|&proof| proof.into()).collect(); + let commitments: Vec = commitments.iter().map(|&c| c.into()).collect(); + match kzg.verify_cell_proof_batch( + cells.as_slice(), + &proofs, + &coordinates, + &commitments, + ) { + Ok(_) => Ok(true), + Err(KzgError::KzgVerificationFailed) => Ok(false), + Err(e) => Err(Error::InternalError(format!( + "Failed to validate cells: {:?}", + e + ))), + } + }); + + compare_result::(&result, &self.output) + } +} diff --git a/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs index e395558e0e1..4468176c277 100644 --- a/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs @@ -33,6 +33,10 @@ impl Case for KZGVerifyKZGProof { 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)?; diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index cbba73e0880..39dbc6ed88b 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use types::{BeaconState, EthSpec, ForkName}; const EIP7594_FORK: ForkName = ForkName::Deneb; -const EIP7594_TESTS: [&str; 2] = ["ssz_static", "single_merkle_proof"]; +const EIP7594_TESTS: [&str; 4] = ["ssz_static", "merkle_proof", "networking", "kzg"]; pub trait Handler { type Case: Case + LoadCase; @@ -32,13 +32,20 @@ pub trait Handler { Self::Case::is_enabled_for_fork(fork_name) } + fn is_enabled_for_feature(&self, feature_name: FeatureName) -> bool { + Self::Case::is_enabled_for_feature(feature_name) + } + fn run(&self) { 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.run_for_feature(FeatureName::Eip7594); + 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); } } } @@ -90,7 +97,7 @@ pub trait Handler { crate::results::assert_tests_pass(&name, &handler_path, &results); } - fn run_for_feature(&self, feature_name: FeatureName) { + fn run_for_feature(&self, fork_name: ForkName, feature_name: FeatureName) { let feature_name_str = feature_name.to_string(); let handler_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) @@ -101,9 +108,6 @@ pub trait Handler { .join(Self::runner_name()) .join(self.handler_name()); - // Feature is currently built on top of Deneb without type changes. - let fork_name = ForkName::Deneb; - // Iterate through test suites let as_directory = |entry: Result| -> Option { entry @@ -842,6 +846,46 @@ impl Handler for GetCustodyColumnsHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] +pub struct KZGComputeCellsAndKZGProofHandler(PhantomData); + +impl Handler for KZGComputeCellsAndKZGProofHandler { + type Case = cases::KZGComputeCellsAndKZGProofs; + + fn config_name() -> &'static str { + "general" + } + + fn runner_name() -> &'static str { + "kzg" + } + + fn handler_name(&self) -> String { + "compute_cells_and_kzg_proofs".into() + } +} + +#[derive(Derivative)] +#[derivative(Default(bound = ""))] +pub struct KZGVerifyCellKZGProofBatchHandler(PhantomData); + +impl Handler for KZGVerifyCellKZGProofBatchHandler { + type Case = cases::KZGVerifyCellKZGProofBatch; + + fn config_name() -> &'static str { + "general" + } + + fn runner_name() -> &'static str { + "kzg" + } + + fn handler_name(&self) -> String { + "verify_cell_kzg_proof_batch".into() + } +} + #[derive(Derivative)] #[derivative(Default(bound = ""))] pub struct MerkleProofValidityHandler(PhantomData); diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 87e5bfd49d3..2a5d933acff 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -726,6 +726,18 @@ fn kzg_verify_kzg_proof() { KZGVerifyKZGProofHandler::::default().run(); } +#[test] +fn kzg_compute_cells_and_proofs() { + KZGComputeCellsAndKZGProofHandler::::default() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); +} + +#[test] +fn kzg_verify_cell_proof_batch() { + KZGVerifyCellKZGProofBatchHandler::::default() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); +} + #[test] fn merkle_proof_validity() { MerkleProofValidityHandler::::default().run(); @@ -748,6 +760,8 @@ fn rewards() { #[test] fn get_custody_columns() { - GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); - GetCustodyColumnsHandler::::default().run_for_feature(FeatureName::Eip7594); + GetCustodyColumnsHandler::::default() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); + GetCustodyColumnsHandler::::default() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); } From 146b30ebf433a67e92f43cff5bfe03cd69c15eef Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 22:49:09 +0300 Subject: [PATCH 5/7] Load KZG only once per process, exclude electra tests and add missing SSZ tests. --- Cargo.lock | 1 + consensus/types/src/lib.rs | 4 +-- testing/ef_tests/Cargo.toml | 1 + testing/ef_tests/check_all_files_accessed.py | 4 ++- testing/ef_tests/src/cases.rs | 4 ++- .../src/cases/kzg_blob_to_kzg_commitment.rs | 4 +-- .../src/cases/kzg_compute_blob_kzg_proof.rs | 3 +-- .../cases/kzg_compute_cells_and_kzg_proofs.rs | 16 +++++++----- .../src/cases/kzg_compute_kzg_proof.rs | 3 +-- .../src/cases/kzg_verify_blob_kzg_proof.rs | 25 +++++++++++-------- .../cases/kzg_verify_blob_kzg_proof_batch.rs | 4 +-- .../cases/kzg_verify_cell_kzg_proof_batch.rs | 6 ++--- .../src/cases/kzg_verify_kzg_proof.rs | 3 +-- testing/ef_tests/src/type_name.rs | 3 ++- testing/ef_tests/tests/tests.rs | 23 ++++++++++++++--- 15 files changed, 63 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ee0c68c83e..ce71a00af37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2158,6 +2158,7 @@ dependencies = [ "fs2", "hex", "kzg", + "lazy_static", "logging", "rayon", "serde", diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 66e8cdd2914..98c0944d2c5 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -134,7 +134,7 @@ pub use crate::beacon_block_body::{ pub use crate::beacon_block_header::BeaconBlockHeader; pub use crate::beacon_committee::{BeaconCommittee, OwnedBeaconCommittee}; pub use crate::beacon_state::{Error as BeaconStateError, *}; -pub use crate::blob_sidecar::{BlobSidecar, BlobSidecarList, BlobsList}; +pub use crate::blob_sidecar::{BlobIdentifier, BlobSidecar, BlobSidecarList, BlobsList}; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; pub use crate::checkpoint::Checkpoint; @@ -143,7 +143,7 @@ pub use crate::config_and_preset::{ }; pub use crate::consolidation::Consolidation; pub use crate::contribution_and_proof::ContributionAndProof; -pub use crate::data_column_sidecar::{ColumnIndex, DataColumnSidecar}; +pub use crate::data_column_sidecar::{ColumnIndex, DataColumnIdentifier, DataColumnSidecar}; pub use crate::data_column_subnet_id::DataColumnSubnetId; pub use crate::deposit::{Deposit, DEPOSIT_TREE_DEPTH}; pub use crate::deposit_data::DepositData; diff --git a/testing/ef_tests/Cargo.toml b/testing/ef_tests/Cargo.toml index f3d00fa035c..c1b34a2dc87 100644 --- a/testing/ef_tests/Cargo.toml +++ b/testing/ef_tests/Cargo.toml @@ -40,3 +40,4 @@ store = { workspace = true } fork_choice = { workspace = true } execution_layer = { workspace = true } logging = { workspace = true } +lazy_static = { workspace = true } diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index 8cfdf988fa1..97adf3a8736 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -20,6 +20,8 @@ # following regular expressions, we will assume they are to be ignored (i.e., we are purposefully # *not* running the spec tests). excluded_paths = [ + # TODO(das): remove once electra tests are on unstable + "tests/.*/electra/", # Eth1Block and PowBlock # # Intentionally omitted, as per https://github.com/sigp/lighthouse/issues/1835 @@ -34,7 +36,7 @@ # Unused kzg methods "tests/.*/.*/kzg/compute_cells", "tests/.*/.*/kzg/recover_all_cells", - "tests/.*/.*/kzg/verify_cell_kzg_batch", + "tests/.*/.*/kzg/verify_cell_kzg_proof", # One of the EF researchers likes to pack the tarballs on a Mac ".*\.DS_Store.*", # More Mac weirdness. diff --git a/testing/ef_tests/src/cases.rs b/testing/ef_tests/src/cases.rs index d89f3a3d302..2137452d46d 100644 --- a/testing/ef_tests/src/cases.rs +++ b/testing/ef_tests/src/cases.rs @@ -77,7 +77,9 @@ pub enum FeatureName { impl Display for FeatureName { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.write_str("eip7594") + match self { + FeatureName::Eip7594 => f.write_str("eip7594"), + } } } diff --git a/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs b/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs index d1110492fe4..78889cc0271 100644 --- a/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs +++ b/testing/ef_tests/src/cases/kzg_blob_to_kzg_commitment.rs @@ -36,10 +36,8 @@ impl Case for KZGBlobToKZGCommitment { } fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { - let kzg = get_kzg()?; - let commitment = parse_blob::(&self.input.blob).and_then(|blob| { - blob_to_kzg_commitment::(&kzg, &blob).map_err(|e| { + blob_to_kzg_commitment::(&KZG, &blob).map_err(|e| { Error::InternalError(format!("Failed to compute kzg commitment: {:?}", e)) }) }); diff --git a/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs index 61e7248deeb..15bcc6f7d0b 100644 --- a/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_compute_blob_kzg_proof.rs @@ -43,9 +43,8 @@ impl Case for KZGComputeBlobKZGProof { Ok((blob, commitment)) }; - let kzg = get_kzg()?; let proof = parse_input(&self.input).and_then(|(blob, commitment)| { - compute_blob_kzg_proof::(&kzg, &blob, commitment) + compute_blob_kzg_proof::(&KZG, &blob, commitment) .map_err(|e| Error::InternalError(format!("Failed to compute kzg proof: {:?}", e))) }); diff --git a/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs b/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs index cb1c7f5b1b9..63d528467bd 100644 --- a/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs +++ b/testing/ef_tests/src/cases/kzg_compute_cells_and_kzg_proofs.rs @@ -1,7 +1,7 @@ use super::*; use crate::case_result::compare_result; -use kzg::KzgProof; use kzg::{Blob as KzgBlob, Cell}; +use kzg::{KzgProof, CELLS_PER_EXT_BLOB}; use serde::Deserialize; use std::marker::PhantomData; @@ -32,19 +32,17 @@ impl Case for KZGComputeCellsAndKZGProofs { } fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { - // TODO(das): lazy init Kzg as a static ref - let kzg = get_kzg()?; let cells_and_proofs = parse_blob::(&self.input.blob).and_then(|blob| { let blob = KzgBlob::from_bytes(&blob).map_err(|e| { Error::InternalError(format!("Failed to convert blob to kzg blob: {e:?}")) })?; - kzg.compute_cells_and_proofs(&blob).map_err(|e| { + KZG.compute_cells_and_proofs(&blob).map_err(|e| { Error::InternalError(format!("Failed to compute cells and kzg proofs: {e:?}")) }) }); let expected = self.output.as_ref().and_then(|(cells, proofs)| { - parse_cells_and_proofs::(cells, proofs) + parse_cells_and_proofs(cells, proofs) .map(|(cells, proofs)| { ( cells @@ -64,6 +62,12 @@ impl Case for KZGComputeCellsAndKZGProofs { .ok() }); - compare_result::<(Box<[Cell; 128]>, Box<[KzgProof; 128]>), _>(&cells_and_proofs, &expected) + compare_result::< + ( + Box<[Cell; CELLS_PER_EXT_BLOB]>, + Box<[KzgProof; CELLS_PER_EXT_BLOB]>, + ), + _, + >(&cells_and_proofs, &expected) } } diff --git a/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs index ca19882d501..88e90dbf1f2 100644 --- a/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_compute_kzg_proof.rs @@ -50,9 +50,8 @@ impl Case for KZGComputeKZGProof { Ok((blob, z)) }; - let kzg = get_kzg()?; let proof = parse_input(&self.input).and_then(|(blob, z)| { - compute_kzg_proof::(&kzg, &blob, z) + compute_kzg_proof::(&KZG, &blob, z) .map_err(|e| Error::InternalError(format!("Failed to compute kzg proof: {:?}", e))) }); diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs index 58fe84c7dcd..dcbe948fbe7 100644 --- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs @@ -3,25 +3,29 @@ use crate::case_result::compare_result; use beacon_chain::kzg_utils::validate_blob; use eth2_network_config::TRUSTED_SETUP_BYTES; use kzg::{Cell, Error as KzgError, Kzg, KzgCommitment, KzgProof, TrustedSetup}; +use lazy_static::lazy_static; use serde::Deserialize; use std::marker::PhantomData; +use std::sync::Arc; use types::Blob; -pub fn get_kzg() -> Result { - let trusted_setup: TrustedSetup = serde_json::from_reader(TRUSTED_SETUP_BYTES) - .map_err(|e| Error::InternalError(format!("Failed to initialize kzg: {:?}", e)))?; - Kzg::new_from_trusted_setup(trusted_setup) - .map_err(|e| Error::InternalError(format!("Failed to initialize kzg: {:?}", e))) +lazy_static! { + pub static ref KZG: Arc = { + let trusted_setup: TrustedSetup = serde_json::from_reader(TRUSTED_SETUP_BYTES) + .map_err(|e| format!("Unable to read trusted setup file: {}", e)) + .expect("should have trusted setup"); + let kzg = Kzg::new_from_trusted_setup(trusted_setup).expect("should create kzg"); + Arc::new(kzg) + }; } -// TODO(das) `EthSpec` can be removed once KZG lib exposes the `BytesPerCell` constant. -pub fn parse_cells_and_proofs( +pub fn parse_cells_and_proofs( cells: &Vec, proofs: &Vec, ) -> Result<(Vec, Vec), Error> { let cells = cells .iter() - .map(|s| parse_cell::(s.as_str())) + .map(|s| parse_cell(s.as_str())) .collect::, Error>>()?; let proofs = proofs @@ -32,7 +36,7 @@ pub fn parse_cells_and_proofs( Ok((cells, proofs)) } -pub fn parse_cell(cell: &str) -> Result { +pub fn parse_cell(cell: &str) -> Result { hex::decode(strip_0x(cell)?) .map_err(|e| Error::FailedToParseTest(format!("Failed to parse cell: {:?}", e))) .and_then(|bytes| { @@ -119,9 +123,8 @@ impl Case for KZGVerifyBlobKZGProof { Ok((blob, commitment, proof)) }; - let kzg = get_kzg()?; let result = parse_input(&self.input).and_then(|(blob, commitment, proof)| { - match validate_blob::(&kzg, &blob, commitment, proof) { + match validate_blob::(&KZG, &blob, commitment, proof) { Ok(_) => Ok(true), Err(KzgError::KzgVerificationFailed) => Ok(false), Err(e) => Err(Error::InternalError(format!( diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs index 2566cb7ea19..d4cb581c634 100644 --- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs +++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof_batch.rs @@ -57,12 +57,10 @@ impl Case for KZGVerifyBlobKZGProofBatch { Ok((commitments, blobs, proofs)) }; - let kzg = get_kzg()?; - let result = parse_input(&self.input).and_then( |(commitments, blobs, proofs)| match validate_blobs::( - &kzg, + &KZG, &commitments, blobs.iter().collect(), &proofs, diff --git a/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs b/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs index 24829be577d..150cc47770f 100644 --- a/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs +++ b/testing/ef_tests/src/cases/kzg_verify_cell_kzg_proof_batch.rs @@ -36,7 +36,7 @@ impl Case for KZGVerifyCellKZGProofBatch { fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let parse_input = |input: &KZGVerifyCellKZGProofBatchInput| -> Result<_, Error> { - let (cells, proofs) = parse_cells_and_proofs::(&input.cells, &input.proofs)?; + let (cells, proofs) = parse_cells_and_proofs(&input.cells, &input.proofs)?; let row_commitments = input .row_commitments .iter() @@ -52,13 +52,11 @@ impl Case for KZGVerifyCellKZGProofBatch { Ok((cells, proofs, coordinates, row_commitments)) }; - let kzg = get_kzg()?; - let result = parse_input(&self.input).and_then(|(cells, proofs, coordinates, commitments)| { let proofs: Vec = proofs.iter().map(|&proof| proof.into()).collect(); let commitments: Vec = commitments.iter().map(|&c| c.into()).collect(); - match kzg.verify_cell_proof_batch( + match KZG.verify_cell_proof_batch( cells.as_slice(), &proofs, &coordinates, diff --git a/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs index 4468176c277..1839333fa53 100644 --- a/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_verify_kzg_proof.rs @@ -46,9 +46,8 @@ impl Case for KZGVerifyKZGProof { Ok((commitment, z, y, proof)) }; - let kzg = get_kzg()?; let result = parse_input(&self.input).and_then(|(commitment, z, y, proof)| { - verify_kzg_proof::(&kzg, commitment, proof, z, y) + verify_kzg_proof::(&KZG, commitment, proof, z, y) .map_err(|e| Error::InternalError(format!("Failed to validate proof: {:?}", e))) }); diff --git a/testing/ef_tests/src/type_name.rs b/testing/ef_tests/src/type_name.rs index 49ebbe81909..bde05d779ce 100644 --- a/testing/ef_tests/src/type_name.rs +++ b/testing/ef_tests/src/type_name.rs @@ -1,5 +1,4 @@ //! Mapping from types to canonical string identifiers used in testing. -use types::blob_sidecar::BlobIdentifier; use types::historical_summary::HistoricalSummary; use types::*; @@ -52,7 +51,9 @@ type_name_generic!(BeaconBlockBodyDeneb, "BeaconBlockBody"); type_name!(BeaconBlockHeader); type_name_generic!(BeaconState); type_name!(BlobIdentifier); +type_name!(DataColumnIdentifier); type_name_generic!(BlobSidecar); +type_name_generic!(DataColumnSidecar); type_name!(Checkpoint); type_name_generic!(ContributionAndProof); type_name!(Deposit); diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 2a5d933acff..11c504c75b0 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -214,10 +214,11 @@ macro_rules! ssz_static_test_no_run { #[cfg(feature = "fake_crypto")] mod ssz_static { - use ef_tests::{Handler, SszStaticHandler, SszStaticTHCHandler, SszStaticWithSpecHandler}; - use types::blob_sidecar::BlobIdentifier; + use ef_tests::{ + FeatureName, Handler, SszStaticHandler, SszStaticTHCHandler, SszStaticWithSpecHandler, + }; use types::historical_summary::HistoricalSummary; - use types::{LightClientBootstrapAltair, *}; + use types::*; ssz_static_test!(aggregate_and_proof, AggregateAndProof<_>); ssz_static_test!(attestation, Attestation<_>); @@ -516,6 +517,22 @@ mod ssz_static { SszStaticHandler::::capella_and_later().run(); SszStaticHandler::::capella_and_later().run(); } + + #[test] + fn data_column_sidecar() { + SszStaticHandler::, MinimalEthSpec>::deneb_only() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); + SszStaticHandler::, MainnetEthSpec>::deneb_only() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); + } + + #[test] + fn data_column_identifier() { + SszStaticHandler::::deneb_only() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); + SszStaticHandler::::deneb_only() + .run_for_feature(ForkName::Deneb, FeatureName::Eip7594); + } } #[test] From f114b99882e05f6160c2cb26ead69b45da697050 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 13 May 2024 23:17:42 +0300 Subject: [PATCH 6/7] Fix lint and missing changes. --- testing/ef_tests/Makefile | 2 +- testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index 508c284275a..5dc3d2a0404 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v1.4.0-beta.6 +TESTS_TAG := v1.5.0-alpha.2 TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs index dcbe948fbe7..dca9af96161 100644 --- a/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs +++ b/testing/ef_tests/src/cases/kzg_verify_blob_kzg_proof.rs @@ -20,8 +20,8 @@ lazy_static! { } pub fn parse_cells_and_proofs( - cells: &Vec, - proofs: &Vec, + cells: &[String], + proofs: &[String], ) -> Result<(Vec, Vec), Error> { let cells = cells .iter() From 271614ea4725e931faf9298711cb7068a25c012d Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 14 May 2024 00:23:02 +0300 Subject: [PATCH 7/7] Ignore macOS generated file. --- testing/ef_tests/check_all_files_accessed.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index 97adf3a8736..8c7891a9030 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -41,6 +41,7 @@ ".*\.DS_Store.*", # More Mac weirdness. "tests/mainnet/bellatrix/operations/deposit/pyspec_tests/deposit_with_previous_fork_version__valid_ineffective/._meta.yaml", + "tests/mainnet/eip7594/networking/get_custody_columns/pyspec_tests/get_custody_columns__short_node_id/._meta.yaml", # bls tests are moved to bls12-381-tests directory "tests/general/phase0/bls", # some bls tests are not included now