From 3e89cb6b439bef804e2d7c3d55ab4a1416154b3f Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 21 May 2021 10:41:13 +0000 Subject: [PATCH] programs/stake: cancel deactivate (backport #17344) (#17375) * programs/stake: cancel deactivate (#17344) fix: remove stray println add error for inconsistent input. fix: lamports don't need to match when redelegating to same vote account Improve comments bump Apply suggestions from code review Add assert in test Use stake_program_v4 Co-Authored-By: Trent Nelson Co-authored-by: Trent Nelson (cherry picked from commit 662c2aaeec05b5f203fb1724693ad9b92b59f7ea) # Conflicts: # programs/stake/src/stake_instruction.rs # programs/stake/src/stake_state.rs * Fix conflicts Co-authored-by: jon-chuang <9093549+jon-chuang@users.noreply.github.com> Co-authored-by: Tyera Eulberg --- programs/stake/src/stake_instruction.rs | 3 + programs/stake/src/stake_state.rs | 134 +++++++++++++++++++++--- 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 83abcb02ce..b7fa8e3fbf 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -572,6 +572,8 @@ pub fn process_instruction( } } StakeInstruction::DelegateStake => { + let can_reverse_deactivation = + invoke_context.is_feature_active(&feature_set::stake_program_v4::id()); let vote = next_keyed_account(keyed_accounts)?; me.delegate( @@ -580,6 +582,7 @@ pub fn process_instruction( &from_keyed_account::(next_keyed_account(keyed_accounts)?)?, &config::from_keyed_account(next_keyed_account(keyed_accounts)?)?, &signers, + can_reverse_deactivation, ) } StakeInstruction::Split(lamports) => { diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 49ff658c03..06dc74905b 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -755,13 +755,28 @@ impl Stake { clock: &Clock, stake_history: &StakeHistory, config: &Config, + can_reverse_deactivation: bool, ) -> Result<(), StakeError> { - // can't redelegate if stake is active. either the stake - // is freshly activated or has fully de-activated. redelegation - // implies re-activation + // If stake is currently active: if self.stake(clock.epoch, Some(stake_history), true) != 0 { - return Err(StakeError::TooSoonToRedelegate); + // If pubkey of new voter is the same as current, + // and we are scheduled to start deactivating this epoch, + // we rescind deactivation + if self.delegation.voter_pubkey == *voter_pubkey + && clock.epoch == self.delegation.deactivation_epoch + && can_reverse_deactivation + { + self.delegation.deactivation_epoch = std::u64::MAX; + return Ok(()); + } else { + // can't redelegate to another pubkey if stake is active. + return Err(StakeError::TooSoonToRedelegate); + } } + // Either the stake is freshly activated, is active but has been + // deactivated this epoch, or has fully de-activated. + // Redelegation implies either re-activation or un-deactivation + self.delegation.stake = stake_lamports; self.delegation.activation_epoch = clock.epoch; self.delegation.deactivation_epoch = std::u64::MAX; @@ -852,6 +867,7 @@ pub trait StakeAccount { stake_history: &StakeHistory, config: &Config, signers: &HashSet, + can_reverse_deactivation: bool, ) -> Result<(), InstructionError>; fn deactivate(&self, clock: &Clock, signers: &HashSet) -> Result<(), InstructionError>; fn set_lockup( @@ -989,6 +1005,7 @@ impl<'a> StakeAccount for KeyedAccount<'a> { stake_history: &StakeHistory, config: &Config, signers: &HashSet, + can_reverse_deactivation: bool, ) -> Result<(), InstructionError> { if vote_account.owner()? != solana_vote_program::id() { return Err(InstructionError::IncorrectProgramId); @@ -1015,6 +1032,7 @@ impl<'a> StakeAccount for KeyedAccount<'a> { clock, stake_history, config, + can_reverse_deactivation, )?; self.set_state(&StakeState::Stake(meta, stake)) } @@ -1907,10 +1925,16 @@ mod tests { }; let vote_pubkey = solana_sdk::pubkey::new_rand(); + let vote_pubkey_2 = solana_sdk::pubkey::new_rand(); + let mut vote_state = VoteState::default(); for i in 0..1000 { vote_state.process_slot_vote_unchecked(i); } + let mut vote_state_2 = VoteState::default(); + for i in 0..1000 { + vote_state_2.process_slot_vote_unchecked(i); + } let vote_account = RefCell::new(vote_state::create_account( &vote_pubkey, @@ -1918,11 +1942,23 @@ mod tests { 0, 100, )); + let vote_account_2 = RefCell::new(vote_state::create_account( + &vote_pubkey_2, + &solana_sdk::pubkey::new_rand(), + 0, + 100, + )); let vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &vote_account); + let vote_keyed_account_2 = KeyedAccount::new(&vote_pubkey_2, false, &vote_account_2); + let vote_state_credits = vote_state.credits(); vote_keyed_account .set_state(&VoteStateVersions::new_current(vote_state)) .unwrap(); + let vote_state_credits_2 = vote_state_2.credits(); + vote_keyed_account_2 + .set_state(&VoteStateVersions::new_current(vote_state_2)) + .unwrap(); let stake_pubkey = solana_sdk::pubkey::new_rand(); let stake_lamports = 42; @@ -1951,6 +1987,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ), Err(InstructionError::MissingRequiredSignature) ); @@ -1965,6 +2002,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ) .is_ok()); @@ -1986,14 +2024,63 @@ mod tests { clock.epoch += 1; - // verify that delegate fails if stake is still active + // verify that delegate fails as stake is active and not deactivating assert_eq!( stake_keyed_account.delegate( &vote_keyed_account, &clock, &StakeHistory::default(), &Config::default(), - &signers + &signers, + true + ), + Err(StakeError::TooSoonToRedelegate.into()) + ); + + // deactivate + stake_keyed_account.deactivate(&clock, &signers).unwrap(); + + // verify that delegate to a different vote account fails + // during deactivation + assert_eq!( + stake_keyed_account.delegate( + &vote_keyed_account_2, + &clock, + &StakeHistory::default(), + &Config::default(), + &signers, + true, + ), + Err(StakeError::TooSoonToRedelegate.into()) + ); + + // verify that delegate succeeds to same vote account + // when stake is deactivating + stake_keyed_account + .delegate( + &vote_keyed_account, + &clock, + &StakeHistory::default(), + &Config::default(), + &signers, + true, + ) + .unwrap(); + + // verify that deactivation has been cleared + let stake = StakeState::stake_from(&stake_keyed_account.account.borrow()).unwrap(); + assert_eq!(stake.delegation.deactivation_epoch, std::u64::MAX); + + // verify that delegate to a different vote account fails + // if stake is still active + assert_eq!( + stake_keyed_account.delegate( + &vote_keyed_account_2, + &clock, + &StakeHistory::default(), + &Config::default(), + &signers, + true, ), Err(StakeError::TooSoonToRedelegate.into()) ); @@ -2004,21 +2091,23 @@ mod tests { // without stake history, cool down is instantaneous clock.epoch += 1; - // verify that delegate can be called twice, 2nd is redelegate + // verify that delegate can be called to new vote account, 2nd is redelegate assert!(stake_keyed_account .delegate( - &vote_keyed_account, + &vote_keyed_account_2, &clock, &StakeHistory::default(), &Config::default(), - &signers + &signers, + true, ) .is_ok()); // signed but faked vote account - let faked_vote_account = vote_account.clone(); + let faked_vote_account = vote_account_2.clone(); faked_vote_account.borrow_mut().owner = solana_sdk::pubkey::new_rand(); - let faked_vote_keyed_account = KeyedAccount::new(&vote_pubkey, false, &faked_vote_account); + let faked_vote_keyed_account = + KeyedAccount::new(&vote_pubkey_2, false, &faked_vote_account); assert_eq!( stake_keyed_account.delegate( &faked_vote_keyed_account, @@ -2026,6 +2115,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ), Err(solana_sdk::instruction::InstructionError::IncorrectProgramId) ); @@ -2036,13 +2126,13 @@ mod tests { stake, Stake { delegation: Delegation { - voter_pubkey: vote_pubkey, + voter_pubkey: vote_pubkey_2, stake: stake_lamports, activation_epoch: clock.epoch, deactivation_epoch: std::u64::MAX, ..Delegation::default() }, - credits_observed: vote_state_credits, + credits_observed: vote_state_credits_2, } ); @@ -2056,7 +2146,8 @@ mod tests { &clock, &StakeHistory::default(), &Config::default(), - &signers + &signers, + true, ) .is_err()); } @@ -2832,7 +2923,8 @@ mod tests { &clock, &StakeHistory::default(), &Config::default(), - &signers + &signers, + true, ), Ok(()) ); @@ -2925,6 +3017,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &vec![stake_pubkey].into_iter().collect(), + true, ) .unwrap(); @@ -3171,6 +3264,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ), Ok(()) ); @@ -3347,6 +3441,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ), Ok(()) ); @@ -5923,6 +6018,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &signers, + true, ) .unwrap(); @@ -5970,6 +6066,7 @@ mod tests { &StakeHistory::default(), &Config::default(), &other_signers, + true, ), Err(InstructionError::MissingRequiredSignature) ); @@ -5982,7 +6079,8 @@ mod tests { &clock, &StakeHistory::default(), &Config::default(), - &new_signers + &new_signers, + true, ), Ok(()) ); @@ -6035,6 +6133,7 @@ mod tests { &stake_history, &config, &signers, + true, ) .unwrap(); @@ -6042,6 +6141,7 @@ mod tests { stake_keyed_account.deactivate(&clock, &signers).unwrap(); clock.epoch += 1; + // Once deactivated, we withdraw stake to new keyed account let to = Pubkey::new_unique(); let to_account = AccountSharedData::new_ref(1, 0, &system_program::id()); let to_keyed_account = KeyedAccount::new(&to, false, &to_account); @@ -6068,6 +6168,7 @@ mod tests { &stake_history, &config, &signers, + true, ) .unwrap(); let stake = StakeState::stake_from(&stake_account.borrow()).unwrap(); @@ -6090,6 +6191,7 @@ mod tests { &stake_history, &config, &signers, + true, ) .unwrap(); let stake = StakeState::stake_from(&stake_account.borrow()).unwrap();