Skip to content

Commit

Permalink
Fix race condition
Browse files Browse the repository at this point in the history
An invariant was violated:  For every block hash in head_tracker, that
block is accessible from the store.
  • Loading branch information
adaszko committed May 14, 2020
1 parent 7dcb6f0 commit d9c0a71
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
7 changes: 6 additions & 1 deletion beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

metrics::stop_timer(fork_choice_register_timer);

self.head_tracker.register_block(block_root, &block);
metrics::observe(
&metrics::OPERATIONS_PER_BLOCK_ATTESTATION,
block.body.attestations.len() as f64,
Expand All @@ -1503,6 +1502,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.store.put_state(&block.state_root, &state)?;
self.store.put_block(&block_root, signed_block.clone())?;

let parent_root = block.parent_root;
let slot = block.slot;

self.snapshot_cache
.try_write_for(BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT)
.map(|mut snapshot_cache| {
Expand All @@ -1522,6 +1524,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
);
});

self.head_tracker
.register_block(block_root, parent_root, slot);

metrics::stop_timer(db_write_timer);

metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES);
Expand Down
8 changes: 4 additions & 4 deletions beacon_node/beacon_chain/src/head_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use parking_lot::RwLock;
use ssz_derive::{Decode, Encode};
use std::collections::HashMap;
use std::iter::FromIterator;
use types::{BeaconBlock, EthSpec, Hash256, Slot};
use types::{Hash256, Slot};

#[derive(Debug, PartialEq)]
pub enum Error {
Expand All @@ -23,10 +23,10 @@ impl HeadTracker {
/// This function assumes that no block is imported without its parent having already been
/// imported. It cannot detect an error if this is not the case, it is the responsibility of
/// the upstream user.
pub fn register_block<E: EthSpec>(&self, block_root: Hash256, block: &BeaconBlock<E>) {
pub fn register_block(&self, block_root: Hash256, parent_root: Hash256, slot: Slot) {
let mut map = self.0.write();
map.remove(&block.parent_root);
map.insert(block_root, block.slot);
map.remove(&parent_root);
map.insert(block_root, slot);
}

/// Removes abandoned head.
Expand Down

0 comments on commit d9c0a71

Please sign in to comment.