From a7562c9be13412c33bdf101b528587f992dbded0 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 26 Nov 2018 23:25:02 -0700 Subject: [PATCH] Extract execute_transaction() from the bank --- src/bank.rs | 51 +++++---------------------------------- src/budget_program.rs | 2 +- src/program.rs | 2 +- src/runtime.rs | 54 ++++++++++++++++++++++++++++++++++++++++-- src/storage_program.rs | 2 +- src/system_program.rs | 2 +- src/vote_program.rs | 2 +- 7 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/bank.rs b/src/bank.rs index 594a12b308..ebba8a76f2 100644 --- a/src/bank.rs +++ b/src/bank.rs @@ -22,7 +22,7 @@ use poh_service::NUM_TICKS_PER_SECOND; use program::ProgramError; use rayon::prelude::*; use rpc::RpcSignatureStatus; -use runtime; +use runtime::{self, RuntimeError}; use signature::Keypair; use signature::Signature; use solana_sdk::account::Account; @@ -707,24 +707,6 @@ impl Bank { }).collect() } - /// Execute a function with a subset of accounts as writable references. - /// Since the subset can point to the same references, in any order there is no way - /// for the borrow checker to track them with regards to the original set. - fn with_subset(accounts: &mut [Account], ixes: &[u8], func: F) -> A - where - F: FnOnce(&mut [&mut Account]) -> A, - { - let mut subset: Vec<&mut Account> = ixes - .iter() - .map(|ix| { - let ptr = &mut accounts[*ix as usize] as *mut Account; - // lifetime of this unsafe is only within the scope of the closure - // there is no way to reorder them without breaking borrow checker rules - unsafe { &mut *ptr } - }).collect(); - func(&mut subset) - } - fn load_executable_accounts(&self, mut program_id: Pubkey) -> Result> { if runtime::is_legacy_program(&program_id) { return Ok(vec![]); @@ -769,31 +751,6 @@ impl Bank { }).collect() } - /// Execute a transaction. - /// This method calls each instruction in the transaction over the set of loaded Accounts - /// The accounts are committed back to the bank only if every instruction succeeds - fn execute_transaction( - tx: &Transaction, - loaders: &mut [Vec<(Pubkey, Account)>], - tx_accounts: &mut [Account], - tick_height: u64, - ) -> Result<()> { - for (instruction_index, instruction) in tx.instructions.iter().enumerate() { - let ref mut executable_accounts = &mut loaders[instruction.program_ids_index as usize]; - Self::with_subset(tx_accounts, &instruction.accounts, |program_accounts| { - runtime::execute_instruction( - tx, - instruction_index, - executable_accounts, - program_accounts, - tick_height, - ).map_err(|err| BankError::ProgramError(instruction_index as u8, err))?; - Ok(()) - })?; - } - Ok(()) - } - pub fn store_accounts( &self, txs: &[Transaction], @@ -903,7 +860,11 @@ impl Bank { Err(e) => Err(e.clone()), Ok(ref mut accounts) => { let mut loaders = self.load_loaders(tx)?; - Self::execute_transaction(tx, &mut loaders, accounts, tick_height) + runtime::execute_transaction(tx, &mut loaders, accounts, tick_height).map_err( + |RuntimeError::ProgramError(index, err)| { + BankError::ProgramError(index, err) + }, + ) } }).collect(); let execution_elapsed = now.elapsed(); diff --git a/src/budget_program.rs b/src/budget_program.rs index d7a0a55eef..8a14f24426 100644 --- a/src/budget_program.rs +++ b/src/budget_program.rs @@ -139,7 +139,7 @@ pub fn process( instruction_index: usize, accounts: &mut [&mut Account], ) -> std::result::Result<(), ProgramError> { - process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::RuntimeError) + process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::GenericError) } //TODO the contract needs to provide a "get_balance" introspection call of the userdata diff --git a/src/program.rs b/src/program.rs index 716fc7a683..5804d5f73d 100644 --- a/src/program.rs +++ b/src/program.rs @@ -7,7 +7,7 @@ pub enum ProgramError { ResultWithNegativeTokens, /// The program returned an error - RuntimeError, + GenericError, /// Program's instruction token balance does not equal the balance after the instruction UnbalancedInstruction, diff --git a/src/runtime.rs b/src/runtime.rs index 2bed2c663c..e8234aa073 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -8,6 +8,13 @@ use system_program; use transaction::Transaction; use vote_program; +/// Reasons the runtime might have rejected a transaction. +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum RuntimeError { + /// Executing the instruction at the given index produced an error. + ProgramError(u8, ProgramError), +} + pub fn is_legacy_program(program_id: &Pubkey) -> bool { system_program::check_id(program_id) || budget_program::check_id(program_id) @@ -57,7 +64,7 @@ fn process_instruction( &tx.instructions[instruction_index].userdata, tick_height, ) { - return Err(ProgramError::RuntimeError); + return Err(ProgramError::GenericError); } } Ok(()) @@ -86,7 +93,7 @@ fn verify_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(_). -pub fn execute_instruction( +fn execute_instruction( tx: &Transaction, instruction_index: usize, executable_accounts: &mut [(Pubkey, Account)], @@ -122,3 +129,46 @@ pub fn execute_instruction( } Ok(()) } + +/// Execute a function with a subset of accounts as writable references. +/// Since the subset can point to the same references, in any order there is no way +/// for the borrow checker to track them with regards to the original set. +fn with_subset(accounts: &mut [Account], ixes: &[u8], func: F) -> A +where + F: FnOnce(&mut [&mut Account]) -> A, +{ + let mut subset: Vec<&mut Account> = ixes + .iter() + .map(|ix| { + let ptr = &mut accounts[*ix as usize] as *mut Account; + // lifetime of this unsafe is only within the scope of the closure + // there is no way to reorder them without breaking borrow checker rules + unsafe { &mut *ptr } + }).collect(); + func(&mut subset) +} + +/// Execute a transaction. +/// This method calls each instruction in the transaction over the set of loaded Accounts +/// The accounts are committed back to the bank only if every instruction succeeds +pub fn execute_transaction( + tx: &Transaction, + loaders: &mut [Vec<(Pubkey, Account)>], + tx_accounts: &mut [Account], + tick_height: u64, +) -> Result<(), RuntimeError> { + for (instruction_index, instruction) in tx.instructions.iter().enumerate() { + let executable_accounts = &mut (&mut loaders[instruction.program_ids_index as usize]); + with_subset(tx_accounts, &instruction.accounts, |program_accounts| { + execute_instruction( + tx, + instruction_index, + executable_accounts, + program_accounts, + tick_height, + ).map_err(|err| RuntimeError::ProgramError(instruction_index as u8, err))?; + Ok(()) + })?; + } + Ok(()) +} diff --git a/src/storage_program.rs b/src/storage_program.rs index 9af5c7c48d..bc540c1a8d 100644 --- a/src/storage_program.rs +++ b/src/storage_program.rs @@ -57,7 +57,7 @@ pub fn process( instruction_index: usize, accounts: &mut [&mut Account], ) -> std::result::Result<(), ProgramError> { - process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::RuntimeError) + process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::GenericError) } #[cfg(test)] diff --git a/src/system_program.rs b/src/system_program.rs index bcdd93651b..896ac77c1d 100644 --- a/src/system_program.rs +++ b/src/system_program.rs @@ -107,7 +107,7 @@ pub fn process( ) -> std::result::Result<(), ProgramError> { process_instruction(&tx, instruction_index, accounts).map_err(|err| match err { Error::ResultWithNegativeTokens => ProgramError::ResultWithNegativeTokens, - _ => ProgramError::RuntimeError, + _ => ProgramError::GenericError, }) } diff --git a/src/vote_program.rs b/src/vote_program.rs index d02c0839a6..e7d66e2188 100644 --- a/src/vote_program.rs +++ b/src/vote_program.rs @@ -122,7 +122,7 @@ pub fn process( instruction_index: usize, accounts: &mut [&mut Account], ) -> std::result::Result<(), ProgramError> { - process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::RuntimeError) + process_instruction(&tx, instruction_index, accounts).map_err(|_| ProgramError::GenericError) } pub fn get_max_size() -> usize {