Forwards block root iterators (#672)

* Implement forwards block root iterators

* Clean up errors and docs

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -94,7 +94,7 @@ pub enum AttestationProcessingOutcome {
 }
 
 pub trait BeaconChainTypes: Send + Sync + 'static {
-    type Store: store::Store;
+    type Store: store::Store<Self::EthSpec>;
     type StoreMigrator: store::Migrate<Self::Store, Self::EthSpec>;
     type SlotClock: slot_clock::SlotClock;
     type LmdGhost: LmdGhost<Self::Store, Self::EthSpec>;

beacon_node/beacon_chain/src/builder.rs

@@ -52,7 +52,7 @@ impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec, TEve
         TEventHandler,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
     TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
@@ -108,7 +108,7 @@ impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec, TEve
         >,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
     TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
@@ -402,7 +402,7 @@ impl<TStore, TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler>
         >,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec> + 'static,
@@ -462,7 +462,7 @@ impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEthSpec, TEventHandler>
         >,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
     TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
@@ -514,7 +514,7 @@ impl<TStore, TStoreMigrator, TLmdGhost, TEth1Backend, TEthSpec, TEventHandler>
         >,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,
     TEth1Backend: Eth1ChainBackend<TEthSpec> + 'static,
@@ -555,7 +555,7 @@ impl<TStore, TStoreMigrator, TSlotClock, TLmdGhost, TEth1Backend, TEthSpec>
         >,
     >
 where
-    TStore: Store + 'static,
+    TStore: Store<TEthSpec> + 'static,
     TStoreMigrator: store::Migrate<TStore, TEthSpec> + 'static,
     TSlotClock: SlotClock + 'static,
     TLmdGhost: LmdGhost<TStore, TEthSpec> + 'static,

beacon_node/beacon_chain/src/eth1_chain.rs

@@ -184,7 +184,7 @@ pub struct CachingEth1Backend<T: EthSpec, S> {
     _phantom: PhantomData<T>,
 }
 
-impl<T: EthSpec, S: Store> CachingEth1Backend<T, S> {
+impl<T: EthSpec, S: Store<T>> CachingEth1Backend<T, S> {
     /// Instantiates `self` with empty caches.
     ///
     /// Does not connect to the eth1 node or start any tasks to keep the cache updated.
@@ -213,7 +213,7 @@ impl<T: EthSpec, S: Store> CachingEth1Backend<T, S> {
     }
 }
 
-impl<T: EthSpec, S: Store> Eth1ChainBackend<T> for CachingEth1Backend<T, S> {
+impl<T: EthSpec, S: Store<T>> Eth1ChainBackend<T> for CachingEth1Backend<T, S> {
     fn eth1_data(&self, state: &BeaconState<T>, spec: &ChainSpec) -> Result<Eth1Data, Error> {
         // Note: we do not return random junk if this function call fails as it would be caused by
         // an internal error.
@@ -372,7 +372,7 @@ fn random_eth1_data() -> Eth1Data {
 
 /// Returns `state.eth1_data.block_hash` at the start of eth1 voting period defined by
 /// `state.slot`.
-fn eth1_block_hash_at_start_of_voting_period<T: EthSpec, S: Store>(
+fn eth1_block_hash_at_start_of_voting_period<T: EthSpec, S: Store<T>>(
     store: Arc<S>,
     state: &BeaconState<T>,
 ) -> Result<Hash256, Error> {
@@ -392,7 +392,7 @@ fn eth1_block_hash_at_start_of_voting_period<T: EthSpec, S: Store>(
             .map_err(Error::UnableToGetPreviousStateRoot)?;
 
         store
-            .get_state::<T>(&prev_state_root, Some(slot))
+            .get_state(&prev_state_root, Some(slot))
             .map_err(Error::StoreError)?
             .map(|state| state.eth1_data.block_hash)
             .ok_or_else(|| Error::PreviousStateNotInDB(*prev_state_root))
@@ -581,7 +581,7 @@ mod test {
         use store::MemoryStore;
         use types::test_utils::{generate_deterministic_keypair, TestingDepositBuilder};
 
-        fn get_eth1_chain() -> Eth1Chain<CachingEth1Backend<E, MemoryStore>, E> {
+        fn get_eth1_chain() -> Eth1Chain<CachingEth1Backend<E, MemoryStore<E>>, E> {
             let eth1_config = Eth1Config {
                 ..Eth1Config::default()
             };

beacon_node/beacon_chain/src/test_utils.rs

@@ -40,8 +40,8 @@ pub type BaseHarnessType<TStore, TStoreMigrator, TEthSpec> = Witness<
     NullEventHandler<TEthSpec>,
 >;
 
-pub type HarnessType<E> = BaseHarnessType<MemoryStore, NullMigrator, E>;
-pub type DiskHarnessType<E> = BaseHarnessType<DiskStore, BlockingMigrator<DiskStore>, E>;
+pub type HarnessType<E> = BaseHarnessType<MemoryStore<E>, NullMigrator, E>;
+pub type DiskHarnessType<E> = BaseHarnessType<DiskStore<E>, BlockingMigrator<DiskStore<E>>, E>;
 
 /// Indicates how the `BeaconChainHarness` should produce blocks.
 #[derive(Clone, Copy, Debug)]
@@ -120,7 +120,7 @@ impl<E: EthSpec> BeaconChainHarness<DiskHarnessType<E>> {
     /// Instantiate a new harness with `validator_count` initial validators.
     pub fn new_with_disk_store(
         eth_spec_instance: E,
-        store: Arc<DiskStore>,
+        store: Arc<DiskStore<E>>,
         keypairs: Vec<Keypair>,
     ) -> Self {
         let spec = E::default_spec();
@@ -160,7 +160,7 @@ impl<E: EthSpec> BeaconChainHarness<DiskHarnessType<E>> {
     /// Instantiate a new harness with `validator_count` initial validators.
     pub fn resume_from_disk_store(
         eth_spec_instance: E,
-        store: Arc<DiskStore>,
+        store: Arc<DiskStore<E>>,
         keypairs: Vec<Keypair>,
     ) -> Self {
         let spec = E::default_spec();
@@ -197,7 +197,7 @@ impl<E: EthSpec> BeaconChainHarness<DiskHarnessType<E>> {
 
 impl<S, M, E> BeaconChainHarness<BaseHarnessType<S, M, E>>
 where
-    S: Store,
+    S: Store<E>,
     M: Migrate<S, E>,
     E: EthSpec,
 {

beacon_node/beacon_chain/tests/persistence_tests.rs

@@ -23,14 +23,14 @@ lazy_static! {
     static ref KEYPAIRS: Vec<Keypair> = types::test_utils::generate_deterministic_keypairs(VALIDATOR_COUNT);
 }
 
-fn get_store(db_path: &TempDir) -> Arc<DiskStore> {
+fn get_store(db_path: &TempDir) -> Arc<DiskStore<E>> {
     let spec = E::default_spec();
     let hot_path = db_path.path().join("hot_db");
     let cold_path = db_path.path().join("cold_db");
     let slots_per_restore_point = MinimalEthSpec::slots_per_historical_root() as u64;
     let log = NullLoggerBuilder.build().expect("logger should build");
     Arc::new(
-        DiskStore::open::<E>(&hot_path, &cold_path, slots_per_restore_point, spec, log)
+        DiskStore::open(&hot_path, &cold_path, slots_per_restore_point, spec, log)
             .expect("disk store should initialize"),
     )
 }

beacon_node/beacon_chain/tests/store_tests.rs

@@ -26,19 +26,19 @@ lazy_static! {
 type E = MinimalEthSpec;
 type TestHarness = BeaconChainHarness<DiskHarnessType<E>>;
 
-fn get_store(db_path: &TempDir) -> Arc<DiskStore> {
+fn get_store(db_path: &TempDir) -> Arc<DiskStore<E>> {
     let spec = MinimalEthSpec::default_spec();
     let hot_path = db_path.path().join("hot_db");
     let cold_path = db_path.path().join("cold_db");
     let slots_per_restore_point = MinimalEthSpec::slots_per_historical_root() as u64;
     let log = NullLoggerBuilder.build().expect("logger should build");
     Arc::new(
-        DiskStore::open::<E>(&hot_path, &cold_path, slots_per_restore_point, spec, log)
+        DiskStore::open(&hot_path, &cold_path, slots_per_restore_point, spec, log)
             .expect("disk store should initialize"),
     )
 }
 
-fn get_harness(store: Arc<DiskStore>, validator_count: usize) -> TestHarness {
+fn get_harness(store: Arc<DiskStore<E>>, validator_count: usize) -> TestHarness {
     let harness = BeaconChainHarness::new_with_disk_store(
         MinimalEthSpec,
         store,
@@ -265,7 +265,7 @@ fn check_finalization(harness: &TestHarness, expected_slot: u64) {
 }
 
 /// Check that the DiskStore's split_slot is equal to the start slot of the last finalized epoch.
-fn check_split_slot(harness: &TestHarness, store: Arc<DiskStore>) {
+fn check_split_slot(harness: &TestHarness, store: Arc<DiskStore<E>>) {
     let split_slot = store.get_split_slot();
     assert_eq!(
         harness
@@ -286,7 +286,7 @@ fn check_chain_dump(harness: &TestHarness, expected_len: u64) {
 
     assert_eq!(chain_dump.len() as u64, expected_len);
 
-    for checkpoint in chain_dump {
+    for checkpoint in &chain_dump {
         // Check that the tree hash of the stored state is as expected
         assert_eq!(
             checkpoint.beacon_state_root,
@@ -300,13 +300,41 @@ fn check_chain_dump(harness: &TestHarness, expected_len: u64) {
             harness
                 .chain
                 .store
-                .get_state::<E>(&checkpoint.beacon_state_root, None)
+                .get_state(&checkpoint.beacon_state_root, None)
                 .expect("no error")
                 .expect("state exists")
                 .slot,
             checkpoint.beacon_state.slot
         );
     }
+
+    // Check the forwards block roots iterator against the chain dump
+    let chain_dump_block_roots = chain_dump
+        .iter()
+        .map(|checkpoint| (checkpoint.beacon_block_root, checkpoint.beacon_block.slot))
+        .collect::<Vec<_>>();
+
+    let head = harness.chain.head();
+    let mut forward_block_roots = Store::forwards_block_roots_iterator(
+        harness.chain.store.clone(),
+        Slot::new(0),
+        head.beacon_state,
+        head.beacon_block_root,
+        &harness.spec,
+    )
+    .collect::<Vec<_>>();
+
+    // Drop the block roots for skipped slots.
+    forward_block_roots.dedup_by_key(|(block_root, _)| *block_root);
+
+    for i in 0..std::cmp::max(chain_dump_block_roots.len(), forward_block_roots.len()) {
+        assert_eq!(
+            chain_dump_block_roots[i],
+            forward_block_roots[i],
+            "split slot is {}",
+            harness.chain.store.get_split_slot()
+        );
+    }
 }
 
 /// Check that state and block root iterators can reach genesis