Introduce AttestationValidationContext

This reduced code duplication.

lighthouse/state/attestation_record/mod.rs

@@ -19,6 +19,6 @@ pub use self::ssz_splitter::{
     AttestationSplitError,
 };
 pub use self::validation::{
-    validate_attestation,
+    AttestationValidationContext,
     AttestationValidationError,
 };

lighthouse/state/attestation_record/validation/attestation_validation.rs

@@ -45,114 +45,133 @@ pub enum AttestationValidationError {
     DBError(String),
 }
 
-pub fn validate_attestation<T>(a: &AttestationRecord,
+pub struct AttestationValidationContext<T>
-                               block_slot: u64,
-                               cycle_length: u8,
-                               known_last_justified_slot: u64,
-                               known_parent_hashes: &Arc<Vec<Hash256>>,
-                               block_store: &Arc<BlockStore<T>>,
-                               validator_store: &Arc<ValidatorStore<T>>,
-                               attester_map: &Arc<AttesterMap>)
-    -> Result<Option<HashSet<usize>>, AttestationValidationError>
     where T: ClientDB + Sized
 {
-    /*
+    pub block_slot: u64,
-     * The attesation slot must not be higher than the block that contained it.
+    pub cycle_length: u8,
-     */
+    pub last_justified_slot: u64,
-    if a.slot > block_slot {
+    pub parent_hashes: Arc<Vec<Hash256>>,
-        return Err(AttestationValidationError::SlotTooHigh);
+    pub block_store: Arc<BlockStore<T>>,
-    }
+    pub validator_store: Arc<ValidatorStore<T>>,
+    pub attester_map: Arc<AttesterMap>,
+}
 
