From 71ef86be46a82f1e9d044c790d46e081ef4ad2ec Mon Sep 17 00:00:00 2001 From: Gua00va Date: Thu, 26 Oct 2023 12:27:51 +0530 Subject: [PATCH 1/8] Added Block Rewards --- beacon_node/beacon_chain/tests/rewards.rs | 119 +++++++++++++++++----- 1 file changed, 93 insertions(+), 26 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 7c8f01cf55c..231d25b1dd7 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -134,11 +134,7 @@ async fn test_verify_attestation_rewards_base() { 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), - ) + .extend_slots_some_validators(E::slots_per_epoch() as usize, two_thirds_validators.clone()) .await; let initial_balances: Vec = harness.get_current_state().balances().clone().into(); @@ -146,6 +142,31 @@ async fn test_verify_attestation_rewards_base() { // extend slots to beginning of epoch N + 2 harness.extend_slots(E::slots_per_epoch() as usize).await; + let mut proposal_rewards_map: HashMap = HashMap::new(); + + for _ in 0..E::slots_per_epoch() { + let state = harness.get_current_state(); + let slot = state.slot() + Slot::new(1); + + let ((signed_block, _maybe_blob_sidecars), mut state) = + harness.make_block_return_pre_state(state, slot).await; + let beacon_block_reward = harness + .chain + .compute_beacon_block_reward( + signed_block.message(), + signed_block.canonical_root(), + &mut state, + ) + .unwrap(); + let total_proposer_reward = proposal_rewards_map + .get(&beacon_block_reward.proposer_index) + .unwrap_or(&0u64) + + beacon_block_reward.total; + proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + + harness.extend_slots(1).await; + } + // compute reward deltas for all validators in epoch N let StandardAttestationRewards { ideal_rewards, @@ -161,6 +182,7 @@ async fn test_verify_attestation_rewards_base() { // apply attestation rewards to initial balances let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); @@ -177,26 +199,45 @@ async fn test_verify_attestation_rewards_base_inactivity_leak() { // target epoch is the epoch where the chain enters inactivity leak let target_epoch = &spec.min_epochs_to_inactivity_penalty + 1; - // advance until beginning of epoch N + 1 and get balances + //advance until beginning of epoch N + 1 and get balances harness - .extend_chain( + .extend_slots_some_validators( (E::slots_per_epoch() * (target_epoch + 1)) as usize, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::SomeValidators(half_validators.clone()), + half_validators.clone(), ) .await; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); - // extend slots to beginning of epoch N + 2 - harness.advance_slot(); - harness - .extend_chain( - E::slots_per_epoch() as usize, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::SomeValidators(half_validators), - ) - .await; - let _slot = harness.get_current_slot(); + let initial_balances = harness.get_current_state().balances().clone(); + + // advance until epoch N + 2 and build proposal rewards map + let mut proposal_rewards_map: HashMap = HashMap::new(); + + for _ in 0..E::slots_per_epoch() { + let state = harness.get_current_state(); + let slot = state.slot() + Slot::new(1); + + let ((signed_block, _maybe_blob_sidecars), mut state) = + harness.make_block_return_pre_state(state, slot).await; + let beacon_block_reward = harness + .chain + .compute_beacon_block_reward( + signed_block.message(), + signed_block.canonical_root(), + &mut state, + ) + .unwrap(); + + let total_proposer_reward = proposal_rewards_map + .get(&beacon_block_reward.proposer_index) + .unwrap_or(&0u64) + + beacon_block_reward.total; + + proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + + harness + .extend_slots_some_validators(1, half_validators.clone()) + .await; + } // compute reward deltas for all validators in epoch N let StandardAttestationRewards { @@ -213,6 +254,7 @@ async fn test_verify_attestation_rewards_base_inactivity_leak() { // apply attestation rewards to initial balances let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); @@ -321,11 +363,7 @@ async fn test_verify_attestation_rewards_base_subset_only() { 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), - ) + .extend_slots_some_validators(E::slots_per_epoch() as usize, two_thirds_validators.clone()) .await; // a small subset of validators to compute attestation rewards for @@ -335,7 +373,35 @@ async fn test_verify_attestation_rewards_base_subset_only() { 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 mut proposal_rewards_map: HashMap = HashMap::new(); + + for _ in 0..E::slots_per_epoch() { + let state = harness.get_current_state(); + let slot = state.slot() + Slot::new(1); + + // calculate beacon block rewards / penalties + let ((signed_block, _maybe_blob_sidecars), mut state) = + harness.make_block_return_pre_state(state, slot).await; + let beacon_block_reward = harness + .chain + .compute_beacon_block_reward( + signed_block.message(), + signed_block.canonical_root(), + &mut state, + ) + .unwrap(); + + let total_proposer_reward = proposal_rewards_map + .get(&beacon_block_reward.proposer_index) + .unwrap_or(&0u64) + + beacon_block_reward.total; + + proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + + harness + .extend_slots_some_validators(1, two_thirds_validators.clone()) + .await; + } let validators_subset_ids: Vec = validators_subset .into_iter() @@ -353,6 +419,7 @@ async fn test_verify_attestation_rewards_base_subset_only() { // apply attestation rewards to initial balances let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); // verify expected balances against actual balances let balances = get_validator_balances(harness.get_current_state(), &validators_subset); From 9bcaa83f846ea79fe36e8902eae24c916ba4a566 Mon Sep 17 00:00:00 2001 From: Gua00va Date: Fri, 10 Nov 2023 16:51:14 +0530 Subject: [PATCH 2/8] added new type --- beacon_node/beacon_chain/tests/rewards.rs | 3 -- .../base/rewards_and_penalties.rs | 31 ++++++++++++++----- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 231d25b1dd7..4cc706a3fe7 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -139,9 +139,6 @@ async fn test_verify_attestation_rewards_base() { let initial_balances: Vec = harness.get_current_state().balances().clone().into(); - // extend slots to beginning of epoch N + 2 - harness.extend_slots(E::slots_per_epoch() as usize).await; - let mut proposal_rewards_map: HashMap = HashMap::new(); for _ in 0..E::slots_per_epoch() { 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 74c96d8aee5..13a27018a86 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 @@ -42,6 +42,12 @@ impl AttestationDelta { } } +#[derive(Debug)] +pub enum RewardsCalculationType { + Consensus, + API(Vec), +} + /// Apply attester and proposer rewards. pub fn process_rewards_and_penalties( state: &mut BeaconState, @@ -78,7 +84,7 @@ pub fn get_attestation_deltas_all( validator_statuses: &ValidatorStatuses, spec: &ChainSpec, ) -> Result, Error> { - get_attestation_deltas(state, validator_statuses, None, spec) + get_attestation_deltas(state, validator_statuses, RewardsCalculationType::API(vec![]), spec) } /// Apply rewards for participation in attestations during the previous epoch, and only compute @@ -89,7 +95,7 @@ pub fn get_attestation_deltas_subset( validators_subset: &Vec, spec: &ChainSpec, ) -> Result, Error> { - get_attestation_deltas(state, validator_statuses, Some(validators_subset), spec).map(|deltas| { + get_attestation_deltas(state, validator_statuses, RewardsCalculationType::API(validators_subset.clone()), spec).map(|deltas| { deltas .into_iter() .enumerate() @@ -106,7 +112,7 @@ pub fn get_attestation_deltas_subset( fn get_attestation_deltas( state: &BeaconState, validator_statuses: &ValidatorStatuses, - maybe_validators_subset: Option<&Vec>, + maybe_validators_subset: RewardsCalculationType, spec: &ChainSpec, ) -> Result, Error> { let previous_epoch = state.previous_epoch(); @@ -119,11 +125,20 @@ fn get_attestation_deltas( let total_balances = &validator_statuses.total_balances; - // 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, + // Check if the calculation is for Consensus or Rewards API and Ignore validator if a subset is specified and validator is not in the subset + let include_validator_delta = |idx| match &maybe_validators_subset { + RewardsCalculationType::Consensus => true, + RewardsCalculationType::API(validator_subset) => { + if validator_subset.is_empty() { + true + } + else if validator_subset.contains(&idx) { + true + } + else { + false + } + } }; for (index, validator) in validator_statuses.statuses.iter().enumerate() { From 4e9725c1d0e9d52b9de047a51b90d032d17987fe Mon Sep 17 00:00:00 2001 From: Gua00va Date: Sun, 12 Nov 2023 12:11:08 +0530 Subject: [PATCH 3/8] added enum --- .../base/rewards_and_penalties.rs | 53 ++++++++++++------- 1 file changed, 34 insertions(+), 19 deletions(-) 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 13a27018a86..598bed0df33 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 @@ -65,7 +65,12 @@ pub fn process_rewards_and_penalties( return Err(Error::ValidatorStatusesInconsistent); } - let deltas = get_attestation_deltas_all(state, validator_statuses, spec)?; + let deltas = get_attestation_deltas( + state, + validator_statuses, + RewardsCalculationType::Consensus, + spec, + )?; // Apply the deltas, erroring on overflow above but not on overflow below (saturating at 0 // instead). @@ -84,7 +89,12 @@ pub fn get_attestation_deltas_all( validator_statuses: &ValidatorStatuses, spec: &ChainSpec, ) -> Result, Error> { - get_attestation_deltas(state, validator_statuses, RewardsCalculationType::API(vec![]), spec) + get_attestation_deltas( + state, + validator_statuses, + RewardsCalculationType::API(vec![]), + spec, + ) } /// Apply rewards for participation in attestations during the previous epoch, and only compute @@ -95,7 +105,13 @@ pub fn get_attestation_deltas_subset( validators_subset: &Vec, spec: &ChainSpec, ) -> Result, Error> { - get_attestation_deltas(state, validator_statuses, RewardsCalculationType::API(validators_subset.clone()), spec).map(|deltas| { + get_attestation_deltas( + state, + validator_statuses, + RewardsCalculationType::API(validators_subset.clone()), + spec, + ) + .map(|deltas| { deltas .into_iter() .enumerate() @@ -128,17 +144,12 @@ fn get_attestation_deltas( // Check if the calculation is for Consensus or Rewards API and Ignore validator if a subset is specified and validator is not in the subset let include_validator_delta = |idx| match &maybe_validators_subset { RewardsCalculationType::Consensus => true, - RewardsCalculationType::API(validator_subset) => { - if validator_subset.is_empty() { - true - } - else if validator_subset.contains(&idx) { - true - } - else { - false - } + RewardsCalculationType::API(validator_subset) + if validator_subset.is_empty() || validator_subset.contains(&idx) => + { + true } + _ => false, }; for (index, validator) in validator_statuses.statuses.iter().enumerate() { @@ -178,12 +189,16 @@ fn get_attestation_deltas( } if let Some((proposer_index, proposer_delta)) = proposer_delta { - if include_validator_delta(proposer_index) { - deltas - .get_mut(proposer_index) - .ok_or(Error::ValidatorStatusesInconsistent)? - .inclusion_delay_delta - .combine(proposer_delta)?; + match maybe_validators_subset { + RewardsCalculationType::Consensus => { + deltas + .get_mut(proposer_index) + .ok_or(Error::ValidatorStatusesInconsistent)? + .inclusion_delay_delta + .combine(proposer_delta)?; + } + + _ => (), } } } From 4ce8f2aa601918eb5102c143655bc8e6bd8844c3 Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Sat, 20 Jan 2024 21:02:52 +0100 Subject: [PATCH 4/8] Fix phase0 block reward in rewards API (#4929) --- .../beacon_chain/src/beacon_block_reward.rs | 90 +++++++++++++++---- beacon_node/beacon_chain/src/beacon_chain.rs | 2 +- beacon_node/beacon_chain/tests/rewards.rs | 18 +--- .../http_api/src/standard_block_rewards.rs | 4 +- 4 files changed, 76 insertions(+), 38 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index d05f7cb4ffd..d93169b086f 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -1,11 +1,10 @@ -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; +use crate::{BeaconChain, BeaconChainError, BeaconChainTypes, StateSkipConfig}; use eth2::lighthouse::StandardBlockReward; -use operation_pool::RewardCache; use safe_arith::SafeArith; use slog::error; use state_processing::{ common::{ - altair, get_attestation_participation_flag_indices, get_attesting_indices_from_state, + altair, base, get_attestation_participation_flag_indices, get_attesting_indices_from_state, }, per_block_processing::{ altair::sync_committee::compute_sync_aggregate_rewards, get_slashable_indices, @@ -15,7 +14,7 @@ use store::{ consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}, RelativeEpoch, }; -use types::{AbstractExecPayload, BeaconBlockRef, BeaconState, BeaconStateError, Hash256}; +use types::{AbstractExecPayload, BeaconBlockRef, BeaconState, BeaconStateError, EthSpec}; type BeaconBlockSubRewardValue = u64; @@ -23,7 +22,6 @@ impl BeaconChain { pub fn compute_beacon_block_reward>( &self, block: BeaconBlockRef<'_, T::EthSpec, Payload>, - block_root: Hash256, state: &mut BeaconState, ) -> Result { if block.slot() != state.slot() { @@ -33,7 +31,7 @@ impl BeaconChain { state.build_committee_cache(RelativeEpoch::Previous, &self.spec)?; state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; - self.compute_beacon_block_reward_with_cache(block, block_root, state) + self.compute_beacon_block_reward_with_cache(block, state) } // This should only be called after a committee cache has been built @@ -41,7 +39,6 @@ impl BeaconChain { fn compute_beacon_block_reward_with_cache>( &self, block: BeaconBlockRef<'_, T::EthSpec, Payload>, - block_root: Hash256, state: &BeaconState, ) -> Result { let proposer_index = block.proposer_index(); @@ -72,7 +69,7 @@ impl BeaconChain { })?; let block_attestation_reward = if let BeaconState::Base(_) = state { - self.compute_beacon_block_attestation_reward_base(block, block_root, state) + self.compute_beacon_block_attestation_reward_base(block, state) .map_err(|e| { error!( self.log, @@ -169,19 +166,74 @@ impl BeaconChain { fn compute_beacon_block_attestation_reward_base>( &self, block: BeaconBlockRef<'_, T::EthSpec, Payload>, - block_root: Hash256, state: &BeaconState, ) -> Result { - // Call compute_block_reward in the base case - // Since base does not have sync aggregate, we only grab attesation portion of the returned - // value - let mut reward_cache = RewardCache::default(); - let block_attestation_reward = self - .compute_block_reward(block, block_root, state, &mut reward_cache, true)? - .attestation_rewards - .total; - - Ok(block_attestation_reward) + // In phase0, rewards for including attestations are awarded at epoch boundaries when the corresponding + // attestations are contained in state.previous_epoch_attestations. So, if an attestation within this block has + // target = previous_epoch, it is directly inserted into previous_epoch_attestations and we need the state at + // the end of this epoch, or the attestation has target = current_epoch and thus we need the state at the end + // of the next epoch. + // We fetch these lazily, as only one might be needed depending on the block's content. + let mut current_epoch_end = None; + let mut next_epoch_end = None; + + let epoch = block.epoch(); + let mut block_reward = 0; + + for attestation in block.body().attestations() { + let processing_epoch_end = if attestation.data.target.epoch == epoch { + let next_epoch_end = match &mut next_epoch_end { + Some(next_epoch_end) => next_epoch_end, + None => { + let mut state = self.state_at_slot( + epoch.safe_add(1)?.end_slot(T::EthSpec::slots_per_epoch()), + StateSkipConfig::WithoutStateRoots, + )?; + state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; + next_epoch_end.get_or_insert(state) + } + }; + + // If the next epoch end is no longer phase0, no proposer rewards are awarded, as Altair epoch boundry + // processing kicks in. We check this here, as we know that current_epoch_end will always be phase0. + if !matches!(next_epoch_end, BeaconState::Base(_)) { + continue; + } + + next_epoch_end + } else if attestation.data.target.epoch == epoch.safe_sub(1)? { + match &mut current_epoch_end { + Some(current_epoch_end) => current_epoch_end, + None => { + let mut state = self.state_at_slot( + epoch.end_slot(T::EthSpec::slots_per_epoch()), + StateSkipConfig::WithoutStateRoots, + )?; + state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; + current_epoch_end.get_or_insert(state) + } + } + } else { + return Err(BeaconChainError::BlockRewardAttestationError); + }; + + for attester in get_attesting_indices_from_state(state, attestation)? { + let attester = attester as usize; + if !processing_epoch_end.get_validator(attester)?.slashed { + let base_reward = base::get_base_reward( + state, + attester, + processing_epoch_end.get_total_active_balance()?, + &self.spec, + )?; + let proposer_reward = + base_reward.safe_div(self.spec.proposer_reward_quotient)?; + block_reward.safe_add_assign(proposer_reward)?; + } + } + } + + Ok(block_reward) } fn compute_beacon_block_attestation_reward_altair_deneb< diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index bf418054799..4219186dcb3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5241,7 +5241,7 @@ impl BeaconChain { let mut ctxt = ConsensusContext::new(block.slot()); let consensus_block_value = self - .compute_beacon_block_reward(block.message(), Hash256::zero(), &mut state) + .compute_beacon_block_reward(block.message(), &mut state) .map(|reward| reward.total) .unwrap_or(0); diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index a78463ef5d7..734b06b0c7b 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -312,11 +312,7 @@ async fn test_verify_attestation_rewards_altair() { harness.make_block_return_pre_state(state, slot).await; let beacon_block_reward = harness .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, - ) + .compute_beacon_block_reward(signed_block.message(), &mut state) .unwrap(); let total_proposer_reward = proposal_rewards_map @@ -400,11 +396,7 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { harness.make_block_return_pre_state(state, slot).await; let beacon_block_reward = harness .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, - ) + .compute_beacon_block_reward(signed_block.message(), &mut state) .unwrap(); let total_proposer_reward = proposal_rewards_map @@ -506,11 +498,7 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep harness.make_block_return_pre_state(state, slot).await; let beacon_block_reward = harness .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, - ) + .compute_beacon_block_reward(signed_block.message(), &mut state) .unwrap(); let total_proposer_reward = proposal_rewards_map diff --git a/beacon_node/http_api/src/standard_block_rewards.rs b/beacon_node/http_api/src/standard_block_rewards.rs index 97e5a87fd3a..1ab75374ea8 100644 --- a/beacon_node/http_api/src/standard_block_rewards.rs +++ b/beacon_node/http_api/src/standard_block_rewards.rs @@ -15,12 +15,10 @@ pub fn compute_beacon_block_rewards( let block_ref = block.message(); - let block_root = block.canonical_root(); - let mut state = get_state_before_applying_block(chain.clone(), &block)?; let rewards = chain - .compute_beacon_block_reward(block_ref, block_root, &mut state) + .compute_beacon_block_reward(block_ref, &mut state) .map_err(beacon_chain_error)?; Ok((rewards, execution_optimistic, finalized)) From 320417295d7f765c056fc6f638a48c8d66d9499d Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Wed, 31 Jan 2024 02:12:56 +0100 Subject: [PATCH 5/8] Revamp phase0 reward API tests - Add test_rewards_base_slashings (testing #5101) - Improve fix to not include proposer reward in attestation reward API calculation (#4856) - Adjust test approach for phase0 tests: Pad with empty epochs to include all rewards in calculation - Simplify and unify code across all reward tests --- .../beacon_chain/src/attestation_rewards.rs | 23 +- beacon_node/beacon_chain/tests/rewards.rs | 503 +++++++----------- .../base/rewards_and_penalties.rs | 47 +- testing/ef_tests/src/cases/rewards.rs | 2 + 4 files changed, 237 insertions(+), 338 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index abd676d7389..8b63bb9301f 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -25,7 +25,7 @@ use eth2::types::ValidatorId; use state_processing::common::base::get_base_reward_from_effective_balance; use state_processing::per_epoch_processing::base::rewards_and_penalties::{ get_attestation_component_delta, get_attestation_deltas_all, get_attestation_deltas_subset, - get_inactivity_penalty_delta, get_inclusion_delay_delta, + get_inactivity_penalty_delta, get_inclusion_delay_delta, ProposerRewardCalculation, }; use state_processing::per_epoch_processing::base::validator_statuses::InclusionInfo; use state_processing::per_epoch_processing::base::{ @@ -81,13 +81,24 @@ impl BeaconChain { self.compute_ideal_rewards_base(&state, &validator_statuses.total_balances)?; let indices_to_attestation_delta = if validators.is_empty() { - get_attestation_deltas_all(&state, &validator_statuses, spec)? - .into_iter() - .enumerate() - .collect() + get_attestation_deltas_all( + &state, + &validator_statuses, + ProposerRewardCalculation::Exclude, + spec, + )? + .into_iter() + .enumerate() + .collect() } else { let validator_indices = Self::validators_ids_to_indices(&mut state, validators)?; - get_attestation_deltas_subset(&state, &validator_statuses, &validator_indices, spec)? + get_attestation_deltas_subset( + &state, + &validator_statuses, + ProposerRewardCalculation::Exclude, + &validator_indices, + spec, + )? }; let mut total_rewards = vec![]; diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index b9384494e6b..d7814bf938d 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -1,5 +1,6 @@ #![cfg(test)] +use std::array::IntoIter; use std::collections::HashMap; use beacon_chain::test_utils::{ @@ -8,13 +9,14 @@ use beacon_chain::test_utils::{ use beacon_chain::{ test_utils::{AttestationStrategy, BlockStrategy, RelativeSyncCommittee}, types::{Epoch, EthSpec, Keypair, MinimalEthSpec}, + ChainConfig, StateSkipConfig, WhenSlotSkipped, }; use eth2::lighthouse::attestation_rewards::TotalAttestationRewards; use eth2::lighthouse::StandardAttestationRewards; use eth2::types::ValidatorId; use lazy_static::lazy_static; -use types::beacon_state::Error as BeaconStateError; -use types::{BeaconState, ChainSpec, ForkName, Slot}; +use state_processing::{BlockReplayError, BlockReplayer}; +use types::{ChainSpec, ForkName, Slot}; pub const VALIDATOR_COUNT: usize = 64; @@ -25,10 +27,16 @@ lazy_static! { } fn get_harness(spec: ChainSpec) -> BeaconChainHarness> { + let chain_config = ChainConfig { + reconstruct_historic_states: true, + ..Default::default() + }; + let harness = BeaconChainHarness::builder(E::default()) .spec(spec) .keypairs(KEYPAIRS.to_vec()) .fresh_ephemeral_store() + .chain_config(chain_config) .build(); harness.advance_slot(); @@ -38,9 +46,7 @@ fn get_harness(spec: ChainSpec) -> BeaconChainHarness> { #[tokio::test] async fn test_sync_committee_rewards() { - let mut spec = E::default_spec(); - spec.altair_fork_epoch = Some(Epoch::new(0)); - + let spec = ForkName::Altair.make_genesis_spec(E::default_spec()); let harness = get_harness(spec); let num_block_produced = E::slots_per_epoch(); @@ -127,162 +133,65 @@ async fn test_sync_committee_rewards() { } #[tokio::test] -async fn test_verify_attestation_rewards_base() { - let harness = get_harness(E::default_spec()); +async fn test_rewards_base() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec); + let initial_balances = harness.get_current_state().balances().to_vec(); - // 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_slots_some_validators(E::slots_per_epoch() as usize, two_thirds_validators.clone()) + .extend_slots(E::slots_per_epoch() as usize * 2 - 1) .await; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); - - let mut proposal_rewards_map: HashMap = HashMap::new(); - - for _ in 0..E::slots_per_epoch() { - let state = harness.get_current_state(); - let slot = state.slot() + Slot::new(1); - - let ((signed_block, _maybe_blob_sidecars), mut state) = - harness.make_block_return_pre_state(state, slot).await; - let beacon_block_reward = harness - .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, - ) - .unwrap(); - let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); - - harness.extend_slots(1).await; - } - - // compute reward deltas for all validators in epoch N - let StandardAttestationRewards { - ideal_rewards, - total_rewards, - } = harness - .chain - .compute_attestation_rewards(Epoch::new(0), vec![]) - .unwrap(); - - // assert no inactivity penalty for both ideal rewards and individual validators - assert!(ideal_rewards.iter().all(|reward| reward.inactivity == 0)); - assert!(total_rewards.iter().all(|reward| reward.inactivity == 0)); - - // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); - - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().clone().into(); - assert_eq!(expected_balances, balances); + check_all_base_rewards(&harness, initial_balances).await; } #[tokio::test] -async fn test_verify_attestation_rewards_base_inactivity_leak() { - let spec = E::default_spec(); +async fn test_rewards_base_inactivity_leak() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); let harness = get_harness(spec.clone()); + let initial_balances = harness.get_current_state().balances().to_vec(); let half = VALIDATOR_COUNT / 2; let half_validators: Vec = (0..half).collect(); // target epoch is the epoch where the chain enters inactivity leak let target_epoch = &spec.min_epochs_to_inactivity_penalty + 1; - //advance until beginning of epoch N + 1 and get balances + // advance until end of target epoch harness .extend_slots_some_validators( - (E::slots_per_epoch() * (target_epoch + 1)) as usize, + ((E::slots_per_epoch() * target_epoch) - 1) as usize, half_validators.clone(), ) .await; - let initial_balances = harness.get_current_state().balances().clone(); - - // advance until epoch N + 2 and build proposal rewards map - let mut proposal_rewards_map: HashMap = HashMap::new(); - - for _ in 0..E::slots_per_epoch() { - let state = harness.get_current_state(); - let slot = state.slot() + Slot::new(1); - - let ((signed_block, _maybe_blob_sidecars), mut state) = - harness.make_block_return_pre_state(state, slot).await; - let beacon_block_reward = harness - .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, - ) - .unwrap(); - - let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); - - harness - .extend_slots_some_validators(1, half_validators.clone()) - .await; - } - - // compute reward deltas for all validators in epoch N - let StandardAttestationRewards { - ideal_rewards, - total_rewards, - } = harness - .chain - .compute_attestation_rewards(Epoch::new(target_epoch), vec![]) - .unwrap(); - - // assert inactivity penalty for both ideal rewards and individual validators - assert!(ideal_rewards.iter().all(|reward| reward.inactivity < 0)); - assert!(total_rewards.iter().all(|reward| reward.inactivity < 0)); - - // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); - - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().clone().into(); - assert_eq!(expected_balances, balances); + check_all_base_rewards(&harness, initial_balances).await; } #[tokio::test] -async fn test_verify_attestation_rewards_base_inactivity_leak_justification_epoch() { - let spec = E::default_spec(); +async fn test_rewards_base_inactivity_leak_justification_epoch() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); let harness = get_harness(spec.clone()); + let initial_balances = harness.get_current_state().balances().to_vec(); let half = VALIDATOR_COUNT / 2; let half_validators: Vec = (0..half).collect(); // target epoch is the epoch where the chain enters inactivity leak - let mut target_epoch = &spec.min_epochs_to_inactivity_penalty + 2; + let mut target_epoch = &spec.min_epochs_to_inactivity_penalty + 1; - // advance until beginning of epoch N + 2 + // advance until end of target epoch harness .extend_chain( - (E::slots_per_epoch() * (target_epoch + 1)) as usize, + ((E::slots_per_epoch() * target_epoch) - 1) as usize, BlockStrategy::OnCanonicalHead, AttestationStrategy::SomeValidators(half_validators.clone()), ) .await; - // advance to create first justification epoch and get initial balances + // advance to create first justification epoch harness.extend_slots(E::slots_per_epoch() as usize).await; target_epoch += 1; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); - //assert previous_justified_checkpoint matches 0 as we were in inactivity leak from beginning + // assert previous_justified_checkpoint matches 0 as we were in inactivity leak from beginning assert_eq!( 0, harness @@ -292,10 +201,12 @@ async fn test_verify_attestation_rewards_base_inactivity_leak_justification_epoc .as_u64() ); - // extend slots to beginning of epoch N + 1 + // extend slots to end of epoch target_epoch + 2 harness.extend_slots(E::slots_per_epoch() as usize).await; - //assert target epoch and previous_justified_checkpoint match + check_all_base_rewards(&harness, initial_balances).await; + + // assert target epoch and previous_justified_checkpoint match assert_eq!( target_epoch, harness @@ -304,31 +215,29 @@ async fn test_verify_attestation_rewards_base_inactivity_leak_justification_epoc .epoch .as_u64() ); +} - // compute reward deltas for all validators in epoch N - let StandardAttestationRewards { - ideal_rewards, - total_rewards, - } = harness - .chain - .compute_attestation_rewards(Epoch::new(target_epoch), vec![]) - .unwrap(); +#[tokio::test] +async fn test_rewards_base_slashings() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec); + let mut initial_balances = harness.get_current_state().balances().to_vec(); - // assert we successfully get ideal rewards for justified epoch out of inactivity leak - assert!(ideal_rewards - .iter() - .all(|reward| reward.head > 0 && reward.target > 0 && reward.source > 0)); + harness + .extend_slots(E::slots_per_epoch() as usize - 1) + .await; - // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + harness.add_attester_slashing(vec![0]).unwrap(); + let slashed_balance = initial_balances.get_mut(0).unwrap(); + *slashed_balance -= *slashed_balance / harness.spec.min_slashing_penalty_quotient; - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().clone().into(); - assert_eq!(expected_balances, balances); + harness.extend_slots(E::slots_per_epoch() as usize).await; + + check_all_base_rewards(&harness, initial_balances).await; } #[tokio::test] -async fn test_verify_attestation_rewards_altair() { +async fn test_rewards_altair() { let spec = ForkName::Altair.make_genesis_spec(E::default_spec()); let harness = get_harness(spec.clone()); let target_epoch = 0; @@ -337,11 +246,11 @@ async fn test_verify_attestation_rewards_altair() { harness .extend_slots((E::slots_per_epoch() * (target_epoch + 1)) as usize) .await; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); + let mut expected_balances = harness.get_current_state().balances().to_vec(); // advance until epoch N + 2 and build proposal rewards map - let mut proposal_rewards_map: HashMap = HashMap::new(); - let mut sync_committee_rewards_map: HashMap = HashMap::new(); + let mut proposal_rewards_map = HashMap::new(); + let mut sync_committee_rewards_map = HashMap::new(); for _ in 0..E::slots_per_epoch() { let state = harness.get_current_state(); let slot = state.slot() + Slot::new(1); @@ -355,11 +264,9 @@ async fn test_verify_attestation_rewards_altair() { .unwrap(); let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + .entry(beacon_block_reward.proposer_index) + .or_insert(0); + *total_proposer_reward += beacon_block_reward.total as i64; // calculate sync committee rewards / penalties let reward_payload = harness @@ -367,13 +274,12 @@ async fn test_verify_attestation_rewards_altair() { .compute_sync_committee_rewards(signed_block.message(), &mut state) .unwrap(); - reward_payload.iter().for_each(|reward| { - let mut amount = *sync_committee_rewards_map - .get(&reward.validator_index) - .unwrap_or(&0); - amount += reward.reward; - sync_committee_rewards_map.insert(reward.validator_index, amount); - }); + for reward in reward_payload { + let total_sync_reward = sync_committee_rewards_map + .entry(reward.validator_index) + .or_insert(0); + *total_sync_reward += reward.reward; + } harness.extend_slots(1).await; } @@ -393,10 +299,9 @@ async fn test_verify_attestation_rewards_altair() { .all(|reward| reward.head > 0 && reward.target > 0 && reward.source > 0)); // apply attestation, proposal, and sync committee rewards and penalties to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); - let expected_balances = - apply_sync_committee_rewards(&sync_committee_rewards_map, expected_balances); + apply_attestation_rewards(&mut expected_balances, total_rewards); + apply_other_rewards(&mut expected_balances, &proposal_rewards_map); + apply_other_rewards(&mut expected_balances, &sync_committee_rewards_map); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); @@ -405,7 +310,7 @@ async fn test_verify_attestation_rewards_altair() { } #[tokio::test] -async fn test_verify_attestation_rewards_altair_inactivity_leak() { +async fn test_rewards_altair_inactivity_leak() { let spec = ForkName::Altair.make_genesis_spec(E::default_spec()); let harness = get_harness(spec.clone()); @@ -421,11 +326,11 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { half_validators.clone(), ) .await; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); + let mut expected_balances = harness.get_current_state().balances().to_vec(); // advance until epoch N + 2 and build proposal rewards map - let mut proposal_rewards_map: HashMap = HashMap::new(); - let mut sync_committee_rewards_map: HashMap = HashMap::new(); + let mut proposal_rewards_map = HashMap::new(); + let mut sync_committee_rewards_map = HashMap::new(); for _ in 0..E::slots_per_epoch() { let state = harness.get_current_state(); let slot = state.slot() + Slot::new(1); @@ -439,11 +344,9 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { .unwrap(); let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + .entry(beacon_block_reward.proposer_index) + .or_insert(0i64); + *total_proposer_reward += beacon_block_reward.total as i64; // calculate sync committee rewards / penalties let reward_payload = harness @@ -451,13 +354,12 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { .compute_sync_committee_rewards(signed_block.message(), &mut state) .unwrap(); - reward_payload.iter().for_each(|reward| { - let mut amount = *sync_committee_rewards_map - .get(&reward.validator_index) - .unwrap_or(&0); - amount += reward.reward; - sync_committee_rewards_map.insert(reward.validator_index, amount); - }); + for reward in reward_payload { + let total_sync_reward = sync_committee_rewards_map + .entry(reward.validator_index) + .or_insert(0); + *total_sync_reward += reward.reward; + } harness .extend_slots_some_validators(1, half_validators.clone()) @@ -483,10 +385,9 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { .all(|reward| reward.inactivity < 0)); // apply attestation, proposal, and sync committee rewards and penalties to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); - let expected_balances = - apply_sync_committee_rewards(&sync_committee_rewards_map, expected_balances); + apply_attestation_rewards(&mut expected_balances, total_rewards); + apply_other_rewards(&mut expected_balances, &proposal_rewards_map); + apply_other_rewards(&mut expected_balances, &sync_committee_rewards_map); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); @@ -495,7 +396,7 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { } #[tokio::test] -async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_epoch() { +async fn test_rewards_altair_inactivity_leak_justification_epoch() { let spec = ForkName::Altair.make_genesis_spec(E::default_spec()); let harness = get_harness(spec.clone()); @@ -523,11 +424,11 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep // advance for first justification epoch and get balances harness.extend_slots(E::slots_per_epoch() as usize).await; target_epoch += 1; - let initial_balances: Vec = harness.get_current_state().balances().clone().into(); + let mut expected_balances = harness.get_current_state().balances().to_vec(); // advance until epoch N + 2 and build proposal rewards map - let mut proposal_rewards_map: HashMap = HashMap::new(); - let mut sync_committee_rewards_map: HashMap = HashMap::new(); + let mut proposal_rewards_map = HashMap::new(); + let mut sync_committee_rewards_map = HashMap::new(); for _ in 0..E::slots_per_epoch() { let state = harness.get_current_state(); let slot = state.slot() + Slot::new(1); @@ -541,11 +442,9 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep .unwrap(); let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + .entry(beacon_block_reward.proposer_index) + .or_insert(0); + *total_proposer_reward += beacon_block_reward.total as i64; // calculate sync committee rewards / penalties let reward_payload = harness @@ -553,13 +452,12 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep .compute_sync_committee_rewards(signed_block.message(), &mut state) .unwrap(); - reward_payload.iter().for_each(|reward| { - let mut amount = *sync_committee_rewards_map - .get(&reward.validator_index) - .unwrap_or(&0); - amount += reward.reward; - sync_committee_rewards_map.insert(reward.validator_index, amount); - }); + for reward in reward_payload { + let total_sync_reward = sync_committee_rewards_map + .entry(reward.validator_index) + .or_insert(0); + *total_sync_reward += reward.reward; + } harness.extend_slots(1).await; } @@ -589,10 +487,9 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep .all(|reward| reward.head > 0 && reward.target > 0 && reward.source > 0)); // apply attestation, proposal, and sync committee rewards and penalties to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); - let expected_balances = - apply_sync_committee_rewards(&sync_committee_rewards_map, expected_balances); + apply_attestation_rewards(&mut expected_balances, total_rewards); + apply_other_rewards(&mut expected_balances, &proposal_rewards_map); + apply_other_rewards(&mut expected_balances, &sync_committee_rewards_map); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); @@ -600,8 +497,13 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak_justification_ep } #[tokio::test] -async fn test_verify_attestation_rewards_base_subset_only() { - let harness = get_harness(E::default_spec()); +async fn test_rewards_base_subset_only() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec); + let initial_balances = harness.get_current_state().balances().to_vec(); + + // a subset of validators to compute attestation rewards for + let validators_subset = (0..16).chain(56..64).collect::>(); // epoch 0 (N), only two thirds of validators vote. let two_thirds = (VALIDATOR_COUNT / 3) * 2; @@ -610,124 +512,115 @@ async fn test_verify_attestation_rewards_base_subset_only() { .extend_slots_some_validators(E::slots_per_epoch() as usize, two_thirds_validators.clone()) .await; - // a small subset of validators to compute attestation rewards for - let validators_subset = [0, VALIDATOR_COUNT / 2, VALIDATOR_COUNT - 1]; + check_all_base_rewards_for_subset(&harness, initial_balances, validators_subset).await; +} - // capture balances before transitioning to N + 2 - let initial_balances = get_validator_balances(harness.get_current_state(), &validators_subset); +async fn check_all_base_rewards( + harness: &BeaconChainHarness>, + balances: Vec, +) { + check_all_base_rewards_for_subset(harness, balances, vec![]).await; +} - // extend slots to beginning of epoch N + 2 - let mut proposal_rewards_map: HashMap = HashMap::new(); +async fn check_all_base_rewards_for_subset( + harness: &BeaconChainHarness>, + mut balances: Vec, + validator_subset: Vec, +) { + let validator_subset_ids: Vec = validator_subset + .iter() + .map(|&idx| ValidatorId::Index(idx)) + .collect(); - for _ in 0..E::slots_per_epoch() { - let state = harness.get_current_state(); - let slot = state.slot() + Slot::new(1); + // capture the amount of epochs generated by the caller + let epochs = harness.get_current_slot().epoch(E::slots_per_epoch()) + 1; - // calculate beacon block rewards / penalties - let ((signed_block, _maybe_blob_sidecars), mut state) = - harness.make_block_return_pre_state(state, slot).await; - let beacon_block_reward = harness + // advance two empty epochs to ensure balances are updated by the epoch boundaries + for _ in 0..E::slots_per_epoch() * 2 { + harness.advance_slot(); + } + // fill one slot to ensure state is updated + harness.extend_slots(1).await; + + // calculate proposal awards + let mut proposal_rewards_map = HashMap::new(); + for slot in 1..(E::slots_per_epoch() * epochs.as_u64()) { + if let Some(block) = harness .chain - .compute_beacon_block_reward( - signed_block.message(), - signed_block.canonical_root(), - &mut state, + .block_at_slot(Slot::new(slot), WhenSlotSkipped::None) + .unwrap() + { + let parent_state = harness + .chain + .state_at_slot(Slot::new(slot - 1), StateSkipConfig::WithoutStateRoots) + .unwrap(); + + let mut pre_state = BlockReplayer::>::new( + parent_state, + &harness.spec, ) - .unwrap(); - - let total_proposer_reward = proposal_rewards_map - .get(&beacon_block_reward.proposer_index) - .unwrap_or(&0u64) - + beacon_block_reward.total; - - proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); - - harness - .extend_slots_some_validators(1, two_thirds_validators.clone()) - .await; + .no_signature_verification() + .minimal_block_root_verification() + .apply_blocks(vec![], Some(block.slot())) + .unwrap() + .into_state(); + + let beacon_block_reward = harness + .chain + .compute_beacon_block_reward(block.message(), &mut pre_state) + .unwrap(); + let total_proposer_reward = proposal_rewards_map + .entry(beacon_block_reward.proposer_index) + .or_insert(0); + *total_proposer_reward += beacon_block_reward.total as i64; + } } + apply_other_rewards(&mut balances, &proposal_rewards_map); - 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) - .unwrap(); + for epoch in 0..epochs.as_u64() { + // compute reward deltas in epoch + let total_rewards = harness + .chain + .compute_attestation_rewards(Epoch::new(epoch), validator_subset_ids.clone()) + .unwrap() + .total_rewards; - // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - let expected_balances = apply_beacon_block_rewards(&proposal_rewards_map, expected_balances); + // apply attestation rewards to balances + apply_attestation_rewards(&mut 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); + let actual_balances: Vec = harness.get_current_state().balances().to_vec(); + if validator_subset.is_empty() { + assert_eq!(balances, actual_balances); + } else { + for validator in validator_subset { + assert_eq!( + balances[validator as usize], + actual_balances[validator as usize] + ); + } + } } /// Apply a vec of `TotalAttestationRewards` to initial balances, and return fn apply_attestation_rewards( - initial_balances: &[u64], + balances: &mut [u64], attestation_rewards: Vec, -) -> Vec { - initial_balances - .iter() - .zip(attestation_rewards) - .map(|(&initial_balance, rewards)| { - let expected_balance = initial_balance as i64 - + rewards.head - + rewards.source - + rewards.target - + rewards.inclusion_delay.map(|q| q.value).unwrap_or(0) as i64 - + rewards.inactivity; - expected_balance as u64 - }) - .collect::>() -} - -fn get_validator_balances(state: BeaconState, validators: &[usize]) -> Vec { - validators - .iter() - .flat_map(|&id| { - state - .balances() - .get(id) - .cloned() - .ok_or(BeaconStateError::BalancesOutOfBounds(id)) - }) - .collect() -} - -fn apply_beacon_block_rewards( - proposal_rewards_map: &HashMap, - expected_balances: Vec, -) -> Vec { - let calculated_balances = expected_balances - .iter() - .enumerate() - .map(|(i, balance)| balance + proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64)) - .collect(); - - calculated_balances +) { + for rewards in attestation_rewards { + let balance = balances.get_mut(rewards.validator_index as usize).unwrap(); + *balance = (*balance as i64 + + rewards.head + + rewards.source + + rewards.target + + rewards.inclusion_delay.map(|q| q.value).unwrap_or(0) as i64 + + rewards.inactivity) as u64; + } } -fn apply_sync_committee_rewards( - sync_committee_rewards_map: &HashMap, - expected_balances: Vec, -) -> Vec { - let calculated_balances = expected_balances - .iter() - .enumerate() - .map(|(i, balance)| { - (*balance as i64 + sync_committee_rewards_map.get(&(i as u64)).unwrap_or(&0i64)) - .unsigned_abs() - }) - .collect(); - - calculated_balances +fn apply_other_rewards(balances: &mut [u64], rewards_map: &HashMap) { + for (i, balance) in balances.iter_mut().enumerate() { + *balance = balance.saturating_add_signed(*rewards_map.get(&(i as u64)).unwrap_or(&0)); + } } 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 598bed0df33..a7feda0c569 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 @@ -43,9 +43,9 @@ impl AttestationDelta { } #[derive(Debug)] -pub enum RewardsCalculationType { - Consensus, - API(Vec), +pub enum ProposerRewardCalculation { + Include, + Exclude, } /// Apply attester and proposer rewards. @@ -65,10 +65,10 @@ pub fn process_rewards_and_penalties( return Err(Error::ValidatorStatusesInconsistent); } - let deltas = get_attestation_deltas( + let deltas = get_attestation_deltas_all( state, validator_statuses, - RewardsCalculationType::Consensus, + ProposerRewardCalculation::Include, spec, )?; @@ -87,14 +87,10 @@ pub fn process_rewards_and_penalties( pub fn get_attestation_deltas_all( state: &BeaconState, validator_statuses: &ValidatorStatuses, + proposer_reward: ProposerRewardCalculation, spec: &ChainSpec, ) -> Result, Error> { - get_attestation_deltas( - state, - validator_statuses, - RewardsCalculationType::API(vec![]), - spec, - ) + get_attestation_deltas(state, validator_statuses, proposer_reward, None, spec) } /// Apply rewards for participation in attestations during the previous epoch, and only compute @@ -102,13 +98,15 @@ pub fn get_attestation_deltas_all( pub fn get_attestation_deltas_subset( state: &BeaconState, validator_statuses: &ValidatorStatuses, + proposer_reward: ProposerRewardCalculation, validators_subset: &Vec, spec: &ChainSpec, ) -> Result, Error> { get_attestation_deltas( state, validator_statuses, - RewardsCalculationType::API(validators_subset.clone()), + proposer_reward, + Some(validators_subset), spec, ) .map(|deltas| { @@ -128,7 +126,8 @@ pub fn get_attestation_deltas_subset( fn get_attestation_deltas( state: &BeaconState, validator_statuses: &ValidatorStatuses, - maybe_validators_subset: RewardsCalculationType, + proposer_reward: ProposerRewardCalculation, + maybe_validators_subset: Option<&Vec>, spec: &ChainSpec, ) -> Result, Error> { let previous_epoch = state.previous_epoch(); @@ -141,15 +140,11 @@ fn get_attestation_deltas( let total_balances = &validator_statuses.total_balances; - // Check if the calculation is for Consensus or Rewards API and Ignore validator if a subset is specified and validator is not in the subset - let include_validator_delta = |idx| match &maybe_validators_subset { - RewardsCalculationType::Consensus => true, - RewardsCalculationType::API(validator_subset) - if validator_subset.is_empty() || validator_subset.contains(&idx) => - { - true - } - _ => false, + // 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() { @@ -188,17 +183,15 @@ fn get_attestation_deltas( .combine(inactivity_penalty_delta)?; } - if let Some((proposer_index, proposer_delta)) = proposer_delta { - match maybe_validators_subset { - RewardsCalculationType::Consensus => { + if let ProposerRewardCalculation::Include = proposer_reward { + if let Some((proposer_index, proposer_delta)) = proposer_delta { + if include_validator_delta(proposer_index) { deltas .get_mut(proposer_index) .ok_or(Error::ValidatorStatusesInconsistent)? .inclusion_delay_delta .combine(proposer_delta)?; } - - _ => (), } } } diff --git a/testing/ef_tests/src/cases/rewards.rs b/testing/ef_tests/src/cases/rewards.rs index bb41f6fe12f..403bbd1aaf5 100644 --- a/testing/ef_tests/src/cases/rewards.rs +++ b/testing/ef_tests/src/cases/rewards.rs @@ -5,6 +5,7 @@ use compare_fields_derive::CompareFields; use serde::Deserialize; use ssz::four_byte_option_impl; use ssz_derive::{Decode, Encode}; +use state_processing::per_epoch_processing::base::rewards_and_penalties::ProposerRewardCalculation; use state_processing::{ per_epoch_processing::{ altair::{self, rewards_and_penalties::get_flag_index_deltas, ParticipationCache}, @@ -121,6 +122,7 @@ impl Case for RewardsTest { let deltas = base::rewards_and_penalties::get_attestation_deltas_all( &state, &validator_statuses, + ProposerRewardCalculation::Include, spec, )?; From e45de97684638d4f39244218b012d02704909747 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 10 Sep 2024 12:10:49 +1000 Subject: [PATCH 6/8] Fix merge fallout --- .../beacon_chain/src/beacon_block_reward.rs | 19 ++++++++++--------- beacon_node/beacon_chain/tests/rewards.rs | 6 +++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index bc7462c92f4..852111c0058 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -4,9 +4,9 @@ use safe_arith::SafeArith; use slog::error; use state_processing::{ common::{ - altair, base, get_attestation_participation_flag_indices, get_attesting_indices_from_state, + base::{self, SqrtTotalActiveBalance}, + get_attestation_participation_flag_indices, get_attesting_indices_from_state, }, - common::{get_attestation_participation_flag_indices, get_attesting_indices_from_state}, epoch_cache::initialize_epoch_cache, per_block_processing::{ altair::sync_committee::compute_sync_aggregate_rewards, get_slashable_indices, @@ -184,7 +184,7 @@ impl BeaconChain { let mut block_reward = 0; for attestation in block.body().attestations() { - let processing_epoch_end = if attestation.data.target.epoch == epoch { + let processing_epoch_end = if attestation.data().target.epoch == epoch { let next_epoch_end = match &mut next_epoch_end { Some(next_epoch_end) => next_epoch_end, None => { @@ -204,7 +204,7 @@ impl BeaconChain { } next_epoch_end - } else if attestation.data.target.epoch == epoch.safe_sub(1)? { + } else if attestation.data().target.epoch == epoch.safe_sub(1)? { match &mut current_epoch_end { Some(current_epoch_end) => current_epoch_end, None => { @@ -220,13 +220,14 @@ impl BeaconChain { return Err(BeaconChainError::BlockRewardAttestationError); }; + let sqrt_total_active_balance = + SqrtTotalActiveBalance::new(processing_epoch_end.get_total_active_balance()?); for attester in get_attesting_indices_from_state(state, attestation)? { - let attester = attester as usize; - if !processing_epoch_end.get_validator(attester)?.slashed { + let validator = processing_epoch_end.get_validator(attester as usize)?; + if !validator.slashed { let base_reward = base::get_base_reward( - state, - attester, - processing_epoch_end.get_total_active_balance()?, + validator.effective_balance, + sqrt_total_active_balance, &self.spec, )?; let proposer_reward = diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index b5a4fac7269..aae70d028df 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -16,7 +16,7 @@ use eth2::lighthouse::attestation_rewards::TotalAttestationRewards; use eth2::lighthouse::StandardAttestationRewards; use eth2::types::ValidatorId; use state_processing::{BlockReplayError, BlockReplayer}; -use types::{BeaconState, BeaconStateError, ChainSpec, ForkName, Slot}; +use types::{ChainSpec, ForkName, Slot}; pub const VALIDATOR_COUNT: usize = 64; @@ -189,7 +189,7 @@ async fn test_rewards_base_inactivity_leak_justification_epoch() { // advance to create first justification epoch harness.extend_slots(E::slots_per_epoch() as usize).await; target_epoch += 1; - let initial_balances: Vec = harness.get_current_state().balances().to_vec(); + let mut expected_balances = harness.get_current_state().balances().to_vec(); // assert previous_justified_checkpoint matches 0 as we were in inactivity leak from beginning assert_eq!( @@ -231,7 +231,7 @@ async fn test_rewards_base_inactivity_leak_justification_epoch() { .all(|reward| reward.head > 0 && reward.target > 0 && reward.source > 0)); // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + apply_attestation_rewards(&mut expected_balances, total_rewards); // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().to_vec(); From 4fd0e50bf17e4d63b2905a797c8322322fc3a216 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 10 Sep 2024 12:15:25 +1000 Subject: [PATCH 7/8] Remove junk revived in merge --- beacon_node/beacon_chain/tests/rewards.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index aae70d028df..709f4738301 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -189,7 +189,6 @@ async fn test_rewards_base_inactivity_leak_justification_epoch() { // advance to create first justification epoch harness.extend_slots(E::slots_per_epoch() as usize).await; target_epoch += 1; - let mut expected_balances = harness.get_current_state().balances().to_vec(); // assert previous_justified_checkpoint matches 0 as we were in inactivity leak from beginning assert_eq!( @@ -215,27 +214,6 @@ async fn test_rewards_base_inactivity_leak_justification_epoch() { .epoch .as_u64() ); - - // compute reward deltas for all validators in epoch N - let StandardAttestationRewards { - ideal_rewards, - total_rewards, - } = harness - .chain - .compute_attestation_rewards(Epoch::new(target_epoch), vec![]) - .unwrap(); - - // assert we successfully get ideal rewards for justified epoch out of inactivity leak - assert!(ideal_rewards - .iter() - .all(|reward| reward.head > 0 && reward.target > 0 && reward.source > 0)); - - // apply attestation rewards to initial balances - apply_attestation_rewards(&mut expected_balances, total_rewards); - - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().to_vec(); - assert_eq!(expected_balances, balances); } #[tokio::test] From 2007fd406675058a7b32defc297f8dc8b4ccfe73 Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Wed, 11 Sep 2024 20:15:31 +0200 Subject: [PATCH 8/8] Address review - check for attestations with lower inclusion delay - check for double attestations in block - add test --- .../beacon_chain/src/beacon_block_reward.rs | 45 +++++++++-- beacon_node/beacon_chain/tests/rewards.rs | 75 +++++++++++++++++-- 2 files changed, 110 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index 852111c0058..e0bb79bf38b 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -1,7 +1,9 @@ use crate::{BeaconChain, BeaconChainError, BeaconChainTypes, StateSkipConfig}; +use attesting_indices_base::get_attesting_indices; use eth2::lighthouse::StandardBlockReward; use safe_arith::SafeArith; use slog::error; +use state_processing::common::attesting_indices_base; use state_processing::{ common::{ base::{self, SqrtTotalActiveBalance}, @@ -12,6 +14,7 @@ use state_processing::{ altair::sync_committee::compute_sync_aggregate_rewards, get_slashable_indices, }, }; +use std::collections::HashSet; use store::{ consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}, RelativeEpoch, @@ -183,16 +186,17 @@ impl BeaconChain { let epoch = block.epoch(); let mut block_reward = 0; + let mut rewarded_attesters = HashSet::new(); + for attestation in block.body().attestations() { let processing_epoch_end = if attestation.data().target.epoch == epoch { let next_epoch_end = match &mut next_epoch_end { Some(next_epoch_end) => next_epoch_end, None => { - let mut state = self.state_at_slot( + let state = self.state_at_slot( epoch.safe_add(1)?.end_slot(T::EthSpec::slots_per_epoch()), StateSkipConfig::WithoutStateRoots, )?; - state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; next_epoch_end.get_or_insert(state) } }; @@ -208,11 +212,10 @@ impl BeaconChain { match &mut current_epoch_end { Some(current_epoch_end) => current_epoch_end, None => { - let mut state = self.state_at_slot( + let state = self.state_at_slot( epoch.end_slot(T::EthSpec::slots_per_epoch()), StateSkipConfig::WithoutStateRoots, )?; - state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; current_epoch_end.get_or_insert(state) } } @@ -220,11 +223,20 @@ impl BeaconChain { return Err(BeaconChainError::BlockRewardAttestationError); }; + let inclusion_delay = state.slot().safe_sub(attestation.data().slot)?.as_u64(); let sqrt_total_active_balance = SqrtTotalActiveBalance::new(processing_epoch_end.get_total_active_balance()?); for attester in get_attesting_indices_from_state(state, attestation)? { let validator = processing_epoch_end.get_validator(attester as usize)?; - if !validator.slashed { + if !validator.slashed + && !rewarded_attesters.contains(&attester) + && !has_earlier_attestation( + state, + processing_epoch_end, + inclusion_delay, + attester, + )? + { let base_reward = base::get_base_reward( validator.effective_balance, sqrt_total_active_balance, @@ -233,6 +245,7 @@ impl BeaconChain { let proposer_reward = base_reward.safe_div(self.spec.proposer_reward_quotient)?; block_reward.safe_add_assign(proposer_reward)?; + rewarded_attesters.insert(attester); } } } @@ -300,3 +313,25 @@ impl BeaconChain { Ok(total_proposer_reward) } } + +fn has_earlier_attestation( + state: &BeaconState, + processing_epoch_end: &BeaconState, + inclusion_delay: u64, + attester: u64, +) -> Result { + if inclusion_delay > 1 { + for epoch_att in processing_epoch_end.previous_epoch_attestations()? { + if epoch_att.inclusion_delay < inclusion_delay { + let committee = + state.get_beacon_committee(epoch_att.data.slot, epoch_att.data.index)?; + let earlier_attesters = + get_attesting_indices::(committee.committee, &epoch_att.aggregation_bits)?; + if earlier_attesters.contains(&attester) { + return Ok(true); + } + } + } + } + Ok(false) +} diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 709f4738301..323f4f38eb2 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -1,21 +1,21 @@ #![cfg(test)] -use std::array::IntoIter; -use std::collections::HashMap; -use std::sync::LazyLock; - +use beacon_chain::block_verification_types::AsBlock; use beacon_chain::test_utils::{ generate_deterministic_keypairs, BeaconChainHarness, EphemeralHarnessType, }; use beacon_chain::{ test_utils::{AttestationStrategy, BlockStrategy, RelativeSyncCommittee}, types::{Epoch, EthSpec, Keypair, MinimalEthSpec}, - ChainConfig, StateSkipConfig, WhenSlotSkipped, + BlockError, ChainConfig, StateSkipConfig, WhenSlotSkipped, }; use eth2::lighthouse::attestation_rewards::TotalAttestationRewards; use eth2::lighthouse::StandardAttestationRewards; use eth2::types::ValidatorId; use state_processing::{BlockReplayError, BlockReplayer}; +use std::array::IntoIter; +use std::collections::HashMap; +use std::sync::{Arc, LazyLock}; use types::{ChainSpec, ForkName, Slot}; pub const VALIDATOR_COUNT: usize = 64; @@ -235,6 +235,71 @@ async fn test_rewards_base_slashings() { check_all_base_rewards(&harness, initial_balances).await; } +#[tokio::test] +async fn test_rewards_base_multi_inclusion() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec); + let initial_balances = harness.get_current_state().balances().to_vec(); + + harness.extend_slots(2).await; + + let prev_block = harness.chain.head_beacon_block(); + + harness.extend_slots(1).await; + + harness.advance_slot(); + let slot = harness.get_current_slot(); + let mut block = + // pin to reduce stack size for clippy + Box::pin( + harness.make_block_with_modifier(harness.get_current_state(), slot, |block| { + // add one attestation from the same block + let attestations = &mut block.body_base_mut().unwrap().attestations; + attestations + .push(attestations.first().unwrap().clone()) + .unwrap(); + + // add one attestation from the previous block + let attestation = prev_block + .as_block() + .message_base() + .unwrap() + .body + .attestations + .first() + .unwrap() + .clone(); + attestations.push(attestation).unwrap(); + }), + ) + .await + .0; + + // funky hack: on first try, the state root will mismatch due to our modification + // thankfully, the correct state root is reported back, so we just take that one :^) + // there probably is a better way... + let Err(BlockError::StateRootMismatch { local, .. }) = harness + .process_block(slot, block.0.canonical_root(), block.clone()) + .await + else { + panic!("unexpected match of state root"); + }; + let mut new_block = block.0.message_base().unwrap().clone(); + new_block.state_root = local; + block.0 = Arc::new(harness.sign_beacon_block(new_block.into(), &harness.get_current_state())); + harness + .process_block(slot, block.0.canonical_root(), block.clone()) + .await + .unwrap(); + + harness + .extend_slots(E::slots_per_epoch() as usize * 2 - 4) + .await; + + // pin to reduce stack size for clippy + Box::pin(check_all_base_rewards(&harness, initial_balances)).await; +} + #[tokio::test] async fn test_rewards_altair() { let spec = ForkName::Altair.make_genesis_spec(E::default_spec());