Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ solana-sdk-ids = "3.0.0"
solana-signature = "3.0.0"
solana-signer = "3.0.0"
solana-svm-log-collector = "3.0.0"
solana-stake-client = { path = "../clients/rust" }
solana-system-interface = { version = "2.0.0", features = ["bincode"] }
solana-transaction = "3.0.0"
test-case = "3.3.1"
Expand Down
128 changes: 128 additions & 0 deletions program/tests/deactivate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
#![allow(clippy::arithmetic_side_effects)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not covered via the parent mod.s allow?


mod helpers;

use {
helpers::{context::StakeTestContext, lifecycle::StakeLifecycle},
mollusk_svm::result::Check,
solana_account::ReadableAccount,
solana_program_error::ProgramError,
solana_pubkey::Pubkey,
solana_stake_client::instructions::{DeactivateBuilder, DelegateStakeBuilder},
solana_stake_interface::{error::StakeError, state::StakeStateV2},
solana_stake_program::id,
test_case::test_case,
};

#[test_case(false; "activating")]
#[test_case(true; "active")]
fn test_deactivate(activate: bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but when we later migrate this instruction, I think it would help to revisit these tests and likely break them up into specific cases versus a single test with multiple inside.

let mut ctx = StakeTestContext::with_delegation();
let min_delegation = ctx.minimum_delegation.unwrap();
let rent_exempt_reserve = ctx.rent_exempt_reserve;
let staker = ctx.staker;
let withdrawer = ctx.withdrawer;

let (stake, mut stake_account) = ctx
.stake_account(StakeLifecycle::Initialized)
.staked_amount(min_delegation)
.build();

let (vote, vote_account_data) = ctx.vote_account.clone().unwrap();

// Deactivating an undelegated account fails
ctx.checks(&[Check::err(ProgramError::InvalidAccountData)])
.test_missing_signers(false)
.execute(
DeactivateBuilder::new()
.stake(stake)
.stake_authority(staker)
.instruction(),
Comment on lines +36 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had per-instruction builders, we could essentially could pull all of the work passing ctx defaults into the instructions to be internal to the builder. If we did that, I think we could make individual tests like 50% smaller. Maybe I am still enamored by the token-wrap pattern of having tests like this small:

#[test]
fn test_incorrect_escrow_owner() {
    let incorrect_escrow_owner = Pubkey::new_unique();
    WrapBuilder::default()
        .unwrapped_escrow_owner(incorrect_escrow_owner)
        .check(Check::err(TokenWrapError::EscrowOwnerMismatch.into()))
        .execute();
}

&[(&stake, &stake_account)],
);

// Delegate
let result = ctx.execute(
DelegateStakeBuilder::new()
.stake(stake)
.vote(vote)
.unused(Pubkey::new_unique())
.stake_authority(staker)
.instruction(),
&[(&stake, &stake_account), (&vote, &vote_account_data)],
);
stake_account = result.resulting_accounts[0].1.clone().into();

if activate {
// Advance epoch to activate
let current_slot = ctx.mollusk.sysvars.clock.slot;
let slots_per_epoch = ctx.mollusk.sysvars.epoch_schedule.slots_per_epoch;
ctx.mollusk.warp_to_slot(current_slot + slots_per_epoch);
}

// Deactivate with withdrawer fails
ctx.checks(&[Check::err(ProgramError::MissingRequiredSignature)])
.test_missing_signers(false)
.execute(
DeactivateBuilder::new()
.stake(stake)
.stake_authority(withdrawer)
.instruction(),
&[(&stake, &stake_account)],
);

// Deactivate succeeds
let result = ctx
.checks(&[
Check::success(),
Check::all_rent_exempt(),
Check::account(&stake)
.lamports(rent_exempt_reserve + min_delegation)
.owner(&id())
.space(StakeStateV2::size_of())
.build(),
])
.execute(
DeactivateBuilder::new()
.stake(stake)
.stake_authority(staker)
.instruction(),
&[(&stake, &stake_account)],
);
stake_account = result.resulting_accounts[0].1.clone().into();

let clock = ctx.mollusk.sysvars.clock.clone();
let stake_state: StakeStateV2 = bincode::deserialize(stake_account.data()).unwrap();
if let StakeStateV2::Stake(_, stake_data, _) = stake_state {
assert_eq!(stake_data.delegation.deactivation_epoch, clock.epoch);
} else {
panic!("Expected StakeStateV2::Stake");
}

// Deactivate again fails
ctx.checks(&[Check::err(StakeError::AlreadyDeactivated.into())])
.test_missing_signers(false)
.execute(
DeactivateBuilder::new()
.stake(stake)
.stake_authority(staker)
.instruction(),
&[(&stake, &stake_account)],
);

// Advance epoch
let current_slot = ctx.mollusk.sysvars.clock.slot;
let slots_per_epoch = ctx.mollusk.sysvars.epoch_schedule.slots_per_epoch;
ctx.mollusk.warp_to_slot(current_slot + slots_per_epoch);

// Deactivate again still fails
ctx.checks(&[Check::err(StakeError::AlreadyDeactivated.into())])
.test_missing_signers(false)
.execute(
DeactivateBuilder::new()
.stake(stake)
.stake_authority(staker)
.instruction(),
&[(&stake, &stake_account)],
);
}
238 changes: 238 additions & 0 deletions program/tests/helpers/context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
use {
super::{
execution::ExecutionWithChecks,
lifecycle::StakeLifecycle,
utils::{add_sysvars, create_vote_account, STAKE_RENT_EXEMPTION},
},
mollusk_svm::{result::Check, Mollusk},
solana_account::AccountSharedData,
solana_instruction::Instruction,
solana_program_error::ProgramError,
solana_pubkey::Pubkey,
solana_stake_interface::state::Lockup,
solana_stake_program::id,
};

