diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 06395857e4..d937976658 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -208,8 +208,13 @@ pub fn split( split_stake_pubkey: &Pubkey, ) -> Vec { vec![ - system_instruction::allocate(split_stake_pubkey, std::mem::size_of::() as u64), - system_instruction::assign(split_stake_pubkey, &id()), + system_instruction::create_account( + authorized_pubkey, // Sending 0, so any signer will suffice + split_stake_pubkey, + 0, + std::mem::size_of::() as u64, + &id(), + ), _split( stake_pubkey, authorized_pubkey, @@ -228,10 +233,12 @@ pub fn split_with_seed( seed: &str, // seed ) -> Vec { vec![ - system_instruction::allocate_with_seed( + system_instruction::create_account_with_seed( + authorized_pubkey, // Sending 0, so any signer will suffice split_stake_pubkey, base, seed, + 0, std::mem::size_of::() as u64, &id(), ), @@ -489,7 +496,7 @@ mod tests { &Pubkey::default(), 100, &Pubkey::default() - )[2] + )[1] ), Err(InstructionError::InvalidAccountData), ); diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index fba020f84a..56a2346f75 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -129,6 +129,15 @@ fn create_account( owner: &Pubkey, signers: &HashSet, ) -> Result<(), InstructionError> { + // if it looks like the `to` account is already in use, bail + if to.lamports > 0 { + debug!( + "Create Account: invalid argument; account {:?} already in use", + to_address + ); + return Err(SystemError::AccountAlreadyInUse.into()); + } + allocate_and_assign(to, to_address, space, owner, signers)?; transfer(from, to, lamports) } @@ -605,8 +614,9 @@ mod tests { assert_eq!(from_lamports, 100); assert_eq!(owned_account, unchanged_account); - // Verify that create_account works even if `to` has a non-zero balance + // Attempt to create an account that already has lamports let mut owned_account = Account::new(1, 0, &Pubkey::default()); + let unchanged_account = owned_account.clone(); let result = create_account( &KeyedAccount::new(&from, true, &from_account), &mut owned_account, @@ -616,9 +626,9 @@ mod tests { &new_owner, &signers, ); - assert_eq!(result, Ok(())); - assert_eq!(from_account.borrow().lamports, from_lamports - 50); - assert_eq!(owned_account.lamports, 1 + 50); + assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); + assert_eq!(from_lamports, 100); + assert_eq!(owned_account, unchanged_account); } #[test] diff --git a/runtime/tests/stake.rs b/runtime/tests/stake.rs index c4329addea..535a30c5d3 100644 --- a/runtime/tests/stake.rs +++ b/runtime/tests/stake.rs @@ -9,7 +9,9 @@ use solana_sdk::{ message::Message, pubkey::Pubkey, signature::{Keypair, Signer}, + system_instruction::SystemError, sysvar::{self, stake_history::StakeHistory, Sysvar}, + transaction::TransactionError, }; use solana_stake_program::{ stake_instruction::{self}, @@ -156,6 +158,84 @@ fn test_stake_create_and_split_single_signature() { // w00t! } +#[test] +fn test_stake_create_and_split_to_existing_system_account() { + // Ensure stake-split does not allow the user to promote an existing system account into + // a stake account. + + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair: staker_keypair, + .. + } = create_genesis_config_with_leader(100_000_000_000, &Pubkey::new_rand(), 1_000_000); + + let staker_pubkey = staker_keypair.pubkey(); + + let bank_client = BankClient::new_shared(&Arc::new(Bank::new(&genesis_config))); + + let stake_address = + Pubkey::create_with_seed(&staker_pubkey, "stake", &solana_stake_program::id()).unwrap(); + + let authorized = stake_state::Authorized::auto(&staker_pubkey); + + let lamports = 1_000_000; + + // Create stake account with seed + let message = Message::new(&stake_instruction::create_account_with_seed( + &staker_pubkey, // from + &stake_address, // to + &staker_pubkey, // base + "stake", // seed + &authorized, + &stake_state::Lockup::default(), + lamports, + )); + + bank_client + .send_message(&[&staker_keypair], message) + .expect("failed to create and delegate stake account"); + + let split_stake_address = + Pubkey::create_with_seed(&staker_pubkey, "split_stake", &solana_stake_program::id()) + .unwrap(); + + // First, put a system account where we want the new stake account + let existing_lamports = 42; + bank_client + .transfer(existing_lamports, &staker_keypair, &split_stake_address) + .unwrap(); + assert_eq!( + bank_client.get_balance(&split_stake_address).unwrap(), + existing_lamports + ); + + // Verify the split fails because the account is already in use + let message = Message::new_with_payer( + &stake_instruction::split_with_seed( + &stake_address, // original + &staker_pubkey, // authorized + lamports / 2, + &split_stake_address, // new address + &staker_pubkey, // base + "split_stake", // seed + ), + Some(&staker_keypair.pubkey()), + ); + assert_eq!( + bank_client + .send_message(&[&staker_keypair], message) + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, SystemError::AccountAlreadyInUse.into()) + ); + assert_eq!( + bank_client.get_balance(&split_stake_address).unwrap(), + existing_lamports + ); +} + #[test] fn test_stake_account_lifetime() { let stake_keypair = Keypair::new();