Skip to content

Commit

Permalink
get sysvars from invoke context instead of account data (simd already…
Browse files Browse the repository at this point in the history 
… updated for this)
  • Loading branch information
@2501babe
2501babe committed Jun 19, 2024
1 parent aa1d9ba commit 5d85ea7
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 106 deletions.
39 changes: 2 additions & 37 deletions programs/stake-tests/tests/test_move_stake_and_lamports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use {
state::{Authorized, Lockup, Meta, Stake, StakeStateV2},
},
system_instruction, system_program,
sysvar::{clock::Clock, stake_history::StakeHistory, SysvarId},
sysvar::{clock::Clock, stake_history::StakeHistory},
transaction::{Transaction, TransactionError},
},
solana_vote_program::{
Expand Down Expand Up @@ -952,14 +952,7 @@ async fn test_move_general_fail(
return;
}

let fake_clock = Pubkey::new_unique();
let fake_stake_history = Pubkey::new_unique();

let mut program_test = program_test();
program_test.add_sysvar_account(fake_clock, &Clock::default());
program_test.add_sysvar_account(fake_stake_history, &StakeHistory::default());

let mut context = program_test.start_with_context().await;
let mut context = program_test().start_with_context().await;
let accounts = Accounts::default();
accounts.initialize(&mut context).await;

Expand Down Expand Up @@ -1041,34 +1034,6 @@ async fn test_move_general_fail(
.unwrap_err();
assert_eq!(e, ProgramError::MissingRequiredSignature);

// spoofed clock fails
let mut instruction = mk_ixn(
&move_source,
&move_dest,
&staker_keypair.pubkey(),
minimum_delegation,
);
assert_eq!(instruction.accounts[2].pubkey, Clock::id());
instruction.accounts[2].pubkey = fake_clock;
let e = process_instruction(&mut context, &instruction, &vec![&staker_keypair])
.await
.unwrap_err();
assert_eq!(e, ProgramError::InvalidArgument);

// spoofed stake history fails
let mut instruction = mk_ixn(
&move_source,
&move_dest,
&staker_keypair.pubkey(),
minimum_delegation,
);
assert_eq!(instruction.accounts[3].pubkey, StakeHistory::id());
instruction.accounts[3].pubkey = fake_stake_history;
let e = process_instruction(&mut context, &instruction, &vec![&staker_keypair])
.await
.unwrap_err();
assert_eq!(e, ProgramError::InvalidArgument);

// good place to test source lockup
let move_locked_source_keypair = Keypair::new();
move_source_type
Expand Down
27 changes: 5 additions & 22 deletions programs/stake/src/stake_instruction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,7 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
.get_feature_set()
.is_active(&feature_set::move_stake_and_move_lamports_ixs::id())
{
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 2)?;
let stake_history = get_sysvar_with_account_check::stake_history(
invoke_context,
instruction_context,
3,
)?;
instruction_context.check_number_of_instruction_accounts(5)?;
instruction_context.check_number_of_instruction_accounts(3)?;
drop(me);
move_stake(
invoke_context,
Expand All @@ -375,9 +368,7 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
0,
lamports,
1,
&clock,
&stake_history,
4,
2,
)
} else {
Err(InstructionError::InvalidInstructionData)
Expand All @@ -389,14 +380,7 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
.get_feature_set()
.is_active(&feature_set::move_stake_and_move_lamports_ixs::id())
{
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 2)?;
let stake_history = get_sysvar_with_account_check::stake_history(
invoke_context,
instruction_context,
3,
)?;
instruction_context.check_number_of_instruction_accounts(5)?;
instruction_context.check_number_of_instruction_accounts(3)?;
drop(me);
move_lamports(
invoke_context,
Expand All @@ -405,9 +389,7 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
0,
lamports,
1,
&clock,
&stake_history,
4,
2,
)
} else {
Err(InstructionError::InvalidInstructionData)
Expand Down Expand Up @@ -537,6 +519,7 @@ mod tests {
.collect();
pubkeys.insert(clock::id());
pubkeys.insert(epoch_schedule::id());
pubkeys.insert(stake_history::id());
#[allow(deprecated)]
pubkeys
.iter()
Expand Down
23 changes: 8 additions & 15 deletions programs/stake/src/stake_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ fn move_stake_or_lamports_shared_checks(
source_account: &BorrowedAccount,
lamports: u64,
destination_account: &BorrowedAccount,
clock: &Clock,
stake_history: &StakeHistory,
stake_authority_index: IndexOfAccount,
) -> Result<(MergeKind, MergeKind), InstructionError> {
// authority must sign
Expand Down Expand Up @@ -175,14 +173,17 @@ fn move_stake_or_lamports_shared_checks(
return Err(InstructionError::InvalidArgument);
}

let clock = invoke_context.get_sysvar_cache().get_clock()?;
let stake_history = invoke_context.get_sysvar_cache().get_stake_history()?;

// get_if_mergeable ensures accounts are not partly activated or in any form of deactivating
// we still need to exclude activating state ourselves
let source_merge_kind = MergeKind::get_if_mergeable(
invoke_context,
&source_account.get_state()?,
source_account.get_lamports(),
clock,
stake_history,
&clock,
&stake_history,
)?;

// Authorized staker is allowed to move stake
Expand All @@ -196,16 +197,16 @@ fn move_stake_or_lamports_shared_checks(
invoke_context,
&destination_account.get_state()?,
destination_account.get_lamports(),
clock,
stake_history,
&clock,
&stake_history,
)?;

// ensure all authorities match and lockups match if lockup is in force
MergeKind::metas_can_merge(
invoke_context,
source_merge_kind.meta(),
destination_merge_kind.meta(),
clock,
&clock,
)?;

Ok((source_merge_kind, destination_merge_kind))
Expand Down Expand Up @@ -791,8 +792,6 @@ pub fn move_stake(
source_account_index: IndexOfAccount,
lamports: u64,
destination_account_index: IndexOfAccount,
clock: &Clock,
stake_history: &StakeHistory,
stake_authority_index: IndexOfAccount,
) -> Result<(), InstructionError> {
let mut source_account = instruction_context
Expand All @@ -808,8 +807,6 @@ pub fn move_stake(
&source_account,
lamports,
&destination_account,
clock,
stake_history,
stake_authority_index,
)?;

Expand Down Expand Up @@ -929,8 +926,6 @@ pub fn move_lamports(
source_account_index: IndexOfAccount,
lamports: u64,
destination_account_index: IndexOfAccount,
clock: &Clock,
stake_history: &StakeHistory,
stake_authority_index: IndexOfAccount,
) -> Result<(), InstructionError> {
let mut source_account = instruction_context
Expand All @@ -946,8 +941,6 @@ pub fn move_lamports(
&source_account,
lamports,
&destination_account,
clock,
stake_history,
stake_authority_index,
)?;

Expand Down
46 changes: 14 additions & 32 deletions sdk/program/src/stake/instruction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,7 @@ pub enum StakeInstruction {
/// # Account references
/// 0. `[WRITE]` Active source stake account
/// 1. `[WRITE]` Active or inactive destination stake account
/// 2. `[]` Clock sysvar
/// 3. `[]` Stake history sysvar that carries stake warmup/cooldown history
/// 4. `[SIGNER]` Stake authority
/// 2. `[SIGNER]` Stake authority
///
/// The u64 is the portion of the stake to move, which may be the entire delegation
MoveStake(u64),
Expand All @@ -341,9 +339,7 @@ pub enum StakeInstruction {
/// # Account references
/// 0. `[WRITE]` Active or inactive source stake account
/// 1. `[WRITE]` Mergeable destination stake account
/// 2. `[]` Clock sysvar
/// 3. `[]` Stake history sysvar that carries stake warmup/cooldown history
/// 4. `[SIGNER]` Stake authority
/// 2. `[SIGNER]` Stake authority
///
/// The u64 is the portion of available lamports to move
MoveLamports(u64),
Expand Down Expand Up @@ -893,46 +889,32 @@ pub fn move_stake(
authorized_pubkey: &Pubkey,
lamports: u64,
) -> Instruction {
move_stake_or_lamports(
source_stake_pubkey,
destination_stake_pubkey,
authorized_pubkey,
lamports,
&(StakeInstruction::MoveStake as fn(u64) -> StakeInstruction),
)
}
let account_metas = vec![
AccountMeta::new(*source_stake_pubkey, false),
AccountMeta::new(*destination_stake_pubkey, false),
AccountMeta::new_readonly(*authorized_pubkey, true),
];

pub fn move_lamports(
source_stake_pubkey: &Pubkey,
destination_stake_pubkey: &Pubkey,
authorized_pubkey: &Pubkey,
lamports: u64,
) -> Instruction {
move_stake_or_lamports(
source_stake_pubkey,
destination_stake_pubkey,
authorized_pubkey,
lamports,
&(StakeInstruction::MoveLamports as fn(u64) -> StakeInstruction),
)
Instruction::new_with_bincode(id(), &StakeInstruction::MoveStake(lamports), account_metas)
}

fn move_stake_or_lamports(
pub fn move_lamports(
source_stake_pubkey: &Pubkey,
destination_stake_pubkey: &Pubkey,
authorized_pubkey: &Pubkey,
lamports: u64,
value_constructor: &fn(u64) -> StakeInstruction,
) -> Instruction {
let account_metas = vec![
AccountMeta::new(*source_stake_pubkey, false),
AccountMeta::new(*destination_stake_pubkey, false),
AccountMeta::new_readonly(sysvar::clock::id(), false),
AccountMeta::new_readonly(sysvar::stake_history::id(), false),
AccountMeta::new_readonly(*authorized_pubkey, true),
];

Instruction::new_with_bincode(id(), &value_constructor(lamports), account_metas)
Instruction::new_with_bincode(
id(),
&StakeInstruction::MoveLamports(lamports),
account_metas,
)
}

#[cfg(test)]
Expand Down

0 comments on commit 5d85ea7

Please sign in to comment.