From a86f7fa51b82c0463013596be8c2b4ca618aa3dc Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 1 Feb 2019 15:09:30 +1100 Subject: [PATCH] Tidy `AttestationAggregator`, add docstrings. --- .../src/attestation_aggregator.rs | 56 +++++++++++++++---- .../src/attestation_processing.rs | 2 +- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index 40b574399..855cb711b 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -6,63 +6,91 @@ use types::{ const PHASE_0_CUSTODY_BIT: bool = false; +/// Provides the functionality to: +/// +/// - Recieve a `FreeAttestation` and aggregate it into an `Attestation` (or create a new if it +/// doesn't exist). +/// - Store all aggregated or created `Attestation`s. +/// - Produce a list of attestations that would be valid for inclusion in some `BeaconState` (and +/// therefore valid for inclusion in a `BeaconBlock`. +/// +/// Note: `Attestations` are stored in memory and never deleted. This is not scalable and must be +/// rectified in a future revision. pub struct AttestationAggregator { store: HashMap, Attestation>, } +/// The outcome of sucessfully processing a `FreeAttestation`. #[derive(Debug, PartialEq)] pub enum ProcessOutcome { - AggregationNotRequired, + /// The free attestation was added to an existing attestation. Aggregated, + /// The free attestation has already been aggregated to an existing attestation. + AggregationNotRequired, + /// The free attestation was transformed into a new attestation. NewAttestationCreated, } #[derive(Debug, PartialEq)] -pub enum ProcessError { +pub enum Error { + /// The supplied `validator_index` is not in the committee for the given `shard` and `slot`. BadValidatorIndex, + /// The given `signature` did not match the `pubkey` in the given + /// `state.validator_registry`. BadSignature, + /// The given `slot` does not match the validators committee assignment. BadSlot, + /// The given `shard` does not match the validators committee assignment. BadShard, + /// There was an error finding the committee for the given `validator_index`. CommitteesError(CommitteesError), } impl AttestationAggregator { + /// Instantiates a new AttestationAggregator with an empty database. pub fn new() -> Self { Self { store: HashMap::new(), } } + /// Accepts some `FreeAttestation`, validates it and either aggregates it upon some existing + /// `Attestation` or produces a new `Attestation`. + /// + /// The "validation" provided is not complete, instead the following points are checked: + /// - The given `validator_index` is in the committee for the given `shard` for the given + /// `slot`. + /// - The signature is verified against that of the validator at `validator_index`. pub fn process_free_attestation( &mut self, state: &BeaconState, free_attestation: &FreeAttestation, spec: &ChainSpec, - ) -> Result { + ) -> Result { let (slot, shard, committee_index) = state.attestation_slot_and_shard_for_validator( free_attestation.validator_index as usize, spec, )?; if free_attestation.data.slot != slot { - return Err(ProcessError::BadSlot); + return Err(Error::BadSlot); } if free_attestation.data.shard != shard { - return Err(ProcessError::BadShard); + return Err(Error::BadShard); } let signable_message = free_attestation.data.signable_message(PHASE_0_CUSTODY_BIT); let validator_pubkey = &state .validator_registry .get(free_attestation.validator_index as usize) - .ok_or_else(|| ProcessError::BadValidatorIndex)? + .ok_or_else(|| Error::BadValidatorIndex)? .pubkey; if !free_attestation .signature .verify(&signable_message, &validator_pubkey) { - return Err(ProcessError::BadSignature); + return Err(Error::BadSignature); } if let Some(existing_attestation) = self.store.get(&signable_message) { @@ -94,8 +122,8 @@ impl AttestationAggregator { /// Returns all known attestations which are: /// - /// a) valid for the given state - /// b) not already in `state.latest_attestations`. + /// - Valid for the given state + /// - Not already in `state.latest_attestations`. pub fn get_attestations_for_state( &self, state: &BeaconState, @@ -124,6 +152,10 @@ impl AttestationAggregator { } } +/// Produces a new `Attestation` where: +/// +/// - `signature` is added to `Attestation.aggregate_signature` +/// - Attestation.aggregation_bitfield[committee_index]` is set to true. fn aggregate_attestation( existing_attestation: &Attestation, signature: &Signature, @@ -150,8 +182,8 @@ fn aggregate_attestation( } } -impl From for ProcessError { - fn from(e: CommitteesError) -> ProcessError { - ProcessError::CommitteesError(e) +impl From for Error { + fn from(e: CommitteesError) -> Error { + Error::CommitteesError(e) } } diff --git a/beacon_node/beacon_chain/src/attestation_processing.rs b/beacon_node/beacon_chain/src/attestation_processing.rs index 1b7e8ace4..bc7e2cda1 100644 --- a/beacon_node/beacon_chain/src/attestation_processing.rs +++ b/beacon_node/beacon_chain/src/attestation_processing.rs @@ -1,5 +1,5 @@ use super::{BeaconChain, ClientDB, SlotClock}; -pub use crate::attestation_aggregator::{ProcessError as AggregatorError, ProcessOutcome}; +pub use crate::attestation_aggregator::{Error as AggregatorError, ProcessOutcome}; use crate::canonical_head::Error as HeadError; use types::FreeAttestation;