diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index 5cbff6bdf5..e505018daf 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -11,6 +11,7 @@ use serde_derive::{Deserialize, Serialize}; use solana_metrics::inc_new_counter_info; use solana_sdk::{ decode_error::DecodeError, + feature_set, hash::Hash, instruction::{AccountMeta, Instruction, InstructionError}, keyed_account::{from_keyed_account, get_signers, next_keyed_account, KeyedAccount}, @@ -278,7 +279,7 @@ pub fn process_instruction( _program_id: &Pubkey, keyed_accounts: &[KeyedAccount], data: &[u8], - _invoke_context: &mut dyn InvokeContext, + invoke_context: &mut dyn InvokeContext, ) -> Result<(), InstructionError> { trace!("process_instruction: {:?}", data); trace!("keyed_accounts: {:?}", keyed_accounts); @@ -296,6 +297,7 @@ pub fn process_instruction( &vote_init, &signers, &from_keyed_account::(next_keyed_account(keyed_accounts)?)?, + invoke_context.is_feature_active(&feature_set::check_init_vote_data::id()), ) } VoteInstruction::Authorize(voter_pubkey, vote_authorize) => vote_state::authorize( diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index f967d58b4f..223f9f92ec 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -33,6 +33,9 @@ pub const INITIAL_LOCKOUT: usize = 2; // Maximum number of credits history to keep around pub const MAX_EPOCH_CREDITS_HISTORY: usize = 64; +// Offset of VoteState::prior_voters, for determining initialization status without deserialization +const DEFAULT_PRIOR_VOTERS_OFFSET: usize = 82; + #[frozen_abi(digest = "Ch2vVEwos2EjAVqSHCyJjnN2MNX1yrpapZTGhMSCjWUH")] #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone, AbiExample)] pub struct Vote { @@ -569,6 +572,13 @@ impl VoteState { self.last_timestamp = BlockTimestamp { slot, timestamp }; Ok(()) } + + pub fn is_uninitialized_no_deser(data: &[u8]) -> bool { + const VERSION_OFFSET: usize = 4; + data.len() != VoteState::size_of() + || data[VERSION_OFFSET..VERSION_OFFSET + DEFAULT_PRIOR_VOTERS_OFFSET] + == [0; DEFAULT_PRIOR_VOTERS_OFFSET] + } } /// Authorize the given pubkey to withdraw or sign votes. This may be called multiple times, @@ -684,7 +694,11 @@ pub fn initialize_account( vote_init: &VoteInit, signers: &HashSet, clock: &Clock, + check_data_size: bool, ) -> Result<(), InstructionError> { + if check_data_size && vote_account.data_len()? != VoteState::size_of() { + return Err(InstructionError::InvalidAccountData); + } let versioned = State::::state(vote_account)?; if !versioned.is_uninitialized() { @@ -812,6 +826,7 @@ mod tests { }, &signers, &Clock::default(), + true, ); assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); @@ -829,6 +844,7 @@ mod tests { }, &signers, &Clock::default(), + true, ); assert_eq!(res, Ok(())); @@ -843,8 +859,27 @@ mod tests { }, &signers, &Clock::default(), + true, ); assert_eq!(res, Err(InstructionError::AccountAlreadyInitialized)); + + //init should fail, account is too big + let large_vote_account = Account::new_ref(100, 2 * VoteState::size_of(), &id()); + let large_vote_account = + KeyedAccount::new(&vote_account_pubkey, false, &large_vote_account); + let res = initialize_account( + &large_vote_account, + &VoteInit { + node_pubkey, + authorized_voter: vote_account_pubkey, + authorized_withdrawer: vote_account_pubkey, + commission: 0, + }, + &signers, + &Clock::default(), + true, + ); + assert_eq!(res, Err(InstructionError::InvalidAccountData)); } fn create_test_account() -> (Pubkey, RefCell) { @@ -2028,4 +2063,39 @@ mod tests { // when called on a `VoteStateVersions` that stores it assert!(VoteStateVersions::new_current(VoteState::default()).is_uninitialized()); } + + #[test] + fn test_is_uninitialized_no_deser() { + // Check all zeroes + let mut vote_account_data = vec![0; VoteState::size_of()]; + assert!(VoteState::is_uninitialized_no_deser(&vote_account_data)); + + // Check default VoteState + let default_account_state = VoteStateVersions::new_current(VoteState::default()); + VoteState::serialize(&default_account_state, &mut vote_account_data).unwrap(); + assert!(VoteState::is_uninitialized_no_deser(&vote_account_data)); + + // Check non-zero data shorter than offset index used + let short_data = vec![1; DEFAULT_PRIOR_VOTERS_OFFSET]; + assert!(VoteState::is_uninitialized_no_deser(&short_data)); + + // Check non-zero large account + let mut large_vote_data = vec![1; 2 * VoteState::size_of()]; + let default_account_state = VoteStateVersions::new_current(VoteState::default()); + VoteState::serialize(&default_account_state, &mut large_vote_data).unwrap(); + assert!(VoteState::is_uninitialized_no_deser(&vote_account_data)); + + // Check populated VoteState + let account_state = VoteStateVersions::new_current(VoteState::new( + &VoteInit { + node_pubkey: Pubkey::new_unique(), + authorized_voter: Pubkey::new_unique(), + authorized_withdrawer: Pubkey::new_unique(), + commission: 0, + }, + &Clock::default(), + )); + VoteState::serialize(&account_state, &mut vote_account_data).unwrap(); + assert!(!VoteState::is_uninitialized_no_deser(&vote_account_data)); + } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5dcd58e17a..e1e29bfb31 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3865,10 +3865,12 @@ impl Bank { .store_slow_cached(self.slot(), pubkey, account); if Stakes::is_stake(account) { - self.stakes - .write() - .unwrap() - .store(pubkey, account, self.stake_program_v2_enabled()); + self.stakes.write().unwrap().store( + pubkey, + account, + self.stake_program_v2_enabled(), + self.check_init_vote_data_enabled(), + ); } } @@ -4398,6 +4400,7 @@ impl Bank { pubkey, account, self.stake_program_v2_enabled(), + self.check_init_vote_data_enabled(), ) { overwritten_vote_accounts.push(OverwrittenVoteAccount { account: old_vote_account, @@ -4627,6 +4630,11 @@ impl Bank { .is_active(&feature_set::stake_program_v2::id()) } + pub fn check_init_vote_data_enabled(&self) -> bool { + self.feature_set + .is_active(&feature_set::check_init_vote_data::id()) + } + pub fn simple_capitalization_enabled(&self) -> bool { self.simple_capitalization_enabled_at_genesis() || self diff --git a/runtime/src/stakes.rs b/runtime/src/stakes.rs index daf6e9e6c5..675a0b3189 100644 --- a/runtime/src/stakes.rs +++ b/runtime/src/stakes.rs @@ -5,6 +5,7 @@ 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::{borrow::Borrow, collections::HashMap}; #[derive(Default, Clone, PartialEq, Debug, Deserialize, Serialize, AbiExample)] @@ -116,16 +117,18 @@ impl Stakes { pubkey: &Pubkey, account: &Account, fix_stake_deactivate: bool, + check_vote_init: bool, ) -> Option { if solana_vote_program::check_id(&account.owner) { // unconditionally remove existing at first; there is no dependent calculated state for // votes, not like stakes (stake codepath maintains calculated stake value grouped by // delegated vote pubkey) let old = self.vote_accounts.remove(pubkey); - // when account is removed (lamports == 0), don't readd so that given - // `pubkey` can be used for any owner in the future, while not - // affecting Stakes. - if account.lamports != 0 { + // when account is removed (lamports == 0 or data uninitialized), don't read so that + // given `pubkey` can be used for any owner in the future, while not affecting Stakes. + if account.lamports != 0 + && !(check_vote_init && VoteState::is_uninitialized_no_deser(&account.data)) + { let stake = old.as_ref().map_or_else( || { self.calculate_stake( @@ -226,7 +229,7 @@ pub mod tests { use super::*; use solana_sdk::{pubkey::Pubkey, rent::Rent}; use solana_stake_program::stake_state; - use solana_vote_program::vote_state::{self, VoteState}; + use solana_vote_program::vote_state::{self, VoteState, VoteStateVersions}; // set up some dummies for a staked node (( vote ) ( stake )) pub fn create_staked_node_accounts(stake: u64) -> ((Pubkey, Account), (Pubkey, Account)) { @@ -298,8 +301,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, mut stake_account)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); let stake = StakeState::stake_from(&stake_account).unwrap(); { let vote_accounts = stakes.vote_accounts(); @@ -311,7 +314,7 @@ pub mod tests { } stake_account.lamports = 42; - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&stake_pubkey, &stake_account, true, true); { let vote_accounts = stakes.vote_accounts(); assert!(vote_accounts.get(&vote_pubkey).is_some()); @@ -323,7 +326,7 @@ pub mod tests { // activate more let (_stake_pubkey, mut stake_account) = create_stake_account(42, &vote_pubkey); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&stake_pubkey, &stake_account, true, true); let stake = StakeState::stake_from(&stake_account).unwrap(); { let vote_accounts = stakes.vote_accounts(); @@ -335,7 +338,7 @@ pub mod tests { } stake_account.lamports = 0; - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&stake_pubkey, &stake_account, true, true); { let vote_accounts = stakes.vote_accounts(); assert!(vote_accounts.get(&vote_pubkey).is_some()); @@ -353,14 +356,14 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); let ((vote11_pubkey, vote11_account), (stake11_pubkey, stake11_account)) = create_staked_node_accounts(20); - stakes.store(&vote11_pubkey, &vote11_account, true); - stakes.store(&stake11_pubkey, &stake11_account, true); + stakes.store(&vote11_pubkey, &vote11_account, true, true); + stakes.store(&stake11_pubkey, &stake11_account, true, true); let vote11_node_pubkey = VoteState::from(&vote11_account).unwrap().node_pubkey; @@ -377,8 +380,8 @@ pub mod tests { let ((vote_pubkey, mut vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); { let vote_accounts = stakes.vote_accounts(); @@ -387,14 +390,45 @@ pub mod tests { } vote_account.lamports = 0; - stakes.store(&vote_pubkey, &vote_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); { let vote_accounts = stakes.vote_accounts(); assert!(vote_accounts.get(&vote_pubkey).is_none()); } + vote_account.lamports = 1; - stakes.store(&vote_pubkey, &vote_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + + { + let vote_accounts = stakes.vote_accounts(); + assert!(vote_accounts.get(&vote_pubkey).is_some()); + assert_eq!(vote_accounts.get(&vote_pubkey).unwrap().0, 10); + } + + // Vote account too big + let cache_data = vote_account.data.clone(); + vote_account.data.push(0); + stakes.store(&vote_pubkey, &vote_account, true, true); + + { + let vote_accounts = stakes.vote_accounts(); + assert!(vote_accounts.get(&vote_pubkey).is_none()); + } + + // Vote account uninitialized + let default_vote_state = VoteState::default(); + let versioned = VoteStateVersions::new_current(default_vote_state); + VoteState::to(&versioned, &mut vote_account).unwrap(); + stakes.store(&vote_pubkey, &vote_account, true, true); + + { + let vote_accounts = stakes.vote_accounts(); + assert!(vote_accounts.get(&vote_pubkey).is_none()); + } + + vote_account.data = cache_data; + stakes.store(&vote_pubkey, &vote_account, true, true); { let vote_accounts = stakes.vote_accounts(); @@ -416,11 +450,11 @@ pub mod tests { let ((vote_pubkey2, vote_account2), (_stake_pubkey2, stake_account2)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&vote_pubkey2, &vote_account2, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&vote_pubkey2, &vote_account2, true, true); // delegates to vote_pubkey - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&stake_pubkey, &stake_account, true, true); let stake = StakeState::stake_from(&stake_account).unwrap(); @@ -436,7 +470,7 @@ pub mod tests { } // delegates to vote_pubkey2 - stakes.store(&stake_pubkey, &stake_account2, true); + stakes.store(&stake_pubkey, &stake_account2, true, true); { let vote_accounts = stakes.vote_accounts(); @@ -461,11 +495,11 @@ pub mod tests { let (stake_pubkey2, stake_account2) = create_stake_account(10, &vote_pubkey); - stakes.store(&vote_pubkey, &vote_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); // delegates to vote_pubkey - stakes.store(&stake_pubkey, &stake_account, true); - stakes.store(&stake_pubkey2, &stake_account2, true); + stakes.store(&stake_pubkey, &stake_account, true, true); + stakes.store(&stake_pubkey2, &stake_account2, true, true); { let vote_accounts = stakes.vote_accounts(); @@ -480,8 +514,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); let stake = StakeState::stake_from(&stake_account).unwrap(); { @@ -511,8 +545,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); { let vote_accounts = stakes.vote_accounts(); @@ -525,6 +559,7 @@ pub mod tests { &stake_pubkey, &Account::new(1, 0, &solana_stake_program::id()), true, + true, ); { let vote_accounts = stakes.vote_accounts(); @@ -554,8 +589,8 @@ pub mod tests { let genesis_epoch = 0; let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_warming_staked_node_accounts(10, genesis_epoch); - stakes.store(&vote_pubkey, &vote_account, true); - stakes.store(&stake_pubkey, &stake_account, true); + stakes.store(&vote_pubkey, &vote_account, true, true); + stakes.store(&stake_pubkey, &stake_account, true, true); assert_eq!(stakes.vote_balance_and_staked(), 11); assert_eq!(stakes.vote_balance_and_warmed_staked(), 1); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 1a18e6bb16..c66ccbbd94 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -119,6 +119,10 @@ pub mod per_byte_logging_cost { solana_sdk::declare_id!("59dM4SV6dPEKXPfkrkhFkRdn4K6xwKxdNAPMyXG7J1wT"); } +pub mod check_init_vote_data { + solana_sdk::declare_id!("3ccR6QpxGYsAbWyfevEtBNGfWV4xBffxRj2tD6A9i39F"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -148,7 +152,8 @@ lazy_static! { (full_inflation::mainnet::certusone::enable::id(), "Full inflation enabled by Certus One"), (full_inflation::mainnet::certusone::vote::id(), "Community vote allowing Certus One to enable full inflation"), (warp_timestamp_again::id(), "warp timestamp again, adjust bounding to 25% fast 80% slow #15204"), - (per_byte_logging_cost::id(), "charge the compute budget per byte for logging") + (per_byte_logging_cost::id(), "charge the compute budget per byte for logging"), + (check_init_vote_data::id(), "check initialized Vote data") /*************** ADD NEW FEATURES HERE ***************/ ] .iter()