Fixes to diff checks in EF tests

consensus/types/src/beacon_state/committee_cache.rs

@@ -3,6 +3,7 @@
 use super::BeaconState;
 use crate::*;
 use core::num::NonZeroUsize;
+use derivative::Derivative;
 use safe_arith::SafeArith;
 use serde_derive::{Deserialize, Serialize};
 use ssz::{four_byte_option_impl, Decode, DecodeError, Encode};
@@ -20,16 +21,43 @@ four_byte_option_impl!(four_byte_option_non_zero_usize, NonZeroUsize);
 
 /// Computes and stores the shuffling for an epoch. Provides various getters to allow callers to
 /// read the committees for the given epoch.
-#[derive(Debug, Default, PartialEq, Clone, Serialize, Deserialize, Encode, Decode)]
+#[derive(Derivative, Debug, Default, Clone, Serialize, Deserialize, Encode, Decode)]
+#[derivative(PartialEq)]
 pub struct CommitteeCache {
     #[ssz(with = "four_byte_option_epoch")]
     initialized_epoch: Option<Epoch>,
     shuffling: Vec<usize>,
+    #[derivative(PartialEq(compare_with = "compare_shuffling_positions"))]
     shuffling_positions: Vec<NonZeroUsizeOption>,
     committees_per_slot: u64,
     slots_per_epoch: u64,
 }
 
+/// Equivalence function for `shuffling_positions` that ignores trailing `None` entries.
+///
+/// It can happen that states from different epochs computing the same cache have different
+/// numbers of validators in `state.validators()` due to recent deposits. These new validators
+/// cannot be active however and will always be ommitted from the shuffling. This function checks
+/// that two lists of shuffling positions are equivalent by ensuring that they are identical on all
+/// common entries, and that new entries at the end are all `None`.
+///
+/// In practice this is only used in tests.
+fn compare_shuffling_positions(xs: &Vec<NonZeroUsizeOption>, ys: &Vec<NonZeroUsizeOption>) -> bool {
+    use std::cmp::Ordering;
+
+    let (shorter, longer) = match xs.len().cmp(&ys.len()) {
+        Ordering::Equal => {
+            return xs == ys;
+        }
+        Ordering::Less => (xs, ys),
+        Ordering::Greater => (ys, xs),
+    };
+    shorter == &longer[..shorter.len()]
+        && longer[shorter.len()..]
+            .iter()
+            .all(|new| *new == NonZeroUsizeOption(None))
+}
+
 impl CommitteeCache {
     /// Return a new, fully initialized cache.
     ///

testing/ef_tests/src/cases/sanity_blocks.rs

@@ -127,8 +127,15 @@ impl<E: EthSpec> Case for SanityBlocks<E> {
         };
         compare_beacon_state_results_without_caches(&mut indiv_result, &mut expected)?;
         compare_beacon_state_results_without_caches(&mut bulk_result, &mut expected)?;
-        check_state_diff(&self.pre, &self.post)?;
 
+        // Check state diff (requires fully built committee caches).
+        let mut pre = self.pre.clone();
+        pre.build_all_committee_caches(spec).unwrap();
+        let post = self.post.clone().map(|mut post| {
+            post.build_all_committee_caches(spec).unwrap();
+            post
+        });
+        check_state_diff(&pre, &post)?;
         Ok(())
     }
 }

testing/ef_tests/src/cases/sanity_slots.rs

@@ -68,6 +68,14 @@ impl<E: EthSpec> Case for SanitySlots<E> {
             .map(|_| state);
 
         compare_beacon_state_results_without_caches(&mut result, &mut expected)?;
-        check_state_diff(&self.pre, &self.post)
+
+        // Check state diff (requires fully built committee caches).
+        let mut pre = self.pre.clone();
+        pre.build_all_committee_caches(spec).unwrap();
+        let post = self.post.clone().map(|mut post| {
+            post.build_all_committee_caches(spec).unwrap();
+            post
+        });
+        check_state_diff(&pre, &post)
     }
 }