diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index deaea3eb24..3c1fd1e7bc 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -481,7 +481,8 @@ impl DataAvailabilityCheckerInner { if let Some(available_block) = pending_components.make_available( &self.spec, - self.custody_context.sampling_size(Some(epoch), &self.spec), + self.custody_context + .num_of_data_columns_to_sample(Some(epoch), &self.spec), |block| self.state_cache.recover_pending_executed_block(block), )? { // We keep the pending components in the availability cache during block import (#5845). @@ -526,7 +527,9 @@ impl DataAvailabilityCheckerInner { // Merge in the data columns. pending_components.merge_data_columns(kzg_verified_data_columns)?; - let num_expected_columns = self.custody_context.sampling_size(Some(epoch), &self.spec); + let num_expected_columns = self + .custody_context + .num_of_data_columns_to_sample(Some(epoch), &self.spec); debug!( component = "data_columns", ?block_root, @@ -622,7 +625,9 @@ impl DataAvailabilityCheckerInner { // Merge in the block. pending_components.merge_block(diet_executed_block); - let num_expected_columns = self.custody_context.sampling_size(Some(epoch), &self.spec); + let num_expected_columns = self + .custody_context + .num_of_data_columns_to_sample(Some(epoch), &self.spec); debug!( component = "block", ?block_root, @@ -631,11 +636,11 @@ impl DataAvailabilityCheckerInner { ); // Check if we have all components and entire set is consistent. - if let Some(available_block) = pending_components.make_available( - &self.spec, - self.custody_context.sampling_size(Some(epoch), &self.spec), - |block| self.state_cache.recover_pending_executed_block(block), - )? { + if let Some(available_block) = + pending_components.make_available(&self.spec, num_expected_columns, |block| { + self.state_cache.recover_pending_executed_block(block) + })? + { // We keep the pending components in the availability cache during block import (#5845). write_lock.put(block_root, pending_components); drop(write_lock); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index db4e2fab26..2c4981078d 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -777,7 +777,7 @@ where self.chain .data_availability_checker .custody_context() - .sampling_size(None, &self.chain.spec) as usize + .num_of_data_columns_to_sample(None, &self.chain.spec) as usize } pub fn slots_per_epoch(&self) -> u64 { diff --git a/beacon_node/beacon_chain/src/validator_custody.rs b/beacon_node/beacon_chain/src/validator_custody.rs index 5f037fabf3..7dc5b18ae4 100644 --- a/beacon_node/beacon_chain/src/validator_custody.rs +++ b/beacon_node/beacon_chain/src/validator_custody.rs @@ -215,7 +215,8 @@ impl CustodyContext { ); return Some(CustodyCountChanged { new_custody_group_count: updated_cgc, - sampling_count: self.sampling_size(Some(effective_epoch), spec), + sampling_count: self + .num_of_custody_groups_to_sample(Some(effective_epoch), spec), }); } } @@ -240,9 +241,13 @@ impl CustodyContext { } } - /// Returns the count of custody columns this node must sample for a block at `epoch` to import. - /// If an `epoch` is not specified, returns the *current* validator custody requirement. - pub fn sampling_size(&self, epoch_opt: Option, spec: &ChainSpec) -> u64 { + /// This function is used to determine the custody group count at a given epoch. + /// + /// This differs from the number of custody groups sampled per slot, as the spec requires a + /// minimum sampling size which may exceed the custody group count (CGC). + /// + /// See also: [`Self::num_of_custody_groups_to_sample`]. + fn custody_group_count_at_epoch(&self, epoch_opt: Option, spec: &ChainSpec) -> u64 { let custody_group_count = if self.current_is_supernode { spec.number_of_custody_groups } else if let Some(epoch) = epoch_opt { @@ -253,8 +258,26 @@ impl CustodyContext { } else { self.custody_group_count_at_head(spec) }; + custody_group_count + } - spec.sampling_size(custody_group_count) + /// Returns the count of custody groups this node must _sample_ for a block at `epoch` to import. + /// If an `epoch` is not specified, returns the *current* validator custody requirement. + pub fn num_of_custody_groups_to_sample( + &self, + epoch_opt: Option, + spec: &ChainSpec, + ) -> u64 { + let custody_group_count = self.custody_group_count_at_epoch(epoch_opt, spec); + spec.sampling_size_custody_groups(custody_group_count) + .expect("should compute node sampling size from valid chain spec") + } + + /// Returns the count of columns this node must _sample_ for a block at `epoch` to import. + /// If an `epoch` is not specified, returns the *current* validator custody requirement. + pub fn num_of_data_columns_to_sample(&self, epoch_opt: Option, spec: &ChainSpec) -> u64 { + let custody_group_count = self.custody_group_count_at_epoch(epoch_opt, spec); + spec.sampling_size_columns(custody_group_count) .expect("should compute node sampling size from valid chain spec") } } @@ -307,7 +330,7 @@ mod tests { spec.number_of_custody_groups ); assert_eq!( - custody_context.sampling_size(None, &spec), + custody_context.num_of_custody_groups_to_sample(None, &spec), spec.number_of_custody_groups ); } @@ -322,7 +345,7 @@ mod tests { "head custody count should be minimum spec custody requirement" ); assert_eq!( - custody_context.sampling_size(None, &spec), + custody_context.num_of_custody_groups_to_sample(None, &spec), spec.samples_per_slot ); } @@ -412,7 +435,7 @@ mod tests { register_validators_and_assert_cgc(&custody_context, validators_and_expected_cgc, &spec); assert_eq!( - custody_context.sampling_size(None, &spec), + custody_context.num_of_custody_groups_to_sample(None, &spec), spec.number_of_custody_groups ); } @@ -423,7 +446,7 @@ mod tests { let spec = E::default_spec(); let current_slot = Slot::new(10); let current_epoch = current_slot.epoch(E::slots_per_epoch()); - let default_sampling_size = custody_context.sampling_size(None, &spec); + let default_sampling_size = custody_context.num_of_custody_groups_to_sample(None, &spec); let validator_custody_units = 10; let _cgc_changed = custody_context.register_validators::( @@ -437,12 +460,12 @@ mod tests { // CGC update is not applied for `current_epoch`. assert_eq!( - custody_context.sampling_size(Some(current_epoch), &spec), + custody_context.num_of_custody_groups_to_sample(Some(current_epoch), &spec), default_sampling_size ); // CGC update is applied for the next epoch. assert_eq!( - custody_context.sampling_size(Some(current_epoch + 1), &spec), + custody_context.num_of_custody_groups_to_sample(Some(current_epoch + 1), &spec), validator_custody_units ); } diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index d1ed1c33b0..cc4d758b4a 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -66,7 +66,7 @@ impl NetworkGlobals { // The below `expect` calls will panic on start up if the chain spec config values used // are invalid let sampling_size = spec - .sampling_size(custody_group_count) + .sampling_size_custody_groups(custody_group_count) .expect("should compute node sampling size from valid chain spec"); let custody_groups = get_custody_groups(node_id, sampling_size, &spec) .expect("should compute node custody groups"); @@ -114,7 +114,7 @@ impl NetworkGlobals { // are invalid let sampling_size = self .spec - .sampling_size(custody_group_count) + .sampling_size_custody_groups(custody_group_count) .expect("should compute node sampling size from valid chain spec"); let custody_groups = get_custody_groups(self.local_enr().node_id().raw(), sampling_size, &self.spec) @@ -298,7 +298,13 @@ mod test { spec.fulu_fork_epoch = Some(Epoch::new(0)); let custody_group_count = spec.number_of_custody_groups / 2; - let subnet_sampling_size = spec.sampling_size(custody_group_count).unwrap(); + let sampling_size_custody_groups = spec + .sampling_size_custody_groups(custody_group_count) + .unwrap(); + let expected_sampling_subnet_count = sampling_size_custody_groups + * spec.data_column_sidecar_subnet_count + / spec.number_of_custody_groups; + let metadata = get_metadata(custody_group_count); let config = Arc::new(NetworkConfig::default()); @@ -310,7 +316,7 @@ mod test { ); assert_eq!( globals.sampling_subnets.read().len(), - subnet_sampling_size as usize + expected_sampling_subnet_count as usize ); } @@ -321,7 +327,7 @@ mod test { spec.fulu_fork_epoch = Some(Epoch::new(0)); let custody_group_count = spec.number_of_custody_groups / 2; - let subnet_sampling_size = spec.sampling_size(custody_group_count).unwrap(); + let expected_sampling_columns = spec.sampling_size_columns(custody_group_count).unwrap(); let metadata = get_metadata(custody_group_count); let config = Arc::new(NetworkConfig::default()); @@ -333,7 +339,7 @@ mod test { ); assert_eq!( globals.sampling_columns.read().len(), - subnet_sampling_size as usize + expected_sampling_columns as usize ); } diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index b4fd5afe87..38cd4b9217 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -720,17 +720,24 @@ impl ChainSpec { } /// Returns the number of column sidecars to sample per slot. - pub fn sampling_size(&self, custody_group_count: u64) -> Result { + pub fn sampling_size_columns(&self, custody_group_count: u64) -> Result { + let sampling_size_groups = self.sampling_size_custody_groups(custody_group_count)?; + let columns_per_custody_group = self .number_of_columns .safe_div(self.number_of_custody_groups) .map_err(|_| "number_of_custody_groups must be greater than 0")?; - let custody_column_count = columns_per_custody_group - .safe_mul(custody_group_count) + let sampling_size_columns = columns_per_custody_group + .safe_mul(sampling_size_groups) .map_err(|_| "Computing sampling size should not overflow")?; - Ok(std::cmp::max(custody_column_count, self.samples_per_slot)) + Ok(sampling_size_columns) + } + + /// Returns the number of custody groups to sample per slot. + pub fn sampling_size_custody_groups(&self, custody_group_count: u64) -> Result { + Ok(std::cmp::max(custody_group_count, self.samples_per_slot)) } pub fn all_data_column_sidecar_subnets(&self) -> impl Iterator {