From 2c55819e7bfdeeb6cc798afb18373cd86573af63 Mon Sep 17 00:00:00 2001 From: Lucas Steuernagel <38472950+LucasSte@users.noreply.github.com> Date: Fri, 8 Mar 2024 09:28:04 -0300 Subject: [PATCH] Gather recording booleans in a data structure (#134) --- core/src/banking_stage/consumer.rs | 5 ++-- ledger/src/blockstore_processor.rs | 16 +++++------ programs/sbf/tests/programs.rs | 13 +++++---- runtime/src/bank.rs | 37 +++++++++++------------- runtime/src/bank/tests.rs | 24 ++++++++-------- svm/src/transaction_processor.rs | 45 +++++++++++++++++++----------- 6 files changed, 74 insertions(+), 66 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 957e190c873f64..c5ed22a34278ce 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -34,6 +34,7 @@ use { solana_svm::{ account_loader::{validate_fee_payer, TransactionCheckResult}, transaction_error_metrics::TransactionErrorMetrics, + transaction_processor::ExecutionRecordingConfig, }, std::{ sync::{atomic::Ordering, Arc}, @@ -593,9 +594,7 @@ impl Consumer { .load_and_execute_transactions( batch, MAX_PROCESSING_AGE, - transaction_status_sender_enabled, - transaction_status_sender_enabled, - transaction_status_sender_enabled, + ExecutionRecordingConfig::new_single_setting(transaction_status_sender_enabled), &mut execute_and_commit_timings.execute_timings, None, // account_overrides self.log_messages_bytes_limit, diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index c999eab1a56fd4..e4ae5f368b2afd 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -57,8 +57,11 @@ use { VersionedTransaction, }, }, - solana_svm::transaction_results::{ - TransactionExecutionDetails, TransactionExecutionResult, TransactionResults, + solana_svm::{ + transaction_processor::ExecutionRecordingConfig, + transaction_results::{ + TransactionExecutionDetails, TransactionExecutionResult, TransactionResults, + }, }, solana_transaction_status::token_balances::TransactionTokenBalancesSet, solana_vote::{vote_account::VoteAccountsHashMap, vote_sender_types::ReplayVoteSender}, @@ -163,9 +166,7 @@ pub fn execute_batch( batch, MAX_PROCESSING_AGE, transaction_status_sender.is_some(), - transaction_status_sender.is_some(), - transaction_status_sender.is_some(), - transaction_status_sender.is_some(), + ExecutionRecordingConfig::new_single_setting(transaction_status_sender.is_some()), timings, log_messages_bytes_limit, ); @@ -1972,6 +1973,7 @@ pub mod tests { system_transaction, transaction::{Transaction, TransactionError}, }, + solana_svm::transaction_processor::ExecutionRecordingConfig, solana_vote::vote_account::VoteAccount, solana_vote_program::{ self, @@ -3962,9 +3964,7 @@ pub mod tests { &batch, MAX_PROCESSING_AGE, false, - false, - false, - false, + ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, ); diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index dc4867ce7e40fd..22969bc482a28e 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -48,6 +48,7 @@ use { sysvar::{self, clock}, transaction::VersionedTransaction, }, + solana_svm::transaction_processor::ExecutionRecordingConfig, solana_svm::transaction_results::{ DurableNonceFee, InnerInstruction, TransactionExecutionDetails, TransactionExecutionResult, TransactionResults, @@ -104,9 +105,11 @@ fn process_transaction_and_record_inner( &tx_batch, MAX_PROCESSING_AGE, false, - true, - true, - false, + ExecutionRecordingConfig { + enable_cpi_recording: true, + enable_log_recording: true, + enable_return_data_recording: false, + }, &mut ExecuteTimings::default(), None, ) @@ -152,9 +155,7 @@ fn execute_transactions( &batch, std::usize::MAX, true, - true, - true, - true, + ExecutionRecordingConfig::new_single_setting(true), &mut timings, None, ); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ee04f20787cb9a..3e504d470de744 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -269,6 +269,7 @@ pub struct BankRc { #[cfg(RUSTC_WITH_SPECIALIZATION)] use solana_frozen_abi::abi_example::AbiExample; +use solana_svm::transaction_processor::ExecutionRecordingConfig; #[cfg(RUSTC_WITH_SPECIALIZATION)] impl AbiExample for BankRc { @@ -4297,9 +4298,11 @@ impl Bank { // for processing. During forwarding, the transaction could expire if the // delay is not accounted for. MAX_PROCESSING_AGE - MAX_TRANSACTION_FORWARDING_DELAY, - enable_cpi_recording, - true, - true, + ExecutionRecordingConfig { + enable_cpi_recording, + enable_log_recording: true, + enable_return_data_recording: true, + }, &mut timings, Some(&account_overrides), None, @@ -4548,9 +4551,7 @@ impl Bank { &self, batch: &TransactionBatch, max_age: usize, - enable_cpi_recording: bool, - enable_log_recording: bool, - enable_return_data_recording: bool, + recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, account_overrides: Option<&AccountOverrides>, log_messages_bytes_limit: Option, @@ -4614,9 +4615,7 @@ impl Bank { sanitized_txs, &mut check_results, &mut error_counters, - enable_cpi_recording, - enable_log_recording, - enable_return_data_recording, + recording_config, timings, account_overrides, self.builtin_programs.iter(), @@ -5642,9 +5641,7 @@ impl Bank { batch: &TransactionBatch, max_age: usize, collect_balances: bool, - enable_cpi_recording: bool, - enable_log_recording: bool, - enable_return_data_recording: bool, + recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, log_messages_bytes_limit: Option, ) -> (TransactionResults, TransactionBalancesSet) { @@ -5665,9 +5662,7 @@ impl Bank { } = self.load_and_execute_transactions( batch, max_age, - enable_cpi_recording, - enable_log_recording, - enable_return_data_recording, + recording_config, timings, None, log_messages_bytes_limit, @@ -5735,9 +5730,11 @@ impl Bank { &batch, MAX_PROCESSING_AGE, false, // collect_balances - false, // enable_cpi_recording - true, // enable_log_recording - true, // enable_return_data_recording + ExecutionRecordingConfig { + enable_cpi_recording: false, + enable_log_recording: true, + enable_return_data_recording: true, + }, &mut ExecuteTimings::default(), Some(1000 * 1000), ); @@ -5773,9 +5770,7 @@ impl Bank { batch, MAX_PROCESSING_AGE, false, - false, - false, - false, + ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, ) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 753116ff878e18..f9b846d85b1512 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -3122,9 +3122,7 @@ fn test_interleaving_locks() { &lock_result, MAX_PROCESSING_AGE, false, - false, - false, - false, + ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, ) @@ -5948,9 +5946,7 @@ fn test_pre_post_transaction_balances() { &lock_result, MAX_PROCESSING_AGE, true, - false, - false, - false, + ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, ); @@ -9230,9 +9226,11 @@ fn test_tx_log_order() { &batch, MAX_PROCESSING_AGE, false, - false, - true, - false, + ExecutionRecordingConfig { + enable_cpi_recording: false, + enable_log_recording: true, + enable_return_data_recording: false, + }, &mut ExecuteTimings::default(), None, ) @@ -9338,9 +9336,11 @@ fn test_tx_return_data() { &batch, MAX_PROCESSING_AGE, false, - false, - false, - true, + ExecutionRecordingConfig { + enable_cpi_recording: false, + enable_log_recording: false, + enable_return_data_recording: true, + }, &mut ExecuteTimings::default(), None, ) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index e44b426df96b0d..d90afb0a428ea3 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -65,6 +65,24 @@ pub struct LoadAndExecuteSanitizedTransactionsOutput { pub execution_results: Vec, } +/// Configuration of the recording capabilities for transaction execution +#[derive(Copy, Clone)] +pub struct ExecutionRecordingConfig { + pub enable_cpi_recording: bool, + pub enable_log_recording: bool, + pub enable_return_data_recording: bool, +} + +impl ExecutionRecordingConfig { + pub fn new_single_setting(option: bool) -> Self { + ExecutionRecordingConfig { + enable_return_data_recording: option, + enable_log_recording: option, + enable_cpi_recording: option, + } + } +} + pub trait TransactionProcessingCallback { fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option; @@ -184,9 +202,7 @@ impl TransactionBatchProcessor { sanitized_txs: &[SanitizedTransaction], check_results: &mut [TransactionCheckResult], error_counters: &mut TransactionErrorMetrics, - enable_cpi_recording: bool, - enable_log_recording: bool, - enable_return_data_recording: bool, + recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, account_overrides: Option<&AccountOverrides>, builtin_programs: impl Iterator, @@ -266,9 +282,7 @@ impl TransactionBatchProcessor { loaded_transaction, compute_budget, nonce.as_ref().map(DurableNonceFee::from), - enable_cpi_recording, - enable_log_recording, - enable_return_data_recording, + recording_config, timings, error_counters, log_messages_bytes_limit, @@ -466,9 +480,7 @@ impl TransactionBatchProcessor { loaded_transaction: &mut LoadedTransaction, compute_budget: ComputeBudget, durable_nonce_fee: Option, - enable_cpi_recording: bool, - enable_log_recording: bool, - enable_return_data_recording: bool, + recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, error_counters: &mut TransactionErrorMetrics, log_messages_bytes_limit: Option, @@ -506,7 +518,7 @@ impl TransactionBatchProcessor { tx.message(), ); - let log_collector = if enable_log_recording { + let log_collector = if recording_config.enable_log_recording { match log_messages_bytes_limit { None => Some(LogCollector::new_ref()), Some(log_messages_bytes_limit) => Some(LogCollector::new_ref_with_limit(Some( @@ -585,7 +597,7 @@ impl TransactionBatchProcessor { .ok() }); - let inner_instructions = if enable_cpi_recording { + let inner_instructions = if recording_config.enable_cpi_recording { Some(Self::inner_instructions_list_from_instruction_trace( &transaction_context, )) @@ -616,11 +628,12 @@ impl TransactionBatchProcessor { ); saturating_add_assign!(timings.details.changed_account_count, touched_account_count); - let return_data = if enable_return_data_recording && !return_data.data.is_empty() { - Some(return_data) - } else { - None - }; + let return_data = + if recording_config.enable_return_data_recording && !return_data.data.is_empty() { + Some(return_data) + } else { + None + }; TransactionExecutionResult::Executed { details: TransactionExecutionDetails {