Refactor: Remove KeyedAccounts (2) (#22274)

* Adds InstructionContext::get_signers().
Improves error messages when modifying borrowed accounts.

* Removes keyed_accounts from InvokeContext tests.

* Removes keyed_accounts from message_processor.rs

* Removes keyed_accounts from bank.rs

* Removes keyed_accounts from bpf serialization.

program-runtime/src/invoke_context.rs

@@ -1064,10 +1064,7 @@ mod tests {
     use {
         super::*,
         serde::{Deserialize, Serialize},
-        solana_sdk::{
+        solana_sdk::account::{ReadableAccount, WritableAccount},
-            account::{ReadableAccount, WritableAccount},
-            keyed_account::keyed_account_at_index,
-        },
     };
 
     #[derive(Debug, Serialize, Deserialize)]
@@ -1167,41 +1164,41 @@ mod tests {
 
     #[allow(clippy::integer_arithmetic)]
     fn mock_process_instruction(
-        first_instruction_account: usize,
+        _first_instruction_account: usize,
         data: &[u8],
         invoke_context: &mut InvokeContext,
     ) -> Result<(), InstructionError> {
         let transaction_context = &invoke_context.transaction_context;
+        let instruction_context = transaction_context.get_current_instruction_context()?;
         let program_id = transaction_context.get_program_key()?;
-        let keyed_accounts = invoke_context.get_keyed_accounts()?;
         assert_eq!(
-            *program_id,
+            program_id,
-            keyed_account_at_index(keyed_accounts, first_instruction_account)?.owner()?
+            instruction_context
+                .try_borrow_instruction_account(transaction_context, 0)?
+                .get_owner()
         );
         assert_ne!(
-            keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?.owner()?,
+            instruction_context
-            *keyed_account_at_index(keyed_accounts, first_instruction_account)?.unsigned_key()
+                .try_borrow_instruction_account(transaction_context, 1)?
+                .get_owner(),
+            instruction_context
+                .try_borrow_instruction_account(transaction_context, 0)?
+                .get_key()
         );
 
         if let Ok(instruction) = bincode::deserialize(data) {
             match instruction {
                 MockInstruction::NoopSuccess => (),
                 MockInstruction::NoopFail => return Err(InstructionError::GenericError),
-                MockInstruction::ModifyOwned => {
+                MockInstruction::ModifyOwned => instruction_context
-                    keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                    .try_borrow_instruction_account(transaction_context, 0)?
-                        .try_account_ref_mut()?
+                    .set_data(&[1])?,
-                        .data_as_mut_slice()[0] = 1
+                MockInstruction::ModifyNotOwned => instruction_context
-                }
+                    .try_borrow_instruction_account(transaction_context, 1)?
-                MockInstruction::ModifyNotOwned => {
+                    .set_data(&[1])?,
-                    keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+                MockInstruction::ModifyReadonly => instruction_context
-                        .try_account_ref_mut()?
+                    .try_borrow_instruction_account(transaction_context, 2)?
-                        .data_as_mut_slice()[0] = 1
+                    .set_data(&[1])?,
-                }
-                MockInstruction::ModifyReadonly => {
-                    keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?
-                        .try_account_ref_mut()?
-                        .data_as_mut_slice()[0] = 1
-                }
                 MockInstruction::ConsumeComputeUnits {
                     compute_units_to_consume,
                     desired_result,
@@ -1213,12 +1210,9 @@ mod tests {
                         .unwrap();
                     return desired_result;
                 }
-                MockInstruction::Resize { new_len } => {
+                MockInstruction::Resize { new_len } => instruction_context
-                    keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                    .try_borrow_instruction_account(transaction_context, 0)?
-                        .try_account_ref_mut()?
+                    .set_data(&vec![0; new_len])?,
-                        .data_mut()
-                        .resize_with(new_len, Default::default)
-                }
             }
         } else {
             return Err(InstructionError::InvalidInstructionData);

programs/bpf_loader/src/serialization.rs

@@ -5,7 +5,6 @@ use {
         bpf_loader_deprecated,
         entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE},
         instruction::InstructionError,
-        keyed_account::KeyedAccount,
         pubkey::Pubkey,
         system_instruction::MAX_PERMITTED_DATA_LENGTH,
         transaction_context::{InstructionContext, TransactionContext},
@@ -66,7 +65,12 @@ pub fn deserialize_parameters(
         .get_owner()
         == bpf_loader_deprecated::id();
     if is_loader_deprecated {
-        deserialize_parameters_unaligned(transaction_context, instruction_context, buffer)
+        deserialize_parameters_unaligned(
+            transaction_context,
+            instruction_context,
+            buffer,
+            account_lengths,
+        )
     } else {
         deserialize_parameters_aligned(
             transaction_context,
@@ -78,23 +82,6 @@ pub fn deserialize_parameters(
     }
 }
 
-pub fn get_serialized_account_size_unaligned(
-    keyed_account: &KeyedAccount,
-) -> Result<usize, InstructionError> {
-    let data_len = keyed_account.data_len()?;
-    Ok(
-        size_of::<u8>() // is_signer
-            + size_of::<u8>() // is_writable
-            + size_of::<Pubkey>() // key
-            + size_of::<u64>()  // lamports
-            + size_of::<u64>()  // data len
-            + data_len // data
-            + size_of::<Pubkey>() // owner
-            + size_of::<u8>() // executable
-            + size_of::<u64>(), // rent_epoch
-    )
-}
-
 pub fn serialize_parameters_unaligned(
     transaction_context: &TransactionContext,
     instruction_context: &InstructionContext,
@@ -179,10 +166,12 @@ pub fn deserialize_parameters_unaligned(
     transaction_context: &TransactionContext,
     instruction_context: &InstructionContext,
     buffer: &[u8],
+    account_lengths: &[usize],
 ) -> Result<(), InstructionError> {
     let mut start = size_of::<u64>(); // number of accounts
-    for index_in_instruction in instruction_context.get_number_of_program_accounts()
+    for (index_in_instruction, pre_len) in (instruction_context.get_number_of_program_accounts()
-        ..instruction_context.get_number_of_accounts()
+        ..instruction_context.get_number_of_accounts())
+        .zip(account_lengths.iter())
     {
         let duplicate = is_duplicate(instruction_context, index_in_instruction);
         start += 1; // is_dup
@@ -195,9 +184,8 @@ pub fn deserialize_parameters_unaligned(
             let _ = borrowed_account.set_lamports(LittleEndian::read_u64(&buffer[start..]));
             start += size_of::<u64>() // lamports
                 + size_of::<u64>(); // data length
-            let end = start + borrowed_account.get_data().len();
+            let _ = borrowed_account.set_data(&buffer[start..start + pre_len]);
-            let _ = borrowed_account.set_data(&buffer[start..end]);
+            start += pre_len // data
-            start += borrowed_account.get_data().len() // data
                 + size_of::<Pubkey>() // owner
                 + size_of::<u8>() // executable
                 + size_of::<u64>(); // rent_epoch
@@ -363,7 +351,7 @@ mod tests {
         super::*,
         solana_program_runtime::invoke_context::{prepare_mock_invoke_context, InvokeContext},
         solana_sdk::{
-            account::{Account, AccountSharedData, ReadableAccount},
+            account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
             account_info::AccountInfo,
             bpf_loader,
             entrypoint::deserialize,
@@ -462,6 +450,7 @@ mod tests {
             .collect();
         let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
         let program_indices = [0];
+        let mut original_accounts = transaction_accounts.clone();
         let preparation = prepare_mock_invoke_context(
             transaction_accounts,
             instruction_accounts,
@@ -524,7 +513,15 @@ mod tests {
             );
         }
 
-        let de_keyed_accounts = invoke_context.get_keyed_accounts().unwrap();
+        for index_in_transaction in 1..original_accounts.len() {
+            let mut account = invoke_context
+                .transaction_context
+                .get_account_at_index(index_in_transaction)
+                .borrow_mut();
+            account.set_lamports(0);
+            account.set_data(vec![0; 0]);
+            account.set_owner(Pubkey::default());
+        }
         deserialize_parameters(
             invoke_context.transaction_context,
             instruction_context,
@@ -533,20 +530,19 @@ mod tests {
             true,
         )
         .unwrap();
-        for keyed_account in de_keyed_accounts {
+        for (index_in_transaction, (_key, original_account)) in original_accounts.iter().enumerate()
-            let index_in_transaction = invoke_context
+        {
-                .transaction_context
-                .find_index_of_account(keyed_account.unsigned_key())
-                .unwrap();
             let account = invoke_context
                 .transaction_context
                 .get_account_at_index(index_in_transaction)
                 .borrow();
-            assert_eq!(account.executable(), keyed_account.executable().unwrap());
+            assert_eq!(&*account, original_account);
-            assert_eq!(account.rent_epoch(), keyed_account.rent_epoch().unwrap());
         }
 
         // check serialize_parameters_unaligned
+        original_accounts[0]
+            .1
+            .set_owner(bpf_loader_deprecated::id());
         let _ = invoke_context
             .transaction_context
             .get_current_instruction_context()
@@ -578,7 +574,14 @@ mod tests {
             assert_eq!(account.rent_epoch(), account_info.rent_epoch);
         }
 
-        let de_keyed_accounts = invoke_context.get_keyed_accounts().unwrap();
+        for index_in_transaction in 1..original_accounts.len() {
+            let mut account = invoke_context
+                .transaction_context
+                .get_account_at_index(index_in_transaction)
+                .borrow_mut();
+            account.set_lamports(0);
+            account.set_data(vec![0; 0]);
+        }
         deserialize_parameters(
             invoke_context.transaction_context,
             instruction_context,
@@ -587,23 +590,13 @@ mod tests {
             true,
         )
         .unwrap();
-        for keyed_account in de_keyed_accounts {
+        for (index_in_transaction, (_key, original_account)) in original_accounts.iter().enumerate()
-            let index_in_transaction = invoke_context
+        {
-                .transaction_context
-                .find_index_of_account(keyed_account.unsigned_key())
-                .unwrap();
             let account = invoke_context
                 .transaction_context
                 .get_account_at_index(index_in_transaction)
                 .borrow();
-            assert_eq!(account.lamports(), keyed_account.lamports().unwrap());
+            assert_eq!(&*account, original_account);
-            assert_eq!(
-                account.data(),
-                keyed_account.try_account_ref().unwrap().data()
-            );
-            assert_eq!(*account.owner(), keyed_account.owner().unwrap());
-            assert_eq!(account.executable(), keyed_account.executable().unwrap());
-            assert_eq!(account.rent_epoch(), keyed_account.rent_epoch().unwrap());
         }
     }
 

runtime/src/bank.rs

@@ -6285,7 +6285,6 @@ pub(crate) mod tests {
             genesis_config::create_genesis_config,
             hash,
             instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError},
-            keyed_account::keyed_account_at_index,
             message::{Message, MessageHeader},
             nonce,
             poh_config::PohConfig,
@@ -6737,63 +6736,6 @@ pub(crate) mod tests {
         assert_eq!(bank_with_success_txs_hash, bank_hash);
     }
 
-    #[derive(Serialize, Deserialize)]
-    enum MockInstruction {
-        Deduction,
-    }
-
-    fn mock_process_instruction(
-        first_instruction_account: usize,
-        data: &[u8],
-        invoke_context: &mut InvokeContext,
-    ) -> result::Result<(), InstructionError> {
-        let keyed_accounts = invoke_context.get_keyed_accounts()?;
-        if let Ok(instruction) = bincode::deserialize(data) {
-            match instruction {
-                MockInstruction::Deduction => {
-                    keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
-                        .account
-                        .borrow_mut()
-                        .checked_add_lamports(1)?;
-                    keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?
-                        .account
-                        .borrow_mut()
-                        .checked_sub_lamports(1)?;
-                    Ok(())
-                }
-            }
-        } else {
-            Err(InstructionError::InvalidInstructionData)
-        }
-    }
-
-    fn create_mock_transaction(
-        payer: &Keypair,
-        keypair1: &Keypair,
-        keypair2: &Keypair,
-        read_only_keypair: &Keypair,
-        mock_program_id: Pubkey,
-        recent_blockhash: Hash,
-    ) -> Transaction {
-        let account_metas = vec![
-            AccountMeta::new(payer.pubkey(), true),
-            AccountMeta::new(keypair1.pubkey(), true),
-            AccountMeta::new(keypair2.pubkey(), true),
-            AccountMeta::new_readonly(read_only_keypair.pubkey(), false),
-        ];
-        let deduct_instruction = Instruction::new_with_bincode(
-            mock_program_id,
-            &MockInstruction::Deduction,
-            account_metas,
-        );
-        Transaction::new_signed_with_payer(
-            &[deduct_instruction],
-            Some(&payer.pubkey()),
-            &[payer, keypair1, keypair2],
-            recent_blockhash,
-        )
-    }
-
     fn store_accounts_for_rent_test(
         bank: &Bank,
         keypairs: &mut Vec<Keypair>,
@@ -6893,7 +6835,6 @@ pub(crate) mod tests {
     fn create_child_bank_for_rent_test(
         root_bank: &Arc<Bank>,
         genesis_config: &GenesisConfig,
-        mock_program_id: Pubkey,
     ) -> Bank {
         let mut bank = Bank::new_from_parent(
             root_bank,
@@ -6905,8 +6846,6 @@ pub(crate) mod tests {
             ) as u64,
         );
         bank.rent_collector.slots_per_year = 421_812.0;
-        bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction);
-
         bank
     }
 
@@ -7305,11 +7244,7 @@ pub(crate) mod tests {
         };
 
         let root_bank = Arc::new(Bank::new_for_tests(&genesis_config));
-        let bank = create_child_bank_for_rent_test(
+        let bank = create_child_bank_for_rent_test(&root_bank, &genesis_config);
-            &root_bank,
-            &genesis_config,
-            solana_sdk::pubkey::new_rand(),
-        );
 
         let account_pubkey = solana_sdk::pubkey::new_rand();
         let account_balance = 1;
@@ -7339,6 +7274,35 @@ pub(crate) mod tests {
         solana_logger::setup();
         let mock_program_id = Pubkey::new(&[2u8; 32]);
 
+        #[derive(Serialize, Deserialize)]
+        enum MockInstruction {
+            Deduction,
+        }
+
+        fn mock_process_instruction(
+            _first_instruction_account: usize,
+            data: &[u8],
+            invoke_context: &mut InvokeContext,
+        ) -> result::Result<(), InstructionError> {
+            let transaction_context = &invoke_context.transaction_context;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
+            if let Ok(instruction) = bincode::deserialize(data) {
+                match instruction {
+                    MockInstruction::Deduction => {
+                        instruction_context
+                            .try_borrow_instruction_account(transaction_context, 1)?
+                            .checked_add_lamports(1)?;
+                        instruction_context
+                            .try_borrow_instruction_account(transaction_context, 2)?
+                            .checked_sub_lamports(1)?;
+                        Ok(())
+                    }
+                }
+            } else {
+                Err(InstructionError::InvalidInstructionData)
+            }
+        }
+
         let (mut genesis_config, _mint_keypair) = create_genesis_config(10);
         let mut keypairs: Vec<Keypair> = Vec::with_capacity(14);
         for _i in 0..14 {
@@ -7356,7 +7320,8 @@ pub(crate) mod tests {
         // we must ensure lazy rent collection doens't get broken!
         root_bank.restore_old_behavior_for_fragile_tests();
         let root_bank = Arc::new(root_bank);
-        let bank = create_child_bank_for_rent_test(&root_bank, &genesis_config, mock_program_id);
+        let mut bank = create_child_bank_for_rent_test(&root_bank, &genesis_config);
+        bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction);
 
         assert_eq!(bank.last_blockhash(), genesis_config.hash());
 
@@ -7413,12 +7378,21 @@ pub(crate) mod tests {
             genesis_config.hash(),
         );
 
-        let t6 = create_mock_transaction(
+        let account_metas = vec![
-            &keypairs[10],
+            AccountMeta::new(keypairs[10].pubkey(), true),
-            &keypairs[11],
+            AccountMeta::new(keypairs[11].pubkey(), true),
-            &keypairs[12],
+            AccountMeta::new(keypairs[12].pubkey(), true),
-            &keypairs[13],
+            AccountMeta::new_readonly(keypairs[13].pubkey(), false),
+        ];
+        let deduct_instruction = Instruction::new_with_bincode(
             mock_program_id,
+            &MockInstruction::Deduction,
+            account_metas,
+        );
+        let t6 = Transaction::new_signed_with_payer(
+            &[deduct_instruction],
+            Some(&keypairs[10].pubkey()),
+            &[&keypairs[10], &keypairs[11], &keypairs[12]],
             genesis_config.hash(),
         );
 
@@ -11491,23 +11465,24 @@ pub(crate) mod tests {
         let mut bank = Bank::new_for_tests(&genesis_config);
 
         fn mock_process_instruction(
-            first_instruction_account: usize,
+            _first_instruction_account: usize,
             data: &[u8],
             invoke_context: &mut InvokeContext,
         ) -> result::Result<(), InstructionError> {
-            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            let transaction_context = &invoke_context.transaction_context;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
             let lamports = data[0] as u64;
-            keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?
+            instruction_context
-                .try_account_ref_mut()?
+                .try_borrow_instruction_account(transaction_context, 2)?
                 .checked_sub_lamports(lamports)?;
-            keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+            instruction_context
-                .try_account_ref_mut()?
+                .try_borrow_instruction_account(transaction_context, 1)?
                 .checked_add_lamports(lamports)?;
-            keyed_account_at_index(keyed_accounts, first_instruction_account)?
+            instruction_context
-                .try_account_ref_mut()?
+                .try_borrow_instruction_account(transaction_context, 0)?
                 .checked_sub_lamports(lamports)?;
-            keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+            instruction_context
-                .try_account_ref_mut()?
+                .try_borrow_instruction_account(transaction_context, 1)?
                 .checked_add_lamports(lamports)?;
             Ok(())
         }
@@ -12022,14 +11997,15 @@ pub(crate) mod tests {
     #[test]
     fn test_same_program_id_uses_unqiue_executable_accounts() {
         fn nested_processor(
-            first_instruction_account: usize,
+            _first_instruction_account: usize,
             _data: &[u8],
             invoke_context: &mut InvokeContext,
         ) -> result::Result<(), InstructionError> {
-            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            let transaction_context = &invoke_context.transaction_context;
-            let account = keyed_account_at_index(keyed_accounts, first_instruction_account)?;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
-            assert_eq!(42, account.lamports().unwrap());
+            let _ = instruction_context
-            account.try_account_ref_mut()?.checked_add_lamports(1)?;
+                .try_borrow_account(transaction_context, 1)?
+                .checked_add_lamports(1);
             Ok(())
         }
 
@@ -14761,15 +14737,15 @@ pub(crate) mod tests {
         let mut bank = Bank::new_for_tests(&genesis_config);
 
         fn mock_ix_processor(
-            first_instruction_account: usize,
+            _first_instruction_account: usize,
             _data: &[u8],
             invoke_context: &mut InvokeContext,
         ) -> std::result::Result<(), InstructionError> {
-            use solana_sdk::account::WritableAccount;
+            let transaction_context = &invoke_context.transaction_context;
-            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
-            let data = keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?;
+            instruction_context
-            data.try_account_ref_mut()?.data_as_mut_slice()[0] = 5;
+                .try_borrow_instruction_account(transaction_context, 1)?
-            Ok(())
+                .set_data(&[0; 40])
         }
 
         let program_id = solana_sdk::pubkey::new_rand();
@@ -14780,7 +14756,6 @@ pub(crate) mod tests {
         let blockhash_sysvar = sysvar::clock::id();
         #[allow(deprecated)]
         let orig_lamports = bank.get_account(&sysvar::clock::id()).unwrap().lamports();
-        info!("{:?}", bank.get_account(&sysvar::clock::id()));
         let tx = system_transaction::transfer(&mint_keypair, &blockhash_sysvar, 10, blockhash);
         assert_eq!(
             bank.process_transaction(&tx),
@@ -14793,7 +14768,6 @@ pub(crate) mod tests {
             bank.get_account(&sysvar::clock::id()).unwrap().lamports(),
             orig_lamports
         );
-        info!("{:?}", bank.get_account(&sysvar::clock::id()));
 
         let accounts = vec![
             AccountMeta::new(mint_keypair.pubkey(), true),

runtime/src/message_processor.rs

@@ -160,7 +160,6 @@ mod tests {
         solana_sdk::{
             account::{AccountSharedData, ReadableAccount},
             instruction::{AccountMeta, Instruction, InstructionError},
-            keyed_account::keyed_account_at_index,
             message::Message,
             native_loader::{self, create_loadable_account_for_test},
             secp256k1_instruction::new_secp256k1_instruction,
@@ -182,36 +181,33 @@ mod tests {
         #[derive(Serialize, Deserialize)]
         enum MockSystemInstruction {
             Correct,
-            AttemptCredit { lamports: u64 },
+            TransferLamports { lamports: u64 },
-            AttemptDataChange { data: u8 },
+            ChangeData { data: u8 },
         }
 
         fn mock_system_process_instruction(
-            first_instruction_account: usize,
+            _first_instruction_account: usize,
             data: &[u8],
             invoke_context: &mut InvokeContext,
         ) -> Result<(), InstructionError> {
-            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            let transaction_context = &invoke_context.transaction_context;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
             if let Ok(instruction) = bincode::deserialize(data) {
                 match instruction {
                     MockSystemInstruction::Correct => Ok(()),
-                    MockSystemInstruction::AttemptCredit { lamports } => {
+                    MockSystemInstruction::TransferLamports { lamports } => {
-                        keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                        instruction_context
-                            .account
+                            .try_borrow_instruction_account(transaction_context, 0)?
-                            .borrow_mut()
                             .checked_sub_lamports(lamports)?;
-                        keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+                        instruction_context
-                            .account
+                            .try_borrow_instruction_account(transaction_context, 1)?
-                            .borrow_mut()
                             .checked_add_lamports(lamports)?;
                         Ok(())
                     }
-                    // Change data in a read-only account
+                    MockSystemInstruction::ChangeData { data } => {
-                    MockSystemInstruction::AttemptDataChange { data } => {
+                        instruction_context
-                        keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+                            .try_borrow_instruction_account(transaction_context, 1)?
-                            .account
+                            .set_data(&[data])?;
-                            .borrow_mut()
-                            .set_data(vec![data]);
                         Ok(())
                     }
                 }
@@ -293,7 +289,7 @@ mod tests {
         let message = Message::new(
             &[Instruction::new_with_bincode(
                 mock_system_program_id,
-                &MockSystemInstruction::AttemptCredit { lamports: 50 },
+                &MockSystemInstruction::TransferLamports { lamports: 50 },
                 account_metas.clone(),
             )],
             Some(transaction_context.get_key_of_account_at_index(0)),
@@ -326,7 +322,7 @@ mod tests {
         let message = Message::new(
             &[Instruction::new_with_bincode(
                 mock_system_program_id,
-                &MockSystemInstruction::AttemptDataChange { data: 50 },
+                &MockSystemInstruction::ChangeData { data: 50 },
                 account_metas,
             )],
             Some(transaction_context.get_key_of_account_at_index(0)),
@@ -367,67 +363,49 @@ mod tests {
         }
 
         fn mock_system_process_instruction(
-            first_instruction_account: usize,
+            _first_instruction_account: usize,
             data: &[u8],
             invoke_context: &mut InvokeContext,
         ) -> Result<(), InstructionError> {
-            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            let transaction_context = &invoke_context.transaction_context;
+            let instruction_context = transaction_context.get_current_instruction_context()?;
+            let mut to_account =
+                instruction_context.try_borrow_instruction_account(transaction_context, 1)?;
             if let Ok(instruction) = bincode::deserialize(data) {
                 match instruction {
                     MockSystemInstruction::BorrowFail => {
-                        let from_account =
+                        let from_account = instruction_context
-                            keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                            .try_borrow_instruction_account(transaction_context, 0)?;
-                                .try_account_ref_mut()?;
+                        let dup_account = instruction_context
-                        let dup_account =
+                            .try_borrow_instruction_account(transaction_context, 2)?;
-                            keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?
+                        if from_account.get_lamports() != dup_account.get_lamports() {
-                                .try_account_ref_mut()?;
-                        if from_account.lamports() != dup_account.lamports() {
                             return Err(InstructionError::InvalidArgument);
                         }
                         Ok(())
                     }
                     MockSystemInstruction::MultiBorrowMut => {
-                        let from_lamports = {
+                        let lamports_a = instruction_context
-                            let from_account =
+                            .try_borrow_instruction_account(transaction_context, 0)?
-                                keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                            .get_lamports();
-                                    .try_account_ref_mut()?;
+                        let lamports_b = instruction_context
-                            from_account.lamports()
+                            .try_borrow_instruction_account(transaction_context, 2)?
-                        };
+                            .get_lamports();
-                        let dup_lamports = {
+                        if lamports_a != lamports_b {
-                            let dup_account = keyed_account_at_index(
-                                keyed_accounts,
-                                first_instruction_account + 2,
-                            )?
-                            .try_account_ref_mut()?;
-                            dup_account.lamports()
-                        };
-                        if from_lamports != dup_lamports {
                             return Err(InstructionError::InvalidArgument);
                         }
                         Ok(())
                     }
                     MockSystemInstruction::DoWork { lamports, data } => {
-                        {
+                        let mut dup_account = instruction_context
-                            let mut to_account = keyed_account_at_index(
+                            .try_borrow_instruction_account(transaction_context, 2)?;
-                                keyed_accounts,
-                                first_instruction_account + 1,
-                            )?
-                            .try_account_ref_mut()?;
-                            let mut dup_account = keyed_account_at_index(
-                                keyed_accounts,
-                                first_instruction_account + 2,
-                            )?
-                            .try_account_ref_mut()?;
                         dup_account.checked_sub_lamports(lamports)?;
                         to_account.checked_add_lamports(lamports)?;
-                            dup_account.set_data(vec![data]);
+                        dup_account.set_data(&[data])?;
-                        }
+                        drop(dup_account);
-                        keyed_account_at_index(keyed_accounts, first_instruction_account)?
+                        let mut from_account = instruction_context
-                            .try_account_ref_mut()?
+                            .try_borrow_instruction_account(transaction_context, 0)?;
-                            .checked_sub_lamports(lamports)?;
+                        from_account.checked_sub_lamports(lamports)?;
-                        keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?
+                        to_account.checked_add_lamports(lamports)?;
-                            .try_account_ref_mut()?
-                            .checked_add_lamports(lamports)?;
                         Ok(())
                     }
                 }
@@ -528,7 +506,7 @@ mod tests {
         );
         assert!(result.is_ok());
 
-        // Do work on the same account but at different location in keyed_accounts[]
+        // Do work on the same transaction account but at different instruction accounts
         let message = Message::new(
             &[Instruction::new_with_bincode(
                 mock_program_id,

sdk/src/transaction_context.rs

@@ -8,6 +8,7 @@ use crate::{
 };
 use std::{
     cell::{RefCell, RefMut},
+    collections::HashSet,
     pin::Pin,
 };
 
@@ -284,7 +285,7 @@ impl InstructionContext {
         })
     }
 
-    /// Gets the last program account of the current InstructionContext
+    /// Gets the last program account of this Instruction
     pub fn try_borrow_program_account<'a, 'b: 'a>(
         &'a self,
         transaction_context: &'b TransactionContext,
@@ -295,7 +296,7 @@ impl InstructionContext {
         )
     }
 
-    /// Gets an instruction account of the current InstructionContext
+    /// Gets an instruction account of this Instruction
     pub fn try_borrow_instruction_account<'a, 'b: 'a>(
         &'a self,
         transaction_context: &'b TransactionContext,
@@ -308,6 +309,19 @@ impl InstructionContext {
                 .saturating_add(index_in_instruction),
         )
     }
+
+    /// Calculates the set of all keys of signer accounts in this Instruction
+    pub fn get_signers(&self, transaction_context: &TransactionContext) -> HashSet<Pubkey> {
+        let mut result = HashSet::new();
+        for instruction_account in self.instruction_accounts.iter() {
+            if instruction_account.is_signer {
+                result.insert(
+                    transaction_context.account_keys[instruction_account.index_in_transaction],
+                );
+            }
+        }
+        result
+    }
 }
 
 /// Shared account borrowed from the TransactionContext and an InstructionContext.
@@ -344,11 +358,8 @@ impl<'a> BorrowedAccount<'a> {
     /// Assignes the owner of this account (transaction wide)
     pub fn set_owner(&mut self, pubkey: &[u8]) -> Result<(), InstructionError> {
         self.account.copy_into_owner_from_slice(pubkey);
-        if self.is_writable() {
+        self.verify_writability()
-            Ok(())
+        // TODO: return Err(InstructionError::ModifiedProgramId);
-        } else {
-            Err(InstructionError::Immutable)
-        }
     }
 
     /// Returns the number of lamports of this account (transaction wide)
@@ -359,11 +370,14 @@ impl<'a> BorrowedAccount<'a> {
     /// Overwrites the number of lamports of this account (transaction wide)
     pub fn set_lamports(&mut self, lamports: u64) -> Result<(), InstructionError> {
         self.account.set_lamports(lamports);
-        if self.is_writable() {
+        if self.index_in_instruction < self.instruction_context.program_accounts.len() {
-            Ok(())
+            return Err(InstructionError::ExecutableLamportChange);
-        } else {
-            Err(InstructionError::Immutable)
         }
+        if !self.is_writable() {
+            return Err(InstructionError::ReadonlyLamportChange);
+        }
+        // TODO: return Err(InstructionError::ExternalAccountLamportSpend);
+        Ok(())
     }
 
     /// Adds lamports to this account (transaction wide)
@@ -384,6 +398,18 @@ impl<'a> BorrowedAccount<'a> {
         )
     }
 
+    /// Verifies that this account is writable (instruction wide)
+    fn verify_writability(&self) -> Result<(), InstructionError> {
+        if self.index_in_instruction < self.instruction_context.program_accounts.len() {
+            return Err(InstructionError::ExecutableDataModified);
+        }
+        if !self.is_writable() {
+            return Err(InstructionError::ReadonlyDataModified);
+        }
+        // TODO: return Err(InstructionError::ExternalAccountDataModified);
+        Ok(())
+    }
+
     /// Returns a read-only slice of the account data (transaction wide)
     pub fn get_data(&self) -> &[u8] {
         self.account.data()
@@ -395,16 +421,14 @@ impl<'a> BorrowedAccount<'a> {
             self.account.data_as_mut_slice().copy_from_slice(data);
         } else {
             self.account.set_data_from_slice(data);
+            // TODO: return Err(InstructionError::AccountDataSizeChanged);
         }
-        if self.is_writable() {
+        self.verify_writability()
-            Ok(())
-        } else {
-            Err(InstructionError::Immutable)
-        }
     }
 
     /*pub fn realloc(&self, new_len: usize, zero_init: bool) {
-        // TODO
+        // TODO: return Err(InstructionError::InvalidRealloc);
+        // TODO: return Err(InstructionError::AccountDataSizeChanged);
     }*/
 
     /// Deserializes the account data into a state
@@ -423,11 +447,7 @@ impl<'a> BorrowedAccount<'a> {
             return Err(InstructionError::AccountDataTooSmall);
         }
         bincode::serialize_into(&mut *data, state).map_err(|_| InstructionError::GenericError)?;
-        if self.is_writable() {
+        self.verify_writability()
-            Ok(())
-        } else {
-            Err(InstructionError::Immutable)
-        }
     }
 
     /// Returns whether this account is executable (transaction wide)
@@ -438,11 +458,9 @@ impl<'a> BorrowedAccount<'a> {
     /// Configures whether this account is executable (transaction wide)
     pub fn set_executable(&mut self, is_executable: bool) -> Result<(), InstructionError> {
         self.account.set_executable(is_executable);
-        if self.is_writable() {
+        self.verify_writability()
-            Ok(())
+        // TODO: return Err(InstructionError::ExecutableAccountNotRentExempt);
-        } else {
+        // TODO: return Err(InstructionError::ExecutableModified);
-            Err(InstructionError::Immutable)
-        }
     }
 
     /// Returns the rent epoch of this account (transaction wide)