From 865919e39814b8f87871492a4911a7aa209e1041 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 1 Feb 2019 17:50:19 +1100 Subject: [PATCH] Improve attester errors, move info -> helpers - Ensured one can distingush between a committee error and an invalid validator index when using `validator_attesation_slot_and_shard`. - Renamed the `info.rs` file to `getters.rs`, for clarity. --- .../src/attestation_aggregator.rs | 10 +- beacon_node/beacon_chain/src/getters.rs | 100 ++++++++++++++++++ beacon_node/beacon_chain/src/info.rs | 76 ------------- beacon_node/beacon_chain/src/lib.rs | 2 +- .../src/validator/direct_duties.rs | 9 +- .../types/src/beacon_state/slot_processing.rs | 7 +- 6 files changed, 116 insertions(+), 88 deletions(-) create mode 100644 beacon_node/beacon_chain/src/getters.rs delete mode 100644 beacon_node/beacon_chain/src/info.rs diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index 855cb711ba..dd08b22eae 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -67,10 +67,12 @@ impl AttestationAggregator { free_attestation: &FreeAttestation, spec: &ChainSpec, ) -> Result { - let (slot, shard, committee_index) = state.attestation_slot_and_shard_for_validator( - free_attestation.validator_index as usize, - spec, - )?; + let (slot, shard, committee_index) = state + .attestation_slot_and_shard_for_validator( + free_attestation.validator_index as usize, + spec, + )? + .ok_or_else(|| Error::BadValidatorIndex)?; if free_attestation.data.slot != slot { return Err(Error::BadSlot); diff --git a/beacon_node/beacon_chain/src/getters.rs b/beacon_node/beacon_chain/src/getters.rs new file mode 100644 index 0000000000..cba5197435 --- /dev/null +++ b/beacon_node/beacon_chain/src/getters.rs @@ -0,0 +1,100 @@ +use super::{BeaconChain, ClientDB, SlotClock}; +use types::{beacon_state::CommitteesError, PublicKey}; + +impl BeaconChain +where + T: ClientDB, + U: SlotClock, +{ + /// Returns the the validator index (if any) for the given public key. + /// + /// Information is retrieved from the present `beacon_state.validator_registry`. + pub fn validator_index(&self, pubkey: &PublicKey) -> Option { + for (i, validator) in self + .head() + .beacon_state + .validator_registry + .iter() + .enumerate() + { + if validator.pubkey == *pubkey { + return Some(i); + } + } + None + } + + /// Returns the number of slots the validator has been required to propose. + /// + /// Returns `None` if the `validator_index` is invalid. + /// + /// Information is retrieved from the present `beacon_state.validator_registry`. + pub fn proposer_slots(&self, validator_index: usize) -> Option { + if let Some(validator) = self.state.read().validator_registry.get(validator_index) { + Some(validator.proposer_slots) + } else { + None + } + } + + /// Reads the slot clock, returns `None` if the slot is unavailable. + /// + /// The slot might be unavailable due to an error with the system clock, or if the present time + /// is before genesis (i.e., a negative slot). + /// + /// This is distinct to `present_slot`, which simply reads the latest state. If a + /// call to `read_slot_clock` results in a higher slot than a call to `present_slot`, + /// `self.state` should undergo per slot processing. + pub fn read_slot_clock(&self) -> Option { + match self.slot_clock.present_slot() { + Ok(some_slot) => some_slot, + _ => None, + } + } + + /// Returns slot of the present state. + /// + /// This is distinct to `read_slot_clock`, which reads from the actual system clock. If + /// `self.state` has not been transitioned it is possible for the system clock to be on a + /// different slot to what is returned from this call. + pub fn present_slot(&self) -> u64 { + self.state.read().slot + } + + /// Returns the block proposer for a given slot. + /// + /// Information is read from the present `beacon_state` shuffling, so only information from the + /// present and prior epoch is available. + pub fn block_proposer(&self, slot: u64) -> Result { + let index = self + .state + .read() + .get_beacon_proposer_index(slot, &self.spec)?; + + Ok(index) + } + + /// Returns the justified slot for the present state. + pub fn justified_slot(&self) -> u64 { + self.state.read().justified_slot + } + + /// Returns the attestation slot and shard for a given validator index. + /// + /// Information is read from the current state, so only information from the present and prior + /// epoch is available. + pub fn validator_attestion_slot_and_shard( + &self, + validator_index: usize, + ) -> Result, CommitteesError> { + if let Some((slot, shard, _committee)) = self + .state + .read() + .attestation_slot_and_shard_for_validator(validator_index, &self.spec)? + { + Ok(Some((slot, shard))) + } else { + Ok(None) + } + } +} diff --git a/beacon_node/beacon_chain/src/info.rs b/beacon_node/beacon_chain/src/info.rs deleted file mode 100644 index 70405ab323..0000000000 --- a/beacon_node/beacon_chain/src/info.rs +++ /dev/null @@ -1,76 +0,0 @@ -use super::{BeaconChain, ClientDB, SlotClock}; -use types::{beacon_state::CommitteesError, PublicKey}; - -#[derive(Debug, PartialEq)] -pub enum Error { - SlotClockError, - CommitteesError(CommitteesError), -} - -impl BeaconChain -where - T: ClientDB, - U: SlotClock, -{ - pub fn validator_index(&self, pubkey: &PublicKey) -> Option { - for (i, validator) in self - .head() - .beacon_state - .validator_registry - .iter() - .enumerate() - { - if validator.pubkey == *pubkey { - return Some(i); - } - } - None - } - - pub fn proposer_slots(&self, validator_index: usize) -> Option { - if let Some(validator) = self.state.read().validator_registry.get(validator_index) { - Some(validator.proposer_slots) - } else { - None - } - } - - pub fn read_slot_clock(&self) -> Option { - match self.slot_clock.present_slot() { - Ok(some_slot) => some_slot, - _ => None, - } - } - - pub fn present_slot(&self) -> u64 { - self.state.read().slot - } - - pub fn block_proposer(&self, slot: u64) -> Result { - let index = self - .state - .read() - .get_beacon_proposer_index(slot, &self.spec)?; - - Ok(index) - } - - pub fn justified_slot(&self) -> u64 { - self.state.read().justified_slot - } - - pub fn validator_attestion_slot_and_shard(&self, validator_index: usize) -> Option<(u64, u64)> { - let (slot, shard, _committee) = self - .state - .read() - .attestation_slot_and_shard_for_validator(validator_index, &self.spec) - .ok()?; - Some((slot, shard)) - } -} - -impl From for Error { - fn from(e: CommitteesError) -> Error { - Error::CommitteesError(e) - } -} diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 459e5c5092..e0ce5d8a7d 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -9,7 +9,7 @@ mod canonical_head; mod checkpoint; pub mod dump; mod finalized_head; -mod info; +mod getters; mod lmd_ghost; mod state; diff --git a/beacon_node/beacon_chain/test_harness/src/validator/direct_duties.rs b/beacon_node/beacon_chain/test_harness/src/validator/direct_duties.rs index 943bf6399e..9eda378a07 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator/direct_duties.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator/direct_duties.rs @@ -53,9 +53,12 @@ impl AttesterDutiesReader for DirectDuties { .beacon_chain .validator_attestion_slot_and_shard(validator_index as usize) { - Some((attest_slot, attest_shard)) if attest_slot == slot => Ok(Some(attest_shard)), - Some(_) => Ok(None), - None => Err(AttesterDutiesReaderError::UnknownEpoch), + Ok(Some((attest_slot, attest_shard))) if attest_slot == slot => { + Ok(Some(attest_shard)) + } + Ok(Some(_)) => Ok(None), + Ok(None) => Err(AttesterDutiesReaderError::UnknownEpoch), + Err(_) => panic!("Error when getting validator attestation shard."), } } else { Err(AttesterDutiesReaderError::UnknownValidator) diff --git a/eth2/types/src/beacon_state/slot_processing.rs b/eth2/types/src/beacon_state/slot_processing.rs index 4693bd0692..76135210f9 100644 --- a/eth2/types/src/beacon_state/slot_processing.rs +++ b/eth2/types/src/beacon_state/slot_processing.rs @@ -2,7 +2,6 @@ use crate::{ beacon_state::{CommitteesError, EpochProcessingError}, BeaconState, ChainSpec, Hash256, }; -use log::debug; #[derive(Debug, PartialEq)] pub enum Error { @@ -43,17 +42,17 @@ impl BeaconState { &self, validator_index: usize, spec: &ChainSpec, - ) -> Result<(u64, u64, u64), CommitteesError> { + ) -> Result, CommitteesError> { let mut result = None; for slot in self.get_current_epoch_boundaries(spec.epoch_length) { for (committee, shard) in self.get_crosslink_committees_at_slot(slot, spec)? { if let Some(committee_index) = committee.iter().position(|&i| i == validator_index) { - result = Some(Ok((slot, shard, committee_index as u64))); + result = Some((slot, shard, committee_index as u64)); } } } - result.unwrap() + Ok(result) } }