Several changes

* Fix state cache pruning of finalized state from block map
* Update to latest `milhouse`
* Check beacon state diffs in EF tests

Cargo.lock

@@ -638,6 +638,9 @@ name = "cc"
 version = "1.0.72"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "22a9137b95ea06864e018375b72adfb7db6e6f68cfc8df5a04d00288050485ee"
+dependencies = [
+ "jobserver",
+]
 
 [[package]]
 name = "cexpr"
@@ -1339,6 +1342,7 @@ dependencies = [
  "fork_choice",
  "fs2",
  "hex",
+ "logging",
  "malloc_utils",
  "rayon",
  "serde",
@@ -2753,6 +2757,15 @@ dependencies = [
  "libc",
 ]
 
+[[package]]
+name = "jobserver"
+version = "0.1.24"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "af25a77299a7f711a01975c35a6a424eb6862092cc2d6c72c4ed6cbc56dfc1fa"
+dependencies = [
+ "libc",
+]
+
 [[package]]
 name = "js-sys"
 version = "0.3.56"
@@ -5817,12 +5830,10 @@ name = "store"
 version = "0.2.0"
 dependencies = [
  "beacon_chain",
- "bincode",
  "db-key",
  "directory",
  "eth2_ssz",
  "eth2_ssz_derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "flate2",
  "itertools",
  "lazy_static",
  "leveldb",
@@ -5839,6 +5850,7 @@ dependencies = [
  "tempfile",
  "tree_hash",
  "types",
+ "zstd",
 ]
 
 [[package]]
@@ -7286,3 +7298,32 @@ dependencies = [
  "thiserror",
  "time 0.1.43",
 ]
+
+[[package]]
+name = "zstd"
+version = "0.10.0+zstd.1.5.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3b1365becbe415f3f0fcd024e2f7b45bacfb5bdd055f0dc113571394114e7bdd"
+dependencies = [
+ "zstd-safe",
+]
+
+[[package]]
+name = "zstd-safe"
+version = "4.1.4+zstd.1.5.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2f7cd17c9af1a4d6c24beb1cc54b17e2ef7b593dc92f19e9d9acad8b182bbaee"
+dependencies = [
+ "libc",
+ "zstd-sys",
+]
+
+[[package]]
+name = "zstd-sys"
+version = "1.6.3+zstd.1.5.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fc49afa5c8d634e75761feda8c592051e7eeb4683ba827211eb0d731d3402ea8"
+dependencies = [
+ "cc",
+ "libc",
+]

beacon_node/beacon_chain/src/block_verification.rs

@@ -1077,10 +1077,9 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
                 // Store the state immediately, marking it as temporary, and staging the deletion
                 // of its temporary status as part of the larger atomic operation.
                 let txn_lock = chain.store.hot_db.begin_rw_transaction();
-                chain.store.do_atomically(vec![
-                    StoreOp::PutState(state_root, &state),
-                    StoreOp::PutStateTemporaryFlag(state_root),
-                ])?;
+                chain
+                    .store
+                    .do_atomically(vec![StoreOp::PutState(state_root, &state)])?;
                 drop(txn_lock);
 
                 confirmation_db_batch.push(StoreOp::DeleteStateTemporaryFlag(state_root));

beacon_node/beacon_chain/tests/store_tests.rs

@@ -25,7 +25,6 @@ use store::{
     HotColdDB, LevelDB, StoreConfig,
 };
 use tempfile::{tempdir, TempDir};
-use tree_hash::TreeHash;
 use types::test_utils::{SeedableRng, XorShiftRng};
 use types::*;
 

beacon_node/store/Cargo.toml

@@ -28,8 +28,7 @@ sloggers = { version = "2.1.1", features = ["json"] }
 directory = { path = "../../common/directory" }
 tree_hash = "0.4.0"
 take-until = "0.1.0"
-flate2 = { version = "1.0.22", features = ["zlib"], default-features = false }
-bincode = "1.3.3"
+zstd = "0.10.0"
 
 [features]
 milhouse = ["state_processing/milhouse"]

beacon_node/store/src/chunked_vector.rs

@@ -543,7 +543,7 @@ pub fn load_variable_list_from_db<F: VariableLengthField<E>, E: EthSpec, S: KeyV
 
     let chunks: Vec<Chunk<F::Value>> = range_query(store, F::column(), start_cindex, end_cindex)?;
 
-    let mut result = VList::empty()?;
+    let mut result = VList::empty();
 
     for (chunk_index, chunk) in chunks.into_iter().enumerate() {
         for (i, value) in chunk.values.into_iter().enumerate() {

beacon_node/store/src/errors.rs

@@ -47,8 +47,7 @@ pub enum Error {
     BlockReplayError(BlockReplayError),
     #[cfg(feature = "milhouse")]
     MilhouseError(milhouse::Error),
-    Bincode(Box<bincode::ErrorKind>),
-    FlateCompression(std::io::Error),
+    Compression(std::io::Error),
 }
 
 pub trait HandleUnavailable<T> {
@@ -114,12 +113,6 @@ impl From<BlockReplayError> for Error {
     }
 }
 
-impl From<Box<bincode::ErrorKind>> for Error {
-    fn from(e: Box<bincode::ErrorKind>) -> Self {
-        Self::Bincode(e)
-    }
-}
-
 #[derive(Debug)]
 pub struct DBError {
     pub message: String,

beacon_node/store/src/state_cache.rs

@@ -95,8 +95,7 @@ impl<E: EthSpec> StateCache<E> {
                 finalized_state.state_root == state_root
             })
         {
-            // FIXME(sproul): this should technically be true
-            return Ok(false);
+            return Ok(true);
         }
         if self.states.peek(&state_root).is_some() {
             return Ok(true);
@@ -167,7 +166,7 @@ impl BlockMap {
 
         self.blocks.retain(|_, slot_map| {
             slot_map.slots.retain(|slot, state_root| {
-                let keep = *slot > finalized_slot;
+                let keep = *slot >= finalized_slot;
                 if !keep {
                     pruned_states.insert(*state_root);
                 }

beacon_node/store/src/state_diff.rs

@@ -1,8 +1,18 @@
 use crate::{metrics, DBColumn, Error, StoreItem};
-use flate2::bufread::{ZlibDecoder, ZlibEncoder};
 use ssz::{Decode, Encode};
-use std::io::Read;
+use std::io::{Read, Write};
 use types::{beacon_state::BeaconStateDiff, EthSpec};
+use zstd::{Decoder, Encoder};
+
+const EST_COMPRESSION_FACTOR: usize = 2;
+
+fn estimate_compressed_size(len: usize, compression_level: i32) -> usize {
+    if compression_level == 0 {
+        len
+    } else {
+        len / EST_COMPRESSION_FACTOR
+    }
+}
 
 impl<E: EthSpec> StoreItem for BeaconStateDiff<E> {
     fn db_column() -> DBColumn {
@@ -14,13 +24,13 @@ impl<E: EthSpec> StoreItem for BeaconStateDiff<E> {
         let value = self.as_ssz_bytes();
         drop(encode_timer);
 
-        // FIXME(sproul): try vec with capacity
         let compression_timer = metrics::start_timer(&metrics::BEACON_STATE_DIFF_COMPRESSION_TIME);
-        let mut encoder = ZlibEncoder::new(&value[..], flate2::Compression::fast());
-        let mut compressed_value = vec![];
-        encoder
-            .read_to_end(&mut compressed_value)
-            .map_err(Error::FlateCompression)?;
+
+        let level = 1;
+        let mut compressed_value = Vec::with_capacity(estimate_compressed_size(value.len(), level));
+        let mut encoder = Encoder::new(&mut compressed_value, level).map_err(Error::Compression)?;
+        encoder.write_all(&value).map_err(Error::Compression)?;
+        encoder.finish().map_err(Error::Compression)?;
         drop(compression_timer);
 
         let compression_ratio = value.len() as f64 / compressed_value.len() as f64;
@@ -39,11 +49,11 @@ impl<E: EthSpec> StoreItem for BeaconStateDiff<E> {
     }
 
     fn from_store_bytes(bytes: &[u8]) -> Result<Self, Error> {
-        let mut ssz_bytes = vec![];
-        let mut decoder = ZlibDecoder::new(bytes);
+        let mut ssz_bytes = Vec::with_capacity(EST_COMPRESSION_FACTOR * bytes.len());
+        let mut decoder = Decoder::new(bytes).map_err(Error::Compression)?;
         decoder
             .read_to_end(&mut ssz_bytes)
-            .map_err(Error::FlateCompression)?;
+            .map_err(Error::Compression)?;
         Ok(Self::from_ssz_bytes(&ssz_bytes)?)
     }
 }

consensus/state_processing/src/genesis.rs

@@ -26,7 +26,7 @@ pub fn initialize_beacon_state_from_eth1<T: EthSpec>(
     let mut state = BeaconState::new(genesis_time, eth1_data, spec);
 
     // Seed RANDAO with Eth1 entropy
-    state.fill_randao_mixes_with(eth1_block_hash);
+    state.fill_randao_mixes_with(eth1_block_hash)?;
 
     let mut deposit_tree = DepositDataTree::create(&[], 0, DEPOSIT_TREE_DEPTH);
 

consensus/state_processing/src/per_epoch_processing/resets.rs

@@ -13,7 +13,7 @@ pub fn process_eth1_data_reset<T: EthSpec>(
         .safe_rem(T::SlotsPerEth1VotingPeriod::to_u64())?
         == 0
     {
-        *state.eth1_data_votes_mut() = VList::empty()?;
+        *state.eth1_data_votes_mut() = VList::empty();
     }
     Ok(())
 }

consensus/types/src/beacon_state.rs

@@ -363,8 +363,8 @@ impl<T: EthSpec> BeaconState<T> {
 
             // History
             latest_block_header: BeaconBlock::<T>::empty(spec).temporary_block_header(),
-            block_roots: FixedVector::from_elem(Hash256::zero()),
-            state_roots: FixedVector::from_elem(Hash256::zero()),
+            block_roots: FixedVector::default(),
+            state_roots: FixedVector::default(),
             historical_roots: VList::default(),
 
             // Eth1
@@ -377,10 +377,10 @@ impl<T: EthSpec> BeaconState<T> {
             balances: VList::default(),   // Set later.
 
             // Randomness
-            randao_mixes: FixedVector::from_elem(Hash256::zero()),
+            randao_mixes: FixedVector::default(),
 
             // Slashings
-            slashings: FixedVector::from_elem(0),
+            slashings: FixedVector::default(),
 
             // Attestations
             previous_epoch_attestations: VList::default(),
@@ -970,8 +970,9 @@ impl<T: EthSpec> BeaconState<T> {
     }
 
     /// Fill `randao_mixes` with
-    pub fn fill_randao_mixes_with(&mut self, index_root: Hash256) {
-        *self.randao_mixes_mut() = FixedVector::from_elem(index_root);
+    pub fn fill_randao_mixes_with(&mut self, index_root: Hash256) -> Result<(), Error> {
+        *self.randao_mixes_mut() = FixedVector::from_elem(index_root)?;
+        Ok(())
     }
 
     /// Safely obtains the index for `randao_mixes`

testing/ef_tests/Cargo.toml

@@ -36,3 +36,4 @@ beacon_chain = { path = "../../beacon_node/beacon_chain" }
 store = { path = "../../beacon_node/store" }
 fork_choice = { path = "../../consensus/fork_choice" }
 malloc_utils = { path = "../../common/malloc_utils" }
+logging = { path = "../../common/logging" }

testing/ef_tests/src/case_result.rs

@@ -2,7 +2,7 @@ use super::*;
 use compare_fields::{CompareFields, Comparison, FieldComparison};
 use std::fmt::Debug;
 use std::path::{Path, PathBuf};
-use types::BeaconState;
+use types::{beacon_state::BeaconStateDiff, milhouse::diff::Diff, BeaconState};
 
 pub const MAX_VALUE_STRING_LEN: usize = 500;
 
@@ -118,6 +118,23 @@ where
     }
 }
 
+pub fn check_state_diff<T: EthSpec>(
+    pre_state: &BeaconState<T>,
+    opt_post_state: &Option<BeaconState<T>>,
+) -> Result<(), Error> {
+    if let Some(post_state) = opt_post_state {
+        let diff = BeaconStateDiff::compute_diff(pre_state, post_state)
+            .expect("BeaconStateDiff should compute");
+        let mut diffed_state = pre_state.clone();
+        diff.apply_diff(&mut diffed_state)
+            .expect("BeaconStateDiff should apply");
+
+        compare_result_detailed::<_, ()>(&Ok(diffed_state), opt_post_state)
+    } else {
+        Ok(())
+    }
+}
+
 fn fmt_val<T: Debug>(val: T) -> String {
     let mut string = format!("{:?}", val);
     string.truncate(MAX_VALUE_STRING_LEN);

testing/ef_tests/src/cases/epoch_processing.rs

@@ -1,6 +1,6 @@
 use super::*;
 use crate::bls_setting::BlsSetting;
-use crate::case_result::compare_beacon_state_results_without_caches;
+use crate::case_result::{check_state_diff, compare_beacon_state_results_without_caches};
 use crate::decode::{ssz_decode_state, yaml_decode_file};
 use crate::type_name;
 use crate::type_name::TypeName;
@@ -274,7 +274,7 @@ impl<E: EthSpec, T: EpochTransition<E>> Case for EpochProcessing<E, T> {
                     && T::name() != "inactivity_updates"
                     && T::name() != "participation_flag_updates"
             }
-            ForkName::Altair | ForkName::Merge => true, // TODO: revisit when tests are out
+            ForkName::Altair | ForkName::Merge => true,
         }
     }
 
@@ -293,6 +293,7 @@ impl<E: EthSpec, T: EpochTransition<E>> Case for EpochProcessing<E, T> {
             T::run(&mut state, spec).map(|_| state)
         })();
 
-        compare_beacon_state_results_without_caches(&mut result, &mut expected)
+        compare_beacon_state_results_without_caches(&mut result, &mut expected)?;
+        check_state_diff(&self.pre, &self.post)
     }
 }

testing/ef_tests/src/cases/operations.rs

@@ -1,6 +1,6 @@
 use super::*;
 use crate::bls_setting::BlsSetting;
-use crate::case_result::compare_beacon_state_results_without_caches;
+use crate::case_result::{check_state_diff, compare_beacon_state_results_without_caches};
 use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file};
 use crate::testing_spec;
 use crate::type_name::TypeName;
@@ -335,6 +335,7 @@ impl<E: EthSpec, O: Operation<E>> Case for Operations<E, O> {
             .apply_to(&mut state, spec, self)
             .map(|()| state);
 
-        compare_beacon_state_results_without_caches(&mut result, &mut expected)
+        compare_beacon_state_results_without_caches(&mut result, &mut expected)?;
+        check_state_diff(&self.pre, &self.post)
     }
 }

testing/ef_tests/src/cases/sanity_blocks.rs

@@ -1,6 +1,6 @@
 use super::*;
 use crate::bls_setting::BlsSetting;
-use crate::case_result::compare_beacon_state_results_without_caches;
+use crate::case_result::{check_state_diff, compare_beacon_state_results_without_caches};
 use crate::decode::{ssz_decode_file_with, ssz_decode_state, yaml_decode_file};
 use serde_derive::Deserialize;
 use state_processing::{
@@ -129,6 +129,9 @@ impl<E: EthSpec> Case for SanityBlocks<E> {
             Ok(res) => (Ok(res.0), Ok(res.1)),
         };
         compare_beacon_state_results_without_caches(&mut indiv_result, &mut expected)?;
-        compare_beacon_state_results_without_caches(&mut bulk_result, &mut expected)
+        compare_beacon_state_results_without_caches(&mut bulk_result, &mut expected)?;
+        check_state_diff(&self.pre, &self.post)?;
+
+        Ok(())
     }
 }

testing/ef_tests/src/cases/sanity_slots.rs

@@ -1,6 +1,6 @@
 use super::*;
 use crate::bls_setting::BlsSetting;
-use crate::case_result::compare_beacon_state_results_without_caches;
+use crate::case_result::{check_state_diff, compare_beacon_state_results_without_caches};
 use crate::decode::{ssz_decode_state, yaml_decode_file};
 use serde_derive::Deserialize;
 use state_processing::per_slot_processing;
@@ -70,6 +70,7 @@ impl<E: EthSpec> Case for SanitySlots<E> {
             .try_for_each(|_| per_slot_processing(&mut state, None, spec).map(|_| ()))
             .map(|_| state);
 
-        compare_beacon_state_results_without_caches(&mut result, &mut expected)
+        compare_beacon_state_results_without_caches(&mut result, &mut expected)?;
+        check_state_diff(&self.pre, &self.post)
     }
 }