Enr fork (#967)

* Start fixing enr-fork-id

* Fix time-until-next-fork logic

* Remove fork crate

Cargo.toml

@@ -34,7 +34,6 @@ members = [
     "beacon_node/client",
     "beacon_node/eth1",
     "beacon_node/eth2-libp2p",
-    "beacon_node/fork",
     "beacon_node/network",
     "beacon_node/rest_api",
     "beacon_node/store",

beacon_node/beacon_chain/Cargo.toml

@@ -12,7 +12,6 @@ write_ssz_files = []  # Writes debugging .ssz files to /tmp during block process
 eth2_config = { path = "../../eth2/utils/eth2_config" }
 merkle_proof = { path = "../../eth2/utils/merkle_proof" }
 store = { path = "../store" }
-fork = { path = "../fork" }
 parking_lot = "0.9.0"
 lazy_static = "1.4.0"
 lighthouse_metrics = { path = "../../eth2/utils/lighthouse_metrics" }

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -15,7 +15,6 @@ use crate::snapshot_cache::SnapshotCache;
 use crate::timeout_rw_lock::TimeoutRwLock;
 use crate::validator_pubkey_cache::ValidatorPubkeyCache;
 use crate::BeaconSnapshot;
-use ::fork::{next_fork_epoch, next_fork_version};
 use operation_pool::{OperationPool, PersistedOperationPool};
 use slog::{crit, debug, error, info, trace, warn, Logger};
 use slot_clock::SlotClock;
@@ -2095,51 +2094,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     }
 
     /// Gets the current EnrForkId.
-    ///
-    /// v0.11
-    pub fn enr_fork_id(&self) -> Result<EnrForkId, Error> {
-        Ok(EnrForkId {
-            // TODO: To be implemented with v0.11 updates
-            fork_digest: [0, 0, 0, 0],
-            next_fork_version: next_fork_version(self.slot()?, &self.disabled_forks),
-            next_fork_epoch: next_fork_epoch::<T::EthSpec>(
-                &self.spec,
-                self.slot()?,
-                &self.disabled_forks,
-            ),
-        })
+    pub fn enr_fork_id(&self) -> EnrForkId {
+        // If we are unable to read the slot clock we assume that it is prior to genesis and
+        // therefore use the genesis slot.
+        let slot = self.slot().unwrap_or_else(|_| self.spec.genesis_slot);
+        self.spec.enr_fork_id(slot)
     }
 
-    /// Calculates the duration (in millis) to the next fork, if one exists.
-    ///
-    /// This is required by the network thread to instantiate timeouts to update networking
-    /// constants
-    pub fn duration_to_next_fork(&self) -> Result<Option<tokio::timer::Delay>, Error> {
-        let current_slot = self.slot()?;
-        let next_fork_epoch =
-            next_fork_epoch::<T::EthSpec>(&self.spec, current_slot, &self.disabled_forks);
-        if next_fork_epoch != self.spec.far_future_epoch {
-            // There is an upcoming fork
-            let current_epoch = self.slot()?.epoch(T::EthSpec::slots_per_epoch());
-            let epochs_until_fork = next_fork_epoch
-                .saturating_sub(current_epoch)
-                .saturating_sub(1u64);
-            let millis_until_fork = T::EthSpec::slots_per_epoch()
-                * self.spec.milliseconds_per_slot
-                * epochs_until_fork.as_u64();
-            Ok(Some(tokio::timer::Delay::new(
-                Instant::now()
-                    + self
-                        .slot_clock
-                        .duration_to_next_epoch(T::EthSpec::slots_per_epoch())
-                        .unwrap_or_else(|| Duration::from_secs(0))
-                    + Duration::from_millis(millis_until_fork)
-                    // add a short timeout to start within the new fork period
-                    + Duration::from_millis(200),
-            )))
-        } else {
-            Ok(None)
-        }
+    /// Calculates the `Duration` to the next fork, if one exists.
+    pub fn duration_to_next_fork(&self) -> Option<Duration> {
+        let epoch = self.spec.next_fork_epoch()?;
+        self.slot_clock
+            .duration_to_slot(epoch.start_slot(T::EthSpec::slots_per_epoch()))
     }
 }
 

beacon_node/network/src/service.rs

@@ -78,14 +78,10 @@ impl<T: BeaconChainTypes> NetworkService<T> {
         let propagation_percentage = config.propagation_percentage;
 
         // build the current enr_fork_id for adding to our local ENR
-        let enr_fork_id = beacon_chain
-            .enr_fork_id()
-            .map_err(|e| format!("Could not get the current ENR fork version: {:?}", e))?;
+        let enr_fork_id = beacon_chain.enr_fork_id();
 
         // keep track of when our fork_id needs to be updated
-        let next_fork_update = beacon_chain
-            .duration_to_next_fork()
-            .map_err(|e| format!("Could not get the next fork update duration: {:?}", e))?;
+        let next_fork_update = next_fork_delay(&beacon_chain);
 
         // launch libp2p service
         let (network_globals, mut libp2p) =
@@ -357,10 +353,8 @@ fn spawn_service<T: BeaconChainTypes>(
         if let Some(mut update_fork_delay) =  service.next_fork_update.take() {
             if !update_fork_delay.is_elapsed() {
                 if let Ok(Async::Ready(_)) = update_fork_delay.poll() {
-                        if let Ok(enr_fork_id) = service.beacon_chain.enr_fork_id() {
-                            service.libp2p.swarm.update_fork_version(enr_fork_id);
-                        }
-                        service.next_fork_update = service.beacon_chain.duration_to_next_fork().unwrap_or_else(|_| None);
+                        service.libp2p.swarm.update_fork_version(service.beacon_chain.enr_fork_id());
+                        service.next_fork_update = next_fork_delay(&service.beacon_chain);
                 }
             }
         }
@@ -373,6 +367,18 @@ fn spawn_service<T: BeaconChainTypes>(
     Ok(network_exit)
 }
 
+/// Returns a `Delay` that triggers shortly after the next change in the beacon chain fork version.
+/// If there is no scheduled fork, `None` is returned.
+fn next_fork_delay<T: BeaconChainTypes>(
+    beacon_chain: &BeaconChain<T>,
+) -> Option<tokio::timer::Delay> {
+    beacon_chain.duration_to_next_fork().map(|until_fork| {
+        // Add a short time-out to start within the new fork period.
+        let delay = Duration::from_millis(200);
+        tokio::timer::Delay::new(Instant::now() + until_fork + delay)
+    })
+}
+
 /// Types of messages that the network service can receive.
 #[derive(Debug)]
 pub enum NetworkMessage<T: EthSpec> {

eth2/types/src/chain_spec.rs

@@ -3,6 +3,7 @@ use int_to_bytes::int_to_bytes4;
 use serde_derive::{Deserialize, Serialize};
 use std::fs::File;
 use std::path::Path;
+use tree_hash::TreeHash;
 use utils::{
     fork_from_hex_str, fork_to_hex_str, u32_from_hex_str, u32_to_hex_str, u8_from_hex_str,
     u8_to_hex_str,
@@ -123,6 +124,33 @@ pub struct ChainSpec {
 }
 
 impl ChainSpec {
+    /// Returns an `EnrForkId` for the given `slot`.
+    ///
+    /// Presently, we don't have any forks so we just ignore the slot. In the future this function
+    /// may return something different based upon the slot.
+    pub fn enr_fork_id(&self, _slot: Slot) -> EnrForkId {
+        // TODO: set this to something sensible once v0.11.0 is ready.
+        let genesis_validators_root = Hash256::zero();
+
+        EnrForkId {
+            fork_digest: Self::compute_fork_digest(
+                self.genesis_fork_version,
+                genesis_validators_root,
+            )
+            .to_le_bytes(),
+            next_fork_version: self.genesis_fork_version,
+            next_fork_epoch: self.far_future_epoch,
+        }
+    }
+
+    /// Returns the epoch of the next scheduled change in the `fork.current_version`.
+    ///
+    /// There are no future forks scheduled so this function always returns `None`. This may not
+    /// always be the case in the future, though.
+    pub fn next_fork_epoch(&self) -> Option<Epoch> {
+        None
+    }
+
     /// Get the domain number, unmodified by the fork.
     ///
     /// Spec v0.10.1
@@ -156,6 +184,35 @@ impl ChainSpec {
         self.compute_domain(Domain::Deposit, self.genesis_fork_version)
     }
 
+    /// Return the 32-byte fork data root for the `current_version` and `genesis_validators_root`.
+    ///
+    /// This is used primarily in signature domains to avoid collisions across forks/chains.
+    ///
+    /// Spec v0.11.0
+    pub fn compute_fork_data_root(
+        current_version: [u8; 4],
+        genesis_validators_root: Hash256,
+    ) -> Hash256 {
+        ForkData {
+            current_version,
+            genesis_validators_root,
+        }
+        .tree_hash_root()
+    }
+
+    /// Return the 4-byte fork digest for the `current_version` and `genesis_validators_root`.
+    ///
+    /// This is a digest primarily used for domain separation on the p2p layer.
+    /// 4-bytes suffices for practical separation of forks/chains.
+    pub fn compute_fork_digest(current_version: [u8; 4], genesis_validators_root: Hash256) -> u32 {
+        let fork_data_root = Self::compute_fork_data_root(current_version, genesis_validators_root);
+
+        let mut bytes = [0; 4];
+        bytes.copy_from_slice(&fork_data_root[0..4]);
+
+        u32::from_le_bytes(bytes)
+    }
+
     /// Compute a domain by applying the given `fork_version`.
     ///
     /// Spec v0.10.1

eth2/types/src/fork_data.rs

@@ -0,0 +1,32 @@
+use crate::test_utils::TestRandom;
+use crate::utils::{fork_from_hex_str, fork_to_hex_str};
+use crate::{Hash256, SignedRoot};
+
+use serde_derive::{Deserialize, Serialize};
+use ssz_derive::{Decode, Encode};
+use test_random_derive::TestRandom;
+use tree_hash_derive::TreeHash;
+
+/// Specifies a fork of the `BeaconChain`, to prevent replay attacks.
+///
+/// Spec v0.11.0
+#[derive(
+    Debug, Clone, PartialEq, Default, Serialize, Deserialize, Encode, Decode, TreeHash, TestRandom,
+)]
+pub struct ForkData {
+    #[serde(
+        serialize_with = "fork_to_hex_str",
+        deserialize_with = "fork_from_hex_str"
+    )]
+    pub current_version: [u8; 4],
+    pub genesis_validators_root: Hash256,
+}
+
+impl SignedRoot for ForkData {}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    ssz_and_tree_hash_tests!(ForkData);
+}

eth2/types/src/lib.rs

@@ -25,6 +25,7 @@ pub mod enr_fork_id;
 pub mod eth1_data;
 pub mod eth_spec;
 pub mod fork;
+pub mod fork_data;
 pub mod free_attestation;
 pub mod historical_batch;
 pub mod indexed_attestation;
@@ -80,6 +81,7 @@ pub use crate::slot_epoch::{Epoch, Slot, FAR_FUTURE_EPOCH};
 pub use crate::subnet_id::SubnetId;
 pub use crate::validator::Validator;
 pub use crate::voluntary_exit::VoluntaryExit;
+pub use fork_data::ForkData;
 
 pub type CommitteeIndex = u64;
 pub type Hash256 = H256;

eth2/utils/slot_clock/src/lib.rs

@@ -35,6 +35,9 @@ pub trait SlotClock: Send + Sync + Sized {
     /// Returns the duration between slots
     fn slot_duration(&self) -> Duration;
 
+    /// Returns the duration from now until `slot`.
+    fn duration_to_slot(&self, slot: Slot) -> Option<Duration>;
+
     /// Returns the duration until the next slot.
     fn duration_to_next_slot(&self) -> Option<Duration>;
 

eth2/utils/slot_clock/src/manual_slot_clock.rs

@@ -135,6 +135,10 @@ impl SlotClock for ManualSlotClock {
         self.slot_duration
     }
 
+    fn duration_to_slot(&self, slot: Slot) -> Option<Duration> {
+        self.duration_to_slot(slot, *self.current_time.read())
+    }
+
     fn genesis_slot(&self) -> Slot {
         self.genesis_slot
     }