From 4946e724bd70ee8d6f07c0ff97e524de85debfe9 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 16 May 2024 11:29:44 +0300 Subject: [PATCH] Update subscription filter. (#5797) --- .../lighthouse_network/src/service/mod.rs | 7 ++-- consensus/types/src/chain_spec.rs | 13 -------- consensus/types/src/data_column_subnet_id.rs | 13 ++++++++ consensus/types/src/eth_spec.rs | 32 +++---------------- 4 files changed, 21 insertions(+), 44 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 4509027c7c9..ba2d7a25ce9 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -240,7 +240,8 @@ impl Network { let max_topics = ctx.chain_spec.attestation_subnet_count as usize + SYNC_COMMITTEE_SUBNET_COUNT as usize + ctx.chain_spec.blob_sidecar_subnet_count as usize - + ctx.chain_spec.data_column_sidecar_subnet_count as usize + // TODO: move to chainspec + + E::data_column_subnet_count() + BASE_CORE_TOPICS.len() + ALTAIR_CORE_TOPICS.len() + CAPELLA_CORE_TOPICS.len() @@ -254,11 +255,11 @@ impl Network { ctx.chain_spec.attestation_subnet_count, SYNC_COMMITTEE_SUBNET_COUNT, ctx.chain_spec.blob_sidecar_subnet_count, - ctx.chain_spec.data_column_sidecar_subnet_count, + E::data_column_subnet_count() as u64, ), // during a fork we subscribe to both the old and new topics max_subscribed_topics: max_topics * 4, - // 162 in theory = (64 attestation + 4 sync committee + 7 core topics + 6 blob topics) * 2 + // 209 in theory = (64 attestation + 4 sync committee + 7 core topics + 6 blob topics + 64 column topics) * 2 max_subscriptions_per_request: max_topics * 2, }; diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 3ce2b5e8edc..facca8faddb 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -227,7 +227,6 @@ pub struct ChainSpec { pub max_request_data_column_sidecars: u64, pub min_epochs_for_blob_sidecars_requests: u64, pub blob_sidecar_subnet_count: u64, - pub data_column_sidecar_subnet_count: u64, /* * Networking Derived @@ -804,7 +803,6 @@ impl ChainSpec { max_request_data_column_sidecars: default_max_request_data_column_sidecars(), min_epochs_for_blob_sidecars_requests: default_min_epochs_for_blob_sidecars_requests(), blob_sidecar_subnet_count: default_blob_sidecar_subnet_count(), - data_column_sidecar_subnet_count: default_data_column_sidecar_subnet_count(), /* * Derived Deneb Specific @@ -1115,7 +1113,6 @@ impl ChainSpec { max_request_data_column_sidecars: default_max_request_data_column_sidecars(), min_epochs_for_blob_sidecars_requests: 16384, blob_sidecar_subnet_count: default_blob_sidecar_subnet_count(), - data_column_sidecar_subnet_count: default_data_column_sidecar_subnet_count(), /* * Derived Deneb Specific @@ -1316,9 +1313,6 @@ pub struct Config { #[serde(default = "default_blob_sidecar_subnet_count")] #[serde(with = "serde_utils::quoted_u64")] blob_sidecar_subnet_count: u64, - #[serde(default = "default_data_column_sidecar_subnet_count")] - #[serde(with = "serde_utils::quoted_u64")] - data_column_sidecar_subnet_count: u64, #[serde(default = "default_min_per_epoch_churn_limit_electra")] #[serde(with = "serde_utils::quoted_u64")] @@ -1444,10 +1438,6 @@ const fn default_blob_sidecar_subnet_count() -> u64 { 6 } -const fn default_data_column_sidecar_subnet_count() -> u64 { - 32 -} - const fn default_min_per_epoch_churn_limit_electra() -> u64 { 128_000_000_000 } @@ -1655,7 +1645,6 @@ impl Config { max_request_data_column_sidecars: spec.max_request_data_column_sidecars, min_epochs_for_blob_sidecars_requests: spec.min_epochs_for_blob_sidecars_requests, blob_sidecar_subnet_count: spec.blob_sidecar_subnet_count, - data_column_sidecar_subnet_count: spec.data_column_sidecar_subnet_count, min_per_epoch_churn_limit_electra: spec.min_per_epoch_churn_limit_electra, max_per_epoch_activation_exit_churn_limit: spec @@ -1729,7 +1718,6 @@ impl Config { max_request_data_column_sidecars, min_epochs_for_blob_sidecars_requests, blob_sidecar_subnet_count, - data_column_sidecar_subnet_count, min_per_epoch_churn_limit_electra, max_per_epoch_activation_exit_churn_limit, @@ -1795,7 +1783,6 @@ impl Config { max_request_data_column_sidecars, min_epochs_for_blob_sidecars_requests, blob_sidecar_subnet_count, - data_column_sidecar_subnet_count, min_per_epoch_churn_limit_electra, max_per_epoch_activation_exit_churn_limit, diff --git a/consensus/types/src/data_column_subnet_id.rs b/consensus/types/src/data_column_subnet_id.rs index d57f47352e2..4813f20aa34 100644 --- a/consensus/types/src/data_column_subnet_id.rs +++ b/consensus/types/src/data_column_subnet_id.rs @@ -204,4 +204,17 @@ mod test { } } } + + #[test] + fn test_columns_subnet_conversion() { + for subnet in 0..E::data_column_subnet_count() as u64 { + let subnet_id = DataColumnSubnetId::new(subnet); + for column_index in subnet_id.columns::() { + assert_eq!( + subnet_id, + DataColumnSubnetId::from_column_index::(column_index as usize) + ); + } + } + } } diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 157287c4836..0afdcdb9f34 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -120,6 +120,7 @@ pub trait EthSpec: type KzgCommitmentsInclusionProofDepth: Unsigned + Clone + Sync + Send + Debug + PartialEq; /* * Config values in PeerDAS + * TODO(das) move to `ChainSpec` */ type CustodyRequirement: Unsigned + Clone + Sync + Send + Debug + PartialEq; type DataColumnSidecarSubnetCount: Unsigned + Clone + Sync + Send + Debug + PartialEq; @@ -152,11 +153,6 @@ pub trait EthSpec: /// Must be set to `BytesPerFieldElement * FieldElementsPerCell`. type BytesPerCell: Unsigned + Clone + Sync + Send + Debug + PartialEq; - /// Number of data columns per subnet. - /// - /// Must be set to `NumberOfColumns / DataColumnSidecarSubnetCount` - type DataColumnsPerSubnet: Unsigned + Clone + Sync + Send + Debug + PartialEq; - /* * New in Electra */ @@ -369,7 +365,9 @@ pub trait EthSpec: } fn data_columns_per_subnet() -> usize { - Self::DataColumnsPerSubnet::to_usize() + Self::number_of_columns() + .safe_div(Self::data_column_subnet_count()) + .expect("Subnet count must be greater than 0") } fn custody_requirement() -> usize { @@ -434,7 +432,6 @@ impl EthSpec for MainnetEthSpec { type CustodyRequirement = U4; type DataColumnSidecarSubnetCount = U64; type NumberOfColumns = U128; - type DataColumnsPerSubnet = U2; type KzgCommitmentsInclusionProofDepth = U4; // inclusion of the whole list of commitments type SyncSubcommitteeSize = U128; // 512 committee size / 4 sync committee subnet count type MaxPendingAttestations = U4096; // 128 max attestations * 32 slots per epoch @@ -488,7 +485,6 @@ impl EthSpec for MinimalEthSpec { type CustodyRequirement = U4; type DataColumnSidecarSubnetCount = U64; type NumberOfColumns = U128; - type DataColumnsPerSubnet = U2; type KzgCommitmentsInclusionProofDepth = U4; params_from_eth_spec!(MainnetEthSpec { @@ -582,7 +578,6 @@ impl EthSpec for GnosisEthSpec { type CustodyRequirement = U4; type DataColumnSidecarSubnetCount = U64; type NumberOfColumns = U128; - type DataColumnsPerSubnet = U2; type KzgCommitmentsInclusionProofDepth = U4; fn default_spec() -> ChainSpec { @@ -593,22 +588,3 @@ impl EthSpec for GnosisEthSpec { EthSpecId::Gnosis } } - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_peer_das_config_all_specs() { - test_peer_das_config::(); - test_peer_das_config::(); - test_peer_das_config::(); - } - - fn test_peer_das_config() { - assert_eq!( - E::data_columns_per_subnet(), - E::number_of_columns() / E::data_column_subnet_count() - ); - } -}