Fix bug of same-epoch stake deactivation after stake redelegation (#32606)

* fix stake deactivation in the same epoch after redelegation bug

add tests

refactor common code into fn

avoid early return

add feature gate for the new stake redelegate behavior

move stake tests out of cli

add stake-program-test crate

reimplemnt stake test with program-test

remove stake-program-test crate

reviews

add setup.rs

remove clippy

reveiws

* reviews

* review comments

---------

Co-authored-by: HaoranYi <haoran.yi@solana.com>
This commit is contained in:
HaoranYi 2023-09-25 16:35:40 -05:00 committed by GitHub
parent 23ad476ffb
commit d25d53e979
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 425 additions and 92 deletions

1
Cargo.lock generated
View File

@ -6587,6 +6587,7 @@ dependencies = [
"solana-sdk",
"solana-stake-program",
"solana-vote-program",
"test-case",
"thiserror",
"tokio",
]

View File

@ -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"] }

View File

@ -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()
}

193
program-test/tests/stake.rs Normal file
View File

@ -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();
}

View File

@ -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);

View File

@ -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"

View File

@ -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 {

View File

@ -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<Pubkey>,
) -> 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())

View File

@ -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,
}

View File

@ -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) {