Make active stake consistent in split (#33295)

* Add feature gate

* Add helper fn

* Require split destination to be rent-exempt if it is active

* Update cli to prefund split accounts

* cli: require rent param with sign-only

* Update tokens to prefund split accounts

* Update split tests with sysvar accounts

* Fix test_split_to_account_with_rent_exempt_reserve

* Fix test_staked_split_destination_minimum_balance

* Fix test_split_more_than_staked

* Fix test_split_minimum_stake_delegation and remove misleading StakeState::Initialized case

* Fix test_split_from_larger_sized_account

* Add test for pre-/post-activation behavior splitting some or all of stake account

* Assert active stake

* Fix runtime test

* Ignore stake-pool downstream

* Review comments

* Feature gate sysvar reads
This commit is contained in:
Tyera 2023-09-20 00:00:51 -06:00 committed by GitHub
parent 7a8a492d4c
commit bca41edf20
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 813 additions and 125 deletions

View File

@ -128,7 +128,7 @@ jobs:
- [governance/addin-mock/program, governance/program]
- [memo/program]
- [name-service/program]
- [stake-pool/program]
# - [stake-pool/program]
- [single-pool/program]
steps:

View File

@ -238,6 +238,7 @@ pub enum CliCommand {
lamports: u64,
fee_payer: SignerIndex,
compute_unit_price: Option<u64>,
rent_exempt_reserve: Option<u64>,
},
MergeStake {
stake_account_pubkey: Pubkey,
@ -1226,6 +1227,7 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
lamports,
fee_payer,
compute_unit_price,
rent_exempt_reserve,
} => process_split_stake(
&rpc_client,
config,
@ -1242,6 +1244,7 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
*lamports,
*fee_payer,
compute_unit_price.as_ref(),
rent_exempt_reserve.as_ref(),
),
CliCommand::MergeStake {
stake_account_pubkey,
@ -2243,6 +2246,7 @@ mod tests {
lamports: 30,
fee_payer: 0,
compute_unit_price: None,
rent_exempt_reserve: None,
};
config.signers = vec![&keypair, &split_stake_account];
let result = process_command(&config);

View File

@ -55,7 +55,7 @@ use {
tools::{acceptable_reference_epoch_credits, eligible_for_deactivate_delinquent},
},
stake_history::{Epoch, StakeHistory},
system_instruction::SystemError,
system_instruction::{self, SystemError},
sysvar::{clock, stake_history},
transaction::Transaction,
},
@ -121,6 +121,13 @@ pub struct StakeAuthorizationIndexed {
pub new_authority_signer: Option<SignerIndex>,
}
struct SignOnlySplitNeedsRent {}
impl ArgsConfig for SignOnlySplitNeedsRent {
fn sign_only_arg<'a, 'b>(&self, arg: Arg<'a, 'b>) -> Arg<'a, 'b> {
arg.requires("rent_exempt_reserve_sol")
}
}
pub trait StakeSubCommands {
fn stake_subcommands(self) -> Self;
}
@ -493,11 +500,21 @@ impl StakeSubCommands for App<'_, '_> {
will be at a derived address of SPLIT_STAKE_ACCOUNT")
)
.arg(stake_authority_arg())
.offline_args()
.offline_args_config(&SignOnlySplitNeedsRent{})
.nonce_args(false)
.arg(fee_payer_arg())
.arg(memo_arg())
.arg(compute_unit_price_arg())
.arg(
Arg::with_name("rent_exempt_reserve_sol")
.long("rent-exempt-reserve-sol")
.value_name("AMOUNT")
.takes_value(true)
.validator(is_amount)
.requires("sign_only")
.help("Offline signing only: the rent-exempt amount to move into the new \
stake account, in SOL")
)
)
.subcommand(
SubCommand::with_name("merge-stake")
@ -1027,6 +1044,7 @@ pub fn parse_split_stake(
let signer_info =
default_signer.generate_unique_signers(bulk_signers, matches, wallet_manager)?;
let compute_unit_price = value_of(matches, COMPUTE_UNIT_PRICE_ARG.name);
let rent_exempt_reserve = lamports_of_sol(matches, "rent_exempt_reserve_sol");
Ok(CliCommandInfo {
command: CliCommand::SplitStake {
@ -1043,6 +1061,7 @@ pub fn parse_split_stake(
lamports,
fee_payer: signer_info.index_of(fee_payer_pubkey).unwrap(),
compute_unit_price,
rent_exempt_reserve,
},
signers: signer_info.signers,
})
@ -1852,6 +1871,7 @@ pub fn process_split_stake(
lamports: u64,
fee_payer: SignerIndex,
compute_unit_price: Option<&u64>,
rent_exempt_reserve: Option<&u64>,
) -> ProcessResult {
let split_stake_account = config.signers[split_stake_account];
let fee_payer = config.signers[fee_payer];
@ -1885,7 +1905,7 @@ pub fn process_split_stake(
split_stake_account.pubkey()
};
if !sign_only {
let rent_exempt_reserve = if !sign_only {
if let Ok(stake_account) = rpc_client.get_account(&split_stake_account_address) {
let err_msg = if stake_account.owner == stake::program::id() {
format!("Stake account {split_stake_account_address} already exists")
@ -1906,30 +1926,44 @@ pub fn process_split_stake(
))
.into());
}
}
minimum_balance
} else {
rent_exempt_reserve
.cloned()
.expect("rent_exempt_reserve_sol is required with sign_only")
};
let recent_blockhash = blockhash_query.get_blockhash(rpc_client, config.commitment)?;
let ixs = if let Some(seed) = split_stake_account_seed {
stake_instruction::split_with_seed(
stake_account_pubkey,
&stake_authority.pubkey(),
lamports,
&split_stake_account_address,
&split_stake_account.pubkey(),
seed,
let mut ixs = vec![system_instruction::transfer(
&fee_payer.pubkey(),
&split_stake_account_address,
rent_exempt_reserve,
)];
if let Some(seed) = split_stake_account_seed {
ixs.append(
&mut stake_instruction::split_with_seed(
stake_account_pubkey,
&stake_authority.pubkey(),
lamports,
&split_stake_account_address,
&split_stake_account.pubkey(),
seed,
)
.with_memo(memo)
.with_compute_unit_price(compute_unit_price),
)
.with_memo(memo)
.with_compute_unit_price(compute_unit_price)
} else {
stake_instruction::split(
stake_account_pubkey,
&stake_authority.pubkey(),
lamports,
&split_stake_account_address,
ixs.append(
&mut stake_instruction::split(
stake_account_pubkey,
&stake_authority.pubkey(),
lamports,
&split_stake_account_address,
)
.with_memo(memo)
.with_compute_unit_price(compute_unit_price),
)
.with_memo(memo)
.with_compute_unit_price(compute_unit_price)
};
let nonce_authority = config.signers[nonce_authority];
@ -4848,6 +4882,7 @@ mod tests {
lamports: 50_000_000_000,
fee_payer: 0,
compute_unit_price: None,
rent_exempt_reserve: None,
},
signers: vec![
read_keypair_file(&default_keypair_file).unwrap().into(),
@ -4915,6 +4950,7 @@ mod tests {
lamports: 50_000_000_000,
fee_payer: 1,
compute_unit_price: None,
rent_exempt_reserve: None,
},
signers: vec![
Presigner::new(&stake_auth_pubkey, &stake_sig).into(),

View File

@ -1469,6 +1469,10 @@ fn test_stake_split() {
config.json_rpc_url = test_validator.rpc_url();
config.signers = vec![&default_signer];
let minimum_balance = rpc_client
.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())
.unwrap();
let mut config_offline = CliConfig::recent_for_tests();
config_offline.json_rpc_url = String::default();
config_offline.signers = vec![&offline_signer];
@ -1496,10 +1500,7 @@ fn test_stake_split() {
check_balance!(1_000_000_000_000, &rpc_client, &offline_pubkey);
// Create stake account, identity is authority
let stake_balance = rpc_client
.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())
.unwrap()
+ 10_000_000_000;
let stake_balance = minimum_balance + 10_000_000_000;
let stake_keypair = keypair_from_seed(&[0u8; 32]).unwrap();
let stake_account_pubkey = stake_keypair.pubkey();
config.signers.push(&stake_keypair);
@ -1569,6 +1570,7 @@ fn test_stake_split() {
lamports: 2 * stake_balance,
fee_payer: 0,
compute_unit_price: None,
rent_exempt_reserve: Some(minimum_balance),
};
config_offline.output_format = OutputFormat::JsonCompact;
let sig_response = process_command(&config_offline).unwrap();
@ -1593,10 +1595,15 @@ fn test_stake_split() {
lamports: 2 * stake_balance,
fee_payer: 0,
compute_unit_price: None,
rent_exempt_reserve: None,
};
process_command(&config).unwrap();
check_balance!(8 * stake_balance, &rpc_client, &stake_account_pubkey,);
check_balance!(2 * stake_balance, &rpc_client, &split_account.pubkey(),);
check_balance!(8 * stake_balance, &rpc_client, &stake_account_pubkey);
check_balance!(
2 * stake_balance + minimum_balance,
&rpc_client,
&split_account.pubkey()
);
}
#[test]

File diff suppressed because it is too large Load Diff

View File

@ -103,6 +103,19 @@ pub(crate) fn new_warmup_cooldown_rate_epoch(invoke_context: &InvokeContext) ->
.new_warmup_cooldown_rate_epoch(epoch_schedule.as_ref())
}
fn get_stake_status(
invoke_context: &InvokeContext,
stake: &Stake,
clock: &Clock,
) -> Result<StakeActivationStatus, InstructionError> {
let stake_history = invoke_context.get_sysvar_cache().get_stake_history()?;
Ok(stake.delegation.stake_activating_and_deactivating(
clock.epoch,
Some(&stake_history),
new_warmup_cooldown_rate_epoch(invoke_context),
))
}
fn redelegate_stake(
invoke_context: &InvokeContext,
stake: &mut Stake,
@ -688,6 +701,16 @@ pub fn split(
StakeStateV2::Stake(meta, mut stake, stake_flags) => {
meta.authorized.check(signers, StakeAuthorize::Staker)?;
let minimum_delegation = crate::get_minimum_delegation(&invoke_context.feature_set);
let is_active = if invoke_context
.feature_set
.is_active(&feature_set::require_rent_exempt_split_destination::id())
{
let clock = invoke_context.get_sysvar_cache().get_clock()?;
let status = get_stake_status(invoke_context, &stake, &clock)?;
status.effective > 0
} else {
false
};
let validated_split_info = validate_split_amount(
invoke_context,
transaction_context,
@ -697,6 +720,7 @@ pub fn split(
lamports,
&meta,
minimum_delegation,
is_active,
)?;
// split the stake, subtract rent_exempt_balance unless
@ -763,6 +787,7 @@ pub fn split(
lamports,
&meta,
0, // additional_required_lamports
false,
)?;
let mut split_meta = meta;
split_meta.rent_exempt_reserve = validated_split_info.destination_rent_exempt_reserve;
@ -925,12 +950,7 @@ pub fn redelegate(
let (stake_meta, effective_stake) =
if let StakeStateV2::Stake(meta, stake, _stake_flags) = stake_account.get_state()? {
let stake_history = invoke_context.get_sysvar_cache().get_stake_history()?;
let status = stake.delegation.stake_activating_and_deactivating(
clock.epoch,
Some(&stake_history),
new_warmup_cooldown_rate_epoch(invoke_context),
);
let status = get_stake_status(invoke_context, &stake, &clock)?;
if status.effective == 0 || status.activating != 0 || status.deactivating != 0 {
ic_msg!(invoke_context, "stake is not active");
return Err(StakeError::RedelegateTransientOrInactiveStake.into());
@ -1192,6 +1212,7 @@ fn validate_split_amount(
lamports: u64,
source_meta: &Meta,
additional_required_lamports: u64,
source_is_active: bool,
) -> Result<ValidatedSplitInfo, InstructionError> {
let source_account = instruction_context
.try_borrow_instruction_account(transaction_context, source_account_index)?;
@ -1232,12 +1253,27 @@ fn validate_split_amount(
// nothing to do here
}
let rent = invoke_context.get_sysvar_cache().get_rent()?;
let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len);
// As of feature `require_rent_exempt_split_destination`, if the source is active stake, one of
// these criteria must be met:
// 1. the destination account must be prefunded with at least the rent-exempt reserve, or
// 2. the split must consume 100% of the source
if invoke_context
.feature_set
.is_active(&feature_set::require_rent_exempt_split_destination::id())
&& source_is_active
&& source_remaining_balance != 0
&& destination_lamports < destination_rent_exempt_reserve
{
return Err(InstructionError::InsufficientFunds);
}
// Verify the destination account meets the minimum balance requirements
// This must handle:
// 1. The destination account having a different rent exempt reserve due to data size changes
// 2. The destination account being prefunded, which would lower the minimum split amount
let rent = invoke_context.get_sysvar_cache().get_rent()?;
let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len);
let destination_minimum_balance =
destination_rent_exempt_reserve.saturating_add(additional_required_lamports);
let destination_balance_deficit =

View File

@ -428,15 +428,21 @@ fn test_stake_account_lifetime() {
let split_stake_keypair = Keypair::new();
let split_stake_pubkey = split_stake_keypair.pubkey();
bank.transfer(
stake_rent_exempt_reserve,
&mint_keypair,
&split_stake_pubkey,
)
.unwrap();
let bank_client = BankClient::new_shared(bank.clone());
// Test split
let split_starting_delegation = stake_minimum_delegation + bonus_delegation;
let split_starting_balance = split_starting_delegation + stake_rent_exempt_reserve;
let message = Message::new(
&stake_instruction::split(
&stake_pubkey,
&stake_pubkey,
split_starting_balance,
split_starting_delegation,
&split_stake_pubkey,
),
Some(&mint_pubkey),
@ -451,7 +457,7 @@ fn test_stake_account_lifetime() {
get_staked(&bank, &split_stake_pubkey),
split_starting_delegation,
);
let stake_remaining_balance = balance - split_starting_balance;
let stake_remaining_balance = balance - split_starting_delegation;
// Deactivate the split
let message = Message::new(

View File

@ -689,6 +689,10 @@ pub mod enable_program_runtime_v2_and_loader_v4 {
solana_sdk::declare_id!("8oBxsYqnCvUTGzgEpxPcnVf7MLbWWPYddE33PftFeBBd");
}
pub mod require_rent_exempt_split_destination {
solana_sdk::declare_id!("D2aip4BBr8NPWtU9vLrwrBvbuaQ8w1zV38zFLxx4pfBV");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -856,6 +860,7 @@ lazy_static! {
(timely_vote_credits::id(), "use timeliness of votes in determining credits to award"),
(remaining_compute_units_syscall_enabled::id(), "enable the remaining_compute_units syscall"),
(enable_program_runtime_v2_and_loader_v4::id(), "Enable Program-Runtime-v2 and Loader-v4 #33293"),
(require_rent_exempt_split_destination::id(), "Require stake split destination account to be rent exempt"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

View File

@ -559,6 +559,7 @@ fn parse_distribute_stake_args(
stake_authority,
withdraw_authority,
lockup_authority,
rent_exempt_reserve: None,
};
let stake_args = StakeArgs {
unlocked_sol: sol_to_lamports(value_t_or_exit!(matches, "unlocked_sol", f64)),

View File

@ -5,6 +5,7 @@ pub struct SenderStakeArgs {
pub stake_authority: Box<dyn Signer>,
pub withdraw_authority: Box<dyn Signer>,
pub lockup_authority: Option<Box<dyn Signer>>,
pub rent_exempt_reserve: Option<u64>,
}
pub struct StakeArgs {

View File

@ -31,7 +31,7 @@ use {
signature::{unique_signers, Signature, Signer},
stake::{
instruction::{self as stake_instruction, LockupArgs},
state::{Authorized, Lockup, StakeAuthorize},
state::{Authorized, Lockup, StakeAuthorize, StakeStateV2},
},
system_instruction,
transaction::Transaction,
@ -234,12 +234,24 @@ fn distribution_instructions(
Some(sender_stake_args) => {
let stake_authority = sender_stake_args.stake_authority.pubkey();
let withdraw_authority = sender_stake_args.withdraw_authority.pubkey();
let mut instructions = stake_instruction::split(
let rent_exempt_reserve = sender_stake_args
.rent_exempt_reserve
.expect("SenderStakeArgs.rent_exempt_reserve should be populated");
// Transfer some tokens to stake account to cover rent-exempt reserve.
let mut instructions = vec![system_instruction::transfer(
&sender_pubkey,
new_stake_account_address,
rent_exempt_reserve,
)];
// Split to stake account
instructions.append(&mut stake_instruction::split(
&sender_stake_args.stake_account_address,
&stake_authority,
allocation.amount - unlocked_sol,
allocation.amount - unlocked_sol - rent_exempt_reserve,
new_stake_account_address,
);
));
// Make the recipient the new stake authority
instructions.push(stake_instruction::authorize(
@ -1174,11 +1186,15 @@ pub fn test_process_distribute_stake_with_client(client: &RpcClient, sender_keyp
let output_file = NamedTempFile::new().unwrap();
let output_path = output_file.path().to_str().unwrap().to_string();
let rent_exempt_reserve = client
.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())
.unwrap();
let sender_stake_args = SenderStakeArgs {
stake_account_address,
stake_authority: Box::new(stake_authority),
withdraw_authority: Box::new(withdraw_authority),
lockup_authority: None,
rent_exempt_reserve: Some(rent_exempt_reserve),
};
let stake_args = StakeArgs {
unlocked_sol: sol_to_lamports(1.0),
@ -1529,14 +1545,14 @@ mod tests {
)); // Same recipient, same lockups
}
const SET_LOCKUP_INDEX: usize = 5;
const SET_LOCKUP_INDEX: usize = 6;
#[test]
fn test_set_split_stake_lockup() {
let lockup_date_str = "2021-01-07T00:00:00Z";
let allocation = Allocation {
recipient: Pubkey::default().to_string(),
amount: sol_to_lamports(1.0),
amount: sol_to_lamports(1.002_282_880),
lockup_date: lockup_date_str.to_string(),
};
let stake_account_address = solana_sdk::pubkey::new_rand();
@ -1548,6 +1564,7 @@ mod tests {
stake_authority: Box::new(Keypair::new()),
withdraw_authority: Box::new(Keypair::new()),
lockup_authority: Some(Box::new(lockup_authority)),
rent_exempt_reserve: Some(2_282_880),
};
let stake_args = StakeArgs {
lockup_authority: Some(lockup_authority_address),
@ -1821,6 +1838,7 @@ mod tests {
stake_authority: Box::new(stake_authority),
withdraw_authority: Box::new(withdraw_authority),
lockup_authority: None,
rent_exempt_reserve: Some(2_282_880),
};
StakeArgs {

View File

@ -4,4 +4,5 @@ pub mod args;
pub mod commands;
mod db;
pub mod spl_token;
pub mod stake;
pub mod token_display;

View File

@ -2,7 +2,7 @@ use {
solana_clap_utils::input_validators::normalize_to_url_if_moniker,
solana_cli_config::{Config, CONFIG_FILE},
solana_rpc_client::rpc_client::RpcClient,
solana_tokens::{arg_parser::parse_args, args::Command, commands, spl_token},
solana_tokens::{arg_parser::parse_args, args::Command, commands, spl_token, stake},
std::{
env,
error::Error,
@ -43,6 +43,7 @@ fn main() -> Result<(), Box<dyn Error>> {
match command_args.command {
Command::DistributeTokens(mut args) => {
spl_token::update_token_args(&client, &mut args.spl_token_args)?;
stake::update_stake_args(&client, &mut args.stake_args)?;
commands::process_allocations(&client, &args, exit)?;
}
Command::Balances(mut args) => {

15
tokens/src/stake.rs Normal file
View File

@ -0,0 +1,15 @@
use {
crate::{args::StakeArgs, commands::Error},
solana_rpc_client::rpc_client::RpcClient,
solana_sdk::stake::state::StakeStateV2,
};
pub fn update_stake_args(client: &RpcClient, args: &mut Option<StakeArgs>) -> Result<(), Error> {
if let Some(stake_args) = args {
if let Some(sender_args) = &mut stake_args.sender_stake_args {
let rent = client.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())?;
sender_args.rent_exempt_reserve = Some(rent);
}
}
Ok(())
}