/// Builder for creating stake accounts with customizable parameters
pub struct StakeAccountBuilder<'a> {
ctx: &'a mut StakeTestContext,
lifecycle: StakeLifecycle,
staked_amount: u64,
stake_authority: Option<Pubkey>,
withdraw_authority: Option<Pubkey>,
lockup: Option<Lockup>,
vote_account: Option<Pubkey>,
stake_pubkey: Option<Pubkey>,
}

impl StakeAccountBuilder<'_> {
/// Set the staked amount (lamports delegated to validator)
pub fn staked_amount(mut self, amount: u64) -> Self {
self.staked_amount = amount;
self
}

/// Set a custom stake authority (defaults to ctx.staker)
pub fn stake_authority(mut self, authority: &Pubkey) -> Self {
self.stake_authority = Some(*authority);
self
}

/// Set a custom withdraw authority (defaults to ctx.withdrawer)
pub fn withdraw_authority(mut self, authority: &Pubkey) -> Self {
self.withdraw_authority = Some(*authority);
self
}

/// Set a custom lockup (defaults to Lockup::default())
pub fn lockup(mut self, lockup: &Lockup) -> Self {
self.lockup = Some(*lockup);
self
}

/// Set a custom vote account (defaults to ctx.vote_account)
pub fn vote_account(mut self, vote_account: &Pubkey) -> Self {
self.vote_account = Some(*vote_account);
self
}

/// Set a specific stake account pubkey (defaults to Pubkey::new_unique())
pub fn stake_pubkey(mut self, pubkey: &Pubkey) -> Self {
self.stake_pubkey = Some(*pubkey);
self
}

/// Build the stake account and return (pubkey, account_data)
pub fn build(self) -> (Pubkey, AccountSharedData) {
let stake_pubkey = self.stake_pubkey.unwrap_or_else(Pubkey::new_unique);
let vote_account_ref = self.vote_account.as_ref().unwrap_or_else(|| {
self.ctx
.vote_account
.as_ref()
.map(|(pk, _)| pk)
.expect("vote_account required for this lifecycle")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of gives me a feeling that we aren't using the type system enough if it's possible to run a test with an incorrect setup state. Some ideas...

What about instead of choosing a lifecycle up front, you start uninitialized and transition via methods:

let stake = ctx
  .stake()                 // -> StakeBuilder<Uninitialized>
  .fund(amount)
  .initialize(auth, lockup) // -> StakeBuilder<Initialized>
  .delegate(vote)           // requires vote here
  .activate_epoch();        // -> StakeBuilder<Active>

or have builders for the different lifecycles:

ctx.uninitialized_stake()
ctx.initialized_stake().lockup(...)
ctx.activating_stake(vote).staked_amount(...)
ctx.active_stake(vote).staked_amount(...)

But in general I feel like we may be trying to serve all instruction case parameters at once in the current design.

});
let account = self.lifecycle.create_stake_account_fully_specified(
&mut self.ctx.mollusk,
&stake_pubkey,
vote_account_ref,
self.staked_amount,
self.stake_authority.as_ref().unwrap_or(&self.ctx.staker),
self.withdraw_authority
.as_ref()
.unwrap_or(&self.ctx.withdrawer),
self.lockup.as_ref().unwrap_or(&Lockup::default()),
);
(stake_pubkey, account)
}
}

pub struct StakeTestContext {
pub mollusk: Mollusk,
pub rent_exempt_reserve: u64,
pub staker: Pubkey,
pub withdrawer: Pubkey,
pub minimum_delegation: Option<u64>,
/// Vote account (pubkey, account_data) for delegation tests
pub vote_account: Option<(Pubkey, AccountSharedData)>,
}

