From 901f7a43cdfd89a47d6049a865792c82acbe4f13 Mon Sep 17 00:00:00 2001
From: Greg Fitzgerald <greg@solana.com>
Date: Fri, 29 May 2020 22:16:26 -0600
Subject: [PATCH 1/3] Allow paying to oneself

---
 runtime/src/accounts.rs                     |  32 -----
 runtime/src/bank.rs                         |   6 +-
 runtime/src/system_instruction_processor.rs | 135 ++++++++++----------
 3 files changed, 71 insertions(+), 102 deletions(-)

diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
index 9e7c567a340065..0bfa67533321be 100644
--- a/runtime/src/accounts.rs
+++ b/runtime/src/accounts.rs
@@ -1367,38 +1367,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_load_account_pay_to_self() {
-        let mut accounts: Vec<(Pubkey, Account)> = Vec::new();
-        let mut error_counters = ErrorCounters::default();
-
-        let keypair = Keypair::new();
-        let pubkey = keypair.pubkey();
-
-        let account = Account::new(10, 1, &Pubkey::default());
-        accounts.push((pubkey, account));
-
-        let instructions = vec![CompiledInstruction::new(0, &(), vec![0, 1])];
-        // Simulate pay-to-self transaction, which loads the same account twice
-        let tx = Transaction::new_with_compiled_instructions(
-            &[&keypair],
-            &[pubkey],
-            Hash::default(),
-            vec![native_loader::id()],
-            instructions,
-        );
-        let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters);
-
-        assert_eq!(loaded_accounts.len(), 1);
-        assert_eq!(
-            loaded_accounts[0],
-            (
-                Err(TransactionError::InvalidAccountForFee),
-                Some(HashAgeKind::Extant)
-            )
-        );
-    }
-
     #[test]
     fn test_load_by_program_slot() {
         let accounts = Accounts::new(Vec::new());
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
index 721fca827c3faa..13c6425f60976e 100644
--- a/runtime/src/bank.rs
+++ b/runtime/src/bank.rs
@@ -4820,13 +4820,9 @@ mod tests {
         let _res = bank.process_transaction(&tx);
 
         assert_eq!(bank.get_balance(&key1.pubkey()), 1);
-
-        // TODO: Why do we convert errors to Oks?
-        //res[0].clone().unwrap_err();
-
         bank.get_signature_status(&tx.signatures[0])
             .unwrap()
-            .unwrap_err();
+            .unwrap();
     }
 
     fn new_from_parent(parent: &Arc<Bank>) -> Bank {
diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs
index 56a2346f75c6d6..b463ba30c65093 100644
--- a/runtime/src/system_instruction_processor.rs
+++ b/runtime/src/system_instruction_processor.rs
@@ -122,7 +122,7 @@ fn allocate_and_assign(
 
 fn create_account(
     from: &KeyedAccount,
-    to: &mut Account,
+    to: &KeyedAccount,
     to_address: &Address,
     lamports: u64,
     space: u64,
@@ -130,19 +130,22 @@ fn create_account(
     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());
-    }
+    {
+        let to = &mut to.try_account_ref_mut()?;
+        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)?;
+        allocate_and_assign(to, to_address, space, owner, signers)?;
+    }
     transfer(from, to, lamports)
 }
 
-fn transfer(from: &KeyedAccount, to: &mut Account, lamports: u64) -> Result<(), InstructionError> {
+fn transfer(from: &KeyedAccount, to: &KeyedAccount, lamports: u64) -> Result<(), InstructionError> {
     if lamports == 0 {
         return Ok(());
     }
@@ -166,7 +169,7 @@ fn transfer(from: &KeyedAccount, to: &mut Account, lamports: u64) -> Result<(),
     }
 
     from.try_account_ref_mut()?.lamports -= lamports;
-    to.lamports += lamports;
+    to.try_account_ref_mut()?.lamports += lamports;
     Ok(())
 }
 
@@ -191,11 +194,10 @@ pub fn process_instruction(
         } => {
             let from = next_keyed_account(keyed_accounts_iter)?;
             let to = next_keyed_account(keyed_accounts_iter)?;
-            let mut to_account = to.try_account_ref_mut()?;
             let to_address = Address::create(to.unsigned_key(), None)?;
             create_account(
                 from,
-                &mut to_account,
+                to,
                 &to_address,
                 lamports,
                 space,
@@ -212,11 +214,10 @@ pub fn process_instruction(
         } => {
             let from = next_keyed_account(keyed_accounts_iter)?;
             let to = next_keyed_account(keyed_accounts_iter)?;
-            let mut to_account = to.try_account_ref_mut()?;
             let to_address = Address::create(&to.unsigned_key(), Some((&base, &seed, &owner)))?;
             create_account(
                 from,
-                &mut to_account,
+                &to,
                 &to_address,
                 lamports,
                 space,
@@ -232,8 +233,8 @@ pub fn process_instruction(
         }
         SystemInstruction::Transfer { lamports } => {
             let from = next_keyed_account(keyed_accounts_iter)?;
-            let mut to_account = next_keyed_account(keyed_accounts_iter)?.try_account_ref_mut()?;
-            transfer(from, &mut to_account, lamports)
+            let to = next_keyed_account(keyed_accounts_iter)?;
+            transfer(from, to, lamports)
         }
         SystemInstruction::AdvanceNonceAccount => {
             let me = &mut next_keyed_account(keyed_accounts_iter)?;
@@ -452,13 +453,13 @@ mod tests {
         let to = Pubkey::create_with_seed(&from, seed, &new_owner).unwrap();
 
         let from_account = Account::new_ref(100, 0, &system_program::id());
-        let mut to_account = Account::new(0, 0, &Pubkey::default());
+        let to_account = Account::new_ref(0, 0, &Pubkey::default());
         let to_address = Address::create(&to, Some((&from, seed, &new_owner))).unwrap();
 
         assert_eq!(
             create_account(
                 &KeyedAccount::new(&from, false, &from_account),
-                &mut to_account,
+                &KeyedAccount::new(&to, false, &to_account),
                 &to_address,
                 50,
                 2,
@@ -468,7 +469,7 @@ mod tests {
             Err(InstructionError::MissingRequiredSignature)
         );
         assert_eq!(from_account.borrow().lamports, 100);
-        assert_eq!(to_account, Account::default());
+        assert_eq!(*to_account.borrow(), Account::default());
     }
 
     #[test]
@@ -479,12 +480,12 @@ mod tests {
         let from_account = Account::new_ref(100, 1, &Pubkey::new_rand()); // not from system account
 
         let to = Pubkey::new_rand();
-        let mut to_account = Account::new(0, 0, &Pubkey::default());
+        let to_account = Account::new_ref(0, 0, &Pubkey::default());
 
         assert_eq!(
             create_account(
                 &KeyedAccount::new(&from, false, &from_account), // no signer
-                &mut to_account,
+                &KeyedAccount::new(&to, false, &to_account),
                 &to.into(),
                 0,
                 2,
@@ -495,13 +496,13 @@ mod tests {
         );
 
         let from_lamports = from_account.borrow().lamports;
-        let to_lamports = to_account.lamports;
-        let to_owner = to_account.owner;
-        let to_data = to_account.data;
+        let to_lamports = to_account.borrow().lamports;
+        let to_owner = to_account.borrow().owner;
+        let to_data = &to_account.borrow().data;
         assert_eq!(from_lamports, 100);
         assert_eq!(to_lamports, 0);
         assert_eq!(to_owner, new_owner);
-        assert_eq!(to_data, [0, 0]);
+        assert_eq!(*to_data, [0, 0]);
     }
 
     #[test]
@@ -512,11 +513,11 @@ mod tests {
         let from_account = Account::new_ref(100, 0, &system_program::id());
 
         let to = Pubkey::new_rand();
-        let mut to_account = Account::new(0, 0, &Pubkey::default());
+        let to_account = Account::new_ref(0, 0, &Pubkey::default());
 
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut to_account,
+            &KeyedAccount::new(&from, false, &to_account),
             &to.into(),
             150,
             2,
@@ -530,7 +531,7 @@ mod tests {
     fn test_request_more_than_allowed_data_length() {
         let from_account = Account::new_ref(100, 0, &system_program::id());
         let from = Pubkey::new_rand();
-        let mut to_account = Account::default();
+        let to_account = Account::new_ref(0, 0, &system_program::id());
         let to = Pubkey::new_rand();
 
         let signers = &[from, to].iter().cloned().collect::<HashSet<_>>();
@@ -539,7 +540,7 @@ mod tests {
         // Trying to request more data length than permitted will result in failure
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut to_account,
+            &KeyedAccount::new(&to, false, &to_account),
             &address,
             50,
             MAX_PERMITTED_DATA_LENGTH + 1,
@@ -555,7 +556,7 @@ mod tests {
         // Trying to request equal or less data length than permitted will be successful
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut to_account,
+            &KeyedAccount::new(&to, false, &to_account),
             &address,
             50,
             MAX_PERMITTED_DATA_LENGTH,
@@ -563,8 +564,8 @@ mod tests {
             &signers,
         );
         assert!(result.is_ok());
-        assert_eq!(to_account.lamports, 50);
-        assert_eq!(to_account.data.len() as u64, MAX_PERMITTED_DATA_LENGTH);
+        assert_eq!(to_account.borrow().lamports, 50);
+        assert_eq!(to_account.borrow().data.len() as u64, MAX_PERMITTED_DATA_LENGTH);
     }
 
     #[test]
@@ -576,7 +577,7 @@ mod tests {
 
         let original_program_owner = Pubkey::new(&[5; 32]);
         let owned_key = Pubkey::new_rand();
-        let mut owned_account = Account::new(0, 0, &original_program_owner);
+        let owned_account = Account::new_ref(0, 0, &original_program_owner);
         let unchanged_account = owned_account.clone();
 
         let signers = &[from, owned_key].iter().cloned().collect::<HashSet<_>>();
@@ -584,7 +585,7 @@ mod tests {
 
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, false, &owned_account),
             &owned_address,
             50,
             2,
@@ -598,11 +599,11 @@ mod tests {
         assert_eq!(owned_account, unchanged_account);
 
         // Attempt to create system account in account that already has data
-        let mut owned_account = Account::new(0, 1, &Pubkey::default());
-        let unchanged_account = owned_account.clone();
+        let owned_account = Account::new_ref(0, 1, &Pubkey::default());
+        let unchanged_account = owned_account.borrow().clone();
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, false, &owned_account),
             &owned_address,
             50,
             2,
@@ -612,14 +613,14 @@ mod tests {
         assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
         let from_lamports = from_account.borrow().lamports;
         assert_eq!(from_lamports, 100);
-        assert_eq!(owned_account, unchanged_account);
+        assert_eq!(*owned_account.borrow(), unchanged_account);
 
         // 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 owned_account = Account::new_ref(1, 0, &Pubkey::default());
+        let unchanged_account = owned_account.borrow().clone();
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, false, &owned_account),
             &owned_address,
             50,
             2,
@@ -628,7 +629,7 @@ mod tests {
         );
         assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
         assert_eq!(from_lamports, 100);
-        assert_eq!(owned_account, unchanged_account);
+        assert_eq!(*owned_account.borrow(), unchanged_account);
     }
 
     #[test]
@@ -639,14 +640,14 @@ mod tests {
         let from_account = Account::new_ref(100, 0, &system_program::id());
 
         let owned_key = Pubkey::new_rand();
-        let mut owned_account = Account::new(0, 0, &Pubkey::default());
+        let owned_account = Account::new_ref(0, 0, &Pubkey::default());
 
         let owned_address = owned_key.into();
 
         // Haven't signed from account
         let result = create_account(
             &KeyedAccount::new(&from, false, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, false, &owned_account),
             &owned_address,
             50,
             2,
@@ -656,10 +657,10 @@ mod tests {
         assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
 
         // Haven't signed to account
-        let mut owned_account = Account::new(0, 0, &Pubkey::default());
+        let owned_account = Account::new_ref(0, 0, &Pubkey::default());
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, true, &owned_account),
             &owned_address,
             50,
             2,
@@ -669,10 +670,10 @@ mod tests {
         assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
 
         // support creation/assignment with zero lamports (ephemeral account)
-        let mut owned_account = Account::new(0, 0, &Pubkey::default());
+        let owned_account = Account::new_ref(0, 0, &Pubkey::default());
         let result = create_account(
             &KeyedAccount::new(&from, false, &from_account),
-            &mut owned_account,
+            &KeyedAccount::new(&owned_key, false, &owned_account),
             &owned_address,
             0,
             2,
@@ -689,7 +690,7 @@ mod tests {
         let from_account = Account::new_ref(100, 0, &system_program::id());
 
         let to = Pubkey::new_rand();
-        let mut to_account = Account::default();
+        let to_account = Account::new_ref(0, 0, &system_program::id());
 
         let signers = [from, to].iter().cloned().collect::<HashSet<_>>();
         let to_address = to.into();
@@ -697,7 +698,7 @@ mod tests {
         // fail to create a sysvar::id() owned account
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut to_account,
+            &KeyedAccount::new(&to, false, &to_account),
             &to_address,
             50,
             2,
@@ -716,10 +717,10 @@ mod tests {
         let from_account = Account::new_ref(100, 0, &system_program::id());
 
         let populated_key = Pubkey::new_rand();
-        let mut populated_account = Account {
+        let populated_account = Account {
             data: vec![0, 1, 2, 3],
             ..Account::default()
-        };
+        }.into();
 
         let signers = [from, populated_key]
             .iter()
@@ -729,7 +730,7 @@ mod tests {
 
         let result = create_account(
             &KeyedAccount::new(&from, true, &from_account),
-            &mut populated_account,
+            &KeyedAccount::new(&populated_key, false, &populated_account),
             &populated_address,
             50,
             2,
@@ -753,15 +754,16 @@ mod tests {
         let from = KeyedAccount::new(&nonce, true, &nonce_account);
         let new = Pubkey::new_rand();
 
-        let mut new_account = Account::default();
+        let new_account = Account::new_ref(0, 0, &system_program::id());
 
         let signers = [nonce, new].iter().cloned().collect::<HashSet<_>>();
         let new_address = new.into();
+        let new_keyed_account = KeyedAccount::new(&new, false, &new_account);
 
         assert_eq!(
             create_account(
                 &from,
-                &mut new_account,
+                &new_keyed_account,
                 &new_address,
                 42,
                 0,
@@ -850,26 +852,28 @@ mod tests {
     fn test_transfer_lamports() {
         let from = Pubkey::new_rand();
         let from_account = Account::new_ref(100, 0, &Pubkey::new(&[2; 32])); // account owner should not matter
-        let mut to_account = Account::new(1, 0, &Pubkey::new(&[3; 32])); // account owner should not matter
+        let to = Pubkey::new(&[3; 32]);
+        let to_account = Account::new_ref(1, 0, &to); // account owner should not matter
         let from_keyed_account = KeyedAccount::new(&from, true, &from_account);
-        transfer(&from_keyed_account, &mut to_account, 50).unwrap();
+        let to_keyed_account = KeyedAccount::new(&to, false, &to_account);
+        transfer(&from_keyed_account, &to_keyed_account, 50).unwrap();
         let from_lamports = from_keyed_account.account.borrow().lamports;
-        let to_lamports = to_account.lamports;
+        let to_lamports = to_keyed_account.account.borrow().lamports;
         assert_eq!(from_lamports, 50);
         assert_eq!(to_lamports, 51);
 
         // Attempt to move more lamports than remaining in from_account
         let from_keyed_account = KeyedAccount::new(&from, true, &from_account);
-        let result = transfer(&from_keyed_account, &mut to_account, 100);
+        let result = transfer(&from_keyed_account, &to_keyed_account, 100);
         assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into()));
         assert_eq!(from_keyed_account.account.borrow().lamports, 50);
-        assert_eq!(to_account.lamports, 51);
+        assert_eq!(to_keyed_account.account.borrow().lamports, 51);
 
         // test unsigned transfer of zero
         let from_keyed_account = KeyedAccount::new(&from, false, &from_account);
-        assert!(transfer(&from_keyed_account, &mut to_account, 0,).is_ok(),);
+        assert!(transfer(&from_keyed_account, &to_keyed_account, 0,).is_ok(),);
         assert_eq!(from_keyed_account.account.borrow().lamports, 50);
-        assert_eq!(to_account.lamports, 51);
+        assert_eq!(to_keyed_account.account.borrow().lamports, 51);
     }
 
     #[test]
@@ -889,11 +893,12 @@ mod tests {
             Some(SystemAccountKind::Nonce)
         );
 
-        let mut to_account = Account::new(1, 0, &Pubkey::new(&[3; 32])); // account owner should not matter
+        let to = Pubkey::new(&[3; 32]);
+        let to_account = Account::new_ref(1, 0, &to); // account owner should not matter
         assert_eq!(
             transfer(
                 &KeyedAccount::new(&from, true, &from_account),
-                &mut to_account,
+                &KeyedAccount::new(&to, false, &to_account),
                 50,
             ),
             Err(InstructionError::InvalidArgument),

From 7c4b6debffb84c1849a33aa42c8a56610448f62a Mon Sep 17 00:00:00 2001
From: Greg Fitzgerald <greg@solana.com>
Date: Fri, 29 May 2020 22:26:26 -0600
Subject: [PATCH 2/3] cargo fmt

---
 runtime/src/system_instruction_processor.rs | 28 ++++++---------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs
index b463ba30c65093..2f64ab0a09da1e 100644
--- a/runtime/src/system_instruction_processor.rs
+++ b/runtime/src/system_instruction_processor.rs
@@ -195,15 +195,7 @@ pub fn process_instruction(
             let from = next_keyed_account(keyed_accounts_iter)?;
             let to = next_keyed_account(keyed_accounts_iter)?;
             let to_address = Address::create(to.unsigned_key(), None)?;
-            create_account(
-                from,
-                to,
-                &to_address,
-                lamports,
-                space,
-                &owner,
-                &signers,
-            )
+            create_account(from, to, &to_address, lamports, space, &owner, &signers)
         }
         SystemInstruction::CreateAccountWithSeed {
             base,
@@ -215,15 +207,7 @@ pub fn process_instruction(
             let from = next_keyed_account(keyed_accounts_iter)?;
             let to = next_keyed_account(keyed_accounts_iter)?;
             let to_address = Address::create(&to.unsigned_key(), Some((&base, &seed, &owner)))?;
-            create_account(
-                from,
-                &to,
-                &to_address,
-                lamports,
-                space,
-                &owner,
-                &signers,
-            )
+            create_account(from, &to, &to_address, lamports, space, &owner, &signers)
         }
         SystemInstruction::Assign { owner } => {
             let keyed_account = next_keyed_account(keyed_accounts_iter)?;
@@ -565,7 +549,10 @@ mod tests {
         );
         assert!(result.is_ok());
         assert_eq!(to_account.borrow().lamports, 50);
-        assert_eq!(to_account.borrow().data.len() as u64, MAX_PERMITTED_DATA_LENGTH);
+        assert_eq!(
+            to_account.borrow().data.len() as u64,
+            MAX_PERMITTED_DATA_LENGTH
+        );
     }
 
     #[test]
@@ -720,7 +707,8 @@ mod tests {
         let populated_account = Account {
             data: vec![0, 1, 2, 3],
             ..Account::default()
-        }.into();
+        }
+        .into();
 
         let signers = [from, populated_key]
             .iter()

From 2bc3d2d5b9d9488e89703c3e3bf0149b79048799 Mon Sep 17 00:00:00 2001
From: Greg Fitzgerald <greg@solana.com>
Date: Sat, 30 May 2020 08:03:41 -0600
Subject: [PATCH 3/3] Permit pay-to-self in CLI

No test here because we're just removing an [untested] special case.

Fixes #10339
---
 cli/src/cli.rs | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/cli/src/cli.rs b/cli/src/cli.rs
index e1847cabace9aa..307e4993c15753 100644
--- a/cli/src/cli.rs
+++ b/cli/src/cli.rs
@@ -1530,11 +1530,6 @@ fn process_transfer(
 ) -> ProcessResult {
     let from = config.signers[from];
 
-    check_unique_pubkeys(
-        (&from.pubkey(), "cli keypair".to_string()),
-        (to, "to".to_string()),
-    )?;
-
     let (recent_blockhash, fee_calculator) =
         blockhash_query.get_blockhash_and_fee_calculator(rpc_client)?;