diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 149e0bf79..c446dcd85 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/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_slashing(proposer_slashing, &state, spec) - .map_err(|e| e.into_with_index(i))?; + + // 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)) + })?; + + 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. - 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))?; + // Ensure the previous epoch cache exists. + state.build_epoch_cache(RelativeEpoch::Previous, spec)?; + // 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_deposit(state, deposit, VERIFY_DEPOSIT_MERKLE_PROOFS, spec) - .map_err(|e| e.into_with_index(i))?; - state - .process_deposit( - deposit.deposit_data.deposit_input.pubkey.clone(), - deposit.deposit_data.amount, - deposit - .deposit_data - .deposit_input - .proof_of_possession - .clone(), - deposit.deposit_data.deposit_input.withdrawal_credentials, - None, - spec, - ) - .map_err(|_| Error::Invalid(Invalid::DepositProcessingFailed(i)))?; + // 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` 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))?; + + let deposit_data = &deposit.deposit_data; + let deposit_input = &deposit.deposit_data.deposit_input; + + if let Some(index) = validator_index { + // Update the existing validator balance. + safe_add_assign!( + state.validator_balances[index as usize], + deposit_data.amount + ); + } else { + // Create a new validator. + let validator = Validator { + pubkey: deposit_input.pubkey.clone(), + 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); } diff --git a/eth2/state_processing/src/per_block_processing/errors.rs b/eth2/state_processing/src/per_block_processing/errors.rs index 7e71a9b75..f64a3f8aa 100644 --- a/eth2/state_processing/src/per_block_processing/errors.rs +++ b/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, diff --git a/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs b/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs index 71ac97469..2970712c5 100644 --- a/eth2/state_processing/src/per_block_processing/verify_attester_slashing.rs +++ b/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, 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, 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); } } diff --git a/eth2/state_processing/src/per_block_processing/verify_deposit.rs b/eth2/state_processing/src/per_block_processing/verify_deposit.rs index 69dae1533..0cf2a078f 100644 --- a/eth2/state_processing/src/per_block_processing/verify_deposit.rs +++ b/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; + /// 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, - Invalid::BadIndex(state.deposit_index, deposit.index) + // TODO: update proof of possession. + // + // 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, +) -> Result, 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 diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index f3d533527..39970b9a7 100644 --- a/eth2/types/src/beacon_state.rs +++ b/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>, spec: &ChainSpec, ) -> Result { - // 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(()); }