Skip to content

Commit

Permalink
Gather recording booleans in a data structure (#134)
Browse files Browse the repository at this point in the history
  • Loading branch information
LucasSte authored and willhickey committed Mar 9, 2024
1 parent ce10dc1 commit 2c55819
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 66 deletions.
5 changes: 2 additions & 3 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 8 additions & 8 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -3962,9 +3964,7 @@ pub mod tests {
&batch,
MAX_PROCESSING_AGE,
false,
false,
false,
false,
ExecutionRecordingConfig::new_single_setting(false),
&mut ExecuteTimings::default(),
None,
);
Expand Down
13 changes: 7 additions & 6 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use {
sysvar::{self, clock},
transaction::VersionedTransaction,
},
solana_svm::transaction_processor::ExecutionRecordingConfig,
solana_svm::transaction_results::{
DurableNonceFee, InnerInstruction, TransactionExecutionDetails, TransactionExecutionResult,
TransactionResults,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -152,9 +155,7 @@ fn execute_transactions(
&batch,
std::usize::MAX,
true,
true,
true,
true,
ExecutionRecordingConfig::new_single_setting(true),
&mut timings,
None,
);
Expand Down
37 changes: 16 additions & 21 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<usize>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<usize>,
) -> (TransactionResults, TransactionBalancesSet) {
Expand All @@ -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,
Expand Down Expand Up @@ -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),
);
Expand Down Expand Up @@ -5773,9 +5770,7 @@ impl Bank {
batch,
MAX_PROCESSING_AGE,
false,
false,
false,
false,
ExecutionRecordingConfig::new_single_setting(false),
&mut ExecuteTimings::default(),
None,
)
Expand Down
24 changes: 12 additions & 12 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
45 changes: 29 additions & 16 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ pub struct LoadAndExecuteSanitizedTransactionsOutput {
pub execution_results: Vec<TransactionExecutionResult>,
}

/// 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<usize>;

Expand Down Expand Up @@ -184,9 +202,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
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<Item = &'a Pubkey>,
Expand Down Expand Up @@ -266,9 +282,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
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,
Expand Down Expand Up @@ -466,9 +480,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
loaded_transaction: &mut LoadedTransaction,
compute_budget: ComputeBudget,
durable_nonce_fee: Option<DurableNonceFee>,
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<usize>,
Expand Down Expand Up @@ -506,7 +518,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
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(
Expand Down Expand Up @@ -585,7 +597,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
.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,
))
Expand Down Expand Up @@ -616,11 +628,12 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
);
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 {
Expand Down

0 comments on commit 2c55819

Please sign in to comment.