diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 2dee0b89f1..545c6ade97 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -352,7 +352,11 @@ impl<'a> InvokeContext<'a> { } Err(InstructionError::MissingAccount) }; - visit_each_account_once(instruction_accounts, &mut work)?; + visit_each_account_once( + instruction_accounts, + &mut work, + InstructionError::NotEnoughAccountKeys, + )?; } else { let contains = (0..self .transaction_context @@ -517,7 +521,11 @@ impl<'a> InvokeContext<'a> { Ok(()) }; - visit_each_account_once(instruction_accounts, &mut work)?; + visit_each_account_once( + instruction_accounts, + &mut work, + InstructionError::NotEnoughAccountKeys, + )?; // Verify that the total sum of all the lamports did not change if pre_sum != post_sum { @@ -615,7 +623,11 @@ impl<'a> InvokeContext<'a> { } Err(InstructionError::MissingAccount) }; - visit_each_account_once(instruction_accounts, &mut work)?; + visit_each_account_once( + instruction_accounts, + &mut work, + InstructionError::NotEnoughAccountKeys, + )?; // Verify that the total sum of all the lamports did not change if pre_sum != post_sum { @@ -1154,23 +1166,25 @@ pub fn mock_process_instruction( } /// Visit each unique instruction account index once -fn visit_each_account_once( +pub fn visit_each_account_once( instruction_accounts: &[InstructionAccount], - work: &mut dyn FnMut(usize, &InstructionAccount) -> Result<(), InstructionError>, -) -> Result<(), InstructionError> { + work: &mut dyn FnMut(usize, &InstructionAccount) -> Result<(), E>, + inner_error: E, +) -> Result<(), E> { + // Note: This is an O(n^2) algorithm, + // but performed on a very small slice and requires no heap allocations 'root: for (index, instruction_account) in instruction_accounts.iter().enumerate() { - // Note: This is an O(n^2) algorithm, - // but performed on a very small slice and requires no heap allocations - for before in instruction_accounts - .get(..index) - .ok_or(InstructionError::NotEnoughAccountKeys)? - .iter() - { - if before.index_in_transaction == instruction_account.index_in_transaction { - continue 'root; // skip dups + match instruction_accounts.get(..index) { + Some(range) => { + for before in range.iter() { + if before.index_in_transaction == instruction_account.index_in_transaction { + continue 'root; // skip dups + } + } + work(index, instruction_account)?; } + None => return Err(inner_error), } - work(index, instruction_account)?; } Ok(()) } @@ -1211,7 +1225,8 @@ mod tests { index_sum_b += entry.index_in_transaction; Ok(()) }; - visit_each_account_once(accounts, &mut work).unwrap(); + visit_each_account_once(accounts, &mut work, InstructionError::NotEnoughAccountKeys) + .unwrap(); (unique_entries, index_sum_a, index_sum_b) }; diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 1119edf4a7..ee1a9f407b 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -4,7 +4,7 @@ use { alloc::Alloc, solana_program_runtime::{ ic_logger_msg, ic_msg, - invoke_context::{ComputeMeter, InvokeContext}, + invoke_context::{visit_each_account_once, ComputeMeter, InvokeContext}, stable_log, timings::ExecuteTimings, }, @@ -2574,68 +2574,76 @@ where ))?; accounts.push((*program_account_index, None)); - for instruction_account in instruction_accounts.iter() { - let account = invoke_context - .transaction_context - .get_account_at_index(instruction_account.index_in_transaction) - .map_err(SyscallError::InstructionError)?; - let account_key = invoke_context - .transaction_context - .get_key_of_account_at_index(instruction_account.index_in_transaction) - .map_err(SyscallError::InstructionError)?; - if account.borrow().executable() { - // Use the known account - accounts.push((instruction_account.index_in_transaction, None)); - } else if let Some(caller_account_index) = - account_info_keys.iter().position(|key| *key == account_key) - { - let mut caller_account = do_translate( - account_infos - .get(caller_account_index) - .ok_or(SyscallError::InvalidLength)?, - invoke_context, - )?; + visit_each_account_once::>( + instruction_accounts, + &mut |_index: usize, instruction_account: &InstructionAccount| { + let account = invoke_context + .transaction_context + .get_account_at_index(instruction_account.index_in_transaction) + .map_err(SyscallError::InstructionError)?; + let account_key = invoke_context + .transaction_context + .get_key_of_account_at_index(instruction_account.index_in_transaction) + .map_err(SyscallError::InstructionError)?; + if account.borrow().executable() { + // Use the known account + accounts.push((instruction_account.index_in_transaction, None)); + } else if let Some(caller_account_index) = + account_info_keys.iter().position(|key| *key == account_key) { - let mut account = account.borrow_mut(); - account.copy_into_owner_from_slice(caller_account.owner.as_ref()); - account.set_data_from_slice(caller_account.data); - account.set_lamports(*caller_account.lamports); - account.set_executable(caller_account.executable); - account.set_rent_epoch(caller_account.rent_epoch); - } - let caller_account = if instruction_account.is_writable { - let orig_data_len_index = instruction_account - .index_in_caller - .saturating_sub(instruction_context.get_number_of_program_accounts()); - if orig_data_len_index < orig_data_lens.len() { - caller_account.original_data_len = *orig_data_lens - .get(orig_data_len_index) - .ok_or(SyscallError::InvalidLength)?; - } else { - ic_msg!( - invoke_context, - "Internal error: index mismatch for account {}", - account_key - ); - return Err( - SyscallError::InstructionError(InstructionError::MissingAccount).into(), - ); + let mut caller_account = do_translate( + account_infos + .get(caller_account_index) + .ok_or(SyscallError::InvalidLength)?, + invoke_context, + )?; + { + let mut account = account.borrow_mut(); + account.copy_into_owner_from_slice(caller_account.owner.as_ref()); + account.set_data_from_slice(caller_account.data); + account.set_lamports(*caller_account.lamports); + account.set_executable(caller_account.executable); + account.set_rent_epoch(caller_account.rent_epoch); } + let caller_account = if instruction_account.is_writable { + let orig_data_len_index = instruction_account + .index_in_caller + .saturating_sub(instruction_context.get_number_of_program_accounts()); + if orig_data_len_index < orig_data_lens.len() { + caller_account.original_data_len = *orig_data_lens + .get(orig_data_len_index) + .ok_or(SyscallError::InvalidLength)?; + } else { + ic_msg!( + invoke_context, + "Internal error: index mismatch for account {}", + account_key + ); + return Err(SyscallError::InstructionError( + InstructionError::MissingAccount, + ) + .into()); + } - Some(caller_account) + Some(caller_account) + } else { + None + }; + accounts.push((instruction_account.index_in_transaction, caller_account)); } else { - None - }; - accounts.push((instruction_account.index_in_transaction, caller_account)); - } else { - ic_msg!( - invoke_context, - "Instruction references an unknown account {}", - account_key - ); - return Err(SyscallError::InstructionError(InstructionError::MissingAccount).into()); - } - } + ic_msg!( + invoke_context, + "Instruction references an unknown account {}", + account_key + ); + return Err( + SyscallError::InstructionError(InstructionError::MissingAccount).into(), + ); + } + Ok(()) + }, + SyscallError::InstructionError(InstructionError::NotEnoughAccountKeys).into(), + )?; Ok(accounts) }