Remove `index_in_caller` from `InstructionAccount` (#7220)

* Unify definitions of index in instruction

* Calculate index in caller on the fly

* Remove parameter from constructor

* Adjust comment

* Inline function

ledger-tool/src/program.rs

@@ -408,7 +408,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
                 pubkey,
                 AccountSharedData::new(0, allocation_size, &Pubkey::new_unique()),
             ));
-            instruction_accounts.push(InstructionAccount::new(0, 0, 0, false, true));
+            instruction_accounts.push(InstructionAccount::new(0, 0, false, true));
             vec![]
         }
         Err(_) => {
@@ -480,7 +480,6 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
                         idx
                     };
                     InstructionAccount::new(
-                        txn_acct_index as IndexOfAccount,
                         txn_acct_index as IndexOfAccount,
                         txn_acct_index as IndexOfAccount,
                         account_info.is_signer.unwrap_or(false),

program-runtime/src/invoke_context.rs

@@ -368,23 +368,9 @@ impl<'a> InvokeContext<'a> {
                     };
                     instruction_accounts.push(cloned_account);
                 } else {
-                    let index_in_caller = instruction_context
-                        .find_index_of_instruction_account(
-                            self.transaction_context,
-                            &account_meta.pubkey,
-                        )
-                        .ok_or_else(|| {
-                            ic_msg!(
-                                self,
-                                "Instruction references an unknown account {}",
-                                account_meta.pubkey,
-                            );
-                            InstructionError::MissingAccount
-                        })?;
                     *index_in_callee = instruction_accounts.len() as u8;
                     instruction_accounts.push(InstructionAccount::new(
                         index_in_transaction,
-                        index_in_caller,
                         instruction_account_index as IndexOfAccount,
                         account_meta.is_signer,
                         account_meta.is_writable,
@@ -413,10 +399,11 @@ impl<'a> InvokeContext<'a> {
                     continue;
                 }
 
-                let borrowed_account = instruction_context.try_borrow_instruction_account(
-                    self.transaction_context,
-                    instruction_account.index_in_caller,
+                let index_in_caller = instruction_context.get_index_of_account_in_instruction(
+                    instruction_account.index_in_transaction,
                 )?;
+                let borrowed_account = instruction_context
+                    .try_borrow_instruction_account(self.transaction_context, index_in_caller)?;
 
                 // Readonly in caller cannot become writable in callee
                 if instruction_account.is_writable() && !borrowed_account.is_writable() {
@@ -506,7 +493,6 @@ impl<'a> InvokeContext<'a> {
 
             let index_in_transaction = *index_in_transaction as usize;
             instruction_accounts.push(InstructionAccount::new(
-                index_in_transaction as IndexOfAccount,
                 index_in_transaction as IndexOfAccount,
                 *index_in_callee as IndexOfAccount,
                 message.is_signer(index_in_transaction),
@@ -901,7 +887,6 @@ pub fn mock_process_instruction_with_feature_set<
             })
             .unwrap_or(instruction_account_index) as IndexOfAccount;
         instruction_accounts.push(InstructionAccount::new(
-            index_in_transaction,
             index_in_transaction,
             index_in_callee,
             account_meta.is_signer,
@@ -1022,7 +1007,6 @@ mod tests {
             let instruction_accounts = (0..4)
                 .map(|instruction_account_index| {
                     InstructionAccount::new(
-                        instruction_account_index,
                         instruction_account_index,
                         instruction_account_index,
                         false,
@@ -1128,7 +1112,6 @@ mod tests {
                 AccountSharedData::new(index as u64, 1, invoke_stack.get(index).unwrap()),
             ));
             instruction_accounts.push(InstructionAccount::new(
-                index as IndexOfAccount,
                 index as IndexOfAccount,
                 instruction_accounts.len() as IndexOfAccount,
                 false,
@@ -1141,7 +1124,6 @@ mod tests {
                 AccountSharedData::new(1, 1, &solana_pubkey::Pubkey::default()),
             ));
             instruction_accounts.push(InstructionAccount::new(
-                index as IndexOfAccount,
                 index as IndexOfAccount,
                 index as IndexOfAccount,
                 false,
@@ -1219,7 +1201,6 @@ mod tests {
         let instruction_accounts = (0..4)
             .map(|instruction_account_index| {
                 InstructionAccount::new(
-                    instruction_account_index,
                     instruction_account_index,
                     instruction_account_index,
                     false,
@@ -1277,7 +1258,6 @@ mod tests {
         let instruction_accounts = (0..4)
             .map(|instruction_account_index| {
                 InstructionAccount::new(
-                    instruction_account_index,
                     instruction_account_index,
                     instruction_account_index,
                     false,
@@ -1368,8 +1348,8 @@ mod tests {
             (program_key, program_account),
         ];
         let instruction_accounts = vec![
-            InstructionAccount::new(0, 0, 0, false, true),
-            InstructionAccount::new(1, 1, 1, false, false),
+            InstructionAccount::new(0, 0, false, true),
+            InstructionAccount::new(1, 1, false, false),
         ];
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
         let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default();

program-runtime/src/serialization.rs

@@ -631,7 +631,6 @@ mod tests {
                     .position(|account_index| account_index == index_in_transaction)
                     .unwrap_or(index_in_instruction);
                 InstructionAccount::new(
-                    *index_in_transaction,
                     *index_in_transaction,
                     index_in_callee as IndexOfAccount,
                     false,

program-test/src/lib.rs

@@ -295,11 +295,11 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs {
                 .ok_or(InstructionError::MissingAccount)
                 .unwrap();
             let account_info = &account_infos[account_info_index];
+            let index_in_caller = instruction_context
+                .get_index_of_account_in_instruction(instruction_account.index_in_transaction)
+                .unwrap();
             let mut borrowed_account = instruction_context
-                .try_borrow_instruction_account(
-                    transaction_context,
-                    instruction_account.index_in_caller,
-                )
+                .try_borrow_instruction_account(transaction_context, index_in_caller)
                 .unwrap();
             if borrowed_account.get_lamports() != account_info.lamports() {
                 borrowed_account
@@ -324,7 +324,8 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs {
                     .unwrap();
             }
             if instruction_account.is_writable() {
-                account_indices.push((instruction_account.index_in_caller, account_info_index));
+                account_indices
+                    .push((instruction_account.index_in_transaction, account_info_index));
             }
         }
 
@@ -338,7 +339,10 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs {
         let instruction_context = transaction_context
             .get_current_instruction_context()
             .unwrap();
-        for (index_in_caller, account_info_index) in account_indices.into_iter() {
+        for (index_in_transaction, account_info_index) in account_indices.into_iter() {
+            let index_in_caller = instruction_context
+                .get_index_of_account_in_instruction(index_in_transaction)
+                .unwrap();
             let borrowed_account = instruction_context
                 .try_borrow_instruction_account(transaction_context, index_in_caller)
                 .unwrap();

programs/bpf_loader/benches/serialization.rs

@@ -95,7 +95,6 @@ fn create_inputs(owner: Pubkey, num_instruction_accounts: usize) -> TransactionC
             .unwrap_or(instruction_account_index) as IndexOfAccount;
         instruction_accounts.push(InstructionAccount::new(
             index_in_transaction,
-            instruction_account_index as IndexOfAccount,
             index_in_callee,
             false,
             instruction_account_index >= 4,

programs/sbf/benches/bpf_loader.rs

@@ -61,7 +61,7 @@ macro_rules! with_mock_invoke_context {
                 AccountSharedData::new(2, $account_size, &program_key),
             ),
         ];
-        let instruction_accounts = vec![InstructionAccount::new(2, 2, 0, false, true)];
+        let instruction_accounts = vec![InstructionAccount::new(2, 0, false, true)];
         solana_program_runtime::with_mock_invoke_context!(
             $invoke_context,
             transaction_context,

programs/system/src/system_instruction.rs

@@ -294,8 +294,8 @@ mod test {
                 (system_program::id(), AccountSharedData::default()),
             ];
             let $instruction_accounts = vec![
-                InstructionAccount::new(0, 0, 0, true, true),
-                InstructionAccount::new(1, 1, 1, false, true),
+                InstructionAccount::new(0, 0, true, true),
+                InstructionAccount::new(1, 1, false, true),
             ];
             with_mock_invoke_context!($invoke_context, transaction_context, transaction_accounts);
         };

programs/vote/src/vote_state/mod.rs

@@ -1169,7 +1169,7 @@ mod tests {
         let mut instruction_context = InstructionContext::default();
         instruction_context.configure(
             vec![0],
-            vec![InstructionAccount::new(1, 1, 0, false, true)],
+            vec![InstructionAccount::new(1, 0, false, true)],
             &[],
         );
 
@@ -1318,7 +1318,7 @@ mod tests {
         let mut instruction_context = InstructionContext::default();
         instruction_context.configure(
             vec![0],
-            vec![InstructionAccount::new(1, 1, 0, false, true)],
+            vec![InstructionAccount::new(1, 0, false, true)],
             &[],
         );
 

svm/tests/conformance.rs

@@ -29,7 +29,7 @@ use {
     solana_sysvar_id::SysvarId,
     solana_timings::ExecuteTimings,
     solana_transaction_context::{
-        ExecutionRecord, IndexOfAccount, InstructionAccount, TransactionAccount, TransactionContext,
+        ExecutionRecord, IndexOfAccount, TransactionAccount, TransactionContext,
     },
     std::{
         collections::{hash_map::Entry, HashMap},
@@ -389,31 +389,6 @@ fn execute_fixture_as_instr(
         SVMTransactionExecutionCost::default(),
     );
 
-    let mut instruction_accounts: Vec<InstructionAccount> =
-        Vec::with_capacity(sanitized_message.instructions()[0].accounts.len());
-
-    for (instruction_acct_idx, index_txn) in sanitized_message.instructions()[0]
-        .accounts
-        .iter()
-        .enumerate()
-    {
-        let index_in_callee = sanitized_message.instructions()[0]
-            .accounts
-            .get(0..instruction_acct_idx)
-            .unwrap()
-            .iter()
-            .position(|idx| *idx == *index_txn)
-            .unwrap_or(instruction_acct_idx);
-
-        instruction_accounts.push(InstructionAccount::new(
-            *index_txn as IndexOfAccount,
-            *index_txn as IndexOfAccount,
-            index_in_callee as IndexOfAccount,
-            sanitized_message.is_signer(*index_txn as usize),
-            sanitized_message.is_writable(*index_txn as usize),
-        ));
-    }
-
     invoke_context
         .prepare_next_top_level_instruction(
             sanitized_message,

syscalls/src/cpi.rs

@@ -797,10 +797,10 @@ where
             continue; // Skip duplicate account
         }
 
-        let callee_account = instruction_context.try_borrow_instruction_account(
-            transaction_context,
-            instruction_account.index_in_caller,
-        )?;
+        let index_in_caller = instruction_context
+            .get_index_of_account_in_instruction(instruction_account.index_in_transaction)?;
+        let callee_account = instruction_context
+            .try_borrow_instruction_account(transaction_context, index_in_caller)?;
         let account_key = invoke_context
             .transaction_context
             .get_key_of_account_at_index(instruction_account.index_in_transaction)?;
@@ -817,16 +817,17 @@ where
         } else if let Some(caller_account_index) =
             account_info_keys.iter().position(|key| *key == account_key)
         {
-            let serialized_metadata = accounts_metadata
-                .get(instruction_account.index_in_caller as usize)
-                .ok_or_else(|| {
-                    ic_msg!(
-                        invoke_context,
-                        "Internal error: index mismatch for account {}",
-                        account_key
-                    );
-                    Box::new(InstructionError::MissingAccount)
-                })?;
+            let serialized_metadata =
+                accounts_metadata
+                    .get(index_in_caller as usize)
+                    .ok_or_else(|| {
+                        ic_msg!(
+                            invoke_context,
+                            "Internal error: index mismatch for account {}",
+                            account_key
+                        );
+                        Box::new(InstructionError::MissingAccount)
+                    })?;
 
             // build the CallerAccount corresponding to this account.
             if caller_account_index >= account_infos.len() {
@@ -857,7 +858,7 @@ where
             )?;
 
             accounts.push(TranslatedAccount {
-                index_in_caller: instruction_account.index_in_caller,
+                index_in_caller,
                 caller_account,
                 update_caller_account_region: instruction_account.is_writable() || update_caller,
                 update_caller_account_info: instruction_account.is_writable(),
@@ -1309,7 +1310,6 @@ mod tests {
                 .enumerate()
                 .map(|(index_in_callee, index_in_transaction)| {
                     InstructionAccount::new(
-                        *index_in_transaction as IndexOfAccount,
                         *index_in_transaction as IndexOfAccount,
                         index_in_callee as IndexOfAccount,
                         false,
@@ -1855,8 +1855,8 @@ mod tests {
             .configure(
                 vec![0],
                 vec![
-                    InstructionAccount::new(1, 0, 0, false, true),
-                    InstructionAccount::new(1, 0, 0, false, true),
+                    InstructionAccount::new(1, 0, false, true),
+                    InstructionAccount::new(1, 0, false, true),
                 ],
                 &[],
             );

syscalls/src/lib.rs

@@ -4448,7 +4448,6 @@ mod tests {
             {
                 let instruction_accounts = vec![InstructionAccount::new(
                     index_in_trace.saturating_add(1) as IndexOfAccount,
-                    0, // This is incorrect / inconsistent but not required
                     0,
                     false,
                     false,

transaction-context/src/lib.rs

@@ -60,10 +60,6 @@ pub type IndexOfAccount = u16;
 pub struct InstructionAccount {
     /// Points to the account and its key in the `TransactionContext`
     pub index_in_transaction: IndexOfAccount,
-    /// Points to the first occurrence in the parent `InstructionContext`
-    ///
-    /// This excludes the program accounts.
-    pub index_in_caller: IndexOfAccount,
     /// Points to the first occurrence in the current `InstructionContext`
     ///
     /// This excludes the program accounts.
@@ -77,14 +73,12 @@ pub struct InstructionAccount {
 impl InstructionAccount {
     pub fn new(
         index_in_transaction: IndexOfAccount,
-        index_in_caller: IndexOfAccount,
         index_in_callee: IndexOfAccount,
         is_signer: bool,
         is_writable: bool,
     ) -> InstructionAccount {
         InstructionAccount {
             index_in_transaction,
-            index_in_caller,
             index_in_callee,
             is_signer: is_signer as u8,
             is_writable: is_writable as u8,
@@ -736,6 +730,18 @@ impl InstructionContext {
             .index_in_transaction as IndexOfAccount)
     }
 
+    /// Get the index of account in instruction from the index in transaction
+    pub fn get_index_of_account_in_instruction(
+        &self,
+        index_in_transaction: IndexOfAccount,
+    ) -> Result<IndexOfAccount, InstructionError> {
+        self.instruction_accounts
+            .iter()
+            .position(|account| account.index_in_transaction == index_in_transaction)
+            .map(|idx| idx as IndexOfAccount)
+            .ok_or(InstructionError::MissingAccount)
+    }
+
     /// Returns `Some(instruction_account_index)` if this is a duplicate
     /// and `None` if it is the first account with this key
     pub fn is_instruction_account_duplicate(
@@ -771,7 +777,7 @@ impl InstructionContext {
         &'a self,
         transaction_context: &'b TransactionContext,
         index_in_transaction: IndexOfAccount,
-        index_in_instruction: IndexOfAccount,
+        index_in_instruction: Option<IndexOfAccount>,
     ) -> Result<BorrowedAccount<'a>, InstructionError> {
         let account = transaction_context
             .accounts
@@ -783,7 +789,7 @@ impl InstructionContext {
             transaction_context,
             instruction_context: self,
             index_in_transaction,
-            index_in_instruction,
+            index_in_instruction_accounts: index_in_instruction,
             account,
         })
     }
@@ -809,11 +815,7 @@ impl InstructionContext {
     ) -> Result<BorrowedAccount<'a>, InstructionError> {
         let index_in_transaction =
             self.get_index_of_program_account_in_transaction(program_account_index)?;
-        self.try_borrow_account(
-            transaction_context,
-            index_in_transaction,
-            program_account_index,
-        )
+        self.try_borrow_account(transaction_context, index_in_transaction, None)
     }
 
     /// Gets an instruction account of this Instruction
@@ -827,8 +829,7 @@ impl InstructionContext {
         self.try_borrow_account(
             transaction_context,
             index_in_transaction,
-            self.get_number_of_program_accounts()
-                .saturating_add(instruction_account_index),
+            Some(instruction_account_index),
         )
     }
 
@@ -884,7 +885,8 @@ pub struct BorrowedAccount<'a> {
     transaction_context: &'a TransactionContext,
     instruction_context: &'a InstructionContext,
     index_in_transaction: IndexOfAccount,
-    index_in_instruction: IndexOfAccount,
+    // Program accounts are not part of the instruction_accounts vector, and thus None
+    index_in_instruction_accounts: Option<IndexOfAccount>,
     account: RefMut<'a, AccountSharedData>,
 }
 
@@ -1195,28 +1197,24 @@ impl BorrowedAccount<'_> {
 
     /// Returns whether this account is a signer (instruction wide)
     pub fn is_signer(&self) -> bool {
-        if self.index_in_instruction < self.instruction_context.get_number_of_program_accounts() {
-            return false;
+        if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts {
+            self.instruction_context
+                .is_instruction_account_signer(index_in_instruction_accounts)
+                .unwrap_or_default()
+        } else {
+            false
         }
-        self.instruction_context
-            .is_instruction_account_signer(
-                self.index_in_instruction
-                    .saturating_sub(self.instruction_context.get_number_of_program_accounts()),
-            )
-            .unwrap_or_default()
     }
 
     /// Returns whether this account is writable (instruction wide)
     pub fn is_writable(&self) -> bool {
-        if self.index_in_instruction < self.instruction_context.get_number_of_program_accounts() {
-            return false;
+        if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts {
+            self.instruction_context
+                .is_instruction_account_writable(index_in_instruction_accounts)
+                .unwrap_or_default()
+        } else {
+            false
         }
-        self.instruction_context
-            .is_instruction_account_writable(
-                self.index_in_instruction
-                    .saturating_sub(self.instruction_context.get_number_of_program_accounts()),
-            )
-            .unwrap_or_default()
     }
 
     /// Returns true if the owner of this account is the current `InstructionContext`s last program (instruction wide)