From b78f5b6032e167021c1ad70f1a48e8aa021dbc8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 1 Dec 2021 08:54:42 +0100 Subject: [PATCH] Refactor: Cleanup InstructionProcessor (#21404) * Moves create_message(), native_invoke() and process_cross_program_instruction() from the InstructionProcessor to the InvokeContext so that they can have a useful "self" parameter. * Moves InstructionProcessor into InvokeContext and Bank. * Moves ExecuteDetailsTimings into its own file. * Moves Executor into invoke_context.rs * Moves PreAccount into its own file. * impl AbiExample for BuiltinPrograms --- Cargo.lock | 2 - core/src/cost_update_service.rs | 2 +- program-runtime/Cargo.toml | 2 - ...nstruction_processor.rs => pre_account.rs} | 2 +- program-runtime/src/invoke_context.rs | 399 +++++++++++++-- program-runtime/src/lib.rs | 3 +- ...nstruction_processor.rs => pre_account.rs} | 480 +----------------- program-runtime/src/timings.rs | 56 ++ program-test/src/lib.rs | 21 +- programs/bpf/Cargo.lock | 2 - programs/bpf_loader/src/lib.rs | 5 +- programs/bpf_loader/src/syscalls.rs | 23 +- runtime/src/bank.rs | 124 +++-- runtime/src/message_processor.rs | 43 +- runtime/src/serde_snapshot.rs | 1 - runtime/src/serde_snapshot/future.rs | 4 - runtime/src/serde_snapshot/tests.rs | 2 +- 17 files changed, 548 insertions(+), 623 deletions(-) rename program-runtime/benches/{instruction_processor.rs => pre_account.rs} (96%) rename program-runtime/src/{instruction_processor.rs => pre_account.rs} (54%) create mode 100644 program-runtime/src/timings.rs diff --git a/Cargo.lock b/Cargo.lock index f1bed7e81d..861b66c706 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5352,8 +5352,6 @@ dependencies = [ "num-traits", "rustc_version 0.4.0", "serde", - "solana-frozen-abi 1.9.0", - "solana-frozen-abi-macro 1.9.0", "solana-logger 1.9.0", "solana-sdk", "thiserror", diff --git a/core/src/cost_update_service.rs b/core/src/cost_update_service.rs index 44d51d87ef..4f852b3d97 100644 --- a/core/src/cost_update_service.rs +++ b/core/src/cost_update_service.rs @@ -204,7 +204,7 @@ impl CostUpdateService { #[cfg(test)] mod tests { use super::*; - use solana_program_runtime::instruction_processor::ProgramTiming; + use solana_program_runtime::timings::ProgramTiming; use solana_sdk::pubkey::Pubkey; #[test] diff --git a/program-runtime/Cargo.toml b/program-runtime/Cargo.toml index d84f158f38..af5f367039 100644 --- a/program-runtime/Cargo.toml +++ b/program-runtime/Cargo.toml @@ -19,8 +19,6 @@ log = "0.4.14" num-derive = { version = "0.3" } num-traits = { version = "0.2" } serde = { version = "1.0.129", features = ["derive", "rc"] } -solana-frozen-abi = { path = "../frozen-abi", version = "=1.9.0" } -solana-frozen-abi-macro = { path = "../frozen-abi/macro", version = "=1.9.0" } solana-logger = { path = "../logger", version = "=1.9.0" } solana-sdk = { path = "../sdk", version = "=1.9.0" } thiserror = "1.0" diff --git a/program-runtime/benches/instruction_processor.rs b/program-runtime/benches/pre_account.rs similarity index 96% rename from program-runtime/benches/instruction_processor.rs rename to program-runtime/benches/pre_account.rs index b5f8f368bf..3252db6189 100644 --- a/program-runtime/benches/instruction_processor.rs +++ b/program-runtime/benches/pre_account.rs @@ -3,7 +3,7 @@ extern crate test; use log::*; -use solana_program_runtime::instruction_processor::{ExecuteDetailsTimings, PreAccount}; +use solana_program_runtime::{pre_account::PreAccount, timings::ExecuteDetailsTimings}; use solana_sdk::{account::AccountSharedData, pubkey, rent::Rent}; use test::Bencher; diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index fa1542f92c..3edb9f02ab 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -1,11 +1,11 @@ use crate::{ - ic_logger_msg, ic_msg, - instruction_processor::{ExecuteDetailsTimings, Executor, Executors, PreAccount}, - instruction_recorder::InstructionRecorder, - log_collector::LogCollector, + ic_logger_msg, ic_msg, instruction_recorder::InstructionRecorder, log_collector::LogCollector, + native_loader::NativeLoader, pre_account::PreAccount, timings::ExecuteDetailsTimings, }; use solana_sdk::{ account::{AccountSharedData, ReadableAccount}, + account_utils::StateMut, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, compute_budget::ComputeBudget, feature_set::{ demote_program_write_locks, do_support_realloc, neon_evm_compute_budget, @@ -13,13 +13,66 @@ use solana_sdk::{ }, hash::Hash, instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError}, - keyed_account::{create_keyed_accounts_unified, KeyedAccount}, + keyed_account::{create_keyed_accounts_unified, keyed_account_at_index, KeyedAccount}, message::Message, pubkey::Pubkey, rent::Rent, sysvar::Sysvar, }; -use std::{cell::RefCell, rc::Rc, sync::Arc}; +use std::{cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc, sync::Arc}; + +pub type ProcessInstructionWithContext = + fn(usize, &[u8], &mut dyn InvokeContext) -> Result<(), InstructionError>; + +#[derive(Clone)] +pub struct BuiltinProgram { + pub program_id: Pubkey, + pub process_instruction: ProcessInstructionWithContext, +} + +impl std::fmt::Debug for BuiltinProgram { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // These are just type aliases for work around of Debug-ing above pointers + type ErasedProcessInstructionWithContext = fn( + usize, + &'static [u8], + &'static mut dyn InvokeContext, + ) -> Result<(), InstructionError>; + + // rustc doesn't compile due to bug without this work around + // https://github.com/rust-lang/rust/issues/50280 + // https://users.rust-lang.org/t/display-function-pointer/17073/2 + let erased_instruction: ErasedProcessInstructionWithContext = self.process_instruction; + write!(f, "{}: {:p}", self.program_id, erased_instruction) + } +} + +/// Program executor +pub trait Executor: Debug + Send + Sync { + /// Execute the program + fn execute( + &self, + first_instruction_account: usize, + instruction_data: &[u8], + invoke_context: &mut dyn InvokeContext, + use_jit: bool, + ) -> Result<(), InstructionError>; +} + +#[derive(Default)] +pub struct Executors { + pub executors: HashMap>, + pub is_dirty: bool, +} +impl Executors { + pub fn insert(&mut self, key: Pubkey, executor: Arc) { + let _ = self.executors.insert(key, executor); + self.is_dirty = true; + } + pub fn get(&self, key: &Pubkey) -> Option> { + self.executors.get(key).cloned() + } +} /// Compute meter pub struct ComputeMeter { @@ -77,7 +130,7 @@ pub struct ThisInvokeContext<'a> { rent: Rent, pre_accounts: Vec, accounts: &'a [(Pubkey, Rc>)], - programs: &'a [(Pubkey, ProcessInstructionWithContext)], + builtin_programs: &'a [BuiltinProgram], sysvars: &'a [(Pubkey, Vec)], log_collector: Option>>, compute_budget: ComputeBudget, @@ -96,7 +149,7 @@ impl<'a> ThisInvokeContext<'a> { pub fn new( rent: Rent, accounts: &'a [(Pubkey, Rc>)], - programs: &'a [(Pubkey, ProcessInstructionWithContext)], + builtin_programs: &'a [BuiltinProgram], sysvars: &'a [(Pubkey, Vec)], log_collector: Option>>, compute_budget: ComputeBudget, @@ -113,7 +166,7 @@ impl<'a> ThisInvokeContext<'a> { rent, pre_accounts: Vec::new(), accounts, - programs, + builtin_programs, sysvars, log_collector, current_compute_budget: compute_budget, @@ -131,14 +184,14 @@ impl<'a> ThisInvokeContext<'a> { pub fn new_mock_with_sysvars_and_features( accounts: &'a [(Pubkey, Rc>)], - programs: &'a [(Pubkey, ProcessInstructionWithContext)], + builtin_programs: &'a [BuiltinProgram], sysvars: &'a [(Pubkey, Vec)], feature_set: Arc, ) -> Self { Self::new( Rent::default(), accounts, - programs, + builtin_programs, sysvars, None, ComputeBudget::default(), @@ -153,11 +206,11 @@ impl<'a> ThisInvokeContext<'a> { pub fn new_mock( accounts: &'a [(Pubkey, Rc>)], - programs: &'a [(Pubkey, ProcessInstructionWithContext)], + builtin_programs: &'a [BuiltinProgram], ) -> Self { Self::new_mock_with_sysvars_and_features( accounts, - programs, + builtin_programs, &[], Arc::new(FeatureSet::all_enabled()), ) @@ -192,6 +245,28 @@ pub trait InvokeContext { account_indices: &[usize], write_privileges: &[bool], ) -> Result<(), InstructionError>; + /// Entrypoint for a cross-program invocation from a native program + fn native_invoke( + &mut self, + instruction: Instruction, + signers: &[Pubkey], + ) -> Result<(), InstructionError>; + /// Helper to prepare for process_cross_program_instruction() + fn create_message( + &mut self, + instruction: &Instruction, + signers: &[Pubkey], + ) -> Result<(Message, Vec, Vec), InstructionError>; + /// Process a cross-program instruction + fn process_cross_program_instruction( + &mut self, + message: &Message, + program_indices: &[usize], + account_indices: &[usize], + caller_write_privileges: &[bool], + ) -> Result<(), InstructionError>; + /// Calls the instruction's program entrypoint method + fn process_instruction(&mut self, instruction_data: &[u8]) -> Result<(), InstructionError>; /// Get the program ID of the currently executing program fn get_caller(&self) -> Result<&Pubkey, InstructionError>; /// Removes the first keyed account @@ -202,8 +277,6 @@ pub trait InvokeContext { fn remove_first_keyed_account(&mut self) -> Result<(), InstructionError>; /// Get the list of keyed accounts fn get_keyed_accounts(&self) -> Result<&[KeyedAccount], InstructionError>; - /// Get a list of built-in programs - fn get_programs(&self) -> &[(Pubkey, ProcessInstructionWithContext)]; /// Get this invocation's LogCollector fn get_log_collector(&self) -> Option>>; /// Get this invocation's ComputeMeter @@ -497,6 +570,228 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { } Ok(()) } + fn native_invoke( + &mut self, + instruction: Instruction, + signers: &[Pubkey], + ) -> Result<(), InstructionError> { + let (message, caller_write_privileges, program_indices) = + self.create_message(&instruction, signers)?; + let mut account_indices = Vec::with_capacity(message.account_keys.len()); + let mut prev_account_sizes = Vec::with_capacity(message.account_keys.len()); + for account_key in message.account_keys.iter() { + let (account_index, account) = self + .get_account(account_key) + .ok_or(InstructionError::MissingAccount)?; + let account_length = account.borrow().data().len(); + account_indices.push(account_index); + prev_account_sizes.push((account, account_length)); + } + + self.record_instruction(&instruction); + self.process_cross_program_instruction( + &message, + &program_indices, + &account_indices, + &caller_write_privileges, + )?; + + // Verify the called program has not misbehaved + let do_support_realloc = self.is_feature_active(&do_support_realloc::id()); + for (account, prev_size) in prev_account_sizes.iter() { + if !do_support_realloc && *prev_size != account.borrow().data().len() && *prev_size != 0 + { + // Only support for `CreateAccount` at this time. + // Need a way to limit total realloc size across multiple CPI calls + ic_msg!( + self, + "Inner instructions do not support realloc, only SystemProgram::CreateAccount", + ); + return Err(InstructionError::InvalidRealloc); + } + } + + Ok(()) + } + fn create_message( + &mut self, + instruction: &Instruction, + signers: &[Pubkey], + ) -> Result<(Message, Vec, Vec), InstructionError> { + let message = Message::new(&[instruction.clone()], None); + + // Gather keyed_accounts in the order of message.account_keys + let caller_keyed_accounts = self.get_keyed_accounts()?; + let callee_keyed_accounts = message + .account_keys + .iter() + .map(|account_key| { + caller_keyed_accounts + .iter() + .find(|keyed_account| keyed_account.unsigned_key() == account_key) + .ok_or_else(|| { + ic_msg!( + self, + "Instruction references an unknown account {}", + account_key + ); + InstructionError::MissingAccount + }) + }) + .collect::, InstructionError>>()?; + + // Check for privilege escalation + for account in instruction.accounts.iter() { + let keyed_account = callee_keyed_accounts + .iter() + .find_map(|keyed_account| { + if &account.pubkey == keyed_account.unsigned_key() { + Some(keyed_account) + } else { + None + } + }) + .ok_or_else(|| { + ic_msg!( + self, + "Instruction references an unknown account {}", + account.pubkey + ); + InstructionError::MissingAccount + })?; + // Readonly account cannot become writable + if account.is_writable && !keyed_account.is_writable() { + ic_msg!(self, "{}'s writable privilege escalated", account.pubkey); + return Err(InstructionError::PrivilegeEscalation); + } + + if account.is_signer && // If message indicates account is signed + !( // one of the following needs to be true: + keyed_account.signer_key().is_some() // Signed in the parent instruction + || signers.contains(&account.pubkey) // Signed by the program + ) { + ic_msg!(self, "{}'s signer privilege escalated", account.pubkey); + return Err(InstructionError::PrivilegeEscalation); + } + } + let caller_write_privileges = callee_keyed_accounts + .iter() + .map(|keyed_account| keyed_account.is_writable()) + .collect::>(); + + // Find and validate executables / program accounts + let callee_program_id = instruction.program_id; + let (program_account_index, program_account) = callee_keyed_accounts + .iter() + .find(|keyed_account| &callee_program_id == keyed_account.unsigned_key()) + .and_then(|_keyed_account| self.get_account(&callee_program_id)) + .ok_or_else(|| { + ic_msg!(self, "Unknown program {}", callee_program_id); + InstructionError::MissingAccount + })?; + if !program_account.borrow().executable() { + ic_msg!(self, "Account {} is not executable", callee_program_id); + return Err(InstructionError::AccountNotExecutable); + } + let mut program_indices = vec![]; + if program_account.borrow().owner() == &bpf_loader_upgradeable::id() { + if let UpgradeableLoaderState::Program { + programdata_address, + } = program_account.borrow().state()? + { + if let Some((programdata_account_index, _programdata_account)) = + self.get_account(&programdata_address) + { + program_indices.push(programdata_account_index); + } else { + ic_msg!( + self, + "Unknown upgradeable programdata account {}", + programdata_address, + ); + return Err(InstructionError::MissingAccount); + } + } else { + ic_msg!( + self, + "Invalid upgradeable program account {}", + callee_program_id, + ); + return Err(InstructionError::MissingAccount); + } + } + program_indices.push(program_account_index); + + Ok((message, caller_write_privileges, program_indices)) + } + fn process_cross_program_instruction( + &mut self, + message: &Message, + program_indices: &[usize], + account_indices: &[usize], + caller_write_privileges: &[bool], + ) -> Result<(), InstructionError> { + // This function is always called with a valid instruction, if that changes return an error + let instruction = message + .instructions + .get(0) + .ok_or(InstructionError::GenericError)?; + + // Verify the calling program hasn't misbehaved + self.verify_and_update(instruction, account_indices, caller_write_privileges)?; + + self.set_return_data(Vec::new())?; + self.push(message, instruction, program_indices, Some(account_indices))?; + let result = self.process_instruction(&instruction.data).and_then(|_| { + // Verify the called program has not misbehaved + let demote_program_write_locks = + self.is_feature_active(&demote_program_write_locks::id()); + let write_privileges: Vec = (0..message.account_keys.len()) + .map(|i| message.is_writable(i, demote_program_write_locks)) + .collect(); + self.verify_and_update(instruction, account_indices, &write_privileges) + }); + + // Restore previous state + self.pop(); + result + } + fn process_instruction(&mut self, instruction_data: &[u8]) -> Result<(), InstructionError> { + let keyed_accounts = self.get_keyed_accounts()?; + let root_account = keyed_account_at_index(keyed_accounts, 0) + .map_err(|_| InstructionError::UnsupportedProgramId)?; + let root_id = root_account.unsigned_key(); + let owner_id = &root_account.owner()?; + if solana_sdk::native_loader::check_id(owner_id) { + for entry in self.builtin_programs { + if entry.program_id == *root_id { + // Call the builtin program + return (entry.process_instruction)( + 1, // root_id to be skipped + instruction_data, + self, + ); + } + } + if !self.is_feature_active(&remove_native_loader::id()) { + let native_loader = NativeLoader::default(); + // Call the program via the native loader + return native_loader.process_instruction(0, instruction_data, self); + } + } else { + for entry in self.builtin_programs { + if entry.program_id == *owner_id { + // Call the program via a builtin loader + return (entry.process_instruction)( + 0, // no root_id was provided + instruction_data, + self, + ); + } + } + } + Err(InstructionError::UnsupportedProgramId) + } fn get_caller(&self) -> Result<&Pubkey, InstructionError> { self.invoke_stack .last() @@ -520,9 +815,6 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { .map(|frame| &frame.keyed_accounts[frame.keyed_accounts_range.clone()]) .ok_or(InstructionError::CallDepth) } - fn get_programs(&self) -> &[(Pubkey, ProcessInstructionWithContext)] { - self.programs - } fn get_log_collector(&self) -> Option>> { self.log_collector.clone() } @@ -615,9 +907,6 @@ pub fn get_sysvar( }) } -pub type ProcessInstructionWithContext = - fn(usize, &[u8], &mut dyn InvokeContext) -> Result<(), InstructionError>; - pub struct MockInvokeContextPreparation { pub accounts: Vec<(Pubkey, Rc>)>, pub message: Message, @@ -743,12 +1032,10 @@ pub fn mock_process_instruction( #[cfg(test)] mod tests { use super::*; - use crate::instruction_processor::InstructionProcessor; use serde::{Deserialize, Serialize}; use solana_sdk::{ account::{ReadableAccount, WritableAccount}, instruction::{AccountMeta, Instruction, InstructionError}, - keyed_account::keyed_account_at_index, message::Message, native_loader, }; @@ -762,6 +1049,37 @@ mod tests { ModifyReadonly, } + #[test] + fn test_program_entry_debug() { + #[allow(clippy::unnecessary_wraps)] + fn mock_process_instruction( + _first_instruction_account: usize, + _data: &[u8], + _invoke_context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + Ok(()) + } + #[allow(clippy::unnecessary_wraps)] + fn mock_ix_processor( + _first_instruction_account: usize, + _data: &[u8], + _context: &mut dyn InvokeContext, + ) -> Result<(), InstructionError> { + Ok(()) + } + let builtin_programs = &[ + BuiltinProgram { + program_id: solana_sdk::pubkey::new_rand(), + process_instruction: mock_process_instruction, + }, + BuiltinProgram { + program_id: solana_sdk::pubkey::new_rand(), + process_instruction: mock_ix_processor, + }, + ]; + assert!(!format!("{:?}", builtin_programs).is_empty()); + } + #[allow(clippy::integer_arithmetic)] fn mock_process_instruction( first_instruction_account: usize, @@ -973,8 +1291,6 @@ mod tests { let account_indices = [0, 1, 2]; let program_indices = [3, 4]; - let programs: Vec<(_, ProcessInstructionWithContext)> = - vec![(callee_program_id, mock_process_instruction)]; let metas = vec![ AccountMeta::new(accounts[0].0, false), AccountMeta::new(accounts[1].0, false), @@ -990,7 +1306,11 @@ mod tests { ); let message = Message::new(&[callee_instruction], None); - let mut invoke_context = ThisInvokeContext::new_mock(&accounts, programs.as_slice()); + let builtin_programs = &[BuiltinProgram { + program_id: callee_program_id, + process_instruction: mock_process_instruction, + }]; + let mut invoke_context = ThisInvokeContext::new_mock(&accounts, builtin_programs); invoke_context .push(&message, &caller_instruction, &program_indices[..1], None) .unwrap(); @@ -1006,12 +1326,11 @@ mod tests { .collect::>(); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::process_cross_program_instruction( + invoke_context.process_cross_program_instruction( &message, &program_indices[1..], &account_indices, &caller_write_privileges, - &mut invoke_context, ), Err(InstructionError::ExternalAccountDataModified) ); @@ -1020,12 +1339,11 @@ mod tests { // readonly account modified by the invoker accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::process_cross_program_instruction( + invoke_context.process_cross_program_instruction( &message, &program_indices[1..], &account_indices, &caller_write_privileges, - &mut invoke_context, ), Err(InstructionError::ReadonlyDataModified) ); @@ -1059,12 +1377,11 @@ mod tests { .map(|(i, _)| message.is_writable(i, demote_program_write_locks)) .collect::>(); assert_eq!( - InstructionProcessor::process_cross_program_instruction( + invoke_context.process_cross_program_instruction( &message, &program_indices[1..], &account_indices, &caller_write_privileges, - &mut invoke_context, ), case.1 ); @@ -1101,8 +1418,6 @@ mod tests { (callee_program_id, Rc::new(RefCell::new(program_account))), ]; let program_indices = [3]; - let programs: Vec<(_, ProcessInstructionWithContext)> = - vec![(callee_program_id, mock_process_instruction)]; let metas = vec![ AccountMeta::new(accounts[0].0, false), AccountMeta::new(accounts[1].0, false), @@ -1118,7 +1433,11 @@ mod tests { ); let message = Message::new(&[callee_instruction.clone()], None); - let mut invoke_context = ThisInvokeContext::new_mock(&accounts, programs.as_slice()); + let builtin_programs = &[BuiltinProgram { + program_id: callee_program_id, + process_instruction: mock_process_instruction, + }]; + let mut invoke_context = ThisInvokeContext::new_mock(&accounts, builtin_programs); invoke_context .push(&message, &caller_instruction, &program_indices, None) .unwrap(); @@ -1126,11 +1445,7 @@ mod tests { // not owned account modified by the invoker accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::native_invoke( - &mut invoke_context, - callee_instruction.clone(), - &[] - ), + invoke_context.native_invoke(callee_instruction.clone(), &[]), Err(InstructionError::ExternalAccountDataModified) ); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 0; @@ -1138,7 +1453,7 @@ mod tests { // readonly account modified by the invoker accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( - InstructionProcessor::native_invoke(&mut invoke_context, callee_instruction, &[]), + invoke_context.native_invoke(callee_instruction, &[]), Err(InstructionError::ReadonlyDataModified) ); accounts[2].1.borrow_mut().data_as_mut_slice()[0] = 0; @@ -1170,7 +1485,7 @@ mod tests { .push(&message, &caller_instruction, &program_indices, None) .unwrap(); assert_eq!( - InstructionProcessor::native_invoke(&mut invoke_context, callee_instruction, &[]), + invoke_context.native_invoke(callee_instruction, &[]), case.1 ); invoke_context.pop(); diff --git a/program-runtime/src/lib.rs b/program-runtime/src/lib.rs index 46eafcea95..bca6d41321 100644 --- a/program-runtime/src/lib.rs +++ b/program-runtime/src/lib.rs @@ -1,9 +1,10 @@ #![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] -pub mod instruction_processor; pub mod instruction_recorder; pub mod invoke_context; pub mod log_collector; pub mod native_loader; pub mod neon_evm_program; +pub mod pre_account; pub mod stable_log; +pub mod timings; diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/pre_account.rs similarity index 54% rename from program-runtime/src/instruction_processor.rs rename to program-runtime/src/pre_account.rs index 072c0374b5..88139bb984 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/pre_account.rs @@ -1,111 +1,18 @@ -use crate::{ - ic_msg, - invoke_context::{InvokeContext, ProcessInstructionWithContext}, - native_loader::NativeLoader, -}; -use serde::{Deserialize, Serialize}; +use crate::timings::ExecuteDetailsTimings; use solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, - account_utils::StateMut, - bpf_loader_upgradeable::{self, UpgradeableLoaderState}, - feature_set::{demote_program_write_locks, do_support_realloc, remove_native_loader}, - instruction::{Instruction, InstructionError}, - keyed_account::keyed_account_at_index, - message::Message, + instruction::InstructionError, pubkey::Pubkey, rent::Rent, system_instruction::MAX_PERMITTED_DATA_LENGTH, system_program, }; use std::{ - cell::{Ref, RefCell, RefMut}, - collections::HashMap, + cell::{Ref, RefCell}, fmt::Debug, rc::Rc, - sync::Arc, }; -/// Program executor -pub trait Executor: Debug + Send + Sync { - /// Execute the program - fn execute( - &self, - first_instruction_account: usize, - instruction_data: &[u8], - invoke_context: &mut dyn InvokeContext, - use_jit: bool, - ) -> Result<(), InstructionError>; -} - -#[derive(Default)] -pub struct Executors { - pub executors: HashMap>, - pub is_dirty: bool, -} -impl Executors { - pub fn insert(&mut self, key: Pubkey, executor: Arc) { - let _ = self.executors.insert(key, executor); - self.is_dirty = true; - } - pub fn get(&self, key: &Pubkey) -> Option> { - self.executors.get(key).cloned() - } -} - -#[derive(Default, Debug)] -pub struct ProgramTiming { - pub accumulated_us: u64, - pub accumulated_units: u64, - pub count: u32, -} - -#[derive(Default, Debug)] -pub struct ExecuteDetailsTimings { - pub serialize_us: u64, - pub create_vm_us: u64, - pub execute_us: u64, - pub deserialize_us: u64, - pub changed_account_count: u64, - pub total_account_count: u64, - pub total_data_size: usize, - pub data_size_changed: usize, - pub per_program_timings: HashMap, -} -impl ExecuteDetailsTimings { - pub fn accumulate(&mut self, other: &ExecuteDetailsTimings) { - self.serialize_us = self.serialize_us.saturating_add(other.serialize_us); - self.create_vm_us = self.create_vm_us.saturating_add(other.create_vm_us); - self.execute_us = self.execute_us.saturating_add(other.execute_us); - self.deserialize_us = self.deserialize_us.saturating_add(other.deserialize_us); - self.changed_account_count = self - .changed_account_count - .saturating_add(other.changed_account_count); - self.total_account_count = self - .total_account_count - .saturating_add(other.total_account_count); - self.total_data_size = self.total_data_size.saturating_add(other.total_data_size); - self.data_size_changed = self - .data_size_changed - .saturating_add(other.data_size_changed); - for (id, other) in &other.per_program_timings { - let program_timing = self.per_program_timings.entry(*id).or_default(); - program_timing.accumulated_us = program_timing - .accumulated_us - .saturating_add(other.accumulated_us); - program_timing.accumulated_units = program_timing - .accumulated_units - .saturating_add(other.accumulated_units); - program_timing.count = program_timing.count.saturating_add(other.count); - } - } - pub fn accumulate_program(&mut self, program_id: &Pubkey, us: u64, units: u64) { - let program_timing = self.per_program_timings.entry(*program_id).or_default(); - program_timing.accumulated_us = program_timing.accumulated_us.saturating_add(us); - program_timing.accumulated_units = program_timing.accumulated_units.saturating_add(units); - program_timing.count = program_timing.count.saturating_add(1); - } -} - // The relevant state of an account before an Instruction executes, used // to verify account integrity after the Instruction completes #[derive(Clone, Debug, Default)] @@ -283,361 +190,6 @@ impl PreAccount { } } -#[derive(Default, Deserialize, Serialize)] -pub struct InstructionProcessor { - #[serde(skip)] - programs: Vec<(Pubkey, ProcessInstructionWithContext)>, - #[serde(skip)] - native_loader: NativeLoader, -} - -impl std::fmt::Debug for InstructionProcessor { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - #[derive(Debug)] - struct MessageProcessor<'a> { - #[allow(dead_code)] - programs: Vec, - #[allow(dead_code)] - native_loader: &'a NativeLoader, - } - - // These are just type aliases for work around of Debug-ing above pointers - type ErasedProcessInstructionWithContext = fn( - usize, - &'static [u8], - &'static mut dyn InvokeContext, - ) -> Result<(), InstructionError>; - - // rustc doesn't compile due to bug without this work around - // https://github.com/rust-lang/rust/issues/50280 - // https://users.rust-lang.org/t/display-function-pointer/17073/2 - let processor = MessageProcessor { - programs: self - .programs - .iter() - .map(|(pubkey, instruction)| { - let erased_instruction: ErasedProcessInstructionWithContext = *instruction; - format!("{}: {:p}", pubkey, erased_instruction) - }) - .collect::>(), - native_loader: &self.native_loader, - }; - - write!(f, "{:?}", processor) - } -} - -impl Clone for InstructionProcessor { - fn clone(&self) -> Self { - InstructionProcessor { - programs: self.programs.clone(), - native_loader: NativeLoader::default(), - } - } -} - -#[cfg(RUSTC_WITH_SPECIALIZATION)] -impl ::solana_frozen_abi::abi_example::AbiExample for InstructionProcessor { - fn example() -> Self { - // MessageProcessor's fields are #[serde(skip)]-ed and not Serialize - // so, just rely on Default anyway. - InstructionProcessor::default() - } -} - -impl InstructionProcessor { - pub fn programs(&self) -> &[(Pubkey, ProcessInstructionWithContext)] { - &self.programs - } - - /// Add a static entrypoint to intercept instructions before the dynamic loader. - pub fn add_program( - &mut self, - program_id: &Pubkey, - process_instruction: ProcessInstructionWithContext, - ) { - match self.programs.iter_mut().find(|(key, _)| program_id == key) { - Some((_, processor)) => *processor = process_instruction, - None => self.programs.push((*program_id, process_instruction)), - } - } - - /// Remove a program - pub fn remove_program(&mut self, program_id: &Pubkey) { - if let Some(position) = self.programs.iter().position(|(key, _)| program_id == key) { - self.programs.remove(position); - } - } - - /// Process an instruction - /// This method calls the instruction's program entrypoint method - pub fn process_instruction( - &self, - instruction_data: &[u8], - invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - let keyed_accounts = invoke_context.get_keyed_accounts()?; - let root_account = keyed_account_at_index(keyed_accounts, 0) - .map_err(|_| InstructionError::UnsupportedProgramId)?; - let root_id = root_account.unsigned_key(); - let owner_id = &root_account.owner()?; - if solana_sdk::native_loader::check_id(owner_id) { - for (id, process_instruction) in &self.programs { - if id == root_id { - // Call the builtin program - return process_instruction( - 1, // root_id to be skipped - instruction_data, - invoke_context, - ); - } - } - if !invoke_context.is_feature_active(&remove_native_loader::id()) { - // Call the program via the native loader - return self - .native_loader - .process_instruction(0, instruction_data, invoke_context); - } - } else { - for (id, process_instruction) in &self.programs { - if id == owner_id { - // Call the program via a builtin loader - return process_instruction( - 0, // no root_id was provided - instruction_data, - invoke_context, - ); - } - } - } - Err(InstructionError::UnsupportedProgramId) - } - - pub fn create_message( - instruction: &Instruction, - signers: &[Pubkey], - invoke_context: &RefMut<&mut dyn InvokeContext>, - ) -> Result<(Message, Vec, Vec), InstructionError> { - let message = Message::new(&[instruction.clone()], None); - - // Gather keyed_accounts in the order of message.account_keys - let caller_keyed_accounts = invoke_context.get_keyed_accounts()?; - let callee_keyed_accounts = message - .account_keys - .iter() - .map(|account_key| { - caller_keyed_accounts - .iter() - .find(|keyed_account| keyed_account.unsigned_key() == account_key) - .ok_or_else(|| { - ic_msg!( - *invoke_context, - "Instruction references an unknown account {}", - account_key - ); - InstructionError::MissingAccount - }) - }) - .collect::, InstructionError>>()?; - - // Check for privilege escalation - for account in instruction.accounts.iter() { - let keyed_account = callee_keyed_accounts - .iter() - .find_map(|keyed_account| { - if &account.pubkey == keyed_account.unsigned_key() { - Some(keyed_account) - } else { - None - } - }) - .ok_or_else(|| { - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account.pubkey - ); - InstructionError::MissingAccount - })?; - // Readonly account cannot become writable - if account.is_writable && !keyed_account.is_writable() { - ic_msg!( - invoke_context, - "{}'s writable privilege escalated", - account.pubkey - ); - return Err(InstructionError::PrivilegeEscalation); - } - - if account.is_signer && // If message indicates account is signed - !( // one of the following needs to be true: - keyed_account.signer_key().is_some() // Signed in the parent instruction - || signers.contains(&account.pubkey) // Signed by the program - ) { - ic_msg!( - invoke_context, - "{}'s signer privilege escalated", - account.pubkey - ); - return Err(InstructionError::PrivilegeEscalation); - } - } - let caller_write_privileges = callee_keyed_accounts - .iter() - .map(|keyed_account| keyed_account.is_writable()) - .collect::>(); - - // Find and validate executables / program accounts - let callee_program_id = instruction.program_id; - let (program_account_index, program_account) = callee_keyed_accounts - .iter() - .find(|keyed_account| &callee_program_id == keyed_account.unsigned_key()) - .and_then(|_keyed_account| invoke_context.get_account(&callee_program_id)) - .ok_or_else(|| { - ic_msg!(invoke_context, "Unknown program {}", callee_program_id); - InstructionError::MissingAccount - })?; - if !program_account.borrow().executable() { - ic_msg!( - invoke_context, - "Account {} is not executable", - callee_program_id - ); - return Err(InstructionError::AccountNotExecutable); - } - let mut program_indices = vec![]; - if program_account.borrow().owner() == &bpf_loader_upgradeable::id() { - if let UpgradeableLoaderState::Program { - programdata_address, - } = program_account.borrow().state()? - { - if let Some((programdata_account_index, _programdata_account)) = - invoke_context.get_account(&programdata_address) - { - program_indices.push(programdata_account_index); - } else { - ic_msg!( - invoke_context, - "Unknown upgradeable programdata account {}", - programdata_address, - ); - return Err(InstructionError::MissingAccount); - } - } else { - ic_msg!( - invoke_context, - "Invalid upgradeable program account {}", - callee_program_id, - ); - return Err(InstructionError::MissingAccount); - } - } - program_indices.push(program_account_index); - - Ok((message, caller_write_privileges, program_indices)) - } - - /// Entrypoint for a cross-program invocation from a native program - pub fn native_invoke( - invoke_context: &mut dyn InvokeContext, - instruction: Instruction, - signers: &[Pubkey], - ) -> Result<(), InstructionError> { - let do_support_realloc = invoke_context.is_feature_active(&do_support_realloc::id()); - let invoke_context = RefCell::new(invoke_context); - let mut invoke_context = invoke_context.borrow_mut(); - - // Translate and verify caller's data - let (message, caller_write_privileges, program_indices) = - Self::create_message(&instruction, signers, &invoke_context)?; - let mut account_indices = Vec::with_capacity(message.account_keys.len()); - let mut accounts = Vec::with_capacity(message.account_keys.len()); - for account_key in message.account_keys.iter() { - let (account_index, account) = invoke_context - .get_account(account_key) - .ok_or(InstructionError::MissingAccount)?; - let account_length = account.borrow().data().len(); - account_indices.push(account_index); - accounts.push((account, account_length)); - } - - // Record the instruction - invoke_context.record_instruction(&instruction); - - // Process instruction - InstructionProcessor::process_cross_program_instruction( - &message, - &program_indices, - &account_indices, - &caller_write_privileges, - *invoke_context, - )?; - - // Verify the called program has not misbehaved - for (account, prev_size) in accounts.iter() { - if !do_support_realloc && *prev_size != account.borrow().data().len() && *prev_size != 0 - { - // Only support for `CreateAccount` at this time. - // Need a way to limit total realloc size across multiple CPI calls - ic_msg!( - invoke_context, - "Inner instructions do not support realloc, only SystemProgram::CreateAccount", - ); - return Err(InstructionError::InvalidRealloc); - } - } - - Ok(()) - } - - /// Process a cross-program instruction - /// This method calls the instruction's program entrypoint function - pub fn process_cross_program_instruction( - message: &Message, - program_indices: &[usize], - account_indices: &[usize], - caller_write_privileges: &[bool], - invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - // This function is always called with a valid instruction, if that changes return an error - let instruction = message - .instructions - .get(0) - .ok_or(InstructionError::GenericError)?; - - // Verify the calling program hasn't misbehaved - invoke_context.verify_and_update(instruction, account_indices, caller_write_privileges)?; - - // clear the return data - invoke_context.set_return_data(Vec::new())?; - - // Invoke callee - invoke_context.push(message, instruction, program_indices, Some(account_indices))?; - - let mut instruction_processor = InstructionProcessor::default(); - for (program_id, process_instruction) in invoke_context.get_programs().iter() { - instruction_processor.add_program(program_id, *process_instruction); - } - - let mut result = - instruction_processor.process_instruction(&instruction.data, invoke_context); - if result.is_ok() { - // Verify the called program has not misbehaved - let demote_program_write_locks = - invoke_context.is_feature_active(&demote_program_write_locks::id()); - let write_privileges: Vec = (0..message.account_keys.len()) - .map(|i| message.is_writable(i, demote_program_write_locks)) - .collect(); - result = - invoke_context.verify_and_update(instruction, account_indices, &write_privileges); - } - - // Restore previous state - invoke_context.pop(); - result - } -} - #[cfg(test)] mod tests { use super::*; @@ -1074,30 +626,4 @@ mod tests { "program should not be able to change owner and executable at the same time" ); } - - #[test] - fn test_debug() { - let mut instruction_processor = InstructionProcessor::default(); - #[allow(clippy::unnecessary_wraps)] - fn mock_process_instruction( - _first_instruction_account: usize, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - Ok(()) - } - #[allow(clippy::unnecessary_wraps)] - fn mock_ix_processor( - _first_instruction_account: usize, - _data: &[u8], - _context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - Ok(()) - } - let program_id = solana_sdk::pubkey::new_rand(); - instruction_processor.add_program(&program_id, mock_process_instruction); - instruction_processor.add_program(&program_id, mock_ix_processor); - - assert!(!format!("{:?}", instruction_processor).is_empty()); - } } diff --git a/program-runtime/src/timings.rs b/program-runtime/src/timings.rs new file mode 100644 index 0000000000..37653616c3 --- /dev/null +++ b/program-runtime/src/timings.rs @@ -0,0 +1,56 @@ +use solana_sdk::pubkey::Pubkey; +use std::collections::HashMap; + +#[derive(Default, Debug)] +pub struct ProgramTiming { + pub accumulated_us: u64, + pub accumulated_units: u64, + pub count: u32, +} + +#[derive(Default, Debug)] +pub struct ExecuteDetailsTimings { + pub serialize_us: u64, + pub create_vm_us: u64, + pub execute_us: u64, + pub deserialize_us: u64, + pub changed_account_count: u64, + pub total_account_count: u64, + pub total_data_size: usize, + pub data_size_changed: usize, + pub per_program_timings: HashMap, +} +impl ExecuteDetailsTimings { + pub fn accumulate(&mut self, other: &ExecuteDetailsTimings) { + self.serialize_us = self.serialize_us.saturating_add(other.serialize_us); + self.create_vm_us = self.create_vm_us.saturating_add(other.create_vm_us); + self.execute_us = self.execute_us.saturating_add(other.execute_us); + self.deserialize_us = self.deserialize_us.saturating_add(other.deserialize_us); + self.changed_account_count = self + .changed_account_count + .saturating_add(other.changed_account_count); + self.total_account_count = self + .total_account_count + .saturating_add(other.total_account_count); + self.total_data_size = self.total_data_size.saturating_add(other.total_data_size); + self.data_size_changed = self + .data_size_changed + .saturating_add(other.data_size_changed); + for (id, other) in &other.per_program_timings { + let program_timing = self.per_program_timings.entry(*id).or_default(); + program_timing.accumulated_us = program_timing + .accumulated_us + .saturating_add(other.accumulated_us); + program_timing.accumulated_units = program_timing + .accumulated_units + .saturating_add(other.accumulated_units); + program_timing.count = program_timing.count.saturating_add(other.count); + } + } + pub fn accumulate_program(&mut self, program_id: &Pubkey, us: u64, units: u64) { + let program_timing = self.per_program_timings.entry(*program_id).or_default(); + program_timing.accumulated_us = program_timing.accumulated_us.saturating_add(us); + program_timing.accumulated_units = program_timing.accumulated_units.saturating_add(units); + program_timing.count = program_timing.count.saturating_add(1); + } +} diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 699a91c348..a8ca8d8df8 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -9,10 +9,7 @@ use { log::*, solana_banks_client::start_client, solana_banks_server::banks_server::start_local_server, - solana_program_runtime::{ - ic_msg, instruction_processor::InstructionProcessor, - invoke_context::ProcessInstructionWithContext, stable_log, - }, + solana_program_runtime::{ic_msg, invoke_context::ProcessInstructionWithContext, stable_log}, solana_runtime::{ bank::{Bank, ExecuteTimings}, bank_forks::BankForks, @@ -323,14 +320,14 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { invoke_context.record_instruction(instruction); - InstructionProcessor::process_cross_program_instruction( - &message, - &program_indices, - &account_indices, - &caller_privileges, - invoke_context, - ) - .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; + invoke_context + .process_cross_program_instruction( + &message, + &program_indices, + &account_indices, + &caller_privileges, + ) + .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; // Copy writeable account modifications back into the caller's AccountInfos for (account, account_info) in accounts.iter() { diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 95c5955696..494c79ef2d 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3260,8 +3260,6 @@ dependencies = [ "num-traits", "rustc_version 0.4.0", "serde", - "solana-frozen-abi 1.9.0", - "solana-frozen-abi-macro 1.9.0", "solana-logger 1.9.0", "solana-sdk", "thiserror", diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 00a279f689..509160cf44 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -16,8 +16,7 @@ use log::{log_enabled, trace, Level::Trace}; use solana_measure::measure::Measure; use solana_program_runtime::{ ic_logger_msg, ic_msg, - instruction_processor::{Executor, InstructionProcessor}, - invoke_context::{ComputeMeter, InvokeContext}, + invoke_context::{ComputeMeter, Executor, InvokeContext}, log_collector::LogCollector, stable_log, }; @@ -486,7 +485,7 @@ fn process_loader_upgradeable_instruction( .iter() .map(|seeds| Pubkey::create_program_address(*seeds, caller_program_id)) .collect::, solana_sdk::pubkey::PubkeyError>>()?; - InstructionProcessor::native_invoke(invoke_context, instruction, signers.as_slice())?; + invoke_context.native_invoke(instruction, signers.as_slice())?; // Load and verify the program bits let executor = create_executor( diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 33c63f04f1..81c6b7d4c6 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2,7 +2,6 @@ use crate::{alloc, BpfError}; use alloc::Alloc; use solana_program_runtime::{ ic_logger_msg, ic_msg, - instruction_processor::InstructionProcessor, invoke_context::{ComputeMeter, InvokeContext}, log_collector::LogCollector, stable_log, @@ -2157,9 +2156,9 @@ fn call<'a>( signers_seeds_len, memory_mapping, )?; - let (message, caller_write_privileges, program_indices) = - InstructionProcessor::create_message(&instruction, &signers, &invoke_context) - .map_err(SyscallError::InstructionError)?; + let (message, caller_write_privileges, program_indices) = invoke_context + .create_message(&instruction, &signers) + .map_err(SyscallError::InstructionError)?; check_authorized_program(&instruction.program_id, &instruction.data, *invoke_context)?; let (account_indices, mut accounts) = syscall.translate_accounts( &message, @@ -2173,14 +2172,14 @@ fn call<'a>( invoke_context.record_instruction(&instruction); // Process instruction - InstructionProcessor::process_cross_program_instruction( - &message, - &program_indices, - &account_indices, - &caller_write_privileges, - *invoke_context, - ) - .map_err(SyscallError::InstructionError)?; + invoke_context + .process_cross_program_instruction( + &message, + &program_indices, + &account_indices, + &caller_write_privileges, + ) + .map_err(SyscallError::InstructionError)?; // Copy results back to caller for (callee_account, caller_account) in accounts.iter_mut() { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 8641321600..edd7b25db6 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -70,10 +70,12 @@ use rayon::{ use solana_measure::measure::Measure; use solana_metrics::{inc_new_counter_debug, inc_new_counter_info}; use solana_program_runtime::{ - instruction_processor::{ExecuteDetailsTimings, Executor, Executors, InstructionProcessor}, instruction_recorder::InstructionRecorder, - invoke_context::{ComputeMeter, ProcessInstructionWithContext}, + invoke_context::{ + BuiltinProgram, ComputeMeter, Executor, Executors, ProcessInstructionWithContext, + }, log_collector::LogCollector, + timings::ExecuteDetailsTimings, }; #[allow(deprecated)] use solana_sdk::recent_blockhashes_account; @@ -872,6 +874,18 @@ impl AbiExample for OptionalDropCallback { } } +#[derive(Debug, Clone, Default)] +pub struct BuiltinPrograms { + pub vec: Vec, +} + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl AbiExample for BuiltinPrograms { + fn example() -> Self { + Self::default() + } +} + /// Manager for the state of all accounts and programs after processing its entries. /// AbiExample is needed even without Serialize/Deserialize; actual (de-)serialization /// are implemented elsewhere for versioning @@ -989,8 +1003,8 @@ pub struct Bank { /// stream for the slot == self.slot is_delta: AtomicBool, - /// The InstructionProcessor - instruction_processor: InstructionProcessor, + /// The builtin programs + builtin_programs: BuiltinPrograms, compute_budget: Option, @@ -1149,7 +1163,7 @@ impl Bank { stakes: RwLock::::default(), epoch_stakes: HashMap::::default(), is_delta: AtomicBool::default(), - instruction_processor: InstructionProcessor::default(), + builtin_programs: BuiltinPrograms::default(), compute_budget: Option::::default(), feature_builtins: Arc::>::default(), rewards: RwLock::>::default(), @@ -1389,7 +1403,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(), + builtin_programs: parent.builtin_programs.clone(), compute_budget: parent.compute_budget, feature_builtins: parent.feature_builtins.clone(), hard_forks: parent.hard_forks.clone(), @@ -1598,7 +1612,7 @@ impl Bank { stakes: RwLock::new(fields.stakes), epoch_stakes: fields.epoch_stakes, is_delta: AtomicBool::new(fields.is_delta), - instruction_processor: new(), + builtin_programs: new(), compute_budget: None, feature_builtins: new(), rewards: new(), @@ -3040,7 +3054,7 @@ impl Bank { &genesis_config.rent, ); - // Add additional native programs specified in the genesis config + // Add additional builtin programs specified in the genesis config for (name, program_id) in &genesis_config.native_instruction_processors { self.add_builtin_account(name, program_id, false); } @@ -3073,10 +3087,10 @@ impl Bank { }); if must_replace { - // updating native program + // updating builtin program match &existing_genuine_program { None => panic!( - "There is no account to replace with native program ({}, {}).", + "There is no account to replace with builtin program ({}, {}).", name, program_id ), Some(account) => { @@ -3087,7 +3101,7 @@ impl Bank { } } } else { - // introducing native program + // introducing builtin program if existing_genuine_program.is_some() { // The existing account is sufficient return; @@ -3096,13 +3110,13 @@ impl Bank { assert!( !self.freeze_started(), - "Can't change frozen bank by adding not-existing new native program ({}, {}). \ + "Can't change frozen bank by adding not-existing new builtin program ({}, {}). \ Maybe, inconsistent program activation is detected on snapshot restore?", name, program_id ); - // Add a bogus executable native account, which will be loaded and ignored. + // Add a bogus executable builtin account, which will be loaded and ignored. let account = native_loader::create_loadable_account_with_fields( name, self.inherit_specially_retained_account_fields(&existing_genuine_program), @@ -3867,7 +3881,7 @@ impl Bank { if let Some(legacy_message) = tx.message().legacy_message() { process_result = MessageProcessor::process_message( - &self.instruction_processor, + &self.builtin_programs.vec, legacy_message, &loaded_transaction.program_indices, &account_refcells, @@ -5895,12 +5909,23 @@ impl Bank { &mut self, name: &str, program_id: &Pubkey, - process_instruction_with_context: ProcessInstructionWithContext, + process_instruction: ProcessInstructionWithContext, ) { debug!("Adding program {} under {:?}", name, program_id); self.add_builtin_account(name, program_id, false); - self.instruction_processor - .add_program(program_id, process_instruction_with_context); + if let Some(entry) = self + .builtin_programs + .vec + .iter_mut() + .find(|entry| entry.program_id == *program_id) + { + entry.process_instruction = process_instruction; + } else { + self.builtin_programs.vec.push(BuiltinProgram { + program_id: *program_id, + process_instruction, + }); + } debug!("Added program {} under {:?}", name, program_id); } @@ -5909,12 +5934,18 @@ impl Bank { &mut self, name: &str, program_id: &Pubkey, - process_instruction_with_context: ProcessInstructionWithContext, + process_instruction: ProcessInstructionWithContext, ) { debug!("Replacing program {} under {:?}", name, program_id); self.add_builtin_account(name, program_id, true); - self.instruction_processor - .add_program(program_id, process_instruction_with_context); + if let Some(entry) = self + .builtin_programs + .vec + .iter_mut() + .find(|entry| entry.program_id == *program_id) + { + entry.process_instruction = process_instruction; + } debug!("Replaced program {} under {:?}", name, program_id); } @@ -5923,7 +5954,14 @@ impl Bank { debug!("Removing program {} under {:?}", name, program_id); // Don't remove the account since the bank expects the account state to // be idempotent - self.instruction_processor.remove_program(program_id); + if let Some(position) = self + .builtin_programs + .vec + .iter() + .position(|entry| entry.program_id == *program_id) + { + self.builtin_programs.vec.remove(position); + } debug!("Removed program {} under {:?}", name, program_id); } @@ -6787,16 +6825,16 @@ pub(crate) mod tests { cluster_type: ClusterType::MainnetBeta, ..GenesisConfig::default() })); - let sysvar_and_native_program_delta0 = 11; + let sysvar_and_builtin_program_delta0 = 11; assert_eq!( bank0.capitalization(), - 42 * 42 + sysvar_and_native_program_delta0 + 42 * 42 + sysvar_and_builtin_program_delta0 ); let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), 1); - let sysvar_and_native_program_delta1 = 2; + let sysvar_and_builtin_program_delta1 = 2; assert_eq!( bank1.capitalization(), - 42 * 42 + sysvar_and_native_program_delta0 + sysvar_and_native_program_delta1, + 42 * 42 + sysvar_and_builtin_program_delta0 + sysvar_and_builtin_program_delta1, ); } @@ -6866,7 +6904,7 @@ pub(crate) mod tests { bank_with_success_txs.store_account(&keypair5.pubkey(), &account5); bank_with_success_txs.store_account(&keypair6.pubkey(), &account6); - // Make native instruction loader rent exempt + // Make builtin instruction loader rent exempt let system_program_id = system_program::id(); let mut system_program_account = bank.get_account(&system_program_id).unwrap(); system_program_account.set_lamports( @@ -7393,10 +7431,10 @@ pub(crate) mod tests { let current_capitalization = bank.capitalization.load(Relaxed); // only slot history is newly created - let sysvar_and_native_program_delta = + let sysvar_and_builtin_program_delta = min_rent_excempt_balance_for_sysvars(&bank, &[sysvar::slot_history::id()]); assert_eq!( - previous_capitalization - (current_capitalization - sysvar_and_native_program_delta), + previous_capitalization - (current_capitalization - sysvar_and_builtin_program_delta), burned_portion ); @@ -8531,10 +8569,10 @@ pub(crate) mod tests { // not being eagerly-collected for exact rewards calculation bank0.restore_old_behavior_for_fragile_tests(); - let sysvar_and_native_program_delta0 = 11; + let sysvar_and_builtin_program_delta0 = 11; assert_eq!( bank0.capitalization(), - 42 * 1_000_000_000 + sysvar_and_native_program_delta0 + 42 * 1_000_000_000 + sysvar_and_builtin_program_delta0 ); assert!(bank0.rewards.read().unwrap().is_empty()); @@ -8596,9 +8634,9 @@ pub(crate) mod tests { assert_ne!(bank1.capitalization(), bank0.capitalization()); // verify the inflation is represented in validator_points * - let sysvar_and_native_program_delta1 = 2; + let sysvar_and_builtin_program_delta1 = 2; let paid_rewards = - bank1.capitalization() - bank0.capitalization() - sysvar_and_native_program_delta1; + bank1.capitalization() - bank0.capitalization() - sysvar_and_builtin_program_delta1; let rewards = bank1 .get_account(&sysvar::rewards::id()) @@ -8665,10 +8703,10 @@ pub(crate) mod tests { // not being eagerly-collected for exact rewards calculation bank.restore_old_behavior_for_fragile_tests(); - let sysvar_and_native_program_delta = 11; + let sysvar_and_builtin_program_delta = 11; assert_eq!( bank.capitalization(), - 42 * 1_000_000_000 + sysvar_and_native_program_delta + 42 * 1_000_000_000 + sysvar_and_builtin_program_delta ); assert!(bank.rewards.read().unwrap().is_empty()); @@ -9116,9 +9154,9 @@ pub(crate) mod tests { ); // Leader collects fee after the bank is frozen // verify capitalization - let sysvar_and_native_program_delta = 1; + let sysvar_and_builtin_program_delta = 1; assert_eq!( - capitalization - expected_fee_burned + sysvar_and_native_program_delta, + capitalization - expected_fee_burned + sysvar_and_builtin_program_delta, bank.capitalization() ); @@ -10782,7 +10820,7 @@ pub(crate) mod tests { Err(InstructionError::Custom(42)) } - // Non-native loader accounts can not be used for instruction processing + // Non-builtin loader accounts can not be used for instruction processing { let stakes = bank.stakes.read().unwrap(); assert!(stakes.vote_accounts().as_ref().is_empty()); @@ -12544,7 +12582,7 @@ pub(crate) mod tests { assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); let bank = Arc::new(new_from_parent(&bank)); - // When replacing native_program, name must change to disambiguate from repeated + // When replacing builtin_program, name must change to disambiguate from repeated // invocations. assert_capitalization_diff( &bank, @@ -12608,7 +12646,7 @@ pub(crate) mod tests { #[test] #[should_panic( - expected = "Can't change frozen bank by adding not-existing new native \ + expected = "Can't change frozen bank by adding not-existing new builtin \ program (mock_program, CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ Maybe, inconsistent program activation is detected on snapshot restore?" )] @@ -12631,7 +12669,7 @@ pub(crate) mod tests { #[test] #[should_panic( - expected = "There is no account to replace with native program (mock_program, \ + expected = "There is no account to replace with builtin program (mock_program, \ CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre)." )] fn test_add_builtin_account_replace_none() { @@ -12836,9 +12874,9 @@ pub(crate) mod tests { // assert that everything gets in order.... assert!(bank1.get_account(&reward_pubkey).is_none()); - let sysvar_and_native_program_delta = 1; + let sysvar_and_builtin_program_delta = 1; assert_eq!( - bank0.capitalization() + 1 + 1_000_000_000 + sysvar_and_native_program_delta, + bank0.capitalization() + 1 + 1_000_000_000 + sysvar_and_builtin_program_delta, bank1.capitalization() ); assert_eq!(bank1.capitalization(), bank1.calculate_capitalization(true)); @@ -13377,7 +13415,7 @@ pub(crate) mod tests { .accounts .remove(&feature_set::deprecate_rewards_sysvar::id()); - // intentionally create bogus native programs + // intentionally create bogus builtin programs #[allow(clippy::unnecessary_wraps)] fn mock_process_instruction( _first_instruction_account: usize, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 02a8425c7d..23d425ac2b 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1,10 +1,10 @@ use serde::{Deserialize, Serialize}; use solana_measure::measure::Measure; use solana_program_runtime::{ - instruction_processor::{ExecuteDetailsTimings, Executors, InstructionProcessor}, instruction_recorder::InstructionRecorder, - invoke_context::{ComputeMeter, InvokeContext, ThisInvokeContext}, + invoke_context::{BuiltinProgram, ComputeMeter, Executors, InvokeContext, ThisInvokeContext}, log_collector::LogCollector, + timings::ExecuteDetailsTimings, }; use solana_sdk::{ account::{AccountSharedData, WritableAccount}, @@ -40,7 +40,7 @@ impl MessageProcessor { /// The accounts are committed back to the bank only if every instruction succeeds. #[allow(clippy::too_many_arguments)] pub fn process_message( - instruction_processor: &InstructionProcessor, + builtin_programs: &[BuiltinProgram], message: &Message, program_indices: &[Vec], accounts: &[(Pubkey, Rc>)], @@ -59,7 +59,7 @@ impl MessageProcessor { let mut invoke_context = ThisInvokeContext::new( rent, accounts, - instruction_processor.programs(), + builtin_programs, sysvars, log_collector, compute_budget, @@ -107,8 +107,7 @@ impl MessageProcessor { let result = invoke_context .push(message, instruction, program_indices, None) .and_then(|_| { - instruction_processor - .process_instruction(&instruction.data, &mut invoke_context)?; + invoke_context.process_instruction(&instruction.data)?; invoke_context.verify(message, instruction, program_indices)?; timings.accumulate(&invoke_context.timings); Ok(()) @@ -199,8 +198,10 @@ mod tests { let mock_system_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); - let mut instruction_processor = InstructionProcessor::default(); - instruction_processor.add_program(&mock_system_program_id, mock_system_process_instruction); + let builtin_programs = &[BuiltinProgram { + program_id: mock_system_program_id, + process_instruction: mock_system_process_instruction, + }]; let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -234,7 +235,7 @@ mod tests { ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -264,7 +265,7 @@ mod tests { ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -298,7 +299,7 @@ mod tests { ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -404,8 +405,10 @@ mod tests { let mock_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); - let mut instruction_processor = InstructionProcessor::default(); - instruction_processor.add_program(&mock_program_id, mock_system_process_instruction); + let builtin_programs = &[BuiltinProgram { + program_id: mock_program_id, + process_instruction: mock_system_process_instruction, + }]; let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -441,7 +444,7 @@ mod tests { Some(&accounts[0].0), ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -475,7 +478,7 @@ mod tests { Some(&accounts[0].0), ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -506,7 +509,7 @@ mod tests { Some(&accounts[0].0), ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &program_indices, &accounts, @@ -538,8 +541,10 @@ mod tests { ) -> Result<(), InstructionError> { Err(InstructionError::Custom(0xbabb1e)) } - let mut instruction_processor = InstructionProcessor::default(); - instruction_processor.add_program(&mock_program_id, mock_process_instruction); + let builtin_programs = &[BuiltinProgram { + program_id: mock_program_id, + process_instruction: mock_process_instruction, + }]; let secp256k1_account = AccountSharedData::new_ref(1, 0, &native_loader::id()); secp256k1_account.borrow_mut().set_executable(true); @@ -562,7 +567,7 @@ mod tests { ); let result = MessageProcessor::process_message( - &instruction_processor, + builtin_programs, &message, &[vec![0], vec![1]], &accounts, diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index c343aca10b..6a17e5ed40 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -23,7 +23,6 @@ use { rayon::prelude::*, serde::{de::DeserializeOwned, Deserialize, Serialize}, solana_measure::measure::Measure, - solana_program_runtime::instruction_processor::InstructionProcessor, solana_sdk::{ clock::{Epoch, Slot, UnixTimestamp}, epoch_schedule::EpochSchedule, diff --git a/runtime/src/serde_snapshot/future.rs b/runtime/src/serde_snapshot/future.rs index 0a951f0628..770384d4b8 100644 --- a/runtime/src/serde_snapshot/future.rs +++ b/runtime/src/serde_snapshot/future.rs @@ -79,8 +79,6 @@ pub(crate) struct DeserializableVersionedBank { pub(crate) unused_accounts: UnusedAccounts, pub(crate) epoch_stakes: HashMap, pub(crate) is_delta: bool, - #[allow(dead_code)] - pub(crate) message_processor: InstructionProcessor, } impl From for BankFieldsToDeserialize { @@ -157,7 +155,6 @@ pub(crate) struct SerializableVersionedBank<'a> { pub(crate) unused_accounts: UnusedAccounts, pub(crate) epoch_stakes: &'a HashMap, pub(crate) is_delta: bool, - pub(crate) message_processor: InstructionProcessor, } impl<'a> From> for SerializableVersionedBank<'a> { @@ -198,7 +195,6 @@ impl<'a> From> for SerializableVersionedB unused_accounts: new(), epoch_stakes: rhs.epoch_stakes, is_delta: rhs.is_delta, - message_processor: new(), } } } diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index c992583486..de76789087 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -312,7 +312,7 @@ mod test_bank_serialize { // This some what long test harness is required to freeze the ABI of // Bank's serialization due to versioned nature - #[frozen_abi(digest = "A9KFf8kLJczP3AMbFXRrqzmruoqMjooTPzdvEwZZ4EP7")] + #[frozen_abi(digest = "5vYNtKztrNKfb6TtRktXdLCbU73rJSWhLwguCHLvFujZ")] #[derive(Serialize, AbiExample)] pub struct BankAbiTestWrapperFuture { #[serde(serialize_with = "wrapper_future")]