From 7a4c3e26acb332418e0dc530d161a68c8b5ab3e0 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 21 Jun 2019 11:30:25 +1000 Subject: [PATCH] Fix bug in reduced tree fork choice --- beacon_node/beacon_chain/src/test_utils.rs | 9 +--- eth2/lmd_ghost/src/reduced_tree.rs | 51 ++++++++++++---------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index bb308c4076..8da68bbdc3 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -284,16 +284,11 @@ mod test { #[test] fn can_finalize() { - let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 1 + 2; + let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 5; let harness = get_harness(VALIDATOR_COUNT); harness.extend_chain(BuildStrategy::OnCanonicalHead, num_blocks_produced as usize); - /* - for _ in 0..num_blocks_produced { - harness.extend_chain(BuildStrategy::OnCanonicalHead); - } - */ let state = &harness.chain.head().beacon_state; @@ -316,7 +311,5 @@ mod test { state.current_epoch() - 2, "the head should be finalized two behind the current epoch" ); - - panic!(); } } diff --git a/eth2/lmd_ghost/src/reduced_tree.rs b/eth2/lmd_ghost/src/reduced_tree.rs index 411c3bc1c2..2d70ea1453 100644 --- a/eth2/lmd_ghost/src/reduced_tree.rs +++ b/eth2/lmd_ghost/src/reduced_tree.rs @@ -1,8 +1,8 @@ -/// An implementation of "reduced tree" LMD GHOST fork choice. -/// -/// This algorithm was concieved at IC3 Cornell, 2019. -/// -/// This implementation is incomplete and has known bugs. +//! An implementation of "reduced tree" LMD GHOST fork choice. +//! +//! This algorithm was concieved at IC3 Cornell, 2019. +//! +//! This implementation is incomplete and has known bugs. Do not use in production. use super::{LmdGhost, Result as SuperResult}; use parking_lot::RwLock; use std::collections::HashMap; @@ -406,6 +406,8 @@ where let mut added_new_ancestor = false; if !prev_in_tree.children.is_empty() { + let mut added = false; + for &child_hash in &prev_in_tree.children { if self .iter_ancestors(child_hash)? @@ -417,34 +419,37 @@ where node.children.push(child_hash); prev_in_tree.replace_child(child_hash, node.block_hash)?; - added_new_ancestor = true; + added = true; break; } } - for &child_hash in &prev_in_tree.children { - let ancestor_hash = self.find_least_common_ancestor(node.block_hash, child_hash)?; + if !added { + for &child_hash in &prev_in_tree.children { + let ancestor_hash = + self.find_least_common_ancestor(node.block_hash, child_hash)?; - if ancestor_hash != prev_in_tree.block_hash { - let child = self.get_mut_node(child_hash)?; - let common_ancestor = Node { - block_hash: ancestor_hash, - parent_hash: Some(prev_in_tree.block_hash), - children: vec![node.block_hash, child_hash], - ..Node::default() - }; - child.parent_hash = Some(common_ancestor.block_hash); - node.parent_hash = Some(common_ancestor.block_hash); + if ancestor_hash != prev_in_tree.block_hash { + let child = self.get_mut_node(child_hash)?; + let common_ancestor = Node { + block_hash: ancestor_hash, + parent_hash: Some(prev_in_tree.block_hash), + children: vec![node.block_hash, child_hash], + ..Node::default() + }; + child.parent_hash = Some(common_ancestor.block_hash); + node.parent_hash = Some(common_ancestor.block_hash); - prev_in_tree.replace_child(child_hash, ancestor_hash)?; + prev_in_tree.replace_child(child_hash, ancestor_hash)?; - self.nodes - .insert(common_ancestor.block_hash, common_ancestor); + self.nodes + .insert(common_ancestor.block_hash, common_ancestor); - added_new_ancestor = true; + added_new_ancestor = true; - break; + break; + } } } }