VoteAccount: remove OnceLock around VoteState (anza-xyz#2659)

* VoteAccount: remove OnceLock around VoteState

The lazy-init OnceLock logic wasn't really being used. Execution/perf
wise this code doesn't change anything since we were already forcing early
deserialization in StakesCache::check_and_store:

-                            // Called to eagerly deserialize vote state
-                            let _res = vote_account.vote_state();

For more context see
https://discord.com/channels/428295358100013066/439194979856809985/1268759289531596872
https://discord.com/channels/428295358100013066/439194979856809985/1272661616399224892

* 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.

* VoteAccount: remove Deserialize impl

VoteAccount is never deserialized individually, but only as part of
VoteAccounts, which has a custom deser that doesn't require VoteAccount
to implement Deserialize

core/Cargo.toml

@@ -110,6 +110,7 @@ solana-stake-program = { workspace = true }
 solana-unified-scheduler-pool = { workspace = true, features = [
     "dev-context-only-utils",
 ] }
+solana-vote = { workspace = true, features = ["dev-context-only-utils"] }
 static_assertions = { workspace = true }
 systemstat = { workspace = true }
 test-case = { workspace = true }

core/src/commitment_service.rs

@@ -202,19 +202,17 @@ impl AggregateCommitmentService {
             }
             let vote_state = if pubkey == node_vote_pubkey {
                 // Override old vote_state in bank with latest one for my own vote pubkey
-                Ok(node_vote_state)
+                node_vote_state
             } else {
                 account.vote_state()
             };
-            if let Ok(vote_state) = vote_state {
-                Self::aggregate_commitment_for_vote_account(
-                    &mut commitment,
-                    &mut rooted_stake,
-                    vote_state,
-                    ancestors,
-                    *lamports,
-                );
-            }
+            Self::aggregate_commitment_for_vote_account(
+                &mut commitment,
+                &mut rooted_stake,
+                vote_state,
+                ancestors,
+                *lamports,
+            );
         }
 
         (commitment, rooted_stake)
