From eaeeebd17fe2505425e383aaa1173bf1f1f8429e Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Tue, 26 Apr 2022 01:44:48 +0200 Subject: [PATCH 1/7] stake: Allow initialized stakes to be below the min delegation --- programs/stake/src/stake_instruction.rs | 254 +++++++++--------- programs/stake/src/stake_state.rs | 56 +++- sdk/program/src/instruction.rs | 4 + sdk/program/src/program_error.rs | 8 + sdk/src/feature_set.rs | 5 + storage-proto/proto/transaction_by_addr.proto | 1 + storage-proto/src/convert.rs | 4 + 7 files changed, 194 insertions(+), 138 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 1c157803884be6..df229e05cea969 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -187,6 +187,7 @@ pub fn process_instruction( &stake_history, &config, &signers, + &invoke_context.feature_set, ) } Ok(StakeInstruction::Split(lamports)) => { @@ -3247,30 +3248,16 @@ mod tests { ]; // should pass, withdrawing account down to minimum balance - process_instruction( - &serialize(&StakeInstruction::Withdraw( - stake_lamports - minimum_delegation, - )) - .unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); - - // should fail, withdrawing account down to only rent-exempt reserve process_instruction( &serialize(&StakeInstruction::Withdraw(stake_lamports)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), - Err(InstructionError::InsufficientFunds), + Ok(()), ); // should fail, withdrawal that would leave less than rent-exempt reserve process_instruction( - &serialize(&StakeInstruction::Withdraw( - stake_lamports + minimum_delegation, - )) - .unwrap(), + &serialize(&StakeInstruction::Withdraw(stake_lamports + 1)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), Err(InstructionError::InsufficientFunds), @@ -3676,8 +3663,9 @@ mod tests { } /// Ensure that `initialize()` respects the minimum delegation requirements - /// - Assert 1: accounts with a balance equal-to the minimum initialize OK - /// - Assert 2: accounts with a balance less-than the minimum do not initialize + /// - Assert 1: accounts with a balance equal-to the minimum delegation initialize OK + /// - Assert 2: accounts with a balance equal-to the rent exemption initialize OK + /// - Assert 3: accounts with a balance less-than the rent exemption do not initialize #[test] fn test_initialize_minimum_stake_delegation() { let feature_set = FeatureSet::all_enabled(); @@ -3702,18 +3690,15 @@ mod tests { is_writable: false, }, ]; - for (stake_delegation, expected_result) in [ - (minimum_delegation, Ok(())), + for (lamports, expected_result) in [ + (minimum_delegation + rent_exempt_reserve, Ok(())), + (rent_exempt_reserve, Ok(())), ( - minimum_delegation - 1, + rent_exempt_reserve - 1, Err(InstructionError::InsufficientFunds), ), ] { - let stake_account = AccountSharedData::new( - stake_delegation + rent_exempt_reserve, - StakeState::size_of(), - &id(), - ); + let stake_account = AccountSharedData::new(lamports, StakeState::size_of(), &id()); process_instruction( &instruction_data, vec![ @@ -3783,7 +3768,10 @@ mod tests { ]; for (stake_delegation, expected_result) in [ (minimum_delegation, Ok(())), - (minimum_delegation - 1, Ok(())), + ( + minimum_delegation - 1, + Err(InstructionError::InsufficientStakeDelegation), + ), ] { for stake_state in &[ StakeState::Initialized(meta), @@ -3862,32 +3850,38 @@ mod tests { is_writable: false, }, ]; - for (source_stake_delegation, dest_stake_delegation, expected_result) in [ - (minimum_delegation, minimum_delegation, Ok(())), + for (source_reserve, dest_reserve, expected_result) in [ + (rent_exempt_reserve, rent_exempt_reserve, Ok(())), ( - minimum_delegation, - minimum_delegation - 1, + rent_exempt_reserve, + rent_exempt_reserve - 1, Err(InstructionError::InsufficientFunds), ), ( - minimum_delegation - 1, - minimum_delegation, + rent_exempt_reserve - 1, + rent_exempt_reserve, Err(InstructionError::InsufficientFunds), ), ( - minimum_delegation - 1, - minimum_delegation - 1, + rent_exempt_reserve - 1, + rent_exempt_reserve - 1, Err(InstructionError::InsufficientFunds), ), ] { // The source account's starting balance is equal to *both* the source and dest // accounts' *final* balance - let source_starting_balance = - source_stake_delegation + dest_stake_delegation + rent_exempt_reserve * 2; - for source_stake_state in &[ - StakeState::Initialized(source_meta), - just_stake(source_meta, source_starting_balance - rent_exempt_reserve), + let mut source_starting_balance = source_reserve + dest_reserve; + for (delegation, source_stake_state) in &[ + (0, StakeState::Initialized(source_meta)), + ( + minimum_delegation, + just_stake( + source_meta, + minimum_delegation * 2 + source_starting_balance - rent_exempt_reserve, + ), + ), ] { + source_starting_balance += delegation * 2; let source_account = AccountSharedData::new_data_with_space( source_starting_balance, source_stake_state, @@ -3896,10 +3890,7 @@ mod tests { ) .unwrap(); process_instruction( - &serialize(&StakeInstruction::Split( - dest_stake_delegation + rent_exempt_reserve, - )) - .unwrap(), + &serialize(&StakeInstruction::Split(dest_reserve + delegation)).unwrap(), vec![ (source_address, source_account), (dest_address, dest_account.clone()), @@ -3953,19 +3944,22 @@ mod tests { is_writable: false, }, ]; - for (stake_delegation, expected_result) in [ - (minimum_delegation, Ok(())), + for (reserve, expected_result) in [ + (rent_exempt_reserve, Ok(())), ( - minimum_delegation - 1, + rent_exempt_reserve - 1, Err(InstructionError::InsufficientFunds), ), ] { - for source_stake_state in &[ - StakeState::Initialized(source_meta), - just_stake(source_meta, stake_delegation), + for (stake_delegation, source_stake_state) in &[ + (0, StakeState::Initialized(source_meta)), + ( + minimum_delegation, + just_stake(source_meta, minimum_delegation), + ), ] { let source_account = AccountSharedData::new_data_with_space( - stake_delegation + rent_exempt_reserve, + stake_delegation + reserve, source_stake_state, StakeState::size_of(), &id(), @@ -3993,7 +3987,6 @@ mod tests { #[test] fn test_split_destination_minimum_stake_delegation() { let feature_set = FeatureSet::all_enabled(); - let minimum_delegation = crate::get_minimum_delegation(&feature_set); let rent = Rent::default(); let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); let source_address = Pubkey::new_unique(); @@ -4014,68 +4007,71 @@ mod tests { is_writable: false, }, ]; - for (destination_starting_balance, split_amount, expected_result) in [ - // split amount must be non zero + let source_balance = u64::MAX; + let source_stake_delegation = source_balance - rent_exempt_reserve; + for (minimum_delegation, source_stake_state) in &[ + (0, StakeState::Initialized(source_meta)), ( - rent_exempt_reserve + minimum_delegation, - 0, - Err(InstructionError::InsufficientFunds), - ), - // any split amount is OK when destination account is already fully funded - (rent_exempt_reserve + minimum_delegation, 1, Ok(())), - // if destination is only short by 1 lamport, then split amount can be 1 lamport - (rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())), - // destination short by 2 lamports, so 1 isn't enough (non-zero split amount) - ( - rent_exempt_reserve + minimum_delegation - 2, - 1, - Err(InstructionError::InsufficientFunds), - ), - // destination is rent exempt, so split enough for minimum delegation - (rent_exempt_reserve, minimum_delegation, Ok(())), - // destination is rent exempt, but split amount less than minimum delegation - ( - rent_exempt_reserve, - minimum_delegation - 1, - Err(InstructionError::InsufficientFunds), - ), - // destination is not rent exempt, so split enough for rent and minimum delegation - (rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())), - // destination is not rent exempt, but split amount only for minimum delegation - ( - rent_exempt_reserve - 1, - minimum_delegation, - Err(InstructionError::InsufficientFunds), - ), - // destination has smallest non-zero balance, so can split the minimum balance - // requirements minus what destination already has - (1, rent_exempt_reserve + minimum_delegation - 1, Ok(())), - // destination has smallest non-zero balance, but cannot split less than the minimum - // balance requirements minus what destination already has - ( - 1, - rent_exempt_reserve + minimum_delegation - 2, - Err(InstructionError::InsufficientFunds), - ), - // destination has zero lamports, so split must be at least rent exempt reserve plus - // minimum delegation - (0, rent_exempt_reserve + minimum_delegation, Ok(())), - // destination has zero lamports, but split amount is less than rent exempt reserve - // plus minimum delegation - ( - 0, - rent_exempt_reserve + minimum_delegation - 1, - Err(InstructionError::InsufficientFunds), + crate::get_minimum_delegation(&feature_set), + just_stake(source_meta, source_stake_delegation), ), ] { - // Set the source's starting balance and stake delegation amount to something large - // to ensure its post-split balance meets all the requirements - let source_balance = u64::MAX; - let source_stake_delegation = source_balance - rent_exempt_reserve; - for source_stake_state in &[ - StakeState::Initialized(source_meta), - just_stake(source_meta, source_stake_delegation), + for (destination_starting_balance, split_amount, expected_result) in [ + // split amount must be non zero + ( + rent_exempt_reserve + minimum_delegation, + 0, + Err(InstructionError::InsufficientFunds), + ), + // any split amount is OK when destination account is already fully funded + (rent_exempt_reserve + minimum_delegation, 1, Ok(())), + // if destination is only short by 1 lamport, then split amount can be 1 lamport + (rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())), + // destination short by 2 lamports, so 1 isn't enough (non-zero split amount) + ( + rent_exempt_reserve + minimum_delegation - 2, + 1, + Err(InstructionError::InsufficientFunds), + ), + // destination is rent exempt, so split enough for minimum delegation + (rent_exempt_reserve, *minimum_delegation, Ok(())), + // destination is rent exempt, but split amount less than minimum delegation + ( + rent_exempt_reserve, + minimum_delegation.saturating_sub(1), // when minimum is 0, this blows up! + Err(InstructionError::InsufficientFunds), + ), + // destination is not rent exempt, so split enough for rent and minimum delegation + (rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())), + // destination is not rent exempt, but split amount only for minimum delegation + ( + rent_exempt_reserve - 1, + *minimum_delegation, + Err(InstructionError::InsufficientFunds), + ), + // destination has smallest non-zero balance, so can split the minimum balance + // requirements minus what destination already has + (1, rent_exempt_reserve + minimum_delegation - 1, Ok(())), + // destination has smallest non-zero balance, but cannot split less than the minimum + // balance requirements minus what destination already has + ( + 1, + rent_exempt_reserve + minimum_delegation - 2, + Err(InstructionError::InsufficientFunds), + ), + // destination has zero lamports, so split must be at least rent exempt reserve plus + // minimum delegation + (0, rent_exempt_reserve + minimum_delegation, Ok(())), + // destination has zero lamports, but split amount is less than rent exempt reserve + // plus minimum delegation + ( + 0, + rent_exempt_reserve + minimum_delegation - 1, + Err(InstructionError::InsufficientFunds), + ), ] { + // Set the source's starting balance and stake delegation amount to something large + // to ensure its post-split balance meets all the requirements let source_account = AccountSharedData::new_data_with_space( source_balance, &source_stake_state, @@ -4090,6 +4086,14 @@ mod tests { &id(), ) .unwrap(); + + // some of these tests don't make perfect sense for both Initialized and + // Stake, so override the expected result if it's incorrect + let expected_result = if split_amount == 0 { + Err(InstructionError::InsufficientFunds) + } else { + expected_result + }; let accounts = process_instruction( &serialize(&StakeInstruction::Split(split_amount)).unwrap(), vec![ @@ -4122,7 +4126,7 @@ mod tests { expected_destination_stake_delegation, destination_stake.delegation.stake ); - assert!(destination_stake.delegation.stake >= minimum_delegation,); + assert!(destination_stake.delegation.stake >= *minimum_delegation,); } else { panic!("destination state must be StakeStake::Stake after successful split when source is also StakeState::Stake!"); } @@ -4182,13 +4186,13 @@ mod tests { Err(InstructionError::InsufficientFunds), ), ] { - for stake_state in &[ - StakeState::Initialized(meta), - just_stake(meta, starting_stake_delegation), + for (stake_delegation, stake_state) in &[ + (0, StakeState::Initialized(meta)), + (minimum_delegation, just_stake(meta, minimum_delegation)), ] { let rewards_balance = 123; let stake_account = AccountSharedData::new_data_with_space( - starting_stake_delegation + rent_exempt_reserve + rewards_balance, + stake_delegation + rent_exempt_reserve + rewards_balance, stake_state, StakeState::size_of(), &id(), @@ -4616,8 +4620,6 @@ mod tests { let rent = Rent::default(); let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); let minimum_delegation = crate::get_minimum_delegation(&FeatureSet::all_enabled()); - let minimum_balance = rent_exempt_reserve + minimum_delegation; - let stake_lamports = minimum_balance * 2; let stake_address = solana_sdk::pubkey::new_rand(); let split_to_address = solana_sdk::pubkey::new_rand(); let split_to_account = AccountSharedData::new_data_with_space( @@ -4646,10 +4648,14 @@ mod tests { }; // test splitting both an Initialized stake and a Staked stake - for state in &[ - StakeState::Initialized(meta), - just_stake(meta, stake_lamports - rent_exempt_reserve), + for (minimum_balance, state) in &[ + (rent_exempt_reserve, StakeState::Initialized(meta)), + ( + rent_exempt_reserve + minimum_delegation, + just_stake(meta, minimum_delegation * 2 + rent_exempt_reserve), + ), ] { + let stake_lamports = minimum_balance * 2; let stake_account = AccountSharedData::new_data_with_space( stake_lamports, state, @@ -4668,7 +4674,7 @@ mod tests { // not enough to make a non-zero stake account process_instruction( - &serialize(&StakeInstruction::Split(rent_exempt_reserve)).unwrap(), + &serialize(&StakeInstruction::Split(minimum_balance - 1)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), Err(InstructionError::InsufficientFunds), @@ -4677,7 +4683,7 @@ mod tests { // doesn't leave enough for initial stake to be non-zero process_instruction( &serialize(&StakeInstruction::Split( - stake_lamports - rent_exempt_reserve, + stake_lamports - minimum_balance + 1, )) .unwrap(), transaction_accounts.clone(), @@ -4686,7 +4692,7 @@ mod tests { ); // split account already has way enough lamports - transaction_accounts[1].1.set_lamports(minimum_balance); + transaction_accounts[1].1.set_lamports(*minimum_balance); let accounts = process_instruction( &serialize(&StakeInstruction::Split(stake_lamports - minimum_balance)).unwrap(), transaction_accounts, @@ -4709,7 +4715,7 @@ mod tests { } )) ); - assert_eq!(accounts[0].lamports(), minimum_balance,); + assert_eq!(accounts[0].lamports(), *minimum_balance,); assert_eq!(accounts[1].lamports(), stake_lamports,); } } diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index e1bb45385c1834..ea0447d4930086 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -15,7 +15,8 @@ use { account_utils::StateMut, clock::{Clock, Epoch}, feature_set::{ - stake_merge_with_unmatched_credits_observed, stake_split_uses_rent_sysvar, FeatureSet, + stake_allow_zero_undelegated_amount, stake_merge_with_unmatched_credits_observed, + stake_split_uses_rent_sysvar, FeatureSet, }, instruction::{checked_add, InstructionError}, pubkey::Pubkey, @@ -453,8 +454,13 @@ pub fn initialize( } if let StakeState::Uninitialized = stake_account.get_state()? { let rent_exempt_reserve = rent.minimum_balance(stake_account.get_data().len()); - let minimum_delegation = crate::get_minimum_delegation(feature_set); - let minimum_balance = rent_exempt_reserve + minimum_delegation; + // when removing this feature, remove `minimum_balance` and just use `rent_exempt_reserve` + let minimum_balance = if feature_set.is_active(&stake_allow_zero_undelegated_amount::id()) { + rent_exempt_reserve + } else { + let minimum_delegation = crate::get_minimum_delegation(feature_set); + rent_exempt_reserve + minimum_delegation + }; if stake_account.get_lamports() >= minimum_balance { stake_account.set_state(&StakeState::Initialized(Meta { @@ -558,6 +564,7 @@ pub fn delegate( stake_history: &StakeHistory, config: &Config, signers: &HashSet, + feature_set: &FeatureSet, ) -> Result<(), InstructionError> { let vote_account = instruction_context.try_borrow_account(transaction_context, vote_account_index)?; @@ -574,7 +581,7 @@ pub fn delegate( StakeState::Initialized(meta) => { meta.authorized.check(signers, StakeAuthorize::Staker)?; let ValidatedDelegatedInfo { stake_amount } = - validate_delegated_amount(&stake_account, &meta)?; + validate_delegated_amount(&stake_account, &meta, feature_set)?; let stake = new_stake( stake_amount, &vote_pubkey, @@ -587,7 +594,7 @@ pub fn delegate( StakeState::Stake(meta, mut stake) => { meta.authorized.check(signers, StakeAuthorize::Staker)?; let ValidatedDelegatedInfo { stake_amount } = - validate_delegated_amount(&stake_account, &meta)?; + validate_delegated_amount(&stake_account, &meta, feature_set)?; redelegate( &mut stake, stake_amount, @@ -669,6 +676,7 @@ pub fn split( match stake_state { StakeState::Stake(meta, mut stake) => { meta.authorized.check(signers, StakeAuthorize::Staker)?; + let minimum_delegation = crate::get_minimum_delegation(&invoke_context.feature_set); let validated_split_info = validate_split_amount( invoke_context, transaction_context, @@ -678,6 +686,7 @@ pub fn split( lamports, &meta, Some(&stake), + minimum_delegation, )?; // split the stake, subtract rent_exempt_balance unless @@ -725,6 +734,14 @@ pub fn split( } StakeState::Initialized(meta) => { meta.authorized.check(signers, StakeAuthorize::Staker)?; + let additional_required_lamports = if invoke_context + .feature_set + .is_active(&stake_allow_zero_undelegated_amount::id()) + { + 0 + } else { + crate::get_minimum_delegation(&invoke_context.feature_set) + }; let validated_split_info = validate_split_amount( invoke_context, transaction_context, @@ -734,6 +751,7 @@ pub fn split( lamports, &meta, None, + additional_required_lamports, )?; let mut split_meta = meta; split_meta.rent_exempt_reserve = validated_split_info.destination_rent_exempt_reserve; @@ -877,11 +895,15 @@ pub fn withdraw( StakeState::Initialized(meta) => { meta.authorized .check(&signers, StakeAuthorize::Withdrawer)?; - // stake accounts must have a balance >= rent_exempt_reserve + minimum_stake_delegation - let reserve = checked_add( - meta.rent_exempt_reserve, - crate::get_minimum_delegation(feature_set), - )?; + // stake accounts must have a balance >= rent_exempt_reserve + let reserve = if feature_set.is_active(&stake_allow_zero_undelegated_amount::id()) { + meta.rent_exempt_reserve + } else { + checked_add( + meta.rent_exempt_reserve, + crate::get_minimum_delegation(feature_set), + )? + }; (meta.lockup, reserve, false) } @@ -1000,10 +1022,16 @@ struct ValidatedDelegatedInfo { fn validate_delegated_amount( account: &BorrowedAccount, meta: &Meta, + feature_set: &FeatureSet, ) -> Result { let stake_amount = account .get_lamports() .saturating_sub(meta.rent_exempt_reserve); // can't stake the rent + if feature_set.is_active(&stake_allow_zero_undelegated_amount::id()) + && stake_amount < crate::get_minimum_delegation(feature_set) + { + return Err(InstructionError::InsufficientStakeDelegation); + } Ok(ValidatedDelegatedInfo { stake_amount }) } @@ -1028,6 +1056,7 @@ fn validate_split_amount( lamports: u64, source_meta: &Meta, source_stake: Option<&Stake>, + additional_required_lamports: u64, ) -> Result { let source_account = instruction_context.try_borrow_account(transaction_context, source_account_index)?; @@ -1054,10 +1083,9 @@ fn validate_split_amount( // 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 minimum_delegation = crate::get_minimum_delegation(&invoke_context.feature_set); let source_minimum_balance = source_meta .rent_exempt_reserve - .saturating_add(minimum_delegation); + .saturating_add(additional_required_lamports); let source_remaining_balance = source_lamports.saturating_sub(lamports); if source_remaining_balance == 0 { // full amount is a withdrawal @@ -1088,7 +1116,7 @@ fn validate_split_amount( ) }; let destination_minimum_balance = - destination_rent_exempt_reserve.saturating_add(minimum_delegation); + destination_rent_exempt_reserve.saturating_add(additional_required_lamports); let destination_balance_deficit = destination_minimum_balance.saturating_sub(destination_lamports); if lamports < destination_balance_deficit { @@ -1105,7 +1133,7 @@ fn validate_split_amount( // account, the split amount must be at least the minimum stake delegation. So if the minimum // stake delegation was 10 lamports, then a split amount of 1 lamport would not meet the // *delegation* requirements. - if source_stake.is_some() && lamports < minimum_delegation { + if source_stake.is_some() && lamports < additional_required_lamports { return Err(InstructionError::InsufficientFunds); } diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index 0de8f9337fcf2e..5a8559231ce447 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -256,6 +256,10 @@ pub enum InstructionError { /// Active vote account close #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, + + // Insufficient stake delegation + #[error("Stake amount is below the minimum delegation requirements")] + InsufficientStakeDelegation, // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added } diff --git a/sdk/program/src/program_error.rs b/sdk/program/src/program_error.rs index d0e7e7643b4c07..32069ad93093f2 100644 --- a/sdk/program/src/program_error.rs +++ b/sdk/program/src/program_error.rs @@ -55,6 +55,8 @@ pub enum ProgramError { MaxAccountsDataSizeExceeded, #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, + #[error("Stake amount is below the minimum delegation requirements")] + InsufficientStakeDelegation, } pub trait PrintProgramError { @@ -95,6 +97,7 @@ impl PrintProgramError for ProgramError { Self::IllegalOwner => msg!("Error: IllegalOwner"), Self::MaxAccountsDataSizeExceeded => msg!("Error: MaxAccountsDataSizeExceeded"), Self::ActiveVoteAccountClose => msg!("Error: ActiveVoteAccountClose"), + Self::InsufficientStakeDelegation => msg!("Error: InsufficientStakeDelegation"), } } } @@ -127,6 +130,7 @@ pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17); pub const ILLEGAL_OWNER: u64 = to_builtin!(18); pub const MAX_ACCOUNTS_DATA_SIZE_EXCEEDED: u64 = to_builtin!(19); pub const ACTIVE_VOTE_ACCOUNT_CLOSE: u64 = to_builtin!(20); +pub const INSUFFICIENT_STAKE_DELEGATION: u64 = to_builtin!(21); // Warning: Any new program errors added here must also be: // - Added to the below conversions // - Added as an equivilent to InstructionError @@ -155,6 +159,7 @@ impl From for u64 { ProgramError::IllegalOwner => ILLEGAL_OWNER, ProgramError::MaxAccountsDataSizeExceeded => MAX_ACCOUNTS_DATA_SIZE_EXCEEDED, ProgramError::ActiveVoteAccountClose => ACTIVE_VOTE_ACCOUNT_CLOSE, + ProgramError::InsufficientStakeDelegation => INSUFFICIENT_STAKE_DELEGATION, ProgramError::Custom(error) => { if error == 0 { CUSTOM_ZERO @@ -189,6 +194,7 @@ impl From for ProgramError { ILLEGAL_OWNER => Self::IllegalOwner, MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded, ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, + INSUFFICIENT_STAKE_DELEGATION => Self::InsufficientStakeDelegation, _ => Self::Custom(error as u32), } } @@ -219,6 +225,7 @@ impl TryFrom for ProgramError { Self::Error::IllegalOwner => Ok(Self::IllegalOwner), Self::Error::MaxAccountsDataSizeExceeded => Ok(Self::MaxAccountsDataSizeExceeded), Self::Error::ActiveVoteAccountClose => Ok(Self::ActiveVoteAccountClose), + Self::Error::InsufficientStakeDelegation => Ok(Self::InsufficientStakeDelegation), _ => Err(error), } } @@ -251,6 +258,7 @@ where ILLEGAL_OWNER => Self::IllegalOwner, MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded, ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, + INSUFFICIENT_STAKE_DELEGATION => Self::InsufficientStakeDelegation, _ => { // A valid custom error has no bits set in the upper 32 if error >> BUILTIN_BIT_SHIFT == 0 { diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 3444b463753984..bc1619bc82fd04 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -379,6 +379,10 @@ pub mod default_units_per_instruction { solana_sdk::declare_id!("J2QdYx8crLbTVK8nur1jeLsmc3krDbfjoxoea2V1Uy5Q"); } +pub mod stake_allow_zero_undelegated_amount { + solana_sdk::declare_id!("sTKz343FM8mqtyGvYWvbLpTThw3ixRM4Xk8QvZ985mw"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -468,6 +472,7 @@ lazy_static! { (spl_token_v3_4_0::id(), "SPL Token Program version 3.4.0 release #24740"), (spl_associated_token_account_v1_1_0::id(), "SPL Associated Token Account Program version 1.1.0 release #24741"), (default_units_per_instruction::id(), "Default max tx-wide compute units calculated per instruction"), + (stake_allow_zero_undelegated_amount::id(), "Allow zero-lamport undelegated amount for initialized stakes") // TODO JC /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index f4896906864efe..0211042b1bf7a7 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -115,6 +115,7 @@ enum InstructionErrorType { ILLEGAL_OWNER = 49; MAX_ACCOUNTS_DATA_SIZE_EXCEEDED = 50; ACTIVE_VOTE_ACCOUNT_CLOSE = 51; + INSUFFICIENT_STAKE_DELEGATION = 52; } message UnixTimestamp { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 997898ea6062f7..8b6fcfa7ab9bad 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -706,6 +706,7 @@ impl TryFrom for TransactionError { 49 => InstructionError::IllegalOwner, 50 => InstructionError::MaxAccountsDataSizeExceeded, 51 => InstructionError::ActiveVoteAccountClose, + 52 => InstructionError::InsufficientStakeDelegation, _ => return Err("Invalid InstructionError"), }; @@ -1003,6 +1004,9 @@ impl From for tx_by_addr::TransactionError { InstructionError::ActiveVoteAccountClose => { tx_by_addr::InstructionErrorType::ActiveVoteAccountClose } + InstructionError::InsufficientStakeDelegation => { + tx_by_addr::InstructionErrorType::InsufficientStakeDelegation + } } as i32, custom: match instruction_error { InstructionError::Custom(custom) => { From 10c241b1aa0dc0111585bf24d141185eb6a0cd94 Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Tue, 26 Apr 2022 22:29:54 +0200 Subject: [PATCH 2/7] Add PR number in feature --- sdk/src/feature_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index bc1619bc82fd04..04187b29c58f23 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -472,7 +472,7 @@ lazy_static! { (spl_token_v3_4_0::id(), "SPL Token Program version 3.4.0 release #24740"), (spl_associated_token_account_v1_1_0::id(), "SPL Associated Token Account Program version 1.1.0 release #24741"), (default_units_per_instruction::id(), "Default max tx-wide compute units calculated per instruction"), - (stake_allow_zero_undelegated_amount::id(), "Allow zero-lamport undelegated amount for initialized stakes") // TODO JC + (stake_allow_zero_undelegated_amount::id(), "Allow zero-lamport undelegated amount for initialized stakes #24670") /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From c1981211b9acf0642e14e37bb252269a13b5e4c4 Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Wed, 27 Apr 2022 15:37:45 +0200 Subject: [PATCH 3/7] Fixup RPC subscription test --- rpc/src/rpc_pubsub.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/rpc/src/rpc_pubsub.rs b/rpc/src/rpc_pubsub.rs index 0e81e2e12b8b8f..a48900108c8b59 100644 --- a/rpc/src/rpc_pubsub.rs +++ b/rpc/src/rpc_pubsub.rs @@ -592,8 +592,8 @@ mod tests { bank_forks::BankForks, commitment::{BlockCommitmentCache, CommitmentSlots}, genesis_utils::{ - create_genesis_config, create_genesis_config_with_vote_accounts, GenesisConfigInfo, - ValidatorVoteKeypairs, + activate_all_features, create_genesis_config, + create_genesis_config_with_vote_accounts, GenesisConfigInfo, ValidatorVoteKeypairs, }, vote_transaction::VoteTransaction, }, @@ -604,6 +604,7 @@ mod tests { hash::Hash, message::Message, pubkey::Pubkey, + rent::Rent, signature::{Keypair, Signer}, stake::{ self, instruction as stake_instruction, @@ -825,10 +826,12 @@ mod tests { #[serial] fn test_account_subscribe() { let GenesisConfigInfo { - genesis_config, + mut genesis_config, mint_keypair: alice, .. } = create_genesis_config(10_000_000_000); + genesis_config.rent = Rent::default(); + activate_all_features(&mut genesis_config); let new_stake_authority = solana_sdk::pubkey::new_rand(); let stake_authority = Keypair::new(); @@ -875,10 +878,7 @@ mod tests { let balance = { let bank = bank_forks.read().unwrap().working_bank(); let rent = &bank.rent_collector().rent; - let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); - let minimum_delegation = - solana_stake_program::get_minimum_delegation(&bank.feature_set); - rent_exempt_reserve + minimum_delegation + rent.minimum_balance(StakeState::size_of()) }; let tx = system_transaction::transfer(&alice, &from.pubkey(), balance, blockhash); @@ -928,7 +928,13 @@ mod tests { serde_json::from_str::(&response).unwrap(), ); - let tx = system_transaction::transfer(&alice, &stake_authority.pubkey(), 1, blockhash); + let balance = { + let bank = bank_forks.read().unwrap().working_bank(); + let rent = &bank.rent_collector().rent; + rent.minimum_balance(0) + }; + let tx = + system_transaction::transfer(&alice, &stake_authority.pubkey(), balance, blockhash); process_transaction_and_notify(&bank_forks, &tx, &rpc_subscriptions, 1).unwrap(); sleep(Duration::from_millis(200)); let ix = stake_instruction::authorize( From 07f3cc0d339ead2cc51eb0e9251b60110210e8c2 Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Thu, 28 Apr 2022 18:20:38 +0200 Subject: [PATCH 4/7] Address feedback pt 1 --- programs/stake/src/stake_instruction.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index df229e05cea969..3813ffbba02579 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -3663,11 +3663,10 @@ mod tests { } /// Ensure that `initialize()` respects the minimum delegation requirements - /// - Assert 1: accounts with a balance equal-to the minimum delegation initialize OK - /// - Assert 2: accounts with a balance equal-to the rent exemption initialize OK - /// - Assert 3: accounts with a balance less-than the rent exemption do not initialize + /// - Assert 1: accounts with a balance equal-to the rent exemption initialize OK + /// - Assert 2: accounts with a balance less-than the rent exemption do not initialize #[test] - fn test_initialize_minimum_stake_delegation() { + fn test_initialize_minimum_balance() { let feature_set = FeatureSet::all_enabled(); let minimum_delegation = crate::get_minimum_delegation(&feature_set); let rent = Rent::default(); @@ -3691,7 +3690,6 @@ mod tests { }, ]; for (lamports, expected_result) in [ - (minimum_delegation + rent_exempt_reserve, Ok(())), (rent_exempt_reserve, Ok(())), ( rent_exempt_reserve - 1, @@ -3716,7 +3714,7 @@ mod tests { /// Ensure that `delegate()` respects the minimum delegation requirements /// - Assert 1: delegating an amount equal-to the minimum delegates OK - /// - Assert 2: delegating an amount less-than the minimum delegates OK + /// - Assert 2: delegating an amount less-than the minimum fails /// Also test both asserts above over both StakeState::{Initialized and Stake}, since the logic /// is slightly different for the variants. /// From 6a642c0cba01565e81c754fd2ffa6f7b214cdd00 Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Thu, 28 Apr 2022 19:48:41 +0200 Subject: [PATCH 5/7] Address feedback pt 2 --- Cargo.lock | 1 + programs/stake/Cargo.toml | 1 + programs/stake/src/stake_instruction.rs | 328 +++++++++++++++--------- 3 files changed, 207 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91d96498542e24..d1e4ffece7f581 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5718,6 +5718,7 @@ dependencies = [ name = "solana-stake-program" version = "1.11.0" dependencies = [ + "assert_matches", "bincode", "log", "num-derive", diff --git a/programs/stake/Cargo.toml b/programs/stake/Cargo.toml index e94ceaea1e8c03..31be25aca9936a 100644 --- a/programs/stake/Cargo.toml +++ b/programs/stake/Cargo.toml @@ -26,6 +26,7 @@ solana-vote-program = { path = "../vote", version = "=1.11.0" } thiserror = "1.0" [dev-dependencies] +assert_matches = "1.5.0" proptest = "1.0" solana-logger = { path = "../../logger", version = "=1.11.0" } diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 3813ffbba02579..9104f5b855d00e 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -457,6 +457,7 @@ mod tests { authorized_from, create_stake_history_from_delegations, from, new_stake, stake_from, Delegation, Meta, Stake, StakeState, }, + assert_matches::assert_matches, bincode::serialize, solana_program_runtime::{ invoke_context::mock_process_instruction, sysvar_cache::SysvarCache, @@ -3662,13 +3663,11 @@ mod tests { ); } - /// Ensure that `initialize()` respects the minimum delegation requirements + /// Ensure that `initialize()` respects the minimum balance requirements /// - Assert 1: accounts with a balance equal-to the rent exemption initialize OK /// - Assert 2: accounts with a balance less-than the rent exemption do not initialize #[test] fn test_initialize_minimum_balance() { - let feature_set = FeatureSet::all_enabled(); - let minimum_delegation = crate::get_minimum_delegation(&feature_set); let rent = Rent::default(); let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); let stake_address = solana_sdk::pubkey::new_rand(); @@ -3980,10 +3979,109 @@ mod tests { } } - /// Ensure that `split()` correctly handles prefunded destination accounts. When a destination - /// account already has funds, ensure the minimum split amount reduces accordingly. + /// Ensure that `split()` correctly handles prefunded destination accounts from + /// initialized stakes. When a destination account already has funds, ensure + /// the minimum split amount reduces accordingly. #[test] - fn test_split_destination_minimum_stake_delegation() { + fn test_initialized_split_destination_minimum_balance() { + let rent = Rent::default(); + let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); + let source_address = Pubkey::new_unique(); + let source_meta = Meta { + rent_exempt_reserve, + ..Meta::auto(&source_address) + }; + let dest_address = Pubkey::new_unique(); + let instruction_accounts = vec![ + AccountMeta { + pubkey: source_address, + is_signer: true, + is_writable: false, + }, + AccountMeta { + pubkey: dest_address, + is_signer: false, + is_writable: false, + }, + ]; + let source_balance = u64::MAX; + let source_stake_state = StakeState::Initialized(source_meta); + for (destination_starting_balance, split_amount, expected_result) in [ + // split amount must be non zero + ( + rent_exempt_reserve, + 0, + Err(InstructionError::InsufficientFunds), + ), + // any split amount is OK when destination account is already fully funded + (rent_exempt_reserve, 1, Ok(())), + // if destination is only short by 1 lamport, then split amount can be 1 lamport + (rent_exempt_reserve - 1, 1, Ok(())), + // destination short by 2 lamports, so 1 isn't enough (non-zero split amount) + ( + rent_exempt_reserve - 2, + 1, + Err(InstructionError::InsufficientFunds), + ), + // destination has smallest non-zero balance, so can split the minimum balance + // requirements minus what destination already has + (1, rent_exempt_reserve - 1, Ok(())), + // destination has smallest non-zero balance, but cannot split less than the minimum + // balance requirements minus what destination already has + ( + 1, + rent_exempt_reserve - 2, + Err(InstructionError::InsufficientFunds), + ), + // destination has zero lamports, so split must be at least rent exempt reserve plus + // minimum delegation + (0, rent_exempt_reserve, Ok(())), + // destination has zero lamports, but split amount is less than rent exempt reserve + // plus minimum delegation + ( + 0, + rent_exempt_reserve - 1, + Err(InstructionError::InsufficientFunds), + ), + ] { + // Set the source's starting balance and stake delegation amount to something large + // to ensure its post-split balance meets all the requirements + let source_account = AccountSharedData::new_data_with_space( + source_balance, + &source_stake_state, + StakeState::size_of(), + &id(), + ) + .unwrap(); + let dest_account = AccountSharedData::new_data_with_space( + destination_starting_balance, + &StakeState::Uninitialized, + StakeState::size_of(), + &id(), + ) + .unwrap(); + + process_instruction( + &serialize(&StakeInstruction::Split(split_amount)).unwrap(), + vec![ + (source_address, source_account), + (dest_address, dest_account), + ( + sysvar::rent::id(), + account::create_account_shared_data_for_test(&rent), + ), + ], + instruction_accounts.clone(), + expected_result.clone(), + ); + } + } + + /// Ensure that `split()` correctly handles prefunded destination accounts from + /// delegated stakes. When a destination account already has funds, ensure + /// the minimum split amount reduces accordingly. + #[test] + fn test_stake_split_destination_minimum_delegation() { let feature_set = FeatureSet::all_enabled(); let rent = Rent::default(); let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of()); @@ -4005,130 +4103,114 @@ mod tests { is_writable: false, }, ]; + // Set the source's starting balance and stake delegation amount to + // something large to ensure its post-split balance meets all the requirements let source_balance = u64::MAX; let source_stake_delegation = source_balance - rent_exempt_reserve; - for (minimum_delegation, source_stake_state) in &[ - (0, StakeState::Initialized(source_meta)), + let minimum_delegation = crate::get_minimum_delegation(&feature_set); + let source_stake_state = just_stake(source_meta, source_stake_delegation); + for (destination_starting_balance, split_amount, expected_result) in [ + // split amount must be non zero ( - crate::get_minimum_delegation(&feature_set), - just_stake(source_meta, source_stake_delegation), + rent_exempt_reserve + minimum_delegation, + 0, + Err(InstructionError::InsufficientFunds), + ), + // any split amount is OK when destination account is already fully funded + (rent_exempt_reserve + minimum_delegation, 1, Ok(())), + // if destination is only short by 1 lamport, then split amount can be 1 lamport + (rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())), + // destination short by 2 lamports, so 1 isn't enough (non-zero split amount) + ( + rent_exempt_reserve + minimum_delegation - 2, + 1, + Err(InstructionError::InsufficientFunds), + ), + // destination is rent exempt, so split enough for minimum delegation + (rent_exempt_reserve, minimum_delegation, Ok(())), + // destination is rent exempt, but split amount less than minimum delegation + ( + rent_exempt_reserve, + minimum_delegation.saturating_sub(1), // when minimum is 0, this blows up! + Err(InstructionError::InsufficientFunds), + ), + // destination is not rent exempt, so split enough for rent and minimum delegation + (rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())), + // destination is not rent exempt, but split amount only for minimum delegation + ( + rent_exempt_reserve - 1, + minimum_delegation, + Err(InstructionError::InsufficientFunds), + ), + // destination has smallest non-zero balance, so can split the minimum balance + // requirements minus what destination already has + (1, rent_exempt_reserve + minimum_delegation - 1, Ok(())), + // destination has smallest non-zero balance, but cannot split less than the minimum + // balance requirements minus what destination already has + ( + 1, + rent_exempt_reserve + minimum_delegation - 2, + Err(InstructionError::InsufficientFunds), + ), + // destination has zero lamports, so split must be at least rent exempt reserve plus + // minimum delegation + (0, rent_exempt_reserve + minimum_delegation, Ok(())), + // destination has zero lamports, but split amount is less than rent exempt reserve + // plus minimum delegation + ( + 0, + rent_exempt_reserve + minimum_delegation - 1, + Err(InstructionError::InsufficientFunds), ), ] { - for (destination_starting_balance, split_amount, expected_result) in [ - // split amount must be non zero - ( - rent_exempt_reserve + minimum_delegation, - 0, - Err(InstructionError::InsufficientFunds), - ), - // any split amount is OK when destination account is already fully funded - (rent_exempt_reserve + minimum_delegation, 1, Ok(())), - // if destination is only short by 1 lamport, then split amount can be 1 lamport - (rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())), - // destination short by 2 lamports, so 1 isn't enough (non-zero split amount) - ( - rent_exempt_reserve + minimum_delegation - 2, - 1, - Err(InstructionError::InsufficientFunds), - ), - // destination is rent exempt, so split enough for minimum delegation - (rent_exempt_reserve, *minimum_delegation, Ok(())), - // destination is rent exempt, but split amount less than minimum delegation - ( - rent_exempt_reserve, - minimum_delegation.saturating_sub(1), // when minimum is 0, this blows up! - Err(InstructionError::InsufficientFunds), - ), - // destination is not rent exempt, so split enough for rent and minimum delegation - (rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())), - // destination is not rent exempt, but split amount only for minimum delegation - ( - rent_exempt_reserve - 1, - *minimum_delegation, - Err(InstructionError::InsufficientFunds), - ), - // destination has smallest non-zero balance, so can split the minimum balance - // requirements minus what destination already has - (1, rent_exempt_reserve + minimum_delegation - 1, Ok(())), - // destination has smallest non-zero balance, but cannot split less than the minimum - // balance requirements minus what destination already has - ( - 1, - rent_exempt_reserve + minimum_delegation - 2, - Err(InstructionError::InsufficientFunds), - ), - // destination has zero lamports, so split must be at least rent exempt reserve plus - // minimum delegation - (0, rent_exempt_reserve + minimum_delegation, Ok(())), - // destination has zero lamports, but split amount is less than rent exempt reserve - // plus minimum delegation - ( - 0, - rent_exempt_reserve + minimum_delegation - 1, - Err(InstructionError::InsufficientFunds), - ), - ] { - // Set the source's starting balance and stake delegation amount to something large - // to ensure its post-split balance meets all the requirements - let source_account = AccountSharedData::new_data_with_space( - source_balance, - &source_stake_state, - StakeState::size_of(), - &id(), - ) - .unwrap(); - let dest_account = AccountSharedData::new_data_with_space( - destination_starting_balance, - &StakeState::Uninitialized, - StakeState::size_of(), - &id(), - ) - .unwrap(); + let source_account = AccountSharedData::new_data_with_space( + source_balance, + &source_stake_state, + StakeState::size_of(), + &id(), + ) + .unwrap(); + let dest_account = AccountSharedData::new_data_with_space( + destination_starting_balance, + &StakeState::Uninitialized, + StakeState::size_of(), + &id(), + ) + .unwrap(); - // some of these tests don't make perfect sense for both Initialized and - // Stake, so override the expected result if it's incorrect - let expected_result = if split_amount == 0 { - Err(InstructionError::InsufficientFunds) + let accounts = process_instruction( + &serialize(&StakeInstruction::Split(split_amount)).unwrap(), + vec![ + (source_address, source_account), + (dest_address, dest_account), + ( + sysvar::rent::id(), + account::create_account_shared_data_for_test(&rent), + ), + ], + instruction_accounts.clone(), + expected_result.clone(), + ); + // For the expected OK cases, when the source's StakeState is Stake, then the + // destination's StakeState *must* also end up as Stake as well. Additionally, + // check to ensure the destination's delegation amount is correct. If the + // destination is already rent exempt, then the destination's stake delegation + // *must* equal the split amount. Otherwise, the split amount must first be used to + // make the destination rent exempt, and then the leftover lamports are delegated. + if expected_result.is_ok() { + assert_matches!(accounts[0].state().unwrap(), StakeState::Stake(_, _)); + if let StakeState::Stake(_, destination_stake) = accounts[1].state().unwrap() { + let destination_initial_rent_deficit = + rent_exempt_reserve.saturating_sub(destination_starting_balance); + let expected_destination_stake_delegation = + split_amount - destination_initial_rent_deficit; + assert_eq!( + expected_destination_stake_delegation, + destination_stake.delegation.stake + ); + assert!(destination_stake.delegation.stake >= minimum_delegation,); } else { - expected_result - }; - let accounts = process_instruction( - &serialize(&StakeInstruction::Split(split_amount)).unwrap(), - vec![ - (source_address, source_account), - (dest_address, dest_account), - ( - sysvar::rent::id(), - account::create_account_shared_data_for_test(&rent), - ), - ], - instruction_accounts.clone(), - expected_result.clone(), - ); - // For the expected OK cases, when the source's StakeState is Stake, then the - // destination's StakeState *must* also end up as Stake as well. Additionally, - // check to ensure the destination's delegation amount is correct. If the - // destination is already rent exempt, then the destination's stake delegation - // *must* equal the split amount. Otherwise, the split amount must first be used to - // make the destination rent exempt, and then the leftover lamports are delegated. - if expected_result.is_ok() { - if let StakeState::Stake(_, _) = accounts[0].state().unwrap() { - if let StakeState::Stake(_, destination_stake) = - accounts[1].state().unwrap() - { - let destination_initial_rent_deficit = - rent_exempt_reserve.saturating_sub(destination_starting_balance); - let expected_destination_stake_delegation = - split_amount - destination_initial_rent_deficit; - assert_eq!( - expected_destination_stake_delegation, - destination_stake.delegation.stake - ); - assert!(destination_stake.delegation.stake >= *minimum_delegation,); - } else { - panic!("destination state must be StakeStake::Stake after successful split when source is also StakeState::Stake!"); - } - } + panic!("destination state must be StakeStake::Stake after successful split when source is also StakeState::Stake!"); } } } From 66e8ec2b3276e610578bfd92cbff811bdc905865 Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Thu, 28 Apr 2022 21:30:46 +0200 Subject: [PATCH 6/7] Update FrozenAbi Digest --- runtime/src/bank.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1ef3761d749e4b..fc311c96cae63e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -239,7 +239,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "BQcJmh4VRCiNNtqjKPyphs9ULFbSUKGGfx6hz9SWBtqU")] +#[frozen_abi(digest = "3JVSVs9JHV2ZZNVTz3R6qKiFsaBYqzgTuxYjFJ1NwPSY")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. From da3efde4c05c02350ea4d0670659bdac49e27dcc Mon Sep 17 00:00:00 2001 From: Jon Cinque Date: Wed, 4 May 2022 00:44:24 +0200 Subject: [PATCH 7/7] Address feedback: no new error type, more comments --- programs/stake/src/stake_instruction.rs | 9 +++------ programs/stake/src/stake_state.rs | 8 +++++++- runtime/src/bank.rs | 2 +- sdk/program/src/instruction.rs | 4 ---- sdk/program/src/program_error.rs | 8 -------- storage-proto/proto/transaction_by_addr.proto | 1 - storage-proto/src/convert.rs | 4 ---- 7 files changed, 11 insertions(+), 25 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 9104f5b855d00e..bac5875bc431f0 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -3248,7 +3248,7 @@ mod tests { }, ]; - // should pass, withdrawing account down to minimum balance + // should pass, withdrawing initialized account down to minimum balance process_instruction( &serialize(&StakeInstruction::Withdraw(stake_lamports)).unwrap(), transaction_accounts.clone(), @@ -3765,10 +3765,7 @@ mod tests { ]; for (stake_delegation, expected_result) in [ (minimum_delegation, Ok(())), - ( - minimum_delegation - 1, - Err(InstructionError::InsufficientStakeDelegation), - ), + (minimum_delegation - 1, Err(StakeError::InsufficientStake)), ] { for stake_state in &[ StakeState::Initialized(meta), @@ -3800,7 +3797,7 @@ mod tests { ), ], instruction_accounts.clone(), - expected_result.clone(), + expected_result.clone().map_err(|e| e.into()), ); } } diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index ea0447d4930086..936110ff49136c 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -1027,10 +1027,16 @@ fn validate_delegated_amount( let stake_amount = account .get_lamports() .saturating_sub(meta.rent_exempt_reserve); // can't stake the rent + + // Previously, `initialize` checked that the stake account balance met + // the minimum delegation amount. + // With the `stake_allow_zero_undelegated_amount` feature, stake accounts + // may be initialized with a lower balance, so check the minimum in this + // function, on delegation. if feature_set.is_active(&stake_allow_zero_undelegated_amount::id()) && stake_amount < crate::get_minimum_delegation(feature_set) { - return Err(InstructionError::InsufficientStakeDelegation); + return Err(StakeError::InsufficientStake.into()); } Ok(ValidatedDelegatedInfo { stake_amount }) } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index fc311c96cae63e..1ef3761d749e4b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -239,7 +239,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "3JVSVs9JHV2ZZNVTz3R6qKiFsaBYqzgTuxYjFJ1NwPSY")] +#[frozen_abi(digest = "BQcJmh4VRCiNNtqjKPyphs9ULFbSUKGGfx6hz9SWBtqU")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index 5a8559231ce447..0de8f9337fcf2e 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -256,10 +256,6 @@ pub enum InstructionError { /// Active vote account close #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, - - // Insufficient stake delegation - #[error("Stake amount is below the minimum delegation requirements")] - InsufficientStakeDelegation, // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added } diff --git a/sdk/program/src/program_error.rs b/sdk/program/src/program_error.rs index 32069ad93093f2..d0e7e7643b4c07 100644 --- a/sdk/program/src/program_error.rs +++ b/sdk/program/src/program_error.rs @@ -55,8 +55,6 @@ pub enum ProgramError { MaxAccountsDataSizeExceeded, #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, - #[error("Stake amount is below the minimum delegation requirements")] - InsufficientStakeDelegation, } pub trait PrintProgramError { @@ -97,7 +95,6 @@ impl PrintProgramError for ProgramError { Self::IllegalOwner => msg!("Error: IllegalOwner"), Self::MaxAccountsDataSizeExceeded => msg!("Error: MaxAccountsDataSizeExceeded"), Self::ActiveVoteAccountClose => msg!("Error: ActiveVoteAccountClose"), - Self::InsufficientStakeDelegation => msg!("Error: InsufficientStakeDelegation"), } } } @@ -130,7 +127,6 @@ pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17); pub const ILLEGAL_OWNER: u64 = to_builtin!(18); pub const MAX_ACCOUNTS_DATA_SIZE_EXCEEDED: u64 = to_builtin!(19); pub const ACTIVE_VOTE_ACCOUNT_CLOSE: u64 = to_builtin!(20); -pub const INSUFFICIENT_STAKE_DELEGATION: u64 = to_builtin!(21); // Warning: Any new program errors added here must also be: // - Added to the below conversions // - Added as an equivilent to InstructionError @@ -159,7 +155,6 @@ impl From for u64 { ProgramError::IllegalOwner => ILLEGAL_OWNER, ProgramError::MaxAccountsDataSizeExceeded => MAX_ACCOUNTS_DATA_SIZE_EXCEEDED, ProgramError::ActiveVoteAccountClose => ACTIVE_VOTE_ACCOUNT_CLOSE, - ProgramError::InsufficientStakeDelegation => INSUFFICIENT_STAKE_DELEGATION, ProgramError::Custom(error) => { if error == 0 { CUSTOM_ZERO @@ -194,7 +189,6 @@ impl From for ProgramError { ILLEGAL_OWNER => Self::IllegalOwner, MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded, ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, - INSUFFICIENT_STAKE_DELEGATION => Self::InsufficientStakeDelegation, _ => Self::Custom(error as u32), } } @@ -225,7 +219,6 @@ impl TryFrom for ProgramError { Self::Error::IllegalOwner => Ok(Self::IllegalOwner), Self::Error::MaxAccountsDataSizeExceeded => Ok(Self::MaxAccountsDataSizeExceeded), Self::Error::ActiveVoteAccountClose => Ok(Self::ActiveVoteAccountClose), - Self::Error::InsufficientStakeDelegation => Ok(Self::InsufficientStakeDelegation), _ => Err(error), } } @@ -258,7 +251,6 @@ where ILLEGAL_OWNER => Self::IllegalOwner, MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded, ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, - INSUFFICIENT_STAKE_DELEGATION => Self::InsufficientStakeDelegation, _ => { // A valid custom error has no bits set in the upper 32 if error >> BUILTIN_BIT_SHIFT == 0 { diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index 0211042b1bf7a7..f4896906864efe 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -115,7 +115,6 @@ enum InstructionErrorType { ILLEGAL_OWNER = 49; MAX_ACCOUNTS_DATA_SIZE_EXCEEDED = 50; ACTIVE_VOTE_ACCOUNT_CLOSE = 51; - INSUFFICIENT_STAKE_DELEGATION = 52; } message UnixTimestamp { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 8b6fcfa7ab9bad..997898ea6062f7 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -706,7 +706,6 @@ impl TryFrom for TransactionError { 49 => InstructionError::IllegalOwner, 50 => InstructionError::MaxAccountsDataSizeExceeded, 51 => InstructionError::ActiveVoteAccountClose, - 52 => InstructionError::InsufficientStakeDelegation, _ => return Err("Invalid InstructionError"), }; @@ -1004,9 +1003,6 @@ impl From for tx_by_addr::TransactionError { InstructionError::ActiveVoteAccountClose => { tx_by_addr::InstructionErrorType::ActiveVoteAccountClose } - InstructionError::InsufficientStakeDelegation => { - tx_by_addr::InstructionErrorType::InsufficientStakeDelegation - } } as i32, custom: match instruction_error { InstructionError::Custom(custom) => {