Fix bug with epoch caches, add tests

eth2/state_processing/src/per_block_processing.rs

@@ -32,7 +32,7 @@ const VERIFY_DEPOSIT_MERKLE_PROOFS: bool = false;
 /// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise
 /// returns an error describing why the block was invalid or how the function failed to execute.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 pub fn per_block_processing(
     state: &mut BeaconState,
     block: &BeaconBlock,
@@ -47,7 +47,7 @@ pub fn per_block_processing(
 /// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise
 /// returns an error describing why the block was invalid or how the function failed to execute.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 pub fn per_block_processing_without_verifying_block_signature(
     state: &mut BeaconState,
     block: &BeaconBlock,
@@ -62,7 +62,7 @@ pub fn per_block_processing_without_verifying_block_signature(
 /// Returns `Ok(())` if the block is valid and the state was successfully updated. Otherwise
 /// returns an error describing why the block was invalid or how the function failed to execute.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 fn per_block_processing_signature_optional(
     mut state: &mut BeaconState,
     block: &BeaconBlock,

eth2/state_processing/src/per_block_processing/verify_slashable_attestation.rs

@@ -10,7 +10,7 @@ use types::*;
 ///
 /// Returns `Ok(())` if the `SlashableAttestation` is valid, otherwise indicates the reason for invalidity.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 pub fn verify_slashable_attestation(
     state: &BeaconState,
     slashable_attestation: &SlashableAttestation,

eth2/state_processing/src/per_block_processing/verify_transfer.rs

@@ -94,7 +94,7 @@ pub fn verify_transfer(
 ///
 /// Does not check that the transfer is valid, however checks for overflow in all actions.
 ///
-/// Spec v0.4.0
+/// Spec v0.5.0
 pub fn execute_transfer(
     state: &mut BeaconState,
     transfer: &Transfer,

eth2/types/src/beacon_state.rs

@@ -279,7 +279,9 @@ impl BeaconState {
     fn cache(&self, relative_epoch: RelativeEpoch, spec: &ChainSpec) -> Result<&EpochCache, Error> {
         let cache = &self.caches[self.cache_index(relative_epoch)];
 
-        if cache.initialized_epoch == Some(self.slot.epoch(spec.slots_per_epoch)) {
+        let epoch = relative_epoch.into_epoch(self.slot.epoch(spec.slots_per_epoch));
+
+        if cache.initialized_epoch == Some(epoch) {
             Ok(cache)
         } else {
             Err(Error::EpochCacheUninitialized(relative_epoch))

eth2/types/src/beacon_state/epoch_cache.rs

@@ -159,7 +159,7 @@ impl EpochCrosslinkCommittees {
         let epoch_start_slot = self.epoch.start_slot(spec.slots_per_epoch);
         let epoch_end_slot = self.epoch.end_slot(spec.slots_per_epoch);
 
-        if (epoch_start_slot < slot) && (slot <= epoch_end_slot) {
+        if (epoch_start_slot <= slot) && (slot <= epoch_end_slot) {
             let index = slot - epoch_start_slot;
             self.crosslink_committees.get(index.as_usize())
         } else {

eth2/types/src/beacon_state/tests.rs

@@ -1,5 +1,57 @@
 #![cfg(test)]
-
 use super::*;
+use crate::test_utils::*;
 
 ssz_tests!(BeaconState);
+
+/// Test that
+///
+/// 1. Using the cache before it's built fails.
+/// 2. Using the cache after it's build passes.
+/// 3. Using the cache after it's dropped fails.
+fn test_cache_initialization<'a>(
+    state: &'a mut BeaconState,
+    relative_epoch: RelativeEpoch,
+    spec: &ChainSpec,
+) {
+    let slot = relative_epoch
+        .into_epoch(state.slot.epoch(spec.slots_per_epoch))
+        .start_slot(spec.slots_per_epoch);
+
+    // Assuming the cache isn't already built, assert that a call to a cache-using function fails.
+    assert_eq!(
+        state.get_beacon_proposer_index(slot, relative_epoch, spec),
+        Err(BeaconStateError::EpochCacheUninitialized(relative_epoch))
+    );
+
+    // Build the cache.
+    state.build_epoch_cache(relative_epoch, spec).unwrap();
+
+    // Assert a call to a cache-using function passes.
+    let _ = state
+        .get_beacon_proposer_index(slot, relative_epoch, spec)
+        .unwrap();
+
+    // Drop the cache.
+    state.drop_cache(relative_epoch);
+
+    // Assert a call to a cache-using function fail.
+    assert_eq!(
+        state.get_beacon_proposer_index(slot, relative_epoch, spec),
+        Err(BeaconStateError::EpochCacheUninitialized(relative_epoch))
+    );
+}
+
+#[test]
+fn cache_initialization() {
+    let spec = ChainSpec::few_validators();
+    let (mut state, _keypairs) =
+        TestingBeaconStateBuilder::from_default_keypairs_file_if_exists(16, &spec).build();
+
+    state.slot = (spec.genesis_epoch + 1).start_slot(spec.slots_per_epoch);
+
+    test_cache_initialization(&mut state, RelativeEpoch::Previous, &spec);
+    test_cache_initialization(&mut state, RelativeEpoch::Current, &spec);
+    test_cache_initialization(&mut state, RelativeEpoch::NextWithRegistryChange, &spec);
+    test_cache_initialization(&mut state, RelativeEpoch::NextWithoutRegistryChange, &spec);
+}

eth2/types/src/relative_epoch.rs

@@ -74,3 +74,61 @@ impl RelativeEpoch {
         )
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_into_epoch() {
+        let base = Epoch::new(10);
+
+        assert_eq!(RelativeEpoch::Current.into_epoch(base), base);
+        assert_eq!(RelativeEpoch::Previous.into_epoch(base), base - 1);
+        assert_eq!(
+            RelativeEpoch::NextWithRegistryChange.into_epoch(base),
+            base + 1
+        );
+        assert_eq!(
+            RelativeEpoch::NextWithoutRegistryChange.into_epoch(base),
+            base + 1
+        );
+    }
+
+    #[test]
+    fn from_epoch() {
+        let base = Epoch::new(10);
+
+        assert_eq!(
+            RelativeEpoch::from_epoch(base, base - 1),
+            Ok(RelativeEpoch::Previous)
+        );
+        assert_eq!(
+            RelativeEpoch::from_epoch(base, base),
+            Ok(RelativeEpoch::Current)
+        );
+        assert_eq!(
+            RelativeEpoch::from_epoch(base, base + 1),
+            Err(RelativeEpochError::AmbiguiousNextEpoch)
+        );
+    }
+
+    #[test]
+    fn from_slot() {
+        let spec = ChainSpec::foundation();
+        let base = Epoch::new(10).start_slot(spec.slots_per_epoch);
+
+        assert_eq!(
+            RelativeEpoch::from_slot(base, base - 1, &spec),
+            Ok(RelativeEpoch::Previous)
+        );
+        assert_eq!(
+            RelativeEpoch::from_slot(base, base, &spec),
+            Ok(RelativeEpoch::Current)
+        );
+        assert_eq!(
+            RelativeEpoch::from_slot(base, base + spec.slots_per_epoch, &spec),
+            Err(RelativeEpochError::AmbiguiousNextEpoch)
+        );
+    }
+}