- pass thrugh spec argument

- parse discard_equivocations test

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1919,6 +1919,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 self.slot()?,
                 verified.indexed_attestation(),
                 AttestationFromBlock::False,
+                &self.spec,
             )
             .map_err(Into::into)
     }
@@ -2701,6 +2702,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 current_slot,
                 &indexed_attestation,
                 AttestationFromBlock::True,
+                &self.spec,
             ) {
                 Ok(()) => Ok(()),
                 // Ignore invalid attestations whilst importing attestations from a block. The

beacon_node/beacon_chain/tests/tests.rs

@@ -457,7 +457,7 @@ fn unaggregated_attestations_added_to_fork_choice_some_none() {
     // Move forward a slot so all queued attestations can be processed.
     harness.advance_slot();
     fork_choice
-        .update_time(harness.chain.slot().unwrap())
+        .update_time(harness.chain.slot().unwrap(), &MinimalEthSpec)
         .unwrap();
 
     let validator_slots: Vec<(usize, Slot)> = (0..VALIDATOR_COUNT)
@@ -566,7 +566,7 @@ fn unaggregated_attestations_added_to_fork_choice_all_updated() {
     // Move forward a slot so all queued attestations can be processed.
     harness.advance_slot();
     fork_choice
-        .update_time(harness.chain.slot().unwrap())
+        .update_time(harness.chain.slot().unwrap(), MinimalEthSpec)
         .unwrap();
 
     let validators: Vec<usize> = (0..VALIDATOR_COUNT).collect();

consensus/fork_choice/src/fork_choice.rs

@@ -406,7 +406,7 @@ where
         current_slot: Slot,
         spec: &ChainSpec,
     ) -> Result<Hash256, Error<T::Error>> {
-        self.update_time(current_slot)?;
+        self.update_time(current_slot, spec)?;
 
         let store = &mut self.fc_store;
 
@@ -451,8 +451,7 @@ where
         current_slot: Slot,
         spec: &ChainSpec,
     ) -> Result<bool, Error<T::Error>> {
-        //TODO(sean) update_time -> on_tick -> update_checkpoints -> should_update_justified_checkpoint -> update_time
-        self.update_time(current_slot)?;
+        self.update_time(current_slot, spec)?;
 
         if compute_slots_since_epoch_start::<E>(self.fc_store.get_current_slot())
             < spec.safe_slots_to_update_justified
@@ -537,7 +536,7 @@ where
         payload_verification_status: PayloadVerificationStatus,
         spec: &ChainSpec,
     ) -> Result<(), Error<T::Error>> {
-        let current_slot = self.update_time(current_slot)?;
+        let current_slot = self.update_time(current_slot, spec)?;
 
         // Parent block must be known.
         if !self.proto_array.contains_block(&block.parent_root()) {
@@ -901,9 +900,10 @@ where
         current_slot: Slot,
         attestation: &IndexedAttestation<E>,
         is_from_block: AttestationFromBlock,
+        spec: &ChainSpec,
     ) -> Result<(), Error<T::Error>> {
         // Ensure the store is up-to-date.
-        self.update_time(current_slot)?;
+        self.update_time(current_slot, spec)?;
 
         // Ignore any attestations to the zero hash.
         //
@@ -948,17 +948,16 @@ where
 
     /// Call `on_tick` for all slots between `fc_store.get_current_slot()` and the provided
     /// `current_slot`. Returns the value of `self.fc_store.get_current_slot`.
-    pub fn update_time(&mut self, current_slot: Slot) -> Result<Slot, Error<T::Error>> {
+    pub fn update_time(
+        &mut self,
+        current_slot: Slot,
+        spec: &ChainSpec,
+    ) -> Result<Slot, Error<T::Error>> {
         while self.fc_store.get_current_slot() < current_slot {
             let previous_slot = self.fc_store.get_current_slot();
             // Note: we are relying upon `on_tick` to update `fc_store.time` to ensure we don't
             // get stuck in a loop.
-            //TODO(sean) fix chain spec
-            dbg!(&previous_slot);
-            if (previous_slot) % MainnetEthSpec::slots_per_epoch() == 0 {
-                dbg!("hitting epoch boundary");
-            }
-            self.on_tick(previous_slot + 1, &ChainSpec::mainnet())?
+            self.on_tick(previous_slot + 1, spec)?
         }
 
         // Process any attestations that might now be eligible.

consensus/fork_choice/tests/tests.rs

@@ -152,7 +152,7 @@ impl ForkChoiceTest {
             .chain
             .fork_choice
             .write()
-            .update_time(self.harness.chain.slot().unwrap())
+            .update_time(self.harness.chain.slot().unwrap(), &self.harness.spec)
             .unwrap();
         func(self.harness.chain.fork_choice.read().queued_attestations());
         self

consensus/proto_array/src/proto_array.rs

@@ -5,7 +5,10 @@ use ssz::four_byte_option_impl;
 use ssz::Encode;
 use ssz_derive::{Decode, Encode};
 use std::collections::{HashMap, HashSet};
-use types::{AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, MainnetEthSpec, Slot};
+use types::{
+    AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256,
+    MainnetEthSpec, Slot,
+};
 
 // Define a "legacy" implementation of `Option<usize>` which uses four bytes for encoding the union
 // selector.
@@ -281,7 +284,11 @@ impl ProtoArray {
 
             // If the node has a parent, try to update its best-child and best-descendant.
             if let Some(parent_index) = node.parent {
-                self.maybe_update_best_child_and_descendant(parent_index, node_index, current_slot)?;
+                self.maybe_update_best_child_and_descendant(
+                    parent_index,
+                    node_index,
+                    current_slot,
+                )?;
             }
         }
 
@@ -291,7 +298,7 @@ impl ProtoArray {
     /// Register a block with the fork choice.
     ///
     /// It is only sane to supply a `None` parent for the genesis block.
-    pub fn on_block(&mut self, block: Block, current_slot:Slot) -> Result<(), Error> {
+    pub fn on_block(&mut self, block: Block, current_slot: Slot) -> Result<(), Error> {
         // If the block is already known, simply ignore it.
         if self.indices.contains_key(&block.root) {
             return Ok(());
@@ -610,7 +617,11 @@ impl ProtoArray {
     /// been called without a subsequent `Self::apply_score_changes` call. This is because
     /// `on_new_block` does not attempt to walk backwards through the tree and update the
     /// best-child/best-descendant links.
-    pub fn find_head(&self, justified_root: &Hash256, current_slot: Slot) -> Result<Hash256, Error> {
+    pub fn find_head(
+        &self,
+        justified_root: &Hash256,
+        current_slot: Slot,
+    ) -> Result<Hash256, Error> {
         let justified_index = self
             .indices
             .get(justified_root)
@@ -743,7 +754,7 @@ impl ProtoArray {
         &mut self,
         parent_index: usize,
         child_index: usize,
-        current_slot: Slot
+        current_slot: Slot,
     ) -> Result<(), Error> {
         let child = self
             .nodes
@@ -755,7 +766,7 @@ impl ProtoArray {
             .get(parent_index)
             .ok_or(Error::InvalidNodeIndex(parent_index))?;
 
-        let child_leads_to_viable_head = self.node_leads_to_viable_head(child,current_slot)?;
+        let child_leads_to_viable_head = self.node_leads_to_viable_head(child, current_slot)?;
 
         // These three variables are aliases to the three options that we may set the
         // `parent.best_child` and `parent.best_descendant` to.
@@ -768,54 +779,54 @@ impl ProtoArray {
         );
         let no_change = (parent.best_child, parent.best_descendant);
 
-        let (new_best_child, new_best_descendant) = if let Some(best_child_index) =
-            parent.best_child
-        {
-            if best_child_index == child_index && !child_leads_to_viable_head {
-                // If the child is already the best-child of the parent but it's not viable for
-                // the head, remove it.
-                change_to_none
-            } else if best_child_index == child_index {
-                // If the child is the best-child already, set it again to ensure that the
-                // best-descendant of the parent is updated.
-                change_to_child
-            } else {
-                let best_child = self
-                    .nodes
-                    .get(best_child_index)
-                    .ok_or(Error::InvalidBestDescendant(best_child_index))?;
+        let (new_best_child, new_best_descendant) =
+            if let Some(best_child_index) = parent.best_child {
+                if best_child_index == child_index && !child_leads_to_viable_head {
+                    // If the child is already the best-child of the parent but it's not viable for
+                    // the head, remove it.
+                    change_to_none
+                } else if best_child_index == child_index {
+                    // If the child is the best-child already, set it again to ensure that the
+                    // best-descendant of the parent is updated.
+                    change_to_child
+                } else {
+                    let best_child = self
+                        .nodes
+                        .get(best_child_index)
+                        .ok_or(Error::InvalidBestDescendant(best_child_index))?;
 
-                let best_child_leads_to_viable_head = self.node_leads_to_viable_head(best_child, current_slot)?;
+                    let best_child_leads_to_viable_head =
+                        self.node_leads_to_viable_head(best_child, current_slot)?;
 
-                if child_leads_to_viable_head && !best_child_leads_to_viable_head {
-                    // The child leads to a viable head, but the current best-child doesn't.
-                    change_to_child
-                } else if !child_leads_to_viable_head && best_child_leads_to_viable_head {
-                    // The best child leads to a viable head, but the child doesn't.
-                    no_change
-                } else if child.weight == best_child.weight {
-                    // Tie-breaker of equal weights by root.
-                    if child.root >= best_child.root {
+                    if child_leads_to_viable_head && !best_child_leads_to_viable_head {
+                        // The child leads to a viable head, but the current best-child doesn't.
                         change_to_child
-                    } else {
+                    } else if !child_leads_to_viable_head && best_child_leads_to_viable_head {
+                        // The best child leads to a viable head, but the child doesn't.
                         no_change
-                    }
-                } else {
-                    // Choose the winner by weight.
-                    if child.weight >= best_child.weight {
-                        change_to_child
+                    } else if child.weight == best_child.weight {
+                        // Tie-breaker of equal weights by root.
+                        if child.root >= best_child.root {
+                            change_to_child
+                        } else {
+                            no_change
+                        }
                     } else {
-                        no_change
+                        // Choose the winner by weight.
+                        if child.weight >= best_child.weight {
+                            change_to_child
+                        } else {
+                            no_change
+                        }
                     }
                 }
-            }
-        } else if child_leads_to_viable_head {
-            // There is no current best-child and the child is viable.
-            change_to_child
-        } else {
-            // There is no current best-child but the child is not viable.
-            no_change
-        };
+            } else if child_leads_to_viable_head {
+                // There is no current best-child and the child is viable.
+                change_to_child
+            } else {
+                // There is no current best-child but the child is not viable.
+                no_change
+            };
 
         let parent = self
             .nodes
@@ -830,7 +841,11 @@ impl ProtoArray {
 
     /// Indicates if the node itself is viable for the head, or if it's best descendant is viable
     /// for the head.
-    fn node_leads_to_viable_head(&self, node: &ProtoNode, current_slot: Slot) -> Result<bool, Error> {
+    fn node_leads_to_viable_head(
+        &self,
+        node: &ProtoNode,
+        current_slot: Slot,
+    ) -> Result<bool, Error> {
         let best_descendant_is_viable_for_head =
             if let Some(best_descendant_index) = node.best_descendant {
                 let best_descendant = self
@@ -877,7 +892,8 @@ impl ProtoArray {
             node.justified_checkpoint,
             node.finalized_checkpoint,
         ) {
-            if node.slot.epoch(MainnetEthSpec::slots_per_epoch()) < current_slot.epoch(MainnetEthSpec::slots_per_epoch())
+            if node.slot.epoch(MainnetEthSpec::slots_per_epoch())
+                < current_slot.epoch(MainnetEthSpec::slots_per_epoch())
             {
                 checkpoint_match_predicate(
                     unrealized_justified_checkpoint,

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -246,7 +246,7 @@ impl ProtoArrayForkChoice {
         Ok(())
     }
 
-    pub fn process_block(&mut self, block: Block, current_slot:Slot) -> Result<(), String> {
+    pub fn process_block(&mut self, block: Block, current_slot: Slot) -> Result<(), String> {
         if block.parent_root.is_none() {
             return Err("Missing parent root".to_string());
         }

testing/ef_tests/src/cases/fork_choice.rs

@@ -14,8 +14,8 @@ use ssz_derive::Decode;
 use state_processing::state_advance::complete_state_advance;
 use std::time::Duration;
 use types::{
-    Attestation, BeaconBlock, BeaconState, Checkpoint, Epoch, EthSpec, ExecutionBlockHash,
-    ForkName, Hash256, IndexedAttestation, SignedBeaconBlock, Slot, Uint256,
+    Attestation, AttesterSlashing, BeaconBlock, BeaconState, Checkpoint, Epoch, EthSpec,
+    ExecutionBlockHash, ForkName, Hash256, IndexedAttestation, SignedBeaconBlock, Slot, Uint256,
 };
 
 #[derive(Default, Debug, PartialEq, Clone, Deserialize, Decode)]
@@ -50,12 +50,13 @@ pub struct Checks {
 
 #[derive(Debug, Clone, Deserialize)]
 #[serde(untagged, deny_unknown_fields)]
-pub enum Step<B, A, P> {
+pub enum Step<B, A, P, S> {
     Tick { tick: u64 },
     ValidBlock { block: B },
     MaybeValidBlock { block: B, valid: bool },
     Attestation { attestation: A },
     PowBlock { pow_block: P },
+    AttesterSlashing { attester_slashing: S },
     Checks { checks: Box<Checks> },
 }
 
@@ -71,7 +72,7 @@ pub struct ForkChoiceTest<E: EthSpec> {
     pub description: String,
     pub anchor_state: BeaconState<E>,
     pub anchor_block: BeaconBlock<E>,
-    pub steps: Vec<Step<SignedBeaconBlock<E>, Attestation<E>, PowBlock>>,
+    pub steps: Vec<Step<SignedBeaconBlock<E>, Attestation<E>, PowBlock, AttesterSlashing<E>>>,
 }
 
 /// Spec for fork choice tests, with proposer boosting enabled.
@@ -92,7 +93,8 @@ impl<E: EthSpec> LoadCase for ForkChoiceTest<E> {
             .expect("path must be valid OsStr")
             .to_string();
         let spec = &fork_choice_spec::<E>(fork_name);
-        let steps: Vec<Step<String, String, String>> = yaml_decode_file(&path.join("steps.yaml"))?;
+        let steps: Vec<Step<String, String, String, String>> =
+            yaml_decode_file(&path.join("steps.yaml"))?;
         // Resolve the object names in `steps.yaml` into actual decoded block/attestation objects.
         let steps = steps
             .into_iter()
@@ -118,6 +120,10 @@ impl<E: EthSpec> LoadCase for ForkChoiceTest<E> {
                     ssz_decode_file(&path.join(format!("{}.ssz_snappy", pow_block)))
                         .map(|pow_block| Step::PowBlock { pow_block })
                 }
+                Step::AttesterSlashing { attester_slashing } => {
+                    ssz_decode_file(&path.join(format!("{}.ssz_snappy", attester_slashing)))
+                        .map(|attester_slashing| Step::AttesterSlashing { attester_slashing })
+                }
                 Step::Checks { checks } => Ok(Step::Checks { checks }),
             })
             .collect::<Result<_, _>>()?;
@@ -159,6 +165,8 @@ impl<E: EthSpec> Case for ForkChoiceTest<E> {
         // This test is skipped until we can do retrospective confirmations of the terminal
         // block after an optimistic sync.
         if self.description == "block_lookup_failed"
+            //TODO(sean): enable once we implement equivocation logic (https://github.com/ethereum/consensus-specs/pull/2845)
+            || self.description == "discard_equivocations"
         {
             return Err(Error::SkippedKnownFailure);
         };
@@ -172,6 +180,10 @@ impl<E: EthSpec> Case for ForkChoiceTest<E> {
                 }
                 Step::Attestation { attestation } => tester.process_attestation(attestation)?,
                 Step::PowBlock { pow_block } => tester.process_pow_block(pow_block),
+                //TODO(sean): enable once we implement equivocation logic (https://github.com/ethereum/consensus-specs/pull/2845)
+                Step::AttesterSlashing {
+                    attester_slashing: _,
+                } => (),
                 Step::Checks { checks } => {
                     let Checks {
                         head,
@@ -328,7 +340,7 @@ impl<E: EthSpec> Tester<E> {
             .chain
             .fork_choice
             .write()
-            .update_time(slot)
+            .update_time(slot, &self.harness.spec)
             .unwrap();
     }
 

testing/ef_tests/src/handler.rs

@@ -468,10 +468,6 @@ impl<E: EthSpec + TypeName> Handler for ForkChoiceHandler<E> {
             return false;
         }
 
-        if ForkName::Merge != fork_name {
-            return false;
-        }
-
         // These tests check block validity (which may include signatures) and there is no need to
         // run them with fake crypto.
         cfg!(not(feature = "fake_crypto"))