Avoid panic on duplicate account indices

This commit is contained in:
Michael Vines
2019-03-19 15:25:46 -07:00
committed by Grimes
parent 061d6ec8fd
commit 8278585545
2 changed files with 19 additions and 14 deletions

View File

@ -8,7 +8,7 @@ use solana_sdk::transaction::Transaction;
/// Reasons the runtime might have rejected a transaction. /// Reasons the runtime might have rejected a transaction.
#[derive(Debug, PartialEq, Eq, Clone)] #[derive(Debug, PartialEq, Eq, Clone)]
pub enum RuntimeError { pub enum RuntimeError {
/// Executing the instruction at the give&n index produced an error. /// Executing the instruction at the given index produced an error.
ProgramError(u8, ProgramError), ProgramError(u8, ProgramError),
} }
@ -144,7 +144,7 @@ pub fn has_duplicates<T: PartialEq>(xs: &[T]) -> bool {
} }
/// Get mut references to a subset of elements. /// Get mut references to a subset of elements.
fn get_subset_unchecked_mut<'a, T>(xs: &'a mut [T], indexes: &[u8]) -> Vec<&'a mut T> { fn get_subset_mut<'a, T>(xs: &'a mut [T], indexes: &[u8]) -> Result<Vec<&'a mut T>, ProgramError> {
// Since the compiler doesn't know the indexes are unique, dereferencing // Since the compiler doesn't know the indexes are unique, dereferencing
// multiple mut elements is assumed to be unsafe. If, however, all // multiple mut elements is assumed to be unsafe. If, however, all
// indexes are unique, it's perfectly safe. The returned elements will share // indexes are unique, it's perfectly safe. The returned elements will share
@ -153,16 +153,16 @@ fn get_subset_unchecked_mut<'a, T>(xs: &'a mut [T], indexes: &[u8]) -> Vec<&'a m
// Make certain there are no duplicate indexes. If there are, panic because we // Make certain there are no duplicate indexes. If there are, panic because we
// can't return multiple mut references to the same element. // can't return multiple mut references to the same element.
if has_duplicates(indexes) { if has_duplicates(indexes) {
panic!("duplicate indexes"); Err(ProgramError::DuplicateAccountIndex)?;
} }
indexes Ok(indexes
.iter() .iter()
.map(|i| { .map(|i| {
let ptr = &mut xs[*i as usize] as *mut T; let ptr = &mut xs[*i as usize] as *mut T;
unsafe { &mut *ptr } unsafe { &mut *ptr }
}) })
.collect() .collect())
} }
/// Execute a transaction. /// Execute a transaction.
@ -176,7 +176,9 @@ pub fn execute_transaction(
) -> Result<(), RuntimeError> { ) -> Result<(), RuntimeError> {
for (instruction_index, instruction) in tx.instructions.iter().enumerate() { for (instruction_index, instruction) in tx.instructions.iter().enumerate() {
let executable_accounts = &mut (&mut loaders[instruction.program_ids_index as usize]); let executable_accounts = &mut (&mut loaders[instruction.program_ids_index as usize]);
let mut program_accounts = get_subset_unchecked_mut(tx_accounts, &instruction.accounts);
let mut program_accounts = get_subset_mut(tx_accounts, &instruction.accounts)
.map_err(|err| RuntimeError::ProgramError(instruction_index as u8, err))?;
execute_instruction( execute_instruction(
tx, tx,
instruction_index, instruction_index,
@ -203,7 +205,7 @@ where
tx_accounts.push(Account::new(0, 0, &system_program::id())); tx_accounts.push(Account::new(0, 0, &system_program::id()));
} }
for (i, ix) in tx.instructions.iter().enumerate() { for (i, ix) in tx.instructions.iter().enumerate() {
let mut ix_accounts = get_subset_unchecked_mut(tx_accounts, &ix.accounts); let mut ix_accounts = get_subset_mut(tx_accounts, &ix.accounts).unwrap();
let mut keyed_accounts: Vec<_> = ix let mut keyed_accounts: Vec<_> = ix
.accounts .accounts
.iter() .iter()
@ -249,26 +251,26 @@ mod tests {
} }
#[test] #[test]
fn test_get_subset_unchecked_mut() { fn test_get_subset_mut() {
assert_eq!(get_subset_unchecked_mut(&mut [7, 8], &[0]), vec![&mut 7]); assert_eq!(get_subset_mut(&mut [7, 8], &[0]).unwrap(), vec![&mut 7]);
assert_eq!( assert_eq!(
get_subset_unchecked_mut(&mut [7, 8], &[0, 1]), get_subset_mut(&mut [7, 8], &[0, 1]).unwrap(),
vec![&mut 7, &mut 8] vec![&mut 7, &mut 8]
); );
} }
#[test] #[test]
#[should_panic] #[should_panic]
fn test_get_subset_unchecked_mut_duplicate_index() { fn test_get_subset_mut_duplicate_index() {
// This panics, because it assumes duplicate detection is done elsewhere. // This panics, because it assumes duplicate detection is done elsewhere.
get_subset_unchecked_mut(&mut [7, 8], &[0, 0]); get_subset_mut(&mut [7, 8], &[0, 0]).unwrap();
} }
#[test] #[test]
#[should_panic] #[should_panic]
fn test_get_subset_unchecked_mut_out_of_bounds() { fn test_get_subset_mut_out_of_bounds() {
// This panics, because it assumes bounds validation is done elsewhere. // This panics, because it assumes bounds validation is done elsewhere.
get_subset_unchecked_mut(&mut [7, 8], &[2]); get_subset_mut(&mut [7, 8], &[2]).unwrap();
} }
#[test] #[test]

View File

@ -40,6 +40,9 @@ pub enum ProgramError {
/// SystemInstruction::Assign was attempted on an account unowned by the system program /// SystemInstruction::Assign was attempted on an account unowned by the system program
AssignOfUnownedAccount, AssignOfUnownedAccount,
/// An account was referenced more than once in a single instruction
DuplicateAccountIndex,
/// CustomError allows on-chain programs to implement program-specific error types and see /// CustomError allows on-chain programs to implement program-specific error types and see
/// them returned by the Solana runtime. A CustomError may be any type that is serialized /// them returned by the Solana runtime. A CustomError may be any type that is serialized
/// to a Vec of bytes, max length 32 bytes. Any CustomError Vec greater than this length will /// to a Vec of bytes, max length 32 bytes. Any CustomError Vec greater than this length will