From 66bd1972924ab654bec6475a93b80d58ce6aa0b6 Mon Sep 17 00:00:00 2001 From: hana <81144685+2501babe@users.noreply.github.com> Date: Thu, 2 Oct 2025 11:29:43 -0700 Subject: [PATCH 1/2] program: rewrite Split --- program/src/helpers/mod.rs | 3 - program/src/helpers/split.rs | 88 ---------- program/src/processor.rs | 272 +++++++++++++++++------------ program/tests/program_test.rs | 42 ++--- program/tests/stake_instruction.rs | 23 ++- 5 files changed, 196 insertions(+), 232 deletions(-) delete mode 100644 program/src/helpers/split.rs diff --git a/program/src/helpers/mod.rs b/program/src/helpers/mod.rs index b532f448..81bd7d2f 100644 --- a/program/src/helpers/mod.rs +++ b/program/src/helpers/mod.rs @@ -3,9 +3,6 @@ use {solana_instruction_error::InstructionError, solana_program_error::ProgramEr pub(crate) mod delegate; pub(crate) use delegate::*; -pub(crate) mod split; -pub(crate) use split::*; - pub(crate) mod merge; pub(crate) use merge::*; diff --git a/program/src/helpers/split.rs b/program/src/helpers/split.rs deleted file mode 100644 index 1695ea53..00000000 --- a/program/src/helpers/split.rs +++ /dev/null @@ -1,88 +0,0 @@ -use { - solana_program_error::ProgramError, solana_rent::Rent, solana_stake_interface::state::Meta, - solana_sysvar::Sysvar, -}; - -/// After calling `validate_split_amount()`, this struct contains calculated -/// values that are used by the caller. -#[derive(Copy, Clone, Debug, Default)] -pub(crate) struct ValidatedSplitInfo { - pub source_remaining_balance: u64, - pub destination_rent_exempt_reserve: u64, -} - -/// Ensure the split amount is valid. This checks the source and destination -/// accounts meet the minimum balance requirements, which is the rent exempt -/// reserve plus the minimum stake delegation, and that the source account has -/// enough lamports for the request split amount. If not, return an error. -pub(crate) fn validate_split_amount( - source_lamports: u64, - destination_lamports: u64, - split_lamports: u64, - source_meta: &Meta, - destination_data_len: usize, - additional_required_lamports: u64, - source_is_active: bool, -) -> Result { - // Split amount has to be something - if split_lamports == 0 { - return Err(ProgramError::InsufficientFunds); - } - - // Obviously cannot split more than what the source account has - if split_lamports > source_lamports { - return Err(ProgramError::InsufficientFunds); - } - - // Verify that the source account still has enough lamports left after - // splitting: EITHER at least the minimum balance, OR zero (in this case the - // source account is transferring all lamports to new destination account, - // and the source account will be closed) - let source_minimum_balance = source_meta - .rent_exempt_reserve - .saturating_add(additional_required_lamports); - let source_remaining_balance = source_lamports.saturating_sub(split_lamports); - if source_remaining_balance == 0 { - // full amount is a withdrawal - // nothing to do here - } else if source_remaining_balance < source_minimum_balance { - // the remaining balance is too low to do the split - return Err(ProgramError::InsufficientFunds); - } else { - // all clear! - // nothing to do here - } - - let rent = Rent::get()?; - let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len); - - // 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 source_is_active - && source_remaining_balance != 0 - && destination_lamports < destination_rent_exempt_reserve - { - return Err(ProgramError::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 destination_minimum_balance = - destination_rent_exempt_reserve.saturating_add(additional_required_lamports); - let destination_balance_deficit = - destination_minimum_balance.saturating_sub(destination_lamports); - if split_lamports < destination_balance_deficit { - return Err(ProgramError::InsufficientFunds); - } - - Ok(ValidatedSplitInfo { - source_remaining_balance, - destination_rent_exempt_reserve, - }) -} diff --git a/program/src/processor.rs b/program/src/processor.rs index 0026488c..14141fc1 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -489,13 +489,10 @@ impl Processor { // we may decide to enforce this if the pattern is not used on mainnet // let _stake_authority_info = next_account_info(account_info_iter); + let rent = Rent::get()?; let clock = Clock::get()?; let stake_history = &StakeHistorySysvar(clock.epoch); - - let destination_data_len = destination_stake_account_info.data_len(); - if destination_data_len != StakeStateV2::size_of() { - return Err(ProgramError::InvalidAccountData); - } + let minimum_delegation = crate::get_minimum_delegation(); if let StakeStateV2::Uninitialized = get_stake_state(destination_stake_account_info)? { // we can split into this @@ -510,84 +507,179 @@ impl Processor { return Err(ProgramError::InsufficientFunds); } - match get_stake_state(source_stake_account_info)? { - StakeStateV2::Stake(source_meta, mut source_stake, stake_flags) => { + if split_lamports == 0 { + return Err(ProgramError::InsufficientFunds); + } + + let source_rent_exempt_reserve = rent.minimum_balance(source_stake_account_info.data_len()); + + let destination_data_len = destination_stake_account_info.data_len(); + if destination_data_len != StakeStateV2::size_of() { + return Err(ProgramError::InvalidAccountData); + } + let destination_rent_exempt_reserve = rent.minimum_balance(destination_data_len); + + // check signers and get delegation status along with a destination meta + let source_stake_state = get_stake_state(source_stake_account_info)?; + let (is_active_or_activating, option_dest_meta) = match source_stake_state { + StakeStateV2::Stake(source_meta, source_stake, _) => { source_meta .authorized .check(&signers, StakeAuthorize::Staker) .map_err(to_program_error)?; - let minimum_delegation = crate::get_minimum_delegation(); - - let status = source_stake.delegation.stake_activating_and_deactivating( + let source_status = source_stake.delegation.stake_activating_and_deactivating( clock.epoch, stake_history, PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH, ); + let is_active_or_activating = + source_status.effective > 0 || source_status.activating > 0; - let is_active = status.effective > 0; + let mut dest_meta = source_meta; + dest_meta.rent_exempt_reserve = destination_rent_exempt_reserve; - // NOTE this function also internally summons Rent via syscall - let validated_split_info = validate_split_amount( - source_lamport_balance, - destination_lamport_balance, - split_lamports, - &source_meta, - destination_data_len, - minimum_delegation, - is_active, - )?; + (is_active_or_activating, Some(dest_meta)) + } + StakeStateV2::Initialized(source_meta) => { + source_meta + .authorized + .check(&signers, StakeAuthorize::Staker) + .map_err(to_program_error)?; - // split the stake, subtract rent_exempt_balance unless - // the destination account already has those lamports - // in place. - // this means that the new stake account will have a stake equivalent to - // lamports minus rent_exempt_reserve if it starts out with a zero balance - let (remaining_stake_delta, split_stake_amount) = - if validated_split_info.source_remaining_balance == 0 { - // If split amount equals the full source stake (as implied by 0 - // source_remaining_balance), the new split stake must equal the same - // amount, regardless of any current lamport balance in the split account. - // Since split accounts retain the state of their source account, this - // prevents any magic activation of stake by prefunding the split account. - // - // The new split stake also needs to ignore any positive delta between the - // original rent_exempt_reserve and the split_rent_exempt_reserve, in order - // to prevent magic activation of stake by splitting between accounts of - // different sizes. - let remaining_stake_delta = - split_lamports.saturating_sub(source_meta.rent_exempt_reserve); - (remaining_stake_delta, remaining_stake_delta) + let mut dest_meta = source_meta; + dest_meta.rent_exempt_reserve = destination_rent_exempt_reserve; + + (false, Some(dest_meta)) + } + StakeStateV2::Uninitialized => { + if !source_stake_account_info.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + + (false, None) + } + StakeStateV2::RewardsPool => return Err(ProgramError::InvalidAccountData), + }; + + // special case: for a full split, we only care that the destination becomes a valid stake account + // this prevents state changes in exceptional cases where a once-valid source has become invalid + // relocate lamports, copy data, and close the original account + if split_lamports == source_lamport_balance { + let mut destination_stake_state = source_stake_state; + let delegation = match (&mut destination_stake_state, option_dest_meta) { + (StakeStateV2::Stake(meta, stake, _), Some(dest_meta)) => { + *meta = dest_meta; + + if is_active_or_activating { + stake.delegation.stake } else { - // Otherwise, the new split stake should reflect the entire split - // requested, less any lamports needed to cover the - // split_rent_exempt_reserve. - if source_stake.delegation.stake.saturating_sub(split_lamports) - < minimum_delegation - { - return Err(StakeError::InsufficientDelegation.into()); - } - - ( - split_lamports, - split_lamports.saturating_sub( - validated_split_info - .destination_rent_exempt_reserve - .saturating_sub(destination_lamport_balance), - ), - ) - }; - - if split_stake_amount < minimum_delegation { + 0 + } + } + (StakeStateV2::Initialized(meta), Some(dest_meta)) => { + *meta = dest_meta; + + 0 + } + (StakeStateV2::Uninitialized, None) => 0, + _ => unreachable!(), + }; + + if destination_lamport_balance + .saturating_add(split_lamports) + .saturating_sub(delegation) + < destination_rent_exempt_reserve + { + return Err(ProgramError::InsufficientFunds); + } + + if is_active_or_activating && delegation < minimum_delegation { + return Err(StakeError::InsufficientDelegation.into()); + } + + set_stake_state(destination_stake_account_info, &destination_stake_state)?; + if source_stake_account_info.key != destination_stake_account_info.key { + source_stake_account_info.resize(0)?; + } + + relocate_lamports( + source_stake_account_info, + destination_stake_account_info, + split_lamports, + )?; + debug_assert!(source_stake_account_info.lamports() == 0); + + return Ok(()); + } + + // special case: if stake is fully inactive, we only care that both accounts meet rent-exemption + if !is_active_or_activating { + let mut destination_stake_state = source_stake_state; + match (&mut destination_stake_state, option_dest_meta) { + (StakeStateV2::Stake(meta, _, _), Some(dest_meta)) + | (StakeStateV2::Initialized(meta), Some(dest_meta)) => { + *meta = dest_meta; + } + (StakeStateV2::Uninitialized, None) => (), + _ => unreachable!(), + } + + let post_source_lamports = source_lamport_balance.saturating_sub(split_lamports); + let post_destination_lamports = + destination_lamport_balance.saturating_add(split_lamports); + + if post_source_lamports < source_rent_exempt_reserve + || post_destination_lamports < destination_rent_exempt_reserve + { + return Err(ProgramError::InsufficientFunds); + } + + set_stake_state(destination_stake_account_info, &destination_stake_state)?; + + relocate_lamports( + source_stake_account_info, + destination_stake_account_info, + split_lamports, + )?; + + return Ok(()); + } + + // at this point, we know we have a StakeStateV2::Stake source that is either activating or has nonzero effective + // this means we must redistribute the delegation across both accounts and enforce: + // * destination has a pre-funded rent exemption + // * source meets rent exemption less its remaining delegation + // * source and destination both meet the minimum delegation + // destination delegation is matched 1:1 by split lamports. in other words, free source lamports are never split + match (source_stake_state, option_dest_meta) { + (StakeStateV2::Stake(source_meta, mut source_stake, stake_flags), Some(dest_meta)) => { + if destination_lamport_balance < destination_rent_exempt_reserve { + return Err(ProgramError::InsufficientFunds); + } + + let mut dest_stake = source_stake; + + source_stake.delegation.stake = + source_stake.delegation.stake.saturating_sub(split_lamports); + + if source_stake.delegation.stake < minimum_delegation { return Err(StakeError::InsufficientDelegation.into()); } - let destination_stake = - source_stake.split(remaining_stake_delta, split_stake_amount)?; + // minimum delegation is by definition nonzero, and we remove one delegated lamport per split lamport + // since the remaining source delegation > 0, it is impossible that we took from its rent-exempt reserve + debug_assert!( + source_lamport_balance + .saturating_sub(split_lamports) + .saturating_sub(source_stake.delegation.stake) + >= source_meta.rent_exempt_reserve + ); - let mut destination_meta = source_meta; - destination_meta.rent_exempt_reserve = - validated_split_info.destination_rent_exempt_reserve; + dest_stake.delegation.stake = split_lamports; + if dest_stake.delegation.stake < minimum_delegation { + return Err(StakeError::InsufficientDelegation.into()); + } set_stake_state( source_stake_account_info, @@ -596,56 +688,18 @@ impl Processor { set_stake_state( destination_stake_account_info, - &StakeStateV2::Stake(destination_meta, destination_stake, stake_flags), + &StakeStateV2::Stake(dest_meta, dest_stake, stake_flags), )?; - } - StakeStateV2::Initialized(source_meta) => { - source_meta - .authorized - .check(&signers, StakeAuthorize::Staker) - .map_err(to_program_error)?; - - // NOTE this function also internally summons Rent via syscall - let validated_split_info = validate_split_amount( - source_lamport_balance, - destination_lamport_balance, - split_lamports, - &source_meta, - destination_data_len, - 0, // additional_required_lamports - false, // is_active - )?; - - let mut destination_meta = source_meta; - destination_meta.rent_exempt_reserve = - validated_split_info.destination_rent_exempt_reserve; - set_stake_state( + relocate_lamports( + source_stake_account_info, destination_stake_account_info, - &StakeStateV2::Initialized(destination_meta), + split_lamports, )?; } - StakeStateV2::Uninitialized => { - if !source_stake_account_info.is_signer { - return Err(ProgramError::MissingRequiredSignature); - } - } - _ => return Err(ProgramError::InvalidAccountData), - } - - // Truncate state upon zero balance - if source_stake_account_info.key != destination_stake_account_info.key - && split_lamports == source_lamport_balance - { - source_stake_account_info.resize(0)?; + _ => unreachable!(), } - relocate_lamports( - source_stake_account_info, - destination_stake_account_info, - split_lamports, - )?; - Ok(()) } diff --git a/program/tests/program_test.rs b/program/tests/program_test.rs index e1988c7a..2098f512 100644 --- a/program/tests/program_test.rs +++ b/program/tests/program_test.rs @@ -984,18 +984,7 @@ impl StakeLifecycle { } } - // NOTE the program enforces that a deactive stake adheres to the split minimum, - // albeit spuriously after solana-program/stake-program #1 is addressed, - // Self::Deactive should move to false equivalently this could be combined - // with withdraw_minimum_enforced into a function minimum_enforced - pub fn split_minimum_enforced(&self) -> bool { - match self { - Self::Activating | Self::Active | Self::Deactivating | Self::Deactive => true, - Self::Uninitialized | Self::Initialized | Self::Closed => false, - } - } - - pub fn withdraw_minimum_enforced(&self) -> bool { + pub fn minimum_delegation_enforced(&self) -> bool { match self { Self::Activating | Self::Active | Self::Deactivating => true, Self::Uninitialized | Self::Initialized | Self::Deactive | Self::Closed => false, @@ -1031,6 +1020,13 @@ async fn program_test_split(split_source_type: StakeLifecycle) { _ => vec![&staker_keypair], }; + // fail, cannot split zero + let instruction = &ixn::split(&split_source, &signers[0].pubkey(), 0, &split_dest)[2]; + let e = process_instruction(&mut context, instruction, &signers) + .await + .unwrap_err(); + assert_eq!(e, ProgramError::InsufficientFunds); + // fail, split more than available (even if not active, would kick source out of // rent exemption) let instruction = &ixn::split( @@ -1043,19 +1039,19 @@ async fn program_test_split(split_source_type: StakeLifecycle) { let e = process_instruction(&mut context, instruction, &signers) .await .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); + assert_eq!( + e, + if split_source_type.minimum_delegation_enforced() { + StakeError::InsufficientDelegation.into() + } else { + ProgramError::InsufficientFunds + } + ); // an active or transitioning stake account cannot have less than the minimum // delegation note this is NOT dependent on the minimum delegation feature. // there was ALWAYS a minimum. it was one lamport! - if split_source_type.split_minimum_enforced() { - // zero split fails - let instruction = &ixn::split(&split_source, &signers[0].pubkey(), 0, &split_dest)[2]; - let e = process_instruction(&mut context, instruction, &signers) - .await - .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); - + if split_source_type.minimum_delegation_enforced() { // underfunded destination fails let instruction = &ixn::split( &split_source, @@ -1080,7 +1076,7 @@ async fn program_test_split(split_source_type: StakeLifecycle) { let e = process_instruction(&mut context, instruction, &signers) .await .unwrap_err(); - assert_eq!(e, ProgramError::InsufficientFunds); + assert_eq!(e, StakeError::InsufficientDelegation.into()); } // split to non-owned account fails @@ -1209,7 +1205,7 @@ async fn program_test_withdraw_stake(withdraw_source_type: StakeLifecycle) { .unwrap_err(); assert_eq!(e, ProgramError::InsufficientFunds); - if withdraw_source_type.withdraw_minimum_enforced() { + if withdraw_source_type.minimum_delegation_enforced() { // withdraw active or activating stake fails let instruction = ixn::withdraw( &withdraw_source, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index 58df8118..7382e9aa 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -3602,6 +3602,8 @@ fn test_split_minimum_stake_delegation() { is_writable: true, }, ]; + // NOTE with a 1lamp minimum delegation, cases 2 and 4 merely hit the zero split error + // these would be InsufficientDelegation if the minimum were 1sol for (source_delegation, split_amount, expected_result) in [ (minimum_delegation * 2, minimum_delegation, Ok(())), ( @@ -3612,7 +3614,7 @@ fn test_split_minimum_stake_delegation() { ( (minimum_delegation * 2) - 1, minimum_delegation, - Err(ProgramError::InsufficientFunds), + Err(StakeError::InsufficientDelegation.into()), ), ( (minimum_delegation - 1) * 2, @@ -4354,11 +4356,21 @@ fn test_split_source_uninitialized() { }, ]; + // splitting zero always fails + { + process_instruction( + &mollusk, + &serialize(&StakeInstruction::Split(0)).unwrap(), + transaction_accounts.clone(), + instruction_accounts.clone(), + Err(ProgramError::InsufficientFunds), + ); + } + // splitting an uninitialized account where the destination is the same as the source { // splitting should work when... // - when split amount is the full balance - // - when split amount is zero // - when split amount is non-zero and less than the full balance // // and splitting should fail when the split amount is greater than the balance @@ -4372,13 +4384,6 @@ fn test_split_source_uninitialized() { ); assert_eq!(accounts[0].data().len(), StakeStateV2::size_of()); - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Split(0)).unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(), From 855fd2862561d60c40c481e5faac2dd32a4e6758 Mon Sep 17 00:00:00 2001 From: hana <81144685+2501babe@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:23:51 -0800 Subject: [PATCH 2/2] ban self-split --- program/src/processor.rs | 8 ++-- program/tests/stake_instruction.rs | 62 ++++++++++++++---------------- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/program/src/processor.rs b/program/src/processor.rs index 14141fc1..04931abd 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -494,6 +494,10 @@ impl Processor { let stake_history = &StakeHistorySysvar(clock.epoch); let minimum_delegation = crate::get_minimum_delegation(); + if source_stake_account_info.key == destination_stake_account_info.key { + return Err(ProgramError::InvalidArgument); + } + if let StakeStateV2::Uninitialized = get_stake_state(destination_stake_account_info)? { // we can split into this } else { @@ -599,9 +603,7 @@ impl Processor { } set_stake_state(destination_stake_account_info, &destination_stake_state)?; - if source_stake_account_info.key != destination_stake_account_info.key { - source_stake_account_info.resize(0)?; - } + source_stake_account_info.resize(0)?; relocate_lamports( source_stake_account_info, diff --git a/program/tests/stake_instruction.rs b/program/tests/stake_instruction.rs index 7382e9aa..0b9e5720 100644 --- a/program/tests/stake_instruction.rs +++ b/program/tests/stake_instruction.rs @@ -4343,65 +4343,58 @@ fn test_split_source_uninitialized() { (stake_address, stake_account), (split_to_address, split_to_account), ]; - let mut instruction_accounts = vec![ + let instruction_accounts = vec![ AccountMeta { pubkey: stake_address, is_signer: true, is_writable: true, }, AccountMeta { - pubkey: stake_address, + pubkey: split_to_address, is_signer: false, is_writable: true, }, ]; // splitting zero always fails - { - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Split(0)).unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Err(ProgramError::InsufficientFunds), - ); - } + process_instruction( + &mollusk, + &serialize(&StakeInstruction::Split(0)).unwrap(), + transaction_accounts.clone(), + instruction_accounts.clone(), + Err(ProgramError::InsufficientFunds), + ); - // splitting an uninitialized account where the destination is the same as the source + // self-split always fails { - // splitting should work when... - // - when split amount is the full balance - // - when split amount is non-zero and less than the full balance - // - // and splitting should fail when the split amount is greater than the balance - // self-splitting also must not resize the source-destination - let accounts = process_instruction( + let mut instruction_accounts = instruction_accounts.clone(); + instruction_accounts[1].pubkey = stake_address; + + process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), - Ok(()), + Err(ProgramError::InvalidArgument), ); - assert_eq!(accounts[0].data().len(), StakeStateV2::size_of()); process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), - Ok(()), + Err(ProgramError::InvalidArgument), ); process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports + 1)).unwrap(), transaction_accounts.clone(), instruction_accounts.clone(), - Err(ProgramError::InsufficientFunds), + Err(ProgramError::InvalidArgument), ); } // this should work - instruction_accounts[1].pubkey = split_to_address; let accounts = process_instruction( &mollusk, &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(), @@ -4425,14 +4418,17 @@ fn test_split_source_uninitialized() { assert_eq!(accounts[1].data().len(), StakeStateV2::size_of()); // no signers should fail - instruction_accounts[0].is_signer = false; - process_instruction( - &mollusk, - &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(), - transaction_accounts, - instruction_accounts, - Err(ProgramError::MissingRequiredSignature), - ); + { + let mut instruction_accounts = instruction_accounts.clone(); + instruction_accounts[0].is_signer = false; + process_instruction( + &mollusk, + &serialize(&StakeInstruction::Split(stake_lamports / 2)).unwrap(), + transaction_accounts, + instruction_accounts, + Err(ProgramError::MissingRequiredSignature), + ); + } } #[test] @@ -4456,7 +4452,7 @@ fn test_split_split_not_uninitialized() { is_writable: true, }, AccountMeta { - pubkey: stake_address, + pubkey: split_to_address, is_signer: false, is_writable: true, },