From 53e7779cd871827179304a4e45d5a6759625ef6c Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Thu, 13 Jun 2024 16:03:04 -0700 Subject: [PATCH] remaining minor cleanups --- programs/stake/src/stake_instruction.rs | 24 ------------------------ programs/stake/src/stake_state.rs | 15 ++++++++++----- sdk/program/src/stake/instruction.rs | 2 +- 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 567e03565b7bd6..1b874cd9750596 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -354,13 +354,11 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context| } } StakeInstruction::MoveStake(lamports) => { - let me = get_stake_account()?; if invoke_context .get_feature_set() .is_active(&feature_set::move_stake_and_move_lamports_ixs::id()) { instruction_context.check_number_of_instruction_accounts(3)?; - drop(me); move_stake( invoke_context, transaction_context, @@ -375,13 +373,11 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context| } } StakeInstruction::MoveLamports(lamports) => { - let me = get_stake_account()?; if invoke_context .get_feature_set() .is_active(&feature_set::move_stake_and_move_lamports_ixs::id()) { instruction_context.check_number_of_instruction_accounts(3)?; - drop(me); move_lamports( invoke_context, transaction_context, @@ -863,26 +859,6 @@ mod tests { )[2], Err(InstructionError::InvalidAccountOwner), ); - process_instruction_as_one_arg( - Arc::clone(&feature_set), - &instruction::move_stake( - &spoofed_stake_state_pubkey(), - &Pubkey::new_unique(), - &Pubkey::new_unique(), - 100, - ), - Err(InstructionError::InvalidAccountOwner), - ); - process_instruction_as_one_arg( - Arc::clone(&feature_set), - &instruction::move_lamports( - &spoofed_stake_state_pubkey(), - &Pubkey::new_unique(), - &Pubkey::new_unique(), - 100, - ), - Err(InstructionError::InvalidAccountOwner), - ); } #[test_case(feature_set_no_minimum_delegation(); "no_min_delegation")] diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 97ea34dc2f2c45..baee081034703d 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -810,7 +810,8 @@ pub fn move_stake( stake_authority_index, )?; - // source and destination will have their data reassigned + // ensure source and destination are the right size for the current version of StakeState + // this a safeguard in case there is a new version of the struct that cannot fit into an old account if source_account.get_data().len() != StakeStateV2::size_of() || destination_account.get_data().len() != StakeStateV2::size_of() { @@ -818,26 +819,27 @@ pub fn move_stake( } // source must be fully active - // destination must not be activating - // if active, destination must be delegated to the same vote account as source - // minimum delegations must be respected for any accounts that become/remain active let MergeKind::FullyActive(source_meta, mut source_stake) = source_merge_kind else { return Err(InstructionError::InvalidAccountData); }; let minimum_delegation = crate::get_minimum_delegation(invoke_context.get_feature_set()); - let source_effective_stake = source_stake.delegation.stake; + + // source cannot move more stake than it has, regardless of how many lamports it has let source_final_stake = source_effective_stake .checked_sub(lamports) .ok_or(InstructionError::InvalidArgument)?; + // unless all stake is being moved, source must retain at least the minimum delegation if source_final_stake != 0 && source_final_stake < minimum_delegation { return Err(InstructionError::InvalidArgument); } + // destination must be fully active or fully inactive let destination_meta = match destination_merge_kind { MergeKind::FullyActive(destination_meta, mut destination_stake) => { + // if active, destination must be delegated to the same vote account as source if source_stake.delegation.voter_pubkey != destination_stake.delegation.voter_pubkey { return Err(StakeError::VoteAddressMismatch.into()); } @@ -847,6 +849,8 @@ pub fn move_stake( .checked_add(lamports) .ok_or(InstructionError::ArithmeticOverflow)?; + // ensure destination meets miniumum delegation + // since it is already active, this only really applies if the minimum is raised if destination_final_stake < minimum_delegation { return Err(InstructionError::InvalidArgument); } @@ -868,6 +872,7 @@ pub fn move_stake( destination_meta } MergeKind::Inactive(destination_meta, _, _) => { + // if destination is inactive, it must be given at least the minimum delegation if lamports < minimum_delegation { return Err(InstructionError::InvalidArgument); } diff --git a/sdk/program/src/stake/instruction.rs b/sdk/program/src/stake/instruction.rs index 4fa50e4f3d764d..e2a5b056e70618 100644 --- a/sdk/program/src/stake/instruction.rs +++ b/sdk/program/src/stake/instruction.rs @@ -314,7 +314,7 @@ pub enum StakeInstruction { /// becomes inactive. Otherwise, at least the minimum delegation of active stake must remain. /// /// The destination account must be fully active or fully inactive. If it is active, it must - /// be delegated to the same vote accouunt as the source. If it is inactive, it + /// be delegated to the same vote account as the source. If it is inactive, it /// immediately becomes active, and must contain at least the minimum delegation. The /// destination must be pre-funded with the rent-exempt reserve. ///