Refactor: Adds `index_in_callee` to `InstructionAccount` (#25490)

* Adds InstructionAccount::index_in_callee

* Adjusts tests and benches.

* Adds documentation for InstructionAccount.

* Adds InstructionContext::is_duplicate()

program-runtime/src/invoke_context.rs

@@ -744,7 +744,7 @@ impl<'a> InvokeContext<'a> {
         let instruction_context = self.transaction_context.get_current_instruction_context()?;
         let mut deduplicated_instruction_accounts: Vec<InstructionAccount> = Vec::new();
         let mut duplicate_indicies = Vec::with_capacity(instruction.accounts.len());
-        for account_meta in instruction.accounts.iter() {
+        for (index_in_instruction, account_meta) in instruction.accounts.iter().enumerate() {
             let index_in_transaction = self
                 .transaction_context
                 .find_index_of_account(&account_meta.pubkey)
@@ -784,6 +784,7 @@ impl<'a> InvokeContext<'a> {
                 deduplicated_instruction_accounts.push(InstructionAccount {
                     index_in_transaction,
                     index_in_caller,
+                    index_in_callee: index_in_instruction,
                     is_signer: account_meta.is_signer,
                     is_writable: account_meta.is_writable,
                 });
@@ -1140,24 +1141,32 @@ pub struct MockInvokeContextPreparation {
 
 pub fn prepare_mock_invoke_context(
     transaction_accounts: Vec<TransactionAccount>,
-    instruction_accounts: Vec<AccountMeta>,
+    instruction_account_metas: Vec<AccountMeta>,
     program_indices: &[usize],
 ) -> MockInvokeContextPreparation {
-    let instruction_accounts = instruction_accounts
+    let mut instruction_accounts: Vec<InstructionAccount> =
-        .iter()
+        Vec::with_capacity(instruction_account_metas.len());
-        .map(|account_meta| {
+    for (index_in_instruction, account_meta) in instruction_account_metas.iter().enumerate() {
-            let index_in_transaction = transaction_accounts
+        let index_in_transaction = transaction_accounts
-                .iter()
+            .iter()
-                .position(|(key, _account)| *key == account_meta.pubkey)
+            .position(|(key, _account)| *key == account_meta.pubkey)
-                .unwrap_or(transaction_accounts.len());
+            .unwrap_or(transaction_accounts.len());
-            InstructionAccount {
+        let index_in_callee = instruction_accounts
-                index_in_transaction,
+            .get(0..index_in_instruction)
-                index_in_caller: program_indices.len().saturating_add(index_in_transaction),
+            .unwrap()
-                is_signer: account_meta.is_signer,
+            .iter()
-                is_writable: account_meta.is_writable,
+            .position(|instruction_account| {
-            }
+                instruction_account.index_in_transaction == index_in_transaction
-        })
+            })
-        .collect();
+            .unwrap_or(index_in_instruction);
+        instruction_accounts.push(InstructionAccount {
+            index_in_transaction,
+            index_in_caller: program_indices.len().saturating_add(index_in_transaction),
+            index_in_callee,
+            is_signer: account_meta.is_signer,
+            is_writable: account_meta.is_writable,
+        });
+    }
     MockInvokeContextPreparation {
         transaction_accounts,
         instruction_accounts,
@@ -1258,11 +1267,13 @@ pub fn visit_each_account_once<E>(
     'root: for (index, instruction_account) in instruction_accounts.iter().enumerate() {
         match instruction_accounts.get(..index) {
             Some(range) => {
-                for before in range.iter() {
+                for (before_index, before) in range.iter().enumerate() {
                     if before.index_in_transaction == instruction_account.index_in_transaction {
+                        debug_assert_eq!(before_index, instruction_account.index_in_callee);
                         continue 'root; // skip dups
                     }
                 }
+                debug_assert_eq!(index, instruction_account.index_in_callee);
                 work(index, instruction_account)?;
             }
             None => return Err(inner_error),
@@ -1320,24 +1331,28 @@ mod tests {
                 InstructionAccount {
                     index_in_transaction: 7,
                     index_in_caller: 0,
+                    index_in_callee: 0,
                     is_signer: false,
                     is_writable: false,
                 },
                 InstructionAccount {
                     index_in_transaction: 3,
                     index_in_caller: 1,
+                    index_in_callee: 1,
                     is_signer: false,
                     is_writable: false,
                 },
                 InstructionAccount {
                     index_in_transaction: 9,
                     index_in_caller: 2,
+                    index_in_callee: 2,
                     is_signer: false,
                     is_writable: false,
                 },
                 InstructionAccount {
                     index_in_transaction: 3,
                     index_in_caller: 1,
+                    index_in_callee: 1,
                     is_signer: false,
                     is_writable: false,
                 },
@@ -1451,6 +1466,7 @@ mod tests {
             instruction_accounts.push(InstructionAccount {
                 index_in_transaction: index,
                 index_in_caller: 1 + index,
+                index_in_callee: instruction_accounts.len(),
                 is_signer: false,
                 is_writable: true,
             });
@@ -1463,6 +1479,7 @@ mod tests {
             instruction_accounts.push(InstructionAccount {
                 index_in_transaction: index,
                 index_in_caller: 1 + index,
+                index_in_callee: index,
                 is_signer: false,
                 is_writable: false,
             });
@@ -1491,12 +1508,14 @@ mod tests {
                 InstructionAccount {
                     index_in_transaction: not_owned_index,
                     index_in_caller: 1 + not_owned_index,
+                    index_in_callee: 0,
                     is_signer: false,
                     is_writable: true,
                 },
                 InstructionAccount {
                     index_in_transaction: owned_index,
                     index_in_caller: 1 + owned_index,
+                    index_in_callee: 1,
                     is_signer: false,
                     is_writable: true,
                 },
@@ -1611,11 +1630,12 @@ mod tests {
             AccountMeta::new_readonly(accounts.get(2).unwrap().0, false),
         ];
         let instruction_accounts = (0..4)
-            .map(|index_in_transaction| InstructionAccount {
+            .map(|index_in_instruction| InstructionAccount {
-                index_in_transaction,
+                index_in_transaction: index_in_instruction,
-                index_in_caller: 1 + index_in_transaction,
+                index_in_caller: 1 + index_in_instruction,
+                index_in_callee: index_in_instruction,
                 is_signer: false,
-                is_writable: index_in_transaction < 2,
+                is_writable: index_in_instruction < 2,
             })
             .collect::<Vec<_>>();
         let mut transaction_context = TransactionContext::new(accounts, 2, 8);
@@ -1826,12 +1846,14 @@ mod tests {
             InstructionAccount {
                 index_in_transaction: 0,
                 index_in_caller: 1,
+                index_in_callee: 0,
                 is_signer: false,
                 is_writable: true,
             },
             InstructionAccount {
                 index_in_transaction: 1,
                 index_in_caller: 2,
+                index_in_callee: 1,
                 is_signer: false,
                 is_writable: false,
             },

programs/bpf_loader/benches/serialization.rs

@@ -95,6 +95,7 @@ fn create_inputs() -> TransactionContext {
             |(index_in_instruction, index_in_transaction)| InstructionAccount {
                 index_in_caller: 1usize.saturating_add(index_in_instruction),
                 index_in_transaction,
+                index_in_callee: index_in_instruction,
                 is_signer: false,
                 is_writable: index_in_instruction >= 4,
             },

programs/bpf_loader/src/serialization.rs

@@ -20,12 +20,16 @@ pub fn is_duplicate(
     index_in_instruction: usize,
 ) -> Option<usize> {
     let index_in_transaction = instruction_context.get_index_in_transaction(index_in_instruction);
-    (instruction_context.get_number_of_program_accounts()..index_in_instruction).position(
+    let old_approach = (instruction_context.get_number_of_program_accounts()..index_in_instruction)
-        |index_in_instruction| {
+        .position(|index_in_instruction| {
             instruction_context.get_index_in_transaction(index_in_instruction)
                 == index_in_transaction
-        },
+        });
-    )
+    let result = instruction_context
+        .is_duplicate(index_in_instruction)
+        .unwrap_or(None);
+    debug_assert_eq!(result, old_approach);
+    old_approach
 }
 
 pub fn serialize_parameters(

programs/vote/benches/process_vote.rs

@@ -81,9 +81,10 @@ fn create_accounts() -> (
         (authority_pubkey, AccountSharedData::default()),
     ];
     let mut instruction_accounts = (0..4)
-        .map(|index| InstructionAccount {
+        .map(|index_in_instruction| InstructionAccount {
-            index_in_transaction: 1usize.saturating_add(index),
+            index_in_transaction: 1usize.saturating_add(index_in_instruction),
-            index_in_caller: 1usize.saturating_add(index),
+            index_in_caller: 1usize.saturating_add(index_in_instruction),
+            index_in_callee: index_in_instruction,
             is_signer: false,
             is_writable: false,
         })

runtime/src/message_processor.rs

@@ -119,19 +119,26 @@ impl MessageProcessor {
                 );
             }
 
-            let instruction_accounts = instruction
+            let mut instruction_accounts = Vec::with_capacity(instruction.accounts.len());
-                .accounts
+            for (index_in_instruction, index_in_transaction) in
-                .iter()
+                instruction.accounts.iter().enumerate()
-                .map(|index_in_transaction| {
+            {
-                    let index_in_transaction = *index_in_transaction as usize;
+                let index_in_callee = instruction
-                    InstructionAccount {
+                    .accounts
-                        index_in_transaction,
+                    .get(0..index_in_instruction)
-                        index_in_caller: program_indices.len().saturating_add(index_in_transaction),
+                    .ok_or(TransactionError::InvalidAccountIndex)?
-                        is_signer: message.is_signer(index_in_transaction),
+                    .iter()
-                        is_writable: message.is_writable(index_in_transaction),
+                    .position(|account_index| account_index == index_in_transaction)
-                    }
+                    .unwrap_or(index_in_instruction);
-                })
+                let index_in_transaction = *index_in_transaction as usize;
-                .collect::<Vec<_>>();
+                instruction_accounts.push(InstructionAccount {
+                    index_in_transaction,
+                    index_in_caller: program_indices.len().saturating_add(index_in_transaction),
+                    index_in_callee,
+                    is_signer: message.is_signer(index_in_transaction),
+                    is_writable: message.is_writable(index_in_transaction),
+                });
+            }
 
             let result = if is_precompile
                 && invoke_context

runtime/src/nonce_keyed_account.rs

@@ -332,12 +332,14 @@ mod test {
                 InstructionAccount {
                     index_in_transaction: 0,
                     index_in_caller: 0,
+                    index_in_callee: 0,
                     is_signer: true,
                     is_writable: true,
                 },
                 InstructionAccount {
                     index_in_transaction: 1,
                     index_in_caller: 1,
+                    index_in_callee: 1,
                     is_signer: false,
                     is_writable: true,
                 },

sdk/src/transaction_context.rs

@@ -16,11 +16,24 @@ use {
 
 pub type TransactionAccount = (Pubkey, AccountSharedData);
 
+/// Contains account meta data which varies between instruction.
+///
+/// It also contains indices to other structures for faster lookup.
 #[derive(Clone, Debug)]
 pub struct InstructionAccount {
+    /// Points to the account and its key in the `TransactionContext`
     pub index_in_transaction: usize,
+    /// Points to the first occurrence in the parent `InstructionContext`
+    ///
+    /// This includes the program accounts.
     pub index_in_caller: usize,
+    /// Points to the first occurrence in the current `InstructionContext`
+    ///
+    /// This excludes the program accounts.
+    pub index_in_callee: usize,
+    /// Is this account supposed to sign
     pub is_signer: bool,
+    /// Is this account allowed to become writable
     pub is_writable: bool,
 }
 
@@ -371,6 +384,28 @@ impl InstructionContext {
         }
     }
 
+    /// Returns `Some(index_in_instruction)` if this is a duplicate
+    /// and `None` if it is the first account with this key
+    pub fn is_duplicate(
+        &self,
+        index_in_instruction: usize,
+    ) -> Result<Option<usize>, InstructionError> {
+        if index_in_instruction < self.program_accounts.len()
+            || index_in_instruction >= self.get_number_of_accounts()
+        {
+            Err(InstructionError::NotEnoughAccountKeys)
+        } else {
+            let index_in_instruction =
+                index_in_instruction.saturating_sub(self.program_accounts.len());
+            let index_in_callee = self.instruction_accounts[index_in_instruction].index_in_callee;
+            Ok(if index_in_callee == index_in_instruction {
+                None
+            } else {
+                Some(index_in_callee)
+            })
+        }
+    }
+
     /// Gets the key of the last program account of this Instruction
     pub fn get_program_key<'a, 'b: 'a>(
         &'a self,