diff --git a/Cargo.lock b/Cargo.lock index 7218a80cb1..11b687ef77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6587,6 +6587,7 @@ dependencies = [ "solana-sdk", "solana-stake-program", "solana-vote-program", + "test-case", "thiserror", "tokio", ] diff --git a/program-test/Cargo.toml b/program-test/Cargo.toml index 07e05e88fb..87a9c88487 100644 --- a/program-test/Cargo.toml +++ b/program-test/Cargo.toml @@ -27,6 +27,7 @@ solana-program-runtime = { workspace = true } solana-runtime = { workspace = true } solana-sdk = { workspace = true } solana-vote-program = { workspace = true } +test-case = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["full"] } diff --git a/program-test/tests/setup.rs b/program-test/tests/setup.rs new file mode 100644 index 0000000000..60c42372b6 --- /dev/null +++ b/program-test/tests/setup.rs @@ -0,0 +1,89 @@ +use { + solana_program_test::ProgramTestContext, + solana_sdk::{ + pubkey::Pubkey, + rent::Rent, + signature::{Keypair, Signer}, + stake::{ + instruction as stake_instruction, + state::{Authorized, Lockup}, + }, + system_instruction, system_program, + transaction::Transaction, + }, + solana_vote_program::{ + vote_instruction, + vote_state::{self, VoteInit, VoteState}, + }, +}; + +pub async fn setup_stake( + context: &mut ProgramTestContext, + user: &Keypair, + vote_address: &Pubkey, + stake_lamports: u64, +) -> Pubkey { + let stake_keypair = Keypair::new(); + let transaction = Transaction::new_signed_with_payer( + &stake_instruction::create_account_and_delegate_stake( + &context.payer.pubkey(), + &stake_keypair.pubkey(), + vote_address, + &Authorized::auto(&user.pubkey()), + &Lockup::default(), + stake_lamports, + ), + Some(&context.payer.pubkey()), + &vec![&context.payer, &stake_keypair, user], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + stake_keypair.pubkey() +} + +pub async fn setup_vote(context: &mut ProgramTestContext) -> Pubkey { + let mut instructions = vec![]; + let validator_keypair = Keypair::new(); + instructions.push(system_instruction::create_account( + &context.payer.pubkey(), + &validator_keypair.pubkey(), + Rent::default().minimum_balance(0), + 0, + &system_program::id(), + )); + let vote_lamports = Rent::default().minimum_balance(VoteState::size_of()); + let vote_keypair = Keypair::new(); + let user_keypair = Keypair::new(); + instructions.append(&mut vote_instruction::create_account_with_config( + &context.payer.pubkey(), + &vote_keypair.pubkey(), + &VoteInit { + node_pubkey: validator_keypair.pubkey(), + authorized_voter: user_keypair.pubkey(), + ..VoteInit::default() + }, + vote_lamports, + vote_instruction::CreateVoteAccountConfig { + space: vote_state::VoteStateVersions::vote_state_size_of(true) as u64, + ..vote_instruction::CreateVoteAccountConfig::default() + }, + )); + + let transaction = Transaction::new_signed_with_payer( + &instructions, + Some(&context.payer.pubkey()), + &vec![&context.payer, &validator_keypair, &vote_keypair], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + vote_keypair.pubkey() +} diff --git a/program-test/tests/stake.rs b/program-test/tests/stake.rs new file mode 100644 index 0000000000..1ad7a756b3 --- /dev/null +++ b/program-test/tests/stake.rs @@ -0,0 +1,193 @@ +#![allow(clippy::arithmetic_side_effects)] + +mod setup; + +use { + setup::{setup_stake, setup_vote}, + solana_program_test::ProgramTest, + solana_sdk::{ + instruction::InstructionError, + signature::{Keypair, Signer}, + stake::{instruction as stake_instruction, instruction::StakeError}, + transaction::{Transaction, TransactionError}, + }, + test_case::test_case, +}; + +#[derive(PartialEq)] +enum PendingStakeActivationTestFlag { + MergeActive, + MergeInactive, + NoMerge, +} + +#[test_case(PendingStakeActivationTestFlag::NoMerge; "test that redelegate stake then deactivate it then withdraw from it is not permitted")] +#[test_case(PendingStakeActivationTestFlag::MergeActive; "test that redelegate stake then merge it with another active stake then deactivate it then withdraw from it is not permitted")] +#[test_case(PendingStakeActivationTestFlag::MergeInactive; "test that redelegate stake then merge it with another inactive stake then deactivate it then withdraw from it is not permitted")] +#[tokio::test] +async fn test_stake_redelegation_pending_activation(merge_flag: PendingStakeActivationTestFlag) { + let program_test = ProgramTest::default(); + let mut context = program_test.start_with_context().await; + + // 1. create first vote accounts + context.warp_to_slot(100).unwrap(); + let vote_address = setup_vote(&mut context).await; + + // 1.1 advance to normal epoch + let first_normal_slot = context.genesis_config().epoch_schedule.first_normal_slot; + let slots_per_epoch = context.genesis_config().epoch_schedule.slots_per_epoch; + let mut current_slot = first_normal_slot + slots_per_epoch; + context.warp_to_slot(current_slot).unwrap(); + context.warp_forward_force_reward_interval_end().unwrap(); + + // 2. create first stake account and delegate to first vote_address + let stake_lamports = 50_000_000_000; + let user_keypair = Keypair::new(); + let stake_address = + setup_stake(&mut context, &user_keypair, &vote_address, stake_lamports).await; + + // 2.1 advance to new epoch so that the stake is activated. + current_slot += slots_per_epoch; + context.warp_to_slot(current_slot).unwrap(); + context.warp_forward_force_reward_interval_end().unwrap(); + + // 2.2 stake is now activated and can't withdrawal directly + let transaction = Transaction::new_signed_with_payer( + &[stake_instruction::withdraw( + &stake_address, + &user_keypair.pubkey(), + &solana_sdk::pubkey::new_rand(), + 1, + None, + )], + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + let r = context.banks_client.process_transaction(transaction).await; + assert_eq!( + r.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::InsufficientFunds) + ); + + // 3. create 2nd vote account + let vote_address2 = setup_vote(&mut context).await; + + // 3.1 relegate stake account to 2nd vote account, which creates 2nd stake account + let stake_keypair2 = Keypair::new(); + let stake_address2 = stake_keypair2.pubkey(); + let transaction = Transaction::new_signed_with_payer( + &stake_instruction::redelegate( + &stake_address, + &user_keypair.pubkey(), + &vote_address2, + &stake_address2, + ), + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair, &stake_keypair2], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + if merge_flag != PendingStakeActivationTestFlag::NoMerge { + // 3.2 create 3rd to-merge stake account + let stake_address3 = + setup_stake(&mut context, &user_keypair, &vote_address2, stake_lamports).await; + + // 3.2.1 deactivate merge stake account + if merge_flag == PendingStakeActivationTestFlag::MergeInactive { + let transaction = Transaction::new_signed_with_payer( + &[stake_instruction::deactivate_stake( + &stake_address3, + &user_keypair.pubkey(), + )], + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + } + + // 3.2.2 merge 3rd stake account to 2nd stake account. However, it should not clear the pending stake activation flags on stake_account2. + let transaction = Transaction::new_signed_with_payer( + &stake_instruction::merge(&stake_address2, &stake_address3, &user_keypair.pubkey()), + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + } + + // 3.3 deactivate 2nd stake account should fail because of pending stake activation. + let transaction = Transaction::new_signed_with_payer( + &[stake_instruction::deactivate_stake( + &stake_address2, + &user_keypair.pubkey(), + )], + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + let r = context.banks_client.process_transaction(transaction).await; + assert_eq!( + r.unwrap_err().unwrap(), + TransactionError::InstructionError( + 0, + InstructionError::Custom( + StakeError::RedelegatedStakeMustFullyActivateBeforeDeactivationIsPermitted as u32 + ) + ) + ); + + // 3.4 withdraw from 2nd stake account should also fail because of pending stake activation. + let transaction = Transaction::new_signed_with_payer( + &[stake_instruction::withdraw( + &stake_address2, + &user_keypair.pubkey(), + &solana_sdk::pubkey::new_rand(), + 1, + None, + )], + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + let r = context.banks_client.process_transaction(transaction).await; + assert_eq!( + r.unwrap_err().unwrap(), + TransactionError::InstructionError(0, InstructionError::InsufficientFunds) + ); + + // 4. advance to new epoch so that the 2nd stake account is fully activated + current_slot += slots_per_epoch; + context.warp_to_slot(current_slot).unwrap(); + context.warp_forward_force_reward_interval_end().unwrap(); + + // 4.1 Now deactivate 2nd stake account should succeed because there is no pending stake activation. + let transaction = Transaction::new_signed_with_payer( + &[stake_instruction::deactivate_stake( + &stake_address2, + &user_keypair.pubkey(), + )], + Some(&context.payer.pubkey()), + &vec![&context.payer, &user_keypair], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); +} diff --git a/program-test/tests/warp.rs b/program-test/tests/warp.rs index 94f497a98f..da0b632ad6 100644 --- a/program-test/tests/warp.rs +++ b/program-test/tests/warp.rs @@ -1,11 +1,13 @@ #![allow(clippy::arithmetic_side_effects)] + +mod setup; + use { bincode::deserialize, log::debug, + setup::{setup_stake, setup_vote}, solana_banks_client::BanksClient, - solana_program_test::{ - processor, ProgramTest, ProgramTestBanksClientExt, ProgramTestContext, ProgramTestError, - }, + solana_program_test::{processor, ProgramTest, ProgramTestBanksClientExt, ProgramTestError}, solana_sdk::{ account::Account, account_info::{next_account_info, AccountInfo}, @@ -18,9 +20,8 @@ use { signature::{Keypair, Signer}, stake::{ instruction as stake_instruction, - state::{Authorized, Lockup, StakeActivationStatus, StakeStateV2}, + state::{StakeActivationStatus, StakeStateV2}, }, - system_instruction, system_program, sysvar::{ clock, stake_history::{self, StakeHistory}, @@ -29,89 +30,13 @@ use { transaction::{Transaction, TransactionError}, }, solana_stake_program::stake_state, - solana_vote_program::{ - vote_instruction, - vote_state::{self, VoteInit, VoteState}, - }, + solana_vote_program::vote_state, std::convert::TryInto, }; // Use a big number to be sure that we get the right error const WRONG_SLOT_ERROR: u32 = 123456; -async fn setup_stake( - context: &mut ProgramTestContext, - user: &Keypair, - vote_address: &Pubkey, - stake_lamports: u64, -) -> Pubkey { - let stake_keypair = Keypair::new(); - let transaction = Transaction::new_signed_with_payer( - &stake_instruction::create_account_and_delegate_stake( - &context.payer.pubkey(), - &stake_keypair.pubkey(), - vote_address, - &Authorized::auto(&user.pubkey()), - &Lockup::default(), - stake_lamports, - ), - Some(&context.payer.pubkey()), - &vec![&context.payer, &stake_keypair, user], - context.last_blockhash, - ); - context - .banks_client - .process_transaction(transaction) - .await - .unwrap(); - stake_keypair.pubkey() -} - -async fn setup_vote(context: &mut ProgramTestContext) -> Pubkey { - // warp once to make sure stake config doesn't get rent-collected - context.warp_to_slot(100).unwrap(); - let mut instructions = vec![]; - let validator_keypair = Keypair::new(); - instructions.push(system_instruction::create_account( - &context.payer.pubkey(), - &validator_keypair.pubkey(), - Rent::default().minimum_balance(0), - 0, - &system_program::id(), - )); - let vote_lamports = Rent::default().minimum_balance(VoteState::size_of()); - let vote_keypair = Keypair::new(); - let user_keypair = Keypair::new(); - instructions.append(&mut vote_instruction::create_account_with_config( - &context.payer.pubkey(), - &vote_keypair.pubkey(), - &VoteInit { - node_pubkey: validator_keypair.pubkey(), - authorized_voter: user_keypair.pubkey(), - ..VoteInit::default() - }, - vote_lamports, - vote_instruction::CreateVoteAccountConfig { - space: vote_state::VoteStateVersions::vote_state_size_of(true) as u64, - ..vote_instruction::CreateVoteAccountConfig::default() - }, - )); - - let transaction = Transaction::new_signed_with_payer( - &instructions, - Some(&context.payer.pubkey()), - &vec![&context.payer, &validator_keypair, &vote_keypair], - context.last_blockhash, - ); - context - .banks_client - .process_transaction(transaction) - .await - .unwrap(); - - vote_keypair.pubkey() -} - fn process_instruction( _program_id: &Pubkey, accounts: &[AccountInfo], @@ -214,6 +139,8 @@ async fn stake_rewards_from_warp() { // Initialize and start the test network let program_test = ProgramTest::default(); let mut context = program_test.start_with_context().await; + + context.warp_to_slot(100).unwrap(); let vote_address = setup_vote(&mut context).await; let user_keypair = Keypair::new(); @@ -415,11 +342,12 @@ async fn check_credits_observed( expected_credits ); } - #[tokio::test] async fn stake_merge_immediately_after_activation() { let program_test = ProgramTest::default(); let mut context = program_test.start_with_context().await; + + context.warp_to_slot(100).unwrap(); let vote_address = setup_vote(&mut context).await; context.increment_vote_account_credits(&vote_address, 100); diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 8863bef090..d6fbe9f6f7 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5331,6 +5331,7 @@ dependencies = [ "solana-runtime", "solana-sdk", "solana-vote-program", + "test-case", "thiserror", "tokio", ] @@ -6828,6 +6829,41 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "test-case" +version = "3.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8f1e820b7f1d95a0cdbf97a5df9de10e1be731983ab943e56703ac1b8e9d425" +dependencies = [ + "test-case-macros", +] + +[[package]] +name = "test-case-core" +version = "3.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54c25e2cb8f5fcd7318157634e8838aa6f7e4715c96637f969fabaccd1ef5462" +dependencies = [ + "cfg-if 1.0.0", + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.37", +] + +[[package]] +name = "test-case-macros" +version = "3.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37cfd7bbc88a0104e304229fba519bdc45501a30b760fb72240342f1289ad257" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.37", + "test-case-core", +] + [[package]] name = "textwrap" version = "0.11.0" diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 6cf5f00074..20b6c9e0eb 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -267,7 +267,7 @@ declare_process_instruction!( let mut me = get_stake_account()?; let clock = get_sysvar_with_account_check::clock(invoke_context, instruction_context, 1)?; - deactivate(&mut me, &clock, &signers) + deactivate(invoke_context, &mut me, &clock, &signers) } Ok(StakeInstruction::SetLockup(lockup)) => { let mut me = get_stake_account()?; @@ -413,6 +413,7 @@ declare_process_instruction!( let clock = invoke_context.get_sysvar_cache().get_clock()?; deactivate_delinquent( + invoke_context, transaction_context, instruction_context, &mut me, @@ -7198,6 +7199,10 @@ mod tests { ..Clock::default() }), ), + ( + stake_history::id(), + create_account_shared_data_for_test(&StakeHistory::default()), + ), ], vec![ AccountMeta { diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 964d2d6ffc..c533728b11 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -633,15 +633,56 @@ pub fn delegate( } } +fn deactivate_stake( + invoke_context: &InvokeContext, + stake: &mut Stake, + stake_flags: &mut StakeFlags, + epoch: Epoch, +) -> Result<(), InstructionError> { + if invoke_context + .feature_set + .is_active(&feature_set::stake_redelegate_instruction::id()) + { + if stake_flags.contains(StakeFlags::MUST_FULLY_ACTIVATE_BEFORE_DEACTIVATION_IS_PERMITTED) { + let stake_history = invoke_context.get_sysvar_cache().get_stake_history()?; + // when MUST_FULLY_ACTIVATE_BEFORE_DEACTIVATION_IS_PERMITTED flag is set on stake_flags, + // deactivation is only permitted when the stake delegation activating amount is zero. + let status = stake.delegation.stake_activating_and_deactivating( + epoch, + Some(stake_history.as_ref()), + new_warmup_cooldown_rate_epoch(invoke_context), + ); + if status.activating != 0 { + Err(InstructionError::from( + StakeError::RedelegatedStakeMustFullyActivateBeforeDeactivationIsPermitted, + )) + } else { + stake.deactivate(epoch)?; + // After deactivation, need to clear `MustFullyActivateBeforeDeactivationIsPermitted` flag if any. + // So that future activation and deactivation are not subject to that restriction. + stake_flags + .remove(StakeFlags::MUST_FULLY_ACTIVATE_BEFORE_DEACTIVATION_IS_PERMITTED); + Ok(()) + } + } else { + stake.deactivate(epoch)?; + Ok(()) + } + } else { + stake.deactivate(epoch)?; + Ok(()) + } +} + pub fn deactivate( + invoke_context: &InvokeContext, stake_account: &mut BorrowedAccount, clock: &Clock, signers: &HashSet, ) -> Result<(), InstructionError> { - if let StakeStateV2::Stake(meta, mut stake, stake_flags) = stake_account.get_state()? { + if let StakeStateV2::Stake(meta, mut stake, mut stake_flags) = stake_account.get_state()? { meta.authorized.check(signers, StakeAuthorize::Staker)?; - stake.deactivate(clock.epoch)?; - + deactivate_stake(invoke_context, &mut stake, &mut stake_flags, clock.epoch)?; stake_account.set_state(&StakeStateV2::Stake(meta, stake, stake_flags)) } else { Err(InstructionError::InvalidAccountData) @@ -975,7 +1016,7 @@ pub fn redelegate( // deactivate `stake_account` // // Note: This function also ensures `signers` contains the `StakeAuthorize::Staker` - deactivate(stake_account, &clock, signers)?; + deactivate(invoke_context, stake_account, &clock, signers)?; // transfer the effective stake to the uninitialized stake account stake_account.checked_sub_lamports(effective_stake)?; @@ -1001,7 +1042,7 @@ pub fn redelegate( &vote_state.convert_to_current(), clock.epoch, ), - StakeFlags::empty(), + StakeFlags::MUST_FULLY_ACTIVATE_BEFORE_DEACTIVATION_IS_PERMITTED, ))?; Ok(()) @@ -1115,6 +1156,7 @@ pub fn withdraw( } pub(crate) fn deactivate_delinquent( + invoke_context: &InvokeContext, transaction_context: &TransactionContext, instruction_context: &InstructionContext, stake_account: &mut BorrowedAccount, @@ -1148,7 +1190,7 @@ pub(crate) fn deactivate_delinquent( return Err(StakeError::InsufficientReferenceVotes.into()); } - if let StakeStateV2::Stake(meta, mut stake, stake_flags) = stake_account.get_state()? { + if let StakeStateV2::Stake(meta, mut stake, mut stake_flags) = stake_account.get_state()? { if stake.delegation.voter_pubkey != *delinquent_vote_account_pubkey { return Err(StakeError::VoteAddressMismatch.into()); } @@ -1156,7 +1198,7 @@ pub(crate) fn deactivate_delinquent( // Deactivate the stake account if its delegated vote account has never voted or has not // voted in the last `MINIMUM_DELINQUENT_EPOCHS_FOR_DEACTIVATION` if eligible_for_deactivate_delinquent(&delinquent_vote_state.epoch_credits, current_epoch) { - stake.deactivate(current_epoch)?; + deactivate_stake(invoke_context, &mut stake, &mut stake_flags, current_epoch)?; stake_account.set_state(&StakeStateV2::Stake(meta, stake, stake_flags)) } else { Err(StakeError::MinimumDelinquentEpochsForDeactivationNotMet.into()) diff --git a/sdk/program/src/stake/instruction.rs b/sdk/program/src/stake/instruction.rs index 89db8e96be..9b964f0eee 100644 --- a/sdk/program/src/stake/instruction.rs +++ b/sdk/program/src/stake/instruction.rs @@ -68,7 +68,6 @@ pub enum StakeError { #[error("stake redelegation to the same vote account is not permitted")] RedelegateToSameVoteAccount, - #[allow(dead_code)] #[error("redelegated stake must be fully activated before deactivation")] RedelegatedStakeMustFullyActivateBeforeDeactivationIsPermitted, } diff --git a/sdk/program/src/stake/state.rs b/sdk/program/src/stake/state.rs index 969e5f2cef..4f94f73b3f 100644 --- a/sdk/program/src/stake/state.rs +++ b/sdk/program/src/stake/state.rs @@ -823,6 +823,45 @@ mod test { ); } + #[test] + fn stake_flag_member_offset() { + const FLAG_OFFSET: usize = 196; + let check_flag = |flag, expected| { + let stake = StakeStateV2::Stake( + Meta { + rent_exempt_reserve: 1, + authorized: Authorized { + staker: Pubkey::new_unique(), + withdrawer: Pubkey::new_unique(), + }, + lockup: Lockup::default(), + }, + Stake { + delegation: Delegation { + voter_pubkey: Pubkey::new_unique(), + stake: u64::MAX, + activation_epoch: Epoch::MAX, + deactivation_epoch: Epoch::MAX, + warmup_cooldown_rate: f64::MAX, + }, + credits_observed: 1, + }, + flag, + ); + + let bincode_serialized = serialize(&stake).unwrap(); + let borsh_serialized = StakeStateV2::try_to_vec(&stake).unwrap(); + + assert_eq!(bincode_serialized[FLAG_OFFSET], expected); + assert_eq!(borsh_serialized[FLAG_OFFSET], expected); + }; + check_flag( + StakeFlags::MUST_FULLY_ACTIVATE_BEFORE_DEACTIVATION_IS_PERMITTED, + 1, + ); + check_flag(StakeFlags::empty(), 0); + } + mod deprecated { use super::*; fn check_borsh_deserialization(stake: StakeState) {