[Merged by Bors] - Add fork choice EF tests

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -1203,7 +1203,7 @@ type CommitteesPerSlot = u64;
 
 /// Returns the `indexed_attestation` and committee count per slot for the `attestation` using the
 /// public keys cached in the `chain`.
-fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
+pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
     chain: &BeaconChain<T>,
     attestation: &Attestation<T::EthSpec>,
 ) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2407,7 +2407,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             let _fork_choice_block_timer =
                 metrics::start_timer(&metrics::FORK_CHOICE_PROCESS_BLOCK_TIMES);
             fork_choice
-                .on_block(current_slot, &block, block_root, &state)
+                .on_block(current_slot, &block, block_root, &state, &self.spec)
                 .map_err(|e| BlockError::BeaconChainError(e.into()))?;
         }
 

beacon_node/beacon_chain/src/fork_revert.rs

@@ -166,7 +166,7 @@ pub fn reset_fork_choice_to_finalization<E: EthSpec, Hot: ItemStore<E>, Cold: It
 
         let (block, _) = block.deconstruct();
         fork_choice
-            .on_block(block.slot(), &block, block.canonical_root(), &state)
+            .on_block(block.slot(), &block, block.canonical_root(), &state, spec)
             .map_err(|e| format!("Error applying replayed block to fork choice: {:?}", e))?;
     }
 

beacon_node/beacon_chain/src/lib.rs

@@ -36,7 +36,7 @@ mod validator_pubkey_cache;
 
 pub use self::beacon_chain::{
     AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BeaconStore, ChainSegmentResult,
-    ForkChoiceError, StateSkipConfig, WhenSlotSkipped, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
+    ForkChoiceError, HeadInfo, StateSkipConfig, WhenSlotSkipped, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
 };
 pub use self::beacon_snapshot::BeaconSnapshot;
 pub use self::chain_config::ChainConfig;

beacon_node/beacon_chain/src/test_utils.rs

