From ac73b168e5b6eb8062c2e3b1e966743281f6ee86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 12 Aug 2022 15:57:04 +0200 Subject: [PATCH 1/7] Flattens TransactionContext::instruction_trace. --- programs/bpf_loader/src/syscalls/mod.rs | 36 ++++------ runtime/src/bank.rs | 88 +++++++++++------------ sdk/src/transaction_context.rs | 93 +++++++++---------------- 3 files changed, 87 insertions(+), 130 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 2175a1f2a12..37dfe8c861c 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -1709,32 +1709,20 @@ 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 mut current_index = 0; + let instruction_context = instruction_trace.iter().rev().find(|instruction_context| { + if instruction_context.get_stack_height() == stack_height { + if index.saturating_add(1) == current_index { + return true; + } + current_index = current_index.saturating_add(1); + } + false + }); if let Some(instruction_context) = instruction_context { let ProcessedSiblingInstruction { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7e5aeef1646..e049739f8e0 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,7 +143,7 @@ use { TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS, }, transaction_context::{ - ExecutionRecord, InstructionTrace, TransactionAccount, TransactionContext, + ExecutionRecord, InstructionContext, TransactionAccount, TransactionContext, TransactionReturnData, }, }, @@ -762,40 +762,44 @@ pub type InnerInstructions = Vec; /// a transaction pub type InnerInstructionsList = Vec; -/// Convert from an InstructionTrace to InnerInstructionsList +/// Convert from an &[InstructionContext] to InnerInstructionsList pub fn inner_instructions_list_from_instruction_trace( - instruction_trace: &InstructionTrace, + instruction_trace: &[InstructionContext], ) -> InnerInstructionsList { - instruction_trace - .iter() - .map(|inner_instructions_trace| { - inner_instructions_trace - .iter() - .skip(1) - .map(|instruction_context| { - CompiledInstruction::new_from_raw_parts( + debug_assert!(instruction_trace + .first() + .map(|instruction_context| instruction_context.get_stack_height() + == TRANSACTION_LEVEL_STACK_HEIGHT) + .unwrap_or(true)); + let mut outer_instructions = Vec::new(); + for instruction_context in instruction_trace.iter() { + 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_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(), + .get_number_of_program_accounts() + .saturating_sub(1), ) - }) - .collect() - }) - .collect() + .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); + } + } + outer_instructions } /// A list of log messages emitted during a transaction @@ -18838,17 +18842,13 @@ 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]), - ], + InstructionContext::new(0, 0, &[], &[], &[0]), + InstructionContext::new(1, 0, &[], &[], &[1]), + InstructionContext::new(0, 0, &[], &[], &[2]), + 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); @@ -18856,7 +18856,7 @@ pub(crate) mod tests { 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/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 25e67523335..8c02b2fcd1f 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, @@ -144,22 +142,13 @@ impl TransactionContext { &self, level: usize, ) -> Result<&InstructionContext, InstructionError> { - let top_level_index = *self + let index_in_trace = *self .instruction_stack - .first() + .get(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)) + .get(index_in_trace) .ok_or(InstructionError::CallDepth)?; debug_assert_eq!(instruction_context.nesting_level, level); Ok(instruction_context) @@ -194,48 +183,31 @@ impl TransactionContext { ) -> 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); - } - } - 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); + 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, + )?; + if original_caller_instruction_accounts_lamport_sum + != current_caller_instruction_accounts_lamport_sum + { + return Err(InstructionError::UnbalancedInstruction); } + } + 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(), }; + 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); } @@ -294,7 +266,7 @@ impl TransactionContext { } /// Returns instruction trace - pub fn get_instruction_trace(&self) -> &InstructionTrace { + pub fn get_instruction_trace(&self) -> &[InstructionContext] { &self.instruction_trace } @@ -340,9 +312,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. @@ -912,7 +881,7 @@ 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 instruction_trace: Vec, pub return_data: TransactionReturnData, pub changed_account_count: u64, pub total_size_of_all_accounts: u64, From 185030a567e12fe9185a513274114df513991026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 12 Aug 2022 18:04:47 +0200 Subject: [PATCH 2/7] Stop the search at transaction level. --- programs/bpf_loader/src/syscalls/mod.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 37dfe8c861c..0ef173d63df 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -1714,15 +1714,21 @@ declare_syscall!( let stack_height = invoke_context.get_stack_height(); let instruction_trace = invoke_context.transaction_context.get_instruction_trace(); let mut current_index = 0; - let instruction_context = instruction_trace.iter().rev().find(|instruction_context| { - if instruction_context.get_stack_height() == stack_height { + let mut instruction_context = None; + for current_instruction_context in instruction_trace.iter().rev() { + if current_instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT + && stack_height > TRANSACTION_LEVEL_STACK_HEIGHT + { + break; + } + if current_instruction_context.get_stack_height() == stack_height { if index.saturating_add(1) == current_index { - return true; + instruction_context = Some(current_instruction_context); + break; } current_index = current_index.saturating_add(1); } - false - }); + } if let Some(instruction_context) = instruction_context { let ProcessedSiblingInstruction { From 0d1beb4d62d1d797d9aa2598689842c206a73cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 16 Aug 2022 11:00:19 +0200 Subject: [PATCH 3/7] Renames get_instruction_context_at => get_instruction_context_at_nesting_level. --- program-runtime/src/invoke_context.rs | 2 +- sdk/src/transaction_context.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index b54e6f98756..ca3582c7ae1 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/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 8c02b2fcd1f..ca5cba14db3 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -138,19 +138,19 @@ impl TransactionContext { } /// Gets an InstructionContext by its nesting level in the stack - pub fn get_instruction_context_at( + pub fn get_instruction_context_at_nesting_level( &self, - level: usize, + nesting_level: usize, ) -> Result<&InstructionContext, InstructionError> { let index_in_trace = *self .instruction_stack - .get(level) + .get(nesting_level) .ok_or(InstructionError::CallDepth)?; let instruction_context = self .instruction_trace .get(index_in_trace) .ok_or(InstructionError::CallDepth)?; - debug_assert_eq!(instruction_context.nesting_level, level); + debug_assert_eq!(instruction_context.nesting_level, nesting_level); Ok(instruction_context) } @@ -171,7 +171,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 From 7c9436a6f24cdb01f776e0d888bb40b6ab7ea92e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 16 Aug 2022 11:14:09 +0200 Subject: [PATCH 4/7] Removes TransactionContext::get_instruction_trace(). Adds TransactionContext::get_instruction_trace_length() and TransactionContext::get_instruction_context_at_index(). --- programs/bpf_loader/src/syscalls/mod.rs | 29 ++++++++++++++++--------- runtime/src/message_processor.rs | 2 +- sdk/src/transaction_context.rs | 25 +++++++++++++-------- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 0ef173d63df..2893849a9f0 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -1712,25 +1712,34 @@ declare_syscall!( // 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 mut current_index = 0; - let mut instruction_context = None; - for current_instruction_context in instruction_trace.iter().rev() { - if current_instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT + 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 current_instruction_context.get_stack_height() == stack_height { - if index.saturating_add(1) == current_index { - instruction_context = Some(current_instruction_context); + 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; } - current_index = current_index.saturating_add(1); + 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/message_processor.rs b/runtime/src/message_processor.rs index 23eb1e800e9..c95bfc0b72e 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 ca5cba14db3..a90755ca16a 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -137,6 +137,21 @@ impl TransactionContext { self.account_keys.iter().rposition(|key| key == pubkey) } + /// 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, + index_in_trace: usize, + ) -> Result<&InstructionContext, InstructionError> { + 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, @@ -146,10 +161,7 @@ impl TransactionContext { .instruction_stack .get(nesting_level) .ok_or(InstructionError::CallDepth)?; - let instruction_context = self - .instruction_trace - .get(index_in_trace) - .ok_or(InstructionError::CallDepth)?; + 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) } @@ -265,11 +277,6 @@ impl TransactionContext { Ok(()) } - /// Returns instruction trace - pub fn get_instruction_trace(&self) -> &[InstructionContext] { - &self.instruction_trace - } - /// Calculates the sum of all lamports within an instruction fn instruction_accounts_lamport_sum( &self, From 8d612b9e00ac8c5a9116fc2642b77c422bd69c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 19 Aug 2022 00:18:42 +0200 Subject: [PATCH 5/7] Have TransactionContext::instruction_accounts_lamport_sum() accept an iterator instead of a slice. --- sdk/src/transaction_context.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index a90755ca16a..3d0ac70c73c 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -194,7 +194,7 @@ impl TransactionContext { instruction_data: &[u8], ) -> Result<(), InstructionError> { let callee_instruction_accounts_lamport_sum = - self.instruction_accounts_lamport_sum(instruction_accounts)?; + self.instruction_accounts_lamport_sum(instruction_accounts.iter())?; if !self.instruction_stack.is_empty() && self.is_early_verification_of_account_modifications_enabled() { @@ -203,7 +203,7 @@ impl TransactionContext { caller_instruction_context.instruction_accounts_lamport_sum; let current_caller_instruction_accounts_lamport_sum = self .instruction_accounts_lamport_sum( - &caller_instruction_context.instruction_accounts, + caller_instruction_context.instruction_accounts.iter(), )?; if original_caller_instruction_accounts_lamport_sum != current_caller_instruction_accounts_lamport_sum @@ -244,7 +244,7 @@ impl TransactionContext { .try_borrow_mut() .map_err(|_| InstructionError::AccountBorrowOutstanding)?; } - self.instruction_accounts_lamport_sum(&instruction_context.instruction_accounts) + 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 @@ -278,16 +278,19 @@ impl TransactionContext { } /// 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() + instruction_accounts.enumerate() { if instruction_account_index != instruction_account.index_in_callee { continue; // Skip duplicate account From 76f48432f8906ec0d5309dca22259982c6e60cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 16 Aug 2022 11:58:34 +0200 Subject: [PATCH 6/7] Removes instruction_trace from ExecutionRecord. --- runtime/src/bank.rs | 105 +++++++++++++++++---------------- sdk/src/transaction_context.rs | 39 ++++++------ 2 files changed, 73 insertions(+), 71 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e049739f8e0..214eda92ea8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -143,8 +143,7 @@ use { TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS, }, transaction_context::{ - ExecutionRecord, InstructionContext, TransactionAccount, TransactionContext, - TransactionReturnData, + ExecutionRecord, TransactionAccount, TransactionContext, TransactionReturnData, }, }, solana_stake_program::stake_state::{ @@ -762,39 +761,45 @@ pub type InnerInstructions = Vec; /// a transaction pub type InnerInstructionsList = Vec; -/// Convert from an &[InstructionContext] to InnerInstructionsList +/// Extract the InnerInstructionsList from a TransactionContext pub fn inner_instructions_list_from_instruction_trace( - instruction_trace: &[InstructionContext], + transaction_context: &TransactionContext, ) -> InnerInstructionsList { - debug_assert!(instruction_trace - .first() + debug_assert!(transaction_context + .get_instruction_context_at_index(0) .map(|instruction_context| instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT) .unwrap_or(true)); let mut outer_instructions = Vec::new(); - for instruction_context in instruction_trace.iter() { - 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(), - )); + for index_in_trace in 0..transaction_context.get_instruction_trace_length() { + if let Ok(instruction_context) = + transaction_context.get_instruction_context_at_index(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); } @@ -4434,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, @@ -4464,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 +8053,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,17 +18844,19 @@ pub(crate) mod tests { #[test] fn test_inner_instructions_list_from_instruction_trace() { - let instruction_trace = vec![ - InstructionContext::new(0, 0, &[], &[], &[0]), - InstructionContext::new(1, 0, &[], &[], &[1]), - InstructionContext::new(0, 0, &[], &[], &[2]), - 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, diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 3d0ac70c73c..a47197c6cce 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -233,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.iter()) + 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? { @@ -289,9 +290,7 @@ impl TransactionContext { return Ok(0); } let mut instruction_accounts_lamport_sum: u128 = 0; - for (instruction_account_index, instruction_account) in - instruction_accounts.enumerate() - { + for (instruction_account_index, instruction_account) in instruction_accounts.enumerate() { if instruction_account_index != instruction_account.index_in_callee { continue; // Skip duplicate account } @@ -891,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: Vec, pub return_data: TransactionReturnData, pub changed_account_count: u64, pub total_size_of_all_accounts: u64, @@ -934,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, From f51dab842a51bfd61b716af830d87eded00887ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 19 Aug 2022 00:23:00 +0200 Subject: [PATCH 7/7] make InstructionContext::new() private --- programs/bpf_loader/src/serialization.rs | 19 ++++++++-------- runtime/src/bank.rs | 4 ++-- sdk/src/transaction_context.rs | 28 ++++++++++++------------ 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 6c046b2c0f2..cb022269534 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 instruction_data = vec![]; - let transaction_context = + let mut 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, - ); + 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/runtime/src/bank.rs b/runtime/src/bank.rs index 214eda92ea8..94e4fdea4ca 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -766,14 +766,14 @@ pub fn inner_instructions_list_from_instruction_trace( transaction_context: &TransactionContext, ) -> InnerInstructionsList { debug_assert!(transaction_context - .get_instruction_context_at_index(0) + .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(index_in_trace) + 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()); diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index a47197c6cce..d22a91372b7 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -211,13 +211,13 @@ impl TransactionContext { return Err(InstructionError::UnbalancedInstruction); } } - 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(), - }; + 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 { @@ -335,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, } }