From ae11cc329702621d97b3e58dd972d238b33b6ab6 Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Mon, 13 Dec 2021 08:48:38 -0700 Subject: [PATCH] Revert "Reject vote withdraws that create non-rent-exempt accounts (backport #21639) (#21644)" This reverts commit 83e01442a7940eb600a6b4b19d37f2d03e6ca3cd. --- programs/vote/src/vote_instruction.rs | 24 ++-- programs/vote/src/vote_state/mod.rs | 153 +++++--------------------- sdk/src/feature_set.rs | 5 - 3 files changed, 37 insertions(+), 145 deletions(-) diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index 22b13bac6f1ea7..012797ff77d457 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::{get_sysvar, InvokeContext}, + process_instruction::InvokeContext, program_utils::limited_deserialize, pubkey::Pubkey, system_instruction, @@ -366,14 +366,7 @@ pub fn process_instruction( } VoteInstruction::Withdraw(lamports) => { let to = keyed_account_at_index(keyed_accounts, 1)?; - 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) + vote_state::withdraw(me, lamports, to, &signers) } VoteInstruction::AuthorizeChecked(vote_authorize) => { if invoke_context.is_feature_active(&feature_set::vote_stake_checked_instructions::id()) @@ -402,7 +395,7 @@ mod tests { bincode::serialize, solana_sdk::{ account::{self, Account, AccountSharedData}, - process_instruction::{mock_set_sysvar, MockInvokeContext}, + process_instruction::MockInvokeContext, rent::Rent, }, std::{cell::RefCell, str::FromStr}, @@ -461,14 +454,11 @@ mod tests { .zip(accounts.iter()) .map(|(meta, account)| KeyedAccount::new(&meta.pubkey, meta.is_signer, account)) .collect(); - let mut invoke_context = MockInvokeContext::new(keyed_accounts); - mock_set_sysvar( - &mut invoke_context, - sysvar::rent::id(), - sysvar::rent::Rent::default(), + super::process_instruction( + &Pubkey::default(), + &instruction.data, + &mut MockInvokeContext::new(keyed_accounts), ) - .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 60fc8987a94ac9..dd49f1a22042da 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -20,6 +20,7 @@ use { }, std::{ boxed::Box, + cmp::Ordering, collections::{HashSet, VecDeque}, }, }; @@ -681,28 +682,20 @@ 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)?; - 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); + 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()))?; } + _ => (), } - vote_account .try_account_ref_mut()? .checked_sub_lamports(lamports)?; @@ -906,8 +899,6 @@ 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, @@ -915,7 +906,7 @@ mod tests { &vote_pubkey, &solana_sdk::pubkey::new_rand(), 0, - balance, + 100, )), ) } @@ -1676,129 +1667,46 @@ 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], - lamports + 1, + 101, &KeyedAccount::new( &solana_sdk::pubkey::new_rand(), false, &RefCell::new(AccountSharedData::default()), ), &signers, - None, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); - // 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(())); - } - - // 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); - } + // 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()); - // 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()); - } - } + // 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(); // authorize authorized_withdrawer let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand(); @@ -1827,7 +1735,6 @@ 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 e070b02a3445b0..edde2db6695d84 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -273,10 +273,6 @@ 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 = [ @@ -345,7 +341,6 @@ 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()