VoteAccounts: discard invalid accounts on deserialization

Before switching to eager vote account deserialization we were
(accidentally) able to load snapshots with invalid vote accounts
(because account parsing was deferred).

This change ensures that we're still able to parse such snapshots.

core/src/consensus/progress_map.rs

@@ -452,7 +452,7 @@ mod test {
         let vote_account_pubkeys: Vec<_> = std::iter::repeat_with(solana_sdk::pubkey::new_rand)
             .take(num_vote_accounts)
             .collect();
-        let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys
+        let epoch_vote_accounts = vote_account_pubkeys
             .iter()
             .skip(num_vote_accounts - staked_vote_accounts)
             .map(|pubkey| (*pubkey, (1, VoteAccount::new_random())))
@@ -494,7 +494,7 @@ mod test {
         let vote_account_pubkeys: Vec<_> = std::iter::repeat_with(solana_sdk::pubkey::new_rand)
             .take(num_vote_accounts)
             .collect();
-        let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys
+        let epoch_vote_accounts = vote_account_pubkeys
             .iter()
             .skip(num_vote_accounts - staked_vote_accounts)
             .map(|pubkey| (*pubkey, (1, VoteAccount::new_random())))

ledger/src/blockstore_processor.rs

@@ -4570,7 +4570,11 @@ pub mod tests {
         let total_stake = 10;
 
         // Supermajority root should be None
-        assert!(supermajority_root_from_vote_accounts(total_stake, &HashMap::default()).is_none());
+        assert!(supermajority_root_from_vote_accounts(
+            total_stake,
+            &VoteAccountsHashMap::default()
+        )
+        .is_none());
 
         // Supermajority root should be None
         let roots_stakes = vec![(8, 1), (3, 1), (4, 1), (8, 1)];

programs/bpf_loader/src/syscalls/mod.rs

@@ -2121,8 +2121,8 @@ mod tests {
                 last_restart_slot::LastRestartSlot,
             },
         },
-        solana_vote::vote_account::VoteAccount,
-        std::{collections::HashMap, mem, str::FromStr},
+        solana_vote::vote_account::{VoteAccount, VoteAccountsHashMap},
+        std::{mem, str::FromStr},
         test_case::test_case,
     };
 
@@ -4817,7 +4817,7 @@ mod tests {
         compute_budget.compute_unit_limit = expected_cus;
 
         let vote_address = Pubkey::new_unique();
-        let mut vote_accounts_map = HashMap::new();
+        let mut vote_accounts_map = VoteAccountsHashMap::default();
         vote_accounts_map.insert(
             vote_address,
             (expected_epoch_stake, VoteAccount::new_random()),

runtime/src/bank/tests.rs

@@ -13037,7 +13037,7 @@ fn test_bank_epoch_stakes() {
                 .unwrap();
                 (authorized_voter, (100_u64, vote_account))
             })
-            .collect::<HashMap<_, _>>(),
+            .collect(),
         1,
     );
     bank1.set_epoch_stakes_for_test(1, new_epoch_stakes);

vote/src/vote_account.rs

@@ -1,6 +1,9 @@
 use {
     itertools::Itertools,
-    serde::ser::{Serialize, Serializer},
+    serde::{
+        de::{MapAccess, Visitor},
+        ser::{Serialize, Serializer},
+    },
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount},
         instruction::InstructionError,
@@ -10,8 +13,10 @@ use {
     std::{
         cmp::Ordering,
         collections::{hash_map::Entry, HashMap},
+        fmt,
         iter::FromIterator,
         mem,
+        ops::{Deref, DerefMut},
         sync::{Arc, OnceLock},
     },
     thiserror::Error,
@@ -37,7 +42,78 @@ struct VoteAccountInner {
     vote_state: VoteState,
 }
 
-pub type VoteAccountsHashMap = HashMap<Pubkey, (/*stake:*/ u64, VoteAccount)>;
+#[derive(Default, Clone, Debug, PartialEq, Serialize)]
+#[repr(transparent)]
+pub struct VoteAccountsHashMap(HashMap<Pubkey, (/*stake:*/ u64, VoteAccount)>);
+
+impl Deref for VoteAccountsHashMap {
+    type Target = HashMap<Pubkey, (/*stake:*/ u64, VoteAccount)>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for VoteAccountsHashMap {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl FromIterator<(Pubkey, (/*stake:*/ u64, VoteAccount))> for VoteAccountsHashMap {
+    fn from_iter<I>(iter: I) -> Self
+    where
+        I: IntoIterator<Item = (Pubkey, (/*stake:*/ u64, VoteAccount))>,
+    {
+        Self(HashMap::from_iter(iter))
+    }
+}
+
+// This custom deserializer is needed to ensure compatibility at snapshot loading with versions
+// before https://github.com/anza-xyz/agave/pull/2659 which would teorethically allow invalid vote
+// accounts in VoteAccounts.
+//
+// In the (near) future we should remove this custom deserializer and make it a hard error when we
+// find invalid vote accounts in snapshots.
+impl<'de> serde::Deserialize<'de> for VoteAccountsHashMap {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        struct VoteAccountsVisitor;
+
+        impl<'de> Visitor<'de> for VoteAccountsVisitor {
+            type Value = VoteAccountsHashMap;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("a map of vote accounts")
+            }
+
+            fn visit_map<M>(self, mut access: M) -> Result<Self::Value, M::Error>
+            where
+                M: MapAccess<'de>,
+            {
+                let mut accounts = HashMap::new();
+
+                while let Some((pubkey, (stake, account))) =
+                    access.next_entry::<Pubkey, (u64, AccountSharedData)>()?
+                {
+                    match VoteAccount::try_from(account) {
+                        Ok(vote_account) => {
+                            accounts.insert(pubkey, (stake, vote_account));
+                        }
+                        Err(e) => {
+                            log::warn!("failed to deserialize vote account: {e}");
+                        }
+                    }
+                }
+
+                Ok(VoteAccountsHashMap(accounts))
+            }
+        }
+
+        deserializer.deserialize_map(VoteAccountsVisitor)
+    }
+}
 
 #[cfg_attr(feature = "frozen-abi", derive(AbiExample))]
 #[derive(Clone, Debug, Deserialize)]
@@ -381,7 +457,7 @@ impl FromIterator<(Pubkey, (/*stake:*/ u64, VoteAccount))> for VoteAccounts {
     where
         I: IntoIterator<Item = (Pubkey, (u64, VoteAccount))>,
     {
-        Self::from(Arc::new(HashMap::from_iter(iter)))
+        Self::from(Arc::new(VoteAccountsHashMap::from_iter(iter)))
     }
 }
 
@@ -549,6 +625,36 @@ mod tests {
         assert_eq!(*vote_accounts.vote_accounts, vote_accounts_hash_map);
     }
 
+    #[test]
+    fn test_vote_accounts_deserialize_invalid_account() {
+        let mut rng = rand::thread_rng();
+        let mut vote_accounts_hash_map = HashMap::<Pubkey, (u64, AccountSharedData)>::new();
+
+        let (valid_account, _) = new_rand_vote_account(&mut rng, None);
+        vote_accounts_hash_map.insert(Pubkey::new_unique(), (0xAA, valid_account.clone()));
+
+        // bad data
+        let invalid_account_data =
+            AccountSharedData::new_data(42, &vec![0xFF; 42], &solana_sdk::vote::program::id())
+                .unwrap();
+        vote_accounts_hash_map.insert(Pubkey::new_unique(), (0xBB, invalid_account_data));
+
+        // wrong owner
+        let invalid_account_key =
+            AccountSharedData::new_data(42, &valid_account.data().to_vec(), &Pubkey::new_unique())
+                .unwrap();
+        vote_accounts_hash_map.insert(Pubkey::new_unique(), (0xCC, invalid_account_key));
+
+        // populate the map with 1 valid and 2 invalid accounts, ensure that we only get the valid
+        // one after deserialiation
+        let data = bincode::serialize(&vote_accounts_hash_map).unwrap();
+        let vote_accounts: VoteAccountsHashMap = bincode::deserialize(&data).unwrap();
+
+        assert_eq!(vote_accounts.len(), 1);
+        let (stake, _account) = vote_accounts.values().next().unwrap();
+        assert_eq!(*stake, 0xAA);
+    }
+
     #[test]
     fn test_staked_nodes() {
         let mut rng = rand::thread_rng();