Add caching for state.eth1_data_votes (#919)

## Issue Addressed

NA

## Proposed Changes

Adds additional tree hash caching for `state.eth1_data_votes`.

Presently, each time we tree hash the `BeaconState`, we recompute the `state.eth1_data_votes` tree in it's entirety. This is because we only previous had support for caching fixed-length lists.

This PR adds the `Eth1DataVotesTreeHashCache` which provides caching for the `state.eth1_data_votes` list. The cache is aware of `SLOTS_PER_ETH1_VOTING_PERIOD` and will reset itself whenever that boundary is crossed.

This cache adds a new (but somewhat fundamental) restriction to tree hash caching:

*For some state `s`, `s.tree_hash_cache` is only valid for `s` or descendants of `s` that have been reached via state transitions that are faithful to the specification (invalid blocks are permitted, as long as they are faithfully processed).*

consensus/tree_hash/benches/benches.rs

@@ -34,7 +34,7 @@ fn bench_suite<T: EthSpec>(c: &mut Criterion, spec_desc: &str, validator_count:
     let state1 = build_state::<T>(validator_count);
     let state2 = state1.clone();
     let mut state3 = state1.clone();
-    state3.build_tree_hash_cache().unwrap();
+    state3.update_tree_hash_cache().unwrap();
 
     c.bench(
         &format!("{}/{}_validators/no_cache", spec_desc, validator_count),

consensus/types/src/beacon_state.rs

@@ -70,6 +70,11 @@ pub enum Error {
     CommitteeCacheUninitialized(Option<RelativeEpoch>),
     SszTypesError(ssz_types::Error),
     TreeHashCacheNotInitialized,
+    NonLinearTreeHashCacheHistory,
+    TreeHashCacheSkippedSlot {
+        cache: Slot,
+        state: Slot,
+    },
     TreeHashError(tree_hash::Error),
     CachedTreeHashError(cached_tree_hash::Error),
     InvalidValidatorPubkey(ssz::DecodeError),
@@ -217,7 +222,7 @@ where
     #[ssz(skip_deserializing)]
     #[tree_hash(skip_hashing)]
     #[test_random(default)]
-    pub tree_hash_cache: Option<BeaconTreeHashCache>,
+    pub tree_hash_cache: Option<BeaconTreeHashCache<T>>,
 }
 
 impl<T: EthSpec> BeaconState<T> {
@@ -879,7 +884,6 @@ impl<T: EthSpec> BeaconState<T> {
     pub fn build_all_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> {
         self.build_all_committee_caches(spec)?;
         self.update_pubkey_cache()?;
-        self.build_tree_hash_cache()?;
         self.exit_cache.build(&self.validators, spec)?;
 
         Ok(())
@@ -1013,17 +1017,6 @@ impl<T: EthSpec> BeaconState<T> {
         }
     }
 
-    /// Build and update the tree hash cache if it isn't already initialized.
-    pub fn build_tree_hash_cache(&mut self) -> Result<(), Error> {
-        self.update_tree_hash_cache().map(|_| ())
-    }
-
-    /// Build the tree hash cache, with blatant disregard for any existing cache.
-    pub fn force_build_tree_hash_cache(&mut self) -> Result<(), Error> {
-        self.tree_hash_cache = None;
-        self.build_tree_hash_cache()
-    }
-
     /// Compute the tree hash root of the state using the tree hash cache.
     ///
     /// Initialize the tree hash cache if it isn't already initialized.
@@ -1125,15 +1118,15 @@ impl<T: EthSpec> BeaconState<T> {
 
 /// This implementation primarily exists to satisfy some testing requirements (ef_tests). It is
 /// recommended to use the methods directly on the beacon state instead.
-impl<T: EthSpec> CachedTreeHash<BeaconTreeHashCache> for BeaconState<T> {
+impl<T: EthSpec> CachedTreeHash<BeaconTreeHashCache<T>> for BeaconState<T> {
-    fn new_tree_hash_cache(&self, _arena: &mut CacheArena) -> BeaconTreeHashCache {
+    fn new_tree_hash_cache(&self, _arena: &mut CacheArena) -> BeaconTreeHashCache<T> {
         BeaconTreeHashCache::new(self)
     }
 
     fn recalculate_tree_hash_root(
         &self,
         _arena: &mut CacheArena,
-        cache: &mut BeaconTreeHashCache,
+        cache: &mut BeaconTreeHashCache<T>,
     ) -> Result<Hash256, cached_tree_hash::Error> {
         cache
             .recalculate_tree_hash_root(self)

consensus/types/src/beacon_state/tests.rs

@@ -180,6 +180,9 @@ fn clone_config() {
     let (mut state, _keypairs) = builder.build();
 
     state.build_all_caches(&spec).unwrap();
+    state
+        .update_tree_hash_cache()
+        .expect("should update tree hash cache");
 
     let num_caches = 4;
     let all_configs = (0..2u8.pow(num_caches)).map(|i| CloneConfig {
@@ -207,8 +210,47 @@ fn tree_hash_cache() {
 
     assert_eq!(root.as_bytes(), &state.tree_hash_root()[..]);
 
+    /*
+     * A cache should hash twice without updating the slot.
+     */
+
+    assert_eq!(
+        state.update_tree_hash_cache().unwrap(),
+        root,
+        "tree hash result should be identical on the same slot"
+    );
+
+    /*
+     * A cache should not hash after updating the slot but not updating the state roots.
+     */
+
+    // The tree hash cache needs to be rebuilt since it was dropped when it failed.
+    state
+        .update_tree_hash_cache()
+        .expect("should rebuild cache");
+
     state.slot += 1;
 
+    assert_eq!(
+        state.update_tree_hash_cache(),
+        Err(BeaconStateError::NonLinearTreeHashCacheHistory),
+        "should not build hash without updating the state root"
+    );
+
+    /*
+     * The cache should update if the slot and state root are updated.
+     */
+
+    // The tree hash cache needs to be rebuilt since it was dropped when it failed.
+    let root = state
+        .update_tree_hash_cache()
+        .expect("should rebuild cache");
+
+    state.slot += 1;
+    state
+        .set_state_root(state.slot - 1, root)
+        .expect("should set state root");
+
     let root = state.update_tree_hash_cache().unwrap();
     assert_eq!(root.as_bytes(), &state.tree_hash_root()[..]);
 }

consensus/types/src/beacon_state/tree_hash_cache.rs

@@ -1,10 +1,11 @@
 #![allow(clippy::integer_arithmetic)]
 
 use super::Error;
-use crate::{BeaconState, EthSpec, Hash256, Unsigned, Validator};
+use crate::{BeaconState, EthSpec, Hash256, Slot, Unsigned, Validator};
 use cached_tree_hash::{int_log, CacheArena, CachedTreeHash, TreeHashCache};
 use rayon::prelude::*;
 use ssz_derive::{Decode, Encode};
+use ssz_types::VariableList;
 use std::cmp::Ordering;
 use tree_hash::{mix_in_length, MerkleHasher, TreeHash};
 
@@ -22,9 +23,66 @@ const NODES_PER_VALIDATOR: usize = 15;
 /// Do not set to 0.
 const VALIDATORS_PER_ARENA: usize = 4_096;
 
+#[derive(Debug, PartialEq, Clone, Encode, Decode)]
+pub struct Eth1DataVotesTreeHashCache<T: EthSpec> {
+    arena: CacheArena,
+    tree_hash_cache: TreeHashCache,
+    voting_period: u64,
+    roots: VariableList<Hash256, T::SlotsPerEth1VotingPeriod>,
+}
+
+impl<T: EthSpec> Eth1DataVotesTreeHashCache<T> {
+    /// Instantiates a new cache.
+    ///
+    /// Allocates the necessary memory to store all of the cached Merkle trees. Only the leaves are
+    /// hashed, leaving the internal nodes as all-zeros.
+    pub fn new(state: &BeaconState<T>) -> Self {
+        let mut arena = CacheArena::default();
+        let roots: VariableList<_, _> = state
+            .eth1_data_votes
+            .iter()
+            .map(|eth1_data| eth1_data.tree_hash_root())
+            .collect::<Vec<_>>()
+            .into();
+        let tree_hash_cache = roots.new_tree_hash_cache(&mut arena);
+
+        Self {
+            arena,
+            tree_hash_cache,
+            voting_period: Self::voting_period(state.slot),
+            roots,
+        }
+    }
+
+    fn voting_period(slot: Slot) -> u64 {
+        slot.as_u64() / T::SlotsPerEth1VotingPeriod::to_u64()
+    }
+
+    pub fn recalculate_tree_hash_root(&mut self, state: &BeaconState<T>) -> Result<Hash256, Error> {
+        if state.eth1_data_votes.len() < self.roots.len()
+            || Self::voting_period(state.slot) != self.voting_period
+        {
+            *self = Self::new(state);
+        }
+
+        state
+            .eth1_data_votes
+            .iter()
+            .skip(self.roots.len())
+            .try_for_each(|eth1_data| self.roots.push(eth1_data.tree_hash_root()))?;
+
+        self.roots
+            .recalculate_tree_hash_root(&mut self.arena, &mut self.tree_hash_cache)
+            .map_err(Into::into)
+    }
+}
+
 /// A cache that performs a caching tree hash of the entire `BeaconState` struct.
-#[derive(Debug, PartialEq, Clone, Default, Encode, Decode)]
+#[derive(Debug, PartialEq, Clone, Encode, Decode)]
-pub struct BeaconTreeHashCache {
+pub struct BeaconTreeHashCache<T: EthSpec> {
+    /// Tracks the previously generated state root to ensure the next state root provided descends
+    /// directly from this state.
+    previous_state: Option<(Hash256, Slot)>,
     // Validators cache
     validators: ValidatorsListTreeHashCache,
     // Arenas
@@ -38,14 +96,15 @@ pub struct BeaconTreeHashCache {
     balances: TreeHashCache,
     randao_mixes: TreeHashCache,
     slashings: TreeHashCache,
+    eth1_data_votes: Eth1DataVotesTreeHashCache<T>,
 }
 
-impl BeaconTreeHashCache {
+impl<T: EthSpec> BeaconTreeHashCache<T> {
     /// Instantiates a new cache.
     ///
-    /// Allocates the necessary memory to store all of the cached Merkle trees but does perform any
+    /// Allocates the necessary memory to store all of the cached Merkle trees. Only the leaves are
-    /// hashing.
+    /// hashed, leaving the internal nodes as all-zeros.
-    pub fn new<T: EthSpec>(state: &BeaconState<T>) -> Self {
+    pub fn new(state: &BeaconState<T>) -> Self {
         let mut fixed_arena = CacheArena::default();
         let block_roots = state.block_roots.new_tree_hash_cache(&mut fixed_arena);
         let state_roots = state.state_roots.new_tree_hash_cache(&mut fixed_arena);
@@ -61,6 +120,7 @@ impl BeaconTreeHashCache {
         let slashings = state.slashings.new_tree_hash_cache(&mut slashings_arena);
 
         Self {
+            previous_state: None,
             validators,
             fixed_arena,
             balances_arena,
@@ -71,6 +131,7 @@ impl BeaconTreeHashCache {
             balances,
             randao_mixes,
             slashings,
+            eth1_data_votes: Eth1DataVotesTreeHashCache::new(state),
         }
     }
 
@@ -78,10 +139,29 @@ impl BeaconTreeHashCache {
     ///
     /// The provided `state` should be a descendant of the last `state` given to this function, or
     /// the `Self::new` function.
-    pub fn recalculate_tree_hash_root<T: EthSpec>(
+    pub fn recalculate_tree_hash_root(&mut self, state: &BeaconState<T>) -> Result<Hash256, Error> {
-        &mut self,
+        // If this cache has previously produced a root, ensure that it is in the state root
-        state: &BeaconState<T>,
+        // history of this state.
-    ) -> Result<Hash256, Error> {
+        //
+        // This ensures that the states applied have a linear history, this
+        // allows us to make assumptions about how the state changes over times and produce a more
+        // efficient algorithm.
+        if let Some((previous_root, previous_slot)) = self.previous_state {
+            // The previously-hashed state must not be newer than `state`.
+            if previous_slot > state.slot {
+                return Err(Error::TreeHashCacheSkippedSlot {
+                    cache: previous_slot,
+                    state: state.slot,
+                });
+            }
+
+            // If the state is newer, the previous root must be in the history of the given state.
+            if previous_slot < state.slot && *state.get_state_root(previous_slot)? != previous_root
+            {
+                return Err(Error::NonLinearTreeHashCacheHistory);
+            }
+        }
+
         let mut hasher = MerkleHasher::with_leaves(NUM_BEACON_STATE_HASHING_FIELDS);
 
         hasher.write(state.genesis_time.tree_hash_root().as_bytes())?;
@@ -108,7 +188,11 @@ impl BeaconTreeHashCache {
                 .as_bytes(),
         )?;
         hasher.write(state.eth1_data.tree_hash_root().as_bytes())?;
-        hasher.write(state.eth1_data_votes.tree_hash_root().as_bytes())?;
+        hasher.write(
+            self.eth1_data_votes
+                .recalculate_tree_hash_root(&state)?
+                .as_bytes(),
+        )?;
         hasher.write(state.eth1_deposit_index.tree_hash_root().as_bytes())?;
         hasher.write(
             self.validators
@@ -155,7 +239,11 @@ impl BeaconTreeHashCache {
         )?;
         hasher.write(state.finalized_checkpoint.tree_hash_root().as_bytes())?;
 
-        hasher.finish().map_err(Into::into)
+        let root = hasher.finish()?;
+
+        self.previous_state = Some((root, state.slot));
+
+        Ok(root)
     }
 
     /// Updates the cache and provides the root of the given `validators`.

testing/ef_tests/tests/tests.rs

@@ -180,8 +180,8 @@ mod ssz_static {
     ssz_static_test!(
         beacon_state,
         SszStaticTHCHandler, {
-            (BeaconState<MinimalEthSpec>, BeaconTreeHashCache, MinimalEthSpec),
+            (BeaconState<MinimalEthSpec>, BeaconTreeHashCache<_>, MinimalEthSpec),
-            (BeaconState<MainnetEthSpec>, BeaconTreeHashCache, MainnetEthSpec)
+            (BeaconState<MainnetEthSpec>, BeaconTreeHashCache<_>, MainnetEthSpec)
         }
     );
     ssz_static_test!(checkpoint, Checkpoint);