From e6526c9895635fc800b957a126a8ef7cf520ac01 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 7 Mar 2019 11:32:53 +1100 Subject: [PATCH] Refactor winning root logic --- eth2/state_processing/src/macros.rs | 11 ++ .../src/per_epoch_processing.rs | 178 ++++++++---------- ...{grouped_attesters.rs => attester_sets.rs} | 4 +- .../src/per_epoch_processing/errors.rs | 20 +- .../inclusion_distance.rs | 61 ++++++ .../src/per_epoch_processing/winning_root.rs | 142 ++++++++------ eth2/types/src/beacon_state.rs | 74 -------- eth2/types/src/lib.rs | 4 +- 8 files changed, 253 insertions(+), 241 deletions(-) rename eth2/state_processing/src/per_epoch_processing/{grouped_attesters.rs => attester_sets.rs} (98%) create mode 100644 eth2/state_processing/src/per_epoch_processing/inclusion_distance.rs diff --git a/eth2/state_processing/src/macros.rs b/eth2/state_processing/src/macros.rs index 5a0d0dc11..93a42764b 100644 --- a/eth2/state_processing/src/macros.rs +++ b/eth2/state_processing/src/macros.rs @@ -11,3 +11,14 @@ macro_rules! invalid { return Err(Error::Invalid($result)); }; } + +macro_rules! safe_add_assign { + ($a: expr, $b: expr) => { + $a = $a.saturating_add($b); + }; +} +macro_rules! safe_sub_assign { + ($a: expr, $b: expr) => { + $a = $a.saturating_sub($b); + }; +} diff --git a/eth2/state_processing/src/per_epoch_processing.rs b/eth2/state_processing/src/per_epoch_processing.rs index a54518364..732a6e48f 100644 --- a/eth2/state_processing/src/per_epoch_processing.rs +++ b/eth2/state_processing/src/per_epoch_processing.rs @@ -1,7 +1,8 @@ -use errors::{EpochProcessingError as Error, WinningRootError}; -use grouped_attesters::GroupedAttesters; +use attester_sets::AttesterSets; +use errors::EpochProcessingError as Error; +use inclusion_distance::{inclusion_distance, inclusion_slot}; use integer_sqrt::IntegerSquareRoot; -use log::{debug, trace}; +use log::debug; use rayon::prelude::*; use ssz::TreeHash; use std::collections::{HashMap, HashSet}; @@ -9,21 +10,11 @@ use std::iter::FromIterator; use types::{validator_registry::get_active_validator_indices, *}; use winning_root::{winning_root, WinningRoot}; +pub mod attester_sets; pub mod errors; -mod grouped_attesters; -mod tests; -mod winning_root; - -macro_rules! safe_add_assign { - ($a: expr, $b: expr) => { - $a = $a.saturating_add($b); - }; -} -macro_rules! safe_sub_assign { - ($a: expr, $b: expr) => { - $a = $a.saturating_sub($b); - }; -} +pub mod inclusion_distance; +pub mod tests; +pub mod winning_root; pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> { let current_epoch = state.current_epoch(spec); @@ -40,7 +31,7 @@ pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result state.build_epoch_cache(RelativeEpoch::Current, spec)?; state.build_epoch_cache(RelativeEpoch::Next, spec)?; - let attesters = GroupedAttesters::new(&state, spec)?; + let attesters = AttesterSets::new(&state, spec)?; let active_validator_indices = get_active_validator_indices( &state.validator_registry, @@ -86,12 +77,14 @@ pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result process_validator_registry(state, spec)?; // Final updates - state.latest_active_index_roots[(next_epoch.as_usize() - + spec.activation_exit_delay as usize) - % spec.latest_active_index_roots_length] = hash_tree_root(get_active_validator_indices( + let active_tree_root = get_active_validator_indices( &state.validator_registry, next_epoch + Epoch::from(spec.activation_exit_delay), - )); + ) + .hash_tree_root(); + state.latest_active_index_roots[(next_epoch.as_usize() + + spec.activation_exit_delay as usize) + % spec.latest_active_index_roots_length] = Hash256::from(&active_tree_root[..]); state.latest_slashed_balances[next_epoch.as_usize() % spec.latest_slashed_exit_length] = state.latest_slashed_balances[current_epoch.as_usize() % spec.latest_slashed_exit_length]; @@ -114,10 +107,6 @@ pub fn per_epoch_processing(state: &mut BeaconState, spec: &ChainSpec) -> Result Ok(()) } -fn hash_tree_root(input: Vec) -> Hash256 { - Hash256::from(&input.hash_tree_root()[..]) -} - /// Spec v0.4.0 fn process_eth1_data(state: &mut BeaconState, spec: &ChainSpec) { let next_epoch = state.next_epoch(spec); @@ -210,7 +199,7 @@ fn process_justification( state.justified_epoch = new_justified_epoch; } -pub type WinningRootHashSet = HashMap>; +pub type WinningRootHashSet = HashMap; fn process_crosslinks( state: &mut BeaconState, @@ -219,33 +208,25 @@ fn process_crosslinks( let current_epoch_attestations: Vec<&PendingAttestation> = state .latest_attestations .par_iter() - .filter(|a| { - (a.data.slot / spec.slots_per_epoch).epoch(spec.slots_per_epoch) - == state.current_epoch(spec) - }) + .filter(|a| a.data.slot.epoch(spec.slots_per_epoch) == state.current_epoch(spec)) .collect(); let previous_epoch_attestations: Vec<&PendingAttestation> = state .latest_attestations .par_iter() - .filter(|a| { - (a.data.slot / spec.slots_per_epoch).epoch(spec.slots_per_epoch) - == state.previous_epoch(spec) - }) + .filter(|a| a.data.slot.epoch(spec.slots_per_epoch) == state.previous_epoch(spec)) .collect(); - let mut winning_root_for_shards: HashMap> = - HashMap::new(); - // for slot in state.slot.saturating_sub(2 * spec.slots_per_epoch)..state.slot { - for slot in state.previous_epoch(spec).slot_iter(spec.slots_per_epoch) { - trace!( - "Finding winning root for slot: {} (epoch: {})", - slot, - slot.epoch(spec.slots_per_epoch) - ); + let mut winning_root_for_shards: WinningRootHashSet = HashMap::new(); - // Clone is used to remove the borrow. It becomes an issue later when trying to mutate - // `state.balances`. + let previous_and_current_epoch_slots: Vec = state + .previous_epoch(spec) + .slot_iter(spec.slots_per_epoch) + .chain(state.current_epoch(spec).slot_iter(spec.slots_per_epoch)) + .collect(); + + for slot in previous_and_current_epoch_slots { + // Clone removes the borrow which becomes an issue when mutating `state.balances`. let crosslink_committees_at_slot = state.get_crosslink_committees_at_slot(slot, spec)?.clone(); @@ -258,31 +239,33 @@ fn process_crosslinks( ¤t_epoch_attestations[..], &previous_epoch_attestations[..], spec, - ); + )?; - if let Ok(winning_root) = &winning_root { + if let Some(winning_root) = winning_root { let total_committee_balance = state.get_total_balance(&crosslink_committee, spec); + // TODO: I think this has a bug. if (3 * winning_root.total_attesting_balance) >= (2 * total_committee_balance) { state.latest_crosslinks[shard as usize] = Crosslink { epoch: state.current_epoch(spec), crosslink_data_root: winning_root.crosslink_data_root, } } + winning_root_for_shards.insert(shard, winning_root); } - winning_root_for_shards.insert(shard, winning_root); } } Ok(winning_root_for_shards) } +/// Spec v0.4.0 fn process_rewards_and_penalities( state: &mut BeaconState, active_validator_indices: HashSet, - attesters: &GroupedAttesters, + attesters: &AttesterSets, previous_total_balance: u64, - winning_root_for_shards: &HashMap>, + winning_root_for_shards: &WinningRootHashSet, spec: &ChainSpec, ) -> Result<(), Error> { let next_epoch = state.next_epoch(spec); @@ -290,62 +273,60 @@ fn process_rewards_and_penalities( let previous_epoch_attestations: Vec<&PendingAttestation> = state .latest_attestations .par_iter() - .filter(|a| { - (a.data.slot / spec.slots_per_epoch).epoch(spec.slots_per_epoch) - == state.previous_epoch(spec) - }) + .filter(|a| a.data.slot.epoch(spec.slots_per_epoch) == state.previous_epoch(spec)) .collect(); let base_reward_quotient = previous_total_balance.integer_sqrt() / spec.base_reward_quotient; + if base_reward_quotient == 0 { return Err(Error::BaseRewardQuotientIsZero); } - /* - * Justification and finalization - */ - let epochs_since_finality = next_epoch - state.finalized_epoch; + // Justification and finalization - let active_validator_indices_hashset: HashSet = - HashSet::from_iter(active_validator_indices.iter().cloned()); + let epochs_since_finality = next_epoch - state.finalized_epoch; if epochs_since_finality <= 4 { for index in 0..state.validator_balances.len() { let base_reward = state.base_reward(index, base_reward_quotient, spec); + // Expected FFG source if attesters.previous_epoch.indices.contains(&index) { safe_add_assign!( state.validator_balances[index], base_reward * attesters.previous_epoch.balance / previous_total_balance ); - } else if active_validator_indices_hashset.contains(&index) { + } else if active_validator_indices.contains(&index) { safe_sub_assign!(state.validator_balances[index], base_reward); } + // Expected FFG target if attesters.previous_epoch_boundary.indices.contains(&index) { safe_add_assign!( state.validator_balances[index], base_reward * attesters.previous_epoch_boundary.balance / previous_total_balance ); - } else if active_validator_indices_hashset.contains(&index) { + } else if active_validator_indices.contains(&index) { safe_sub_assign!(state.validator_balances[index], base_reward); } + // Expected beacon chain head if attesters.previous_epoch_head.indices.contains(&index) { safe_add_assign!( state.validator_balances[index], base_reward * attesters.previous_epoch_head.balance / previous_total_balance ); - } else if active_validator_indices_hashset.contains(&index) { + } else if active_validator_indices.contains(&index) { safe_sub_assign!(state.validator_balances[index], base_reward); } } + // Inclusion distance for &index in &attesters.previous_epoch.indices { let base_reward = state.base_reward(index, base_reward_quotient, spec); let inclusion_distance = - state.inclusion_distance(&previous_epoch_attestations, index, spec)?; + inclusion_distance(state, &previous_epoch_attestations, index, spec)?; safe_add_assign!( state.validator_balances[index], @@ -356,7 +337,8 @@ fn process_rewards_and_penalities( for index in 0..state.validator_balances.len() { let inactivity_penalty = state.inactivity_penalty(index, epochs_since_finality, base_reward_quotient, spec); - if active_validator_indices_hashset.contains(&index) { + + if active_validator_indices.contains(&index) { if !attesters.previous_epoch.indices.contains(&index) { safe_sub_assign!(state.validator_balances[index], inactivity_penalty); } @@ -380,7 +362,7 @@ fn process_rewards_and_penalities( for &index in &attesters.previous_epoch.indices { let base_reward = state.base_reward(index, base_reward_quotient, spec); let inclusion_distance = - state.inclusion_distance(&previous_epoch_attestations, index, spec)?; + inclusion_distance(state, &previous_epoch_attestations, index, spec)?; safe_sub_assign!( state.validator_balances[index], @@ -390,61 +372,69 @@ fn process_rewards_and_penalities( } } - trace!("Processed validator justification and finalization rewards/penalities."); + // Attestation inclusion - /* - * Attestation inclusion - */ for &index in &attesters.previous_epoch.indices { - let inclusion_slot = state.inclusion_slot(&previous_epoch_attestations[..], index, spec)?; + let inclusion_slot = inclusion_slot(state, &previous_epoch_attestations[..], index, spec)?; + let proposer_index = state .get_beacon_proposer_index(inclusion_slot, spec) .map_err(|_| Error::UnableToDetermineProducer)?; + let base_reward = state.base_reward(proposer_index, base_reward_quotient, spec); + safe_add_assign!( state.validator_balances[proposer_index], base_reward / spec.attestation_inclusion_reward_quotient ); } - /* - * Crosslinks - */ + //Crosslinks + for slot in state.previous_epoch(spec).slot_iter(spec.slots_per_epoch) { - // Clone is used to remove the borrow. It becomes an issue later when trying to mutate - // `state.balances`. + // Clone removes the borrow which becomes an issue when mutating `state.balances`. let crosslink_committees_at_slot = state.get_crosslink_committees_at_slot(slot, spec)?.clone(); - for (_crosslink_committee, shard) in crosslink_committees_at_slot { + for (crosslink_committee, shard) in crosslink_committees_at_slot { let shard = shard as u64; - if let Some(Ok(winning_root)) = winning_root_for_shards.get(&shard) { - // TODO: remove the map. + // Note: I'm a little uncertain of the logic here -- I am waiting for spec v0.5.0 to + // clear it up. + // + // What happens here is: + // + // - If there was some crosslink root elected by the super-majority of this committee, + // then we reward all who voted for that root and penalize all that did not. + // - However, if there _was not_ some super-majority-voted crosslink root, then penalize + // all the validators. + // + // I'm not quite sure that the second case (no super-majority crosslink) is correct. + if let Some(winning_root) = winning_root_for_shards.get(&shard) { + // Hash set de-dedups and (hopefully) offers a speed improvement from faster + // lookups. let attesting_validator_indices: HashSet = HashSet::from_iter(winning_root.attesting_validator_indices.iter().cloned()); - for index in 0..state.validator_balances.len() { + for &index in &crosslink_committee { let base_reward = state.base_reward(index, base_reward_quotient, spec); + let total_balance = state.get_total_balance(&crosslink_committee, spec); + if attesting_validator_indices.contains(&index) { safe_add_assign!( state.validator_balances[index], - base_reward * winning_root.total_attesting_balance - / winning_root.total_balance + base_reward * winning_root.total_attesting_balance / total_balance ); } else { safe_sub_assign!(state.validator_balances[index], base_reward); } } + } else { + for &index in &crosslink_committee { + let base_reward = state.base_reward(index, base_reward_quotient, spec); - for index in &winning_root.attesting_validator_indices { - let base_reward = state.base_reward(*index, base_reward_quotient, spec); - safe_add_assign!( - state.validator_balances[*index], - base_reward * winning_root.total_attesting_balance - / winning_root.total_balance - ); + safe_sub_assign!(state.validator_balances[index], base_reward); } } } @@ -453,6 +443,7 @@ fn process_rewards_and_penalities( Ok(()) } +// Spec v0.4.0 fn process_validator_registry(state: &mut BeaconState, spec: &ChainSpec) -> Result<(), Error> { let current_epoch = state.current_epoch(spec); let next_epoch = state.next_epoch(spec); @@ -460,11 +451,6 @@ fn process_validator_registry(state: &mut BeaconState, spec: &ChainSpec) -> Resu state.previous_shuffling_epoch = state.current_shuffling_epoch; state.previous_shuffling_start_shard = state.current_shuffling_start_shard; - debug!( - "setting previous_shuffling_seed to : {}", - state.current_shuffling_seed - ); - state.previous_shuffling_seed = state.current_shuffling_seed; let should_update_validator_registy = if state.finalized_epoch @@ -479,7 +465,6 @@ fn process_validator_registry(state: &mut BeaconState, spec: &ChainSpec) -> Resu }; if should_update_validator_registy { - trace!("updating validator registry."); state.update_validator_registry(spec); state.current_shuffling_epoch = next_epoch; @@ -488,7 +473,6 @@ fn process_validator_registry(state: &mut BeaconState, spec: &ChainSpec) -> Resu % spec.shard_count; state.current_shuffling_seed = state.generate_seed(state.current_shuffling_epoch, spec)? } else { - trace!("not updating validator registry."); let epochs_since_last_registry_update = current_epoch - state.validator_registry_update_epoch; if (epochs_since_last_registry_update > 1) diff --git a/eth2/state_processing/src/per_epoch_processing/grouped_attesters.rs b/eth2/state_processing/src/per_epoch_processing/attester_sets.rs similarity index 98% rename from eth2/state_processing/src/per_epoch_processing/grouped_attesters.rs rename to eth2/state_processing/src/per_epoch_processing/attester_sets.rs index 02dbc4050..2b674e1bc 100644 --- a/eth2/state_processing/src/per_epoch_processing/grouped_attesters.rs +++ b/eth2/state_processing/src/per_epoch_processing/attester_sets.rs @@ -17,7 +17,7 @@ impl Attesters { } } -pub struct GroupedAttesters { +pub struct AttesterSets { pub current_epoch: Attesters, pub current_epoch_boundary: Attesters, pub previous_epoch: Attesters, @@ -25,7 +25,7 @@ pub struct GroupedAttesters { pub previous_epoch_head: Attesters, } -impl GroupedAttesters { +impl AttesterSets { pub fn new(state: &BeaconState, spec: &ChainSpec) -> Result { let mut current_epoch = Attesters::default(); let mut current_epoch_boundary = Attesters::default(); diff --git a/eth2/state_processing/src/per_epoch_processing/errors.rs b/eth2/state_processing/src/per_epoch_processing/errors.rs index 0f7063e3b..51e9b253c 100644 --- a/eth2/state_processing/src/per_epoch_processing/errors.rs +++ b/eth2/state_processing/src/per_epoch_processing/errors.rs @@ -1,11 +1,5 @@ use types::*; -#[derive(Debug, PartialEq)] -pub enum WinningRootError { - NoWinningRoot, - BeaconStateError(BeaconStateError), -} - #[derive(Debug, PartialEq)] pub enum EpochProcessingError { UnableToDetermineProducer, @@ -14,7 +8,6 @@ pub enum EpochProcessingError { NoRandaoSeed, BeaconStateError(BeaconStateError), InclusionError(InclusionError), - WinningRootError(WinningRootError), } impl From for EpochProcessingError { @@ -29,8 +22,15 @@ impl From for EpochProcessingError { } } -impl From for WinningRootError { - fn from(e: BeaconStateError) -> WinningRootError { - WinningRootError::BeaconStateError(e) +#[derive(Debug, PartialEq)] +pub enum InclusionError { + /// The validator did not participate in an attestation in this period. + NoAttestationsForValidator, + BeaconStateError(BeaconStateError), +} + +impl From for InclusionError { + fn from(e: BeaconStateError) -> InclusionError { + InclusionError::BeaconStateError(e) } } diff --git a/eth2/state_processing/src/per_epoch_processing/inclusion_distance.rs b/eth2/state_processing/src/per_epoch_processing/inclusion_distance.rs new file mode 100644 index 000000000..243dc67f0 --- /dev/null +++ b/eth2/state_processing/src/per_epoch_processing/inclusion_distance.rs @@ -0,0 +1,61 @@ +use super::errors::InclusionError; +use types::*; + +/// Returns the distance between the first included attestation for some validator and this +/// slot. +/// +/// Note: In the spec this is defined "inline", not as a helper function. +/// +/// Spec v0.4.0 +pub fn inclusion_distance( + state: &BeaconState, + attestations: &[&PendingAttestation], + validator_index: usize, + spec: &ChainSpec, +) -> Result { + let attestation = earliest_included_attestation(state, attestations, validator_index, spec)?; + Ok((attestation.inclusion_slot - attestation.data.slot).as_u64()) +} + +/// Returns the slot of the earliest included attestation for some validator. +/// +/// Note: In the spec this is defined "inline", not as a helper function. +/// +/// Spec v0.4.0 +pub fn inclusion_slot( + state: &BeaconState, + attestations: &[&PendingAttestation], + validator_index: usize, + spec: &ChainSpec, +) -> Result { + let attestation = earliest_included_attestation(state, attestations, validator_index, spec)?; + Ok(attestation.inclusion_slot) +} + +/// Finds the earliest included attestation for some validator. +/// +/// Note: In the spec this is defined "inline", not as a helper function. +/// +/// Spec v0.4.0 +fn earliest_included_attestation( + state: &BeaconState, + attestations: &[&PendingAttestation], + validator_index: usize, + spec: &ChainSpec, +) -> Result { + let mut included_attestations = vec![]; + + for (i, a) in attestations.iter().enumerate() { + let participants = + state.get_attestation_participants(&a.data, &a.aggregation_bitfield, spec)?; + if participants.iter().any(|i| *i == validator_index) { + included_attestations.push(i); + } + } + + let earliest_attestation_index = included_attestations + .iter() + .min_by_key(|i| attestations[**i].inclusion_slot) + .ok_or_else(|| InclusionError::NoAttestationsForValidator)?; + Ok(attestations[*earliest_attestation_index].clone()) +} diff --git a/eth2/state_processing/src/per_epoch_processing/winning_root.rs b/eth2/state_processing/src/per_epoch_processing/winning_root.rs index c3b650c3d..07678f93b 100644 --- a/eth2/state_processing/src/per_epoch_processing/winning_root.rs +++ b/eth2/state_processing/src/per_epoch_processing/winning_root.rs @@ -1,86 +1,118 @@ -use super::errors::WinningRootError; -use std::collections::HashMap; +use std::collections::HashSet; +use std::iter::FromIterator; use types::*; #[derive(Clone)] pub struct WinningRoot { pub crosslink_data_root: Hash256, pub attesting_validator_indices: Vec, - pub total_balance: u64, pub total_attesting_balance: u64, } +impl WinningRoot { + /// Returns `true` if `self` is a "better" candidate than `other`. + /// + /// A winning root is "better" than another if it has a higher `total_attesting_balance`. Ties + /// are broken by favouring the lower `crosslink_data_root` value. + /// + /// Spec v0.4.0 + pub fn is_better_than(&self, other: &Self) -> bool { + if self.total_attesting_balance > other.total_attesting_balance { + true + } else if self.total_attesting_balance == other.total_attesting_balance { + self.crosslink_data_root < other.crosslink_data_root + } else { + false + } + } +} + +/// Returns the `crosslink_data_root` with the highest total attesting balance for the given shard. +/// Breaks ties by favouring the smaller `crosslink_data_root` hash. +/// +/// The `WinningRoot` object also contains additional fields that are useful in later stages of +/// per-epoch processing. +/// +/// Spec v0.4.0 pub fn winning_root( state: &BeaconState, shard: u64, current_epoch_attestations: &[&PendingAttestation], previous_epoch_attestations: &[&PendingAttestation], spec: &ChainSpec, -) -> Result { - let mut attestations = current_epoch_attestations.to_vec(); - attestations.append(&mut previous_epoch_attestations.to_vec()); +) -> Result, BeaconStateError> { + let mut winning_root: Option = None; - let mut candidates: HashMap = HashMap::new(); - - let mut highest_seen_balance = 0; - - for a in &attestations { - if a.data.shard != shard { - continue; - } - - let crosslink_data_root = &a.data.crosslink_data_root; - - if candidates.contains_key(crosslink_data_root) { - continue; - } - - let attesting_validator_indices = attestations + let crosslink_data_roots: HashSet = HashSet::from_iter( + previous_epoch_attestations .iter() - .try_fold::<_, _, Result<_, BeaconStateError>>(vec![], |mut acc, a| { - if (a.data.shard == shard) && (a.data.crosslink_data_root == *crosslink_data_root) { - acc.append(&mut state.get_attestation_participants( - &a.data, - &a.aggregation_bitfield, - spec, - )?); + .chain(current_epoch_attestations.iter()) + .filter_map(|a| { + if a.data.shard == shard { + Some(a.data.crosslink_data_root) + } else { + None } - Ok(acc) - })?; + }), + ); - let total_balance: u64 = attesting_validator_indices - .iter() - .fold(0, |acc, i| acc + state.get_effective_balance(*i, spec)); + for crosslink_data_root in crosslink_data_roots { + let attesting_validator_indices = get_attesting_validator_indices( + state, + shard, + current_epoch_attestations, + previous_epoch_attestations, + &crosslink_data_root, + spec, + )?; let total_attesting_balance: u64 = attesting_validator_indices .iter() .fold(0, |acc, i| acc + state.get_effective_balance(*i, spec)); - if total_attesting_balance > highest_seen_balance { - highest_seen_balance = total_attesting_balance; - } - - let candidate_root = WinningRoot { - crosslink_data_root: *crosslink_data_root, + let candidate = WinningRoot { + crosslink_data_root, attesting_validator_indices, total_attesting_balance, - total_balance, }; - candidates.insert(*crosslink_data_root, candidate_root); + if let Some(ref winner) = winning_root { + if candidate.is_better_than(&winner) { + winning_root = Some(candidate); + } + } else { + winning_root = Some(candidate); + } } - Ok(candidates - .iter() - .filter_map(|(_hash, candidate)| { - if candidate.total_attesting_balance == highest_seen_balance { - Some(candidate) - } else { - None - } - }) - .min_by_key(|candidate| candidate.crosslink_data_root) - .ok_or_else(|| WinningRootError::NoWinningRoot)? - // TODO: avoid clone. - .clone()) + Ok(winning_root) +} + +/// Returns all indices which voted for a given crosslink. May contain duplicates. +/// +/// Spec v0.4.0 +fn get_attesting_validator_indices( + state: &BeaconState, + shard: u64, + current_epoch_attestations: &[&PendingAttestation], + previous_epoch_attestations: &[&PendingAttestation], + crosslink_data_root: &Hash256, + spec: &ChainSpec, +) -> Result, BeaconStateError> { + let mut indices = vec![]; + + for a in current_epoch_attestations + .iter() + .chain(previous_epoch_attestations.iter()) + { + if (a.data.shard == shard) && (a.data.crosslink_data_root == *crosslink_data_root) { + indices.append(&mut state.get_attestation_participants( + &a.data, + &a.aggregation_bitfield, + spec, + )?); + } + } + + Ok(indices) } diff --git a/eth2/types/src/beacon_state.rs b/eth2/types/src/beacon_state.rs index 76b97b21d..bb77981ab 100644 --- a/eth2/types/src/beacon_state.rs +++ b/eth2/types/src/beacon_state.rs @@ -56,13 +56,6 @@ pub enum Error { EpochCacheUninitialized(RelativeEpoch), } -#[derive(Debug, PartialEq)] -pub enum InclusionError { - /// The validator did not participate in an attestation in this period. - NoAttestationsForValidator, - Error(Error), -} - macro_rules! safe_add_assign { ($a: expr, $b: expr) => { $a = $a.saturating_add($b); @@ -1123,67 +1116,6 @@ impl BeaconState { / 2 } - /// Returns the distance between the first included attestation for some validator and this - /// slot. - /// - /// Note: In the spec this is defined "inline", not as a helper function. - /// - /// Spec v0.4.0 - pub fn inclusion_distance( - &self, - attestations: &[&PendingAttestation], - validator_index: usize, - spec: &ChainSpec, - ) -> Result { - let attestation = - self.earliest_included_attestation(attestations, validator_index, spec)?; - Ok((attestation.inclusion_slot - attestation.data.slot).as_u64()) - } - - /// Returns the slot of the earliest included attestation for some validator. - /// - /// Note: In the spec this is defined "inline", not as a helper function. - /// - /// Spec v0.4.0 - pub fn inclusion_slot( - &self, - attestations: &[&PendingAttestation], - validator_index: usize, - spec: &ChainSpec, - ) -> Result { - let attestation = - self.earliest_included_attestation(attestations, validator_index, spec)?; - Ok(attestation.inclusion_slot) - } - - /// Finds the earliest included attestation for some validator. - /// - /// Note: In the spec this is defined "inline", not as a helper function. - /// - /// Spec v0.4.0 - fn earliest_included_attestation( - &self, - attestations: &[&PendingAttestation], - validator_index: usize, - spec: &ChainSpec, - ) -> Result { - let mut included_attestations = vec![]; - - for (i, a) in attestations.iter().enumerate() { - let participants = - self.get_attestation_participants(&a.data, &a.aggregation_bitfield, spec)?; - if participants.iter().any(|i| *i == validator_index) { - included_attestations.push(i); - } - } - - let earliest_attestation_index = included_attestations - .iter() - .min_by_key(|i| attestations[**i].inclusion_slot) - .ok_or_else(|| InclusionError::NoAttestationsForValidator)?; - Ok(attestations[*earliest_attestation_index].clone()) - } - /// Returns the base reward for some validator. /// /// Note: In the spec this is defined "inline", not as a helper function. @@ -1226,12 +1158,6 @@ fn hash_tree_root(input: Vec) -> Hash256 { Hash256::from(&input.hash_tree_root()[..]) } -impl From for InclusionError { - fn from(e: Error) -> InclusionError { - InclusionError::Error(e) - } -} - impl Encodable for BeaconState { fn ssz_append(&self, s: &mut SszStream) { s.append(&self.slot); diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index e595684bc..9bf60f2c9 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -40,9 +40,7 @@ pub use crate::attestation_data_and_custody_bit::AttestationDataAndCustodyBit; pub use crate::attester_slashing::AttesterSlashing; pub use crate::beacon_block::BeaconBlock; pub use crate::beacon_block_body::BeaconBlockBody; -pub use crate::beacon_state::{ - BeaconState, Error as BeaconStateError, InclusionError, RelativeEpoch, -}; +pub use crate::beacon_state::{BeaconState, Error as BeaconStateError, RelativeEpoch}; pub use crate::chain_spec::{ChainSpec, Domain}; pub use crate::crosslink::Crosslink; pub use crate::deposit::Deposit;