diff --git a/cli/src/stake.rs b/cli/src/stake.rs index 547c3d9fb3..525ea1110c 100644 --- a/cli/src/stake.rs +++ b/cli/src/stake.rs @@ -39,13 +39,11 @@ use solana_sdk::{ stake::{ self, instruction::{self as stake_instruction, LockupArgs, StakeError}, - state::{Authorized, Lockup, Meta, StakeAuthorize, StakeState}, + state::{Authorized, Lockup, Meta, StakeActivationStatus, StakeAuthorize, StakeState}, }, + stake_history::StakeHistory, system_instruction::SystemError, - sysvar::{ - clock, - stake_history::{self, StakeHistory}, - }, + sysvar::{clock, stake_history}, transaction::Transaction, }; use solana_vote_program::vote_state::VoteState; @@ -2013,7 +2011,11 @@ pub fn build_stake_state( stake, ) => { let current_epoch = clock.epoch; - let (active_stake, activating_stake, deactivating_stake) = stake + let StakeActivationStatus { + effective, + activating, + deactivating, + } = stake .delegation .stake_activating_and_deactivating(current_epoch, Some(stake_history)); let lockup = if lockup.is_in_force(clock, None) { @@ -2048,9 +2050,9 @@ pub fn build_stake_state( use_lamports_unit, current_epoch, rent_exempt_reserve: Some(*rent_exempt_reserve), - active_stake: u64_some_if_not_zero(active_stake), - activating_stake: u64_some_if_not_zero(activating_stake), - deactivating_stake: u64_some_if_not_zero(deactivating_stake), + active_stake: u64_some_if_not_zero(effective), + activating_stake: u64_some_if_not_zero(activating), + deactivating_stake: u64_some_if_not_zero(deactivating), ..CliStakeState::default() } } diff --git a/program-test/tests/warp.rs b/program-test/tests/warp.rs index 2b74229f70..bd6b365e88 100644 --- a/program-test/tests/warp.rs +++ b/program-test/tests/warp.rs @@ -1,6 +1,5 @@ #![allow(clippy::integer_arithmetic)] use { - assert_matches::assert_matches, bincode::deserialize, solana_banks_client::BanksClient, solana_program_test::{processor, ProgramTest, ProgramTestContext, ProgramTestError}, @@ -15,7 +14,7 @@ use { signature::{Keypair, Signer}, stake::{ instruction as stake_instruction, - state::{Authorized, Lockup, StakeState}, + state::{Authorized, Lockup, StakeActivationStatus, StakeState}, }, system_instruction, system_program, sysvar::{ @@ -298,11 +297,11 @@ async fn stake_rewards_from_warp() { let stake_history: StakeHistory = deserialize(&stake_history_account.data).unwrap(); let clock: Clock = deserialize(&clock_account.data).unwrap(); let stake = stake_state.stake().unwrap(); - assert_matches!( + assert_eq!( stake .delegation .stake_activating_and_deactivating(clock.epoch, Some(&stake_history)), - (_, 0, 0) + StakeActivationStatus::with_effective(stake.delegation.stake), ); } diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index be02e7d646..a7044ee182 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -875,12 +875,11 @@ impl MergeKind { StakeState::Stake(meta, stake) => { // stake must not be in a transient state. Transient here meaning // activating or deactivating with non-zero effective stake. - match stake + let status = stake .delegation - .stake_activating_and_deactivating(clock.epoch, Some(stake_history)) - { - /* - (e, a, d): e - effective, a - activating, d - deactivating */ + .stake_activating_and_deactivating(clock.epoch, Some(stake_history)); + + match (status.effective, status.activating, status.deactivating) { (0, 0, 0) => Ok(Self::Inactive(meta, stake_keyed_account.lamports()?)), (0, _, _) => Ok(Self::ActivationEpoch(meta, stake)), (_, 0, 0) => Ok(Self::FullyActive(meta, stake)), @@ -1192,20 +1191,9 @@ pub fn new_stake_history_entry<'a, I>( where I: Iterator, { - // whatever the stake says they had for the epoch - // and whatever the were still waiting for - fn add(a: (u64, u64, u64), b: (u64, u64, u64)) -> (u64, u64, u64) { - (a.0 + b.0, a.1 + b.1, a.2 + b.2) - } - let (effective, activating, deactivating) = stakes.fold((0, 0, 0), |sum, stake| { - add(sum, stake.stake_activating_and_deactivating(epoch, history)) - }); - - StakeHistoryEntry { - effective, - activating, - deactivating, - } + stakes.fold(StakeHistoryEntry::default(), |sum, stake| { + sum + stake.stake_activating_and_deactivating(epoch, history) + }) } // genesis investor accounts @@ -1769,19 +1757,19 @@ mod tests { // assert that this stake follows step function if there's no history assert_eq!( stake.stake_activating_and_deactivating(stake.activation_epoch, Some(&stake_history),), - (0, stake.stake, 0) + StakeActivationStatus::with_effective_and_activating(0, stake.stake), ); for epoch in stake.activation_epoch + 1..stake.deactivation_epoch { assert_eq!( stake.stake_activating_and_deactivating(epoch, Some(&stake_history)), - (stake.stake, 0, 0) + StakeActivationStatus::with_effective(stake.stake), ); } // assert that this stake is full deactivating assert_eq!( stake .stake_activating_and_deactivating(stake.deactivation_epoch, Some(&stake_history),), - (stake.stake, 0, stake.stake) + StakeActivationStatus::with_deactivating(stake.stake), ); // assert that this stake is fully deactivated if there's no history assert_eq!( @@ -1789,21 +1777,20 @@ mod tests { stake.deactivation_epoch + 1, Some(&stake_history), ), - (0, 0, 0) + StakeActivationStatus::default(), ); stake_history.add( 0u64, // entry for zero doesn't have my activating amount StakeHistoryEntry { effective: 1_000, - activating: 0, ..StakeHistoryEntry::default() }, ); // assert that this stake is broken, because above setup is broken assert_eq!( stake.stake_activating_and_deactivating(1, Some(&stake_history)), - (0, stake.stake, 0) + StakeActivationStatus::with_effective_and_activating(0, stake.stake), ); stake_history.add( @@ -1818,7 +1805,10 @@ mod tests { // assert that this stake is broken, because above setup is broken assert_eq!( stake.stake_activating_and_deactivating(2, Some(&stake_history)), - (increment, stake.stake - increment, 0) + StakeActivationStatus::with_effective_and_activating( + increment, + stake.stake - increment + ), ); // start over, test deactivation edge cases @@ -1828,7 +1818,6 @@ mod tests { stake.deactivation_epoch, // entry for zero doesn't have my de-activating amount StakeHistoryEntry { effective: 1_000, - activating: 0, ..StakeHistoryEntry::default() }, ); @@ -1838,7 +1827,7 @@ mod tests { stake.deactivation_epoch + 1, Some(&stake_history), ), - (stake.stake, 0, stake.stake) // says "I'm still waiting for deactivation" + StakeActivationStatus::with_deactivating(stake.stake), ); // put in my initial deactivating amount, but don't put in an entry for next @@ -1856,7 +1845,8 @@ mod tests { stake.deactivation_epoch + 2, Some(&stake_history), ), - (stake.stake - increment, 0, stake.stake - increment) // hung, should be lower + // hung, should be lower + StakeActivationStatus::with_deactivating(stake.stake - increment), ); } @@ -1868,7 +1858,10 @@ mod tests { Slow, } - fn do_test(old_behavior: OldDeactivationBehavior, expected_stakes: &[(u64, u64, u64)]) { + fn do_test( + old_behavior: OldDeactivationBehavior, + expected_stakes: &[StakeActivationStatus], + ) { let cluster_stake = 1_000; let activating_stake = 10_000; let some_stake = 700; @@ -1931,13 +1924,13 @@ mod tests { do_test( OldDeactivationBehavior::Slow, &[ - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), ], ); } @@ -1950,13 +1943,13 @@ mod tests { do_test( OldDeactivationBehavior::Stuck, &[ - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), - (0, 0, 0), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), + StakeActivationStatus::default(), ], ); } @@ -2049,37 +2042,34 @@ mod tests { }) .collect::>() }; - let adjust_staking_status = |rate: f64, status: &Vec<_>| { + let adjust_staking_status = |rate: f64, status: &[StakeActivationStatus]| { status - .clone() - .into_iter() - .map(|(a, b, c)| { - ( - (a as f64 * rate) as u64, - (b as f64 * rate) as u64, - (c as f64 * rate) as u64, - ) + .iter() + .map(|entry| StakeActivationStatus { + effective: (entry.effective as f64 * rate) as u64, + activating: (entry.activating as f64 * rate) as u64, + deactivating: (entry.deactivating as f64 * rate) as u64, }) .collect::>() }; let expected_staking_status_transition = vec![ - (0, 700, 0), - (250, 450, 0), - (562, 138, 0), - (700, 0, 0), - (700, 0, 700), - (275, 0, 275), - (0, 0, 0), + StakeActivationStatus::with_effective_and_activating(0, 700), + StakeActivationStatus::with_effective_and_activating(250, 450), + StakeActivationStatus::with_effective_and_activating(562, 138), + StakeActivationStatus::with_effective(700), + StakeActivationStatus::with_deactivating(700), + StakeActivationStatus::with_deactivating(275), + StakeActivationStatus::default(), ]; let expected_staking_status_transition_base = vec![ - (0, 700, 0), - (250, 450, 0), - (562, 138 + 1, 0), // +1 is needed for rounding - (700, 0, 0), - (700, 0, 700), - (275 + 1, 0, 275 + 1), // +1 is needed for rounding - (0, 0, 0), + StakeActivationStatus::with_effective_and_activating(0, 700), + StakeActivationStatus::with_effective_and_activating(250, 450), + StakeActivationStatus::with_effective_and_activating(562, 138 + 1), // +1 is needed for rounding + StakeActivationStatus::with_effective(700), + StakeActivationStatus::with_deactivating(700), + StakeActivationStatus::with_deactivating(275 + 1), // +1 is needed for rounding + StakeActivationStatus::default(), ]; // normal stake activating and deactivating transition test, just in case @@ -2170,7 +2160,11 @@ mod tests { }; assert_eq!( stake.stake_activating_and_deactivating(epoch, Some(&stake_history)), - (expected_stake, expected_activating, expected_deactivating) + StakeActivationStatus { + effective: expected_stake, + activating: expected_activating, + deactivating: expected_deactivating, + }, ); } } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 76af567f3b..7ac353f2d9 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -61,7 +61,7 @@ use { message::{Message, SanitizedMessage}, pubkey::Pubkey, signature::{Keypair, Signature, Signer}, - stake::state::StakeState, + stake::state::{StakeActivationStatus, StakeState}, stake_history::StakeHistory, system_instruction, sysvar::stake_history, @@ -1573,13 +1573,16 @@ impl JsonRpcRequestProcessor { solana_sdk::account::from_account::(&stake_history_account) .ok_or_else(Error::internal_error)?; - let (active, activating, deactivating) = - delegation.stake_activating_and_deactivating(epoch, Some(&stake_history)); + let StakeActivationStatus { + effective, + activating, + deactivating, + } = delegation.stake_activating_and_deactivating(epoch, Some(&stake_history)); let stake_activation_state = if deactivating > 0 { StakeActivationState::Deactivating } else if activating > 0 { StakeActivationState::Activating - } else if active > 0 { + } else if effective > 0 { StakeActivationState::Active } else { StakeActivationState::Inactive @@ -1587,12 +1590,12 @@ impl JsonRpcRequestProcessor { let inactive_stake = match stake_activation_state { StakeActivationState::Activating => activating, StakeActivationState::Active => 0, - StakeActivationState::Deactivating => delegation.stake.saturating_sub(active), + StakeActivationState::Deactivating => delegation.stake.saturating_sub(effective), StakeActivationState::Inactive => delegation.stake, }; Ok(RpcStakeActivation { state: stake_activation_state, - active, + active: effective, inactive: inactive_stake, }) } diff --git a/runtime/src/stakes.rs b/runtime/src/stakes.rs index 1bc1594561..d22fc415ba 100644 --- a/runtime/src/stakes.rs +++ b/runtime/src/stakes.rs @@ -10,7 +10,7 @@ use { self, state::{Delegation, StakeState}, }, - sysvar::stake_history::StakeHistory, + stake_history::StakeHistory, }, solana_stake_program::stake_state, solana_vote_program::vote_state::VoteState, diff --git a/sdk/program/src/stake/state.rs b/sdk/program/src/stake/state.rs index f33fe46475..625b467cc8 100644 --- a/sdk/program/src/stake/state.rs +++ b/sdk/program/src/stake/state.rs @@ -9,12 +9,14 @@ use { config::Config, instruction::{LockupArgs, StakeError}, }, - stake_history::StakeHistory, + stake_history::{StakeHistory, StakeHistoryEntry}, }, borsh::{maybestd::io, BorshDeserialize, BorshSchema}, std::collections::HashSet, }; +pub type StakeActivationStatus = StakeHistoryEntry; + #[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Copy, AbiExample)] #[allow(clippy::large_enum_variant)] pub enum StakeState { @@ -342,28 +344,33 @@ impl Delegation { } pub fn stake(&self, epoch: Epoch, history: Option<&StakeHistory>) -> u64 { - self.stake_activating_and_deactivating(epoch, history).0 + self.stake_activating_and_deactivating(epoch, history) + .effective } - // returned tuple is (effective, activating, deactivating) stake #[allow(clippy::comparison_chain)] pub fn stake_activating_and_deactivating( &self, target_epoch: Epoch, history: Option<&StakeHistory>, - ) -> (u64, u64, u64) { - let delegated_stake = self.stake; - + ) -> StakeActivationStatus { // first, calculate an effective and activating stake let (effective_stake, activating_stake) = self.stake_and_activating(target_epoch, history); // then de-activate some portion if necessary if target_epoch < self.deactivation_epoch { // not deactivated - (effective_stake, activating_stake, 0) + if activating_stake == 0 { + StakeActivationStatus::with_effective(effective_stake) + } else { + StakeActivationStatus::with_effective_and_activating( + effective_stake, + activating_stake, + ) + } } else if target_epoch == self.deactivation_epoch { // can only deactivate what's activated - (effective_stake, 0, effective_stake.min(delegated_stake)) + StakeActivationStatus::with_deactivating(effective_stake) } else if let Some((history, mut prev_epoch, mut prev_cluster_stake)) = history.and_then(|history| { history @@ -420,10 +427,10 @@ impl Delegation { } // deactivating stake should equal to all of currently remaining effective stake - (current_effective_stake, 0, current_effective_stake) + StakeActivationStatus::with_deactivating(current_effective_stake) } else { // no history or I've dropped out of history, so assume fully deactivated - (0, 0, 0) + StakeActivationStatus::default() } } diff --git a/sdk/program/src/stake_history.rs b/sdk/program/src/stake_history.rs index 8a3c89d073..910cb2f18a 100644 --- a/sdk/program/src/stake_history.rs +++ b/sdk/program/src/stake_history.rs @@ -15,6 +15,42 @@ pub struct StakeHistoryEntry { pub deactivating: u64, // requested to be cooled down, not fully deactivated yet } +impl StakeHistoryEntry { + pub fn with_effective(effective: u64) -> Self { + Self { + effective, + ..Self::default() + } + } + + pub fn with_effective_and_activating(effective: u64, activating: u64) -> Self { + Self { + effective, + activating, + ..Self::default() + } + } + + pub fn with_deactivating(deactivating: u64) -> Self { + Self { + effective: deactivating, + deactivating, + ..Self::default() + } + } +} + +impl std::ops::Add for StakeHistoryEntry { + type Output = StakeHistoryEntry; + fn add(self, rhs: StakeHistoryEntry) -> Self::Output { + Self { + effective: self.effective.saturating_add(rhs.effective), + activating: self.activating.saturating_add(rhs.activating), + deactivating: self.deactivating.saturating_add(rhs.deactivating), + } + } +} + #[repr(C)] #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Default, Clone, AbiExample)] pub struct StakeHistory(Vec<(Epoch, StakeHistoryEntry)>);