From 76602a65fc1a4eeb032d10579d0dc1a8c1423e92 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 27 May 2019 15:12:51 +1000 Subject: [PATCH] Add `new` fns to `ForkChoice` and `SlotClock` --- beacon_node/beacon_chain/src/beacon_chain.rs | 2 +- beacon_node/client/src/beacon_chain_types.rs | 20 +++++------ eth2/fork_choice/src/bitwise_lmd_ghost.rs | 26 +++++++------- eth2/fork_choice/src/lib.rs | 24 +++---------- eth2/fork_choice/src/longest_chain.rs | 6 ++-- eth2/fork_choice/src/optimized_lmd_ghost.rs | 26 +++++++------- eth2/fork_choice/src/slow_lmd_ghost.rs | 20 +++++------ eth2/utils/slot_clock/src/lib.rs | 7 +++- .../slot_clock/src/system_time_slot_clock.rs | 36 ++++++++----------- .../slot_clock/src/testing_slot_clock.rs | 26 +++++++------- validator_client/src/service.rs | 3 +- 11 files changed, 88 insertions(+), 108 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 9f08b6f64..a614698c4 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -82,7 +82,7 @@ impl BlockProcessingOutcome { pub trait BeaconChainTypes { type Store: store::Store; type SlotClock: slot_clock::SlotClock; - type ForkChoice: fork_choice::ForkChoice; + type ForkChoice: fork_choice::ForkChoice; type EthSpec: types::EthSpec; } diff --git a/beacon_node/client/src/beacon_chain_types.rs b/beacon_node/client/src/beacon_chain_types.rs index b8236c679..1971f5603 100644 --- a/beacon_node/client/src/beacon_chain_types.rs +++ b/beacon_node/client/src/beacon_chain_types.rs @@ -5,6 +5,8 @@ use beacon_chain::{ store::{DiskStore, MemoryStore, Store}, BeaconChain, BeaconChainTypes, }; +use fork_choice::ForkChoice; +use slot_clock::SlotClock; use std::sync::Arc; use tree_hash::TreeHash; use types::{ @@ -36,7 +38,7 @@ where >, { fn initialise_beacon_chain(_config: &ClientConfig) -> BeaconChain { - initialize_chain(MemoryStore::open()) + initialize_chain::<_, _, FewValidatorsEthSpec>(MemoryStore::open()) } } @@ -62,18 +64,15 @@ where fn initialise_beacon_chain(config: &ClientConfig) -> BeaconChain { let store = DiskStore::open(&config.db_name).expect("Unable to open DB."); - initialize_chain(store) + initialize_chain::<_, _, FewValidatorsEthSpec>(store) } } /// Produces a `BeaconChain` given some pre-initialized `Store`. fn initialize_chain(store: U) -> BeaconChain where - T: BeaconChainTypes< - Store = U, - SlotClock = SystemTimeSlotClock, - ForkChoice = BitwiseLMDGhost, - >, + T: BeaconChainTypes, + T::ForkChoice: ForkChoice, { let spec = T::EthSpec::spec(); @@ -86,14 +85,13 @@ where genesis_block.state_root = Hash256::from_slice(&genesis_state.tree_hash_root()); // Slot clock - let slot_clock = SystemTimeSlotClock::new( + let slot_clock = T::SlotClock::new( spec.genesis_slot, genesis_state.genesis_time, spec.seconds_per_slot, - ) - .expect("Unable to load SystemTimeSlotClock"); + ); // Choose the fork choice - let fork_choice = BitwiseLMDGhost::new(store.clone()); + let fork_choice = T::ForkChoice::new(store.clone()); // Genesis chain //TODO: Handle error correctly diff --git a/eth2/fork_choice/src/bitwise_lmd_ghost.rs b/eth2/fork_choice/src/bitwise_lmd_ghost.rs index 0e579c0b9..a26a94b8a 100644 --- a/eth2/fork_choice/src/bitwise_lmd_ghost.rs +++ b/eth2/fork_choice/src/bitwise_lmd_ghost.rs @@ -48,18 +48,6 @@ pub struct BitwiseLMDGhost { } impl BitwiseLMDGhost { - pub fn new(store: Arc) -> Self { - BitwiseLMDGhost { - cache: HashMap::new(), - ancestors: vec![HashMap::new(); 16], - latest_attestation_targets: HashMap::new(), - children: HashMap::new(), - max_known_height: SlotHeight::new(0), - store, - _phantom: PhantomData, - } - } - /// Finds the latest votes weighted by validator balance. Returns a hashmap of block_hash to /// weighted votes. pub fn get_latest_votes( @@ -229,7 +217,19 @@ impl BitwiseLMDGhost { } } -impl ForkChoice for BitwiseLMDGhost { +impl ForkChoice for BitwiseLMDGhost { + fn new(store: Arc) -> Self { + BitwiseLMDGhost { + cache: HashMap::new(), + ancestors: vec![HashMap::new(); 16], + latest_attestation_targets: HashMap::new(), + children: HashMap::new(), + max_known_height: SlotHeight::new(0), + store, + _phantom: PhantomData, + } + } + fn add_block( &mut self, block: &BeaconBlock, diff --git a/eth2/fork_choice/src/lib.rs b/eth2/fork_choice/src/lib.rs index ffc40e6c6..ce53c1051 100644 --- a/eth2/fork_choice/src/lib.rs +++ b/eth2/fork_choice/src/lib.rs @@ -21,8 +21,7 @@ pub mod longest_chain; pub mod optimized_lmd_ghost; pub mod slow_lmd_ghost; -// use store::stores::BeaconBlockAtSlotError; -// use store::DBError; +use std::sync::Arc; use store::Error as DBError; use types::{BeaconBlock, ChainSpec, Hash256}; @@ -34,7 +33,10 @@ pub use slow_lmd_ghost::SlowLMDGhost; /// Defines the interface for Fork Choices. Each Fork choice will define their own data structures /// which can be built in block processing through the `add_block` and `add_attestation` functions. /// The main fork choice algorithm is specified in `find_head -pub trait ForkChoice: Send + Sync { +pub trait ForkChoice: Send + Sync { + /// Create a new `ForkChoice` which reads from `store`. + fn new(store: Arc) -> Self; + /// Called when a block has been added. Allows generic block-level data structures to be /// built for a given fork-choice. fn add_block( @@ -78,22 +80,6 @@ impl From for ForkChoiceError { } } -/* -impl From for ForkChoiceError { - fn from(e: BeaconBlockAtSlotError) -> ForkChoiceError { - match e { - BeaconBlockAtSlotError::UnknownBeaconBlock(hash) => { - ForkChoiceError::MissingBeaconBlock(hash) - } - BeaconBlockAtSlotError::InvalidBeaconBlock(hash) => { - ForkChoiceError::MissingBeaconBlock(hash) - } - BeaconBlockAtSlotError::DBError(string) => ForkChoiceError::StorageError(string), - } - } -} -*/ - /// Fork choice options that are currently implemented. #[derive(Debug, Clone)] pub enum ForkChoiceAlgorithm { diff --git a/eth2/fork_choice/src/longest_chain.rs b/eth2/fork_choice/src/longest_chain.rs index 11453cf49..08e47cf39 100644 --- a/eth2/fork_choice/src/longest_chain.rs +++ b/eth2/fork_choice/src/longest_chain.rs @@ -10,16 +10,14 @@ pub struct LongestChain { store: Arc, } -impl LongestChain { - pub fn new(store: Arc) -> Self { +impl ForkChoice for LongestChain { + fn new(store: Arc) -> Self { LongestChain { head_block_hashes: Vec::new(), store, } } -} -impl ForkChoice for LongestChain { fn add_block( &mut self, block: &BeaconBlock, diff --git a/eth2/fork_choice/src/optimized_lmd_ghost.rs b/eth2/fork_choice/src/optimized_lmd_ghost.rs index dba6e60da..2c1063a2b 100644 --- a/eth2/fork_choice/src/optimized_lmd_ghost.rs +++ b/eth2/fork_choice/src/optimized_lmd_ghost.rs @@ -48,18 +48,6 @@ pub struct OptimizedLMDGhost { } impl OptimizedLMDGhost { - pub fn new(store: Arc) -> Self { - OptimizedLMDGhost { - cache: HashMap::new(), - ancestors: vec![HashMap::new(); 16], - latest_attestation_targets: HashMap::new(), - children: HashMap::new(), - max_known_height: SlotHeight::new(0), - store, - _phantom: PhantomData, - } - } - /// Finds the latest votes weighted by validator balance. Returns a hashmap of block_hash to /// weighted votes. pub fn get_latest_votes( @@ -200,7 +188,19 @@ impl OptimizedLMDGhost { } } -impl ForkChoice for OptimizedLMDGhost { +impl ForkChoice for OptimizedLMDGhost { + fn new(store: Arc) -> Self { + OptimizedLMDGhost { + cache: HashMap::new(), + ancestors: vec![HashMap::new(); 16], + latest_attestation_targets: HashMap::new(), + children: HashMap::new(), + max_known_height: SlotHeight::new(0), + store, + _phantom: PhantomData, + } + } + fn add_block( &mut self, block: &BeaconBlock, diff --git a/eth2/fork_choice/src/slow_lmd_ghost.rs b/eth2/fork_choice/src/slow_lmd_ghost.rs index 888356417..38b1e8dab 100644 --- a/eth2/fork_choice/src/slow_lmd_ghost.rs +++ b/eth2/fork_choice/src/slow_lmd_ghost.rs @@ -20,15 +20,6 @@ pub struct SlowLMDGhost { } impl SlowLMDGhost { - pub fn new(store: Arc) -> Self { - SlowLMDGhost { - latest_attestation_targets: HashMap::new(), - children: HashMap::new(), - store, - _phantom: PhantomData, - } - } - /// Finds the latest votes weighted by validator balance. Returns a hashmap of block_hash to /// weighted votes. pub fn get_latest_votes( @@ -94,7 +85,16 @@ impl SlowLMDGhost { } } -impl ForkChoice for SlowLMDGhost { +impl ForkChoice for SlowLMDGhost { + fn new(store: Arc) -> Self { + SlowLMDGhost { + latest_attestation_targets: HashMap::new(), + children: HashMap::new(), + store, + _phantom: PhantomData, + } + } + /// Process when a block is added fn add_block( &mut self, diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs index fd5a2d1d7..7b86684fa 100644 --- a/eth2/utils/slot_clock/src/lib.rs +++ b/eth2/utils/slot_clock/src/lib.rs @@ -6,9 +6,14 @@ pub use crate::testing_slot_clock::{Error as TestingSlotClockError, TestingSlotC use std::time::Duration; pub use types::Slot; -pub trait SlotClock: Send + Sync { +pub trait SlotClock: Send + Sync + Sized { type Error; + /// Create a new `SlotClock`. + /// + /// Returns an Error if `slot_duration_seconds == 0`. + fn new(genesis_slot: Slot, genesis_seconds: u64, slot_duration_seconds: u64) -> Self; + fn present_slot(&self) -> Result, Self::Error>; fn duration_to_next_slot(&self) -> Result, Self::Error>; 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 4dfc6b37d..7c184b02b 100644 --- a/eth2/utils/slot_clock/src/system_time_slot_clock.rs +++ b/eth2/utils/slot_clock/src/system_time_slot_clock.rs @@ -18,31 +18,25 @@ pub struct SystemTimeSlotClock { slot_duration_seconds: u64, } -impl SystemTimeSlotClock { - /// Create a new `SystemTimeSlotClock`. - /// - /// Returns an Error if `slot_duration_seconds == 0`. - pub fn new( - genesis_slot: Slot, - genesis_seconds: u64, - slot_duration_seconds: u64, - ) -> Result { - if slot_duration_seconds == 0 { - Err(Error::SlotDurationIsZero) - } else { - Ok(Self { - genesis_slot, - genesis_seconds, - slot_duration_seconds, - }) - } - } -} - impl SlotClock for SystemTimeSlotClock { type Error = Error; + /// Create a new `SystemTimeSlotClock`. + /// + /// Returns an Error if `slot_duration_seconds == 0`. + fn new(genesis_slot: Slot, genesis_seconds: u64, slot_duration_seconds: u64) -> Self { + Self { + genesis_slot, + genesis_seconds, + slot_duration_seconds, + } + } + fn present_slot(&self) -> Result, Error> { + if self.slot_duration_seconds == 0 { + return Err(Error::SlotDurationIsZero); + } + let syslot_time = SystemTime::now(); let duration_since_epoch = syslot_time.duration_since(SystemTime::UNIX_EPOCH)?; let duration_since_genesis = diff --git a/eth2/utils/slot_clock/src/testing_slot_clock.rs b/eth2/utils/slot_clock/src/testing_slot_clock.rs index b5c36dfa0..fc9b7201b 100644 --- a/eth2/utils/slot_clock/src/testing_slot_clock.rs +++ b/eth2/utils/slot_clock/src/testing_slot_clock.rs @@ -8,30 +8,28 @@ pub enum Error {} /// Determines the present slot based upon the present system time. pub struct TestingSlotClock { - slot: RwLock, + slot: RwLock, } impl TestingSlotClock { - /// Create a new `TestingSlotClock`. - /// - /// Returns an Error if `slot_duration_seconds == 0`. - pub fn new(slot: u64) -> TestingSlotClock { - TestingSlotClock { - slot: RwLock::new(slot), - } - } - pub fn set_slot(&self, slot: u64) { - *self.slot.write().expect("TestingSlotClock poisoned.") = slot; + *self.slot.write().expect("TestingSlotClock poisoned.") = Slot::from(slot); } } impl SlotClock for TestingSlotClock { type Error = Error; + /// Create a new `TestingSlotClock` at `genesis_slot`. + fn new(genesis_slot: Slot, _genesis_seconds: u64, _slot_duration_seconds: u64) -> Self { + TestingSlotClock { + slot: RwLock::new(genesis_slot), + } + } + fn present_slot(&self) -> Result, Error> { let slot = *self.slot.read().expect("TestingSlotClock poisoned."); - Ok(Some(Slot::new(slot))) + Ok(Some(slot)) } /// Always returns a duration of 1 second. @@ -46,7 +44,9 @@ mod tests { #[test] fn test_slot_now() { - let clock = TestingSlotClock::new(10); + let null = 0; + + let clock = TestingSlotClock::new(Slot::new(10), null, null); assert_eq!(clock.present_slot(), Ok(Some(Slot::new(10)))); clock.set_slot(123); assert_eq!(clock.present_slot(), Ok(Some(Slot::new(123)))); diff --git a/validator_client/src/service.rs b/validator_client/src/service.rs index a340f99fc..033394a0d 100644 --- a/validator_client/src/service.rs +++ b/validator_client/src/service.rs @@ -155,8 +155,7 @@ impl Service { // build the validator slot clock let slot_clock = - SystemTimeSlotClock::new(genesis_slot, genesis_time, config.spec.seconds_per_slot) - .expect("Unable to instantiate SystemTimeSlotClock."); + SystemTimeSlotClock::new(genesis_slot, genesis_time, config.spec.seconds_per_slot); let current_slot = slot_clock .present_slot()