Merge branch 'iter-fixes' into attestation-processing

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1,6 +1,7 @@
 use crate::checkpoint::CheckPoint;
 use crate::errors::{BeaconChainError as Error, BlockProductionError};
 use crate::fork_choice::{Error as ForkChoiceError, ForkChoice};
+use crate::iter::{ReverseBlockRootIterator, ReverseStateRootIterator};
 use crate::metrics::Metrics;
 use crate::persisted_beacon_chain::{PersistedBeaconChain, BEACON_CHAIN_DB_KEY};
 use crate::BeaconChainError;
@@ -20,7 +21,7 @@ use state_processing::{
     per_slot_processing, BlockProcessingError,
 };
 use std::sync::Arc;
-use store::iter::{BestBlockRootsIterator, BlockIterator, BlockRootsIterator, StateRootsIterator};
+use store::iter::{BlockRootsIterator, StateRootsIterator};
 use store::{Error as DBError, Store};
 use tree_hash::TreeHash;
 use types::*;
@@ -231,45 +232,53 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         Ok(headers?)
     }
-    /// Iterate in reverse (highest to lowest slot) through all blocks from the block at `slot`
-    /// through to the genesis block.
-    ///
-    /// Returns `None` for headers prior to genesis or when there is an error reading from `Store`.
-    ///
-    /// Contains duplicate headers when skip slots are encountered.
-    pub fn rev_iter_blocks(&self, slot: Slot) -> BlockIterator<T::EthSpec, T::Store> {
-        BlockIterator::owned(self.store.clone(), self.state.read().clone(), slot)
-    }
 
-    /// Iterates in reverse (highest to lowest slot) through all block roots from `slot` through to
-    /// genesis.
+    /// Iterates through all the `BeaconBlock` roots and slots, first returning
+    /// `self.head().beacon_block` then all prior blocks until either genesis or if the database
+    /// fails to return a prior block.
     ///
-    /// Returns `None` for roots prior to genesis or when there is an error reading from `Store`.
+    /// Returns duplicate roots for skip-slots.
     ///
-    /// Contains duplicate roots when skip slots are encountered.
-    pub fn rev_iter_block_roots(&self, slot: Slot) -> BlockRootsIterator<T::EthSpec, T::Store> {
-        BlockRootsIterator::owned(self.store.clone(), self.state.read().clone(), slot)
-    }
-
-    /// Iterates in reverse (highest to lowest slot) through all block roots from largest
-    /// `slot <= beacon_state.slot` through to genesis.
+    /// Iterator returns `(Hash256, Slot)`.
     ///
-    /// Returns `None` for roots prior to genesis or when there is an error reading from `Store`.
+    /// ## Note
     ///
