From b2471eca494369e946fa212610af6b2c8be44802 Mon Sep 17 00:00:00 2001
From: Grant Wuerker <gwuerker@gmail.com>
Date: Tue, 23 Jul 2019 20:50:18 +0200
Subject: [PATCH] Checking if the an attestation contains a latest message

---
 beacon_node/beacon_chain/src/beacon_chain.rs | 19 ++++------------
 beacon_node/beacon_chain/src/fork_choice.rs  | 24 ++++++++++++++++++++
 eth2/lmd_ghost/src/lib.rs                    |  2 +-
 eth2/lmd_ghost/src/reduced_tree.rs           |  2 +-
 4 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index d02ab31763..f215465f2f 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -494,9 +494,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let timer = self.metrics.attestation_processing_times.start_timer();
 
         if let Some(state) = self.get_attestation_state(&attestation) {
-            let indexed_attestation = common::convert_to_indexed(&state, &attestation)?;
-            per_block_processing::verify_indexed_attestation(&state, &indexed_attestation, &self.spec)?;
-            self.fork_choice.process_attestation(&state, &attestation);
+            if self.fork_choice.should_process_attestation(&state, &attestation) {
+                let indexed_attestation = common::convert_to_indexed(&state, &attestation)?;
+                per_block_processing::verify_indexed_attestation(&state, &indexed_attestation, &self.spec)?;
+                self.fork_choice.process_attestation(&state, &attestation);
+            }
         }
 
         let result = self
@@ -509,17 +511,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             self.metrics.attestation_processing_successes.inc();
         }
 
-        // TODO: process attestation. Please consider:
-        //
-        //  - Because a block was not added to the op pool does not mean it's invalid (it might
-        //  just be old).
-        //  - The attestation should be rejected if we don't know the block (ideally it should be
-        //  queued, but this may be overkill).
-        //  - The attestation _must_ be validated against it's state before being added to fork
-        //  choice.
-        //  - You can avoid verifying some attestations by first checking if they're a latest
-        //  message. This would involve expanding the `LmdGhost` API.
-
         result
     }
 
diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs
index cdda563868..6b69e3e084 100644
--- a/beacon_node/beacon_chain/src/fork_choice.rs
+++ b/beacon_node/beacon_chain/src/fork_choice.rs
@@ -4,6 +4,7 @@ use state_processing::common::get_attesting_indices_unsorted;
 use std::sync::Arc;
 use store::{Error as StoreError, Store};
 use types::{Attestation, BeaconBlock, BeaconState, BeaconStateError, Epoch, EthSpec, Hash256};
+use state_processing::common;
 
 type Result<T> = std::result::Result<T, Error>;
 
@@ -120,6 +121,9 @@ impl<T: BeaconChainTypes> ForkChoice<T> {
         Ok(())
     }
 
+    /// Process an attestation.
+    ///
+    /// Assumes the attestation is valid.
     pub fn process_attestation(
         &self,
         state: &BeaconState<T::EthSpec>,
@@ -162,6 +166,26 @@ impl<T: BeaconChainTypes> ForkChoice<T> {
         Ok(())
     }
 
+    /// Determines whether or not the given attestation contains a latest messages.
+    pub fn should_process_attestation(&self, state: &BeaconState<T::EthSpec>, attestation: &Attestation) -> bool {
+        let validator_indices = common::get_attesting_indices_unsorted(
+            state,
+            &attestation.data,
+            &attestation.aggregation_bitfield,
+        ).unwrap();
+
+        let target_slot = attestation.data.target_epoch.start_slot(T::EthSpec::slots_per_epoch());
+
+        validator_indices
+            .iter()
+            .find(|&&v| {
+                match self.backend.latest_message(v) {
+                    Some((_, slot)) => target_slot > slot,
+                    None => true
+                }
+            }).is_some()
+    }
+
     /// Inform the fork choice that the given block (and corresponding root) have been finalized so
     /// it may prune it's storage.
     ///
diff --git a/eth2/lmd_ghost/src/lib.rs b/eth2/lmd_ghost/src/lib.rs
index f18b5b81f7..183d45c9a4 100644
--- a/eth2/lmd_ghost/src/lib.rs
+++ b/eth2/lmd_ghost/src/lib.rs
@@ -45,5 +45,5 @@ pub trait LmdGhost<S: Store, E: EthSpec>: Send + Sync {
     ) -> Result<()>;
 
     /// Returns the latest message for a given validator index.
-    fn latest_message(&mut self, validator_index: usize) -> Option<(Hash256, Slot)>;
+    fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Slot)>;
 }
diff --git a/eth2/lmd_ghost/src/reduced_tree.rs b/eth2/lmd_ghost/src/reduced_tree.rs
index f069ae68c5..0985441dfc 100644
--- a/eth2/lmd_ghost/src/reduced_tree.rs
+++ b/eth2/lmd_ghost/src/reduced_tree.rs
@@ -88,7 +88,7 @@ where
             .map_err(|e| format!("update_finalized_root failed: {:?}", e))
     }
 
-    fn latest_message(&mut self, validator_index: usize) -> Option<(Hash256, Slot)> {
+    fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Slot)> {
         self.core
             .write()
             .latest_message(validator_index)