diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index ebd58bc590..f9ba714bd5 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -208,8 +208,8 @@ impl BankingStage { .zip(txs.iter()) .filter_map(|(r, x)| match r { Ok(_) => Some(x.clone()), - Err(BankError::ProgramError(index, err)) => { - info!("program error {:?}, {:?}", index, err); + Err(BankError::InstructionError(index, err)) => { + info!("instruction error {:?}, {:?}", index, err); Some(x.clone()) } Err(ref e) => { @@ -445,6 +445,7 @@ mod tests { use crate::entry::EntrySlice; use crate::packet::to_packets; use crate::poh_recorder::WorkingBank; + use solana_runtime::runtime::InstructionError; use solana_sdk::genesis_block::GenesisBlock; use solana_sdk::native_program::ProgramError; use solana_sdk::signature::{Keypair, KeypairUtil}; @@ -682,9 +683,9 @@ mod tests { assert_eq!(entries[0].0.transactions.len(), transactions.len()); // ProgramErrors should still be recorded - results[0] = Err(BankError::ProgramError( + results[0] = Err(BankError::InstructionError( 1, - ProgramError::ResultWithNegativeLamports, + InstructionError::ProgramError(ProgramError::ResultWithNegativeLamports), )); BankingStage::record_transactions(&transactions, &results, &poh_recorder).unwrap(); let (_, entries) = entry_receiver.recv().unwrap(); diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 88844b24ee..ed2da0b506 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -263,7 +263,9 @@ impl RpcSol for RpcSolImpl { Ok(_) => RpcSignatureStatus::Confirmed, Err(BankError::AccountInUse) => RpcSignatureStatus::AccountInUse, Err(BankError::AccountLoadedTwice) => RpcSignatureStatus::AccountLoadedTwice, - Err(BankError::ProgramError(_, _)) => RpcSignatureStatus::ProgramRuntimeError, + Err(BankError::InstructionError(_, _)) => { + RpcSignatureStatus::ProgramRuntimeError + } Err(err) => { trace!("mapping {:?} to GenericFailure", err); RpcSignatureStatus::GenericFailure diff --git a/core/src/rpc_subscriptions.rs b/core/src/rpc_subscriptions.rs index 4bf54db8f3..9b5835c7d3 100644 --- a/core/src/rpc_subscriptions.rs +++ b/core/src/rpc_subscriptions.rs @@ -99,7 +99,7 @@ impl RpcSubscriptions { let status = match bank_error { Ok(_) => RpcSignatureStatus::Confirmed, Err(BankError::AccountInUse) => RpcSignatureStatus::AccountInUse, - Err(BankError::ProgramError(_, _)) => RpcSignatureStatus::ProgramRuntimeError, + Err(BankError::InstructionError(_, _)) => RpcSignatureStatus::ProgramRuntimeError, Err(_) => RpcSignatureStatus::GenericFailure, }; diff --git a/programs/failure/tests/failure.rs b/programs/failure/tests/failure.rs index 6ee24e96d4..a8f6a3eee4 100644 --- a/programs/failure/tests/failure.rs +++ b/programs/failure/tests/failure.rs @@ -1,6 +1,7 @@ use solana_runtime::bank::Bank; use solana_runtime::bank::BankError; use solana_runtime::loader_utils::load_program; +use solana_runtime::runtime::InstructionError; use solana_sdk::genesis_block::GenesisBlock; use solana_sdk::native_loader; use solana_sdk::native_program::ProgramError; @@ -25,6 +26,9 @@ fn test_program_native_failure() { ); assert_eq!( bank.process_transaction(&tx), - Err(BankError::ProgramError(0, ProgramError::GenericError)) + Err(BankError::InstructionError( + 0, + InstructionError::ProgramError(ProgramError::GenericError) + )) ); } diff --git a/programs/vote/tests/vote.rs b/programs/vote/tests/vote.rs index 8167269b02..b19db9050b 100644 --- a/programs/vote/tests/vote.rs +++ b/programs/vote/tests/vote.rs @@ -1,5 +1,6 @@ use solana_runtime::bank::BankError; use solana_runtime::bank::{Bank, Result}; +use solana_runtime::runtime::InstructionError; use solana_sdk::genesis_block::GenesisBlock; use solana_sdk::hash::hash; use solana_sdk::native_program::ProgramError; @@ -134,6 +135,9 @@ fn test_vote_via_bank_with_no_signature() { assert_eq!( result, - Err(BankError::ProgramError(1, ProgramError::InvalidArgument)) + Err(BankError::InstructionError( + 1, + InstructionError::ProgramError(ProgramError::InvalidArgument) + )) ); } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cdc26faa45..bcecb5e45c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5,7 +5,7 @@ use crate::accounts::{Accounts, ErrorCounters, InstructionAccounts, InstructionLoaders}; use crate::hash_queue::HashQueue; -use crate::runtime::{self, RuntimeError}; +use crate::runtime::{self, InstructionError, TransactionError}; use crate::status_cache::StatusCache; use bincode::serialize; use hashbrown::HashMap; @@ -15,7 +15,6 @@ use solana_sdk::account::Account; use solana_sdk::genesis_block::GenesisBlock; use solana_sdk::hash::{extend_and_hash, Hash}; use solana_sdk::native_loader; -use solana_sdk::native_program::ProgramError; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, Signature}; use solana_sdk::system_transaction::SystemTransaction; @@ -134,7 +133,7 @@ pub enum BankError { LedgerVerificationFailed, /// The program returned an error - ProgramError(u8, ProgramError), + InstructionError(u8, InstructionError), /// Recoding into PoH failed RecordFailure, @@ -577,8 +576,8 @@ impl Bank { Err(e) => Err(e.clone()), Ok((ref mut accounts, ref mut loaders)) => { runtime::execute_transaction(tx, loaders, accounts, tick_height).map_err( - |RuntimeError::ProgramError(index, err)| { - BankError::ProgramError(index, err) + |TransactionError::InstructionError(index, err)| { + BankError::InstructionError(index, err) }, ) } @@ -661,8 +660,8 @@ impl Bank { .iter() .zip(executed.iter()) .map(|(tx, res)| match *res { - Err(BankError::ProgramError(_, _)) => { - // Charge the transaction fee even in case of ProgramError + Err(BankError::InstructionError(_, _)) => { + // Charge the transaction fee even in case of InstructionError self.withdraw(&tx.account_keys[0], tx.fee)?; fees += tx.fee; Ok(()) @@ -1001,9 +1000,9 @@ mod tests { assert_eq!(bank.get_balance(&key2), 0); assert_eq!( bank.get_signature_status(&t1.signatures[0]), - Some(Err(BankError::ProgramError( + Some(Err(BankError::InstructionError( 1, - ProgramError::ResultWithNegativeLamports + InstructionError::ProgramError(ProgramError::ResultWithNegativeLamports) ))) ); } @@ -1049,9 +1048,9 @@ mod tests { assert_eq!( bank.process_transaction(&tx), - Err(BankError::ProgramError( + Err(BankError::InstructionError( 0, - ProgramError::ResultWithNegativeLamports + InstructionError::ProgramError(ProgramError::ResultWithNegativeLamports) )) ); @@ -1085,9 +1084,9 @@ mod tests { assert_eq!(bank.get_balance(&pubkey), 1_000); assert_eq!( bank.transfer(10_001, &mint_keypair, &pubkey, genesis_block.hash()), - Err(BankError::ProgramError( + Err(BankError::InstructionError( 0, - ProgramError::ResultWithNegativeLamports + InstructionError::ProgramError(ProgramError::ResultWithNegativeLamports) )) ); assert_eq!(bank.transaction_count(), 1); @@ -1186,9 +1185,9 @@ mod tests { let results = vec![ Ok(()), - Err(BankError::ProgramError( + Err(BankError::InstructionError( 1, - ProgramError::ResultWithNegativeLamports, + InstructionError::ProgramError(ProgramError::ResultWithNegativeLamports), )), ]; @@ -1539,7 +1538,7 @@ mod tests { Err(BankError::MissingSignatureForFee) ); - // Set the fee to 0, this should give a ProgramError + // Set the fee to 0, this should give an InstructionError // but since no signature we cannot look up the error. tx.fee = 0; diff --git a/runtime/src/runtime.rs b/runtime/src/runtime.rs index 2c176048cc..c11ab2a444 100644 --- a/runtime/src/runtime.rs +++ b/runtime/src/runtime.rs @@ -5,11 +5,30 @@ use solana_sdk::pubkey::Pubkey; use solana_sdk::system_program; use solana_sdk::transaction::Transaction; +/// Reasons the runtime might have rejected an instruction. +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum InstructionError { + /// Executing the instruction produced an error. + ProgramError(ProgramError), + + /// Program's instruction lamport balance does not equal the balance after the instruction + UnbalancedInstruction, + + /// Program modified an account's program id + ModifiedProgramId, + + /// Program spent the lamports of an account that doesn't belong to it + ExternalAccountLamportSpend, + + /// Program modified the userdata of an account that doesn't belong to it + ExternalAccountUserdataModified, +} + /// Reasons the runtime might have rejected a transaction. #[derive(Debug, PartialEq, Eq, Clone)] -pub enum RuntimeError { - /// Executing the instruction at the give&n index produced an error. - ProgramError(u8, ProgramError), +pub enum TransactionError { + /// Executing the instruction at the given index produced an error. + InstructionError(u8, InstructionError), } /// Process an instruction @@ -60,23 +79,23 @@ fn verify_instruction( pre_lamports: u64, pre_userdata: &[u8], account: &Account, -) -> Result<(), ProgramError> { +) -> Result<(), InstructionError> { // Verify the transaction // Make sure that program_id is still the same or this was just assigned by the system program if *pre_program_id != account.owner && !system_program::check_id(&program_id) { - return Err(ProgramError::ModifiedProgramId); + return Err(InstructionError::ModifiedProgramId); } // For accounts unassigned to the program, the individual balance of each accounts cannot decrease. if *program_id != account.owner && pre_lamports > account.lamports { - return Err(ProgramError::ExternalAccountLamportSpend); + return Err(InstructionError::ExternalAccountLamportSpend); } // For accounts unassigned to the program, the userdata may not change. if *program_id != account.owner && !system_program::check_id(&program_id) && pre_userdata != &account.userdata[..] { - return Err(ProgramError::ExternalAccountUserdataModified); + return Err(InstructionError::ExternalAccountUserdataModified); } Ok(()) } @@ -91,7 +110,7 @@ fn execute_instruction( executable_accounts: &mut [(Pubkey, Account)], program_accounts: &mut [&mut Account], tick_height: u64, -) -> Result<(), ProgramError> { +) -> Result<(), InstructionError> { let program_id = tx.program_id(instruction_index); // TODO: the runtime should be checking read/write access to memory // we are trusting the hard-coded programs not to clobber or allocate @@ -108,7 +127,8 @@ fn execute_instruction( program_accounts, tick_height, ) - .map_err(verify_error)?; + .map_err(verify_error) + .map_err(InstructionError::ProgramError)?; // Verify the instruction for ((pre_program_id, pre_lamports, pre_userdata), post_account) in @@ -125,7 +145,7 @@ fn execute_instruction( // The total sum of all the lamports in all the accounts cannot change. let post_total: u64 = program_accounts.iter().map(|a| a.lamports).sum(); if pre_total != post_total { - return Err(ProgramError::UnbalancedInstruction); + return Err(InstructionError::UnbalancedInstruction); } Ok(()) } @@ -173,7 +193,7 @@ pub fn execute_transaction( loaders: &mut [Vec<(Pubkey, Account)>], tx_accounts: &mut [Account], tick_height: u64, -) -> Result<(), RuntimeError> { +) -> Result<(), TransactionError> { for (instruction_index, instruction) in tx.instructions.iter().enumerate() { let executable_accounts = &mut (&mut loaders[instruction.program_ids_index as usize]); let mut program_accounts = get_subset_unchecked_mut(tx_accounts, &instruction.accounts); @@ -184,7 +204,7 @@ pub fn execute_transaction( &mut program_accounts, tick_height, ) - .map_err(|err| RuntimeError::ProgramError(instruction_index as u8, err))?; + .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; } Ok(()) } @@ -273,7 +293,11 @@ mod tests { #[test] fn test_verify_instruction_change_program_id() { - fn change_program_id(ix: &Pubkey, pre: &Pubkey, post: &Pubkey) -> Result<(), ProgramError> { + fn change_program_id( + ix: &Pubkey, + pre: &Pubkey, + post: &Pubkey, + ) -> Result<(), InstructionError> { verify_instruction(&ix, &pre, 0, &[], &Account::new(0, 0, post)) } @@ -288,14 +312,14 @@ mod tests { ); assert_eq!( change_program_id(&mallory_program_id, &system_program_id, &alice_program_id), - Err(ProgramError::ModifiedProgramId), + Err(InstructionError::ModifiedProgramId), "malicious Mallory should not be able to change the account owner" ); } #[test] fn test_verify_instruction_change_userdata() { - fn change_userdata(program_id: &Pubkey) -> Result<(), ProgramError> { + fn change_userdata(program_id: &Pubkey) -> Result<(), InstructionError> { let alice_program_id = Keypair::new().pubkey(); let account = Account::new(0, 0, &alice_program_id); verify_instruction(&program_id, &alice_program_id, 0, &[42], &account) @@ -311,7 +335,7 @@ mod tests { ); assert_eq!( change_userdata(&mallory_program_id), - Err(ProgramError::ExternalAccountUserdataModified), + Err(InstructionError::ExternalAccountUserdataModified), "malicious Mallory should not be able to change the account userdata" ); } diff --git a/runtime/tests/system.rs b/runtime/tests/system.rs index b9cebebe0e..0a3febc7dd 100644 --- a/runtime/tests/system.rs +++ b/runtime/tests/system.rs @@ -1,4 +1,5 @@ use solana_runtime::bank::{Bank, BankError}; +use solana_runtime::runtime::InstructionError; use solana_sdk::genesis_block::GenesisBlock; use solana_sdk::native_program::ProgramError; use solana_sdk::signature::{Keypair, KeypairUtil}; @@ -46,7 +47,10 @@ fn test_system_unsigned_transaction() { .sign(&[&to_keypair], blockhash); assert_eq!( system_bank.bank.process_transaction(&tx), - Err(BankError::ProgramError(0, ProgramError::InvalidArgument)) + Err(BankError::InstructionError( + 0, + InstructionError::ProgramError(ProgramError::InvalidArgument) + )) ); assert_eq!(system_bank.bank.get_balance(&from_keypair.pubkey()), 50); assert_eq!(system_bank.bank.get_balance(&to_keypair.pubkey()), 50); diff --git a/sdk/src/native_program.rs b/sdk/src/native_program.rs index 6c9e8e548d..e088f36081 100644 --- a/sdk/src/native_program.rs +++ b/sdk/src/native_program.rs @@ -16,18 +16,6 @@ pub enum ProgramError { /// contract ResultWithNegativeLamports, - /// Program's instruction lamport balance does not equal the balance after the instruction - UnbalancedInstruction, - - /// Program modified an account's program id - ModifiedProgramId, - - /// Program spent the lamports of an account that doesn't belong to it - ExternalAccountLamportSpend, - - /// Program modified the userdata of an account that doesn't belong to it - ExternalAccountUserdataModified, - /// An account's userdata contents was invalid InvalidUserdata,