From 3a2baca10c2885025483d9405cf28a3851011fe7 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 10 Nov 2025 18:40:32 +1100 Subject: [PATCH 1/7] Fix custdoy context initialisation race condition. --- beacon_node/beacon_chain/src/builder.rs | 20 ++++++++- .../beacon_chain/src/custody_context.rs | 44 +++++++++---------- beacon_node/client/src/builder.rs | 32 +++++--------- beacon_node/lighthouse_network/src/lib.rs | 2 +- .../lighthouse_network/src/service/mod.rs | 4 +- beacon_node/network/src/service.rs | 5 +++ beacon_node/src/lib.rs | 6 ++- 7 files changed, 64 insertions(+), 49 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 719c24b9561..88be69cdaf9 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -40,6 +40,7 @@ use std::time::Duration; use store::{Error as StoreError, HotColdDB, ItemStore, KeyValueStoreOp}; use task_executor::{ShutdownReason, TaskExecutor}; use tracing::{debug, error, info}; +use types::data_column_custody_group::{CustodyIndex, get_custody_groups_ordered}; use types::{ BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, FixedBytesExtended, Hash256, Signature, SignedBeaconBlock, Slot, @@ -102,6 +103,7 @@ pub struct BeaconChainBuilder { task_executor: Option, validator_monitor_config: Option, node_custody_type: NodeCustodyType, + node_id: Option<[u8; 32]>, rng: Option>, } @@ -141,6 +143,7 @@ where task_executor: None, validator_monitor_config: None, node_custody_type: NodeCustodyType::Fullnode, + node_id: None, rng: None, } } @@ -647,6 +650,11 @@ where self } + pub fn node_id(mut self, node_id: [u8; 32]) -> Self { + self.node_id = Some(node_id); + self + } + /// Sets the `BeaconChain` event handler backend. /// /// For example, provide `ServerSentEventHandler` as a `handler`. @@ -740,6 +748,7 @@ where .genesis_state_root .ok_or("Cannot build without a genesis state root")?; let validator_monitor_config = self.validator_monitor_config.unwrap_or_default(); + let node_id = self.node_id.ok_or("Cannot build without a node id")?; let rng = self.rng.ok_or("Cannot build without an RNG")?; let beacon_proposer_cache: Arc> = <_>::default(); @@ -929,6 +938,10 @@ where } }; + let all_custody_groups_ordered = + get_custody_groups_ordered(node_id, self.spec.number_of_custody_groups, &self.spec) + .map_err(|e| format!("Failed to compute custody groups: {:?}", e))?; + // Load the persisted custody context from the db and initialize // the context for this run let (custody_context, cgc_changed_opt) = if let Some(custody) = @@ -942,11 +955,16 @@ where custody, self.node_custody_type, head_epoch, + all_custody_groups_ordered, &self.spec, ) } else { ( - CustodyContext::new(self.node_custody_type, &self.spec), + CustodyContext::new( + self.node_custody_type, + all_custody_groups_ordered, + &self.spec, + ), None, ) }; diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index a5ef3ed2f65..115b11fc365 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -250,7 +250,7 @@ pub struct CustodyContext { validator_registrations: RwLock, /// Stores an immutable, ordered list of all custody columns as determined by the node's NodeID /// on startup. - all_custody_columns_ordered: OnceLock>, + all_custody_columns_ordered: Vec, _phantom_data: PhantomData, } @@ -259,15 +259,22 @@ impl CustodyContext { /// exists. /// /// The `node_custody_type` value is based on current cli parameters. - pub fn new(node_custody_type: NodeCustodyType, spec: &ChainSpec) -> Self { + pub fn new( + node_custody_type: NodeCustodyType, + all_custody_groups_ordered: Vec, + spec: &ChainSpec, + ) -> Self { let cgc_override = node_custody_type.get_custody_count_override(spec); // If there's no override, we initialise `validator_custody_count` to 0. This has been the // existing behaviour and we maintain this for now to avoid a semantic schema change until // a later release. + // FIXME: remove unwrap + let all_custody_columns_ordered = + Self::compute_ordered_data_columns(all_custody_groups_ordered, spec).unwrap(); Self { validator_custody_count: AtomicU64::new(cgc_override.unwrap_or(0)), validator_registrations: RwLock::new(ValidatorRegistrations::new(cgc_override)), - all_custody_columns_ordered: OnceLock::new(), + all_custody_columns_ordered, _phantom_data: PhantomData, } } @@ -290,6 +297,7 @@ impl CustodyContext { ssz_context: CustodyContextSsz, node_custody_type: NodeCustodyType, head_epoch: Epoch, + all_custody_groups_ordered: Vec, spec: &ChainSpec, ) -> (Self, Option) { let CustodyContextSsz { @@ -347,6 +355,10 @@ impl CustodyContext { } } + // FIXME: remove unwrap + let all_custody_columns_ordered = + Self::compute_ordered_data_columns(all_custody_groups_ordered, spec).unwrap(); + let custody_context = CustodyContext { validator_custody_count: AtomicU64::new(validator_custody_at_head), validator_registrations: RwLock::new(ValidatorRegistrations { @@ -355,7 +367,7 @@ impl CustodyContext { .into_iter() .collect(), }), - all_custody_columns_ordered: OnceLock::new(), + all_custody_columns_ordered, _phantom_data: PhantomData, }; @@ -370,22 +382,17 @@ impl CustodyContext { /// /// # Returns /// Ok(()) if initialization succeeds, Err with description string if it fails - pub fn init_ordered_data_columns_from_custody_groups( - &self, + fn compute_ordered_data_columns( all_custody_groups_ordered: Vec, spec: &ChainSpec, - ) -> Result<(), String> { + ) -> Result, String> { let mut ordered_custody_columns = vec![]; for custody_index in all_custody_groups_ordered { let columns = compute_columns_for_custody_group::(custody_index, spec) .map_err(|e| format!("Failed to compute columns for custody group {e:?}"))?; ordered_custody_columns.extend(columns); } - self.all_custody_columns_ordered - .set(ordered_custody_columns.into_boxed_slice()) - .map_err(|_| { - "Failed to initialise CustodyContext with computed custody columns".to_string() - }) + Ok(ordered_custody_columns) } /// Register a new validator index and updates the list of validators if required. @@ -497,11 +504,7 @@ impl CustodyContext { /// A slice of ordered column indices that should be sampled for this epoch based on the node's custody configuration pub fn sampling_columns_for_epoch(&self, epoch: Epoch, spec: &ChainSpec) -> &[ColumnIndex] { let num_of_columns_to_sample = self.num_of_data_columns_to_sample(epoch, spec); - let all_columns_ordered = self - .all_custody_columns_ordered - .get() - .expect("all_custody_columns_ordered should be initialized"); - &all_columns_ordered[..num_of_columns_to_sample] + &self.all_custody_columns_ordered[..num_of_columns_to_sample] } /// Returns the ordered list of column indices that the node is assigned to custody @@ -528,12 +531,7 @@ impl CustodyContext { self.custody_group_count_at_head(spec) as usize }; - let all_columns_ordered = self - .all_custody_columns_ordered - .get() - .expect("all_custody_columns_ordered should be initialized"); - - &all_columns_ordered[..custody_group_count] + &self.all_custody_columns_ordered[..custody_group_count] } /// The node has completed backfill for this epoch. Update the internal records so the function diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index c3c827f0aae..d9dea1b174f 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -2,7 +2,7 @@ use crate::Client; use crate::compute_light_client_updates::{ LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, compute_light_client_updates, }; -use crate::config::{ClientGenesis, Config as ClientConfig}; +use crate::config::{ClientGenesis, Config as ClientConfig, Config}; use crate::notifier::spawn_notifier; use beacon_chain::attestation_simulator::start_attestation_simulator_service; use beacon_chain::data_availability_checker::start_availability_cache_maintenance_service; @@ -28,6 +28,8 @@ use execution_layer::ExecutionLayer; use execution_layer::test_utils::generate_genesis_header; use futures::channel::mpsc::Receiver; use genesis::{DEFAULT_ETH1_BLOCK_HASH, interop_genesis_state}; +use lighthouse_network::discv5::enr::NodeId; +use lighthouse_network::identity::Keypair; use lighthouse_network::{NetworkGlobals, prometheus_client::registry::Registry}; use monitoring_api::{MonitoringHttpClient, ProcessType}; use network::{NetworkConfig, NetworkSenders, NetworkService}; @@ -42,7 +44,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use store::database::interface::BeaconNodeBackend; use timer::spawn_timer; use tracing::{debug, info, warn}; -use types::data_column_custody_group::get_custody_groups_ordered; +use types::data_column_custody_group::{CustodyIndex, get_custody_groups_ordered}; use types::{ BeaconState, BlobSidecarList, ChainSpec, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, test_utils::generate_deterministic_keypairs, @@ -154,6 +156,7 @@ where mut self, client_genesis: ClientGenesis, config: ClientConfig, + local_keypair: Keypair, ) -> Result { let store = self.store.clone(); let chain_spec = self.chain_spec.clone(); @@ -203,6 +206,7 @@ where .event_handler(event_handler) .execution_layer(execution_layer) .node_custody_type(config.chain.node_custody_type) + .node_id(NodeId::from(local_keypair.public()).raw()) .validator_monitor_config(config.validator_monitor.clone()) .rng(Box::new( StdRng::try_from_rng(&mut OsRng) @@ -453,7 +457,11 @@ where } /// Starts the networking stack. - pub async fn network(mut self, config: Arc) -> Result { + pub async fn network( + mut self, + config: Arc, + local_keypair: Keypair, + ) -> Result { let beacon_chain = self .beacon_chain .clone() @@ -481,12 +489,11 @@ where context.executor, libp2p_registry.as_mut(), beacon_processor_channels.beacon_processor_tx.clone(), + local_keypair, ) .await .map_err(|e| format!("Failed to start network: {:?}", e))?; - init_custody_context(beacon_chain, &network_globals)?; - self.network_globals = Some(network_globals); self.network_senders = Some(network_senders); self.libp2p_registry = libp2p_registry; @@ -788,21 +795,6 @@ where } } -fn init_custody_context( - chain: Arc>, - network_globals: &NetworkGlobals, -) -> Result<(), String> { - let node_id = network_globals.local_enr().node_id().raw(); - let spec = &chain.spec; - let custody_groups_ordered = - get_custody_groups_ordered(node_id, spec.number_of_custody_groups, spec) - .map_err(|e| format!("Failed to compute custody groups: {:?}", e))?; - chain - .data_availability_checker - .custody_context() - .init_ordered_data_columns_from_custody_groups(custody_groups_ordered, spec) -} - impl ClientBuilder> where diff --git a/beacon_node/lighthouse_network/src/lib.rs b/beacon_node/lighthouse_network/src/lib.rs index b6be9b52223..3d96a08357d 100644 --- a/beacon_node/lighthouse_network/src/lib.rs +++ b/beacon_node/lighthouse_network/src/lib.rs @@ -109,7 +109,7 @@ pub use discovery::Eth2Enr; pub use discv5; pub use gossipsub::{IdentTopic, MessageAcceptance, MessageId, Topic, TopicHash}; pub use libp2p; -pub use libp2p::{Multiaddr, multiaddr}; +pub use libp2p::{Multiaddr, identity, multiaddr}; pub use libp2p::{PeerId, Swarm, core::ConnectedPoint}; pub use peer_manager::{ ConnectionDirection, PeerConnectionStatus, PeerInfo, PeerManager, SyncInfo, SyncStatus, diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index ea2c53a07fe..93c69ee097b 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -26,6 +26,7 @@ use gossipsub::{ TopicScoreParams, }; use gossipsub_scoring_parameters::{PeerScoreSettings, lighthouse_gossip_thresholds}; +use libp2p::identity::Keypair; use libp2p::multiaddr::{self, Multiaddr, Protocol as MProtocol}; use libp2p::swarm::behaviour::toggle::Toggle; use libp2p::swarm::{NetworkBehaviour, Swarm, SwarmEvent}; @@ -171,11 +172,10 @@ impl Network { executor: task_executor::TaskExecutor, mut ctx: ServiceContext<'_>, custody_group_count: u64, + local_keypair: Keypair, ) -> Result<(Self, Arc>), String> { let config = ctx.config.clone(); trace!("Libp2p Service starting"); - // initialise the node's ID - let local_keypair = utils::load_private_key(&config); // Trusted peers will also be marked as explicit in GossipSub. // Cfr. https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.1.md#explicit-peering-agreements diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 4bd649ba824..a416f5cb123 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -12,6 +12,7 @@ use futures::future::OptionFuture; use futures::prelude::*; use lighthouse_network::Enr; +use lighthouse_network::identity::Keypair; use lighthouse_network::rpc::InboundRequestId; use lighthouse_network::rpc::RequestType; use lighthouse_network::rpc::methods::RpcResponse; @@ -212,6 +213,7 @@ impl NetworkService { executor: task_executor::TaskExecutor, libp2p_registry: Option<&'_ mut Registry>, beacon_processor_send: BeaconProcessorSend, + local_keypair: Keypair, ) -> Result< ( NetworkService, @@ -284,6 +286,7 @@ impl NetworkService { .data_availability_checker .custody_context() .custody_group_count_at_head(&beacon_chain.spec), + local_keypair, ) .await?; @@ -366,6 +369,7 @@ impl NetworkService { executor: task_executor::TaskExecutor, libp2p_registry: Option<&'_ mut Registry>, beacon_processor_send: BeaconProcessorSend, + local_keypair: Keypair, ) -> Result<(Arc>, NetworkSenders), String> { let (network_service, network_globals, network_senders) = Self::build( beacon_chain, @@ -373,6 +377,7 @@ impl NetworkService { executor.clone(), libp2p_registry, beacon_processor_send, + local_keypair, ) .await?; diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index 2ceb94729d5..2e9b4036be0 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -9,6 +9,7 @@ pub use client::{Client, ClientBuilder, ClientConfig, ClientGenesis}; pub use config::{get_config, get_data_dir, set_network_config}; use environment::RuntimeContext; pub use eth2_config::Eth2Config; +use lighthouse_network::load_private_key; use slasher::{DatabaseBackendOverride, Slasher}; use std::ops::{Deref, DerefMut}; use std::sync::Arc; @@ -120,8 +121,9 @@ impl ProductionBeaconNode { builder }; + let local_keypair = load_private_key(&client_config.network); let builder = builder - .beacon_chain_builder(client_genesis, client_config.clone()) + .beacon_chain_builder(client_genesis, client_config.clone(), local_keypair) .await?; info!("Block production enabled"); @@ -133,7 +135,7 @@ impl ProductionBeaconNode { builder .build_beacon_chain()? - .network(Arc::new(client_config.network)) + .network(Arc::new(client_config.network), local_keypair) .await? .notifier()? .http_metrics_config(client_config.http_metrics.clone()) From 0f44256ecf23fa6455aba2dade77018f5e490c82 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 12 Nov 2025 16:02:00 +1100 Subject: [PATCH 2/7] Simplify things and update tests --- beacon_node/beacon_chain/src/builder.rs | 23 +-- .../beacon_chain/src/custody_context.rs | 160 +++++++++--------- .../src/data_availability_checker.rs | 39 ++--- .../overflow_lru_cache.rs | 7 +- beacon_node/beacon_chain/src/test_utils.rs | 10 +- beacon_node/beacon_chain/tests/store_tests.rs | 9 +- beacon_node/client/src/builder.rs | 18 +- .../lighthouse_network/tests/common.rs | 14 +- beacon_node/network/src/service/tests.rs | 3 + beacon_node/src/lib.rs | 7 +- common/network_utils/src/enr_ext.rs | 1 - 11 files changed, 152 insertions(+), 139 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 88be69cdaf9..da1a59dca2d 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -40,7 +40,7 @@ use std::time::Duration; use store::{Error as StoreError, HotColdDB, ItemStore, KeyValueStoreOp}; use task_executor::{ShutdownReason, TaskExecutor}; use tracing::{debug, error, info}; -use types::data_column_custody_group::{CustodyIndex, get_custody_groups_ordered}; +use types::data_column_custody_group::CustodyIndex; use types::{ BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, FixedBytesExtended, Hash256, Signature, SignedBeaconBlock, Slot, @@ -103,7 +103,7 @@ pub struct BeaconChainBuilder { task_executor: Option, validator_monitor_config: Option, node_custody_type: NodeCustodyType, - node_id: Option<[u8; 32]>, + all_custody_groups_ordered: Option>, rng: Option>, } @@ -143,7 +143,7 @@ where task_executor: None, validator_monitor_config: None, node_custody_type: NodeCustodyType::Fullnode, - node_id: None, + all_custody_groups_ordered: None, rng: None, } } @@ -650,8 +650,13 @@ where self } - pub fn node_id(mut self, node_id: [u8; 32]) -> Self { - self.node_id = Some(node_id); + /// Sets the custody group order for this node. + /// This is used to determine the data columns the node is required to custody. + pub fn all_custody_groups_ordered( + mut self, + all_custody_groups_ordered: Vec, + ) -> Self { + self.all_custody_groups_ordered = Some(all_custody_groups_ordered); self } @@ -748,7 +753,9 @@ where .genesis_state_root .ok_or("Cannot build without a genesis state root")?; let validator_monitor_config = self.validator_monitor_config.unwrap_or_default(); - let node_id = self.node_id.ok_or("Cannot build without a node id")?; + let all_custody_groups_ordered = self + .all_custody_groups_ordered + .ok_or("Cannot build without ordered custody groups")?; let rng = self.rng.ok_or("Cannot build without an RNG")?; let beacon_proposer_cache: Arc> = <_>::default(); @@ -938,10 +945,6 @@ where } }; - let all_custody_groups_ordered = - get_custody_groups_ordered(node_id, self.spec.number_of_custody_groups, &self.spec) - .map_err(|e| format!("Failed to compute custody groups: {:?}", e))?; - // Load the persisted custody context from the db and initialize // the context for this run let (custody_context, cgc_changed_opt) = if let Some(custody) = diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index 115b11fc365..f0b14f460e3 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -2,7 +2,6 @@ use parking_lot::RwLock; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use std::marker::PhantomData; -use std::sync::OnceLock; use std::{ collections::{BTreeMap, HashMap}, sync::atomic::{AtomicU64, Ordering}, @@ -206,7 +205,7 @@ fn get_validators_custody_requirement(validator_custody_units: u64, spec: &Chain /// Therefore, the custody count at any point in time is calculated as the max of /// the validator custody at that time and the current cli params. /// -/// Choosing the max ensures that we always have the minimum required columns and +/// Choosing the max ensures that we always have the minimum required columns, and /// we can adjust the `status.earliest_available_slot` value to indicate to our peers /// the columns that we can guarantee to serve. #[derive(Debug, Copy, Clone, PartialEq, Eq, Default, Deserialize, Serialize)] @@ -218,7 +217,7 @@ pub enum NodeCustodyType { /// wants to subscribe to the minimum number of columns to enable /// reconstruction (50%) of the full blob data on demand. SemiSupernode, - /// The node isn't running with with any explicit cli parameters + /// The node isn't running with any explicit cli parameters /// or is running with cli parameters to indicate that it wants /// to only subscribe to the minimal custody requirements. #[default] @@ -248,8 +247,8 @@ pub struct CustodyContext { validator_custody_count: AtomicU64, /// Maintains all the validators that this node is connected to currently validator_registrations: RwLock, - /// Stores an immutable, ordered list of all custody columns as determined by the node's NodeID - /// on startup. + /// Stores an immutable, ordered list of all data columns as determined by the node's NodeID + /// on startup. This used to determine the node's custody columns. all_custody_columns_ordered: Vec, _phantom_data: PhantomData, } @@ -531,7 +530,11 @@ impl CustodyContext { self.custody_group_count_at_head(spec) as usize }; - &self.all_custody_columns_ordered[..custody_group_count] + // This is an unnecessary conversion for spec compliance, basically just multiplying by 1. + let columns_per_custody_group = spec.data_columns_per_group::() as usize; + let custody_column_count = columns_per_custody_group * (custody_group_count); + + &self.all_custody_columns_ordered[..custody_column_count] } /// The node has completed backfill for this epoch. Update the internal records so the function @@ -597,8 +600,6 @@ impl From<&CustodyContext> for CustodyContextSsz { #[cfg(test)] mod tests { - use rand::rng; - use rand::seq::SliceRandom; use types::MainnetEthSpec; use super::*; @@ -621,16 +622,17 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, head_epoch, + generate_custody_indices(spec), spec, ); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); - custody_context - .init_ordered_data_columns_from_custody_groups(all_custody_groups_ordered, spec) - .expect("should initialise ordered data columns"); custody_context } + fn generate_custody_indices(spec: &ChainSpec) -> Vec { + (0..spec.number_of_custody_groups).collect::>() + } + fn complete_backfill_for_epochs( custody_context: &CustodyContext, start_epoch: Epoch, @@ -666,6 +668,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, + generate_custody_indices(spec), spec, ); @@ -736,6 +739,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, + generate_custody_indices(spec), spec, ); @@ -757,7 +761,11 @@ mod tests { #[test] fn no_validators_supernode_default() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Supernode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Supernode, + generate_custody_indices(&spec), + &spec, + ); assert_eq!( custody_context.custody_group_count_at_head(&spec), spec.number_of_custody_groups @@ -771,7 +779,11 @@ mod tests { #[test] fn no_validators_semi_supernode_default() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::SemiSupernode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::SemiSupernode, + generate_custody_indices(&spec), + &spec, + ); assert_eq!( custody_context.custody_group_count_at_head(&spec), spec.number_of_custody_groups / 2 @@ -785,7 +797,11 @@ mod tests { #[test] fn no_validators_fullnode_default() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); assert_eq!( custody_context.custody_group_count_at_head(&spec), spec.custody_requirement, @@ -800,7 +816,11 @@ mod tests { #[test] fn register_single_validator_should_update_cgc() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); let bal_per_additional_group = spec.balance_per_additional_custody_group; let min_val_custody_requirement = spec.validator_custody_requirement; // One single node increases its balance over 3 epochs. @@ -824,7 +844,11 @@ mod tests { #[test] fn register_multiple_validators_should_update_cgc() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); let bal_per_additional_group = spec.balance_per_additional_custody_group; let min_val_custody_requirement = spec.validator_custody_requirement; // Add 3 validators over 3 epochs. @@ -861,7 +885,11 @@ mod tests { #[test] fn register_validators_should_not_update_cgc_for_supernode() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Supernode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Supernode, + generate_custody_indices(&spec), + &spec, + ); let bal_per_additional_group = spec.balance_per_additional_custody_group; // Add 3 validators over 3 epochs. @@ -899,7 +927,11 @@ mod tests { #[test] fn cgc_change_should_be_effective_to_sampling_after_delay() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); let current_slot = Slot::new(10); let current_epoch = current_slot.epoch(E::slots_per_epoch()); let default_sampling_size = @@ -930,7 +962,11 @@ mod tests { #[test] fn validator_dropped_after_no_registrations_within_expiry_should_not_reduce_cgc() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); let current_slot = Slot::new(10); let val_custody_units_1 = 10; let val_custody_units_2 = 5; @@ -972,7 +1008,11 @@ mod tests { #[test] fn validator_dropped_after_no_registrations_within_expiry() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + generate_custody_indices(&spec), + &spec, + ); let current_slot = Slot::new(10); let val_custody_units_1 = 10; let val_custody_units_2 = 5; @@ -1019,37 +1059,6 @@ mod tests { ); } - #[test] - fn should_init_ordered_data_columns_and_return_sampling_columns() { - let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); - let sampling_size = custody_context.num_of_data_columns_to_sample(Epoch::new(0), &spec); - - // initialise ordered columns - let mut all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); - all_custody_groups_ordered.shuffle(&mut rng()); - - custody_context - .init_ordered_data_columns_from_custody_groups( - all_custody_groups_ordered.clone(), - &spec, - ) - .expect("should initialise ordered data columns"); - - let actual_sampling_columns = - custody_context.sampling_columns_for_epoch(Epoch::new(0), &spec); - - let expected_sampling_columns = &all_custody_groups_ordered - .iter() - .flat_map(|custody_index| { - compute_columns_for_custody_group::(*custody_index, &spec) - .expect("should compute columns for custody group") - }) - .collect::>()[0..sampling_size]; - - assert_eq!(actual_sampling_columns, expected_sampling_columns) - } - /// Update the validator every epoch and assert cgc against expected values. fn register_validators_and_assert_cgc( custody_context: &CustodyContext, @@ -1075,12 +1084,9 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_fullnode() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); - - custody_context - .init_ordered_data_columns_from_custody_groups(all_custody_groups_ordered, &spec) - .expect("should initialise ordered data columns"); + let all_custody_groups_ordered = generate_custody_indices(&spec); + let custody_context = + CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); assert_eq!( custody_context.custody_columns_for_epoch(None, &spec).len(), @@ -1091,12 +1097,12 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_supernode() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Supernode, &spec); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); - - custody_context - .init_ordered_data_columns_from_custody_groups(all_custody_groups_ordered, &spec) - .expect("should initialise ordered data columns"); + let all_custody_groups_ordered = generate_custody_indices(&spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::Supernode, + all_custody_groups_ordered, + &spec, + ); assert_eq!( custody_context.custody_columns_for_epoch(None, &spec).len(), @@ -1107,14 +1113,11 @@ mod tests { #[test] fn custody_columns_for_epoch_with_validators_should_match_cgc() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); + let all_custody_groups_ordered = generate_custody_indices(&spec); + let custody_context = + CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); let val_custody_units = 10; - custody_context - .init_ordered_data_columns_from_custody_groups(all_custody_groups_ordered, &spec) - .expect("should initialise ordered data columns"); - let _ = custody_context.register_validators( vec![( 0, @@ -1133,14 +1136,11 @@ mod tests { #[test] fn custody_columns_for_epoch_specific_epoch_uses_epoch_cgc() { let spec = E::default_spec(); - let custody_context = CustodyContext::::new(NodeCustodyType::Fullnode, &spec); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect::>(); + let all_custody_groups_ordered = generate_custody_indices(&spec); + let custody_context = + CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); let test_epoch = Epoch::new(5); - custody_context - .init_ordered_data_columns_from_custody_groups(all_custody_groups_ordered, &spec) - .expect("should initialise ordered data columns"); - let expected_cgc = custody_context.custody_group_count_at_epoch(test_epoch, &spec); assert_eq!( custody_context @@ -1163,6 +1163,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(0), + generate_custody_indices(&spec), &spec, ); @@ -1196,7 +1197,11 @@ mod tests { fn restore_semi_supernode_with_validators_can_exceed_64() { let spec = E::default_spec(); let semi_supernode_cgc = spec.number_of_custody_groups / 2; // 64 - let custody_context = CustodyContext::::new(NodeCustodyType::SemiSupernode, &spec); + let custody_context = CustodyContext::::new( + NodeCustodyType::SemiSupernode, + generate_custody_indices(&spec), + &spec, + ); // Verify initial CGC is 64 (semi-supernode) assert_eq!( @@ -1346,6 +1351,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(20), + generate_custody_indices(&spec), &spec, ); diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 644c4716985..fcdf5898ec7 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -892,8 +892,6 @@ mod test { let da_checker = new_da_checker(spec.clone()); let custody_context = &da_checker.custody_context; - let all_column_indices_ordered = - init_custody_context_with_ordered_columns(custody_context, &mut rng, &spec); // GIVEN a single 32 ETH validator is attached slot 0 let epoch = Epoch::new(0); @@ -926,7 +924,8 @@ mod test { &spec, ); let block_root = Hash256::random(); - let requested_columns = &all_column_indices_ordered[..10]; + let custody_columns = custody_context.custody_columns_for_epoch(None, &spec); + let requested_columns = &custody_columns[..10]; da_checker .put_rpc_custody_columns( block_root, @@ -971,8 +970,6 @@ mod test { let da_checker = new_da_checker(spec.clone()); let custody_context = &da_checker.custody_context; - let all_column_indices_ordered = - init_custody_context_with_ordered_columns(custody_context, &mut rng, &spec); // GIVEN a single 32 ETH validator is attached slot 0 let epoch = Epoch::new(0); @@ -1006,7 +1003,8 @@ mod test { &spec, ); let block_root = Hash256::random(); - let requested_columns = &all_column_indices_ordered[..10]; + let custody_columns = custody_context.custody_columns_for_epoch(None, &spec); + let requested_columns = &custody_columns[..10]; let gossip_columns = data_columns .into_iter() .filter(|d| requested_columns.contains(&d.index)) @@ -1096,8 +1094,6 @@ mod test { let da_checker = new_da_checker(spec.clone()); let custody_context = &da_checker.custody_context; - let all_column_indices_ordered = - init_custody_context_with_ordered_columns(custody_context, &mut rng, &spec); // Set custody requirement to 65 columns (enough to trigger reconstruction) let epoch = Epoch::new(1); @@ -1127,7 +1123,8 @@ mod test { // Add 64 columns to the da checker (enough to be able to reconstruct) // Order by all_column_indices_ordered, then take first 64 - let custody_columns = all_column_indices_ordered + let custody_columns = custody_context.custody_columns_for_epoch(None, &spec); + let custody_columns = custody_columns .iter() .filter_map(|&col_idx| data_columns.iter().find(|d| d.index == col_idx).cloned()) .take(64) @@ -1177,19 +1174,6 @@ mod test { ); } - fn init_custody_context_with_ordered_columns( - custody_context: &Arc>, - mut rng: &mut StdRng, - spec: &ChainSpec, - ) -> Vec { - let mut all_data_columns = (0..spec.number_of_custody_groups).collect::>(); - all_data_columns.shuffle(&mut rng); - custody_context - .init_ordered_data_columns_from_custody_groups(all_data_columns.clone(), spec) - .expect("should initialise ordered custody columns"); - all_data_columns - } - fn new_da_checker(spec: Arc) -> DataAvailabilityChecker { let slot_clock = TestingSlotClock::new( Slot::new(0), @@ -1198,7 +1182,16 @@ mod test { ); let kzg = get_kzg(&spec); let store = Arc::new(HotColdDB::open_ephemeral(<_>::default(), spec.clone()).unwrap()); - let custody_context = Arc::new(CustodyContext::new(NodeCustodyType::Fullnode, &spec)); + let all_custody_groups_ordered = { + let mut all_custody_groups = (0..spec.number_of_custody_groups).collect::>(); + all_custody_groups.shuffle(&mut StdRng::seed_from_u64(42)); + all_custody_groups + }; + let custody_context = Arc::new(CustodyContext::new( + NodeCustodyType::Fullnode, + all_custody_groups_ordered, + &spec, + )); let complete_blob_backfill = false; DataAvailabilityChecker::new( complete_blob_backfill, 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 402dac1fa8c..bac6e701e8e 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 @@ -1023,7 +1023,12 @@ mod test { let spec = harness.spec.clone(); let test_store = harness.chain.store.clone(); let capacity_non_zero = new_non_zero_usize(capacity); - let custody_context = Arc::new(CustodyContext::new(NodeCustodyType::Fullnode, &spec)); + let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect(); + let custody_context = Arc::new(CustodyContext::new( + NodeCustodyType::Fullnode, + all_custody_groups_ordered, + &spec, + )); let cache = Arc::new( DataAvailabilityCheckerInner::::new( capacity_non_zero, diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 38797d0264d..079217b3b01 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -567,6 +567,7 @@ where .shutdown_sender(shutdown_tx) .chain_config(chain_config) .node_custody_type(self.node_custody_type) + .all_custody_groups_ordered((0..spec.number_of_custody_groups).collect()) .event_handler(Some(ServerSentEventHandler::new_with_capacity(5))) .validator_monitor_config(validator_monitor_config) .rng(Box::new(StdRng::seed_from_u64(42))); @@ -596,15 +597,6 @@ where let chain = builder.build().expect("should build"); - chain - .data_availability_checker - .custody_context() - .init_ordered_data_columns_from_custody_groups( - (0..spec.number_of_custody_groups).collect(), - &spec, - ) - .expect("should initialise custody context"); - BeaconChainHarness { spec: chain.spec.clone(), chain: Arc::new(chain), diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 0c83244f447..abfe38d347b 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2881,17 +2881,10 @@ async fn weak_subjectivity_sync_test( .shutdown_sender(shutdown_tx) .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) .execution_layer(Some(mock.el)) + .all_custody_groups_ordered((0..spec.number_of_custody_groups).collect()) .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"); - beacon_chain - .data_availability_checker - .custody_context() - .init_ordered_data_columns_from_custody_groups( - (0..spec.number_of_custody_groups).collect(), - &spec, - ) - .unwrap(); let beacon_chain = Arc::new(beacon_chain); let wss_block_root = wss_block.canonical_root(); diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index d9dea1b174f..8033a94232c 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -2,7 +2,7 @@ use crate::Client; use crate::compute_light_client_updates::{ LIGHT_CLIENT_SERVER_CHANNEL_CAPACITY, compute_light_client_updates, }; -use crate::config::{ClientGenesis, Config as ClientConfig, Config}; +use crate::config::{ClientGenesis, Config as ClientConfig}; use crate::notifier::spawn_notifier; use beacon_chain::attestation_simulator::start_attestation_simulator_service; use beacon_chain::data_availability_checker::start_availability_cache_maintenance_service; @@ -28,7 +28,6 @@ use execution_layer::ExecutionLayer; use execution_layer::test_utils::generate_genesis_header; use futures::channel::mpsc::Receiver; use genesis::{DEFAULT_ETH1_BLOCK_HASH, interop_genesis_state}; -use lighthouse_network::discv5::enr::NodeId; use lighthouse_network::identity::Keypair; use lighthouse_network::{NetworkGlobals, prometheus_client::registry::Registry}; use monitoring_api::{MonitoringHttpClient, ProcessType}; @@ -44,7 +43,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use store::database::interface::BeaconNodeBackend; use timer::spawn_timer; use tracing::{debug, info, warn}; -use types::data_column_custody_group::{CustodyIndex, get_custody_groups_ordered}; +use types::data_column_custody_group::get_custody_groups_ordered; use types::{ BeaconState, BlobSidecarList, ChainSpec, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, test_utils::generate_deterministic_keypairs, @@ -156,7 +155,7 @@ where mut self, client_genesis: ClientGenesis, config: ClientConfig, - local_keypair: Keypair, + node_id: [u8; 32], ) -> Result { let store = self.store.clone(); let chain_spec = self.chain_spec.clone(); @@ -194,6 +193,10 @@ where Kzg::new_from_trusted_setup_no_precomp(&config.trusted_setup).map_err(kzg_err_msg)? }; + let all_custody_groups_ordered = + get_custody_groups_ordered(node_id, spec.number_of_custody_groups, &spec) + .map_err(|e| format!("Failed to compute custody groups: {:?}", e))?; + let builder = BeaconChainBuilder::new(eth_spec_instance, Arc::new(kzg)) .store(store) .task_executor(context.executor.clone()) @@ -206,13 +209,18 @@ where .event_handler(event_handler) .execution_layer(execution_layer) .node_custody_type(config.chain.node_custody_type) - .node_id(NodeId::from(local_keypair.public()).raw()) + .all_custody_groups_ordered(all_custody_groups_ordered.clone()) .validator_monitor_config(config.validator_monitor.clone()) .rng(Box::new( StdRng::try_from_rng(&mut OsRng) .map_err(|e| format!("Failed to create RNG: {:?}", e))?, )); + println!( + "all_custody_groups_ordered: {:?}", + all_custody_groups_ordered + ); + let builder = if let Some(slasher) = self.slasher.clone() { builder.slasher(slasher) } else { diff --git a/beacon_node/lighthouse_network/tests/common.rs b/beacon_node/lighthouse_network/tests/common.rs index 8a3047692f3..9e8b243698b 100644 --- a/beacon_node/lighthouse_network/tests/common.rs +++ b/beacon_node/lighthouse_network/tests/common.rs @@ -16,6 +16,7 @@ use types::{ type E = MinimalEthSpec; +use lighthouse_network::identity::secp256k1; use lighthouse_network::rpc::config::InboundRateLimiterConfig; use tempfile::Builder as TempBuilder; @@ -138,10 +139,15 @@ pub async fn build_libp2p_instance( libp2p_registry: None, }; Libp2pInstance( - LibP2PService::new(executor, libp2p_context, custody_group_count) - .await - .expect("should build libp2p instance") - .0, + LibP2PService::new( + executor, + libp2p_context, + custody_group_count, + secp256k1::Keypair::generate().into(), + ) + .await + .expect("should build libp2p instance") + .0, signal, ) } diff --git a/beacon_node/network/src/service/tests.rs b/beacon_node/network/src/service/tests.rs index 64815ab2bb4..8ff1e0488df 100644 --- a/beacon_node/network/src/service/tests.rs +++ b/beacon_node/network/src/service/tests.rs @@ -6,6 +6,7 @@ use beacon_chain::BeaconChainTypes; use beacon_chain::test_utils::BeaconChainHarness; use beacon_processor::{BeaconProcessorChannels, BeaconProcessorConfig}; use futures::StreamExt; +use lighthouse_network::identity::secp256k1; use lighthouse_network::types::{GossipEncoding, GossipKind}; use lighthouse_network::{Enr, GossipTopic}; use std::str::FromStr; @@ -66,6 +67,7 @@ fn test_dht_persistence() { executor, None, beacon_processor_tx, + secp256k1::Keypair::generate().into(), ) .await .unwrap(); @@ -134,6 +136,7 @@ fn test_removing_topic_weight_on_old_topics() { executor.clone(), None, beacon_processor_channels.beacon_processor_tx, + secp256k1::Keypair::generate().into(), ) .await .unwrap() diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index 2e9b4036be0..ce9f04a161f 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -10,6 +10,7 @@ pub use config::{get_config, get_data_dir, set_network_config}; use environment::RuntimeContext; pub use eth2_config::Eth2Config; use lighthouse_network::load_private_key; +use network_utils::enr_ext::peer_id_to_node_id; use slasher::{DatabaseBackendOverride, Slasher}; use std::ops::{Deref, DerefMut}; use std::sync::Arc; @@ -122,8 +123,10 @@ impl ProductionBeaconNode { }; let local_keypair = load_private_key(&client_config.network); + // ASSERT same result + let node_id = peer_id_to_node_id(&local_keypair.public().to_peer_id())?.raw(); let builder = builder - .beacon_chain_builder(client_genesis, client_config.clone(), local_keypair) + .beacon_chain_builder(client_genesis, client_config.clone(), node_id) .await?; info!("Block production enabled"); @@ -197,6 +200,8 @@ impl lighthouse_network::discv5::Executor for Discv5Executor { #[cfg(test)] mod test { use super::*; + + use types::MainnetEthSpec; #[test] diff --git a/common/network_utils/src/enr_ext.rs b/common/network_utils/src/enr_ext.rs index 627dd15559f..3fe4c412920 100644 --- a/common/network_utils/src/enr_ext.rs +++ b/common/network_utils/src/enr_ext.rs @@ -353,7 +353,6 @@ pub fn peer_id_to_node_id(peer_id: &PeerId) -> Result Date: Wed, 12 Nov 2025 16:35:38 +1100 Subject: [PATCH 3/7] Update interface again --- beacon_node/beacon_chain/src/builder.rs | 26 ++--- .../beacon_chain/src/custody_context.rs | 109 ++++++++---------- .../src/data_availability_checker.rs | 10 +- .../overflow_lru_cache.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 2 +- beacon_node/beacon_chain/tests/store_tests.rs | 2 +- beacon_node/client/src/builder.rs | 14 +-- beacon_node/src/lib.rs | 3 +- .../types/src/data_column_custody_group.rs | 23 +++- 9 files changed, 97 insertions(+), 96 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index da1a59dca2d..7d01380d05c 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -42,8 +42,8 @@ use task_executor::{ShutdownReason, TaskExecutor}; use tracing::{debug, error, info}; use types::data_column_custody_group::CustodyIndex; use types::{ - BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, - FixedBytesExtended, Hash256, Signature, SignedBeaconBlock, Slot, + BeaconBlock, BeaconState, BlobSidecarList, ChainSpec, ColumnIndex, DataColumnSidecarList, + Epoch, EthSpec, FixedBytesExtended, Hash256, Signature, SignedBeaconBlock, Slot, }; /// An empty struct used to "witness" all the `BeaconChainTypes` traits. It has no user-facing @@ -103,7 +103,7 @@ pub struct BeaconChainBuilder { task_executor: Option, validator_monitor_config: Option, node_custody_type: NodeCustodyType, - all_custody_groups_ordered: Option>, + ordered_custody_column_indices: Option>, rng: Option>, } @@ -143,7 +143,7 @@ where task_executor: None, validator_monitor_config: None, node_custody_type: NodeCustodyType::Fullnode, - all_custody_groups_ordered: None, + ordered_custody_column_indices: None, rng: None, } } @@ -650,13 +650,13 @@ where self } - /// Sets the custody group order for this node. + /// Sets the ordered custody column indices for this node. /// This is used to determine the data columns the node is required to custody. - pub fn all_custody_groups_ordered( + pub fn ordered_custody_column_indices( mut self, - all_custody_groups_ordered: Vec, + ordered_custody_column_indices: Vec, ) -> Self { - self.all_custody_groups_ordered = Some(all_custody_groups_ordered); + self.ordered_custody_column_indices = Some(ordered_custody_column_indices); self } @@ -753,9 +753,9 @@ where .genesis_state_root .ok_or("Cannot build without a genesis state root")?; let validator_monitor_config = self.validator_monitor_config.unwrap_or_default(); - let all_custody_groups_ordered = self - .all_custody_groups_ordered - .ok_or("Cannot build without ordered custody groups")?; + let ordered_custody_column_indices = self + .ordered_custody_column_indices + .ok_or("Cannot build without ordered custody column indices")?; let rng = self.rng.ok_or("Cannot build without an RNG")?; let beacon_proposer_cache: Arc> = <_>::default(); @@ -958,14 +958,14 @@ where custody, self.node_custody_type, head_epoch, - all_custody_groups_ordered, + ordered_custody_column_indices, &self.spec, ) } else { ( CustodyContext::new( self.node_custody_type, - all_custody_groups_ordered, + ordered_custody_column_indices, &self.spec, ), None, diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index f0b14f460e3..4b2c8e4cd9f 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -247,9 +247,9 @@ pub struct CustodyContext { validator_custody_count: AtomicU64, /// Maintains all the validators that this node is connected to currently validator_registrations: RwLock, - /// Stores an immutable, ordered list of all data columns as determined by the node's NodeID + /// Stores an immutable, ordered list of all data column indices as determined by the node's NodeID /// on startup. This used to determine the node's custody columns. - all_custody_columns_ordered: Vec, + ordered_custody_column_indices: Vec, _phantom_data: PhantomData, } @@ -260,20 +260,17 @@ impl CustodyContext { /// The `node_custody_type` value is based on current cli parameters. pub fn new( node_custody_type: NodeCustodyType, - all_custody_groups_ordered: Vec, + ordered_custody_column_indices: Vec, spec: &ChainSpec, ) -> Self { let cgc_override = node_custody_type.get_custody_count_override(spec); // If there's no override, we initialise `validator_custody_count` to 0. This has been the // existing behaviour and we maintain this for now to avoid a semantic schema change until // a later release. - // FIXME: remove unwrap - let all_custody_columns_ordered = - Self::compute_ordered_data_columns(all_custody_groups_ordered, spec).unwrap(); Self { validator_custody_count: AtomicU64::new(cgc_override.unwrap_or(0)), validator_registrations: RwLock::new(ValidatorRegistrations::new(cgc_override)), - all_custody_columns_ordered, + ordered_custody_column_indices, _phantom_data: PhantomData, } } @@ -296,7 +293,7 @@ impl CustodyContext { ssz_context: CustodyContextSsz, node_custody_type: NodeCustodyType, head_epoch: Epoch, - all_custody_groups_ordered: Vec, + ordered_custody_column_indices: Vec, spec: &ChainSpec, ) -> (Self, Option) { let CustodyContextSsz { @@ -354,10 +351,6 @@ impl CustodyContext { } } - // FIXME: remove unwrap - let all_custody_columns_ordered = - Self::compute_ordered_data_columns(all_custody_groups_ordered, spec).unwrap(); - let custody_context = CustodyContext { validator_custody_count: AtomicU64::new(validator_custody_at_head), validator_registrations: RwLock::new(ValidatorRegistrations { @@ -366,34 +359,13 @@ impl CustodyContext { .into_iter() .collect(), }), - all_custody_columns_ordered, + ordered_custody_column_indices, _phantom_data: PhantomData, }; (custody_context, custody_count_changed) } - /// Initializes an ordered list of data columns based on provided custody groups. - /// - /// # Arguments - /// * `all_custody_groups_ordered` - Vector of custody group indices to map to columns - /// * `spec` - Chain specification containing custody parameters - /// - /// # Returns - /// Ok(()) if initialization succeeds, Err with description string if it fails - fn compute_ordered_data_columns( - all_custody_groups_ordered: Vec, - spec: &ChainSpec, - ) -> Result, String> { - let mut ordered_custody_columns = vec![]; - for custody_index in all_custody_groups_ordered { - let columns = compute_columns_for_custody_group::(custody_index, spec) - .map_err(|e| format!("Failed to compute columns for custody group {e:?}"))?; - ordered_custody_columns.extend(columns); - } - Ok(ordered_custody_columns) - } - /// Register a new validator index and updates the list of validators if required. /// /// Also modifies the internal structures if the validator custody has changed to @@ -503,7 +475,7 @@ impl CustodyContext { /// A slice of ordered column indices that should be sampled for this epoch based on the node's custody configuration pub fn sampling_columns_for_epoch(&self, epoch: Epoch, spec: &ChainSpec) -> &[ColumnIndex] { let num_of_columns_to_sample = self.num_of_data_columns_to_sample(epoch, spec); - &self.all_custody_columns_ordered[..num_of_columns_to_sample] + &self.ordered_custody_column_indices[..num_of_columns_to_sample] } /// Returns the ordered list of column indices that the node is assigned to custody @@ -534,7 +506,7 @@ impl CustodyContext { let columns_per_custody_group = spec.data_columns_per_group::() as usize; let custody_column_count = columns_per_custody_group * (custody_group_count); - &self.all_custody_columns_ordered[..custody_column_count] + &self.ordered_custody_column_indices[..custody_column_count] } /// The node has completed backfill for this epoch. Update the internal records so the function @@ -622,15 +594,15 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, head_epoch, - generate_custody_indices(spec), + generate_custody_column_indices(), spec, ); custody_context } - fn generate_custody_indices(spec: &ChainSpec) -> Vec { - (0..spec.number_of_custody_groups).collect::>() + fn generate_custody_column_indices() -> Vec { + (0..E::number_of_columns() as u64).collect::>() } fn complete_backfill_for_epochs( @@ -668,7 +640,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, - generate_custody_indices(spec), + generate_custody_column_indices(), spec, ); @@ -739,7 +711,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, - generate_custody_indices(spec), + generate_custody_column_indices(), spec, ); @@ -763,7 +735,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); assert_eq!( @@ -781,7 +753,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::SemiSupernode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); assert_eq!( @@ -799,7 +771,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); assert_eq!( @@ -818,7 +790,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -846,7 +818,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -887,7 +859,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -929,7 +901,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let current_slot = Slot::new(10); @@ -964,7 +936,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let current_slot = Slot::new(10); @@ -1010,7 +982,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); let current_slot = Slot::new(10); @@ -1084,9 +1056,12 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_fullnode() { let spec = E::default_spec(); - let all_custody_groups_ordered = generate_custody_indices(&spec); - let custody_context = - CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); + let ordered_custody_column_indices = generate_custody_column_indices(); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + ordered_custody_column_indices, + &spec, + ); assert_eq!( custody_context.custody_columns_for_epoch(None, &spec).len(), @@ -1097,10 +1072,10 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_supernode() { let spec = E::default_spec(); - let all_custody_groups_ordered = generate_custody_indices(&spec); + let ordered_custody_column_indices = generate_custody_column_indices(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, - all_custody_groups_ordered, + ordered_custody_column_indices, &spec, ); @@ -1113,9 +1088,12 @@ mod tests { #[test] fn custody_columns_for_epoch_with_validators_should_match_cgc() { let spec = E::default_spec(); - let all_custody_groups_ordered = generate_custody_indices(&spec); - let custody_context = - CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); + let ordered_custody_column_indices = generate_custody_column_indices(); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + ordered_custody_column_indices, + &spec, + ); let val_custody_units = 10; let _ = custody_context.register_validators( @@ -1136,9 +1114,12 @@ mod tests { #[test] fn custody_columns_for_epoch_specific_epoch_uses_epoch_cgc() { let spec = E::default_spec(); - let all_custody_groups_ordered = generate_custody_indices(&spec); - let custody_context = - CustodyContext::::new(NodeCustodyType::Fullnode, all_custody_groups_ordered, &spec); + let ordered_custody_column_indices = generate_custody_column_indices(); + let custody_context = CustodyContext::::new( + NodeCustodyType::Fullnode, + ordered_custody_column_indices, + &spec, + ); let test_epoch = Epoch::new(5); let expected_cgc = custody_context.custody_group_count_at_epoch(test_epoch, &spec); @@ -1163,7 +1144,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(0), - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); @@ -1199,7 +1180,7 @@ mod tests { let semi_supernode_cgc = spec.number_of_custody_groups / 2; // 64 let custody_context = CustodyContext::::new( NodeCustodyType::SemiSupernode, - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); @@ -1351,7 +1332,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(20), - generate_custody_indices(&spec), + generate_custody_column_indices(), &spec, ); diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index fcdf5898ec7..3bb24ef726e 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1182,14 +1182,14 @@ mod test { ); let kzg = get_kzg(&spec); let store = Arc::new(HotColdDB::open_ephemeral(<_>::default(), spec.clone()).unwrap()); - let all_custody_groups_ordered = { - let mut all_custody_groups = (0..spec.number_of_custody_groups).collect::>(); - all_custody_groups.shuffle(&mut StdRng::seed_from_u64(42)); - all_custody_groups + let ordered_custody_column_indices = { + let mut column_indices = (0..E::number_of_columns() as u64).collect::>(); + column_indices.shuffle(&mut StdRng::seed_from_u64(42)); + column_indices }; let custody_context = Arc::new(CustodyContext::new( NodeCustodyType::Fullnode, - all_custody_groups_ordered, + ordered_custody_column_indices, &spec, )); let complete_blob_backfill = false; 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 bac6e701e8e..6f9f39c7dbe 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 @@ -1023,10 +1023,10 @@ mod test { let spec = harness.spec.clone(); let test_store = harness.chain.store.clone(); let capacity_non_zero = new_non_zero_usize(capacity); - let all_custody_groups_ordered = (0..spec.number_of_custody_groups).collect(); + let ordered_custody_column_indices = (0..E::number_of_columns() as u64).collect(); let custody_context = Arc::new(CustodyContext::new( NodeCustodyType::Fullnode, - all_custody_groups_ordered, + ordered_custody_column_indices, &spec, )); let cache = Arc::new( diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 079217b3b01..9b8f7d40e9a 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -567,7 +567,7 @@ where .shutdown_sender(shutdown_tx) .chain_config(chain_config) .node_custody_type(self.node_custody_type) - .all_custody_groups_ordered((0..spec.number_of_custody_groups).collect()) + .ordered_custody_column_indices((0..E::number_of_columns() as u64).collect()) .event_handler(Some(ServerSentEventHandler::new_with_capacity(5))) .validator_monitor_config(validator_monitor_config) .rng(Box::new(StdRng::seed_from_u64(42))); diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index abfe38d347b..6f3140a3d0b 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2881,7 +2881,7 @@ async fn weak_subjectivity_sync_test( .shutdown_sender(shutdown_tx) .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) .execution_layer(Some(mock.el)) - .all_custody_groups_ordered((0..spec.number_of_custody_groups).collect()) + .ordered_custody_column_indices((0..E::number_of_columns() as u64).collect()) .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"); diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 8033a94232c..5714921035a 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -43,7 +43,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use store::database::interface::BeaconNodeBackend; use timer::spawn_timer; use tracing::{debug, info, warn}; -use types::data_column_custody_group::get_custody_groups_ordered; +use types::data_column_custody_group::compute_ordered_custody_column_indices; use types::{ BeaconState, BlobSidecarList, ChainSpec, EthSpec, ExecutionBlockHash, Hash256, SignedBeaconBlock, test_utils::generate_deterministic_keypairs, @@ -193,9 +193,8 @@ where Kzg::new_from_trusted_setup_no_precomp(&config.trusted_setup).map_err(kzg_err_msg)? }; - let all_custody_groups_ordered = - get_custody_groups_ordered(node_id, spec.number_of_custody_groups, &spec) - .map_err(|e| format!("Failed to compute custody groups: {:?}", e))?; + let ordered_custody_column_indices = compute_ordered_custody_column_indices(node_id, &spec) + .map_err(|e| format!("Failed to compute ordered custody column indices: {:?}", e))?; let builder = BeaconChainBuilder::new(eth_spec_instance, Arc::new(kzg)) .store(store) @@ -209,16 +208,17 @@ where .event_handler(event_handler) .execution_layer(execution_layer) .node_custody_type(config.chain.node_custody_type) - .all_custody_groups_ordered(all_custody_groups_ordered.clone()) + .ordered_custody_column_indices(ordered_custody_column_indices.clone()) .validator_monitor_config(config.validator_monitor.clone()) .rng(Box::new( StdRng::try_from_rng(&mut OsRng) .map_err(|e| format!("Failed to create RNG: {:?}", e))?, )); + // FIXME: remove println!( - "all_custody_groups_ordered: {:?}", - all_custody_groups_ordered + "ordered_custody_column_indices: {:?}", + ordered_custody_column_indices ); let builder = if let Some(slasher) = self.slasher.clone() { diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index ce9f04a161f..bd6cf0be975 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -200,8 +200,7 @@ impl lighthouse_network::discv5::Executor for Discv5Executor { #[cfg(test)] mod test { use super::*; - - + use types::MainnetEthSpec; #[test] diff --git a/consensus/types/src/data_column_custody_group.rs b/consensus/types/src/data_column_custody_group.rs index 0c44608e460..7ecabab0abc 100644 --- a/consensus/types/src/data_column_custody_group.rs +++ b/consensus/types/src/data_column_custody_group.rs @@ -42,7 +42,7 @@ pub fn get_custody_groups( /// /// # Returns /// Vector of custody group indices in computation order or error if parameters are invalid -pub fn get_custody_groups_ordered( +fn get_custody_groups_ordered( raw_node_id: [u8; 32], custody_group_count: u64, spec: &ChainSpec, @@ -76,6 +76,27 @@ pub fn get_custody_groups_ordered( Ok(custody_groups) } +/// Returns a deterministically ordered list of custody columns assigned to a node, +/// preserving the order in which they were computed during iteration. +/// +/// # Arguments +/// * `raw_node_id` - 32-byte node identifier +/// * `spec` - Chain specification containing custody parameters +pub fn compute_ordered_custody_column_indices( + raw_node_id: [u8; 32], + spec: &ChainSpec, +) -> Result, DataColumnCustodyGroupError> { + let all_custody_groups_ordered = + get_custody_groups_ordered(raw_node_id, spec.number_of_custody_groups, spec)?; + + let mut ordered_custody_columns = vec![]; + for custody_index in all_custody_groups_ordered { + let columns = compute_columns_for_custody_group::(custody_index, spec)?; + ordered_custody_columns.extend(columns); + } + Ok(ordered_custody_columns) +} + /// Returns the columns that are associated with a given custody group. /// /// spec: https://github.com/ethereum/consensus-specs/blob/8e0d0d48e81d6c7c5a8253ab61340f5ea5bac66a/specs/fulu/das-core.md#compute_columns_for_custody_group From 935db191fde98a6bc8da0f26cb8da74a72102c51 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 12 Nov 2025 16:53:25 +1100 Subject: [PATCH 4/7] Clean up --- beacon_node/beacon_chain/src/custody_context.rs | 3 +-- beacon_node/client/src/builder.rs | 12 ++++-------- beacon_node/src/lib.rs | 4 ++-- common/network_utils/src/enr_ext.rs | 1 + 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index 4b2c8e4cd9f..a1f87aedddf 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -7,7 +7,6 @@ use std::{ sync::atomic::{AtomicU64, Ordering}, }; use tracing::{debug, warn}; -use types::data_column_custody_group::{CustodyIndex, compute_columns_for_custody_group}; use types::{ChainSpec, ColumnIndex, Epoch, EthSpec, Slot}; /// A delay before making the CGC change effective to the data availability checker. @@ -504,7 +503,7 @@ impl CustodyContext { // This is an unnecessary conversion for spec compliance, basically just multiplying by 1. let columns_per_custody_group = spec.data_columns_per_group::() as usize; - let custody_column_count = columns_per_custody_group * (custody_group_count); + let custody_column_count = columns_per_custody_group * custody_group_count; &self.ordered_custody_column_indices[..custody_column_count] } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 5714921035a..8e5837a7adf 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -193,8 +193,10 @@ where Kzg::new_from_trusted_setup_no_precomp(&config.trusted_setup).map_err(kzg_err_msg)? }; - let ordered_custody_column_indices = compute_ordered_custody_column_indices(node_id, &spec) - .map_err(|e| format!("Failed to compute ordered custody column indices: {:?}", e))?; + let ordered_custody_column_indices = + compute_ordered_custody_column_indices::(node_id, &spec).map_err(|e| { + format!("Failed to compute ordered custody column indices: {:?}", e) + })?; let builder = BeaconChainBuilder::new(eth_spec_instance, Arc::new(kzg)) .store(store) @@ -215,12 +217,6 @@ where .map_err(|e| format!("Failed to create RNG: {:?}", e))?, )); - // FIXME: remove - println!( - "ordered_custody_column_indices: {:?}", - ordered_custody_column_indices - ); - let builder = if let Some(slasher) = self.slasher.clone() { builder.slasher(slasher) } else { diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index bd6cf0be975..6db2150e5f5 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -122,9 +122,10 @@ impl ProductionBeaconNode { builder }; + // Generate or load the node id. let local_keypair = load_private_key(&client_config.network); - // ASSERT same result let node_id = peer_id_to_node_id(&local_keypair.public().to_peer_id())?.raw(); + let builder = builder .beacon_chain_builder(client_genesis, client_config.clone(), node_id) .await?; @@ -200,7 +201,6 @@ impl lighthouse_network::discv5::Executor for Discv5Executor { #[cfg(test)] mod test { use super::*; - use types::MainnetEthSpec; #[test] diff --git a/common/network_utils/src/enr_ext.rs b/common/network_utils/src/enr_ext.rs index 3fe4c412920..627dd15559f 100644 --- a/common/network_utils/src/enr_ext.rs +++ b/common/network_utils/src/enr_ext.rs @@ -353,6 +353,7 @@ pub fn peer_id_to_node_id(peer_id: &PeerId) -> Result Date: Wed, 12 Nov 2025 17:18:12 +1100 Subject: [PATCH 5/7] Fix tests --- beacon_node/beacon_chain/src/builder.rs | 3 +++ beacon_node/client/src/builder.rs | 2 +- beacon_node/network/src/subnet_service/tests/mod.rs | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 7d01380d05c..6ef7f26f468 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1293,6 +1293,9 @@ mod test { .expect("should configure testing slot clock") .shutdown_sender(shutdown_tx) .rng(Box::new(StdRng::seed_from_u64(42))) + .ordered_custody_column_indices( + (0..MinimalEthSpec::number_of_columns() as u64).collect(), + ) .build() .expect("should build"); diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 8e5837a7adf..8305ec50f82 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -210,7 +210,7 @@ where .event_handler(event_handler) .execution_layer(execution_layer) .node_custody_type(config.chain.node_custody_type) - .ordered_custody_column_indices(ordered_custody_column_indices.clone()) + .ordered_custody_column_indices(ordered_custody_column_indices) .validator_monitor_config(config.validator_monitor.clone()) .rng(Box::new( StdRng::try_from_rng(&mut OsRng) diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index 0df28cff6b7..e2f188489ad 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -73,6 +73,9 @@ impl TestBeaconChain { Duration::from_secs(recent_genesis_time()), Duration::from_millis(SLOT_DURATION_MILLIS), )) + .ordered_custody_column_indices( + (0..MainnetEthSpec::number_of_columns() as u64).collect(), + ) .shutdown_sender(shutdown_tx) .rng(Box::new(StdRng::seed_from_u64(42))) .build() From abbd4b2c73bde6d40d4dd0e61a9fbf307f5e9bfb Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 17 Nov 2025 14:54:08 +1100 Subject: [PATCH 6/7] Generate custody column indices in tests in arbitrary order. --- beacon_node/beacon_chain/src/builder.rs | 6 ++- .../beacon_chain/src/custody_context.rs | 46 +++++++++---------- .../src/data_availability_checker.rs | 9 ++-- .../overflow_lru_cache.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 10 +++- beacon_node/beacon_chain/tests/store_tests.rs | 2 +- .../network/src/subnet_service/tests/mod.rs | 7 +-- 7 files changed, 44 insertions(+), 40 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 6ef7f26f468..ef438b16e0f 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1241,7 +1241,9 @@ fn build_data_columns_from_blobs( #[cfg(test)] mod test { use super::*; - use crate::test_utils::{EphemeralHarnessType, get_kzg}; + use crate::test_utils::{ + EphemeralHarnessType, generate_data_column_indices_rand_order, get_kzg, + }; use ethereum_hashing::hash; use genesis::{ DEFAULT_ETH1_BLOCK_HASH, generate_deterministic_keypairs, interop_genesis_state, @@ -1294,7 +1296,7 @@ mod test { .shutdown_sender(shutdown_tx) .rng(Box::new(StdRng::seed_from_u64(42))) .ordered_custody_column_indices( - (0..MinimalEthSpec::number_of_columns() as u64).collect(), + generate_data_column_indices_rand_order::(), ) .build() .expect("should build"); diff --git a/beacon_node/beacon_chain/src/custody_context.rs b/beacon_node/beacon_chain/src/custody_context.rs index a1f87aedddf..c512ce616a1 100644 --- a/beacon_node/beacon_chain/src/custody_context.rs +++ b/beacon_node/beacon_chain/src/custody_context.rs @@ -571,9 +571,9 @@ impl From<&CustodyContext> for CustodyContextSsz { #[cfg(test)] mod tests { - use types::MainnetEthSpec; - use super::*; + use crate::test_utils::generate_data_column_indices_rand_order; + use types::MainnetEthSpec; type E = MainnetEthSpec; @@ -593,17 +593,13 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, head_epoch, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), spec, ); custody_context } - fn generate_custody_column_indices() -> Vec { - (0..E::number_of_columns() as u64).collect::>() - } - fn complete_backfill_for_epochs( custody_context: &CustodyContext, start_epoch: Epoch, @@ -639,7 +635,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), spec, ); @@ -710,7 +706,7 @@ mod tests { ssz_context, target_node_custody_type, head_epoch, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), spec, ); @@ -734,7 +730,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); assert_eq!( @@ -752,7 +748,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::SemiSupernode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); assert_eq!( @@ -770,7 +766,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); assert_eq!( @@ -789,7 +785,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -817,7 +813,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -858,7 +854,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let bal_per_additional_group = spec.balance_per_additional_custody_group; @@ -900,7 +896,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let current_slot = Slot::new(10); @@ -935,7 +931,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let current_slot = Slot::new(10); @@ -981,7 +977,7 @@ mod tests { let spec = E::default_spec(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); let current_slot = Slot::new(10); @@ -1055,7 +1051,7 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_fullnode() { let spec = E::default_spec(); - let ordered_custody_column_indices = generate_custody_column_indices(); + let ordered_custody_column_indices = generate_data_column_indices_rand_order::(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, ordered_custody_column_indices, @@ -1071,7 +1067,7 @@ mod tests { #[test] fn custody_columns_for_epoch_no_validators_supernode() { let spec = E::default_spec(); - let ordered_custody_column_indices = generate_custody_column_indices(); + let ordered_custody_column_indices = generate_data_column_indices_rand_order::(); let custody_context = CustodyContext::::new( NodeCustodyType::Supernode, ordered_custody_column_indices, @@ -1087,7 +1083,7 @@ mod tests { #[test] fn custody_columns_for_epoch_with_validators_should_match_cgc() { let spec = E::default_spec(); - let ordered_custody_column_indices = generate_custody_column_indices(); + let ordered_custody_column_indices = generate_data_column_indices_rand_order::(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, ordered_custody_column_indices, @@ -1113,7 +1109,7 @@ mod tests { #[test] fn custody_columns_for_epoch_specific_epoch_uses_epoch_cgc() { let spec = E::default_spec(); - let ordered_custody_column_indices = generate_custody_column_indices(); + let ordered_custody_column_indices = generate_data_column_indices_rand_order::(); let custody_context = CustodyContext::::new( NodeCustodyType::Fullnode, ordered_custody_column_indices, @@ -1143,7 +1139,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(0), - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); @@ -1179,7 +1175,7 @@ mod tests { let semi_supernode_cgc = spec.number_of_custody_groups / 2; // 64 let custody_context = CustodyContext::::new( NodeCustodyType::SemiSupernode, - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); @@ -1331,7 +1327,7 @@ mod tests { ssz_context, NodeCustodyType::Fullnode, Epoch::new(20), - generate_custody_column_indices(), + generate_data_column_indices_rand_order::(), &spec, ); diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 3bb24ef726e..e2a8f6fef35 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -866,7 +866,8 @@ mod test { use crate::CustodyContext; use crate::custody_context::NodeCustodyType; use crate::test_utils::{ - EphemeralHarnessType, NumBlobs, generate_rand_block_and_data_columns, get_kzg, + EphemeralHarnessType, NumBlobs, generate_data_column_indices_rand_order, + generate_rand_block_and_data_columns, get_kzg, }; use rand::SeedableRng; use rand::prelude::StdRng; @@ -1182,11 +1183,7 @@ mod test { ); let kzg = get_kzg(&spec); let store = Arc::new(HotColdDB::open_ephemeral(<_>::default(), spec.clone()).unwrap()); - let ordered_custody_column_indices = { - let mut column_indices = (0..E::number_of_columns() as u64).collect::>(); - column_indices.shuffle(&mut StdRng::seed_from_u64(42)); - column_indices - }; + let ordered_custody_column_indices = generate_data_column_indices_rand_order::(); let custody_context = Arc::new(CustodyContext::new( NodeCustodyType::Fullnode, ordered_custody_column_indices, 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 6f9f39c7dbe..aa232502969 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 @@ -823,6 +823,7 @@ impl DataAvailabilityCheckerInner { mod test { use super::*; + use crate::test_utils::generate_data_column_indices_rand_order; use crate::{ blob_verification::GossipVerifiedBlob, block_verification::PayloadVerificationOutcome, @@ -1023,10 +1024,9 @@ mod test { let spec = harness.spec.clone(); let test_store = harness.chain.store.clone(); let capacity_non_zero = new_non_zero_usize(capacity); - let ordered_custody_column_indices = (0..E::number_of_columns() as u64).collect(); let custody_context = Arc::new(CustodyContext::new( NodeCustodyType::Fullnode, - ordered_custody_column_indices, + generate_data_column_indices_rand_order::(), &spec, )); let cache = Arc::new( diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 9b8f7d40e9a..b626fcd862c 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -42,6 +42,7 @@ use parking_lot::{Mutex, RwLockWriteGuard}; use rand::Rng; use rand::SeedableRng; use rand::rngs::StdRng; +use rand::seq::SliceRandom; use rayon::prelude::*; use sensitive_url::SensitiveUrl; use slot_clock::{SlotClock, TestingSlotClock}; @@ -59,6 +60,7 @@ use store::{HotColdDB, ItemStore, MemoryStore, config::StoreConfig}; use task_executor::TaskExecutor; use task_executor::{ShutdownReason, test_utils::TestRuntime}; use tree_hash::TreeHash; +use types::data_column_custody_group::CustodyIndex; use types::indexed_attestation::IndexedAttestationBase; use types::payload::BlockProductionVersion; use types::test_utils::TestRandom; @@ -567,7 +569,7 @@ where .shutdown_sender(shutdown_tx) .chain_config(chain_config) .node_custody_type(self.node_custody_type) - .ordered_custody_column_indices((0..E::number_of_columns() as u64).collect()) + .ordered_custody_column_indices(generate_data_column_indices_rand_order::()) .event_handler(Some(ServerSentEventHandler::new_with_capacity(5))) .validator_monitor_config(validator_monitor_config) .rng(Box::new(StdRng::seed_from_u64(42))); @@ -3423,3 +3425,9 @@ pub fn generate_data_column_sidecars_from_block( ) .unwrap() } + +pub fn generate_data_column_indices_rand_order() -> Vec { + let mut indices = (0..E::number_of_columns() as u64).collect::>(); + indices.shuffle(&mut StdRng::seed_from_u64(42)); + indices +} diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 6f3140a3d0b..eb827a5d172 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2881,7 +2881,7 @@ async fn weak_subjectivity_sync_test( .shutdown_sender(shutdown_tx) .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) .execution_layer(Some(mock.el)) - .ordered_custody_column_indices((0..E::number_of_columns() as u64).collect()) + .ordered_custody_column_indices(generate_data_column_indices_rand_order::()) .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"); diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index e2f188489ad..bee6569b7b3 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -1,4 +1,5 @@ use super::*; +use beacon_chain::test_utils::generate_data_column_indices_rand_order; use beacon_chain::{ BeaconChain, builder::{BeaconChainBuilder, Witness}, @@ -73,9 +74,9 @@ impl TestBeaconChain { Duration::from_secs(recent_genesis_time()), Duration::from_millis(SLOT_DURATION_MILLIS), )) - .ordered_custody_column_indices( - (0..MainnetEthSpec::number_of_columns() as u64).collect(), - ) + .ordered_custody_column_indices(generate_data_column_indices_rand_order::< + MainnetEthSpec, + >()) .shutdown_sender(shutdown_tx) .rng(Box::new(StdRng::seed_from_u64(42))) .build() From 6e554872d0af59c6956640c829193b0e724c7288 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 17 Nov 2025 14:59:02 +1100 Subject: [PATCH 7/7] Fix lint --- beacon_node/beacon_chain/src/data_availability_checker.rs | 1 - beacon_node/beacon_chain/tests/store_tests.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index e2a8f6fef35..3e859456b18 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -871,7 +871,6 @@ mod test { }; use rand::SeedableRng; use rand::prelude::StdRng; - use rand::seq::SliceRandom; use slot_clock::{SlotClock, TestingSlotClock}; use std::collections::HashSet; use std::sync::Arc; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index eb827a5d172..806c9dce7c1 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -7,11 +7,11 @@ use beacon_chain::custody_context::CUSTODY_CHANGE_DA_EFFECTIVE_DELAY_SECONDS; use beacon_chain::data_availability_checker::AvailableBlock; use beacon_chain::historical_data_columns::HistoricalDataColumnError; use beacon_chain::schema_change::migrate_schema; -use beacon_chain::test_utils::SyncCommitteeStrategy; use beacon_chain::test_utils::{ AttestationStrategy, BeaconChainHarness, BlockStrategy, DiskHarnessType, get_kzg, mock_execution_layer_from_parts, test_spec, }; +use beacon_chain::test_utils::{SyncCommitteeStrategy, generate_data_column_indices_rand_order}; use beacon_chain::{ BeaconChain, BeaconChainError, BeaconChainTypes, BeaconSnapshot, BlockError, ChainConfig, NotifyExecutionLayer, ServerSentEventHandler, WhenSlotSkipped,