Revert "Refactor: Merge MessageProcessor into InvokeContext (#20165)" (#20301)

This reverts commit df6905c3a6.

program-runtime/src/instruction_processor.rs

@@ -603,7 +603,7 @@ impl InstructionProcessor {
             message,
             instruction,
             program_indices,
-            Some(account_indices),
+            account_indices,
         )?;
 
         let mut instruction_processor = InstructionProcessor::default();

runtime/src/bank.rs

@@ -8,7 +8,7 @@
 //! `Bank::process_transactions`
 //!
 //! It does this by loading the accounts using the reference it holds on the account store,
-//! and then passing those to an InvokeContext which handles loading the programs specified
+//! and then passing those to the message_processor which handles loading the programs specified
 //! by the Transaction and executing it.
 //!
 //! The bank then stores the results to the accounts store.
@@ -65,7 +65,7 @@ use log::*;
 use rayon::ThreadPool;
 use solana_measure::measure::Measure;
 use solana_metrics::{datapoint_debug, inc_new_counter_debug, inc_new_counter_info};
-use solana_program_runtime::{ExecuteDetailsTimings, Executors, InstructionProcessor};
+use solana_program_runtime::{ExecuteDetailsTimings, Executors};
 #[allow(deprecated)]
 use solana_sdk::recent_blockhashes_account;
 use solana_sdk::{
@@ -945,8 +945,8 @@ pub struct Bank {
     /// stream for the slot == self.slot
     is_delta: AtomicBool,
 
-    /// The InstructionProcessor
+    /// The Message processor
-    instruction_processor: InstructionProcessor,
+    message_processor: MessageProcessor,
 
     compute_budget: Option<ComputeBudget>,
 
@@ -1095,7 +1095,7 @@ impl Bank {
             stakes: RwLock::<Stakes>::default(),
             epoch_stakes: HashMap::<Epoch, EpochStakes>::default(),
             is_delta: AtomicBool::default(),
-            instruction_processor: InstructionProcessor::default(),
+            message_processor: MessageProcessor::default(),
             compute_budget: Option::<ComputeBudget>::default(),
             feature_builtins: Arc::<Vec<(Builtin, Pubkey, ActivationType)>>::default(),
             last_vote_sync: AtomicU64::default(),
@@ -1327,7 +1327,7 @@ impl Bank {
             is_delta: AtomicBool::new(false),
             tick_height: AtomicU64::new(parent.tick_height.load(Relaxed)),
             signature_count: AtomicU64::new(0),
-            instruction_processor: parent.instruction_processor.clone(),
+            message_processor: parent.message_processor.clone(),
             compute_budget: parent.compute_budget,
             feature_builtins: parent.feature_builtins.clone(),
             hard_forks: parent.hard_forks.clone(),
@@ -1482,7 +1482,7 @@ impl Bank {
             stakes: RwLock::new(fields.stakes),
             epoch_stakes: fields.epoch_stakes,
             is_delta: AtomicBool::new(fields.is_delta),
-            instruction_processor: new(),
+            message_processor: new(),
             compute_budget: None,
             feature_builtins: new(),
             last_vote_sync: new(),
@@ -3389,8 +3389,7 @@ impl Bank {
                         };
 
                         if let Some(legacy_message) = tx.message().legacy_message() {
-                            process_result = MessageProcessor::process_message(
+                            process_result = self.message_processor.process_message(
-                                &self.instruction_processor,
                                 legacy_message,
                                 &loaded_transaction.program_indices,
                                 &account_refcells,
@@ -5326,7 +5325,7 @@ impl Bank {
     ) {
         debug!("Adding program {} under {:?}", name, program_id);
         self.add_native_program(name, &program_id, false);
-        self.instruction_processor
+        self.message_processor
             .add_program(program_id, process_instruction_with_context);
     }
 
@@ -5339,7 +5338,7 @@ impl Bank {
     ) {
         debug!("Replacing program {} under {:?}", name, program_id);
         self.add_native_program(name, &program_id, true);
-        self.instruction_processor
+        self.message_processor
             .add_program(program_id, process_instruction_with_context);
     }
 

runtime/src/message_processor.rs

@@ -47,7 +47,6 @@ impl ComputeMeter for ThisComputeMeter {
     }
 }
 pub struct ThisInvokeContext<'a> {
-    instruction_index: usize,
     invoke_stack: Vec<InvokeContextStackFrame<'a>>,
     rent: Rent,
     pre_accounts: Vec<PreAccount>,
@@ -59,7 +58,7 @@ pub struct ThisInvokeContext<'a> {
     bpf_compute_budget: solana_sdk::process_instruction::BpfComputeBudget,
     compute_meter: Rc<RefCell<dyn ComputeMeter>>,
     executors: Rc<RefCell<Executors>>,
-    instruction_recorders: Option<&'a [InstructionRecorder]>,
+    instruction_recorder: Option<InstructionRecorder>,
     feature_set: Arc<FeatureSet>,
     pub timings: ExecuteDetailsTimings,
     account_db: Arc<Accounts>,
@@ -74,20 +73,25 @@ pub struct ThisInvokeContext<'a> {
 impl<'a> ThisInvokeContext<'a> {
     #[allow(clippy::too_many_arguments)]
     pub fn new(
+        program_id: &Pubkey,
         rent: Rent,
+        message: &'a Message,
+        instruction: &'a CompiledInstruction,
+        program_indices: &[usize],
         accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
         programs: &'a [(Pubkey, ProcessInstructionWithContext)],
         log_collector: Option<Rc<LogCollector>>,
         compute_budget: ComputeBudget,
         compute_meter: Rc<RefCell<dyn ComputeMeter>>,
         executors: Rc<RefCell<Executors>>,
-        instruction_recorders: Option<&'a [InstructionRecorder]>,
+        instruction_recorder: Option<InstructionRecorder>,
         feature_set: Arc<FeatureSet>,
         account_db: Arc<Accounts>,
         ancestors: &'a Ancestors,
         blockhash: &'a Hash,
         fee_calculator: &'a FeeCalculator,
-    ) -> Self {
+    ) -> Result<Self, InstructionError> {
+        let pre_accounts = MessageProcessor::create_pre_accounts(message, instruction, accounts);
         let compute_meter = if feature_set.is_active(&tx_wide_compute_cap::id()) {
             compute_meter
         } else {
@@ -95,11 +99,10 @@ impl<'a> ThisInvokeContext<'a> {
                 remaining: compute_budget.max_units,
             }))
         };
-        Self {
+        let mut invoke_context = Self {
-            instruction_index: 0,
             invoke_stack: Vec::with_capacity(compute_budget.max_invoke_depth),
             rent,
-            pre_accounts: Vec::new(),
+            pre_accounts,
             accounts,
             programs,
             logger: Rc::new(RefCell::new(ThisLogger { log_collector })),
@@ -107,7 +110,7 @@ impl<'a> ThisInvokeContext<'a> {
             bpf_compute_budget: compute_budget.into(),
             compute_meter,
             executors,
-            instruction_recorders,
+            instruction_recorder,
             feature_set,
             timings: ExecuteDetailsTimings::default(),
             account_db,
@@ -116,7 +119,16 @@ impl<'a> ThisInvokeContext<'a> {
             blockhash,
             fee_calculator,
             return_data: None,
-        }
+        };
+        let account_indices = (0..accounts.len()).collect::<Vec<usize>>();
+        invoke_context.push(
+            program_id,
+            message,
+            instruction,
+            program_indices,
+            &account_indices,
+        )?;
+        Ok(invoke_context)
     }
 }
 impl<'a> InvokeContext for ThisInvokeContext<'a> {
@@ -126,27 +138,12 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
         message: &Message,
         instruction: &CompiledInstruction,
         program_indices: &[usize],
-        account_indices: Option<&[usize]>,
+        account_indices: &[usize],
     ) -> Result<(), InstructionError> {
         if self.invoke_stack.len() > self.compute_budget.max_invoke_depth {
             return Err(InstructionError::CallDepth);
         }
 
-        if self.invoke_stack.is_empty() {
-            self.pre_accounts = Vec::with_capacity(instruction.accounts.len());
-            let mut work = |_unique_index: usize, account_index: usize| {
-                if account_index < message.account_keys.len() && account_index < self.accounts.len()
-                {
-                    let account = self.accounts[account_index].1.borrow();
-                    self.pre_accounts
-                        .push(PreAccount::new(&self.accounts[account_index].0, &account));
-                    return Ok(());
-                }
-                Err(InstructionError::MissingAccount)
-            };
-            instruction.visit_each_account(&mut work)?;
-        }
-
         let contains = self.invoke_stack.iter().any(|frame| frame.key == *key);
         let is_last = if let Some(last_frame) = self.invoke_stack.last() {
             last_frame.key == *key
@@ -174,11 +171,7 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
             })
             .chain(instruction.accounts.iter().map(|index_in_instruction| {
                 let index_in_instruction = *index_in_instruction as usize;
-                let account_index = if let Some(account_indices) = account_indices {
+                let account_index = account_indices[index_in_instruction];
-                    account_indices[index_in_instruction]
-                } else {
-                    index_in_instruction
-                };
                 (
                     message.is_signer(index_in_instruction),
                     message.is_writable(index_in_instruction, demote_program_write_locks),
@@ -199,66 +192,6 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
     fn invoke_depth(&self) -> usize {
         self.invoke_stack.len()
     }
-    fn verify(
-        &mut self,
-        message: &Message,
-        instruction: &CompiledInstruction,
-        program_indices: &[usize],
-    ) -> Result<(), InstructionError> {
-        let program_id = instruction.program_id(&message.account_keys);
-        let demote_program_write_locks = self.is_feature_active(&demote_program_write_locks::id());
-        let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id());
-
-        // Verify all executable accounts have zero outstanding refs
-        for account_index in program_indices.iter() {
-            self.accounts[*account_index]
-                .1
-                .try_borrow_mut()
-                .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
-        }
-
-        // Verify the per-account instruction results
-        let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
-        let mut work = |unique_index: usize, account_index: usize| {
-            {
-                // Verify account has no outstanding references
-                let _ = self.accounts[account_index]
-                    .1
-                    .try_borrow_mut()
-                    .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
-            }
-            let account = self.accounts[account_index].1.borrow();
-            self.pre_accounts[unique_index]
-                .verify(
-                    program_id,
-                    message.is_writable(account_index, demote_program_write_locks),
-                    &self.rent,
-                    &account,
-                    &mut self.timings,
-                    true,
-                    do_support_realloc,
-                )
-                .map_err(|err| {
-                    ic_logger_msg!(
-                        self.logger,
-                        "failed to verify account {}: {}",
-                        self.pre_accounts[unique_index].key(),
-                        err
-                    );
-                    err
-                })?;
-            pre_sum += u128::from(self.pre_accounts[unique_index].lamports());
-            post_sum += u128::from(account.lamports());
-            Ok(())
-        };
-        instruction.visit_each_account(&mut work)?;
-
-        // Verify that the total sum of all the lamports did not change
-        if pre_sum != post_sum {
-            return Err(InstructionError::UnbalancedInstruction);
-        }
-        Ok(())
-    }
     fn verify_and_update(
         &mut self,
         instruction: &CompiledInstruction,
@@ -272,7 +205,7 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
             .ok_or(InstructionError::CallDepth)?;
         let program_id = &stack_frame.key;
         let rent = &self.rent;
-        let logger = &self.logger;
+        let logger = self.get_logger();
         let accounts = &self.accounts;
         let pre_accounts = &mut self.pre_accounts;
         let timings = &mut self.timings;
@@ -369,12 +302,9 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
     fn get_executor(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
         self.executors.borrow().get(pubkey)
     }
-    fn set_instruction_index(&mut self, instruction_index: usize) {
-        self.instruction_index = instruction_index;
-    }
     fn record_instruction(&self, instruction: &Instruction) {
-        if let Some(instruction_recorders) = &self.instruction_recorders {
+        if let Some(recorder) = &self.instruction_recorder {
-            instruction_recorders[self.instruction_index].record_instruction(instruction.clone());
+            recorder.record_instruction(instruction.clone());
         }
     }
     fn is_feature_active(&self, feature_id: &Pubkey) -> bool {
@@ -452,7 +382,10 @@ impl Logger for ThisLogger {
 }
 
 #[derive(Debug, Default, Clone, Deserialize, Serialize)]
-pub struct MessageProcessor {}
+pub struct MessageProcessor {
+    #[serde(skip)]
+    instruction_processor: InstructionProcessor,
+}
 
 #[cfg(RUSTC_WITH_SPECIALIZATION)]
 impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor {
@@ -464,15 +397,215 @@ impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor {
 }
 
 impl MessageProcessor {
-    /// Process a message.
+    /// Add a static entrypoint to intercept instructions before the dynamic loader.
-    /// This method calls each instruction in the message over the set of loaded accounts.
+    pub fn add_program(
-    /// For each instruction it calls the program entrypoint method and verifies that the result of
+        &mut self,
+        program_id: Pubkey,
+        process_instruction: ProcessInstructionWithContext,
+    ) {
+        self.instruction_processor
+            .add_program(program_id, process_instruction);
+    }
+
+    /// Record the initial state of the accounts so that they can be compared
+    /// after the instruction is processed
+    pub fn create_pre_accounts(
+        message: &Message,
+        instruction: &CompiledInstruction,
+        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+    ) -> Vec<PreAccount> {
+        let mut pre_accounts = Vec::with_capacity(instruction.accounts.len());
+        {
+            let mut work = |_unique_index: usize, account_index: usize| {
+                if account_index < message.account_keys.len() && account_index < accounts.len() {
+                    let account = accounts[account_index].1.borrow();
+                    pre_accounts.push(PreAccount::new(&accounts[account_index].0, &account));
+                    return Ok(());
+                }
+                Err(InstructionError::MissingAccount)
+            };
+            let _ = instruction.visit_each_account(&mut work);
+        }
+        pre_accounts
+    }
+
+    /// Verify there are no outstanding borrows
+    pub fn verify_account_references(
+        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        program_indices: &[usize],
+    ) -> Result<(), InstructionError> {
+        for account_index in program_indices.iter() {
+            accounts[*account_index]
+                .1
+                .try_borrow_mut()
+                .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
+        }
+        Ok(())
+    }
+
+    /// Verify the results of an instruction
+    #[allow(clippy::too_many_arguments)]
+    pub fn verify(
+        message: &Message,
+        instruction: &CompiledInstruction,
+        pre_accounts: &[PreAccount],
+        program_indices: &[usize],
+        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        rent: &Rent,
+        timings: &mut ExecuteDetailsTimings,
+        logger: Rc<RefCell<dyn Logger>>,
+        demote_program_write_locks: bool,
+        do_support_realloc: bool,
+    ) -> Result<(), InstructionError> {
+        // Verify all executable accounts have zero outstanding refs
+        Self::verify_account_references(accounts, program_indices)?;
+
+        // Verify the per-account instruction results
+        let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
+        {
+            let program_id = instruction.program_id(&message.account_keys);
+            let mut work = |unique_index: usize, account_index: usize| {
+                {
+                    // Verify account has no outstanding references
+                    let _ = accounts[account_index]
+                        .1
+                        .try_borrow_mut()
+                        .map_err(|_| InstructionError::AccountBorrowOutstanding)?;
+                }
+                let account = accounts[account_index].1.borrow();
+                pre_accounts[unique_index]
+                    .verify(
+                        program_id,
+                        message.is_writable(account_index, demote_program_write_locks),
+                        rent,
+                        &account,
+                        timings,
+                        true,
+                        do_support_realloc,
+                    )
+                    .map_err(|err| {
+                        ic_logger_msg!(
+                            logger,
+                            "failed to verify account {}: {}",
+                            pre_accounts[unique_index].key(),
+                            err
+                        );
+                        err
+                    })?;
+                pre_sum += u128::from(pre_accounts[unique_index].lamports());
+                post_sum += u128::from(account.lamports());
+                Ok(())
+            };
+            instruction.visit_each_account(&mut work)?;
+        }
+
+        // Verify that the total sum of all the lamports did not change
+        if pre_sum != post_sum {
+            return Err(InstructionError::UnbalancedInstruction);
+        }
+        Ok(())
+    }
+
+    /// Execute an instruction
+    /// This method calls the instruction's program entrypoint method and verifies that the result of
     /// the call does not violate the bank's accounting rules.
-    /// The accounts are committed back to the bank only if every instruction succeeds.
+    /// The accounts are committed back to the bank only if this function returns Ok(_).
+    #[allow(clippy::too_many_arguments)]
+    fn execute_instruction(
+        &self,
+        message: &Message,
+        instruction: &CompiledInstruction,
+        program_indices: &[usize],
+        accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
+        rent_collector: &RentCollector,
+        log_collector: Option<Rc<LogCollector>>,
+        executors: Rc<RefCell<Executors>>,
+        instruction_recorder: Option<InstructionRecorder>,
+        instruction_index: usize,
+        feature_set: Arc<FeatureSet>,
+        compute_budget: ComputeBudget,
+        compute_meter: Rc<RefCell<dyn ComputeMeter>>,
+        timings: &mut ExecuteDetailsTimings,
+        account_db: Arc<Accounts>,
+        ancestors: &Ancestors,
+        blockhash: &Hash,
+        fee_calculator: &FeeCalculator,
+    ) -> Result<(), InstructionError> {
+        // Fixup the special instructions key if present
+        // before the account pre-values are taken care of
+        for (pubkey, accont) in accounts.iter().take(message.account_keys.len()) {
+            if instructions::check_id(pubkey) {
+                let mut mut_account_ref = accont.borrow_mut();
+                instructions::store_current_index(
+                    mut_account_ref.data_as_mut_slice(),
+                    instruction_index as u16,
+                );
+                break;
+            }
+        }
+
+        let program_id = instruction.program_id(&message.account_keys);
+
+        let mut compute_budget = compute_budget;
+        if feature_set.is_active(&neon_evm_compute_budget::id())
+            && *program_id == crate::neon_evm_program::id()
+        {
+            // Bump the compute budget for neon_evm
+            compute_budget.max_units = compute_budget.max_units.max(500_000);
+            compute_budget.heap_size = Some(256 * 1024);
+        }
+
+        let programs = self.instruction_processor.programs();
+        let mut invoke_context = ThisInvokeContext::new(
+            program_id,
+            rent_collector.rent,
+            message,
+            instruction,
+            program_indices,
+            accounts,
+            programs,
+            log_collector,
+            compute_budget,
+            compute_meter,
+            executors,
+            instruction_recorder,
+            feature_set,
+            account_db,
+            ancestors,
+            blockhash,
+            fee_calculator,
+        )?;
+
+        self.instruction_processor.process_instruction(
+            program_id,
+            &instruction.data,
+            &mut invoke_context,
+        )?;
+        Self::verify(
+            message,
+            instruction,
+            &invoke_context.pre_accounts,
+            program_indices,
+            accounts,
+            &rent_collector.rent,
+            timings,
+            invoke_context.get_logger(),
+            invoke_context.is_feature_active(&demote_program_write_locks::id()),
+            invoke_context.is_feature_active(&do_support_realloc::id()),
+        )?;
+
+        timings.accumulate(&invoke_context.timings);
+
+        Ok(())
+    }
+
+    /// Process a message.
+    /// This method calls each instruction in the message over the set of loaded Accounts
+    /// The accounts are committed back to the bank only if every instruction succeeds
     #[allow(clippy::too_many_arguments)]
     #[allow(clippy::type_complexity)]
     pub fn process_message(
-        instruction_processor: &InstructionProcessor,
+        &self,
         message: &Message,
         program_indices: &[Vec<usize>],
         accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
@@ -489,80 +622,43 @@ impl MessageProcessor {
         blockhash: Hash,
         fee_calculator: FeeCalculator,
     ) -> Result<(), TransactionError> {
-        let mut invoke_context = ThisInvokeContext::new(
+        for (instruction_index, instruction) in message.instructions.iter().enumerate() {
-            rent_collector.rent,
-            accounts,
-            instruction_processor.programs(),
-            log_collector,
-            compute_budget,
-            compute_meter,
-            executors,
-            instruction_recorders,
-            feature_set,
-            account_db,
-            ancestors,
-            &blockhash,
-            &fee_calculator,
-        );
-        let compute_meter = invoke_context.get_compute_meter();
-        for (instruction_index, (instruction, program_indices)) in message
-            .instructions
-            .iter()
-            .zip(program_indices.iter())
-            .enumerate()
-        {
             let mut time = Measure::start("execute_instruction");
             let pre_remaining_units = compute_meter.borrow().get_remaining();
-
+            let instruction_recorder = instruction_recorders
-            // Fixup the special instructions key if present
+                .as_ref()
-            // before the account pre-values are taken care of
+                .map(|recorders| recorders[instruction_index].clone());
-            for (pubkey, account) in accounts.iter().take(message.account_keys.len()) {
+            let err = self
-                if instructions::check_id(pubkey) {
+                .execute_instruction(
-                    let mut mut_account_ref = account.borrow_mut();
+                    message,
-                    instructions::store_current_index(
+                    instruction,
-                        mut_account_ref.data_as_mut_slice(),
+                    &program_indices[instruction_index],
-                        instruction_index as u16,
+                    accounts,
-                    );
+                    rent_collector,
-                    break;
+                    log_collector.clone(),
-                }
+                    executors.clone(),
-            }
+                    instruction_recorder,
-
+                    instruction_index,
-            let program_id = instruction.program_id(&message.account_keys);
+                    feature_set.clone(),
-
+                    compute_budget,
-            let mut compute_budget = compute_budget;
+                    compute_meter.clone(),
-            if invoke_context.is_feature_active(&neon_evm_compute_budget::id())
+                    timings,
-                && *program_id == crate::neon_evm_program::id()
+                    account_db.clone(),
-            {
+                    ancestors,
-                // Bump the compute budget for neon_evm
+                    &blockhash,
-                compute_budget.max_units = compute_budget.max_units.max(500_000);
+                    &fee_calculator,
-                compute_budget.heap_size = Some(256 * 1024);
+                )
-            }
-
-            invoke_context.set_instruction_index(instruction_index);
-            let result = invoke_context
-                .push(program_id, message, instruction, program_indices, None)
-                .and_then(|()| {
-                    instruction_processor.process_instruction(
-                        program_id,
-                        &instruction.data,
-                        &mut invoke_context,
-                    )?;
-                    invoke_context.verify(message, instruction, program_indices)?;
-                    timings.accumulate(&invoke_context.timings);
-                    Ok(())
-                })
                 .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err));
-            invoke_context.pop();
-
             time.stop();
             let post_remaining_units = compute_meter.borrow().get_remaining();
+
             timings.accumulate_program(
                 instruction.program_id(&message.account_keys),
                 time.as_us(),
                 pre_remaining_units - post_remaining_units,
             );
 
-            result?;
+            err?;
         }
         Ok(())
     }
@@ -578,47 +674,6 @@ mod tests {
         process_instruction::MockComputeMeter,
     };
 
-    #[derive(Debug, Serialize, Deserialize)]
-    enum MockInstruction {
-        NoopSuccess,
-        NoopFail,
-        ModifyOwned,
-        ModifyNotOwned,
-        ModifyReadonly,
-    }
-
-    fn mock_process_instruction(
-        program_id: &Pubkey,
-        data: &[u8],
-        invoke_context: &mut dyn InvokeContext,
-    ) -> Result<(), InstructionError> {
-        let keyed_accounts = invoke_context.get_keyed_accounts()?;
-        assert_eq!(*program_id, keyed_accounts[0].owner()?);
-        assert_ne!(
-            keyed_accounts[1].owner()?,
-            *keyed_accounts[0].unsigned_key()
-        );
-
-        if let Ok(instruction) = bincode::deserialize(data) {
-            match instruction {
-                MockInstruction::NoopSuccess => (),
-                MockInstruction::NoopFail => return Err(InstructionError::GenericError),
-                MockInstruction::ModifyOwned => {
-                    keyed_accounts[0].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
-                }
-                MockInstruction::ModifyNotOwned => {
-                    keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
-                }
-                MockInstruction::ModifyReadonly => {
-                    keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
-                }
-            }
-        } else {
-            return Err(InstructionError::InvalidInstructionData);
-        }
-        Ok(())
-    }
-
     #[test]
     fn test_invoke_context() {
         const MAX_DEPTH: usize = 10;
@@ -658,7 +713,11 @@ mod tests {
         let blockhash = Hash::default();
         let fee_calculator = FeeCalculator::default();
         let mut invoke_context = ThisInvokeContext::new(
+            &invoke_stack[0],
             Rent::default(),
+            &message,
+            &message.instructions[0],
+            &[],
             &accounts,
             &[],
             None,
@@ -671,13 +730,20 @@ mod tests {
             &ancestors,
             &blockhash,
             &fee_calculator,
-        );
+        )
+        .unwrap();
 
         // Check call depth increases and has a limit
-        let mut depth_reached = 0;
+        let mut depth_reached = 1;
-        for program_id in invoke_stack.iter() {
+        for program_id in invoke_stack.iter().skip(1) {
             if Err(InstructionError::CallDepth)
-                == invoke_context.push(program_id, &message, &message.instructions[0], &[], None)
+                == invoke_context.push(
+                    program_id,
+                    &message,
+                    &message.instructions[0],
+                    &[],
+                    &account_indices,
+                )
             {
                 break;
             }
@@ -740,53 +806,17 @@ mod tests {
     }
 
     #[test]
-    fn test_invoke_context_verify() {
+    fn test_verify_account_references() {
         let accounts = vec![(
             solana_sdk::pubkey::new_rand(),
             Rc::new(RefCell::new(AccountSharedData::default())),
         )];
-        let message = Message::new(
+
-            &[Instruction::new_with_bincode(
+        assert!(MessageProcessor::verify_account_references(&accounts, &[0]).is_ok());
-                accounts[0].0,
-                &MockInstruction::NoopSuccess,
-                vec![AccountMeta::new_readonly(accounts[0].0, false)],
-            )],
-            None,
-        );
-        let ancestors = Ancestors::default();
-        let blockhash = Hash::default();
-        let fee_calculator = FeeCalculator::default();
-        let mut invoke_context = ThisInvokeContext::new(
-            Rent::default(),
-            &accounts,
-            &[],
-            None,
-            ComputeBudget::default(),
-            Rc::new(RefCell::new(MockComputeMeter::default())),
-            Rc::new(RefCell::new(Executors::default())),
-            None,
-            Arc::new(FeatureSet::all_enabled()),
-            Arc::new(Accounts::default_for_tests()),
-            &ancestors,
-            &blockhash,
-            &fee_calculator,
-        );
-        invoke_context
-            .push(
-                &accounts[0].0,
-                &message,
-                &message.instructions[0],
-                &[0],
-                None,
-            )
-            .unwrap();
-        assert!(invoke_context
-            .verify(&message, &message.instructions[0], &[0])
-            .is_ok());
 
         let mut _borrowed = accounts[0].1.borrow();
         assert_eq!(
-            invoke_context.verify(&message, &message.instructions[0], &[0]),
+            MessageProcessor::verify_account_references(&accounts, &[0]),
             Err(InstructionError::AccountBorrowOutstanding)
         );
     }
@@ -833,8 +863,8 @@ mod tests {
 
         let mock_system_program_id = Pubkey::new(&[2u8; 32]);
         let rent_collector = RentCollector::default();
-        let mut instruction_processor = InstructionProcessor::default();
+        let mut message_processor = MessageProcessor::default();
-        instruction_processor.add_program(mock_system_program_id, mock_system_process_instruction);
+        message_processor.add_program(mock_system_program_id, mock_system_process_instruction);
 
         let program_account = Rc::new(RefCell::new(create_loadable_account_for_test(
             "mock_system_program",
@@ -868,8 +898,7 @@ mod tests {
             Some(&accounts[0].0),
         );
 
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -899,8 +928,7 @@ mod tests {
             Some(&accounts[0].0),
         );
 
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -934,8 +962,7 @@ mod tests {
             Some(&accounts[0].0),
         );
 
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -1024,8 +1051,8 @@ mod tests {
 
         let mock_program_id = Pubkey::new(&[2u8; 32]);
         let rent_collector = RentCollector::default();
-        let mut instruction_processor = InstructionProcessor::default();
+        let mut message_processor = MessageProcessor::default();
-        instruction_processor.add_program(mock_program_id, mock_system_process_instruction);
+        message_processor.add_program(mock_program_id, mock_system_process_instruction);
 
         let program_account = Rc::new(RefCell::new(create_loadable_account_for_test(
             "mock_system_program",
@@ -1061,8 +1088,7 @@ mod tests {
             )],
             Some(&accounts[0].0),
         );
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -1096,8 +1122,7 @@ mod tests {
             )],
             Some(&accounts[0].0),
         );
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -1129,8 +1154,7 @@ mod tests {
             Some(&accounts[0].0),
         );
         let ancestors = Ancestors::default();
-        let result = MessageProcessor::process_message(
+        let result = message_processor.process_message(
-            &instruction_processor,
             &message,
             &program_indices,
             &accounts,
@@ -1155,6 +1179,47 @@ mod tests {
 
     #[test]
     fn test_process_cross_program() {
+        #[derive(Debug, Serialize, Deserialize)]
+        enum MockInstruction {
+            NoopSuccess,
+            NoopFail,
+            ModifyOwned,
+            ModifyNotOwned,
+            ModifyReadonly,
+        }
+
+        fn mock_process_instruction(
+            program_id: &Pubkey,
+            data: &[u8],
+            invoke_context: &mut dyn InvokeContext,
+        ) -> Result<(), InstructionError> {
+            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            assert_eq!(*program_id, keyed_accounts[0].owner()?);
+            assert_ne!(
+                keyed_accounts[1].owner()?,
+                *keyed_accounts[0].unsigned_key()
+            );
+
+            if let Ok(instruction) = bincode::deserialize(data) {
+                match instruction {
+                    MockInstruction::NoopSuccess => (),
+                    MockInstruction::NoopFail => return Err(InstructionError::GenericError),
+                    MockInstruction::ModifyOwned => {
+                        keyed_accounts[0].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                    MockInstruction::ModifyNotOwned => {
+                        keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                    MockInstruction::ModifyReadonly => {
+                        keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                }
+            } else {
+                return Err(InstructionError::InvalidInstructionData);
+            }
+            Ok(())
+        }
+
         let caller_program_id = solana_sdk::pubkey::new_rand();
         let callee_program_id = solana_sdk::pubkey::new_rand();
 
@@ -1164,6 +1229,7 @@ mod tests {
         let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
         program_account.set_executable(true);
 
+        #[allow(unused_mut)]
         let accounts = vec![
             (
                 solana_sdk::pubkey::new_rand(),
@@ -1205,7 +1271,11 @@ mod tests {
         let blockhash = Hash::default();
         let fee_calculator = FeeCalculator::default();
         let mut invoke_context = ThisInvokeContext::new(
+            &caller_program_id,
             Rent::default(),
+            &message,
+            &caller_instruction,
+            &program_indices,
             &accounts,
             programs.as_slice(),
             None,
@@ -1218,16 +1288,8 @@ mod tests {
             &ancestors,
             &blockhash,
             &fee_calculator,
-        );
+        )
-        invoke_context
+        .unwrap();
-            .push(
-                &caller_program_id,
-                &message,
-                &caller_instruction,
-                &program_indices,
-                None,
-            )
-            .unwrap();
 
         // not owned account modified by the caller (before the invoke)
         let caller_write_privileges = message
@@ -1285,7 +1347,11 @@ mod tests {
             let blockhash = Hash::default();
             let fee_calculator = FeeCalculator::default();
             let mut invoke_context = ThisInvokeContext::new(
+                &caller_program_id,
                 Rent::default(),
+                &message,
+                &caller_instruction,
+                &program_indices,
                 &accounts,
                 programs.as_slice(),
                 None,
@@ -1298,16 +1364,8 @@ mod tests {
                 &ancestors,
                 &blockhash,
                 &fee_calculator,
-            );
+            )
-            invoke_context
+            .unwrap();
-                .push(
-                    &caller_program_id,
-                    &message,
-                    &caller_instruction,
-                    &program_indices,
-                    None,
-                )
-                .unwrap();
 
             let caller_write_privileges = message
                 .account_keys
@@ -1330,6 +1388,47 @@ mod tests {
 
     #[test]
     fn test_native_invoke() {
+        #[derive(Debug, Serialize, Deserialize)]
+        enum MockInstruction {
+            NoopSuccess,
+            NoopFail,
+            ModifyOwned,
+            ModifyNotOwned,
+            ModifyReadonly,
+        }
+
+        fn mock_process_instruction(
+            program_id: &Pubkey,
+            data: &[u8],
+            invoke_context: &mut dyn InvokeContext,
+        ) -> Result<(), InstructionError> {
+            let keyed_accounts = invoke_context.get_keyed_accounts()?;
+            assert_eq!(*program_id, keyed_accounts[0].owner()?);
+            assert_ne!(
+                keyed_accounts[1].owner()?,
+                *keyed_accounts[0].unsigned_key()
+            );
+
+            if let Ok(instruction) = bincode::deserialize(data) {
+                match instruction {
+                    MockInstruction::NoopSuccess => (),
+                    MockInstruction::NoopFail => return Err(InstructionError::GenericError),
+                    MockInstruction::ModifyOwned => {
+                        keyed_accounts[0].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                    MockInstruction::ModifyNotOwned => {
+                        keyed_accounts[1].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                    MockInstruction::ModifyReadonly => {
+                        keyed_accounts[2].try_account_ref_mut()?.data_as_mut_slice()[0] = 1
+                    }
+                }
+            } else {
+                return Err(InstructionError::InvalidInstructionData);
+            }
+            Ok(())
+        }
+
         let caller_program_id = solana_sdk::pubkey::new_rand();
         let callee_program_id = solana_sdk::pubkey::new_rand();
 
@@ -1339,6 +1438,7 @@ mod tests {
         let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
         program_account.set_executable(true);
 
+        #[allow(unused_mut)]
         let accounts = vec![
             (
                 solana_sdk::pubkey::new_rand(),
@@ -1376,7 +1476,11 @@ mod tests {
         let blockhash = Hash::default();
         let fee_calculator = FeeCalculator::default();
         let mut invoke_context = ThisInvokeContext::new(
+            &caller_program_id,
             Rent::default(),
+            &message,
+            &caller_instruction,
+            &program_indices,
             &accounts,
             programs.as_slice(),
             None,
@@ -1389,16 +1493,8 @@ mod tests {
             &ancestors,
             &blockhash,
             &fee_calculator,
-        );
+        )
-        invoke_context
+        .unwrap();
-            .push(
-                &caller_program_id,
-                &message,
-                &caller_instruction,
-                &program_indices,
-                None,
-            )
-            .unwrap();
 
         // not owned account modified by the invoker
         accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1;
@@ -1452,7 +1548,11 @@ mod tests {
             let blockhash = Hash::default();
             let fee_calculator = FeeCalculator::default();
             let mut invoke_context = ThisInvokeContext::new(
+                &caller_program_id,
                 Rent::default(),
+                &message,
+                &caller_instruction,
+                &program_indices,
                 &accounts,
                 programs.as_slice(),
                 None,
@@ -1465,16 +1565,8 @@ mod tests {
                 &ancestors,
                 &blockhash,
                 &fee_calculator,
-            );
+            )
-            invoke_context
+            .unwrap();
-                .push(
-                    &caller_program_id,
-                    &message,
-                    &caller_instruction,
-                    &program_indices,
-                    None,
-                )
-                .unwrap();
 
             assert_eq!(
                 InstructionProcessor::native_invoke(

sdk/src/process_instruction.rs

@@ -58,19 +58,12 @@ pub trait InvokeContext {
         message: &Message,
         instruction: &CompiledInstruction,
         program_indices: &[usize],
-        account_indices: Option<&[usize]>,
+        account_indices: &[usize],
     ) -> Result<(), InstructionError>;
     /// Pop a stack frame from the invocation stack
     fn pop(&mut self);
     /// Current depth of the invocation stake
     fn invoke_depth(&self) -> usize;
-    /// Verify the results of an instruction
-    fn verify(
-        &mut self,
-        message: &Message,
-        instruction: &CompiledInstruction,
-        program_indices: &[usize],
-    ) -> Result<(), InstructionError>;
     /// Verify and update PreAccount state based on program execution
     fn verify_and_update(
         &mut self,
@@ -99,8 +92,6 @@ pub trait InvokeContext {
     fn add_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>);
     /// Get the completed loader work that can be re-used across executions
     fn get_executor(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>>;
-    /// Set which instruction in the message is currently being recorded
-    fn set_instruction_index(&mut self, instruction_index: usize);
     /// Record invoked instruction
     fn record_instruction(&self, instruction: &Instruction);
     /// Get the bank's active feature set
@@ -501,7 +492,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
         _message: &Message,
         _instruction: &CompiledInstruction,
         _program_indices: &[usize],
-        _account_indices: Option<&[usize]>,
+        _account_indices: &[usize],
     ) -> Result<(), InstructionError> {
         self.invoke_stack.push(InvokeContextStackFrame::new(
             *_key,
@@ -515,14 +506,6 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
     fn invoke_depth(&self) -> usize {
         self.invoke_stack.len()
     }
-    fn verify(
-        &mut self,
-        _message: &Message,
-        _instruction: &CompiledInstruction,
-        _program_indices: &[usize],
-    ) -> Result<(), InstructionError> {
-        Ok(())
-    }
     fn verify_and_update(
         &mut self,
         _instruction: &CompiledInstruction,
@@ -570,7 +553,6 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
     fn get_executor(&self, _pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
         None
     }
-    fn set_instruction_index(&mut self, _instruction_index: usize) {}
     fn record_instruction(&self, _instruction: &Instruction) {}
     fn is_feature_active(&self, feature_id: &Pubkey) -> bool {
         !self.disabled_features.contains(feature_id)