Updates optimised fork choice.

- Bug fixes in optimised fork choice.
- YAML tests functioning.
- Implement Clippy suggestions.
- Remove Copywrite notices.

eth2/fork_choice/Cargo.toml

@@ -9,8 +9,8 @@ db = { path = "../../beacon_node/db" }
 ssz = { path = "../utils/ssz" }
 types = { path = "../types" }
 fast-math = "0.1.1"
-byteorder = "1.3.1"
 log = "0.4.6"
+bit-vec = "0.5.0"
 
 [dev-dependencies]
 yaml-rust = "0.4.2"

eth2/fork_choice/src/lib.rs

@@ -1,23 +1,3 @@
-// Copyright 2019 Sigma Prime Pty Ltd.
-//
-// Permission is hereby granted, free of charge, to any person obtaining a
-// copy of this software and associated documentation files (the "Software"),
-// to deal in the Software without restriction, including without limitation
-// the rights to use, copy, modify, merge, publish, distribute, sublicense,
-// and/or sell copies of the Software, and to permit persons to whom the
-// Software is furnished to do so, subject to the following conditions:
-//
-// The above copyright notice and this permission notice shall be included in
-// all copies or substantial portions of the Software.
-//
-// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
-// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
-// DEALINGS IN THE SOFTWARE.
-
 //! This crate stores the various implementations of fork-choice rules that can be used for the
 //! beacon blockchain.
 //!

eth2/fork_choice/src/longest_chain.rs

