Test corrections (#925)

* Merge #913

* Correct release tests

* Completed release test corrections
This commit is contained in:
Age Manning 2020-03-17 23:05:55 +11:00 committed by GitHub
parent 95c8e476bc
commit 41c3294c16
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 71 additions and 38 deletions

View File

@ -56,7 +56,7 @@ fn attestation_validity() {
.expect("should get at least one attestation"); .expect("should get at least one attestation");
assert_eq!( assert_eq!(
chain.process_attestation(valid_attestation.clone()), chain.process_attestation(valid_attestation.clone(), Some(false)),
Ok(AttestationProcessingOutcome::Processed), Ok(AttestationProcessingOutcome::Processed),
"should accept valid attestation" "should accept valid attestation"
); );
@ -71,7 +71,7 @@ fn attestation_validity() {
assert_eq!( assert_eq!(
harness harness
.chain .chain
.process_attestation(epoch_mismatch_attestation), .process_attestation(epoch_mismatch_attestation, Some(false)),
Ok(AttestationProcessingOutcome::BadTargetEpoch), Ok(AttestationProcessingOutcome::BadTargetEpoch),
"should not accept attestation where the slot is not in the same epoch as the target" "should not accept attestation where the slot is not in the same epoch as the target"
); );
@ -85,7 +85,9 @@ fn attestation_validity() {
early_attestation.data.slot = (current_epoch + 1).start_slot(MainnetEthSpec::slots_per_epoch()); early_attestation.data.slot = (current_epoch + 1).start_slot(MainnetEthSpec::slots_per_epoch());
assert_eq!( assert_eq!(
harness.chain.process_attestation(early_attestation), harness
.chain
.process_attestation(early_attestation, Some(false)),
Ok(AttestationProcessingOutcome::FutureEpoch { Ok(AttestationProcessingOutcome::FutureEpoch {
attestation_epoch: current_epoch + 1, attestation_epoch: current_epoch + 1,
current_epoch current_epoch
@ -118,7 +120,9 @@ fn attestation_validity() {
.expect("should get at least one late attestation"); .expect("should get at least one late attestation");
assert_eq!( assert_eq!(
harness.chain.process_attestation(late_attestation), harness
.chain
.process_attestation(late_attestation, Some(false)),
Ok(AttestationProcessingOutcome::PastEpoch { Ok(AttestationProcessingOutcome::PastEpoch {
attestation_epoch: current_epoch - 2, attestation_epoch: current_epoch - 2,
current_epoch current_epoch
@ -134,7 +138,9 @@ fn attestation_validity() {
bad_target_attestation.data.target.root = Hash256::from_low_u64_be(42); bad_target_attestation.data.target.root = Hash256::from_low_u64_be(42);
assert_eq!( assert_eq!(
harness.chain.process_attestation(bad_target_attestation), harness
.chain
.process_attestation(bad_target_attestation, Some(false)),
Ok(AttestationProcessingOutcome::UnknownTargetRoot( Ok(AttestationProcessingOutcome::UnknownTargetRoot(
Hash256::from_low_u64_be(42) Hash256::from_low_u64_be(42)
)), )),
@ -149,7 +155,9 @@ fn attestation_validity() {
future_block_attestation.data.slot -= 1; future_block_attestation.data.slot -= 1;
assert_eq!( assert_eq!(
harness.chain.process_attestation(future_block_attestation), harness
.chain
.process_attestation(future_block_attestation, Some(false)),
Ok(AttestationProcessingOutcome::AttestsToFutureBlock { Ok(AttestationProcessingOutcome::AttestsToFutureBlock {
block: current_slot, block: current_slot,
attestation: current_slot - 1 attestation: current_slot - 1
@ -165,7 +173,9 @@ fn attestation_validity() {
bad_head_attestation.data.beacon_block_root = Hash256::from_low_u64_be(42); bad_head_attestation.data.beacon_block_root = Hash256::from_low_u64_be(42);
assert_eq!( assert_eq!(
harness.chain.process_attestation(bad_head_attestation), harness
.chain
.process_attestation(bad_head_attestation, Some(false)),
Ok(AttestationProcessingOutcome::UnknownHeadBlock { Ok(AttestationProcessingOutcome::UnknownHeadBlock {
beacon_block_root: Hash256::from_low_u64_be(42) beacon_block_root: Hash256::from_low_u64_be(42)
}), }),
@ -183,7 +193,9 @@ fn attestation_validity() {
bad_signature_attestation.signature = agg_sig; bad_signature_attestation.signature = agg_sig;
assert_eq!( assert_eq!(
harness.chain.process_attestation(bad_signature_attestation), harness
.chain
.process_attestation(bad_signature_attestation, Some(false)),
Ok(AttestationProcessingOutcome::InvalidSignature), Ok(AttestationProcessingOutcome::InvalidSignature),
"should not accept bad_signature attestation" "should not accept bad_signature attestation"
); );
@ -199,7 +211,7 @@ fn attestation_validity() {
assert_eq!( assert_eq!(
harness harness
.chain .chain
.process_attestation(empty_bitfield_attestation), .process_attestation(empty_bitfield_attestation, Some(false)),
Ok(AttestationProcessingOutcome::EmptyAggregationBitfield), Ok(AttestationProcessingOutcome::EmptyAggregationBitfield),
"should not accept empty_bitfield attestation" "should not accept empty_bitfield attestation"
); );
@ -247,7 +259,7 @@ fn attestation_that_skips_epochs() {
.expect("should get at least one attestation"); .expect("should get at least one attestation");
assert_eq!( assert_eq!(
harness.chain.process_attestation(attestation), harness.chain.process_attestation(attestation, Some(false)),
Ok(AttestationProcessingOutcome::Processed), Ok(AttestationProcessingOutcome::Processed),
"should process attestation that skips slots" "should process attestation that skips slots"
); );

View File

@ -306,7 +306,7 @@ fn epoch_boundary_state_attestation_processing() {
.epoch; .epoch;
let res = harness let res = harness
.chain .chain
.process_attestation_internal(attestation.clone()); .process_attestation_internal(attestation.clone(), Some(true));
let current_epoch = harness.chain.epoch().expect("should get epoch"); let current_epoch = harness.chain.epoch().expect("should get epoch");
let attestation_epoch = attestation.data.target.epoch; let attestation_epoch = attestation.data.target.epoch;

View File

@ -449,7 +449,7 @@ fn attestations_with_increasing_slots() {
for attestation in attestations { for attestation in attestations {
let attestation_epoch = attestation.data.target.epoch; let attestation_epoch = attestation.data.target.epoch;
let res = harness.chain.process_attestation(attestation); let res = harness.chain.process_attestation(attestation, Some(false));
if attestation_epoch + 1 < current_epoch { if attestation_epoch + 1 < current_epoch {
assert_eq!( assert_eq!(

View File

@ -1,15 +1,13 @@
use beacon_chain::BeaconChainTypes;
use eth2_libp2p::Enr; use eth2_libp2p::Enr;
use rlp; use rlp;
use std::sync::Arc; use std::sync::Arc;
use store::Store; use store::{DBColumn, Error as StoreError, SimpleStoreItem, Store};
use store::{DBColumn, Error as StoreError, SimpleStoreItem}; use types::{EthSpec, Hash256};
use types::Hash256;
/// 32-byte key for accessing the `DhtEnrs`. /// 32-byte key for accessing the `DhtEnrs`.
pub const DHT_DB_KEY: &str = "PERSISTEDDHTPERSISTEDDHTPERSISTE"; pub const DHT_DB_KEY: &str = "PERSISTEDDHTPERSISTEDDHTPERSISTE";
pub fn load_dht<T: BeaconChainTypes>(store: Arc<T::Store>) -> Vec<Enr> { pub fn load_dht<T: Store<E>, E: EthSpec>(store: Arc<T>) -> Vec<Enr> {
// Load DHT from store // Load DHT from store
let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes()); let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes());
match store.get(&key) { match store.get(&key) {
@ -22,8 +20,8 @@ pub fn load_dht<T: BeaconChainTypes>(store: Arc<T::Store>) -> Vec<Enr> {
} }
/// Attempt to persist the ENR's in the DHT to `self.store`. /// Attempt to persist the ENR's in the DHT to `self.store`.
pub fn persist_dht<T: BeaconChainTypes>( pub fn persist_dht<T: Store<E>, E: EthSpec>(
store: Arc<T::Store>, store: Arc<T>,
enrs: Vec<Enr>, enrs: Vec<Enr>,
) -> Result<(), store::Error> { ) -> Result<(), store::Error> {
let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes()); let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes());

View File

@ -75,7 +75,7 @@ impl<T: BeaconChainTypes> NetworkService<T> {
// launch libp2p service // launch libp2p service
let (network_globals, mut libp2p) = LibP2PService::new(config, network_log.clone())?; let (network_globals, mut libp2p) = LibP2PService::new(config, network_log.clone())?;
for enr in load_dht::<T>(store.clone()) { for enr in load_dht::<T::Store, T::EthSpec>(store.clone()) {
libp2p.swarm.add_enr(enr); libp2p.swarm.add_enr(enr);
} }
@ -135,7 +135,7 @@ fn spawn_service<T: BeaconChainTypes>(
"Number of peers" => format!("{}", enrs.len()), "Number of peers" => format!("{}", enrs.len()),
); );
match persist_dht::<T>(service.store.clone(), enrs) { match persist_dht::<T::Store, T::EthSpec>(service.store.clone(), enrs) {
Err(e) => error!( Err(e) => error!(
log, log,
"Failed to persist DHT on drop"; "Failed to persist DHT on drop";

View File

@ -2,7 +2,7 @@
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::persisted_dht::load_dht; use crate::persisted_dht::load_dht;
use crate::{NetworkConfig, Service}; use crate::{NetworkConfig, NetworkService};
use beacon_chain::test_utils::BeaconChainHarness; use beacon_chain::test_utils::BeaconChainHarness;
use eth2_libp2p::Enr; use eth2_libp2p::Enr;
use futures::{Future, IntoFuture}; use futures::{Future, IntoFuture};
@ -10,7 +10,6 @@ mod tests {
use sloggers::{null::NullLoggerBuilder, Build}; use sloggers::{null::NullLoggerBuilder, Build};
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::Arc;
use store::MemoryStore;
use tokio::runtime::Runtime; use tokio::runtime::Runtime;
use types::{test_utils::generate_deterministic_keypairs, MinimalEthSpec}; use types::{test_utils::generate_deterministic_keypairs, MinimalEthSpec};
@ -44,14 +43,14 @@ mod tests {
.block_on_all( .block_on_all(
// Create a new network service which implicitly gets dropped at the // Create a new network service which implicitly gets dropped at the
// end of the block. // end of the block.
Service::new(beacon_chain.clone(), &config, &executor, log.clone()) NetworkService::start(beacon_chain.clone(), &config, &executor, log.clone())
.into_future() .into_future()
.and_then(move |(_service, _)| Ok(())), .and_then(move |(_globals, _service, _exit)| Ok(())),
) )
.unwrap(); .unwrap();
// Load the persisted dht from the store // Load the persisted dht from the store
let persisted_enrs = load_dht::<MemoryStore<MinimalEthSpec>, MinimalEthSpec>(store); let persisted_enrs = load_dht(store);
assert!( assert!(
persisted_enrs.contains(&enrs[0]), persisted_enrs.contains(&enrs[0]),
"should have persisted the first ENR to store" "should have persisted the first ENR to store"

View File

@ -669,11 +669,16 @@ mod release_tests {
spec, spec,
None, None,
); );
op_pool.insert_attestation(att, &state.fork, spec).unwrap(); op_pool
.insert_aggregate_attestation(att, &state.fork, spec)
.unwrap();
} }
} }
assert_eq!(op_pool.attestations.read().len(), committees.len()); assert_eq!(
op_pool.aggregate_attestations.read().len(),
committees.len()
);
assert_eq!(op_pool.num_attestations(), committees.len()); assert_eq!(op_pool.num_attestations(), committees.len());
// Before the min attestation inclusion delay, get_attestations shouldn't return anything. // Before the min attestation inclusion delay, get_attestations shouldn't return anything.
@ -701,13 +706,15 @@ mod release_tests {
); );
// Prune attestations shouldn't do anything at this point. // Prune attestations shouldn't do anything at this point.
op_pool.prune_attestations(state); let epoch = state.slot.epoch(MainnetEthSpec::slots_per_epoch());
op_pool.prune_attestations(&epoch);
assert_eq!(op_pool.num_attestations(), committees.len()); assert_eq!(op_pool.num_attestations(), committees.len());
// But once we advance to more than an epoch after the attestation, it should prune it // But once we advance to more than an epoch after the attestation, it should prune it
// out of existence. // out of existence.
state.slot += 2 * MainnetEthSpec::slots_per_epoch(); state.slot += 2 * MainnetEthSpec::slots_per_epoch();
op_pool.prune_attestations(state); let epoch = state.slot.epoch(MainnetEthSpec::slots_per_epoch());
op_pool.prune_attestations(&epoch);
assert_eq!(op_pool.num_attestations(), 0); assert_eq!(op_pool.num_attestations(), 0);
} }
@ -738,9 +745,11 @@ mod release_tests {
None, None,
); );
op_pool op_pool
.insert_attestation(att.clone(), &state.fork, spec) .insert_aggregate_attestation(att.clone(), &state.fork, spec)
.unwrap();
op_pool
.insert_aggregate_attestation(att, &state.fork, spec)
.unwrap(); .unwrap();
op_pool.insert_attestation(att, &state.fork, spec).unwrap();
} }
assert_eq!(op_pool.num_attestations(), committees.len()); assert_eq!(op_pool.num_attestations(), committees.len());
@ -777,13 +786,18 @@ mod release_tests {
spec, spec,
None, None,
); );
op_pool.insert_attestation(att, &state.fork, spec).unwrap(); op_pool
.insert_aggregate_attestation(att, &state.fork, spec)
.unwrap();
} }
} }
// The attestations should get aggregated into two attestations that comprise all // The attestations should get aggregated into two attestations that comprise all
// validators. // validators.
assert_eq!(op_pool.attestations.read().len(), committees.len()); assert_eq!(
op_pool.aggregate_attestations.read().len(),
committees.len()
);
assert_eq!(op_pool.num_attestations(), 2 * committees.len()); assert_eq!(op_pool.num_attestations(), 2 * committees.len());
} }
@ -825,7 +839,9 @@ mod release_tests {
spec, spec,
if i == 0 { None } else { Some(0) }, if i == 0 { None } else { Some(0) },
); );
op_pool.insert_attestation(att, &state.fork, spec).unwrap(); op_pool
.insert_aggregate_attestation(att, &state.fork, spec)
.unwrap();
} }
}; };
@ -840,7 +856,10 @@ mod release_tests {
let num_small = target_committee_size / small_step_size; let num_small = target_committee_size / small_step_size;
let num_big = target_committee_size / big_step_size; let num_big = target_committee_size / big_step_size;
assert_eq!(op_pool.attestations.read().len(), committees.len()); assert_eq!(
op_pool.aggregate_attestations.read().len(),
committees.len()
);
assert_eq!( assert_eq!(
op_pool.num_attestations(), op_pool.num_attestations(),
(num_small + num_big) * committees.len() (num_small + num_big) * committees.len()
@ -898,7 +917,9 @@ mod release_tests {
spec, spec,
if i == 0 { None } else { Some(0) }, if i == 0 { None } else { Some(0) },
); );
op_pool.insert_attestation(att, &state.fork, spec).unwrap(); op_pool
.insert_aggregate_attestation(att, &state.fork, spec)
.unwrap();
} }
}; };
@ -913,7 +934,10 @@ mod release_tests {
let num_small = target_committee_size / small_step_size; let num_small = target_committee_size / small_step_size;
let num_big = target_committee_size / big_step_size; let num_big = target_committee_size / big_step_size;
assert_eq!(op_pool.attestations.read().len(), committees.len()); assert_eq!(
op_pool.aggregate_attestations.read().len(),
committees.len()
);
assert_eq!( assert_eq!(
op_pool.num_attestations(), op_pool.num_attestations(),
(num_small + num_big) * committees.len() (num_small + num_big) * committees.len()