From d7b65e031c94c34f0cf1fb453549c2f1065d9556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 14 Mar 2022 17:10:10 +0100 Subject: [PATCH] Revert "Replaces `KeyedAccount` by `BorrowedAccount` in vote processor (#23348)" e2fa6a0f7a906a6f58925ba282115f08b9dfd487 --- programs/vote/src/vote_processor.rs | 242 +++++++++++----------------- programs/vote/src/vote_state/mod.rs | 164 +++++++------------ 2 files changed, 152 insertions(+), 254 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index cb40f229e0f030..24cc8f054203ac 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -5,157 +5,87 @@ use { log::*, solana_metrics::inc_new_counter_info, solana_program_runtime::{ - invoke_context::InvokeContext, sysvar_cache::get_sysvar_with_account_check2, + invoke_context::InvokeContext, sysvar_cache::get_sysvar_with_account_check, }, - solana_sdk::{feature_set, instruction::InstructionError, program_utils::limited_deserialize}, + solana_sdk::{ + feature_set, + instruction::InstructionError, + keyed_account::{get_signers, keyed_account_at_index, KeyedAccount}, + program_utils::limited_deserialize, + pubkey::Pubkey, + sysvar::rent::Rent, + }, + std::collections::HashSet, }; -pub mod instruction_account_indices { - pub enum InitializeAccount { - VoteAccount = 0, - Rent = 1, - Clock = 2, - } - - pub enum Authorize { - VoteAccount = 0, - Clock = 1, - } - - pub enum UpdateValidatorIdentity { - VoteAccount = 0, - Node = 1, - } - - pub enum UpdateCommission { - VoteAccount = 0, - } - - pub enum Vote { - VoteAccount = 0, - SlotHashes = 1, - Clock = 2, - } - - pub enum UpdateVoteState { - VoteAccount = 0, - } - - pub enum Withdraw { - VoteAccount = 0, - Recipient = 1, - } - - pub enum AuthorizeChecked { - VoteAccount = 0, - Clock = 1, - // Ignores = 2, - Voter = 3, - } -} - pub fn process_instruction( - _first_instruction_account: usize, + first_instruction_account: usize, data: &[u8], invoke_context: &mut InvokeContext, ) -> Result<(), InstructionError> { - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; + let keyed_accounts = invoke_context.get_keyed_accounts()?; trace!("process_instruction: {:?}", data); + trace!("keyed_accounts: {:?}", keyed_accounts); - { - let vote_account = - instruction_context.try_borrow_instruction_account(transaction_context, 0)?; - if vote_account.get_owner() != &id() { - return Err(InstructionError::InvalidAccountOwner); - } + let me = &mut keyed_account_at_index(keyed_accounts, first_instruction_account)?; + if me.owner()? != id() { + return Err(InstructionError::InvalidAccountOwner); } - let signers = instruction_context.get_signers(transaction_context); + let signers: HashSet = get_signers(&keyed_accounts[first_instruction_account..]); match limited_deserialize(data)? { VoteInstruction::InitializeAccount(vote_init) => { - let rent = get_sysvar_with_account_check2::rent( + let rent = get_sysvar_with_account_check::rent( + keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?, invoke_context, - instruction_context, - instruction_account_indices::InitializeAccount::Rent as usize, )?; - { - // Verify rent exemption - let vote_account = instruction_context.try_borrow_instruction_account( - transaction_context, - instruction_account_indices::InitializeAccount::VoteAccount as usize, - )?; - if !rent.is_exempt(vote_account.get_lamports(), vote_account.get_data().len()) { - return Err(InstructionError::InsufficientFunds); - } - } - let _clock = get_sysvar_with_account_check2::clock( + verify_rent_exemption(me, &rent)?; + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?, invoke_context, - instruction_context, - instruction_account_indices::InitializeAccount::Clock as usize, )?; - vote_state::initialize_account( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::InitializeAccount::VoteAccount as usize, - &vote_init, - ) + vote_state::initialize_account(me, &vote_init, &signers, &clock) } VoteInstruction::Authorize(voter_pubkey, vote_authorize) => { - let _clock = get_sysvar_with_account_check2::clock( + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?, invoke_context, - instruction_context, - instruction_account_indices::Authorize::Clock as usize, )?; vote_state::authorize( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::Authorize::VoteAccount as usize, + me, &voter_pubkey, vote_authorize, - ) - } - VoteInstruction::UpdateValidatorIdentity => { - instruction_context.check_number_of_instruction_accounts(2)?; - vote_state::update_validator_identity( - invoke_context, - instruction_context, &signers, - instruction_account_indices::UpdateValidatorIdentity::VoteAccount as usize, - instruction_context.get_instruction_account_key( - transaction_context, - instruction_account_indices::UpdateValidatorIdentity::Node as usize, - )?, + &clock, + &invoke_context.feature_set, ) } - VoteInstruction::UpdateCommission(commission) => vote_state::update_commission( - invoke_context, - instruction_context, + VoteInstruction::UpdateValidatorIdentity => vote_state::update_validator_identity( + me, + keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?.unsigned_key(), &signers, - instruction_account_indices::UpdateCommission::VoteAccount as usize, - commission, ), + VoteInstruction::UpdateCommission(commission) => { + vote_state::update_commission(me, commission, &signers) + } VoteInstruction::Vote(vote) | VoteInstruction::VoteSwitch(vote, _) => { inc_new_counter_info!("vote-native", 1); - let _slot_hashes = get_sysvar_with_account_check2::slot_hashes( + let slot_hashes = get_sysvar_with_account_check::slot_hashes( + keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?, invoke_context, - instruction_context, - instruction_account_indices::Vote::SlotHashes as usize, )?; - let _clock = get_sysvar_with_account_check2::clock( + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?, invoke_context, - instruction_context, - instruction_account_indices::Vote::Clock as usize, )?; vote_state::process_vote( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::Vote::VoteAccount as usize, + me, + &slot_hashes, + &clock, &vote, + &signers, + &invoke_context.feature_set, ) } VoteInstruction::UpdateVoteState(vote_state_update) @@ -165,26 +95,47 @@ pub fn process_instruction( .is_active(&feature_set::allow_votes_to_directly_update_vote_state::id()) { inc_new_counter_info!("vote-state-native", 1); + let sysvar_cache = invoke_context.get_sysvar_cache(); + let slot_hashes = sysvar_cache.get_slot_hashes()?; + let clock = sysvar_cache.get_clock()?; vote_state::process_vote_state_update( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::UpdateVoteState::VoteAccount as usize, + me, + slot_hashes.slot_hashes(), + &clock, vote_state_update, + &signers, ) } else { Err(InstructionError::InvalidInstructionData) } } VoteInstruction::Withdraw(lamports) => { - instruction_context.check_number_of_instruction_accounts(2)?; + let to = keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?; + let rent_sysvar = if invoke_context + .feature_set + .is_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id()) + { + Some(invoke_context.get_sysvar_cache().get_rent()?) + } else { + None + }; + + let clock_if_feature_active = if invoke_context + .feature_set + .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()) + { + Some(invoke_context.get_sysvar_cache().get_clock()?) + } else { + None + }; + vote_state::withdraw( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::Withdraw::VoteAccount as usize, - instruction_account_indices::Withdraw::Recipient as usize, + me, lamports, + to, + &signers, + rent_sysvar.as_deref(), + clock_if_feature_active.as_deref(), ) } VoteInstruction::AuthorizeChecked(vote_authorize) => { @@ -192,28 +143,21 @@ pub fn process_instruction( .feature_set .is_active(&feature_set::vote_stake_checked_instructions::id()) { - instruction_context.check_number_of_instruction_accounts(4)?; - if !instruction_context.is_signer( - instruction_context.get_number_of_program_accounts() - + instruction_account_indices::AuthorizeChecked::Voter as usize, - )? { - return Err(InstructionError::MissingRequiredSignature); - } - let _clock = get_sysvar_with_account_check2::clock( + let voter_pubkey = + &keyed_account_at_index(keyed_accounts, first_instruction_account + 3)? + .signer_key() + .ok_or(InstructionError::MissingRequiredSignature)?; + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?, invoke_context, - instruction_context, - instruction_account_indices::AuthorizeChecked::Clock as usize, )?; vote_state::authorize( - invoke_context, - instruction_context, - &signers, - instruction_account_indices::AuthorizeChecked::VoteAccount as usize, - instruction_context.get_instruction_account_key( - transaction_context, - instruction_account_indices::AuthorizeChecked::Voter as usize, - )?, + me, + voter_pubkey, vote_authorize, + &signers, + &clock, + &invoke_context.feature_set, ) } else { Err(InstructionError::InvalidInstructionData) @@ -222,6 +166,17 @@ pub fn process_instruction( } } +fn verify_rent_exemption( + keyed_account: &KeyedAccount, + rent: &Rent, +) -> Result<(), InstructionError> { + if !rent.is_exempt(keyed_account.lamports()?, keyed_account.data_len()?) { + Err(InstructionError::InsufficientFunds) + } else { + Ok(()) + } +} + #[cfg(test)] mod tests { use { @@ -246,10 +201,9 @@ mod tests { feature_set::FeatureSet, hash::Hash, instruction::{AccountMeta, Instruction}, - pubkey::Pubkey, - sysvar::{self, clock::Clock, rent::Rent, slot_hashes::SlotHashes}, + sysvar::{self, clock::Clock, slot_hashes::SlotHashes}, }, - std::{collections::HashSet, str::FromStr}, + std::str::FromStr, }; fn create_default_account() -> AccountSharedData { diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 1b61ecbe0c56d6..187e11a403507a 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -5,19 +5,19 @@ use { bincode::{deserialize, serialize_into, serialized_size, ErrorKind}, log::*, serde_derive::{Deserialize, Serialize}, - solana_program_runtime::invoke_context::InvokeContext, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, + account_utils::State, clock::{Epoch, Slot, UnixTimestamp}, epoch_schedule::MAX_LEADER_SCHEDULE_EPOCH_OFFSET, feature_set::{self, filter_votes_outside_slot_hashes, FeatureSet}, hash::Hash, instruction::InstructionError, + keyed_account::KeyedAccount, pubkey::Pubkey, rent::Rent, slot_hashes::SlotHash, sysvar::clock::Clock, - transaction_context::{BorrowedAccount, InstructionContext}, }, std::{ cmp::Ordering, @@ -1165,24 +1165,19 @@ impl VoteState { /// but will implicitly withdraw authorization from the previously authorized /// key pub fn authorize( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, authorized: &Pubkey, vote_authorize: VoteAuthorize, + signers: &HashSet, + clock: &Clock, + feature_set: &FeatureSet, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let clock = invoke_context.get_sysvar_cache().get_clock()?; - let mut vote_state: VoteState = vote_account - .get_state::()? - .convert_to_current(); + let mut vote_state: VoteState = + State::::state(vote_account)?.convert_to_current(); match vote_authorize { VoteAuthorize::Voter => { - let authorized_withdrawer_signer = if invoke_context - .feature_set + let authorized_withdrawer_signer = if feature_set .is_active(&feature_set::vote_withdraw_authority_may_change_authorized_voter::id()) { verify_authorized_signer(&vote_state.authorized_withdrawer, signers).is_ok() @@ -1216,17 +1211,12 @@ pub fn authorize( /// Update the node_pubkey, requires signature of the authorized voter pub fn update_validator_identity( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, node_pubkey: &Pubkey, + signers: &HashSet, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let mut vote_state: VoteState = vote_account - .get_state::()? - .convert_to_current(); + let mut vote_state: VoteState = + State::::state(vote_account)?.convert_to_current(); // current authorized withdrawer must say "yay" verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; @@ -1241,17 +1231,12 @@ pub fn update_validator_identity( /// Update the vote account's commission pub fn update_commission( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, commission: u8, + signers: &HashSet, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let mut vote_state: VoteState = vote_account - .get_state::()? - .convert_to_current(); + let mut vote_state: VoteState = + State::::state(vote_account)?.convert_to_current(); // current authorized withdrawer must say "yay" verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; @@ -1274,42 +1259,20 @@ fn verify_authorized_signer( /// Withdraw funds from the vote account pub fn withdraw( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, - recipient_account_index: usize, + vote_account: &KeyedAccount, lamports: u64, + to_account: &KeyedAccount, + signers: &HashSet, + rent_sysvar: Option<&Rent>, + clock: Option<&Clock>, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - - let rent_sysvar = if invoke_context - .feature_set - .is_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id()) - { - Some(invoke_context.get_sysvar_cache().get_rent()?) - } else { - None - }; - - let clock = if invoke_context - .feature_set - .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()) - { - Some(invoke_context.get_sysvar_cache().get_clock()?) - } else { - None - }; - - let vote_state: VoteState = vote_account - .get_state::()? - .convert_to_current(); + let vote_state: VoteState = + State::::state(vote_account)?.convert_to_current(); verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; let remaining_balance = vote_account - .get_lamports() + .lamports()? .checked_sub(lamports) .ok_or(InstructionError::InsufficientFunds)?; @@ -1332,19 +1295,18 @@ pub fn withdraw( vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; } } else if let Some(rent_sysvar) = rent_sysvar { - let min_rent_exempt_balance = rent_sysvar.minimum_balance(vote_account.get_data().len()); + let min_rent_exempt_balance = rent_sysvar.minimum_balance(vote_account.data_len()?); if remaining_balance < min_rent_exempt_balance { return Err(InstructionError::InsufficientFunds); } } - vote_account.checked_sub_lamports(lamports)?; - drop(vote_account); - let mut recipient_account = instruction_context.try_borrow_instruction_account( - invoke_context.transaction_context, - recipient_account_index, - )?; - recipient_account.checked_add_lamports(lamports)?; + vote_account + .try_account_ref_mut()? + .checked_sub_lamports(lamports)?; + to_account + .try_account_ref_mut()? + .checked_add_lamports(lamports)?; Ok(()) } @@ -1352,19 +1314,15 @@ pub fn withdraw( /// Assumes that the account is being init as part of a account creation or balance transfer and /// that the transaction must be signed by the staker's keys pub fn initialize_account( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, vote_init: &VoteInit, + signers: &HashSet, + clock: &Clock, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let clock = invoke_context.get_sysvar_cache().get_clock()?; - if vote_account.get_data().len() != VoteState::size_of() { + if vote_account.data_len()? != VoteState::size_of() { return Err(InstructionError::InvalidAccountData); } - let versioned = vote_account.get_state::()?; + let versioned = State::::state(vote_account)?; if !versioned.is_uninitialized() { return Err(InstructionError::AccountAlreadyInitialized); @@ -1374,16 +1332,16 @@ pub fn initialize_account( verify_authorized_signer(&vote_init.node_pubkey, signers)?; vote_account.set_state(&VoteStateVersions::new_current(VoteState::new( - vote_init, &clock, + vote_init, clock, ))) } fn verify_and_get_vote_state( - vote_account: &BorrowedAccount, + vote_account: &KeyedAccount, clock: &Clock, signers: &HashSet, ) -> Result { - let versioned = vote_account.get_state::()?; + let versioned = State::::state(vote_account)?; if versioned.is_uninitialized() { return Err(InstructionError::UninitializedAccount); @@ -1397,25 +1355,16 @@ fn verify_and_get_vote_state( } pub fn process_vote( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, + slot_hashes: &[SlotHash], + clock: &Clock, vote: &Vote, + signers: &HashSet, + feature_set: &FeatureSet, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let sysvar_cache = invoke_context.get_sysvar_cache(); - let slot_hashes = sysvar_cache.get_slot_hashes()?; - let clock = sysvar_cache.get_clock()?; - let mut vote_state = verify_and_get_vote_state(&vote_account, &clock, signers)?; - - vote_state.process_vote( - vote, - slot_hashes.slot_hashes(), - clock.epoch, - Some(&invoke_context.feature_set), - )?; + let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; + + vote_state.process_vote(vote, slot_hashes, clock.epoch, Some(feature_set))?; if let Some(timestamp) = vote.timestamp { vote.slots .iter() @@ -1427,19 +1376,14 @@ pub fn process_vote( } pub fn process_vote_state_update( - invoke_context: &InvokeContext, - instruction_context: &InstructionContext, - signers: &HashSet, - vote_account_index: usize, + vote_account: &KeyedAccount, + slot_hashes: &[SlotHash], + clock: &Clock, mut vote_state_update: VoteStateUpdate, + signers: &HashSet, ) -> Result<(), InstructionError> { - let mut vote_account = instruction_context - .try_borrow_instruction_account(invoke_context.transaction_context, vote_account_index)?; - let sysvar_cache = invoke_context.get_sysvar_cache(); - let slot_hashes = sysvar_cache.get_slot_hashes()?; - let clock = sysvar_cache.get_clock()?; - let mut vote_state = verify_and_get_vote_state(&vote_account, &clock, signers)?; - vote_state.check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes)?; + let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; + vote_state.check_update_vote_state_slots_are_valid(&mut vote_state_update, slot_hashes)?; vote_state.process_new_vote_state( vote_state_update.lockouts, vote_state_update.root,