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

Upgrade VoteState #8348

Merged
solana-grimes merged 1 commit into from
Feb 26, 2020
Merged

Upgrade VoteState #8348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions core/src/commitment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ mod tests {
use crate::genesis_utils::{create_genesis_config, GenesisConfigInfo};
use solana_sdk::pubkey::Pubkey;
use solana_stake_program::stake_state;
use solana_vote_program::vote_state;
use solana_vote_program::vote_state::{self, VoteStateVersions};

#[test]
fn test_block_commitment() {
Expand Down Expand Up @@ -446,13 +446,15 @@ mod tests {
let mut vote_state1 = VoteState::from(&vote_account1).unwrap();
vote_state1.process_slot_vote_unchecked(3);
vote_state1.process_slot_vote_unchecked(5);
vote_state1.to(&mut vote_account1).unwrap();
let versioned = VoteStateVersions::Current(Box::new(vote_state1));
VoteState::to(&versioned, &mut vote_account1).unwrap();
bank.store_account(&pk1, &vote_account1);

let mut vote_state2 = VoteState::from(&vote_account2).unwrap();
vote_state2.process_slot_vote_unchecked(9);
vote_state2.process_slot_vote_unchecked(10);
vote_state2.to(&mut vote_account2).unwrap();
let versioned = VoteStateVersions::Current(Box::new(vote_state2));
VoteState::to(&versioned, &mut vote_account2).unwrap();
bank.store_account(&pk2, &vote_account2);

let commitment = AggregateCommitmentService::aggregate_commitment(&ancestors, &bank);
Expand Down
13 changes: 9 additions & 4 deletions core/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,10 @@ pub mod test {
signature::{Keypair, Signer},
transaction::Transaction,
};
use solana_vote_program::{vote_instruction, vote_state::Vote};
use solana_vote_program::{
vote_instruction,
vote_state::{Vote, VoteStateVersions},
};
use std::collections::{HashMap, VecDeque};
use std::sync::RwLock;
use std::{thread::sleep, time::Duration};
Expand Down Expand Up @@ -706,9 +709,11 @@ pub mod test {
for slot in *votes {
vote_state.process_slot_vote_unchecked(*slot);
}
vote_state
.serialize(&mut account.data)
.expect("serialize state");
VoteState::serialize(
&VoteStateVersions::Current(Box::new(vote_state)),
&mut account.data,
)
.expect("serialize state");
stakes.push((Pubkey::new_rand(), (*lamports, account)));
}
stakes
Expand Down
8 changes: 5 additions & 3 deletions core/src/replay_stage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ pub(crate) mod tests {
transaction::TransactionError,
};
use solana_stake_program::stake_state;
use solana_vote_program::vote_state::{self, Vote, VoteState};
use solana_vote_program::vote_state::{self, Vote, VoteState, VoteStateVersions};
use std::{
fs::remove_dir_all,
iter,
Expand Down Expand Up @@ -1122,7 +1122,8 @@ pub(crate) mod tests {
let mut vote_account = bank.get_account(&pubkey).unwrap();
let mut vote_state = VoteState::from(&vote_account).unwrap();
vote_state.process_slot_vote_unchecked(slot);
vote_state.to(&mut vote_account).unwrap();
let versioned = VoteStateVersions::Current(Box::new(vote_state));
VoteState::to(&versioned, &mut vote_account).unwrap();
bank.store_account(&pubkey, &vote_account);
}

Expand Down Expand Up @@ -1706,7 +1707,8 @@ pub(crate) mod tests {
let mut leader_vote_account = bank.get_account(&pubkey).unwrap();
let mut vote_state = VoteState::from(&leader_vote_account).unwrap();
vote_state.process_slot_vote_unchecked(bank.slot());
vote_state.to(&mut leader_vote_account).unwrap();
let versioned = VoteStateVersions::Current(Box::new(vote_state));
VoteState::to(&versioned, &mut leader_vote_account).unwrap();
bank.store_account(&pubkey, &leader_vote_account);
}

Expand Down
34 changes: 23 additions & 11 deletions programs/stake/src/stake_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use solana_sdk::{
rent::Rent,
stake_history::{StakeHistory, StakeHistoryEntry},
};
use solana_vote_program::vote_state::VoteState;
use solana_vote_program::vote_state::{VoteState, VoteStateVersions};
use std::collections::HashSet;

#[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Copy)]
Expand Down Expand Up @@ -595,7 +595,7 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
let stake = Stake::new(
self.lamports()?.saturating_sub(meta.rent_exempt_reserve), // can't stake the rent ;)
vote_account.unsigned_key(),
&vote_account.state()?,
&State::<VoteStateVersions>::state(vote_account)?.convert_to_current(),
Copy link
Copy Markdown
Contributor Author

@carllin carllin Feb 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvines, I dislike this state() function because it requires whoever modifies this code to know that vote accounts are special/need special handling, and can't directly be deserialized into a VoteState object like its other counterparts(stake, storage, nonce, etc.). Ideally the Account type carries the type of the object it's storing so we can avoid this footgun (even in this PR, hopefully I haven't missed any conversions directly to VoteState from account.data), but that's a more sweeping change I can add as a follow-up.

Copy link
Copy Markdown
Contributor

@t-nelson t-nelson Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solana/sdk/src/account_utils.rs

Lines 38 to 40 in b18e405

fn state(&self) -> Result<T, InstructionError> {
self.try_account_ref()?.state()
}

We could add a trait that requires an implementation of fn convert_to_current() then make it a bound on T for State<T> and StateMut<T> hide it in the fn state() implementations

clock.epoch,
config,
);
Expand All @@ -605,7 +605,7 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
meta.authorized.check(signers, StakeAuthorize::Staker)?;
stake.redelegate(
vote_account.unsigned_key(),
&vote_account.state()?,
&State::<VoteStateVersions>::state(vote_account)?.convert_to_current(),
clock,
stake_history,
config,
Expand Down Expand Up @@ -778,7 +778,8 @@ pub fn redeem_rewards(
stake_history: Option<&StakeHistory>,
) -> Result<(u64, u64), InstructionError> {
if let StakeState::Stake(meta, mut stake) = stake_account.state()? {
let vote_state = vote_account.state()?;
let vote_state: VoteState =
StateMut::<VoteStateVersions>::state(vote_account)?.convert_to_current();

if let Some((voters_reward, stakers_reward)) =
stake.redeem_rewards(point_value, &vote_state, stake_history)
Expand Down Expand Up @@ -999,7 +1000,10 @@ mod tests {
100,
));
let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account);
vote_keyed_account.set_state(&vote_state).unwrap();
let vote_state_credits = vote_state.credits();
vote_keyed_account
.set_state(&VoteStateVersions::Current(Box::new(vote_state)))
.unwrap();

let stake_pubkey = Pubkey::new_rand();
let stake_lamports = 42;
Expand Down Expand Up @@ -1057,7 +1061,7 @@ mod tests {
deactivation_epoch: std::u64::MAX,
..Delegation::default()
},
credits_observed: vote_state.credits(),
credits_observed: vote_state_credits,
..Stake::default()
}
);
Expand Down Expand Up @@ -1105,7 +1109,7 @@ mod tests {
deactivation_epoch: std::u64::MAX,
..Delegation::default()
},
credits_observed: vote_state.credits(),
credits_observed: vote_state_credits,
..Stake::default()
}
);
Expand Down Expand Up @@ -1535,7 +1539,9 @@ mod tests {
100,
));
let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account);
vote_keyed_account.set_state(&VoteState::default()).unwrap();
vote_keyed_account
.set_state(&VoteStateVersions::Current(Box::new(VoteState::default())))
.unwrap();
assert_eq!(
stake_keyed_account.delegate(
&vote_keyed_account,
Expand Down Expand Up @@ -1624,7 +1630,9 @@ mod tests {
100,
));
let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account);
vote_keyed_account.set_state(&VoteState::default()).unwrap();
vote_keyed_account
.set_state(&VoteStateVersions::Current(Box::new(VoteState::default())))
.unwrap();

