stake-pool: Allow removal of force-destaked validator (#5439)

joncinque · web-flow · commit 9d3cb4718595 · 2023-10-11T15:36:41.000-04:00
* stake-pool: Allow removal of force-destaked validator

* Update comment
diff --git a/stake-pool/program/src/instruction.rs b/stake-pool/program/src/instruction.rs
@@ -112,7 +112,7 @@ pub enum StakePoolInstruction {
     ///   2. `[]` Stake pool withdraw authority
     ///   3. `[w]` Validator stake list storage account
     ///   4. `[w]` Stake account to remove from the pool
-    ///   5. `[]` Transient stake account, to check that that we're not trying to activate
+    ///   5. `[w]` Transient stake account, to deactivate if necessary
     ///   6. `[]` Sysvar clock
     ///   7. `[]` Stake program id,
     RemoveValidatorFromPool,
@@ -777,7 +777,7 @@ pub fn remove_validator_from_pool(
         AccountMeta::new_readonly(*stake_pool_withdraw, false),
         AccountMeta::new(*validator_list, false),
         AccountMeta::new(*stake_account, false),
-        AccountMeta::new_readonly(*transient_stake_account, false),
+        AccountMeta::new(*transient_stake_account, false),
         AccountMeta::new_readonly(sysvar::clock::id(), false),
         AccountMeta::new_readonly(stake::program::id(), false),
     ];
diff --git a/stake-pool/program/src/processor.rs b/stake-pool/program/src/processor.rs
@@ -1193,32 +1193,35 @@ impl Processor {
                     ) =>
                 {
                     if stake.delegation.deactivation_epoch == Epoch::MAX {
-                        msg!(
-                            "Transient stake {} activating, can't remove stake {} on validator {}",
-                            transient_stake_account_info.key,
-                            stake_account_info.key,
-                            vote_account_address
-                        );
-                        return Err(StakePoolError::WrongStakeState.into());
-                    } else {
-                        StakeStatus::DeactivatingAll
+                        Self::stake_deactivate(
+                            transient_stake_account_info.clone(),
+                            clock_info.clone(),
+                            withdraw_authority_info.clone(),
+                            stake_pool_info.key,
+                            AUTHORITY_WITHDRAW,
+                            stake_pool.stake_withdraw_bump_seed,
+                        )?;
                     }
+                    StakeStatus::DeactivatingAll
                 }
                 _ => StakeStatus::DeactivatingValidator,
             }
         } else {
             StakeStatus::DeactivatingValidator
         };
 
-        // deactivate stake
-        Self::stake_deactivate(
-            stake_account_info.clone(),
-            clock_info.clone(),
-            withdraw_authority_info.clone(),
-            stake_pool_info.key,
-            AUTHORITY_WITHDRAW,
-            stake_pool.stake_withdraw_bump_seed,
-        )?;
+        // If the stake was force-deactivated through deactivate-delinquent or
+        // some other means, we *do not* need to deactivate it again
+        if stake.delegation.deactivation_epoch == Epoch::MAX {
+            Self::stake_deactivate(
+                stake_account_info.clone(),
+                clock_info.clone(),
+                withdraw_authority_info.clone(),
+                stake_pool_info.key,
+                AUTHORITY_WITHDRAW,
+                stake_pool.stake_withdraw_bump_seed,
+            )?;
+        }
 
         validator_stake_info.status = new_status.into();
 
