From 2ee71aa80819da94489675e01e1b905abe5677a5 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 15 Jun 2019 14:03:29 -0400 Subject: [PATCH] Add new fork choice struct to beacon chain --- beacon_node/beacon_chain/Cargo.toml | 2 +- beacon_node/beacon_chain/src/beacon_chain.rs | 36 +++---- beacon_node/beacon_chain/src/errors.rs | 2 +- beacon_node/beacon_chain/src/fork_choice.rs | 98 ++++++++++++++++++++ beacon_node/beacon_chain/src/lib.rs | 2 +- eth2/lmd_ghost/src/lib.rs | 91 ++---------------- eth2/lmd_ghost/src/reduced_tree.rs | 25 ++--- 7 files changed, 127 insertions(+), 129 deletions(-) create mode 100644 beacon_node/beacon_chain/src/fork_choice.rs diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index 4f007cbb7..abf1bc647 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -11,7 +11,6 @@ store = { path = "../store" } failure = "0.1" failure_derive = "0.1" hashing = { path = "../../eth2/utils/hashing" } -fork_choice = { path = "../../eth2/fork_choice" } parking_lot = "0.7" prometheus = "^0.6" log = "0.4" @@ -26,3 +25,4 @@ ssz_derive = { path = "../../eth2/utils/ssz_derive" } state_processing = { path = "../../eth2/state_processing" } tree_hash = { path = "../../eth2/utils/tree_hash" } types = { path = "../../eth2/types" } +lmd_ghost = { path = "../../eth2/lmd_ghost" } diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index dc2cc16df..b0fe9278c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1,9 +1,10 @@ use crate::checkpoint::CheckPoint; use crate::errors::{BeaconChainError as Error, BlockProductionError}; +use crate::fork_choice::{Error as ForkChoiceError, ForkChoice}; use crate::iter::{BlockIterator, BlockRootsIterator}; use crate::metrics::Metrics; use crate::persisted_beacon_chain::{PersistedBeaconChain, BEACON_CHAIN_DB_KEY}; -use fork_choice::{ForkChoice, ForkChoiceError}; +use lmd_ghost::LmdGhost; use log::{debug, trace}; use operation_pool::DepositInsertStatus; use operation_pool::OperationPool; @@ -48,7 +49,7 @@ pub enum BlockProcessingOutcome { pub trait BeaconChainTypes { type Store: store::Store; type SlotClock: slot_clock::SlotClock; - type ForkChoice: fork_choice::ForkChoice; + type LmdGhost: LmdGhost; type EthSpec: types::EthSpec; } @@ -73,7 +74,7 @@ pub struct BeaconChain { genesis_block_root: Hash256, /// A state-machine that is updated with information from the network and chooses a canonical /// head block. - pub fork_choice: RwLock, + pub fork_choice: ForkChoice, /// Stores metrics about this `BeaconChain`. pub metrics: Metrics, } @@ -86,7 +87,7 @@ impl BeaconChain { mut genesis_state: BeaconState, genesis_block: BeaconBlock, spec: ChainSpec, - fork_choice: T::ForkChoice, + fork_choice: ForkChoice, ) -> Result { let state_root = genesis_state.canonical_root(); store.put(&state_root, &genesis_state)?; @@ -115,7 +116,7 @@ impl BeaconChain { state: RwLock::new(genesis_state), canonical_head, genesis_block_root, - fork_choice: RwLock::new(fork_choice), + fork_choice, metrics: Metrics::new()?, }) } @@ -138,18 +139,19 @@ impl BeaconChain { spec.seconds_per_slot, ); - let fork_choice = T::ForkChoice::new(store.clone()); + // let fork_choice = T::ForkChoice::new(store.clone()); + // let fork_choice: ForkChoice = ForkChoice::new(store.clone()); Ok(Some(BeaconChain { spec, - store, slot_clock, op_pool: OperationPool::default(), canonical_head: RwLock::new(p.canonical_head), state: RwLock::new(p.state), - fork_choice: RwLock::new(fork_choice), + fork_choice: ForkChoice::new(store.clone()), genesis_block_root: p.genesis_block_root, metrics: Metrics::new()?, + store, })) } @@ -613,9 +615,7 @@ impl BeaconChain { self.store.put(&state_root, &state)?; // Register the new block with the fork choice service. - self.fork_choice - .write() - .add_block(&block, &block_root, &self.spec)?; + self.fork_choice.process_block(&state, &block)?; // Execute the fork choice algorithm, enthroning a new head if discovered. // @@ -713,20 +713,8 @@ impl BeaconChain { // Start fork choice metrics timer. let timer = self.metrics.fork_choice_times.start_timer(); - let justified_root = { - let root = self.head().beacon_state.current_justified_root; - if root == self.spec.zero_hash { - self.genesis_block_root - } else { - root - } - }; - // Determine the root of the block that is the head of the chain. - let beacon_block_root = self - .fork_choice - .write() - .find_head(&justified_root, &self.spec)?; + let beacon_block_root = self.fork_choice.find_head()?; // End fork choice metrics timer. timer.observe_duration(); diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 75f2fd84d..8e04948df 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -1,5 +1,5 @@ +use crate::fork_choice::Error as ForkChoiceError; use crate::metrics::Error as MetricsError; -use fork_choice::ForkChoiceError; use state_processing::BlockProcessingError; use state_processing::SlotProcessingError; use types::*; diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs new file mode 100644 index 000000000..b2a45baa2 --- /dev/null +++ b/beacon_node/beacon_chain/src/fork_choice.rs @@ -0,0 +1,98 @@ +use crate::BeaconChain; +use lmd_ghost::LmdGhost; +use state_processing::common::get_attesting_indices_unsorted; +use std::marker::PhantomData; +use std::sync::Arc; +use store::Store; +use types::{Attestation, BeaconBlock, BeaconState, BeaconStateError, EthSpec, Hash256}; + +type Result = std::result::Result; + +#[derive(Debug, PartialEq)] +pub enum Error { + BackendError(String), + BeaconStateError(BeaconStateError), +} + +pub struct ForkChoice { + backend: L, + _phantom_a: PhantomData, + _phantom_b: PhantomData, +} + +impl ForkChoice +where + L: LmdGhost, + S: Store, + E: EthSpec, +{ + pub fn new(store: Arc) -> Self { + Self { + backend: L::new(store), + _phantom_a: PhantomData, + _phantom_b: PhantomData, + } + } + + pub fn find_head(&self) -> Result { + self.backend.find_head().map_err(Into::into) + } + + pub fn process_attestation( + &self, + state: &BeaconState, + attestation: &Attestation, + ) -> Result<()> { + // Note: `get_attesting_indices_unsorted` requires that the beacon state caches be built. + let validator_indices = get_attesting_indices_unsorted( + state, + &attestation.data, + &attestation.aggregation_bitfield, + )?; + + let block_hash = attestation.data.target_root; + + // TODO: what happens when the target root is not the same slot as the block? + let block_slot = attestation + .data + .target_epoch + .start_slot(E::slots_per_epoch()); + + for validator_index in validator_indices { + self.backend + .process_message(validator_index, block_hash, block_slot)?; + } + + Ok(()) + } + + /// A helper function which runs `self.process_attestation` on all `Attestation` in the given `BeaconBlock`. + /// + /// Assumes the block (and therefore it's attestations) are valid. It is a logic error to + /// provide an invalid block. + pub fn process_block(&self, state: &BeaconState, block: &BeaconBlock) -> Result<()> { + // Note: we never count the block as a latest message, only attestations. + // + // I (Paul H) do not have an explicit reference to this, however I derive it from this + // document: + // + // https://github.com/ethereum/eth2.0-specs/blob/v0.7.0/specs/core/0_fork-choice.md + for attestation in &block.body.attestations { + self.process_attestation(state, attestation)?; + } + + Ok(()) + } +} + +impl From for Error { + fn from(e: BeaconStateError) -> Error { + Error::BeaconStateError(e) + } +} + +impl From for Error { + fn from(e: String) -> Error { + Error::BackendError(e) + } +} diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 21edb7859..b4250fe0f 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -1,6 +1,7 @@ mod beacon_chain; mod checkpoint; mod errors; +mod fork_choice; pub mod iter; mod metrics; mod persisted_beacon_chain; @@ -8,7 +9,6 @@ mod persisted_beacon_chain; pub use self::beacon_chain::{BeaconChain, BeaconChainTypes, BlockProcessingOutcome}; pub use self::checkpoint::CheckPoint; pub use self::errors::{BeaconChainError, BlockProductionError}; -pub use fork_choice; pub use parking_lot; pub use slot_clock; pub use state_processing::per_block_processing::errors::{ diff --git a/eth2/lmd_ghost/src/lib.rs b/eth2/lmd_ghost/src/lib.rs index 27801595a..5cd1b1892 100644 --- a/eth2/lmd_ghost/src/lib.rs +++ b/eth2/lmd_ghost/src/lib.rs @@ -1,20 +1,15 @@ -pub mod reduced_tree; +mod reduced_tree; -use state_processing::common::get_attesting_indices_unsorted; -use std::marker::PhantomData; use std::sync::Arc; -use types::{Attestation, BeaconBlock, BeaconState, BeaconStateError, EthSpec, Hash256, Slot}; +use store::Store; +use types::{EthSpec, Hash256, Slot}; -type Result = std::result::Result; +pub use reduced_tree::ThreadSafeReducedTree; -#[derive(Debug, PartialEq)] -pub enum Error { - BackendError(String), - BeaconStateError(BeaconStateError), -} +pub type Result = std::result::Result; -pub trait LmdGhostBackend: Send + Sync { - fn new(store: Arc) -> Self; +pub trait LmdGhost: Send + Sync { + fn new(store: Arc) -> Self; fn process_message( &self, @@ -25,75 +20,3 @@ pub trait LmdGhostBackend: Send + Sync { fn find_head(&self) -> Result; } - -pub struct ForkChoice { - backend: T, - _phantom: PhantomData, -} - -impl ForkChoice -where - T: LmdGhostBackend, - E: EthSpec, -{ - pub fn new(store: Arc) -> Self { - Self { - backend: T::new(store), - _phantom: PhantomData, - } - } - - pub fn find_head(&self) -> Result { - self.backend.find_head() - } - - pub fn process_attestation( - &self, - state: &BeaconState, - attestation: &Attestation, - ) -> Result<()> { - let validator_indices = get_attesting_indices_unsorted( - state, - &attestation.data, - &attestation.aggregation_bitfield, - )?; - - let block_hash = attestation.data.target_root; - - // TODO: what happens when the target root is not the same slot as the block? - let block_slot = attestation - .data - .target_epoch - .start_slot(E::slots_per_epoch()); - - for validator_index in validator_indices { - self.backend - .process_message(validator_index, block_hash, block_slot)?; - } - - Ok(()) - } - - pub fn process_block( - &self, - state: &BeaconState, - block: &BeaconBlock, - block_hash: Hash256, - block_proposer: usize, - ) -> Result<()> { - self.backend - .process_message(block_proposer, block_hash, block.slot)?; - - for attestation in &block.body.attestations { - self.process_attestation(state, attestation)?; - } - - Ok(()) - } -} - -impl From for Error { - fn from(e: BeaconStateError) -> Error { - Error::BeaconStateError(e) - } -} diff --git a/eth2/lmd_ghost/src/reduced_tree.rs b/eth2/lmd_ghost/src/reduced_tree.rs index b0e82d984..6f2b31c99 100644 --- a/eth2/lmd_ghost/src/reduced_tree.rs +++ b/eth2/lmd_ghost/src/reduced_tree.rs @@ -1,4 +1,4 @@ -use super::{Error as SuperError, LmdGhostBackend}; +use super::{LmdGhost, Result as SuperResult}; use parking_lot::RwLock; use std::collections::HashMap; use std::marker::PhantomData; @@ -8,8 +8,6 @@ use types::{BeaconBlock, BeaconState, EthSpec, Hash256, Slot}; type Result = std::result::Result; -pub const SKIP_LIST_LEN: usize = 16; - #[derive(Debug, PartialEq)] pub enum Error { MissingNode(Hash256), @@ -51,10 +49,6 @@ impl Node { pub fn has_votes(&self) -> bool { !self.voters.is_empty() } - - pub fn is_genesis(&self) -> bool { - self.parent_hash.is_some() - } } impl Node { @@ -69,7 +63,7 @@ pub struct Vote { slot: Slot, } -impl LmdGhostBackend for ThreadSafeReducedTree +impl LmdGhost for ThreadSafeReducedTree where T: Store, E: EthSpec, @@ -85,21 +79,21 @@ where validator_index: usize, block_hash: Hash256, block_slot: Slot, - ) -> std::result::Result<(), SuperError> { + ) -> SuperResult<()> { self.core .write() .process_message(validator_index, block_hash, block_slot) .map_err(Into::into) } - fn find_head(&self) -> std::result::Result { + fn find_head(&self) -> SuperResult { unimplemented!(); } } -impl From for SuperError { - fn from(e: Error) -> SuperError { - SuperError::BackendError(format!("{:?}", e)) +impl From for String { + fn from(e: Error) -> String { + format!("{:?}", e) } } @@ -415,11 +409,6 @@ where &self.0[i] } - pub fn get_mut(&mut self, i: usize) -> &mut T { - self.ensure(i); - &mut self.0[i] - } - pub fn insert(&mut self, i: usize, element: T) { self.ensure(i); self.0[i] = element;