Allow the same account to be passed multiple times to a single instruction (#7795)

runtime/src/bank.rs

@@ -3,7 +3,7 @@
 //! on behalf of the caller, and a low-level API for when they have
 //! already been signed and verified.
 use crate::{
-    accounts::{Accounts, TransactionLoadResult},
+    accounts::{Accounts, TransactionAccounts, TransactionLoadResult, TransactionLoaders},
     accounts_db::{AccountStorageEntry, AccountsDBSerialize, AppendVecId, ErrorCounters},
     blockhash_queue::BlockhashQueue,
     message_processor::{MessageProcessor, ProcessInstruction},
@@ -51,9 +51,11 @@ use solana_sdk::{
 use solana_stake_program::stake_state::Delegation;
 use solana_vote_program::vote_state::VoteState;
 use std::{
+    cell::RefCell,
     collections::HashMap,
     io::{BufReader, Cursor, Error as IOError, Read},
     path::{Path, PathBuf},
+    rc::Rc,
     sync::atomic::{AtomicBool, AtomicU64, Ordering},
     sync::{Arc, RwLock, RwLockReadGuard},
 };
@@ -64,6 +66,8 @@ pub const MAX_SNAPSHOT_DATA_FILE_SIZE: u64 = 32 * 1024 * 1024 * 1024; // 32 GiB
 pub const MAX_LEADER_SCHEDULE_STAKES: Epoch = 5;
 
 type BankStatusCache = StatusCache<Result<()>>;
+type TransactionAccountRefCells = Vec<Rc<RefCell<Account>>>;
+type TransactionLoaderRefCells = Vec<Vec<(Pubkey, RefCell<Account>)>>;
 
 #[derive(Default)]
 pub struct BankRc {
@@ -1167,6 +1171,48 @@ impl Bank {
         }
     }
 
+    /// Converts Accounts into RefCell<Account>, this involves moving
+    /// ownership by draining the source
+    #[allow(clippy::wrong_self_convention)]
+    fn into_refcells(
+        accounts: &mut TransactionAccounts,
+        loaders: &mut TransactionLoaders,
+    ) -> (TransactionAccountRefCells, TransactionLoaderRefCells) {
+        let account_refcells: Vec<_> = accounts
+            .drain(..)
+            .map(|account| Rc::new(RefCell::new(account)))
+            .collect();
+        let loader_refcells: Vec<Vec<_>> = loaders
+            .iter_mut()
+            .map(|v| {
+                v.drain(..)
+                    .map(|(pubkey, account)| (pubkey, RefCell::new(account)))
+                    .collect()
+            })
+            .collect();
+        (account_refcells, loader_refcells)
+    }
+
+    /// Converts back from RefCell<Account> to Account, this involves moving
+    /// ownership by draining the sources
+    fn from_refcells(
+        accounts: &mut TransactionAccounts,
+        loaders: &mut TransactionLoaders,
+        mut account_refcells: TransactionAccountRefCells,
+        loader_refcells: TransactionLoaderRefCells,
+    ) {
+        account_refcells.drain(..).for_each(|account_refcell| {
+            accounts.push(Rc::try_unwrap(account_refcell).unwrap().into_inner())
+        });
+        loaders
+            .iter_mut()
+            .zip(loader_refcells)
+            .for_each(|(ls, mut lrcs)| {
+                lrcs.drain(..)
+                    .for_each(|(pubkey, lrc)| ls.push((pubkey, lrc.into_inner())))
+            });
+    }
+
     #[allow(clippy::type_complexity)]
     pub fn load_and_execute_transactions(
         &self,
@@ -1222,9 +1268,18 @@ impl Bank {
                 (Err(e), hash_age_kind) => (Err(e.clone()), hash_age_kind.clone()),
                 (Ok((accounts, loaders, _rents)), hash_age_kind) => {
                     signature_count += u64::from(tx.message().header.num_required_signatures);
-                    let process_result =
-                        self.message_processor
-                            .process_message(tx.message(), loaders, accounts);
+
+                    let (mut account_refcells, mut loader_refcells) =
+                        Self::into_refcells(accounts, loaders);
+
+                    let process_result = self.message_processor.process_message(
+                        tx.message(),
+                        &mut loader_refcells,
+                        &mut account_refcells,
+                    );
+
+                    Self::from_refcells(accounts, loaders, account_refcells, loader_refcells);
+
                     if let Err(TransactionError::InstructionError(_, _)) = &process_result {
                         error_counters.instruction_error += 1;
                     }
@@ -2285,8 +2340,8 @@ mod tests {
         if let Ok(instruction) = bincode::deserialize(data) {
             match instruction {
                 MockInstruction::Deduction => {
-                    keyed_accounts[1].account.lamports += 1;
-                    keyed_accounts[2].account.lamports -= 1;
+                    keyed_accounts[1].account.borrow_mut().lamports += 1;
+                    keyed_accounts[2].account.borrow_mut().lamports -= 1;
                     Ok(())
                 }
             }
@@ -2873,8 +2928,8 @@ mod tests {
 
         // set up stakes, vote, and storage accounts
         bank.store_account(&stake.0, &stake.1);
-        bank.store_account(&validator_id, &validator_account);
-        bank.store_account(&archiver_id, &archiver_account);
+        bank.store_account(&validator_id, &validator_account.borrow());
+        bank.store_account(&archiver_id, &archiver_account.borrow());
 
         // generate some rewards
         let mut vote_state = VoteState::from(&vote_account).unwrap();
@@ -5136,4 +5191,56 @@ mod tests {
         assert_eq!(transaction_balances_set.pre_balances[2], vec![9, 0, 1]);
         assert_eq!(transaction_balances_set.post_balances[2], vec![8, 0, 1]);
     }
+
+    #[test]
+    fn test_transaction_with_duplicate_accounts_in_instruction() {
+        let (genesis_config, mint_keypair) = create_genesis_config(500);
+        let mut bank = Bank::new(&genesis_config);
+
+        fn mock_process_instruction(
+            _program_id: &Pubkey,
+            keyed_accounts: &mut [KeyedAccount],
+            data: &[u8],
+        ) -> result::Result<(), InstructionError> {
+            let lamports = data[0] as u64;
+            {
+                let mut to_account = keyed_accounts[1].try_account_ref_mut()?;
+                let mut dup_account = keyed_accounts[2].try_account_ref_mut()?;
+                dup_account.lamports -= lamports;
+                to_account.lamports += lamports;
+            }
+            keyed_accounts[0].try_account_ref_mut()?.lamports -= lamports;
+            keyed_accounts[1].try_account_ref_mut()?.lamports += lamports;
+            Ok(())
+        }
+
+        let mock_program_id = Pubkey::new(&[2u8; 32]);
+        bank.add_instruction_processor(mock_program_id, mock_process_instruction);
+
+        let from_pubkey = Pubkey::new_rand();
+        let to_pubkey = Pubkey::new_rand();
+        let dup_pubkey = from_pubkey.clone();
+        let from_account = Account::new(100, 1, &mock_program_id);
+        let to_account = Account::new(0, 1, &mock_program_id);
+        bank.store_account(&from_pubkey, &from_account);
+        bank.store_account(&to_pubkey, &to_account);
+
+        let account_metas = vec![
+            AccountMeta::new(from_pubkey, false),
+            AccountMeta::new(to_pubkey, false),
+            AccountMeta::new(dup_pubkey, false),
+        ];
+        let instruction = Instruction::new(mock_program_id, &10, account_metas);
+        let tx = Transaction::new_signed_with_payer(
+            vec![instruction],
+            Some(&mint_keypair.pubkey()),
+            &[&mint_keypair],
+            bank.last_blockhash(),
+        );
+
+        let result = bank.process_transaction(&tx);
+        assert_eq!(result, Ok(()));
+        assert_eq!(bank.get_balance(&from_pubkey), 80);
+        assert_eq!(bank.get_balance(&to_pubkey), 20);
+    }
 }