From 58e51675f374519a404a6b95d496dae80bd68365 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 26 Mar 2020 17:35:12 +1100 Subject: [PATCH] Enr fork (#967) * Start fixing enr-fork-id * Fix time-until-next-fork logic * Remove fork crate --- Cargo.lock | 8 -- Cargo.toml | 1 - beacon_node/beacon_chain/Cargo.toml | 1 - beacon_node/beacon_chain/src/beacon_chain.rs | 54 +++---------- beacon_node/fork/Cargo.toml | 8 -- beacon_node/fork/src/forks.rs | 5 -- beacon_node/fork/src/lib.rs | 75 ------------------- beacon_node/network/src/service.rs | 26 ++++--- eth2/types/src/chain_spec.rs | 57 ++++++++++++++ eth2/types/src/fork_data.rs | 32 ++++++++ eth2/types/src/lib.rs | 2 + eth2/utils/slot_clock/src/lib.rs | 3 + .../utils/slot_clock/src/manual_slot_clock.rs | 4 + .../slot_clock/src/system_time_slot_clock.rs | 5 ++ 14 files changed, 129 insertions(+), 152 deletions(-) delete mode 100644 beacon_node/fork/Cargo.toml delete mode 100644 beacon_node/fork/src/forks.rs delete mode 100644 beacon_node/fork/src/lib.rs create mode 100644 eth2/types/src/fork_data.rs diff --git a/Cargo.lock b/Cargo.lock index 32be4e46d88..013752d3071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -234,7 +234,6 @@ dependencies = [ "eth2_ssz", "eth2_ssz_derive", "eth2_ssz_types", - "fork", "futures", "genesis", "integer-sqrt", @@ -1479,13 +1478,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" -[[package]] -name = "fork" -version = "0.2.0" -dependencies = [ - "types", -] - [[package]] name = "fuchsia-cprng" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index a0510b4d532..37f2550269c 100644 --- a/Cargo.toml +++ b/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", diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index 6a0f566cef0..428d6b79b93 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/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" } diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a084f1dae4e..726898e99ea 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/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 BeaconChain { } /// Gets the current EnrForkId. - /// - /// v0.11 - pub fn enr_fork_id(&self) -> Result { - 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::( - &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, Error> { - let current_slot = self.slot()?; - let next_fork_epoch = - next_fork_epoch::(&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 { + let epoch = self.spec.next_fork_epoch()?; + self.slot_clock + .duration_to_slot(epoch.start_slot(T::EthSpec::slots_per_epoch())) } } diff --git a/beacon_node/fork/Cargo.toml b/beacon_node/fork/Cargo.toml deleted file mode 100644 index dc3d82222c1..00000000000 --- a/beacon_node/fork/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "fork" -version = "0.2.0" -authors = ["Age Manning "] -edition = "2018" - -[dependencies] -types = { path = "../../eth2/types" } diff --git a/beacon_node/fork/src/forks.rs b/beacon_node/fork/src/forks.rs deleted file mode 100644 index ce0d65a2f69..00000000000 --- a/beacon_node/fork/src/forks.rs +++ /dev/null @@ -1,5 +0,0 @@ -///! List of known forks - -// Add known forks to this mapping in slot order. -/// List of known forks. The format is (Fork Name, Slot to be activated, Fork Version). -pub const KNOWN_FORKS: [(&'static str, u64, [u8; 4]); 1] = [("genesis", 0, [0, 0, 0, 0])]; diff --git a/beacon_node/fork/src/lib.rs b/beacon_node/fork/src/lib.rs deleted file mode 100644 index a85f66055bc..00000000000 --- a/beacon_node/fork/src/lib.rs +++ /dev/null @@ -1,75 +0,0 @@ -///! Maintains a hard-coded list of known forks and their slots at which they were activated. -use types::{ChainSpec, Epoch, EthSpec, Slot}; - -mod forks; - -/// A state-less function that provides the fork version given a set of active forks and a slot -/// number. -/// -/// The disabled_forks parameter select which forks are disabled by their name. -pub fn current_fork_version(slot: Slot, disabled_forks: &[String]) -> [u8; 4] { - let mut version = [0, 0, 0, 0]; - for (fork_name, fork_slot_no, fork_version) in forks::KNOWN_FORKS.iter() { - if *fork_slot_no <= slot.as_u64() { - if disabled_forks - .iter() - .find(|fork| **fork == String::from(*fork_name)) - .is_none() - { - version = fork_version.clone(); - } - } else { - break; - } - } - version -} - -pub fn next_fork_version(slot: Slot, disabled_forks: &[String]) -> [u8; 4] { - let mut version = None; - for (fork_name, fork_slot_no, fork_version) in forks::KNOWN_FORKS.iter() { - if *fork_slot_no > slot.as_u64() { - if disabled_forks - .iter() - .find(|fork| **fork == String::from(*fork_name)) - .is_none() - { - version = Some(fork_version.clone()); - break; - } - } - } - - if let Some(result_version) = version { - result_version - } else { - // if there is no next fork, use the current fork version - current_fork_version(slot, disabled_forks) - } -} - -pub fn next_fork_epoch( - spec: &ChainSpec, - slot: Slot, - disabled_forks: &[String], -) -> Epoch { - let mut next_fork_slot = None; - for (fork_name, fork_slot_no, _fork_version) in forks::KNOWN_FORKS.iter() { - if *fork_slot_no > slot.as_u64() { - if disabled_forks - .iter() - .find(|fork| **fork == String::from(*fork_name)) - .is_none() - { - next_fork_slot = Some(Slot::new(*fork_slot_no)); - break; - } - } - } - - if let Some(fork_slot) = next_fork_slot { - fork_slot.epoch(T::slots_per_epoch()) - } else { - Epoch::from(spec.far_future_epoch) - } -} diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 8c1cd6cae19..cdee58f7b1c 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -78,14 +78,10 @@ impl NetworkService { 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( 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( 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( + beacon_chain: &BeaconChain, +) -> Option { + 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 { diff --git a/eth2/types/src/chain_spec.rs b/eth2/types/src/chain_spec.rs index f0ad2f07e03..1bd3d9e5301 100644 --- a/eth2/types/src/chain_spec.rs +++ b/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 { + 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 diff --git a/eth2/types/src/fork_data.rs b/eth2/types/src/fork_data.rs new file mode 100644 index 00000000000..439f0ca2e5f --- /dev/null +++ b/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); +} diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index dbd71fefd9b..7f185fdda66 100644 --- a/eth2/types/src/lib.rs +++ b/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; diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs index e9566a9a05b..69e1b8cac7b 100644 --- a/eth2/utils/slot_clock/src/lib.rs +++ b/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; + /// Returns the duration until the next slot. fn duration_to_next_slot(&self) -> Option; diff --git a/eth2/utils/slot_clock/src/manual_slot_clock.rs b/eth2/utils/slot_clock/src/manual_slot_clock.rs index fc0e27e3a3a..d198e24e12c 100644 --- a/eth2/utils/slot_clock/src/manual_slot_clock.rs +++ b/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 { + self.duration_to_slot(slot, *self.current_time.read()) + } + fn genesis_slot(&self) -> Slot { self.genesis_slot } diff --git a/eth2/utils/slot_clock/src/system_time_slot_clock.rs b/eth2/utils/slot_clock/src/system_time_slot_clock.rs index 81a5e59123c..adfb68b256f 100644 --- a/eth2/utils/slot_clock/src/system_time_slot_clock.rs +++ b/eth2/utils/slot_clock/src/system_time_slot_clock.rs @@ -44,6 +44,11 @@ impl SlotClock for SystemTimeSlotClock { self.clock.slot_duration() } + fn duration_to_slot(&self, slot: Slot) -> Option { + let now = SystemTime::now().duration_since(UNIX_EPOCH).ok()?; + self.clock.duration_to_slot(slot, now) + } + fn genesis_slot(&self) -> Slot { self.clock.genesis_slot() }