diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index 9efdac1567abe3..7e0d14a6a69302 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -409,13 +409,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { pubkey, AccountSharedData::new(0, allocation_size, &Pubkey::new_unique()), )); - instruction_accounts.push(InstructionAccount { - index_in_transaction: 0, - index_in_caller: 0, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }); + instruction_accounts.push(InstructionAccount::new(0, 0, 0, false, true)); vec![] } Err(_) => { @@ -486,13 +480,13 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { transaction_accounts.push((pubkey, account)); idx }; - InstructionAccount { - index_in_transaction: txn_acct_index as IndexOfAccount, - index_in_caller: txn_acct_index as IndexOfAccount, - index_in_callee: txn_acct_index as IndexOfAccount, - is_signer: account_info.is_signer.unwrap_or(false), - is_writable: account_info.is_writable.unwrap_or(false), - } + InstructionAccount::new( + txn_acct_index as IndexOfAccount, + txn_acct_index as IndexOfAccount, + txn_acct_index as IndexOfAccount, + account_info.is_signer.unwrap_or(false), + account_info.is_writable.unwrap_or(false), + ) }) .collect(); input.instruction_data diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index ce577e23c7964d..70a8f3de580225 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -357,8 +357,10 @@ impl<'a> InvokeContext<'a> { let instruction_account = deduplicated_instruction_accounts .get_mut(duplicate_index) .ok_or(InstructionError::NotEnoughAccountKeys)?; - instruction_account.is_signer |= account_meta.is_signer; - instruction_account.is_writable |= account_meta.is_writable; + instruction_account + .set_is_signer(instruction_account.is_signer() || account_meta.is_signer); + instruction_account + .set_is_writable(instruction_account.is_writable() || account_meta.is_writable); } else { let index_in_caller = instruction_context .find_index_of_instruction_account( @@ -374,13 +376,13 @@ impl<'a> InvokeContext<'a> { InstructionError::MissingAccount })?; duplicate_indicies.push(deduplicated_instruction_accounts.len()); - deduplicated_instruction_accounts.push(InstructionAccount { + deduplicated_instruction_accounts.push(InstructionAccount::new( index_in_transaction, index_in_caller, - index_in_callee: instruction_account_index as IndexOfAccount, - is_signer: account_meta.is_signer, - is_writable: account_meta.is_writable, - }); + instruction_account_index as IndexOfAccount, + account_meta.is_signer, + account_meta.is_writable, + )); } } for instruction_account in deduplicated_instruction_accounts.iter() { @@ -390,7 +392,7 @@ impl<'a> InvokeContext<'a> { )?; // Readonly in caller cannot become writable in callee - if instruction_account.is_writable && !borrowed_account.is_writable() { + if instruction_account.is_writable() && !borrowed_account.is_writable() { ic_msg!( self, "{}'s writable privilege escalated", @@ -401,7 +403,7 @@ impl<'a> InvokeContext<'a> { // To be signed in the callee, // it must be either signed in the caller or by the program - if instruction_account.is_signer + if instruction_account.is_signer() && !(borrowed_account.is_signer() || signers.contains(borrowed_account.get_key())) { ic_msg!( @@ -838,13 +840,13 @@ pub fn mock_process_instruction_with_feature_set< instruction_account.index_in_transaction == index_in_transaction }) .unwrap_or(instruction_account_index) as IndexOfAccount; - instruction_accounts.push(InstructionAccount { + instruction_accounts.push(InstructionAccount::new( + index_in_transaction, index_in_transaction, - index_in_caller: index_in_transaction, index_in_callee, - is_signer: account_meta.is_signer, - is_writable: account_meta.is_writable, - }); + account_meta.is_signer, + account_meta.is_writable, + )); } if program_indices.is_empty() { program_indices.insert(0, transaction_accounts.len() as IndexOfAccount); @@ -959,12 +961,14 @@ mod tests { let instruction_data = instruction_context.get_instruction_data(); let program_id = instruction_context.get_last_program_key(transaction_context)?; let instruction_accounts = (0..4) - .map(|instruction_account_index| InstructionAccount { - index_in_transaction: instruction_account_index, - index_in_caller: instruction_account_index, - index_in_callee: instruction_account_index, - is_signer: false, - is_writable: false, + .map(|instruction_account_index| { + InstructionAccount::new( + instruction_account_index, + instruction_account_index, + instruction_account_index, + false, + false, + ) }) .collect::>(); assert_eq!( @@ -1064,26 +1068,26 @@ mod tests { solana_pubkey::new_rand(), AccountSharedData::new(index as u64, 1, invoke_stack.get(index).unwrap()), )); - instruction_accounts.push(InstructionAccount { - index_in_transaction: index as IndexOfAccount, - index_in_caller: index as IndexOfAccount, - index_in_callee: instruction_accounts.len() as IndexOfAccount, - is_signer: false, - is_writable: true, - }); + instruction_accounts.push(InstructionAccount::new( + index as IndexOfAccount, + index as IndexOfAccount, + instruction_accounts.len() as IndexOfAccount, + false, + true, + )); } for (index, program_id) in invoke_stack.iter().enumerate() { transaction_accounts.push(( *program_id, AccountSharedData::new(1, 1, &solana_pubkey::Pubkey::default()), )); - instruction_accounts.push(InstructionAccount { - index_in_transaction: index as IndexOfAccount, - index_in_caller: index as IndexOfAccount, - index_in_callee: index as IndexOfAccount, - is_signer: false, - is_writable: false, - }); + instruction_accounts.push(InstructionAccount::new( + index as IndexOfAccount, + index as IndexOfAccount, + index as IndexOfAccount, + false, + false, + )); } with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); @@ -1154,12 +1158,14 @@ mod tests { AccountMeta::new_readonly(transaction_accounts.get(2).unwrap().0, false), ]; let instruction_accounts = (0..4) - .map(|instruction_account_index| InstructionAccount { - index_in_transaction: instruction_account_index, - index_in_caller: instruction_account_index, - index_in_callee: instruction_account_index, - is_signer: false, - is_writable: instruction_account_index < 2, + .map(|instruction_account_index| { + InstructionAccount::new( + instruction_account_index, + instruction_account_index, + instruction_account_index, + false, + instruction_account_index < 2, + ) }) .collect::>(); with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); @@ -1210,12 +1216,14 @@ mod tests { AccountMeta::new_readonly(transaction_accounts.get(2).unwrap().0, false), ]; let instruction_accounts = (0..4) - .map(|instruction_account_index| InstructionAccount { - index_in_transaction: instruction_account_index, - index_in_caller: instruction_account_index, - index_in_callee: instruction_account_index, - is_signer: false, - is_writable: instruction_account_index < 2, + .map(|instruction_account_index| { + InstructionAccount::new( + instruction_account_index, + instruction_account_index, + instruction_account_index, + false, + instruction_account_index < 2, + ) }) .collect::>(); with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts); @@ -1307,20 +1315,8 @@ mod tests { (program_key, program_account), ]; let instruction_accounts = [ - InstructionAccount { - index_in_transaction: 0, - index_in_caller: 0, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }, - InstructionAccount { - index_in_transaction: 1, - index_in_caller: 1, - index_in_callee: 1, - is_signer: false, - is_writable: false, - }, + InstructionAccount::new(0, 0, 0, false, true), + InstructionAccount::new(1, 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 563b37ad17ff2e..1d8544f84ba0a6 100644 --- a/program-runtime/src/serialization.rs +++ b/program-runtime/src/serialization.rs @@ -628,13 +628,13 @@ mod tests { .iter() .position(|account_index| account_index == index_in_transaction) .unwrap_or(index_in_instruction); - InstructionAccount { - index_in_transaction: *index_in_transaction, - index_in_caller: *index_in_transaction, - index_in_callee: index_in_callee as IndexOfAccount, - is_signer: false, - is_writable: is_writable(index_in_instruction), - } + InstructionAccount::new( + *index_in_transaction, + *index_in_transaction, + index_in_callee as IndexOfAccount, + false, + is_writable(index_in_instruction), + ) }) .collect() } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 9ca3c158036ce5..6d74e8abb40961 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -324,7 +324,7 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs { .set_owner(account_info.owner.as_ref()) .unwrap(); } - if instruction_account.is_writable { + if instruction_account.is_writable() { account_indices.push((instruction_account.index_in_caller, account_info_index)); } } diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index f2d25fffdab729..218a4e3512d967 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -93,13 +93,13 @@ fn create_inputs(owner: Pubkey, num_instruction_accounts: usize) -> TransactionC .iter() .position(|account| account.index_in_transaction == index_in_transaction) .unwrap_or(instruction_account_index) as IndexOfAccount; - instruction_accounts.push(InstructionAccount { - index_in_caller: instruction_account_index as IndexOfAccount, + instruction_accounts.push(InstructionAccount::new( + instruction_account_index as IndexOfAccount, index_in_transaction, index_in_callee, - is_signer: false, - is_writable: instruction_account_index >= 4, - }); + false, + instruction_account_index >= 4, + )); } let mut transaction_context = diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index f5ed17c877380e..3142ab7fc8d4ed 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -875,8 +875,8 @@ where accounts.push(TranslatedAccount { index_in_caller: instruction_account.index_in_caller, caller_account, - update_caller_account_region: instruction_account.is_writable || update_caller, - update_caller_account_info: instruction_account.is_writable, + update_caller_account_region: instruction_account.is_writable() || update_caller, + update_caller_account_info: instruction_account.is_writable(), }); } else { ic_msg!( @@ -1324,15 +1324,15 @@ mod tests { let instruction_accounts = $instruction_accounts .iter() .enumerate() - .map( - |(index_in_callee, index_in_transaction)| InstructionAccount { - index_in_transaction: *index_in_transaction as IndexOfAccount, - index_in_caller: *index_in_transaction as IndexOfAccount, - index_in_callee: index_in_callee as IndexOfAccount, - is_signer: false, - is_writable: $transaction_accounts[*index_in_transaction as usize].2, - }, - ) + .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, + $transaction_accounts[*index_in_transaction as usize].2, + ) + }) .collect::>(); let transaction_accounts = $transaction_accounts .into_iter() @@ -1879,20 +1879,8 @@ mod tests { let accounts = SyscallInvokeSignedRust::translate_accounts( &[ - InstructionAccount { - index_in_transaction: 1, - index_in_caller: 0, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }, - InstructionAccount { - index_in_transaction: 1, - index_in_caller: 0, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }, + InstructionAccount::new(1, 0, 0, false, true), + InstructionAccount::new(1, 0, 0, false, true), ], vm_addr, 1, diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 5f5490d90b83fd..dd7aeec805f5d5 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -4438,13 +4438,13 @@ mod tests { .transaction_context .get_instruction_context_stack_height() { - let instruction_accounts = [InstructionAccount { - index_in_transaction: index_in_trace.saturating_add(1) as IndexOfAccount, - index_in_caller: 0, // This is incorrect / inconsistent but not required - index_in_callee: 0, - is_signer: false, - is_writable: false, - }]; + let instruction_accounts = [InstructionAccount::new( + index_in_trace.saturating_add(1) as IndexOfAccount, + 0, // This is incorrect / inconsistent but not required + 0, + false, + false, + )]; invoke_context .transaction_context .get_next_instruction_context() diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index 8b0a7a48237da9..8a11eabe911b8c 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -60,13 +60,7 @@ macro_rules! with_mock_invoke_context { AccountSharedData::new(2, $account_size, &program_key), ), ]; - let instruction_accounts = vec![InstructionAccount { - index_in_transaction: 2, - index_in_caller: 2, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }]; + let instruction_accounts = vec![InstructionAccount::new(2, 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 9b41013c5bfd82..3f9753e6fa7063 100644 --- a/programs/system/src/system_instruction.rs +++ b/programs/system/src/system_instruction.rs @@ -294,20 +294,8 @@ mod test { (system_program::id(), AccountSharedData::default()), ]; let $instruction_accounts = vec![ - InstructionAccount { - index_in_transaction: 0, - index_in_caller: 0, - index_in_callee: 0, - is_signer: true, - is_writable: true, - }, - InstructionAccount { - index_in_transaction: 1, - index_in_caller: 1, - index_in_callee: 1, - is_signer: false, - is_writable: true, - }, + InstructionAccount::new(0, 0, 0, true, true), + InstructionAccount::new(1, 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 2616dbe6d1d43a..1f0efcacd7bf21 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1158,17 +1158,7 @@ mod tests { 0, ); let mut instruction_context = InstructionContext::default(); - instruction_context.configure( - &[0], - &[InstructionAccount { - index_in_transaction: 1, - index_in_caller: 1, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }], - &[], - ); + instruction_context.configure(&[0], &[InstructionAccount::new(1, 1, 0, false, true)], &[]); // Get the BorrowedAccount from the InstructionContext which is what is used to manipulate and inspect account // state @@ -1313,17 +1303,7 @@ mod tests { 0, ); let mut instruction_context = InstructionContext::default(); - instruction_context.configure( - &[0], - &[InstructionAccount { - index_in_transaction: 1, - index_in_caller: 1, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }], - &[], - ); + instruction_context.configure(&[0], &[InstructionAccount::new(1, 1, 0, false, true)], &[]); // Get the BorrowedAccount from the InstructionContext which is what is used to manipulate and inspect account // state diff --git a/svm/src/message_processor.rs b/svm/src/message_processor.rs index e236f1302eec57..3e8de728a4d74f 100644 --- a/svm/src/message_processor.rs +++ b/svm/src/message_processor.rs @@ -38,13 +38,13 @@ pub(crate) fn process_message( .unwrap_or(instruction_account_index) as IndexOfAccount; let index_in_transaction = *index_in_transaction as usize; - instruction_accounts.push(InstructionAccount { - index_in_transaction: index_in_transaction as IndexOfAccount, - index_in_caller: index_in_transaction as IndexOfAccount, + instruction_accounts.push(InstructionAccount::new( + index_in_transaction as IndexOfAccount, + index_in_transaction as IndexOfAccount, index_in_callee, - is_signer: message.is_signer(index_in_transaction), - is_writable: message.is_writable(index_in_transaction), - }); + message.is_signer(index_in_transaction), + message.is_writable(index_in_transaction), + )); } let mut compute_units_consumed = 0; diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index e1484c1d4fd8c6..550af99904d6f9 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -383,13 +383,13 @@ fn execute_fixture_as_instr( .position(|idx| *idx == *index_txn) .unwrap_or(instruction_acct_idx); - instruction_accounts.push(InstructionAccount { - index_in_transaction: *index_txn as IndexOfAccount, - index_in_caller: *index_txn as IndexOfAccount, - index_in_callee: index_in_callee as IndexOfAccount, - is_signer: sanitized_message.is_signer(*index_txn as usize), - is_writable: sanitized_message.is_writable(*index_txn as usize), - }); + 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), + )); } let mut compute_units_consumed = 0u64; diff --git a/transaction-context/src/lib.rs b/transaction-context/src/lib.rs index 1b5368258b4f85..ce475f555ffb19 100644 --- a/transaction-context/src/lib.rs +++ b/transaction-context/src/lib.rs @@ -55,6 +55,7 @@ pub type IndexOfAccount = u16; /// Contains account meta data which varies between instruction. /// /// It also contains indices to other structures for faster lookup. +#[repr(C)] #[derive(Clone, Debug, Eq, PartialEq)] pub struct InstructionAccount { /// Points to the account and its key in the `TransactionContext` @@ -68,9 +69,43 @@ pub struct InstructionAccount { /// This excludes the program accounts. pub index_in_callee: IndexOfAccount, /// Is this account supposed to sign - pub is_signer: bool, + is_signer: u8, /// Is this account allowed to become writable - pub is_writable: bool, + is_writable: u8, +} + +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, + } + } + + pub fn is_signer(&self) -> bool { + self.is_signer != 0 + } + + pub fn is_writable(&self) -> bool { + self.is_writable != 0 + } + + pub fn set_is_signer(&mut self, value: bool) { + self.is_signer = value as u8; + } + + pub fn set_is_writable(&mut self, value: bool) { + self.is_writable = value as u8; + } } /// An account key and the matching account @@ -797,7 +832,7 @@ impl InstructionContext { .instruction_accounts .get(instruction_account_index as usize) .ok_or(InstructionError::MissingAccount)? - .is_signer) + .is_signer()) } /// Returns whether an instruction account is writable @@ -809,7 +844,7 @@ impl InstructionContext { .instruction_accounts .get(instruction_account_index as usize) .ok_or(InstructionError::MissingAccount)? - .is_writable) + .is_writable()) } /// Calculates the set of all keys of signer instruction accounts in this Instruction @@ -819,7 +854,7 @@ impl InstructionContext { ) -> Result, InstructionError> { let mut result = HashSet::new(); for instruction_account in self.instruction_accounts.iter() { - if instruction_account.is_signer { + if instruction_account.is_signer() { result.insert( *transaction_context .get_key_of_account_at_index(instruction_account.index_in_transaction)?,