Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Reject vote withdraws that create non-rent-exempt accounts (backport #21639) #21645

Merged
merged 2 commits into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,20 @@ impl<'a> InvokeContext<'a> {
pub fn new_mock(
accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
builtin_programs: &'a [BuiltinProgram],
) -> Self {
Self::new_mock_with_sysvars(accounts, builtin_programs, &[])
}

pub fn new_mock_with_sysvars(
accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
builtin_programs: &'a [BuiltinProgram],
sysvars: &'a [(Pubkey, Vec<u8>)],
) -> Self {
Self::new(
Rent::default(),
accounts,
builtin_programs,
&[],
sysvars,
Some(LogCollector::new_ref()),
ComputeBudget::default(),
Rc::new(RefCell::new(Executors::default())),
Expand Down Expand Up @@ -915,19 +923,21 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
callback(&mut invoke_context)
}

pub fn mock_process_instruction(
pub fn mock_process_instruction_with_sysvars(
loader_id: &Pubkey,
mut program_indices: Vec<usize>,
instruction_data: &[u8],
keyed_accounts: &[(bool, bool, Pubkey, Rc<RefCell<AccountSharedData>>)],
sysvars: &[(Pubkey, Vec<u8>)],
process_instruction: ProcessInstructionWithContext,
) -> Result<(), InstructionError> {
let mut preparation =
prepare_mock_invoke_context(&program_indices, instruction_data, keyed_accounts);
let processor_account = AccountSharedData::new_ref(0, 0, &solana_sdk::native_loader::id());
program_indices.insert(0, preparation.accounts.len());
preparation.accounts.push((*loader_id, processor_account));
let mut invoke_context = InvokeContext::new_mock(&preparation.accounts, &[]);
let mut invoke_context =
InvokeContext::new_mock_with_sysvars(&preparation.accounts, &[], sysvars);
invoke_context.push(
&preparation.message,
&preparation.message.instructions[0],
Expand All @@ -937,6 +947,23 @@ pub fn mock_process_instruction(
process_instruction(1, instruction_data, &mut invoke_context)
}

pub fn mock_process_instruction(
loader_id: &Pubkey,
program_indices: Vec<usize>,
instruction_data: &[u8],
keyed_accounts: &[(bool, bool, Pubkey, Rc<RefCell<AccountSharedData>>)],
process_instruction: ProcessInstructionWithContext,
) -> Result<(), InstructionError> {
mock_process_instruction_with_sysvars(
loader_id,
program_indices,
instruction_data,
keyed_accounts,
&[],
process_instruction,
)
}

#[cfg(test)]
mod tests {
use {
Expand Down
16 changes: 14 additions & 2 deletions programs/vote/src/vote_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,15 @@ pub fn process_instruction(
}
VoteInstruction::Withdraw(lamports) => {
let to = keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?;
vote_state::withdraw(me, lamports, to, &signers)
let rent_sysvar = if invoke_context
.feature_set
.is_active(&feature_set::reject_non_rent_exempt_vote_withdraws::id())
{
Some(invoke_context.get_sysvar(&sysvar::rent::id())?)
} else {
None
};
vote_state::withdraw(me, lamports, to, &signers, rent_sysvar)
}
VoteInstruction::AuthorizeChecked(vote_authorize) => {
if invoke_context
Expand Down Expand Up @@ -500,11 +508,15 @@ mod tests {
.zip(accounts.into_iter())
.map(|(meta, account)| (meta.is_signer, meta.is_writable, meta.pubkey, account))
.collect();
solana_program_runtime::invoke_context::mock_process_instruction(

let rent = Rent::default();
let rent_sysvar = (sysvar::rent::id(), bincode::serialize(&rent).unwrap());
solana_program_runtime::invoke_context::mock_process_instruction_with_sysvars(
&id(),
Vec::new(),
&instruction.data,
&keyed_accounts,
&[rent_sysvar],
super::process_instruction,
)
}
Expand Down
152 changes: 123 additions & 29 deletions programs/vote/src/vote_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,20 +890,28 @@ pub fn withdraw<S: std::hash::BuildHasher>(
lamports: u64,
to_account: &KeyedAccount,
signers: &HashSet<Pubkey, S>,
rent_sysvar: Option<Rent>,
) -> Result<(), InstructionError> {
let vote_state: VoteState =
State::<VoteStateVersions>::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)?;
Expand Down Expand Up @@ -1107,14 +1115,16 @@ mod tests {
}

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();
(
vote_pubkey,
RefCell::new(vote_state::create_account(
&vote_pubkey,
&solana_sdk::pubkey::new_rand(),
0,
100,
balance,
)),
)
}
Expand Down Expand Up @@ -1875,46 +1885,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<Pubkey> = 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<Pubkey> = 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<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,
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<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
let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand();
Expand Down Expand Up @@ -1943,6 +2036,7 @@ mod tests {
lamports,
withdrawer_keyed_account,
&signers,
None,
);
assert_eq!(res, Ok(()));
assert_eq!(vote_account.borrow().lamports(), 0);
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ pub mod reject_empty_instruction_without_program {
solana_sdk::declare_id!("9kdtFSrXHQg3hKkbXkQ6trJ3Ja1xpJ22CTFSNAciEwmL");
}

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<Pubkey, &'static str> = [
Expand Down Expand Up @@ -323,6 +327,7 @@ lazy_static! {
(spl_token_v3_3_0_release::id(), "spl-token v3.3.0 release"),
(leave_nonce_on_success::id(), "leave nonce as is on success"),
(reject_empty_instruction_without_program::id(), "fail instructions which have native_loader as program_id directly"),
(reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down