diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 7fff70ced6..33f4eb69d9 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -15,6 +15,7 @@ use { bpf_loader_upgradeable::{self, UpgradeableLoaderState}, feature_set::{ cap_accounts_data_len, do_support_realloc, neon_evm_compute_budget, + record_instruction_in_transaction_context_push, reject_empty_instruction_without_program, remove_native_loader, requestable_heap_size, tx_wide_compute_cap, FeatureSet, }, @@ -280,10 +281,13 @@ impl<'a> InvokeContext<'a> { program_indices: &[usize], instruction_data: &[u8], ) -> Result<(), InstructionError> { - if self - .transaction_context - .get_instruction_context_stack_height() - > self.compute_budget.max_invoke_depth + if !self + .feature_set + .is_active(&record_instruction_in_transaction_context_push::id()) + && self + .transaction_context + .get_instruction_context_stack_height() + > self.compute_budget.max_invoke_depth { return Err(InstructionError::CallDepth); } @@ -409,8 +413,13 @@ impl<'a> InvokeContext<'a> { std::mem::transmute(keyed_accounts.as_slice()) }), )); - self.transaction_context - .push(program_indices, instruction_accounts, instruction_data) + self.transaction_context.push( + program_indices, + instruction_accounts, + instruction_data, + self.feature_set + .is_active(&record_instruction_in_transaction_context_push::id()), + ) } /// Pop a stack frame from the invocation stack @@ -814,12 +823,10 @@ impl<'a> InvokeContext<'a> { }) .unwrap_or_else(|| Ok(native_loader::id()))?; - let stack_height = self + let nesting_level = self .transaction_context .get_instruction_context_stack_height(); - - let is_top_level_instruction = stack_height == 0; - + let is_top_level_instruction = nesting_level == 0; if !is_top_level_instruction { // Verify the calling program hasn't misbehaved let mut verify_caller_time = Measure::start("verify_caller_time"); @@ -834,10 +841,18 @@ impl<'a> InvokeContext<'a> { ); verify_caller_result?; - self.transaction_context.record_instruction( - stack_height.saturating_add(1), - InstructionContext::new(program_indices, instruction_accounts, instruction_data), - ); + if !self + .feature_set + .is_active(&record_instruction_in_transaction_context_push::id()) + { + self.transaction_context + .record_instruction(InstructionContext::new( + nesting_level, + program_indices, + instruction_accounts, + instruction_data, + )); + } } let result = self @@ -1004,11 +1019,6 @@ impl<'a> InvokeContext<'a> { &self.sysvar_cache } - /// Get instruction trace - pub fn get_instruction_trace(&self) -> &[Vec<(usize, InstructionContext)>] { - self.transaction_context.get_instruction_trace() - } - // Get pubkey of account at index pub fn get_key_of_account_at_index( &self, @@ -1017,13 +1027,6 @@ impl<'a> InvokeContext<'a> { self.transaction_context .get_key_of_account_at_index(index_in_transaction) } - - /// Get an instruction context - pub fn get_instruction_context_at(&self, level: usize) -> Option<&InstructionContext> { - self.transaction_context - .get_instruction_context_at(level) - .ok() - } } pub struct MockInvokeContextPreparation { @@ -1342,7 +1345,8 @@ mod tests { is_writable: false, }); } - let mut transaction_context = TransactionContext::new(accounts, MAX_DEPTH, 1); + let mut transaction_context = + TransactionContext::new(accounts, ComputeBudget::default().max_invoke_depth, 1); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); // Check call depth increases and has a limit diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 703a32bc35..8f2c0e1b28 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -103,7 +103,7 @@ fn create_inputs() -> TransactionContext { let mut transaction_context = TransactionContext::new(transaction_accounts, 1, 1); let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; transaction_context - .push(&[0], &instruction_accounts, &instruction_data) + .push(&[0], &instruction_accounts, &instruction_data, true) .unwrap(); transaction_context } diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index d7f8b9ecba..975e0c2532 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -3037,7 +3037,7 @@ impl<'a, 'b> SyscallObject for SyscallGetProcessedSiblingInstruction<' ); let stack_height = invoke_context.get_stack_height(); - let instruction_trace = invoke_context.get_instruction_trace(); + 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 @@ -3050,8 +3050,8 @@ impl<'a, 'b> SyscallObject for SyscallGetProcessedSiblingInstruction<' // Walk the last list of inner instructions instruction_trace.last().and_then(|inners| { let mut current_index = 0; - inners.iter().rev().skip(1).find(|(this_stack_height, _)| { - if stack_height == *this_stack_height { + inners.iter().rev().skip(1).find(|instruction_context| { + if stack_height == instruction_context.get_stack_height() { if index == current_index { return true; } else { @@ -3061,8 +3061,7 @@ impl<'a, 'b> SyscallObject for SyscallGetProcessedSiblingInstruction<' false }) }) - } - .map(|(_, instruction_context)| instruction_context); + }; if let Some(instruction_context) = instruction_context { let ProcessedSiblingInstruction { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 01ce351511..1def36220d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -702,7 +702,7 @@ pub fn inner_instructions_list_from_instruction_trace( inner_instructions_trace .iter() .skip(1) - .map(|(_, instruction_context)| { + .map(|instruction_context| { CompiledInstruction::new_from_raw_parts( instruction_context.get_program_id_index() as u8, instruction_context.get_instruction_data().to_vec(), @@ -15872,15 +15872,15 @@ pub(crate) mod tests { fn test_inner_instructions_list_from_instruction_trace() { let instruction_trace = vec![ vec![ - (1, InstructionContext::new(&[], &[], &[1])), - (2, InstructionContext::new(&[], &[], &[2])), + InstructionContext::new(0, &[], &[], &[1]), + InstructionContext::new(1, &[], &[], &[2]), ], vec![], vec![ - (1, InstructionContext::new(&[], &[], &[3])), - (2, InstructionContext::new(&[], &[], &[4])), - (3, InstructionContext::new(&[], &[], &[5])), - (2, InstructionContext::new(&[], &[], &[6])), + InstructionContext::new(0, &[], &[], &[3]), + InstructionContext::new(1, &[], &[], &[4]), + InstructionContext::new(2, &[], &[], &[5]), + InstructionContext::new(1, &[], &[], &[6]), ], ]; diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index a1145c6c0f..7ae2607933 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -307,6 +307,10 @@ pub mod disable_bpf_unresolved_symbols_at_runtime { solana_sdk::declare_id!("4yuaYAj2jGMGTh1sSmi4G2eFscsDq8qjugJXZoBN6YEa"); } +pub mod record_instruction_in_transaction_context_push { + solana_sdk::declare_id!("3aJdcZqxoLpSBxgeYGjPwaYS1zzcByxUDqJkbzWAH1Zb"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -378,6 +382,7 @@ lazy_static! { (bank_tranaction_count_fix::id(), "Fixes Bank::transaction_count to include all committed transactions, not just successful ones"), (disable_bpf_deprecated_load_instructions::id(), "Disable ldabs* and ldind* BPF instructions"), (disable_bpf_unresolved_symbols_at_runtime::id(), "Disable reporting of unresolved BPF symbols at runtime"), + (record_instruction_in_transaction_context_push::id(), "Move the CPI stack overflow check to the end of push"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 3afe806468..60934ad9a8 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -2,7 +2,7 @@ use crate::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, - instruction::{InstructionError, TRANSACTION_LEVEL_STACK_HEIGHT}, + instruction::InstructionError, lamports::LamportsError, pubkey::Pubkey, sysvar::Sysvar, @@ -31,7 +31,7 @@ pub struct TransactionContext { account_keys: Pin>, accounts: Pin]>>, instruction_context_capacity: usize, - instruction_context_stack: Vec, + instruction_stack: Vec, number_of_instructions_at_transaction_level: usize, instruction_trace: InstructionTrace, return_data: (Pubkey, Vec), @@ -53,7 +53,7 @@ impl TransactionContext { account_keys: Pin::new(account_keys.into_boxed_slice()), accounts: Pin::new(accounts.into_boxed_slice()), instruction_context_capacity, - instruction_context_stack: Vec::with_capacity(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), return_data: (Pubkey::default(), Vec::new()), @@ -61,12 +61,7 @@ impl TransactionContext { } /// Used by the bank in the runtime to write back the processed accounts and recorded instructions - pub fn deconstruct( - self, - ) -> ( - Vec, - Vec>, - ) { + pub fn deconstruct(self) -> (Vec, Vec>) { ( Vec::from(Pin::into_inner(self.account_keys)) .into_iter() @@ -82,7 +77,7 @@ impl TransactionContext { /// Used in mock_process_instruction pub fn deconstruct_without_keys(self) -> Result, InstructionError> { - if !self.instruction_context_stack.is_empty() { + if !self.instruction_stack.is_empty() { return Err(InstructionError::CallDepth); } Ok(Vec::from(Pin::into_inner(self.accounts)) @@ -137,15 +132,30 @@ impl TransactionContext { self.account_keys.iter().rposition(|key| key == pubkey) } - /// Gets an InstructionContext by its height in the stack + /// Gets an InstructionContext by its nesting level in the stack pub fn get_instruction_context_at( &self, level: usize, ) -> Result<&InstructionContext, InstructionError> { - if level >= self.instruction_context_stack.len() { - return Err(InstructionError::CallDepth); - } - Ok(&self.instruction_context_stack[level]) + let top_level_index = *self + .instruction_stack + .get(0) + .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); + Ok(instruction_context) } /// Gets the max height of the InstructionContext stack @@ -156,14 +166,13 @@ impl TransactionContext { /// Gets instruction stack height, top-level instructions are height /// `solana_sdk::instruction::TRANSACTION_LEVEL_STACK_HEIGHT` pub fn get_instruction_context_stack_height(&self) -> usize { - self.instruction_context_stack.len() + self.instruction_stack.len() } /// Returns the current InstructionContext pub fn get_current_instruction_context(&self) -> Result<&InstructionContext, InstructionError> { let level = self - .instruction_context_stack - .len() + .get_instruction_context_stack_height() .checked_sub(1) .ok_or(InstructionError::CallDepth)?; self.get_instruction_context_at(level) @@ -175,36 +184,47 @@ impl TransactionContext { program_accounts: &[usize], instruction_accounts: &[InstructionAccount], instruction_data: &[u8], + record_instruction_in_transaction_context_push: bool, ) -> Result<(), InstructionError> { - if self.instruction_context_stack.len() >= self.instruction_context_capacity { - return Err(InstructionError::CallDepth); - } - - let instruction_context = InstructionContext { - program_accounts: program_accounts.to_vec(), - instruction_accounts: instruction_accounts.to_vec(), - instruction_data: instruction_data.to_vec(), - }; - if self.instruction_context_stack.is_empty() { + let index_in_trace = if self.instruction_stack.is_empty() { debug_assert!( self.instruction_trace.len() < self.number_of_instructions_at_transaction_level ); - self.instruction_trace.push(vec![( - TRANSACTION_LEVEL_STACK_HEIGHT, - instruction_context.clone(), - )]); + let instruction_context = InstructionContext { + nesting_level: self.instruction_stack.len(), + 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 let Some(instruction_trace) = self.instruction_trace.last_mut() { + if record_instruction_in_transaction_context_push { + let instruction_context = InstructionContext { + nesting_level: self.instruction_stack.len(), + 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.len() >= self.instruction_context_capacity { + return Err(InstructionError::CallDepth); } - - self.instruction_context_stack.push(instruction_context); + self.instruction_stack.push(index_in_trace); Ok(()) } /// Pops the current InstructionContext pub fn pop(&mut self) -> Result<(), InstructionError> { - if self.instruction_context_stack.is_empty() { + if self.instruction_stack.is_empty() { return Err(InstructionError::CallDepth); } - self.instruction_context_stack.pop(); + self.instruction_stack.pop(); Ok(()) } @@ -238,9 +258,12 @@ impl TransactionContext { } /// Used by the runtime when a new CPI instruction begins - pub fn record_instruction(&mut self, stack_height: usize, instruction: InstructionContext) { + /// + /// Deprecated, automatically done in push() + /// once record_instruction_in_transaction_context_push is activated. + pub fn record_instruction(&mut self, instruction: InstructionContext) { if let Some(records) = self.instruction_trace.last_mut() { - records.push((stack_height, instruction)); + records.push(instruction); } } @@ -251,13 +274,14 @@ impl TransactionContext { } /// List of (stack height, instruction) for each top-level instruction -pub type InstructionTrace = Vec>; +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. #[derive(Debug, Clone)] pub struct InstructionContext { + nesting_level: usize, program_accounts: Vec, instruction_accounts: Vec, instruction_data: Vec, @@ -266,17 +290,26 @@ pub struct InstructionContext { impl InstructionContext { /// New pub fn new( + nesting_level: usize, program_accounts: &[usize], instruction_accounts: &[InstructionAccount], instruction_data: &[u8], ) -> Self { InstructionContext { + nesting_level, program_accounts: program_accounts.to_vec(), instruction_accounts: instruction_accounts.to_vec(), instruction_data: instruction_data.to_vec(), } } + /// How many Instructions were on the stack after this one was pushed + /// + /// That is the number of nested parent Instructions plus one (itself). + pub fn get_stack_height(&self) -> usize { + self.nesting_level.saturating_add(1) + } + /// Number of program accounts pub fn get_number_of_program_accounts(&self) -> usize { self.program_accounts.len()