diff --git a/eth2/state_processing/benches/bench_epoch_processing.rs b/eth2/state_processing/benches/bench_epoch_processing.rs index ab4f61c009..d95f1c8194 100644 --- a/eth2/state_processing/benches/bench_epoch_processing.rs +++ b/eth2/state_processing/benches/bench_epoch_processing.rs @@ -4,14 +4,13 @@ use ssz::TreeHash; use state_processing::{ per_epoch_processing, per_epoch_processing::{ - calculate_active_validator_indices, calculate_attester_sets, clean_attestations, - process_crosslinks, process_eth1_data, process_justification, - process_rewards_and_penalities, process_validator_registry, update_active_tree_index_roots, - update_latest_slashed_balances, + calculate_attester_sets, clean_attestations, process_crosslinks, process_eth1_data, + process_justification, process_rewards_and_penalities, process_validator_registry, + update_active_tree_index_roots, update_latest_slashed_balances, }, }; use types::test_utils::TestingBeaconStateBuilder; -use types::{validator_registry::get_active_validator_indices, *}; +use types::*; pub const BENCHING_SAMPLE_SIZE: usize = 10; pub const SMALL_BENCHING_SAMPLE_SIZE: usize = 10; @@ -73,64 +72,6 @@ pub fn bench_epoch_processing_n_validators(c: &mut Criterion, validator_count: u /// /// `desc` will be added to the title of each bench. fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSpec, desc: &str) { - let state_clone = state.clone(); - let spec_clone = spec.clone(); - c.bench( - &format!("{}/epoch_processing", desc), - Benchmark::new("calculate_active_validator_indices", move |b| { - b.iter_batched( - || state_clone.clone(), - |mut state| { - calculate_active_validator_indices(&mut state, &spec_clone); - state - }, - criterion::BatchSize::SmallInput, - ) - }) - .sample_size(BENCHING_SAMPLE_SIZE), - ); - - let state_clone = state.clone(); - let spec_clone = spec.clone(); - let active_validator_indices = calculate_active_validator_indices(&state, &spec); - c.bench( - &format!("{}/epoch_processing", desc), - Benchmark::new("calculate_current_total_balance", move |b| { - b.iter_batched( - || state_clone.clone(), - |state| { - state.get_total_balance(&active_validator_indices[..], &spec_clone); - state - }, - criterion::BatchSize::SmallInput, - ) - }) - .sample_size(BENCHING_SAMPLE_SIZE), - ); - - let state_clone = state.clone(); - let spec_clone = spec.clone(); - c.bench( - &format!("{}/epoch_processing", desc), - Benchmark::new("calculate_previous_total_balance", move |b| { - b.iter_batched( - || state_clone.clone(), - |state| { - state.get_total_balance( - &get_active_validator_indices( - &state.validator_registry, - state.previous_epoch(&spec_clone), - )[..], - &spec_clone, - ); - state - }, - criterion::BatchSize::SmallInput, - ) - }) - .sample_size(BENCHING_SAMPLE_SIZE), - ); - let state_clone = state.clone(); let spec_clone = spec.clone(); c.bench( @@ -150,15 +91,13 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp let state_clone = state.clone(); let spec_clone = spec.clone(); - let active_validator_indices = calculate_active_validator_indices(&state, &spec); c.bench( &format!("{}/epoch_processing", desc), Benchmark::new("calculate_attester_sets", move |b| { b.iter_batched( || state_clone.clone(), |mut state| { - calculate_attester_sets(&mut state, &active_validator_indices, &spec_clone) - .unwrap(); + calculate_attester_sets(&mut state, &spec_clone).unwrap(); state }, criterion::BatchSize::SmallInput, @@ -169,14 +108,7 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp let state_clone = state.clone(); let spec_clone = spec.clone(); - let previous_epoch = state.previous_epoch(&spec); - let active_validator_indices = calculate_active_validator_indices(&state, &spec); - let attesters = calculate_attester_sets(&state, &active_validator_indices, &spec).unwrap(); - let current_total_balance = state.get_total_balance(&active_validator_indices[..], &spec); - let previous_total_balance = state.get_total_balance( - &get_active_validator_indices(&state.validator_registry, previous_epoch)[..], - &spec, - ); + let attesters = calculate_attester_sets(&state, &spec).unwrap(); c.bench( &format!("{}/epoch_processing", desc), Benchmark::new("process_justification", move |b| { @@ -185,10 +117,10 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp |mut state| { process_justification( &mut state, - current_total_balance, - previous_total_balance, - attesters.balances.previous_epoch_boundary, - attesters.balances.current_epoch_boundary, + attesters.balances.current_epoch_total, + attesters.balances.previous_epoch_total, + attesters.balances.previous_epoch_boundary_attesters, + attesters.balances.current_epoch_boundary_attesters, &spec_clone, ); state @@ -215,13 +147,7 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp let mut state_clone = state.clone(); let spec_clone = spec.clone(); - let previous_epoch = state.previous_epoch(&spec); - let active_validator_indices = calculate_active_validator_indices(&state, &spec); - let attesters = calculate_attester_sets(&state, &active_validator_indices, &spec).unwrap(); - let previous_total_balance = state.get_total_balance( - &get_active_validator_indices(&state.validator_registry, previous_epoch)[..], - &spec, - ); + let attesters = calculate_attester_sets(&state, &spec).unwrap(); let winning_root_for_shards = process_crosslinks(&mut state_clone, &spec).unwrap(); c.bench( &format!("{}/epoch_processing", desc), @@ -232,7 +158,6 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp process_rewards_and_penalities( &mut state, &mut attesters, - previous_total_balance, &winning_root_for_shards, &spec_clone, ) @@ -262,32 +187,8 @@ fn bench_epoch_processing(c: &mut Criterion, state: &BeaconState, spec: &ChainSp .sample_size(BENCHING_SAMPLE_SIZE), ); - let mut state_clone = state.clone(); + let state_clone = state.clone(); let spec_clone = spec.clone(); - let previous_epoch = state.previous_epoch(&spec); - let active_validator_indices = calculate_active_validator_indices(&state, &spec); - let attesters = calculate_attester_sets(&state, &active_validator_indices, &spec).unwrap(); - let current_total_balance = state.get_total_balance(&active_validator_indices[..], spec); - let previous_total_balance = state.get_total_balance( - &get_active_validator_indices(&state.validator_registry, previous_epoch)[..], - &spec, - ); - assert_eq!( - state_clone.finalized_epoch, state_clone.validator_registry_update_epoch, - "The last registry update should be at the last finalized epoch." - ); - process_justification( - &mut state_clone, - current_total_balance, - previous_total_balance, - attesters.balances.previous_epoch_boundary, - attesters.balances.current_epoch_boundary, - spec, - ); - assert!( - state_clone.finalized_epoch > state_clone.validator_registry_update_epoch, - "The state should have been finalized." - ); c.bench( &format!("{}/epoch_processing", desc), Benchmark::new("process_validator_registry", move |b| { diff --git a/eth2/state_processing/src/per_epoch_processing.rs b/eth2/state_processing/src/per_epoch_processing.rs index 2377d7ded1..03135df66b 100644 --- a/eth2/state_processing/src/per_epoch_processing.rs +++ b/eth2/state_processing/src/per_epoch_processing.rs @@ -26,32 +26,21 @@ pub type WinningRootHashSet = HashMap; /// /// Spec v0.4.0 pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> { - let previous_epoch = state.previous_epoch(spec); - // Ensure all of the caches are built. state.build_epoch_cache(RelativeEpoch::Previous, spec)?; state.build_epoch_cache(RelativeEpoch::Current, spec)?; state.build_epoch_cache(RelativeEpoch::Next, spec)?; - let active_validator_indices = calculate_active_validator_indices(&state, spec); - - let current_total_balance = state.get_total_balance(&active_validator_indices[..], spec); - - let previous_total_balance = state.get_total_balance( - &get_active_validator_indices(&state.validator_registry, previous_epoch)[..], - spec, - ); - - let mut attesters = calculate_attester_sets(&state, &active_validator_indices, spec)?; + let mut attesters = calculate_attester_sets(&state, spec)?; process_eth1_data(state, spec); process_justification( state, - current_total_balance, - previous_total_balance, - attesters.balances.previous_epoch_boundary, - attesters.balances.current_epoch_boundary, + attesters.balances.current_epoch_total, + attesters.balances.previous_epoch_total, + attesters.balances.previous_epoch_boundary_attesters, + attesters.balances.current_epoch_boundary_attesters, spec, ); @@ -59,13 +48,7 @@ pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result let winning_root_for_shards = process_crosslinks(state, spec)?; // Rewards and Penalities - process_rewards_and_penalities( - state, - &mut attesters, - previous_total_balance, - &winning_root_for_shards, - spec, - )?; + process_rewards_and_penalities(state, &mut attesters, &winning_root_for_shards, spec)?; // Ejections state.process_ejections(spec); @@ -104,12 +87,12 @@ pub fn calculate_active_validator_indices(state: &BeaconState, spec: &ChainSpec) /// Spec v0.4.0 pub fn calculate_attester_sets( state: &BeaconState, - active_validator_indices: &[usize], spec: &ChainSpec, ) -> Result { - let mut attesters = Attesters::empty(state.validator_registry.len()); - attesters.process_active_validator_indices(&active_validator_indices); + let mut attesters = Attesters::new(state, spec); + attesters.process_attestations(&state, &state.latest_attestations, spec)?; + Ok(attesters) } @@ -285,12 +268,13 @@ pub fn process_crosslinks( pub fn process_rewards_and_penalities( state: &mut BeaconState, attesters: &mut Attesters, - previous_total_balance: u64, winning_root_for_shards: &WinningRootHashSet, spec: &ChainSpec, ) -> Result<(), Error> { let next_epoch = state.next_epoch(spec); + let previous_total_balance = attesters.balances.previous_epoch_total; + let base_reward_quotient = previous_total_balance.integer_sqrt() / spec.base_reward_quotient; if base_reward_quotient == 0 { @@ -317,34 +301,35 @@ pub fn process_rewards_and_penalities( if epochs_since_finality <= 4 { // Expected FFG source - if status.is_previous_epoch { + if status.is_previous_epoch_attester { safe_add_assign!( balance, - base_reward * attesters.balances.previous_epoch / previous_total_balance + base_reward * attesters.balances.previous_epoch_attesters + / previous_total_balance ); - } else if status.is_active { + } else if status.is_active_in_previous_epoch { safe_sub_assign!(balance, base_reward); } // Expected FFG target - if status.is_previous_epoch_boundary { + if status.is_previous_epoch_boundary_attester { safe_add_assign!( balance, - base_reward * attesters.balances.previous_epoch_boundary + base_reward * attesters.balances.previous_epoch_boundary_attesters / previous_total_balance ); - } else if status.is_active { + } else if status.is_active_in_previous_epoch { safe_sub_assign!(balance, base_reward); } // Expected beacon chain head - if status.is_previous_epoch_head { + if status.is_previous_epoch_head_attester { safe_add_assign!( balance, - base_reward * attesters.balances.previous_epoch_head + base_reward * attesters.balances.previous_epoch_head_attesters / previous_total_balance ); - } else if status.is_active { + } else if status.is_active_in_previous_epoch { safe_sub_assign!(balance, base_reward); }; } else { @@ -355,14 +340,14 @@ pub fn process_rewards_and_penalities( spec, ); - if status.is_active { - if !status.is_previous_epoch { + if status.is_active_in_previous_epoch { + if !status.is_previous_epoch_attester { safe_sub_assign!(balance, inactivity_penalty); } - if !status.is_previous_epoch_boundary { + if !status.is_previous_epoch_boundary_attester { safe_sub_assign!(balance, inactivity_penalty); } - if !status.is_previous_epoch_head { + if !status.is_previous_epoch_head_attester { safe_sub_assign!(balance, inactivity_penalty); } @@ -393,7 +378,7 @@ pub fn process_rewards_and_penalities( for (index, _validator) in state.validator_registry.iter().enumerate() { let status = &attesters.statuses[index]; - if status.is_previous_epoch { + if status.is_previous_epoch_attester { let proposer_index = status.inclusion_info.proposer_index; let inclusion_distance = status.inclusion_info.distance; diff --git a/eth2/state_processing/src/per_epoch_processing/attesters.rs b/eth2/state_processing/src/per_epoch_processing/attesters.rs index ef26d338d6..1ffbdf6525 100644 --- a/eth2/state_processing/src/per_epoch_processing/attesters.rs +++ b/eth2/state_processing/src/per_epoch_processing/attesters.rs @@ -42,28 +42,31 @@ impl InclusionInfo { #[derive(Default, Clone)] pub struct AttesterStatus { - pub is_active: bool, + pub is_active_in_current_epoch: bool, + pub is_active_in_previous_epoch: bool, - pub is_current_epoch: bool, - pub is_current_epoch_boundary: bool, - pub is_previous_epoch: bool, - pub is_previous_epoch_boundary: bool, - pub is_previous_epoch_head: bool, + pub is_current_epoch_attester: bool, + pub is_current_epoch_boundary_attester: bool, + pub is_previous_epoch_attester: bool, + pub is_previous_epoch_boundary_attester: bool, + pub is_previous_epoch_head_attester: bool, pub inclusion_info: InclusionInfo, pub winning_root_info: Option, } impl AttesterStatus { + /// Note: does not update the winning root info. pub fn update(&mut self, other: &Self) { // Update all the bool fields, only updating `self` if `other` is true (never setting // `self` to false). - set_self_if_other_is_true!(self, other, is_active); - set_self_if_other_is_true!(self, other, is_current_epoch); - set_self_if_other_is_true!(self, other, is_current_epoch_boundary); - set_self_if_other_is_true!(self, other, is_previous_epoch); - set_self_if_other_is_true!(self, other, is_previous_epoch_boundary); - set_self_if_other_is_true!(self, other, is_previous_epoch_head); + set_self_if_other_is_true!(self, other, is_active_in_current_epoch); + set_self_if_other_is_true!(self, other, is_active_in_previous_epoch); + set_self_if_other_is_true!(self, other, is_current_epoch_attester); + set_self_if_other_is_true!(self, other, is_current_epoch_boundary_attester); + set_self_if_other_is_true!(self, other, is_previous_epoch_attester); + set_self_if_other_is_true!(self, other, is_previous_epoch_boundary_attester); + set_self_if_other_is_true!(self, other, is_previous_epoch_head_attester); self.inclusion_info.update(&other.inclusion_info); } @@ -71,11 +74,13 @@ impl AttesterStatus { #[derive(Default, Clone)] pub struct TotalBalances { - pub current_epoch: u64, - pub current_epoch_boundary: u64, - pub previous_epoch: u64, - pub previous_epoch_boundary: u64, - pub previous_epoch_head: u64, + pub current_epoch_total: u64, + pub previous_epoch_total: u64, + pub current_epoch_attesters: u64, + pub current_epoch_boundary_attesters: u64, + pub previous_epoch_attesters: u64, + pub previous_epoch_boundary_attesters: u64, + pub previous_epoch_head_attesters: u64, } #[derive(Clone)] @@ -85,22 +90,27 @@ pub struct Attesters { } impl Attesters { - pub fn empty(num_validators: usize) -> Self { - Self { - statuses: vec![AttesterStatus::default(); num_validators], - balances: TotalBalances::default(), - } - } + pub fn new(state: &BeaconState, spec: &ChainSpec) -> Self { + let mut statuses = Vec::with_capacity(state.validator_registry.len()); + let mut balances = TotalBalances::default(); - pub fn process_active_validator_indices(&mut self, active_validator_indices: &[usize]) { - let status = AttesterStatus { - is_active: true, - ..AttesterStatus::default() - }; + for (i, validator) in state.validator_registry.iter().enumerate() { + let mut status = AttesterStatus::default(); - for &i in active_validator_indices { - self.statuses[i].update(&status); + if validator.is_active_at(state.current_epoch(spec)) { + status.is_active_in_current_epoch = true; + balances.current_epoch_total += state.get_effective_balance(i, spec); + } + + if validator.is_active_at(state.previous_epoch(spec)) { + status.is_active_in_previous_epoch = true; + balances.previous_epoch_total += state.get_effective_balance(i, spec); + } + + statuses.push(status); } + + Self { statuses, balances } } pub fn process_attestations( @@ -119,16 +129,16 @@ impl Attesters { // Profile this attestation, updating the total balances and generating an // `AttesterStatus` object that applies to all participants in the attestation. if is_from_epoch(a, state.current_epoch(spec), spec) { - self.balances.current_epoch += attesting_balance; - status.is_current_epoch = true; + self.balances.current_epoch_attesters += attesting_balance; + status.is_current_epoch_attester = true; if has_common_epoch_boundary_root(a, state, state.current_epoch(spec), spec)? { - self.balances.current_epoch_boundary += attesting_balance; - status.is_current_epoch_boundary = true; + self.balances.current_epoch_boundary_attesters += attesting_balance; + status.is_current_epoch_boundary_attester = true; } } else if is_from_epoch(a, state.previous_epoch(spec), spec) { - self.balances.previous_epoch += attesting_balance; - status.is_previous_epoch = true; + self.balances.previous_epoch_attesters += attesting_balance; + status.is_previous_epoch_attester = true; // The inclusion slot and distance are only required for previous epoch attesters. status.inclusion_info = InclusionInfo { @@ -138,13 +148,13 @@ impl Attesters { }; if has_common_epoch_boundary_root(a, state, state.previous_epoch(spec), spec)? { - self.balances.previous_epoch_boundary += attesting_balance; - status.is_previous_epoch_boundary = true; + self.balances.previous_epoch_boundary_attesters += attesting_balance; + status.is_previous_epoch_boundary_attester = true; } if has_common_beacon_block_root(a, state, spec)? { - self.balances.previous_epoch_head += attesting_balance; - status.is_previous_epoch_head = true; + self.balances.previous_epoch_head_attesters += attesting_balance; + status.is_previous_epoch_head_attester = true; } }