stake_keyed_account
.delegate(
Expand Down Expand Up @@ -1748,7 +1756,9 @@ mod tests {
100,
));
let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account);
vote_keyed_account.set_state(&VoteState::default()).unwrap();
vote_keyed_account
.set_state(&VoteStateVersions::Current(Box::new(VoteState::default())))
.unwrap();
assert_eq!(
stake_keyed_account.delegate(
&vote_keyed_account,
Expand Down Expand Up @@ -1857,7 +1867,9 @@ mod tests {
100,
));
let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account);
vote_keyed_account.set_state(&VoteState::default()).unwrap();
vote_keyed_account
.set_state(&VoteStateVersions::Current(Box::new(VoteState::default())))
.unwrap();
let signers = vec![stake_pubkey].into_iter().collect();
assert_eq!(
stake_keyed_account.delegate(
Expand Down
3 changes: 2 additions & 1 deletion programs/vote/src/vote_instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use solana_sdk::{
system_instruction,
sysvar::{self, clock::Clock, slot_hashes::SlotHashes, Sysvar},
};
use std::collections::HashSet;
use thiserror::Error;

/// Reasons the stake might have had an error
Expand Down Expand Up @@ -187,7 +188,7 @@ pub fn process_instruction(
trace!("process_instruction: {:?}", data);
trace!("keyed_accounts: {:?}", keyed_accounts);

let signers = get_signers(keyed_accounts);
let signers: HashSet<Pubkey> = get_signers(keyed_accounts);

let keyed_accounts = &mut keyed_accounts.iter();
let me = &mut next_keyed_account(keyed_accounts)?;
Expand Down
Loading