From 4f756600d1e1569711f8623452435365bc5a037d Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Wed, 4 Oct 2023 23:35:55 -0500 Subject: [PATCH 01/10] #4512 inactivity calculation for Altair --- .../beacon_chain/src/attestation_rewards.rs | 17 +++-- beacon_node/beacon_chain/tests/rewards.rs | 71 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 94bd28f98fd..010f532b0da 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -189,10 +189,10 @@ impl BeaconChain { let mut head_reward = 0i64; let mut target_reward = 0i64; let mut source_reward = 0i64; + let mut inactivity_penalty = 0i64; if eligible { let effective_balance = state.get_effective_balance(*validator_index)?; - for flag_index in 0..PARTICIPATION_FLAG_WEIGHTS.len() { let (ideal_reward, penalty) = ideal_rewards_hashmap .get(&(flag_index, effective_balance)) @@ -218,6 +218,17 @@ impl BeaconChain { source_reward = *penalty; } } + + let inactivity_score = state.get_inactivity_score(*validator_index)?; + let penalty_numerator = effective_balance.safe_mul(inactivity_score)?; + let penalty_denominator = spec + .inactivity_score_bias + .safe_mul(spec.inactivity_penalty_quotient_for_state(&state))?; + inactivity_penalty = penalty_numerator.safe_div(penalty_denominator)? as i64; + println!( + "Inactivity penalty info {:#?} | {:#?} | {:#?} | {:#?}", + penalty_numerator, penalty_denominator, inactivity_penalty, inactivity_score + ); } total_rewards.push(TotalAttestationRewards { validator_index: *validator_index as u64, @@ -225,8 +236,7 @@ impl BeaconChain { target: target_reward, source: source_reward, inclusion_delay: None, - // TODO: altair calculation logic needs to be updated to include inactivity penalty - inactivity: 0, + inactivity: inactivity_penalty * -1, }); } @@ -249,7 +259,6 @@ impl BeaconChain { target: 0, source: 0, inclusion_delay: None, - // TODO: altair calculation logic needs to be updated to include inactivity penalty inactivity: 0, }); match *flag_index { diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index be271804b99..bec9e69e473 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -14,7 +14,7 @@ use eth2::lighthouse::StandardAttestationRewards; use eth2::types::ValidatorId; use lazy_static::lazy_static; use types::beacon_state::Error as BeaconStateError; -use types::{BeaconState, ChainSpec}; +use types::{BeaconState, ChainSpec, ForkName}; pub const VALIDATOR_COUNT: usize = 64; @@ -219,6 +219,75 @@ async fn test_verify_attestation_rewards_base_inactivity_leak() { assert_eq!(expected_balances, balances); } +#[tokio::test] +async fn test_verify_attestation_rewards_altair_inactivity_leak() { + let spec = ForkName::Altair.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec.clone()); + + 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 + 2; + + // advance until beginning of epoch N + 1 and get balances + harness + .extend_chain( + (E::slots_per_epoch() * (target_epoch + 1)) as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::SomeValidators(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(); + + // 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[..half] + .iter() + .all(|reward| reward.inactivity == 0)); + assert!(total_rewards[half..] + .iter() + .all(|reward| reward.inactivity < 0)); + + // apply attestation rewards to initial balances + let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); + + for initial_balance in &initial_balances { + println!("initial {initial_balance}"); + } + + for expected_balance in &expected_balances { + println!("expected {expected_balance}"); + } + // verify expected balances against actual balances + let balances: Vec = harness.get_current_state().balances().clone().into(); + + for balance in &balances { + println!("actual {balance}"); + } + // Failing test statement + assert_eq!(expected_balances, balances); +} + #[tokio::test] async fn test_verify_attestation_rewards_base_subset_only() { let harness = get_harness(E::default_spec()); From 70084693950d7460ddc31428c64e05d8697bc8a6 Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Thu, 5 Oct 2023 11:31:34 -0500 Subject: [PATCH 02/10] Update attestation_rewards.rs Updated inactivity calculation to only occur during incorrect voting --- .../beacon_chain/src/attestation_rewards.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 010f532b0da..bcfeead8cb3 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -193,6 +193,7 @@ impl BeaconChain { if eligible { let effective_balance = state.get_effective_balance(*validator_index)?; + for flag_index in 0..PARTICIPATION_FLAG_WEIGHTS.len() { let (ideal_reward, penalty) = ideal_rewards_hashmap .get(&(flag_index, effective_balance)) @@ -214,21 +215,18 @@ impl BeaconChain { head_reward = 0; } else if flag_index == TIMELY_TARGET_FLAG_INDEX { target_reward = *penalty; + + let penalty_numerator = effective_balance + .safe_mul(state.get_inactivity_score(*validator_index)?)?; + let penalty_denominator = spec + .inactivity_score_bias + .safe_mul(spec.inactivity_penalty_quotient_for_state(&state))?; + inactivity_penalty = + penalty_numerator.safe_div(penalty_denominator)? as i64; } else if flag_index == TIMELY_SOURCE_FLAG_INDEX { source_reward = *penalty; } } - - let inactivity_score = state.get_inactivity_score(*validator_index)?; - let penalty_numerator = effective_balance.safe_mul(inactivity_score)?; - let penalty_denominator = spec - .inactivity_score_bias - .safe_mul(spec.inactivity_penalty_quotient_for_state(&state))?; - inactivity_penalty = penalty_numerator.safe_div(penalty_denominator)? as i64; - println!( - "Inactivity penalty info {:#?} | {:#?} | {:#?} | {:#?}", - penalty_numerator, penalty_denominator, inactivity_penalty, inactivity_score - ); } total_rewards.push(TotalAttestationRewards { validator_index: *validator_index as u64, From 9d866073803032a1b4689e7dff65f8aca00becbf Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Mon, 9 Oct 2023 21:35:07 -0500 Subject: [PATCH 03/10] Update rewards.rs --- beacon_node/beacon_chain/tests/rewards.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index bec9e69e473..32e899df3ba 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -229,6 +229,7 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { // target epoch is the epoch where the chain enters inactivity leak let target_epoch = &spec.min_epochs_to_inactivity_penalty + 2; + println!("{:#?}", E::slots_per_epoch()); // advance until beginning of epoch N + 1 and get balances harness .extend_chain( @@ -238,6 +239,12 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { ) .await; let initial_balances: Vec = harness.get_current_state().balances().clone().into(); + let initial_slot = harness.get_current_slot(); + + for (index, initial_balance) in initial_balances.iter().enumerate() { + println!("Validator: {index} | initial value {initial_balance} | slot {initial_slot}"); + break + } // extend slots to beginning of epoch N + 2 harness.advance_slot(); @@ -248,7 +255,7 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { AttestationStrategy::SomeValidators(half_validators), ) .await; - let _slot = harness.get_current_slot(); + let advanced_slot = harness.get_current_slot(); // compute reward deltas for all validators in epoch N let StandardAttestationRewards { @@ -271,18 +278,16 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { // apply attestation rewards to initial balances let expected_balances = apply_attestation_rewards(&initial_balances, total_rewards); - for initial_balance in &initial_balances { - println!("initial {initial_balance}"); - } - - for expected_balance in &expected_balances { - println!("expected {expected_balance}"); + for (index, expected_balance) in expected_balances.iter().enumerate() { + println!("Validator: {index} | expected value {expected_balance} | slot {advanced_slot}"); + break } // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); - for balance in &balances { - println!("actual {balance}"); + for (index, balance) in balances.iter().enumerate() { + println!("Validator: {index} | actual value {balance} | slot {advanced_slot}"); + break } // Failing test statement assert_eq!(expected_balances, balances); From d42e9845370b730c17c27bd470e3a641b6764cb9 Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Tue, 10 Oct 2023 22:55:32 -0500 Subject: [PATCH 04/10] Added in item to test_utils to support some validators with extend_slots --- .../beacon_chain/src/attestation_rewards.rs | 6 ++- beacon_node/beacon_chain/src/test_utils.rs | 23 ++++++++++ beacon_node/beacon_chain/tests/rewards.rs | 44 +++++++++++-------- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index bcfeead8cb3..77a9ab0ae60 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -5,6 +5,7 @@ use participation_cache::ParticipationCache; use safe_arith::SafeArith; use serde_utils::quoted_u64::Quoted; use slog::debug; +use state_processing::per_epoch_processing::altair::process_inactivity_updates; use state_processing::{ common::altair::BaseRewardPerIncrement, per_epoch_processing::altair::{participation_cache, rewards_and_penalties::get_flag_weight}, @@ -123,6 +124,7 @@ impl BeaconChain { // Calculate ideal_rewards let participation_cache = ParticipationCache::new(&state, spec)?; + process_inactivity_updates(&mut state, &participation_cache, spec)?; let previous_epoch = state.previous_epoch(); @@ -222,7 +224,7 @@ impl BeaconChain { .inactivity_score_bias .safe_mul(spec.inactivity_penalty_quotient_for_state(&state))?; inactivity_penalty = - penalty_numerator.safe_div(penalty_denominator)? as i64; + penalty_numerator.safe_div(penalty_denominator)? as i64 * -1; } else if flag_index == TIMELY_SOURCE_FLAG_INDEX { source_reward = *penalty; } @@ -234,7 +236,7 @@ impl BeaconChain { target: target_reward, source: source_reward, inclusion_delay: None, - inactivity: inactivity_penalty * -1, + inactivity: inactivity_penalty, }); } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 5e54b1194d4..eb4b078ed36 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2133,6 +2133,29 @@ where .await } + /// Uses `Self::extend_chain` to `num_slots` blocks. + /// + /// Utilizes: + /// + /// - BlockStrategy::OnCanonicalHead, + /// - AttestationStrategy::SomeValidators(validators), + pub async fn extend_slots_some_validators( + &self, + num_slots: usize, + validators: Vec, + ) -> Hash256 { + if self.chain.slot().unwrap() == self.chain.canonical_head.cached_head().head_slot() { + self.advance_slot(); + } + + self.extend_chain( + num_slots, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::SomeValidators(validators), + ) + .await + } + /// Extend the `BeaconChain` with some blocks and attestations. Returns the root of the /// last-produced block (the head of the chain). /// diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 32e899df3ba..76173ff983a 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -14,7 +14,7 @@ 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}; +use types::{BeaconState, ChainSpec, ForkName, SignedBeaconBlockHash, Slot}; pub const VALIDATOR_COUNT: usize = 64; @@ -227,15 +227,13 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { 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 + 2; + let target_epoch = &spec.min_epochs_to_inactivity_penalty + 1; - println!("{:#?}", E::slots_per_epoch()); // 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(); @@ -243,19 +241,29 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { for (index, initial_balance) in initial_balances.iter().enumerate() { println!("Validator: {index} | initial value {initial_balance} | slot {initial_slot}"); - break + break; } // 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 advanced_slot = harness.get_current_slot(); + let mut advanced_slot = harness.get_current_slot(); + + for _ in 0..E::slots_per_epoch() { + harness.advance_slot(); + let last_block_hash = harness + .extend_slots_some_validators(1, half_validators.clone()) + .await; + advanced_slot = harness.get_current_slot(); + + let mut current_state = harness.get_current_state(); + let block = harness + .get_block(SignedBeaconBlockHash::from(last_block_hash)) + .unwrap(); + let beacon_block_reward = harness + .chain + .compute_beacon_block_reward(block.message(), last_block_hash, &mut current_state) + .unwrap(); + println!("Beacon block reward: {:#?}", beacon_block_reward); + } // compute reward deltas for all validators in epoch N let StandardAttestationRewards { @@ -280,14 +288,14 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { for (index, expected_balance) in expected_balances.iter().enumerate() { println!("Validator: {index} | expected value {expected_balance} | slot {advanced_slot}"); - break + break; } // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); for (index, balance) in balances.iter().enumerate() { println!("Validator: {index} | actual value {balance} | slot {advanced_slot}"); - break + break; } // Failing test statement assert_eq!(expected_balances, balances); From 55cd681be74abacdb2ae0c400f8f1874afefce8b Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Thu, 12 Oct 2023 13:13:16 -0500 Subject: [PATCH 05/10] Updated reward.rs test to correctly compute beacon block reward given the pre_state --- beacon_node/beacon_chain/tests/rewards.rs | 30 +++++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 76173ff983a..5a6558205e0 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -14,7 +14,7 @@ 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, SignedBeaconBlockHash, Slot}; +use types::{BeaconState, ChainSpec, ForkName, Slot}; pub const VALIDATOR_COUNT: usize = 64; @@ -248,19 +248,33 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { let mut advanced_slot = harness.get_current_slot(); for _ in 0..E::slots_per_epoch() { - harness.advance_slot(); - let last_block_hash = harness + harness .extend_slots_some_validators(1, half_validators.clone()) .await; + // harness.advance_slot(); + // harness + // .extend_chain( + // 1, + // BlockStrategy::OnCanonicalHead, + // AttestationStrategy::SomeValidators(half_validators.clone()), + // ) + // .await; advanced_slot = harness.get_current_slot(); - let mut current_state = harness.get_current_state(); - let block = harness - .get_block(SignedBeaconBlockHash::from(last_block_hash)) - .unwrap(); + let current_state = harness.get_current_state(); + let slot = current_state.slot() + Slot::new(1); + + let (signed_block, mut state) = harness + .make_block_return_pre_state(current_state, slot) + .await; + let beacon_block_reward = harness .chain - .compute_beacon_block_reward(block.message(), last_block_hash, &mut current_state) + .compute_beacon_block_reward( + signed_block.message(), + signed_block.canonical_root(), + &mut state, + ) .unwrap(); println!("Beacon block reward: {:#?}", beacon_block_reward); } From a6a1d913d335ce78c708e70d192f54540a88a9bc Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Thu, 12 Oct 2023 22:13:04 -0500 Subject: [PATCH 06/10] Updated test logic to account for proposal rewards --- beacon_node/beacon_chain/tests/rewards.rs | 71 ++++++++++++++--------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 5a6558205e0..8716bcaf349 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -244,29 +244,13 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { break; } - // extend slots to beginning of epoch N + 2 let mut advanced_slot = harness.get_current_slot(); - + let mut proposal_rewards_map: HashMap = HashMap::new(); for _ in 0..E::slots_per_epoch() { - harness - .extend_slots_some_validators(1, half_validators.clone()) - .await; - // harness.advance_slot(); - // harness - // .extend_chain( - // 1, - // BlockStrategy::OnCanonicalHead, - // AttestationStrategy::SomeValidators(half_validators.clone()), - // ) - // .await; - advanced_slot = harness.get_current_slot(); + let state = harness.get_current_state(); + let slot = state.slot() + Slot::new(1); - let current_state = harness.get_current_state(); - let slot = current_state.slot() + Slot::new(1); - - let (signed_block, mut state) = harness - .make_block_return_pre_state(current_state, slot) - .await; + let (signed_block, mut state) = harness.make_block_return_pre_state(state, slot).await; let beacon_block_reward = harness .chain @@ -276,6 +260,19 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { &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; + + advanced_slot = harness.get_current_slot(); println!("Beacon block reward: {:#?}", beacon_block_reward); } @@ -299,18 +296,22 @@ async fn test_verify_attestation_rewards_altair_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(); for (index, expected_balance) in expected_balances.iter().enumerate() { + let current_balance = balances[index]; + let is_proposal_validator = proposal_rewards_map.contains_key(&(index as u64)); println!("Validator: {index} | expected value {expected_balance} | slot {advanced_slot}"); - break; + println!("Validator: {index} | actual value {current_balance} | slot {advanced_slot}"); + println!( + "Validator: {index} | difference expected - current: {:#?}", + expected_balance - current_balance + ); + println!("Validator: {index} | get proposal reward {is_proposal_validator}") } - // verify expected balances against actual balances - let balances: Vec = harness.get_current_state().balances().clone().into(); - for (index, balance) in balances.iter().enumerate() { - println!("Validator: {index} | actual value {balance} | slot {advanced_slot}"); - break; - } // Failing test statement assert_eq!(expected_balances, balances); } @@ -393,3 +394,19 @@ fn get_validator_balances(state: BeaconState, validators: &[usize]) -> Vec, + expected_balances: Vec, +) -> Vec { + let calculated_balances = expected_balances + .iter() + .enumerate() + .map(|(i, balance)| { + // calculated_balances.push(proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64) + balance) + balance + proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64) + }) + .collect(); + + calculated_balances +} From 4cccf2296ed09ac8ea4d9053f39cb1b95d931447 Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Thu, 12 Oct 2023 22:28:04 -0500 Subject: [PATCH 07/10] Removed commented item and unused for loop --- beacon_node/beacon_chain/tests/rewards.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 8716bcaf349..404b3c75ed6 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -237,13 +237,8 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { ) .await; let initial_balances: Vec = harness.get_current_state().balances().clone().into(); - let initial_slot = harness.get_current_slot(); - - for (index, initial_balance) in initial_balances.iter().enumerate() { - println!("Validator: {index} | initial value {initial_balance} | slot {initial_slot}"); - break; - } + // advance until epoch N + 2 and build proposal rewards map let mut advanced_slot = harness.get_current_slot(); let mut proposal_rewards_map: HashMap = HashMap::new(); for _ in 0..E::slots_per_epoch() { @@ -303,13 +298,14 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { for (index, expected_balance) in expected_balances.iter().enumerate() { let current_balance = balances[index]; let is_proposal_validator = proposal_rewards_map.contains_key(&(index as u64)); + println!("**************************************************************"); println!("Validator: {index} | expected value {expected_balance} | slot {advanced_slot}"); println!("Validator: {index} | actual value {current_balance} | slot {advanced_slot}"); println!( "Validator: {index} | difference expected - current: {:#?}", expected_balance - current_balance ); - println!("Validator: {index} | get proposal reward {is_proposal_validator}") + println!("Validator: {index} | get proposal reward {is_proposal_validator}"); } // Failing test statement @@ -402,10 +398,7 @@ fn apply_beacon_block_rewards( let calculated_balances = expected_balances .iter() .enumerate() - .map(|(i, balance)| { - // calculated_balances.push(proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64) + balance) - balance + proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64) - }) + .map(|(i, balance)| balance + proposal_rewards_map.get(&(i as u64)).unwrap_or(&0u64)) .collect(); calculated_balances From e46500a3abc4f3d525651c80154d0306c36b24e1 Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Fri, 13 Oct 2023 21:41:55 -0500 Subject: [PATCH 08/10] Updated test to account for sync committee --- beacon_node/beacon_chain/tests/rewards.rs | 56 +++++++++++++++-------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 404b3c75ed6..10c639cc077 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -239,14 +239,14 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { let initial_balances: Vec = harness.get_current_state().balances().clone().into(); // advance until epoch N + 2 and build proposal rewards map - let mut advanced_slot = harness.get_current_slot(); let mut proposal_rewards_map: HashMap = HashMap::new(); + let mut sync_committee_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, mut state) = harness.make_block_return_pre_state(state, slot).await; - let beacon_block_reward = harness .chain .compute_beacon_block_reward( @@ -263,12 +263,23 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { proposal_rewards_map.insert(beacon_block_reward.proposer_index, total_proposer_reward); + // calculate sync committee rewards / penalties + let reward_payload = harness + .chain + .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); + }); + harness .extend_slots_some_validators(1, half_validators.clone()) .await; - - advanced_slot = harness.get_current_slot(); - println!("Beacon block reward: {:#?}", beacon_block_reward); } // compute reward deltas for all validators in epoch N @@ -289,26 +300,15 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { .iter() .all(|reward| reward.inactivity < 0)); - // apply attestation rewards to initial balances + // 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); + // verify expected balances against actual balances let balances: Vec = harness.get_current_state().balances().clone().into(); - for (index, expected_balance) in expected_balances.iter().enumerate() { - let current_balance = balances[index]; - let is_proposal_validator = proposal_rewards_map.contains_key(&(index as u64)); - println!("**************************************************************"); - println!("Validator: {index} | expected value {expected_balance} | slot {advanced_slot}"); - println!("Validator: {index} | actual value {current_balance} | slot {advanced_slot}"); - println!( - "Validator: {index} | difference expected - current: {:#?}", - expected_balance - current_balance - ); - println!("Validator: {index} | get proposal reward {is_proposal_validator}"); - } - - // Failing test statement assert_eq!(expected_balances, balances); } @@ -403,3 +403,19 @@ fn apply_beacon_block_rewards( 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 +} From c3562829ee5037e8aabe5a901b45b7a740943659 Mon Sep 17 00:00:00 2001 From: Zack Scott Date: Thu, 19 Oct 2023 09:56:31 -0500 Subject: [PATCH 09/10] Fixed lint issue with make lint-fix --- beacon_node/beacon_chain/src/attestation_rewards.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 77a9ab0ae60..c3368d73402 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -224,7 +224,7 @@ impl BeaconChain { .inactivity_score_bias .safe_mul(spec.inactivity_penalty_quotient_for_state(&state))?; inactivity_penalty = - penalty_numerator.safe_div(penalty_denominator)? as i64 * -1; + -(penalty_numerator.safe_div(penalty_denominator)? as i64); } else if flag_index == TIMELY_SOURCE_FLAG_INDEX { source_reward = *penalty; } From 7914240ec51233e6e9fdacd5d61aa1c6eec7b882 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Fri, 20 Oct 2023 10:25:32 +1100 Subject: [PATCH 10/10] Fix failing test after deneb merge --- beacon_node/beacon_chain/tests/rewards.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 10c639cc077..7c8f01cf55c 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -246,7 +246,8 @@ async fn test_verify_attestation_rewards_altair_inactivity_leak() { let slot = state.slot() + Slot::new(1); // calculate beacon block rewards / penalties - let (signed_block, mut state) = harness.make_block_return_pre_state(state, slot).await; + 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(