No longer allow create-account to add funds to an existing account (#10192)

automerge

programs/stake/src/stake_instruction.rs

@@ -208,8 +208,13 @@ pub fn split(
     split_stake_pubkey: &Pubkey,
 ) -> Vec<Instruction> {
     vec![
-        system_instruction::allocate(split_stake_pubkey, std::mem::size_of::<StakeState>() as u64),
+        system_instruction::create_account(
-        system_instruction::assign(split_stake_pubkey, &id()),
+            authorized_pubkey, // Sending 0, so any signer will suffice
+            split_stake_pubkey,
+            0,
+            std::mem::size_of::<StakeState>() as u64,
+            &id(),
+        ),
         _split(
             stake_pubkey,
             authorized_pubkey,
@@ -228,10 +233,12 @@ pub fn split_with_seed(
     seed: &str,                  // seed
 ) -> Vec<Instruction> {
     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::<StakeState>() as u64,
             &id(),
         ),
@@ -489,7 +496,7 @@ mod tests {
                     &Pubkey::default(),
                     100,
                     &Pubkey::default()
-                )[2]
+                )[1]
             ),
             Err(InstructionError::InvalidAccountData),
         );

runtime/src/system_instruction_processor.rs

@@ -129,6 +129,15 @@ fn create_account(
     owner: &Pubkey,
     signers: &HashSet<Pubkey>,
 ) -> 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!(result, Err(SystemError::AccountAlreadyInUse.into()));
-        assert_eq!(from_account.borrow().lamports, from_lamports - 50);
+        assert_eq!(from_lamports, 100);
-        assert_eq!(owned_account.lamports, 1 + 50);
+        assert_eq!(owned_account, unchanged_account);
     }
 
     #[test]

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();