Skip to content

Commit

Permalink
Address feedback pt 2
Browse files Browse the repository at this point in the history
  • Loading branch information
@joncinque
joncinque committed Apr 28, 2022
1 parent 203cfa0 commit 740e5b7
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 123 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

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

1 change: 1 addition & 0 deletions programs/stake/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ solana-vote-program = { path = "../vote", version = "=1.11.0" }
thiserror = "1.0"

[dev-dependencies]
assert_matches = "1.5.0"
proptest = "1.0"
solana-logger = { path = "../../logger", version = "=1.11.0" }

Expand Down
328 changes: 205 additions & 123 deletions programs/stake/src/stake_instruction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ mod tests {
authorized_from, create_stake_history_from_delegations, from, new_stake, stake_from,
Delegation, Meta, Stake, StakeState,
},
assert_matches::assert_matches,
bincode::serialize,
solana_program_runtime::{
invoke_context::mock_process_instruction, sysvar_cache::SysvarCache,
Expand Down Expand Up @@ -3662,13 +3663,11 @@ mod tests {
);
}

/// Ensure that `initialize()` respects the minimum delegation requirements
/// Ensure that `initialize()` respects the minimum balance requirements
/// - Assert 1: accounts with a balance equal-to the rent exemption initialize OK
/// - Assert 2: accounts with a balance less-than the rent exemption do not initialize
#[test]
fn test_initialize_minimum_balance() {
let feature_set = FeatureSet::all_enabled();
let minimum_delegation = crate::get_minimum_delegation(&feature_set);
let rent = Rent::default();
let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of());
let stake_address = solana_sdk::pubkey::new_rand();
Expand Down Expand Up @@ -3980,10 +3979,109 @@ mod tests {
}
}

/// Ensure that `split()` correctly handles prefunded destination accounts. When a destination
/// account already has funds, ensure the minimum split amount reduces accordingly.
/// Ensure that `split()` correctly handles prefunded destination accounts from
/// initialized stakes. When a destination account already has funds, ensure
/// the minimum split amount reduces accordingly.
#[test]
fn test_split_destination_minimum_stake_delegation() {
fn test_initialized_split_destination_minimum_balance() {
let rent = Rent::default();
let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of());
let source_address = Pubkey::new_unique();
let source_meta = Meta {
rent_exempt_reserve,
..Meta::auto(&source_address)
};
let dest_address = Pubkey::new_unique();
let instruction_accounts = vec![
AccountMeta {
pubkey: source_address,
is_signer: true,
is_writable: false,
},
AccountMeta {
pubkey: dest_address,
is_signer: false,
is_writable: false,
},
];
let source_balance = u64::MAX;
let source_stake_state = StakeState::Initialized(source_meta);
for (destination_starting_balance, split_amount, expected_result) in [
// split amount must be non zero
(
rent_exempt_reserve,
0,
Err(InstructionError::InsufficientFunds),
),
// any split amount is OK when destination account is already fully funded
(rent_exempt_reserve, 1, Ok(())),
// if destination is only short by 1 lamport, then split amount can be 1 lamport
(rent_exempt_reserve - 1, 1, Ok(())),
// destination short by 2 lamports, so 1 isn't enough (non-zero split amount)
(
rent_exempt_reserve - 2,
1,
Err(InstructionError::InsufficientFunds),
),
// destination has smallest non-zero balance, so can split the minimum balance
// requirements minus what destination already has
(1, rent_exempt_reserve - 1, Ok(())),
// destination has smallest non-zero balance, but cannot split less than the minimum
// balance requirements minus what destination already has
(
1,
rent_exempt_reserve - 2,
Err(InstructionError::InsufficientFunds),
),
// destination has zero lamports, so split must be at least rent exempt reserve plus
// minimum delegation
(0, rent_exempt_reserve, Ok(())),
// destination has zero lamports, but split amount is less than rent exempt reserve
// plus minimum delegation
(
0,
rent_exempt_reserve - 1,
Err(InstructionError::InsufficientFunds),
),
] {
// Set the source's starting balance and stake delegation amount to something large
// to ensure its post-split balance meets all the requirements
let source_account = AccountSharedData::new_data_with_space(
source_balance,
&source_stake_state,
StakeState::size_of(),
&id(),
)
.unwrap();
let dest_account = AccountSharedData::new_data_with_space(
destination_starting_balance,
&StakeState::Uninitialized,
StakeState::size_of(),
&id(),
)
.unwrap();

process_instruction(
&serialize(&StakeInstruction::Split(split_amount)).unwrap(),
vec![
(source_address, source_account),
(dest_address, dest_account),
(
sysvar::rent::id(),
account::create_account_shared_data_for_test(&rent),
),
],
instruction_accounts.clone(),
expected_result.clone(),
);
}
}

