GetMinimumDelegation does not require a stake account (#24192)

This commit is contained in:
Brooks Prumo 2022-04-11 16:26:36 -05:00 committed by GitHub
parent b38833923d
commit f7b00ada1b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 180 additions and 21 deletions

View File

@ -55,14 +55,18 @@ pub fn process_instruction(
trace!("process_instruction: {:?}", data);
let mut me = instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
if *me.get_owner() != id() {
return Err(InstructionError::InvalidAccountOwner);
}
let get_stake_account = || {
let me = instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
if *me.get_owner() != id() {
return Err(InstructionError::InvalidAccountOwner);
}
Ok(me)
};
let signers = instruction_context.get_signers(transaction_context);
match limited_deserialize(data)? {
StakeInstruction::Initialize(authorized, lockup) => {
match limited_deserialize(data) {
Ok(StakeInstruction::Initialize(authorized, lockup)) => {
let mut me = get_stake_account()?;
let rent = get_sysvar_with_account_check::rent(invoke_context, instruction_context, 1)?;
initialize(
&mut me,
@ -72,7 +76,8 @@ pub fn process_instruction(
&invoke_context.feature_set,
)
}
StakeInstruction::Authorize(authorized_pubkey, stake_authorize) => {
Ok(StakeInstruction::Authorize(authorized_pubkey, stake_authorize)) => {
let mut me = get_stake_account()?;
let require_custodian_for_locked_stake_authorize = invoke_context
.feature_set
.is_active(&feature_set::require_custodian_for_locked_stake_authorize::id());
@ -109,7 +114,8 @@ pub fn process_instruction(
)
}
}
StakeInstruction::AuthorizeWithSeed(args) => {
Ok(StakeInstruction::AuthorizeWithSeed(args)) => {
let mut me = get_stake_account()?;
instruction_context.check_number_of_instruction_accounts(2)?;
let require_custodian_for_locked_stake_authorize = invoke_context
.feature_set
@ -153,7 +159,8 @@ pub fn process_instruction(
)
}
}
StakeInstruction::DelegateStake => {
Ok(StakeInstruction::DelegateStake) => {
let me = get_stake_account()?;
instruction_context.check_number_of_instruction_accounts(2)?;
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 2)?;
@ -182,7 +189,8 @@ pub fn process_instruction(
&signers,
)
}
StakeInstruction::Split(lamports) => {
Ok(StakeInstruction::Split(lamports)) => {
let me = get_stake_account()?;
instruction_context.check_number_of_instruction_accounts(2)?;
drop(me);
split(
@ -195,7 +203,8 @@ pub fn process_instruction(
&signers,
)
}
StakeInstruction::Merge => {
Ok(StakeInstruction::Merge) => {
let me = get_stake_account()?;
instruction_context.check_number_of_instruction_accounts(2)?;
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 2)?;
@ -216,7 +225,8 @@ pub fn process_instruction(
&signers,
)
}
StakeInstruction::Withdraw(lamports) => {
Ok(StakeInstruction::Withdraw(lamports)) => {
let me = get_stake_account()?;
instruction_context.check_number_of_instruction_accounts(2)?;
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 2)?;
@ -244,16 +254,19 @@ pub fn process_instruction(
&invoke_context.feature_set,
)
}
StakeInstruction::Deactivate => {
Ok(StakeInstruction::Deactivate) => {
let mut me = get_stake_account()?;
let clock =
get_sysvar_with_account_check::clock(invoke_context, instruction_context, 1)?;
deactivate(&mut me, &clock, &signers)
}
StakeInstruction::SetLockup(lockup) => {
Ok(StakeInstruction::SetLockup(lockup)) => {
let mut me = get_stake_account()?;
let clock = invoke_context.get_sysvar_cache().get_clock()?;
set_lockup(&mut me, &lockup, &signers, &clock)
}
StakeInstruction::InitializeChecked => {
Ok(StakeInstruction::InitializeChecked) => {
let mut me = get_stake_account()?;
if invoke_context
.feature_set
.is_active(&feature_set::vote_stake_checked_instructions::id())
@ -287,7 +300,8 @@ pub fn process_instruction(
Err(InstructionError::InvalidInstructionData)
}
}
StakeInstruction::AuthorizeChecked(stake_authorize) => {
Ok(StakeInstruction::AuthorizeChecked(stake_authorize)) => {
let mut me = get_stake_account()?;
if invoke_context
.feature_set
.is_active(&feature_set::vote_stake_checked_instructions::id())
@ -321,7 +335,8 @@ pub fn process_instruction(
Err(InstructionError::InvalidInstructionData)
}
}
StakeInstruction::AuthorizeCheckedWithSeed(args) => {
Ok(StakeInstruction::AuthorizeCheckedWithSeed(args)) => {
let mut me = get_stake_account()?;
if invoke_context
.feature_set
.is_active(&feature_set::vote_stake_checked_instructions::id())
@ -360,7 +375,8 @@ pub fn process_instruction(
Err(InstructionError::InvalidInstructionData)
}
}
StakeInstruction::SetLockupChecked(lockup_checked) => {
Ok(StakeInstruction::SetLockupChecked(lockup_checked)) => {
let mut me = get_stake_account()?;
if invoke_context
.feature_set
.is_active(&feature_set::vote_stake_checked_instructions::id())
@ -383,21 +399,31 @@ pub fn process_instruction(
Err(InstructionError::InvalidInstructionData)
}
}
StakeInstruction::GetMinimumDelegation => {
Ok(StakeInstruction::GetMinimumDelegation) => {
let feature_set = invoke_context.feature_set.as_ref();
if !feature_set.is_active(
&feature_set::add_get_minimum_delegation_instruction_to_stake_program::id(),
) {
// Retain previous behavior of always checking that the first account
// is a stake account until the feature is activated
let _ = get_stake_account()?;
return Err(InstructionError::InvalidInstructionData);
}
let minimum_delegation = crate::get_minimum_delegation(feature_set);
let minimum_delegation = Vec::from(minimum_delegation.to_le_bytes());
drop(me);
invoke_context
.transaction_context
.set_return_data(id(), minimum_delegation)
}
Err(err) => {
if !invoke_context.feature_set.is_active(
&feature_set::add_get_minimum_delegation_instruction_to_stake_program::id(),
) {
let _ = get_stake_account()?;
}
Err(err)
}
}
}
@ -434,7 +460,7 @@ mod tests {
system_program, sysvar,
},
solana_vote_program::vote_state::{self, VoteState, VoteStateVersions},
std::{collections::HashSet, str::FromStr},
std::{collections::HashSet, str::FromStr, sync::Arc},
};
fn create_default_account() -> AccountSharedData {
@ -5969,4 +5995,137 @@ mod tests {
},
);
}
// Ensure that the correct errors are returned when processing instructions
//
// The GetMinimumDelegation instruction does not take any accounts; so when it was added,
// `process_instruction()` needed to be updated to *not* need a stake account passed in, which
// changes the error *ordering* conditions. These changes shall only occur when the
// `add_get_minimum_delegation_instruction_to_stake_program` feature is enabled, and this test
// ensures it.
//
// For the following combinations of the feature enabled/disabled, if the instruction is
// valid/invalid, and if a stake account is passed in or not, assert the result:
//
// feature | instruction | account || result
// ---------+-------------+---------++--------
// enabled | good | some || Ok
// enabled | bad | some || Err InvalidInstructionData
// enabled | good | none || Err NotEnoughAccountKeys
// enabled | bad | none || Err InvalidInstructionData
// disabled | good | some || Ok
// disabled | bad | some || Err InvalidInstructionData
// disabled | good | none || Err NotEnoughAccountKeys
// disabled | bad | none || Err NotEnoughAccountKeys
#[test]
fn test_stake_process_instruction_error_ordering() {
let rent = Rent::default();
let rent_address = sysvar::rent::id();
let rent_account = account::create_account_shared_data_for_test(&rent);
let good_stake_address = Pubkey::new_unique();
let good_stake_account = AccountSharedData::new(
u64::MAX,
std::mem::size_of::<crate::stake_state::StakeState>(),
&id(),
);
let good_instruction = instruction::initialize(
&good_stake_address,
&Authorized::auto(&good_stake_address),
&Lockup::default(),
);
let good_transaction_accounts = vec![
(good_stake_address, good_stake_account),
(rent_address, rent_account),
];
let good_instruction_accounts = vec![
AccountMeta {
pubkey: good_stake_address,
is_signer: false,
is_writable: false,
},
AccountMeta {
pubkey: rent_address,
is_signer: false,
is_writable: false,
},
];
let good_accounts = (good_transaction_accounts, good_instruction_accounts);
// The instruction data needs to deserialize to a bogus StakeInstruction. We likely never
// will have `usize::MAX`-number of instructions, so this should be a safe constant to
// always map to an invalid stake instruction.
let bad_instruction = Instruction::new_with_bincode(id(), &usize::MAX, Vec::default());
let bad_transaction_accounts = Vec::default();
let bad_instruction_accounts = Vec::default();
let bad_accounts = (bad_transaction_accounts, bad_instruction_accounts);
for (
is_feature_enabled,
instruction,
(transaction_accounts, instruction_accounts),
expected_result,
) in [
(true, &good_instruction, &good_accounts, Ok(())),
(
true,
&bad_instruction,
&good_accounts,
Err(InstructionError::InvalidInstructionData),
),
(
true,
&good_instruction,
&bad_accounts,
Err(InstructionError::NotEnoughAccountKeys),
),
(
true,
&bad_instruction,
&bad_accounts,
Err(InstructionError::InvalidInstructionData),
),
(false, &good_instruction, &good_accounts, Ok(())),
(
false,
&bad_instruction,
&good_accounts,
Err(InstructionError::InvalidInstructionData),
),
(
false,
&good_instruction,
&bad_accounts,
Err(InstructionError::NotEnoughAccountKeys),
),
(
false,
&bad_instruction,
&bad_accounts,
Err(InstructionError::NotEnoughAccountKeys),
),
] {
mock_process_instruction(
&id(),
Vec::new(),
&instruction.data,
transaction_accounts.clone(),
instruction_accounts.clone(),
None,
expected_result,
if is_feature_enabled {
|first_instruction_account, invoke_context| {
super::process_instruction(first_instruction_account, invoke_context)
}
} else {
|first_instruction_account, invoke_context| {
let mut feature_set = FeatureSet::all_enabled();
feature_set.deactivate(&feature_set::add_get_minimum_delegation_instruction_to_stake_program::id());
invoke_context.feature_set = Arc::new(feature_set);
super::process_instruction(first_instruction_account, invoke_context)
}
},
);
}
}
}