Optimise deposits processing.

eth2/state_processing/src/per_block_processing.rs

@@ -1,12 +1,17 @@
 use self::verify_proposer_slashing::verify_proposer_slashing;
 use errors::{BlockInvalid as Invalid, BlockProcessingError as Error, IntoWithIndex};
 use hashing::hash;
+use rayon::prelude::*;
 use ssz::{ssz_encode, SignedRoot, TreeHash};
 use types::*;
 
-pub use self::verify_attester_slashing::verify_attester_slashing;
+pub use self::verify_attester_slashing::{
+    gather_attester_slashing_indices, verify_attester_slashing,
+};
 pub use validate_attestation::{validate_attestation, validate_attestation_without_signature};
-pub use verify_deposit::verify_deposit;
+pub use verify_deposit::{
+    build_public_key_hashmap, get_existing_validator_index, verify_deposit, verify_deposit_index,
+};
 pub use verify_exit::verify_exit;
 pub use verify_transfer::{execute_transfer, verify_transfer};
 
@@ -226,9 +231,17 @@ pub fn process_proposer_slashings(
         proposer_slashings.len() as u64 <= spec.max_proposer_slashings,
         Invalid::MaxProposerSlashingsExceeded
     );
-    for (i, proposer_slashing) in proposer_slashings.iter().enumerate() {
+
+    // Verify proposer slashings in parallel.
+    proposer_slashings
+        .par_iter()
+        .enumerate()
+        .try_for_each(|(i, proposer_slashing)| {
             verify_proposer_slashing(proposer_slashing, &state, spec)
-            .map_err(|e| e.into_with_index(i))?;
+                .map_err(|e| e.into_with_index(i))
+        })?;
+
+    for proposer_slashing in proposer_slashings {
         state.slash_validator(proposer_slashing.proposer_index as usize, spec)?;
     }
 
@@ -250,8 +263,19 @@ pub fn process_attester_slashings(
         attester_slashings.len() as u64 <= spec.max_attester_slashings,
         Invalid::MaxAttesterSlashingsExceed
     );
+
+    // Verify attester slashings in parallel.
+    attester_slashings
+        .par_iter()
+        .enumerate()
+        .try_for_each(|(i, attester_slashing)| {
+            verify_attester_slashing(&state, &attester_slashing, spec)
+                .map_err(|e| e.into_with_index(i))
+        })?;
+
+    // Gather the slashable indices and update the state in series.
     for (i, attester_slashing) in attester_slashings.iter().enumerate() {
-        let slashable_indices = verify_attester_slashing(&state, &attester_slashing, spec)
+        let slashable_indices = gather_attester_slashing_indices(&state, &attester_slashing)
             .map_err(|e| e.into_with_index(i))?;
         for i in slashable_indices {
             state.slash_validator(i as usize, spec)?;
@@ -276,14 +300,20 @@ pub fn process_attestations(
         attestations.len() as u64 <= spec.max_attestations,
         Invalid::MaxAttestationsExceeded
     );
-    for (i, attestation) in attestations.iter().enumerate() {
+
-        // Build the previous epoch cache only if required by an attestation.
+    // Ensure the previous epoch cache exists.
-        if attestation.data.slot.epoch(spec.slots_per_epoch) == state.previous_epoch(spec) {
     state.build_epoch_cache(RelativeEpoch::Previous, spec)?;
-        }
 
-        validate_attestation(state, attestation, spec).map_err(|e| e.into_with_index(i))?;
+    // Verify attestations in parallel.
+    attestations
+        .par_iter()
+        .enumerate()
+        .try_for_each(|(i, attestation)| {
+            validate_attestation(state, attestation, spec).map_err(|e| e.into_with_index(i))
+        })?;
 
+    // Update the state in series.
+    for attestation in attestations {
         let pending_attestation = PendingAttestation {
             data: attestation.data.clone(),
             aggregation_bitfield: attestation.aggregation_bitfield.clone(),
@@ -311,24 +341,53 @@ pub fn process_deposits(
         deposits.len() as u64 <= spec.max_deposits,
         Invalid::MaxDepositsExceeded
     );
-    for (i, deposit) in deposits.iter().enumerate() {
+
+    // Verify deposits in parallel.
+    deposits
+        .par_iter()
+        .enumerate()
+        .try_for_each(|(i, deposit)| {
             verify_deposit(state, deposit, VERIFY_DEPOSIT_MERKLE_PROOFS, spec)
+                .map_err(|e| e.into_with_index(i))
+        })?;
+
+    let public_key_to_index_hashmap = build_public_key_hashmap(&state);
+
+    // Check `state.deposit_index` and update the state in series.
+    for (i, deposit) in deposits.iter().enumerate() {
+        verify_deposit_index(state, deposit).map_err(|e| e.into_with_index(i))?;
+
+        // Get an `Option<u64>` where `u64` is the validator index if this deposit public key
+        // already exists in the beacon_state.
+        //
+        // This function also verifies the withdrawal credentials.
+        let validator_index =
+            get_existing_validator_index(state, deposit, &public_key_to_index_hashmap)
                 .map_err(|e| e.into_with_index(i))?;
 
-        state
+        let deposit_data = &deposit.deposit_data;
-            .process_deposit(
+        let deposit_input = &deposit.deposit_data.deposit_input;
-                deposit.deposit_data.deposit_input.pubkey.clone(),
+
-                deposit.deposit_data.amount,
+        if let Some(index) = validator_index {
-                deposit
+            // Update the existing validator balance.
-                    .deposit_data
+            safe_add_assign!(
-                    .deposit_input
+                state.validator_balances[index as usize],
-                    .proof_of_possession
+                deposit_data.amount
-                    .clone(),
+            );
-                deposit.deposit_data.deposit_input.withdrawal_credentials,
+        } else {
-                None,
+            // Create a new validator.
-                spec,
+            let validator = Validator {
-            )
+                pubkey: deposit_input.pubkey.clone(),
-            .map_err(|_| Error::Invalid(Invalid::DepositProcessingFailed(i)))?;
+                withdrawal_credentials: deposit_input.withdrawal_credentials.clone(),
+                activation_epoch: spec.far_future_epoch,
+                exit_epoch: spec.far_future_epoch,
+                withdrawable_epoch: spec.far_future_epoch,
+                initiated_exit: false,
+                slashed: false,
+            };
+            state.validator_registry.push(validator);
+            state.validator_balances.push(deposit_data.amount);
+        }
 
         state.deposit_index += 1;
     }
@@ -351,9 +410,17 @@ pub fn process_exits(
         voluntary_exits.len() as u64 <= spec.max_voluntary_exits,
         Invalid::MaxExitsExceeded
     );
-    for (i, exit) in voluntary_exits.iter().enumerate() {
-        verify_exit(&state, exit, spec).map_err(|e| e.into_with_index(i))?;
 
+    // Verify exits in parallel.
+    voluntary_exits
+        .par_iter()
+        .enumerate()
+        .try_for_each(|(i, exit)| {
+            verify_exit(&state, exit, spec).map_err(|e| e.into_with_index(i))
+        })?;
+
+    // Update the state in series.
+    for exit in voluntary_exits {
         state.initiate_validator_exit(exit.validator_index as usize);
     }
 

eth2/state_processing/src/per_block_processing/errors.rs

@@ -294,6 +294,11 @@ pub enum DepositInvalid {
     ///
     /// (state_index, deposit_index)
     BadIndex(u64, u64),
+    /// The proof-of-possession does not match the given pubkey.
+    BadProofOfPossession,
+    /// The withdrawal credentials for the depositing validator did not match the withdrawal
+    /// credentials of an existing validator with the same public key.
+    BadWithdrawalCredentials,
     /// The specified `branch` and `index` did not form a valid proof that the deposit is included
     /// in the eth1 deposit root.
     BadMerkleProof,

eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs

@@ -12,7 +12,7 @@ pub fn verify_attester_slashing(
     state: &BeaconState,
     attester_slashing: &AttesterSlashing,
     spec: &ChainSpec,
-) -> Result<Vec<u64>, Error> {
+) -> Result<(), Error> {
     let slashable_attestation_1 = &attester_slashing.slashable_attestation_1;
     let slashable_attestation_2 = &attester_slashing.slashable_attestation_2;
 
@@ -31,6 +31,21 @@ pub fn verify_attester_slashing(
     verify_slashable_attestation(state, &slashable_attestation_2, spec)
         .map_err(|e| Error::Invalid(Invalid::SlashableAttestation2Invalid(e.into())))?;
 
+    Ok(())
+}
+
+/// For a given attester slashing, return the indices able to be slashed.
+///
+/// Returns Ok(indices) if `indices.len() > 0`.
+///
+/// Spec v0.4.0
+pub fn gather_attester_slashing_indices(
+    state: &BeaconState,
+    attester_slashing: &AttesterSlashing,
+) -> Result<Vec<u64>, Error> {
+    let slashable_attestation_1 = &attester_slashing.slashable_attestation_1;
+    let slashable_attestation_2 = &attester_slashing.slashable_attestation_2;
+
     let mut slashable_indices = vec![];
     for i in &slashable_attestation_1.validator_indices {
         let validator = state
@@ -38,7 +53,7 @@ pub fn verify_attester_slashing(
             .get(*i as usize)
             .ok_or_else(|| Error::Invalid(Invalid::UnknownValidator(*i)))?;
 
-        if slashable_attestation_1.validator_indices.contains(&i) & !validator.slashed {
+        if slashable_attestation_2.validator_indices.contains(&i) & !validator.slashed {
             slashable_indices.push(*i);
         }
     }

eth2/state_processing/src/per_block_processing/verify_deposit.rs

@@ -1,15 +1,22 @@
 use super::errors::{DepositInvalid as Invalid, DepositValidationError as Error};
+use bls::verify_proof_of_possession;
 use hashing::hash;
 use merkle_proof::verify_merkle_proof;
 use ssz::ssz_encode;
 use ssz_derive::Encode;
+use std::collections::HashMap;
 use types::*;
 
+pub type PublicKeyValidatorIndexHashmap = HashMap<PublicKey, u64>;
+
 /// Indicates if a `Deposit` is valid to be included in a block in the current epoch of the given
 /// state.
 ///
 /// Returns `Ok(())` if the `Deposit` is valid, otherwise indicates the reason for invalidity.
 ///
+/// This function _does not_ check `state.deposit_index` so this function may be run in parallel.
+/// See the `verify_deposit_index` function for this.
+///
 /// Note: this function is incomplete.
 ///
 /// Spec v0.4.0
@@ -20,8 +27,14 @@ pub fn verify_deposit(
     spec: &ChainSpec,
 ) -> Result<(), Error> {
     verify!(
-        deposit.index == state.deposit_index,
+        // TODO: update proof of possession.
-        Invalid::BadIndex(state.deposit_index, deposit.index)
+        //
+        // https://github.com/sigp/lighthouse/issues/239
+        verify_proof_of_possession(
+            &deposit.deposit_data.deposit_input.proof_of_possession,
+            &deposit.deposit_data.deposit_input.pubkey
+        ),
+        Invalid::BadProofOfPossession
     );
 
     if verify_merkle_branch {
@@ -34,6 +47,50 @@ pub fn verify_deposit(
     Ok(())
 }
 
+/// Verify that the `Deposit` index is correct.
+///
+/// Spec v0.4.0
+pub fn verify_deposit_index(state: &BeaconState, deposit: &Deposit) -> Result<(), Error> {
+    verify!(
+        deposit.index == state.deposit_index,
+        Invalid::BadIndex(state.deposit_index, deposit.index)
+    );
+
+    Ok(())
+}
+
+pub fn build_public_key_hashmap(state: &BeaconState) -> PublicKeyValidatorIndexHashmap {
+    let mut hashmap = HashMap::with_capacity(state.validator_registry.len());
+
+    for (i, validator) in state.validator_registry.iter().enumerate() {
+        hashmap.insert(validator.pubkey.clone(), i as u64);
+    }
+
+    hashmap
+}
+
+pub fn get_existing_validator_index(
+    state: &BeaconState,
+    deposit: &Deposit,
+    pubkey_map: &HashMap<PublicKey, u64>,
+) -> Result<Option<u64>, Error> {
+    let deposit_input = &deposit.deposit_data.deposit_input;
+
+    let validator_index = pubkey_map.get(&deposit_input.pubkey).and_then(|i| Some(*i));
+
+    match validator_index {
+        None => Ok(None),
+        Some(index) => {
+            verify!(
+                deposit_input.withdrawal_credentials
+                    == state.validator_registry[index as usize].withdrawal_credentials,
+                Invalid::BadWithdrawalCredentials
+            );
+            Ok(Some(index))
+        }
+    }
+}
+
 /// Verify that a deposit is included in the state's eth1 deposit root.
 ///
 /// Spec v0.4.0

eth2/types/src/beacon_state.rs

@@ -608,6 +608,9 @@ impl BeaconState {
     /// this hashmap, each call to `process_deposits` requires an iteration though
     /// `self.validator_registry`. This becomes highly inefficient at scale.
     ///
+    /// TODO: this function also exists in a more optimal form in the `state_processing` crate as
+    /// `process_deposits`; unify these two functions.
+    ///
     /// Spec v0.4.0
     pub fn process_deposit(
         &mut self,
@@ -618,10 +621,7 @@ impl BeaconState {
         pubkey_map: Option<&HashMap<PublicKey, usize>>,
         spec: &ChainSpec,
     ) -> Result<usize, ()> {
-        // TODO: update proof of possession to function written above (
-        // requires bls::create_proof_of_possession to be updated
         //
-        // https://github.com/sigp/lighthouse/issues/239
         if !verify_proof_of_possession(&proof_of_possession, &pubkey) {
             return Err(());
         }