From 2d2ba6576b5b916449fb17f1e29d49764f690ac2 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 11:24:46 +1100 Subject: [PATCH 01/10] Remove old, superseded benches --- .../beacon_chain/test_harness/Cargo.toml | 5 -- .../test_harness/benches/state_transition.rs | 69 ------------------- 2 files changed, 74 deletions(-) delete mode 100644 beacon_node/beacon_chain/test_harness/benches/state_transition.rs diff --git a/beacon_node/beacon_chain/test_harness/Cargo.toml b/beacon_node/beacon_chain/test_harness/Cargo.toml index 448934eb3..50d154732 100644 --- a/beacon_node/beacon_chain/test_harness/Cargo.toml +++ b/beacon_node/beacon_chain/test_harness/Cargo.toml @@ -12,12 +12,7 @@ path = "src/bin.rs" name = "test_harness" path = "src/lib.rs" -[[bench]] -name = "state_transition" -harness = false - [dev-dependencies] -criterion = "0.2" state_processing = { path = "../../../eth2/state_processing" } [dependencies] diff --git a/beacon_node/beacon_chain/test_harness/benches/state_transition.rs b/beacon_node/beacon_chain/test_harness/benches/state_transition.rs deleted file mode 100644 index 7d1c44653..000000000 --- a/beacon_node/beacon_chain/test_harness/benches/state_transition.rs +++ /dev/null @@ -1,69 +0,0 @@ -use criterion::Criterion; -use criterion::{black_box, criterion_group, criterion_main, Benchmark}; -// use env_logger::{Builder, Env}; -use state_processing::SlotProcessable; -use test_harness::BeaconChainHarness; -use types::{ChainSpec, Hash256}; - -fn mid_epoch_state_transition(c: &mut Criterion) { - // Builder::from_env(Env::default().default_filter_or("debug")).init(); - - let validator_count = 1000; - let mut rig = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); - - let epoch_depth = (rig.spec.slots_per_epoch * 2) + (rig.spec.slots_per_epoch / 2); - - for _ in 0..epoch_depth { - rig.advance_chain_with_block(); - } - - let state = rig.beacon_chain.state.read().clone(); - - assert!((state.slot + 1) % rig.spec.slots_per_epoch != 0); - - c.bench_function("mid-epoch state transition 10k validators", move |b| { - let state = state.clone(); - b.iter(|| { - let mut state = state.clone(); - black_box(state.per_slot_processing(Hash256::zero(), &rig.spec)) - }) - }); -} - -fn epoch_boundary_state_transition(c: &mut Criterion) { - // Builder::from_env(Env::default().default_filter_or("debug")).init(); - - let validator_count = 10000; - let mut rig = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); - - let epoch_depth = rig.spec.slots_per_epoch * 2; - - for _ in 0..(epoch_depth - 1) { - rig.advance_chain_with_block(); - } - - let state = rig.beacon_chain.state.read().clone(); - - assert_eq!((state.slot + 1) % rig.spec.slots_per_epoch, 0); - - c.bench( - "routines", - Benchmark::new("routine_1", move |b| { - let state = state.clone(); - b.iter(|| { - let mut state = state.clone(); - black_box(black_box( - state.per_slot_processing(Hash256::zero(), &rig.spec), - )) - }) - }) - .sample_size(5), // sample size is low because function is sloooow. - ); -} - -criterion_group!( - benches, - mid_epoch_state_transition, - epoch_boundary_state_transition -); -criterion_main!(benches); From 6c4e457c8adf69266fe9088d3ed470302d204705 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 11:25:17 +1100 Subject: [PATCH 02/10] Fix test_harness tests They were broken by changes to TestingBeaconStateBuilder and where the keypairs file is stored. --- .../test_harness/src/beacon_chain_harness.rs | 90 ++----------------- .../beacon_chain/test_harness/src/run_test.rs | 7 +- .../test_harness/src/test_case.rs | 4 +- .../beacon_chain/test_harness/tests/chain.rs | 4 +- 4 files changed, 10 insertions(+), 95 deletions(-) diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs index c442c05db..28723a203 100644 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs +++ b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs @@ -1,7 +1,6 @@ use super::ValidatorHarness; use beacon_chain::{BeaconChain, BlockProcessingOutcome}; pub use beacon_chain::{BeaconChainError, CheckPoint}; -use bls::get_withdrawal_credentials; use db::{ stores::{BeaconBlockStore, BeaconStateStore}, MemoryDB, @@ -12,11 +11,10 @@ use rayon::prelude::*; use slot_clock::TestingSlotClock; use ssz::TreeHash; use std::collections::HashSet; -use std::fs::File; use std::iter::FromIterator; use std::path::Path; use std::sync::Arc; -use types::{beacon_state::BeaconStateBuilder, test_utils::generate_deterministic_keypairs, *}; +use types::{test_utils::TestingBeaconStateBuilder, *}; mod generate_deposits; @@ -42,95 +40,17 @@ impl BeaconChainHarness { /// /// - A keypair, `BlockProducer` and `Attester` for each validator. /// - A new BeaconChain struct where the given validators are in the genesis. - pub fn new( - spec: ChainSpec, - validator_count: usize, - validators_dir: Option<&Path>, - skip_deposit_verification: bool, - ) -> Self { + pub fn new(spec: ChainSpec, validator_count: usize) -> Self { let db = Arc::new(MemoryDB::open()); let block_store = Arc::new(BeaconBlockStore::new(db.clone())); let state_store = Arc::new(BeaconStateStore::new(db.clone())); - let genesis_time = 1_549_935_547; // 12th Feb 2018 (arbitrary value in the past). let slot_clock = TestingSlotClock::new(spec.genesis_slot.as_u64()); let fork_choice = BitwiseLMDGhost::new(block_store.clone(), state_store.clone()); - let latest_eth1_data = Eth1Data { - deposit_root: Hash256::zero(), - block_hash: Hash256::zero(), - }; - let mut state_builder = BeaconStateBuilder::new(genesis_time, latest_eth1_data, &spec); + let state_builder = + TestingBeaconStateBuilder::from_default_keypairs_file_if_exists(validator_count, &spec); + let (genesis_state, keypairs) = state_builder.build(); - // If a `validators_dir` is specified, load the keypairs a YAML file. - // - // Otherwise, generate them deterministically where the first validator has a secret key of - // `1`, etc. - let keypairs = if let Some(path) = validators_dir { - debug!("Loading validator keypairs from file..."); - let keypairs_file = File::open(path.join("keypairs.yaml")).unwrap(); - let mut keypairs: Vec = serde_yaml::from_reader(&keypairs_file).unwrap(); - keypairs.truncate(validator_count); - keypairs - } else { - debug!("Generating validator keypairs..."); - generate_deterministic_keypairs(validator_count) - }; - - // Skipping deposit verification means directly generating `Validator` records, instead - // of generating `Deposit` objects, verifying them and converting them into `Validator` - // records. - // - // It is much faster to skip deposit verification, however it does not test the initial - // validator induction part of beacon chain genesis. - if skip_deposit_verification { - let validators = keypairs - .iter() - .map(|keypair| { - let withdrawal_credentials = Hash256::from_slice(&get_withdrawal_credentials( - &keypair.pk, - spec.bls_withdrawal_prefix_byte, - )); - - Validator { - pubkey: keypair.pk.clone(), - withdrawal_credentials, - activation_epoch: spec.far_future_epoch, - exit_epoch: spec.far_future_epoch, - withdrawable_epoch: spec.far_future_epoch, - initiated_exit: false, - slashed: false, - } - }) - .collect(); - - let balances = vec![32_000_000_000; validator_count]; - - state_builder.import_existing_validators( - validators, - balances, - validator_count as u64, - &spec, - ); - } else { - debug!("Generating initial validator deposits..."); - let deposits = generate_deposits_from_keypairs( - &keypairs, - genesis_time, - spec.get_domain( - spec.genesis_epoch, - Domain::Deposit, - &Fork { - previous_version: spec.genesis_fork_version, - current_version: spec.genesis_fork_version, - epoch: spec.genesis_epoch, - }, - ), - &spec, - ); - state_builder.process_initial_deposits(&deposits, &spec); - }; - - let genesis_state = state_builder.build(&spec).unwrap(); let state_root = Hash256::from_slice(&genesis_state.hash_tree_root()); let genesis_block = BeaconBlock::genesis(state_root, &spec); diff --git a/beacon_node/beacon_chain/test_harness/src/run_test.rs b/beacon_node/beacon_chain/test_harness/src/run_test.rs index d4e2e1cf2..4caa299d6 100644 --- a/beacon_node/beacon_chain/test_harness/src/run_test.rs +++ b/beacon_node/beacon_chain/test_harness/src/run_test.rs @@ -1,6 +1,5 @@ use crate::test_case::TestCase; use clap::ArgMatches; -use std::path::Path; use std::{fs::File, io::prelude::*}; use yaml_rust::YamlLoader; @@ -17,10 +16,6 @@ pub fn run_test(matches: &ArgMatches) { }; for doc in &docs { - let validators_dir = matches - .value_of("validators_dir") - .and_then(|dir_str| Some(Path::new(dir_str))); - // For each `test_cases` YAML in the document, build a `TestCase`, execute it and // assert that the execution result matches the test_case description. // @@ -35,7 +30,7 @@ pub fn run_test(matches: &ArgMatches) { // panics with a message. for test_case in doc["test_cases"].as_vec().unwrap() { let test_case = TestCase::from_yaml(test_case); - test_case.assert_result_valid(test_case.execute(validators_dir)) + test_case.assert_result_valid(test_case.execute()) } } } diff --git a/beacon_node/beacon_chain/test_harness/src/test_case.rs b/beacon_node/beacon_chain/test_harness/src/test_case.rs index b6b1ea5cc..cee78f6c4 100644 --- a/beacon_node/beacon_chain/test_harness/src/test_case.rs +++ b/beacon_node/beacon_chain/test_harness/src/test_case.rs @@ -69,7 +69,7 @@ impl TestCase { /// Executes the test case, returning an `ExecutionResult`. #[allow(clippy::cyclomatic_complexity)] - pub fn execute(&self, validators_dir: Option<&Path>) -> ExecutionResult { + pub fn execute(&self) -> ExecutionResult { let spec = self.spec(); let validator_count = self.config.deposits_for_chain_start; let slots = self.config.num_slots; @@ -79,7 +79,7 @@ impl TestCase { validator_count ); - let mut harness = BeaconChainHarness::new(spec, validator_count, validators_dir, true); + let mut harness = BeaconChainHarness::new(spec, validator_count); info!("Starting simulation across {} slots...", slots); diff --git a/beacon_node/beacon_chain/test_harness/tests/chain.rs b/beacon_node/beacon_chain/test_harness/tests/chain.rs index e5a52a314..e72c3a5aa 100644 --- a/beacon_node/beacon_chain/test_harness/tests/chain.rs +++ b/beacon_node/beacon_chain/test_harness/tests/chain.rs @@ -10,7 +10,7 @@ fn it_can_build_on_genesis_block() { let spec = ChainSpec::few_validators(); let validator_count = 8; - let mut harness = BeaconChainHarness::new(spec, validator_count as usize, None, true); + let mut harness = BeaconChainHarness::new(spec, validator_count as usize); harness.advance_chain_with_block(); } @@ -25,7 +25,7 @@ fn it_can_produce_past_first_epoch_boundary() { debug!("Starting harness build..."); - let mut harness = BeaconChainHarness::new(spec, validator_count, None, true); + let mut harness = BeaconChainHarness::new(spec, validator_count); debug!("Harness built, tests starting.."); From bfa2e71b468e34eedcf847076aa29f3a53038bfd Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 14:41:43 +1100 Subject: [PATCH 03/10] Move `PublicKey` to store uncompressed bytes. This is an optimisation that allows for faster hashing of a public key, however it adds a penalty to SSZ encoding because we need to go decompressed -> PublicKey -> compressed. The spec presently uses compressed bytes to store public keys, however I'm hoping it will change. --- eth2/utils/bls/src/aggregate_public_key.rs | 2 +- eth2/utils/bls/src/public_key.rs | 56 ++++++++++++++++------ eth2/utils/bls/src/signature.rs | 4 +- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/eth2/utils/bls/src/aggregate_public_key.rs b/eth2/utils/bls/src/aggregate_public_key.rs index 2174a43cb..47c95d6c9 100644 --- a/eth2/utils/bls/src/aggregate_public_key.rs +++ b/eth2/utils/bls/src/aggregate_public_key.rs @@ -14,7 +14,7 @@ impl AggregatePublicKey { } pub fn add(&mut self, public_key: &PublicKey) { - self.0.add(public_key.as_raw()) + self.0.add(&public_key.as_raw()) } /// Returns the underlying signature. diff --git a/eth2/utils/bls/src/public_key.rs b/eth2/utils/bls/src/public_key.rs index c85760bbf..3e2bff19e 100644 --- a/eth2/utils/bls/src/public_key.rs +++ b/eth2/utils/bls/src/public_key.rs @@ -10,39 +10,58 @@ use ssz::{ use std::default; use std::hash::{Hash, Hasher}; -/// A single BLS signature. +/// A single BLS public key. +/// +/// This struct stores an uncompressed public key as a byte vec. The reason we store bytes instead +/// of the `RawPublicKey` struct is because it allows for building a hashmap of `PublicKey` much +/// faster. +/// +/// Storing as uncompressed bytes costs ~0.02% more time when adding a `PublicKey` to an +/// `AggregateKey`, however it saves ~0.5ms each time you need to add a pubkey to a hashmap. /// /// This struct is a wrapper upon a base type and provides helper functions (e.g., SSZ /// serialization). #[derive(Debug, Clone, Eq)] -pub struct PublicKey(RawPublicKey); +pub struct PublicKey { + bytes: Vec, +} impl PublicKey { pub fn from_secret_key(secret_key: &SecretKey) -> Self { - PublicKey(RawPublicKey::from_secret_key(secret_key.as_raw())) + let mut raw_key = RawPublicKey::from_secret_key(secret_key.as_raw()); + let uncompressed_bytes = raw_key.as_uncompressed_bytes(); + Self { + bytes: uncompressed_bytes, + } } /// Returns the underlying signature. - pub fn as_raw(&self) -> &RawPublicKey { - &self.0 + pub fn as_raw(&self) -> RawPublicKey { + RawPublicKey::from_uncompressed_bytes(&self.bytes).expect("PublicKey in invalid state") } /// Converts compressed bytes to PublicKey pub fn from_bytes(bytes: &[u8]) -> Result { - let pubkey = RawPublicKey::from_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; - Ok(PublicKey(pubkey)) + let mut pubkey = RawPublicKey::from_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; + Ok(Self { + bytes: pubkey.as_uncompressed_bytes(), + }) } /// Returns the PublicKey as (x, y) bytes pub fn as_uncompressed_bytes(&self) -> Vec { - RawPublicKey::as_uncompressed_bytes(&mut self.0.clone()) + self.bytes.clone() } /// Converts (x, y) bytes to PublicKey pub fn from_uncompressed_bytes(bytes: &[u8]) -> Result { - let pubkey = + // Do a conversion to check the bytes are valid. + let _pubkey = RawPublicKey::from_uncompressed_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; - Ok(PublicKey(pubkey)) + + Ok(Self { + bytes: bytes.to_vec(), + }) } /// Returns the last 6 bytes of the SSZ encoding of the public key, as a hex string. @@ -64,15 +83,22 @@ impl default::Default for PublicKey { impl Encodable for PublicKey { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.0.as_bytes()); + s.append_vec(&self.as_raw().as_bytes()); } } impl Decodable for PublicKey { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let raw_sig = RawPublicKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; - Ok((PublicKey(raw_sig), i)) + let mut raw_sig = + RawPublicKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; + + Ok(( + Self { + bytes: raw_sig.as_uncompressed_bytes(), + }, + i, + )) } } @@ -99,7 +125,7 @@ impl<'de> Deserialize<'de> for PublicKey { impl TreeHash for PublicKey { fn hash_tree_root(&self) -> Vec { - hash(&self.0.as_bytes()) + hash(&self.as_raw().as_bytes()) } } @@ -117,7 +143,7 @@ impl Hash for PublicKey { /// /// Use `ssz::Encode` to obtain the bytes required for consensus hashing. fn hash(&self, state: &mut H) { - self.as_uncompressed_bytes().hash(state) + self.bytes.hash(state) } } diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index 47598bc66..47e1dad2e 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -33,7 +33,7 @@ impl Signature { /// Verify the Signature against a PublicKey. pub fn verify(&self, msg: &[u8], domain: u64, pk: &PublicKey) -> bool { - self.0.verify(msg, domain, pk.as_raw()) + self.0.verify(msg, domain, &pk.as_raw()) } /// Verify the Signature against a PublicKey, where the message has already been hashed. @@ -44,7 +44,7 @@ impl Signature { pk: &PublicKey, ) -> bool { self.0 - .verify_hashed(x_real_hashed, x_imaginary_hashed, pk.as_raw()) + .verify_hashed(x_real_hashed, x_imaginary_hashed, &pk.as_raw()) } /// Returns the underlying signature. From 6cd3c4bd1a6531009a16a2a1894ff7c70636d5d8 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 16:40:28 +1100 Subject: [PATCH 04/10] Add a cache for public keys to BeaconState This allows for a fast lookup of "is this public key already in the validator registry". --- eth2/state_processing/benches/benches.rs | 5 +- .../src/per_block_processing.rs | 9 ++-- .../src/per_block_processing/errors.rs | 5 +- .../per_block_processing/verify_deposit.rs | 7 +-- eth2/types/src/beacon_state.rs | 46 +++++++++++++++++++ eth2/types/src/beacon_state/pubkey_cache.rs | 45 ++++++++++++++++++ eth2/types/src/lib.rs | 3 +- .../testing_beacon_state_builder.rs | 2 + 8 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 eth2/types/src/beacon_state/pubkey_cache.rs diff --git a/eth2/state_processing/benches/benches.rs b/eth2/state_processing/benches/benches.rs index ad8c4f714..4de97a298 100644 --- a/eth2/state_processing/benches/benches.rs +++ b/eth2/state_processing/benches/benches.rs @@ -1,14 +1,11 @@ -use criterion::Benchmark; use criterion::Criterion; use criterion::{criterion_group, criterion_main}; use env_logger::{Builder, Env}; -use types::test_utils::TestingBeaconStateBuilder; -use types::*; mod bench_block_processing; mod bench_epoch_processing; -pub const VALIDATOR_COUNT: usize = 300_032; +pub const VALIDATOR_COUNT: usize = 16_384; // `LOG_LEVEL == "debug"` gives logs, but they're very noisy and slow down benching. pub const LOG_LEVEL: &str = ""; diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 7b5aafa7f..13a47836b 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -373,19 +373,20 @@ pub fn process_deposits( .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))?; + // 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` 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))?; + get_existing_validator_index(state, deposit).map_err(|e| e.into_with_index(i))?; let deposit_data = &deposit.deposit_data; let deposit_input = &deposit.deposit_data.deposit_input; diff --git a/eth2/state_processing/src/per_block_processing/errors.rs b/eth2/state_processing/src/per_block_processing/errors.rs index a3e3ebad1..8366a6584 100644 --- a/eth2/state_processing/src/per_block_processing/errors.rs +++ b/eth2/state_processing/src/per_block_processing/errors.rs @@ -294,6 +294,8 @@ impl_into_with_index_without_beacon_error!( pub enum DepositValidationError { /// Validation completed successfully and the object is invalid. Invalid(DepositInvalid), + /// Encountered a `BeaconStateError` whilst attempting to determine validity. + BeaconStateError(BeaconStateError), } /// Describes why an object is invalid. @@ -313,7 +315,8 @@ pub enum DepositInvalid { BadMerkleProof, } -impl_into_with_index_without_beacon_error!(DepositValidationError, DepositInvalid); +impl_from_beacon_state_error!(DepositValidationError); +impl_into_with_index_with_beacon_error!(DepositValidationError, DepositInvalid); /* * `Exit` Validation 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 1aabbb973..aad38f616 100644 --- a/eth2/state_processing/src/per_block_processing/verify_deposit.rs +++ b/eth2/state_processing/src/per_block_processing/verify_deposit.rs @@ -72,11 +72,12 @@ pub fn build_public_key_hashmap(state: &BeaconState) -> PublicKeyValidatorIndexH 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)); + let validator_index = state + .get_validator_index(&deposit_input.pubkey)? + .and_then(|i| Some(i)); match validator_index { None => Ok(None), @@ -86,7 +87,7 @@ pub fn get_existing_validator_index( == state.validator_registry[index as usize].withdrawal_credentials, Invalid::BadWithdrawalCredentials ); - Ok(Some(index)) + Ok(Some(index as u64)) } } } diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 878d13b86..fc410fe64 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -5,6 +5,7 @@ use helpers::*; use honey_badger_split::SplitExt; use int_to_bytes::int_to_bytes32; use log::{debug, error, trace}; +use pubkey_cache::PubkeyCache; use rand::RngCore; use serde_derive::Serialize; use ssz::{hash, Decodable, DecodeError, Encodable, SignedRoot, SszStream, TreeHash}; @@ -16,6 +17,7 @@ pub use builder::BeaconStateBuilder; mod builder; mod epoch_cache; pub mod helpers; +mod pubkey_cache; mod tests; pub type Committee = Vec; @@ -52,6 +54,11 @@ pub enum Error { InsufficientAttestations, InsufficientCommittees, EpochCacheUninitialized(RelativeEpoch), + PubkeyCacheInconsistent, + PubkeyCacheIncomplete { + cache_len: usize, + registry_len: usize, + }, } macro_rules! safe_add_assign { @@ -108,6 +115,7 @@ pub struct BeaconState { // Caching (not in the spec) pub cache_index_offset: usize, pub caches: Vec, + pub pubkey_cache: PubkeyCache, } impl BeaconState { @@ -186,6 +194,7 @@ impl BeaconState { */ cache_index_offset: 0, caches: vec![EpochCache::empty(); CACHED_EPOCHS], + pubkey_cache: PubkeyCache::empty(), } } @@ -293,6 +302,41 @@ impl BeaconState { } } + /// Updates the pubkey cache, if required. + /// + /// Adds all `pubkeys` from the `validator_registry` which are not already in the cache. Will + /// never re-add a pubkey. + pub fn update_pubkey_cache(&mut self) -> Result<(), Error> { + for (i, validator) in self + .validator_registry + .iter() + .enumerate() + .skip(self.pubkey_cache.len()) + { + let success = self.pubkey_cache.insert(validator.pubkey.clone(), i); + if !success { + return Err(Error::PubkeyCacheInconsistent); + } + } + + Ok(()) + } + + /// 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: &PublicKey) -> Result, Error> { + if self.pubkey_cache.len() == self.validator_registry.len() { + Ok(self.pubkey_cache.get(pubkey)) + } else { + Err(Error::PubkeyCacheIncomplete { + cache_len: self.pubkey_cache.len(), + registry_len: self.validator_registry.len(), + }) + } + } + /// The epoch corresponding to `self.slot`. /// /// Spec v0.4.0 @@ -1188,6 +1232,7 @@ impl Decodable for BeaconState { deposit_index, cache_index_offset: 0, caches: vec![EpochCache::empty(); CACHED_EPOCHS], + pubkey_cache: PubkeyCache::empty(), }, i, )) @@ -1258,6 +1303,7 @@ impl TestRandom for BeaconState { deposit_index: <_>::random_for_test(rng), cache_index_offset: 0, caches: vec![EpochCache::empty(); CACHED_EPOCHS], + pubkey_cache: PubkeyCache::empty(), } } } diff --git a/eth2/types/src/beacon_state/pubkey_cache.rs b/eth2/types/src/beacon_state/pubkey_cache.rs new file mode 100644 index 000000000..c05147579 --- /dev/null +++ b/eth2/types/src/beacon_state/pubkey_cache.rs @@ -0,0 +1,45 @@ +use crate::*; +use serde_derive::Serialize; +use std::collections::HashMap; + +type ValidatorIndex = usize; + +#[derive(Debug, PartialEq, Clone, Default, Serialize)] +pub struct PubkeyCache { + map: HashMap, +} + +impl PubkeyCache { + /// Instantiates a new, empty cache. + pub fn empty() -> Self { + Self { + map: HashMap::new(), + } + } + + /// Returns the number of validator indices already in the map. + pub fn len(&self) -> ValidatorIndex { + self.map.len() + } + + /// Inserts a validator index into the map. + /// + /// The added index must equal the number of validators already added to the map. This ensures + /// that an index is never skipped. + pub fn insert(&mut self, pubkey: PublicKey, index: ValidatorIndex) -> bool { + if index == self.map.len() { + self.map.insert(pubkey, index); + true + } else { + false + } + } + + /// Inserts a validator index into the map. + /// + /// The added index must equal the number of validators already added to the map. This ensures + /// that an index is never skipped. + pub fn get(&self, pubkey: &PublicKey) -> Option { + self.map.get(pubkey).cloned() + } +} diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 76fcb43ed..3da6a497f 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -1,4 +1,4 @@ -pub mod test_utils; +//! Ethereum 2.0 types pub mod attestation; pub mod attestation_data; @@ -22,6 +22,7 @@ pub mod proposer_slashing; pub mod readers; pub mod shard_reassignment_record; pub mod slashable_attestation; +pub mod test_utils; pub mod transfer; pub mod voluntary_exit; #[macro_use] diff --git a/eth2/types/src/test_utils/testing_beacon_state_builder.rs b/eth2/types/src/test_utils/testing_beacon_state_builder.rs index b2cf28c8a..b9f3c63e0 100644 --- a/eth2/types/src/test_utils/testing_beacon_state_builder.rs +++ b/eth2/types/src/test_utils/testing_beacon_state_builder.rs @@ -144,6 +144,8 @@ impl TestingBeaconStateBuilder { state.build_epoch_cache(RelativeEpoch::Current, &spec)?; state.build_epoch_cache(RelativeEpoch::Next, &spec)?; + state.update_pubkey_cache()?; + Ok(()) } From 587be831b57528eb016b1c66c0e3eb8679c74f04 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 16:49:32 +1100 Subject: [PATCH 05/10] Add method for dropping pubkey cache. Add bench. --- .../benches/bench_block_processing.rs | 17 +++++++++++++++++ eth2/types/src/beacon_state.rs | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/eth2/state_processing/benches/bench_block_processing.rs b/eth2/state_processing/benches/bench_block_processing.rs index 031942473..128b1051b 100644 --- a/eth2/state_processing/benches/bench_block_processing.rs +++ b/eth2/state_processing/benches/bench_block_processing.rs @@ -426,6 +426,23 @@ fn bench_block_processing( .sample_size(10), ); + let mut state = initial_state.clone(); + state.drop_pubkey_cache(); + c.bench( + &format!("{}/block_processing", desc), + Benchmark::new("build_pubkey_cache", move |b| { + b.iter_batched( + || state.clone(), + |mut state| { + state.update_pubkey_cache().unwrap(); + state + }, + criterion::BatchSize::SmallInput, + ) + }) + .sample_size(10), + ); + let block = initial_block.clone(); c.bench( &format!("{}/block_processing", desc), diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index fc410fe64..a1dd8983c 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -322,6 +322,11 @@ impl BeaconState { Ok(()) } + /// Completely drops the `pubkey_cache`, replacing it with a new, empty cache. + pub fn drop_pubkey_cache(&mut self) { + self.pubkey_cache = PubkeyCache::empty() + } + /// If a validator pubkey exists in the validator registry, returns `Some(i)`, otherwise /// returns `None`. /// From b2fb2afb2813c4288a462c4a84bf6bf9ea7f7528 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 13 Mar 2019 16:51:37 +1100 Subject: [PATCH 06/10] Revert "Move `PublicKey` to store uncomp. bytes." This reverts commit bfa2e71b468e34eedcf847076aa29f3a53038bfd. --- eth2/utils/bls/src/aggregate_public_key.rs | 2 +- eth2/utils/bls/src/public_key.rs | 56 ++++++---------------- eth2/utils/bls/src/signature.rs | 4 +- 3 files changed, 18 insertions(+), 44 deletions(-) diff --git a/eth2/utils/bls/src/aggregate_public_key.rs b/eth2/utils/bls/src/aggregate_public_key.rs index 47c95d6c9..2174a43cb 100644 --- a/eth2/utils/bls/src/aggregate_public_key.rs +++ b/eth2/utils/bls/src/aggregate_public_key.rs @@ -14,7 +14,7 @@ impl AggregatePublicKey { } pub fn add(&mut self, public_key: &PublicKey) { - self.0.add(&public_key.as_raw()) + self.0.add(public_key.as_raw()) } /// Returns the underlying signature. diff --git a/eth2/utils/bls/src/public_key.rs b/eth2/utils/bls/src/public_key.rs index 3e2bff19e..c85760bbf 100644 --- a/eth2/utils/bls/src/public_key.rs +++ b/eth2/utils/bls/src/public_key.rs @@ -10,58 +10,39 @@ use ssz::{ use std::default; use std::hash::{Hash, Hasher}; -/// A single BLS public key. -/// -/// This struct stores an uncompressed public key as a byte vec. The reason we store bytes instead -/// of the `RawPublicKey` struct is because it allows for building a hashmap of `PublicKey` much -/// faster. -/// -/// Storing as uncompressed bytes costs ~0.02% more time when adding a `PublicKey` to an -/// `AggregateKey`, however it saves ~0.5ms each time you need to add a pubkey to a hashmap. +/// A single BLS signature. /// /// This struct is a wrapper upon a base type and provides helper functions (e.g., SSZ /// serialization). #[derive(Debug, Clone, Eq)] -pub struct PublicKey { - bytes: Vec, -} +pub struct PublicKey(RawPublicKey); impl PublicKey { pub fn from_secret_key(secret_key: &SecretKey) -> Self { - let mut raw_key = RawPublicKey::from_secret_key(secret_key.as_raw()); - let uncompressed_bytes = raw_key.as_uncompressed_bytes(); - Self { - bytes: uncompressed_bytes, - } + PublicKey(RawPublicKey::from_secret_key(secret_key.as_raw())) } /// Returns the underlying signature. - pub fn as_raw(&self) -> RawPublicKey { - RawPublicKey::from_uncompressed_bytes(&self.bytes).expect("PublicKey in invalid state") + pub fn as_raw(&self) -> &RawPublicKey { + &self.0 } /// Converts compressed bytes to PublicKey pub fn from_bytes(bytes: &[u8]) -> Result { - let mut pubkey = RawPublicKey::from_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; - Ok(Self { - bytes: pubkey.as_uncompressed_bytes(), - }) + let pubkey = RawPublicKey::from_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; + Ok(PublicKey(pubkey)) } /// Returns the PublicKey as (x, y) bytes pub fn as_uncompressed_bytes(&self) -> Vec { - self.bytes.clone() + RawPublicKey::as_uncompressed_bytes(&mut self.0.clone()) } /// Converts (x, y) bytes to PublicKey pub fn from_uncompressed_bytes(bytes: &[u8]) -> Result { - // Do a conversion to check the bytes are valid. - let _pubkey = + let pubkey = RawPublicKey::from_uncompressed_bytes(&bytes).map_err(|_| DecodeError::Invalid)?; - - Ok(Self { - bytes: bytes.to_vec(), - }) + Ok(PublicKey(pubkey)) } /// Returns the last 6 bytes of the SSZ encoding of the public key, as a hex string. @@ -83,22 +64,15 @@ impl default::Default for PublicKey { impl Encodable for PublicKey { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.as_raw().as_bytes()); + s.append_vec(&self.0.as_bytes()); } } impl Decodable for PublicKey { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let mut raw_sig = - RawPublicKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; - - Ok(( - Self { - bytes: raw_sig.as_uncompressed_bytes(), - }, - i, - )) + let raw_sig = RawPublicKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; + Ok((PublicKey(raw_sig), i)) } } @@ -125,7 +99,7 @@ impl<'de> Deserialize<'de> for PublicKey { impl TreeHash for PublicKey { fn hash_tree_root(&self) -> Vec { - hash(&self.as_raw().as_bytes()) + hash(&self.0.as_bytes()) } } @@ -143,7 +117,7 @@ impl Hash for PublicKey { /// /// Use `ssz::Encode` to obtain the bytes required for consensus hashing. fn hash(&self, state: &mut H) { - self.bytes.hash(state) + self.as_uncompressed_bytes().hash(state) } } diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index 47e1dad2e..47598bc66 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -33,7 +33,7 @@ impl Signature { /// Verify the Signature against a PublicKey. pub fn verify(&self, msg: &[u8], domain: u64, pk: &PublicKey) -> bool { - self.0.verify(msg, domain, &pk.as_raw()) + self.0.verify(msg, domain, pk.as_raw()) } /// Verify the Signature against a PublicKey, where the message has already been hashed. @@ -44,7 +44,7 @@ impl Signature { pk: &PublicKey, ) -> bool { self.0 - .verify_hashed(x_real_hashed, x_imaginary_hashed, &pk.as_raw()) + .verify_hashed(x_real_hashed, x_imaginary_hashed, pk.as_raw()) } /// Returns the underlying signature. From 3dfdfc95ac84aac141ee24ddc89c8f35b53c87e7 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 14 Mar 2019 17:53:13 +1100 Subject: [PATCH 07/10] Fix test_utils macro definition It needed to be defined before it was used in an module. --- eth2/types/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 4d13fd16c..7b1d84837 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -2,6 +2,7 @@ #[macro_use] pub mod test_utils; + pub mod attestation; pub mod attestation_data; pub mod attestation_data_and_custody_bit; @@ -24,7 +25,6 @@ pub mod proposer_slashing; pub mod readers; pub mod shard_reassignment_record; pub mod slashable_attestation; -pub mod test_utils; pub mod transfer; pub mod voluntary_exit; #[macro_use] From 2bfc8ed4dadc86848a6a98c5427131d2db6dee9f Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 14 Mar 2019 18:08:09 +1100 Subject: [PATCH 08/10] Fix failing doc test --- beacon_node/beacon_chain/test_harness/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/test_harness/src/lib.rs b/beacon_node/beacon_chain/test_harness/src/lib.rs index f58c1b598..0703fd4a5 100644 --- a/beacon_node/beacon_chain/test_harness/src/lib.rs +++ b/beacon_node/beacon_chain/test_harness/src/lib.rs @@ -15,7 +15,7 @@ //! let validator_count = 8; //! let spec = ChainSpec::few_validators(); //! -//! let mut harness = BeaconChainHarness::new(spec, validator_count, None, true); +//! let mut harness = BeaconChainHarness::new(spec, validator_count); //! //! harness.advance_chain_with_block(); //! From 8cc89b98206f0cf3abe9707e2dd2641303e9bb8b Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 14 Mar 2019 18:08:21 +1100 Subject: [PATCH 09/10] Fix clippy warnings --- .../test_harness/src/beacon_chain_harness.rs | 5 -- .../beacon_chain_harness/generate_deposits.rs | 46 ------------------- .../test_harness/src/test_case.rs | 1 - 3 files changed, 52 deletions(-) delete mode 100644 beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs index 28723a203..d74464ad4 100644 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs +++ b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs @@ -12,14 +12,9 @@ use slot_clock::TestingSlotClock; use ssz::TreeHash; use std::collections::HashSet; use std::iter::FromIterator; -use std::path::Path; use std::sync::Arc; use types::{test_utils::TestingBeaconStateBuilder, *}; -mod generate_deposits; - -pub use generate_deposits::generate_deposits_from_keypairs; - /// The beacon chain harness simulates a single beacon node with `validator_count` validators connected /// to it. Each validator is provided a borrow to the beacon chain, where it may read /// information and submit blocks/attestations for processing. diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs deleted file mode 100644 index bba3aec1c..000000000 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness/generate_deposits.rs +++ /dev/null @@ -1,46 +0,0 @@ -use bls::get_withdrawal_credentials; -use log::debug; -use rayon::prelude::*; -use types::*; - -/// Generates a `Deposit` for each keypairs -pub fn generate_deposits_from_keypairs( - keypairs: &[Keypair], - genesis_time: u64, - domain: u64, - spec: &ChainSpec, -) -> Vec { - debug!( - "Generating {} validator deposits from random keypairs...", - keypairs.len() - ); - - let initial_validator_deposits = keypairs - .par_iter() - .map(|keypair| { - let withdrawal_credentials = Hash256::from_slice( - &get_withdrawal_credentials(&keypair.pk, spec.bls_withdrawal_prefix_byte)[..], - ); - Deposit { - branch: vec![], // branch verification is not specified. - index: 0, // index verification is not specified. - deposit_data: DepositData { - amount: 32_000_000_000, // 32 ETH (in Gwei) - timestamp: genesis_time - 1, - deposit_input: DepositInput { - pubkey: keypair.pk.clone(), - // Validator can withdraw using their main keypair. - withdrawal_credentials: withdrawal_credentials.clone(), - proof_of_possession: DepositInput::create_proof_of_possession( - &keypair, - &withdrawal_credentials, - domain, - ), - }, - }, - } - }) - .collect(); - - initial_validator_deposits -} diff --git a/beacon_node/beacon_chain/test_harness/src/test_case.rs b/beacon_node/beacon_chain/test_harness/src/test_case.rs index cee78f6c4..0a6206972 100644 --- a/beacon_node/beacon_chain/test_harness/src/test_case.rs +++ b/beacon_node/beacon_chain/test_harness/src/test_case.rs @@ -6,7 +6,6 @@ use beacon_chain::CheckPoint; use bls::get_withdrawal_credentials; use log::{info, warn}; use ssz::SignedRoot; -use std::path::Path; use types::*; use types::test_utils::{TestingAttesterSlashingBuilder, TestingProposerSlashingBuilder}; From 65e3b388a0c7ffe0e578d9bcc6746170c5227b49 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 14 Mar 2019 18:17:32 +1100 Subject: [PATCH 10/10] Update signature-scheme to v0.6.1 --- eth2/utils/bls/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth2/utils/bls/Cargo.toml b/eth2/utils/bls/Cargo.toml index 468ed8050..2466605b0 100644 --- a/eth2/utils/bls/Cargo.toml +++ b/eth2/utils/bls/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Paul Hauner "] edition = "2018" [dependencies] -bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "0.6.0" } +bls-aggregates = { git = "https://github.com/sigp/signature-schemes", tag = "0.6.1" } hashing = { path = "../hashing" } hex = "0.3" serde = "1.0"