Skip to content

Commit

Permalink
Add a test for computing attestation rewards for a subset and fixed a…
Browse files Browse the repository at this point in the history 
… pretty terrible bug.
  • Loading branch information
@jimmygchen
jimmygchen committed Jul 11, 2023
1 parent cc16392 commit 7251507
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 39 deletions.
78 changes: 69 additions & 9 deletions beacon_node/beacon_chain/tests/rewards.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@ use eth2::lighthouse::StandardAttestationRewards;
use eth2::types::ValidatorId;
use lazy_static::lazy_static;
use task_executor::test_utils::null_logger;
use types::ChainSpec;
use types::beacon_state::Error as BeaconStateError;
use types::{BeaconState, ChainSpec};

pub const VALIDATOR_COUNT: usize = 64;

type E = MinimalEthSpec;

lazy_static! {
static ref KEYPAIRS: Vec<Keypair> = generate_deterministic_keypairs(VALIDATOR_COUNT);
}

fn get_harness<E: EthSpec>(spec: ChainSpec) -> BeaconChainHarness<EphemeralHarnessType<E>> {
fn get_harness(spec: ChainSpec) -> BeaconChainHarness<EphemeralHarnessType<E>> {
let harness = BeaconChainHarness::builder(E::default())
.spec(spec)
.keypairs(KEYPAIRS.to_vec())
Expand All @@ -36,11 +39,10 @@ fn get_harness<E: EthSpec>(spec: ChainSpec) -> BeaconChainHarness<EphemeralHarne

#[tokio::test]
async fn test_sync_committee_rewards() {
type E = MinimalEthSpec;
let mut spec = E::default_spec();
spec.altair_fork_epoch = Some(Epoch::new(0));

let harness = get_harness::<E>(spec);
let harness = get_harness(spec);
let num_block_produced = E::slots_per_epoch();

let latest_block_root = harness
Expand Down Expand Up @@ -127,9 +129,7 @@ async fn test_sync_committee_rewards() {

#[tokio::test]
async fn test_verify_attestation_rewards_base() {
type E = MinimalEthSpec;
let spec = E::default_spec();
let harness = get_harness::<E>(spec);
let harness = get_harness(E::default_spec());

// epoch 0 (N), only two thirds of validators vote.
let two_thirds = (VALIDATOR_COUNT / 3) * 2;
Expand Down Expand Up @@ -174,9 +174,8 @@ async fn test_verify_attestation_rewards_base() {

#[tokio::test]
async fn test_verify_attestation_rewards_base_inactivity_leak() {
type E = MinimalEthSpec;
let spec = E::default_spec();
let harness = get_harness::<E>(spec.clone());
let harness = get_harness(spec.clone());

let half = VALIDATOR_COUNT / 2;
let half_validators: Vec<usize> = (0..half).collect();
Expand Down Expand Up @@ -233,6 +232,53 @@ async fn test_verify_attestation_rewards_base_inactivity_leak() {
assert_eq!(expected_balances, balances);
}

#[tokio::test]
async fn test_verify_attestation_rewards_base_subset_only() {
let harness = get_harness(E::default_spec());

// epoch 0 (N), only two thirds of validators vote.
let two_thirds = (VALIDATOR_COUNT / 3) * 2;
let two_thirds_validators: Vec<usize> = (0..two_thirds).collect();
harness
.extend_chain(
E::slots_per_epoch() as usize,
BlockStrategy::OnCanonicalHead,
AttestationStrategy::SomeValidators(two_thirds_validators),
)
.await;

// a small subset of validators to compute attestation rewards for
let validators_subset = [0, VALIDATOR_COUNT / 2, VALIDATOR_COUNT - 1];

// capture balances before transitioning to N + 2
let initial_balances = get_validator_balances(harness.get_current_state(), &validators_subset);

// extend slots to beginning of epoch N + 2
harness.extend_slots(E::slots_per_epoch() as usize).await;

let validators_subset_ids: Vec<ValidatorId> = validators_subset
.into_iter()
.map(|idx| ValidatorId::Index(idx as u64))
.collect();

// compute reward deltas for the subset of validators in epoch N
let StandardAttestationRewards {
ideal_rewards: _,
total_rewards,
} = harness
.chain
.compute_attestation_rewards(Epoch::new(0), validators_subset_ids, null_logger().unwrap())
.unwrap();

// apply attestation rewards to initial balances
let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards);

// verify expected balances against actual balances
let balances = get_validator_balances(harness.get_current_state(), &validators_subset);
assert_eq!(expected_balances, balances);
}

/// Apply a vec of `TotalAttestationRewards` to initial balances, and return
fn apply_attestation_rewards(
initial_balances: &[u64],
attestation_rewards: Vec<TotalAttestationRewards>,
Expand All @@ -251,3 +297,17 @@ fn apply_attestation_rewards(
})
.collect::<Vec<u64>>()
}

fn get_validator_balances(state: BeaconState<E>, validators: &[usize]) -> Vec<u64> {
validators
.iter()
.map(|&id| {
state
.balances()
.get(id)
.cloned()
.ok_or(BeaconStateError::BalancesOutOfBounds(id))
})
.flatten()
.collect()
}
59 changes: 29 additions & 30 deletions consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,14 @@ pub fn get_attestation_deltas_subset<T: EthSpec>(

let total_balances = &validator_statuses.total_balances;

for (index, validator) in validator_statuses.statuses.iter().enumerate() {
let delta = deltas
.entry(index)
.or_insert_with(AttestationDelta::default);
// Ignore validator if a subset is specified and validator is not in the subset
let include_validator_delta = |idx| match maybe_validators_subset.as_ref() {
None => true,
Some(validators_subset) if validators_subset.contains(&idx) => true,
Some(_) => false,
};

for (index, validator) in validator_statuses.statuses.iter().enumerate() {
// Ignore ineligible validators. All sub-functions of the spec do this except for
// `get_inclusion_delay_deltas`. It's safe to do so here because any validator that is in
// the unslashed indices of the matching source attestations is active, and therefore
Expand All @@ -118,36 +121,32 @@ pub fn get_attestation_deltas_subset<T: EthSpec>(
continue;
}

let include_validator_delta = |idx| match maybe_validators_subset.as_ref() {
None => true,
Some(validators_subset) if validators_subset.contains(&idx) => true,
Some(_) => false,
};

if !include_validator_delta(index) {
continue;
}

let base_reward = get_base_reward(state, index, total_balances.current_epoch(), spec)?;

let source_delta =
get_source_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let target_delta =
get_target_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let head_delta =
get_head_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let (inclusion_delay_delta, proposer_delta) =
get_inclusion_delay_delta(validator, base_reward, spec)?;
let inactivity_penalty_delta =
get_inactivity_penalty_delta(validator, base_reward, finality_delay, spec)?;

delta.source_delta.combine(source_delta)?;
delta.target_delta.combine(target_delta)?;
delta.head_delta.combine(head_delta)?;
delta.inclusion_delay_delta.combine(inclusion_delay_delta)?;
delta
.inactivity_penalty_delta
.combine(inactivity_penalty_delta)?;

if include_validator_delta(index) {
let source_delta =
get_source_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let target_delta =
get_target_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let head_delta =
get_head_delta(validator, base_reward, total_balances, finality_delay, spec)?;
let inactivity_penalty_delta =
get_inactivity_penalty_delta(validator, base_reward, finality_delay, spec)?;

let delta = deltas
.entry(index)
.or_insert_with(AttestationDelta::default);
delta.source_delta.combine(source_delta)?;
delta.target_delta.combine(target_delta)?;
delta.head_delta.combine(head_delta)?;
delta.inclusion_delay_delta.combine(inclusion_delay_delta)?;
delta
.inactivity_penalty_delta
.combine(inactivity_penalty_delta)?;
}

if let Some((proposer_index, proposer_delta)) = proposer_delta {
if include_validator_delta(proposer_index) {
Expand Down

0 comments on commit 7251507

Please sign in to comment.