Revert "Reject vote withdraws that create non-rent-exempt accounts (backport #21639) (#21644)"

This reverts commit 83e01442a7.
This commit is contained in:
Trent Nelson
2021-12-13 08:48:38 -07:00
committed by Tao Zhu
parent a0b73d5658
commit ae11cc3297
3 changed files with 37 additions and 145 deletions

View File

@ -16,7 +16,7 @@ use {
hash::Hash, hash::Hash,
instruction::{AccountMeta, Instruction, InstructionError}, instruction::{AccountMeta, Instruction, InstructionError},
keyed_account::{from_keyed_account, get_signers, keyed_account_at_index, KeyedAccount}, keyed_account::{from_keyed_account, get_signers, keyed_account_at_index, KeyedAccount},
process_instruction::{get_sysvar, InvokeContext}, process_instruction::InvokeContext,
program_utils::limited_deserialize, program_utils::limited_deserialize,
pubkey::Pubkey, pubkey::Pubkey,
system_instruction, system_instruction,
@ -366,14 +366,7 @@ pub fn process_instruction(
} }
VoteInstruction::Withdraw(lamports) => { VoteInstruction::Withdraw(lamports) => {
let to = keyed_account_at_index(keyed_accounts, 1)?; let to = keyed_account_at_index(keyed_accounts, 1)?;
let rent_sysvar = if invoke_context vote_state::withdraw(me, lamports, to, &signers)
.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) => { VoteInstruction::AuthorizeChecked(vote_authorize) => {
if invoke_context.is_feature_active(&feature_set::vote_stake_checked_instructions::id()) if invoke_context.is_feature_active(&feature_set::vote_stake_checked_instructions::id())
@ -402,7 +395,7 @@ mod tests {
bincode::serialize, bincode::serialize,
solana_sdk::{ solana_sdk::{
account::{self, Account, AccountSharedData}, account::{self, Account, AccountSharedData},
process_instruction::{mock_set_sysvar, MockInvokeContext}, process_instruction::MockInvokeContext,
rent::Rent, rent::Rent,
}, },
std::{cell::RefCell, str::FromStr}, std::{cell::RefCell, str::FromStr},
@ -461,14 +454,11 @@ mod tests {
.zip(accounts.iter()) .zip(accounts.iter())
.map(|(meta, account)| KeyedAccount::new(&meta.pubkey, meta.is_signer, account)) .map(|(meta, account)| KeyedAccount::new(&meta.pubkey, meta.is_signer, account))
.collect(); .collect();
let mut invoke_context = MockInvokeContext::new(keyed_accounts); super::process_instruction(
mock_set_sysvar( &Pubkey::default(),
&mut invoke_context, &instruction.data,
sysvar::rent::id(), &mut MockInvokeContext::new(keyed_accounts),
sysvar::rent::Rent::default(),
) )
.unwrap();
super::process_instruction(&Pubkey::default(), &instruction.data, &mut invoke_context)
} }
} }

View File

@ -20,6 +20,7 @@ use {
}, },
std::{ std::{
boxed::Box, boxed::Box,
cmp::Ordering,
collections::{HashSet, VecDeque}, collections::{HashSet, VecDeque},
}, },
}; };
@ -681,28 +682,20 @@ pub fn withdraw<S: std::hash::BuildHasher>(
lamports: u64, lamports: u64,
to_account: &KeyedAccount, to_account: &KeyedAccount,
signers: &HashSet<Pubkey, S>, signers: &HashSet<Pubkey, S>,
rent_sysvar: Option<Rent>,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
let vote_state: VoteState = let vote_state: VoteState =
State::<VoteStateVersions>::state(vote_account)?.convert_to_current(); State::<VoteStateVersions>::state(vote_account)?.convert_to_current();
verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?; verify_authorized_signer(&vote_state.authorized_withdrawer, signers)?;
let remaining_balance = vote_account match vote_account.lamports()?.cmp(&lamports) {
.lamports()? Ordering::Less => return Err(InstructionError::InsufficientFunds),
.checked_sub(lamports) Ordering::Equal => {
.ok_or(InstructionError::InsufficientFunds)?; // Deinitialize upon zero-balance
vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?;
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 vote_account
.try_account_ref_mut()? .try_account_ref_mut()?
.checked_sub_lamports(lamports)?; .checked_sub_lamports(lamports)?;
@ -906,8 +899,6 @@ mod tests {
} }
fn create_test_account() -> (Pubkey, RefCell<AccountSharedData>) { fn create_test_account() -> (Pubkey, RefCell<AccountSharedData>) {
let rent = Rent::default();
let balance = VoteState::get_rent_exempt_reserve(&rent);
let vote_pubkey = solana_sdk::pubkey::new_rand(); let vote_pubkey = solana_sdk::pubkey::new_rand();
( (
vote_pubkey, vote_pubkey,
@ -915,7 +906,7 @@ mod tests {
&vote_pubkey, &vote_pubkey,
&solana_sdk::pubkey::new_rand(), &solana_sdk::pubkey::new_rand(),
0, 0,
balance, 100,
)), )),
) )
} }
@ -1676,129 +1667,46 @@ mod tests {
&RefCell::new(AccountSharedData::default()), &RefCell::new(AccountSharedData::default()),
), ),
&signers, &signers,
None,
); );
assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); assert_eq!(res, Err(InstructionError::MissingRequiredSignature));
// insufficient funds // insufficient funds
let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports();
let signers: HashSet<Pubkey> = get_signers(keyed_accounts); let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let res = withdraw( let res = withdraw(
&keyed_accounts[0], &keyed_accounts[0],
lamports + 1, 101,
&KeyedAccount::new( &KeyedAccount::new(
&solana_sdk::pubkey::new_rand(), &solana_sdk::pubkey::new_rand(),
false, false,
&RefCell::new(AccountSharedData::default()), &RefCell::new(AccountSharedData::default()),
), ),
&signers, &signers,
None,
); );
assert_eq!(res, Err(InstructionError::InsufficientFunds)); assert_eq!(res, Err(InstructionError::InsufficientFunds));
// non rent exempt withdraw, before feature activation // all good
{ 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 keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)];
let lamports = vote_account.borrow().lamports(); let signers: HashSet<Pubkey> = get_signers(keyed_accounts);
let rent_sysvar = Rent::default(); let pre_state: VoteStateVersions = vote_account.borrow().state().unwrap();
let minimum_balance = rent_sysvar let res = withdraw(
.minimum_balance(vote_account.borrow().data().len()) &keyed_accounts[0],
.max(1); lamports,
assert!(minimum_balance <= lamports); &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account),
let signers: HashSet<Pubkey> = get_signers(keyed_accounts); &signers,
let res = withdraw( );
&keyed_accounts[0], assert_eq!(res, Ok(()));
lamports - minimum_balance + 1, assert_eq!(vote_account.borrow().lamports(), 0);
&KeyedAccount::new( assert_eq!(to_account.borrow().lamports(), lamports);
&solana_sdk::pubkey::new_rand(), let post_state: VoteStateVersions = vote_account.borrow().state().unwrap();
false, // State has been deinitialized since balance is zero
&RefCell::new(AccountSharedData::default()), assert!(post_state.is_uninitialized());
),
&signers,
None,
);
assert_eq!(res, Ok(()));
}
// non rent exempt withdraw, after feature activation // reset balance and restore state, verify that authorized_withdrawer works
{ vote_account.borrow_mut().set_lamports(lamports);
let (vote_pubkey, vote_account) = create_test_account(); vote_account.borrow_mut().set_state(&pre_state).unwrap();
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<Pubkey> = 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<Pubkey> = 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<Pubkey> = 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 // authorize authorized_withdrawer
let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand(); let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand();
@ -1827,7 +1735,6 @@ mod tests {
lamports, lamports,
withdrawer_keyed_account, withdrawer_keyed_account,
&signers, &signers,
None,
); );
assert_eq!(res, Ok(())); assert_eq!(res, Ok(()));
assert_eq!(vote_account.borrow().lamports(), 0); assert_eq!(vote_account.borrow().lamports(), 0);

View File

@ -273,10 +273,6 @@ pub mod reject_section_virtual_address_file_offset_mismatch {
solana_sdk::declare_id!("5N4NikcJLEiZNqwndhNyvZw15LvFXp1oF7AJQTNTZY5k"); solana_sdk::declare_id!("5N4NikcJLEiZNqwndhNyvZw15LvFXp1oF7AJQTNTZY5k");
} }
pub mod reject_non_rent_exempt_vote_withdraws {
solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1");
}
lazy_static! { lazy_static! {
/// Map of feature identifiers to user-visible description /// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [ pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -345,7 +341,6 @@ lazy_static! {
(spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"), (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_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_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 ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()