Migrate to sign_checked()

This commit is contained in:
Greg Fitzgerald 2019-03-14 15:32:12 -06:00
parent bd8502e87e
commit 7b4568b9bf
4 changed files with 36 additions and 20 deletions

View File

@ -497,8 +497,6 @@ mod tests {
let fee = 2; let fee = 2;
let blockhash = Hash::default(); let blockhash = Hash::default();
let keys = vec![keypair0.pubkey(), keypair1.pubkey()];
let system_instruction = SystemInstruction::Move { lamports }; let system_instruction = SystemInstruction::Move { lamports };
let program_ids = vec![system_program::id(), solana_budget_api::id()]; let program_ids = vec![system_program::id(), solana_budget_api::id()];
@ -507,7 +505,7 @@ mod tests {
let tx = Transaction::new_with_instructions( let tx = Transaction::new_with_instructions(
&keypairs, &keypairs,
&keys, &[],
blockhash, blockhash,
fee, fee,
program_ids, program_ids,

View File

@ -1246,7 +1246,13 @@ mod tests {
let res = bank.process_transactions(&vec![tx.clone()]); let res = bank.process_transactions(&vec![tx.clone()]);
assert_eq!(res.len(), 1); assert_eq!(res.len(), 1);
assert_eq!(bank.get_balance(&key1.pubkey()), 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>) -> Bank { fn new_from_parent(parent: &Arc<Bank>) -> Bank {

View File

@ -138,25 +138,28 @@ 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_unchecked_mut<'a, T>(
xs: &'a mut [T],
indexes: &[u8],
) -> Result<Vec<&'a mut T>, InstructionError> {
// 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
// the liftime of the input slice. // the liftime of the input slice.
// Make certain there are no duplicate indexes. If there are, panic because we // Make certain there are no duplicate indexes. If there are, return an error
// can't return multiple mut references to the same element. // because we can't return multiple mut references to the same element.
if has_duplicates(indexes) { if has_duplicates(indexes) {
panic!("duplicate indexes"); return Err(InstructionError::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.
@ -170,7 +173,8 @@ pub fn execute_transaction(
) -> Result<(), TransactionError> { ) -> Result<(), TransactionError> {
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_unchecked_mut(tx_accounts, &instruction.accounts)
.map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;
execute_instruction( execute_instruction(
tx, tx,
instruction_index, instruction_index,
@ -197,7 +201,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_unchecked_mut(tx_accounts, &ix.accounts).unwrap();
let mut keyed_accounts: Vec<_> = ix let mut keyed_accounts: Vec<_> = ix
.accounts .accounts
.iter() .iter()
@ -244,25 +248,30 @@ mod tests {
#[test] #[test]
fn test_get_subset_unchecked_mut() { fn test_get_subset_unchecked_mut() {
assert_eq!(get_subset_unchecked_mut(&mut [7, 8], &[0]), vec![&mut 7]);
assert_eq!( 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] vec![&mut 7, &mut 8]
); );
} }
#[test] #[test]
#[should_panic]
fn test_get_subset_unchecked_mut_duplicate_index() { fn test_get_subset_unchecked_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]); assert_eq!(
get_subset_unchecked_mut(&mut [7, 8], &[0, 0]).unwrap_err(),
InstructionError::DuplicateAccountIndex
);
} }
#[test] #[test]
#[should_panic] #[should_panic]
fn test_get_subset_unchecked_mut_out_of_bounds() { fn test_get_subset_unchecked_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_unchecked_mut(&mut [7, 8], &[2]).unwrap();
} }
#[test] #[test]

View File

@ -35,6 +35,9 @@ pub enum InstructionError {
/// Program modified the data of an account that doesn't belong to it /// Program modified the data of an account that doesn't belong to it
ExternalAccountDataModified, ExternalAccountDataModified,
/// An account was referenced more than once in a single instruction
DuplicateAccountIndex,
} }
impl InstructionError { impl InstructionError {
@ -177,7 +180,7 @@ impl Transaction {
Hash::default(), Hash::default(),
fee, fee,
); );
transaction.sign(&[from_keypair], recent_blockhash); transaction.sign_checked(&[from_keypair], recent_blockhash);
transaction transaction
} }
pub fn new_unsigned<T: Serialize>( pub fn new_unsigned<T: Serialize>(
@ -219,14 +222,14 @@ impl Transaction {
.collect(); .collect();
account_keys.extend_from_slice(keys); account_keys.extend_from_slice(keys);
let mut tx = Transaction { let mut tx = Transaction {
signatures: vec![], signatures: Vec::with_capacity(from_keypairs.len()),
account_keys, account_keys,
recent_blockhash: Hash::default(), recent_blockhash: Hash::default(),
fee, fee,
program_ids, program_ids,
instructions, instructions,
}; };
tx.sign(from_keypairs, recent_blockhash); tx.sign_checked(from_keypairs, recent_blockhash);
tx tx
} }
pub fn data(&self, instruction_index: usize) -> &[u8] { pub fn data(&self, instruction_index: usize) -> &[u8] {