diff --git a/consensus/types/src/beacon_state/committee_cache.rs b/consensus/types/src/beacon_state/committee_cache.rs index 4c613c3f54..125ac289a7 100644 --- a/consensus/types/src/beacon_state/committee_cache.rs +++ b/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, shuffling: Vec, + #[derivative(PartialEq(compare_with = "compare_shuffling_positions"))] shuffling_positions: Vec, 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, ys: &Vec) -> 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. /// diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index b40fa7b280..cc47853c5a 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -127,8 +127,15 @@ impl Case for SanityBlocks { }; 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(()) } } diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index f809081839..bc4fe8d0b1 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -68,6 +68,14 @@ impl Case for SanitySlots { .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) } }