Optimize attestation processing (#841)

* Start updating types

* WIP

* Signature hacking

* Existing EF tests passing with fake_crypto

* Updates

* Delete outdated API spec

* The refactor continues

* It compiles

* WIP test fixes

* All release tests passing bar genesis state parsing

* Update and test YamlConfig

* Update to spec v0.10 compatible BLS

* Updates to BLS EF tests

* Add EF test for AggregateVerify

And delete unused hash2curve tests for uncompressed points

* Update EF tests to v0.10.1

* Use optional block root correctly in block proc

* Use genesis fork in deposit domain. All tests pass

* Cargo fmt

* Fast aggregate verify test

* Update REST API docs

* Cargo fmt

* Fix unused import

* Bump spec tags to v0.10.1

* Add `seconds_per_eth1_block` to chainspec

* Update to timestamp based eth1 voting scheme

* Return None from `get_votes_to_consider` if block cache is empty

* Handle overflows in `is_candidate_block`

* Revert to failing tests

* Fix eth1 data sets test

* Choose default vote according to spec

* Fix collect_valid_votes tests

* Fix `get_votes_to_consider` to choose all eligible blocks

* Uncomment winning_vote tests

* Add comments; remove unused code

* Reduce seconds_per_eth1_block for simulation

* Addressed review comments

* Add test for default vote case

* Fix logs

* Remove unused functions

* Meter default eth1 votes

* Fix comments

* Address review comments; remove unused dependency

* Add first attempt at attestation proc. re-write

* Add version 2 of attestation processing

* Minor fixes

* Add validator pubkey cache

* Make get_indexed_attestation take a committee

* Link signature processing into new attn verification

* First working version

* Ensure pubkey cache is updated

* Add more metrics, slight optimizations

* Clone committee cache during attestation processing

* Update shuffling cache during block processing

* Remove old commented-out code

* Fix shuffling cache insert bug

* Used indexed attestation in fork choice

* Restructure attn processing, add metrics

* Add more detailed metrics

* Tidy, fix failing tests

* Fix failing tests, tidy

* Disable/delete two outdated tests

* Tidy

* Add pubkey cache persistence file

* Add more comments

* Integrate persistence file into builder

* Add pubkey cache tests

* Add data_dir to beacon chain builder

* Remove Option in pubkey cache persistence file

* Ensure consistency between datadir/data_dir

* Fix failing network test

* Tidy

* Fix todos

* Add attestation processing tests

* Add another test

* Only run attestation tests in release

* Make attestation tests MainnetEthSpec

* Address Michael's comments

* Remove redundant check

* Fix warning

* Fix failing test

Co-authored-by: Michael Sproul <micsproul@gmail.com>
Co-authored-by: Pawan Dhananjay <pawandhananjay@gmail.com>

beacon_node/network/src/message_processor.rs

@@ -581,10 +581,16 @@ impl<T: BeaconChainTypes> MessageProcessor<T> {
                     // we don't know the block, get the sync manager to handle the block lookup
                     self.send_to_sync(SyncMessage::UnknownBlockHash(peer_id, beacon_block_root));
                 }
-                AttestationProcessingOutcome::AttestsToFutureState { .. }
+                AttestationProcessingOutcome::FutureEpoch { .. }
+                | AttestationProcessingOutcome::PastEpoch { .. }
+                | AttestationProcessingOutcome::UnknownTargetRoot { .. }
                 | AttestationProcessingOutcome::FinalizedSlot { .. } => {} // ignore the attestation
                 AttestationProcessingOutcome::Invalid { .. }
-                | AttestationProcessingOutcome::EmptyAggregationBitfield { .. } => {
+                | AttestationProcessingOutcome::EmptyAggregationBitfield { .. }
+                | AttestationProcessingOutcome::AttestsToFutureBlock { .. }
+                | AttestationProcessingOutcome::InvalidSignature
+                | AttestationProcessingOutcome::NoCommitteeForSlotAndIndex { .. }
+                | AttestationProcessingOutcome::BadTargetEpoch { .. } => {
                     // the peer has sent a bad attestation. Remove them.
                     self.network.disconnect(peer_id, GoodbyeReason::Fault);
                 }

beacon_node/network/src/persisted_dht.rs

@@ -1,15 +1,13 @@
-use beacon_chain::BeaconChainTypes;
 use eth2_libp2p::Enr;
 use rlp;
 use std::sync::Arc;
-use store::Store;
-use store::{DBColumn, Error as StoreError, SimpleStoreItem};
-use types::Hash256;
+use store::{DBColumn, Error as StoreError, SimpleStoreItem, Store};
+use types::{EthSpec, Hash256};
 
 /// 32-byte key for accessing the `DhtEnrs`.
 pub const DHT_DB_KEY: &str = "PERSISTEDDHTPERSISTEDDHTPERSISTE";
 
-pub fn load_dht<T: BeaconChainTypes>(store: Arc<T::Store>) -> Vec<Enr> {
+pub fn load_dht<T: Store<E>, E: EthSpec>(store: Arc<T>) -> Vec<Enr> {
     // Load DHT from store
     let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes());
     match store.get(&key) {
@@ -22,8 +20,8 @@ pub fn load_dht<T: BeaconChainTypes>(store: Arc<T::Store>) -> Vec<Enr> {
 }
 
 /// Attempt to persist the ENR's in the DHT to `self.store`.
-pub fn persist_dht<T: BeaconChainTypes>(
-    store: Arc<T::Store>,
+pub fn persist_dht<T: Store<E>, E: EthSpec>(
+    store: Arc<T>,
     enrs: Vec<Enr>,
 ) -> Result<(), store::Error> {
     let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes());

beacon_node/network/src/service.rs

@@ -56,7 +56,7 @@ impl<T: BeaconChainTypes> Service<T> {
         let (network_globals, mut libp2p_service) =
             LibP2PService::new(config, network_log.clone())?;
 
-        for enr in load_dht::<T>(store.clone()) {
+        for enr in load_dht::<T::Store, T::EthSpec>(store.clone()) {
             libp2p_service.swarm.add_enr(enr);
         }
 
@@ -154,7 +154,7 @@ fn spawn_service<T: BeaconChainTypes>(
                         "Number of peers" => format!("{}", enrs.len()),
                     );
 
-                    match persist_dht::<T>(store.clone(), enrs) {
+                    match persist_dht::<T::Store, T::EthSpec>(store.clone(), enrs) {
                         Err(e) => error!(
                             log,
                             "Failed to persist DHT on drop";

beacon_node/network/src/service/tests.rs

@@ -3,19 +3,16 @@
 mod tests {
     use crate::persisted_dht::load_dht;
     use crate::{NetworkConfig, Service};
-    use beacon_chain::builder::BeaconChainBuilder;
-    use beacon_chain::slot_clock::TestingSlotClock;
+    use beacon_chain::test_utils::BeaconChainHarness;
     use eth2_libp2p::Enr;
     use futures::{Future, IntoFuture};
-    use genesis::{generate_deterministic_keypairs, interop_genesis_state};
     use slog::Logger;
     use sloggers::{null::NullLoggerBuilder, Build};
     use std::str::FromStr;
     use std::sync::Arc;
-    use store::{migrate::NullMigrator, SimpleDiskStore};
-    use tempdir::TempDir;
+    use store::MemoryStore;
     use tokio::runtime::Runtime;
-    use types::{EthSpec, MinimalEthSpec};
+    use types::{test_utils::generate_deterministic_keypairs, MinimalEthSpec};
 
     fn get_logger() -> Logger {
         let builder = NullLoggerBuilder;
@@ -24,39 +21,14 @@ mod tests {
 
     #[test]
     fn test_dht_persistence() {
-        // Create new LevelDB store
-        let path = TempDir::new("persistence_test").unwrap();
-        let store = Arc::new(SimpleDiskStore::open(&path.into_path()).unwrap());
-        // Create a `BeaconChain` object to pass to `Service`
-        let validator_count = 1;
-        let genesis_time = 13371337;
-
         let log = get_logger();
-        let spec = MinimalEthSpec::default_spec();
 
-        let genesis_state = interop_genesis_state(
-            &generate_deterministic_keypairs(validator_count),
-            genesis_time,
-            &spec,
-        )
-        .expect("should create interop genesis state");
-        let chain = BeaconChainBuilder::new(MinimalEthSpec)
-            .logger(log.clone())
-            .store(store.clone())
-            .store_migrator(NullMigrator)
-            .genesis_state(genesis_state)
-            .expect("should build state using recent genesis")
-            .dummy_eth1_backend()
-            .expect("should build the dummy eth1 backend")
-            .null_event_handler()
-            .testing_slot_clock(std::time::Duration::from_secs(1))
-            .expect("should configure testing slot clock")
-            .reduced_tree_fork_choice()
-            .expect("should add fork choice to builder")
-            .build()
-            .expect("should build");
+        let beacon_chain = Arc::new(
+            BeaconChainHarness::new(MinimalEthSpec, generate_deterministic_keypairs(8)).chain,
+        );
+
+        let store = beacon_chain.store.clone();
 
-        let beacon_chain = Arc::new(chain);
         let enr1 = Enr::from_str("enr:-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8").unwrap();
         let enr2 = Enr::from_str("enr:-IS4QJ2d11eu6dC7E7LoXeLMgMP3kom1u3SE8esFSWvaHoo0dP1jg8O3-nx9ht-EO3CmG7L6OkHcMmoIh00IYWB92QABgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQIB_c-jQMOXsbjWkbN-Oj99H57gfId5pfb4wa1qxwV4CIN1ZHCCIyk").unwrap();
         let enrs = vec![enr1, enr2];
@@ -79,19 +51,7 @@ mod tests {
             .unwrap();
 
         // Load the persisted dht from the store
-        let persisted_enrs = load_dht::<
-            beacon_chain::builder::Witness<
-                SimpleDiskStore<types::eth_spec::MinimalEthSpec>,
-                store::migrate::NullMigrator,
-                TestingSlotClock,
-                beacon_chain::eth1_chain::CachingEth1Backend<
-                    types::eth_spec::MinimalEthSpec,
-                    SimpleDiskStore<types::eth_spec::MinimalEthSpec>,
-                >,
-                types::eth_spec::MinimalEthSpec,
-                beacon_chain::events::NullEventHandler<types::eth_spec::MinimalEthSpec>,
-            >,
-        >(store);
+        let persisted_enrs = load_dht::<MemoryStore<MinimalEthSpec>, MinimalEthSpec>(store);
         assert!(
             persisted_enrs.contains(&enrs[0]),
             "should have persisted the first ENR to store"