Refactor: CPI Instruction Recording (#22111)

* Unifies all InstructionRecorders of a transaction into one.

* Stops explicitly compiling CPI instructions for recording,
uses the indices gathered from instruction_accounts instead.

program-runtime/src/instruction_recorder.rs

@@ -1,30 +1,32 @@
 use {
-    solana_sdk::{
-        instruction::{CompiledInstruction, Instruction},
-        message::SanitizedMessage,
-    },
+    solana_sdk::instruction::CompiledInstruction,
     std::{cell::RefCell, rc::Rc},
 };
 
 /// Records and compiles cross-program invoked instructions
-#[derive(Clone, Default)]
+#[derive(Clone)]
 pub struct InstructionRecorder {
-    inner: Rc<RefCell<Vec<Instruction>>>,
+    records: Vec<Vec<CompiledInstruction>>,
 }
 
 impl InstructionRecorder {
-    pub fn compile_instructions(
-        &self,
-        message: &SanitizedMessage,
-    ) -> Option<Vec<CompiledInstruction>> {
-        self.inner
-            .borrow()
-            .iter()
-            .map(|ix| message.try_compile_instruction(ix))
-            .collect()
+    pub fn new_ref(instructions_in_message: usize) -> Rc<RefCell<Self>> {
+        Rc::new(RefCell::new(Self {
+            records: Vec::with_capacity(instructions_in_message),
+        }))
     }
 
-    pub fn record_instruction(&self, instruction: Instruction) {
-        self.inner.borrow_mut().push(instruction);
+    pub fn deconstruct(self) -> Vec<Vec<CompiledInstruction>> {
+        self.records
+    }
+
+    pub fn begin_next_recording(&mut self) {
+        self.records.push(Vec::new());
+    }
+
+    pub fn record_compiled_instruction(&mut self, instruction: CompiledInstruction) {
+        if let Some(records) = self.records.last_mut() {
+            records.push(instruction);
+        }
     }
 }

program-runtime/src/invoke_context.rs

@@ -14,7 +14,7 @@ use {
             remove_native_loader, requestable_heap_size, tx_wide_compute_cap, FeatureSet,
         },
         hash::Hash,
-        instruction::{AccountMeta, Instruction, InstructionError},
+        instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError},
         keyed_account::{create_keyed_accounts_unified, keyed_account_at_index, KeyedAccount},
         native_loader,
         pubkey::Pubkey,
@@ -155,7 +155,7 @@ pub struct InvokeContext<'a> {
     current_compute_budget: ComputeBudget,
     compute_meter: Rc<RefCell<ComputeMeter>>,
     executors: Rc<RefCell<Executors>>,
-    pub instruction_recorder: Option<&'a InstructionRecorder>,
+    pub instruction_recorder: Option<Rc<RefCell<InstructionRecorder>>>,
     pub feature_set: Arc<FeatureSet>,
     pub timings: ExecuteDetailsTimings,
     pub blockhash: Hash,
@@ -173,6 +173,7 @@ impl<'a> InvokeContext<'a> {
         log_collector: Option<Rc<RefCell<LogCollector>>>,
         compute_budget: ComputeBudget,
         executors: Rc<RefCell<Executors>>,
+        instruction_recorder: Option<Rc<RefCell<InstructionRecorder>>>,
         feature_set: Arc<FeatureSet>,
         blockhash: Hash,
         lamports_per_signature: u64,
@@ -189,7 +190,7 @@ impl<'a> InvokeContext<'a> {
             compute_budget,
             compute_meter: ComputeMeter::new_ref(compute_budget.max_units),
             executors,
-            instruction_recorder: None,
+            instruction_recorder,
             feature_set,
             timings: ExecuteDetailsTimings::default(),
             blockhash,
@@ -210,6 +211,7 @@ impl<'a> InvokeContext<'a> {
             Some(LogCollector::new_ref()),
             ComputeBudget::default(),
             Rc::new(RefCell::new(Executors::default())),
+            None,
             Arc::new(FeatureSet::all_enabled()),
             Hash::default(),
             0,
@@ -493,9 +495,6 @@ impl<'a> InvokeContext<'a> {
             prev_account_sizes.push((instruction_account.index, account_length));
         }
 
-        if let Some(instruction_recorder) = &self.instruction_recorder {
-            instruction_recorder.record_instruction(instruction.clone());
-        }
         self.process_instruction(
             &instruction.data,
             &instruction_accounts,
@@ -683,9 +682,32 @@ impl<'a> InvokeContext<'a> {
             .unwrap_or_else(native_loader::id);
 
         let is_lowest_invocation_level = self.invoke_stack.is_empty();
-        if !is_lowest_invocation_level {
+        if is_lowest_invocation_level {
+            if let Some(instruction_recorder) = &self.instruction_recorder {
+                instruction_recorder.borrow_mut().begin_next_recording();
+            }
+        } else {
             // Verify the calling program hasn't misbehaved
             self.verify_and_update(instruction_accounts, caller_write_privileges)?;
+
+            // Record instruction
+            if let Some(instruction_recorder) = &self.instruction_recorder {
+                let compiled_instruction = CompiledInstruction {
+                    program_id_index: self
+                        .accounts
+                        .iter()
+                        .position(|(key, _account)| *key == program_id)
+                        .unwrap_or(0) as u8,
+                    data: instruction_data.to_vec(),
+                    accounts: instruction_accounts
+                        .iter()
+                        .map(|instruction_account| instruction_account.index as u8)
+                        .collect(),
+                };
+                instruction_recorder
+                    .borrow_mut()
+                    .record_compiled_instruction(compiled_instruction);
+            }
         }
 
         let result = self

program-test/src/lib.rs

@@ -273,9 +273,6 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
             }
         }
 
-        if let Some(instruction_recorder) = &invoke_context.instruction_recorder {
-            instruction_recorder.record_instruction(instruction.clone());
-        }
         invoke_context
             .process_instruction(
                 &instruction.data,

programs/bpf_loader/src/syscalls.rs

@@ -2382,11 +2382,6 @@ fn call<'a, 'b: 'a>(
         *invoke_context,
     )?;
 
-    // Record the instruction
-    if let Some(instruction_recorder) = &invoke_context.instruction_recorder {
-        instruction_recorder.record_instruction(instruction.clone());
-    }
-
     // Process instruction
     invoke_context
         .process_instruction(

runtime/src/bank.rs

@@ -3576,11 +3576,10 @@ impl Bank {
                         let account_refcells =
                             Self::accounts_to_refcells(&mut loaded_transaction.accounts);
 
-                        let instruction_recorders = if enable_cpi_recording {
-                            let ix_count = tx.message().instructions().len();
-                            let mut recorders = Vec::with_capacity(ix_count);
-                            recorders.resize_with(ix_count, InstructionRecorder::default);
-                            Some(recorders)
+                        let instruction_recorder = if enable_cpi_recording {
+                            Some(InstructionRecorder::new_ref(
+                                tx.message().instructions().len(),
+                            ))
                         } else {
                             None
                         };
@@ -3603,7 +3602,7 @@ impl Bank {
                                 self.rent_collector.rent,
                                 log_collector.clone(),
                                 executors.clone(),
-                                instruction_recorders.as_deref(),
+                                instruction_recorder.clone(),
                                 feature_set,
                                 compute_budget,
                                 &mut timings.details,
@@ -3628,14 +3627,15 @@ impl Bank {
                                     .ok()
                             });
                         transaction_log_messages.push(log_messages);
-                        let inner_instruction_list: Option<InnerInstructionsList> =
-                            instruction_recorders.and_then(|instruction_recorders| {
-                                instruction_recorders
-                                    .into_iter()
-                                    .map(|r| r.compile_instructions(tx.message()))
-                                    .collect()
-                            });
-                        inner_instructions.push(inner_instruction_list);
+                        inner_instructions.push(
+                            instruction_recorder
+                                .and_then(|instruction_recorder| {
+                                    Rc::try_unwrap(instruction_recorder).ok()
+                                })
+                                .map(|instruction_recorder| {
+                                    instruction_recorder.into_inner().deconstruct()
+                                }),
+                        );
 
                         Self::refcells_to_accounts(
                             &mut loaded_transaction.accounts,

runtime/src/message_processor.rs

@@ -58,7 +58,7 @@ impl MessageProcessor {
         rent: Rent,
         log_collector: Option<Rc<RefCell<LogCollector>>>,
         executors: Rc<RefCell<Executors>>,
-        instruction_recorders: Option<&[InstructionRecorder]>,
+        instruction_recorder: Option<Rc<RefCell<InstructionRecorder>>>,
         feature_set: Arc<FeatureSet>,
         compute_budget: ComputeBudget,
         timings: &mut ExecuteDetailsTimings,
@@ -74,6 +74,7 @@ impl MessageProcessor {
             log_collector,
             compute_budget,
             executors,
+            instruction_recorder,
             feature_set,
             blockhash,
             lamports_per_signature,
@@ -109,10 +110,6 @@ impl MessageProcessor {
                 }
             }
 
-            if let Some(instruction_recorders) = instruction_recorders {
-                invoke_context.instruction_recorder =
-                    Some(&instruction_recorders[instruction_index]);
-            }
             let instruction_accounts = instruction
                 .accounts
                 .iter()