-
Notifications
You must be signed in to change notification settings - Fork 33
tests: Mollusk StakeLifecycle, Deactivate tests (2/9) #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
423127d
695ce95
edf1713
080449a
1293590
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| #![allow(clippy::arithmetic_side_effects)] | ||
|
|
||
| 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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)], | ||
| ); | ||
| } | ||
| 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") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)], | ||
| ); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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?