gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add InstructionError for runtime instruction errors

Browse Source
This commit is contained in:
Greg Fitzgerald
2019-03-13 11:46:49 -06:00
parent 959961b596
commit c14cce4c85
9 changed files with 79 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
core/src/banking_stage.rs
View File

@ -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();

4
core/src/rpc.rs
View File

@ -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

2
core/src/rpc_subscriptions.rs
View File

@ -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,
};

6
programs/failure/tests/failure.rs
View File

@ -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)
))
);
}

6
programs/vote/tests/vote.rs
View File

@ -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)
))
);
}

31
runtime/src/bank.rs
View File

@ -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;

56
runtime/src/runtime.rs
View File

@ -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"
);
}

6
runtime/tests/system.rs
View File

@ -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);

12
sdk/src/native_program.rs
View File

@ -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,