diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index d48a83130e..87b7384ea6 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -11,7 +11,7 @@ use state_processing::per_epoch_processing::altair::{ }; 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/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index 33567001e3..e0bb79bf38 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -1,20 +1,25 @@ -use crate::{BeaconChain, BeaconChainError, BeaconChainTypes}; +use crate::{BeaconChain, BeaconChainError, BeaconChainTypes, StateSkipConfig}; +use attesting_indices_base::get_attesting_indices; use eth2::lighthouse::StandardBlockReward; -use operation_pool::RewardCache; use safe_arith::SafeArith; use slog::error; +use state_processing::common::attesting_indices_base; use state_processing::{ - common::{get_attestation_participation_flag_indices, get_attesting_indices_from_state}, + common::{ + base::{self, SqrtTotalActiveBalance}, + 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, }, }; +use std::collections::HashSet; 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; @@ -22,7 +27,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 +37,7 @@ impl BeaconChain { state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; initialize_epoch_cache(state, &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 +45,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 +75,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 +172,85 @@ 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; + // 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; - Ok(block_attestation_reward) + 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 state = self.state_at_slot( + epoch.safe_add(1)?.end_slot(T::EthSpec::slots_per_epoch()), + StateSkipConfig::WithoutStateRoots, + )?; + 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 state = self.state_at_slot( + epoch.end_slot(T::EthSpec::slots_per_epoch()), + StateSkipConfig::WithoutStateRoots, + )?; + current_epoch_end.get_or_insert(state) + } + } + } else { + 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 + && !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, + &self.spec, + )?; + let proposer_reward = + base_reward.safe_div(self.spec.proposer_reward_quotient)?; + block_reward.safe_add_assign(proposer_reward)?; + rewarded_attesters.insert(attester); + } + } + } + + Ok(block_reward) } fn compute_beacon_block_attestation_reward_altair_deneb< @@ -244,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/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index bf660c9eaf..d83955854d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5639,7 +5639,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 f04f4062f1..323f4f38eb 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -1,20 +1,22 @@ #![cfg(test)] -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}, + BlockError, ChainConfig, StateSkipConfig, WhenSlotSkipped, }; use eth2::lighthouse::attestation_rewards::TotalAttestationRewards; use eth2::lighthouse::StandardAttestationRewards; use eth2::types::ValidatorId; -use types::beacon_state::Error as BeaconStateError; -use types::{BeaconState, ChainSpec, ForkName, Slot}; +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; @@ -24,10 +26,16 @@ static KEYPAIRS: LazyLock> = LazyLock::new(|| generate_deterministic_keypairs(VALIDATOR_COUNT)); 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(); @@ -37,9 +45,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(); @@ -126,123 +132,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_chain( - E::slots_per_epoch() as usize, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::SomeValidators(two_thirds_validators), - ) + .extend_slots(E::slots_per_epoch() as usize * 2 - 1) .await; - let initial_balances: Vec = harness.get_current_state().balances().to_vec(); - - // extend slots to beginning of epoch N + 2 - harness.extend_slots(E::slots_per_epoch() as usize).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); - - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().to_vec(); - 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_chain( - (E::slots_per_epoch() * (target_epoch + 1)) as usize, - BlockStrategy::OnCanonicalHead, - AttestationStrategy::SomeValidators(half_validators.clone()), + .extend_slots_some_validators( + ((E::slots_per_epoch() * target_epoch) - 1) as usize, + half_validators.clone(), ) .await; - let initial_balances: Vec = harness.get_current_state().balances().to_vec(); - // 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(); - - // 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); - - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().to_vec(); - 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().to_vec(); - //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 @@ -252,10 +200,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 @@ -264,31 +214,94 @@ 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(); - - // 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 - let expected_balances = apply_attestation_rewards(&initial_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] -async fn test_verify_attestation_rewards_altair() { +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(); + + harness + .extend_slots(E::slots_per_epoch() as usize - 1) + .await; + + 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; + + harness.extend_slots(E::slots_per_epoch() as usize).await; + + 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()); let harness = get_harness(spec.clone()); let target_epoch = 0; @@ -297,11 +310,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().to_vec(); + 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); @@ -311,19 +324,13 @@ 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 - .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 @@ -331,13 +338,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; } @@ -357,10 +363,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().to_vec(); @@ -369,7 +374,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()); @@ -385,11 +390,11 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { half_validators.clone(), ) .await; - let initial_balances: Vec = harness.get_current_state().balances().to_vec(); + 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); @@ -399,19 +404,13 @@ 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 - .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 @@ -419,13 +418,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()) @@ -451,10 +449,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().to_vec(); @@ -463,7 +460,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()); @@ -491,11 +488,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().to_vec(); + 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); @@ -505,19 +502,13 @@ 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 - .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 @@ -525,13 +516,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; } @@ -561,10 +551,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().to_vec(); @@ -572,109 +561,130 @@ 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; 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 - 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 - 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)) +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(); - // 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(); + // capture the amount of epochs generated by the caller + let epochs = harness.get_current_slot().epoch(E::slots_per_epoch()) + 1; - // apply attestation rewards to initial balances - let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + // 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 + .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, + ) + .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); + + 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 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::>() +) { + 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 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 -} - -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/beacon_node/http_api/src/standard_block_rewards.rs b/beacon_node/http_api/src/standard_block_rewards.rs index 97e5a87fd3..1ab75374ea 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)) 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 ecea0b554e..a316c55bef 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 @@ -45,6 +45,12 @@ impl AttestationDelta { } } +#[derive(Debug)] +pub enum ProposerRewardCalculation { + Include, + Exclude, +} + /// Apply attester and proposer rewards. pub fn process_rewards_and_penalties( state: &mut BeaconState, @@ -62,7 +68,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_all( + state, + validator_statuses, + ProposerRewardCalculation::Include, + spec, + )?; // Apply the deltas, erroring on overflow above but not on overflow below (saturating at 0 // instead). @@ -79,9 +90,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, None, spec) + get_attestation_deltas(state, validator_statuses, proposer_reward, None, spec) } /// Apply rewards for participation in attestations during the previous epoch, and only compute @@ -89,10 +101,18 @@ 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, Some(validators_subset), spec).map(|deltas| { + get_attestation_deltas( + state, + validator_statuses, + proposer_reward, + Some(validators_subset), + spec, + ) + .map(|deltas| { deltas .into_iter() .enumerate() @@ -109,6 +129,7 @@ pub fn get_attestation_deltas_subset( fn get_attestation_deltas( state: &BeaconState, validator_statuses: &ValidatorStatuses, + proposer_reward: ProposerRewardCalculation, maybe_validators_subset: Option<&Vec>, spec: &ChainSpec, ) -> Result, Error> { @@ -169,13 +190,15 @@ fn get_attestation_deltas( .combine(inactivity_penalty_delta)?; } - 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)?; + 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 ea75c69c35..c5879f5c9c 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, @@ -130,6 +131,7 @@ impl Case for RewardsTest { let deltas = base::rewards_and_penalties::get_attestation_deltas_all( &state, &validator_statuses, + ProposerRewardCalculation::Include, spec, )?;