minimize the number of places we are calling update_pubkey_cache (#1626)

## Issue Addressed

- Resolves #1080

## Proposed Changes

- Call `update_pubkey_cache` only in the `build_all_caches` method and `get_validator_index` method. 

## Additional Info

This does reduce the number of places the cache is updated, making it simpler. But the `get_validator_index` method is used a couple times when we are iterating through the entire validator registry (or set of active validators). Before, we would only call `update_pubkey_cache` once before iterating through all validators.  So I'm not _totally_ sure this change is worth it.
This commit is contained in:
realbigsean 2020-09-23 01:19:56 +00:00
parent 1801dd1a34
commit b75df29501
6 changed files with 18 additions and 37 deletions

View File

@ -192,12 +192,11 @@ pub fn get_all_validators<T: BeaconChainTypes>(
};
let mut state = get_state_from_root_opt(&ctx.beacon_chain, state_root_opt)?;
state.update_pubkey_cache()?;
state
.validators
let validators = state.validators.clone();
validators
.iter()
.map(|validator| validator_response_by_pubkey(&state, validator.pubkey.clone()))
.map(|validator| validator_response_by_pubkey(&mut state, validator.pubkey.clone()))
.collect::<Result<Vec<_>, _>>()
}
@ -215,13 +214,14 @@ pub fn get_active_validators<T: BeaconChainTypes>(
};
let mut state = get_state_from_root_opt(&ctx.beacon_chain, state_root_opt)?;
state.update_pubkey_cache()?;
state
.validators
let validators = state.validators.clone();
let current_epoch = state.current_epoch();
validators
.iter()
.filter(|validator| validator.is_active_at(state.current_epoch()))
.map(|validator| validator_response_by_pubkey(&state, validator.pubkey.clone()))
.filter(|validator| validator.is_active_at(current_epoch))
.map(|validator| validator_response_by_pubkey(&mut state, validator.pubkey.clone()))
.collect::<Result<Vec<_>, _>>()
}
@ -279,11 +279,10 @@ fn validator_responses_by_pubkey<T: BeaconChainTypes>(
validator_pubkeys: Vec<PublicKeyBytes>,
) -> Result<Vec<ValidatorResponse>, ApiError> {
let mut state = get_state_from_root_opt(beacon_chain, state_root_opt)?;
state.update_pubkey_cache()?;
validator_pubkeys
.into_iter()
.map(|validator_pubkey| validator_response_by_pubkey(&state, validator_pubkey))
.map(|validator_pubkey| validator_response_by_pubkey(&mut state, validator_pubkey))
.collect::<Result<Vec<_>, ApiError>>()
}
@ -291,7 +290,7 @@ fn validator_responses_by_pubkey<T: BeaconChainTypes>(
///
/// The provided `state` must have a fully up-to-date pubkey cache.
fn validator_response_by_pubkey<E: EthSpec>(
state: &BeaconState<E>,
state: &mut BeaconState<E>,
validator_pubkey: PublicKeyBytes,
) -> Result<ValidatorResponse, ApiError> {
let validator_index_opt = state

View File

@ -92,10 +92,6 @@ pub fn post_individual_votes<T: BeaconChainTypes>(
let mut validator_statuses = ValidatorStatuses::new(&state, spec)?;
validator_statuses.process_attestations(&state, spec)?;
state.update_pubkey_cache().map_err(|e| {
ApiError::ServerError(format!("Unable to build pubkey cache: {:?}", e))
})?;
body.pubkeys
.into_iter()
.map(|pubkey| {

View File

@ -156,9 +156,6 @@ fn return_validator_duties<T: BeaconChainTypes>(
state
.build_committee_cache(relative_epoch, &beacon_chain.spec)
.map_err(|e| ApiError::ServerError(format!("Unable to build committee cache: {:?}", e)))?;
state
.update_pubkey_cache()
.map_err(|e| ApiError::ServerError(format!("Unable to build pubkey cache: {:?}", e)))?;
// Get a list of all validators for this epoch.
//

View File

@ -446,10 +446,6 @@ pub fn process_deposit<T: EthSpec>(
state.eth1_deposit_index.increment()?;
// Ensure the state's pubkey cache is fully up-to-date, it will be used to check to see if the
// depositing validator already exists in the registry.
state.update_pubkey_cache()?;
// Get an `Option<u64>` where `u64` is the validator index if this deposit public key
// already exists in the beacon_state.
let validator_index = get_existing_validator_index(state, &deposit.data.pubkey)

View File

@ -35,7 +35,7 @@ pub fn verify_deposit_signature(deposit_data: &DepositData, spec: &ChainSpec) ->
///
/// Errors if the state's `pubkey_cache` is not current.
pub fn get_existing_validator_index<T: EthSpec>(
state: &BeaconState<T>,
state: &mut BeaconState<T>,
pub_key: &PublicKeyBytes,
) -> Result<Option<u64>> {
let validator_index = state.get_validator_index(pub_key)?;

View File

@ -300,19 +300,12 @@ impl<T: EthSpec> BeaconState<T> {
}
}
/// If a validator pubkey exists in the validator registry, returns `Some(i)`, otherwise
/// returns `None`.
///
/// Requires a fully up-to-date `pubkey_cache`, returns an error if this is not the case.
pub fn get_validator_index(&self, pubkey: &PublicKeyBytes) -> Result<Option<usize>, Error> {
if self.pubkey_cache.len() == self.validators.len() {
/// This method ensures the state's pubkey cache is fully up-to-date before checking if the validator
/// exists in the registry. If a validator pubkey exists in the validator registry, returns `Some(i)`,
/// otherwise returns `None`.
pub fn get_validator_index(&mut self, pubkey: &PublicKeyBytes) -> Result<Option<usize>, Error> {
self.update_pubkey_cache()?;
Ok(self.pubkey_cache.get(pubkey))
} else {
Err(Error::PubkeyCacheIncomplete {
cache_len: self.pubkey_cache.len(),
registry_len: self.validators.len(),
})
}
}
/// The epoch corresponding to `self.slot`.