diff --git a/core/src/sigverify.rs b/core/src/sigverify.rs index 1d56939e68..225befe908 100644 --- a/core/src/sigverify.rs +++ b/core/src/sigverify.rs @@ -497,8 +497,6 @@ mod tests { let fee = 2; let blockhash = Hash::default(); - let keys = vec![keypair0.pubkey(), keypair1.pubkey()]; - let system_instruction = SystemInstruction::Move { lamports }; let program_ids = vec![system_program::id(), solana_budget_api::id()]; @@ -507,7 +505,7 @@ mod tests { let tx = Transaction::new_with_instructions( &keypairs, - &keys, + &[], blockhash, fee, program_ids, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d3454394db..3d5077fbc3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1246,7 +1246,13 @@ mod tests { let res = bank.process_transactions(&vec![tx.clone()]); assert_eq!(res.len(), 1); assert_eq!(bank.get_balance(&key1.pubkey()), 1); - res[0].clone().unwrap_err(); + + // TODO: Why do we convert errors to Oks? + //res[0].clone().unwrap_err(); + + bank.get_signature_status(&tx.signatures[0]) + .unwrap() + .unwrap_err(); } fn new_from_parent(parent: &Arc) -> Bank { diff --git a/runtime/src/runtime.rs b/runtime/src/runtime.rs index 9232da3b0d..527c7b117e 100644 --- a/runtime/src/runtime.rs +++ b/runtime/src/runtime.rs @@ -138,25 +138,28 @@ pub fn has_duplicates(xs: &[T]) -> bool { } /// 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_unchecked_mut<'a, T>( + xs: &'a mut [T], + indexes: &[u8], +) -> Result, InstructionError> { // Since the compiler doesn't know the indexes are unique, dereferencing // multiple mut elements is assumed to be unsafe. If, however, all // indexes are unique, it's perfectly safe. The returned elements will share // the liftime of the input slice. - // Make certain there are no duplicate indexes. If there are, panic because we - // can't return multiple mut references to the same element. + // Make certain there are no duplicate indexes. If there are, return an error + // because we can't return multiple mut references to the same element. if has_duplicates(indexes) { - panic!("duplicate indexes"); + return Err(InstructionError::DuplicateAccountIndex); } - indexes + Ok(indexes .iter() .map(|i| { let ptr = &mut xs[*i as usize] as *mut T; unsafe { &mut *ptr } }) - .collect() + .collect()) } /// Execute a transaction. @@ -170,7 +173,8 @@ pub fn execute_transaction( ) -> 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); + let mut program_accounts = get_subset_unchecked_mut(tx_accounts, &instruction.accounts) + .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; execute_instruction( tx, instruction_index, @@ -197,7 +201,7 @@ where tx_accounts.push(Account::new(0, 0, &system_program::id())); } 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_unchecked_mut(tx_accounts, &ix.accounts).unwrap(); let mut keyed_accounts: Vec<_> = ix .accounts .iter() @@ -244,25 +248,30 @@ mod tests { #[test] fn test_get_subset_unchecked_mut() { - assert_eq!(get_subset_unchecked_mut(&mut [7, 8], &[0]), vec![&mut 7]); assert_eq!( - get_subset_unchecked_mut(&mut [7, 8], &[0, 1]), + get_subset_unchecked_mut(&mut [7, 8], &[0]).unwrap(), + vec![&mut 7] + ); + assert_eq!( + get_subset_unchecked_mut(&mut [7, 8], &[0, 1]).unwrap(), vec![&mut 7, &mut 8] ); } #[test] - #[should_panic] fn test_get_subset_unchecked_mut_duplicate_index() { // This panics, because it assumes duplicate detection is done elsewhere. - get_subset_unchecked_mut(&mut [7, 8], &[0, 0]); + assert_eq!( + get_subset_unchecked_mut(&mut [7, 8], &[0, 0]).unwrap_err(), + InstructionError::DuplicateAccountIndex + ); } #[test] #[should_panic] fn test_get_subset_unchecked_mut_out_of_bounds() { // This panics, because it assumes bounds validation is done elsewhere. - get_subset_unchecked_mut(&mut [7, 8], &[2]); + get_subset_unchecked_mut(&mut [7, 8], &[2]).unwrap(); } #[test] diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index f7c1fad956..2fd4fe453e 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -35,6 +35,9 @@ pub enum InstructionError { /// Program modified the data of an account that doesn't belong to it ExternalAccountDataModified, + + /// An account was referenced more than once in a single instruction + DuplicateAccountIndex, } impl InstructionError { @@ -177,7 +180,7 @@ impl Transaction { Hash::default(), fee, ); - transaction.sign(&[from_keypair], recent_blockhash); + transaction.sign_checked(&[from_keypair], recent_blockhash); transaction } pub fn new_unsigned( @@ -219,14 +222,14 @@ impl Transaction { .collect(); account_keys.extend_from_slice(keys); let mut tx = Transaction { - signatures: vec![], + signatures: Vec::with_capacity(from_keypairs.len()), account_keys, recent_blockhash: Hash::default(), fee, program_ids, instructions, }; - tx.sign(from_keypairs, recent_blockhash); + tx.sign_checked(from_keypairs, recent_blockhash); tx } pub fn data(&self, instruction_index: usize) -> &[u8] {