diff --git a/interface/src/instruction.rs b/interface/src/instruction.rs index 728e4aa2..fd202f39 100644 --- a/interface/src/instruction.rs +++ b/interface/src/instruction.rs @@ -29,6 +29,10 @@ const RENT_ID: Pubkey = Pubkey::from_str_const("SysvarRent1111111111111111111111 const STAKE_HISTORY_ID: Pubkey = Pubkey::from_str_const("SysvarStakeHistory1111111111111111111111111"); +// NOTE the stake program is in the process of removing dependence on all sysvars +// once this version of the program is live on all clusters, we can remove them here +// namely, from all doc comments in `StakeInstruction` and in all instruction builders +// we may also remove all use of and reference to the stake config account #[cfg_attr( feature = "serde", derive(serde_derive::Deserialize, serde_derive::Serialize) diff --git a/program/Cargo.toml b/program/Cargo.toml index fff2ae6b..7370f430 100644 --- a/program/Cargo.toml +++ b/program/Cargo.toml @@ -18,6 +18,7 @@ borsh = { version = "1.5.1", features = ["derive", "unstable__schema"] } num-derive = "0.4" num-traits = "0.2" num_enum = "0.7.3" +solana-sdk-ids = "2.2.1" solana-program = "2.2.1" thiserror = "1.0.63" @@ -36,7 +37,6 @@ solana-stake-interface = { version = "1", features = ["bincode"] } solana-system-interface = { version = "1", features = ["bincode"] } solana-vote-program = "2.2.0" solana-sdk = "2.2.1" -solana-sdk-ids = "2.2.1" solana-sysvar = { version = "2.2.1", features = ["bincode"] } rand = "0.8.5" test-case = "3.3.1" diff --git a/program/src/helpers/mod.rs b/program/src/helpers/mod.rs index 89523b0e..91a7469b 100644 --- a/program/src/helpers/mod.rs +++ b/program/src/helpers/mod.rs @@ -3,9 +3,6 @@ use solana_program::{instruction::InstructionError, program_error::ProgramError} pub(crate) mod delegate; pub(crate) use delegate::*; -pub(crate) mod split; -pub(crate) use split::*; - pub(crate) mod merge; pub(crate) use merge::*; diff --git a/program/src/helpers/split.rs b/program/src/helpers/split.rs deleted file mode 100644 index f3cb3bcd..00000000 --- a/program/src/helpers/split.rs +++ /dev/null @@ -1,85 +0,0 @@ -use solana_program::{program_error::ProgramError, rent::Rent, stake::state::Meta, sysvar::Sysvar}; - -/// After calling `validate_split_amount()`, this struct contains calculated -/// values that are used by the caller. -#[derive(Copy, Clone, Debug, Default)] -pub(crate) struct ValidatedSplitInfo { - pub source_remaining_balance: u64, - pub destination_rent_exempt_reserve: u64, -} - -/// Ensure the split amount is valid. This checks the source and destination -/// accounts meet the minimum balance requirements, which is the rent exempt -/// reserve plus the minimum stake delegation, and that the source account has -/// enough lamports for the request split amount. If not, return an error. -pub(crate) fn validate_split_amount( - source_lamports: u64, - destination_lamports: u64, - split_lamports: u64, - source_meta: &Meta, - destination_data_len: usize, - additional_required_lamports: u64, - source_is_active: bool, -) -> Result { - // Split amount has to be something - if split_lamports == 0 { - return Err(ProgramError::InsufficientFunds); - } - - // Obviously cannot split more than what the source account has - if split_lamports > source_lamports { - return Err(ProgramError::InsufficientFunds); - } - - // Verify that the source account still has enough lamports left after - // splitting: EITHER at least the minimum balance, OR zero (in this case the - // source account is transferring all lamports to new destination account, - // and the source account will be closed) - let source_minimum_balance = source_meta - .rent_exempt_reserve - .saturating_add(additional_required_lamports); - let source_remaining_balance = source_lamports.saturating_sub(split_lamports); - if source_remaining_balance == 0 { - // full amount is a withdrawal - // nothing to do here - } else if source_remaining_balance < source_minimum_balance { - // the remaining balance is too low to do the split - return Err(ProgramError::InsufficientFunds); - } else { - // all clear! - // nothing to do here - } - - let rent = Rent::get()?; - let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len); - - // If the source is active stake, one of these criteria must be met: - // 1. the destination account must be prefunded with at least the rent-exempt - // reserve, or - // 2. the split must consume 100% of the source - if source_is_active - && source_remaining_balance != 0 - && destination_lamports < destination_rent_exempt_reserve - { - return Err(ProgramError::InsufficientFunds); - } - - // Verify the destination account meets the minimum balance requirements - // This must handle: - // 1. The destination account having a different rent exempt reserve due to data - // size changes - // 2. The destination account being prefunded, which would lower the minimum - // split amount - let destination_minimum_balance = - destination_rent_exempt_reserve.saturating_add(additional_required_lamports); - let destination_balance_deficit = - destination_minimum_balance.saturating_sub(destination_lamports); - if split_lamports < destination_balance_deficit { - return Err(ProgramError::InsufficientFunds); - } - - Ok(ValidatedSplitInfo { - source_remaining_balance, - destination_rent_exempt_reserve, - }) -} diff --git a/program/src/processor.rs b/program/src/processor.rs index c533e1f6..7b8a229e 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -1,7 +1,7 @@ use { crate::{helpers::*, id, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH}, solana_program::{ - account_info::{next_account_info, AccountInfo}, + account_info::AccountInfo, clock::Clock, entrypoint::ProgramResult, msg, @@ -18,12 +18,35 @@ use { state::{Authorized, Lockup, Meta, StakeAuthorize, StakeStateV2}, tools::{acceptable_reference_epoch_credits, eligible_for_deactivate_delinquent}, }, - sysvar::{epoch_rewards::EpochRewards, stake_history::StakeHistorySysvar, Sysvar}, + sysvar::{ + epoch_rewards::EpochRewards, + stake_history::{StakeHistory, StakeHistorySysvar}, + Sysvar, SysvarId, + }, vote::{program as solana_vote_program, state::VoteState}, }, std::{collections::HashSet, mem::MaybeUninit}, }; +#[allow(deprecated)] +fn is_stake_program_sysvar_or_config(pubkey: Pubkey) -> bool { + pubkey == Clock::id() + || pubkey == Rent::id() + || pubkey == StakeHistory::id() + || pubkey == solana_sdk_ids::stake::config::id() +} + +fn next_non_sysvar_account<'a, 'b, I: Iterator>>( + iter: &mut I, +) -> Result { + loop { + let account_info = iter.next().ok_or(ProgramError::NotEnoughAccountKeys)?; + if !is_stake_program_sysvar_or_config(*account_info.key) { + return Ok(account_info); + } + } +} + fn get_vote_state(vote_account_info: &AccountInfo) -> Result, ProgramError> { if *vote_account_info.owner != solana_vote_program::id() { return Err(ProgramError::IncorrectProgramId); @@ -131,12 +154,13 @@ fn do_initialize( stake_account_info: &AccountInfo, authorized: Authorized, lockup: Lockup, - rent: &Rent, ) -> ProgramResult { if stake_account_info.data_len() != StakeStateV2::size_of() { return Err(ProgramError::InvalidAccountData); } + let rent = Rent::get()?; + if let StakeStateV2::Uninitialized = get_stake_state(stake_account_info)? { let rent_exempt_reserve = rent.minimum_balance(stake_account_info.data_len()); if stake_account_info.lamports() >= rent_exempt_reserve { @@ -161,8 +185,9 @@ fn do_authorize( new_authority: &Pubkey, authority_type: StakeAuthorize, custodian: Option<&Pubkey>, - clock: &Clock, ) -> ProgramResult { + let clock = Clock::get()?; + match get_stake_state(stake_account_info)? { StakeStateV2::Initialized(mut meta) => { meta.authorized @@ -170,7 +195,7 @@ fn do_authorize( signers, new_authority, authority_type, - Some((&meta.lockup, clock, custodian)), + Some((&meta.lockup, &clock, custodian)), ) .map_err(to_program_error)?; @@ -182,7 +207,7 @@ fn do_authorize( signers, new_authority, authority_type, - Some((&meta.lockup, clock, custodian)), + Some((&meta.lockup, &clock, custodian)), ) .map_err(to_program_error)?; @@ -291,19 +316,22 @@ fn move_stake_or_lamports_shared_checks( // when lengths are asserted in setup, accounts are retrieved via hardcoded index from InstructionContext // but after control is passed to main processing functions, they are pulled from the TransactionContext // -// we aim to implement this behavior exactly, such that both programs are consensus compatible: +// when porting to bpf, we reimplemented this behavior exactly, such that both programs would be consensus compatible: // * all transactions that would fail on one program also fail on the other // * all transactions that would succeed on one program also succeed on the other // * for successful transactions, all account state transitions are identical // error codes and log output may differ // -// this is not strictly necessary, since the switchover will be feature-gated. so this is not a security issue -// mostly its so no one can blame the bpf switchover for breaking their usecase, even pathological ones +// now that the switchover is complete, there may be opportunities to tighten up the interface +// particularly by asserting that some "optional" accounts must exist +// we should avoid breaking odd workflows (such as self-signed stake accounts) but may consider breaking pathological flows +// commented-out `next_non_sysvar_account()` calls mark accounts that should typically be included +// this differs from `.ok()` account retrievals (lockup custodians) which are optional by design // -// in service to this end, all accounts iters are commented with how the native program uses them -// for accounts that should always, or almost always, exist, which the native program does not assert... -// ...we leave a commented-out `next_account_info()` call, to aid in a future refactor -// after the bpf switchover ships, we may update to strictly assert these accounts exist +// the native stake program also accepted some sysvars as input accounts, but pulled others from `InvokeContext` +// this was done for backwards compatibility but the end result was highly inconsistent +// now, we skip all sysvar accounts previously required (clock, rent, stake history) and retrieve them via syscall +// we also skip the stake config account, which was removed from native stake but is still included in instruction builders pub struct Processor {} impl Processor { fn process_initialize( @@ -313,14 +341,11 @@ impl Processor { ) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 2 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let rent_info = next_account_info(account_info_iter)?; - - let rent = &Rent::from_account_info(rent_info)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; // `get_stake_state()` is called unconditionally, which checks owner - do_initialize(stake_account_info, authorized, lockup, rent)?; + do_initialize(stake_account_info, authorized, lockup)?; Ok(()) } @@ -333,15 +358,12 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 3 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let _stake_or_withdraw_authority_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let _stake_or_withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let option_lockup_authority_info = next_account_info(account_info_iter).ok(); - - let clock = &Clock::from_account_info(clock_info)?; + let option_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); let custodian = option_lockup_authority_info .filter(|a| a.is_signer) @@ -354,7 +376,6 @@ impl Processor { &new_authority, authority_type, custodian, - clock, )?; Ok(()) @@ -364,17 +385,14 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 5 accounts (2 sysvars + stake config) - let stake_account_info = next_account_info(account_info_iter)?; - let vote_account_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let _stake_history_info = next_account_info(account_info_iter)?; - let _stake_config_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let vote_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - // let _stake_authority_info = next_account_info(account_info_iter); + // let _stake_authority_info = next_non_sysvar_account(account_info_iter); - let clock = &Clock::from_account_info(clock_info)?; + let clock = &Clock::get()?; let stake_history = &StakeHistorySysvar(clock.epoch); let vote_state = get_vote_state(vote_account_info)?; @@ -429,20 +447,17 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 2 accounts - let source_stake_account_info = next_account_info(account_info_iter)?; - let destination_stake_account_info = next_account_info(account_info_iter)?; + // required accounts + let source_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let destination_stake_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - // let _stake_authority_info = next_account_info(account_info_iter); + // let _stake_authority_info = next_non_sysvar_account(account_info_iter); + let rent = Rent::get()?; let clock = Clock::get()?; let stake_history = &StakeHistorySysvar(clock.epoch); - - let destination_data_len = destination_stake_account_info.data_len(); - if destination_data_len != StakeStateV2::size_of() { - return Err(ProgramError::InvalidAccountData); - } + let minimum_delegation = crate::get_minimum_delegation(); if let StakeStateV2::Uninitialized = get_stake_state(destination_stake_account_info)? { // we can split into this @@ -457,84 +472,179 @@ impl Processor { return Err(ProgramError::InsufficientFunds); } - match get_stake_state(source_stake_account_info)? { - StakeStateV2::Stake(source_meta, mut source_stake, stake_flags) => { + if split_lamports == 0 { + return Err(ProgramError::InsufficientFunds); + } + + let source_rent_exempt_reserve = rent.minimum_balance(source_stake_account_info.data_len()); + + let destination_data_len = destination_stake_account_info.data_len(); + if destination_data_len != StakeStateV2::size_of() { + return Err(ProgramError::InvalidAccountData); + } + let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len); + + // check signers and get delegation status along with a destination meta + let source_stake_state = get_stake_state(source_stake_account_info)?; + let (is_active_or_activating, option_dest_meta) = match source_stake_state { + StakeStateV2::Stake(source_meta, source_stake, _) => { source_meta .authorized .check(&signers, StakeAuthorize::Staker) .map_err(to_program_error)?; - let minimum_delegation = crate::get_minimum_delegation(); - - let status = source_stake.delegation.stake_activating_and_deactivating( + let source_status = source_stake.delegation.stake_activating_and_deactivating( clock.epoch, stake_history, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, ); + let is_active_or_activating = + source_status.effective > 0 || source_status.activating > 0; - let is_active = status.effective > 0; + let mut dest_meta = source_meta; + dest_meta.rent_exempt_reserve = destination_rent_exempt_reserve; - // NOTE this function also internally summons Rent via syscall - let validated_split_info = validate_split_amount( - source_lamport_balance, - destination_lamport_balance, - split_lamports, - &source_meta, - destination_data_len, - minimum_delegation, - is_active, - )?; + (is_active_or_activating, Some(dest_meta)) + } + StakeStateV2::Initialized(source_meta) => { + source_meta + .authorized + .check(&signers, StakeAuthorize::Staker) + .map_err(to_program_error)?; - // split the stake, subtract rent_exempt_balance unless - // the destination account already has those lamports - // in place. - // this means that the new stake account will have a stake equivalent to - // lamports minus rent_exempt_reserve if it starts out with a zero balance - let (remaining_stake_delta, split_stake_amount) = - if validated_split_info.source_remaining_balance == 0 { - // If split amount equals the full source stake (as implied by 0 - // source_remaining_balance), the new split stake must equal the same - // amount, regardless of any current lamport balance in the split account. - // Since split accounts retain the state of their source account, this - // prevents any magic activation of stake by prefunding the split account. - // - // The new split stake also needs to ignore any positive delta between the - // original rent_exempt_reserve and the split_rent_exempt_reserve, in order - // to prevent magic activation of stake by splitting between accounts of - // different sizes. - let remaining_stake_delta = - split_lamports.saturating_sub(source_meta.rent_exempt_reserve); - (remaining_stake_delta, remaining_stake_delta) + let mut dest_meta = source_meta; + dest_meta.rent_exempt_reserve = destination_rent_exempt_reserve; + + (false, Some(dest_meta)) + } + StakeStateV2::Uninitialized => { + if !source_stake_account_info.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + + (false, None) + } + StakeStateV2::RewardsPool => return Err(ProgramError::InvalidAccountData), + }; + + // special case: for a full split, we only care that the destination becomes a valid stake account + // this prevents state changes in exceptional cases where a once-valid source has become invalid + // relocate lamports, copy data, and close the original account + if split_lamports == source_lamport_balance { + let mut destination_stake_state = source_stake_state; + let delegation = match (&mut destination_stake_state, option_dest_meta) { + (StakeStateV2::Stake(meta, stake, _), Some(dest_meta)) => { + *meta = dest_meta; + + if is_active_or_activating { + stake.delegation.stake } else { - // Otherwise, the new split stake should reflect the entire split - // requested, less any lamports needed to cover the - // split_rent_exempt_reserve. - if source_stake.delegation.stake.saturating_sub(split_lamports) - < minimum_delegation - { - return Err(StakeError::InsufficientDelegation.into()); - } - - ( - split_lamports, - split_lamports.saturating_sub( - validated_split_info - .destination_rent_exempt_reserve - .saturating_sub(destination_lamport_balance), - ), - ) - }; - - if split_stake_amount < minimum_delegation { + 0 + } + } + (StakeStateV2::Initialized(meta), Some(dest_meta)) => { + *meta = dest_meta; + + 0 + } + (StakeStateV2::Uninitialized, None) => 0, + _ => unreachable!(), + }; + + if destination_lamport_balance + .saturating_add(split_lamports) + .saturating_sub(delegation) + < destination_rent_exempt_reserve + { + return Err(ProgramError::InsufficientFunds); + } + + if is_active_or_activating && delegation < minimum_delegation { + return Err(StakeError::InsufficientDelegation.into()); + } + + set_stake_state(destination_stake_account_info, &destination_stake_state)?; + if source_stake_account_info.key != destination_stake_account_info.key { + source_stake_account_info.realloc(0, false)?; + } + + relocate_lamports( + source_stake_account_info, + destination_stake_account_info, + split_lamports, + )?; + debug_assert!(source_stake_account_info.lamports() == 0); + + return Ok(()); + } + + // special case: if stake is fully inactive, we only care that both accounts meet rent-exemption + if !is_active_or_activating { + let mut destination_stake_state = source_stake_state; + match (&mut destination_stake_state, option_dest_meta) { + (StakeStateV2::Stake(meta, _, _), Some(dest_meta)) + | (StakeStateV2::Initialized(meta), Some(dest_meta)) => { + *meta = dest_meta; + } + (StakeStateV2::Uninitialized, None) => (), + _ => unreachable!(), + } + + let post_source_lamports = source_lamport_balance.saturating_sub(split_lamports); + let post_destination_lamports = + destination_lamport_balance.saturating_add(split_lamports); + + if post_source_lamports < source_rent_exempt_reserve + || post_destination_lamports < destination_rent_exempt_reserve + { + return Err(ProgramError::InsufficientFunds); + } + + set_stake_state(destination_stake_account_info, &destination_stake_state)?; + + relocate_lamports( + source_stake_account_info, + destination_stake_account_info, + split_lamports, + )?; + + return Ok(()); + } + + // at this point, we know we have a StakeStateV2::Stake source that is either activating or has nonzero effective + // this means we must redistribute the delegation across both accounts and enforce: + // * destination has a pre-funded rent exemption + // * source meets rent exemption less its remaining delegation + // * source and destination both meet the minimum delegation + // destination delegation is matched 1:1 by split lamports. in other words, free source lamports are never split + match (source_stake_state, option_dest_meta) { + (StakeStateV2::Stake(source_meta, mut source_stake, stake_flags), Some(dest_meta)) => { + if destination_lamport_balance < destination_rent_exempt_reserve { + return Err(ProgramError::InsufficientFunds); + } + + let mut dest_stake = source_stake; + + source_stake.delegation.stake = + source_stake.delegation.stake.saturating_sub(split_lamports); + + if source_stake.delegation.stake < minimum_delegation { return Err(StakeError::InsufficientDelegation.into()); } - let destination_stake = - source_stake.split(remaining_stake_delta, split_stake_amount)?; + // minimum delegation is by definition nonzero, and we remove one delegated lamport per split lamport + // since the remaining source delegation > 0, it is impossible that we took from its rent-exempt reserve + debug_assert!( + source_lamport_balance + .saturating_sub(split_lamports) + .saturating_sub(source_stake.delegation.stake) + >= source_meta.rent_exempt_reserve + ); - let mut destination_meta = source_meta; - destination_meta.rent_exempt_reserve = - validated_split_info.destination_rent_exempt_reserve; + dest_stake.delegation.stake = split_lamports; + if dest_stake.delegation.stake < minimum_delegation { + return Err(StakeError::InsufficientDelegation.into()); + } set_stake_state( source_stake_account_info, @@ -543,71 +653,33 @@ impl Processor { set_stake_state( destination_stake_account_info, - &StakeStateV2::Stake(destination_meta, destination_stake, stake_flags), - )?; - } - StakeStateV2::Initialized(source_meta) => { - source_meta - .authorized - .check(&signers, StakeAuthorize::Staker) - .map_err(to_program_error)?; - - // NOTE this function also internally summons Rent via syscall - let validated_split_info = validate_split_amount( - source_lamport_balance, - destination_lamport_balance, - split_lamports, - &source_meta, - destination_data_len, - 0, // additional_required_lamports - false, // is_active + &StakeStateV2::Stake(dest_meta, dest_stake, stake_flags), )?; - let mut destination_meta = source_meta; - destination_meta.rent_exempt_reserve = - validated_split_info.destination_rent_exempt_reserve; - - set_stake_state( + relocate_lamports( + source_stake_account_info, destination_stake_account_info, - &StakeStateV2::Initialized(destination_meta), + split_lamports, )?; } - StakeStateV2::Uninitialized => { - if !source_stake_account_info.is_signer { - return Err(ProgramError::MissingRequiredSignature); - } - } - _ => return Err(ProgramError::InvalidAccountData), - } - - // Truncate state upon zero balance - if split_lamports == source_lamport_balance { - source_stake_account_info.realloc(0, false)?; + _ => unreachable!(), } - relocate_lamports( - source_stake_account_info, - destination_stake_account_info, - split_lamports, - )?; - Ok(()) } fn process_withdraw(accounts: &[AccountInfo], withdraw_lamports: u64) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 5 accounts (2 sysvars) - let source_stake_account_info = next_account_info(account_info_iter)?; - let destination_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let _stake_history_info = next_account_info(account_info_iter)?; - let withdraw_authority_info = next_account_info(account_info_iter)?; + // required accounts + let source_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let destination_info = next_non_sysvar_account(account_info_iter)?; + let withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let option_lockup_authority_info = next_account_info(account_info_iter).ok(); + let option_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); - let clock = &Clock::from_account_info(clock_info)?; + let clock = &Clock::get()?; let stake_history = &StakeHistorySysvar(clock.epoch); // this is somewhat subtle. for Initialized and Stake, there is a real authority @@ -699,14 +771,13 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 2 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - // let _stake_authority_info = next_account_info(account_info_iter); + // let _stake_authority_info = next_non_sysvar_account(account_info_iter); - let clock = &Clock::from_account_info(clock_info)?; + let clock = &Clock::get()?; match get_stake_state(stake_account_info)? { StakeStateV2::Stake(meta, mut stake, stake_flags) => { @@ -731,11 +802,11 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 1 account - let stake_account_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - // let _old_withdraw_or_lockup_authority_info = next_account_info(account_info_iter); + // let _old_withdraw_or_lockup_authority_info = next_non_sysvar_account(account_info_iter); let clock = Clock::get()?; @@ -749,16 +820,14 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 4 accounts (2 sysvars) - let destination_stake_account_info = next_account_info(account_info_iter)?; - let source_stake_account_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let _stake_history_info = next_account_info(account_info_iter)?; + // required accounts + let destination_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let source_stake_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - // let _stake_authority_info = next_account_info(account_info_iter); + // let _stake_authority_info = next_non_sysvar_account(account_info_iter); - let clock = &Clock::from_account_info(clock_info)?; + let clock = &Clock::get()?; let stake_history = &StakeHistorySysvar(clock.epoch); if source_stake_account_info.key == destination_stake_account_info.key { @@ -812,15 +881,12 @@ impl Processor { ) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 3 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let stake_or_withdraw_authority_base_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let stake_or_withdraw_authority_base_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let option_lockup_authority_info = next_account_info(account_info_iter).ok(); - - let clock = &Clock::from_account_info(clock_info)?; + let option_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); let (mut signers, custodian) = collect_signers_checked(None, option_lockup_authority_info)?; @@ -839,7 +905,6 @@ impl Processor { &authorize_args.new_authorized_pubkey, authorize_args.stake_authorize, custodian, - clock, )?; Ok(()) @@ -848,13 +913,10 @@ impl Processor { fn process_initialize_checked(accounts: &[AccountInfo]) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 4 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let rent_info = next_account_info(account_info_iter)?; - let stake_authority_info = next_account_info(account_info_iter)?; - let withdraw_authority_info = next_account_info(account_info_iter)?; - - let rent = &Rent::from_account_info(rent_info)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let stake_authority_info = next_non_sysvar_account(account_info_iter)?; + let withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; if !withdraw_authority_info.is_signer { return Err(ProgramError::MissingRequiredSignature); @@ -866,7 +928,7 @@ impl Processor { }; // `get_stake_state()` is called unconditionally, which checks owner - do_initialize(stake_account_info, authorized, Lockup::default(), rent)?; + do_initialize(stake_account_info, authorized, Lockup::default())?; Ok(()) } @@ -878,16 +940,13 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 4 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let _old_stake_or_withdraw_authority_info = next_account_info(account_info_iter)?; - let new_stake_or_withdraw_authority_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let _old_stake_or_withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; + let new_stake_or_withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let option_lockup_authority_info = next_account_info(account_info_iter).ok(); - - let clock = &Clock::from_account_info(clock_info)?; + let option_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); if !new_stake_or_withdraw_authority_info.is_signer { return Err(ProgramError::MissingRequiredSignature); @@ -904,7 +963,6 @@ impl Processor { new_stake_or_withdraw_authority_info.key, authority_type, custodian, - clock, )?; Ok(()) @@ -916,16 +974,13 @@ impl Processor { ) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 4 accounts (1 sysvar) - let stake_account_info = next_account_info(account_info_iter)?; - let old_stake_or_withdraw_authority_base_info = next_account_info(account_info_iter)?; - let clock_info = next_account_info(account_info_iter)?; - let new_stake_or_withdraw_authority_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let old_stake_or_withdraw_authority_base_info = next_non_sysvar_account(account_info_iter)?; + let new_stake_or_withdraw_authority_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let option_lockup_authority_info = next_account_info(account_info_iter).ok(); - - let clock = &Clock::from_account_info(clock_info)?; + let option_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); let (mut signers, custodian) = collect_signers_checked( Some(new_stake_or_withdraw_authority_info), @@ -947,7 +1002,6 @@ impl Processor { new_stake_or_withdraw_authority_info.key, authorize_args.stake_authorize, custodian, - clock, )?; Ok(()) @@ -960,12 +1014,12 @@ impl Processor { let signers = collect_signers(accounts); let account_info_iter = &mut accounts.iter(); - // native asserts: 1 account - let stake_account_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; // other accounts - let _old_withdraw_or_lockup_authority_info = next_account_info(account_info_iter); - let option_new_lockup_authority_info = next_account_info(account_info_iter).ok(); + let _old_withdraw_or_lockup_authority_info = next_non_sysvar_account(account_info_iter); + let option_new_lockup_authority_info = next_non_sysvar_account(account_info_iter).ok(); let clock = Clock::get()?; @@ -992,10 +1046,10 @@ impl Processor { fn process_deactivate_delinquent(accounts: &[AccountInfo]) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 3 accounts - let stake_account_info = next_account_info(account_info_iter)?; - let delinquent_vote_account_info = next_account_info(account_info_iter)?; - let reference_vote_account_info = next_account_info(account_info_iter)?; + // required accounts + let stake_account_info = next_non_sysvar_account(account_info_iter)?; + let delinquent_vote_account_info = next_non_sysvar_account(account_info_iter)?; + let reference_vote_account_info = next_non_sysvar_account(account_info_iter)?; let clock = Clock::get()?; @@ -1037,10 +1091,10 @@ impl Processor { fn process_move_stake(accounts: &[AccountInfo], lamports: u64) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 3 accounts - let source_stake_account_info = next_account_info(account_info_iter)?; - let destination_stake_account_info = next_account_info(account_info_iter)?; - let stake_authority_info = next_account_info(account_info_iter)?; + // required accounts + let source_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let destination_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let stake_authority_info = next_non_sysvar_account(account_info_iter)?; let (source_merge_kind, destination_merge_kind) = move_stake_or_lamports_shared_checks( source_stake_account_info, @@ -1174,10 +1228,10 @@ impl Processor { fn process_move_lamports(accounts: &[AccountInfo], lamports: u64) -> ProgramResult { let account_info_iter = &mut accounts.iter(); - // native asserts: 3 accounts - let source_stake_account_info = next_account_info(account_info_iter)?; - let destination_stake_account_info = next_account_info(account_info_iter)?; - let stake_authority_info = next_account_info(account_info_iter)?; + // required accounts + let source_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let destination_stake_account_info = next_non_sysvar_account(account_info_iter)?; + let stake_authority_info = next_non_sysvar_account(account_info_iter)?; let (source_merge_kind, _) = move_stake_or_lamports_shared_checks( source_stake_account_info, diff --git a/program/tests/interface.rs b/program/tests/interface.rs index 2a911521..b3b0f9c3 100644 --- a/program/tests/interface.rs +++ b/program/tests/interface.rs @@ -994,3 +994,30 @@ fn test_no_use_dealloc() { } } } + +// the original stake interface passed in sysvars and stake config +// we no longer retrieve these via account info in any instruction processors +// since instruction builders still provide them, we test the program does not require them +// NOTE this function and test can be deleted after the instruction builders are updated +#[allow(deprecated)] +fn is_stake_program_sysvar_or_config(pubkey: Pubkey) -> bool { + pubkey == Clock::id() + || pubkey == Rent::id() + || pubkey == StakeHistory::id() + || pubkey == solana_sdk_ids::stake::config::id() +} +#[test] +fn test_all_success_no_sysvars() { + let mut env = Env::init(); + + for declaration in &*INSTRUCTION_DECLARATIONS { + let mut instruction = declaration.to_instruction(&mut env); + + instruction + .accounts + .retain(|account| !is_stake_program_sysvar_or_config(account.pubkey)); + + env.process_success(&instruction); + env.reset(); + } +} diff --git a/program/tests/program_test.rs b/program/tests/program_test.rs index 3b835184..6e62f2a9 100644 --- a/program/tests/program_test.rs +++ b/program/tests/program_test.rs @@ -986,18 +986,7 @@ impl StakeLifecycle { } } - // NOTE the program enforces that a deactive stake adheres to the split minimum, - // albeit spuriously after solana-program/stake-program #1 is addressed, - // Self::Deactive should move to false equivalently this could be combined - // with withdraw_minimum_enforced into a function minimum_enforced - pub fn split_minimum_enforced(&self) -> bool { - match self { - Self::Activating | Self::Active | Self::Deactivating | Self::Deactive => true, - Self::Uninitialized | Self::Initialized | Self::Closed => false, - } - } - - pub fn withdraw_minimum_enforced(&self) -> bool { + pub fn minimum_delegation_enforced(&self) -> bool { match self { Self::Activating | Self::Active | Self::Deactivating => true, Self::Uninitialized | Self::Initialized | Self::Deactive | Self::Closed => false, @@ -1033,6 +1022,13 @@ async fn program_test_split(split_source_type: StakeLifecycle) { _ => vec![&staker_keypair], }; + // fail, cannot split zero + let instruction = &ixn::split(&split_source, &signers[0].pubkey(), 0, &split_dest)[2]; + let e = process_instruction(&mut context, instruction, &signers) + .await + .unwrap_err(); + assert_eq!(e, ProgramError::InsufficientFunds); + // fail, split more than available (even if not active, would kick source out of // rent exemption) let instruction = &ixn::split( @@ -1045,19 +1041,19 @@ async fn program_test_split(split_source_type: StakeLifecycle) { let e = process_instruction(&mut context, instruction, &signers) .await .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); + assert_eq!( + e, + if split_source_type.minimum_delegation_enforced() { + StakeError::InsufficientDelegation.into() + } else { + ProgramError::InsufficientFunds + } + ); // an active or transitioning stake account cannot have less than the minimum // delegation note this is NOT dependent on the minimum delegation feature. // there was ALWAYS a minimum. it was one lamport! - if split_source_type.split_minimum_enforced() { - // zero split fails - let instruction = &ixn::split(&split_source, &signers[0].pubkey(), 0, &split_dest)[2]; - let e = process_instruction(&mut context, instruction, &signers) - .await - .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); - + if split_source_type.minimum_delegation_enforced() { // underfunded destination fails let instruction = &ixn::split( &split_source, @@ -1082,7 +1078,7 @@ async fn program_test_split(split_source_type: StakeLifecycle) { let e = process_instruction(&mut context, instruction, &signers) .await .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); + assert_eq!(e, StakeError::InsufficientDelegation.into()); } // split to non-owned account fails @@ -1211,7 +1207,7 @@ async fn program_test_withdraw_stake(withdraw_source_type: StakeLifecycle) { .unwrap_err(); assert_eq!(e, ProgramError::InsufficientFunds); - if withdraw_source_type.withdraw_minimum_enforced() { + if withdraw_source_type.minimum_delegation_enforced() { // withdraw active or activating stake fails let instruction = ixn::withdraw( &withdraw_source, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index e465f307..7581b94d 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -455,8 +455,6 @@ fn test_stake_process_instruction_decode_bail() { let rent_address = rent::id(); let rent = Rent::default(); let rent_account = create_account_shared_data_for_test(&rent); - let rewards_address = rewards::id(); - let rewards_account = create_account_shared_data_for_test(&rewards::Rewards::new(0.0)); let stake_history_address = stake_history::id(); let stake_history_account = create_account_shared_data_for_test(&StakeHistory::default()); let vote_address = Pubkey::new_unique(); @@ -484,23 +482,6 @@ fn test_stake_process_instruction_decode_bail() { Err(ProgramError::NotEnoughAccountKeys), ); - // no account for rent - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Initialize( - Authorized::default(), - Lockup::default(), - )) - .unwrap(), - vec![(stake_address, stake_account.clone())], - vec![AccountMeta { - pubkey: stake_address, - is_signer: false, - is_writable: true, - }], - Err(ProgramError::NotEnoughAccountKeys), - ); - // fails to deserialize stake state process_instruction( &mollusk, @@ -595,46 +576,6 @@ fn test_stake_process_instruction_decode_bail() { Err(ProgramError::InvalidAccountData), ); - // Tests 3rd keyed account is of correct type (Clock instead of rewards) in withdraw - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Withdraw(withdrawal_amount)).unwrap(), - vec![ - (stake_address, stake_account.clone()), - (vote_address, vote_account.clone()), - (rewards_address, rewards_account.clone()), - (stake_history_address, stake_history_account), - ], - vec![ - AccountMeta { - pubkey: stake_address, - is_signer: false, - is_writable: true, - }, - AccountMeta { - pubkey: vote_address, - is_signer: false, - is_writable: false, - }, - AccountMeta { - pubkey: rewards_address, - is_signer: false, - is_writable: false, - }, - AccountMeta { - pubkey: stake_history_address, - is_signer: false, - is_writable: false, - }, - AccountMeta { - pubkey: stake_address, - is_signer: true, - is_writable: false, - }, - ], - Err(ProgramError::InvalidArgument), - ); - // Tests correct number of accounts are provided in withdraw process_instruction( &mollusk, @@ -648,29 +589,6 @@ fn test_stake_process_instruction_decode_bail() { Err(ProgramError::NotEnoughAccountKeys), ); - // Tests 2nd keyed account is of correct type (Clock instead of rewards) in deactivate - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Deactivate).unwrap(), - vec![ - (stake_address, stake_account.clone()), - (rewards_address, rewards_account), - ], - vec![ - AccountMeta { - pubkey: stake_address, - is_signer: false, - is_writable: true, - }, - AccountMeta { - pubkey: rewards_address, - is_signer: false, - is_writable: false, - }, - ], - Err(ProgramError::InvalidArgument), - ); - // Tests correct number of accounts are provided in deactivate process_instruction( &mollusk, @@ -3659,6 +3577,8 @@ fn test_split_minimum_stake_delegation() { is_writable: true, }, ]; + // NOTE with a 1lamp minimum delegation, cases 2 and 4 merely hit the zero split error + // these would be InsufficientDelegation if the minimum were 1sol for (source_delegation, split_amount, expected_result) in [ (minimum_delegation * 2, minimum_delegation, Ok(())), ( @@ -3669,7 +3589,7 @@ fn test_split_minimum_stake_delegation() { ( (minimum_delegation * 2) - 1, minimum_delegation, - Err(ProgramError::InsufficientFunds), + Err(StakeError::InsufficientDelegation.into()), ), ( (minimum_delegation - 1) * 2, @@ -4412,11 +4332,21 @@ fn test_split_source_uninitialized() { }, ]; + // splitting zero always fails + { + process_instruction( + &mollusk, + &serialize(&StakeInstruction::Split(0)).unwrap(), + transaction_accounts.clone(), + instruction_accounts.clone(), + Err(ProgramError::InsufficientFunds), + ); + } + // splitting an uninitialized account where the destination is the same as the source { // splitting should work when... // - when split amount is the full balance - // - when split amount is zero // - when split amount is non-zero and less than the full balance // // and splitting should fail when the split amount is greater than the balance @@ -4427,13 +4357,6 @@ fn test_split_source_uninitialized() { instruction_accounts.clone(), Ok(()), ); - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Split(0)).unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(),