From 3f06e5dfbac1e280a8411362079b9356628e5e7a Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 17 Jul 2025 23:51:11 -0500 Subject: [PATCH] Fix enr loading from disk with cgc (#7754) N/A During building an enr on startup, we weren't using the value in the custody context. This was resulting in the enr value getting updated when the cgc updates, the change getting persisted, but getting set back to the default on restart. This PR takes the value explicitly from the custody context. --- .../lighthouse_network/src/discovery/enr.rs | 56 ++++++++++++++----- .../lighthouse_network/src/discovery/mod.rs | 1 + .../lighthouse_network/src/service/mod.rs | 9 ++- lcli/src/generate_bootnode_enr.rs | 11 +++- 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index 4c05560497..053527f119 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -159,6 +159,7 @@ pub fn build_or_load_enr( local_key: Keypair, config: &NetworkConfig, enr_fork_id: &EnrForkId, + custody_group_count: Option, next_fork_digest: [u8; 4], spec: &ChainSpec, ) -> Result { @@ -166,7 +167,14 @@ pub fn build_or_load_enr( // Note: Discovery should update the ENR record's IP to the external IP as seen by the // majority of our peers, if the CLI doesn't expressly forbid it. let enr_key = CombinedKey::from_libp2p(local_key)?; - let mut local_enr = build_enr::(&enr_key, config, enr_fork_id, next_fork_digest, spec)?; + let mut local_enr = build_enr::( + &enr_key, + config, + enr_fork_id, + custody_group_count, + next_fork_digest, + spec, + )?; use_or_load_enr(&enr_key, &mut local_enr, config)?; Ok(local_enr) @@ -177,6 +185,7 @@ pub fn build_enr( enr_key: &CombinedKey, config: &NetworkConfig, enr_fork_id: &EnrForkId, + custody_group_count: Option, next_fork_digest: [u8; 4], spec: &ChainSpec, ) -> Result { @@ -271,14 +280,15 @@ 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(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 - }; + 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); } @@ -361,18 +371,22 @@ mod test { spec } - fn build_enr_with_config(config: NetworkConfig, spec: &ChainSpec) -> (Enr, CombinedKey) { + fn build_enr_with_config( + config: NetworkConfig, + cgc: Option, + spec: &ChainSpec, + ) -> (Enr, CombinedKey) { let keypair = libp2p::identity::secp256k1::Keypair::generate(); let enr_key = CombinedKey::from_secp256k1(&keypair); let enr_fork_id = EnrForkId::default(); - let enr = build_enr::(&enr_key, &config, &enr_fork_id, TEST_NFD, spec).unwrap(); + let enr = build_enr::(&enr_key, &config, &enr_fork_id, cgc, TEST_NFD, spec).unwrap(); (enr, enr_key) } #[test] fn test_nfd_enr_encoding() { let spec = make_fulu_spec(); - let enr = build_enr_with_config(NetworkConfig::default(), &spec).0; + let enr = build_enr_with_config(NetworkConfig::default(), None, &spec).0; assert_eq!(enr.next_fork_digest().unwrap(), TEST_NFD); } @@ -384,7 +398,7 @@ mod test { }; let spec = make_fulu_spec(); - let enr = build_enr_with_config(config, &spec).0; + let enr = build_enr_with_config(config, None, &spec).0; assert_eq!( enr.custody_group_count::(&spec).unwrap(), @@ -399,7 +413,7 @@ mod test { ..NetworkConfig::default() }; let spec = make_fulu_spec(); - let enr = build_enr_with_config(config, &spec).0; + let enr = build_enr_with_config(config, None, &spec).0; assert_eq!( enr.custody_group_count::(&spec).unwrap(), @@ -407,9 +421,21 @@ mod test { ); } + #[test] + fn custody_group_value() { + let config = NetworkConfig { + subscribe_all_data_column_subnets: true, + ..NetworkConfig::default() + }; + let spec = make_fulu_spec(); + let enr = build_enr_with_config(config, Some(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(), &E::default_spec()); + let (enr, _key) = build_enr_with_config(NetworkConfig::default(), None, &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 df866dfc64..bc7802ce9a 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -1235,6 +1235,7 @@ mod tests { &enr_key, &config, &EnrForkId::default(), + None, next_fork_digest, &spec, ) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index a880fdb3e7..4a6f34c76d 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -197,18 +197,21 @@ impl Network { .fork_context .next_fork_digest() .unwrap_or_else(|| ctx.fork_context.current_fork_digest()); + + let advertised_cgc = config + .advertise_false_custody_group_count + .unwrap_or(custody_group_count); let enr = crate::discovery::enr::build_or_load_enr::( local_keypair.clone(), &config, &ctx.enr_fork_id, + Some(advertised_cgc), next_fork_digest, &ctx.chain_spec, )?; // Construct the metadata - let advertised_cgc = config - .advertise_false_custody_group_count - .unwrap_or(custody_group_count); + let meta_data = utils::load_or_build_metadata(&config.network_dir, advertised_cgc); let seq_number = *meta_data.seq_number(); let globals = NetworkGlobals::new( diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index 98ef0b96d4..b2fd7e7ec7 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/lcli/src/generate_bootnode_enr.rs @@ -38,8 +38,15 @@ pub fn run(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str next_fork_version: genesis_fork_version, next_fork_epoch: Epoch::max_value(), // FAR_FUTURE_EPOCH }; - let enr = build_enr::(&enr_key, &config, &enr_fork_id, genesis_fork_digest, spec) - .map_err(|e| format!("Unable to create ENR: {:?}", e))?; + let enr = build_enr::( + &enr_key, + &config, + &enr_fork_id, + None, + genesis_fork_digest, + spec, + ) + .map_err(|e| format!("Unable to create ENR: {:?}", e))?; fs::create_dir_all(&output_dir).map_err(|e| format!("Unable to create output-dir: {:?}", e))?;