-    /*
+impl<T> AttestationValidationContext<T>
-     * The slot of this attestation must not be more than cycle_length + 1 distance
+    where T: ClientDB
-     * from the block that contained it.
+{
-     *
+    pub fn validate_attestation(&self, a: &AttestationRecord)
-     * The below code stays overflow-safe as long as cycle length is a < 64 bit integer.
+                                   /*
-     */
+                                   block_slot: u64,
-    if a.slot < block_slot.saturating_sub(u64::from(cycle_length) + 1) {
+                                   cycle_length: u8,
-        return Err(AttestationValidationError::SlotTooLow);
+                                   known_last_justified_slot: u64,
-    }
+                                   known_parent_hashes: &Arc<Vec<Hash256>>,
-
+                                   block_store: &Arc<BlockStore<T>>,
-    /*
+                                   validator_store: &Arc<ValidatorStore<T>>,
-     * The attestation must indicate that its last justified slot is the same as the last justified
+                                   attester_map: &Arc<AttesterMap>)
-     * slot known to us.
+                                   */
-     */
+        -> Result<Option<HashSet<usize>>, AttestationValidationError>
-    if a.justified_slot > known_last_justified_slot {
+        where T: ClientDB + Sized
-        return Err(AttestationValidationError::JustifiedSlotTooHigh);
-    }
-
-    /*
-     * There is no need to include more oblique parents hashes than there are blocks
-     * in a cycle.
-     */
-    if a.oblique_parent_hashes.len() > usize::from(cycle_length) {
-        return Err(AttestationValidationError::TooManyObliqueHashes);
-    }
-
-    /*
-     * Retrieve the set of attestation indices for this slot and shard id.
-     *
-     * This is an array mapping the order that validators will appear in the bitfield to the
-     * canonincal index of a validator.
-     */
-    let attestation_indices = attester_map.get(&(a.slot, a.shard_id))
-        .ok_or(AttestationValidationError::BadAttesterMap)?;
-
-    /*
-     * The bitfield must be no longer than the minimum required to represent each validator in the
-     * attestation indicies for this slot and shard id.
-     */
-    if a.attester_bitfield.num_bytes() !=
-        bytes_for_bits(attestation_indices.len())
     {
-        return Err(AttestationValidationError::BadBitfieldLength);
+        /*
+         * The attesation slot must not be higher than the block that contained it.
+         */
+        if a.slot > self.block_slot {
+            return Err(AttestationValidationError::SlotTooHigh);
+        }
+
+        /*
+         * The slot of this attestation must not be more than cycle_length + 1 distance
+         * from the block that contained it.
+         *
+         * The below code stays overflow-safe as long as cycle length is a < 64 bit integer.
+         */
+        if a.slot < self.block_slot
+            .saturating_sub(u64::from(self.cycle_length).saturating_add(1)) {
+            return Err(AttestationValidationError::SlotTooLow);
+        }
+
+        /*
+         * The attestation must indicate that its last justified slot is the same as the last justified
+         * slot known to us.
+         */
+        if a.justified_slot > self.last_justified_slot {
+            return Err(AttestationValidationError::JustifiedSlotTooHigh);
+        }
+
+        /*
+         * There is no need to include more oblique parents hashes than there are blocks
+         * in a cycle.
+         */
+        if a.oblique_parent_hashes.len() > usize::from(self.cycle_length) {
+            return Err(AttestationValidationError::TooManyObliqueHashes);
+        }
+
+        /*
+         * Retrieve the set of attestation indices for this slot and shard id.
+         *
+         * This is an array mapping the order that validators will appear in the bitfield to the
+         * canonincal index of a validator.
+         */
+        let attestation_indices = self.attester_map.get(&(a.slot, a.shard_id))
+            .ok_or(AttestationValidationError::BadAttesterMap)?;
+
+        /*
+         * The bitfield must be no longer than the minimum required to represent each validator in the
+         * attestation indicies for this slot and shard id.
+         */
+        if a.attester_bitfield.num_bytes() !=
+            bytes_for_bits(attestation_indices.len())
+        {
+            return Err(AttestationValidationError::BadBitfieldLength);
+        }
+
+        /*
+         * If there are excess bits in the bitfield because the number of a validators in not a
+         * multiple of 8, reject this attestation record.
+         *
+         * Allow extra set bits would permit mutliple different byte layouts (and therefore hashes) to
+         * refer to the same AttesationRecord.
+         */
+        let last_byte =
+            a.attester_bitfield.get_byte(a.attester_bitfield.num_bytes() - 1)
+            .ok_or(AttestationValidationError::InvalidBitfield)?;
+        if any_of_last_n_bits_are_set(*last_byte, a.attester_bitfield.len() % 8) {
+            return Err(AttestationValidationError::InvalidBitfieldEndBits)
+        }
+
+        /*
+         * The specified justified block hash must be known to us
+         */
+        if !self.block_store.block_exists(&a.justified_block_hash)? {
+            return Err(AttestationValidationError::UnknownJustifiedBlock)
+        }
+
+        let signed_message = {
+            let parent_hashes = attestation_parent_hashes(
+                self.cycle_length,
+                self.block_slot,
+                a.slot,
+                &self.parent_hashes,
+                &a.oblique_parent_hashes)?;
+            generate_signed_message(
+                a.slot,
+                &parent_hashes,
+                a.shard_id,
+                &a.shard_block_hash,
+                a.justified_slot)
+        };
+
+        let voted_hashmap =
+            verify_aggregate_signature_for_indices(
+                &signed_message,
+                &a.aggregate_sig,
+                &attestation_indices,
+                &a.attester_bitfield,
+                &self.validator_store)?;
+
+        Ok(voted_hashmap)
     }
