From e1793e5a135a0b26a68d5e1debe14e082a90bf6d Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Mon, 30 Nov 2020 17:18:33 +0000 Subject: [PATCH] caches vote-state de-serialized from vote accounts (#13795) Gossip and other places repeatedly de-serialize vote-state stored in vote accounts. Ideally the first de-serialization should cache the result. This commit adds new VoteAccount type which lazily de-serializes VoteState from Account data and caches the result internally. Serialize and Deserialize traits are manually implemented to match existing code. So, despite changes to frozen_abi, this commit should be backward compatible. --- core/src/commitment_service.rs | 31 ++--- core/src/consensus.rs | 76 ++++++------ core/src/progress_map.rs | 10 +- core/src/replay_stage.rs | 36 +++--- core/src/rpc.rs | 4 +- core/src/validator.rs | 16 ++- frozen-abi/src/abi_example.rs | 1 + ledger-tool/src/main.rs | 9 +- ledger/src/blockstore.rs | 12 +- ledger/src/blockstore_processor.rs | 48 ++++---- ledger/src/staking_utils.rs | 82 +++++++------ runtime/src/bank.rs | 38 ++++-- runtime/src/epoch_stakes.rs | 45 ++++--- runtime/src/lib.rs | 1 + runtime/src/serde_snapshot/tests.rs | 2 +- runtime/src/stakes.rs | 24 ++-- runtime/src/vote_account.rs | 181 ++++++++++++++++++++++++++++ sdk/src/stake_weighted_timestamp.rs | 15 ++- 18 files changed, 433 insertions(+), 198 deletions(-) create mode 100644 runtime/src/vote_account.rs diff --git a/core/src/commitment_service.rs b/core/src/commitment_service.rs index 7742c7e1aa..6e71105048 100644 --- a/core/src/commitment_service.rs +++ b/core/src/commitment_service.rs @@ -176,19 +176,15 @@ impl AggregateCommitmentService { if lamports == 0 { continue; } - let vote_state = VoteState::from(&account); - if vote_state.is_none() { - continue; + if let Ok(vote_state) = account.vote_state().as_ref() { + Self::aggregate_commitment_for_vote_account( + &mut commitment, + &mut rooted_stake, + vote_state, + ancestors, + lamports, + ); } - - let vote_state = vote_state.unwrap(); - Self::aggregate_commitment_for_vote_account( - &mut commitment, - &mut rooted_stake, - &vote_state, - ancestors, - lamports, - ); } (commitment, rooted_stake) @@ -482,9 +478,14 @@ mod tests { #[test] fn test_highest_confirmed_root_advance() { fn get_vote_account_root_slot(vote_pubkey: Pubkey, bank: &Arc) -> Slot { - let account = &bank.vote_accounts()[&vote_pubkey].1; - let vote_state = VoteState::from(account).unwrap(); - vote_state.root_slot.unwrap() + let (_stake, vote_account) = bank.get_vote_account(&vote_pubkey).unwrap(); + let slot = vote_account + .vote_state() + .as_ref() + .unwrap() + .root_slot + .unwrap(); + slot } let block_commitment_cache = RwLock::new(BlockCommitmentCache::new_for_tests()); diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 1b3ef0993c..121ad4e57f 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -5,9 +5,11 @@ use crate::{ use chrono::prelude::*; use solana_ledger::{ancestor_iterator::AncestorIterator, blockstore::Blockstore, blockstore_db}; use solana_measure::measure::Measure; -use solana_runtime::{bank::Bank, bank_forks::BankForks, commitment::VOTE_THRESHOLD_SIZE}; +use solana_runtime::{ + bank::Bank, bank_forks::BankForks, commitment::VOTE_THRESHOLD_SIZE, + vote_account::ArcVoteAccount, +}; use solana_sdk::{ - account::Account, clock::{Slot, UnixTimestamp}, hash::Hash, instruction::Instruction, @@ -214,7 +216,7 @@ impl Tower { all_pubkeys: &mut PubkeyReferences, ) -> ComputedBankState where - F: Iterator, + F: IntoIterator, { let mut voted_stakes = HashMap::new(); let mut total_stake = 0; @@ -228,20 +230,20 @@ impl Tower { continue; } trace!("{} {} with stake {}", node_pubkey, key, voted_stake); - let vote_state = VoteState::from(&account); - if vote_state.is_none() { - datapoint_warn!( - "tower_warn", - ( - "warn", - format!("Unable to get vote_state from account {}", key), - String - ), - ); - continue; - } - let mut vote_state = vote_state.unwrap(); - + let mut vote_state = match account.vote_state().as_ref() { + Err(_) => { + datapoint_warn!( + "tower_warn", + ( + "warn", + format!("Unable to get vote_state from account {}", key), + String + ), + ); + continue; + } + Ok(vote_state) => vote_state.clone(), + }; for vote in &vote_state.votes { let key = all_pubkeys.get_or_insert(&key); lockout_intervals @@ -376,9 +378,9 @@ impl Tower { } fn last_voted_slot_in_bank(bank: &Bank, vote_account_pubkey: &Pubkey) -> Option { - let vote_account = bank.vote_accounts().get(vote_account_pubkey)?.1.clone(); - let bank_vote_state = VoteState::deserialize(&vote_account.data).ok()?; - bank_vote_state.last_voted_slot() + let (_stake, vote_account) = bank.get_vote_account(vote_account_pubkey)?; + let slot = vote_account.vote_state().as_ref().ok()?.last_voted_slot(); + slot } pub fn new_vote_from_bank(&self, bank: &Bank, vote_account_pubkey: &Pubkey) -> (Vote, usize) { @@ -509,7 +511,7 @@ impl Tower { descendants: &HashMap>, progress: &ProgressMap, total_stake: u64, - epoch_vote_accounts: &HashMap, + epoch_vote_accounts: &HashMap, ) -> SwitchForkDecision { self.last_voted_slot() .map(|last_voted_slot| { @@ -703,7 +705,7 @@ impl Tower { descendants: &HashMap>, progress: &ProgressMap, total_stake: u64, - epoch_vote_accounts: &HashMap, + epoch_vote_accounts: &HashMap, ) -> SwitchForkDecision { let decision = self.make_check_switch_threshold_decision( switch_slot, @@ -1058,10 +1060,12 @@ impl Tower { root: Slot, bank: &Bank, ) { - if let Some((_stake, vote_account)) = bank.vote_accounts().get(vote_account_pubkey) { - let vote_state = VoteState::deserialize(&vote_account.data) - .expect("vote_account isn't a VoteState?"); - self.lockouts = vote_state; + if let Some((_stake, vote_account)) = bank.get_vote_account(vote_account_pubkey) { + self.lockouts = vote_account + .vote_state() + .as_ref() + .expect("vote_account isn't a VoteState?") + .clone(); self.initialize_root(root); self.initialize_lockouts(|v| v.slot > root); trace!( @@ -1286,7 +1290,8 @@ pub mod test { }, }; use solana_sdk::{ - clock::Slot, hash::Hash, pubkey::Pubkey, signature::Signer, slot_history::SlotHistory, + account::Account, clock::Slot, hash::Hash, pubkey::Pubkey, signature::Signer, + slot_history::SlotHistory, }; use solana_vote_program::{ vote_state::{Vote, VoteStateVersions, MAX_LOCKOUT_HISTORY}, @@ -1604,7 +1609,7 @@ pub mod test { (bank_forks, progress, heaviest_subtree_fork_choice) } - fn gen_stakes(stake_votes: &[(u64, &[u64])]) -> Vec<(Pubkey, (u64, Account))> { + fn gen_stakes(stake_votes: &[(u64, &[u64])]) -> Vec<(Pubkey, (u64, ArcVoteAccount))> { let mut stakes = vec![]; for (lamports, votes) in stake_votes { let mut account = Account::default(); @@ -1619,7 +1624,10 @@ pub mod test { &mut account.data, ) .expect("serialize state"); - stakes.push((solana_sdk::pubkey::new_rand(), (*lamports, account))); + stakes.push(( + solana_sdk::pubkey::new_rand(), + (*lamports, ArcVoteAccount::from(account)), + )); } stakes } @@ -1973,16 +1981,16 @@ pub mod test { } info!("local tower: {:#?}", tower.lockouts.votes); - let vote_accounts = vote_simulator + let observed = vote_simulator .bank_forks .read() .unwrap() .get(next_unlocked_slot) .unwrap() - .vote_accounts(); - let observed = vote_accounts.get(&vote_pubkey).unwrap(); - let state = VoteState::from(&observed.1).unwrap(); - info!("observed tower: {:#?}", state.votes); + .get_vote_account(&vote_pubkey) + .unwrap(); + let state = observed.1.vote_state(); + info!("observed tower: {:#?}", state.as_ref().unwrap().votes); let num_slots_to_try = 200; cluster_votes diff --git a/core/src/progress_map.rs b/core/src/progress_map.rs index edd144a819..8b27687f83 100644 --- a/core/src/progress_map.rs +++ b/core/src/progress_map.rs @@ -6,8 +6,8 @@ use crate::{ {consensus::Stake, consensus::VotedStakes}, }; use solana_ledger::blockstore_processor::{ConfirmationProgress, ConfirmationTiming}; -use solana_runtime::{bank::Bank, bank_forks::BankForks}; -use solana_sdk::{account::Account, clock::Slot, hash::Hash, pubkey::Pubkey}; +use solana_runtime::{bank::Bank, bank_forks::BankForks, vote_account::ArcVoteAccount}; +use solana_sdk::{clock::Slot, hash::Hash, pubkey::Pubkey}; use std::{ collections::{BTreeMap, HashMap, HashSet}, rc::Rc, @@ -262,7 +262,7 @@ impl PropagatedStats { node_pubkey: &Pubkey, all_pubkeys: &mut PubkeyReferences, vote_account_pubkeys: &[Pubkey], - epoch_vote_accounts: &HashMap, + epoch_vote_accounts: &HashMap, ) { let cached_pubkey = all_pubkeys.get_or_insert(node_pubkey); self.propagated_node_ids.insert(cached_pubkey); @@ -440,7 +440,7 @@ mod test { let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys .iter() .skip(num_vote_accounts - staked_vote_accounts) - .map(|pubkey| (*pubkey, (1, Account::default()))) + .map(|pubkey| (*pubkey, (1, ArcVoteAccount::default()))) .collect(); let mut stats = PropagatedStats::default(); @@ -507,7 +507,7 @@ mod test { let epoch_vote_accounts: HashMap<_, _> = vote_account_pubkeys .iter() .skip(num_vote_accounts - staked_vote_accounts) - .map(|pubkey| (*pubkey, (1, Account::default()))) + .map(|pubkey| (*pubkey, (1, ArcVoteAccount::default()))) .collect(); stats.add_node_pubkey_internal( &node_pubkey, diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index bbac7babe1..0a4bc9bc06 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -42,10 +42,7 @@ use solana_sdk::{ timing::timestamp, transaction::Transaction, }; -use solana_vote_program::{ - vote_instruction, - vote_state::{Vote, VoteState}, -}; +use solana_vote_program::{vote_instruction, vote_state::Vote}; use std::{ collections::{HashMap, HashSet}, ops::Deref, @@ -1132,26 +1129,27 @@ impl ReplayStage { if authorized_voter_keypairs.is_empty() { return; } - - let vote_state = - if let Some((_, vote_account)) = bank.vote_accounts().get(vote_account_pubkey) { - if let Some(vote_state) = VoteState::from(&vote_account) { - vote_state - } else { - warn!( - "Vote account {} is unreadable. Unable to vote", - vote_account_pubkey, - ); - return; - } - } else { + let vote_account = match bank.get_vote_account(vote_account_pubkey) { + None => { warn!( "Vote account {} does not exist. Unable to vote", vote_account_pubkey, ); return; - }; - + } + Some((_stake, 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; + } + Ok(vote_state) => vote_state, + }; let authorized_voter_pubkey = if let Some(authorized_voter_pubkey) = vote_state.get_authorized_voter(bank.epoch()) { authorized_voter_pubkey diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 33bb7db99c..f22f8afc4a 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -537,13 +537,15 @@ 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 (current_vote_accounts, delinquent_vote_accounts): ( Vec, Vec, ) = vote_accounts .iter() .map(|(pubkey, (activated_stake, account))| { - let vote_state = VoteState::from(&account).unwrap_or_default(); + let vote_state = account.vote_state(); + let vote_state = vote_state.as_ref().unwrap_or(&default_vote_state); let last_vote = if let Some(vote) = vote_state.votes.iter().last() { vote.slot } else { diff --git a/core/src/validator.rs b/core/src/validator.rs index 74c44187f8..6a671359e6 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -1125,33 +1125,37 @@ fn get_stake_percent_in_gossip(bank: &Bank, cluster_info: &ClusterInfo, log: boo let my_id = cluster_info.id(); for (activated_stake, vote_account) in bank.vote_accounts().values() { - let vote_state = VoteState::from(&vote_account).unwrap_or_default(); total_activated_stake += activated_stake; if *activated_stake == 0 { continue; } + let vote_state_node_pubkey = vote_account + .vote_state() + .as_ref() + .map(|vote_state| vote_state.node_pubkey) + .unwrap_or_default(); if let Some(peer) = all_tvu_peers .iter() - .find(|peer| peer.id == vote_state.node_pubkey) + .find(|peer| peer.id == vote_state_node_pubkey) { if peer.shred_version == my_shred_version { trace!( "observed {} in gossip, (activated_stake={})", - vote_state.node_pubkey, + vote_state_node_pubkey, activated_stake ); online_stake += activated_stake; } else { wrong_shred_stake += activated_stake; - wrong_shred_nodes.push((*activated_stake, vote_state.node_pubkey)); + wrong_shred_nodes.push((*activated_stake, vote_state_node_pubkey)); } - } else if vote_state.node_pubkey == my_id { + } else if vote_state_node_pubkey == my_id { online_stake += activated_stake; // This node is online } else { offline_stake += activated_stake; - offline_nodes.push((*activated_stake, vote_state.node_pubkey)); + offline_nodes.push((*activated_stake, vote_state_node_pubkey)); } } diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 3b38a38e86..2d24d05f23 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -188,6 +188,7 @@ example_impls! { f32, 0.0f32 } example_impls! { f64, 0.0f64 } example_impls! { String, String::new() } example_impls! { std::time::Duration, std::time::Duration::from_secs(0) } +example_impls! { std::sync::Once, std::sync::Once::new() } use std::sync::atomic::*; diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 3927b14a88..47e49e6f4d 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -299,6 +299,7 @@ fn graph_forks(bank_forks: &BankForks, include_all_votes: bool) -> 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]; @@ -308,7 +309,8 @@ fn graph_forks(bank_forks: &BankForks, include_all_votes: bool) -> String { .map(|(_, (stake, _))| stake) .sum(); for (_, (stake, vote_account)) in bank.vote_accounts() { - let vote_state = VoteState::from(&vote_account).unwrap_or_default(); + let vote_state = vote_account.vote_state(); + let vote_state = vote_state.as_ref().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, @@ -317,7 +319,7 @@ fn graph_forks(bank_forks: &BankForks, include_all_votes: bool) -> String { total_stake, )); if entry.0 < last_vote.slot { - *entry = (last_vote.slot, vote_state, stake, total_stake); + *entry = (last_vote.slot, vote_state.clone(), stake, total_stake); } } } @@ -348,7 +350,8 @@ fn graph_forks(bank_forks: &BankForks, include_all_votes: bool) -> String { let mut first = true; loop { for (_, (_, vote_account)) in bank.vote_accounts() { - let vote_state = VoteState::from(&vote_account).unwrap_or_default(); + let vote_state = vote_account.vote_state(); + let vote_state = vote_state.as_ref().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 diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 76de896e83..65d15c2f73 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -24,9 +24,11 @@ use rocksdb::DBRawIterator; use solana_measure::measure::Measure; use solana_metrics::{datapoint_debug, datapoint_error}; use solana_rayon_threadlimit::get_thread_count; -use solana_runtime::hardened_unpack::{unpack_genesis_archive, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}; +use solana_runtime::{ + hardened_unpack::{unpack_genesis_archive, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}, + vote_account::ArcVoteAccount, +}; use solana_sdk::{ - account::Account, clock::{Slot, UnixTimestamp, DEFAULT_TICKS_PER_SECOND, MS_PER_TICK}, genesis_config::GenesisConfig, hash::Hash, @@ -1623,7 +1625,7 @@ impl Blockstore { &self, slot: Slot, slot_duration: Duration, - stakes: &HashMap, + stakes: &HashMap, ) -> Result<()> { if !self.is_root(slot) { return Err(BlockstoreError::SlotNotRooted); @@ -5817,7 +5819,7 @@ pub mod tests { // Build epoch vote_accounts HashMap to test stake-weighted block time for (i, keypair) in vote_keypairs.iter().enumerate() { - stakes.insert(keypair.pubkey(), (1 + i as u64, Account::default())); + stakes.insert(keypair.pubkey(), (1 + i as u64, ArcVoteAccount::default())); } for slot in &[1, 2, 3, 8] { blockstore @@ -5876,7 +5878,7 @@ pub mod tests { // Build epoch vote_accounts HashMap to test stake-weighted block time let mut stakes = HashMap::new(); for (i, keypair) in vote_keypairs.iter().enumerate() { - stakes.insert(keypair.pubkey(), (1 + i as u64, Account::default())); + stakes.insert(keypair.pubkey(), (1 + i as u64, ArcVoteAccount::default())); } let slot_duration = Duration::from_millis(400); for slot in &[1, 2, 3, 8] { diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 28f17ff68b..9f689ae630 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -24,10 +24,10 @@ use solana_runtime::{ commitment::VOTE_THRESHOLD_SIZE, transaction_batch::TransactionBatch, transaction_utils::OrderedIterator, + vote_account::ArcVoteAccount, vote_sender_types::ReplayVoteSender, }; use solana_sdk::{ - account::Account, clock::{Slot, MAX_PROCESSING_AGE}, genesis_config::GenesisConfig, hash::Hash, @@ -36,7 +36,6 @@ use solana_sdk::{ timing::duration_as_ms, transaction::{Result, Transaction, TransactionError}, }; -use solana_vote_program::vote_state::VoteState; use std::{ cell::RefCell, collections::{HashMap, HashSet}, @@ -868,8 +867,11 @@ fn load_frozen_forks( // for newer cluster confirmed roots let new_root_bank = { if *root == max_root { - supermajority_root_from_vote_accounts(bank.slot(), bank.total_epoch_stake(), bank.vote_accounts() - .into_iter()).and_then(|supermajority_root| { + supermajority_root_from_vote_accounts( + bank.slot(), + bank.total_epoch_stake(), + bank.vote_accounts(), + ).and_then(|supermajority_root| { if supermajority_root > *root { // If there's a cluster confirmed root greater than our last // replayed root, then beccause the cluster confirmed root should @@ -960,30 +962,28 @@ fn supermajority_root(roots: &[(Slot, u64)], total_epoch_stake: u64) -> Option( bank_slot: Slot, total_epoch_stake: u64, - vote_accounts_iter: I, + vote_accounts: I, ) -> Option where - I: Iterator, + I: IntoIterator, { - let mut roots_stakes: Vec<(Slot, u64)> = vote_accounts_iter + let mut roots_stakes: Vec<(Slot, u64)> = vote_accounts + .into_iter() .filter_map(|(key, (stake, account))| { if stake == 0 { return None; } - let vote_state = VoteState::from(&account); - if vote_state.is_none() { - warn!( - "Unable to get vote_state from account {} in bank: {}", - key, bank_slot - ); - 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) => vote_state.root_slot.map(|root_slot| (root_slot, stake)), } - - vote_state - .unwrap() - .root_slot - .map(|root_slot| (root_slot, stake)) }) .collect(); @@ -1112,6 +1112,7 @@ pub mod tests { self, create_genesis_config_with_vote_accounts, ValidatorVoteKeypairs, }; use solana_sdk::{ + account::Account, epoch_schedule::EpochSchedule, hash::Hash, pubkey::Pubkey, @@ -1122,7 +1123,7 @@ pub mod tests { }; use solana_vote_program::{ self, - vote_state::{VoteStateVersions, MAX_LOCKOUT_HISTORY}, + vote_state::{VoteState, VoteStateVersions, MAX_LOCKOUT_HISTORY}, vote_transaction, }; use std::{collections::BTreeSet, sync::RwLock}; @@ -3146,7 +3147,7 @@ pub mod tests { #[test] fn test_supermajority_root_from_vote_accounts() { let convert_to_vote_accounts = - |roots_stakes: Vec<(Slot, u64)>| -> Vec<(Pubkey, (u64, Account))> { + |roots_stakes: Vec<(Slot, u64)>| -> Vec<(Pubkey, (u64, ArcVoteAccount))> { roots_stakes .into_iter() .map(|(root, stake)| { @@ -3156,7 +3157,10 @@ pub mod tests { Account::new(1, VoteState::size_of(), &solana_vote_program::id()); let versioned = VoteStateVersions::Current(Box::new(vote_state)); VoteState::serialize(&versioned, &mut vote_account.data).unwrap(); - (solana_sdk::pubkey::new_rand(), (stake, vote_account)) + ( + solana_sdk::pubkey::new_rand(), + (stake, ArcVoteAccount::from(vote_account)), + ) }) .collect_vec() }; diff --git a/ledger/src/staking_utils.rs b/ledger/src/staking_utils.rs index dcc16439a2..7436d55c51 100644 --- a/ledger/src/staking_utils.rs +++ b/ledger/src/staking_utils.rs @@ -1,10 +1,8 @@ -use solana_runtime::bank::Bank; +use solana_runtime::{bank::Bank, vote_account::ArcVoteAccount}; use solana_sdk::{ - account::Account, clock::{Epoch, Slot}, pubkey::Pubkey, }; -use solana_vote_program::vote_state::VoteState; use std::{borrow::Borrow, collections::HashMap}; /// Looks through vote accounts, and finds the latest slot that has achieved @@ -28,48 +26,42 @@ pub fn vote_account_stakes(bank: &Bank) -> HashMap { /// Collect the staked nodes, as named by staked vote accounts from the given bank pub fn staked_nodes(bank: &Bank) -> HashMap { - to_staked_nodes(to_vote_states(bank.vote_accounts().into_iter())) + to_staked_nodes(bank.vote_accounts()) } /// At the specified epoch, collect the delegate account balance and vote states for delegates /// that have non-zero balance in any of their managed staking accounts pub fn staked_nodes_at_epoch(bank: &Bank, epoch: Epoch) -> Option> { - bank.epoch_vote_accounts(epoch) - .map(|vote_accounts| to_staked_nodes(to_vote_states(vote_accounts.iter()))) + bank.epoch_vote_accounts(epoch).map(to_staked_nodes) } -// input (vote_pubkey, (stake, vote_account)) => (stake, vote_state) -fn to_vote_states( - node_staked_accounts: impl Iterator, impl Borrow<(u64, Account)>)>, -) -> impl Iterator { - node_staked_accounts.filter_map(|(_, stake_account)| { - VoteState::deserialize(&stake_account.borrow().1.data) - .ok() - .map(|vote_state| (stake_account.borrow().0, vote_state)) - }) -} - -// (stake, vote_state) => (node, stake) -fn to_staked_nodes( - node_staked_accounts: impl Iterator, -) -> HashMap { - let mut map: HashMap = HashMap::new(); - node_staked_accounts.for_each(|(stake, state)| { - map.entry(state.node_pubkey) - .and_modify(|s| *s += stake) - .or_insert(stake); - }); - map +fn to_staked_nodes( + vote_accounts: I, +) -> HashMap +where + I: IntoIterator, + V: Borrow<(u64 /*stake*/, ArcVoteAccount)>, +{ + let mut out: HashMap = HashMap::new(); + for (_ /*vote pubkey*/, stake_vote_account) in vote_accounts { + let (stake, vote_account) = stake_vote_account.borrow(); + if let Ok(vote_state) = vote_account.vote_state().as_ref() { + out.entry(vote_state.node_pubkey) + .and_modify(|s| *s += *stake) + .or_insert(*stake); + } + } + out } fn epoch_stakes_and_lockouts(bank: &Bank, epoch: Epoch) -> Vec<(u64, Option)> { - let node_staked_accounts = bank - .epoch_vote_accounts(epoch) + bank.epoch_vote_accounts(epoch) .expect("Bank state for epoch is missing") - .iter(); - - to_vote_states(node_staked_accounts) - .map(|(stake, states)| (stake, states.root_slot)) + .iter() + .filter_map(|(_ /*vote pubkey*/, (stake, vote_account))| { + let root_slot = vote_account.vote_state().as_ref().ok()?.root_slot; + Some((*stake, root_slot)) + }) .collect() } @@ -103,8 +95,9 @@ pub(crate) mod tests { use crate::genesis_utils::{ bootstrap_validator_stake_lamports, create_genesis_config, GenesisConfigInfo, }; + use rand::Rng; use solana_sdk::{ - account::from_account, + account::{from_account, Account}, clock::Clock, instruction::Instruction, pubkey::Pubkey, @@ -117,7 +110,10 @@ pub(crate) mod tests { stake_instruction, stake_state::{Authorized, Delegation, Lockup, Stake}, }; - use solana_vote_program::{vote_instruction, vote_state::VoteInit}; + use solana_vote_program::{ + vote_instruction, + vote_state::{VoteInit, VoteState, VoteStateVersions}, + }; use std::sync::Arc; fn new_from_parent(parent: &Arc, slot: Slot) -> Bank { @@ -340,8 +336,18 @@ pub(crate) mod tests { &Clock::default(), ), )); - - let result = to_staked_nodes(stakes.into_iter()); + let mut rng = rand::thread_rng(); + let vote_accounts = stakes.into_iter().map(|(stake, vote_state)| { + let account = Account::new_data( + rng.gen(), // lamports + &VoteStateVersions::Current(Box::new(vote_state)), + &Pubkey::new_unique(), // owner + ) + .unwrap(); + let vote_pubkey = Pubkey::new_unique(); + (vote_pubkey, (stake, ArcVoteAccount::from(account))) + }); + let result = to_staked_nodes(vote_accounts); assert_eq!(result.len(), 2); assert_eq!(result[&node1], 3); assert_eq!(result[&node2], 5); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 67ffe7e8e9..89898431e8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -21,6 +21,7 @@ use crate::{ system_instruction_processor::{get_system_account_kind, SystemAccountKind}, transaction_batch::TransactionBatch, transaction_utils::OrderedIterator, + vote_account::ArcVoteAccount, }; use byteorder::{ByteOrder, LittleEndian}; use itertools::Itertools; @@ -68,7 +69,7 @@ use solana_sdk::{ use solana_stake_program::stake_state::{ self, Delegation, InflationPointCalculationEvent, PointValue, }; -use solana_vote_program::{vote_instruction::VoteInstruction, vote_state::VoteState}; +use solana_vote_program::vote_instruction::VoteInstruction; use std::{ cell::RefCell, collections::{HashMap, HashSet}, @@ -379,7 +380,7 @@ pub struct TransactionBalancesSet { pub post_balances: TransactionBalances, } pub struct OverwrittenVoteAccount { - pub account: Account, + pub account: ArcVoteAccount, pub transaction_index: usize, pub transaction_result_index: usize, } @@ -1700,7 +1701,7 @@ impl Bank { .vote_accounts() .into_iter() .filter_map(|(pubkey, (_, account))| { - VoteState::from(&account).and_then(|state| { + account.vote_state().as_ref().ok().and_then(|state| { let timestamp_slot = state.last_timestamp.slot; if (self .feature_set @@ -2955,7 +2956,7 @@ impl Bank { #[allow(clippy::needless_collect)] fn distribute_rent_to_validators( &self, - vote_account_hashmap: &HashMap, + vote_account_hashmap: &HashMap, rent_to_be_distributed: u64, ) { let mut total_staked = 0; @@ -2970,9 +2971,8 @@ impl Bank { None } else { total_staked += *staked; - VoteState::deserialize(&account.data) - .ok() - .map(|vote_state| (vote_state.node_pubkey, *staked)) + let node_pubkey = account.vote_state().as_ref().ok()?.node_pubkey; + Some((node_pubkey, *staked)) } }) .collect::>(); @@ -4098,10 +4098,25 @@ impl Bank { /// current vote accounts for this bank along with the stake /// attributed to each account - pub fn vote_accounts(&self) -> HashMap { + /// Note: This clones the entire vote-accounts hashmap. For a single + /// account lookup use get_vote_account instead. + pub fn vote_accounts(&self) -> HashMap { self.stakes.read().unwrap().vote_accounts().clone() } + /// Vote account for the given vote account pubkey along with the stake. + pub fn get_vote_account( + &self, + vote_account: &Pubkey, + ) -> Option<(u64 /*stake*/, ArcVoteAccount)> { + self.stakes + .read() + .unwrap() + .vote_accounts() + .get(vote_account) + .cloned() + } + /// Get the EpochStakes for a given epoch pub fn epoch_stakes(&self, epoch: Epoch) -> Option<&EpochStakes> { self.epoch_stakes.get(&epoch) @@ -4113,7 +4128,10 @@ impl Bank { /// vote accounts for the specific epoch along with the stake /// attributed to each account - pub fn epoch_vote_accounts(&self, epoch: Epoch) -> Option<&HashMap> { + pub fn epoch_vote_accounts( + &self, + epoch: Epoch, + ) -> Option<&HashMap> { self.epoch_stakes .get(&epoch) .map(|epoch_stakes| Stakes::vote_accounts(epoch_stakes.stakes())) @@ -7741,7 +7759,7 @@ pub(crate) mod tests { accounts .iter() .filter_map(|(pubkey, (stake, account))| { - if let Ok(vote_state) = VoteState::deserialize(&account.data) { + if let Ok(vote_state) = account.vote_state().as_ref() { if vote_state.node_pubkey == leader_pubkey { Some((*pubkey, *stake)) } else { diff --git a/runtime/src/epoch_stakes.rs b/runtime/src/epoch_stakes.rs index ad55b88a4e..b50cafaffc 100644 --- a/runtime/src/epoch_stakes.rs +++ b/runtime/src/epoch_stakes.rs @@ -1,7 +1,6 @@ -use crate::stakes::Stakes; +use crate::{stakes::Stakes, vote_account::ArcVoteAccount}; use serde::{Deserialize, Serialize}; -use solana_sdk::{account::Account, clock::Epoch, pubkey::Pubkey}; -use solana_vote_program::vote_state::VoteState; +use solana_sdk::{clock::Epoch, pubkey::Pubkey}; use std::{collections::HashMap, sync::Arc}; pub type NodeIdToVoteAccounts = HashMap; @@ -58,7 +57,7 @@ impl EpochStakes { } fn parse_epoch_vote_accounts( - epoch_vote_accounts: &HashMap, + epoch_vote_accounts: &HashMap, leader_schedule_epoch: Epoch, ) -> (u64, NodeIdToVoteAccounts, EpochAuthorizedVoters) { let mut node_id_to_vote_accounts: NodeIdToVoteAccounts = HashMap::new(); @@ -69,19 +68,21 @@ impl EpochStakes { let epoch_authorized_voters = epoch_vote_accounts .iter() .filter_map(|(key, (stake, account))| { - let vote_state = VoteState::from(&account); - if vote_state.is_none() { - datapoint_warn!( - "parse_epoch_vote_accounts", - ( - "warn", - format!("Unable to get vote_state from account {}", key), - String - ), - ); - return None; - } - let vote_state = vote_state.unwrap(); + let vote_state = account.vote_state(); + let vote_state = match vote_state.as_ref() { + Err(_) => { + datapoint_warn!( + "parse_epoch_vote_accounts", + ( + "warn", + format!("Unable to get vote_state from account {}", key), + String + ), + ); + return None; + } + Ok(vote_state) => vote_state, + }; if *stake > 0 { // Read out the authorized voters let authorized_voter = vote_state @@ -113,6 +114,7 @@ impl EpochStakes { #[cfg(test)] pub(crate) mod tests { use super::*; + use solana_sdk::account::Account; use solana_vote_program::vote_state::create_account_with_authorized; use std::iter; @@ -181,9 +183,12 @@ pub(crate) mod tests { let epoch_vote_accounts: HashMap<_, _> = vote_accounts_map .iter() .flat_map(|(_, vote_accounts)| { - vote_accounts - .iter() - .map(|v| (v.vote_account, (stake_per_account, v.account.clone()))) + vote_accounts.iter().map(|v| { + ( + v.vote_account, + (stake_per_account, ArcVoteAccount::from(v.account.clone())), + ) + }) }) .collect(); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 35bc6c730d..a1eacdff22 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -29,6 +29,7 @@ pub mod status_cache; mod system_instruction_processor; pub mod transaction_batch; pub mod transaction_utils; +pub mod vote_account; pub mod vote_sender_types; extern crate solana_config_program; diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index eeb633ba38..d9560b0328 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -261,7 +261,7 @@ mod test_bank_serialize { // These some what long test harness is required to freeze the ABI of // Bank's serialization due to versioned nature - #[frozen_abi(digest = "Giao4XJq9QgW78sqmT3nRMvENt4BgHXdzphCDGFPbXqW")] + #[frozen_abi(digest = "8bNY87hccyDYCRar1gM3NSvpvtiUM3W3rGeJLJayz42e")] #[derive(Serialize, AbiExample)] pub struct BankAbiTestWrapperFuture { #[serde(serialize_with = "wrapper_future")] diff --git a/runtime/src/stakes.rs b/runtime/src/stakes.rs index 073547ef33..abee464982 100644 --- a/runtime/src/stakes.rs +++ b/runtime/src/stakes.rs @@ -1,16 +1,16 @@ //! Stakes serve as a cache of stake and vote accounts to derive //! node stakes +use crate::vote_account::ArcVoteAccount; use solana_sdk::{ account::Account, clock::Epoch, pubkey::Pubkey, sysvar::stake_history::StakeHistory, }; use solana_stake_program::stake_state::{new_stake_history_entry, Delegation, StakeState}; -use solana_vote_program::vote_state::VoteState; use std::collections::HashMap; #[derive(Default, Clone, PartialEq, Debug, Deserialize, Serialize, AbiExample)] pub struct Stakes { /// vote accounts - vote_accounts: HashMap, + vote_accounts: HashMap, /// stake_delegations stake_delegations: HashMap, @@ -106,7 +106,7 @@ impl Stakes { + self .vote_accounts .iter() - .map(|(_pubkey, (_staked, vote_account))| vote_account.lamports) + .map(|(_pubkey, (_staked, vote_account))| vote_account.lamports()) .sum::() } @@ -121,7 +121,7 @@ impl Stakes { pubkey: &Pubkey, account: &Account, fix_stake_deactivate: bool, - ) -> Option { + ) -> Option { if solana_vote_program::check_id(&account.owner) { let old = self.vote_accounts.remove(pubkey); if account.lamports != 0 { @@ -137,7 +137,8 @@ impl Stakes { |v| v.0, ); - self.vote_accounts.insert(*pubkey, (stake, account.clone())); + self.vote_accounts + .insert(*pubkey, (stake, ArcVoteAccount::from(account.clone()))); } old.map(|(_, account)| account) } else if solana_stake_program::check_id(&account.owner) { @@ -191,7 +192,7 @@ impl Stakes { } } - pub fn vote_accounts(&self) -> &HashMap { + pub fn vote_accounts(&self) -> &HashMap { &self.vote_accounts } @@ -200,11 +201,12 @@ impl Stakes { } pub fn highest_staked_node(&self) -> Option { - self.vote_accounts + let (_pubkey, (_stake, vote_account)) = self + .vote_accounts .iter() - .max_by(|(_ak, av), (_bk, bv)| av.0.cmp(&bv.0)) - .and_then(|(_k, (_stake, account))| VoteState::from(account)) - .map(|vote_state| vote_state.node_pubkey) + .max_by(|(_ak, av), (_bk, bv)| av.0.cmp(&bv.0))?; + let node_pubkey = vote_account.vote_state().as_ref().ok()?.node_pubkey; + Some(node_pubkey) } } @@ -523,7 +525,7 @@ pub mod tests { pub fn vote_balance_and_warmed_staked(&self) -> u64 { self.vote_accounts .iter() - .map(|(_pubkey, (staked, account))| staked + account.lamports) + .map(|(_pubkey, (staked, account))| staked + account.lamports()) .sum() } } diff --git a/runtime/src/vote_account.rs b/runtime/src/vote_account.rs new file mode 100644 index 0000000000..d6eea0d948 --- /dev/null +++ b/runtime/src/vote_account.rs @@ -0,0 +1,181 @@ +use serde::de::{Deserialize, Deserializer}; +use serde::ser::{Serialize, Serializer}; +use solana_sdk::{account::Account, instruction::InstructionError}; +use solana_vote_program::vote_state::VoteState; +use std::ops::Deref; +use std::sync::{Arc, Once, RwLock, RwLockReadGuard}; + +// The value here does not matter. It will be overwritten +// at the first call to VoteAccount::vote_state(). +const INVALID_VOTE_STATE: Result = + Err(InstructionError::InvalidAccountData); + +#[derive(Clone, Debug, Default, PartialEq, AbiExample)] +pub struct ArcVoteAccount(Arc); + +#[derive(Debug, AbiExample)] +pub struct VoteAccount { + account: Account, + vote_state: RwLock>, + vote_state_once: Once, +} + +impl VoteAccount { + pub fn lamports(&self) -> u64 { + self.account.lamports + } + + pub fn vote_state(&self) -> RwLockReadGuard> { + self.vote_state_once.call_once(|| { + *self.vote_state.write().unwrap() = VoteState::deserialize(&self.account.data); + }); + self.vote_state.read().unwrap() + } +} + +impl Deref for ArcVoteAccount { + type Target = VoteAccount; + + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + +impl Serialize for ArcVoteAccount { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.account.serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for ArcVoteAccount { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let account = Account::deserialize(deserializer)?; + Ok(Self::from(account)) + } +} + +impl From for ArcVoteAccount { + fn from(account: Account) -> Self { + Self(Arc::new(VoteAccount::from(account))) + } +} + +impl From for VoteAccount { + fn from(account: Account) -> Self { + Self { + account, + vote_state: RwLock::new(INVALID_VOTE_STATE), + vote_state_once: Once::new(), + } + } +} + +impl Default for VoteAccount { + fn default() -> Self { + Self { + account: Account::default(), + vote_state: RwLock::new(INVALID_VOTE_STATE), + vote_state_once: Once::new(), + } + } +} + +impl PartialEq for VoteAccount { + fn eq(&self, other: &Self) -> bool { + self.account == other.account + } +} + +#[cfg(test)] +mod tests { + use super::*; + use rand::Rng; + use solana_sdk::{pubkey::Pubkey, sysvar::clock::Clock}; + use solana_vote_program::vote_state::{VoteInit, VoteStateVersions}; + + fn new_rand_vote_account(rng: &mut R) -> (Account, VoteState) { + let vote_init = VoteInit { + node_pubkey: Pubkey::new_unique(), + authorized_voter: Pubkey::new_unique(), + authorized_withdrawer: Pubkey::new_unique(), + commission: rng.gen(), + }; + let clock = Clock { + slot: rng.gen(), + epoch_start_timestamp: rng.gen(), + epoch: rng.gen(), + leader_schedule_epoch: rng.gen(), + unix_timestamp: rng.gen(), + }; + let vote_state = VoteState::new(&vote_init, &clock); + let account = Account::new_data( + rng.gen(), // lamports + &VoteStateVersions::Current(Box::new(vote_state.clone())), + &Pubkey::new_unique(), // owner + ) + .unwrap(); + (account, vote_state) + } + + #[test] + fn test_vote_account() { + let mut rng = rand::thread_rng(); + let (account, vote_state) = new_rand_vote_account(&mut rng); + let lamports = account.lamports; + let vote_account = ArcVoteAccount::from(account); + assert_eq!(lamports, vote_account.lamports()); + assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap()); + // 2nd call to .vote_state() should return the cached value. + assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap()); + } + + #[test] + fn test_vote_account_serialize() { + let mut rng = rand::thread_rng(); + let (account, vote_state) = new_rand_vote_account(&mut rng); + let vote_account = ArcVoteAccount::from(account.clone()); + assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap()); + // Assert than ArcVoteAccount has the same wire format as Account. + assert_eq!( + bincode::serialize(&account).unwrap(), + bincode::serialize(&vote_account).unwrap() + ); + } + + #[test] + fn test_vote_account_deserialize() { + let mut rng = rand::thread_rng(); + let (account, vote_state) = new_rand_vote_account(&mut rng); + let data = bincode::serialize(&account).unwrap(); + let vote_account = ArcVoteAccount::from(account); + assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap()); + let other_vote_account: ArcVoteAccount = bincode::deserialize(&data).unwrap(); + assert_eq!(vote_account, other_vote_account); + assert_eq!( + vote_state, + *other_vote_account.vote_state().as_ref().unwrap() + ); + } + + #[test] + fn test_vote_account_round_trip() { + let mut rng = rand::thread_rng(); + let (account, vote_state) = new_rand_vote_account(&mut rng); + let vote_account = ArcVoteAccount::from(account); + assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap()); + let data = bincode::serialize(&vote_account).unwrap(); + let other_vote_account: ArcVoteAccount = bincode::deserialize(&data).unwrap(); + // Assert that serialize->deserialized returns the same ArcVoteAccount. + assert_eq!(vote_account, other_vote_account); + assert_eq!( + vote_state, + *other_vote_account.vote_state().as_ref().unwrap() + ); + } +} diff --git a/sdk/src/stake_weighted_timestamp.rs b/sdk/src/stake_weighted_timestamp.rs index 2ea172cd87..2da3822ee1 100644 --- a/sdk/src/stake_weighted_timestamp.rs +++ b/sdk/src/stake_weighted_timestamp.rs @@ -1,7 +1,6 @@ /// A helper for calculating a stake-weighted timestamp estimate from a set of timestamps and epoch /// stake. use solana_sdk::{ - account::Account, clock::{Slot, UnixTimestamp}, pubkey::Pubkey, }; @@ -19,9 +18,9 @@ pub enum EstimateType { Unbounded, // Deprecated. Remove in the Solana v1.6.0 timeframe } -pub fn calculate_stake_weighted_timestamp( +pub fn calculate_stake_weighted_timestamp( unique_timestamps: &HashMap, - stakes: &HashMap, + stakes: &HashMap, slot: Slot, slot_duration: Duration, estimate_type: EstimateType, @@ -44,9 +43,9 @@ pub fn calculate_stake_weighted_timestamp( } } -fn calculate_unbounded_stake_weighted_timestamp( +fn calculate_unbounded_stake_weighted_timestamp( unique_timestamps: &HashMap, - stakes: &HashMap, + stakes: &HashMap, slot: Slot, slot_duration: Duration, ) -> Option { @@ -71,9 +70,9 @@ fn calculate_unbounded_stake_weighted_timestamp( } } -fn calculate_bounded_stake_weighted_timestamp( +fn calculate_bounded_stake_weighted_timestamp( unique_timestamps: &HashMap, - stakes: &HashMap, + stakes: &HashMap, slot: Slot, slot_duration: Duration, epoch_start_timestamp: Option<(Slot, UnixTimestamp)>, @@ -135,7 +134,7 @@ fn calculate_bounded_stake_weighted_timestamp( #[cfg(test)] pub mod tests { use super::*; - use solana_sdk::native_token::sol_to_lamports; + use solana_sdk::{account::Account, native_token::sol_to_lamports}; #[test] fn test_calculate_stake_weighted_timestamp() {