spec: simplify cache_state

The `latest_block_root` input argument was unnecessary as we were always setting it to something
almost equivalent to `state.latest_block_root` anyway, and more importantly, it was messing up the
caching of the state root. Previously it was possible for the function to update the state's latest
block root, and then hash the outdated block root that was passed in as an argument.

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -303,8 +303,6 @@ where
     /// then having it iteratively updated -- in such a case it's possible for another thread to
     /// find the state at an old slot.
     pub fn update_state(&self, mut state: BeaconState) -> Result<(), Error> {
-        let latest_block_header = self.head().beacon_block.block_header();
-
         let present_slot = match self.slot_clock.present_slot() {
             Ok(Some(slot)) => slot,
             _ => return Err(Error::UnableToReadSlot),
@@ -312,7 +310,7 @@ where
 
         // If required, transition the new state to the present slot.
         for _ in state.slot.as_u64()..present_slot.as_u64() {
-            per_slot_processing(&mut state, &latest_block_header, &self.spec)?;
+            per_slot_processing(&mut state, &self.spec)?;
         }
 
         state.build_all_caches(&self.spec)?;
@@ -324,8 +322,6 @@ where
 
     /// Ensures the current canonical `BeaconState` has been transitioned to match the `slot_clock`.
     pub fn catchup_state(&self) -> Result<(), Error> {
-        let latest_block_header = self.head().beacon_block.block_header();
-
         let present_slot = match self.slot_clock.present_slot() {
             Ok(Some(slot)) => slot,
             _ => return Err(Error::UnableToReadSlot),
@@ -339,7 +335,7 @@ where
             state.build_epoch_cache(RelativeEpoch::NextWithoutRegistryChange, &self.spec)?;
             state.build_epoch_cache(RelativeEpoch::NextWithRegistryChange, &self.spec)?;
 
-            per_slot_processing(&mut *state, &latest_block_header, &self.spec)?;
+            per_slot_processing(&mut *state, &self.spec)?;
         }
 
         state.build_all_caches(&self.spec)?;
@@ -617,9 +613,8 @@ where
 
         // Transition the parent state to the block slot.
         let mut state = parent_state;
-        let previous_block_header = parent_block.block_header();
         for _ in state.slot.as_u64()..block.slot.as_u64() {
-            if let Err(e) = per_slot_processing(&mut state, &previous_block_header, &self.spec) {
+            if let Err(e) = per_slot_processing(&mut state, &self.spec) {
                 return Ok(BlockProcessingOutcome::InvalidBlock(
                     InvalidBlock::SlotProcessingError(e),
                 ));

eth2/state_processing/src/per_slot_processing.rs

@@ -11,12 +11,8 @@ pub enum Error {
 /// Advances a state forward by one slot, performing per-epoch processing if required.
 ///
 /// Spec v0.5.0
-pub fn per_slot_processing(
-    state: &mut BeaconState,
-    latest_block_header: &BeaconBlockHeader,
-    spec: &ChainSpec,
-) -> Result<(), Error> {
-    cache_state(state, latest_block_header, spec)?;
+pub fn per_slot_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> {
+    cache_state(state, spec)?;
 
     if (state.slot + 1) % spec.slots_per_epoch == 0 {
         per_epoch_processing(state, spec)?;
@@ -27,11 +23,7 @@ pub fn per_slot_processing(
     Ok(())
 }
 
-fn cache_state(
-    state: &mut BeaconState,
-    latest_block_header: &BeaconBlockHeader,
-    spec: &ChainSpec,
-) -> Result<(), Error> {
+fn cache_state(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> {
     let previous_slot_state_root = Hash256::from_slice(&state.tree_hash_root()[..]);
 
     // Note: increment the state slot here to allow use of our `state_root` and `block_root`
@@ -46,7 +38,7 @@ fn cache_state(
         state.latest_block_header.state_root = previous_slot_state_root
     }
 
-    let latest_block_root = Hash256::from_slice(&latest_block_header.tree_hash_root()[..]);
+    let latest_block_root = Hash256::from_slice(&state.latest_block_header.tree_hash_root()[..]);
     state.set_block_root(previous_slot, latest_block_root, spec)?;
 
     // Set the state slot back to what it should be.

eth2/state_processing/tests/tests.rs

@@ -105,8 +105,7 @@ fn run_state_transition_test(test_name: &str) {
         let mut state = test_case.initial_state.clone();
         for (j, block) in test_case.blocks.iter().enumerate() {
             while block.slot > state.slot {
-                let latest_block_header = state.latest_block_header.clone();
-                per_slot_processing(&mut state, &latest_block_header, &test_case.config).unwrap();
+                per_slot_processing(&mut state, &test_case.config).unwrap();
             }
             let res = per_block_processing(&mut state, &block, &test_case.config);
             if res.is_err() {