From fe2402b361b9e7d8bc6f23cb022da875c32c050c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 7 Aug 2019 16:02:30 +1000 Subject: [PATCH] Add another attestation processing test --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 ++--- beacon_node/beacon_chain/src/lib.rs | 4 ++- beacon_node/beacon_chain/src/test_utils.rs | 30 +++++++++++++++-- beacon_node/beacon_chain/tests/tests.rs | 35 ++++++++++++++++++++ 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index c58e619bc..8d5922850 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -543,7 +543,7 @@ impl BeaconChain { // Attempt to process the attestation using the `self.head()` state. // // This is purely an effort to avoid loading a `BeaconState` unnecessarily from the DB. - let outcome: Option> = { + let optional_outcome: Option> = { // Take a read lock on the head beacon state. // // The purpose of this whole `let processed ...` block is to ensure that the read @@ -553,10 +553,11 @@ impl BeaconChain { // If it turns out that the attestation was made using the head state, then there // is no need to load a state from the database to process the attestation. if state.current_epoch() == attestation_head_block.epoch() - && state + && (state .get_block_root(attestation_head_block.slot) .map(|root| *root == attestation.data.beacon_block_root) .unwrap_or_else(|_| false) + || attestation.data.beacon_block_root == self.head().beacon_block_root) { // The head state is able to be used to validate this attestation. No need to load // anything from the database. @@ -573,8 +574,8 @@ impl BeaconChain { // TODO: we could try and see if the "speculative state" (e.g., self.state) can support // this, without needing to load it from the db. - if let Some(result) = outcome { - result + if let Some(outcome) = optional_outcome { + outcome } else { // The state required to verify this attestation must be loaded from the database. let mut state: BeaconState = self diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index c2efcad13..3188760a4 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -7,7 +7,9 @@ mod metrics; mod persisted_beacon_chain; pub mod test_utils; -pub use self::beacon_chain::{BeaconChain, BeaconChainTypes, BlockProcessingOutcome}; +pub use self::beacon_chain::{ + AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BlockProcessingOutcome, +}; pub use self::checkpoint::CheckPoint; pub use self::errors::{BeaconChainError, BlockProductionError}; pub use lmd_ghost; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 1049c66ad..ce6d4d20c 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -268,6 +268,28 @@ where head_block_root: Hash256, head_block_slot: Slot, ) { + self.get_free_attestations( + attestation_strategy, + state, + head_block_root, + head_block_slot, + ) + .into_iter() + .for_each(|attestation| { + self.chain + .process_attestation(attestation) + .expect("should process attestation"); + }); + } + + /// Generates a `Vec` for some attestation strategy and head_block. + pub fn get_free_attestations( + &self, + attestation_strategy: &AttestationStrategy, + state: &BeaconState, + head_block_root: Hash256, + head_block_slot: Slot, + ) -> Vec> { let spec = &self.spec; let fork = &state.fork; @@ -276,6 +298,8 @@ where AttestationStrategy::SomeValidators(vec) => vec.clone(), }; + let mut vec = vec![]; + state .get_crosslink_committees_at_slot(state.slot) .expect("should get committees") @@ -328,12 +352,12 @@ where signature, }; - self.chain - .process_attestation(attestation) - .expect("should process attestation"); + vec.push(attestation) } } }); + + vec } /// Creates two forks: diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 5b8a09faf..1f8400849 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -4,6 +4,7 @@ use beacon_chain::test_utils::{ AttestationStrategy, BeaconChainHarness, BlockStrategy, CommonTypes, PersistedBeaconChain, BEACON_CHAIN_DB_KEY, }; +use beacon_chain::AttestationProcessingOutcome; use lmd_ghost::ThreadSafeReducedTree; use rand::Rng; use store::{MemoryStore, Store}; @@ -298,6 +299,40 @@ fn free_attestations_added_to_fork_choice_some_none() { } } +#[test] +fn free_attestations_over_slots() { + let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 5; + + let harness = get_harness(VALIDATOR_COUNT); + + let mut attestations = vec![]; + + for _ in 0..num_blocks_produced { + harness.extend_chain( + 2, + BlockStrategy::OnCanonicalHead, + // Don't produce & include any attestations (we'll collect them later). + AttestationStrategy::SomeValidators(vec![]), + ); + + attestations.append(&mut harness.get_free_attestations( + &AttestationStrategy::AllValidators, + &harness.chain.head().beacon_state, + harness.chain.head().beacon_block_root, + harness.chain.head().beacon_block.slot, + )); + + harness.advance_slot(); + } + + for attestation in attestations { + assert_eq!( + harness.chain.process_attestation(attestation), + Ok(AttestationProcessingOutcome::Processed) + ) + } +} + #[test] fn free_attestations_added_to_fork_choice_all_updated() { let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 2 - 1;