Skip to content

Commit

Permalink
Move remaining bits; partitioned epoch-rewards reorg, 5 of 5 (#553)
Browse files Browse the repository at this point in the history
* Move epoch_rewards_hasher into submodule

* Move unit test into epoch_rewards_hasher sub-submodule

* Move integration-like tests into submodule

* Move compare functionality into sub-submodule
  • Loading branch information
CriesofCarrots authored Apr 3, 2024
1 parent f6e02e6 commit f4307ad
Show file tree
Hide file tree
Showing 8 changed files with 453 additions and 414 deletions.
97 changes: 2 additions & 95 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ use {
builtins::{BuiltinPrototype, BUILTINS},
metrics::*,
partitioned_epoch_rewards::{
EpochRewardCalculateParamInfo, EpochRewardStatus, PartitionedRewardsCalculation,
RewardInterval, StakeRewards, VoteRewardsAccounts,
EpochRewardCalculateParamInfo, EpochRewardStatus, RewardInterval, StakeRewards,
VoteRewardsAccounts,
},
},
bank_forks::BankForks,
Expand Down Expand Up @@ -2476,99 +2476,6 @@ impl Bank {
}
}

/// compare the vote and stake accounts between the normal rewards calculation code
/// and the partitioned rewards calculation code
/// `stake_rewards_expected` and `vote_rewards_expected` are the results of the normal rewards calculation code
/// This fn should have NO side effects.
/// This fn is only called in tests or with a debug cli arg prior to partitioned rewards feature activation.
fn compare_with_partitioned_rewards_results(
stake_rewards_expected: &[StakeReward],
vote_rewards_expected: &DashMap<Pubkey, VoteReward>,
partitioned_rewards: PartitionedRewardsCalculation,
) {
// put partitioned stake rewards in a hashmap
let mut stake_rewards: HashMap<Pubkey, &StakeReward> = HashMap::default();
partitioned_rewards
.stake_rewards_by_partition
.stake_rewards_by_partition
.iter()
.flatten()
.for_each(|stake_reward| {
stake_rewards.insert(stake_reward.stake_pubkey, stake_reward);
});

// verify stake rewards match expected
stake_rewards_expected.iter().for_each(|stake_reward| {
let partitioned = stake_rewards.remove(&stake_reward.stake_pubkey).unwrap();
assert_eq!(partitioned, stake_reward);
});
assert!(stake_rewards.is_empty(), "{stake_rewards:?}");

let mut vote_rewards: HashMap<Pubkey, (RewardInfo, AccountSharedData)> = HashMap::default();
partitioned_rewards
.vote_account_rewards
.accounts_to_store
.iter()
.enumerate()
.for_each(|(i, account)| {
if let Some(account) = account {
let reward = &partitioned_rewards.vote_account_rewards.rewards[i];
vote_rewards.insert(reward.0, (reward.1, account.clone()));
}
});

// verify vote rewards match expected
vote_rewards_expected.iter().for_each(|entry| {
if entry.value().vote_needs_store {
let partitioned = vote_rewards.remove(entry.key()).unwrap();
let mut to_store_partitioned = partitioned.1.clone();
to_store_partitioned.set_lamports(partitioned.0.post_balance);
let mut to_store_normal = entry.value().vote_account.clone();
_ = to_store_normal.checked_add_lamports(entry.value().vote_rewards);
assert_eq!(to_store_partitioned, to_store_normal, "{:?}", entry.key());
}
});
assert!(vote_rewards.is_empty(), "{vote_rewards:?}");
info!(
"verified partitioned rewards calculation matching: {}, {}",
partitioned_rewards
.stake_rewards_by_partition
.stake_rewards_by_partition
.iter()
.map(|rewards| rewards.len())
.sum::<usize>(),
partitioned_rewards
.vote_account_rewards
.accounts_to_store
.len()
);
}

