Refactor reward payout code - part 2 (#31156)

* extract load_vote_stake_accounts fn

* rename

runtime/src/bank.rs

@@ -2552,7 +2552,7 @@ impl Bank {
     ///   ( Vec<(staker info)> (voter account) ) keyed by voter pubkey
     ///
     /// Filters out invalid pairs
-    fn load_vote_and_stake_accounts_with_thread_pool(
+    fn _load_vote_and_stake_accounts_with_thread_pool(
         &self,
         thread_pool: &ThreadPool,
         reward_calc_tracer: Option<impl RewardCalcTracer>,
@@ -2744,7 +2744,7 @@ impl Bank {
         }
     }
 
-    fn load_vote_and_stake_accounts(
+    fn _load_vote_and_stake_accounts(
         &self,
         thread_pool: &ThreadPool,
         reward_calc_tracer: Option<impl RewardCalcTracer>,
@@ -2861,53 +2861,24 @@ impl Bank {
         }
     }
 
-    /// iterate over all stakes, redeem vote credits for each stake we can
-    /// successfully load and parse, return the lamport value of one point
+    /// Load, calculate and payout epoch rewards for stake and vote accounts
     fn pay_validator_rewards_with_thread_pool(
         &mut self,
         rewarded_epoch: Epoch,
         rewards: u64,
-        reward_calc_tracer: Option<impl Fn(&RewardCalculationEvent) + Send + Sync>,
+        reward_calc_tracer: Option<impl RewardCalcTracer>,
         credits_auto_rewind: bool,
         thread_pool: &ThreadPool,
         metrics: &mut RewardsMetrics,
         update_rewards_from_cached_accounts: bool,
     ) {
         let stake_history = self.stakes_cache.stakes().history().clone();
-        let vote_with_stake_delegations_map = {
-            let mut m = Measure::start("load_vote_and_stake_accounts_us");
-            let LoadVoteAndStakeAccountsResult {
-                vote_with_stake_delegations_map,
-                invalid_stake_keys,
-                invalid_vote_keys,
-                invalid_cached_vote_accounts,
-                invalid_cached_stake_accounts,
-                invalid_cached_stake_accounts_rent_epoch,
-                vote_accounts_cache_miss_count,
-            } = if update_rewards_from_cached_accounts {
-                self.load_vote_and_stake_accounts(thread_pool, reward_calc_tracer.as_ref())
-            } else {
-                self.load_vote_and_stake_accounts_with_thread_pool(
-                    thread_pool,
-                    reward_calc_tracer.as_ref(),
-                )
-            };
-            m.stop();
-            metrics
-                .load_vote_and_stake_accounts_us
-                .fetch_add(m.as_us(), Relaxed);
-            metrics.invalid_cached_vote_accounts += invalid_cached_vote_accounts;
-            metrics.invalid_cached_stake_accounts += invalid_cached_stake_accounts;
-            metrics.invalid_cached_stake_accounts_rent_epoch +=
-                invalid_cached_stake_accounts_rent_epoch;
-            metrics.vote_accounts_cache_miss_count += vote_accounts_cache_miss_count;
-            self.stakes_cache.handle_invalid_keys(
-                invalid_stake_keys,
-                invalid_vote_keys,
-                self.slot(),
-            );
-            vote_with_stake_delegations_map
-        };
+        let vote_with_stake_delegations_map = self.load_vote_and_stake_accounts(
+            thread_pool,
+            reward_calc_tracer.as_ref(),
+            metrics,
+            update_rewards_from_cached_accounts,
+        );
 
         let mut m = Measure::start("calculate_points");
         let points: u128 = thread_pool.install(|| {
@@ -3027,6 +2998,47 @@ impl Bank {
         self.update_reward_history(stake_rewards, vote_rewards);
     }
 
+    fn load_vote_and_stake_accounts(
+        &mut self,
+        thread_pool: &ThreadPool,
+        reward_calc_tracer: Option<impl RewardCalcTracer>,
+        metrics: &mut RewardsMetrics,
+        update_rewards_from_cached_accounts: bool,
+    ) -> VoteWithStakeDelegationsMap {
+        let (
+            LoadVoteAndStakeAccountsResult {
+                vote_with_stake_delegations_map,
+                invalid_stake_keys,
+                invalid_vote_keys,
+                invalid_cached_vote_accounts,
+                invalid_cached_stake_accounts,
+                invalid_cached_stake_accounts_rent_epoch,
+                vote_accounts_cache_miss_count,
+            },
+            measure,
+        ) = measure!({
+            if update_rewards_from_cached_accounts {
+                self._load_vote_and_stake_accounts(thread_pool, reward_calc_tracer.as_ref())
+            } else {
+                self._load_vote_and_stake_accounts_with_thread_pool(
+                    thread_pool,
+                    reward_calc_tracer.as_ref(),
+                )
+            }
+        });
+        metrics
+            .load_vote_and_stake_accounts_us
+            .fetch_add(measure.as_us(), Relaxed);
+        metrics.invalid_cached_vote_accounts += invalid_cached_vote_accounts;
+        metrics.invalid_cached_stake_accounts += invalid_cached_stake_accounts;
+        metrics.invalid_cached_stake_accounts_rent_epoch +=
+            invalid_cached_stake_accounts_rent_epoch;
+        metrics.vote_accounts_cache_miss_count += vote_accounts_cache_miss_count;
+        self.stakes_cache
+            .handle_invalid_keys(invalid_stake_keys, invalid_vote_keys, self.slot());
+        vote_with_stake_delegations_map
+    }
+
     fn store_stake_accounts(&self, stake_rewards: &[StakeReward], metrics: &mut RewardsMetrics) {
         // store stake account even if stakers_reward is 0
         // because credits observed has changed

runtime/src/bank/tests.rs

@@ -2413,10 +2413,10 @@ fn test_rent_eager_collect_rent_zero_lamport_deterministic() {
 fn test_bank_update_vote_stake_rewards() {
     let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
     check_bank_update_vote_stake_rewards(|bank: &Bank| {
-        bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
+        bank._load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
     });
     check_bank_update_vote_stake_rewards(|bank: &Bank| {
-        bank.load_vote_and_stake_accounts(&thread_pool, null_tracer())
+        bank._load_vote_and_stake_accounts(&thread_pool, null_tracer())
     });
 }
 #[cfg(test)]
@@ -9678,7 +9678,7 @@ fn test_stake_vote_account_validity() {
     check_stake_vote_account_validity(
         true, // check owner change,
         |bank: &Bank| {
-            bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
+            bank._load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
         },
     );
     // TODO: stakes cache should be hardened for the case when the account
@@ -9686,7 +9686,7 @@ fn test_stake_vote_account_validity() {
     // https://github.com/solana-labs/solana/pull/24200#discussion_r849935444
     check_stake_vote_account_validity(
         false, // check owner change
-        |bank: &Bank| bank.load_vote_and_stake_accounts(&thread_pool, null_tracer()),
+        |bank: &Bank| bank._load_vote_and_stake_accounts(&thread_pool, null_tracer()),
     );
 }