diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 20a93e31e8d..21db349505b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1462,50 +1462,6 @@ impl BeaconChain { self.state_at_slot(self.slot()?, StateSkipConfig::WithStateRoots) } - /// Returns the validator index (if any) for the given public key. - /// - /// ## Notes - /// - /// This query uses the `validator_pubkey_cache` which contains _all_ validators ever seen, - /// even if those validators aren't included in the head state. It is important to remember - /// that just because a validator exists here, it doesn't necessarily exist in all - /// `BeaconStates`. - /// - /// ## Errors - /// - /// May return an error if acquiring a read-lock on the `validator_pubkey_cache` times out. - pub fn validator_index(&self, pubkey: &PublicKeyBytes) -> Result, Error> { - let pubkey_cache = self - .validator_pubkey_cache - .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT) - .ok_or(Error::ValidatorPubkeyCacheLockTimeout)?; - - Ok(pubkey_cache.get_index(pubkey)) - } - - /// Return the validator indices of all public keys fetched from an iterator. - /// - /// If any public key doesn't belong to a known validator then an error will be returned. - /// We could consider relaxing this by returning `Vec>` in future. - pub fn validator_indices<'a>( - &self, - validator_pubkeys: impl Iterator, - ) -> Result, Error> { - let pubkey_cache = self - .validator_pubkey_cache - .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT) - .ok_or(Error::ValidatorPubkeyCacheLockTimeout)?; - - validator_pubkeys - .map(|pubkey| { - pubkey_cache - .get_index(pubkey) - .map(|id| id as u64) - .ok_or(Error::ValidatorPubkeyUnknown(*pubkey)) - }) - .collect() - } - /// Returns the validator pubkey (if any) for the given validator index. /// /// ## Notes diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index ac3d3e3ab80..9d2679f5d58 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -2083,7 +2083,7 @@ fn get_signature_verifier<'a, T: BeaconChainTypes>( let decompressor = move |pk_bytes| { // Map compressed pubkey to validator index. - let validator_index = validator_pubkey_cache.get_index(pk_bytes)?; + let validator_index = state.get_validator_index_readonly(pk_bytes)?; // Map validator index to pubkey (respecting guard on unknown validators). get_pubkey(validator_index) }; diff --git a/beacon_node/beacon_chain/src/sync_committee_verification.rs b/beacon_node/beacon_chain/src/sync_committee_verification.rs index 5c6710bfd6c..4eec3848cb2 100644 --- a/beacon_node/beacon_chain/src/sync_committee_verification.rs +++ b/beacon_node/beacon_chain/src/sync_committee_verification.rs @@ -376,12 +376,22 @@ impl VerifiedSyncContribution { .filter_map(|(pubkey, bit)| bit.then_some(pubkey)) .collect::>(); + let participant_indices = { + // TODO(lion): okay to get head state here? + let head_state = &chain.head_snapshot().beacon_state; + + participant_pubkeys + .iter() + .map(|pubkey| { + head_state + .get_validator_index_readonly(pubkey) + .ok_or(Error::UnknownValidatorPubkey(*pubkey)) + }) + .collect::, _>>()? + }; + // Ensure that all signatures are valid. - if !verify_signed_aggregate_signatures( - chain, - &signed_aggregate, - participant_pubkeys.as_slice(), - )? { + if !verify_signed_aggregate_signatures(chain, &signed_aggregate, &participant_indices)? { return Err(Error::InvalidSignature); } @@ -617,7 +627,7 @@ pub fn verify_propagation_slot_range( pub fn verify_signed_aggregate_signatures( chain: &BeaconChain, signed_aggregate: &SignedContributionAndProof, - participant_pubkeys: &[PublicKeyBytes], + participant_indexes: &[usize], ) -> Result { let pubkey_cache = chain .validator_pubkey_cache @@ -651,12 +661,8 @@ pub fn verify_signed_aggregate_signatures( ) .map_err(BeaconChainError::SignatureSetError)?, sync_committee_contribution_signature_set_from_pubkeys::( - |validator_index| { - pubkey_cache - .get_pubkey_from_pubkey_bytes(validator_index) - .map(Cow::Borrowed) - }, - participant_pubkeys, + |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), + participant_indexes, &signed_aggregate.message.contribution.signature, signed_aggregate .message @@ -683,13 +689,20 @@ pub fn verify_sync_committee_message( let signature_setup_timer = metrics::start_timer(&metrics::SYNC_MESSAGE_PROCESSING_SIGNATURE_SETUP_TIMES); + let index = chain + .head_snapshot() + .beacon_state + .pubkey_cache() + .get(pubkey_bytes) + .ok_or(Error::UnknownValidatorPubkey(*pubkey_bytes))?; + let pubkey_cache = chain .validator_pubkey_cache .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT) .ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)?; let pubkey = pubkey_cache - .get_pubkey_from_pubkey_bytes(pubkey_bytes) + .get(index) .map(Cow::Borrowed) .ok_or(Error::UnknownValidatorPubkey(*pubkey_bytes))?; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 6b85c8e4931..fe8d722bb9f 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1191,11 +1191,9 @@ where .iter() .enumerate() .map(|(subcommittee_position, pubkey)| { - let validator_index = self - .chain - .validator_index(pubkey) - .expect("should find validator index") - .expect("pubkey should exist in the beacon chain"); + let validator_index = state + .get_validator_index_readonly(pubkey) + .expect("should find validator index"); let sync_message = SyncCommitteeMessage::new::( message_slot, @@ -1403,11 +1401,9 @@ where .unwrap() .iter() .find_map(|pubkey| { - let validator_index = self - .chain - .validator_index(pubkey) - .expect("should find validator index") - .expect("pubkey should exist in the beacon chain"); + let validator_index = state + .get_validator_index_readonly(pubkey) + .expect("should find validator index"); let selection_proof = SyncSelectionProof::new::( slot, diff --git a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs index 00140dd6ec0..51b871b8d31 100644 --- a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs +++ b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs @@ -1,7 +1,6 @@ use crate::errors::BeaconChainError; use crate::{BeaconChainTypes, BeaconStore}; use ssz::{Decode, Encode}; -use std::collections::HashMap; use std::convert::TryInto; use std::marker::PhantomData; use store::{DBColumn, Error as StoreError, StoreItem, StoreOp}; @@ -17,7 +16,6 @@ use types::{BeaconState, Hash256, PublicKey, PublicKeyBytes}; /// Decompression is expensive when many keys are involved. pub struct ValidatorPubkeyCache { pubkeys: Vec, - indices: HashMap, pubkey_bytes: Vec, _phantom: PhantomData, } @@ -32,7 +30,6 @@ impl ValidatorPubkeyCache { ) -> Result { let mut cache = Self { pubkeys: vec![], - indices: HashMap::new(), pubkey_bytes: vec![], _phantom: PhantomData, }; @@ -46,7 +43,6 @@ impl ValidatorPubkeyCache { /// Load the pubkey cache from the given on-disk database. pub fn load_from_store(store: BeaconStore) -> Result { let mut pubkeys = vec![]; - let mut indices = HashMap::new(); let mut pubkey_bytes = vec![]; for validator_index in 0.. { @@ -57,7 +53,6 @@ impl ValidatorPubkeyCache { BeaconChainError::ValidatorPubkeyCacheError(format!("{:?}", e)) })?); pubkey_bytes.push(pubkey); - indices.insert(pubkey, validator_index); } else { break; } @@ -65,7 +60,6 @@ impl ValidatorPubkeyCache { Ok(ValidatorPubkeyCache { pubkeys, - indices, pubkey_bytes, _phantom: PhantomData, }) @@ -101,16 +95,11 @@ impl ValidatorPubkeyCache { { self.pubkey_bytes.reserve(validator_keys.len()); self.pubkeys.reserve(validator_keys.len()); - self.indices.reserve(validator_keys.len()); let mut store_ops = Vec::with_capacity(validator_keys.len()); for pubkey in validator_keys { let i = self.pubkeys.len(); - if self.indices.contains_key(&pubkey) { - return Err(BeaconChainError::DuplicateValidatorPublicKey); - } - // Stage the new validator key for writing to disk. // It will be committed atomically when the block that introduced it is written to disk. // Notably it is NOT written while the write lock on the cache is held. @@ -125,8 +114,6 @@ impl ValidatorPubkeyCache { .map_err(BeaconChainError::InvalidValidatorPubkeyBytes)?, ); self.pubkey_bytes.push(pubkey); - - self.indices.insert(pubkey, i); } Ok(store_ops) @@ -137,29 +124,19 @@ impl ValidatorPubkeyCache { self.pubkeys.get(i) } - /// Get the `PublicKey` for a validator with `PublicKeyBytes`. - pub fn get_pubkey_from_pubkey_bytes(&self, pubkey: &PublicKeyBytes) -> Option<&PublicKey> { - self.get_index(pubkey).and_then(|index| self.get(index)) - } - /// Get the public key (in bytes form) for a validator with index `i`. pub fn get_pubkey_bytes(&self, i: usize) -> Option<&PublicKeyBytes> { self.pubkey_bytes.get(i) } - /// Get the index of a validator with `pubkey`. - pub fn get_index(&self, pubkey: &PublicKeyBytes) -> Option { - self.indices.get(pubkey).copied() - } - /// Returns the number of validators in the cache. pub fn len(&self) -> usize { - self.indices.len() + self.pubkeys.len() } /// Returns `true` if there are no validators in the cache. pub fn is_empty(&self) -> bool { - self.indices.is_empty() + self.pubkeys.is_empty() } } @@ -226,16 +203,6 @@ mod test { if i < validator_count { let pubkey = cache.get(i).expect("pubkey should be present"); assert_eq!(pubkey, &keypairs[i].pk, "pubkey should match cache"); - - let pubkey_bytes: PublicKeyBytes = pubkey.clone().into(); - - assert_eq!( - i, - cache - .get_index(&pubkey_bytes) - .expect("should resolve index"), - "index should match cache" - ); } else { assert_eq!( cache.get(i), diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index a9b245e7987..38dd9f290a4 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -25,7 +25,6 @@ mod sync_committees; mod task_spawner; pub mod test_utils; mod ui; -mod validator; mod validator_inclusion; mod validators; mod version; @@ -86,7 +85,6 @@ use types::{ SignedContributionAndProof, SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncCommitteeMessage, SyncContributionData, }; -use validator::pubkey_to_validator_index; use version::{ add_consensus_version_header, add_ssz_content_type_header, execution_optimistic_finalized_fork_versioned_response, inconsistent_fork_rejection, @@ -771,14 +769,9 @@ pub fn serve( &chain, |state, execution_optimistic, finalized| { let index_opt = match &validator_id { - ValidatorId::PublicKey(pubkey) => pubkey_to_validator_index( - &chain, state, pubkey, - ) - .map_err(|e| { - warp_utils::reject::custom_not_found(format!( - "unable to access pubkey cache: {e:?}", - )) - })?, + ValidatorId::PublicKey(pubkey) => { + state.get_validator_index_readonly(pubkey) + } ValidatorId::Index(index) => Some(*index as usize), }; @@ -1017,42 +1010,50 @@ pub fn serve( chain: Arc>, query: api_types::SyncCommitteesQuery| { task_spawner.blocking_json_task(Priority::P1, move || { - let (sync_committee, execution_optimistic, finalized) = state_id + let (validators, execution_optimistic, finalized) = state_id .map_state_and_execution_optimistic_and_finalized( &chain, |state, execution_optimistic, finalized| { let current_epoch = state.current_epoch(); let epoch = query.epoch.unwrap_or(current_epoch); - Ok(( - state - .get_built_sync_committee(epoch, &chain.spec) - .cloned() - .map_err(|e| match e { - BeaconStateError::SyncCommitteeNotKnown { .. } => { - warp_utils::reject::custom_bad_request(format!( - "state at epoch {} has no \ + let sync_committee = state + .get_built_sync_committee(epoch, &chain.spec) + .cloned() + .map_err(|e| match e { + BeaconStateError::SyncCommitteeNotKnown { .. } => { + warp_utils::reject::custom_bad_request(format!( + "state at epoch {} has no \ sync committee for epoch {}", - current_epoch, epoch - )) - } - BeaconStateError::IncorrectStateVariant => { - warp_utils::reject::custom_bad_request(format!( - "state at epoch {} is not activated for Altair", - current_epoch, - )) - } - e => warp_utils::reject::beacon_state_error(e), - })?, - execution_optimistic, - finalized, - )) + current_epoch, epoch + )) + } + BeaconStateError::IncorrectStateVariant => { + warp_utils::reject::custom_bad_request(format!( + "state at epoch {} is not activated for Altair", + current_epoch, + )) + } + e => warp_utils::reject::beacon_state_error(e), + })?; + + let validators = sync_committee + .pubkeys + .iter() + .map(|pubkey| { + state + .get_validator_index_readonly(pubkey) + .map(|index| index as u64) + .ok_or(BeaconChainError::ValidatorPubkeyUnknown( + *pubkey, + )) + }) + .collect::, _>>() + .map_err(warp_utils::reject::beacon_chain_error)?; + + Ok((validators, execution_optimistic, finalized)) }, )?; - let validators = chain - .validator_indices(sync_committee.pubkeys.iter()) - .map_err(warp_utils::reject::beacon_chain_error)?; - let validator_aggregates = validators .chunks_exact(T::EthSpec::sync_subcommittee_size()) .map(|indices| api_types::SyncSubcommittee { @@ -3580,10 +3581,9 @@ pub fn serve( ) = register_val_data .into_iter() .filter_map(|register_data| { - chain - .validator_index(®ister_data.message.pubkey) - .ok() - .flatten() + head_snapshot + .beacon_state + .get_validator_index_readonly(®ister_data.message.pubkey) .and_then(|validator_index| { let validator = head_snapshot .beacon_state diff --git a/beacon_node/http_api/src/validator.rs b/beacon_node/http_api/src/validator.rs deleted file mode 100644 index 7f11ddd8f43..00000000000 --- a/beacon_node/http_api/src/validator.rs +++ /dev/null @@ -1,21 +0,0 @@ -use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes}; -use types::{BeaconState, PublicKeyBytes}; - -/// Uses the `chain.validator_pubkey_cache` to resolve a pubkey to a validator -/// index and then ensures that the validator exists in the given `state`. -pub fn pubkey_to_validator_index( - chain: &BeaconChain, - state: &BeaconState, - pubkey: &PublicKeyBytes, -) -> Result, BeaconChainError> { - chain - .validator_index(pubkey)? - .filter(|&index| { - state - .validators() - .get(index) - .map_or(false, |v| v.pubkey == *pubkey) - }) - .map(Result::Ok) - .transpose() -} diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index fcd324e9eb1..6b054333dd5 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -553,7 +553,7 @@ where #[allow(clippy::too_many_arguments)] pub fn sync_committee_contribution_signature_set_from_pubkeys<'a, T, F>( get_pubkey: F, - pubkey_bytes: &[PublicKeyBytes], + indices: &[usize], signature: &'a AggregateSignature, epoch: Epoch, beacon_block_root: Hash256, @@ -563,11 +563,11 @@ pub fn sync_committee_contribution_signature_set_from_pubkeys<'a, T, F>( ) -> Result> where T: EthSpec, - F: Fn(&PublicKeyBytes) -> Option>, + F: Fn(usize) -> Option>, { let mut pubkeys = Vec::with_capacity(T::SyncSubcommitteeSize::to_usize()); - for pubkey in pubkey_bytes { - pubkeys.push(get_pubkey(pubkey).ok_or(Error::ValidatorPubkeyUnknown(*pubkey))?); + for index in indices { + pubkeys.push(get_pubkey(*index).ok_or(Error::ValidatorUnknown(*index as u64))?); } let domain = spec.get_domain(epoch, Domain::SyncCommittee, fork, genesis_validators_root); diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 4c0ee1bfa20..bcfc2f687bd 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -500,6 +500,10 @@ impl BeaconState { Ok(self.pubkey_cache().get(pubkey)) } + pub fn get_validator_index_readonly(&self, pubkey: &PublicKeyBytes) -> Option { + self.pubkey_cache().get(pubkey) + } + /// The epoch corresponding to `self.slot()`. pub fn current_epoch(&self) -> Epoch { self.slot().epoch(T::slots_per_epoch()) diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 23b0ae26206..434e6c06118 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -354,9 +354,11 @@ fn do_transition( .map(Cow::Borrowed) }; + // TODO(lion): shit hack, revisit latter + let state = pre_state.clone(); let decompressor = move |pk_bytes| { // Map compressed pubkey to validator index. - let validator_index = validator_pubkey_cache.get_index(pk_bytes)?; + let validator_index = state.get_validator_index_readonly(pk_bytes)?; // Map validator index to pubkey (respecting guard on unknown validators). get_pubkey(validator_index) };