stake-pool: Wait at least two epoch boundaries to set fee (#3979)

Author: joncinque

docs/src/stake-pool/cli.md

@@ -169,18 +169,19 @@ Signature: 5yPXfVj5cbKBfZiEVi2UR5bXzVDuc2c3ruBwSjkAqpvxPHigwGHiS1mXQVE4qwok5moMW
 ```
 
 In order to protect stake pool depositors from malicious managers, the program
-applies the new fee for the following epoch.
+applies the new fee after crossing two epoch boundaries, giving a minimum wait
+time of one full epoch.
 
 For example, if the fee is 1% at epoch 100, and the manager sets it to 10%, the
-manager will still gain 1% for the rewards earned during epoch 100. Starting
-with epoch 101, the manager will earn 10%.
+manager will still gain 1% for the rewards earned during epochs 100 and 101. Starting
+with epoch 102, the manager will earn 10%.
 
 Additionally, to prevent a malicious manager from immediately setting the withdrawal
 fee to a very high amount, making it practically impossible for users to withdraw,
 the stake pool program currently enforces a limit of 1.5x increase per epoch.
 
-For example, if the current withdrawal fee is 2.5%, the maximum that can be set
-for the next epoch is 3.75%.
+For example, if the current withdrawal fee is 2.5%, the maximum settable fee is
+3.75%, and will take effect after two epoch boundaries.
 
 The possible options for the fee type are `epoch`, `sol-withdrawal`,
 `stake-withdrawal`, `sol-deposit`, and `stake-deposit`.

stake-pool/cli/src/output.rs

@@ -475,7 +475,8 @@ impl From<(Pubkey, StakePool, ValidatorList, Pubkey)> for CliStakePool {
             last_update_epoch: stake_pool.last_update_epoch,
             lockup: CliStakePoolLockup::from(stake_pool.lockup),
             epoch_fee: CliStakePoolFee::from(stake_pool.epoch_fee),
-            next_epoch_fee: stake_pool.next_epoch_fee.map(CliStakePoolFee::from),
+            next_epoch_fee: Option::<Fee>::from(stake_pool.next_epoch_fee)
+                .map(CliStakePoolFee::from),
             preferred_deposit_validator_vote_address: stake_pool
                 .preferred_deposit_validator_vote_address
                 .map(|x| x.to_string()),
@@ -484,17 +485,15 @@ impl From<(Pubkey, StakePool, ValidatorList, Pubkey)> for CliStakePool {
                 .map(|x| x.to_string()),
             stake_deposit_fee: CliStakePoolFee::from(stake_pool.stake_deposit_fee),
             stake_withdrawal_fee: CliStakePoolFee::from(stake_pool.stake_withdrawal_fee),
-            next_stake_withdrawal_fee: stake_pool
-                .next_stake_withdrawal_fee
+            next_stake_withdrawal_fee: Option::<Fee>::from(stake_pool.next_stake_withdrawal_fee)
                 .map(CliStakePoolFee::from),
             stake_referral_fee: stake_pool.stake_referral_fee,
             sol_deposit_authority: stake_pool.sol_deposit_authority.map(|x| x.to_string()),
             sol_deposit_fee: CliStakePoolFee::from(stake_pool.sol_deposit_fee),
             sol_referral_fee: stake_pool.sol_referral_fee,
             sol_withdraw_authority: stake_pool.sol_withdraw_authority.map(|x| x.to_string()),
             sol_withdrawal_fee: CliStakePoolFee::from(stake_pool.sol_withdrawal_fee),
-            next_sol_withdrawal_fee: stake_pool
-                .next_sol_withdrawal_fee
+            next_sol_withdrawal_fee: Option::<Fee>::from(stake_pool.next_sol_withdrawal_fee)
                 .map(CliStakePoolFee::from),
             last_epoch_pool_token_supply: stake_pool.last_epoch_pool_token_supply,
             last_epoch_total_lamports: stake_pool.last_epoch_total_lamports,

stake-pool/program/src/processor.rs

@@ -7,8 +7,9 @@ use {
         instruction::{FundingType, PreferredValidatorType, StakePoolInstruction},
         minimum_delegation, minimum_reserve_lamports, minimum_stake_lamports,
         state::{
-            is_extension_supported_for_mint, AccountType, Fee, FeeType, StakePool, StakeStatus,
-            StakeWithdrawSource, ValidatorList, ValidatorListHeader, ValidatorStakeInfo,
+            is_extension_supported_for_mint, AccountType, Fee, FeeType, FutureEpoch, StakePool,
+            StakeStatus, StakeWithdrawSource, ValidatorList, ValidatorListHeader,
+            ValidatorStakeInfo,
         },
         AUTHORITY_DEPOSIT, AUTHORITY_WITHDRAW, EPHEMERAL_STAKE_SEED_PREFIX,
         TRANSIENT_STAKE_SEED_PREFIX,
@@ -905,19 +906,19 @@ impl Processor {
         stake_pool.last_update_epoch = Clock::get()?.epoch;
         stake_pool.lockup = stake::state::Lockup::default();
         stake_pool.epoch_fee = epoch_fee;
-        stake_pool.next_epoch_fee = None;
+        stake_pool.next_epoch_fee = FutureEpoch::None;
         stake_pool.preferred_deposit_validator_vote_address = None;
         stake_pool.preferred_withdraw_validator_vote_address = None;
         stake_pool.stake_deposit_fee = deposit_fee;
         stake_pool.stake_withdrawal_fee = withdrawal_fee;
-        stake_pool.next_stake_withdrawal_fee = None;
+        stake_pool.next_stake_withdrawal_fee = FutureEpoch::None;
         stake_pool.stake_referral_fee = referral_fee;
         stake_pool.sol_deposit_authority = sol_deposit_authority;
         stake_pool.sol_deposit_fee = deposit_fee;
         stake_pool.sol_referral_fee = referral_fee;
         stake_pool.sol_withdraw_authority = None;
         stake_pool.sol_withdrawal_fee = withdrawal_fee;
-        stake_pool.next_sol_withdrawal_fee = None;
+        stake_pool.next_sol_withdrawal_fee = FutureEpoch::None;
         stake_pool.last_epoch_pool_token_supply = 0;
         stake_pool.last_epoch_total_lamports = 0;
 
@@ -2532,18 +2533,21 @@ impl Processor {
         }
 
         if stake_pool.last_update_epoch < clock.epoch {
-            if let Some(fee) = stake_pool.next_epoch_fee {
-                stake_pool.epoch_fee = fee;
-                stake_pool.next_epoch_fee = None;
+            if let Some(fee) = stake_pool.next_epoch_fee.get() {
+                stake_pool.epoch_fee = *fee;
             }
-            if let Some(fee) = stake_pool.next_stake_withdrawal_fee {
-                stake_pool.stake_withdrawal_fee = fee;
-                stake_pool.next_stake_withdrawal_fee = None;
+            stake_pool.next_epoch_fee.update_epoch();
+
+            if let Some(fee) = stake_pool.next_stake_withdrawal_fee.get() {
+                stake_pool.stake_withdrawal_fee = *fee;
             }
-            if let Some(fee) = stake_pool.next_sol_withdrawal_fee {
-                stake_pool.sol_withdrawal_fee = fee;
-                stake_pool.next_sol_withdrawal_fee = None;
+            stake_pool.next_stake_withdrawal_fee.update_epoch();
+
+            if let Some(fee) = stake_pool.next_sol_withdrawal_fee.get() {
+                stake_pool.sol_withdrawal_fee = *fee;
             }
+            stake_pool.next_sol_withdrawal_fee.update_epoch();
+
             stake_pool.last_update_epoch = clock.epoch;
             stake_pool.last_epoch_total_lamports = previous_lamports;
             stake_pool.last_epoch_pool_token_supply = previous_pool_token_supply;

stake-pool/program/src/state.rs

@@ -104,7 +104,7 @@ pub struct StakePool {
     pub epoch_fee: Fee,
 
     /// Fee for next epoch
-    pub next_epoch_fee: Option<Fee>,
+    pub next_epoch_fee: FutureEpoch<Fee>,
 
     /// Preferred deposit validator vote account pubkey
     pub preferred_deposit_validator_vote_address: Option<Pubkey>,
@@ -119,7 +119,7 @@ pub struct StakePool {
     pub stake_withdrawal_fee: Fee,
 
     /// Future stake withdrawal fee, to be set for the following epoch
-    pub next_stake_withdrawal_fee: Option<Fee>,
+    pub next_stake_withdrawal_fee: FutureEpoch<Fee>,
 
     /// Fees paid out to referrers on referred stake deposits.
     /// Expressed as a percentage (0 - 100) of deposit fees.
@@ -148,7 +148,7 @@ pub struct StakePool {
     pub sol_withdrawal_fee: Fee,
 
     /// Future SOL withdrawal fee, to be set for the following epoch
-    pub next_sol_withdrawal_fee: Option<Fee>,
+    pub next_sol_withdrawal_fee: FutureEpoch<Fee>,
 
     /// Last epoch's total pool tokens, used only for APR estimation
     pub last_epoch_pool_token_supply: u64,
@@ -483,14 +483,14 @@ impl StakePool {
         match fee {
             FeeType::SolReferral(new_fee) => self.sol_referral_fee = *new_fee,
             FeeType::StakeReferral(new_fee) => self.stake_referral_fee = *new_fee,
-            FeeType::Epoch(new_fee) => self.next_epoch_fee = Some(*new_fee),
+            FeeType::Epoch(new_fee) => self.next_epoch_fee = FutureEpoch::new(*new_fee),
             FeeType::StakeWithdrawal(new_fee) => {
                 new_fee.check_withdrawal(&self.stake_withdrawal_fee)?;
-                self.next_stake_withdrawal_fee = Some(*new_fee)
+                self.next_stake_withdrawal_fee = FutureEpoch::new(*new_fee)
             }
             FeeType::SolWithdrawal(new_fee) => {
                 new_fee.check_withdrawal(&self.sol_withdrawal_fee)?;
-                self.next_sol_withdrawal_fee = Some(*new_fee)
+                self.next_sol_withdrawal_fee = FutureEpoch::new(*new_fee)
             }
             FeeType::SolDeposit(new_fee) => self.sol_deposit_fee = *new_fee,
             FeeType::StakeDeposit(new_fee) => self.stake_deposit_fee = *new_fee,
@@ -793,6 +793,62 @@ impl ValidatorListHeader {
     }
 }
 
+/// Wrapper type that "counts down" epochs, which is Borsh-compatible with the
+/// native `Option`
+#[repr(C)]
+#[derive(Clone, Copy, Debug, PartialEq, BorshSerialize, BorshDeserialize, BorshSchema)]
+pub enum FutureEpoch<T> {
+    /// Nothing is set
+    None,
+    /// Value is ready after the next epoch boundary
+    One(T),
+    /// Value is ready after two epoch boundaries
+    Two(T),
+}
+impl<T> Default for FutureEpoch<T> {
+    fn default() -> Self {
+        Self::None
+    }
+}
+impl<T> FutureEpoch<T> {
+    /// Create a new value to be unlocked in a two epochs
+    pub fn new(value: T) -> Self {
+        Self::Two(value)
+    }
+}
+impl<T: Clone> FutureEpoch<T> {
+    /// Update the epoch, to be done after `get`ting the underlying value
+    pub fn update_epoch(&mut self) {
+        match self {
+            Self::None => {}
+            Self::One(_) => {
+                // The value has waited its last epoch
+                *self = Self::None;
+            }
+            // The value still has to wait one more epoch after this
+            Self::Two(v) => {
+                *self = Self::One(v.clone());
+            }
+        }
+    }
+
+    /// Get the value if it's ready, which is only at `One` epoch remaining
+    pub fn get(&self) -> Option<&T> {
+        match self {
+            Self::None | Self::Two(_) => None,
+            Self::One(v) => Some(v),
+        }
+    }
+}
+impl<T> From<FutureEpoch<T>> for Option<T> {
+    fn from(v: FutureEpoch<T>) -> Option<T> {
+        match v {
+            FutureEpoch::None => None,
+            FutureEpoch::One(inner) | FutureEpoch::Two(inner) => Some(inner),
+        }
+    }
+}
+
 /// Fee rate as a ratio, minted on `UpdateStakePoolBalance` as a proportion of
 /// the rewards
 /// If either the numerator or the denominator is 0, the fee is considered to be 0

stake-pool/program/tests/helpers/mod.rs

@@ -30,7 +30,7 @@ use {
         find_stake_program_address, find_transient_stake_program_address,
         find_withdraw_authority_program_address, id, instruction, minimum_delegation,
         processor::Processor,
-        state::{self, FeeType, StakePool, ValidatorList},
+        state::{self, FeeType, FutureEpoch, StakePool, ValidatorList},
         MINIMUM_RESERVE_LAMPORTS,
     },
     spl_token_2022::{
@@ -1776,19 +1776,19 @@ impl StakePoolAccounts {
             last_update_epoch: 0,
             lockup: stake::state::Lockup::default(),
             epoch_fee: self.epoch_fee,
-            next_epoch_fee: None,
+            next_epoch_fee: FutureEpoch::None,
             preferred_deposit_validator_vote_address: None,
             preferred_withdraw_validator_vote_address: None,
             stake_deposit_fee: state::Fee::default(),
             sol_deposit_fee: state::Fee::default(),
             stake_withdrawal_fee: state::Fee::default(),
-            next_stake_withdrawal_fee: None,
+            next_stake_withdrawal_fee: FutureEpoch::None,
             stake_referral_fee: 0,
             sol_referral_fee: 0,
             sol_deposit_authority: None,
             sol_withdraw_authority: None,
             sol_withdrawal_fee: state::Fee::default(),
-            next_sol_withdrawal_fee: None,
+            next_sol_withdrawal_fee: FutureEpoch::None,
             last_epoch_pool_token_supply: 0,
             last_epoch_total_lamports: 0,
         };

stake-pool/program/tests/set_epoch_fee.rs

@@ -14,7 +14,7 @@ use {
     },
     spl_stake_pool::{
         error, id, instruction,
-        state::{Fee, FeeType, StakePool},
+        state::{Fee, FeeType, FutureEpoch, StakePool},
         MINIMUM_RESERVE_LAMPORTS,
     },
 };
@@ -76,7 +76,7 @@ async fn success() {
     let stake_pool = try_from_slice_unchecked::<StakePool>(stake_pool.data.as_slice()).unwrap();
 
     assert_eq!(stake_pool.epoch_fee, old_fee);
-    assert_eq!(stake_pool.next_epoch_fee, Some(new_fee));
+    assert_eq!(stake_pool.next_epoch_fee, FutureEpoch::Two(new_fee));
 
     let first_normal_slot = context.genesis_config().epoch_schedule.first_normal_slot;
     let slots_per_epoch = context.genesis_config().epoch_schedule.slots_per_epoch;
@@ -94,14 +94,41 @@ async fn success() {
         )
         .await;
 
+    let stake_pool = get_account(
+        &mut context.banks_client,
+        &stake_pool_accounts.stake_pool.pubkey(),
+    )
+    .await;
+    let stake_pool = try_from_slice_unchecked::<StakePool>(stake_pool.data.as_slice()).unwrap();
+    assert_eq!(stake_pool.epoch_fee, old_fee);
+    assert_eq!(stake_pool.next_epoch_fee, FutureEpoch::One(new_fee));
+
+    let last_blockhash = context
+        .banks_client
+        .get_new_latest_blockhash(&context.last_blockhash)
+        .await
+        .unwrap();
+    context
+        .warp_to_slot(first_normal_slot + 2 * slots_per_epoch)
+        .unwrap();
+    stake_pool_accounts
+        .update_all(
+            &mut context.banks_client,
+            &context.payer,
+            &last_blockhash,
+            &[],
+            false,
+        )
+        .await;
+
     let stake_pool = get_account(
         &mut context.banks_client,
         &stake_pool_accounts.stake_pool.pubkey(),
     )
     .await;
     let stake_pool = try_from_slice_unchecked::<StakePool>(stake_pool.data.as_slice()).unwrap();
     assert_eq!(stake_pool.epoch_fee, new_fee);
-    assert_eq!(stake_pool.next_epoch_fee, None);
+    assert_eq!(stake_pool.next_epoch_fee, FutureEpoch::None);
 }
 
 #[tokio::test]