From acc60fba066911188ed15d29403b850c4b241c46 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sat, 3 Aug 2024 04:07:59 +0000 Subject: [PATCH] feat: support committing fee-only transactions --- core/src/banking_stage/committer.rs | 24 +- core/src/banking_stage/consume_worker.rs | 36 +- core/src/banking_stage/consumer.rs | 49 +-- core/src/banking_stage/leader_slot_metrics.rs | 54 +-- ledger-tool/src/main.rs | 6 +- ledger/src/blockstore_processor.rs | 9 +- rpc/src/transaction_status_service.rs | 19 +- runtime/src/bank.rs | 321 ++++++++++-------- runtime/src/bank/bank_hash_details.rs | 31 +- sdk/src/feature_set.rs | 5 + svm/src/account_loader.rs | 102 ++++-- svm/src/account_saver.rs | 86 +++-- svm/src/lib.rs | 1 + svm/src/rollback_accounts.rs | 13 + svm/src/transaction_commit_result.rs | 28 +- svm/src/transaction_execution_result.rs | 71 +--- svm/src/transaction_processing_result.rs | 80 +++++ svm/src/transaction_processor.rs | 38 ++- 18 files changed, 581 insertions(+), 392 deletions(-) create mode 100644 svm/src/transaction_processing_result.rs diff --git a/core/src/banking_stage/committer.rs b/core/src/banking_stage/committer.rs index d91900299107c8..ca62ebe8f46389 100644 --- a/core/src/banking_stage/committer.rs +++ b/core/src/banking_stage/committer.rs @@ -6,7 +6,7 @@ use { }, solana_measure::measure_us, solana_runtime::{ - bank::{Bank, ExecutedTransactionCounts, TransactionBalancesSet}, + bank::{Bank, ProcessedTransactionCounts, TransactionBalancesSet}, bank_utils, prioritization_fee_cache::PrioritizationFeeCache, transaction_batch::TransactionBatch, @@ -15,7 +15,9 @@ use { solana_sdk::{hash::Hash, pubkey::Pubkey, saturating_add_assign}, solana_svm::{ transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions}, - transaction_execution_result::TransactionExecutionResult, + transaction_processing_result::{ + TransactionProcessingResult, TransactionProcessingResultExtensions, + }, }, solana_transaction_status::{ token_balances::TransactionTokenBalancesSet, TransactionTokenBalance, @@ -67,27 +69,27 @@ impl Committer { pub(super) fn commit_transactions( &self, batch: &TransactionBatch, - execution_results: Vec, + processing_results: Vec, last_blockhash: Hash, lamports_per_signature: u64, starting_transaction_index: Option, bank: &Arc, pre_balance_info: &mut PreBalanceInfo, execute_and_commit_timings: &mut LeaderExecuteAndCommitTimings, - execution_counts: &ExecutedTransactionCounts, + processed_counts: &ProcessedTransactionCounts, ) -> (u64, Vec) { - let executed_transactions = execution_results + let processed_transactions = processing_results .iter() .zip(batch.sanitized_transactions()) - .filter_map(|(execution_result, tx)| execution_result.was_executed().then_some(tx)) + .filter_map(|(processing_result, tx)| processing_result.was_processed().then_some(tx)) .collect_vec(); let (commit_results, commit_time_us) = measure_us!(bank.commit_transactions( batch.sanitized_transactions(), - execution_results, + processing_results, last_blockhash, lamports_per_signature, - execution_counts, + processed_counts, &mut execute_and_commit_timings.execute_timings, )); execute_and_commit_timings.commit_us = commit_time_us; @@ -99,7 +101,7 @@ impl Committer { // transaction committed to block. qos_service uses these information to adjust // reserved block space. Ok(committed_tx) => CommitTransactionDetails::Committed { - compute_units: committed_tx.execution_details.executed_units, + compute_units: committed_tx.executed_units, loaded_accounts_data_size: committed_tx .loaded_account_stats .loaded_accounts_data_size, @@ -122,7 +124,7 @@ impl Committer { starting_transaction_index, ); self.prioritization_fee_cache - .update(bank, executed_transactions.into_iter()); + .update(bank, processed_transactions.into_iter()); }); execute_and_commit_timings.find_and_send_votes_us = find_and_send_votes_us; (commit_time_us, commit_transaction_statuses) @@ -145,7 +147,7 @@ impl Committer { let batch_transaction_indexes: Vec<_> = commit_results .iter() .map(|commit_result| { - if commit_result.was_executed() { + if commit_result.was_committed() { let this_transaction_index = transaction_index; saturating_add_assign!(transaction_index, 1); this_transaction_index diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 449ea9ab963a39..3902ce8829f163 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -234,18 +234,18 @@ impl ConsumeWorkerMetrics { }: &ExecuteAndCommitTransactionsOutput, ) { self.count_metrics - .transactions_attempted_execution_count + .transactions_attempted_processing_count .fetch_add( - transaction_counts.attempted_execution_count, + transaction_counts.attempted_processing_count, Ordering::Relaxed, ); self.count_metrics - .executed_transactions_count - .fetch_add(transaction_counts.executed_count, Ordering::Relaxed); + .processed_transactions_count + .fetch_add(transaction_counts.processed_count, Ordering::Relaxed); self.count_metrics - .executed_with_successful_result_count + .processed_with_successful_result_count .fetch_add( - transaction_counts.executed_with_successful_result_count, + transaction_counts.processed_with_successful_result_count, Ordering::Relaxed, ); self.count_metrics @@ -410,9 +410,9 @@ impl ConsumeWorkerMetrics { } struct ConsumeWorkerCountMetrics { - transactions_attempted_execution_count: AtomicU64, - executed_transactions_count: AtomicU64, - executed_with_successful_result_count: AtomicU64, + transactions_attempted_processing_count: AtomicU64, + processed_transactions_count: AtomicU64, + processed_with_successful_result_count: AtomicU64, retryable_transaction_count: AtomicUsize, retryable_expired_bank_count: AtomicUsize, cost_model_throttled_transactions_count: AtomicU64, @@ -423,9 +423,9 @@ struct ConsumeWorkerCountMetrics { impl Default for ConsumeWorkerCountMetrics { fn default() -> Self { Self { - transactions_attempted_execution_count: AtomicU64::default(), - executed_transactions_count: AtomicU64::default(), - executed_with_successful_result_count: AtomicU64::default(), + transactions_attempted_processing_count: AtomicU64::default(), + processed_transactions_count: AtomicU64::default(), + processed_with_successful_result_count: AtomicU64::default(), retryable_transaction_count: AtomicUsize::default(), retryable_expired_bank_count: AtomicUsize::default(), cost_model_throttled_transactions_count: AtomicU64::default(), @@ -441,19 +441,19 @@ impl ConsumeWorkerCountMetrics { "banking_stage_worker_counts", "id" => id, ( - "transactions_attempted_execution_count", - self.transactions_attempted_execution_count + "transactions_attempted_processing_count", + self.transactions_attempted_processing_count .swap(0, Ordering::Relaxed), i64 ), ( - "executed_transactions_count", - self.executed_transactions_count.swap(0, Ordering::Relaxed), + "processed_transactions_count", + self.processed_transactions_count.swap(0, Ordering::Relaxed), i64 ), ( - "executed_with_successful_result_count", - self.executed_with_successful_result_count + "processed_with_successful_result_count", + self.processed_with_successful_result_count .swap(0, Ordering::Relaxed), i64 ), diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 9965b1c3214c3d..d3718f438b99b1 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -3,7 +3,7 @@ use { committer::{CommitTransactionDetails, Committer, PreBalanceInfo}, immutable_deserialized_packet::ImmutableDeserializedPacket, leader_slot_metrics::{ - LeaderSlotMetricsTracker, ProcessTransactionsCounts, ProcessTransactionsSummary, + CommittedTransactionsCounts, LeaderSlotMetricsTracker, ProcessTransactionsSummary, }, leader_slot_timing_metrics::LeaderExecuteAndCommitTimings, qos_service::QosService, @@ -34,6 +34,7 @@ use { solana_svm::{ account_loader::{validate_fee_payer, TransactionCheckResult}, transaction_error_metrics::TransactionErrorMetrics, + transaction_processing_result::TransactionProcessingResultExtensions, transaction_processor::{ExecutionRecordingConfig, TransactionProcessingConfig}, }, solana_timings::ExecuteTimings, @@ -73,13 +74,13 @@ pub struct ExecuteAndCommitTransactionsOutput { #[derive(Debug, Default, PartialEq)] pub struct ExecuteAndCommitTransactionsCounts { // Total number of transactions that were passed as candidates for execution - pub(crate) attempted_execution_count: u64, - // The number of transactions of that were executed. See description of in `ProcessTransactionsSummary` + pub(crate) attempted_processing_count: u64, + // The number of transactions of that were processed. See description of in `ProcessTransactionsSummary` // for possible outcomes of execution. - pub(crate) executed_count: u64, - // Total number of the executed transactions that returned success/not + pub(crate) processed_count: u64, + // Total number of the processed transactions that returned success/not // an error. - pub(crate) executed_with_successful_result_count: u64, + pub(crate) processed_with_successful_result_count: u64, } pub struct Consumer { @@ -284,7 +285,7 @@ impl Consumer { ) -> ProcessTransactionsSummary { let mut chunk_start = 0; let mut all_retryable_tx_indexes = vec![]; - let mut total_transaction_counts = ProcessTransactionsCounts::default(); + let mut total_transaction_counts = CommittedTransactionsCounts::default(); let mut total_cost_model_throttled_transactions_count: u64 = 0; let mut total_cost_model_us: u64 = 0; let mut total_execute_and_commit_timings = LeaderExecuteAndCommitTimings::default(); @@ -622,22 +623,22 @@ impl Consumer { execute_and_commit_timings.load_execute_us = load_execute_us; let LoadAndExecuteTransactionsOutput { - execution_results, - execution_counts, + processing_results, + processed_counts, } = load_and_execute_transactions_output; let transaction_counts = ExecuteAndCommitTransactionsCounts { - executed_count: execution_counts.executed_transactions_count, - executed_with_successful_result_count: execution_counts.executed_successfully_count, - attempted_execution_count: execution_results.len() as u64, + processed_count: processed_counts.processed_transactions_count, + processed_with_successful_result_count: processed_counts.processed_successfully, + attempted_processing_count: processing_results.len() as u64, }; - let (executed_transactions, execution_results_to_transactions_us) = - measure_us!(execution_results + let (processed_transactions, execution_results_to_transactions_us) = + measure_us!(processing_results .iter() .zip(batch.sanitized_transactions()) - .filter_map(|(execution_result, tx)| { - if execution_result.was_executed() { + .filter_map(|(processing_result, tx)| { + if processing_result.was_processed() { Some(tx.to_versioned_transaction()) } else { None @@ -661,7 +662,7 @@ impl Consumer { let (record_transactions_summary, record_us) = measure_us!(self .transaction_recorder - .record_transactions(bank.slot(), executed_transactions)); + .record_transactions(bank.slot(), processed_transactions)); execute_and_commit_timings.record_us = record_us; let RecordTransactionsSummary { @@ -675,8 +676,8 @@ impl Consumer { }; if let Err(recorder_err) = record_transactions_result { - retryable_transaction_indexes.extend(execution_results.iter().enumerate().filter_map( - |(index, execution_result)| execution_result.was_executed().then_some(index), + retryable_transaction_indexes.extend(processing_results.iter().enumerate().filter_map( + |(index, processing_result)| processing_result.was_processed().then_some(index), )); return ExecuteAndCommitTransactionsOutput { @@ -691,22 +692,22 @@ impl Consumer { } let (commit_time_us, commit_transaction_statuses) = - if execution_counts.executed_transactions_count != 0 { + if processed_counts.processed_transactions_count != 0 { self.committer.commit_transactions( batch, - execution_results, + processing_results, last_blockhash, lamports_per_signature, starting_transaction_index, bank, &mut pre_balance_info, &mut execute_and_commit_timings, - &execution_counts, + &processed_counts, ) } else { ( 0, - vec![CommitTransactionDetails::NotCommitted; execution_results.len()], + vec![CommitTransactionDetails::NotCommitted; processing_results.len()], ) }; @@ -727,7 +728,7 @@ impl Consumer { ); debug_assert_eq!( - transaction_counts.attempted_execution_count, + transaction_counts.attempted_processing_count, commit_transaction_statuses.len() as u64, ); diff --git a/core/src/banking_stage/leader_slot_metrics.rs b/core/src/banking_stage/leader_slot_metrics.rs index 2e5999fb8ed49d..eecd2fc9131198 100644 --- a/core/src/banking_stage/leader_slot_metrics.rs +++ b/core/src/banking_stage/leader_slot_metrics.rs @@ -13,15 +13,15 @@ use { std::time::Instant, }; -/// A summary of what happened to transactions passed to the execution pipeline. +/// A summary of what happened to transactions passed to the processing pipeline. /// Transactions can -/// 1) Did not even make it to execution due to being filtered out by things like AccountInUse +/// 1) Did not even make it to processing due to being filtered out by things like AccountInUse /// lock conflicts or CostModel compute limits. These types of errors are retryable and /// counted in `Self::retryable_transaction_indexes`. -/// 2) Did not execute due to some fatal error like too old, or duplicate signature. These +/// 2) Did not process due to some fatal error like too old, or duplicate signature. These /// will be dropped from the transactions queue and not counted in `Self::retryable_transaction_indexes` -/// 3) Were executed and committed, captured by `transaction_counts` below. -/// 4) Were executed and failed commit, captured by `transaction_counts` below. +/// 3) Were processed and committed, captured by `transaction_counts` below. +/// 4) Were processed and failed commit, captured by `transaction_counts` below. pub(crate) struct ProcessTransactionsSummary { // Returns true if we hit the end of the block/max PoH height for the block before // processing all the transactions in the batch. @@ -29,7 +29,7 @@ pub(crate) struct ProcessTransactionsSummary { // Total transaction counts tracked for reporting `LeaderSlotMetrics`. See // description of struct above for possible outcomes for these transactions - pub transaction_counts: ProcessTransactionsCounts, + pub transaction_counts: CommittedTransactionsCounts, // Indexes of transactions in the transactions slice that were not committed but are retryable pub retryable_transaction_indexes: Vec, @@ -51,42 +51,42 @@ pub(crate) struct ProcessTransactionsSummary { } #[derive(Debug, Default, PartialEq)] -pub struct ProcessTransactionsCounts { - // Total number of transactions that were passed as candidates for execution - pub attempted_execution_count: u64, +pub struct CommittedTransactionsCounts { + // Total number of transactions that were passed as candidates for processing + pub attempted_processing_count: u64, // Total number of transactions that made it into the block pub committed_transactions_count: u64, // Total number of transactions that made it into the block where the transactions - // output from execution was success/no error. + // output from processing was success/no error. pub committed_transactions_with_successful_result_count: u64, - // All transactions that were executed but then failed record because the + // All transactions that were processed but then failed record because the // slot ended - pub executed_but_failed_commit: u64, + pub processed_but_failed_commit: u64, } -impl ProcessTransactionsCounts { +impl CommittedTransactionsCounts { pub fn accumulate( &mut self, transaction_counts: &ExecuteAndCommitTransactionsCounts, committed: bool, ) { saturating_add_assign!( - self.attempted_execution_count, - transaction_counts.attempted_execution_count + self.attempted_processing_count, + transaction_counts.attempted_processing_count ); if committed { saturating_add_assign!( self.committed_transactions_count, - transaction_counts.executed_count + transaction_counts.processed_count ); saturating_add_assign!( self.committed_transactions_with_successful_result_count, - transaction_counts.executed_with_successful_result_count + transaction_counts.processed_with_successful_result_count ); } else { saturating_add_assign!( - self.executed_but_failed_commit, - transaction_counts.executed_count + self.processed_but_failed_commit, + transaction_counts.processed_count ); } } @@ -171,10 +171,10 @@ struct LeaderSlotPacketCountMetrics { // duplicate signature checks retryable_packets_filtered_count: u64, - // total number of transactions that attempted execution in this slot. Should equal the sum + // total number of transactions that attempted processing in this slot. Should equal the sum // of `committed_transactions_count`, `retryable_errored_transaction_count`, and // `nonretryable_errored_transactions_count`. - transactions_attempted_execution_count: u64, + transactions_attempted_processing_count: u64, // total number of transactions that were executed and committed into the block // on this thread @@ -303,8 +303,8 @@ impl LeaderSlotPacketCountMetrics { i64 ), ( - "transactions_attempted_execution_count", - self.transactions_attempted_execution_count, + "transactions_attempted_processing_count", + self.transactions_attempted_processing_count, i64 ), ( @@ -605,8 +605,8 @@ impl LeaderSlotMetricsTracker { saturating_add_assign!( leader_slot_metrics .packet_count_metrics - .transactions_attempted_execution_count, - transaction_counts.attempted_execution_count + .transactions_attempted_processing_count, + transaction_counts.attempted_processing_count ); saturating_add_assign!( @@ -627,7 +627,7 @@ impl LeaderSlotMetricsTracker { leader_slot_metrics .packet_count_metrics .executed_transactions_failed_commit_count, - transaction_counts.executed_but_failed_commit + transaction_counts.processed_but_failed_commit ); saturating_add_assign!( @@ -642,7 +642,7 @@ impl LeaderSlotMetricsTracker { .packet_count_metrics .nonretryable_errored_transactions_count, transaction_counts - .attempted_execution_count + .attempted_processing_count .saturating_sub(transaction_counts.committed_transactions_count) .saturating_sub(retryable_transaction_indexes.len() as u64) ); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 78c36acf910ca7..e25cbe1ff17a72 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -708,16 +708,14 @@ fn record_transactions( .collect(); let is_simple_vote_tx = tx.is_simple_vote_transaction(); - let execution_results = commit_result - .ok() - .map(|committed_tx| committed_tx.execution_details); + let commit_details = commit_result.ok().map(|committed_tx| committed_tx.into()); TransactionDetails { signature: tx.signature().to_string(), accounts, instructions, is_simple_vote_tx, - execution_results, + commit_details, index, } }) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 24ff5ce8fb7baa..a84570c2704201 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -57,7 +57,7 @@ use { }, }, solana_svm::{ - transaction_commit_result::TransactionCommitResult, + transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions}, transaction_processor::ExecutionRecordingConfig, }, solana_timings::{report_execute_timings, ExecuteTimingType, ExecuteTimings}, @@ -190,7 +190,7 @@ pub fn execute_batch( let committed_transactions = commit_results .iter() .zip(batch.sanitized_transactions()) - .filter_map(|(commit_result, tx)| commit_result.is_ok().then_some(tx)) + .filter_map(|(commit_result, tx)| commit_result.was_committed().then_some(tx)) .collect_vec(); let first_err = get_first_error(batch, &commit_results); @@ -238,7 +238,7 @@ fn check_block_cost_limits( if let Ok(committed_tx) = commit_result { Some(CostModel::calculate_cost_for_executed_transaction( tx, - committed_tx.execution_details.executed_units, + committed_tx.executed_units, committed_tx.loaded_account_stats.loaded_accounts_data_size, &bank.feature_set, )) @@ -2245,9 +2245,6 @@ pub mod tests { }, solana_svm::{ transaction_commit_result::CommittedTransaction, - transaction_execution_result::{ - TransactionExecutionDetails, TransactionLoadedAccountsStats, - }, transaction_processor::ExecutionRecordingConfig, }, solana_vote::vote_account::VoteAccount, diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 49a78f22db7752..4ac10f14d16726 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -6,10 +6,7 @@ use { blockstore::Blockstore, blockstore_processor::{TransactionStatusBatch, TransactionStatusMessage}, }, - solana_svm::{ - transaction_commit_result::CommittedTransaction, - transaction_execution_result::TransactionExecutionDetails, - }, + solana_svm::transaction_commit_result::CommittedTransaction, solana_transaction_status::{ extract_and_fmt_memos, map_inner_instructions, Reward, TransactionStatusMeta, }, @@ -96,15 +93,11 @@ impl TransactionStatusService { ) { if let Ok(committed_tx) = commit_result { let CommittedTransaction { - execution_details: - TransactionExecutionDetails { - status, - log_messages, - inner_instructions, - return_data, - executed_units, - .. - }, + status, + log_messages, + inner_instructions, + return_data, + executed_units, fee_details, rent_debits, .. diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cebf31f2d7f945..a2efbb810dcc43 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -152,7 +152,8 @@ use { }, solana_svm::{ account_loader::{ - collect_rent_from_account, CheckedTransactionDetails, TransactionCheckResult, + collect_rent_from_account, CheckedTransactionDetails, LoadedTransaction, + TransactionCheckResult, }, account_overrides::AccountOverrides, account_saver::collect_accounts_to_store, @@ -160,9 +161,13 @@ use { transaction_commit_result::{CommittedTransaction, TransactionCommitResult}, transaction_error_metrics::TransactionErrorMetrics, transaction_execution_result::{ - TransactionExecutionDetails, TransactionExecutionResult, TransactionLoadedAccountsStats, + TransactionExecutionDetails, TransactionLoadedAccountsStats, }, transaction_processing_callback::TransactionProcessingCallback, + transaction_processing_result::{ + ProcessedTransaction, TransactionProcessingResult, + TransactionProcessingResultExtensions, + }, transaction_processor::{ ExecutionRecordingConfig, TransactionBatchProcessor, TransactionLogMessages, TransactionProcessingConfig, TransactionProcessingEnvironment, @@ -320,10 +325,10 @@ impl BankRc { pub struct LoadAndExecuteTransactionsOutput { // Vector of results indicating whether a transaction was executed or could not // be executed. Note executed transactions can still have failed! - pub execution_results: Vec, + pub processing_results: Vec, // Executed transaction counts used to update bank transaction counts and // for metrics reporting. - pub execution_counts: ExecutedTransactionCounts, + pub processed_counts: ProcessedTransactionCounts, } pub struct TransactionSimulationResult { @@ -886,10 +891,10 @@ struct PrevEpochInflationRewards { } #[derive(Debug, Default, PartialEq)] -pub struct ExecutedTransactionCounts { - pub executed_transactions_count: u64, - pub executed_successfully_count: u64, - pub executed_non_vote_transactions_count: u64, +pub struct ProcessedTransactionCounts { + pub processed_transactions_count: u64, + pub processed_non_vote_transactions_count: u64, + pub processed_successfully: u64, pub signature_count: u64, } @@ -3086,19 +3091,19 @@ impl Bank { fn update_transaction_statuses( &self, sanitized_txs: &[SanitizedTransaction], - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) { let mut status_cache = self.status_cache.write().unwrap(); - assert_eq!(sanitized_txs.len(), execution_results.len()); - for (tx, execution_result) in sanitized_txs.iter().zip(execution_results) { - if let Some(details) = execution_result.details() { + assert_eq!(sanitized_txs.len(), processing_results.len()); + for (tx, processing_result) in sanitized_txs.iter().zip(processing_results) { + if let Ok(processed_tx) = &processing_result { // Add the message hash to the status cache to ensure that this message // won't be processed again with a different signature. status_cache.insert( tx.message().recent_blockhash(), tx.message_hash(), self.slot(), - details.status.clone(), + processed_tx.status(), ); // Add the transaction signature to the status cache so that transaction status // can be queried by transaction signature over RPC. In the future, this should @@ -3107,7 +3112,7 @@ impl Bank { tx.message().recent_blockhash(), tx.signature(), self.slot(), - details.status.clone(), + processed_tx.status(), ); } } @@ -3319,7 +3324,7 @@ impl Bank { let mut timings = ExecuteTimings::default(); let LoadAndExecuteTransactionsOutput { - mut execution_results, + mut processing_results, .. } = self.load_and_execute_transactions( &batch, @@ -3356,36 +3361,38 @@ impl Bank { debug!("simulate_transaction: {:?}", timings); - let execution_result = - execution_results - .pop() - .unwrap_or(TransactionExecutionResult::NotExecuted( - TransactionError::InvalidProgramForExecution, - )); - let flattened_result = execution_result.flattened_result(); - let (post_simulation_accounts, logs, return_data, inner_instructions) = - match execution_result { - TransactionExecutionResult::Executed(executed_tx) => { - let details = executed_tx.execution_details; - let post_simulation_accounts = executed_tx - .loaded_transaction - .accounts - .into_iter() - .take(number_of_accounts) - .collect::>(); - ( - post_simulation_accounts, - details.log_messages, - details.return_data, - details.inner_instructions, - ) - } - TransactionExecutionResult::NotExecuted(_) => (vec![], None, None, None), + let processing_result = processing_results + .pop() + .unwrap_or(Err(TransactionError::InvalidProgramForExecution)); + let (post_simulation_accounts, result, logs, return_data, inner_instructions) = + match processing_result { + Ok(processed_tx) => match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + let details = executed_tx.execution_details; + let post_simulation_accounts = executed_tx + .loaded_transaction + .accounts + .into_iter() + .take(number_of_accounts) + .collect::>(); + ( + post_simulation_accounts, + details.status, + details.log_messages, + details.return_data, + details.inner_instructions, + ) + } + ProcessedTransaction::FeesOnly(fees_only_tx) => { + (vec![], Err(fees_only_tx.load_error), None, None, None) + } + }, + Err(error) => (vec![], Err(error), None, None, None), }; let logs = logs.unwrap_or_default(); TransactionSimulationResult { - result: flattened_result, + result, logs, post_simulation_accounts, units_consumed, @@ -3628,39 +3635,43 @@ impl Bank { timings.accumulate(&sanitized_output.execute_timings); let ((), collect_logs_us) = - measure_us!(self.collect_logs(sanitized_txs, &sanitized_output.execution_results)); + measure_us!(self.collect_logs(sanitized_txs, &sanitized_output.processing_results)); timings.saturating_add_in_place(ExecuteTimingType::CollectLogsUs, collect_logs_us); - let mut execution_counts = ExecutedTransactionCounts::default(); + let mut processed_counts = ProcessedTransactionCounts::default(); let err_count = &mut error_counters.total; - for (execution_result, tx) in sanitized_output.execution_results.iter().zip(sanitized_txs) { + for (processing_result, tx) in sanitized_output + .processing_results + .iter() + .zip(sanitized_txs) + { if let Some(debug_keys) = &self.transaction_debug_keys { for key in tx.message().account_keys().iter() { if debug_keys.contains(key) { - let result = execution_result.flattened_result(); + let result = processing_result.flattened_result(); info!("slot: {} result: {:?} tx: {:?}", self.slot, result, tx); break; } } } - if execution_result.was_executed() { + if processing_result.was_processed() { // Signature count must be accumulated only if the transaction - // is executed, otherwise a mismatched count between banking and - // replay could occur - execution_counts.signature_count += + // is processed, otherwise a mismatched count between banking + // and replay could occur + processed_counts.signature_count += u64::from(tx.message().header().num_required_signatures); - execution_counts.executed_transactions_count += 1; + processed_counts.processed_transactions_count += 1; if !tx.is_simple_vote_transaction() { - execution_counts.executed_non_vote_transactions_count += 1; + processed_counts.processed_non_vote_transactions_count += 1; } } - match execution_result.flattened_result() { + match processing_result.flattened_result() { Ok(()) => { - execution_counts.executed_successfully_count += 1; + processed_counts.processed_successfully += 1; } Err(err) => { if *err_count == 0 { @@ -3672,15 +3683,15 @@ impl Bank { } LoadAndExecuteTransactionsOutput { - execution_results: sanitized_output.execution_results, - execution_counts, + processing_results: sanitized_output.processing_results, + processed_counts, } } fn collect_logs( &self, transactions: &[SanitizedTransaction], - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) { let transaction_log_collector_config = self.transaction_log_collector_config.read().unwrap(); @@ -3688,12 +3699,14 @@ impl Bank { return; } - let collected_logs: Vec<_> = execution_results + let collected_logs: Vec<_> = processing_results .iter() .zip(transactions) - .filter_map(|(execution_result, transaction)| { + .filter_map(|(processing_result, transaction)| { // Skip log collection for unprocessed transactions - let execution_details = execution_result.details()?; + let processed_tx = processing_result.processed_transaction()?; + // Skip log collection for unexecuted transactions + let execution_details = processed_tx.execution_details()?; Self::collect_transaction_logs( &transaction_log_collector_config, transaction, @@ -3835,17 +3848,17 @@ impl Bank { fn filter_program_errors_and_collect_fee( &self, - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) { let mut fees = 0; - execution_results + processing_results .iter() - .for_each(|execution_result| match execution_result { - TransactionExecutionResult::Executed(executed_tx) => { - fees += executed_tx.loaded_transaction.fee_details.total_fee(); + .for_each(|processing_result| match processing_result { + Ok(processed_tx) => { + fees += processed_tx.fee_details().total_fee(); } - TransactionExecutionResult::NotExecuted(_) => {} + Err(_) => {} }); self.collector_fees.fetch_add(fees, Relaxed); @@ -3854,17 +3867,17 @@ impl Bank { // Note: this function is not yet used; next PR will call it behind a feature gate fn filter_program_errors_and_collect_fee_details( &self, - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) { let mut accumulated_fee_details = FeeDetails::default(); - execution_results + processing_results .iter() - .for_each(|execution_result| match execution_result { - TransactionExecutionResult::Executed(executed_tx) => { - accumulated_fee_details.accumulate(&executed_tx.loaded_transaction.fee_details); + .for_each(|processing_result| match processing_result { + Ok(processed_tx) => { + accumulated_fee_details.accumulate(&processed_tx.fee_details()); } - TransactionExecutionResult::NotExecuted(_) => {} + Err(_) => {} }); self.collector_fee_details @@ -3876,10 +3889,10 @@ impl Bank { pub fn commit_transactions( &self, sanitized_txs: &[SanitizedTransaction], - mut execution_results: Vec, + mut processing_results: Vec, last_blockhash: Hash, lamports_per_signature: u64, - execution_counts: &ExecutedTransactionCounts, + processed_counts: &ProcessedTransactionCounts, timings: &mut ExecuteTimings, ) -> Vec { assert!( @@ -3887,39 +3900,36 @@ impl Bank { "commit_transactions() working on a bank that is already frozen or is undergoing freezing!" ); - let ExecutedTransactionCounts { - executed_transactions_count, - executed_non_vote_transactions_count, - executed_successfully_count, + let ProcessedTransactionCounts { + processed_transactions_count, + processed_non_vote_transactions_count, + processed_successfully, signature_count, - } = *execution_counts; + } = *processed_counts; - self.increment_transaction_count(executed_transactions_count); + self.increment_transaction_count(processed_transactions_count); self.increment_non_vote_transaction_count_since_restart( - executed_non_vote_transactions_count, + processed_non_vote_transactions_count, ); self.increment_signature_count(signature_count); - let executed_with_failure_result_count = - executed_transactions_count.saturating_sub(executed_successfully_count); - if executed_with_failure_result_count > 0 { - self.transaction_error_count - .fetch_add(executed_with_failure_result_count, Relaxed); - } + let processed_with_failure_result_count = + processed_transactions_count.saturating_sub(processed_successfully); + self.transaction_error_count + .fetch_add(processed_with_failure_result_count, Relaxed); - // Should be equivalent to checking `executed_transactions_count > 0` - if execution_results.iter().any(|result| result.was_executed()) { + if processed_transactions_count > 0 { self.is_delta.store(true, Relaxed); self.transaction_entries_count.fetch_add(1, Relaxed); self.transactions_per_entry_max - .fetch_max(executed_transactions_count, Relaxed); + .fetch_max(processed_transactions_count, Relaxed); } let ((), store_accounts_us) = measure_us!({ let durable_nonce = DurableNonce::from_blockhash(&last_blockhash); let (accounts_to_store, transactions) = collect_accounts_to_store( sanitized_txs, - &mut execution_results, + &mut processing_results, &durable_nonce, lamports_per_signature, ); @@ -3928,17 +3938,19 @@ impl Bank { .store_cached((self.slot(), accounts_to_store.as_slice()), &transactions); }); - self.collect_rent(&execution_results); + self.collect_rent(&processing_results); // Cached vote and stake accounts are synchronized with accounts-db // after each transaction. let ((), update_stakes_cache_us) = - measure_us!(self.update_stakes_cache(sanitized_txs, &execution_results)); + measure_us!(self.update_stakes_cache(sanitized_txs, &processing_results)); let ((), update_executors_us) = measure_us!({ let mut cache = None; - for execution_result in &execution_results { - if let TransactionExecutionResult::Executed(executed_tx) = execution_result { + for processing_result in &processing_results { + if let Some(ProcessedTransaction::Executed(executed_tx)) = + processing_result.processed_transaction() + { let programs_modified_by_tx = &executed_tx.programs_modified_by_tx; if executed_tx.was_successful() && !programs_modified_by_tx.is_empty() { cache @@ -3951,9 +3963,10 @@ impl Bank { } }); - let accounts_data_len_delta = execution_results + let accounts_data_len_delta = processing_results .iter() - .filter_map(TransactionExecutionResult::details) + .filter_map(|processing_result| processing_result.processed_transaction()) + .filter_map(|processed_tx| processed_tx.execution_details()) .filter_map(|details| { details .status @@ -3964,12 +3977,12 @@ impl Bank { self.update_accounts_data_size_delta_on_chain(accounts_data_len_delta); let ((), update_transaction_statuses_us) = - measure_us!(self.update_transaction_statuses(sanitized_txs, &execution_results)); + measure_us!(self.update_transaction_statuses(sanitized_txs, &processing_results)); if self.feature_set.is_active(&reward_full_priority_fee::id()) { - self.filter_program_errors_and_collect_fee_details(&execution_results) + self.filter_program_errors_and_collect_fee_details(&processing_results) } else { - self.filter_program_errors_and_collect_fee(&execution_results) + self.filter_program_errors_and_collect_fee(&processing_results) }; timings.saturating_add_in_place(ExecuteTimingType::StoreUs, store_accounts_us); @@ -3983,45 +3996,71 @@ impl Bank { update_transaction_statuses_us, ); - Self::create_commit_results(execution_results) + Self::create_commit_results(processing_results) } fn create_commit_results( - execution_results: Vec, + processing_results: Vec, ) -> Vec { - execution_results + processing_results .into_iter() - .map(|execution_result| match execution_result { - TransactionExecutionResult::Executed(executed_tx) => { - let loaded_tx = &executed_tx.loaded_transaction; - let loaded_account_stats = TransactionLoadedAccountsStats { - loaded_accounts_data_size: loaded_tx.loaded_accounts_data_size, - loaded_accounts_count: loaded_tx.accounts.len(), - }; - - // Rent is only collected for successfully executed transactions - let rent_debits = if executed_tx.was_successful() { - executed_tx.loaded_transaction.rent_debits - } else { - RentDebits::default() - }; - - Ok(CommittedTransaction { - loaded_account_stats, - execution_details: executed_tx.execution_details, - fee_details: executed_tx.loaded_transaction.fee_details, - rent_debits, - }) + .map(|processing_result| { + match processing_result? { + ProcessedTransaction::Executed(executed_tx) => { + let execution_details = executed_tx.execution_details; + let LoadedTransaction { + rent_debits, + accounts: loaded_accounts, + loaded_accounts_data_size, + fee_details, + .. + } = executed_tx.loaded_transaction; + + // Rent is only collected for successfully executed transactions + let rent_debits = if execution_details.was_successful() { + rent_debits + } else { + RentDebits::default() + }; + + Ok(CommittedTransaction { + status: execution_details.status, + log_messages: execution_details.log_messages, + inner_instructions: execution_details.inner_instructions, + return_data: execution_details.return_data, + executed_units: execution_details.executed_units, + fee_details, + rent_debits, + loaded_account_stats: TransactionLoadedAccountsStats { + loaded_accounts_count: loaded_accounts.len(), + loaded_accounts_data_size, + }, + }) + } + ProcessedTransaction::FeesOnly(fees_only_tx) => Ok(CommittedTransaction { + status: Err(fees_only_tx.load_error), + log_messages: None, + inner_instructions: None, + return_data: None, + executed_units: 0, + rent_debits: RentDebits::default(), + fee_details: fees_only_tx.fee_details, + loaded_account_stats: TransactionLoadedAccountsStats { + loaded_accounts_count: fees_only_tx.rollback_accounts.count(), + loaded_accounts_data_size: fees_only_tx.rollback_accounts.data_size() + as u32, + }, + }), } - TransactionExecutionResult::NotExecuted(err) => Err(err), }) .collect() } - fn collect_rent(&self, execution_results: &[TransactionExecutionResult]) { - let collected_rent = execution_results + fn collect_rent(&self, processing_results: &[TransactionProcessingResult]) { + let collected_rent = processing_results .iter() - .filter_map(|executed_result| executed_result.executed_transaction()) + .filter_map(|processing_result| processing_result.processed_transaction()) + .filter_map(|processed_tx| processed_tx.executed_transaction()) .filter(|executed_tx| executed_tx.was_successful()) .map(|executed_tx| executed_tx.loaded_transaction.rent) .sum(); @@ -4694,8 +4733,8 @@ impl Bank { }; let LoadAndExecuteTransactionsOutput { - execution_results, - execution_counts, + processing_results, + processed_counts, } = self.load_and_execute_transactions( batch, max_age, @@ -4716,10 +4755,10 @@ impl Bank { self.last_blockhash_and_lamports_per_signature(); let commit_results = self.commit_transactions( batch.sanitized_transactions(), - execution_results, + processing_results, last_blockhash, lamports_per_signature, - &execution_counts, + &processed_counts, timings, ); let post_balances = if collect_balances { @@ -4747,7 +4786,7 @@ impl Bank { pub fn process_transaction_with_metadata( &self, tx: impl Into, - ) -> Result { + ) -> Result { let txs = vec![tx.into()]; let batch = self.prepare_entry_batch(txs)?; @@ -4764,8 +4803,7 @@ impl Bank { Some(1000 * 1000), ); - let committed_tx = commit_results.remove(0)?; - Ok(committed_tx.execution_details) + commit_results.remove(0) } /// Process multiple transaction in a single batch. This is used for benches and unit tests. @@ -5959,14 +5997,19 @@ impl Bank { fn update_stakes_cache( &self, txs: &[SanitizedTransaction], - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) { - debug_assert_eq!(txs.len(), execution_results.len()); + debug_assert_eq!(txs.len(), processing_results.len()); let new_warmup_cooldown_rate_epoch = self.new_warmup_cooldown_rate_epoch(); txs.iter() - .zip(execution_results) - .filter_map(|(tx, execution_result)| { - execution_result + .zip(processing_results) + .filter_map(|(tx, processing_result)| { + processing_result + .processed_transaction() + .map(|processed_tx| (tx, processed_tx)) + }) + .filter_map(|(tx, processed_tx)| { + processed_tx .executed_transaction() .map(|executed_tx| (tx, executed_tx)) }) diff --git a/runtime/src/bank/bank_hash_details.rs b/runtime/src/bank/bank_hash_details.rs index a6c449a50c72b8..5ab13d85c4d89b 100644 --- a/runtime/src/bank/bank_hash_details.rs +++ b/runtime/src/bank/bank_hash_details.rs @@ -15,10 +15,14 @@ use { solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount}, clock::{Epoch, Slot}, + fee::FeeDetails, hash::Hash, + inner_instruction::InnerInstructionsList, pubkey::Pubkey, + transaction::Result as TransactionResult, + transaction_context::TransactionReturnData, }, - solana_svm::transaction_execution_result::TransactionExecutionDetails, + solana_svm::transaction_commit_result::CommittedTransaction, solana_transaction_status::UiInstruction, std::str::FromStr, }; @@ -74,7 +78,30 @@ pub struct TransactionDetails { pub accounts: Vec, pub instructions: Vec, pub is_simple_vote_tx: bool, - pub execution_results: Option, + pub commit_details: Option, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct TransactionCommitDetails { + pub status: TransactionResult<()>, + pub log_messages: Option>, + pub inner_instructions: Option, + pub return_data: Option, + pub executed_units: u64, + pub fee_details: FeeDetails, +} + +impl From for TransactionCommitDetails { + fn from(committed_tx: CommittedTransaction) -> Self { + Self { + status: committed_tx.status, + log_messages: committed_tx.log_messages, + inner_instructions: committed_tx.inner_instructions, + return_data: committed_tx.return_data, + executed_units: committed_tx.executed_units, + fee_details: committed_tx.fee_details, + } + } } /// The components that go into a bank hash calculation for a single bank/slot. diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 887d2e547f19b2..466db24abe0f46 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -725,6 +725,10 @@ pub mod disable_rent_fees_collection { solana_sdk::declare_id!("CJzY83ggJHqPGDq8VisV3U91jDJLuEaALZooBrXtnnLU"); } +pub mod enable_transaction_failure_fees { + solana_sdk::declare_id!("My11111111111111111111111111111111111111111"); +} + pub mod enable_zk_transfer_with_fee { solana_sdk::declare_id!("zkNLP7EQALfC1TYeB3biDU7akDckj8iPkvh9y2Mt2K3"); } @@ -1016,6 +1020,7 @@ lazy_static! { (update_hashes_per_tick6::id(), "Update desired hashes per tick to 10M"), (validate_fee_collector_account::id(), "validate fee collector account #33888"), (disable_rent_fees_collection::id(), "Disable rent fees collection #33945"), + (enable_transaction_failure_fees::id(), "Enable fees for some additional transaction failures #XXXXX"), (enable_zk_transfer_with_fee::id(), "enable Zk Token proof program transfer with fee"), (drop_legacy_shreds::id(), "drops legacy shreds #34328"), (allow_commission_decrease_at_any_time::id(), "Allow commission decrease at any time in epoch #33843"), diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index b62833e404004b..95ffde27255cd0 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -35,7 +35,11 @@ pub(crate) type TransactionRent = u64; pub(crate) type TransactionProgramIndices = Vec>; pub type TransactionCheckResult = Result; pub type TransactionValidationResult = Result; -pub type TransactionLoadResult = Result; +pub enum TransactionLoadResult { + Loaded(LoadedTransaction), + FeesOnly(FeesOnlyTransaction), + NotLoaded(TransactionError), +} #[derive(PartialEq, Eq, Debug, Clone)] pub struct CheckedTransactionDetails { @@ -66,6 +70,13 @@ pub struct LoadedTransaction { pub loaded_accounts_data_size: u32, } +#[derive(PartialEq, Eq, Debug, Clone)] +pub struct FeesOnlyTransaction { + pub load_error: TransactionError, + pub rollback_accounts: RollbackAccounts, + pub fee_details: FeeDetails, +} + /// Collect rent from an account if rent is still enabled and regardless of /// whether rent is enabled, set the rent epoch to u64::MAX if the account is /// rent exempt. @@ -167,35 +178,85 @@ pub(crate) fn load_accounts( ) -> Vec { txs.iter() .zip(validation_results) - .map(|etx| match etx { - (tx, Ok(tx_details)) => { - // load transactions - load_transaction_accounts( - callbacks, - tx, - tx_details, - error_metrics, - account_overrides, - feature_set, - rent_collector, - loaded_programs, - ) - } - (_, Err(e)) => Err(e), + .map(|(transaction, validation_result)| { + load_transaction( + callbacks, + transaction, + validation_result, + error_metrics, + account_overrides, + feature_set, + rent_collector, + loaded_programs, + ) }) .collect() } +fn load_transaction( + callbacks: &CB, + message: &impl SVMMessage, + validation_result: TransactionValidationResult, + error_metrics: &mut TransactionErrorMetrics, + account_overrides: Option<&AccountOverrides>, + feature_set: &FeatureSet, + rent_collector: &RentCollector, + loaded_programs: &ProgramCacheForTxBatch, +) -> TransactionLoadResult { + match validation_result { + Err(e) => TransactionLoadResult::NotLoaded(e), + Ok(tx_details) => { + let load_result = load_transaction_accounts( + callbacks, + message, + &tx_details, + error_metrics, + account_overrides, + feature_set, + rent_collector, + loaded_programs, + ); + + match load_result { + Ok(loaded_tx_accounts) => TransactionLoadResult::Loaded(LoadedTransaction { + accounts: loaded_tx_accounts.accounts, + program_indices: loaded_tx_accounts.program_indices, + fee_details: tx_details.fee_details, + rent: loaded_tx_accounts.rent, + rent_debits: loaded_tx_accounts.rent_debits, + rollback_accounts: tx_details.rollback_accounts, + compute_budget_limits: tx_details.compute_budget_limits, + loaded_accounts_data_size: loaded_tx_accounts.loaded_accounts_data_size, + }), + Err(err) => TransactionLoadResult::FeesOnly(FeesOnlyTransaction { + load_error: err, + fee_details: tx_details.fee_details, + rollback_accounts: tx_details.rollback_accounts, + }), + } + } + } +} + +#[derive(PartialEq, Eq, Debug, Clone)] +struct LoadedTransactionAccounts { + pub accounts: Vec, + pub program_indices: TransactionProgramIndices, + pub rent: TransactionRent, + pub rent_debits: RentDebits, + pub loaded_accounts_data_size: u32, +} + fn load_transaction_accounts( callbacks: &CB, message: &impl SVMMessage, - tx_details: ValidatedTransactionDetails, + tx_details: &ValidatedTransactionDetails, error_metrics: &mut TransactionErrorMetrics, account_overrides: Option<&AccountOverrides>, feature_set: &FeatureSet, rent_collector: &RentCollector, loaded_programs: &ProgramCacheForTxBatch, -) -> Result { +) -> Result { let mut tx_rent: TransactionRent = 0; let account_keys = message.account_keys(); let mut accounts_found = Vec::with_capacity(account_keys.len()); @@ -346,12 +407,9 @@ fn load_transaction_accounts( }) .collect::>>>()?; - Ok(LoadedTransaction { + Ok(LoadedTransactionAccounts { accounts, program_indices, - fee_details: tx_details.fee_details, - rollback_accounts: tx_details.rollback_accounts, - compute_budget_limits: tx_details.compute_budget_limits, rent: tx_rent, rent_debits, loaded_accounts_data_size: accumulated_accounts_data_size, diff --git a/svm/src/account_saver.rs b/svm/src/account_saver.rs index 4ba2ec259fd87b..a1c7c3d44d679f 100644 --- a/svm/src/account_saver.rs +++ b/svm/src/account_saver.rs @@ -1,7 +1,10 @@ use { crate::{ rollback_accounts::RollbackAccounts, - transaction_execution_result::TransactionExecutionResult, + transaction_processing_result::{ + ProcessedTransaction, TransactionProcessingResult, + TransactionProcessingResultExtensions, + }, }, solana_sdk::{ account::AccountSharedData, nonce::state::DurableNonce, pubkey::Pubkey, @@ -14,59 +17,76 @@ use { // optimization edge cases where some write locked accounts have skip storage. fn max_number_of_accounts_to_collect( txs: &[SanitizedTransaction], - execution_results: &[TransactionExecutionResult], + processing_results: &[TransactionProcessingResult], ) -> usize { - execution_results + processing_results .iter() .zip(txs) - .filter_map(|(execution_result, tx)| { - execution_result - .executed_transaction() - .map(|executed_tx| (executed_tx, tx)) + .filter_map(|(processing_result, tx)| { + processing_result + .processed_transaction() + .map(|processed_tx| (processed_tx, tx)) + }) + .map(|(processed_tx, tx)| match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + match executed_tx.execution_details.status { + Ok(_) => tx.message().num_write_locks() as usize, + Err(_) => executed_tx.loaded_transaction.rollback_accounts.count(), + } + } + ProcessedTransaction::FeesOnly(fees_only_tx) => fees_only_tx.rollback_accounts.count(), }) - .map( - |(executed_tx, tx)| match executed_tx.execution_details.status { - Ok(_) => tx.message().num_write_locks() as usize, - Err(_) => executed_tx.loaded_transaction.rollback_accounts.count(), - }, - ) .sum() } pub fn collect_accounts_to_store<'a>( txs: &'a [SanitizedTransaction], - execution_results: &'a mut [TransactionExecutionResult], + processing_results: &'a mut [TransactionProcessingResult], durable_nonce: &DurableNonce, lamports_per_signature: u64, ) -> ( Vec<(&'a Pubkey, &'a AccountSharedData)>, Vec>, ) { - let collect_capacity = max_number_of_accounts_to_collect(txs, execution_results); + let collect_capacity = max_number_of_accounts_to_collect(txs, processing_results); let mut accounts = Vec::with_capacity(collect_capacity); let mut transactions = Vec::with_capacity(collect_capacity); - for (execution_result, tx) in execution_results.iter_mut().zip(txs) { - let Some(executed_tx) = execution_result.executed_transaction_mut() else { + for (processing_result, transaction) in processing_results.iter_mut().zip(txs) { + let Some(processed_tx) = processing_result.processed_transaction_mut() else { // Don't store any accounts if tx wasn't executed continue; }; - if executed_tx.execution_details.status.is_ok() { - collect_accounts_for_successful_tx( - &mut accounts, - &mut transactions, - tx, - &executed_tx.loaded_transaction.accounts, - ); - } else { - collect_accounts_for_failed_tx( - &mut accounts, - &mut transactions, - tx, - &mut executed_tx.loaded_transaction.rollback_accounts, - durable_nonce, - lamports_per_signature, - ); + match processed_tx { + ProcessedTransaction::Executed(executed_tx) => { + if executed_tx.execution_details.status.is_ok() { + collect_accounts_for_successful_tx( + &mut accounts, + &mut transactions, + transaction, + &executed_tx.loaded_transaction.accounts, + ); + } else { + collect_accounts_for_failed_tx( + &mut accounts, + &mut transactions, + transaction, + &mut executed_tx.loaded_transaction.rollback_accounts, + durable_nonce, + lamports_per_signature, + ); + } + } + ProcessedTransaction::FeesOnly(fees_only_tx) => { + collect_accounts_for_failed_tx( + &mut accounts, + &mut transactions, + transaction, + &mut fees_only_tx.rollback_accounts, + durable_nonce, + lamports_per_signature, + ); + } } } (accounts, transactions) diff --git a/svm/src/lib.rs b/svm/src/lib.rs index cbfef2305e41f9..b031ce7d6e1c53 100644 --- a/svm/src/lib.rs +++ b/svm/src/lib.rs @@ -15,6 +15,7 @@ pub mod transaction_commit_result; pub mod transaction_error_metrics; pub mod transaction_execution_result; pub mod transaction_processing_callback; +pub mod transaction_processing_result; pub mod transaction_processor; #[macro_use] diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index 71b670d37c4f85..60afa7c45a93f8 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -79,6 +79,19 @@ impl RollbackAccounts { Self::SeparateNonceAndFeePayer { .. } => 2, } } + + /// Size of accounts tracked for rollback, used when calculating the actual + /// cost of transaction processing in the cost model. + pub fn data_size(&self) -> usize { + match self { + Self::FeePayerOnly { fee_payer_account } => fee_payer_account.data().len(), + Self::SameNonceAndFeePayer { nonce } => nonce.account().data().len(), + Self::SeparateNonceAndFeePayer { + nonce, + fee_payer_account, + } => fee_payer_account.data().len() + nonce.account().data().len(), + } + } } #[cfg(test)] diff --git a/svm/src/transaction_commit_result.rs b/svm/src/transaction_commit_result.rs index 5cc413d7b175f9..8bbada73634ad9 100644 --- a/svm/src/transaction_commit_result.rs +++ b/svm/src/transaction_commit_result.rs @@ -1,9 +1,8 @@ use { - crate::transaction_execution_result::{ - TransactionExecutionDetails, TransactionLoadedAccountsStats, - }, + crate::transaction_execution_result::TransactionLoadedAccountsStats, solana_sdk::{ - fee::FeeDetails, rent_debits::RentDebits, transaction::Result as TransactionResult, + fee::FeeDetails, inner_instruction::InnerInstructionsList, rent_debits::RentDebits, + transaction::Result as TransactionResult, transaction_context::TransactionReturnData, }, }; @@ -11,33 +10,30 @@ pub type TransactionCommitResult = TransactionResult; #[derive(Clone, Debug)] pub struct CommittedTransaction { - pub loaded_account_stats: TransactionLoadedAccountsStats, - pub execution_details: TransactionExecutionDetails, + pub status: TransactionResult<()>, + pub log_messages: Option>, + pub inner_instructions: Option, + pub return_data: Option, + pub executed_units: u64, pub fee_details: FeeDetails, pub rent_debits: RentDebits, + pub loaded_account_stats: TransactionLoadedAccountsStats, } pub trait TransactionCommitResultExtensions { - fn was_executed(&self) -> bool; + fn was_committed(&self) -> bool; fn was_executed_successfully(&self) -> bool; - fn transaction_result(&self) -> TransactionResult<()>; } impl TransactionCommitResultExtensions for TransactionCommitResult { - fn was_executed(&self) -> bool { + fn was_committed(&self) -> bool { self.is_ok() } fn was_executed_successfully(&self) -> bool { match self { - Ok(committed_tx) => committed_tx.execution_details.status.is_ok(), + Ok(committed_tx) => committed_tx.status.is_ok(), Err(_) => false, } } - - fn transaction_result(&self) -> TransactionResult<()> { - self.as_ref() - .map_err(|err| err.clone()) - .and_then(|committed_tx| committed_tx.execution_details.status.clone()) - } } diff --git a/svm/src/transaction_execution_result.rs b/svm/src/transaction_execution_result.rs index a7c965fbed01bd..4be551de4e223d 100644 --- a/svm/src/transaction_execution_result.rs +++ b/svm/src/transaction_execution_result.rs @@ -6,13 +6,8 @@ pub use solana_sdk::inner_instruction::{InnerInstruction, InnerInstructionsList}; use { crate::account_loader::LoadedTransaction, - serde::{Deserialize, Serialize}, solana_program_runtime::loaded_programs::ProgramCacheEntry, - solana_sdk::{ - pubkey::Pubkey, - transaction::{self, TransactionError}, - transaction_context::TransactionReturnData, - }, + solana_sdk::{pubkey::Pubkey, transaction, transaction_context::TransactionReturnData}, std::{collections::HashMap, sync::Arc}, }; @@ -22,22 +17,6 @@ pub struct TransactionLoadedAccountsStats { pub loaded_accounts_count: usize, } -/// Type safe representation of a transaction execution attempt which -/// differentiates between a transaction that was executed (will be -/// committed to the ledger) and a transaction which wasn't executed -/// and will be dropped. -/// -/// Note: `Result` is not -/// used because it's easy to forget that the inner `details.status` field -/// is what should be checked to detect a successful transaction. This -/// enum provides a convenience method `Self::was_executed_successfully` to -/// make such checks hard to do incorrectly. -#[derive(Debug, Clone)] -pub enum TransactionExecutionResult { - Executed(Box), - NotExecuted(TransactionError), -} - #[derive(Debug, Clone)] pub struct ExecutedTransaction { pub loaded_transaction: LoadedTransaction, @@ -47,49 +26,11 @@ pub struct ExecutedTransaction { impl ExecutedTransaction { pub fn was_successful(&self) -> bool { - self.execution_details.status.is_ok() - } -} - -impl TransactionExecutionResult { - pub fn was_executed_successfully(&self) -> bool { - self.executed_transaction() - .map(|executed_tx| executed_tx.was_successful()) - .unwrap_or(false) - } - - pub fn was_executed(&self) -> bool { - self.executed_transaction().is_some() - } - - pub fn details(&self) -> Option<&TransactionExecutionDetails> { - self.executed_transaction() - .map(|executed_tx| &executed_tx.execution_details) - } - - pub fn flattened_result(&self) -> transaction::Result<()> { - match self { - Self::Executed(executed_tx) => executed_tx.execution_details.status.clone(), - Self::NotExecuted(err) => Err(err.clone()), - } - } - - pub fn executed_transaction(&self) -> Option<&ExecutedTransaction> { - match self { - Self::Executed(executed_tx) => Some(executed_tx.as_ref()), - Self::NotExecuted { .. } => None, - } - } - - pub fn executed_transaction_mut(&mut self) -> Option<&mut ExecutedTransaction> { - match self { - Self::Executed(executed_tx) => Some(executed_tx.as_mut()), - Self::NotExecuted { .. } => None, - } + self.execution_details.was_successful() } } -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct TransactionExecutionDetails { pub status: transaction::Result<()>, pub log_messages: Option>, @@ -100,3 +41,9 @@ pub struct TransactionExecutionDetails { /// NOTE: This value is valid IFF `status` is `Ok`. pub accounts_data_len_delta: i64, } + +impl TransactionExecutionDetails { + pub fn was_successful(&self) -> bool { + self.status.is_ok() + } +} diff --git a/svm/src/transaction_processing_result.rs b/svm/src/transaction_processing_result.rs new file mode 100644 index 00000000000000..e000dd3ddbe86e --- /dev/null +++ b/svm/src/transaction_processing_result.rs @@ -0,0 +1,80 @@ +use { + crate::{ + account_loader::FeesOnlyTransaction, + transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, + }, + solana_sdk::{ + fee::FeeDetails, + transaction::{Result as TransactionResult, TransactionError}, + }, +}; + +pub type TransactionProcessingResult = TransactionResult; + +pub trait TransactionProcessingResultExtensions { + fn was_processed(&self) -> bool; + fn processed_transaction(&self) -> Option<&ProcessedTransaction>; + fn processed_transaction_mut(&mut self) -> Option<&mut ProcessedTransaction>; + fn flattened_result(&self) -> TransactionResult<()>; +} + +pub enum ProcessedTransaction { + Executed(Box), + FeesOnly(Box), +} + +impl TransactionProcessingResultExtensions for TransactionProcessingResult { + fn was_processed(&self) -> bool { + self.is_ok() + } + + fn processed_transaction(&self) -> Option<&ProcessedTransaction> { + match self { + Ok(processed_tx) => Some(processed_tx), + Err(_) => None, + } + } + + fn processed_transaction_mut(&mut self) -> Option<&mut ProcessedTransaction> { + match self { + Ok(processed_tx) => Some(processed_tx), + Err(_) => None, + } + } + + fn flattened_result(&self) -> TransactionResult<()> { + self.as_ref() + .map_err(|err| err.clone()) + .and_then(|processed_tx| processed_tx.status()) + } +} + +impl ProcessedTransaction { + pub fn status(&self) -> TransactionResult<()> { + match self { + Self::Executed(executed_tx) => executed_tx.execution_details.status.clone(), + Self::FeesOnly(details) => Err(TransactionError::clone(&details.load_error)), + } + } + + pub fn fee_details(&self) -> FeeDetails { + match self { + Self::Executed(executed_tx) => executed_tx.loaded_transaction.fee_details, + Self::FeesOnly(details) => details.fee_details, + } + } + + pub fn executed_transaction(&self) -> Option<&ExecutedTransaction> { + match self { + Self::Executed(context) => Some(context), + Self::FeesOnly { .. } => None, + } + } + + pub fn execution_details(&self) -> Option<&TransactionExecutionDetails> { + match self { + Self::Executed(context) => Some(&context.execution_details), + Self::FeesOnly { .. } => None, + } + } +} diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index c2fb997b3f522f..b4e95992dee09b 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -5,7 +5,7 @@ use { account_loader::{ collect_rent_from_account, load_accounts, validate_fee_payer, CheckedTransactionDetails, LoadedTransaction, TransactionCheckResult, - TransactionValidationResult, ValidatedTransactionDetails, + TransactionLoadResult, TransactionValidationResult, ValidatedTransactionDetails, }, account_overrides::AccountOverrides, message_processor::MessageProcessor, @@ -13,10 +13,9 @@ use { rollback_accounts::RollbackAccounts, transaction_account_state_info::TransactionAccountStateInfo, transaction_error_metrics::TransactionErrorMetrics, - transaction_execution_result::{ - ExecutedTransaction, TransactionExecutionDetails, TransactionExecutionResult, - }, + transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, transaction_processing_callback::TransactionProcessingCallback, + transaction_processing_result::{ProcessedTransaction, TransactionProcessingResult}, }, log::debug, percentage::Percentage, @@ -37,7 +36,7 @@ use { solana_sdk::{ account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, clock::{Epoch, Slot}, - feature_set::{remove_rounding_in_fee_calculation, FeatureSet}, + feature_set::{self, remove_rounding_in_fee_calculation, FeatureSet}, fee::{FeeBudgetLimits, FeeStructure}, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, @@ -71,7 +70,7 @@ pub struct LoadAndExecuteSanitizedTransactionsOutput { pub execute_timings: ExecuteTimings, // Vector of results indicating whether a transaction was executed or could not // be executed. Note executed transactions can still have failed! - pub execution_results: Vec, + pub processing_results: Vec, } /// Configuration of the recording capabilities for transaction execution @@ -267,13 +266,12 @@ impl TransactionBatchProcessor { ); if program_cache_for_tx_batch.hit_max_limit { - const ERROR: TransactionError = TransactionError::ProgramCacheHitMaxLimit; - let execution_results = - vec![TransactionExecutionResult::NotExecuted(ERROR); sanitized_txs.len()]; return LoadAndExecuteSanitizedTransactionsOutput { error_metrics, execute_timings, - execution_results, + processing_results: (0..sanitized_txs.len()) + .map(|_| Err(TransactionError::ProgramCacheHitMaxLimit)) + .collect(), }; } @@ -293,13 +291,23 @@ impl TransactionBatchProcessor { &program_cache_for_tx_batch, )); - let (execution_results, execution_us): (Vec, u64) = + let enable_transaction_failure_fees = environment + .feature_set + .is_active(&feature_set::enable_transaction_failure_fees::id()); + let (processing_results, execution_us): (Vec, u64) = measure_us!(loaded_transactions .into_iter() .zip(sanitized_txs.iter()) .map(|(load_result, tx)| match load_result { - Err(e) => TransactionExecutionResult::NotExecuted(e.clone()), - Ok(loaded_transaction) => { + TransactionLoadResult::NotLoaded(err) => Err(err), + TransactionLoadResult::FeesOnly(fees_only_tx) => { + if enable_transaction_failure_fees { + Ok(ProcessedTransaction::FeesOnly(Box::new(fees_only_tx))) + } else { + Err(fees_only_tx.load_error) + } + } + TransactionLoadResult::Loaded(loaded_transaction) => { let executed_tx = self.execute_loaded_transaction( tx, loaded_transaction, @@ -316,7 +324,7 @@ impl TransactionBatchProcessor { program_cache_for_tx_batch.merge(&executed_tx.programs_modified_by_tx); } - TransactionExecutionResult::Executed(Box::new(executed_tx)) + Ok(ProcessedTransaction::Executed(Box::new(executed_tx))) } }) .collect()); @@ -353,7 +361,7 @@ impl TransactionBatchProcessor { LoadAndExecuteSanitizedTransactionsOutput { error_metrics, execute_timings, - execution_results, + processing_results, } }