Refactor: Merge message processor into invoke context (#20308)

* Inlines MessageProcessor::execute_instruction() in MessageProcessor::process_message().

* Moves MessageProcessor::create_pre_accounts() into ThisInvokeContext::push().

* Move instruction_recorders slice into InvokeContext.

* Makes account_indices optional in InvokeContext::push().

* Separates initial InvokeContext::push() from ThisInvokeContext::new().

* invoke_context.pop() the base invocation frame.

* Zip message.instructions.iter() and program_indices.iter().

* Moves ThisInvokeContext::new() to the beginning of the loop inside MessageProcessor::process_message().

* Hoists ThisInvokeContext::new() out of loop inside MessageProcessor::process_message().

* Removes unnecessary clone() from ThisInvokeContext::new() in MessageProcessor ::process_message().

* Stop ignoring errors from MessageProcessor::create_pre_accounts().

* Moves MessageProcessor::verify_account_references() and MessageProcessor::verify() into InvokeContext::verify().
This commit is contained in:
Alexander Meißner
2021-09-29 19:05:25 +02:00
committed by GitHub
parent 53a810dbad
commit 57c8abf499
4 changed files with 354 additions and 435 deletions

View File

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

View File

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

File diff suppressed because it is too large Load Diff

View File

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