@@ -546,7 +544,7 @@ mod tests {
     fn test_highest_super_majority_root_advance() {
         fn get_vote_state(vote_pubkey: Pubkey, bank: &Bank) -> VoteState {
             let vote_account = bank.get_vote_account(&vote_pubkey).unwrap();
-            vote_account.vote_state().cloned().unwrap()
+            vote_account.vote_state().clone()
         }
 
         let block_commitment_cache = RwLock::new(BlockCommitmentCache::new_for_tests());

core/src/consensus.rs

@@ -379,20 +379,7 @@ impl Tower {
                 continue;
             }
             trace!("{} {} with stake {}", vote_account_pubkey, key, voted_stake);
-            let mut vote_state = match account.vote_state().cloned() {
-                Err(_) => {
-                    datapoint_warn!(
-                        "tower_warn",
-                        (
-                            "warn",
-                            format!("Unable to get vote_state from account {key}"),
-                            String
-                        ),
-                    );
-                    continue;
-                }
-                Ok(vote_state) => vote_state,
-            };
+            let mut vote_state = account.vote_state().clone();
             for vote in &vote_state.votes {
                 lockout_intervals
                     .entry(vote.lockout.last_locked_out_slot())
@@ -591,7 +578,7 @@ impl Tower {
     pub fn last_voted_slot_in_bank(bank: &Bank, vote_account_pubkey: &Pubkey) -> Option<Slot> {
         let vote_account = bank.get_vote_account(vote_account_pubkey)?;
         let vote_state = vote_account.vote_state();
-        vote_state.as_ref().ok()?.last_voted_slot()
+        vote_state.last_voted_slot()
     }
 
     pub fn record_bank_vote(&mut self, bank: &Bank) -> Option<Slot> {
@@ -1576,10 +1563,7 @@ impl Tower {
         bank: &Bank,
     ) {
         if let Some(vote_account) = bank.get_vote_account(vote_account_pubkey) {
-            self.vote_state = vote_account
-                .vote_state()
-                .cloned()
-                .expect("vote_account isn't a VoteState?");
+            self.vote_state = vote_account.vote_state().clone();
             self.initialize_root(root);
             self.initialize_lockouts(|v| v.slot() > root);
             trace!(
@@ -2428,7 +2412,7 @@ pub mod test {
             .get_vote_account(&vote_pubkey)
             .unwrap();
         let state = observed.vote_state();
-        info!("observed tower: {:#?}", state.as_ref().unwrap().votes);
+        info!("observed tower: {:#?}", state.votes);
 
         let num_slots_to_try = 200;
         cluster_votes

core/src/consensus/progress_map.rs

@@ -420,19 +420,7 @@ impl ProgressMap {
 
 #[cfg(test)]
 mod test {
-    use {
-        super::*,
-        solana_sdk::account::{Account, AccountSharedData},
-        solana_vote::vote_account::VoteAccount,
-    };
-
-    fn new_test_vote_account() -> VoteAccount {
-        let account = AccountSharedData::from(Account {
-            owner: solana_vote_program::id(),
-            ..Account::default()
-        });
-        VoteAccount::try_from(account).unwrap()
-    }
+    use {super::*, solana_vote::vote_account::VoteAccount};
 
     #[test]
     fn test_add_vote_pubkey() {
@@ -467,7 +455,7 @@ mod test {
         let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys
             .iter()
             .skip(num_vote_accounts - staked_vote_accounts)
-            .map(|pubkey| (*pubkey, (1, new_test_vote_account())))
+            .map(|pubkey| (*pubkey, (1, VoteAccount::new_random())))
             .collect();
 
         let mut stats = PropagatedStats::default();
@@ -509,7 +497,7 @@ mod test {
         let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys
             .iter()
             .skip(num_vote_accounts - staked_vote_accounts)
-            .map(|pubkey| (*pubkey, (1, new_test_vote_account())))
+            .map(|pubkey| (*pubkey, (1, VoteAccount::new_random())))
             .collect();
         stats.add_node_pubkey_internal(&node_pubkey, &vote_account_pubkeys, &epoch_vote_accounts);
         assert!(stats.propagated_node_ids.contains(&node_pubkey));

core/src/replay_stage.rs

@@ -2485,17 +2485,6 @@ impl ReplayStage {
             Some(vote_account) => vote_account,
         };
         let vote_state = vote_account.vote_state();
-        let vote_state = match vote_state.as_ref() {
-            Err(_) => {
-                warn!(
-                    "Vote account {} is unreadable.  Unable to vote",
-                    vote_account_pubkey,
-                );
-                return GenerateVoteTxResult::Failed;
-            }
-            Ok(vote_state) => vote_state,
-        };
-
         if vote_state.node_pubkey != node_keypair.pubkey() {
             info!(
                 "Vote account node_pubkey mismatch: {} (expected: {}).  Unable to vote",
@@ -3473,9 +3462,7 @@ impl ReplayStage {
         let Some(vote_account) = bank.get_vote_account(my_vote_pubkey) else {
             return;
         };
-        let Ok(mut bank_vote_state) = vote_account.vote_state().cloned() else {
-            return;
-        };
+        let mut bank_vote_state = vote_account.vote_state().clone();
         if bank_vote_state.last_voted_slot() <= tower.vote_state.last_voted_slot() {
             return;
         }
@@ -7612,10 +7599,7 @@ pub(crate) mod tests {
         let vote_account = expired_bank_child
             .get_vote_account(&my_vote_pubkey)
             .unwrap();
-        assert_eq!(
-            vote_account.vote_state().as_ref().unwrap().tower(),
-            vec![0, 1]
-        );
+        assert_eq!(vote_account.vote_state().tower(), vec![0, 1]);
         expired_bank_child.fill_bank_with_ticks_for_tests();
         expired_bank_child.freeze();
 

core/src/validator.rs

@@ -2488,7 +2488,7 @@ fn get_stake_percent_in_gossip(bank: &Bank, cluster_info: &ClusterInfo, log: boo
         if activated_stake == 0 {
             continue;
         }
-        let vote_state_node_pubkey = vote_account.node_pubkey().copied().unwrap_or_default();
+        let vote_state_node_pubkey = *vote_account.node_pubkey();
 
         if let Some(peer) = peers.get(&vote_state_node_pubkey) {
             if peer.shred_version() == my_shred_version {

core/src/vote_simulator.rs

@@ -104,7 +104,7 @@ impl VoteSimulator {
                     let tower_sync = if let Some(vote_account) =
                         parent_bank.get_vote_account(&keypairs.vote_keypair.pubkey())
                     {
-                        let mut vote_state = vote_account.vote_state().unwrap().clone();
+                        let mut vote_state = vote_account.vote_state().clone();
                         process_vote_unchecked(
                             &mut vote_state,
                             solana_vote_program::vote_state::Vote::new(
@@ -143,12 +143,7 @@ impl VoteSimulator {
                         .get_vote_account(&keypairs.vote_keypair.pubkey())
                         .unwrap();
                     let state = vote_account.vote_state();
-                    assert!(state
-                        .as_ref()
-                        .unwrap()
-                        .votes
-                        .iter()
-                        .any(|lockout| lockout.slot() == parent));
+                    assert!(state.votes.iter().any(|lockout| lockout.slot() == parent));
                 }
             }
             while new_bank.tick_height() < new_bank.max_tick_height() {

ledger-tool/src/main.rs

@@ -206,7 +206,6 @@ fn graph_forks(bank_forks: &BankForks, config: &GraphConfig) -> String {
 
     // Search all forks and collect the last vote made by each validator
     let mut last_votes = HashMap::new();
-    let default_vote_state = VoteState::default();
     for fork_slot in &fork_slots {
         let bank = &bank_forks[*fork_slot];
 
@@ -217,7 +216,6 @@ fn graph_forks(bank_forks: &BankForks, config: &GraphConfig) -> String {
             .sum();
         for (stake, vote_account) in bank.vote_accounts().values() {
             let vote_state = vote_account.vote_state();
-            let vote_state = vote_state.unwrap_or(&default_vote_state);
             if let Some(last_vote) = vote_state.votes.iter().last() {
                 let entry = last_votes.entry(vote_state.node_pubkey).or_insert((
                     last_vote.slot(),
@@ -258,7 +256,6 @@ fn graph_forks(bank_forks: &BankForks, config: &GraphConfig) -> String {
         loop {
             for (_, vote_account) in bank.vote_accounts().values() {
                 let vote_state = vote_account.vote_state();
-                let vote_state = vote_state.unwrap_or(&default_vote_state);
                 if let Some(last_vote) = vote_state.votes.iter().last() {
                     let validator_votes = all_votes.entry(vote_state.node_pubkey).or_default();
                     validator_votes

ledger/src/blockstore_processor.rs

@@ -1870,7 +1870,6 @@ fn load_frozen_forks(
             let new_root_bank = {
                 if bank_forks.read().unwrap().root() >= max_root {
                     supermajority_root_from_vote_accounts(
-                        bank.slot(),
                         bank.total_epoch_stake(),
                         &bank.vote_accounts(),
                     ).and_then(|supermajority_root| {
@@ -2005,27 +2004,17 @@ fn supermajority_root(roots: &[(Slot, u64)], total_epoch_stake: u64) -> Option<S
 }
 
 fn supermajority_root_from_vote_accounts(
-    bank_slot: Slot,
     total_epoch_stake: u64,
     vote_accounts: &VoteAccountsHashMap,
 ) -> Option<Slot> {
     let mut roots_stakes: Vec<(Slot, u64)> = vote_accounts
-        .iter()
-        .filter_map(|(key, (stake, account))| {
+        .values()
+        .filter_map(|(stake, account)| {
             if *stake == 0 {
                 return None;
             }
 
-            match account.vote_state().as_ref() {
-                Err(_) => {
-                    warn!(
-                        "Unable to get vote_state from account {} in bank: {}",
-                        key, bank_slot
-                    );
-                    None
-                }
-                Ok(vote_state) => Some((vote_state.root_slot?, *stake)),
-            }
+            Some((account.vote_state().root_slot?, *stake))
         })
         .collect();
 
@@ -4603,31 +4592,28 @@ pub mod tests {
         };
 
         let total_stake = 10;
-        let slot = 100;
 
         // Supermajority root should be None
-        assert!(
-            supermajority_root_from_vote_accounts(slot, total_stake, &HashMap::default()).is_none()
-        );
+        assert!(supermajority_root_from_vote_accounts(total_stake, &HashMap::default()).is_none());
 
         // Supermajority root should be None
         let roots_stakes = vec![(8, 1), (3, 1), (4, 1), (8, 1)];
         let accounts = convert_to_vote_accounts(roots_stakes);
-        assert!(supermajority_root_from_vote_accounts(slot, total_stake, &accounts).is_none());
+        assert!(supermajority_root_from_vote_accounts(total_stake, &accounts).is_none());
 
         // Supermajority root should be 4, has 7/10 of the stake
         let roots_stakes = vec![(8, 1), (3, 1), (4, 1), (8, 5)];
         let accounts = convert_to_vote_accounts(roots_stakes);
         assert_eq!(
-            supermajority_root_from_vote_accounts(slot, total_stake, &accounts).unwrap(),
+            supermajority_root_from_vote_accounts(total_stake, &accounts).unwrap(),
             4
         );
 
         // Supermajority root should be 8, it has 7/10 of the stake
         let roots_stakes = vec![(8, 1), (3, 1), (4, 1), (8, 6)];
         let accounts = convert_to_vote_accounts(roots_stakes);
         assert_eq!(
-            supermajority_root_from_vote_accounts(slot, total_stake, &accounts).unwrap(),
+            supermajority_root_from_vote_accounts(total_stake, &accounts).unwrap(),
             8
         );
     }

programs/bpf_loader/Cargo.toml

@@ -34,7 +34,7 @@ assert_matches = { workspace = true }
 memoffset = { workspace = true }
 rand = { workspace = true }
 solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }
-solana-vote = { workspace = true }
+solana-vote = { workspace = true, features = ["dev-context-only-utils"] }
 test-case = { workspace = true }
 
 [lib]

programs/bpf_loader/src/syscalls/mod.rs

@@ -4820,15 +4820,7 @@ mod tests {
         let mut vote_accounts_map = HashMap::new();
         vote_accounts_map.insert(
             vote_address,
-            (
-                expected_epoch_stake,
-                VoteAccount::try_from(AccountSharedData::new(
-                    0,
-                    0,
-                    &solana_sdk::vote::program::id(),
-                ))
-                .unwrap(),
-            ),
+            (expected_epoch_stake, VoteAccount::new_random()),
         );
 
         with_mock_invoke_context!(invoke_context, transaction_context, vec![]);

rpc/src/rpc.rs

@@ -92,7 +92,7 @@ use {
         TransactionBinaryEncoding, TransactionConfirmationStatus, TransactionStatus,
         UiConfirmedBlock, UiTransactionEncoding,
     },
-    solana_vote_program::vote_state::{VoteState, MAX_LOCKOUT_HISTORY},
+    solana_vote_program::vote_state::MAX_LOCKOUT_HISTORY,
     spl_token_2022::{
         extension::{
             interest_bearing_mint::InterestBearingConfig, BaseStateWithExtensions,
@@ -1004,7 +1004,6 @@ impl JsonRpcRequestProcessor {
         let epoch_vote_accounts = bank
             .epoch_vote_accounts(bank.get_epoch_and_slot_index(bank.slot()).0)
             .ok_or_else(Error::invalid_request)?;
-        let default_vote_state = VoteState::default();
         let delinquent_validator_slot_distance = config
             .delinquent_slot_distance
             .unwrap_or(DELINQUENT_VALIDATOR_SLOT_DISTANCE);
@@ -1021,7 +1020,6 @@ impl JsonRpcRequestProcessor {
                 }
 
                 let vote_state = account.vote_state();
-                let vote_state = vote_state.unwrap_or(&default_vote_state);
                 let last_vote = if let Some(vote) = vote_state.votes.iter().last() {
                     vote.slot()
                 } else {
@@ -4360,6 +4358,7 @@ pub mod tests {
             transaction::{
                 self, SimpleAddressLoader, Transaction, TransactionError, TransactionVersion,
             },
+            vote::state::VoteState,
         },
         solana_transaction_status::{
             EncodedConfirmedBlock, EncodedTransaction, EncodedTransactionWithStatusMeta,

runtime/src/bank.rs

@@ -2282,12 +2282,8 @@ impl Bank {
                 invalid_vote_keys.insert(vote_pubkey, InvalidCacheEntryReason::WrongOwner);
                 return None;
             }
-            let Ok(vote_state) = vote_account.vote_state().cloned() else {
-                invalid_vote_keys.insert(vote_pubkey, InvalidCacheEntryReason::BadState);
-                return None;
-            };
             let vote_with_stake_delegations = VoteWithStakeDelegations {
-                vote_state: Arc::new(vote_state),
+                vote_state: Arc::new(vote_account.vote_state().clone()),
                 vote_account: AccountSharedData::from(vote_account),
                 delegations: Vec::default(),
             };
@@ -2705,7 +2701,6 @@ impl Bank {
         let vote_accounts = self.vote_accounts();
         let recent_timestamps = vote_accounts.iter().filter_map(|(pubkey, (_, account))| {
             let vote_state = account.vote_state();
-            let vote_state = vote_state.as_ref().ok()?;
             let slot_delta = self.slot().checked_sub(vote_state.last_timestamp.slot)?;
             (slot_delta <= slots_per_epoch).then_some({
                 (
@@ -2881,7 +2876,7 @@ impl Bank {
         // up and can be used to set the collector id to the highest staked
         // node. If no staked nodes exist, allow fallback to an unstaked test
         // collector id during tests.
-        let collector_id = self.stakes_cache.stakes().highest_staked_node();
+        let collector_id = self.stakes_cache.stakes().highest_staked_node().copied();
         #[cfg(feature = "dev-context-only-utils")]
         let collector_id = collector_id.or(collector_id_for_tests);
         self.collector_id =