cerc-io/lighthouse
15
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

spec: update reward processing to v0.6.1 + bugfix

Browse Source 
Two bugs fixed by this commit:

* Reward proposers rather than attesters in `get_proposer_deltas`
* Prevent double-counting of validator balances towards the total when
  computing validator statuses
This commit is contained in:
Michael Sproul 2019-05-06 12:27:59 +10:00
parent 1ad0024045
commit caff553af9
No known key found for this signature in database
GPG Key ID: 77B1309D2E54E914
2 changed files with 136 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

239
eth2/state_processing/src/per_epoch_processing/apply_rewards.rs
View File

@ -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<Delta>,
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<Delta>,
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<u64, BeaconStateError> {
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<u64, BeaconStateError> {
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
}

62
eth2/state_processing/src/per_epoch_processing/validator_statuses.rs
View File

@ -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<Self, BeaconStateError> {
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,