From aa8095086c43745faac9efce2b1d3eb7aeb7e886 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 May 2024 14:12:41 +0300 Subject: [PATCH] PeerDAS parameter changes for devnet-0 (#5779) * Update PeerDAS parameters to latest values. * Lint fix * Fix lint. --- .../lighthouse_network/src/discovery/enr.rs | 17 +++++++---------- .../src/discovery/subnet_predicate.rs | 12 ++++-------- .../lighthouse_network/src/rpc/config.rs | 9 ++++++--- .../lighthouse_network/src/types/globals.rs | 5 ++--- beacon_node/network/src/service.rs | 3 +-- beacon_node/network/src/sync/network_context.rs | 5 +---- consensus/types/src/chain_spec.rs | 5 +---- consensus/types/src/data_column_subnet_id.rs | 9 ++++----- consensus/types/src/eth_spec.rs | 6 +++--- 9 files changed, 29 insertions(+), 42 deletions(-) diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index 00ce790da28..9235b2bfe70 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -38,7 +38,7 @@ pub trait Eth2Enr { ) -> Result, &'static str>; /// The peerdas custody subnet count associated with the ENR. - fn custody_subnet_count(&self) -> Result; + fn custody_subnet_count(&self) -> u64; fn eth2(&self) -> Result; } @@ -64,15 +64,12 @@ impl Eth2Enr for Enr { .map_err(|_| "Could not decode the ENR syncnets bitfield") } - fn custody_subnet_count(&self) -> Result { - // NOTE: if the custody value is non-existent in the ENR, then we assume the minimum - // custody value defined in the spec. - let min_custody_bytes = E::min_custody_requirement().as_ssz_bytes(); - let custody_bytes = self - .get(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY) - .unwrap_or(&min_custody_bytes); - u64::from_ssz_bytes(custody_bytes) - .map_err(|_| "Could not decode the ENR custody subnet count") + /// if the custody value is non-existent in the ENR, then we assume the minimum custody value + /// defined in the spec. + fn custody_subnet_count(&self) -> u64 { + self.get(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY) + .and_then(|custody_bytes| u64::from_ssz_bytes(custody_bytes).ok()) + .unwrap_or(E::min_custody_requirement() as u64) } fn eth2(&self) -> Result { diff --git a/beacon_node/lighthouse_network/src/discovery/subnet_predicate.rs b/beacon_node/lighthouse_network/src/discovery/subnet_predicate.rs index f1dd44c6969..8f737eec6ec 100644 --- a/beacon_node/lighthouse_network/src/discovery/subnet_predicate.rs +++ b/beacon_node/lighthouse_network/src/discovery/subnet_predicate.rs @@ -25,11 +25,7 @@ where let sync_committee_bitfield: Result, _> = enr.sync_committee_bitfield::(); - // Pre-fork/fork-boundary enrs may not contain a peerdas custody field. - // Don't return early here. - // - // NOTE: we could map to minimum custody requirement here. - let custody_subnet_count: Result = enr.custody_subnet_count::(); + let custody_subnet_count = enr.custody_subnet_count::(); let predicate = subnets.iter().any(|subnet| match subnet { Subnet::Attestation(s) => attestation_bitfield @@ -38,13 +34,13 @@ where Subnet::SyncCommittee(s) => sync_committee_bitfield .as_ref() .map_or(false, |b| b.get(*s.deref() as usize).unwrap_or(false)), - Subnet::DataColumn(s) => custody_subnet_count.map_or(false, |count| { + Subnet::DataColumn(s) => { let mut subnets = DataColumnSubnetId::compute_custody_subnets::( enr.node_id().raw().into(), - count, + custody_subnet_count, ); subnets.contains(s) - }), + } }); if !predicate { diff --git a/beacon_node/lighthouse_network/src/rpc/config.rs b/beacon_node/lighthouse_network/src/rpc/config.rs index 7f1595d5295..169ea234092 100644 --- a/beacon_node/lighthouse_network/src/rpc/config.rs +++ b/beacon_node/lighthouse_network/src/rpc/config.rs @@ -107,9 +107,12 @@ impl RateLimiterConfig { pub const DEFAULT_BLOCKS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); pub const DEFAULT_BLOBS_BY_RANGE_QUOTA: Quota = Quota::n_every(768, 10); pub const DEFAULT_BLOBS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); - // TODO(das): random value without thought - pub const DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA: Quota = Quota::n_every(128, 10); - pub const DEFAULT_DATA_COLUMNS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); + // 320 blocks worth of columns for regular node, or 40 blocks for supernode. + // Range sync load balances when requesting blocks, and each batch is 32 blocks. + pub const DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA: Quota = Quota::n_every(5120, 10); + // 512 columns per request from spec. This should be plenty as peers are unlikely to send all + // sampling requests to a single peer. + pub const DEFAULT_DATA_COLUMNS_BY_ROOT_QUOTA: Quota = Quota::n_every(512, 10); pub const DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_OPTIMISTIC_UPDATE_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_FINALITY_UPDATE_QUOTA: Quota = Quota::one_every(10); diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index 5dd74f25726..263755ca6cb 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -115,8 +115,7 @@ impl NetworkGlobals { pub fn custody_columns(&self, _epoch: Epoch) -> Result, &'static str> { let enr = self.local_enr(); let node_id = enr.node_id().raw().into(); - // TODO(das): cache this number at start-up to not make this fallible - let custody_subnet_count = enr.custody_subnet_count::()?; + let custody_subnet_count = enr.custody_subnet_count::(); Ok( DataColumnSubnetId::compute_custody_columns::(node_id, custody_subnet_count) .collect(), @@ -152,7 +151,7 @@ mod test { fn test_custody_count_default() { let log = logging::test_logger(); let default_custody_requirement_column_count = - E::number_of_columns() / E::data_column_subnet_count(); + E::number_of_columns() / E::data_column_subnet_count() * E::min_custody_requirement(); let globals = NetworkGlobals::::new_test_globals(vec![], &log); let any_epoch = Epoch::new(0); let columns = globals.custody_columns(any_epoch).unwrap(); diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index fd899f5d45f..5f922e25c9d 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -759,8 +759,7 @@ impl NetworkService { self.network_globals.local_enr().node_id().raw().into(), self.network_globals .local_enr() - .custody_subnet_count::<::EthSpec>() - .unwrap_or(self.beacon_chain.spec.custody_requirement), + .custody_subnet_count::<::EthSpec>(), ) { for fork_digest in self.required_gossip_fork_digests() { let gossip_kind = Subnet::DataColumn(column_subnet).into(); diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index d516221e941..d67ad63cc3a 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -208,10 +208,7 @@ impl SyncNetworkContext { for (peer_id, peer_info) in self.network_globals().peers.read().connected_peers() { if let Some(enr) = peer_info.enr() { - // TODO(das): ignores decode errors - let custody_subnet_count = enr - .custody_subnet_count::() - .unwrap_or(T::EthSpec::min_custody_requirement() as u64); + let custody_subnet_count = enr.custody_subnet_count::(); // TODO(das): consider caching a map of subnet -> Vec and invalidating // whenever a peer connected or disconnect event in received let mut subnets = DataColumnSubnetId::compute_custody_subnets::( diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index b166e547dbb..3ce2b5e8edc 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -195,7 +195,6 @@ pub struct ChainSpec { * DAS params */ pub eip7594_fork_epoch: Option, - pub custody_requirement: u64, /* * Networking @@ -772,7 +771,6 @@ impl ChainSpec { * DAS params */ eip7594_fork_epoch: None, - custody_requirement: 1, /* * Network specific @@ -1085,7 +1083,6 @@ impl ChainSpec { * DAS params */ eip7594_fork_epoch: None, - custody_requirement: 1, /* * Network specific */ @@ -1436,7 +1433,7 @@ const fn default_max_request_blob_sidecars() -> u64 { } const fn default_max_request_data_column_sidecars() -> u64 { - 16384 + 512 } const fn default_min_epochs_for_blob_sidecars_requests() -> u64 { diff --git a/consensus/types/src/data_column_subnet_id.rs b/consensus/types/src/data_column_subnet_id.rs index 787a0db332c..d57f47352e2 100644 --- a/consensus/types/src/data_column_subnet_id.rs +++ b/consensus/types/src/data_column_subnet_id.rs @@ -160,7 +160,7 @@ impl From for Error { mod test { use crate::data_column_subnet_id::DataColumnSubnetId; use crate::EthSpec; - use crate::{ChainSpec, MainnetEthSpec}; + use crate::MainnetEthSpec; type E = MainnetEthSpec; @@ -182,15 +182,14 @@ mod test { .map(|v| ethereum_types::U256::from_dec_str(v).unwrap()) .collect::>(); - let spec = ChainSpec::mainnet(); - + let custody_requirement = 4; for node_id in node_ids { let computed_subnets = - DataColumnSubnetId::compute_custody_subnets::(node_id, spec.custody_requirement); + DataColumnSubnetId::compute_custody_subnets::(node_id, custody_requirement); let computed_subnets: Vec<_> = computed_subnets.collect(); // the number of subnets is equal to the custody requirement - assert_eq!(computed_subnets.len() as u64, spec.custody_requirement); + assert_eq!(computed_subnets.len() as u64, custody_requirement); let subnet_count = E::data_column_subnet_count(); for subnet in computed_subnets { diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 4ed7bfc9615..973d4cd693c 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -417,7 +417,7 @@ impl EthSpec for MainnetEthSpec { type BytesPerBlob = U131072; type BytesPerCell = U2048; type KzgCommitmentInclusionProofDepth = U17; - type MinCustodyRequirement = U1; + type MinCustodyRequirement = U4; type DataColumnSubnetCount = U32; type DataColumnCount = U128; type DataColumnsPerSubnet = U4; @@ -471,7 +471,7 @@ impl EthSpec for MinimalEthSpec { type MaxDepositReceiptsPerPayload = U4; type MaxWithdrawalRequestsPerPayload = U2; // DAS spec values copied from `MainnetEthSpec` - type MinCustodyRequirement = U1; + type MinCustodyRequirement = U4; type DataColumnSubnetCount = U32; type DataColumnCount = U128; type DataColumnsPerSubnet = U4; @@ -565,7 +565,7 @@ impl EthSpec for GnosisEthSpec { type MaxAttestationsElectra = U8; type MaxWithdrawalRequestsPerPayload = U16; // DAS spec values copied from `MainnetEthSpec` - type MinCustodyRequirement = U1; + type MinCustodyRequirement = U4; type DataColumnSubnetCount = U32; type DataColumnCount = U128; type DataColumnsPerSubnet = U4;