diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 3d9564da711..2b6fdc7d031 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -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 = generate_deterministic_keypairs(VALIDATOR_COUNT); } -fn get_harness(spec: ChainSpec) -> BeaconChainHarness> { +fn get_harness(spec: ChainSpec) -> BeaconChainHarness> { let harness = BeaconChainHarness::builder(E::default()) .spec(spec) .keypairs(KEYPAIRS.to_vec()) @@ -36,11 +39,10 @@ fn get_harness(spec: ChainSpec) -> BeaconChainHarness(spec); + let harness = get_harness(spec); let num_block_produced = E::slots_per_epoch(); let latest_block_root = harness @@ -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::(spec); + let harness = get_harness(E::default_spec()); // epoch 0 (N), only two thirds of validators vote. let two_thirds = (VALIDATOR_COUNT / 3) * 2; @@ -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::(spec.clone()); + let harness = get_harness(spec.clone()); let half = VALIDATOR_COUNT / 2; let half_validators: Vec = (0..half).collect(); @@ -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 = (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 = 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, @@ -251,3 +297,17 @@ fn apply_attestation_rewards( }) .collect::>() } + +fn get_validator_balances(state: BeaconState, validators: &[usize]) -> Vec { + validators + .iter() + .map(|&id| { + state + .balances() + .get(id) + .cloned() + .ok_or(BeaconStateError::BalancesOutOfBounds(id)) + }) + .flatten() + .collect() +} diff --git a/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs b/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs index 6179f4d9fbb..2acf12a0d6d 100644 --- a/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs +++ b/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs @@ -105,11 +105,14 @@ pub fn get_attestation_deltas_subset( 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 @@ -118,36 +121,32 @@ pub fn get_attestation_deltas_subset( 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) {