Verify attestation justified_block_hash.

Previously there was not a check that the hash was in the chain, just
that it was known (in the database in any chain)

beacon_chain/validation/src/attestation_validation.rs

@@ -12,7 +12,11 @@ use super::db::{
     ClientDB,
     DBError
 };
-use super::db::stores::ValidatorStore;
+use super::db::stores::{
+    BlockStore,
+    BlockAtSlotError,
+    ValidatorStore,
+};
 use super::types::{
     Hash256,
 };
@@ -27,7 +31,7 @@ pub enum AttestationValidationError {
     SlotTooHigh,
     SlotTooLow,
     JustifiedSlotIncorrect,
-    UnknownJustifiedBlock,
+    InvalidJustifiedBlockHash,
     TooManyObliqueHashes,
     BadCurrentHashes,
     BadObliqueHashes,
@@ -54,10 +58,10 @@ pub struct AttestationValidationContext<T>
     pub cycle_length: u8,
     /// The last justified slot as per the client's view of the canonical chain.
     pub last_justified_slot: u64,
-    /// The last justified block hash as per the client's view of the canonical chain.
-    pub last_justified_block_hash: Hash256,
     /// A vec of the hashes of the blocks preceeding the present slot.
     pub parent_hashes: Arc<Vec<Hash256>>,
+    /// The store containing block information.
+    pub block_store: Arc<BlockStore<T>>,
     /// The store containing validator information.
     pub validator_store: Arc<ValidatorStore<T>>,
     /// A map of (slot, shard_id) to the attestation set of validation indices.
@@ -97,18 +101,10 @@ impl<T> AttestationValidationContext<T>
          * 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 {
+        if a.justified_slot > self.last_justified_slot {
             return Err(AttestationValidationError::JustifiedSlotIncorrect);
         }
 
-        /*
-         * The specified justified block hash supplied in the attestation must match our knowledge
-         * of the last justified block this chain.
-         */
-        if a.justified_block_hash != self.last_justified_block_hash {
-            return Err(AttestationValidationError::UnknownJustifiedBlock)
-        }
-
         /*
          * There is no need to include more oblique parents hashes than there are blocks
          * in a cycle.
@@ -147,13 +143,35 @@ impl<T> AttestationValidationContext<T>
             return Err(AttestationValidationError::InvalidBitfieldEndBits)
         }
 
+        /*
+         * Generate the parent hashes for this attestation
+         */
+        let parent_hashes = attestation_parent_hashes(
+            self.cycle_length,
+            self.block_slot,
+            a.slot,
+            &self.parent_hashes,
+            &a.oblique_parent_hashes)?;
+
+        /*
+         * The specified justified block hash supplied in the attestation must be in the chain at
+         * the given slot number.
+         *
+         * First, we find the latest parent hash from the parent_hashes array. Then, using the
+         * block store (database) we iterate back through the blocks until we find (or fail to
+         * find) the justified block hash referenced in the attestation record.
+         */
+        let latest_parent_hash = parent_hashes.last()
+            .ok_or(AttestationValidationError::BadCurrentHashes)?;
+        match self.block_store.block_at_slot(&latest_parent_hash, a.justified_slot)? {
+            Some((ref hash, _)) if *hash == a.justified_block_hash.to_vec() => (),
+            _ => return Err(AttestationValidationError::InvalidJustifiedBlockHash)
+        };
+
+        /*
+         * Generate the message that this attestation aggregate signature must sign across.
+         */
         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,
@@ -201,6 +219,16 @@ impl From<ParentHashesError> for AttestationValidationError {
     }
 }
 
+impl From<BlockAtSlotError> for AttestationValidationError {
+    fn from(e: BlockAtSlotError) -> Self {
+        match e {
+            BlockAtSlotError::DBError(s) => AttestationValidationError::DBError(s),
+            _ => AttestationValidationError::InvalidJustifiedBlockHash
+
+        }
+    }
+}
+
 impl From<DBError> for AttestationValidationError {
     fn from(e: DBError) -> Self {
         AttestationValidationError::DBError(e.message)

beacon_chain/validation/src/block_validation.rs

@@ -207,8 +207,8 @@ impl<T> BlockValidationContext<T>
             block_slot,
             cycle_length: self.cycle_length,
             last_justified_slot: self.last_justified_slot,
-            last_justified_block_hash: self.last_justified_block_hash,
             parent_hashes: self.parent_hashes.clone(),
+            block_store: self.block_store.clone(),
             validator_store: self.validator_store.clone(),
             attester_map: self.attester_map.clone(),
         });

beacon_chain/validation/tests/attestation_validation/helpers.rs

@@ -3,11 +3,15 @@ use std::sync::Arc;
 use super::db::{
     MemoryDB,
 };
-use super::db::stores::ValidatorStore;
+use super::db::stores::{
+    ValidatorStore,
+    BlockStore,
+};
 use super::types::{
     AttestationRecord,
     AttesterMap,
     Bitfield,
+    Block,
     Hash256,
 };
 use super::validation::attestation_validation::{
@@ -28,15 +32,18 @@ use super::hashing::{
 pub struct TestStore {
     pub db: Arc<MemoryDB>,
     pub validator: Arc<ValidatorStore<MemoryDB>>,
+    pub block: Arc<BlockStore<MemoryDB>>,
 }
 
 impl TestStore {
     pub fn new() -> Self {
         let db = Arc::new(MemoryDB::open());
         let validator = Arc::new(ValidatorStore::new(db.clone()));
+        let block = Arc::new(BlockStore::new(db.clone()));
         Self {
             db,
             validator,
+            block,
         }
     }
 }
@@ -73,7 +80,8 @@ pub fn generate_attestation(shard_id: u16,
                             justified_block_hash: &Hash256,
                             cycle_length: u8,
                             parent_hashes: &[Hash256],
-                            signing_keys: &[Option<SecretKey>])
+                            signing_keys: &[Option<SecretKey>],
+                            block_store: &BlockStore<MemoryDB>)
     -> AttestationRecord
 {
     let mut attester_bitfield = Bitfield::new();
@@ -86,6 +94,11 @@ pub fn generate_attestation(shard_id: u16,
         &parent_hashes[first..last]
     };
 
+    /*
+     * Create a justified block at the correct slot and store it in the db.
+     */
+    create_block_at_slot(&block_store, &justified_block_hash, justified_slot);
+
     /*
      * Generate the message that will be signed across for this attr record.
      */
@@ -120,6 +133,31 @@ pub fn generate_attestation(shard_id: u16,
     }
 }
 
+/// Create a minimum viable block at some slot.
+///
+/// Allows the validation function to read the block and verify its slot.
+pub fn create_block_at_slot(block_store: &BlockStore<MemoryDB>, hash: &Hash256, slot: u64) {
+    let mut justified_block = Block::zero();
+    justified_block.attestations.push(AttestationRecord::zero());
+    justified_block.slot_number = slot;
+    let mut s = SszStream::new();
+    s.append(&justified_block);
+    let justified_block_ssz = s.drain();
+    block_store.put_serialized_block(&hash.to_vec(), &justified_block_ssz).unwrap();
+}
+
+/// Inserts a justified_block_hash in a position that will be referenced by an attestation record.
+pub fn insert_justified_block_hash(
+    parent_hashes: &mut Vec<Hash256>,
+    justified_block_hash: &Hash256,
+    block_slot: u64,
+    attestation_slot: u64)
+{
+    let attestation_parent_hash_index = parent_hashes.len() - 1 -
+        (block_slot as usize - attestation_slot as usize);
+    parent_hashes[attestation_parent_hash_index] = justified_block_hash.clone();
+}
+
 pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
     -> TestRig
 {
@@ -127,15 +165,24 @@ pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
 
     let block_slot = 10000;
     let cycle_length: u8 = 64;
-    let last_justified_slot = block_slot - u64::from(cycle_length);
+    let mut parent_hashes: Vec<Hash256> = (0..(cycle_length * 2))
-    let parent_hashes: Vec<Hash256> = (0..(cycle_length * 2))
         .map(|i| Hash256::from(i as u64))
         .collect();
-    let parent_hashes = Arc::new(parent_hashes);
+    let attestation_slot = block_slot - 1;
+    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());
 
-    let attestation_slot = block_slot - 1;
+    /*
+     * Insert the required justified_block_hash into parent_hashes
+     */
+    insert_justified_block_hash(
+        &mut parent_hashes,
+        &justified_block_hash,
+        block_slot,
+        attestation_slot);
+
+    let parent_hashes = Arc::new(parent_hashes);
 
     let mut keypairs = vec![];
     let mut signing_keys = vec![];
@@ -159,8 +206,8 @@ pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
         block_slot,
         cycle_length,
         last_justified_slot,
-        last_justified_block_hash: justified_block_hash,
         parent_hashes: parent_hashes.clone(),
+        block_store: stores.block.clone(),
         validator_store: stores.validator.clone(),
         attester_map: Arc::new(attester_map),
     };
@@ -173,7 +220,8 @@ pub fn setup_attestation_validation_test(shard_id: u16, attester_count: usize)
         &justified_block_hash,
         cycle_length,
         &parent_hashes.clone(),
-        &signing_keys);
+        &signing_keys,
+        &stores.block);
 
     TestRig {
         attestation,

beacon_chain/validation/tests/attestation_validation/tests.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 use super::helpers::{
     TestRig,
     setup_attestation_validation_test,
+    create_block_at_slot,
 };
 use super::validation::attestation_validation::{
     AttestationValidationError,
@@ -56,12 +57,25 @@ fn test_attestation_validation_invalid_justified_slot_incorrect() {
 
     let original = rig.attestation.justified_slot;
     rig.attestation.justified_slot = original - 1;
+    // Ensures we don't get a bad justified block error instead.
+    create_block_at_slot(
+        &rig.stores.block,
+        &rig.attestation.justified_block_hash,
+        rig.attestation.justified_slot);
     let result = rig.context.validate_attestation(&rig.attestation);
-    assert_eq!(result, Err(AttestationValidationError::JustifiedSlotIncorrect));
+    assert_eq!(result, Err(AttestationValidationError::BadAggregateSignature));
 
     rig.attestation.justified_slot = original + 1;
+    // Ensures we don't get a bad justified block error instead.
+    create_block_at_slot(
+        &rig.stores.block,
+        &rig.attestation.justified_block_hash,
+        rig.attestation.justified_slot);
+    // Ensures we don't get an error that the last justified slot is ahead of the context justified
+    // slot.
+    rig.context.last_justified_slot = rig.attestation.justified_slot;
     let result = rig.context.validate_attestation(&rig.attestation);
-    assert_eq!(result, Err(AttestationValidationError::JustifiedSlotIncorrect));
+    assert_eq!(result, Err(AttestationValidationError::BadAggregateSignature));
 }
 
 #[test]
@@ -139,13 +153,38 @@ fn test_attestation_validation_invalid_invalid_bitfield_end_bit_with_irreguar_bi
 }
 
 #[test]
-fn test_attestation_validation_invalid_unknown_justfied_block_hash() {
+fn test_attestation_validation_invalid_unknown_justified_block_hash() {
     let mut rig = generic_rig();
 
     rig.attestation.justified_block_hash = Hash256::from("unknown block hash".as_bytes());
 
     let result = rig.context.validate_attestation(&rig.attestation);
-    assert_eq!(result, Err(AttestationValidationError::UnknownJustifiedBlock));
+    assert_eq!(result, Err(AttestationValidationError::InvalidJustifiedBlockHash));
+}
+
+#[test]
+fn test_attestation_validation_invalid_unknown_justified_block_hash_wrong_slot() {
+    let rig = generic_rig();
+
+    /*
+     * justified_block_hash points to a block with a slot that is too high.
+     */
+    create_block_at_slot(
+        &rig.stores.block,
+        &rig.attestation.justified_block_hash,
+        rig.attestation.justified_slot + 1);
+    let result = rig.context.validate_attestation(&rig.attestation);
+    assert_eq!(result, Err(AttestationValidationError::InvalidJustifiedBlockHash));
+
+    /*
+     * justified_block_hash points to a block with a slot that is too low.
+     */
+    create_block_at_slot(
+        &rig.stores.block,
+        &rig.attestation.justified_block_hash,
+        rig.attestation.justified_slot - 1);
+    let result = rig.context.validate_attestation(&rig.attestation);
+    assert_eq!(result, Err(AttestationValidationError::InvalidJustifiedBlockHash));
 }
 
 #[test]

beacon_chain/validation/tests/block_validation/helpers.rs

@@ -1,6 +1,9 @@
 use std::sync::Arc;
 
-use super::generate_attestation;
+use super::attestation_validation::helpers::{
+    generate_attestation,
+    insert_justified_block_hash,
+};
 use super::bls::{
     Keypair,
 };
@@ -82,7 +85,7 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams)
     let block_slot = params.block_slot;
     let attestations_justified_slot = params.attestations_justified_slot;
 
-    let parent_hashes: Vec<Hash256> = (0..(cycle_length * 2))
+    let mut parent_hashes: Vec<Hash256> = (0..(cycle_length * 2))
         .map(|i| Hash256::from(i as u64))
         .collect();
     let parent_hash = Hash256::from("parent_hash".as_bytes());
@@ -93,6 +96,9 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams)
     let crystallized_state_root = Hash256::from("cry_state".as_bytes());
     let shard_block_hash = Hash256::from("shard_block_hash".as_bytes());
 
+    /*
+     * Store a valid PoW chain ref
+     */
     stores.pow_chain.put_block_hash(pow_chain_ref.as_ref()).unwrap();
 
     /*
@@ -117,6 +123,15 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams)
         let mut attester_map = AttesterMap::new();
         let mut attestations = vec![];
         let mut keypairs = vec![];
+
+        /*
+         * Insert the required justified_block_hash into parent_hashes
+         */
+        insert_justified_block_hash(
+            &mut parent_hashes,
+            &justified_block_hash,
+            block_slot,
+            attestation_slot);
         /*
          * For each shard in this slot, generate an attestation.
          */
@@ -146,7 +161,8 @@ pub fn setup_block_validation_scenario(params: &BlockTestParams)
                 &justified_block_hash,
                 cycle_length,
                 &parent_hashes,
-                &signing_keys[..]);
+                &signing_keys[..],
+                &stores.block);
             attestations.push(attestation);
         }
         (attester_map, attestations, keypairs)

beacon_chain/validation/tests/block_validation/mod.rs

@@ -9,4 +9,4 @@ use super::ssz_helpers;
 use super::types;
 use super::validation;
 
-use super::attestation_validation::helpers::generate_attestation;
+use super::attestation_validation;