From 8baae0e02ee79bdb79190a4d9a31ea16af27ae64 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 19 Feb 2019 11:58:17 +1100 Subject: [PATCH] Updates optimised fork choice. - Bug fixes in optimised fork choice. - YAML tests functioning. - Implement Clippy suggestions. - Remove Copywrite notices. --- eth2/fork_choice/Cargo.toml | 2 +- eth2/fork_choice/src/lib.rs | 20 ---- eth2/fork_choice/src/longest_chain.rs | 2 +- eth2/fork_choice/src/optimised_lmd_ghost.rs | 110 ++++++++++-------- eth2/fork_choice/src/slow_lmd_ghost.rs | 30 +---- .../tests/optimised_lmd_ghost_test.rs | 89 ++++++-------- 6 files changed, 104 insertions(+), 149 deletions(-) diff --git a/eth2/fork_choice/Cargo.toml b/eth2/fork_choice/Cargo.toml index 2765622d1..d12f5ae7a 100644 --- a/eth2/fork_choice/Cargo.toml +++ b/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" diff --git a/eth2/fork_choice/src/lib.rs b/eth2/fork_choice/src/lib.rs index 11da12304..3923fca9b 100644 --- a/eth2/fork_choice/src/lib.rs +++ b/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. //! diff --git a/eth2/fork_choice/src/longest_chain.rs b/eth2/fork_choice/src/longest_chain.rs index ea2cc33bb..333553c02 100644 --- a/eth2/fork_choice/src/longest_chain.rs +++ b/eth2/fork_choice/src/longest_chain.rs @@ -30,7 +30,7 @@ impl ForkChoice for LongestChain { &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 diff --git a/eth2/fork_choice/src/optimised_lmd_ghost.rs b/eth2/fork_choice/src/optimised_lmd_ghost.rs index e60af4e66..f211359ea 100644 --- a/eth2/fork_choice/src/optimised_lmd_ghost.rs +++ b/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, 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 = HashMap::new(); // gets the current weighted votes let current_state = self @@ -95,10 +98,6 @@ where ¤t_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 { // 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 at_height > block_height { + if target_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 = 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) -> Option { - println!("Votes: {:?}", votes); - let mut bitmask = 0; - for bit in (0..=255).rev() { + if votes.is_empty() { + return None; + } + 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); - if candidate_uint >> (bit + 1) != bitmask { + let candidate_bit: BitVec = BitVec::from_bytes(&candidate); + + // 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) + { - if one_votes > zero_votes { - 1 - } else { - 0 - } - }; + bitmask.push(one_votes > zero_votes); 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 ForkChoice for OptimisedLMDGhost { .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 ForkChoice for OptimisedLMDGhost { // 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 ForkChoice for OptimisedLMDGhost { .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 ForkChoice for OptimisedLMDGhost { .get_deserialized(&attestation_target)? .ok_or_else(|| ForkChoiceError::MissingBeaconBlock(*attestation_target))? .slot() - .height(Slot::from(spec.genesis_slot)); - trace!("FORKCHOICE: Old block height: {:?}", past_block_height); - trace!("FORKCHOICE: New block height: {:?}", block_height); + .height(spec.genesis_slot); // 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 ForkChoice for OptimisedLMDGhost { justified_block_start: &Hash256, spec: &ChainSpec, ) -> Result { - 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(¤t_head) { Some(children) => children.clone(), @@ -367,6 +374,7 @@ impl ForkChoice for OptimisedLMDGhost { 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 ForkChoice for OptimisedLMDGhost { *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 ForkChoice for OptimisedLMDGhost { trace!("FORKCHOICE: Best child found: {}", current_head); } - // No head was found, re-iterate - // update the block height for the next iteration - let block_height = self + // didn't find head yet, proceed to next iteration + // update block height + block_height = self .block_store .get_deserialized(¤t_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) }); diff --git a/eth2/fork_choice/src/slow_lmd_ghost.rs b/eth2/fork_choice/src/slow_lmd_ghost.rs index 81bc5d628..7041b1b46 100644 --- a/eth2/fork_choice/src/slow_lmd_ghost.rs +++ b/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, 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 ForkChoice for SlowLMDGhost { .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 ForkChoice for SlowLMDGhost { .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 ForkChoice for SlowLMDGhost { 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(); diff --git a/eth2/fork_choice/tests/optimised_lmd_ghost_test.rs b/eth2/fork_choice/tests/optimised_lmd_ghost_test.rs index ac0b6888c..3cff5b546 100644 --- a/eth2/fork_choice/tests/optimised_lmd_ghost_test.rs +++ b/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 types::test_utils::{SeedableRng, TestRandom, XorShiftRng}; -use types::validator_registry::get_active_validator_indices; +use std::{fs::File, io::prelude::*, path::PathBuf}; use types::{ - BeaconBlock, BeaconBlockBody, BeaconState, ChainSpec, Deposit, DepositData, DepositInput, - Eth1Data, Hash256, Slot, Validator, + BeaconBlock, BeaconBlockBody, BeaconState, ChainSpec, Epoch, 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 { - 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_registry.push(default_validator.clone()); 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 = 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).unwrap(); + let head = fork_choice.find_head(&zero_hash, &spec).unwrap(); - let mut found_id = None; - for (id, block_hash) in block_id_map.iter() { - if *block_hash == head { - found_id = Some(id); - } - } + let (found_id, _) = block_id_map + .iter() + .find(|(_, hash)| **hash == head) + .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"); } }