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

Update csc format in ENR and spec tests for devnet-1 #5966

Merged
merged 6 commits into from
Jun 25, 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
2 changes: 1 addition & 1 deletion beacon_node/lighthouse_network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub struct Config {
pub network_dir: PathBuf,

/// IP addresses to listen on.
listen_addresses: ListenAddress,
pub(crate) listen_addresses: ListenAddress,

/// The address to broadcast to peers about which address we are listening on. None indicates
/// that no discovery address has been set in the CLI args.
Expand Down
79 changes: 73 additions & 6 deletions beacon_node/lighthouse_network/src/discovery/enr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub const ATTESTATION_BITFIELD_ENR_KEY: &str = "attnets";
/// The ENR field specifying the sync committee subnet bitfield.
pub const SYNC_COMMITTEE_BITFIELD_ENR_KEY: &str = "syncnets";
/// The ENR field specifying the peerdas custody subnet count.
pub const PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY: &str = "custody_subnet_count";
pub const PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY: &str = "csc";

/// Extension trait for ENR's within Eth2.
pub trait Eth2Enr {
Expand Down Expand Up @@ -68,7 +68,9 @@ impl Eth2Enr for Enr {
/// defined in the spec.
fn custody_subnet_count<E: EthSpec>(&self, spec: &ChainSpec) -> u64 {
self.get(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY)
.and_then(|custody_bytes| u64::from_ssz_bytes(custody_bytes).ok())
.and_then(|custody_bytes| custody_bytes.try_into().map(u64::from_be_bytes).ok())
// If value supplied in ENR is invalid, fallback to `custody_requirement`
.filter(|csc| csc <= &spec.data_column_sidecar_subnet_count)
.unwrap_or(spec.custody_requirement)
}

Expand Down Expand Up @@ -243,10 +245,8 @@ pub fn build_enr<E: EthSpec>(
spec.custody_requirement
};

builder.add_value(
PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY,
&custody_subnet_count.as_ssz_bytes(),
);
let csc_bytes = custody_subnet_count.to_be_bytes();
builder.add_value(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY, &csc_bytes.as_slice());

builder
.build(enr_key)
Expand Down Expand Up @@ -309,3 +309,70 @@ pub fn save_enr_to_disk(dir: &Path, enr: &Enr, log: &slog::Logger) {
}
}
}

#[cfg(test)]
mod test {
use super::*;
use crate::config::Config as NetworkConfig;
use types::MainnetEthSpec;

type E = MainnetEthSpec;

#[test]
fn custody_subnet_count_default() {
let config = NetworkConfig {
subscribe_all_data_column_subnets: false,
..NetworkConfig::default()
};
let spec = E::default_spec();
let enr = build_enr_with_config(config, &spec).0;

assert_eq!(
enr.custody_subnet_count::<E>(&spec),
spec.custody_requirement,
);
}

#[test]
fn custody_subnet_count_all() {
let config = NetworkConfig {
subscribe_all_data_column_subnets: true,
..NetworkConfig::default()
};
let spec = E::default_spec();
let enr = build_enr_with_config(config, &spec).0;

assert_eq!(
enr.custody_subnet_count::<E>(&spec),
spec.data_column_sidecar_subnet_count,
);
}

#[test]
fn custody_subnet_count_fallback_default() {
let config = NetworkConfig::default();
let spec = E::default_spec();
let (mut enr, enr_key) = build_enr_with_config(config, &spec);
let invalid_subnet_count = 999u64;

enr.insert(
PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY,
&invalid_subnet_count.to_be_bytes().as_slice(),
&enr_key,
)
.unwrap();

assert_eq!(
enr.custody_subnet_count::<E>(&spec),
spec.custody_requirement,
);
}

fn build_enr_with_config(config: NetworkConfig, spec: &ChainSpec) -> (Enr, CombinedKey) {
let keypair = libp2p::identity::secp256k1::Keypair::generate();
let enr_key = CombinedKey::from_secp256k1(&keypair);
let enr_fork_id = EnrForkId::default();
let enr = build_enr::<E>(&enr_key, &config, &enr_fork_id, spec).unwrap();
(enr, enr_key)
}
}
6 changes: 4 additions & 2 deletions beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use peer_info::{ConnectionDirection, PeerConnectionStatus, PeerInfo};
use rand::seq::SliceRandom;
use score::{PeerAction, ReportSource, Score, ScoreState};
use slog::{crit, debug, error, trace, warn};
use ssz::Encode;
use std::net::IpAddr;
use std::time::Instant;
use std::{cmp::Ordering, fmt::Display};
Expand Down Expand Up @@ -687,7 +686,10 @@ impl<E: EthSpec> PeerDB<E> {
if supernode {
enr.insert(
PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY,
&spec.data_column_sidecar_subnet_count.as_ssz_bytes(),
&spec
.data_column_sidecar_subnet_count
.to_be_bytes()
.as_slice(),
&enr_key,
)
.expect("u64 can be encoded");
Expand Down
12 changes: 6 additions & 6 deletions consensus/types/src/data_column_subnet_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@ impl From<u64> for DataColumnSubnetId {
}
}

impl Into<u64> for DataColumnSubnetId {
fn into(self) -> u64 {
self.0
impl From<DataColumnSubnetId> for u64 {
fn from(val: DataColumnSubnetId) -> Self {
val.0
}
}

impl Into<u64> for &DataColumnSubnetId {
fn into(self) -> u64 {
self.0
impl From<&DataColumnSubnetId> for u64 {
fn from(val: &DataColumnSubnetId) -> Self {
val.0
}
}

Expand Down
2 changes: 1 addition & 1 deletion testing/ef_tests/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
TESTS_TAG := v1.5.0-alpha.2
TESTS_TAG := v1.5.0-alpha.3
TESTS = general minimal mainnet
TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS))

Expand Down
6 changes: 3 additions & 3 deletions testing/ef_tests/check_all_files_accessed.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@
"tests/.*/.*/ssz_static/LightClientStore",
# LightClientSnapshot
"tests/.*/.*/ssz_static/LightClientSnapshot",
# Unused container for das
"tests/.*/.*/ssz_static/MatrixEntry",
# Unused kzg methods
"tests/.*/.*/kzg/compute_cells",
"tests/.*/.*/kzg/recover_all_cells",
"tests/.*/.*/kzg/verify_cell_kzg_proof",
# One of the EF researchers likes to pack the tarballs on a Mac
".*\.DS_Store.*",
".*/.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",
Expand Down
2 changes: 2 additions & 0 deletions testing/ef_tests/src/cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ 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_recover_cells_and_kzg_proofs;
mod kzg_verify_blob_kzg_proof;
mod kzg_verify_blob_kzg_proof_batch;
mod kzg_verify_cell_kzg_proof_batch;
Expand Down Expand Up @@ -56,6 +57,7 @@ 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_recover_cells_and_kzg_proofs::*;
pub use kzg_verify_blob_kzg_proof::*;
pub use kzg_verify_blob_kzg_proof_batch::*;
pub use kzg_verify_cell_kzg_proof_batch::*;
Expand Down
95 changes: 95 additions & 0 deletions testing/ef_tests/src/cases/kzg_recover_cells_and_kzg_proofs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
use super::*;
use crate::case_result::compare_result;
use kzg::{CellsAndKzgProofs, KzgProof};
use serde::Deserialize;
use std::convert::Infallible;
use std::marker::PhantomData;

#[derive(Debug, Clone, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct KZGRecoverCellsAndKzgProofsInput {
pub cell_indices: Vec<u64>,
pub cells: Vec<String>,
pub proofs: Vec<String>,
}

#[derive(Debug, Clone, Deserialize)]
#[serde(bound = "E: EthSpec", deny_unknown_fields)]
pub struct KZGRecoverCellsAndKZGProofs<E: EthSpec> {
pub input: KZGRecoverCellsAndKzgProofsInput,
pub output: Option<(Vec<String>, Vec<String>)>,
#[serde(skip)]
_phantom: PhantomData<E>,
}

impl<E: EthSpec> LoadCase for KZGRecoverCellsAndKZGProofs<E> {
fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result<Self, Error> {
decode::yaml_decode_file(path.join("data.yaml").as_path())
}
}

impl<E: EthSpec> Case for KZGRecoverCellsAndKZGProofs<E> {
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: &KZGRecoverCellsAndKzgProofsInput| {
// Proofs are not used for `recover_cells_and_compute_kzg_proofs`, they are only checked
// to satisfy the spec tests.
if input.proofs.len() != input.cell_indices.len() {
return Err(Error::SkippedKnownFailure);
}

let proofs = input
.proofs
.iter()
.map(|s| parse_proof(s))
.collect::<Result<Vec<_>, Error>>()?;

let cells = input
.cells
.iter()
.map(|s| parse_cell(s))
.collect::<Result<Vec<_>, Error>>()?;

Ok((proofs, cells, input.cell_indices.clone()))
};

let result =
parse_input(&self.input).and_then(|(input_proofs, input_cells, cell_indices)| {
let (cells, proofs) = KZG
.recover_cells_and_compute_kzg_proofs(
cell_indices.as_slice(),
input_cells.as_slice(),
)
.map_err(|e| {
Error::InternalError(format!(
"Failed to recover cells and kzg proofs: {e:?}"
))
})?;

// Check recovered proofs matches inputs proofs. This is done only to satisfy the
// spec tests, as the ckzg library recomputes all proofs and does not require
// proofs to recover.
for (input_proof, cell_id) in input_proofs.iter().zip(cell_indices) {
if let Err(e) = compare_result::<KzgProof, Infallible>(
&Ok(*input_proof),
&proofs.get(cell_id as usize).cloned(),
) {
return Err(e);
}
}

Ok((cells, proofs))
});

let expected = self
.output
.as_ref()
.and_then(|(cells, proofs)| parse_cells_and_proofs(cells, proofs).ok())
.map(|(cells, proofs)| (cells.try_into().unwrap(), proofs.try_into().unwrap()));

compare_result::<CellsAndKzgProofs, _>(&result, &expected)
}
}
20 changes: 20 additions & 0 deletions testing/ef_tests/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,26 @@ impl<E: EthSpec> Handler for KZGVerifyCellKZGProofBatchHandler<E> {
}
}

#[derive(Derivative)]
#[derivative(Default(bound = ""))]
pub struct KZGRecoverCellsAndKZGProofHandler<E>(PhantomData<E>);

impl<E: EthSpec> Handler for KZGRecoverCellsAndKZGProofHandler<E> {
type Case = cases::KZGRecoverCellsAndKZGProofs<E>;

fn config_name() -> &'static str {
"general"
}

fn runner_name() -> &'static str {
"kzg"
}

fn handler_name(&self) -> String {
"recover_cells_and_kzg_proofs".into()
}
}

#[derive(Derivative)]
#[derivative(Default(bound = ""))]
pub struct MerkleProofValidityHandler<E>(PhantomData<E>);
Expand Down
6 changes: 6 additions & 0 deletions testing/ef_tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,12 @@ fn kzg_verify_cell_proof_batch() {
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
}

#[test]
fn kzg_recover_cells_and_proofs() {
KZGRecoverCellsAndKZGProofHandler::<MainnetEthSpec>::default()
.run_for_feature(ForkName::Deneb, FeatureName::Eip7594);
}

#[test]
fn merkle_proof_validity() {
MerkleProofValidityHandler::<MainnetEthSpec>::default().run();
Expand Down