From 21bcc8848d3009cc8116c356993c91f1c0e454ba Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 24 Jul 2020 02:19:47 +0000 Subject: [PATCH] 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 | 2 +- consensus/types/src/beacon_state.rs | 25 ++-- consensus/types/src/beacon_state/tests.rs | 42 +++++++ .../types/src/beacon_state/tree_hash_cache.rs | 114 ++++++++++++++++-- testing/ef_tests/tests/tests.rs | 4 +- 5 files changed, 155 insertions(+), 32 deletions(-) diff --git a/consensus/tree_hash/benches/benches.rs b/consensus/tree_hash/benches/benches.rs index 0bde5f78d..b2cf25172 100644 --- a/consensus/tree_hash/benches/benches.rs +++ b/consensus/tree_hash/benches/benches.rs @@ -34,7 +34,7 @@ fn bench_suite(c: &mut Criterion, spec_desc: &str, validator_count: let state1 = build_state::(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), diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index eca1543d4..cb3dbbded 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -70,6 +70,11 @@ pub enum Error { CommitteeCacheUninitialized(Option), 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, + pub tree_hash_cache: Option>, } impl BeaconState { @@ -879,7 +884,6 @@ impl BeaconState { 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 BeaconState { } } - /// 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 BeaconState { /// 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 CachedTreeHash for BeaconState { - fn new_tree_hash_cache(&self, _arena: &mut CacheArena) -> BeaconTreeHashCache { +impl CachedTreeHash> for BeaconState { + fn new_tree_hash_cache(&self, _arena: &mut CacheArena) -> BeaconTreeHashCache { BeaconTreeHashCache::new(self) } fn recalculate_tree_hash_root( &self, _arena: &mut CacheArena, - cache: &mut BeaconTreeHashCache, + cache: &mut BeaconTreeHashCache, ) -> Result { cache .recalculate_tree_hash_root(self) diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index 79a30b68a..9c2df38d4 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/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()[..]); } diff --git a/consensus/types/src/beacon_state/tree_hash_cache.rs b/consensus/types/src/beacon_state/tree_hash_cache.rs index 3e1fa29d0..0c8899c02 100644 --- a/consensus/types/src/beacon_state/tree_hash_cache.rs +++ b/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 { + arena: CacheArena, + tree_hash_cache: TreeHashCache, + voting_period: u64, + roots: VariableList, +} + +impl Eth1DataVotesTreeHashCache { + /// 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) -> Self { + let mut arena = CacheArena::default(); + let roots: VariableList<_, _> = state + .eth1_data_votes + .iter() + .map(|eth1_data| eth1_data.tree_hash_root()) + .collect::>() + .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) -> Result { + 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)] -pub struct BeaconTreeHashCache { +#[derive(Debug, PartialEq, Clone, Encode, Decode)] +pub struct BeaconTreeHashCache { + /// 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, } -impl BeaconTreeHashCache { +impl BeaconTreeHashCache { /// Instantiates a new cache. /// - /// Allocates the necessary memory to store all of the cached Merkle trees but does perform any - /// hashing. - pub fn new(state: &BeaconState) -> Self { + /// 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) -> 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( - &mut self, - state: &BeaconState, - ) -> Result { + pub fn recalculate_tree_hash_root(&mut self, state: &BeaconState) -> Result { + // If this cache has previously produced a root, ensure that it is in the state root + // history of this state. + // + // 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`. diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index d20ac5ec7..99bbe2a86 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -180,8 +180,8 @@ mod ssz_static { ssz_static_test!( beacon_state, SszStaticTHCHandler, { - (BeaconState, BeaconTreeHashCache, MinimalEthSpec), - (BeaconState, BeaconTreeHashCache, MainnetEthSpec) + (BeaconState, BeaconTreeHashCache<_>, MinimalEthSpec), + (BeaconState, BeaconTreeHashCache<_>, MainnetEthSpec) } ); ssz_static_test!(checkpoint, Checkpoint);