Merge branch 'fix-fork-choice' into interop

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -745,8 +745,19 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         } else {
             // Provide the attestation to fork choice, updating the validator latest messages but
             // _without_ finding and updating the head.
-            self.fork_choice
+            if let Err(e) = self
-                .process_attestation(&state, &attestation, block)?;
+                .fork_choice
+                .process_attestation(&state, &attestation, block)
+            {
+                error!(
+                    self.log,
+                    "Add attestation to fork choice failed";
+                    "fork_choice_integrity" => format!("{:?}", self.fork_choice.verify_integrity()),
+                    "beacon_block_root" =>  format!("{}", attestation.data.beacon_block_root),
+                    "error" => format!("{:?}", e)
+                );
+                return Err(e.into());
+            }
 
             // Provide the valid attestation to op pool, which may choose to retain the
             // attestation for inclusion in a future block.
@@ -953,10 +964,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         if let Err(e) = self.fork_choice.process_block(&state, &block, block_root) {
             error!(
                 self.log,
-                "fork choice failed to process_block";
+                "Add block to fork choice failed";
-                "error" => format!("{:?}", e),
+                "fork_choice_integrity" => format!("{:?}", self.fork_choice.verify_integrity()),
                 "block_root" =>  format!("{}", block_root),
-                "block_slot" => format!("{}", block.slot)
+                "error" => format!("{:?}", e),
             )
         }
 

beacon_node/beacon_chain/src/fork_choice.rs

@@ -199,6 +199,14 @@ impl<T: BeaconChainTypes> ForkChoice<T> {
         self.backend.latest_message(validator_index)
     }
 
+    /// Runs an integrity verification function on the underlying fork choice algorithm.
+    ///
+    /// Returns `Ok(())` if the underlying fork choice has maintained it's integrity,
+    /// `Err(description)` otherwise.
+    pub fn verify_integrity(&self) -> core::result::Result<(), String> {
+        self.backend.verify_integrity()
+    }
+
     /// Inform the fork choice that the given block (and corresponding root) have been finalized so
     /// it may prune it's storage.
     ///

eth2/lmd_ghost/src/lib.rs

@@ -46,4 +46,10 @@ pub trait LmdGhost<S: Store, E: EthSpec>: Send + Sync {
 
     /// Returns the latest message for a given validator index.
     fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Slot)>;
+
+    /// Runs an integrity verification function on fork choice algorithm.
+    ///
+    /// Returns `Ok(())` if the underlying fork choice has maintained it's integrity,
+    /// `Err(description)` otherwise.
+    fn verify_integrity(&self) -> Result<()>;
 }

eth2/lmd_ghost/src/reduced_tree.rs

@@ -43,16 +43,6 @@ impl<T, E> fmt::Debug for ThreadSafeReducedTree<T, E> {
     }
 }
 
-impl<T, E> ThreadSafeReducedTree<T, E>
-where
-    T: Store,
-    E: EthSpec,
-{
-    pub fn verify_integrity(&self) -> std::result::Result<(), String> {
-        self.core.read().verify_integrity()
-    }
-}
-
 impl<T, E> LmdGhost<T, E> for ThreadSafeReducedTree<T, E>
 where
     T: Store,
@@ -80,7 +70,7 @@ where
     fn process_block(&self, block: &BeaconBlock<E>, block_hash: Hash256) -> SuperResult<()> {
         self.core
             .write()
-            .add_weightless_node(block.slot, block_hash)
+            .maybe_add_weightless_node(block.slot, block_hash)
             .map_err(|e| format!("process_block failed: {:?}", e))
     }
 
@@ -113,6 +103,10 @@ where
     fn latest_message(&self, validator_index: usize) -> Option<(Hash256, Slot)> {
         self.core.read().latest_message(validator_index)
     }
+
+    fn verify_integrity(&self) -> std::result::Result<(), String> {
+        self.core.read().verify_integrity()
+    }
 }
 
 struct ReducedTree<T, E> {
@@ -163,15 +157,7 @@ where
     /// The given `new_root` must be in the block tree (but not necessarily in the reduced tree).
     /// Any nodes which are not a descendant of `new_root` will be removed from the store.
     pub fn update_root(&mut self, new_slot: Slot, new_root: Hash256) -> Result<()> {
-        if !self.nodes.contains_key(&new_root) {
+        self.maybe_add_weightless_node(new_slot, new_root)?;
-            let node = Node {
-                block_hash: new_root,
-                voters: vec![],
-                ..Node::default()
-            };
-
-            self.add_node(node)?;
-        }
 
         self.retain_subtree(self.root.0, new_root)?;
 
@@ -247,7 +233,7 @@ where
         //
         // In this case, we add a weightless node at `start_block_root`.
         if !self.nodes.contains_key(&start_block_root) {
-            self.add_weightless_node(start_block_slot, start_block_root)?;
+            self.maybe_add_weightless_node(start_block_slot, start_block_root)?;
         };
 
         let _root_weight = self.update_weight(start_block_root, weight_fn)?;
@@ -430,7 +416,7 @@ where
         Ok(())
     }
 
-    fn add_weightless_node(&mut self, slot: Slot, hash: Hash256) -> Result<()> {
+    fn maybe_add_weightless_node(&mut self, slot: Slot, hash: Hash256) -> Result<()> {
         if slot > self.root_slot() && !self.nodes.contains_key(&hash) {
             let node = Node {
                 block_hash: hash,