remaining minor cleanups

anza-xyz · Jun 13, 2024 · 53e7779 · 53e7779
1 parent 19df757
commit 53e7779
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 30 deletions.
diff --git 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
@@ -810,34 +810,36 @@ 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()
     {
         return Err(InstructionError::InvalidAccountData);
     }
 
     // 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
@@ -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.
     ///