/// compare the vote and stake accounts between the normal rewards calculation code
/// and the partitioned rewards calculation code
/// `stake_rewards_expected` and `vote_rewards_expected` are the results of the normal rewards calculation code
/// This fn should have NO side effects.
fn compare_with_partitioned_rewards(
&self,
stake_rewards_expected: &[StakeReward],
vote_rewards_expected: &DashMap<Pubkey, VoteReward>,
rewarded_epoch: Epoch,
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl RewardCalcTracer>,
) {
let partitioned_rewards = self.calculate_rewards_for_partitioning(
rewarded_epoch,
reward_calc_tracer,
thread_pool,
&mut RewardsMetrics::default(),
);
Self::compare_with_partitioned_rewards_results(
stake_rewards_expected,
vote_rewards_expected,
partitioned_rewards,
);
}

fn load_vote_and_stake_accounts(
&mut self,
thread_pool: &ThreadPool,
Expand Down
12 changes: 5 additions & 7 deletions runtime/src/bank/partitioned_epoch_rewards/calculation.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
use {
super::{
Bank, CalculateRewardsAndDistributeVoteRewardsResult, EpochRewardCalculateParamInfo,
epoch_rewards_hasher::hash_rewards_into_partitions, Bank,
CalculateRewardsAndDistributeVoteRewardsResult, EpochRewardCalculateParamInfo,
PartitionedRewardsCalculation, StakeRewardCalculationPartitioned, VoteRewardsAccounts,
},
crate::{
bank::{
PrevEpochInflationRewards, RewardCalcTracer, RewardCalculationEvent, RewardsMetrics,
StakeRewardCalculation, VoteAccount,
},
epoch_rewards_hasher::hash_rewards_into_partitions,
crate::bank::{
PrevEpochInflationRewards, RewardCalcTracer, RewardCalculationEvent, RewardsMetrics,
StakeRewardCalculation, VoteAccount,
},
log::info,
rayon::{
Expand Down
110 changes: 110 additions & 0 deletions runtime/src/bank/partitioned_epoch_rewards/compare.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use {
super::{Bank, PartitionedRewardsCalculation},
crate::bank::{RewardCalcTracer, RewardsMetrics, VoteReward},
dashmap::DashMap,
log::info,
rayon::ThreadPool,
solana_accounts_db::stake_rewards::StakeReward,
solana_sdk::{
account::{AccountSharedData, WritableAccount},
clock::Epoch,
pubkey::Pubkey,
reward_info::RewardInfo,
},
std::collections::HashMap,
};

impl Bank {
/// compare the vote and stake accounts between the normal rewards calculation code
/// and the partitioned rewards calculation code
/// `stake_rewards_expected` and `vote_rewards_expected` are the results of the normal rewards calculation code
/// This fn should have NO side effects.
pub(in crate::bank) fn compare_with_partitioned_rewards(
&self,
stake_rewards_expected: &[StakeReward],
vote_rewards_expected: &DashMap<Pubkey, VoteReward>,
rewarded_epoch: Epoch,
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl RewardCalcTracer>,
) {
let partitioned_rewards = self.calculate_rewards_for_partitioning(
rewarded_epoch,
reward_calc_tracer,
thread_pool,
&mut RewardsMetrics::default(),
);
Self::compare_with_partitioned_rewards_results(
stake_rewards_expected,
vote_rewards_expected,
partitioned_rewards,
);
}

/// compare the vote and stake accounts between the normal rewards calculation code
/// and the partitioned rewards calculation code
/// `stake_rewards_expected` and `vote_rewards_expected` are the results of the normal rewards calculation code
/// This fn should have NO side effects.
/// This fn is only called in tests or with a debug cli arg prior to partitioned rewards feature activation.
fn compare_with_partitioned_rewards_results(
stake_rewards_expected: &[StakeReward],
vote_rewards_expected: &DashMap<Pubkey, VoteReward>,
partitioned_rewards: PartitionedRewardsCalculation,
) {
// put partitioned stake rewards in a hashmap
let mut stake_rewards: HashMap<Pubkey, &StakeReward> = HashMap::default();
partitioned_rewards
.stake_rewards_by_partition
.stake_rewards_by_partition
.iter()
.flatten()
.for_each(|stake_reward| {
stake_rewards.insert(stake_reward.stake_pubkey, stake_reward);
});

// verify stake rewards match expected
stake_rewards_expected.iter().for_each(|stake_reward| {
let partitioned = stake_rewards.remove(&stake_reward.stake_pubkey).unwrap();
assert_eq!(partitioned, stake_reward);
});
assert!(stake_rewards.is_empty(), "{stake_rewards:?}");

let mut vote_rewards: HashMap<Pubkey, (RewardInfo, AccountSharedData)> = HashMap::default();
partitioned_rewards
.vote_account_rewards
.accounts_to_store
.iter()
.enumerate()
.for_each(|(i, account)| {
if let Some(account) = account {
let reward = &partitioned_rewards.vote_account_rewards.rewards[i];
vote_rewards.insert(reward.0, (reward.1, account.clone()));
}
});

// verify vote rewards match expected
vote_rewards_expected.iter().for_each(|entry| {
if entry.value().vote_needs_store {
let partitioned = vote_rewards.remove(entry.key()).unwrap();
let mut to_store_partitioned = partitioned.1.clone();
to_store_partitioned.set_lamports(partitioned.0.post_balance);
let mut to_store_normal = entry.value().vote_account.clone();
_ = to_store_normal.checked_add_lamports(entry.value().vote_rewards);
assert_eq!(to_store_partitioned, to_store_normal, "{:?}", entry.key());
}
});
assert!(vote_rewards.is_empty(), "{vote_rewards:?}");
info!(
"verified partitioned rewards calculation matching: {}, {}",
partitioned_rewards
.stake_rewards_by_partition
.stake_rewards_by_partition
.iter()
.map(|rewards| rewards.len())
.sum::<usize>(),
partitioned_rewards
.vote_account_rewards
.accounts_to_store
.len()
);
}
}
5 changes: 3 additions & 2 deletions runtime/src/bank/partitioned_epoch_rewards/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ impl Bank {
mod tests {
use {
super::*,
crate::{
bank::tests::create_genesis_config, epoch_rewards_hasher::hash_rewards_into_partitions,
crate::bank::{
partitioned_epoch_rewards::epoch_rewards_hasher::hash_rewards_into_partitions,
tests::create_genesis_config,
},
rand::Rng,
solana_sdk::{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use {
solana_sdk::{epoch_rewards_hasher::EpochRewardsHasher, hash::Hash},
};

pub(crate) fn hash_rewards_into_partitions(
pub(in crate::bank::partitioned_epoch_rewards) fn hash_rewards_into_partitions(
stake_rewards: StakeRewards,
parent_blockhash: &Hash,
num_partitions: usize,
Expand All @@ -25,7 +25,13 @@ pub(crate) fn hash_rewards_into_partitions(

#[cfg(test)]
mod tests {
use {super::*, solana_accounts_db::stake_rewards::StakeReward, std::collections::HashMap};
use {
super::*,
crate::bank::{tests::create_genesis_config, Bank},
solana_accounts_db::stake_rewards::StakeReward,
solana_sdk::{epoch_schedule::EpochSchedule, native_token::LAMPORTS_PER_SOL},
std::collections::HashMap,
};

#[test]
fn test_hash_rewards_into_partitions() {
Expand Down Expand Up @@ -85,6 +91,30 @@ mod tests {
}
}

/// Test that reward partition range panics when passing out of range partition index
#[test]
#[should_panic(expected = "index out of bounds: the len is 10 but the index is 15")]
fn test_get_stake_rewards_partition_range_panic() {
let (mut genesis_config, _mint_keypair) =
create_genesis_config(1_000_000 * LAMPORTS_PER_SOL);
genesis_config.epoch_schedule = EpochSchedule::custom(432000, 432000, false);
let mut bank = Bank::new_for_tests(&genesis_config);

// simulate 40K - 1 rewards, the expected num of credit blocks should be 10.
let expected_num = 40959;
let stake_rewards = (0..expected_num)
.map(|_| StakeReward::new_random())
.collect::<Vec<_>>();

let stake_rewards_bucket =
hash_rewards_into_partitions(stake_rewards, &Hash::new(&[1; 32]), 10);

bank.set_epoch_reward_status_active(stake_rewards_bucket.clone());

// This call should panic, i.e. 15 is out of the num_credit_blocks
let _range = &stake_rewards_bucket[15];
}

fn compare(a: &StakeRewards, b: &StakeRewards) {
let mut a = a
.iter()
Expand Down
Loading

0 comments on commit f4307ad

Please sign in to comment.