From 552ec3e3a586ea77efda78e599faa417ee3ce64b Mon Sep 17 00:00:00 2001
From: behzad nouri <behzadnouri@gmail.com>
Date: Fri, 19 Aug 2022 14:07:56 -0400
Subject: [PATCH] patches invalid cached vote/stake accounts metrics

Invalid cached vote accounts is overcounting actual mismatches, and
invalid cached stake accounts is undercounting.
---
 runtime/src/bank.rs         | 65 +++++++++++++++++--------------------
 runtime/src/vote_account.rs | 12 +++----
 2 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
index 7cc04184ce02be..5b137e31d8634b 100644
--- a/runtime/src/bank.rs
+++ b/runtime/src/bank.rs
@@ -2798,9 +2798,26 @@ impl Bank {
                         None => {
                             invalid_stake_keys
                                 .insert(*stake_pubkey, InvalidCacheEntryReason::Missing);
+                            invalid_cached_stake_accounts.fetch_add(1, Relaxed);
                             return;
                         }
                     };
+                    if cached_stake_account.account() != &stake_account {
+                        invalid_cached_stake_accounts.fetch_add(1, Relaxed);
+                        let cached_stake_account = cached_stake_account.account();
+                        if cached_stake_account.lamports() == stake_account.lamports()
+                            && cached_stake_account.data() == stake_account.data()
+                            && cached_stake_account.owner() == stake_account.owner()
+                            && cached_stake_account.executable() == stake_account.executable()
+                        {
+                            invalid_cached_stake_accounts_rent_epoch.fetch_add(1, Relaxed);
+                        } else {
+                            debug!(
+                                "cached stake account mismatch: {}: {:?}, {:?}",
+                                stake_pubkey, stake_account, cached_stake_account
+                            );
+                        }
+                    }
                     let stake_account = match StakeAccount::<()>::try_from(stake_account) {
                         Ok(stake_account) => stake_account,
                         Err(stake_account::Error::InvalidOwner { .. }) => {
@@ -2823,33 +2840,6 @@ impl Bank {
                             return;
                         }
                     };
-                    if cached_stake_account != &stake_account {
-                        invalid_cached_stake_accounts.fetch_add(1, Relaxed);
-                        let mut cached_account = cached_stake_account.account().clone();
-                        // We could have collected rent on the loaded account already in this new epoch (we could be at partition_index 12, for example).
-                        // So, we may need to adjust the rent_epoch of the cached account. So, update rent_epoch and compare just the accounts.
-                        ExpectedRentCollection::maybe_update_rent_epoch_on_load(
-                            &mut cached_account,
-                            &SlotInfoInEpoch::new_small(self.slot()),
-                            &SlotInfoInEpoch::new_small(self.slot()),
-                            self.epoch_schedule(),
-                            self.rent_collector(),
-                            stake_pubkey,
-                            &self.rewrites_skipped_this_slot,
-                        );
-                        if &cached_account != stake_account.account() {
-                            info!(
-                                "cached stake account mismatch: {}: {:?}, {:?}",
-                                stake_pubkey,
-                                cached_account,
-                                stake_account.account()
-                            );
-                        } else {
-                            // track how many of 'invalid_cached_stake_accounts' were due to rent_epoch changes
-                            // subtract these to find real invalid cached accounts
-                            invalid_cached_stake_accounts_rent_epoch.fetch_add(1, Relaxed);
-                        }
-                    }
                     let stake_delegation = (*stake_pubkey, stake_account);
                     let mut vote_delegations = if let Some(vote_delegations) =
                         vote_with_stake_delegations_map.get_mut(vote_pubkey)
@@ -2859,16 +2849,12 @@ impl Bank {
                         let cached_vote_account = cached_vote_accounts.get(vote_pubkey);
                         let vote_account = match self.get_account_with_fixed_root(vote_pubkey) {
                             Some(vote_account) => {
-                                match cached_vote_account {
-                                    Some(cached_vote_account)
-                                        if cached_vote_account == &vote_account => {}
-                                    _ => {
-                                        invalid_cached_vote_accounts.fetch_add(1, Relaxed);
-                                    }
-                                };
                                 if vote_account.owner() != &solana_vote_program::id() {
                                     invalid_vote_keys
                                         .insert(*vote_pubkey, InvalidCacheEntryReason::WrongOwner);
+                                    if cached_vote_account.is_some() {
+                                        invalid_cached_vote_accounts.fetch_add(1, Relaxed);
+                                    }
                                     return;
                                 }
                                 vote_account
@@ -2890,9 +2876,18 @@ impl Bank {
                         } else {
                             invalid_vote_keys
                                 .insert(*vote_pubkey, InvalidCacheEntryReason::BadState);
+                            if cached_vote_account.is_some() {
+                                invalid_cached_vote_accounts.fetch_add(1, Relaxed);
+                            }
                             return;
                         };
-
+                        match cached_vote_account {
+                            Some(cached_vote_account)
+                                if cached_vote_account.account() == &vote_account => {}
+                            _ => {
+                                invalid_cached_vote_accounts.fetch_add(1, Relaxed);
+                            }
+                        };
                         vote_with_stake_delegations_map
                             .entry(*vote_pubkey)
                             .or_insert_with(|| VoteWithStakeDelegations {
diff --git a/runtime/src/vote_account.rs b/runtime/src/vote_account.rs
index 1feefb6ffaee46..c37dd7b2487c4b 100644
--- a/runtime/src/vote_account.rs
+++ b/runtime/src/vote_account.rs
@@ -3,7 +3,7 @@ use {
     once_cell::sync::OnceCell,
     serde::ser::{Serialize, Serializer},
     solana_sdk::{
-        account::{accounts_equal, AccountSharedData, ReadableAccount},
+        account::{AccountSharedData, ReadableAccount},
         instruction::InstructionError,
         pubkey::Pubkey,
     },
@@ -53,6 +53,10 @@ pub struct VoteAccounts {
 }
 
 impl VoteAccount {
+    pub(crate) fn account(&self) -> &AccountSharedData {
+        &self.0.account
+    }
+
     pub(crate) fn lamports(&self) -> u64 {
         self.0.account.lamports()
     }
@@ -255,12 +259,6 @@ impl PartialEq<VoteAccountInner> for VoteAccountInner {
     }
 }
 
-impl PartialEq<AccountSharedData> for VoteAccount {
-    fn eq(&self, other: &AccountSharedData) -> bool {
-        accounts_equal(&self.0.account, other)
-    }
-}
-
 impl Default for VoteAccounts {
     fn default() -> Self {
         Self {