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

* Update `csc` format in ENR.

* Add spec tests for `recover_cells_and_kzg_proofs`.

* Add tests for ENR.

* Fix failing tests.

* Add protection against invalid csc value in ENR.

* Fix lint

Author: jimmygchen

beacon_node/lighthouse_network/src/config.rs

@@ -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.

beacon_node/lighthouse_network/src/discovery/enr.rs

@@ -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 {
@@ -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)
     }
 
@@ -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)
@@ -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)
+    }
+}

beacon_node/lighthouse_network/src/peer_manager/peerdb.rs

@@ -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};
@@ -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");

consensus/types/src/data_column_subnet_id.rs

@@ -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
     }
 }
 

testing/ef_tests/Makefile

@@ -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))
 

testing/ef_tests/check_all_files_accessed.py

@@ -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",

testing/ef_tests/src/cases.rs

@@ -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;
@@ -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::*;

testing/ef_tests/src/cases/kzg_recover_cells_and_kzg_proofs.rs

@@ -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)
+    }
+}

testing/ef_tests/src/handler.rs

@@ -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>);

testing/ef_tests/tests/tests.rs

@@ -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();