Move remaining bits; partitioned epoch-rewards reorg, 5 of 5 (solana-…

…labs#553)

* 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

runtime/src/bank.rs

@@ -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,
@@ -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,

runtime/src/bank/partitioned_epoch_rewards/calculation.rs

@@ -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::{

runtime/src/bank/partitioned_epoch_rewards/compare.rs

@@ -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()
+        );
+    }
+}

runtime/src/bank/partitioned_epoch_rewards/distribution.rs

@@ -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::{

runtime/src/epoch_rewards_hasher.rs → runtime/src/bank/partitioned_epoch_rewards/epoch_rewards_hasher.rs

@@ -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,
@@ -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() {
@@ -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()