From cbb74a190f669d53804816918853af0cef817994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 21 Jul 2022 12:49:34 +0200 Subject: [PATCH] Cleanup: `record_instruction_in_transaction_context_push` (#26658) Cleanup feature gate of record_instruction_in_transaction_context_push. --- program-runtime/src/invoke_context.rs | 75 +++++--------------- programs/bpf_loader/benches/serialization.rs | 2 +- runtime/src/message_processor.rs | 26 +------ sdk/src/transaction_context.rs | 31 +++----- 4 files changed, 32 insertions(+), 102 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 8eb418ce8..054e34b4a 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -16,8 +16,7 @@ use { account::{AccountSharedData, ReadableAccount}, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, feature_set::{ - cap_accounts_data_len, enable_early_verification_of_account_modifications, - record_instruction_in_transaction_context_push, FeatureSet, + cap_accounts_data_len, enable_early_verification_of_account_modifications, FeatureSet, }, hash::Hash, instruction::{AccountMeta, Instruction, InstructionError}, @@ -25,9 +24,7 @@ use { pubkey::Pubkey, rent::Rent, saturating_add_assign, - transaction_context::{ - InstructionAccount, InstructionContext, TransactionAccount, TransactionContext, - }, + transaction_context::{InstructionAccount, TransactionAccount, TransactionContext}, }, std::{ alloc::Layout, @@ -322,17 +319,6 @@ impl<'a> InvokeContext<'a> { program_indices: &[usize], instruction_data: &[u8], ) -> Result<(), InstructionError> { - 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); - } - let program_id = self.transaction_context.get_key_of_account_at_index( *program_indices .last() @@ -437,13 +423,8 @@ impl<'a> InvokeContext<'a> { }), )); self.syscall_context.push(None); - self.transaction_context.push( - program_indices, - instruction_accounts, - instruction_data, - self.feature_set - .is_active(&record_instruction_in_transaction_context_push::id()), - ) + self.transaction_context + .push(program_indices, instruction_accounts, instruction_data) } /// Pop a stack frame from the invocation stack @@ -836,41 +817,23 @@ impl<'a> InvokeContext<'a> { .transaction_context .get_instruction_context_stack_height(); let is_top_level_instruction = nesting_level == 0; - if !is_top_level_instruction { - if !self + if !is_top_level_instruction + && !self .feature_set .is_active(&enable_early_verification_of_account_modifications::id()) - { - // Verify the calling program hasn't misbehaved - let mut verify_caller_time = Measure::start("verify_caller_time"); - let verify_caller_result = self.verify_and_update(instruction_accounts, true); - verify_caller_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .process_instructions - .verify_caller_us, - verify_caller_time.as_us() - ); - verify_caller_result?; - } - - if !self - .feature_set - .is_active(&record_instruction_in_transaction_context_push::id()) - { - let instruction_accounts_lamport_sum = self - .transaction_context - .instruction_accounts_lamport_sum(instruction_accounts)?; - self.transaction_context - .record_instruction(InstructionContext::new( - nesting_level, - instruction_accounts_lamport_sum, - program_indices, - instruction_accounts, - instruction_data, - )); - } + { + // Verify the calling program hasn't misbehaved + let mut verify_caller_time = Measure::start("verify_caller_time"); + let verify_caller_result = self.verify_and_update(instruction_accounts, true); + verify_caller_time.stop(); + saturating_add_assign!( + timings + .execute_accessories + .process_instructions + .verify_caller_us, + verify_caller_time.as_us() + ); + verify_caller_result?; } self.push(instruction_accounts, program_indices, instruction_data)?; diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 7d6639453..1ccdde1ec 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -106,7 +106,7 @@ fn create_inputs() -> TransactionContext { TransactionContext::new(transaction_accounts, Some(Rent::default()), 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, true) + .push(&[0], &instruction_accounts, &instruction_data) .unwrap(); transaction_context } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 8afa00950..c1b06c141 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -10,10 +10,7 @@ use { }, solana_sdk::{ account::WritableAccount, - feature_set::{ - prevent_calling_precompiles_as_programs, - record_instruction_in_transaction_context_push, FeatureSet, - }, + feature_set::{prevent_calling_precompiles_as_programs, FeatureSet}, hash::Hash, message::SanitizedMessage, precompiles::is_precompile, @@ -93,14 +90,6 @@ impl MessageProcessor { .feature_set .is_active(&prevent_calling_precompiles_as_programs::id()) && is_precompile(program_id, |id| invoke_context.feature_set.is_active(id)); - if is_precompile - && !invoke_context - .feature_set - .is_active(&record_instruction_in_transaction_context_push::id()) - { - // Precompiled programs don't have an instruction processor - continue; - } // Fixup the special instructions key if present // before the account pre-values are taken care of @@ -140,19 +129,10 @@ impl MessageProcessor { }); } - let result = if is_precompile - && invoke_context - .feature_set - .is_active(&record_instruction_in_transaction_context_push::id()) - { + let result = if is_precompile { invoke_context .transaction_context - .push( - program_indices, - &instruction_accounts, - &instruction.data, - true, - ) + .push(program_indices, &instruction_accounts, &instruction.data) .and_then(|_| invoke_context.transaction_context.pop()) } else { let mut time = Measure::start("execute_instruction"); diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index ba1eba71c..eabd59c17 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -191,7 +191,6 @@ impl TransactionContext { program_accounts: &[usize], instruction_accounts: &[InstructionAccount], instruction_data: &[u8], - record_instruction_in_transaction_context_push: bool, ) -> Result<(), InstructionError> { let callee_instruction_accounts_lamport_sum = self.instruction_accounts_lamport_sum(instruction_accounts)?; @@ -224,16 +223,14 @@ impl TransactionContext { } } 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(), - 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); - } + 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); @@ -296,23 +293,13 @@ impl TransactionContext { Ok(()) } - /// Used by the runtime when a new CPI instruction begins - /// - /// 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(instruction); - } - } - /// Returns instruction trace pub fn get_instruction_trace(&self) -> &InstructionTrace { &self.instruction_trace } /// Calculates the sum of all lamports within an instruction - pub fn instruction_accounts_lamport_sum( + fn instruction_accounts_lamport_sum( &self, instruction_accounts: &[InstructionAccount], ) -> Result {