From d96b73152e0e4bcedb8e747016ba8070029db50d Mon Sep 17 00:00:00 2001 From: SunnysidedJ Date: Wed, 9 Apr 2025 16:35:15 +0100 Subject: [PATCH] Fix for #6296: Deterministic RNG in peer DAS publish block tests (#7192) #6296: Deterministic RNG in peer DAS publish block tests Made test functions to call publish-block APIs with true for the deterministic RNG boolean parameter while production code with false. This will deterministically shuffle columns for unit tests under broadcast_validation_tests.rs. --- Cargo.lock | 1 + beacon_node/beacon_chain/src/beacon_chain.rs | 3 +++ beacon_node/beacon_chain/src/builder.rs | 16 ++++++++++++++++ beacon_node/beacon_chain/src/test_utils.rs | 6 +++--- beacon_node/beacon_chain/tests/store_tests.rs | 2 ++ beacon_node/client/Cargo.toml | 1 + beacon_node/client/src/builder.rs | 7 ++++++- beacon_node/http_api/src/publish_blocks.rs | 2 +- .../network/src/subnet_service/tests/mod.rs | 3 +++ 9 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 86019c913d..b11b585173 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1417,6 +1417,7 @@ dependencies = [ "monitoring_api", "network", "operation_pool", + "rand 0.8.5", "sensitive_url", "serde", "serde_json", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d6475de243..4d8e94f86d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -92,6 +92,7 @@ use operation_pool::{ }; use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; use proto_array::{DoNotReOrg, ProposerHeadError}; +use rand::RngCore; use safe_arith::SafeArith; use slasher::Slasher; use slot_clock::SlotClock; @@ -491,6 +492,8 @@ pub struct BeaconChain { pub data_availability_checker: Arc>, /// The KZG trusted setup used by this chain. pub kzg: Arc, + /// RNG instance used by the chain. Currently used for shuffling column sidecars in block publishing. + pub rng: Arc>>, } pub enum BeaconBlockResponseWrapper { diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 975be33f0b..812dcbeda7 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -30,6 +30,7 @@ use logging::crit; use operation_pool::{OperationPool, PersistedOperationPool}; use parking_lot::{Mutex, RwLock}; use proto_array::{DisallowedReOrgOffsets, ReOrgThreshold}; +use rand::RngCore; use rayon::prelude::*; use slasher::Slasher; use slot_clock::{SlotClock, TestingSlotClock}; @@ -105,6 +106,7 @@ pub struct BeaconChainBuilder { task_executor: Option, validator_monitor_config: Option, import_all_data_columns: bool, + rng: Option>, } impl @@ -145,6 +147,7 @@ where task_executor: None, validator_monitor_config: None, import_all_data_columns: false, + rng: None, } } @@ -691,6 +694,14 @@ where self } + /// Sets the `rng` field. + /// + /// Currently used for shuffling column sidecars in block publishing. + pub fn rng(mut self, rng: Box) -> Self { + self.rng = Some(rng); + self + } + /// Consumes `self`, returning a `BeaconChain` if all required parameters have been supplied. /// /// An error will be returned at runtime if all required parameters have not been configured. @@ -716,6 +727,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 rng = self.rng.ok_or("Cannot build without an RNG")?; let beacon_proposer_cache: Arc> = <_>::default(); let mut validator_monitor = @@ -979,6 +991,7 @@ where .map_err(|e| format!("Error initializing DataAvailabilityChecker: {:?}", e))?, ), kzg: self.kzg.clone(), + rng: Arc::new(Mutex::new(rng)), }; let head = beacon_chain.head_snapshot(); @@ -1184,6 +1197,8 @@ mod test { use genesis::{ generate_deterministic_keypairs, interop_genesis_state, DEFAULT_ETH1_BLOCK_HASH, }; + use rand::rngs::StdRng; + use rand::SeedableRng; use ssz::Encode; use std::time::Duration; use store::config::StoreConfig; @@ -1230,6 +1245,7 @@ mod test { .testing_slot_clock(Duration::from_secs(1)) .expect("should configure testing slot clock") .shutdown_sender(shutdown_tx) + .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index bcab512a4b..e007d46fc3 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -38,8 +38,7 @@ use kzg::{Kzg, TrustedSetup}; use logging::create_test_tracing_subscriber; use merkle_proof::MerkleTree; use operation_pool::ReceivedPreCapella; -use parking_lot::Mutex; -use parking_lot::RwLockWriteGuard; +use parking_lot::{Mutex, RwLockWriteGuard}; use rand::rngs::StdRng; use rand::Rng; use rand::SeedableRng; @@ -588,7 +587,8 @@ where .chain_config(chain_config) .import_all_data_columns(self.import_all_data_columns) .event_handler(Some(ServerSentEventHandler::new_with_capacity(5))) - .validator_monitor_config(validator_monitor_config); + .validator_monitor_config(validator_monitor_config) + .rng(Box::new(StdRng::seed_from_u64(42))); builder = if let Some(mutator) = self.initial_mutator { mutator(builder) diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index e41f547fb5..2fe1ecc08f 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -16,6 +16,7 @@ use beacon_chain::{ }; use logging::create_test_tracing_subscriber; use maplit::hashset; +use rand::rngs::StdRng; use rand::Rng; use slot_clock::{SlotClock, TestingSlotClock}; use state_processing::{state_advance::complete_state_advance, BlockReplayer}; @@ -2373,6 +2374,7 @@ async fn weak_subjectivity_sync_test(slots: Vec, checkpoint_slot: Slot) { .chain_config(ChainConfig::default()) .event_handler(Some(ServerSentEventHandler::new_with_capacity(1))) .execution_layer(Some(mock.el)) + .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"); diff --git a/beacon_node/client/Cargo.toml b/beacon_node/client/Cargo.toml index e11fc23072..195c53c4a0 100644 --- a/beacon_node/client/Cargo.toml +++ b/beacon_node/client/Cargo.toml @@ -31,6 +31,7 @@ logging = { workspace = true } metrics = { workspace = true } monitoring_api = { workspace = true } network = { workspace = true } +rand = { workspace = true } sensitive_url = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index c8ff6521c8..3cb7b33aae 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -33,6 +33,8 @@ use genesis::{interop_genesis_state, Eth1GenesisService, DEFAULT_ETH1_BLOCK_HASH use lighthouse_network::{prometheus_client::registry::Registry, NetworkGlobals}; use monitoring_api::{MonitoringHttpClient, ProcessType}; use network::{NetworkConfig, NetworkSenders, NetworkService}; +use rand::rngs::{OsRng, StdRng}; +use rand::SeedableRng; use slasher::Slasher; use slasher_service::SlasherService; use std::net::TcpListener; @@ -210,7 +212,10 @@ where .event_handler(event_handler) .execution_layer(execution_layer) .import_all_data_columns(config.network.subscribe_all_data_column_subnets) - .validator_monitor_config(config.validator_monitor.clone()); + .validator_monitor_config(config.validator_monitor.clone()) + .rng(Box::new( + StdRng::from_rng(OsRng).map_err(|e| format!("Failed to create RNG: {:?}", e))?, + )); let builder = if let Some(slasher) = self.slasher.clone() { builder.slasher(slasher) diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index ab70521686..3d152f8852 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -521,7 +521,7 @@ fn publish_column_sidecars( .len() .saturating_sub(malicious_withhold_count); // Randomize columns before dropping the last malicious_withhold_count items - data_column_sidecars.shuffle(&mut rand::thread_rng()); + data_column_sidecars.shuffle(&mut **chain.rng.lock()); data_column_sidecars.truncate(columns_to_keep); } let pubsub_messages = data_column_sidecars diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index 6f9e8cd41a..7fdf9047fc 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -7,6 +7,8 @@ use beacon_chain::{ }; use genesis::{generate_deterministic_keypairs, interop_genesis_state, DEFAULT_ETH1_BLOCK_HASH}; use lighthouse_network::NetworkConfig; +use rand::rngs::StdRng; +use rand::SeedableRng; use slot_clock::{SlotClock, SystemTimeSlotClock}; use std::sync::{Arc, LazyLock}; use std::time::{Duration, SystemTime}; @@ -76,6 +78,7 @@ impl TestBeaconChain { Duration::from_millis(SLOT_DURATION_MILLIS), )) .shutdown_sender(shutdown_tx) + .rng(Box::new(StdRng::seed_from_u64(42))) .build() .expect("should build"), );