From ed6c39e25a7ee7fae51ef4d20522ac171a2202aa Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 27 Aug 2019 11:19:50 +1000 Subject: [PATCH] Add log for fork choice integrity in beacon chain --- beacon_node/beacon_chain/src/beacon_chain.rs | 21 ++++++++++---- beacon_node/beacon_chain/src/fork_choice.rs | 8 ++++++ eth2/lmd_ghost/src/lib.rs | 6 ++++ eth2/lmd_ghost/src/reduced_tree.rs | 30 ++++++-------------- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 5feefd841..0fc71fe7b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -739,8 +739,19 @@ impl BeaconChain { } else { // Provide the attestation to fork choice, updating the validator latest messages but // _without_ finding and updating the head. - self.fork_choice - .process_attestation(&state, &attestation, block)?; + if let Err(e) = self + .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. @@ -947,10 +958,10 @@ impl BeaconChain { if let Err(e) = self.fork_choice.process_block(&state, &block, block_root) { error!( self.log, - "fork choice failed to process_block"; - "error" => format!("{:?}", e), + "Add block to fork choice failed"; + "fork_choice_integrity" => format!("{:?}", self.fork_choice.verify_integrity()), "block_root" => format!("{}", block_root), - "block_slot" => format!("{}", block.slot) + "error" => format!("{:?}", e), ) } diff --git a/beacon_node/beacon_chain/src/fork_choice.rs b/beacon_node/beacon_chain/src/fork_choice.rs index 77fdaacdc..26084e04a 100644 --- a/beacon_node/beacon_chain/src/fork_choice.rs +++ b/beacon_node/beacon_chain/src/fork_choice.rs @@ -199,6 +199,14 @@ impl ForkChoice { 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. /// diff --git a/eth2/lmd_ghost/src/lib.rs b/eth2/lmd_ghost/src/lib.rs index 95cd0679c..167cd36ea 100644 --- a/eth2/lmd_ghost/src/lib.rs +++ b/eth2/lmd_ghost/src/lib.rs @@ -46,4 +46,10 @@ pub trait LmdGhost: 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<()>; } diff --git a/eth2/lmd_ghost/src/reduced_tree.rs b/eth2/lmd_ghost/src/reduced_tree.rs index deda02e1f..cd3a38c46 100644 --- a/eth2/lmd_ghost/src/reduced_tree.rs +++ b/eth2/lmd_ghost/src/reduced_tree.rs @@ -43,16 +43,6 @@ impl fmt::Debug for ThreadSafeReducedTree { } } -impl ThreadSafeReducedTree -where - T: Store, - E: EthSpec, -{ - pub fn verify_integrity(&self) -> std::result::Result<(), String> { - self.core.read().verify_integrity() - } -} - impl LmdGhost for ThreadSafeReducedTree where T: Store, @@ -80,7 +70,7 @@ where fn process_block(&self, block: &BeaconBlock, 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 { @@ -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) { - let node = Node { - block_hash: new_root, - voters: vec![], - ..Node::default() - }; - - self.add_node(node)?; - } + self.maybe_add_weightless_node(new_slot, new_root)?; 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,