-
-    /*
-     * If there are excess bits in the bitfield because the number of a validators in not a
-     * multiple of 8, reject this attestation record.
-     *
-     * Allow extra set bits would permit mutliple different byte layouts (and therefore hashes) to
-     * refer to the same AttesationRecord.
-     */
-    let last_byte =
-        a.attester_bitfield.get_byte(a.attester_bitfield.num_bytes() - 1)
-        .ok_or(AttestationValidationError::InvalidBitfield)?;
-    if any_of_last_n_bits_are_set(*last_byte, a.attester_bitfield.len() % 8) {
-        return Err(AttestationValidationError::InvalidBitfieldEndBits)
-    }
-
-    /*
-     * The specified justified block hash must be known to us
-     */
-    if !block_store.block_exists(&a.justified_block_hash)? {
-        return Err(AttestationValidationError::UnknownJustifiedBlock)
-    }
-
-    let signed_message = {
-        let parent_hashes = attestation_parent_hashes(
-            cycle_length,
-            block_slot,
-            a.slot,
-            &known_parent_hashes,
-            &a.oblique_parent_hashes)?;
-        generate_signed_message(
-            a.slot,
-            &parent_hashes,
-            a.shard_id,
-            &a.shard_block_hash,
-            a.justified_slot)
-    };
-
-    let voted_hashmap =
-        verify_aggregate_signature_for_indices(
-            &signed_message,
-            &a.aggregate_sig,
-            &attestation_indices,
-            &a.attester_bitfield,
-            &validator_store)?;
-
-    Ok(voted_hashmap)
 }
 
 fn bytes_for_bits(bits: usize) -> usize {

lighthouse/state/attestation_record/validation/mod.rs

@@ -11,6 +11,6 @@ mod signature_verification;
 mod message_generation;
 
 pub use self::attestation_validation::{
-    validate_attestation,
+    AttestationValidationContext,
     AttestationValidationError,
 };

lighthouse/state/block/validation/validate_ssz_block.rs

@@ -7,7 +7,7 @@ use std::sync::{
     RwLock,
 };
 use super::attestation_record::{
-    validate_attestation,
+    AttestationValidationContext,
     AttestationValidationError,
 };
 use super::attestation_record::{
@@ -99,10 +99,6 @@ impl<T> BlockValidationContext<T>
     /// a suspicion that the block might be invalid. Such a suspicion should be applied to
     /// all blocks coming from the network.
     ///
-    /// Of course, this function will only be more efficient if a block is already serialized.
-    /// Serializing a complete block and then validating with this function will be less
-    /// efficient than just validating the original block.
-    ///
     /// This function will determine if the block is new, already known or invalid (either
     /// intrinsically or due to some application error.)
     ///
@@ -183,18 +179,24 @@ impl<T> BlockValidationContext<T>
             return Err(SszBlockValidationError::ProposerAttestationHasObliqueHashes);
         }
 
+        /*
+         * Generate the context in which attestations will be validated.
+         */
+        let attestation_validation_context = Arc::new(AttestationValidationContext {
+            block_slot,
+            cycle_length: self.cycle_length,
+            last_justified_slot: self.last_justified_slot,
+            parent_hashes: self.parent_hashes.clone(),
+            block_store: self.block_store.clone(),
+            validator_store: self.validator_store.clone(),
+            attester_map: self.attester_map.clone(),
+        });
+
         /*
          * Validate this first attestation.
          */
-        let attestation_voters = validate_attestation(
+        let attestation_voters = attestation_validation_context
-            &first_attestation,
+            .validate_attestation(&first_attestation)?;
-            block_slot,
-            self.cycle_length,
-            self.last_justified_slot,
-            &self.parent_hashes,
-            &self.block_store,
-            &self.validator_store,
-            &self.attester_map)?;
 
         /*
          * If the set of voters is None, the attestation was invalid.
@@ -274,16 +276,7 @@ impl<T> BlockValidationContext<T>
                      * Deserialization succeeded and the attestation should be validated.
                      */
                     Ok((attestation, _)) => {
-                        let result = validate_attestation(
+                        match attestation_validation_context.validate_attestation(&attestation) {
-                            &attestation,
-                            block_slot,
-                            self.cycle_length,
-                            self.last_justified_slot,
-                            &self.parent_hashes,
-                            &self.block_store,
-                            &self.validator_store,
-                            &self.attester_map);
-                        match result {
                             /*
                              * Attestation validation failed with some error.
                              */