-    /// Contains duplicate roots when skip slots are encountered.
-    pub fn rev_iter_best_block_roots(
+    /// Because this iterator starts at the `head` of the chain (viz., the best block), the first slot
+    /// returned may be earlier than the wall-clock slot.
+    pub fn rev_iter_block_roots(
         &self,
         slot: Slot,
-    ) -> BestBlockRootsIterator<T::EthSpec, T::Store> {
-        BestBlockRootsIterator::owned(self.store.clone(), self.state.read().clone(), slot)
+    ) -> ReverseBlockRootIterator<T::EthSpec, T::Store> {
+        let state = &self.head().beacon_state;
+        let block_root = self.head().beacon_block_root;
+        let block_slot = state.slot;
+
+        let iter = BlockRootsIterator::owned(self.store.clone(), state.clone(), slot);
+
+        ReverseBlockRootIterator::new((block_root, block_slot), iter)
     }
 
-    /// Iterates in reverse (highest to lowest slot) through all state roots from `slot` through to
-    /// genesis.
+    /// Iterates through all the `BeaconState` roots and slots, first returning
+    /// `self.head().beacon_state` then all prior states until either genesis or if the database
+    /// fails to return a prior state.
     ///
-    /// Returns `None` for roots prior to genesis or when there is an error reading from `Store`.
-    pub fn rev_iter_state_roots(&self, slot: Slot) -> StateRootsIterator<T::EthSpec, T::Store> {
-        StateRootsIterator::owned(self.store.clone(), self.state.read().clone(), slot)
+    /// Iterator returns `(Hash256, Slot)`.
+    ///
+    /// ## Note
+    ///
+    /// Because this iterator starts at the `head` of the chain (viz., the best block), the first slot
+    /// returned may be earlier than the wall-clock slot.
+    pub fn rev_iter_state_roots(
+        &self,
+        slot: Slot,
+    ) -> ReverseStateRootIterator<T::EthSpec, T::Store> {
+        let state = &self.head().beacon_state;
+        let state_root = self.head().beacon_state_root;
+        let state_slot = state.slot;
+
+        let iter = StateRootsIterator::owned(self.store.clone(), state.clone(), slot);
+
+        ReverseStateRootIterator::new((state_root, state_slot), iter)
     }
 
     /// Returns the block at the given root, if any.
@@ -286,8 +295,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
     /// Returns a read-lock guarded `BeaconState` which is the `canonical_head` that has been
     /// updated to match the current slot clock.
-    pub fn current_state(&self) -> RwLockReadGuard<BeaconState<T::EthSpec>> {
-        self.state.read()
+    pub fn speculative_state(&self) -> Result<RwLockReadGuard<BeaconState<T::EthSpec>>, Error> {
+        // TODO: ensure the state has done a catch-up.
+
+        Ok(self.state.read())
     }
 
     /// Returns a read-lock guarded `CheckPoint` struct for reading the head (as chosen by the

beacon_node/beacon_chain/src/fork_choice.rs

@@ -54,7 +54,7 @@ impl<T: BeaconChainTypes> ForkChoice<T> {
         // been justified for at least 1 epoch ... If no such descendant exists,
         // set justified_head to finalized_head.
         let (start_state, start_block_root, start_block_slot) = {
-            let state = chain.current_state();
+            let state = &chain.head().beacon_state;
 
             let (block_root, block_slot) =
                 if state.current_epoch() + 1 > state.current_justified_checkpoint.epoch {

beacon_node/beacon_chain/src/iter.rs

@@ -0,0 +1,48 @@
+use store::iter::{BlockRootsIterator, StateRootsIterator};
+use types::{Hash256, Slot};
+
+pub type ReverseBlockRootIterator<'a, E, S> =
+    ReverseHashAndSlotIterator<BlockRootsIterator<'a, E, S>>;
+pub type ReverseStateRootIterator<'a, E, S> =
+    ReverseHashAndSlotIterator<StateRootsIterator<'a, E, S>>;
+
+pub type ReverseHashAndSlotIterator<I> = ReverseChainIterator<(Hash256, Slot), I>;
+
+/// Provides a wrapper for an iterator that returns a given `T` before it starts returning results of
+/// the `Iterator`.
+pub struct ReverseChainIterator<T, I> {
+    first_value_used: bool,
+    first_value: T,
+    iter: I,
+}
+
+impl<T, I> ReverseChainIterator<T, I>
+where
+    T: Sized,
+    I: Iterator<Item = T> + Sized,
+{
+    pub fn new(first_value: T, iter: I) -> Self {
+        Self {
+            first_value_used: false,
+            first_value,
+            iter,
+        }
+    }
+}
+
+impl<T, I> Iterator for ReverseChainIterator<T, I>
+where
+    T: Clone,
+    I: Iterator<Item = T>,
+{
+    type Item = T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.first_value_used {
+            self.iter.next()
+        } else {
+            self.first_value_used = true;
+            Some(self.first_value.clone())
+        }
+    }
+}

beacon_node/beacon_chain/src/lib.rs

@@ -2,6 +2,7 @@ mod beacon_chain;
 mod checkpoint;
 mod errors;
 mod fork_choice;
+mod iter;
 mod metrics;
 mod persisted_beacon_chain;
 pub mod test_utils;

beacon_node/beacon_chain/src/test_utils.rs

@@ -193,7 +193,7 @@ where
     fn get_state_at_slot(&self, state_slot: Slot) -> BeaconState<E> {
         let state_root = self
             .chain
-            .rev_iter_state_roots(self.chain.current_state().slot - 1)
+            .rev_iter_state_roots(self.chain.head().beacon_state.slot - 1)
             .find(|(_hash, slot)| *slot == state_slot)
             .map(|(hash, _slot)| hash)
             .expect("could not find state root");