Add proto_array fork choice (#804)

* Start implementing proto_array

* Add progress

* Add unfinished progress

* Add further progress

* Add progress

* Add tree filtering

* Add half-finished modifications

* Add refactored version

* Tidy, add incomplete LmdGhost impl

* Move impls in LmdGhost trait def

* Remove old reduced_tree fork choice

* Combine two functions in to `compute_deltas`

* Start testing

* Add more compute_deltas tests

* Add fork choice testing

* Add more fork choice testing

* Add more fork choice tests

* Add more testing to proto-array

* Remove old tests

* Modify tests

* Add more tests

* Add more testing

* Add comments and fixes

* Re-organise crate

* Tidy, finish pruning tests

* Add ssz encoding, other pub fns

* Rename lmd_ghost > proto_array_fork_choice

* Integrate proto_array into lighthouse

* Add first pass at fixing filter

* Clean out old comments

* Add more comments

* Attempt to fix prune error

* Adjust TODO

* Fix test compile errors

* Add extra justification change check

* Update cargo.lock

* Fix fork choice test compile errors

* Most remove ffg_update_required

* Fix bug with epoch of attestation votes

* Start adding new test format

* Make fork choice tests declarative

* Create test def concept

* Move test defs into crate

* Add binary, re-org crate

* Shuffle files

* Start adding ffg tests

* Add more fork choice tests

* Add fork choice JSON dumping

* Add more detail to best node error

* Ensure fin+just checkpoints from from same block

* Rename JustificationManager

* Move checkpoint manager into own file

* Tidy

* Add targetted logging for sneaky sync bug

* Fix justified balances bug

* Add cache metrics

* Add metrics for log levels

* Fix bug in checkpoint manager

* Fix compile error in fork choice tests

* Ignore duplicate blocks in fork choice

* Add block to fock choice before db

* Rename on_new_block fn

* Fix spec inconsistency in `CheckpointManager`

* Remove BlockRootTree

* Remove old reduced_tree code fragment

* Add API endpoint for fork choice

* Add more ffg tests

* Remove block_root_tree reminents

* Ensure effective balances are used

* Remove old debugging code, fix API fault

* Add check to ensure parent block is in fork choice

* Update readme dates

* Fix readme

* Tidy checkpoint manager

* Remove fork choice yaml files from repo

* Remove fork choice yaml from repo

* General tidy

* Address majority of Michael's comments

* Tidy bin/lib business

* Remove dangling file

* Undo changes for rpc/handler from master

* Revert "Undo changes for rpc/handler from master"

This reverts commit 876edff0e4.

Co-authored-by: Age Manning <Age@AgeManning.com>

beacon_node/beacon_chain/src/builder.rs

@@ -9,54 +9,35 @@ use crate::{
     ForkChoice,
 };
 use eth1::Config as Eth1Config;
-use lmd_ghost::{LmdGhost, ThreadSafeReducedTree};
 use operation_pool::OperationPool;
+use proto_array_fork_choice::ProtoArrayForkChoice;
 use slog::{info, Logger};
 use slot_clock::{SlotClock, TestingSlotClock};
 use std::marker::PhantomData;
 use std::sync::Arc;
 use std::time::Duration;
-use store::{BlockRootTree, Store};
+use store::Store;
 use types::{BeaconBlock, BeaconState, ChainSpec, EthSpec, Hash256, Slot};
 
 /// An empty struct used to "witness" all the `BeaconChainTypes` traits. It has no user-facing
 /// functionality and only exists to satisfy the type system.
-pub struct Witness<
-    TStore,
-    TStoreMigrator,
-    TSlotClock,
-    TLmdGhost,
-    TEth1Backend,
-    TEthSpec,
-    TEventHandler,
->(
+pub struct Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>(
     PhantomData<(
         TStore,
         TStoreMigrator,
         TSlotClock,
-        TLmdGhost,
         TEth1Backend,
         TEthSpec,
         TEventHandler,
     )>,
 );
 
-impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec, TEventHandler>
-    BeaconChainTypes
-    for Witness<
-        TStore,
-        TStoreMigrator,
-        TSlotClock,
-        TLmdGhost,
-        TEth1Backend,
-        TEthSpec,
-        TEventHandler,
-    >
+impl<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler> BeaconChainTypes
+    for Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>
 where
     TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
-    TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec, TStore> + 'static,
     TEthSpec: EthSpec + 'static,
     TEventHandler: EventHandler<TEthSpec> + 'static,
@@ -64,7 +45,6 @@ where
     type Store = TStore;
     type StoreMigrator = TStoreMigrator;
     type SlotClock = TSlotClock;
-    type LmdGhost = TLmdGhost;
     type Eth1Chain = TEth1Backend;
     type EthSpec = TEthSpec;
     type EventHandler = TEventHandler;
@@ -92,28 +72,18 @@ pub struct BeaconChainBuilder<T: BeaconChainTypes> {
     slot_clock: Option<T::SlotClock>,
     persisted_beacon_chain: Option<PersistedBeaconChain<T>>,
     head_tracker: Option<HeadTracker>,
-    block_root_tree: Option<Arc<BlockRootTree>>,
     spec: ChainSpec,
     log: Option<Logger>,
 }
 
-impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec, TEventHandler>
+impl<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>
     BeaconChainBuilder<
-        Witness<
-            TStore,
-            TStoreMigrator,
-            TSlotClock,
-            TLmdGhost,
-            TEth1Backend,
-            TEthSpec,
-            TEventHandler,
-        >,
+        Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>,
     >
 where
     TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
-    TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec, TStore> + 'static,
     TEthSpec: EthSpec + 'static,
     TEventHandler: EventHandler<TEthSpec> + 'static,
@@ -135,7 +105,6 @@ where
             slot_clock: None,
             persisted_beacon_chain: None,
             head_tracker: None,
-            block_root_tree: None,
             spec: TEthSpec::default_spec(),
             log: None,
         }
@@ -194,15 +163,7 @@ where
 
         let key = Hash256::from_slice(&BEACON_CHAIN_DB_KEY.as_bytes());
         let p: PersistedBeaconChain<
-            Witness<
-                TStore,
-                TStoreMigrator,
-                TSlotClock,
-                TLmdGhost,
-                TEth1Backend,
-                TEthSpec,
-                TEventHandler,
-            >,
+            Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>,
         > = match store.get(&key) {
             Err(e) => {
                 return Err(format!(
@@ -230,7 +191,6 @@ where
             Some(cache) => Some(Eth1Chain::from_ssz_container(cache, config, store, log)?),
             None => None,
         };
-        self.block_root_tree = Some(Arc::new(p.block_root_tree.clone().into()));
         self.persisted_beacon_chain = Some(p);
 
         Ok(self)
@@ -273,11 +233,6 @@ where
             )
         })?;
 
-        self.block_root_tree = Some(Arc::new(BlockRootTree::new(
-            beacon_block_root,
-            beacon_block.slot,
-        )));
-
         self.finalized_checkpoint = Some(CheckPoint {
             beacon_block_root,
             beacon_block,
@@ -327,15 +282,7 @@ where
         self,
     ) -> Result<
         BeaconChain<
-            Witness<
-                TStore,
-                TStoreMigrator,
-                TSlotClock,
-                TLmdGhost,
-                TEth1Backend,
-                TEthSpec,
-                TEventHandler,
-            >,
+            Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>,
         >,
         String,
     > {
@@ -387,9 +334,6 @@ where
                 .event_handler
                 .ok_or_else(|| "Cannot build without an event handler".to_string())?,
             head_tracker: self.head_tracker.unwrap_or_default(),
-            block_root_tree: self
-                .block_root_tree
-                .ok_or_else(|| "Cannot build without a block root tree".to_string())?,
             checkpoint_cache: CheckPointCache::default(),
             log: log.clone(),
         };
@@ -412,15 +356,7 @@ where
 
 impl<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>
     BeaconChainBuilder<
-        Witness<
-            TStore,
-            TStoreMigrator,
-            TSlotClock,
-            ThreadSafeReducedTree<TStore, TEthSpec>,
-            TEth1Backend,
-            TEthSpec,
-            TEventHandler,
-        >,
+        Witness<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>,
     >
 where
     TStore: Store<TEthSpec> + 'static,
@@ -435,23 +371,9 @@ where
     /// If this builder is being "resumed" from disk, then rebuild the last fork choice stored to
     /// the database. Otherwise, create a new, empty fork choice.
     pub fn reduced_tree_fork_choice(mut self) -> Result<Self, String> {
-        let store = self
-            .store
-            .clone()
-            .ok_or_else(|| "reduced_tree_fork_choice requires a store")?;
-
-        let block_root_tree = self
-            .block_root_tree
-            .clone()
-            .ok_or_else(|| "reduced_tree_fork_choice requires a block root tree")?;
-
         let fork_choice = if let Some(persisted_beacon_chain) = &self.persisted_beacon_chain {
-            ForkChoice::from_ssz_container(
-                persisted_beacon_chain.fork_choice.clone(),
-                store,
-                block_root_tree,
-            )
-            .map_err(|e| format!("Unable to decode fork choice from db: {:?}", e))?
+            ForkChoice::from_ssz_container(persisted_beacon_chain.fork_choice.clone())
+                .map_err(|e| format!("Unable to decode fork choice from db: {:?}", e))?
         } else {
             let finalized_checkpoint = &self
                 .finalized_checkpoint
@@ -461,14 +383,22 @@ where
                 .genesis_block_root
                 .ok_or_else(|| "fork_choice_backend requires a genesis_block_root")?;
 
-            let backend = ThreadSafeReducedTree::new(
-                store,
-                block_root_tree,
-                &finalized_checkpoint.beacon_block,
+            let backend = ProtoArrayForkChoice::new(
+                finalized_checkpoint.beacon_block.slot,
+                // Note: here we set the `justified_epoch` to be the same as the epoch of the
+                // finalized checkpoint. Whilst this finalized checkpoint may actually point to
+                // a _later_ justified checkpoint, that checkpoint won't yet exist in the fork
+                // choice.
+                finalized_checkpoint.beacon_state.current_epoch(),
+                finalized_checkpoint.beacon_state.current_epoch(),
                 finalized_checkpoint.beacon_block_root,
-            );
+            )?;
 
-            ForkChoice::new(backend, genesis_block_root, self.spec.genesis_slot)
+            ForkChoice::new(
+                backend,
+                genesis_block_root,
+                &finalized_checkpoint.beacon_state,
+            )
         };
 
         self.fork_choice = Some(fork_choice);
@@ -477,13 +407,12 @@ where
     }
 }
 
-impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEthSpec, TEventHandler>
+impl<TStore, TStoreMigrator, TSlotClock, TEthSpec, TEventHandler>
     BeaconChainBuilder<
         Witness<
             TStore,
             TStoreMigrator,
             TSlotClock,
-            TLmdGhost,
             CachingEth1Backend<TEthSpec, TStore>,
             TEthSpec,
             TEventHandler,
@@ -493,7 +422,6 @@ where
     TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
-    TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEthSpec: EthSpec + 'static,
     TEventHandler: EventHandler<TEthSpec> + 'static,
 {
@@ -529,22 +457,13 @@ where
     }
 }
 
-impl<TStore, TStoreMigrator, TLmdGhost, TEth1Backend, TEthSpec, TEventHandler>
+impl<TStore, TStoreMigrator, TEth1Backend, TEthSpec, TEventHandler>
     BeaconChainBuilder<
-        Witness<
-            TStore,
-            TStoreMigrator,
-            TestingSlotClock,
-            TLmdGhost,
-            TEth1Backend,
-            TEthSpec,
-            TEventHandler,
-        >,
+        Witness<TStore, TStoreMigrator, TestingSlotClock, TEth1Backend, TEthSpec, TEventHandler>,
     >
 where
     TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
-    TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec, TStore> + 'static,
     TEthSpec: EthSpec + 'static,
     TEventHandler: EventHandler<TEthSpec> + 'static,
@@ -570,13 +489,12 @@ where
     }
 }
 
-impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec>
+impl<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec>
     BeaconChainBuilder<
         Witness<
             TStore,
             TStoreMigrator,
             TSlotClock,
-            TLmdGhost,
             TEth1Backend,
             TEthSpec,
             NullEventHandler<TEthSpec>,
@@ -586,7 +504,6 @@ where
     TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
-    TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec, TStore> + 'static,
     TEthSpec: EthSpec + 'static,
 {