Arc-ify immutable Validator fields

This commit is contained in:
Michael Sproul
2022-03-07 17:33:59 +11:00
parent 73af0b6282
commit f93dfd0c28
9 changed files with 80 additions and 188 deletions

View File

@@ -297,10 +297,10 @@ impl<T: EthSpec> ValidatorMonitor<T> {
.skip(self.indices.len())
.for_each(|(i, validator)| {
let i = i as u64;
if let Some(validator) = self.validators.get_mut(&validator.pubkey) {
if let Some(validator) = self.validators.get_mut(validator.pubkey()) {
validator.set_index(i)
}
self.indices.insert(i, validator.pubkey);
self.indices.insert(i, *validator.pubkey());
});
// Update metrics for individual validators.

View File

@@ -116,7 +116,7 @@ impl<T: BeaconChainTypes> ValidatorPubkeyCache<T> {
.validators()
.iter_from(self.pubkeys.len())
.unwrap() // FIXME(sproul)
.map(|v| v.pubkey),
.map(|v| *v.pubkey()),
)
} else {
Ok(())

View File

@@ -530,7 +530,7 @@ pub fn serve<T: BeaconChainTypes>(
query.id.as_ref().map_or(true, |ids| {
ids.iter().any(|id| match id {
ValidatorId::PublicKey(pubkey) => {
&validator.pubkey == pubkey
validator.pubkey() == pubkey
}
ValidatorId::Index(param_index) => {
*param_index == *index as u64
@@ -578,7 +578,7 @@ pub fn serve<T: BeaconChainTypes>(
query.id.as_ref().map_or(true, |ids| {
ids.iter().any(|id| match id {
ValidatorId::PublicKey(pubkey) => {
&validator.pubkey == pubkey
validator.pubkey() == pubkey
}
ValidatorId::Index(param_index) => {
*param_index == *index as u64
@@ -635,7 +635,7 @@ pub fn serve<T: BeaconChainTypes>(
.map_state(&chain, |state| {
let index_opt = match &validator_id {
ValidatorId::PublicKey(pubkey) => {
state.validators().iter().position(|v| v.pubkey == *pubkey)
state.validators().iter().position(|v| v.pubkey() == pubkey)
}
ValidatorId::Index(index) => Some(*index as usize),
};

View File

@@ -6,6 +6,7 @@ use crate::common::{
use crate::per_block_processing::errors::{BlockProcessingError, IntoWithIndex};
use crate::VerifySignatures;
use safe_arith::SafeArith;
use std::sync::Arc;
use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR};
pub fn process_operations<'a, T: EthSpec>(
@@ -340,8 +341,10 @@ pub fn process_deposit<T: EthSpec>(
// Create a new validator.
let validator = Validator {
pubkey: deposit.data.pubkey,
withdrawal_credentials: deposit.data.withdrawal_credentials,
immutable: Arc::new(ValidatorImmutable {
pubkey: deposit.data.pubkey,
withdrawal_credentials: deposit.data.withdrawal_credentials,
}),
activation_eligibility_epoch: spec.far_future_epoch,
activation_epoch: spec.far_future_epoch,
exit_epoch: spec.far_future_epoch,

View File

@@ -63,7 +63,7 @@ where
.validators()
.get(validator_index)
.and_then(|v| {
let pk: Option<PublicKey> = v.pubkey.decompress().ok();
let pk: Option<PublicKey> = v.pubkey().decompress().ok();
pk
})
.map(Cow::Owned)

View File

@@ -865,7 +865,7 @@ impl<T: EthSpec> BeaconState<T> {
.map(|&index| {
self.validators()
.get(index)
.map(|v| v.pubkey)
.map(|v| *v.pubkey())
.ok_or(Error::UnknownValidator(index))
})
.collect::<Result<Vec<_>, _>>()?;
@@ -896,7 +896,7 @@ impl<T: EthSpec> BeaconState<T> {
validator_indices
.iter()
.map(|&validator_index| {
let pubkey = self.get_validator(validator_index as usize)?.pubkey;
let pubkey = *self.get_validator(validator_index as usize)?.pubkey();
Ok(SyncDuty::from_sync_committee(
validator_index,
@@ -1555,7 +1555,7 @@ impl<T: EthSpec> BeaconState<T> {
for (i, validator) in self.validators().iter_from(start_index)?.enumerate() {
let index = start_index.safe_add(i)?;
let success = pubkey_cache.insert(validator.pubkey, index);
let success = pubkey_cache.insert(*validator.pubkey(), index);
if !success {
return Err(Error::PubkeyCacheInconsistent);
}

View File

@@ -80,7 +80,6 @@ pub mod sync_committee_contribution;
pub mod sync_committee_message;
pub mod sync_selection_proof;
pub mod sync_subnet_id;
mod tree_hash_impls;
pub mod slot_data;
#[cfg(feature = "sqlite")]
@@ -154,7 +153,7 @@ pub use crate::sync_committee_subscription::SyncCommitteeSubscription;
pub use crate::sync_duty::SyncDuty;
pub use crate::sync_selection_proof::SyncSelectionProof;
pub use crate::sync_subnet_id::SyncSubnetId;
pub use crate::validator::Validator;
pub use crate::validator::{Validator, ValidatorImmutable};
pub use crate::validator_subscription::ValidatorSubscription;
pub use crate::voluntary_exit::VoluntaryExit;

View File

@@ -1,166 +0,0 @@
//! This module contains custom implementations of `CachedTreeHash` for ETH2-specific types.
//!
//! It makes some assumptions about the layouts and update patterns of other structs in this
//! crate, and should be updated carefully whenever those structs are changed.
use crate::{Epoch, Hash256, PublicKeyBytes, Validator};
use cached_tree_hash::{int_log, CacheArena, CachedTreeHash, Error, TreeHashCache};
use int_to_bytes::int_to_fixed_bytes32;
use tree_hash::merkle_root;
/// Number of struct fields on `Validator`.
const NUM_VALIDATOR_FIELDS: usize = 8;
impl CachedTreeHash<TreeHashCache> for Validator {
fn new_tree_hash_cache(&self, arena: &mut CacheArena) -> TreeHashCache {
TreeHashCache::new(arena, int_log(NUM_VALIDATOR_FIELDS), NUM_VALIDATOR_FIELDS)
}
/// Efficiently tree hash a `Validator`, assuming it was updated by a valid state transition.
///
/// Specifically, we assume that the `pubkey` and `withdrawal_credentials` fields are constant.
fn recalculate_tree_hash_root(
&self,
arena: &mut CacheArena,
cache: &mut TreeHashCache,
) -> Result<Hash256, Error> {
// Otherwise just check the fields which might have changed.
let dirty_indices = cache
.leaves()
.iter_mut(arena)?
.enumerate()
.flat_map(|(i, leaf)| {
// Fields pubkey and withdrawal_credentials are constant
if (i == 0 || i == 1) && cache.initialized {
None
} else if process_field_by_index(self, i, leaf, !cache.initialized) {
Some(i)
} else {
None
}
})
.collect();
cache.update_merkle_root(arena, dirty_indices)
}
}
fn process_field_by_index(
v: &Validator,
field_idx: usize,
leaf: &mut Hash256,
force_update: bool,
) -> bool {
match field_idx {
0 => process_pubkey_bytes_field(&v.pubkey, leaf, force_update),
1 => process_slice_field(v.withdrawal_credentials.as_bytes(), leaf, force_update),
2 => process_u64_field(v.effective_balance, leaf, force_update),
3 => process_bool_field(v.slashed, leaf, force_update),
4 => process_epoch_field(v.activation_eligibility_epoch, leaf, force_update),
5 => process_epoch_field(v.activation_epoch, leaf, force_update),
6 => process_epoch_field(v.exit_epoch, leaf, force_update),
7 => process_epoch_field(v.withdrawable_epoch, leaf, force_update),
_ => panic!(
"Validator type only has {} fields, {} out of bounds",
NUM_VALIDATOR_FIELDS, field_idx
),
}
}
fn process_pubkey_bytes_field(
val: &PublicKeyBytes,
leaf: &mut Hash256,
force_update: bool,
) -> bool {
let new_tree_hash = merkle_root(val.as_serialized(), 0);
process_slice_field(new_tree_hash.as_bytes(), leaf, force_update)
}
fn process_slice_field(new_tree_hash: &[u8], leaf: &mut Hash256, force_update: bool) -> bool {
if force_update || leaf.as_bytes() != new_tree_hash {
leaf.assign_from_slice(new_tree_hash);
true
} else {
false
}
}
fn process_u64_field(val: u64, leaf: &mut Hash256, force_update: bool) -> bool {
let new_tree_hash = int_to_fixed_bytes32(val);
process_slice_field(&new_tree_hash[..], leaf, force_update)
}
fn process_epoch_field(val: Epoch, leaf: &mut Hash256, force_update: bool) -> bool {
process_u64_field(val.as_u64(), leaf, force_update)
}
fn process_bool_field(val: bool, leaf: &mut Hash256, force_update: bool) -> bool {
process_u64_field(val as u64, leaf, force_update)
}
#[cfg(test)]
mod test {
use super::*;
use crate::test_utils::TestRandom;
use crate::Epoch;
use rand::SeedableRng;
use rand_xorshift::XorShiftRng;
use tree_hash::TreeHash;
fn test_validator_tree_hash(v: &Validator) {
let arena = &mut CacheArena::default();
let mut cache = v.new_tree_hash_cache(arena);
// With a fresh cache
assert_eq!(
&v.tree_hash_root()[..],
v.recalculate_tree_hash_root(arena, &mut cache)
.unwrap()
.as_bytes(),
"{:?}",
v
);
// With a completely up-to-date cache
assert_eq!(
&v.tree_hash_root()[..],
v.recalculate_tree_hash_root(arena, &mut cache)
.unwrap()
.as_bytes(),
"{:?}",
v
);
}
#[test]
fn default_validator() {
test_validator_tree_hash(&Validator::default());
}
#[test]
fn zeroed_validator() {
let v = Validator {
activation_eligibility_epoch: Epoch::from(0u64),
activation_epoch: Epoch::from(0u64),
..Default::default()
};
test_validator_tree_hash(&v);
}
#[test]
fn random_validators() {
let mut rng = XorShiftRng::from_seed([0xf1; 16]);
let num_validators = 1000;
(0..num_validators)
.map(|_| Validator::random_for_test(&mut rng))
.for_each(|v| test_validator_tree_hash(&v));
}
#[test]
#[allow(clippy::assertions_on_constants)]
pub fn smallvec_size_check() {
// If this test fails we need to go and reassess the length of the `SmallVec` in
// `cached_tree_hash::TreeHashCache`. If the size of the `SmallVec` is too slow we're going
// to start doing heap allocations for each validator, this will fragment memory and slow
// us down.
assert!(NUM_VALIDATOR_FIELDS <= 8,);
}
}

View File

@@ -3,17 +3,17 @@ use crate::{
};
use serde_derive::{Deserialize, Serialize};
use ssz_derive::{Decode, Encode};
use std::sync::Arc;
use test_random_derive::TestRandom;
use tree_hash_derive::TreeHash;
use tree_hash::TreeHash;
const NUM_FIELDS: usize = 8;
/// Information about a `BeaconChain` validator.
///
/// Spec v0.12.1
#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Encode, Decode, TestRandom, TreeHash)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Encode, Decode, TestRandom)]
pub struct Validator {
pub pubkey: PublicKeyBytes,
pub withdrawal_credentials: Hash256,
pub immutable: Arc<ValidatorImmutable>,
#[serde(with = "eth2_serde_utils::quoted_u64")]
pub effective_balance: u64,
pub slashed: bool,
@@ -23,7 +23,23 @@ pub struct Validator {
pub withdrawable_epoch: Epoch,
}
/// The immutable fields of a validator, behind an `Arc` to enable sharing.
#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Encode, Decode, TestRandom)]
pub struct ValidatorImmutable {
pub pubkey: PublicKeyBytes,
pub withdrawal_credentials: Hash256,
}
impl Validator {
pub fn pubkey(&self) -> &PublicKeyBytes {
&self.immutable.pubkey
}
pub fn withdrawal_credentials(&self) -> Hash256 {
self.immutable.withdrawal_credentials
}
/// Returns `true` if the validator is considered active at some epoch.
pub fn is_active_at(&self, epoch: Epoch) -> bool {
self.activation_epoch <= epoch && epoch < self.exit_epoch
@@ -65,14 +81,35 @@ impl Validator {
// Has not yet been activated
&& self.activation_epoch == spec.far_future_epoch
}
fn tree_hash_root_internal(&self) -> Result<Hash256, tree_hash::Error> {
let mut hasher = tree_hash::MerkleHasher::with_leaves(NUM_FIELDS);
hasher.write(self.pubkey().tree_hash_root().as_bytes())?;
hasher.write(self.withdrawal_credentials().tree_hash_root().as_bytes())?;
hasher.write(self.effective_balance.tree_hash_root().as_bytes())?;
hasher.write(self.slashed.tree_hash_root().as_bytes())?;
hasher.write(
self.activation_eligibility_epoch
.tree_hash_root()
.as_bytes(),
)?;
hasher.write(self.activation_epoch.tree_hash_root().as_bytes())?;
hasher.write(self.exit_epoch.tree_hash_root().as_bytes())?;
hasher.write(self.withdrawable_epoch.tree_hash_root().as_bytes())?;
hasher.finish()
}
}
impl Default for Validator {
/// Yields a "default" `Validator`. Primarily used for testing.
fn default() -> Self {
Self {
pubkey: PublicKeyBytes::empty(),
withdrawal_credentials: Hash256::default(),
immutable: Arc::new(ValidatorImmutable {
pubkey: PublicKeyBytes::empty(),
withdrawal_credentials: Hash256::default(),
}),
activation_eligibility_epoch: Epoch::from(std::u64::MAX),
activation_epoch: Epoch::from(std::u64::MAX),
exit_epoch: Epoch::from(std::u64::MAX),
@@ -83,6 +120,25 @@ impl Default for Validator {
}
}
impl TreeHash for Validator {
fn tree_hash_type() -> tree_hash::TreeHashType {
tree_hash::TreeHashType::Container
}
fn tree_hash_packed_encoding(&self) -> tree_hash::PackedEncoding {
unreachable!("Struct should never be packed.")
}
fn tree_hash_packing_factor() -> usize {
unreachable!("Struct should never be packed.")
}
fn tree_hash_root(&self) -> Hash256 {
self.tree_hash_root_internal()
.expect("Validator tree_hash_root should not fail")
}
}
#[cfg(test)]
mod tests {
use super::*;