@@ -30,7 +30,7 @@ impl<T: ClientDB + Sized> ForkChoice for LongestChain<T> {
         &mut self,
         block: &BeaconBlock,
         block_hash: &Hash256,
-        spec: &ChainSpec,
+        _: &ChainSpec,
     ) -> Result<(), ForkChoiceError> {
         // add the block hash to head_block_hashes removing the parent if it exists
         self.head_block_hashes

eth2/fork_choice/src/optimised_lmd_ghost.rs

@@ -1,7 +1,8 @@
-extern crate byteorder;
+extern crate bit_vec;
 extern crate fast_math;
+
 use crate::{ForkChoice, ForkChoiceError};
-use byteorder::{BigEndian, ByteOrder};
+use bit_vec::BitVec;
 use db::{
     stores::{BeaconBlockStore, BeaconStateStore},
     ClientDB,
@@ -25,6 +26,9 @@ use types::{
 // the comparison. Log2_raw takes 2ns according to the documentation.
 #[inline]
 fn log2_int(x: u32) -> u32 {
+    if x == 0 {
+        return 0;
+    }
     log2_raw(x as f32) as u32
 }
 
@@ -76,14 +80,13 @@ where
     pub fn get_latest_votes(
         &self,
         state_root: &Hash256,
-        block_slot: &Slot,
+        block_slot: Slot,
         spec: &ChainSpec,
     ) -> Result<HashMap<Hash256, u64>, ForkChoiceError> {
         // get latest votes
         // Note: Votes are weighted by min(balance, MAX_DEPOSIT_AMOUNT) //
         // FORK_CHOICE_BALANCE_INCREMENT
         // build a hashmap of block_hash to weighted votes
-        trace!("FORKCHOICE: Getting the latest votes");
         let mut latest_votes: HashMap<Hash256, u64> = HashMap::new();
         // gets the current weighted votes
         let current_state = self
@@ -95,10 +98,6 @@ where
             &current_state.validator_registry[..],
             block_slot.epoch(spec.epoch_length),
         );
-        trace!(
-            "FORKCHOICE: Active validator indicies: {:?}",
-            active_validator_indices
-        );
 
         for index in active_validator_indices {
             let balance = std::cmp::min(
@@ -119,7 +118,7 @@ where
     fn get_ancestor(
         &mut self,
         block_hash: Hash256,
-        at_height: SlotHeight,
+        target_height: SlotHeight,
         spec: &ChainSpec,
     ) -> Option<Hash256> {
         // return None if we can't get the block from the db.
@@ -131,32 +130,31 @@ where
                 .expect("Should have returned already if None")
                 .slot;
 
-            block_slot.height(Slot::from(spec.genesis_slot))
+            block_slot.height(spec.genesis_slot)
         };
 
         // verify we haven't exceeded the block height
-        if at_height >= block_height {
+        if target_height >= block_height {
-            if at_height > block_height {
+            if target_height > block_height {
                 return None;
             } else {
                 return Some(block_hash);
             }
         }
         // check if the result is stored in our cache
-        let cache_key = CacheKey::new(&block_hash, at_height.as_u32());
+        let cache_key = CacheKey::new(&block_hash, target_height.as_u32());
         if let Some(ancestor) = self.cache.get(&cache_key) {
             return Some(*ancestor);
         }
 
         // not in the cache recursively search for ancestors using a log-lookup
-
         if let Some(ancestor) = {
             let ancestor_lookup = self.ancestors
-                [log2_int((block_height - at_height - 1u64).as_u32()) as usize]
+                [log2_int((block_height - target_height - 1u64).as_u32()) as usize]
                 .get(&block_hash)
                 //TODO: Panic if we can't lookup and fork choice fails
                 .expect("All blocks should be added to the ancestor log lookup table");
-            self.get_ancestor(*ancestor_lookup, at_height, &spec)
+            self.get_ancestor(*ancestor_lookup, target_height, &spec)
         } {
             // add the result to the cache
             self.cache.insert(cache_key, ancestor);
@@ -177,6 +175,7 @@ where
         let mut current_votes: HashMap<Hash256, u64> = HashMap::new();
         let mut total_vote_count = 0;
 
+        trace!("FORKCHOICE: Clear winner at block height: {}", block_height);
         // loop through the latest votes and count all votes
         // these have already been weighted by balance
         for (hash, votes) in latest_votes.iter() {
@@ -199,19 +198,30 @@ where
 
     // Finds the best child, splitting children into a binary tree, based on their hashes
     fn choose_best_child(&self, votes: &HashMap<Hash256, u64>) -> Option<Hash256> {
-        println!("Votes: {:?}", votes);
+        if votes.is_empty() {
-        let mut bitmask = 0;
+            return None;
-        for bit in (0..=255).rev() {
+        }
+        let mut bitmask: BitVec = BitVec::new();
+        // loop through bytes then bits
+        for bit in 0..256 {
             let mut zero_votes = 0;
             let mut one_votes = 0;
             let mut single_candidate = None;
 
             for (candidate, votes) in votes.iter() {
-                let candidate_uint = BigEndian::read_u32(candidate);
+                let candidate_bit: BitVec = BitVec::from_bytes(&candidate);
-                if candidate_uint >> (bit + 1) != bitmask {
+
+                // if the bitmasks don't match
+                if !bitmask.iter().eq(candidate_bit.iter().take(bit)) {
+                    trace!(
+                        "FORKCHOICE: Child: {} was removed in bit: {} with the bitmask: {:?}",
+                        candidate,
+                        bit,
+                        bitmask
+                    );
                     continue;
                 }
-                if (candidate_uint >> bit) % 2 == 0 {
+                if candidate_bit.get(bit) == Some(false) {
                     zero_votes += votes;
                 } else {
                     one_votes += votes;
@@ -223,18 +233,10 @@ where
                     single_candidate = None;
                 }
             }
-            bitmask = (bitmask * 2) + {
+            bitmask.push(one_votes > zero_votes);
-                if one_votes > zero_votes {
-                    1
-                } else {
-                    0
-                }
-            };
             if let Some(candidate) = single_candidate {
                 return Some(*candidate);
             }
-            //TODO Remove this during benchmark after testing
-            assert!(bit >= 1);
         }
         // should never reach here
         None
@@ -254,7 +256,7 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
             .get_deserialized(&block.parent_root)?
             .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(block.parent_root))?
             .slot()
-            .height(Slot::from(spec.genesis_slot));
+            .height(spec.genesis_slot);
 
         let parent_hash = &block.parent_root;
 
@@ -289,7 +291,7 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
         // simply add the attestation to the latest_attestation_target if the block_height is
         // larger
         trace!(
-            "FORKCHOICE: Adding attestation of validator: {:?} for block: {:?}",
+            "FORKCHOICE: Adding attestation of validator: {:?} for block: {}",
             validator_index,
             target_block_root
         );
@@ -309,7 +311,7 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
                 .get_deserialized(&target_block_root)?
                 .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*target_block_root))?
                 .slot()
-                .height(Slot::from(spec.genesis_slot));
+                .height(spec.genesis_slot);
 
             // get the height of the past target block
             let past_block_height = self
@@ -317,9 +319,7 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
                 .get_deserialized(&attestation_target)?
                 .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*attestation_target))?
                 .slot()
-                .height(Slot::from(spec.genesis_slot));
+                .height(spec.genesis_slot);
-            trace!("FORKCHOICE: Old block height: {:?}", past_block_height);
-            trace!("FORKCHOICE: New block height: {:?}", block_height);
             // update the attestation only if the new target is higher
             if past_block_height < block_height {
                 trace!("FORKCHOICE: Updating old attestation");
@@ -335,27 +335,34 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
         justified_block_start: &Hash256,
         spec: &ChainSpec,
     ) -> Result<Hash256, ForkChoiceError> {
-        trace!("Starting optimised fork choice");
+        debug!(
+            "Starting optimised fork choice at block: {}",
+            justified_block_start
+        );
         let block = self
             .block_store
             .get_deserialized(&justified_block_start)?
             .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*justified_block_start))?;
 
         let block_slot = block.slot();
-        let block_height = block_slot.height(Slot::from(spec.genesis_slot));
         let state_root = block.state_root();
+        let mut block_height = block_slot.height(spec.genesis_slot);
 
         let mut current_head = *justified_block_start;
 
-        let mut latest_votes = self.get_latest_votes(&state_root, &block_slot, spec)?;
+        let mut latest_votes = self.get_latest_votes(&state_root, block_slot, spec)?;
 
         // remove any votes that don't relate to our current head.
         latest_votes
             .retain(|hash, _| self.get_ancestor(*hash, block_height, spec) == Some(current_head));
-        trace!("FORKCHOICE: Latest votes: {:?}", latest_votes);
 
         // begin searching for the head
         loop {
+            debug!(
+                "FORKCHOICE: Iteration for block: {} with vote length: {}",
+                current_head,
+                latest_votes.len()
+            );
             // if there are no children, we are done, return the current_head
             let children = match self.children.get(&current_head) {
                 Some(children) => children.clone(),
@@ -367,6 +374,7 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
             let mut step =
                 power_of_2_below(self.max_known_height.saturating_sub(block_height).as_u32()) / 2;
             while step > 0 {
+                trace!("Current Step: {}", step);
                 if let Some(clear_winner) = self.get_clear_winner(
                     &latest_votes,
                     block_height - (block_height % u64::from(step)) + u64::from(step),
@@ -399,7 +407,6 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
                         *child_votes.entry(child).or_insert_with(|| 0) += vote;
                     }
                 }
-                println!("Child votes: {:?}", child_votes);
                 // given the votes on the children, find the best child
                 current_head = self
                     .choose_best_child(&child_votes)
@@ -407,17 +414,24 @@ impl<T: ClientDB + Sized> ForkChoice for OptimisedLMDGhost<T> {
                 trace!("FORKCHOICE: Best child found: {}", current_head);
             }
 
-            // No head was found, re-iterate
+            // didn't find head yet, proceed to next iteration
-            // update the block height for the next iteration
+            // update block height
-            let block_height = self
+            block_height = self
                 .block_store
                 .get_deserialized(&current_head)?
-                .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*justified_block_start))?
+                .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(current_head))?
                 .slot()
-                .height(Slot::from(spec.genesis_slot));
+                .height(spec.genesis_slot);
-
             // prune the latest votes for votes that are not part of current chosen chain
             // more specifically, only keep votes that have head as an ancestor
+            for hash in latest_votes.keys() {
+                trace!(
+                    "FORKCHOICE: Ancestor for vote: {} at height: {} is: {:?}",
+                    hash,
+                    block_height,
+                    self.get_ancestor(*hash, block_height, spec)
+                );
+            }
             latest_votes.retain(|hash, _| {
                 self.get_ancestor(*hash, block_height, spec) == Some(current_head)
             });

eth2/fork_choice/src/slow_lmd_ghost.rs

@@ -1,23 +1,3 @@
-// Copyright 2019 Sigma Prime Pty Ltd.
-//
-// Permission is hereby granted, free of charge, to any person obtaining a
-// copy of this software and associated documentation files (the "Software"),
-// to deal in the Software without restriction, including without limitation
-// the rights to use, copy, modify, merge, publish, distribute, sublicense,
-// and/or sell copies of the Software, and to permit persons to whom the
-// Software is furnished to do so, subject to the following conditions:
-//
-// The above copyright notice and this permission notice shall be included in
-// all copies or substantial portions of the Software.
-//
-// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
-// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
-// DEALINGS IN THE SOFTWARE.
-
 extern crate db;
 
 use crate::{ForkChoice, ForkChoiceError};
@@ -64,7 +44,7 @@ where
     pub fn get_latest_votes(
         &self,
         state_root: &Hash256,
-        block_slot: &Slot,
+        block_slot: Slot,
         spec: &ChainSpec,
     ) -> Result<HashMap<Hash256, u64>, ForkChoiceError> {
         // get latest votes
@@ -122,7 +102,7 @@ where
             let (root_at_slot, _) = self
                 .block_store
                 .block_at_slot(&block_root, block_slot)?
-                .ok_or(ForkChoiceError::MissingBeaconBlock(*block_root))?;
+                .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*block_root))?;
             if root_at_slot == *target_hash {
                 count += votes;
             }
@@ -171,7 +151,7 @@ impl<T: ClientDB + Sized> ForkChoice for SlowLMDGhost<T> {
                 .get_deserialized(&target_block_root)?
                 .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*target_block_root))?
                 .slot()
-                .height(Slot::from(spec.genesis_slot));
+                .height(spec.genesis_slot);
 
             // get the height of the past target block
             let past_block_height = self
@@ -179,7 +159,7 @@ impl<T: ClientDB + Sized> ForkChoice for SlowLMDGhost<T> {
                 .get_deserialized(&attestation_target)?
                 .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*attestation_target))?
                 .slot()
-                .height(Slot::from(spec.genesis_slot));
+                .height(spec.genesis_slot);
             // update the attestation only if the new target is higher
             if past_block_height < block_height {
                 *attestation_target = *target_block_root;
@@ -201,7 +181,7 @@ impl<T: ClientDB + Sized> ForkChoice for SlowLMDGhost<T> {
 
         let start_state_root = start.state_root();
 
-        let latest_votes = self.get_latest_votes(&start_state_root, &start.slot(), spec)?;
+        let latest_votes = self.get_latest_votes(&start_state_root, start.slot(), spec)?;
 
         let mut head_hash = Hash256::zero();
 

eth2/fork_choice/tests/optimised_lmd_ghost_test.rs

@@ -15,16 +15,13 @@ use bls::{PublicKey, Signature};
 use db::stores::{BeaconBlockStore, BeaconStateStore};
 use db::MemoryDB;
 use env_logger::{Builder, Env};
-use fork_choice::{ForkChoice, ForkChoiceError, OptimisedLMDGhost};
+use fork_choice::{ForkChoice, OptimisedLMDGhost};
 use ssz::ssz_encode;
 use std::collections::HashMap;
 use std::sync::Arc;
-use std::{fs::File, io::prelude::*, path::PathBuf, str::FromStr};
+use std::{fs::File, io::prelude::*, path::PathBuf};
-use types::test_utils::{SeedableRng, TestRandom, XorShiftRng};
-use types::validator_registry::get_active_validator_indices;
 use types::{
-    BeaconBlock, BeaconBlockBody, BeaconState, ChainSpec, Deposit, DepositData, DepositInput,
+    BeaconBlock, BeaconBlockBody, BeaconState, ChainSpec, Epoch, Eth1Data, Hash256, Slot, Validator,
-    Eth1Data, Hash256, Slot, Validator,
 };
 use yaml_rust::yaml;
 
@@ -50,25 +47,6 @@ fn setup_inital_state(
     };
 
     let initial_validator_deposits = vec![];
-
-    /*
-    (0..no_validators)
-        .map(|_| Deposit {
-            branch: vec![],
-            index: 0,
-            deposit_data: DepositData {
-                amount: 32_000_000_000, // 32 ETH (in Gwei)
-                timestamp: genesis_time - 1,
-                deposit_input: DepositInput {
-                    pubkey: PublicKey::default(),
-                    withdrawal_credentials: zero_hash.clone(),
-                    proof_of_possession: Signature::empty_signature(),
-                },
-            },
-        })
-        .collect();
-    */
-
     let spec = ChainSpec::foundation();
 
     // create the state
@@ -80,18 +58,18 @@ fn setup_inital_state(
     )
     .unwrap();
 
+    let default_validator = Validator {
+        pubkey: PublicKey::default(),
+        withdrawal_credentials: zero_hash,
+        activation_epoch: Epoch::from(0u64),
+        exit_epoch: spec.far_future_epoch,
+        withdrawal_epoch: spec.far_future_epoch,
+        penalized_epoch: spec.far_future_epoch,
+        status_flags: None,
+    };
     // activate the validators
     for _ in 0..no_validators {
-        let validator = Validator {
+        state.validator_registry.push(default_validator.clone());
-            pubkey: PublicKey::default(),
-            withdrawal_credentials: zero_hash,
-            activation_epoch: spec.far_future_epoch,
-            exit_epoch: spec.far_future_epoch,
-            withdrawal_epoch: spec.far_future_epoch,
-            penalized_epoch: spec.far_future_epoch,
-            status_flags: None,
-        };
-        state.validator_registry.push(validator);
         state.validator_balances.push(32_000_000_000);
     }
 
@@ -100,13 +78,6 @@ fn setup_inital_state(
         .put(&state_root, &ssz_encode(&state)[..])
         .unwrap();
 
-    println!(
-        "Active: {:?}",
-        get_active_validator_indices(
-            &state.validator_registry,
-            Slot::from(5u64).epoch(spec.EPOCH_LENGTH)
-        )
-    );
     // return initialised vars
     (optimised_lmd_ghost, block_store, state_root)
 }
@@ -115,7 +86,7 @@ fn setup_inital_state(
 #[test]
 fn test_optimised_lmd_ghost() {
     // set up logging
-    Builder::from_env(Env::default().default_filter_or("trace")).init();
+    Builder::from_env(Env::default().default_filter_or("debug")).init();
 
     // load the yaml
     let mut file = {
@@ -141,6 +112,7 @@ fn test_optimised_lmd_ghost() {
     let mut block_slot: HashMap<Hash256, Slot> = HashMap::new();
 
     // default vars
+    let spec = ChainSpec::foundation();
     let zero_hash = Hash256::zero();
     let eth1_data = Eth1Data {
         deposit_root: zero_hash.clone(),
@@ -165,7 +137,7 @@ fn test_optimised_lmd_ghost() {
 
             // default params for genesis
             let mut block_hash = zero_hash.clone();
-            let mut slot = Slot::from(0u64);
+            let mut slot = spec.genesis_slot;
             let mut parent_root = zero_hash;
 
             // set the slot and parent based off the YAML. Start with genesis;
@@ -206,7 +178,9 @@ fn test_optimised_lmd_ghost() {
 
             // run add block for fork choice if not genesis
             if parent_id != block_id {
-                fork_choice.add_block(&beacon_block, &block_hash).unwrap();
+                fork_choice
+                    .add_block(&beacon_block, &block_hash, &spec)
+                    .unwrap();
             }
         }
 
@@ -225,7 +199,7 @@ fn test_optimised_lmd_ghost() {
                 for _ in 0..weight {
                     assert!(current_validator <= total_emulated_validators);
                     fork_choice
-                        .add_attestation(current_validator as u64, &block_root)
+                        .add_attestation(current_validator as u64, &block_root, &spec)
                         .unwrap();
                     current_validator += 1;
                 }
@@ -233,16 +207,23 @@ fn test_optimised_lmd_ghost() {
         }
 
         // everything is set up, run the fork choice, using genesis as the head
-        println!("Running fork choice");
+        let head = fork_choice.find_head(&zero_hash, &spec).unwrap();
-        let head = fork_choice.find_head(&zero_hash).unwrap();
 
-        let mut found_id = None;
+        let (found_id, _) = block_id_map
-        for (id, block_hash) in block_id_map.iter() {
+            .iter()
-            if *block_hash == head {
+            .find(|(_, hash)| **hash == head)
-                found_id = Some(id);
+            .unwrap();
-            }
+
-        }
+        // compare the result to the expected test
+        let success = test_case["heads"]
+            .clone()
+            .into_vec()
+            .unwrap()
+            .iter()
+            .find(|heads| heads["id"].as_str().unwrap() == found_id)
+            .is_some();
 
         println!("Head Block ID: {:?}", found_id);
+        assert!(success, "Did not find one of the possible heads");
     }
 }