/// Ensure that `split()` correctly handles prefunded destination accounts from
/// delegated stakes. When a destination account already has funds, ensure
/// the minimum split amount reduces accordingly.
#[test]
fn test_stake_split_destination_minimum_delegation() {
let feature_set = FeatureSet::all_enabled();
let rent = Rent::default();
let rent_exempt_reserve = rent.minimum_balance(StakeState::size_of());
Expand All @@ -4005,130 +4103,114 @@ mod tests {
is_writable: false,
},
];
// Set the source's starting balance and stake delegation amount to
// something large to ensure its post-split balance meets all the requirements
let source_balance = u64::MAX;
let source_stake_delegation = source_balance - rent_exempt_reserve;
for (minimum_delegation, source_stake_state) in &[
(0, StakeState::Initialized(source_meta)),
let minimum_delegation = crate::get_minimum_delegation(&feature_set);
let source_stake_state = just_stake(source_meta, source_stake_delegation);
for (destination_starting_balance, split_amount, expected_result) in [
// split amount must be non zero
(
crate::get_minimum_delegation(&feature_set),
just_stake(source_meta, source_stake_delegation),
rent_exempt_reserve + minimum_delegation,
0,
Err(InstructionError::InsufficientFunds),
),
// any split amount is OK when destination account is already fully funded
(rent_exempt_reserve + minimum_delegation, 1, Ok(())),
// if destination is only short by 1 lamport, then split amount can be 1 lamport
(rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())),
// destination short by 2 lamports, so 1 isn't enough (non-zero split amount)
(
rent_exempt_reserve + minimum_delegation - 2,
1,
Err(InstructionError::InsufficientFunds),
),
// destination is rent exempt, so split enough for minimum delegation
(rent_exempt_reserve, minimum_delegation, Ok(())),
// destination is rent exempt, but split amount less than minimum delegation
(
rent_exempt_reserve,
minimum_delegation.saturating_sub(1), // when minimum is 0, this blows up!
Err(InstructionError::InsufficientFunds),
),
// destination is not rent exempt, so split enough for rent and minimum delegation
(rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())),
// destination is not rent exempt, but split amount only for minimum delegation
(
rent_exempt_reserve - 1,
minimum_delegation,
Err(InstructionError::InsufficientFunds),
),
// destination has smallest non-zero balance, so can split the minimum balance
// requirements minus what destination already has
(1, rent_exempt_reserve + minimum_delegation - 1, Ok(())),
// destination has smallest non-zero balance, but cannot split less than the minimum
// balance requirements minus what destination already has
(
1,
rent_exempt_reserve + minimum_delegation - 2,
Err(InstructionError::InsufficientFunds),
),
// destination has zero lamports, so split must be at least rent exempt reserve plus
// minimum delegation
(0, rent_exempt_reserve + minimum_delegation, Ok(())),
// destination has zero lamports, but split amount is less than rent exempt reserve
// plus minimum delegation
(
0,
rent_exempt_reserve + minimum_delegation - 1,
Err(InstructionError::InsufficientFunds),
),
] {
for (destination_starting_balance, split_amount, expected_result) in [
// split amount must be non zero
(
rent_exempt_reserve + minimum_delegation,
0,
Err(InstructionError::InsufficientFunds),
),
// any split amount is OK when destination account is already fully funded
(rent_exempt_reserve + minimum_delegation, 1, Ok(())),
// if destination is only short by 1 lamport, then split amount can be 1 lamport
(rent_exempt_reserve + minimum_delegation - 1, 1, Ok(())),
// destination short by 2 lamports, so 1 isn't enough (non-zero split amount)
(
rent_exempt_reserve + minimum_delegation - 2,
1,
Err(InstructionError::InsufficientFunds),
),
// destination is rent exempt, so split enough for minimum delegation
(rent_exempt_reserve, *minimum_delegation, Ok(())),
// destination is rent exempt, but split amount less than minimum delegation
(
rent_exempt_reserve,
minimum_delegation.saturating_sub(1), // when minimum is 0, this blows up!
Err(InstructionError::InsufficientFunds),
),
// destination is not rent exempt, so split enough for rent and minimum delegation
(rent_exempt_reserve - 1, minimum_delegation + 1, Ok(())),
// destination is not rent exempt, but split amount only for minimum delegation
(
rent_exempt_reserve - 1,
*minimum_delegation,
Err(InstructionError::InsufficientFunds),
),
// destination has smallest non-zero balance, so can split the minimum balance
// requirements minus what destination already has
(1, rent_exempt_reserve + minimum_delegation - 1, Ok(())),
// destination has smallest non-zero balance, but cannot split less than the minimum
// balance requirements minus what destination already has
(
1,
rent_exempt_reserve + minimum_delegation - 2,
Err(InstructionError::InsufficientFunds),
),
// destination has zero lamports, so split must be at least rent exempt reserve plus
// minimum delegation
(0, rent_exempt_reserve + minimum_delegation, Ok(())),
// destination has zero lamports, but split amount is less than rent exempt reserve
// plus minimum delegation
(
0,
rent_exempt_reserve + minimum_delegation - 1,
Err(InstructionError::InsufficientFunds),
),
] {
// Set the source's starting balance and stake delegation amount to something large
// to ensure its post-split balance meets all the requirements
let source_account = AccountSharedData::new_data_with_space(
source_balance,
&source_stake_state,
StakeState::size_of(),
&id(),
)
.unwrap();
let dest_account = AccountSharedData::new_data_with_space(
destination_starting_balance,
&StakeState::Uninitialized,
StakeState::size_of(),
&id(),
)
.unwrap();
let source_account = AccountSharedData::new_data_with_space(
source_balance,
&source_stake_state,
StakeState::size_of(),
&id(),
)
.unwrap();
let dest_account = AccountSharedData::new_data_with_space(
destination_starting_balance,
&StakeState::Uninitialized,
StakeState::size_of(),
&id(),
)
.unwrap();

// some of these tests don't make perfect sense for both Initialized and
// Stake, so override the expected result if it's incorrect
let expected_result = if split_amount == 0 {
Err(InstructionError::InsufficientFunds)
let accounts = process_instruction(
&serialize(&StakeInstruction::Split(split_amount)).unwrap(),
vec![
(source_address, source_account),
(dest_address, dest_account),
(
sysvar::rent::id(),
account::create_account_shared_data_for_test(&rent),
),
],
instruction_accounts.clone(),
expected_result.clone(),
);
// For the expected OK cases, when the source's StakeState is Stake, then the
// destination's StakeState *must* also end up as Stake as well. Additionally,
// check to ensure the destination's delegation amount is correct. If the
// destination is already rent exempt, then the destination's stake delegation
// *must* equal the split amount. Otherwise, the split amount must first be used to
// make the destination rent exempt, and then the leftover lamports are delegated.
if expected_result.is_ok() {
assert_matches!(accounts[0].state().unwrap(), StakeState::Stake(_, _));
if let StakeState::Stake(_, destination_stake) = accounts[1].state().unwrap() {
let destination_initial_rent_deficit =
rent_exempt_reserve.saturating_sub(destination_starting_balance);
let expected_destination_stake_delegation =
split_amount - destination_initial_rent_deficit;
assert_eq!(
expected_destination_stake_delegation,
destination_stake.delegation.stake
);
assert!(destination_stake.delegation.stake >= minimum_delegation,);
} else {
expected_result
};
let accounts = process_instruction(
&serialize(&StakeInstruction::Split(split_amount)).unwrap(),
vec![
(source_address, source_account),
(dest_address, dest_account),
(
sysvar::rent::id(),
account::create_account_shared_data_for_test(&rent),
),
],
instruction_accounts.clone(),
expected_result.clone(),
);
// For the expected OK cases, when the source's StakeState is Stake, then the
// destination's StakeState *must* also end up as Stake as well. Additionally,
// check to ensure the destination's delegation amount is correct. If the
// destination is already rent exempt, then the destination's stake delegation
// *must* equal the split amount. Otherwise, the split amount must first be used to
// make the destination rent exempt, and then the leftover lamports are delegated.
if expected_result.is_ok() {
if let StakeState::Stake(_, _) = accounts[0].state().unwrap() {
if let StakeState::Stake(_, destination_stake) =
accounts[1].state().unwrap()
{
let destination_initial_rent_deficit =
rent_exempt_reserve.saturating_sub(destination_starting_balance);
let expected_destination_stake_delegation =
split_amount - destination_initial_rent_deficit;
assert_eq!(
expected_destination_stake_delegation,
destination_stake.delegation.stake
);
assert!(destination_stake.delegation.stake >= *minimum_delegation,);
} else {
panic!("destination state must be StakeStake::Stake after successful split when source is also StakeState::Stake!");
}
}
panic!("destination state must be StakeStake::Stake after successful split when source is also StakeState::Stake!");
}
}
}
Expand Down

0 comments on commit 740e5b7

Please sign in to comment.