diff --git a/bin/ethlambda/src/network/config_file.rs b/bin/ethlambda/src/network/config_file.rs index 789afaf91..3c5731509 100644 --- a/bin/ethlambda/src/network/config_file.rs +++ b/bin/ethlambda/src/network/config_file.rs @@ -33,14 +33,34 @@ pub(crate) struct ConfigFile { pub(crate) enum ConfigFileError { #[error("could not parse the network config: {0}")] Malformed(#[from] serde_yaml_ng::Error), + #[error( + "SECONDS_PER_SLOT ({seconds}) and SLOT_DURATION_MS ({millis}) disagree: \ + SECONDS_PER_SLOT * 1000 must equal SLOT_DURATION_MS" + )] + SlotTimeMismatch { seconds: u64, millis: u64 }, + #[error( + "SLOT_DURATION_MS is {millis}, but a beacon slot is a non-zero whole number of seconds" + )] + SlotDurationNotWholeSeconds { millis: u64 }, + #[error( + "SECONDS_PER_SLOT is {seconds}, too large for its SLOT_DURATION_MS \ + (SECONDS_PER_SLOT * 1000) to fit in a u64" + )] + SlotDurationOverflow { seconds: u64 }, } impl ConfigFile { /// Parse one `config.yaml`'s text. pub(crate) fn parse(text: &str) -> Result { - let config: Config = serde_yaml_ng::from_str(text)?; + let mut config: Config = serde_yaml_ng::from_str(text)?; let document: BTreeMap = serde_yaml_ng::from_str(text)?; + reconcile_slot_time( + &mut config, + document.contains_key("SECONDS_PER_SLOT"), + document.contains_key("SLOT_DURATION_MS"), + )?; + let claimed = config_field_names(); let ignored = document .into_keys() @@ -68,6 +88,58 @@ impl ConfigFile { } } +/// Make `seconds_per_slot` and `slot_duration_ms` agree, from whichever the +/// file actually carried. +/// +/// `Config` has a container-level serde `default`, so a missing key and a key +/// holding the default value parse identically; only the raw key set tells +/// them apart, hence the two flags. A beacon slot is a whole number of +/// seconds, and generated configs (kurtosis' ethereum-genesis-generator) +/// carry only `SLOT_DURATION_MS` since consensus-specs dropped +/// `SECONDS_PER_SLOT`, so: +/// - both present: they must agree, since either could be the stale one and +/// guessing would run every duty at the wrong time; +/// - only `SLOT_DURATION_MS`: it must be a non-zero whole number of seconds, +/// and the seconds are derived from it, rather than left at mainnet's 12; +/// - only `SECONDS_PER_SLOT`: the milliseconds are derived from it, and a +/// value whose milliseconds do not fit in a `u64` is refused rather than +/// saturated, which would leave the two fields disagreeing; +/// - neither: the defaults stand. +/// +/// Shared by a loaded directory and the built-in networks, which both parse +/// through [`ConfigFile::parse`], so one place owns the rule. A zero +/// `SECONDS_PER_SLOT` passes here (0 ms agrees with it) and is refused by the +/// directory loader. +fn reconcile_slot_time( + config: &mut Config, + has_seconds: bool, + has_millis: bool, +) -> Result<(), ConfigFileError> { + match (has_seconds, has_millis) { + (true, true) => { + let (seconds, millis) = (config.seconds_per_slot, config.slot_duration_ms); + if seconds.checked_mul(1_000) != Some(millis) { + return Err(ConfigFileError::SlotTimeMismatch { seconds, millis }); + } + } + (false, true) => { + let millis = config.slot_duration_ms; + if millis == 0 || !millis.is_multiple_of(1_000) { + return Err(ConfigFileError::SlotDurationNotWholeSeconds { millis }); + } + config.seconds_per_slot = millis / 1_000; + } + (true, false) => { + let seconds = config.seconds_per_slot; + config.slot_duration_ms = seconds + .checked_mul(1_000) + .ok_or(ConfigFileError::SlotDurationOverflow { seconds })?; + } + (false, false) => {} + } + Ok(()) +} + /// The keys [`Config`]'s derived `Deserialize` accepts. /// /// serde's derive hands its field list to `Deserializer::deserialize_struct` @@ -129,6 +201,40 @@ mod tests { const DEVNET: &str = include_str!("../../tests/fixtures/networks/devnet/config.yaml"); const MAINNET: &str = include_str!("../../assets/mainnet/config.yaml"); + #[test] + fn slot_time_keys_are_reconciled_whichever_are_present() { + let both_agree = + ConfigFile::parse("SECONDS_PER_SLOT: 12\nSLOT_DURATION_MS: 12000").unwrap(); + assert_eq!(both_agree.config.slot_duration_ms, 12_000); + + let err = ConfigFile::parse("SECONDS_PER_SLOT: 12\nSLOT_DURATION_MS: 6000").unwrap_err(); + assert!( + matches!(err, ConfigFileError::SlotTimeMismatch { .. }), + "got {err}" + ); + + let secs_only = ConfigFile::parse("SECONDS_PER_SLOT: 6").unwrap(); + assert_eq!(secs_only.config.slot_duration_ms, 6_000); + + let millis_only = ConfigFile::parse("SLOT_DURATION_MS: 6000").unwrap(); + assert_eq!(millis_only.config.seconds_per_slot, 6); + } + + #[test] + fn seconds_only_is_refused_once_its_milliseconds_overflow() { + let largest = u64::MAX / 1_000; + let at_limit = ConfigFile::parse(&format!("SECONDS_PER_SLOT: {largest}")).unwrap(); + assert_eq!(at_limit.config.seconds_per_slot, largest); + assert_eq!(at_limit.config.slot_duration_ms, largest * 1_000); + + let past_limit = largest + 1; + let err = ConfigFile::parse(&format!("SECONDS_PER_SLOT: {past_limit}")).unwrap_err(); + assert!( + matches!(err, ConfigFileError::SlotDurationOverflow { seconds } if seconds == past_limit), + "got {err}" + ); + } + #[test] fn the_devnets_values_are_read() { let parsed = ConfigFile::parse(DEVNET).unwrap(); diff --git a/bin/ethlambda/src/network/dir.rs b/bin/ethlambda/src/network/dir.rs index 6bde684cc..05a4d0243 100644 --- a/bin/ethlambda/src/network/dir.rs +++ b/bin/ethlambda/src/network/dir.rs @@ -125,7 +125,7 @@ impl NetworkDir { path: config_path.clone(), }); } - // Two fields cannot come from the file and must be reconciled here, + // `genesis_time` cannot come from the file and must be set here, // once, before anything reads the config. let mut config = parsed.config; super::derive_genesis_fields(&mut config, genesis_state.genesis_time()); @@ -232,7 +232,7 @@ mod tests { } #[test] - fn the_two_derived_fields_are_reconciled_against_the_genesis_state() { + fn genesis_time_comes_from_the_genesis_state() { // The genesis state here MUST carry a genesis_time different from // `Config::mainnet().genesis_time`. `genesis_time` is `#[serde(skip)]` // under a container-level `default`, so an unreconciled config inherits @@ -252,10 +252,7 @@ mod tests { "the test is only meaningful if the state differs from the default" ); - // The devnet fixture deliberately carries an inconsistent pair: - // SECONDS_PER_SLOT is 6 while SLOT_DURATION_MS is still mainnet's - // 12000. The seconds field is authoritative on a beacon chain, so the - // derivation has to win over what the file says. + // The devnet fixture's two slot-time keys agree, so both survive. assert_eq!(loaded.config.seconds_per_slot, 6); assert_eq!(loaded.config.slot_duration_ms, 6_000); } @@ -272,7 +269,11 @@ mod tests { fn a_zero_seconds_per_slot_is_rejected_naming_the_field() { let dir = complete_dir(); let devnet_text = std::fs::read_to_string(fixture("devnet").join("config.yaml")).unwrap(); - let zeroed_text = devnet_text.replacen("SECONDS_PER_SLOT: 6", "SECONDS_PER_SLOT: 0", 1); + // SLOT_DURATION_MS is zeroed too, so the pair agrees and only the + // zero itself is wrong. + let zeroed_text = devnet_text + .replacen("SECONDS_PER_SLOT: 6", "SECONDS_PER_SLOT: 0", 1) + .replacen("SLOT_DURATION_MS: 6000", "SLOT_DURATION_MS: 0", 1); assert_ne!( zeroed_text, devnet_text, "fixture no longer carries SECONDS_PER_SLOT in the expected form" @@ -280,7 +281,17 @@ mod tests { std::fs::write(dir.path().join("config.yaml"), zeroed_text).unwrap(); let err = NetworkDir::load(dir.path()).unwrap_err().to_string(); - assert!(err.contains("SECONDS_PER_SLOT"), "got {err}"); + assert!(err.contains("SECONDS_PER_SLOT is 0"), "got {err}"); + } + + #[test] + fn disagreeing_slot_keys_are_rejected_naming_both_keys_and_values() { + let dir = complete_dir(); + replace_config_line(&dir, "SLOT_DURATION_MS: 6000", "SLOT_DURATION_MS: 12000"); + let err = format!("{:#}", NetworkDir::load(dir.path()).unwrap_err()); + for needle in ["SECONDS_PER_SLOT (6)", "SLOT_DURATION_MS (12000)"] { + assert!(err.contains(needle), "got {err}"); + } } /// Write `dir`'s `config.yaml` as the devnet fixture's with one line @@ -292,6 +303,48 @@ mod tests { std::fs::write(dir.path().join("config.yaml"), text).unwrap(); } + #[test] + fn a_config_with_only_slot_duration_ms_is_read_from_it() { + // Generated configs (kurtosis) carry SLOT_DURATION_MS and no + // SECONDS_PER_SLOT; the serde default would otherwise give 12 s. + let dir = complete_dir(); + replace_config_line(&dir, "SECONDS_PER_SLOT: 6\n", ""); + let loaded = NetworkDir::load(dir.path()).unwrap(); + assert_eq!(loaded.config.seconds_per_slot, 6); + assert_eq!(loaded.config.slot_duration_ms, 6_000); + } + + #[test] + fn a_slot_duration_ms_that_is_not_whole_seconds_is_rejected_naming_the_field() { + for bad in ["6500", "0"] { + let dir = complete_dir(); + replace_config_line(&dir, "SECONDS_PER_SLOT: 6\n", ""); + replace_slot_duration_ms(&dir, &format!("SLOT_DURATION_MS: {bad}")); + let err = NetworkDir::load(dir.path()).unwrap_err().to_string(); + assert!(err.contains("SLOT_DURATION_MS"), "got {err}"); + } + } + + #[test] + fn a_config_with_neither_slot_key_keeps_the_mainnet_default() { + let dir = complete_dir(); + replace_config_line(&dir, "SECONDS_PER_SLOT: 6\n", ""); + replace_slot_duration_ms(&dir, ""); + let loaded = NetworkDir::load(dir.path()).unwrap(); + assert_eq!(loaded.config.seconds_per_slot, 12); + assert_eq!(loaded.config.slot_duration_ms, 12_000); + } + + /// Replace the `SLOT_DURATION_MS` line of the `config.yaml` already + /// written to `dir` (`replace_config_line` reads the pristine fixture). + fn replace_slot_duration_ms(dir: &tempfile::TempDir, to: &str) { + let path = dir.path().join("config.yaml"); + let text = std::fs::read_to_string(&path).unwrap(); + let replaced = text.replacen("SLOT_DURATION_MS: 6000", to, 1); + assert_ne!(replaced, text, "config no longer carries SLOT_DURATION_MS"); + std::fs::write(path, replaced).unwrap(); + } + /// A directory built for the other preset must fail naming the preset, /// not as an SSZ error from decoding its genesis state against this /// build's container bounds. diff --git a/bin/ethlambda/src/network/mod.rs b/bin/ethlambda/src/network/mod.rs index 90d4c8d49..8ff79eaf9 100644 --- a/bin/ethlambda/src/network/mod.rs +++ b/bin/ethlambda/src/network/mod.rs @@ -211,7 +211,7 @@ pub(crate) fn check_constants(config: &Config) -> Result<(), ConstantsMismatch> } } -/// Fill in the two [`Config`] fields a `config.yaml` cannot be trusted for. +/// Fill in the [`Config`] field a `config.yaml` cannot be trusted for. /// /// Shared by a loaded directory and the built-in networks, whose configs both /// come out of [`config_file::ConfigFile::parse`]. @@ -222,15 +222,11 @@ pub(crate) fn check_constants(config: &Config) -> Result<(), ConstantsMismatch> /// before its actual genesis, so reading it from the file would put every slot /// boundary off by that much. /// -/// `slot_duration_ms` is always derived, never read from the file. A beacon -/// chain's slots are a whole number of seconds, so `SECONDS_PER_SLOT` is -/// authoritative and the millisecond field exists for lean's sub-second -/// cadence. A config carrying `SECONDS_PER_SLOT: 6` and no `SLOT_DURATION_MS` -/// would otherwise keep mainnet's 12000 by default, and every duty would fire -/// at the wrong time while the second-resolution field looked correct. +/// The slot-time fields are not touched here: they are reconciled once, at +/// parse time (see `config_file::reconcile_slot_time`), so built-in and +/// loaded networks share the rule. fn derive_genesis_fields(config: &mut Config, genesis_time: u64) { config.genesis_time = genesis_time; - config.slot_duration_ms = config.seconds_per_slot * 1_000; } /// Warn at startup when `config` schedules a fork this build does not follow diff --git a/bin/ethlambda/tests/fixtures/networks/devnet-electra/config.yaml b/bin/ethlambda/tests/fixtures/networks/devnet-electra/config.yaml index c71f28a00..d9bb307b1 100644 --- a/bin/ethlambda/tests/fixtures/networks/devnet-electra/config.yaml +++ b/bin/ethlambda/tests/fixtures/networks/devnet-electra/config.yaml @@ -70,7 +70,7 @@ FULU_FORK_EPOCH: 411392 # December 3, 2025, 09:49:11pm UTC # 12 seconds (*deprecated*) SECONDS_PER_SLOT: 6 # 12000 milliseconds -SLOT_DURATION_MS: 12000 +SLOT_DURATION_MS: 6000 # 14 (estimate from Eth1 mainnet) SECONDS_PER_ETH1_BLOCK: 14 # 2**8 (= 256) epochs diff --git a/bin/ethlambda/tests/fixtures/networks/devnet/config.yaml b/bin/ethlambda/tests/fixtures/networks/devnet/config.yaml index e56bdf93c..5e491f0cb 100644 --- a/bin/ethlambda/tests/fixtures/networks/devnet/config.yaml +++ b/bin/ethlambda/tests/fixtures/networks/devnet/config.yaml @@ -64,7 +64,7 @@ FULU_FORK_EPOCH: 411392 # December 3, 2025, 09:49:11pm UTC # 12 seconds (*deprecated*) SECONDS_PER_SLOT: 6 # 12000 milliseconds -SLOT_DURATION_MS: 12000 +SLOT_DURATION_MS: 6000 # 14 (estimate from Eth1 mainnet) SECONDS_PER_ETH1_BLOCK: 14 # 2**8 (= 256) epochs