From 30094f0c08c451087935ab932d1ac64b635085e7 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 30 Oct 2025 14:42:36 +1100 Subject: [PATCH] Remove redundant `subscribe_all_data_column_subnets` field from network (#8259) Addresses this comment: https://github.com/sigp/lighthouse/pull/8254#discussion_r2447998786 We're currently using `subscribe_all_data_column_subnets` here to subscribe to all subnets https://github.com/sigp/lighthouse/blob/522bd9e9c6ac167f2231525e937c9ebbcb86cf6e/beacon_node/lighthouse_network/src/types/topics.rs#L82-L92 But its unnecessary because the else path also works for supernode (uses `sampling_subnets` instead) The big diffs will disappear once #8254 is merged. Co-Authored-By: Jimmy Chen --- beacon_node/lighthouse_network/src/config.rs | 4 -- .../lighthouse_network/src/discovery/enr.rs | 58 +++---------------- .../lighthouse_network/src/discovery/mod.rs | 5 +- .../lighthouse_network/src/service/mod.rs | 2 +- .../lighthouse_network/src/types/globals.rs | 1 - .../lighthouse_network/src/types/topics.rs | 18 ++---- beacon_node/src/config.rs | 1 - lcli/src/generate_bootnode_enr.rs | 2 +- 8 files changed, 17 insertions(+), 74 deletions(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 89c6c58d4f..416ca73e08 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -94,9 +94,6 @@ pub struct Config { /// Attempt to construct external port mappings with UPnP. pub upnp_enabled: bool, - /// Subscribe to all data column subnets for the duration of the runtime. - pub subscribe_all_data_column_subnets: bool, - /// Subscribe to all subnets for the duration of the runtime. pub subscribe_all_subnets: bool, @@ -355,7 +352,6 @@ impl Default for Config { upnp_enabled: true, network_load: 3, private: false, - subscribe_all_data_column_subnets: false, subscribe_all_subnets: false, import_all_attestations: false, shutdown_after_sync: false, diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index bb9ff299c5..4c285ea86c 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -159,7 +159,7 @@ pub fn build_or_load_enr( local_key: Keypair, config: &NetworkConfig, enr_fork_id: &EnrForkId, - custody_group_count: Option, + custody_group_count: u64, next_fork_digest: [u8; 4], spec: &ChainSpec, ) -> Result { @@ -185,7 +185,7 @@ pub fn build_enr( enr_key: &CombinedKey, config: &NetworkConfig, enr_fork_id: &EnrForkId, - custody_group_count: Option, + custody_group_count: u64, next_fork_digest: [u8; 4], spec: &ChainSpec, ) -> Result { @@ -280,15 +280,6 @@ pub fn build_enr( // only set `cgc` and `nfd` if PeerDAS fork (Fulu) epoch has been scheduled if spec.is_peer_das_scheduled() { - let custody_group_count = if let Some(cgc) = custody_group_count { - cgc - } else if let Some(false_cgc) = config.advertise_false_custody_group_count { - false_cgc - } else if config.subscribe_all_data_column_subnets { - spec.number_of_custody_groups - } else { - spec.custody_requirement - }; builder.add_value(PEERDAS_CUSTODY_GROUP_COUNT_ENR_KEY, &custody_group_count); builder.add_value(NEXT_FORK_DIGEST_ENR_KEY, &next_fork_digest); } @@ -373,7 +364,7 @@ mod test { fn build_enr_with_config( config: NetworkConfig, - cgc: Option, + cgc: u64, spec: &ChainSpec, ) -> (Enr, CombinedKey) { let keypair = libp2p::identity::secp256k1::Keypair::generate(); @@ -386,56 +377,23 @@ mod test { #[test] fn test_nfd_enr_encoding() { let spec = make_fulu_spec(); - let enr = build_enr_with_config(NetworkConfig::default(), None, &spec).0; + let enr = + build_enr_with_config(NetworkConfig::default(), spec.custody_requirement, &spec).0; assert_eq!(enr.next_fork_digest().unwrap(), TEST_NFD); } - #[test] - fn custody_group_count_default() { - let config = NetworkConfig { - subscribe_all_data_column_subnets: false, - ..NetworkConfig::default() - }; - let spec = make_fulu_spec(); - - let enr = build_enr_with_config(config, None, &spec).0; - - assert_eq!( - enr.custody_group_count::(&spec).unwrap(), - spec.custody_requirement, - ); - } - - #[test] - fn custody_group_count_all() { - let config = NetworkConfig { - subscribe_all_data_column_subnets: true, - ..NetworkConfig::default() - }; - let spec = make_fulu_spec(); - let enr = build_enr_with_config(config, None, &spec).0; - - assert_eq!( - enr.custody_group_count::(&spec).unwrap(), - spec.number_of_custody_groups, - ); - } - #[test] fn custody_group_value() { - let config = NetworkConfig { - subscribe_all_data_column_subnets: true, - ..NetworkConfig::default() - }; + let config = NetworkConfig::default(); let spec = make_fulu_spec(); - let enr = build_enr_with_config(config, Some(42), &spec).0; + let enr = build_enr_with_config(config, 42, &spec).0; assert_eq!(enr.custody_group_count::(&spec).unwrap(), 42); } #[test] fn test_encode_decode_eth2_enr() { - let (enr, _key) = build_enr_with_config(NetworkConfig::default(), None, &E::default_spec()); + let (enr, _key) = build_enr_with_config(NetworkConfig::default(), 4, &E::default_spec()); // Check all Eth2 Mappings are decodeable enr.eth2().unwrap(); enr.attestation_bitfield::().unwrap(); diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index 49de62546d..3589882ae9 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -1243,11 +1243,12 @@ mod tests { let config = Arc::new(config); let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair); let next_fork_digest = [0; 4]; + let custody_group_count = spec.custody_requirement; let enr: Enr = build_enr::( &enr_key, &config, &EnrForkId::default(), - None, + custody_group_count, next_fork_digest, &spec, ) @@ -1258,7 +1259,7 @@ mod tests { seq_number: 0, attnets: Default::default(), syncnets: Default::default(), - custody_group_count: spec.custody_requirement, + custody_group_count, }), vec![], false, diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index ea2c53a07f..1df17dffba 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -199,7 +199,7 @@ impl Network { local_keypair.clone(), &config, &ctx.enr_fork_id, - Some(advertised_cgc), + advertised_cgc, next_fork_digest, &ctx.chain_spec, )?; diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index 2a3571c3b7..f46eb05ceb 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -227,7 +227,6 @@ impl NetworkGlobals { TopicConfig { enable_light_client_server: self.config.enable_light_client_server, subscribe_all_subnets: self.config.subscribe_all_subnets, - subscribe_all_data_column_subnets: self.config.subscribe_all_data_column_subnets, sampling_subnets: self.sampling_subnets.read().clone(), } } diff --git a/beacon_node/lighthouse_network/src/types/topics.rs b/beacon_node/lighthouse_network/src/types/topics.rs index b22adfbc48..cfdee907b9 100644 --- a/beacon_node/lighthouse_network/src/types/topics.rs +++ b/beacon_node/lighthouse_network/src/types/topics.rs @@ -29,7 +29,6 @@ pub const LIGHT_CLIENT_OPTIMISTIC_UPDATE: &str = "light_client_optimistic_update pub struct TopicConfig { pub enable_light_client_server: bool, pub subscribe_all_subnets: bool, - pub subscribe_all_data_column_subnets: bool, pub sampling_subnets: HashSet, } @@ -80,14 +79,8 @@ pub fn core_topics_to_subscribe( } if fork_name.fulu_enabled() { - if opts.subscribe_all_data_column_subnets { - for i in 0..spec.data_column_sidecar_subnet_count { - topics.push(GossipKind::DataColumnSidecar(i.into())); - } - } else { - for subnet in &opts.sampling_subnets { - topics.push(GossipKind::DataColumnSidecar(*subnet)); - } + for subnet in &opts.sampling_subnets { + topics.push(GossipKind::DataColumnSidecar(*subnet)); } } @@ -125,7 +118,6 @@ pub fn all_topics_at_fork(fork: ForkName, spec: &ChainSpec) -> Vec(fork, &opts, spec) @@ -520,7 +512,6 @@ mod tests { TopicConfig { enable_light_client_server: false, subscribe_all_subnets: false, - subscribe_all_data_column_subnets: false, sampling_subnets: sampling_subnets.clone(), } } @@ -552,9 +543,8 @@ mod tests { #[test] fn columns_are_subscribed_in_peerdas() { let spec = get_spec(); - let s = get_sampling_subnets(); - let mut topic_config = get_topic_config(&s); - topic_config.subscribe_all_data_column_subnets = true; + let s = HashSet::from_iter([0.into()]); + let topic_config = get_topic_config(&s); assert!( core_topics_to_subscribe::(ForkName::Fulu, &topic_config, &spec) .contains(&GossipKind::DataColumnSidecar(0.into())) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 3b0e80e0b7..0f169ffaad 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -114,7 +114,6 @@ pub fn get_config( let is_semi_supernode = parse_flag(cli_args, "semi-supernode"); client_config.chain.node_custody_type = if is_supernode { - client_config.network.subscribe_all_data_column_subnets = true; NodeCustodyType::Supernode } else if is_semi_supernode { NodeCustodyType::SemiSupernode diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index ddd36e7e7a..71186904d0 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/lcli/src/generate_bootnode_enr.rs @@ -43,7 +43,7 @@ pub fn run(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str &enr_key, &config, &enr_fork_id, - None, + spec.custody_requirement, genesis_fork_digest, spec, )