From f10c80b49f9af86bd78c7527b9aee90f65d53f27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 25 May 2022 13:43:20 +0200 Subject: [PATCH] Refactor: Rebase offset of `index_in_caller` (#25531) * Removes the offset InstructionContext::get_number_of_program_accounts() from InstructionAccount::index_in_caller. * Removes unreachable SyscallError::InvalidLength in orig_data_lens.get(). --- program-runtime/src/invoke_context.rs | 25 ++++++++------- programs/bpf_loader/benches/serialization.rs | 2 +- programs/bpf_loader/src/syscalls.rs | 33 ++++++-------------- programs/vote/benches/process_vote.rs | 2 +- runtime/src/message_processor.rs | 2 +- sdk/src/transaction_context.rs | 2 +- 6 files changed, 28 insertions(+), 38 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 58b86194e..dc69c52e2 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -601,7 +601,7 @@ impl<'a> InvokeContext<'a> { .get_account_at_index(instruction_account.index_in_transaction)?; let is_writable = if before_instruction_context_push { instruction_context - .try_borrow_account( + .try_borrow_instruction_account( self.transaction_context, instruction_account.index_in_caller, )? @@ -772,6 +772,9 @@ impl<'a> InvokeContext<'a> { } else { let index_in_caller = instruction_context .find_index_of_account(self.transaction_context, &account_meta.pubkey) + .map(|index| { + index.saturating_sub(instruction_context.get_number_of_program_accounts()) + }) .ok_or_else(|| { ic_msg!( self, @@ -791,7 +794,7 @@ impl<'a> InvokeContext<'a> { } } for instruction_account in deduplicated_instruction_accounts.iter() { - let borrowed_account = instruction_context.try_borrow_account( + let borrowed_account = instruction_context.try_borrow_instruction_account( self.transaction_context, instruction_account.index_in_caller, )?; @@ -1142,7 +1145,7 @@ pub struct MockInvokeContextPreparation { pub fn prepare_mock_invoke_context( transaction_accounts: Vec, instruction_account_metas: Vec, - program_indices: &[usize], + _program_indices: &[usize], ) -> MockInvokeContextPreparation { let mut instruction_accounts: Vec = Vec::with_capacity(instruction_account_metas.len()); @@ -1161,7 +1164,7 @@ pub fn prepare_mock_invoke_context( .unwrap_or(index_in_instruction); instruction_accounts.push(InstructionAccount { index_in_transaction, - index_in_caller: program_indices.len().saturating_add(index_in_transaction), + index_in_caller: index_in_transaction, index_in_callee, is_signer: account_meta.is_signer, is_writable: account_meta.is_writable, @@ -1465,7 +1468,7 @@ mod tests { )); instruction_accounts.push(InstructionAccount { index_in_transaction: index, - index_in_caller: 1 + index, + index_in_caller: index, index_in_callee: instruction_accounts.len(), is_signer: false, is_writable: true, @@ -1478,7 +1481,7 @@ mod tests { )); instruction_accounts.push(InstructionAccount { index_in_transaction: index, - index_in_caller: 1 + index, + index_in_caller: index, index_in_callee: index, is_signer: false, is_writable: false, @@ -1507,14 +1510,14 @@ mod tests { let instruction_accounts = vec![ InstructionAccount { index_in_transaction: not_owned_index, - index_in_caller: 1 + not_owned_index, + index_in_caller: not_owned_index, index_in_callee: 0, is_signer: false, is_writable: true, }, InstructionAccount { index_in_transaction: owned_index, - index_in_caller: 1 + owned_index, + index_in_caller: owned_index, index_in_callee: 1, is_signer: false, is_writable: true, @@ -1632,7 +1635,7 @@ mod tests { let instruction_accounts = (0..4) .map(|index_in_instruction| InstructionAccount { index_in_transaction: index_in_instruction, - index_in_caller: 1 + index_in_instruction, + index_in_caller: index_in_instruction, index_in_callee: index_in_instruction, is_signer: false, is_writable: index_in_instruction < 2, @@ -1845,14 +1848,14 @@ mod tests { let instruction_accounts = [ InstructionAccount { index_in_transaction: 0, - index_in_caller: 1, + index_in_caller: 0, index_in_callee: 0, is_signer: false, is_writable: true, }, InstructionAccount { index_in_transaction: 1, - index_in_caller: 2, + index_in_caller: 1, index_in_callee: 1, is_signer: false, is_writable: false, diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 1ebdda0fd..dbc62c09d 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -93,7 +93,7 @@ fn create_inputs() -> TransactionContext { .enumerate() .map( |(index_in_instruction, index_in_transaction)| InstructionAccount { - index_in_caller: 1usize.saturating_add(index_in_instruction), + index_in_caller: index_in_instruction, index_in_transaction, index_in_callee: index_in_instruction, is_signer: false, diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index c28ba7f5c..e3faf83ac 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2955,10 +2955,6 @@ fn get_translated_accounts<'a, T, F>( where F: Fn(&T, &InvokeContext) -> Result, EbpfError>, { - let instruction_context = invoke_context - .transaction_context - .get_current_instruction_context() - .map_err(SyscallError::InstructionError)?; let mut accounts = Vec::with_capacity(instruction_accounts.len().saturating_add(1)); let program_account_index = program_indices @@ -3010,28 +3006,19 @@ where 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()); let orig_data_lens = invoke_context .get_orig_account_lengths() .map_err(SyscallError::InstructionError)?; - 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()); - } - + caller_account.original_data_len = *orig_data_lens + .get(instruction_account.index_in_caller) + .ok_or_else(|| { + ic_msg!( + invoke_context, + "Internal error: index mismatch for account {}", + account_key + ); + SyscallError::InstructionError(InstructionError::MissingAccount) + })?; Some(caller_account) } else { None diff --git a/programs/vote/benches/process_vote.rs b/programs/vote/benches/process_vote.rs index 2359fdf60..9c25216a6 100644 --- a/programs/vote/benches/process_vote.rs +++ b/programs/vote/benches/process_vote.rs @@ -83,7 +83,7 @@ fn create_accounts() -> ( let mut instruction_accounts = (0..4) .map(|index_in_instruction| InstructionAccount { index_in_transaction: 1usize.saturating_add(index_in_instruction), - index_in_caller: 1usize.saturating_add(index_in_instruction), + index_in_caller: index_in_instruction, index_in_callee: index_in_instruction, is_signer: false, is_writable: false, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 22dfdc27e..57853e6af 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -133,7 +133,7 @@ impl MessageProcessor { let index_in_transaction = *index_in_transaction as usize; instruction_accounts.push(InstructionAccount { index_in_transaction, - index_in_caller: program_indices.len().saturating_add(index_in_transaction), + index_in_caller: index_in_transaction, index_in_callee, is_signer: message.is_signer(index_in_transaction), is_writable: message.is_writable(index_in_transaction), diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 659b86b12..d238cf5f2 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -25,7 +25,7 @@ pub struct InstructionAccount { pub index_in_transaction: usize, /// Points to the first occurrence in the parent `InstructionContext` /// - /// This includes the program accounts. + /// This excludes the program accounts. pub index_in_caller: usize, /// Points to the first occurrence in the current `InstructionContext` ///