fix to validate fork parameters in ForkParameters::new()

Signed-off-by: Jun Kimura <[email protected]>

crates/consensus/src/compute.rs

@@ -96,7 +96,6 @@ mod tests {
     #[test]
     fn test_compute_timestamp_at_slot() {
         let ctx = DefaultChainContext::new_with_config(1729846322.into(), get_minimal_config());
-        assert!(ctx.validate().is_ok(), "context is invalid");
         assert_eq!(compute_timestamp_at_slot(&ctx, 0.into()), 1729846322.into());
         assert_eq!(compute_timestamp_at_slot(&ctx, 1.into()), 1729846328.into());
         assert_eq!(compute_timestamp_at_slot(&ctx, 2.into()), 1729846334.into());
@@ -109,7 +108,7 @@ mod tests {
     fn get_minimal_config() -> Config {
         Config {
             preset: preset::minimal::PRESET,
-            fork_parameters: ForkParameters::new(Version([0, 0, 0, 1]), vec![]),
+            fork_parameters: ForkParameters::new(Version([0, 0, 0, 1]), vec![]).unwrap(),
             min_genesis_time: U64(1578009600),
         }
     }

crates/consensus/src/config/goerli.rs

@@ -19,7 +19,8 @@ pub fn get_config() -> Config {
                 ForkParameter::new(Version([4, 0, 16, 32]), U64(231680)),
                 ForkParameter::new(Version([5, 0, 16, 32]), U64(u64::MAX)),
             ],
-        ),
+        )
+        .unwrap(),
         min_genesis_time: U64(1614588812),
     }
 }
@@ -30,7 +31,6 @@ mod tests {
 
     #[test]
     fn test_config_validation() {
-        let config = get_config();
-        assert!(config.fork_parameters.validate().is_ok());
+        let _ = get_config();
     }
 }

crates/consensus/src/config/mainnet.rs

@@ -19,7 +19,8 @@ pub fn get_config() -> Config {
                 ForkParameter::new(Version([4, 0, 0, 0]), U64(269568)),
                 ForkParameter::new(Version([5, 0, 0, 0]), U64(u64::MAX)),
             ],
-        ),
+        )
+        .unwrap(),
         min_genesis_time: U64(1606824000),
     }
 }
@@ -30,7 +31,6 @@ mod tests {
 
     #[test]
     fn test_config_validation() {
-        let config = get_config();
-        assert!(config.fork_parameters.validate().is_ok());
+        let _ = get_config();
     }
 }

crates/consensus/src/config/minimal.rs

@@ -22,7 +22,8 @@ pub fn get_config() -> Config {
                 // deneb
                 ForkParameter::new(Version([4, 0, 0, 1]), U64(0)),
             ],
-        ),
+        )
+        .unwrap(),
         min_genesis_time: U64(1578009600),
     }
 }
@@ -33,7 +34,6 @@ mod tests {
 
     #[test]
     fn test_config_validation() {
-        let config = get_config();
-        assert!(config.fork_parameters.validate().is_ok());
+        let _ = get_config();
     }
 }

crates/consensus/src/config/sepolia.rs

@@ -19,7 +19,8 @@ pub fn get_config() -> Config {
                 ForkParameter::new(Version([144, 0, 0, 115]), U64(132608)),
                 ForkParameter::new(Version([144, 0, 0, 116]), U64(u64::MAX)),
             ],
-        ),
+        )
+        .unwrap(),
         min_genesis_time: U64(1655647200),
     }
 }
@@ -30,7 +31,6 @@ mod tests {
 
     #[test]
     fn test_config_validation() {
-        let config = get_config();
-        assert!(config.fork_parameters.validate().is_ok());
+        let _ = get_config();
     }
 }

crates/consensus/src/context.rs

@@ -1,7 +1,6 @@
 use crate::{
     beacon::{Epoch, Slot},
     config::Config,
-    errors::Error,
     fork::ForkParameters,
     types::U64,
 };
@@ -48,11 +47,6 @@ impl DefaultChainContext {
             config.preset.EPOCHS_PER_SYNC_COMMITTEE_PERIOD,
         )
     }
-
-    pub fn validate(&self) -> Result<(), Error> {
-        self.fork_parameters.validate()?;
-        Ok(())
-    }
 }
 
 impl ChainContext for DefaultChainContext {

crates/consensus/src/fork.rs

@@ -28,22 +28,27 @@ impl Fork {
     }
 }
 
+/// Fork parameters for the beacon chain
 #[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct ForkParameters {
     genesis_version: Version,
