From 77053ff28c818d2ae26e35e795edd6a0cacfced3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:47:38 -0300 Subject: [PATCH 1/3] fix(beacon): read SLOT_DURATION_MS when a network config omits SECONDS_PER_SLOT Configs generated by kurtosis' ethereum-genesis-generator carry only SLOT_DURATION_MS, since consensus-specs dropped SECONDS_PER_SLOT. Config's container-level serde default filled in mainnet's 12 seconds and derive_genesis_fields then overwrote the file's millisecond value, so a 6 s devnet silently ran on a 12 s clock. ConfigFile now records whether the file had SLOT_DURATION_MS and no SECONDS_PER_SLOT. In that case the millisecond value is authoritative and seconds_per_slot is derived from it; a value that is zero or not a whole number of seconds is a startup error naming SLOT_DURATION_MS. A file with SECONDS_PER_SLOT, and every built-in network, behaves as before. --- bin/ethlambda/src/network/built_in.rs | 3 +- bin/ethlambda/src/network/config_file.rs | 15 +++++- bin/ethlambda/src/network/dir.rs | 64 +++++++++++++++++++++++- bin/ethlambda/src/network/mod.rs | 26 +++++++--- 4 files changed, 97 insertions(+), 11 deletions(-) diff --git a/bin/ethlambda/src/network/built_in.rs b/bin/ethlambda/src/network/built_in.rs index 1ff084d36..e02ddcbf9 100644 --- a/bin/ethlambda/src/network/built_in.rs +++ b/bin/ethlambda/src/network/built_in.rs @@ -68,8 +68,9 @@ impl BuiltInNetwork { parsed.warn_about_ignored_keys(); let genesis = chain.genesis(); + let slot_duration_only = parsed.slot_duration_only; let mut config = parsed.config; - super::derive_genesis_fields(&mut config, genesis.genesis_time); + super::derive_genesis_fields(&mut config, genesis.genesis_time, slot_duration_only); Ok(BuiltIn { config, diff --git a/bin/ethlambda/src/network/config_file.rs b/bin/ethlambda/src/network/config_file.rs index 93a1e914b..7b67ffc83 100644 --- a/bin/ethlambda/src/network/config_file.rs +++ b/bin/ethlambda/src/network/config_file.rs @@ -27,6 +27,12 @@ pub(crate) struct ConfigFile { pub(crate) config: Config, /// Keys no field claimed: a typo, or a fork this build cannot process. pub(crate) ignored: Vec, + /// The file carries `SLOT_DURATION_MS` and not `SECONDS_PER_SLOT`. + /// + /// `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 + /// can tell them apart. + pub(crate) slot_duration_only: bool, } #[derive(Debug, thiserror::Error)] @@ -41,13 +47,20 @@ impl ConfigFile { let config: Config = serde_yaml_ng::from_str(text)?; let document: BTreeMap = serde_yaml_ng::from_str(text)?; + let slot_duration_only = + document.contains_key("SLOT_DURATION_MS") && !document.contains_key("SECONDS_PER_SLOT"); + let claimed = config_field_names(); let ignored = document .into_keys() .filter(|key| !claimed.contains(&key.as_str())) .collect(); - Ok(Self { config, ignored }) + Ok(Self { + config, + ignored, + slot_duration_only, + }) } /// Log what was ignored, as one line naming each key. diff --git a/bin/ethlambda/src/network/dir.rs b/bin/ethlambda/src/network/dir.rs index 17306c21f..1f037310d 100644 --- a/bin/ethlambda/src/network/dir.rs +++ b/bin/ethlambda/src/network/dir.rs @@ -70,6 +70,11 @@ pub(crate) enum NetworkDirError { path.display() )] ZeroSecondsPerSlot { path: PathBuf }, + #[error( + "{}: SLOT_DURATION_MS is {value}, but a beacon slot is a non-zero whole number of seconds", + path.display() + )] + InvalidSlotDurationMs { path: PathBuf, value: u64 }, } impl NetworkDir { @@ -125,10 +130,24 @@ impl NetworkDir { path: config_path.clone(), }); } + // Only the millisecond key was given (generated configs), so it is + // the one to hold to a whole number of seconds. + let slot_duration_ms = parsed.config.slot_duration_ms; + if parsed.slot_duration_only && (slot_duration_ms == 0 || slot_duration_ms % 1_000 != 0) { + return Err(NetworkDirError::InvalidSlotDurationMs { + path: config_path.clone(), + value: slot_duration_ms, + }); + } // Two fields cannot come from the file and must be reconciled here, // once, before anything reads the config. + let slot_duration_only = parsed.slot_duration_only; let mut config = parsed.config; - super::derive_genesis_fields(&mut config, genesis_state.genesis_time()); + super::derive_genesis_fields( + &mut config, + genesis_state.genesis_time(), + slot_duration_only, + ); Ok(Self { config, @@ -291,6 +310,49 @@ 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", ""); + replace_slot_duration_ms(&dir, "SLOT_DURATION_MS: 6000"); + 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: 12000", 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 10875fea9..f29a11d0f 100644 --- a/bin/ethlambda/src/network/mod.rs +++ b/bin/ethlambda/src/network/mod.rs @@ -221,15 +221,25 @@ 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. -fn derive_genesis_fields(config: &mut Config, genesis_time: u64) { +/// The two slot-time fields are reconciled, not read as a pair. A beacon +/// chain's slots are a whole number of seconds, so when the file has +/// `SECONDS_PER_SLOT` it is authoritative and `slot_duration_ms` is derived +/// from it; otherwise a config carrying `SECONDS_PER_SLOT: 6` and a stale +/// `SLOT_DURATION_MS` would run duties at the wrong time while the +/// second-resolution field looked correct. Generated configs (kurtosis' +/// ethereum-genesis-generator) are the exception: consensus-specs dropped +/// `SECONDS_PER_SLOT`, so they carry only `SLOT_DURATION_MS`, and `Config`'s +/// serde default would silently fill in mainnet's 12 seconds. With +/// `slot_duration_only` set, `SLOT_DURATION_MS` is authoritative and +/// `seconds_per_slot` is derived from it. The caller must have checked it is +/// a non-zero multiple of 1000. +fn derive_genesis_fields(config: &mut Config, genesis_time: u64, slot_duration_only: bool) { config.genesis_time = genesis_time; - config.slot_duration_ms = config.seconds_per_slot * 1_000; + if slot_duration_only { + config.seconds_per_slot = config.slot_duration_ms / 1_000; + } else { + config.slot_duration_ms = config.seconds_per_slot * 1_000; + } } /// A resolved network: everything startup needs before it can build a swarm. From ed827df2fba3002d2677c62e92c9dd391182c0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:56:12 -0300 Subject: [PATCH 2/3] fix(beacon): refuse a config whose SECONDS_PER_SLOT and SLOT_DURATION_MS disagree When both keys are present either could be the stale one, and letting SECONDS_PER_SLOT win hid the mistake. They must now agree, otherwise startup fails naming both keys and both values. The rule moves into ConfigFile::parse (reconcile_slot_time), so built-in networks and loaded directories share it instead of dir.rs owning part of it. The devnet fixtures carried a deliberate 6 / 12000 pair to prove seconds won; they now carry 6 / 6000. --- bin/ethlambda/src/network/built_in.rs | 3 +- bin/ethlambda/src/network/config_file.rs | 96 ++++++++++++++++--- bin/ethlambda/src/network/dir.rs | 51 ++++------ bin/ethlambda/src/network/mod.rs | 24 +---- .../networks/devnet-electra/config.yaml | 2 +- .../fixtures/networks/devnet/config.yaml | 2 +- 6 files changed, 111 insertions(+), 67 deletions(-) diff --git a/bin/ethlambda/src/network/built_in.rs b/bin/ethlambda/src/network/built_in.rs index e02ddcbf9..1ff084d36 100644 --- a/bin/ethlambda/src/network/built_in.rs +++ b/bin/ethlambda/src/network/built_in.rs @@ -68,9 +68,8 @@ impl BuiltInNetwork { parsed.warn_about_ignored_keys(); let genesis = chain.genesis(); - let slot_duration_only = parsed.slot_duration_only; let mut config = parsed.config; - super::derive_genesis_fields(&mut config, genesis.genesis_time, slot_duration_only); + super::derive_genesis_fields(&mut config, genesis.genesis_time); Ok(BuiltIn { config, diff --git a/bin/ethlambda/src/network/config_file.rs b/bin/ethlambda/src/network/config_file.rs index 7b67ffc83..e5ed32ba9 100644 --- a/bin/ethlambda/src/network/config_file.rs +++ b/bin/ethlambda/src/network/config_file.rs @@ -27,28 +27,34 @@ pub(crate) struct ConfigFile { pub(crate) config: Config, /// Keys no field claimed: a typo, or a fork this build cannot process. pub(crate) ignored: Vec, - /// The file carries `SLOT_DURATION_MS` and not `SECONDS_PER_SLOT`. - /// - /// `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 - /// can tell them apart. - pub(crate) slot_duration_only: bool, } #[derive(Debug, thiserror::Error)] 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 }, } 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)?; - let slot_duration_only = - document.contains_key("SLOT_DURATION_MS") && !document.contains_key("SECONDS_PER_SLOT"); + 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 @@ -56,11 +62,7 @@ impl ConfigFile { .filter(|key| !claimed.contains(&key.as_str())) .collect(); - Ok(Self { - config, - ignored, - slot_duration_only, - }) + Ok(Self { config, ignored }) } /// Log what was ignored, as one line naming each key. @@ -81,6 +83,53 @@ 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; +/// - 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) => { + config.slot_duration_ms = config.seconds_per_slot.saturating_mul(1_000); + } + (false, false) => {} + } + Ok(()) +} + /// The keys [`Config`]'s derived `Deserialize` accepts. /// /// serde's derive hands its field list to `Deserializer::deserialize_struct` @@ -142,6 +191,25 @@ 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 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 1f037310d..633a1be56 100644 --- a/bin/ethlambda/src/network/dir.rs +++ b/bin/ethlambda/src/network/dir.rs @@ -70,11 +70,6 @@ pub(crate) enum NetworkDirError { path.display() )] ZeroSecondsPerSlot { path: PathBuf }, - #[error( - "{}: SLOT_DURATION_MS is {value}, but a beacon slot is a non-zero whole number of seconds", - path.display() - )] - InvalidSlotDurationMs { path: PathBuf, value: u64 }, } impl NetworkDir { @@ -130,24 +125,10 @@ impl NetworkDir { path: config_path.clone(), }); } - // Only the millisecond key was given (generated configs), so it is - // the one to hold to a whole number of seconds. - let slot_duration_ms = parsed.config.slot_duration_ms; - if parsed.slot_duration_only && (slot_duration_ms == 0 || slot_duration_ms % 1_000 != 0) { - return Err(NetworkDirError::InvalidSlotDurationMs { - path: config_path.clone(), - value: slot_duration_ms, - }); - } - // 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 slot_duration_only = parsed.slot_duration_only; let mut config = parsed.config; - super::derive_genesis_fields( - &mut config, - genesis_state.genesis_time(), - slot_duration_only, - ); + super::derive_genesis_fields(&mut config, genesis_state.genesis_time()); Ok(Self { config, @@ -250,7 +231,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 @@ -270,10 +251,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); } @@ -290,7 +268,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" @@ -298,7 +280,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 @@ -316,7 +308,6 @@ mod tests { // SECONDS_PER_SLOT; the serde default would otherwise give 12 s. let dir = complete_dir(); replace_config_line(&dir, "SECONDS_PER_SLOT: 6\n", ""); - replace_slot_duration_ms(&dir, "SLOT_DURATION_MS: 6000"); let loaded = NetworkDir::load(dir.path()).unwrap(); assert_eq!(loaded.config.seconds_per_slot, 6); assert_eq!(loaded.config.slot_duration_ms, 6_000); @@ -348,7 +339,7 @@ mod tests { 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: 12000", to, 1); + 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(); } diff --git a/bin/ethlambda/src/network/mod.rs b/bin/ethlambda/src/network/mod.rs index f29a11d0f..54b925184 100644 --- a/bin/ethlambda/src/network/mod.rs +++ b/bin/ethlambda/src/network/mod.rs @@ -210,7 +210,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`]. @@ -221,25 +221,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. /// -/// The two slot-time fields are reconciled, not read as a pair. A beacon -/// chain's slots are a whole number of seconds, so when the file has -/// `SECONDS_PER_SLOT` it is authoritative and `slot_duration_ms` is derived -/// from it; otherwise a config carrying `SECONDS_PER_SLOT: 6` and a stale -/// `SLOT_DURATION_MS` would run duties at the wrong time while the -/// second-resolution field looked correct. Generated configs (kurtosis' -/// ethereum-genesis-generator) are the exception: consensus-specs dropped -/// `SECONDS_PER_SLOT`, so they carry only `SLOT_DURATION_MS`, and `Config`'s -/// serde default would silently fill in mainnet's 12 seconds. With -/// `slot_duration_only` set, `SLOT_DURATION_MS` is authoritative and -/// `seconds_per_slot` is derived from it. The caller must have checked it is -/// a non-zero multiple of 1000. -fn derive_genesis_fields(config: &mut Config, genesis_time: u64, slot_duration_only: bool) { +/// 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; - if slot_duration_only { - config.seconds_per_slot = config.slot_duration_ms / 1_000; - } else { - config.slot_duration_ms = config.seconds_per_slot * 1_000; - } } /// A resolved network: everything startup needs before it can build a swarm. 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 b027ee509..dff3be000 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 From b6c61e694b83f0d8778080366ae2d34eeba71d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:06:20 -0300 Subject: [PATCH 3/3] fix(beacon): refuse a SECONDS_PER_SLOT whose milliseconds overflow A config carrying only SECONDS_PER_SLOT derived SLOT_DURATION_MS with saturating_mul, so a value above u64::MAX / 1000 loaded with slot_duration_ms pinned at u64::MAX: the two fields disagreed and every timing consumer reading one rather than the other would schedule differently. The duration is not representable, so it is now a startup error naming the value, like the other slot-time mismatches. --- bin/ethlambda/src/network/config_file.rs | 29 ++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/bin/ethlambda/src/network/config_file.rs b/bin/ethlambda/src/network/config_file.rs index acc8f0a39..3c5731509 100644 --- a/bin/ethlambda/src/network/config_file.rs +++ b/bin/ethlambda/src/network/config_file.rs @@ -42,6 +42,11 @@ pub(crate) enum ConfigFileError { "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 { @@ -96,7 +101,9 @@ impl ConfigFile { /// 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; +/// - 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 @@ -123,7 +130,10 @@ fn reconcile_slot_time( config.seconds_per_slot = millis / 1_000; } (true, false) => { - config.slot_duration_ms = config.seconds_per_slot.saturating_mul(1_000); + let seconds = config.seconds_per_slot; + config.slot_duration_ms = seconds + .checked_mul(1_000) + .ok_or(ConfigFileError::SlotDurationOverflow { seconds })?; } (false, false) => {} } @@ -210,6 +220,21 @@ mod tests { 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();