diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index b54e6f987..ca3582c7a 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -325,7 +325,7 @@ impl<'a> InvokeContext<'a> { .get_instruction_context_stack_height()) .any(|level| { self.transaction_context - .get_instruction_context_at(level) + .get_instruction_context_at_nesting_level(level) .and_then(|instruction_context| { instruction_context .try_borrow_last_program_account(self.transaction_context) diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 6c046b2c0..cb0222695 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -497,21 +497,20 @@ mod tests { &program_indices, ) .instruction_accounts; - - let transaction_context = - TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); let instruction_data = vec![]; - let instruction_context = InstructionContext::new( - 0, - 0, - &program_indices, - &instruction_accounts, - &instruction_data, - ); + + let mut transaction_context = + TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); + transaction_context + .push(&program_indices, &instruction_accounts, &instruction_data) + .unwrap(); + let instruction_context = transaction_context + .get_instruction_context_at_index_in_trace(0) + .unwrap(); let serialization_result = serialize_parameters( &transaction_context, - &instruction_context, + instruction_context, should_cap_ix_accounts, ); assert_eq!( diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 2175a1f2a..2893849a9 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -1709,34 +1709,37 @@ declare_syscall!( result ); + // Reverse iterate through the instruction trace, + // ignoring anything except instructions on the same level let stack_height = invoke_context.get_stack_height(); - let instruction_trace = invoke_context.transaction_context.get_instruction_trace(); - let instruction_context = if stack_height == TRANSACTION_LEVEL_STACK_HEIGHT { - // pick one of the top-level instructions - instruction_trace - .len() - .checked_sub(2) - .and_then(|result| result.checked_sub(index as usize)) - .and_then(|index| instruction_trace.get(index)) - .and_then(|instruction_list| instruction_list.first()) - } else { - // Walk the last list of inner instructions - instruction_trace.last().and_then(|inners| { - let mut current_index = 0; - inners.iter().rev().skip(1).find(|instruction_context| { - if stack_height == instruction_context.get_stack_height() { - if index == current_index { - return true; - } else { - current_index = current_index.saturating_add(1); - } - } - false - }) - }) - }; + let instruction_trace_length = invoke_context + .transaction_context + .get_instruction_trace_length(); + let mut reverse_index_at_stack_height = 0; + let mut found_instruction_context = None; + for index_in_trace in (0..instruction_trace_length).rev() { + let instruction_context = question_mark!( + invoke_context + .transaction_context + .get_instruction_context_at_index_in_trace(index_in_trace) + .map_err(SyscallError::InstructionError), + result + ); + if instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT + && stack_height > TRANSACTION_LEVEL_STACK_HEIGHT + { + break; + } + if instruction_context.get_stack_height() == stack_height { + if index.saturating_add(1) == reverse_index_at_stack_height { + found_instruction_context = Some(instruction_context); + break; + } + reverse_index_at_stack_height = reverse_index_at_stack_height.saturating_add(1); + } + } - if let Some(instruction_context) = instruction_context { + if let Some(instruction_context) = found_instruction_context { let ProcessedSiblingInstruction { data_len, accounts_len, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index a6bbb8044..a8b47afae 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -120,7 +120,7 @@ use { hash::{extend_and_hash, hashv, Hash}, incinerator, inflation::Inflation, - instruction::CompiledInstruction, + instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, lamports::LamportsError, message::{AccountKeys, SanitizedMessage}, native_loader, @@ -143,8 +143,7 @@ use { TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS, }, transaction_context::{ - ExecutionRecord, InstructionTrace, TransactionAccount, TransactionContext, - TransactionReturnData, + ExecutionRecord, TransactionAccount, TransactionContext, TransactionReturnData, }, }, solana_stake_program::stake_state::{ @@ -762,40 +761,50 @@ pub type InnerInstructions = Vec; /// a transaction pub type InnerInstructionsList = Vec; -/// Convert from an InstructionTrace to InnerInstructionsList +/// Extract the InnerInstructionsList from a TransactionContext pub fn inner_instructions_list_from_instruction_trace( - instruction_trace: &InstructionTrace, + transaction_context: &TransactionContext, ) -> InnerInstructionsList { - instruction_trace - .iter() - .map(|inner_instructions_trace| { - inner_instructions_trace - .iter() - .skip(1) - .map(|instruction_context| { - CompiledInstruction::new_from_raw_parts( - instruction_context - .get_index_of_program_account_in_transaction( - instruction_context - .get_number_of_program_accounts() - .saturating_sub(1), - ) - .unwrap_or_default() as u8, - instruction_context.get_instruction_data().to_vec(), - (0..instruction_context.get_number_of_instruction_accounts()) - .map(|instruction_account_index| { - instruction_context - .get_index_of_instruction_account_in_transaction( - instruction_account_index, - ) - .unwrap_or_default() as u8 - }) - .collect(), - ) - }) - .collect() - }) - .collect() + debug_assert!(transaction_context + .get_instruction_context_at_index_in_trace(0) + .map(|instruction_context| instruction_context.get_stack_height() + == TRANSACTION_LEVEL_STACK_HEIGHT) + .unwrap_or(true)); + let mut outer_instructions = Vec::new(); + for index_in_trace in 0..transaction_context.get_instruction_trace_length() { + if let Ok(instruction_context) = + transaction_context.get_instruction_context_at_index_in_trace(index_in_trace) + { + if instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT { + outer_instructions.push(Vec::new()); + } else if let Some(inner_instructions) = outer_instructions.last_mut() { + inner_instructions.push(CompiledInstruction::new_from_raw_parts( + instruction_context + .get_index_of_program_account_in_transaction( + instruction_context + .get_number_of_program_accounts() + .saturating_sub(1), + ) + .unwrap_or_default() as u8, + instruction_context.get_instruction_data().to_vec(), + (0..instruction_context.get_number_of_instruction_accounts()) + .map(|instruction_account_index| { + instruction_context + .get_index_of_instruction_account_in_transaction( + instruction_account_index, + ) + .unwrap_or_default() as u8 + }) + .collect(), + )); + } else { + debug_assert!(false); + } + } else { + debug_assert!(false); + } + } + outer_instructions } /// A list of log messages emitted during a transaction @@ -4430,9 +4439,16 @@ impl Bank { .ok() }); + let inner_instructions = if enable_cpi_recording { + Some(inner_instructions_list_from_instruction_trace( + &transaction_context, + )) + } else { + None + }; + let ExecutionRecord { accounts, - instruction_trace, mut return_data, changed_account_count, total_size_of_all_accounts, @@ -4460,14 +4476,6 @@ impl Bank { accounts_data_len_delta = status.as_ref().map_or(0, |_| accounts_resize_delta); } - let inner_instructions = if enable_cpi_recording { - Some(inner_instructions_list_from_instruction_trace( - &instruction_trace, - )) - } else { - None - }; - let return_data = if enable_return_data_recording { if let Some(end_index) = return_data.data.iter().rposition(|&x| x != 0) { let end_index = end_index.saturating_add(1); @@ -8049,7 +8057,6 @@ pub(crate) mod tests { system_program, timing::duration_as_s, transaction::MAX_TX_ACCOUNT_LOCKS, - transaction_context::InstructionContext, }, solana_vote_program::{ vote_instruction, @@ -18841,26 +18848,24 @@ pub(crate) mod tests { #[test] fn test_inner_instructions_list_from_instruction_trace() { - let instruction_trace = vec![ - vec![ - InstructionContext::new(0, 0, &[], &[], &[1]), - InstructionContext::new(1, 0, &[], &[], &[2]), - ], - vec![], - vec![ - InstructionContext::new(0, 0, &[], &[], &[3]), - InstructionContext::new(1, 0, &[], &[], &[4]), - InstructionContext::new(2, 0, &[], &[], &[5]), - InstructionContext::new(1, 0, &[], &[], &[6]), - ], - ]; - - let inner_instructions = inner_instructions_list_from_instruction_trace(&instruction_trace); + let mut transaction_context = TransactionContext::new(vec![], None, 3, 3); + for (index_in_trace, stack_height) in [1, 2, 1, 1, 2, 3, 2].into_iter().enumerate() { + while stack_height <= transaction_context.get_instruction_context_stack_height() { + transaction_context.pop().unwrap(); + } + if stack_height > transaction_context.get_instruction_context_stack_height() { + transaction_context + .push(&[], &[], &[index_in_trace as u8]) + .unwrap(); + } + } + let inner_instructions = + inner_instructions_list_from_instruction_trace(&transaction_context); assert_eq!( inner_instructions, vec![ - vec![CompiledInstruction::new_from_raw_parts(0, vec![2], vec![])], + vec![CompiledInstruction::new_from_raw_parts(0, vec![1], vec![])], vec![], vec![ CompiledInstruction::new_from_raw_parts(0, vec![4], vec![]), diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 23eb1e800..c95bfc0b7 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -680,6 +680,6 @@ mod tests { InstructionError::Custom(0xbabb1e) )) ); - assert_eq!(transaction_context.get_instruction_trace().len(), 2); + assert_eq!(transaction_context.get_instruction_trace_length(), 2); } } diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 25e675233..d22a91372 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -48,8 +48,7 @@ pub struct TransactionContext { account_touched_flags: RefCell>>, instruction_context_capacity: usize, instruction_stack: Vec, - number_of_instructions_at_transaction_level: usize, - instruction_trace: InstructionTrace, + instruction_trace: Vec, return_data: TransactionReturnData, accounts_resize_delta: RefCell, rent: Option, @@ -61,7 +60,7 @@ impl TransactionContext { transaction_accounts: Vec, rent: Option, instruction_context_capacity: usize, - number_of_instructions_at_transaction_level: usize, + _number_of_instructions_at_transaction_level: usize, ) -> Self { let (account_keys, accounts): (Vec, Vec>) = transaction_accounts @@ -75,8 +74,7 @@ impl TransactionContext { account_touched_flags: RefCell::new(Pin::new(account_touched_flags.into_boxed_slice())), instruction_context_capacity, instruction_stack: Vec::with_capacity(instruction_context_capacity), - number_of_instructions_at_transaction_level, - instruction_trace: Vec::with_capacity(number_of_instructions_at_transaction_level), + instruction_trace: Vec::new(), return_data: TransactionReturnData::default(), accounts_resize_delta: RefCell::new(0), rent, @@ -139,29 +137,32 @@ impl TransactionContext { self.account_keys.iter().rposition(|key| key == pubkey) } - /// Gets an InstructionContext by its nesting level in the stack - pub fn get_instruction_context_at( + /// Returns instruction trace length + pub fn get_instruction_trace_length(&self) -> usize { + self.instruction_trace.len() + } + + /// Gets an InstructionContext by its index in the trace + pub fn get_instruction_context_at_index_in_trace( &self, - level: usize, + index_in_trace: usize, ) -> Result<&InstructionContext, InstructionError> { - let top_level_index = *self + self.instruction_trace + .get(index_in_trace) + .ok_or(InstructionError::CallDepth) + } + + /// Gets an InstructionContext by its nesting level in the stack + pub fn get_instruction_context_at_nesting_level( + &self, + nesting_level: usize, + ) -> Result<&InstructionContext, InstructionError> { + let index_in_trace = *self .instruction_stack - .first() + .get(nesting_level) .ok_or(InstructionError::CallDepth)?; - let cpi_index = if level == 0 { - 0 - } else { - *self - .instruction_stack - .get(level) - .ok_or(InstructionError::CallDepth)? - }; - let instruction_context = self - .instruction_trace - .get(top_level_index) - .and_then(|instruction_trace| instruction_trace.get(cpi_index)) - .ok_or(InstructionError::CallDepth)?; - debug_assert_eq!(instruction_context.nesting_level, level); + let instruction_context = self.get_instruction_context_at_index_in_trace(index_in_trace)?; + debug_assert_eq!(instruction_context.nesting_level, nesting_level); Ok(instruction_context) } @@ -182,7 +183,7 @@ impl TransactionContext { .get_instruction_context_stack_height() .checked_sub(1) .ok_or(InstructionError::CallDepth)?; - self.get_instruction_context_at(level) + self.get_instruction_context_at_nesting_level(level) } /// Pushes a new InstructionContext @@ -193,49 +194,32 @@ impl TransactionContext { instruction_data: &[u8], ) -> Result<(), InstructionError> { let callee_instruction_accounts_lamport_sum = - self.instruction_accounts_lamport_sum(instruction_accounts)?; - let index_in_trace = if self.instruction_stack.is_empty() { - debug_assert!( - self.instruction_trace.len() < self.number_of_instructions_at_transaction_level - ); - let instruction_context = InstructionContext { - nesting_level: self.instruction_stack.len(), - instruction_accounts_lamport_sum: callee_instruction_accounts_lamport_sum, - program_accounts: program_accounts.to_vec(), - instruction_accounts: instruction_accounts.to_vec(), - instruction_data: instruction_data.to_vec(), - }; - self.instruction_trace.push(vec![instruction_context]); - self.instruction_trace.len().saturating_sub(1) - } else { - if self.is_early_verification_of_account_modifications_enabled() { - let caller_instruction_context = self.get_current_instruction_context()?; - let original_caller_instruction_accounts_lamport_sum = - caller_instruction_context.instruction_accounts_lamport_sum; - let current_caller_instruction_accounts_lamport_sum = self - .instruction_accounts_lamport_sum( - &caller_instruction_context.instruction_accounts, - )?; - if original_caller_instruction_accounts_lamport_sum - != current_caller_instruction_accounts_lamport_sum - { - return Err(InstructionError::UnbalancedInstruction); - } + self.instruction_accounts_lamport_sum(instruction_accounts.iter())?; + if !self.instruction_stack.is_empty() + && self.is_early_verification_of_account_modifications_enabled() + { + let caller_instruction_context = self.get_current_instruction_context()?; + let original_caller_instruction_accounts_lamport_sum = + caller_instruction_context.instruction_accounts_lamport_sum; + let current_caller_instruction_accounts_lamport_sum = self + .instruction_accounts_lamport_sum( + caller_instruction_context.instruction_accounts.iter(), + )?; + if original_caller_instruction_accounts_lamport_sum + != current_caller_instruction_accounts_lamport_sum + { + return Err(InstructionError::UnbalancedInstruction); } - if let Some(instruction_trace) = self.instruction_trace.last_mut() { - let instruction_context = InstructionContext { - nesting_level: self.instruction_stack.len(), - instruction_accounts_lamport_sum: callee_instruction_accounts_lamport_sum, - program_accounts: program_accounts.to_vec(), - instruction_accounts: instruction_accounts.to_vec(), - instruction_data: instruction_data.to_vec(), - }; - instruction_trace.push(instruction_context); - instruction_trace.len().saturating_sub(1) - } else { - return Err(InstructionError::CallDepth); - } - }; + } + let instruction_context = InstructionContext::new( + self.instruction_stack.len(), + callee_instruction_accounts_lamport_sum, + program_accounts.to_vec(), + instruction_accounts.to_vec(), + instruction_data.to_vec(), + ); + let index_in_trace = self.instruction_trace.len(); + self.instruction_trace.push(instruction_context); if self.instruction_stack.len() >= self.instruction_context_capacity { return Err(InstructionError::CallDepth); } @@ -249,26 +233,27 @@ impl TransactionContext { return Err(InstructionError::CallDepth); } // Verify (before we pop) that the total sum of all lamports in this instruction did not change - let detected_an_unbalanced_instruction = if self - .is_early_verification_of_account_modifications_enabled() - { - self.get_current_instruction_context() - .and_then(|instruction_context| { - // Verify all executable accounts have no outstanding refs - for account_index in instruction_context.program_accounts.iter() { - self.get_account_at_index(*account_index)? - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - self.instruction_accounts_lamport_sum(&instruction_context.instruction_accounts) + let detected_an_unbalanced_instruction = + if self.is_early_verification_of_account_modifications_enabled() { + self.get_current_instruction_context() + .and_then(|instruction_context| { + // Verify all executable accounts have no outstanding refs + for account_index in instruction_context.program_accounts.iter() { + self.get_account_at_index(*account_index)? + .try_borrow_mut() + .map_err(|_| InstructionError::AccountBorrowOutstanding)?; + } + self.instruction_accounts_lamport_sum( + instruction_context.instruction_accounts.iter(), + ) .map(|instruction_accounts_lamport_sum| { instruction_context.instruction_accounts_lamport_sum != instruction_accounts_lamport_sum }) - }) - } else { - Ok(false) - }; + }) + } else { + Ok(false) + }; // Always pop, even if we `detected_an_unbalanced_instruction` self.instruction_stack.pop(); if detected_an_unbalanced_instruction? { @@ -293,23 +278,19 @@ impl TransactionContext { Ok(()) } - /// Returns instruction trace - pub fn get_instruction_trace(&self) -> &InstructionTrace { - &self.instruction_trace - } - /// Calculates the sum of all lamports within an instruction - fn instruction_accounts_lamport_sum( - &self, - instruction_accounts: &[InstructionAccount], - ) -> Result { + fn instruction_accounts_lamport_sum<'a, I>( + &'a self, + instruction_accounts: I, + ) -> Result + where + I: Iterator, + { if !self.is_early_verification_of_account_modifications_enabled() { return Ok(0); } let mut instruction_accounts_lamport_sum: u128 = 0; - for (instruction_account_index, instruction_account) in - instruction_accounts.iter().enumerate() - { + for (instruction_account_index, instruction_account) in instruction_accounts.enumerate() { if instruction_account_index != instruction_account.index_in_callee { continue; // Skip duplicate account } @@ -340,9 +321,6 @@ pub struct TransactionReturnData { pub data: Vec, } -/// List of (stack height, instruction) for each top-level instruction -pub type InstructionTrace = Vec>; - /// Loaded instruction shared between runtime and programs. /// /// This context is valid for the entire duration of a (possibly cross program) instruction being processed. @@ -357,19 +335,19 @@ pub struct InstructionContext { impl InstructionContext { /// New - pub fn new( + fn new( nesting_level: usize, instruction_accounts_lamport_sum: u128, - program_accounts: &[usize], - instruction_accounts: &[InstructionAccount], - instruction_data: &[u8], + program_accounts: Vec, + instruction_accounts: Vec, + instruction_data: Vec, ) -> Self { InstructionContext { nesting_level, instruction_accounts_lamport_sum, - program_accounts: program_accounts.to_vec(), - instruction_accounts: instruction_accounts.to_vec(), - instruction_data: instruction_data.to_vec(), + program_accounts, + instruction_accounts, + instruction_data, } } @@ -912,7 +890,6 @@ impl<'a> BorrowedAccount<'a> { /// Everything that needs to be recorded from a TransactionContext after execution pub struct ExecutionRecord { pub accounts: Vec, - pub instruction_trace: InstructionTrace, pub return_data: TransactionReturnData, pub changed_account_count: u64, pub total_size_of_all_accounts: u64, @@ -955,7 +932,6 @@ impl From for ExecutionRecord { .map(|account| account.into_inner()), ) .collect(), - instruction_trace: context.instruction_trace, return_data: context.return_data, changed_account_count, total_size_of_all_accounts,