-    /// Forks in order of increasing epoch
+    /// Forks in order of ascending epoch
+    /// The first element is the first fork after genesis
+    /// i.e., [Altair, Bellatrix, Capella, Deneb, ...]
     forks: Vec<ForkParameter>,
 }
 
 impl ForkParameters {
-    pub const fn new(genesis_version: Version, forks: Vec<ForkParameter>) -> Self {
-        Self {
+    pub fn new(genesis_version: Version, forks: Vec<ForkParameter>) -> Result<Self, Error> {
+        let this = Self {
             genesis_version,
             forks,
-        }
+        };
+        this.validate()?;
+        Ok(this)
     }
 
-    pub fn validate(&self) -> Result<(), Error> {
+    fn validate(&self) -> Result<(), Error> {
         if self.forks.windows(2).all(|f| f[0].epoch <= f[1].epoch) {
             Ok(())
         } else {
@@ -59,6 +64,11 @@ impl ForkParameters {
         &self.genesis_version
     }
 
+    pub fn forks(&self) -> &[ForkParameter] {
+        &self.forks
+    }
+
+    /// Compute the fork version for the given epoch
     pub fn compute_fork_version(&self, epoch: Epoch) -> Result<&Version, Error> {
         for fork in self.forks.iter().rev() {
             if epoch >= fork.epoch {
@@ -68,6 +78,9 @@ impl ForkParameters {
         Ok(&self.genesis_version)
     }
 
+    /// Compute the fork for the given epoch
+    ///
+    /// If `forks` does not contain a fork for the given epoch, it returns an error.
     pub fn compute_fork(&self, epoch: Epoch) -> Result<Fork, Error> {
         for (i, fork) in self.forks.iter().enumerate().rev() {
             if epoch >= fork.epoch {
@@ -85,6 +98,8 @@ impl ForkParameters {
     }
 }
 
+/// Fork parameters for each fork
+/// In the mainnet, you can find the parameters here: https://github.com/ethereum/consensus-specs/blob/9849fb39e75e6228ebd610ef0ad22f5b41543cd5/configs/mainnet.yaml#L35
 #[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct ForkParameter {
     pub version: Version,
@@ -96,3 +111,99 @@ impl ForkParameter {
         Self { version, epoch }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    pub fn test_fork_parameters() {
+        let res = ForkParameters::new(
+            Version([0, 0, 0, 1]),
+            vec![
+                ForkParameter::new(Version([1, 0, 0, 1]), U64(0)),
+                ForkParameter::new(Version([2, 0, 0, 1]), U64(0)),
+                ForkParameter::new(Version([3, 0, 0, 1]), U64(0)),
+                ForkParameter::new(Version([4, 0, 0, 1]), U64(0)),
+            ],
+        );
+        assert!(res.is_ok());
+        let params = res.unwrap();
+        let res = params.compute_fork(0.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Deneb(ForkParameter::new(Version([4, 0, 0, 1]), U64(0)))
+        );
+
+        let res = ForkParameters::new(Version([0, 0, 0, 1]), vec![]);
+        assert!(res.is_ok());
+        let params = res.unwrap();
+        let res = params.compute_fork(0.into());
+        assert!(res.is_ok());
+        assert_eq!(res.unwrap(), Fork::Genesis(Version([0, 0, 0, 1])));
+
+        let res = ForkParameters::new(
+            Version([0, 0, 0, 1]),
+            vec![ForkParameter::new(Version([1, 0, 0, 1]), U64(0))],
+        );
+        let params = res.unwrap();
+        let res = params.compute_fork(0.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Altair(ForkParameter::new(Version([1, 0, 0, 1]), U64(0)))
+        );
+
+        let res = ForkParameters::new(
+            Version([0, 0, 0, 1]),
+            vec![
+                ForkParameter::new(Version([1, 0, 0, 1]), U64(0)),
+                ForkParameter::new(Version([2, 0, 0, 1]), U64(1)),
+                ForkParameter::new(Version([3, 0, 0, 1]), U64(2)),
+                ForkParameter::new(Version([4, 0, 0, 1]), U64(3)),
+            ],
+        );
+        assert!(res.is_ok());
+        let params = res.unwrap();
+        let res = params.compute_fork(0.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Altair(ForkParameter::new(Version([1, 0, 0, 1]), U64(0)))
+        );
+        let res = params.compute_fork(1.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Bellatrix(ForkParameter::new(Version([2, 0, 0, 1]), U64(1)))
+        );
+        let res = params.compute_fork(2.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Capella(ForkParameter::new(Version([3, 0, 0, 1]), U64(2)))
+        );
+        let res = params.compute_fork(3.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Deneb(ForkParameter::new(Version([4, 0, 0, 1]), U64(3)))
+        );
+        let res = params.compute_fork(4.into());
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Fork::Deneb(ForkParameter::new(Version([4, 0, 0, 1]), U64(3)))
+        );
+
+        let res = ForkParameters::new(
+            Version([0, 0, 0, 1]),
+            vec![
+                ForkParameter::new(Version([2, 0, 0, 1]), U64(1)),
+                ForkParameter::new(Version([1, 0, 0, 1]), U64(0)),
+            ],
+        );
+        assert!(res.is_err());
+    }
+}

crates/consensus/src/fork/bellatrix.rs

@@ -495,7 +495,6 @@ mod tests {
         // ensure that signing_root calculation is correct
 
         let ctx = DefaultChainContext::new_with_config(0.into(), config::mainnet::get_config());
-        assert!(ctx.validate().is_ok(), "context is invalid");
         let fork_version =
             compute_fork_version(&ctx, compute_epoch_at_slot(&ctx, update.signature_slot)).unwrap();
         let domain = compute_domain(

crates/light-client-verifier/src/consensus.rs

@@ -484,7 +484,6 @@ mod tests_bellatrix {
             Fraction::new(2, 3),
             1729846322.into(),
         );
-        assert!(ctx.validate().is_ok(), "context is invalid");
 
         let updates = [
             "light_client_update_period_5.json",
@@ -540,7 +539,8 @@ mod tests_bellatrix {
                     ForkParameter::new(Version([1, 0, 0, 1]), U64(0)),
                     ForkParameter::new(Version([2, 0, 0, 1]), U64(0)),
                 ],
-            ),
+            )
+            .unwrap(),
             min_genesis_time: U64(1578009600),
         }
     }

crates/light-client-verifier/src/context.rs

@@ -108,11 +108,6 @@ impl LightClientContext {
             current_timestamp,
         )
     }
-
-    pub fn validate(&self) -> Result<(), Error> {
-        self.fork_parameters.validate()?;
-        Ok(())
-    }
 }
 
 impl ConsensusVerificationContext for LightClientContext {