Skip to content

Commit 3f06e5d

Browse files
authored
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.
1 parent d6de8a7 commit 3f06e5d

File tree

4 files changed

+57
-20
lines changed

4 files changed

+57
-20
lines changed

beacon_node/lighthouse_network/src/discovery/enr.rs

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,22 @@ pub fn build_or_load_enr<E: EthSpec>(
159159
local_key: Keypair,
160160
config: &NetworkConfig,
161161
enr_fork_id: &EnrForkId,
162+
custody_group_count: Option<u64>,
162163
next_fork_digest: [u8; 4],
163164
spec: &ChainSpec,
164165
) -> Result<Enr, String> {
165166
// Build the local ENR.
166167
// Note: Discovery should update the ENR record's IP to the external IP as seen by the
167168
// majority of our peers, if the CLI doesn't expressly forbid it.
168169
let enr_key = CombinedKey::from_libp2p(local_key)?;
169-
let mut local_enr = build_enr::<E>(&enr_key, config, enr_fork_id, next_fork_digest, spec)?;
170+
let mut local_enr = build_enr::<E>(
171+
&enr_key,
172+
config,
173+
enr_fork_id,
174+
custody_group_count,
175+
next_fork_digest,
176+
spec,
177+
)?;
170178

171179
use_or_load_enr(&enr_key, &mut local_enr, config)?;
172180
Ok(local_enr)
@@ -177,6 +185,7 @@ pub fn build_enr<E: EthSpec>(
177185
enr_key: &CombinedKey,
178186
config: &NetworkConfig,
179187
enr_fork_id: &EnrForkId,
188+
custody_group_count: Option<u64>,
180189
next_fork_digest: [u8; 4],
181190
spec: &ChainSpec,
182191
) -> Result<Enr, String> {
@@ -271,14 +280,15 @@ pub fn build_enr<E: EthSpec>(
271280

272281
// only set `cgc` and `nfd` if PeerDAS fork (Fulu) epoch has been scheduled
273282
if spec.is_peer_das_scheduled() {
274-
let custody_group_count =
275-
if let Some(false_cgc) = config.advertise_false_custody_group_count {
276-
false_cgc
277-
} else if config.subscribe_all_data_column_subnets {
278-
spec.number_of_custody_groups
279-
} else {
280-
spec.custody_requirement
281-
};
283+
let custody_group_count = if let Some(cgc) = custody_group_count {
284+
cgc
285+
} else if let Some(false_cgc) = config.advertise_false_custody_group_count {
286+
false_cgc
287+
} else if config.subscribe_all_data_column_subnets {
288+
spec.number_of_custody_groups
289+
} else {
290+
spec.custody_requirement
291+
};
282292
builder.add_value(PEERDAS_CUSTODY_GROUP_COUNT_ENR_KEY, &custody_group_count);
283293
builder.add_value(NEXT_FORK_DIGEST_ENR_KEY, &next_fork_digest);
284294
}
@@ -361,18 +371,22 @@ mod test {
361371
spec
362372
}
363373

364-
fn build_enr_with_config(config: NetworkConfig, spec: &ChainSpec) -> (Enr, CombinedKey) {
374+
fn build_enr_with_config(
375+
config: NetworkConfig,
376+
cgc: Option<u64>,
377+
spec: &ChainSpec,
378+
) -> (Enr, CombinedKey) {
365379
let keypair = libp2p::identity::secp256k1::Keypair::generate();
366380
let enr_key = CombinedKey::from_secp256k1(&keypair);
367381
let enr_fork_id = EnrForkId::default();
368-
let enr = build_enr::<E>(&enr_key, &config, &enr_fork_id, TEST_NFD, spec).unwrap();
382+
let enr = build_enr::<E>(&enr_key, &config, &enr_fork_id, cgc, TEST_NFD, spec).unwrap();
369383
(enr, enr_key)
370384
}
371385

372386
#[test]
373387
fn test_nfd_enr_encoding() {
374388
let spec = make_fulu_spec();
375-
let enr = build_enr_with_config(NetworkConfig::default(), &spec).0;
389+
let enr = build_enr_with_config(NetworkConfig::default(), None, &spec).0;
376390
assert_eq!(enr.next_fork_digest().unwrap(), TEST_NFD);
377391
}
378392

@@ -384,7 +398,7 @@ mod test {
384398
};
385399
let spec = make_fulu_spec();
386400

387-
let enr = build_enr_with_config(config, &spec).0;
401+
let enr = build_enr_with_config(config, None, &spec).0;
388402

389403
assert_eq!(
390404
enr.custody_group_count::<E>(&spec).unwrap(),
@@ -399,17 +413,29 @@ mod test {
399413
..NetworkConfig::default()
400414
};
401415
let spec = make_fulu_spec();
402-
let enr = build_enr_with_config(config, &spec).0;
416+
let enr = build_enr_with_config(config, None, &spec).0;
403417

404418
assert_eq!(
405419
enr.custody_group_count::<E>(&spec).unwrap(),
406420
spec.number_of_custody_groups,
407421
);
408422
}
409423

424+
#[test]
425+
fn custody_group_value() {
426+
let config = NetworkConfig {
427+
subscribe_all_data_column_subnets: true,
428+
..NetworkConfig::default()
429+
};
430+
let spec = make_fulu_spec();
431+
let enr = build_enr_with_config(config, Some(42), &spec).0;
432+
433+
assert_eq!(enr.custody_group_count::<E>(&spec).unwrap(), 42);
434+
}
435+
410436
#[test]
411437
fn test_encode_decode_eth2_enr() {
412-
let (enr, _key) = build_enr_with_config(NetworkConfig::default(), &E::default_spec());
438+
let (enr, _key) = build_enr_with_config(NetworkConfig::default(), None, &E::default_spec());
413439
// Check all Eth2 Mappings are decodeable
414440
enr.eth2().unwrap();
415441
enr.attestation_bitfield::<MainnetEthSpec>().unwrap();

beacon_node/lighthouse_network/src/discovery/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,7 @@ mod tests {
12351235
&enr_key,
12361236
&config,
12371237
&EnrForkId::default(),
1238+
None,
12381239
next_fork_digest,
12391240
&spec,
12401241
)

beacon_node/lighthouse_network/src/service/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,18 +197,21 @@ impl<E: EthSpec> Network<E> {
197197
.fork_context
198198
.next_fork_digest()
199199
.unwrap_or_else(|| ctx.fork_context.current_fork_digest());
200+
201+
let advertised_cgc = config
202+
.advertise_false_custody_group_count
203+
.unwrap_or(custody_group_count);
200204
let enr = crate::discovery::enr::build_or_load_enr::<E>(
201205
local_keypair.clone(),
202206
&config,
203207
&ctx.enr_fork_id,
208+
Some(advertised_cgc),
204209
next_fork_digest,
205210
&ctx.chain_spec,
206211
)?;
207212

208213
// Construct the metadata
209-
let advertised_cgc = config
210-
.advertise_false_custody_group_count
211-
.unwrap_or(custody_group_count);
214+
212215
let meta_data = utils::load_or_build_metadata(&config.network_dir, advertised_cgc);
213216
let seq_number = *meta_data.seq_number();
214217
let globals = NetworkGlobals::new(

lcli/src/generate_bootnode_enr.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,15 @@ pub fn run<E: EthSpec>(matches: &ArgMatches, spec: &ChainSpec) -> Result<(), Str
3838
next_fork_version: genesis_fork_version,
3939
next_fork_epoch: Epoch::max_value(), // FAR_FUTURE_EPOCH
4040
};
41-
let enr = build_enr::<E>(&enr_key, &config, &enr_fork_id, genesis_fork_digest, spec)
42-
.map_err(|e| format!("Unable to create ENR: {:?}", e))?;
41+
let enr = build_enr::<E>(
42+
&enr_key,
43+
&config,
44+
&enr_fork_id,
45+
None,
46+
genesis_fork_digest,
47+
spec,
48+
)
49+
.map_err(|e| format!("Unable to create ENR: {:?}", e))?;
4350

4451
fs::create_dir_all(&output_dir).map_err(|e| format!("Unable to create output-dir: {:?}", e))?;
4552

0 commit comments

Comments
 (0)