diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index 012797ff77..22b13bac6f 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -16,7 +16,7 @@ use { hash::Hash, instruction::{AccountMeta, Instruction, InstructionError}, keyed_account::{from_keyed_account, get_signers, keyed_account_at_index, KeyedAccount}, - process_instruction::InvokeContext, + process_instruction::{get_sysvar, InvokeContext}, program_utils::limited_deserialize, pubkey::Pubkey, system_instruction, @@ -366,7 +366,14 @@ pub fn process_instruction( } VoteInstruction::Withdraw(lamports) => { let to = keyed_account_at_index(keyed_accounts, 1)?; - vote_state::withdraw(me, lamports, to, &signers) + let rent_sysvar = if invoke_context + .is_feature_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id()) + { + Some(get_sysvar(invoke_context, &sysvar::rent::id())?) + } else { + None + }; + vote_state::withdraw(me, lamports, to, &signers, rent_sysvar) } VoteInstruction::AuthorizeChecked(vote_authorize) => { if invoke_context.is_feature_active(&feature_set::vote_stake_checked_instructions::id()) @@ -395,7 +402,7 @@ mod tests { bincode::serialize, solana_sdk::{ account::{self, Account, AccountSharedData}, - process_instruction::MockInvokeContext, + process_instruction::{mock_set_sysvar, MockInvokeContext}, rent::Rent, }, std::{cell::RefCell, str::FromStr}, @@ -454,11 +461,14 @@ mod tests { .zip(accounts.iter()) .map(|(meta, account)| KeyedAccount::new(&meta.pubkey, meta.is_signer, account)) .collect(); - super::process_instruction( - &Pubkey::default(), - &instruction.data, - &mut MockInvokeContext::new(keyed_accounts), + let mut invoke_context = MockInvokeContext::new(keyed_accounts); + mock_set_sysvar( + &mut invoke_context, + sysvar::rent::id(), + sysvar::rent::Rent::default(), ) + .unwrap(); + super::process_instruction(&Pubkey::default(), &instruction.data, &mut invoke_context) } } diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index dd49f1a220..60fc8987a9 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -20,7 +20,6 @@ use { }, std::{ boxed::Box, - cmp::Ordering, collections::{HashSet, VecDeque}, }, }; @@ -682,20 +681,28 @@ pub fn withdraw( lamports: u64, to_account: &KeyedAccount, signers: &HashSet, + rent_sysvar: Option, ) -> Result<(), InstructionError> { let vote_state: VoteState = State::::state(vote_account)?.convert_to_current(); verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; - match vote_account.lamports()?.cmp(&lamports) { - Ordering::Less => return Err(InstructionError::InsufficientFunds), - Ordering::Equal => { - // Deinitialize upon zero-balance - vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; + let remaining_balance = vote_account + .lamports()? + .checked_sub(lamports) + .ok_or(InstructionError::InsufficientFunds)?; + + if remaining_balance == 0 { + // Deinitialize upon zero-balance + 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.data_len()?); + if remaining_balance < min_rent_exempt_balance { + return Err(InstructionError::InsufficientFunds); } - _ => (), } + vote_account .try_account_ref_mut()? .checked_sub_lamports(lamports)?; @@ -899,6 +906,8 @@ mod tests { } fn create_test_account() -> (Pubkey, RefCell) { + let rent = Rent::default(); + let balance = VoteState::get_rent_exempt_reserve(&rent); let vote_pubkey = solana_sdk::pubkey::new_rand(); ( vote_pubkey, @@ -906,7 +915,7 @@ mod tests { &vote_pubkey, &solana_sdk::pubkey::new_rand(), 0, - 100, + balance, )), ) } @@ -1667,46 +1676,129 @@ mod tests { &RefCell::new(AccountSharedData::default()), ), &signers, + None, ); assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); // insufficient funds let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let lamports = vote_account.borrow().lamports(); let signers: HashSet = get_signers(keyed_accounts); let res = withdraw( &keyed_accounts[0], - 101, + lamports + 1, &KeyedAccount::new( &solana_sdk::pubkey::new_rand(), false, &RefCell::new(AccountSharedData::default()), ), &signers, + None, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); - // all good - let to_account = RefCell::new(AccountSharedData::default()); - let lamports = vote_account.borrow().lamports(); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; - let signers: HashSet = get_signers(keyed_accounts); - let pre_state: VoteStateVersions = vote_account.borrow().state().unwrap(); - let res = withdraw( - &keyed_accounts[0], - lamports, - &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), - &signers, - ); - assert_eq!(res, Ok(())); - assert_eq!(vote_account.borrow().lamports(), 0); - assert_eq!(to_account.borrow().lamports(), lamports); - let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); - // State has been deinitialized since balance is zero - assert!(post_state.is_uninitialized()); + // non rent exempt withdraw, before feature activation + { + let (vote_pubkey, vote_account) = create_test_account(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let lamports = vote_account.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + None, + ); + assert_eq!(res, Ok(())); + } - // reset balance and restore state, verify that authorized_withdrawer works - vote_account.borrow_mut().set_lamports(lamports); - vote_account.borrow_mut().set_state(&pre_state).unwrap(); + // non rent exempt withdraw, after feature activation + { + let (vote_pubkey, vote_account) = create_test_account(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let lamports = vote_account.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + Some(rent_sysvar), + ); + assert_eq!(res, Err(InstructionError::InsufficientFunds)); + } + + // partial valid withdraw, after feature activation + { + let to_account = RefCell::new(AccountSharedData::default()); + let (vote_pubkey, vote_account) = create_test_account(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let lamports = vote_account.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let withdraw_lamports = lamports - minimum_balance; + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + withdraw_lamports, + &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), + &signers, + Some(rent_sysvar), + ); + assert_eq!(res, Ok(())); + assert_eq!( + vote_account.borrow().lamports(), + lamports - withdraw_lamports + ); + assert_eq!(to_account.borrow().lamports(), withdraw_lamports); + } + + // full withdraw, before/after activation + { + let rent_sysvar = Rent::default(); + for rent_sysvar in &[None, Some(rent_sysvar)] { + let to_account = RefCell::new(AccountSharedData::default()); + let (vote_pubkey, vote_account) = create_test_account(); + let lamports = vote_account.borrow().lamports(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports, + &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), + &signers, + *rent_sysvar, + ); + assert_eq!(res, Ok(())); + assert_eq!(vote_account.borrow().lamports(), 0); + assert_eq!(to_account.borrow().lamports(), lamports); + let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); + // State has been deinitialized since balance is zero + assert!(post_state.is_uninitialized()); + } + } // authorize authorized_withdrawer let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand(); @@ -1735,6 +1827,7 @@ mod tests { lamports, withdrawer_keyed_account, &signers, + None, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index edde2db669..e070b02a34 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -273,6 +273,10 @@ pub mod reject_section_virtual_address_file_offset_mismatch { solana_sdk::declare_id!("5N4NikcJLEiZNqwndhNyvZw15LvFXp1oF7AJQTNTZY5k"); } +pub mod reject_non_rent_exempt_vote_withdraws { + solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -341,6 +345,7 @@ lazy_static! { (spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"), (reject_deployment_of_unresolved_syscalls::id(), "Reject deployment of programs with unresolved syscall symbols"), (reject_section_virtual_address_file_offset_mismatch::id(), "enforce section virtual addresses and file offsets in ELF to be equal"), + (reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()