diff --git a/stake-pool/program/tests/force_destake.rs b/stake-pool/program/tests/force_destake.rs
@@ -5,7 +5,15 @@ mod helpers;
 
 use {
     helpers::*,
-    solana_program::{instruction::InstructionError, pubkey::Pubkey, stake},
+    solana_program::{
+        borsh0_10::try_from_slice_unchecked,
+        instruction::InstructionError,
+        pubkey::Pubkey,
+        stake::{
+            self,
+            state::{Authorized, Delegation, Lockup, Meta, Stake, StakeState},
+        },
+    },
     solana_program_test::*,
     solana_sdk::{
         account::{Account, WritableAccount},
@@ -16,40 +24,27 @@ use {
     spl_stake_pool::{
         error::StakePoolError,
         find_stake_program_address, find_transient_stake_program_address, id,
-        state::{StakeStatus, ValidatorStakeInfo},
+        state::{AccountType, StakeStatus, ValidatorList, ValidatorListHeader, ValidatorStakeInfo},
         MINIMUM_ACTIVE_STAKE,
     },
     std::num::NonZeroU32,
 };
 
-async fn setup() -> (
-    ProgramTestContext,
-    StakePoolAccounts,
-    Pubkey,
-    Option<NonZeroU32>,
-) {
+async fn setup(
+    stake_pool_accounts: &StakePoolAccounts,
+    forced_stake: &StakeState,
+    voter_pubkey: &Pubkey,
+) -> (ProgramTestContext, Option<NonZeroU32>) {
     let mut program_test = program_test();
-    let stake_pool_accounts = StakePoolAccounts::default();
 
     let stake_pool_pubkey = stake_pool_accounts.stake_pool.pubkey();
     let (mut stake_pool, mut validator_list) = stake_pool_accounts.state();
 
-    let voter_pubkey = add_vote_account(&mut program_test);
-    let meta = stake::state::Meta {
-        rent_exempt_reserve: STAKE_ACCOUNT_RENT_EXEMPTION,
-        authorized: stake::state::Authorized {
-            staker: stake_pool_accounts.withdraw_authority,
-            withdrawer: stake_pool_accounts.withdraw_authority,
-        },
-        lockup: stake_pool.lockup,
-    };
+    let _ = add_vote_account_with_pubkey(voter_pubkey, &mut program_test);
 
     let stake_account = Account::create(
         TEST_STAKE_AMOUNT + STAKE_ACCOUNT_RENT_EXEMPTION,
-        bincode::serialize::<stake::state::StakeState>(&stake::state::StakeState::Initialized(
-            meta,
-        ))
-        .unwrap(),
+        bincode::serialize::<StakeState>(forced_stake).unwrap(),
         stake::program::id(),
         false,
         Epoch::default(),
@@ -58,13 +53,13 @@ async fn setup() -> (
     let raw_validator_seed = 42;
     let validator_seed = NonZeroU32::new(raw_validator_seed);
     let (stake_address, _) =
-        find_stake_program_address(&id(), &voter_pubkey, &stake_pool_pubkey, validator_seed);
+        find_stake_program_address(&id(), voter_pubkey, &stake_pool_pubkey, validator_seed);
     program_test.add_account(stake_address, stake_account);
     let active_stake_lamports = TEST_STAKE_AMOUNT - MINIMUM_ACTIVE_STAKE;
     // add to validator list
     validator_list.validators.push(ValidatorStakeInfo {
         status: StakeStatus::Active.into(),
-        vote_account_address: voter_pubkey,
+        vote_account_address: *voter_pubkey,
         active_stake_lamports: active_stake_lamports.into(),
         transient_stake_lamports: 0.into(),
         last_update_epoch: 0.into(),
@@ -110,12 +105,27 @@ async fn setup() -> (
     );
 
     let context = program_test.start_with_context().await;
-    (context, stake_pool_accounts, voter_pubkey, validator_seed)
+    (context, validator_seed)
 }
 
 #[tokio::test]
 async fn success_update() {
-    let (mut context, stake_pool_accounts, voter_pubkey, validator_seed) = setup().await;
+    let stake_pool_accounts = StakePoolAccounts::default();
+    let meta = Meta {
+        rent_exempt_reserve: STAKE_ACCOUNT_RENT_EXEMPTION,
+        authorized: Authorized {
+            staker: stake_pool_accounts.withdraw_authority,
+            withdrawer: stake_pool_accounts.withdraw_authority,
+        },
+        lockup: Lockup::default(),
+    };
+    let voter_pubkey = Pubkey::new_unique();
+    let (mut context, validator_seed) = setup(
+        &stake_pool_accounts,
+        &StakeState::Initialized(meta),
+        &voter_pubkey,
+    )
+    .await;
     let pre_reserve_lamports = context
         .banks_client
         .get_account(stake_pool_accounts.reserve_stake.pubkey())
@@ -169,7 +179,22 @@ async fn success_update() {
 
 #[tokio::test]
 async fn fail_increase() {
-    let (mut context, stake_pool_accounts, voter_pubkey, validator_seed) = setup().await;
+    let stake_pool_accounts = StakePoolAccounts::default();
+    let meta = Meta {
+        rent_exempt_reserve: STAKE_ACCOUNT_RENT_EXEMPTION,
+        authorized: Authorized {
+            staker: stake_pool_accounts.withdraw_authority,
+            withdrawer: stake_pool_accounts.withdraw_authority,
+        },
+        lockup: Lockup::default(),
+    };
+    let voter_pubkey = Pubkey::new_unique();
+    let (mut context, validator_seed) = setup(
+        &stake_pool_accounts,
+        &StakeState::Initialized(meta),
+        &voter_pubkey,
+    )
+    .await;
     let (stake_address, _) = find_stake_program_address(
         &id(),
         &voter_pubkey,
@@ -206,3 +231,113 @@ async fn fail_increase() {
         )
     );
 }
+
+#[tokio::test]
+async fn success_remove_validator() {
+    let stake_pool_accounts = StakePoolAccounts::default();
+    let meta = Meta {
+        rent_exempt_reserve: STAKE_ACCOUNT_RENT_EXEMPTION,
+        authorized: Authorized {
+            staker: stake_pool_accounts.withdraw_authority,
+            withdrawer: stake_pool_accounts.withdraw_authority,
+        },
+        lockup: Lockup::default(),
+    };
+    let voter_pubkey = Pubkey::new_unique();
+    let stake = Stake {
+        delegation: Delegation {
+            voter_pubkey,
+            stake: TEST_STAKE_AMOUNT,
+            activation_epoch: 0,
+            deactivation_epoch: 0,
+            ..Delegation::default()
+        },
+        credits_observed: 1,
+    };
+    let (mut context, validator_seed) = setup(
+        &stake_pool_accounts,
+        &StakeState::Stake(meta, stake),
+        &voter_pubkey,
+    )
+    .await;
+
+    // move forward to after deactivation
+    let first_normal_slot = context.genesis_config().epoch_schedule.first_normal_slot;
+    context.warp_to_slot(first_normal_slot + 1).unwrap();
+    stake_pool_accounts
+        .update_all(
+            &mut context.banks_client,
+            &context.payer,
+            &context.last_blockhash,
+            &[voter_pubkey],
+            false,
+        )
+        .await;
+
+    let (stake_address, _) = find_stake_program_address(
+        &id(),
+        &voter_pubkey,
+        &stake_pool_accounts.stake_pool.pubkey(),
+        validator_seed,
+    );
+    let transient_stake_seed = 0;
+    let transient_stake_address = find_transient_stake_program_address(
+        &id(),
+        &voter_pubkey,
+        &stake_pool_accounts.stake_pool.pubkey(),
+        transient_stake_seed,
+    )
+    .0;
+
+    let error = stake_pool_accounts
+        .remove_validator_from_pool(
+            &mut context.banks_client,
+            &context.payer,
+            &context.last_blockhash,
+            &stake_address,
+            &transient_stake_address,
+        )
+        .await;
+    assert!(error.is_none(), "{:?}", error);
+
+    // Get a new blockhash for the next update to work
+    context.get_new_latest_blockhash().await.unwrap();
+
+    let error = stake_pool_accounts
+        .update_all(
+            &mut context.banks_client,
+            &context.payer,
+            &context.last_blockhash,
+            &[voter_pubkey],
+            false,
+        )
+        .await;
+    assert!(error.is_none(), "{:?}", error);
+
+    // Check if account was removed from the list of stake accounts
+    let validator_list = get_account(
+        &mut context.banks_client,
+        &stake_pool_accounts.validator_list.pubkey(),
+    )
+    .await;
+    let validator_list =
+        try_from_slice_unchecked::<ValidatorList>(validator_list.data.as_slice()).unwrap();
+    assert_eq!(
+        validator_list,
+        ValidatorList {
+            header: ValidatorListHeader {
+                account_type: AccountType::ValidatorList,
+                max_validators: stake_pool_accounts.max_validators,
+            },
+            validators: vec![]
+        }
+    );
+
+    // Check stake account no longer exists
+    let account = context
+        .banks_client
+        .get_account(stake_address)
+        .await
+        .unwrap();
+    assert!(account.is_none());
+}
diff --git a/stake-pool/program/tests/helpers/mod.rs b/stake-pool/program/tests/helpers/mod.rs
@@ -2384,13 +2384,15 @@ pub async fn get_validator_list_sum(
     validator_sum + reserve_stake.lamports - rent - MINIMUM_RESERVE_LAMPORTS
 }
 
-pub fn add_vote_account(program_test: &mut ProgramTest) -> Pubkey {
+pub fn add_vote_account_with_pubkey(
+    voter_pubkey: &Pubkey,
+    program_test: &mut ProgramTest,
+) -> Pubkey {
     let authorized_voter = Pubkey::new_unique();
     let authorized_withdrawer = Pubkey::new_unique();
     let commission = 1;
 
     // create vote account
-    let vote_pubkey = Pubkey::new_unique();
     let node_pubkey = Pubkey::new_unique();
     let vote_state = VoteStateVersions::new_current(VoteState::new(
         &VoteInit {
@@ -2408,8 +2410,13 @@ pub fn add_vote_account(program_test: &mut ProgramTest) -> Pubkey {
         false,
         Epoch::default(),
     );
-    program_test.add_account(vote_pubkey, vote_account);
-    vote_pubkey
+    program_test.add_account(*voter_pubkey, vote_account);
+    *voter_pubkey
+}
+
+pub fn add_vote_account(program_test: &mut ProgramTest) -> Pubkey {
+    let voter_pubkey = Pubkey::new_unique();
+    add_vote_account_with_pubkey(&voter_pubkey, program_test)
 }
 
 #[allow(clippy::too_many_arguments)]
diff --git a/stake-pool/program/tests/vsa_remove.rs b/stake-pool/program/tests/vsa_remove.rs
diff --git a/stake-pool/py/stake_pool/instructions.py b/stake-pool/py/stake_pool/instructions.py
