Ensure attestation val. check parent.slot

As per comments by Danny Ryan on PR#33

beacon_chain/validation/src/attestation_validation.rs

@@ -28,8 +28,9 @@ use super::signature_verification::{
 
 #[derive(Debug,PartialEq)]
 pub enum AttestationValidationError {
-    SlotTooHigh,
-    SlotTooLow,
+    ParentSlotTooHigh,
+    BlockSlotTooHigh,
+    BlockSlotTooLow,
     JustifiedSlotIncorrect,
     InvalidJustifiedBlockHash,
     TooManyObliqueHashes,
@@ -54,6 +55,8 @@ pub struct AttestationValidationContext<T>
 {
     /// The slot as determined by the system time.
     pub block_slot: u64,
+    /// The slot of the parent of the block that contained this attestation.
+    pub parent_block_slot: u64,
     /// The cycle_length as determined by the chain configuration.
     pub cycle_length: u8,
     /// The last justified slot as per the client's view of the canonical chain.
@@ -82,10 +85,11 @@ impl<T> AttestationValidationContext<T>
         -> Result<HashSet<usize>, AttestationValidationError>
     {
         /*
-         * The attesation slot must not be higher than the block that contained it.
+         * The attesation slot must be less than or equal to the parent of the slot of the block
+         * that contained the attestation.
          */
-        if a.slot > self.block_slot {
-            return Err(AttestationValidationError::SlotTooHigh);
+        if a.slot > self.parent_block_slot {
+            return Err(AttestationValidationError::ParentSlotTooHigh);
         }
 
         /*
@@ -94,7 +98,7 @@ impl<T> AttestationValidationContext<T>
          */
         if a.slot < self.block_slot
             .saturating_sub(u64::from(self.cycle_length).saturating_add(1)) {
-            return Err(AttestationValidationError::SlotTooLow);
+            return Err(AttestationValidationError::BlockSlotTooLow);
         }
 
         /*
@@ -210,9 +214,9 @@ impl From<ParentHashesError> for AttestationValidationError {
             ParentHashesError::BadObliqueHashes
                 => AttestationValidationError::BadObliqueHashes,
             ParentHashesError::SlotTooLow
-                => AttestationValidationError::SlotTooLow,
+                => AttestationValidationError::BlockSlotTooLow,
             ParentHashesError::SlotTooHigh
-                => AttestationValidationError::SlotTooHigh,
+                => AttestationValidationError::BlockSlotTooHigh,
             ParentHashesError::IntWrapping
                 => AttestationValidationError::IntWrapping
         }

beacon_chain/validation/src/block_validation.rs

@@ -196,7 +196,7 @@ impl<T> BlockValidationContext<T>
          * Also, read the slot from the parent block for later use.
          */
         let parent_hash = b.parent_hash();
-        let parent_slot = match self.block_store.get_serialized_block(&parent_hash)? {
+        let parent_block_slot = match self.block_store.get_serialized_block(&parent_hash)? {
             None => return Err(SszBlockValidationError::UnknownParentHash),
             Some(ssz) => {
                 let parent_block = SszBlock::from_slice(&ssz[..])?;
@@ -209,6 +209,7 @@ impl<T> BlockValidationContext<T>
          */
         let attestation_validation_context = Arc::new(AttestationValidationContext {
             block_slot,
+            parent_block_slot,
             cycle_length: self.cycle_length,
             last_justified_slot: self.last_justified_slot,
             parent_hashes: self.parent_hashes.clone(),
@@ -230,7 +231,7 @@ impl<T> BlockValidationContext<T>
          * If the signature of proposer for the parent slot was not present in the first (0'th)
          * attestation of this block, reject the block.
          */
-        let parent_block_proposer = self.proposer_map.get(&parent_slot)
+        let parent_block_proposer = self.proposer_map.get(&parent_block_slot)
             .ok_or(SszBlockValidationError::BadProposerMap)?;
         if !attestation_voters.contains(&parent_block_proposer) {
             return Err(SszBlockValidationError::NoProposerSignature);

beacon_chain/validation/tests/attestation_validation/helpers.rs

@@ -169,6 +169,7 @@ pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
         .map(|i| Hash256::from(i as u64))
         .collect();
     let attestation_slot = block_slot - 1;
+    let parent_block_slot = attestation_slot;
     let last_justified_slot = attestation_slot - 1;
     let justified_block_hash = Hash256::from("justified_block".as_bytes());
     let shard_block_hash = Hash256::from("shard_block".as_bytes());
@@ -204,6 +205,7 @@ pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
 
     let context: AttestationValidationContext<MemoryDB> = AttestationValidationContext {
         block_slot,
+        parent_block_slot,
         cycle_length,
         last_justified_slot,
         parent_hashes: parent_hashes.clone(),

beacon_chain/validation/tests/attestation_validation/tests.rs

@@ -33,22 +33,32 @@ fn test_attestation_validation_valid() {
 }
 
 #[test]
-fn test_attestation_validation_invalid_slot_too_high() {
+fn test_attestation_validation_invalid_parent_slot_too_high() {
     let mut rig = generic_rig();
 
-    rig.attestation.slot = rig.context.block_slot + 1;
+    rig.context.parent_block_slot = rig.attestation.slot - 1;
 
     let result = rig.context.validate_attestation(&rig.attestation);
-    assert_eq!(result, Err(AttestationValidationError::SlotTooHigh));
+    assert_eq!(result, Err(AttestationValidationError::ParentSlotTooHigh));
 }
 
 #[test]
-fn test_attestation_validation_invalid_slot_too_low() {
+fn test_attestation_validation_invalid_block_slot_too_high() {
+    let mut rig = generic_rig();
+
+    rig.context.block_slot = rig.attestation.slot - 1;
+
+    let result = rig.context.validate_attestation(&rig.attestation);
+    assert_eq!(result, Err(AttestationValidationError::BlockSlotTooHigh));
+}
+
+#[test]
+fn test_attestation_validation_invalid_block_slot_too_low() {
     let mut rig = generic_rig();
 
     rig.attestation.slot = rig.context.block_slot - u64::from(rig.context.cycle_length) - 2;
     let result = rig.context.validate_attestation(&rig.attestation);
-    assert_eq!(result, Err(AttestationValidationError::SlotTooLow));
+    assert_eq!(result, Err(AttestationValidationError::BlockSlotTooLow));
 }
 
 #[test]