From eb669ab40f4edff0c47f99d430df5092d297c5c7 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 30 Jul 2019 17:02:38 +1000 Subject: [PATCH] Add v0.8 genesis tests (#466) Closes #452 --- .../src/common/get_compact_committees_root.rs | 13 ++- eth2/state_processing/src/genesis.rs | 16 +++- eth2/state_processing/src/lib.rs | 2 +- .../src/per_block_processing.rs | 92 +++++++++++-------- eth2/types/src/beacon_state/pubkey_cache.rs | 7 +- tests/ef_tests/src/cases.rs | 4 + .../src/cases/genesis_initialization.rs | 45 +++++++++ tests/ef_tests/src/cases/genesis_validity.rs | 42 +++++++++ tests/ef_tests/src/doc.rs | 8 ++ tests/ef_tests/tests/tests.rs | 18 ++++ 10 files changed, 200 insertions(+), 47 deletions(-) create mode 100644 tests/ef_tests/src/cases/genesis_initialization.rs create mode 100644 tests/ef_tests/src/cases/genesis_validity.rs diff --git a/eth2/state_processing/src/common/get_compact_committees_root.rs b/eth2/state_processing/src/common/get_compact_committees_root.rs index 3a1f3998b..75edb3549 100644 --- a/eth2/state_processing/src/common/get_compact_committees_root.rs +++ b/eth2/state_processing/src/common/get_compact_committees_root.rs @@ -14,15 +14,22 @@ pub fn get_compact_committees_root( // FIXME: this is a spec bug, whereby the start shard for the epoch after the next epoch // is mistakenly used. The start shard from the cache SHOULD work. // Waiting on a release to fix https://github.com/ethereum/eth2.0-specs/issues/1315 - // let start_shard = state.get_epoch_start_shard(relative_epoch)?; - let start_shard = state.next_epoch_start_shard(spec)?; + let start_shard = if relative_epoch == RelativeEpoch::Next { + state.next_epoch_start_shard(spec)? + } else { + state.get_epoch_start_shard(relative_epoch)? + }; for committee_number in 0..state.get_committee_count(relative_epoch)? { let shard = (start_shard + committee_number) % T::ShardCount::to_u64(); // FIXME: this is a partial workaround for the above, but it only works in the case // where there's a committee for every shard in every epoch. It works for the minimal // tests but not the mainnet ones. - let fake_shard = (shard + 1) % T::ShardCount::to_u64(); + let fake_shard = if relative_epoch == RelativeEpoch::Next { + (shard + 1) % T::ShardCount::to_u64() + } else { + shard + }; for &index in state .get_crosslink_committee_for_shard(fake_shard, relative_epoch)? diff --git a/eth2/state_processing/src/genesis.rs b/eth2/state_processing/src/genesis.rs index 6f1f2819e..e36261ca3 100644 --- a/eth2/state_processing/src/genesis.rs +++ b/eth2/state_processing/src/genesis.rs @@ -1,4 +1,4 @@ -use super::per_block_processing::{errors::BlockProcessingError, process_deposits}; +use super::per_block_processing::{errors::BlockProcessingError, process_deposit}; use crate::common::get_compact_committees_root; use tree_hash::TreeHash; use types::typenum::U4294967296; @@ -32,7 +32,7 @@ pub fn initialize_beacon_state_from_eth1( for (index, deposit) in deposits.into_iter().enumerate() { let deposit_data_list = VariableList::<_, U4294967296>::from(leaves[..=index].to_vec()); state.eth1_data.deposit_root = Hash256::from_slice(&deposit_data_list.tree_hash_root()); - process_deposits(&mut state, &[deposit], spec)?; + process_deposit(&mut state, &deposit, spec, true)?; } // Process activations @@ -48,6 +48,9 @@ pub fn initialize_beacon_state_from_eth1( } } + // Now that we have our validators, initialize the caches (including the committees) + state.build_all_caches(spec)?; + // Populate active_index_roots and compact_committees_roots let indices_list = VariableList::::from( state.get_active_validator_indices(T::genesis_epoch()), @@ -59,3 +62,12 @@ pub fn initialize_beacon_state_from_eth1( Ok(state) } + +/// Determine whether a candidate genesis state is suitable for starting the chain. +/// +/// Spec v0.8.1 +pub fn is_valid_genesis_state(state: &BeaconState, spec: &ChainSpec) -> bool { + state.genesis_time >= spec.min_genesis_time + && state.get_active_validator_indices(T::genesis_epoch()).len() as u64 + >= spec.min_genesis_active_validator_count +} diff --git a/eth2/state_processing/src/lib.rs b/eth2/state_processing/src/lib.rs index 90f89b599..0539855fc 100644 --- a/eth2/state_processing/src/lib.rs +++ b/eth2/state_processing/src/lib.rs @@ -7,7 +7,7 @@ pub mod per_block_processing; pub mod per_epoch_processing; pub mod per_slot_processing; -pub use genesis::initialize_beacon_state_from_eth1; +pub use genesis::{initialize_beacon_state_from_eth1, is_valid_genesis_state}; pub use per_block_processing::{ errors::{BlockInvalid, BlockProcessingError}, per_block_processing, per_block_processing_without_verifying_block_signature, diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 4d58b6b18..10ca6e370 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -358,7 +358,7 @@ pub fn process_deposits( Invalid::DepositCountInvalid ); - // Verify deposits in parallel. + // Verify merkle proofs in parallel. deposits .par_iter() .enumerate() @@ -368,47 +368,67 @@ pub fn process_deposits( })?; // Update the state in series. - for (i, deposit) in deposits.iter().enumerate() { - state.eth1_deposit_index += 1; + for deposit in deposits { + process_deposit(state, deposit, spec, false)?; + } - // Ensure the state's pubkey cache is fully up-to-date, it will be used to check to see if the - // depositing validator already exists in the registry. - state.update_pubkey_cache()?; + Ok(()) +} - // Get an `Option` where `u64` is the validator index if this deposit public key - // already exists in the beacon_state. - let validator_index = - get_existing_validator_index(state, deposit).map_err(|e| e.into_with_index(i))?; +/// Process a single deposit, optionally verifying its merkle proof. +/// +/// Spec v0.8.1 +pub fn process_deposit( + state: &mut BeaconState, + deposit: &Deposit, + spec: &ChainSpec, + verify_merkle_proof: bool, +) -> Result<(), Error> { + let deposit_index = state.eth1_deposit_index as usize; + if verify_merkle_proof { + verify_deposit_merkle_proof(state, deposit, state.eth1_deposit_index, spec) + .map_err(|e| e.into_with_index(deposit_index))?; + } - let amount = deposit.data.amount; + state.eth1_deposit_index += 1; - if let Some(index) = validator_index { - // Update the existing validator balance. - safe_add_assign!(state.balances[index as usize], amount); - } else { - // The signature should be checked for new validators. Return early for a bad - // signature. - if verify_deposit_signature(state, deposit, spec).is_err() { - return Ok(()); - } + // Ensure the state's pubkey cache is fully up-to-date, it will be used to check to see if the + // depositing validator already exists in the registry. + state.update_pubkey_cache()?; - // Create a new validator. - let validator = Validator { - pubkey: deposit.data.pubkey.clone(), - withdrawal_credentials: deposit.data.withdrawal_credentials, - activation_eligibility_epoch: spec.far_future_epoch, - activation_epoch: spec.far_future_epoch, - exit_epoch: spec.far_future_epoch, - withdrawable_epoch: spec.far_future_epoch, - effective_balance: std::cmp::min( - amount - amount % spec.effective_balance_increment, - spec.max_effective_balance, - ), - slashed: false, - }; - state.validators.push(validator)?; - state.balances.push(deposit.data.amount)?; + // Get an `Option` where `u64` is the validator index if this deposit public key + // already exists in the beacon_state. + let validator_index = get_existing_validator_index(state, deposit) + .map_err(|e| e.into_with_index(deposit_index))?; + + let amount = deposit.data.amount; + + if let Some(index) = validator_index { + // Update the existing validator balance. + safe_add_assign!(state.balances[index as usize], amount); + } else { + // The signature should be checked for new validators. Return early for a bad + // signature. + if verify_deposit_signature(state, deposit, spec).is_err() { + return Ok(()); } + + // Create a new validator. + let validator = Validator { + pubkey: deposit.data.pubkey.clone(), + withdrawal_credentials: deposit.data.withdrawal_credentials, + activation_eligibility_epoch: spec.far_future_epoch, + activation_epoch: spec.far_future_epoch, + exit_epoch: spec.far_future_epoch, + withdrawable_epoch: spec.far_future_epoch, + effective_balance: std::cmp::min( + amount - amount % spec.effective_balance_increment, + spec.max_effective_balance, + ), + slashed: false, + }; + state.validators.push(validator)?; + state.balances.push(deposit.data.amount)?; } Ok(()) diff --git a/eth2/types/src/beacon_state/pubkey_cache.rs b/eth2/types/src/beacon_state/pubkey_cache.rs index 4632a2d9c..b601c3c11 100644 --- a/eth2/types/src/beacon_state/pubkey_cache.rs +++ b/eth2/types/src/beacon_state/pubkey_cache.rs @@ -33,11 +33,8 @@ impl PubkeyCache { } } - /// Inserts a validator index into the map. - /// - /// The added index must equal the number of validators already added to the map. This ensures - /// that an index is never skipped. + /// Looks up a validator index's by their public key. pub fn get(&self, pubkey: &PublicKey) -> Option { - self.map.get(pubkey).cloned() + self.map.get(pubkey).copied() } } diff --git a/tests/ef_tests/src/cases.rs b/tests/ef_tests/src/cases.rs index dbc5d4de6..1ae4ea1d8 100644 --- a/tests/ef_tests/src/cases.rs +++ b/tests/ef_tests/src/cases.rs @@ -12,6 +12,8 @@ mod epoch_processing_final_updates; mod epoch_processing_justification_and_finalization; mod epoch_processing_registry_updates; mod epoch_processing_slashings; +mod genesis_initialization; +mod genesis_validity; mod operations_attestation; mod operations_attester_slashing; mod operations_block_header; @@ -36,6 +38,8 @@ pub use epoch_processing_final_updates::*; pub use epoch_processing_justification_and_finalization::*; pub use epoch_processing_registry_updates::*; pub use epoch_processing_slashings::*; +pub use genesis_initialization::*; +pub use genesis_validity::*; pub use operations_attestation::*; pub use operations_attester_slashing::*; pub use operations_block_header::*; diff --git a/tests/ef_tests/src/cases/genesis_initialization.rs b/tests/ef_tests/src/cases/genesis_initialization.rs new file mode 100644 index 000000000..7ae8eef59 --- /dev/null +++ b/tests/ef_tests/src/cases/genesis_initialization.rs @@ -0,0 +1,45 @@ +use super::*; +use crate::bls_setting::BlsSetting; +use crate::case_result::compare_beacon_state_results_without_caches; +use serde_derive::Deserialize; +use state_processing::initialize_beacon_state_from_eth1; +use types::{BeaconState, Deposit, EthSpec, Hash256}; + +#[derive(Debug, Clone, Deserialize)] +#[serde(bound = "E: EthSpec")] +pub struct GenesisInitialization { + pub description: String, + pub bls_setting: Option, + pub eth1_block_hash: Hash256, + pub eth1_timestamp: u64, + pub deposits: Vec, + pub state: Option>, +} + +impl YamlDecode for GenesisInitialization { + fn yaml_decode(yaml: &str) -> Result { + Ok(serde_yaml::from_str(yaml).unwrap()) + } +} + +impl Case for GenesisInitialization { + fn description(&self) -> String { + self.description.clone() + } + + fn result(&self, _case_index: usize) -> Result<(), Error> { + self.bls_setting.unwrap_or_default().check()?; + let spec = &E::default_spec(); + + let mut result = initialize_beacon_state_from_eth1( + self.eth1_block_hash, + self.eth1_timestamp, + self.deposits.clone(), + spec, + ); + + let mut expected = self.state.clone(); + + compare_beacon_state_results_without_caches(&mut result, &mut expected) + } +} diff --git a/tests/ef_tests/src/cases/genesis_validity.rs b/tests/ef_tests/src/cases/genesis_validity.rs new file mode 100644 index 000000000..7ddd3e8fd --- /dev/null +++ b/tests/ef_tests/src/cases/genesis_validity.rs @@ -0,0 +1,42 @@ +use super::*; +use crate::bls_setting::BlsSetting; +use serde_derive::Deserialize; +use state_processing::is_valid_genesis_state; +use types::{BeaconState, EthSpec}; + +#[derive(Debug, Clone, Deserialize)] +#[serde(bound = "E: EthSpec")] +pub struct GenesisValidity { + pub description: String, + pub bls_setting: Option, + pub genesis: BeaconState, + pub is_valid: bool, +} + +impl YamlDecode for GenesisValidity { + fn yaml_decode(yaml: &str) -> Result { + Ok(serde_yaml::from_str(yaml).unwrap()) + } +} + +impl Case for GenesisValidity { + fn description(&self) -> String { + self.description.clone() + } + + fn result(&self, _case_index: usize) -> Result<(), Error> { + self.bls_setting.unwrap_or_default().check()?; + let spec = &E::default_spec(); + + let is_valid = is_valid_genesis_state(&self.genesis, spec); + + if is_valid == self.is_valid { + Ok(()) + } else { + Err(Error::NotEqual(format!( + "Got {}, expected {}", + is_valid, self.is_valid + ))) + } + } +} diff --git a/tests/ef_tests/src/doc.rs b/tests/ef_tests/src/doc.rs index c3a48f76c..7dfe9954c 100644 --- a/tests/ef_tests/src/doc.rs +++ b/tests/ef_tests/src/doc.rs @@ -134,6 +134,14 @@ impl Doc { // FIXME: skipped due to compact committees issue // run_test::>(self) } + ("genesis", "initialization", "minimal") => { + run_test::>(self) + } + ("genesis", "initialization", "mainnet") => { + run_test::>(self) + } + ("genesis", "validity", "minimal") => run_test::>(self), + ("genesis", "validity", "mainnet") => run_test::>(self), (runner, handler, config) => panic!( "No implementation for runner: \"{}\", handler: \"{}\", config: \"{}\"", runner, handler, config diff --git a/tests/ef_tests/tests/tests.rs b/tests/ef_tests/tests/tests.rs index b7b922e0a..deb699e78 100644 --- a/tests/ef_tests/tests/tests.rs +++ b/tests/ef_tests/tests/tests.rs @@ -205,3 +205,21 @@ fn epoch_processing_final_updates() { Doc::assert_tests_pass(file); }); } + +#[test] +fn genesis_initialization() { + yaml_files_in_test_dir(&Path::new("genesis").join("initialization")) + .into_par_iter() + .for_each(|file| { + Doc::assert_tests_pass(file); + }); +} + +#[test] +fn genesis_validity() { + yaml_files_in_test_dir(&Path::new("genesis").join("validity")) + .into_par_iter() + .for_each(|file| { + Doc::assert_tests_pass(file); + }); +}