diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index a18faeb2293041..47040df6b5b2fb 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -408,7 +408,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { pubkey, AccountSharedData::new(0, allocation_size, &Pubkey::new_unique()), )); - instruction_accounts.push(InstructionAccount::new(0, 0, 0, false, true)); + instruction_accounts.push(InstructionAccount::new(0, 0, false, true)); vec![] } Err(_) => { @@ -480,7 +480,6 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { idx }; InstructionAccount::new( - txn_acct_index as IndexOfAccount, txn_acct_index as IndexOfAccount, txn_acct_index as IndexOfAccount, account_info.is_signer.unwrap_or(false), diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index e03fdf749e43b5..33d1252fc80ba4 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -368,23 +368,9 @@ impl<'a> InvokeContext<'a> { }; instruction_accounts.push(cloned_account); } else { - let index_in_caller = instruction_context - .find_index_of_instruction_account( - self.transaction_context, - &account_meta.pubkey, - ) - .ok_or_else(|| { - ic_msg!( - self, - "Instruction references an unknown account {}", - account_meta.pubkey, - ); - InstructionError::MissingAccount - })?; *index_in_callee = instruction_accounts.len() as u8; instruction_accounts.push(InstructionAccount::new( index_in_transaction, - index_in_caller, instruction_account_index as IndexOfAccount, account_meta.is_signer, account_meta.is_writable, @@ -413,10 +399,11 @@ impl<'a> InvokeContext<'a> { continue; } - let borrowed_account = instruction_context.try_borrow_instruction_account( - self.transaction_context, - instruction_account.index_in_caller, + let index_in_caller = instruction_context.get_index_of_account_in_instruction( + instruction_account.index_in_transaction, )?; + let borrowed_account = instruction_context + .try_borrow_instruction_account(self.transaction_context, index_in_caller)?; // Readonly in caller cannot become writable in callee if instruction_account.is_writable() && !borrowed_account.is_writable() { @@ -506,7 +493,6 @@ impl<'a> InvokeContext<'a> { let index_in_transaction = *index_in_transaction as usize; instruction_accounts.push(InstructionAccount::new( - index_in_transaction as IndexOfAccount, index_in_transaction as IndexOfAccount, *index_in_callee as IndexOfAccount, message.is_signer(index_in_transaction), @@ -901,7 +887,6 @@ pub fn mock_process_instruction_with_feature_set< }) .unwrap_or(instruction_account_index) as IndexOfAccount; instruction_accounts.push(InstructionAccount::new( - index_in_transaction, index_in_transaction, index_in_callee, account_meta.is_signer, @@ -1022,7 +1007,6 @@ mod tests { let instruction_accounts = (0..4) .map(|instruction_account_index| { InstructionAccount::new( - instruction_account_index, instruction_account_index, instruction_account_index, false, @@ -1128,7 +1112,6 @@ mod tests { AccountSharedData::new(index as u64, 1, invoke_stack.get(index).unwrap()), )); instruction_accounts.push(InstructionAccount::new( - index as IndexOfAccount, index as IndexOfAccount, instruction_accounts.len() as IndexOfAccount, false, @@ -1141,7 +1124,6 @@ mod tests { AccountSharedData::new(1, 1, &solana_pubkey::Pubkey::default()), )); instruction_accounts.push(InstructionAccount::new( - index as IndexOfAccount, index as IndexOfAccount, index as IndexOfAccount, false, @@ -1219,7 +1201,6 @@ mod tests { let instruction_accounts = (0..4) .map(|instruction_account_index| { InstructionAccount::new( - instruction_account_index, instruction_account_index, instruction_account_index, false, @@ -1277,7 +1258,6 @@ mod tests { let instruction_accounts = (0..4) .map(|instruction_account_index| { InstructionAccount::new( - instruction_account_index, instruction_account_index, instruction_account_index, false, @@ -1368,8 +1348,8 @@ mod tests { (program_key, program_account), ]; let instruction_accounts = vec![ - InstructionAccount::new(0, 0, 0, false, true), - InstructionAccount::new(1, 1, 1, false, false), + InstructionAccount::new(0, 0, false, true), + InstructionAccount::new(1, 1, false, false), ]; with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); diff --git a/program-runtime/src/serialization.rs b/program-runtime/src/serialization.rs index 77575fd9c8b7b4..c1e263ef51ed5d 100644 --- a/program-runtime/src/serialization.rs +++ b/program-runtime/src/serialization.rs @@ -631,7 +631,6 @@ mod tests { .position(|account_index| account_index == index_in_transaction) .unwrap_or(index_in_instruction); InstructionAccount::new( - *index_in_transaction, *index_in_transaction, index_in_callee as IndexOfAccount, false, diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 5f201e20afe871..fa26b7d993c682 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -295,11 +295,11 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs { .ok_or(InstructionError::MissingAccount) .unwrap(); let account_info = &account_infos[account_info_index]; + let index_in_caller = instruction_context + .get_index_of_account_in_instruction(instruction_account.index_in_transaction) + .unwrap(); let mut borrowed_account = instruction_context - .try_borrow_instruction_account( - transaction_context, - instruction_account.index_in_caller, - ) + .try_borrow_instruction_account(transaction_context, index_in_caller) .unwrap(); if borrowed_account.get_lamports() != account_info.lamports() { borrowed_account @@ -324,7 +324,8 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs { .unwrap(); } if instruction_account.is_writable() { - account_indices.push((instruction_account.index_in_caller, account_info_index)); + account_indices + .push((instruction_account.index_in_transaction, account_info_index)); } } @@ -338,7 +339,10 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); - for (index_in_caller, account_info_index) in account_indices.into_iter() { + for (index_in_transaction, account_info_index) in account_indices.into_iter() { + let index_in_caller = instruction_context + .get_index_of_account_in_instruction(index_in_transaction) + .unwrap(); let borrowed_account = instruction_context .try_borrow_instruction_account(transaction_context, index_in_caller) .unwrap(); diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index a9bf31f940d914..917a530eb18e35 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -95,7 +95,6 @@ fn create_inputs(owner: Pubkey, num_instruction_accounts: usize) -> TransactionC .unwrap_or(instruction_account_index) as IndexOfAccount; instruction_accounts.push(InstructionAccount::new( index_in_transaction, - instruction_account_index as IndexOfAccount, index_in_callee, false, instruction_account_index >= 4, diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index b6fad48796e2a7..87b1fe6d86930c 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -61,7 +61,7 @@ macro_rules! with_mock_invoke_context { AccountSharedData::new(2, $account_size, &program_key), ), ]; - let instruction_accounts = vec![InstructionAccount::new(2, 2, 0, false, true)]; + let instruction_accounts = vec![InstructionAccount::new(2, 0, false, true)]; solana_program_runtime::with_mock_invoke_context!( $invoke_context, transaction_context, diff --git a/programs/system/src/system_instruction.rs b/programs/system/src/system_instruction.rs index fdfec636f2177f..fc2fea06cb4e27 100644 --- a/programs/system/src/system_instruction.rs +++ b/programs/system/src/system_instruction.rs @@ -294,8 +294,8 @@ mod test { (system_program::id(), AccountSharedData::default()), ]; let $instruction_accounts = vec![ - InstructionAccount::new(0, 0, 0, true, true), - InstructionAccount::new(1, 1, 1, false, true), + InstructionAccount::new(0, 0, true, true), + InstructionAccount::new(1, 1, false, true), ]; with_mock_invoke_context!($invoke_context, transaction_context, transaction_accounts); }; diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 146d1964e6ff9c..685f23d46f234a 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1169,7 +1169,7 @@ mod tests { let mut instruction_context = InstructionContext::default(); instruction_context.configure( vec![0], - vec![InstructionAccount::new(1, 1, 0, false, true)], + vec![InstructionAccount::new(1, 0, false, true)], &[], ); @@ -1318,7 +1318,7 @@ mod tests { let mut instruction_context = InstructionContext::default(); instruction_context.configure( vec![0], - vec![InstructionAccount::new(1, 1, 0, false, true)], + vec![InstructionAccount::new(1, 0, false, true)], &[], ); diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index 43df1c7a67ee2c..003ebe1d9b3a4b 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -29,7 +29,7 @@ use { solana_sysvar_id::SysvarId, solana_timings::ExecuteTimings, solana_transaction_context::{ - ExecutionRecord, IndexOfAccount, InstructionAccount, TransactionAccount, TransactionContext, + ExecutionRecord, IndexOfAccount, TransactionAccount, TransactionContext, }, std::{ collections::{hash_map::Entry, HashMap}, @@ -389,31 +389,6 @@ fn execute_fixture_as_instr( SVMTransactionExecutionCost::default(), ); - let mut instruction_accounts: Vec = - Vec::with_capacity(sanitized_message.instructions()[0].accounts.len()); - - for (instruction_acct_idx, index_txn) in sanitized_message.instructions()[0] - .accounts - .iter() - .enumerate() - { - let index_in_callee = sanitized_message.instructions()[0] - .accounts - .get(0..instruction_acct_idx) - .unwrap() - .iter() - .position(|idx| *idx == *index_txn) - .unwrap_or(instruction_acct_idx); - - instruction_accounts.push(InstructionAccount::new( - *index_txn as IndexOfAccount, - *index_txn as IndexOfAccount, - index_in_callee as IndexOfAccount, - sanitized_message.is_signer(*index_txn as usize), - sanitized_message.is_writable(*index_txn as usize), - )); - } - invoke_context .prepare_next_top_level_instruction( sanitized_message, diff --git a/syscalls/src/cpi.rs b/syscalls/src/cpi.rs index f13098b489bac2..e1e6a895b2062e 100644 --- a/syscalls/src/cpi.rs +++ b/syscalls/src/cpi.rs @@ -797,10 +797,10 @@ where continue; // Skip duplicate account } - let callee_account = instruction_context.try_borrow_instruction_account( - transaction_context, - instruction_account.index_in_caller, - )?; + let index_in_caller = instruction_context + .get_index_of_account_in_instruction(instruction_account.index_in_transaction)?; + let callee_account = instruction_context + .try_borrow_instruction_account(transaction_context, index_in_caller)?; let account_key = invoke_context .transaction_context .get_key_of_account_at_index(instruction_account.index_in_transaction)?; @@ -817,16 +817,17 @@ where } else if let Some(caller_account_index) = account_info_keys.iter().position(|key| *key == account_key) { - let serialized_metadata = accounts_metadata - .get(instruction_account.index_in_caller as usize) - .ok_or_else(|| { - ic_msg!( - invoke_context, - "Internal error: index mismatch for account {}", - account_key - ); - Box::new(InstructionError::MissingAccount) - })?; + let serialized_metadata = + accounts_metadata + .get(index_in_caller as usize) + .ok_or_else(|| { + ic_msg!( + invoke_context, + "Internal error: index mismatch for account {}", + account_key + ); + Box::new(InstructionError::MissingAccount) + })?; // build the CallerAccount corresponding to this account. if caller_account_index >= account_infos.len() { @@ -857,7 +858,7 @@ where )?; accounts.push(TranslatedAccount { - index_in_caller: instruction_account.index_in_caller, + index_in_caller, caller_account, update_caller_account_region: instruction_account.is_writable() || update_caller, update_caller_account_info: instruction_account.is_writable(), @@ -1309,7 +1310,6 @@ mod tests { .enumerate() .map(|(index_in_callee, index_in_transaction)| { InstructionAccount::new( - *index_in_transaction as IndexOfAccount, *index_in_transaction as IndexOfAccount, index_in_callee as IndexOfAccount, false, @@ -1855,8 +1855,8 @@ mod tests { .configure( vec![0], vec![ - InstructionAccount::new(1, 0, 0, false, true), - InstructionAccount::new(1, 0, 0, false, true), + InstructionAccount::new(1, 0, false, true), + InstructionAccount::new(1, 0, false, true), ], &[], ); diff --git a/syscalls/src/lib.rs b/syscalls/src/lib.rs index eb52990cac9449..70f32de4ae7d4e 100644 --- a/syscalls/src/lib.rs +++ b/syscalls/src/lib.rs @@ -4448,7 +4448,6 @@ mod tests { { let instruction_accounts = vec![InstructionAccount::new( index_in_trace.saturating_add(1) as IndexOfAccount, - 0, // This is incorrect / inconsistent but not required 0, false, false, diff --git a/transaction-context/src/lib.rs b/transaction-context/src/lib.rs index 74c770d9ac8e33..623f0c845d83ef 100644 --- a/transaction-context/src/lib.rs +++ b/transaction-context/src/lib.rs @@ -60,10 +60,6 @@ pub type IndexOfAccount = u16; pub struct InstructionAccount { /// Points to the account and its key in the `TransactionContext` pub index_in_transaction: IndexOfAccount, - /// Points to the first occurrence in the parent `InstructionContext` - /// - /// This excludes the program accounts. - pub index_in_caller: IndexOfAccount, /// Points to the first occurrence in the current `InstructionContext` /// /// This excludes the program accounts. @@ -77,14 +73,12 @@ pub struct InstructionAccount { impl InstructionAccount { pub fn new( index_in_transaction: IndexOfAccount, - index_in_caller: IndexOfAccount, index_in_callee: IndexOfAccount, is_signer: bool, is_writable: bool, ) -> InstructionAccount { InstructionAccount { index_in_transaction, - index_in_caller, index_in_callee, is_signer: is_signer as u8, is_writable: is_writable as u8, @@ -736,6 +730,18 @@ impl InstructionContext { .index_in_transaction as IndexOfAccount) } + /// Get the index of account in instruction from the index in transaction + pub fn get_index_of_account_in_instruction( + &self, + index_in_transaction: IndexOfAccount, + ) -> Result { + self.instruction_accounts + .iter() + .position(|account| account.index_in_transaction == index_in_transaction) + .map(|idx| idx as IndexOfAccount) + .ok_or(InstructionError::MissingAccount) + } + /// Returns `Some(instruction_account_index)` if this is a duplicate /// and `None` if it is the first account with this key pub fn is_instruction_account_duplicate( @@ -771,7 +777,7 @@ impl InstructionContext { &'a self, transaction_context: &'b TransactionContext, index_in_transaction: IndexOfAccount, - index_in_instruction: IndexOfAccount, + index_in_instruction: Option, ) -> Result, InstructionError> { let account = transaction_context .accounts @@ -783,7 +789,7 @@ impl InstructionContext { transaction_context, instruction_context: self, index_in_transaction, - index_in_instruction, + index_in_instruction_accounts: index_in_instruction, account, }) } @@ -809,11 +815,7 @@ impl InstructionContext { ) -> Result, InstructionError> { let index_in_transaction = self.get_index_of_program_account_in_transaction(program_account_index)?; - self.try_borrow_account( - transaction_context, - index_in_transaction, - program_account_index, - ) + self.try_borrow_account(transaction_context, index_in_transaction, None) } /// Gets an instruction account of this Instruction @@ -827,8 +829,7 @@ impl InstructionContext { self.try_borrow_account( transaction_context, index_in_transaction, - self.get_number_of_program_accounts() - .saturating_add(instruction_account_index), + Some(instruction_account_index), ) } @@ -884,7 +885,8 @@ pub struct BorrowedAccount<'a> { transaction_context: &'a TransactionContext, instruction_context: &'a InstructionContext, index_in_transaction: IndexOfAccount, - index_in_instruction: IndexOfAccount, + // Program accounts are not part of the instruction_accounts vector, and thus None + index_in_instruction_accounts: Option, account: RefMut<'a, AccountSharedData>, } @@ -1195,28 +1197,24 @@ impl BorrowedAccount<'_> { /// Returns whether this account is a signer (instruction wide) pub fn is_signer(&self) -> bool { - if self.index_in_instruction < self.instruction_context.get_number_of_program_accounts() { - return false; + if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts { + self.instruction_context + .is_instruction_account_signer(index_in_instruction_accounts) + .unwrap_or_default() + } else { + false } - self.instruction_context - .is_instruction_account_signer( - self.index_in_instruction - .saturating_sub(self.instruction_context.get_number_of_program_accounts()), - ) - .unwrap_or_default() } /// Returns whether this account is writable (instruction wide) pub fn is_writable(&self) -> bool { - if self.index_in_instruction < self.instruction_context.get_number_of_program_accounts() { - return false; + if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts { + self.instruction_context + .is_instruction_account_writable(index_in_instruction_accounts) + .unwrap_or_default() + } else { + false } - self.instruction_context - .is_instruction_account_writable( - self.index_in_instruction - .saturating_sub(self.instruction_context.get_number_of_program_accounts()), - ) - .unwrap_or_default() } /// Returns true if the owner of this account is the current `InstructionContext`s last program (instruction wide)