diff --git a/eth2/state_processing/src/per_epoch_processing/apply_rewards.rs b/eth2/state_processing/src/per_epoch_processing/apply_rewards.rs index 9af1ee8c3..2c45dd822 100644 --- a/eth2/state_processing/src/per_epoch_processing/apply_rewards.rs +++ b/eth2/state_processing/src/per_epoch_processing/apply_rewards.rs @@ -1,3 +1,7 @@ +use super::common::{ + get_attesting_balance, get_matching_head_attestations, get_matching_target_attestations, + get_total_active_balance, get_unslashed_attesting_indices, +}; use super::validator_statuses::{TotalBalances, ValidatorStatus, ValidatorStatuses}; use super::{Error, WinningRootHashSet}; use integer_sqrt::IntegerSquareRoot; @@ -32,57 +36,52 @@ impl std::ops::AddAssign for Delta { /// Apply attester and proposer rewards. /// -/// Spec v0.5.1 -pub fn apply_rewards( +/// Spec v0.6.1 +pub fn process_rewards_and_penalties( state: &mut BeaconState, validator_statuses: &mut ValidatorStatuses, winning_root_for_shards: &WinningRootHashSet, spec: &ChainSpec, ) -> Result<(), Error> { + if state.current_epoch(spec) == spec.genesis_epoch { + return Ok(()); + } + // Guard against an out-of-bounds during the validator balance update. - if validator_statuses.statuses.len() != state.validator_balances.len() { - return Err(Error::ValidatorStatusesInconsistent); - } - // Guard against an out-of-bounds during the attester inclusion balance update. - if validator_statuses.statuses.len() != state.validator_registry.len() { + if validator_statuses.statuses.len() != state.balances.len() + || validator_statuses.statuses.len() != state.validator_registry.len() + { return Err(Error::ValidatorStatusesInconsistent); } - let mut deltas = vec![Delta::default(); state.validator_balances.len()]; + let mut deltas = vec![Delta::default(); state.balances.len()]; - get_justification_and_finalization_deltas(&mut deltas, state, &validator_statuses, spec)?; + get_attestation_deltas(&mut deltas, state, &validator_statuses, spec)?; get_crosslink_deltas(&mut deltas, state, &validator_statuses, spec)?; - // Apply the proposer deltas if we are finalizing normally. - // - // This is executed slightly differently to the spec because of the way our functions are - // structured. It should be functionally equivalent. - if epochs_since_finality(state, spec) <= 4 { - get_proposer_deltas( - &mut deltas, - state, - validator_statuses, - winning_root_for_shards, - spec, - )?; - } + get_proposer_deltas( + &mut deltas, + state, + validator_statuses, + winning_root_for_shards, + spec, + )?; // Apply the deltas, over-flowing but not under-flowing (saturating at 0 instead). for (i, delta) in deltas.iter().enumerate() { - state.validator_balances[i] += delta.rewards; - state.validator_balances[i] = state.validator_balances[i].saturating_sub(delta.penalties); + state.balances[i] += delta.rewards; + state.balances[i] = state.balances[i].saturating_sub(delta.penalties); } Ok(()) } -/// Applies the attestation inclusion reward to each proposer for every validator who included an -/// attestation in the previous epoch. +/// For each attesting validator, reward the proposer who was first to include their attestation. /// -/// Spec v0.5.1 +/// Spec v0.6.1 fn get_proposer_deltas( deltas: &mut Vec, - state: &mut BeaconState, + state: &BeaconState, validator_statuses: &mut ValidatorStatuses, winning_root_for_shards: &WinningRootHashSet, spec: &ChainSpec, @@ -90,9 +89,7 @@ fn get_proposer_deltas( // Update statuses with the information from winning roots. validator_statuses.process_winning_roots(state, winning_root_for_shards, spec)?; - for (index, validator) in validator_statuses.statuses.iter().enumerate() { - let mut delta = Delta::default(); - + for validator in &validator_statuses.statuses { if validator.is_previous_epoch_attester { let inclusion = validator .inclusion_info @@ -101,7 +98,7 @@ fn get_proposer_deltas( let base_reward = get_base_reward( state, inclusion.proposer_index, - validator_statuses.total_balances.previous_epoch, + validator_statuses.total_balances.current_epoch, spec, )?; @@ -109,10 +106,8 @@ fn get_proposer_deltas( return Err(Error::ValidatorStatusesInconsistent); } - delta.reward(base_reward / spec.attestation_inclusion_reward_quotient); + deltas[inclusion.proposer_index].reward(base_reward / spec.proposer_reward_quotient); } - - deltas[index] += delta; } Ok(()) @@ -120,40 +115,30 @@ fn get_proposer_deltas( /// Apply rewards for participation in attestations during the previous epoch. /// -/// Spec v0.5.1 -fn get_justification_and_finalization_deltas( +/// Spec v0.6.1 +fn get_attestation_deltas( deltas: &mut Vec, state: &BeaconState, validator_statuses: &ValidatorStatuses, spec: &ChainSpec, ) -> Result<(), Error> { - let epochs_since_finality = epochs_since_finality(state, spec); + let finality_delay = (state.previous_epoch(spec) - state.finalized_epoch).as_u64(); for (index, validator) in validator_statuses.statuses.iter().enumerate() { let base_reward = get_base_reward( state, index, - validator_statuses.total_balances.previous_epoch, - spec, - )?; - let inactivity_penalty = get_inactivity_penalty( - state, - index, - epochs_since_finality.as_u64(), - validator_statuses.total_balances.previous_epoch, + validator_statuses.total_balances.current_epoch, spec, )?; - let delta = if epochs_since_finality <= 4 { - compute_normal_justification_and_finalization_delta( - &validator, - &validator_statuses.total_balances, - base_reward, - spec, - ) - } else { - compute_inactivity_leak_delta(&validator, base_reward, inactivity_penalty, spec) - }; + let delta = get_attestation_delta( + &validator, + &validator_statuses.total_balances, + base_reward, + finality_delay, + spec, + ); deltas[index] += delta; } @@ -161,24 +146,36 @@ fn get_justification_and_finalization_deltas( Ok(()) } -/// Determine the delta for a single validator, if the chain is finalizing normally. +/// Determine the delta for a single validator, sans proposer rewards. /// -/// Spec v0.5.1 -fn compute_normal_justification_and_finalization_delta( +/// Spec v0.6.1 +fn get_attestation_delta( validator: &ValidatorStatus, total_balances: &TotalBalances, base_reward: u64, + finality_delay: u64, spec: &ChainSpec, ) -> Delta { let mut delta = Delta::default(); - let boundary_attesting_balance = total_balances.previous_epoch_boundary_attesters; - let total_balance = total_balances.previous_epoch; + // Is this validator eligible to be rewarded or penalized? + // Spec: validator index in `eligible_validator_indices` + let is_eligible = validator.is_active_in_previous_epoch + || (validator.is_slashed && !validator.is_withdrawable_in_current_epoch); + + if !is_eligible { + return delta; + } + + let total_balance = total_balances.current_epoch; let total_attesting_balance = total_balances.previous_epoch_attesters; - let matching_head_balance = total_balances.previous_epoch_boundary_attesters; + let matching_target_balance = total_balances.previous_epoch_target_attesters; + let matching_head_balance = total_balances.previous_epoch_head_attesters; // Expected FFG source. - if validator.is_previous_epoch_attester { + // Spec: + // - validator index in `get_unslashed_attesting_indices(state, matching_source_attestations)` + if validator.is_previous_epoch_attester && !validator.is_slashed { delta.reward(base_reward * total_attesting_balance / total_balance); // Inclusion speed bonus let inclusion = validator @@ -187,25 +184,43 @@ fn compute_normal_justification_and_finalization_delta( delta.reward( base_reward * spec.min_attestation_inclusion_delay / inclusion.distance.as_u64(), ); - } else if validator.is_active_in_previous_epoch { + } else { delta.penalize(base_reward); } // Expected FFG target. - if validator.is_previous_epoch_boundary_attester { - delta.reward(base_reward / boundary_attesting_balance / total_balance); - } else if validator.is_active_in_previous_epoch { + // Spec: + // - validator index in `get_unslashed_attesting_indices(state, matching_target_attestations)` + if validator.is_previous_epoch_target_attester && !validator.is_slashed { + delta.reward(base_reward * matching_target_balance / total_balance); + } else { delta.penalize(base_reward); } // Expected head. - if validator.is_previous_epoch_head_attester { + // Spec: + // - validator index in `get_unslashed_attesting_indices(state, matching_head_attestations)` + if validator.is_previous_epoch_head_attester && !validator.is_slashed { delta.reward(base_reward * matching_head_balance / total_balance); - } else if validator.is_active_in_previous_epoch { + } else { delta.penalize(base_reward); - }; + } - // Proposer bonus is handled in `apply_proposer_deltas`. + // Inactivity penalty + if finality_delay > spec.min_epochs_to_inactivity_penalty { + // All eligible validators are penalized + delta.penalize(spec.base_rewards_per_epoch * base_reward); + + // Additionally, all validators whose FFG target didn't match are penalized extra + if !validator.is_previous_epoch_target_attester { + delta.penalize( + validator.current_epoch_effective_balance * finality_delay + / spec.inactivity_penalty_quotient, + ); + } + } + + // Proposer bonus is handled in `get_proposer_deltas`. // // This function only computes the delta for a single validator, so it cannot also return a // delta for a validator. @@ -213,52 +228,6 @@ fn compute_normal_justification_and_finalization_delta( delta } -/// Determine the delta for a single delta, assuming the chain is _not_ finalizing normally. -/// -/// Spec v0.5.1 -fn compute_inactivity_leak_delta( - validator: &ValidatorStatus, - base_reward: u64, - inactivity_penalty: u64, - spec: &ChainSpec, -) -> Delta { - let mut delta = Delta::default(); - - if validator.is_active_in_previous_epoch { - if !validator.is_previous_epoch_attester { - delta.penalize(inactivity_penalty); - } else { - // If a validator did attest, apply a small penalty for getting attestations included - // late. - let inclusion = validator - .inclusion_info - .expect("It is a logic error for an attester not to have an inclusion distance."); - delta.reward( - base_reward * spec.min_attestation_inclusion_delay / inclusion.distance.as_u64(), - ); - delta.penalize(base_reward); - } - - if !validator.is_previous_epoch_boundary_attester { - delta.reward(inactivity_penalty); - } - - if !validator.is_previous_epoch_head_attester { - delta.penalize(inactivity_penalty); - } - } - - // Penalize slashed-but-inactive validators as though they were active but offline. - if !validator.is_active_in_previous_epoch - & validator.is_slashed - & !validator.is_withdrawable_in_current_epoch - { - delta.penalize(2 * inactivity_penalty + base_reward); - } - - delta -} - /// Calculate the deltas based upon the winning roots for attestations during the previous epoch. /// /// Spec v0.5.1 @@ -295,40 +264,20 @@ fn get_crosslink_deltas( /// Returns the base reward for some validator. /// -/// Spec v0.5.1 +/// Spec v0.6.1 fn get_base_reward( state: &BeaconState, index: usize, - previous_total_balance: u64, + // Should be == get_total_active_balance(state, spec) + total_active_balance: u64, spec: &ChainSpec, ) -> Result { - if previous_total_balance == 0 { + if total_active_balance == 0 { Ok(0) } else { - let adjusted_quotient = previous_total_balance.integer_sqrt() / spec.base_reward_quotient; - Ok(state.get_effective_balance(index, spec)? / adjusted_quotient / 5) + let adjusted_quotient = total_active_balance.integer_sqrt() / spec.base_reward_quotient; + Ok(state.get_effective_balance(index, spec)? + / adjusted_quotient + / spec.base_rewards_per_epoch) } } - -/// Returns the inactivity penalty for some validator. -/// -/// Spec v0.5.1 -fn get_inactivity_penalty( - state: &BeaconState, - index: usize, - epochs_since_finality: u64, - previous_total_balance: u64, - spec: &ChainSpec, -) -> Result { - Ok(get_base_reward(state, index, previous_total_balance, spec)? - + state.get_effective_balance(index, spec)? * epochs_since_finality - / spec.inactivity_penalty_quotient - / 2) -} - -/// Returns the epochs since the last finalized epoch. -/// -/// Spec v0.5.1 -fn epochs_since_finality(state: &BeaconState, spec: &ChainSpec) -> Epoch { - state.current_epoch(spec) + 1 - state.finalized_epoch -} diff --git a/eth2/state_processing/src/per_epoch_processing/validator_statuses.rs b/eth2/state_processing/src/per_epoch_processing/validator_statuses.rs index afa78c9c0..e57c1afc3 100644 --- a/eth2/state_processing/src/per_epoch_processing/validator_statuses.rs +++ b/eth2/state_processing/src/per_epoch_processing/validator_statuses.rs @@ -68,6 +68,8 @@ pub struct ValidatorStatus { pub is_active_in_current_epoch: bool, /// True if the validator was active in the state's _previous_ epoch. pub is_active_in_previous_epoch: bool, + /// The validator's effective balance in the _current_ epoch. + pub current_epoch_effective_balance: u64, /// True if the validator had an attestation included in the _current_ epoch. pub is_current_epoch_attester: bool, @@ -78,7 +80,7 @@ pub struct ValidatorStatus { pub is_previous_epoch_attester: bool, /// True if the validator's beacon block root attestation for the first slot of the _previous_ /// epoch matches the block root known to the state. - pub is_previous_epoch_boundary_attester: bool, + pub is_previous_epoch_target_attester: bool, /// True if the validator's beacon block root attestation in the _previous_ epoch at the /// attestation's slot (`attestation_data.slot`) matches the block root known to the state. pub is_previous_epoch_head_attester: bool, @@ -108,7 +110,7 @@ impl ValidatorStatus { 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_target_attester); set_self_if_other_is_true!(self, other, is_previous_epoch_head_attester); if let Some(other_info) = other.inclusion_info { @@ -138,7 +140,7 @@ pub struct TotalBalances { pub previous_epoch_attesters: u64, /// The total effective balance of all validators who attested during the _previous_ epoch and /// agreed with the state about the beacon block at the first slot of the _previous_ epoch. - pub previous_epoch_boundary_attesters: u64, + pub previous_epoch_target_attesters: u64, /// The total effective balance of all validators who attested during the _previous_ epoch and /// agreed with the state about the beacon block at the time of attestation. pub previous_epoch_head_attesters: u64, @@ -160,27 +162,29 @@ impl ValidatorStatuses { /// - Active validators /// - Total balances for the current and previous epochs. /// - /// Spec v0.5.1 + /// Spec v0.6.1 pub fn new(state: &BeaconState, spec: &ChainSpec) -> Result { let mut statuses = Vec::with_capacity(state.validator_registry.len()); let mut total_balances = TotalBalances::default(); for (i, validator) in state.validator_registry.iter().enumerate() { + let effective_balance = state.get_effective_balance(i, spec)?; let mut status = ValidatorStatus { is_slashed: validator.slashed, is_withdrawable_in_current_epoch: validator .is_withdrawable_at(state.current_epoch(spec)), + current_epoch_effective_balance: effective_balance, ..ValidatorStatus::default() }; if validator.is_active_at(state.current_epoch(spec)) { status.is_active_in_current_epoch = true; - total_balances.current_epoch += state.get_effective_balance(i, spec)?; + total_balances.current_epoch += effective_balance; } if validator.is_active_at(state.previous_epoch(spec)) { status.is_active_in_previous_epoch = true; - total_balances.previous_epoch += state.get_effective_balance(i, spec)?; + total_balances.previous_epoch += effective_balance; } statuses.push(status); @@ -208,22 +212,18 @@ impl ValidatorStatuses { { let attesting_indices = get_attestation_participants(state, &a.data, &a.aggregation_bitfield, spec)?; - let attesting_balance = state.get_total_balance(&attesting_indices, spec)?; let mut status = ValidatorStatus::default(); // Profile this attestation, updating the total balances and generating an // `ValidatorStatus` object that applies to all participants in the attestation. if is_from_epoch(a, state.current_epoch(spec), spec) { - self.total_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.total_balances.current_epoch_boundary_attesters += attesting_balance; + if target_matches_epoch_start_block(a, state, state.current_epoch(spec), spec)? { status.is_current_epoch_boundary_attester = true; } } else if is_from_epoch(a, state.previous_epoch(spec), spec) { - self.total_balances.previous_epoch_attesters += attesting_balance; status.is_previous_epoch_attester = true; // The inclusion slot and distance are only required for previous epoch attesters. @@ -238,13 +238,11 @@ impl ValidatorStatuses { )?, }); - if has_common_epoch_boundary_root(a, state, state.previous_epoch(spec), spec)? { - self.total_balances.previous_epoch_boundary_attesters += attesting_balance; - status.is_previous_epoch_boundary_attester = true; + if target_matches_epoch_start_block(a, state, state.previous_epoch(spec), spec)? { + status.is_previous_epoch_target_attester = true; } if has_common_beacon_block_root(a, state, spec)? { - self.total_balances.previous_epoch_head_attesters += attesting_balance; status.is_previous_epoch_head_attester = true; } } @@ -255,6 +253,30 @@ impl ValidatorStatuses { } } + // Compute the total balances + for (index, v) in self.statuses.iter().enumerate() { + // According to the spec, we only count unslashed validators towards the totals. + if !v.is_slashed { + let validator_balance = state.get_effective_balance(index, spec)?; + + if v.is_current_epoch_attester { + self.total_balances.current_epoch_attesters += validator_balance; + } + if v.is_current_epoch_boundary_attester { + self.total_balances.current_epoch_boundary_attesters += validator_balance; + } + if v.is_previous_epoch_attester { + self.total_balances.previous_epoch_attesters += validator_balance; + } + if v.is_previous_epoch_target_attester { + self.total_balances.previous_epoch_target_attesters += validator_balance; + } + if v.is_previous_epoch_head_attester { + self.total_balances.previous_epoch_head_attesters += validator_balance; + } + } + } + Ok(()) } @@ -309,11 +331,11 @@ fn is_from_epoch(a: &PendingAttestation, epoch: Epoch, spec: &ChainSpec) -> bool a.data.slot.epoch(spec.slots_per_epoch) == epoch } -/// Returns `true` if a `PendingAttestation` and `BeaconState` share the same beacon block hash for -/// the first slot of the given epoch. +/// Returns `true` if the attestation's FFG target is equal to the hash of the `state`'s first +/// beacon block in the given `epoch`. /// -/// Spec v0.5.1 -fn has_common_epoch_boundary_root( +/// Spec v0.6.0 +fn target_matches_epoch_start_block( a: &PendingAttestation, state: &BeaconState, epoch: Epoch, @@ -328,7 +350,7 @@ fn has_common_epoch_boundary_root( /// Returns `true` if a `PendingAttestation` and `BeaconState` share the same beacon block hash for /// the current slot of the `PendingAttestation`. /// -/// Spec v0.5.1 +/// Spec v0.6.0 fn has_common_beacon_block_root( a: &PendingAttestation, state: &BeaconState,