@@ -46,8 +46,6 @@ use types::{
 
 // 4th September 2019
 pub const HARNESS_GENESIS_TIME: u64 = 1_567_552_690;
-// This parameter is required by a builder but not used because we use the `TestingSlotClock`.
-pub const HARNESS_SLOT_TIME: Duration = Duration::from_secs(1);
 // Environment variable to read if `fork_from_env` feature is enabled.
 const FORK_NAME_ENV_VAR: &str = "FORK_NAME";
 
@@ -182,6 +180,27 @@ impl<E: EthSpec> Builder<EphemeralHarnessType<E>> {
         self.store = Some(store);
         self.store_mutator(Box::new(mutator))
     }
+
+    /// Create a new ephemeral store that uses the specified `genesis_state`.
+    pub fn genesis_state_ephemeral_store(mut self, genesis_state: BeaconState<E>) -> Self {
+        let spec = self.spec.as_ref().expect("cannot build without spec");
+
+        let store = Arc::new(
+            HotColdDB::open_ephemeral(
+                self.store_config.clone().unwrap_or_default(),
+                spec.clone(),
+                self.log.clone(),
+            )
+            .unwrap(),
+        );
+        let mutator = move |builder: BeaconChainBuilder<_>| {
+            builder
+                .genesis_state(genesis_state)
+                .expect("should build state using recent genesis")
+        };
+        self.store = Some(store);
+        self.store_mutator(Box::new(mutator))
+    }
 }
 
 impl<E: EthSpec> Builder<DiskHarnessType<E>> {
@@ -297,6 +316,7 @@ where
 
         let log = test_logger();
         let spec = self.spec.expect("cannot build without spec");
+        let seconds_per_slot = spec.seconds_per_slot;
         let validator_keypairs = self
             .validator_keypairs
             .expect("cannot build without validator keypairs");
@@ -331,7 +351,7 @@ where
         // Initialize the slot clock only if it hasn't already been initialized.
         builder = if builder.get_slot_clock().is_none() {
             builder
-                .testing_slot_clock(HARNESS_SLOT_TIME)
+                .testing_slot_clock(Duration::from_secs(seconds_per_slot))
                 .expect("should configure testing slot clock")
         } else {
             builder

beacon_node/beacon_chain/tests/store_tests.rs

@@ -4,7 +4,6 @@ use beacon_chain::attestation_verification::Error as AttnError;
 use beacon_chain::builder::BeaconChainBuilder;
 use beacon_chain::test_utils::{
     test_spec, AttestationStrategy, BeaconChainHarness, BlockStrategy, DiskHarnessType,
-    HARNESS_SLOT_TIME,
 };
 use beacon_chain::{
     historical_blocks::HistoricalBlockError, migrate::MigratorConfig, BeaconChain,
@@ -19,6 +18,7 @@ use std::collections::HashMap;
 use std::collections::HashSet;
 use std::convert::TryInto;
 use std::sync::Arc;
+use std::time::Duration;
 use store::{
     iter::{BlockRootsIterator, StateRootsIterator},
     HotColdDB, LevelDB, StoreConfig,
@@ -1812,6 +1812,8 @@ fn weak_subjectivity_sync() {
     let log = test_logger();
     let temp2 = tempdir().unwrap();
     let store = get_store(&temp2);
+    let spec = test_spec::<E>();
+    let seconds_per_slot = spec.seconds_per_slot;
 
     // Initialise a new beacon chain from the finalized checkpoint
     let beacon_chain = BeaconChainBuilder::new(MinimalEthSpec)
@@ -1823,7 +1825,7 @@ fn weak_subjectivity_sync() {
         .store_migrator_config(MigratorConfig::default().blocking())
         .dummy_eth1_backend()
         .expect("should build dummy backend")
-        .testing_slot_clock(HARNESS_SLOT_TIME)
+        .testing_slot_clock(Duration::from_secs(seconds_per_slot))
         .expect("should configure testing slot clock")
         .shutdown_sender(shutdown_tx)
         .chain_config(ChainConfig::default())
@@ -2055,6 +2057,8 @@ fn revert_minority_fork_on_resume() {
     let mut spec2 = MinimalEthSpec::default_spec();
     spec2.altair_fork_epoch = Some(fork_epoch);
 
+    let seconds_per_slot = spec1.seconds_per_slot;
+
     let all_validators = (0..validator_count).collect::<Vec<usize>>();
 
     // Chain with no fork epoch configured.
@@ -2160,7 +2164,7 @@ fn revert_minority_fork_on_resume() {
             builder = builder
                 .resume_from_db()
                 .unwrap()
-                .testing_slot_clock(HARNESS_SLOT_TIME)
+                .testing_slot_clock(Duration::from_secs(seconds_per_slot))
                 .unwrap();
             builder
                 .get_slot_clock()

consensus/fork_choice/src/fork_choice.rs

@@ -3,18 +3,13 @@ use std::marker::PhantomData;
 use proto_array::{Block as ProtoBlock, ProtoArrayForkChoice};
 use ssz_derive::{Decode, Encode};
 use types::{
-    AttestationShufflingId, BeaconBlock, BeaconState, BeaconStateError, Checkpoint, Epoch, EthSpec,
-    Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot,
+    AttestationShufflingId, BeaconBlock, BeaconState, BeaconStateError, ChainSpec, Checkpoint,
+    Epoch, EthSpec, Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot,
 };
 
 use crate::ForkChoiceStore;
 use std::cmp::Ordering;
 
-/// Defined here:
-///
-/// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#configuration
-pub const SAFE_SLOTS_TO_UPDATE_JUSTIFIED: u64 = 8;
-
 #[derive(Debug)]
 pub enum Error<T> {
     InvalidAttestation(InvalidAttestation),
@@ -379,13 +374,14 @@ where
         &mut self,
         current_slot: Slot,
         state: &BeaconState<E>,
+        spec: &ChainSpec,
     ) -> Result<bool, Error<T::Error>> {
         self.update_time(current_slot)?;
 
         let new_justified_checkpoint = &state.current_justified_checkpoint();
 
         if compute_slots_since_epoch_start::<E>(self.fc_store.get_current_slot())
-            < SAFE_SLOTS_TO_UPDATE_JUSTIFIED
+            < spec.safe_slots_to_update_justified
         {
             return Ok(true);
         }
@@ -442,6 +438,7 @@ where
         block: &BeaconBlock<E>,
         block_root: Hash256,
         state: &BeaconState<E>,
+        spec: &ChainSpec,
     ) -> Result<(), Error<T::Error>> {
         let current_slot = self.update_time(current_slot)?;
 
@@ -500,7 +497,7 @@ where
                 self.fc_store
                     .set_best_justified_checkpoint(state.current_justified_checkpoint());
             }
-            if self.should_update_justified_checkpoint(current_slot, state)? {
+            if self.should_update_justified_checkpoint(current_slot, state, spec)? {
                 self.fc_store
                     .set_justified_checkpoint(state.current_justified_checkpoint())
                     .map_err(Error::UnableToSetJustifiedCheckpoint)?;
@@ -797,6 +794,21 @@ where
         *self.fc_store.finalized_checkpoint()
     }
 
+    /// Return the justified checkpoint.
+    pub fn justified_checkpoint(&self) -> Checkpoint {
+        *self.fc_store.justified_checkpoint()
+    }
+
+    /// Return the best justified checkpoint.
+    ///
+    /// ## Warning
+    ///
+    /// This is distinct to the "justified checkpoint" or the "current justified checkpoint". This
+    /// "best justified checkpoint" value should only be used internally or for testing.
+    pub fn best_justified_checkpoint(&self) -> Checkpoint {
+        *self.fc_store.best_justified_checkpoint()
+    }
+
     /// Returns the latest message for a given validator, if any.
     ///
     /// Returns `(block_root, block_slot)`.

consensus/fork_choice/src/lib.rs

@@ -3,7 +3,6 @@ mod fork_choice_store;
 
 pub use crate::fork_choice::{
     Error, ForkChoice, InvalidAttestation, InvalidBlock, PersistedForkChoice, QueuedAttestation,
-    SAFE_SLOTS_TO_UPDATE_JUSTIFIED,
 };
 pub use fork_choice_store::ForkChoiceStore;
 pub use proto_array::Block as ProtoBlock;

consensus/fork_choice/tests/tests.rs

@@ -10,14 +10,12 @@ use beacon_chain::{
     BeaconChain, BeaconChainError, BeaconForkChoiceStore, ChainConfig, ForkChoiceError,
     StateSkipConfig, WhenSlotSkipped,
 };
-use fork_choice::{
-    ForkChoiceStore, InvalidAttestation, InvalidBlock, QueuedAttestation,
-    SAFE_SLOTS_TO_UPDATE_JUSTIFIED,
-};
+use fork_choice::{ForkChoiceStore, InvalidAttestation, InvalidBlock, QueuedAttestation};
 use store::MemoryStore;
 use types::{
     test_utils::generate_deterministic_keypair, BeaconBlock, BeaconBlockRef, BeaconState,
-    Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation, MainnetEthSpec, Slot, SubnetId,
+    ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation, MainnetEthSpec, Slot,
+    SubnetId,
 };
 
 pub type E = MainnetEthSpec;
@@ -232,15 +230,15 @@ impl ForkChoiceTest {
 
     /// Moves to the next slot that is *outside* the `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` range.
     pub fn move_outside_safe_to_update(self) -> Self {
-        while is_safe_to_update(self.harness.chain.slot().unwrap()) {
+        while is_safe_to_update(self.harness.chain.slot().unwrap(), &self.harness.chain.spec) {
             self.harness.advance_slot()
         }
         self
     }
 
     /// Moves to the next slot that is *inside* the `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` range.
     pub fn move_inside_safe_to_update(self) -> Self {
-        while !is_safe_to_update(self.harness.chain.slot().unwrap()) {
+        while !is_safe_to_update(self.harness.chain.slot().unwrap(), &self.harness.chain.spec) {
             self.harness.advance_slot()
         }
         self
@@ -270,7 +268,13 @@ impl ForkChoiceTest {
             .chain
             .fork_choice
             .write()
-            .on_block(current_slot, &block, block.canonical_root(), &state)
+            .on_block(
+                current_slot,
+                &block,
+                block.canonical_root(),
+                &state,
+                &self.harness.chain.spec,
+            )
             .unwrap();
         self
     }
@@ -305,7 +309,13 @@ impl ForkChoiceTest {
             .chain
             .fork_choice
             .write()
-            .on_block(current_slot, &block, block.canonical_root(), &state)
+            .on_block(
+                current_slot,
+                &block,
+                block.canonical_root(),
+                &state,
+                &self.harness.chain.spec,
+            )
             .err()
             .expect("on_block did not return an error");
         comparison_func(err);
@@ -458,8 +468,8 @@ impl ForkChoiceTest {
     }
 }
 
-fn is_safe_to_update(slot: Slot) -> bool {
-    slot % E::slots_per_epoch() < SAFE_SLOTS_TO_UPDATE_JUSTIFIED
+fn is_safe_to_update(slot: Slot, spec: &ChainSpec) -> bool {
+    slot % E::slots_per_epoch() < spec.safe_slots_to_update_justified
 }
 
 /// - The new justified checkpoint descends from the current.

testing/ef_tests/Cargo.toml

@@ -32,3 +32,5 @@ swap_or_not_shuffle = { path = "../../consensus/swap_or_not_shuffle" }
 types = { path = "../../consensus/types" }
 snap = "1.0.1"
 fs2 = "0.4.3"
+beacon_chain = { path = "../../beacon_node/beacon_chain" }
+store = { path = "../../beacon_node/store" }

testing/ef_tests/check_all_files_accessed.py

@@ -38,11 +38,6 @@
     # LightClientSnapshot
     "tests/minimal/altair/ssz_static/LightClientSnapshot",
     "tests/mainnet/altair/ssz_static/LightClientSnapshot",
-    # Fork choice
-    "tests/mainnet/phase0/fork_choice",
-    "tests/minimal/phase0/fork_choice",
-    "tests/mainnet/altair/fork_choice",
-    "tests/minimal/altair/fork_choice",
     # Merkle-proof tests for light clients
     "tests/mainnet/altair/merkle/single_proof/pyspec_tests/",
     "tests/minimal/altair/merkle/single_proof/pyspec_tests/"

testing/ef_tests/src/cases.rs

@@ -14,6 +14,7 @@ mod bls_verify_msg;
 mod common;
 mod epoch_processing;
 mod fork;
+mod fork_choice;
 mod genesis_initialization;
 mod genesis_validity;
 mod operations;
@@ -35,6 +36,7 @@ pub use bls_verify_msg::*;
 pub use common::SszStaticType;
 pub use epoch_processing::*;
 pub use fork::ForkTest;
+pub use fork_choice::*;
 pub use genesis_initialization::*;
 pub use genesis_validity::*;
 pub use operations::*;