diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 8447719b99..5b775a97e5 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -192,12 +192,20 @@ impl<'a> InvokeContext<'a> { pub fn new_mock( accounts: &'a [(Pubkey, Rc>)], builtin_programs: &'a [BuiltinProgram], + ) -> Self { + Self::new_mock_with_sysvars(accounts, builtin_programs, &[]) + } + + pub fn new_mock_with_sysvars( + accounts: &'a [(Pubkey, Rc>)], + builtin_programs: &'a [BuiltinProgram], + sysvars: &'a [(Pubkey, Vec)], ) -> Self { Self::new( Rent::default(), accounts, builtin_programs, - &[], + sysvars, Some(LogCollector::new_ref()), ComputeBudget::default(), Rc::new(RefCell::new(Executors::default())), @@ -918,11 +926,12 @@ pub fn with_mock_invoke_context R>( callback(&mut invoke_context) } -pub fn mock_process_instruction( +pub fn mock_process_instruction_with_sysvars( loader_id: &Pubkey, mut program_indices: Vec, instruction_data: &[u8], keyed_accounts: &[(bool, bool, Pubkey, Rc>)], + sysvars: &[(Pubkey, Vec)], process_instruction: ProcessInstructionWithContext, ) -> Result<(), InstructionError> { let mut preparation = @@ -930,7 +939,8 @@ pub fn mock_process_instruction( 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], @@ -940,6 +950,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, + instruction_data: &[u8], + keyed_accounts: &[(bool, bool, Pubkey, Rc>)], + 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 { diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index dde8ceda05..f64c94d22a 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -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 @@ -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, ) } diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index eb865ca3d8..7e1be355c3 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -890,20 +890,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)?; @@ -1107,6 +1115,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, @@ -1114,7 +1124,7 @@ mod tests { &vote_pubkey, &solana_sdk::pubkey::new_rand(), 0, - 100, + balance, )), ) } @@ -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 = 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(); @@ -1943,6 +2036,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 b773394147..7afc61c217 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -267,6 +267,10 @@ pub mod fixed_memcpy_nonoverlapping_check { solana_sdk::declare_id!("36PRUK2Dz6HWYdG9SpjeAsF5F3KxnFCakA2BZMbtMhSb"); } +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 = [ @@ -328,6 +332,7 @@ lazy_static! { (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"), (fixed_memcpy_nonoverlapping_check::id(), "use correct check for nonoverlapping regions in memcpy syscall"), + (reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()