From df6905c3a67e46c17d66a282d200741508c9e5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 27 Sep 2021 08:28:45 +0200 Subject: [PATCH] Refactor: Merge MessageProcessor into InvokeContext (#20165) * Inlines MessageProcessor::execute_instruction() in MessageProcessor::process_message(). * Moves MessageProcessor::create_pre_accounts() into ThisInvokeContext::push(). * Hoists ThisInvokeContext::new() out of loop inside MessageProcessor::process_message(). * Moves MessageProcessor::verify_account_references() and MessageProcessor::verify() into InvokeContext::verify(). --- program-runtime/src/instruction_processor.rs | 2 +- runtime/src/bank.rs | 21 +- runtime/src/message_processor.rs | 699 ++++++++----------- sdk/src/process_instruction.rs | 22 +- 4 files changed, 336 insertions(+), 408 deletions(-) diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs index c2a0eef87..e4c450c28 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -584,7 +584,7 @@ impl InstructionProcessor { message, instruction, program_indices, - account_indices, + Some(account_indices), )?; let mut instruction_processor = InstructionProcessor::default(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 136397888..6afc5d026 100644 --- a/runtime/src/bank.rs +++ b/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 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. //! //! 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}; +use solana_program_runtime::{ExecuteDetailsTimings, Executors, InstructionProcessor}; #[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 Message processor - message_processor: MessageProcessor, + /// The InstructionProcessor + instruction_processor: InstructionProcessor, compute_budget: Option, @@ -1095,7 +1095,7 @@ impl Bank { stakes: RwLock::::default(), epoch_stakes: HashMap::::default(), is_delta: AtomicBool::default(), - message_processor: MessageProcessor::default(), + instruction_processor: InstructionProcessor::default(), compute_budget: Option::::default(), feature_builtins: Arc::>::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), - message_processor: parent.message_processor.clone(), + instruction_processor: parent.instruction_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), - message_processor: new(), + instruction_processor: new(), compute_budget: None, feature_builtins: new(), last_vote_sync: new(), @@ -3389,7 +3389,8 @@ impl Bank { }; 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, &loaded_transaction.program_indices, &account_refcells, @@ -5307,7 +5308,7 @@ impl Bank { ) { debug!("Adding program {} under {:?}", name, program_id); self.add_native_program(name, &program_id, false); - self.message_processor + self.instruction_processor .add_program(program_id, process_instruction_with_context); } @@ -5320,7 +5321,7 @@ impl Bank { ) { debug!("Replacing program {} under {:?}", name, program_id); self.add_native_program(name, &program_id, true); - self.message_processor + self.instruction_processor .add_program(program_id, process_instruction_with_context); } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 5216087fb..78cf1ad55 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -46,6 +46,7 @@ impl ComputeMeter for ThisComputeMeter { } } pub struct ThisInvokeContext<'a> { + instruction_index: usize, invoke_stack: Vec>, rent: Rent, pre_accounts: Vec, @@ -57,7 +58,7 @@ pub struct ThisInvokeContext<'a> { bpf_compute_budget: solana_sdk::process_instruction::BpfComputeBudget, compute_meter: Rc>, executors: Rc>, - instruction_recorder: Option, + instruction_recorders: Option<&'a [InstructionRecorder]>, feature_set: Arc, pub timings: ExecuteDetailsTimings, account_db: Arc, @@ -72,25 +73,20 @@ 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>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], log_collector: Option>, compute_budget: ComputeBudget, compute_meter: Rc>, executors: Rc>, - instruction_recorder: Option, + instruction_recorders: Option<&'a [InstructionRecorder]>, feature_set: Arc, account_db: Arc, ancestors: &'a Ancestors, blockhash: &'a Hash, fee_calculator: &'a FeeCalculator, - ) -> Result { - let pre_accounts = MessageProcessor::create_pre_accounts(message, instruction, accounts); + ) -> Self { let compute_meter = if feature_set.is_active(&tx_wide_compute_cap::id()) { compute_meter } else { @@ -98,10 +94,11 @@ impl<'a> ThisInvokeContext<'a> { remaining: compute_budget.max_units, })) }; - let mut invoke_context = Self { + Self { + instruction_index: 0, invoke_stack: Vec::with_capacity(compute_budget.max_invoke_depth), rent, - pre_accounts, + pre_accounts: Vec::new(), accounts, programs, logger: Rc::new(RefCell::new(ThisLogger { log_collector })), @@ -109,7 +106,7 @@ impl<'a> ThisInvokeContext<'a> { bpf_compute_budget: compute_budget.into(), compute_meter, executors, - instruction_recorder, + instruction_recorders, feature_set, timings: ExecuteDetailsTimings::default(), account_db, @@ -118,16 +115,7 @@ impl<'a> ThisInvokeContext<'a> { blockhash, fee_calculator, return_data: None, - }; - let account_indices = (0..accounts.len()).collect::>(); - invoke_context.push( - program_id, - message, - instruction, - program_indices, - &account_indices, - )?; - Ok(invoke_context) + } } } impl<'a> InvokeContext for ThisInvokeContext<'a> { @@ -137,12 +125,27 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { message: &Message, instruction: &CompiledInstruction, program_indices: &[usize], - account_indices: &[usize], + account_indices: Option<&[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 @@ -170,7 +173,11 @@ 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 = account_indices[index_in_instruction]; + let account_index = if let Some(account_indices) = account_indices { + 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), @@ -191,6 +198,64 @@ 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()); + + // 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, + ) + .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, @@ -203,7 +268,7 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { .ok_or(InstructionError::CallDepth)?; let program_id = &stack_frame.key; let rent = &self.rent; - let logger = self.get_logger(); + let logger = &self.logger; let accounts = &self.accounts; let pre_accounts = &mut self.pre_accounts; let timings = &mut self.timings; @@ -292,9 +357,12 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { fn get_executor(&self, pubkey: &Pubkey) -> Option> { 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(recorder) = &self.instruction_recorder { - recorder.record_instruction(instruction.clone()); + if let Some(instruction_recorders) = &self.instruction_recorders { + instruction_recorders[self.instruction_index].record_instruction(instruction.clone()); } } fn is_feature_active(&self, feature_id: &Pubkey) -> bool { @@ -372,10 +440,7 @@ impl Logger for ThisLogger { } #[derive(Debug, Default, Clone, Deserialize, Serialize)] -pub struct MessageProcessor { - #[serde(skip)] - instruction_processor: InstructionProcessor, -} +pub struct MessageProcessor {} #[cfg(RUSTC_WITH_SPECIALIZATION)] impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor { @@ -387,212 +452,15 @@ impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor { } impl MessageProcessor { - /// Add a static entrypoint to intercept instructions before the dynamic loader. - pub fn add_program( - &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>)], - ) -> Vec { - 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>)], - 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>)], - rent: &Rent, - timings: &mut ExecuteDetailsTimings, - logger: Rc>, - demote_program_write_locks: 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, - ) - .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 this function returns Ok(_). - #[allow(clippy::too_many_arguments)] - fn execute_instruction( - &self, - message: &Message, - instruction: &CompiledInstruction, - program_indices: &[usize], - accounts: &[(Pubkey, Rc>)], - rent_collector: &RentCollector, - log_collector: Option>, - executors: Rc>, - instruction_recorder: Option, - instruction_index: usize, - feature_set: Arc, - compute_budget: ComputeBudget, - compute_meter: Rc>, - timings: &mut ExecuteDetailsTimings, - account_db: Arc, - 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()), - )?; - - 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 + /// This method calls each instruction in the message over the set of loaded accounts. + /// For each instruction it calls the 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. #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] pub fn process_message( - &self, + instruction_processor: &InstructionProcessor, message: &Message, program_indices: &[Vec], accounts: &[(Pubkey, Rc>)], @@ -609,43 +477,80 @@ impl MessageProcessor { blockhash: Hash, fee_calculator: FeeCalculator, ) -> Result<(), TransactionError> { - for (instruction_index, instruction) in message.instructions.iter().enumerate() { + let mut invoke_context = ThisInvokeContext::new( + 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 - .as_ref() - .map(|recorders| recorders[instruction_index].clone()); - let err = self - .execute_instruction( - message, - instruction, - &program_indices[instruction_index], - accounts, - rent_collector, - log_collector.clone(), - executors.clone(), - instruction_recorder, - instruction_index, - feature_set.clone(), - compute_budget, - compute_meter.clone(), - timings, - account_db.clone(), - ancestors, - &blockhash, - &fee_calculator, - ) + + // Fixup the special instructions key if present + // before the account pre-values are taken care of + for (pubkey, account) in accounts.iter().take(message.account_keys.len()) { + if instructions::check_id(pubkey) { + let mut mut_account_ref = account.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 invoke_context.is_feature_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); + } + + 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, ); - err?; + result?; } Ok(()) } @@ -661,6 +566,47 @@ 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; @@ -700,11 +646,7 @@ 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, @@ -717,20 +659,13 @@ mod tests { &ancestors, &blockhash, &fee_calculator, - ) - .unwrap(); + ); // Check call depth increases and has a limit - let mut depth_reached = 1; - for program_id in invoke_stack.iter().skip(1) { + let mut depth_reached = 0; + for program_id in invoke_stack.iter() { if Err(InstructionError::CallDepth) - == invoke_context.push( - program_id, - &message, - &message.instructions[0], - &[], - &account_indices, - ) + == invoke_context.push(program_id, &message, &message.instructions[0], &[], None) { break; } @@ -793,17 +728,53 @@ mod tests { } #[test] - fn test_verify_account_references() { + fn test_invoke_context_verify() { let accounts = vec![( solana_sdk::pubkey::new_rand(), Rc::new(RefCell::new(AccountSharedData::default())), )]; - - assert!(MessageProcessor::verify_account_references(&accounts, &[0]).is_ok()); + let message = Message::new( + &[Instruction::new_with_bincode( + 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!( - MessageProcessor::verify_account_references(&accounts, &[0]), + invoke_context.verify(&message, &message.instructions[0], &[0]), Err(InstructionError::AccountBorrowOutstanding) ); } @@ -850,8 +821,8 @@ mod tests { let mock_system_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); - let mut message_processor = MessageProcessor::default(); - message_processor.add_program(mock_system_program_id, mock_system_process_instruction); + let mut instruction_processor = InstructionProcessor::default(); + instruction_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", @@ -885,7 +856,8 @@ mod tests { Some(&accounts[0].0), ); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -915,7 +887,8 @@ mod tests { Some(&accounts[0].0), ); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -949,7 +922,8 @@ mod tests { Some(&accounts[0].0), ); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -1038,8 +1012,8 @@ mod tests { let mock_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); - let mut message_processor = MessageProcessor::default(); - message_processor.add_program(mock_program_id, mock_system_process_instruction); + let mut instruction_processor = InstructionProcessor::default(); + instruction_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", @@ -1075,7 +1049,8 @@ mod tests { )], Some(&accounts[0].0), ); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -1109,7 +1084,8 @@ mod tests { )], Some(&accounts[0].0), ); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -1141,7 +1117,8 @@ mod tests { Some(&accounts[0].0), ); let ancestors = Ancestors::default(); - let result = message_processor.process_message( + let result = MessageProcessor::process_message( + &instruction_processor, &message, &program_indices, &accounts, @@ -1166,47 +1143,6 @@ 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(); @@ -1216,7 +1152,6 @@ 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(), @@ -1258,11 +1193,7 @@ 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, @@ -1275,8 +1206,16 @@ mod tests { &ancestors, &blockhash, &fee_calculator, - ) - .unwrap(); + ); + invoke_context + .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 @@ -1334,11 +1273,7 @@ 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, @@ -1351,8 +1286,16 @@ mod tests { &ancestors, &blockhash, &fee_calculator, - ) - .unwrap(); + ); + invoke_context + .push( + &caller_program_id, + &message, + &caller_instruction, + &program_indices, + None, + ) + .unwrap(); let caller_write_privileges = message .account_keys @@ -1375,47 +1318,6 @@ 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(); @@ -1425,7 +1327,6 @@ 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(), @@ -1463,11 +1364,7 @@ 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, @@ -1480,8 +1377,16 @@ mod tests { &ancestors, &blockhash, &fee_calculator, - ) - .unwrap(); + ); + invoke_context + .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; @@ -1535,11 +1440,7 @@ 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, @@ -1552,8 +1453,16 @@ mod tests { &ancestors, &blockhash, &fee_calculator, - ) - .unwrap(); + ); + invoke_context + .push( + &caller_program_id, + &message, + &caller_instruction, + &program_indices, + None, + ) + .unwrap(); assert_eq!( InstructionProcessor::native_invoke( diff --git a/sdk/src/process_instruction.rs b/sdk/src/process_instruction.rs index afc665efa..c711a6596 100644 --- a/sdk/src/process_instruction.rs +++ b/sdk/src/process_instruction.rs @@ -58,12 +58,19 @@ pub trait InvokeContext { message: &Message, instruction: &CompiledInstruction, program_indices: &[usize], - account_indices: &[usize], + account_indices: Option<&[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, @@ -92,6 +99,8 @@ pub trait InvokeContext { fn add_executor(&self, pubkey: &Pubkey, executor: Arc); /// Get the completed loader work that can be re-used across executions fn get_executor(&self, pubkey: &Pubkey) -> Option>; + /// 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 @@ -492,7 +501,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> { _message: &Message, _instruction: &CompiledInstruction, _program_indices: &[usize], - _account_indices: &[usize], + _account_indices: Option<&[usize]>, ) -> Result<(), InstructionError> { self.invoke_stack.push(InvokeContextStackFrame::new( *_key, @@ -506,6 +515,14 @@ 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, @@ -553,6 +570,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> { fn get_executor(&self, _pubkey: &Pubkey) -> Option> { 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)