impl StakeTestContext {
pub fn new() -> Self {
let mollusk = Mollusk::new(&id(), "solana_stake_program");
Self {
mollusk,
rent_exempt_reserve: STAKE_RENT_EXEMPTION,
staker: Pubkey::new_unique(),
withdrawer: Pubkey::new_unique(),
minimum_delegation: None,
vote_account: None,
}
}

pub fn with_delegation() -> Self {
let mollusk = Mollusk::new(&id(), "solana_stake_program");
let minimum_delegation = solana_stake_program::get_minimum_delegation();
Self {
mollusk,
rent_exempt_reserve: STAKE_RENT_EXEMPTION,
staker: Pubkey::new_unique(),
withdrawer: Pubkey::new_unique(),
minimum_delegation: Some(minimum_delegation),
vote_account: Some((Pubkey::new_unique(), create_vote_account())),
}
}

/// Create a stake account builder for the specified lifecycle stage
///
/// Example:
/// ```
/// let (stake, account) = ctx
/// .stake_account(StakeLifecycle::Active)
/// .staked_amount(1_000_000)
/// .stake_account(StakeLifecycle::Active)
/// .staked_amount(1_000_000)
/// .build();
/// ```
pub fn stake_account(&mut self, lifecycle: StakeLifecycle) -> StakeAccountBuilder<'_> {
StakeAccountBuilder {
ctx: self,
lifecycle,
staked_amount: 0,
stake_authority: None,
withdraw_authority: None,
lockup: None,
vote_account: None,
stake_pubkey: None,
}
}

/// Configure execution with specific checks, then call .execute(instruction, accounts)
///
/// Usage: `ctx.checks(&checks).execute(instruction, accounts)`
pub fn checks<'a, 'b>(&'a mut self, checks: &'b [Check<'b>]) -> ExecutionWithChecks<'a, 'b> {
ExecutionWithChecks::new(self, checks)
}

/// Create a lockup that expires in the future
pub fn create_future_lockup(&self, epochs_ahead: u64) -> Lockup {
Lockup {
unix_timestamp: 0,
epoch: self.mollusk.sysvars.clock.epoch + epochs_ahead,
custodian: Pubkey::new_unique(),
}
}

/// Create a lockup that's currently in force (far future)
pub fn create_in_force_lockup(&self) -> Lockup {
self.create_future_lockup(1_000_000)
}

/// Create a second vote account (for testing different vote accounts)
pub fn create_second_vote_account(&self) -> (Pubkey, AccountSharedData) {
(Pubkey::new_unique(), create_vote_account())
}

/// Execute an instruction with default success checks and missing signer testing
///
/// Usage: `ctx.execute(instruction, accounts)`
pub fn execute(
&mut self,
instruction: Instruction,
accounts: &[(&Pubkey, &AccountSharedData)],
) -> mollusk_svm::result::InstructionResult {
self.execute_internal(instruction, accounts, &[Check::success()], true)
}

/// Internal: execute with given checks and current config
pub(crate) fn execute_internal(
&mut self,
instruction: Instruction,
accounts: &[(&Pubkey, &AccountSharedData)],
checks: &[Check],
test_missing_signers: bool,
) -> mollusk_svm::result::InstructionResult {
let accounts_vec: Vec<(Pubkey, AccountSharedData)> = accounts
.iter()
.map(|(pk, data)| (**pk, (*data).clone()))
.collect();

if test_missing_signers {
verify_all_signers_required(&self.mollusk, &instruction, &accounts_vec);
}

// Process with all signers present
let accounts_with_sysvars = add_sysvars(&self.mollusk, &instruction, accounts_vec);
self.mollusk
.process_and_validate_instruction(&instruction, &accounts_with_sysvars, checks)
}
}

impl Default for StakeTestContext {
fn default() -> Self {
Self::with_delegation()
}
}

/// Verify that removing any signer from the instruction causes MissingRequiredSignature error
fn verify_all_signers_required(
Comment on lines +217 to +218
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this is cool that it runs by default

mollusk: &Mollusk,
instruction: &Instruction,
accounts: &[(Pubkey, AccountSharedData)],
) {
for i in 0..instruction.accounts.len() {
if instruction.accounts[i].is_signer {
let mut modified_instruction = instruction.clone();
modified_instruction.accounts[i].is_signer = false;

let accounts_with_sysvars =
add_sysvars(mollusk, &modified_instruction, accounts.to_vec());

mollusk.process_and_validate_instruction(
&modified_instruction,
&accounts_with_sysvars,
&[Check::err(ProgramError::MissingRequiredSignature)],
);
}
}
}
Loading