From f74bf597dd3c17455cb45a26d3ba2f897874ef29 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 09:52:38 -0600 Subject: [PATCH 01/13] Adds some new fields from the latest spec --- beacon_chain/types/src/validator_record.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 3f4a43261..8a562ceb4 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -35,9 +35,12 @@ pub struct ValidatorRecord { pub withdrawal_credentials: Hash256, pub randao_commitment: Hash256, pub randao_layers: u64, - pub status: ValidatorStatus, - pub latest_status_change_slot: u64, + pub activation_slot: u64, + pub exit_slot: u64, + pub withdrawal_slot: u64, + pub penalized_slot: u64, pub exit_count: u64, + pub status: ValidatorStatus, pub custody_commitment: Hash256, pub latest_custody_reseed_slot: u64, pub penultimate_custody_reseed_slot: u64, From 6928301b91d6a605519ac936da365a51f7e2695e Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 8 Jan 2019 16:55:04 -0600 Subject: [PATCH 02/13] Updates `ValidatorRecord` to match the latest spec --- beacon_chain/types/src/lib.rs | 2 +- beacon_chain/types/src/validator_record.rs | 98 ++++++++++------------ 2 files changed, 43 insertions(+), 57 deletions(-) diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index 42bea9e0a..f6847356a 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -50,7 +50,7 @@ pub use crate::proposer_slashing::ProposerSlashing; pub use crate::shard_committee::ShardCommittee; pub use crate::slashable_vote_data::SlashableVoteData; pub use crate::special_record::{SpecialRecord, SpecialRecordKind}; -pub use crate::validator_record::{ValidatorRecord, ValidatorStatus}; +pub use crate::validator_record::{StatusFlags as ValidatorStatusFlags, ValidatorRecord}; pub type Hash256 = H256; pub type Address = H160; diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 8a562ceb4..05980559e 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -5,25 +5,20 @@ use rand::RngCore; use ssz::{Decodable, DecodeError, Encodable, SszStream}; use std::convert; +const STATUS_FLAG_INITIATED_EXIT: u8 = 1; +const STATUS_FLAG_WITHDRAWABLE: u8 = 2; + #[derive(Debug, PartialEq, Clone, Copy)] -pub enum ValidatorStatus { - PendingActivation, - Active, - PendingExit, - PendingWithdraw, - Withdrawn, - Penalized, +pub enum StatusFlags { + InitiatedExit, + Withdrawable, } -impl convert::From for ValidatorStatus { - fn from(status: u8) -> Self { - match status { - 0 => ValidatorStatus::PendingActivation, - 1 => ValidatorStatus::Active, - 2 => ValidatorStatus::PendingExit, - 3 => ValidatorStatus::PendingWithdraw, - 5 => ValidatorStatus::Withdrawn, - 127 => ValidatorStatus::Penalized, +impl convert::From for StatusFlags { + fn from(status_flag: u8) -> Self { + match status_flag { + STATUS_FLAG_INITIATED_EXIT => StatusFlags::InitiatedExit, + STATUS_FLAG_WITHDRAWABLE => StatusFlags::Withdrawable, _ => unreachable!(), } } @@ -40,59 +35,38 @@ pub struct ValidatorRecord { pub withdrawal_slot: u64, pub penalized_slot: u64, pub exit_count: u64, - pub status: ValidatorStatus, + pub status_flags: StatusFlags, pub custody_commitment: Hash256, pub latest_custody_reseed_slot: u64, pub penultimate_custody_reseed_slot: u64, } -impl ValidatorRecord { - pub fn status_is(&self, status: ValidatorStatus) -> bool { - self.status == status - } -} - -impl Encodable for ValidatorStatus { +impl Encodable for StatusFlags { fn ssz_append(&self, s: &mut SszStream) { let byte: u8 = match self { - ValidatorStatus::PendingActivation => 0, - ValidatorStatus::Active => 1, - ValidatorStatus::PendingExit => 2, - ValidatorStatus::PendingWithdraw => 3, - ValidatorStatus::Withdrawn => 5, - ValidatorStatus::Penalized => 127, + StatusFlags::InitiatedExit => STATUS_FLAG_INITIATED_EXIT, + StatusFlags::Withdrawable => STATUS_FLAG_WITHDRAWABLE, }; s.append(&byte); } } -impl Decodable for ValidatorStatus { +impl Decodable for StatusFlags { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { let (byte, i) = u8::ssz_decode(bytes, i)?; let status = match byte { - 0 => ValidatorStatus::PendingActivation, - 1 => ValidatorStatus::Active, - 2 => ValidatorStatus::PendingExit, - 3 => ValidatorStatus::PendingWithdraw, - 5 => ValidatorStatus::Withdrawn, - 127 => ValidatorStatus::Penalized, + 1 => StatusFlags::InitiatedExit, + 2 => StatusFlags::Withdrawable, _ => return Err(DecodeError::Invalid), }; Ok((status, i)) } } -impl TestRandom for ValidatorStatus { +impl TestRandom for StatusFlags { fn random_for_test(rng: &mut T) -> Self { - let options = vec![ - ValidatorStatus::PendingActivation, - ValidatorStatus::Active, - ValidatorStatus::PendingExit, - ValidatorStatus::PendingWithdraw, - ValidatorStatus::Withdrawn, - ValidatorStatus::Penalized, - ]; - options[(rng.next_u32() as usize) % options.len()] + let options = vec![StatusFlags::InitiatedExit, StatusFlags::Withdrawable]; + options[(rng.next_u32() as usize) % options.len()].clone() } } @@ -102,9 +76,12 @@ impl Encodable for ValidatorRecord { s.append(&self.withdrawal_credentials); s.append(&self.randao_commitment); s.append(&self.randao_layers); - s.append(&self.status); - s.append(&self.latest_status_change_slot); + s.append(&self.activation_slot); + s.append(&self.exit_slot); + s.append(&self.withdrawal_slot); + s.append(&self.penalized_slot); s.append(&self.exit_count); + s.append(&self.status_flags); s.append(&self.custody_commitment); s.append(&self.latest_custody_reseed_slot); s.append(&self.penultimate_custody_reseed_slot); @@ -117,9 +94,12 @@ impl Decodable for ValidatorRecord { let (withdrawal_credentials, i) = <_>::ssz_decode(bytes, i)?; let (randao_commitment, i) = <_>::ssz_decode(bytes, i)?; let (randao_layers, i) = <_>::ssz_decode(bytes, i)?; - let (status, i) = <_>::ssz_decode(bytes, i)?; - let (latest_status_change_slot, i) = <_>::ssz_decode(bytes, i)?; + let (activation_slot, i) = <_>::ssz_decode(bytes, i)?; + let (exit_slot, i) = <_>::ssz_decode(bytes, i)?; + let (withdrawal_slot, i) = <_>::ssz_decode(bytes, i)?; + let (penalized_slot, i) = <_>::ssz_decode(bytes, i)?; let (exit_count, i) = <_>::ssz_decode(bytes, i)?; + let (status_flags, i) = <_>::ssz_decode(bytes, i)?; let (custody_commitment, i) = <_>::ssz_decode(bytes, i)?; let (latest_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; let (penultimate_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; @@ -130,9 +110,12 @@ impl Decodable for ValidatorRecord { withdrawal_credentials, randao_commitment, randao_layers, - status, - latest_status_change_slot, + activation_slot, + exit_slot, + withdrawal_slot, + penalized_slot, exit_count, + status_flags, custody_commitment, latest_custody_reseed_slot, penultimate_custody_reseed_slot, @@ -149,9 +132,12 @@ impl TestRandom for ValidatorRecord { withdrawal_credentials: <_>::random_for_test(rng), randao_commitment: <_>::random_for_test(rng), randao_layers: <_>::random_for_test(rng), - status: <_>::random_for_test(rng), - latest_status_change_slot: <_>::random_for_test(rng), + activation_slot: <_>::random_for_test(rng), + exit_slot: <_>::random_for_test(rng), + withdrawal_slot: <_>::random_for_test(rng), + penalized_slot: <_>::random_for_test(rng), exit_count: <_>::random_for_test(rng), + status_flags: <_>::random_for_test(rng), custody_commitment: <_>::random_for_test(rng), latest_custody_reseed_slot: <_>::random_for_test(rng), penultimate_custody_reseed_slot: <_>::random_for_test(rng), @@ -179,7 +165,7 @@ mod tests { #[test] pub fn test_validator_status_ssz_round_trip() { let mut rng = XorShiftRng::from_seed([42; 16]); - let original = ValidatorStatus::random_for_test(&mut rng); + let original = StatusFlags::random_for_test(&mut rng); let bytes = ssz_encode(&original); let (decoded, _) = <_>::ssz_decode(&bytes, 0).unwrap(); From 16cc8556e83b2a1ec59836871fec849d3e502059 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 11:30:33 -0600 Subject: [PATCH 03/13] Covers the 'missing' `StatusFlag` when the value is `0`. Strongly types the field by using Option<_>. Updates the serialization and test generation to account for the type. --- beacon_chain/types/src/validator_record.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 05980559e..bc0b6b6b5 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -35,7 +35,7 @@ pub struct ValidatorRecord { pub withdrawal_slot: u64, pub penalized_slot: u64, pub exit_count: u64, - pub status_flags: StatusFlags, + pub status_flags: Option, pub custody_commitment: Hash256, pub latest_custody_reseed_slot: u64, pub penultimate_custody_reseed_slot: u64, @@ -81,7 +81,11 @@ impl Encodable for ValidatorRecord { s.append(&self.withdrawal_slot); s.append(&self.penalized_slot); s.append(&self.exit_count); - s.append(&self.status_flags); + if let Some(status_flags) = self.status_flags { + s.append(&status_flags); + } else { + s.append(&(0 as u8)); + } s.append(&self.custody_commitment); s.append(&self.latest_custody_reseed_slot); s.append(&self.penultimate_custody_reseed_slot); @@ -99,11 +103,17 @@ impl Decodable for ValidatorRecord { let (withdrawal_slot, i) = <_>::ssz_decode(bytes, i)?; let (penalized_slot, i) = <_>::ssz_decode(bytes, i)?; let (exit_count, i) = <_>::ssz_decode(bytes, i)?; - let (status_flags, i) = <_>::ssz_decode(bytes, i)?; + let (status_flags_byte, i): (u8, usize) = <_>::ssz_decode(bytes, i)?; let (custody_commitment, i) = <_>::ssz_decode(bytes, i)?; let (latest_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; let (penultimate_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; + let status_flags = if status_flags_byte == 0u8 { + None + } else { + Some(StatusFlags::from(status_flags_byte)) + }; + Ok(( Self { pubkey, @@ -137,7 +147,7 @@ impl TestRandom for ValidatorRecord { withdrawal_slot: <_>::random_for_test(rng), penalized_slot: <_>::random_for_test(rng), exit_count: <_>::random_for_test(rng), - status_flags: <_>::random_for_test(rng), + status_flags: Some(<_>::random_for_test(rng)), custody_commitment: <_>::random_for_test(rng), latest_custody_reseed_slot: <_>::random_for_test(rng), penultimate_custody_reseed_slot: <_>::random_for_test(rng), From 4d3d351b673562321059cd917b3b8db17c3e67cc Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 11:32:54 -0600 Subject: [PATCH 04/13] Adds a `std::default::Default` implementation for `ValidatorRecord` Updates the test generation so that it uses sane values for some marker values like `FAR_FUTURE_SLOT` --- beacon_chain/types/src/validator_record.rs | 26 ++++++++++++++++++---- beacon_chain/utils/bls/src/public_key.rs | 8 +++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index bc0b6b6b5..7077d87e2 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -4,6 +4,7 @@ use crate::test_utils::TestRandom; use rand::RngCore; use ssz::{Decodable, DecodeError, Encodable, SszStream}; use std::convert; +use std::default; const STATUS_FLAG_INITIATED_EXIT: u8 = 1; const STATUS_FLAG_WITHDRAWABLE: u8 = 2; @@ -41,6 +42,26 @@ pub struct ValidatorRecord { pub penultimate_custody_reseed_slot: u64, } +impl default::Default for ValidatorRecord { + fn default() -> Self { + Self { + pubkey: PublicKey::default(), + withdrawal_credentials: Hash256::default(), + randao_commitment: Hash256::default(), + randao_layers: 0, + activation_slot: std::u64::MAX, + exit_slot: std::u64::MAX, + withdrawal_slot: std::u64::MAX, + penalized_slot: std::u64::MAX, + exit_count: 0, + status_flags: None, + custody_commitment: Hash256::default(), + latest_custody_reseed_slot: 0, // NOTE: is `GENESIS_SLOT` + penultimate_custody_reseed_slot: 0, // NOTE: is `GENESIS_SLOT` + } + } +} + impl Encodable for StatusFlags { fn ssz_append(&self, s: &mut SszStream) { let byte: u8 = match self { @@ -142,15 +163,12 @@ impl TestRandom for ValidatorRecord { withdrawal_credentials: <_>::random_for_test(rng), randao_commitment: <_>::random_for_test(rng), randao_layers: <_>::random_for_test(rng), - activation_slot: <_>::random_for_test(rng), - exit_slot: <_>::random_for_test(rng), - withdrawal_slot: <_>::random_for_test(rng), - penalized_slot: <_>::random_for_test(rng), exit_count: <_>::random_for_test(rng), status_flags: Some(<_>::random_for_test(rng)), custody_commitment: <_>::random_for_test(rng), latest_custody_reseed_slot: <_>::random_for_test(rng), penultimate_custody_reseed_slot: <_>::random_for_test(rng), + ..Self::default() } } } diff --git a/beacon_chain/utils/bls/src/public_key.rs b/beacon_chain/utils/bls/src/public_key.rs index 49dbc9e4b..e7950969e 100644 --- a/beacon_chain/utils/bls/src/public_key.rs +++ b/beacon_chain/utils/bls/src/public_key.rs @@ -1,6 +1,7 @@ use super::SecretKey; use bls_aggregates::PublicKey as RawPublicKey; use ssz::{decode_ssz_list, Decodable, DecodeError, Encodable, SszStream}; +use std::default; /// A single BLS signature. /// @@ -20,6 +21,13 @@ impl PublicKey { } } +impl default::Default for PublicKey { + fn default() -> Self { + let secret_key = SecretKey::random(); + PublicKey::from_secret_key(&secret_key) + } +} + impl Encodable for PublicKey { fn ssz_append(&self, s: &mut SszStream) { s.append_vec(&self.0.as_bytes()); From 42d950eb88f27605c505710a1ecea213ac8989af Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 11:33:45 -0600 Subject: [PATCH 05/13] Implements `is_active_validator` helper --- beacon_chain/types/src/validator_record.rs | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 7077d87e2..0e1102a59 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -42,6 +42,12 @@ pub struct ValidatorRecord { pub penultimate_custody_reseed_slot: u64, } +impl ValidatorRecord { + pub fn is_active_at(&self, slot: u64) -> bool { + self.activation_slot <= slot && slot < self.exit_slot + } +} + impl default::Default for ValidatorRecord { fn default() -> Self { Self { @@ -200,4 +206,26 @@ mod tests { assert_eq!(original, decoded); } + + #[test] + fn test_validator_can_be_active() { + let mut rng = XorShiftRng::from_seed([42; 16]); + let mut validator = ValidatorRecord::random_for_test(&mut rng); + + let activation_slot = u64::random_for_test(&mut rng); + let exit_slot = activation_slot + 234; + + validator.activation_slot = activation_slot; + validator.exit_slot = exit_slot; + + for slot in (activation_slot - 100)..(exit_slot + 100) { + if slot < activation_slot { + assert!(!validator.is_active_at(slot)); + } else if slot >= exit_slot { + assert!(!validator.is_active_at(slot)); + } else { + assert!(validator.is_active_at(slot)); + } + } + } } From 9720c548afea1a3b004569f4e3c5cecd4cf2b039 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 12:56:13 -0600 Subject: [PATCH 06/13] Adds `get_active_validator_indices` helper --- beacon_chain/types/src/lib.rs | 1 + beacon_chain/types/src/validator_registry.rs | 170 +++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 beacon_chain/types/src/validator_registry.rs diff --git a/beacon_chain/types/src/lib.rs b/beacon_chain/types/src/lib.rs index f6847356a..b89cbc50f 100644 --- a/beacon_chain/types/src/lib.rs +++ b/beacon_chain/types/src/lib.rs @@ -26,6 +26,7 @@ pub mod shard_reassignment_record; pub mod slashable_vote_data; pub mod special_record; pub mod validator_record; +pub mod validator_registry; pub mod readers; diff --git a/beacon_chain/types/src/validator_registry.rs b/beacon_chain/types/src/validator_registry.rs new file mode 100644 index 000000000..1171f7acf --- /dev/null +++ b/beacon_chain/types/src/validator_registry.rs @@ -0,0 +1,170 @@ +/// Contains logic to manipulate a `&[ValidatorRecord]`. +/// For now, we avoid defining a newtype and just have flat functions here. +use super::validator_record::*; + +pub fn get_active_validator_indices(validators: &[ValidatorRecord], slot: u64) -> Vec { + validators + .iter() + .enumerate() + .filter_map(|(index, validator)| { + if validator.is_active_at(slot) { + Some(index) + } else { + None + } + }) + .collect::>() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::{SeedableRng, TestRandom, XorShiftRng}; + + #[test] + fn can_get_empty_active_validator_indices() { + let mut rng = XorShiftRng::from_seed([42; 16]); + + let validators = vec![]; + let some_slot = u64::random_for_test(&mut rng); + let indices = get_active_validator_indices(&validators, some_slot); + assert_eq!(indices, vec![]); + } + + #[test] + fn can_get_no_active_validator_indices() { + let mut rng = XorShiftRng::from_seed([42; 16]); + let mut validators = vec![]; + let count_validators = 10; + for _ in 0..count_validators { + validators.push(ValidatorRecord::default()) + } + + let some_slot = u64::random_for_test(&mut rng); + let indices = get_active_validator_indices(&validators, some_slot); + assert_eq!(indices, vec![]); + } + + #[test] + fn can_get_all_active_validator_indices() { + let mut rng = XorShiftRng::from_seed([42; 16]); + let count_validators = 10; + let some_slot = u64::random_for_test(&mut rng); + + let mut validators = (0..count_validators) + .into_iter() + .map(|_| { + let mut validator = ValidatorRecord::default(); + + let activation_offset = u64::random_for_test(&mut rng); + let exit_offset = u64::random_for_test(&mut rng); + + validator.activation_slot = some_slot.checked_sub(activation_offset).unwrap_or(0); + validator.exit_slot = some_slot.checked_add(exit_offset).unwrap_or(std::u64::MAX); + + validator + }) + .collect::>(); + + // test boundary condition by ensuring that at least one validator in the list just activated + if let Some(validator) = validators.get_mut(0) { + validator.activation_slot = some_slot; + } + + let indices = get_active_validator_indices(&validators, some_slot); + assert_eq!( + indices, + (0..count_validators).into_iter().collect::>() + ); + } + + fn set_validators_to_default_entry_exit(validators: &mut [ValidatorRecord]) { + for validator in validators.iter_mut() { + validator.activation_slot = std::u64::MAX; + validator.exit_slot = std::u64::MAX; + } + } + + // sets all `validators` to be active as of some slot prior to `slot`. returns the activation slot. + fn set_validators_to_activated(validators: &mut [ValidatorRecord], slot: u64) -> u64 { + let activation_slot = slot - 10; + for validator in validators.iter_mut() { + validator.activation_slot = activation_slot; + } + activation_slot + } + + // sets all `validators` to be exited as of some slot before `slot`. + fn set_validators_to_exited( + validators: &mut [ValidatorRecord], + slot: u64, + activation_slot: u64, + ) { + assert!(activation_slot < slot); + let mut exit_slot = activation_slot + 10; + while exit_slot >= slot { + exit_slot -= 1; + } + assert!(activation_slot < exit_slot && exit_slot < slot); + + for validator in validators.iter_mut() { + validator.exit_slot = exit_slot; + } + } + + #[test] + fn can_get_some_active_validator_indices() { + let mut rng = XorShiftRng::from_seed([42; 16]); + const COUNT_PARTITIONS: usize = 3; + const COUNT_VALIDATORS: usize = 3 * COUNT_PARTITIONS; + let some_slot: u64 = u64::random_for_test(&mut rng); + + let mut validators = (0..COUNT_VALIDATORS) + .into_iter() + .map(|_| { + let mut validator = ValidatorRecord::default(); + + let activation_offset = u64::random_for_test(&mut rng); + let exit_offset = u64::random_for_test(&mut rng); + + validator.activation_slot = some_slot.checked_sub(activation_offset).unwrap_or(0); + validator.exit_slot = some_slot.checked_add(exit_offset).unwrap_or(std::u64::MAX); + + validator + }) + .collect::>(); + + // we partition the set into partitions based on lifecycle: + for (i, chunk) in validators.chunks_exact_mut(COUNT_PARTITIONS).enumerate() { + match i { + 0 => { + // 1. not activated (Default::default()) + set_validators_to_default_entry_exit(chunk); + } + 1 => { + // 2. activated, but not exited + set_validators_to_activated(chunk, some_slot); + // test boundary condition by ensuring that at least one validator in the list just activated + if let Some(validator) = chunk.get_mut(0) { + validator.activation_slot = some_slot; + } + } + 2 => { + // 3. exited + let activation_slot = set_validators_to_activated(chunk, some_slot); + set_validators_to_exited(chunk, some_slot, activation_slot); + // test boundary condition by ensuring that at least one validator in the list just exited + if let Some(validator) = chunk.get_mut(0) { + validator.exit_slot = some_slot; + } + } + _ => unreachable!( + "constants local to this test not in sync with generation of test case" + ), + } + } + + let indices = get_active_validator_indices(&validators, some_slot); + assert_eq!(indices, vec![3, 4, 5]); + } +} From 869f36ffbec78233a91f501ae3d03eedf8a3c71a Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 14:46:56 -0600 Subject: [PATCH 07/13] Use the `Default::default` validator for testing --- beacon_chain/spec/src/foundation.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/beacon_chain/spec/src/foundation.rs b/beacon_chain/spec/src/foundation.rs index 1a493d72c..de433ea6b 100644 --- a/beacon_chain/spec/src/foundation.rs +++ b/beacon_chain/spec/src/foundation.rs @@ -101,15 +101,7 @@ fn initial_validators_for_testing() -> Vec { }; let validator_record = ValidatorRecord { pubkey: keypair.pk.clone(), - withdrawal_credentials: Hash256::zero(), - randao_commitment: Hash256::zero(), - randao_layers: 0, - status: From::from(0), - latest_status_change_slot: 0, - exit_count: 0, - custody_commitment: Hash256::zero(), - latest_custody_reseed_slot: 0, - penultimate_custody_reseed_slot: 0, + ..std::default::Default::default() }; initial_validators.push(validator_record); } From 2d1b61522b8e66461d61c1f6c6764d78f0cc44ef Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 14:47:25 -0600 Subject: [PATCH 08/13] Uses the new `get_active_validator_indices` implementation --- beacon_chain/validator_shuffling/src/shuffle.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/beacon_chain/validator_shuffling/src/shuffle.rs b/beacon_chain/validator_shuffling/src/shuffle.rs index 5b31f8e03..13bdf97fe 100644 --- a/beacon_chain/validator_shuffling/src/shuffle.rs +++ b/beacon_chain/validator_shuffling/src/shuffle.rs @@ -2,7 +2,8 @@ use std::cmp::min; use honey_badger_split::SplitExt; use spec::ChainSpec; -use types::{ShardCommittee, ValidatorRecord, ValidatorStatus}; +use types::validator_registry::get_active_validator_indices; +use types::{ShardCommittee, ValidatorRecord}; use vec_shuffle::{shuffle, ShuffleErr}; type DelegatedCycle = Vec>; @@ -24,17 +25,7 @@ pub fn shard_and_committees_for_cycle( spec: &ChainSpec, ) -> Result { let shuffled_validator_indices = { - let validator_indices = validators - .iter() - .enumerate() - .filter_map(|(i, validator)| { - if validator.status_is(ValidatorStatus::Active) { - Some(i) - } else { - None - } - }) - .collect(); + let validator_indices = get_active_validator_indices(validators, 0); shuffle(seed, validator_indices)? }; let shard_indices: Vec = (0_usize..spec.shard_count as usize).into_iter().collect(); From b09d44c235f1e65aa630345e211f823409bc06fa Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 10 Jan 2019 14:49:06 -0600 Subject: [PATCH 09/13] Gets this module to compile, at expense of deviating from old spec Some changes to integrate with the newer `ValidatorRecord` type. Deviates from the old spec but should be updated shortly with newer logic so the breaking change is only temporary. --- .../validator_induction/src/inductor.rs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/beacon_chain/validator_induction/src/inductor.rs b/beacon_chain/validator_induction/src/inductor.rs index f00c9ec55..4ac9d7924 100644 --- a/beacon_chain/validator_induction/src/inductor.rs +++ b/beacon_chain/validator_induction/src/inductor.rs @@ -1,6 +1,6 @@ use bls::verify_proof_of_possession; use spec::ChainSpec; -use types::{BeaconState, Deposit, ValidatorRecord, ValidatorStatus}; +use types::{BeaconState, Deposit, ValidatorRecord}; #[derive(Debug, PartialEq, Clone)] pub enum ValidatorInductionError { @@ -43,13 +43,10 @@ pub fn process_deposit( pubkey: deposit_input.pubkey.clone(), withdrawal_credentials: deposit_input.withdrawal_credentials, randao_commitment: deposit_input.randao_commitment, - randao_layers: 0, - status: ValidatorStatus::PendingActivation, - latest_status_change_slot: state.validator_registry_latest_change_slot, - exit_count: 0, custody_commitment: deposit_input.custody_commitment, latest_custody_reseed_slot: 0, penultimate_custody_reseed_slot: 0, + ..std::default::Default::default() }; match min_empty_validator_index(state, spec) { @@ -68,13 +65,11 @@ pub fn process_deposit( } } -fn min_empty_validator_index(state: &BeaconState, spec: &ChainSpec) -> Option { +// NOTE: this has been modified from the spec to get tests working +// this function is no longer used in the latest spec so this is simply a transition step +fn min_empty_validator_index(state: &BeaconState, _spec: &ChainSpec) -> Option { for i in 0..state.validator_registry.len() { - if state.validator_balances[i] == 0 - && state.validator_registry[i].latest_status_change_slot - + spec.zero_balance_validator_ttl - <= state.slot - { + if state.validator_balances[i] == 0 { return Some(i); } } @@ -187,8 +182,7 @@ mod tests { let mut state = BeaconState::default(); let spec = ChainSpec::foundation(); - let mut validator = get_validator(); - validator.latest_status_change_slot = 0; + let validator = get_validator(); state.validator_registry.push(validator); state.validator_balances.push(0); From c9a9f0e305f8d1fb4e488225335d130bc038496b Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 15 Jan 2019 17:03:45 -0800 Subject: [PATCH 10/13] Remove imports already in the prelude --- beacon_chain/types/src/validator_record.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 0e1102a59..4f2ada4ad 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -3,8 +3,6 @@ use super::Hash256; use crate::test_utils::TestRandom; use rand::RngCore; use ssz::{Decodable, DecodeError, Encodable, SszStream}; -use std::convert; -use std::default; const STATUS_FLAG_INITIATED_EXIT: u8 = 1; const STATUS_FLAG_WITHDRAWABLE: u8 = 2; @@ -15,7 +13,7 @@ pub enum StatusFlags { Withdrawable, } -impl convert::From for StatusFlags { +impl From for StatusFlags { fn from(status_flag: u8) -> Self { match status_flag { STATUS_FLAG_INITIATED_EXIT => StatusFlags::InitiatedExit, @@ -48,7 +46,7 @@ impl ValidatorRecord { } } -impl default::Default for ValidatorRecord { +impl Default for ValidatorRecord { fn default() -> Self { Self { pubkey: PublicKey::default(), From 1f32da81e733a117c122cc77a16d11b8f4284885 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 15 Jan 2019 17:23:29 -0800 Subject: [PATCH 11/13] Simplify how the `StatusFlags` are handled wrt serialization --- beacon_chain/types/src/validator_record.rs | 75 ++++++++-------------- 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index 4f2ada4ad..e63e3ea2c 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -13,13 +13,31 @@ pub enum StatusFlags { Withdrawable, } -impl From for StatusFlags { - fn from(status_flag: u8) -> Self { - match status_flag { - STATUS_FLAG_INITIATED_EXIT => StatusFlags::InitiatedExit, - STATUS_FLAG_WITHDRAWABLE => StatusFlags::Withdrawable, - _ => unreachable!(), +struct StatusFlagsDecodeError; + +impl From for DecodeError { + fn from(_: StatusFlagsDecodeError) -> DecodeError { + DecodeError::Invalid + } +} + +fn status_flag_to_byte(flag: Option) -> u8 { + if let Some(flag) = flag { + match flag { + StatusFlags::InitiatedExit => STATUS_FLAG_INITIATED_EXIT, + StatusFlags::Withdrawable => STATUS_FLAG_WITHDRAWABLE, } + } else { + 0 + } +} + +fn status_flag_from_byte(flag: u8) -> Result, StatusFlagsDecodeError> { + match flag { + 0 => Ok(None), + 1 => Ok(Some(StatusFlags::InitiatedExit)), + 2 => Ok(Some(StatusFlags::Withdrawable)), + _ => Err(StatusFlagsDecodeError), } } @@ -66,28 +84,6 @@ impl Default for ValidatorRecord { } } -impl Encodable for StatusFlags { - fn ssz_append(&self, s: &mut SszStream) { - let byte: u8 = match self { - StatusFlags::InitiatedExit => STATUS_FLAG_INITIATED_EXIT, - StatusFlags::Withdrawable => STATUS_FLAG_WITHDRAWABLE, - }; - s.append(&byte); - } -} - -impl Decodable for StatusFlags { - fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { - let (byte, i) = u8::ssz_decode(bytes, i)?; - let status = match byte { - 1 => StatusFlags::InitiatedExit, - 2 => StatusFlags::Withdrawable, - _ => return Err(DecodeError::Invalid), - }; - Ok((status, i)) - } -} - impl TestRandom for StatusFlags { fn random_for_test(rng: &mut T) -> Self { let options = vec![StatusFlags::InitiatedExit, StatusFlags::Withdrawable]; @@ -106,11 +102,7 @@ impl Encodable for ValidatorRecord { s.append(&self.withdrawal_slot); s.append(&self.penalized_slot); s.append(&self.exit_count); - if let Some(status_flags) = self.status_flags { - s.append(&status_flags); - } else { - s.append(&(0 as u8)); - } + s.append(&status_flag_to_byte(self.status_flags)); s.append(&self.custody_commitment); s.append(&self.latest_custody_reseed_slot); s.append(&self.penultimate_custody_reseed_slot); @@ -133,11 +125,7 @@ impl Decodable for ValidatorRecord { let (latest_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; let (penultimate_custody_reseed_slot, i) = <_>::ssz_decode(bytes, i)?; - let status_flags = if status_flags_byte == 0u8 { - None - } else { - Some(StatusFlags::from(status_flags_byte)) - }; + let status_flags = status_flag_from_byte(status_flags_byte)?; Ok(( Self { @@ -194,17 +182,6 @@ mod tests { assert_eq!(original, decoded); } - #[test] - pub fn test_validator_status_ssz_round_trip() { - let mut rng = XorShiftRng::from_seed([42; 16]); - let original = StatusFlags::random_for_test(&mut rng); - - let bytes = ssz_encode(&original); - let (decoded, _) = <_>::ssz_decode(&bytes, 0).unwrap(); - - assert_eq!(original, decoded); - } - #[test] fn test_validator_can_be_active() { let mut rng = XorShiftRng::from_seed([42; 16]); From 01a20acb12c9ca682aedd736bbc86f23750ce606 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 15 Jan 2019 18:19:26 -0800 Subject: [PATCH 12/13] Remove import already in prelude --- beacon_chain/spec/src/foundation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/spec/src/foundation.rs b/beacon_chain/spec/src/foundation.rs index de433ea6b..315be168e 100644 --- a/beacon_chain/spec/src/foundation.rs +++ b/beacon_chain/spec/src/foundation.rs @@ -101,7 +101,7 @@ fn initial_validators_for_testing() -> Vec { }; let validator_record = ValidatorRecord { pubkey: keypair.pk.clone(), - ..std::default::Default::default() + ..Default::default() }; initial_validators.push(validator_record); } From 306bcd6f8d235d637ef038f593f98014ccec20c4 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 15 Jan 2019 21:20:09 -0800 Subject: [PATCH 13/13] Add some clarifying doc comments --- beacon_chain/types/src/validator_record.rs | 4 ++++ beacon_chain/types/src/validator_registry.rs | 1 + 2 files changed, 5 insertions(+) diff --git a/beacon_chain/types/src/validator_record.rs b/beacon_chain/types/src/validator_record.rs index e63e3ea2c..df0ce4d4c 100644 --- a/beacon_chain/types/src/validator_record.rs +++ b/beacon_chain/types/src/validator_record.rs @@ -21,6 +21,7 @@ impl From for DecodeError { } } +/// Handles the serialization logic for the `status_flags` field of the `ValidatorRecord`. fn status_flag_to_byte(flag: Option) -> u8 { if let Some(flag) = flag { match flag { @@ -32,6 +33,7 @@ fn status_flag_to_byte(flag: Option) -> u8 { } } +/// Handles the deserialization logic for the `status_flags` field of the `ValidatorRecord`. fn status_flag_from_byte(flag: u8) -> Result, StatusFlagsDecodeError> { match flag { 0 => Ok(None), @@ -59,12 +61,14 @@ pub struct ValidatorRecord { } impl ValidatorRecord { + /// This predicate indicates if the validator represented by this record is considered "active" at `slot`. pub fn is_active_at(&self, slot: u64) -> bool { self.activation_slot <= slot && slot < self.exit_slot } } impl Default for ValidatorRecord { + /// Yields a "default" `ValidatorRecord`. Primarily used for testing. fn default() -> Self { Self { pubkey: PublicKey::default(), diff --git a/beacon_chain/types/src/validator_registry.rs b/beacon_chain/types/src/validator_registry.rs index 1171f7acf..abb2e6b3f 100644 --- a/beacon_chain/types/src/validator_registry.rs +++ b/beacon_chain/types/src/validator_registry.rs @@ -2,6 +2,7 @@ /// For now, we avoid defining a newtype and just have flat functions here. use super::validator_record::*; +/// Given an indexed sequence of `validators`, return the indices corresponding to validators that are active at `slot`. pub fn get_active_validator_indices(validators: &[ValidatorRecord], slot: u64) -> Vec { validators .iter()