Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
runtime: transpose VoteAccount::vote_state() return to improve ergo…
Browse files Browse the repository at this point in the history
…nomics (#31256)
  • Loading branch information
t-nelson authored Apr 18, 2023
1 parent ad30a60 commit f34a6bc
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 29 deletions.
9 changes: 4 additions & 5 deletions core/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl Tower {
continue;
}
trace!("{} {} with stake {}", vote_account_pubkey, key, voted_stake);
let mut vote_state = match account.vote_state().as_ref() {
let mut vote_state = match account.vote_state().cloned() {
Err(_) => {
datapoint_warn!(
"tower_warn",
Expand All @@ -314,7 +314,7 @@ impl Tower {
);
continue;
}
Ok(vote_state) => vote_state.clone(),
Ok(vote_state) => vote_state,
};
for vote in &vote_state.votes {
lockout_intervals
Expand Down Expand Up @@ -1284,9 +1284,8 @@ impl Tower {
if let Some(vote_account) = bank.get_vote_account(vote_account_pubkey) {
self.vote_state = vote_account
.vote_state()
.as_ref()
.expect("vote_account isn't a VoteState?")
.clone();
.cloned()
.expect("vote_account isn't a VoteState?");
self.initialize_root(root);
self.initialize_lockouts(|v| v.slot() > root);
trace!(
Expand Down
4 changes: 1 addition & 3 deletions core/src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2894,9 +2894,7 @@ impl ReplayStage {
Tower::is_direct_vote_state_update_enabled(bank),
bank.get_vote_account(my_vote_pubkey),
) {
if let Some(mut bank_vote_state) =
vote_account.vote_state().as_ref().ok().cloned()
{
if let Ok(mut bank_vote_state) = vote_account.vote_state().cloned() {
if bank_vote_state.last_voted_slot()
> tower.vote_state.last_voted_slot()
{
Expand Down
4 changes: 2 additions & 2 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ 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.as_ref().unwrap_or(&default_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(),
Expand Down Expand Up @@ -561,7 +561,7 @@ 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.as_ref().unwrap_or(&default_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
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ impl JsonRpcRequestProcessor {
}

let vote_state = account.vote_state();
let vote_state = vote_state.as_ref().unwrap_or(&default_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 {
Expand Down
6 changes: 3 additions & 3 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ use {
collections::{HashMap, HashSet},
convert::{TryFrom, TryInto},
fmt, mem,
ops::{AddAssign, Deref, RangeInclusive},
ops::{AddAssign, RangeInclusive},
path::PathBuf,
rc::Rc,
sync::{
Expand Down Expand Up @@ -2740,8 +2740,8 @@ impl Bank {
invalid_vote_keys.insert(vote_pubkey, InvalidCacheEntryReason::WrongOwner);
return None;
}
let vote_state = match vote_account.vote_state().deref() {
Ok(vote_state) => vote_state.clone(),
let vote_state = match vote_account.vote_state().cloned() {
Ok(vote_state) => vote_state,
Err(_) => {
invalid_vote_keys.insert(vote_pubkey, InvalidCacheEntryReason::BadState);
return None;
Expand Down
25 changes: 10 additions & 15 deletions runtime/src/vote_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@ impl VoteAccount {
self.0.account.owner()
}

pub fn vote_state(&self) -> &Result<VoteState, Error> {
pub fn vote_state(&self) -> Result<&VoteState, &Error> {
// VoteState::deserialize deserializes a VoteStateVersions and then
// calls VoteStateVersions::convert_to_current.
self.0
.vote_state
.get_or_init(|| VoteState::deserialize(self.0.account.data()).map_err(Error::from))
.as_ref()
}

pub(crate) fn is_deserialized(&self) -> bool {
Expand All @@ -79,7 +80,7 @@ impl VoteAccount {

/// VoteState.node_pubkey of this vote-account.
pub fn node_pubkey(&self) -> Option<Pubkey> {
Some(self.vote_state().as_ref().ok()?.node_pubkey)
Some(self.vote_state().ok()?.node_pubkey)
}
}

Expand Down Expand Up @@ -395,17 +396,17 @@ mod tests {
let lamports = account.lamports();
let vote_account = VoteAccount::try_from(account).unwrap();
assert_eq!(lamports, vote_account.lamports());
assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap());
assert_eq!(vote_state, *vote_account.vote_state().unwrap());
// 2nd call to .vote_state() should return the cached value.
assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap());
assert_eq!(vote_state, *vote_account.vote_state().unwrap());
}

#[test]
fn test_vote_account_serialize() {
let mut rng = rand::thread_rng();
let (account, vote_state) = new_rand_vote_account(&mut rng, None);
let vote_account = VoteAccount::try_from(account.clone()).unwrap();
assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap());
assert_eq!(vote_state, *vote_account.vote_state().unwrap());
// Assert than VoteAccount has the same wire format as Account.
assert_eq!(
bincode::serialize(&account).unwrap(),
Expand All @@ -419,29 +420,23 @@ mod tests {
let (account, vote_state) = new_rand_vote_account(&mut rng, None);
let data = bincode::serialize(&account).unwrap();
let vote_account = VoteAccount::try_from(account).unwrap();
assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap());
assert_eq!(vote_state, *vote_account.vote_state().unwrap());
let other_vote_account: VoteAccount = bincode::deserialize(&data).unwrap();
assert_eq!(vote_account, other_vote_account);
assert_eq!(
vote_state,
*other_vote_account.vote_state().as_ref().unwrap()
);
assert_eq!(vote_state, *other_vote_account.vote_state().unwrap());
}

#[test]
fn test_vote_account_round_trip() {
let mut rng = rand::thread_rng();
let (account, vote_state) = new_rand_vote_account(&mut rng, None);
let vote_account = VoteAccount::try_from(account).unwrap();
assert_eq!(vote_state, *vote_account.vote_state().as_ref().unwrap());
assert_eq!(vote_state, *vote_account.vote_state().unwrap());
let data = bincode::serialize(&vote_account).unwrap();
let other_vote_account: VoteAccount = bincode::deserialize(&data).unwrap();
// Assert that serialize->deserialized returns the same VoteAccount.
assert_eq!(vote_account, other_vote_account);
assert_eq!(
vote_state,
*other_vote_account.vote_state().as_ref().unwrap()
);
assert_eq!(vote_state, *other_vote_account.vote_state().unwrap());
}

#[test]
Expand Down

0 comments on commit f34a6bc

Please sign in to comment.