Refactor: Improve type safety and readability of transaction execution (backport #22215)

core/src/banking_stage.rs

@@ -804,22 +804,23 @@ impl BankingStage {
     fn record_transactions(
         bank_slot: Slot,
         txs: &[SanitizedTransaction],
-        results: &[TransactionExecutionResult],
+        execution_results: &[TransactionExecutionResult],
         recorder: &TransactionRecorder,
     ) -> (Result<usize, PohRecorderError>, Vec<usize>) {
         let mut processed_generation = Measure::start("record::process_generation");
-        let (processed_transactions, processed_transactions_indexes): (Vec<_>, Vec<_>) = results
-            .iter()
-            .zip(txs)
-            .enumerate()
-            .filter_map(|(i, ((r, _n), tx))| {
-                if Bank::can_commit(r) {
-                    Some((tx.to_versioned_transaction(), i))
-                } else {
-                    None
-                }
-            })
-            .unzip();
+        let (processed_transactions, processed_transactions_indexes): (Vec<_>, Vec<_>) =
+            execution_results
+                .iter()
+                .zip(txs)
+                .enumerate()
+                .filter_map(|(i, (execution_result, tx))| {
+                    if execution_result.was_executed() {
+                        Some((tx.to_versioned_transaction(), i))
+                    } else {
+                        None
+                    }
+                })
+                .unzip();
 
         processed_generation.stop();
         let num_to_commit = processed_transactions.len();
@@ -885,28 +886,25 @@ impl BankingStage {
         };
 
         let mut execute_timings = ExecuteTimings::default();
-        let (
-            mut loaded_accounts,
-            results,
-            inner_instructions,
-            transaction_logs,
-            mut retryable_txs,
-            tx_count,
-            signature_count,
-        ) = bank.load_and_execute_transactions(
-            batch,
-            MAX_PROCESSING_AGE,
-            transaction_status_sender.is_some(),
-            transaction_status_sender.is_some(),
-            &mut execute_timings,
-        );
+        let (mut loaded_accounts, execution_results, mut retryable_txs, tx_count, signature_count) =
+            bank.load_and_execute_transactions(
+                batch,
+                MAX_PROCESSING_AGE,
+                transaction_status_sender.is_some(),
+                transaction_status_sender.is_some(),
+                &mut execute_timings,
+            );
         load_execute_time.stop();
 
         let freeze_lock = bank.freeze_lock();
 
         let mut record_time = Measure::start("record_time");
-        let (num_to_commit, retryable_record_txs) =
-            Self::record_transactions(bank.slot(), batch.sanitized_transactions(), &results, poh);
+        let (num_to_commit, retryable_record_txs) = Self::record_transactions(
+            bank.slot(),
+            batch.sanitized_transactions(),
+            &execution_results,
+            poh,
+        );
         inc_new_counter_info!(
             "banking_stage-record_transactions_num_to_commit",
             *num_to_commit.as_ref().unwrap_or(&0)
@@ -928,7 +926,7 @@ impl BankingStage {
             let tx_results = bank.commit_transactions(
                 sanitized_txs,
                 &mut loaded_accounts,
-                &results,
+                execution_results,
                 tx_count,
                 signature_count,
                 &mut execute_timings,
@@ -945,8 +943,6 @@ impl BankingStage {
                     tx_results.execution_results,
                     TransactionBalancesSet::new(pre_balances, post_balances),
                     TransactionTokenBalancesSet::new(pre_token_balances, post_token_balances),
-                    inner_instructions,
-                    transaction_logs,
                     tx_results.rent_debits,
                 );
             }
@@ -1609,6 +1605,7 @@ mod tests {
             poh_service::PohService,
         },
         solana_rpc::transaction_status_service::TransactionStatusService,
+        solana_runtime::bank::TransactionExecutionDetails,
         solana_sdk::{
             hash::Hash,
             instruction::InstructionError,
@@ -1640,6 +1637,15 @@ mod tests {
         )
     }
 
+    fn new_execution_result(status: Result<(), TransactionError>) -> TransactionExecutionResult {
+        TransactionExecutionResult::Executed(TransactionExecutionDetails {
+            status,
+            log_messages: None,
+            inner_instructions: None,
+            durable_nonce_fee: None,
+        })
+    }
+
     #[test]
     fn test_banking_stage_shutdown1() {
         let genesis_config = create_genesis_config(2).genesis_config;
@@ -2027,19 +2033,16 @@ mod tests {
                 system_transaction::transfer(&keypair2, &pubkey2, 1, genesis_config.hash()),
             ]);
 
-            let mut results = vec![(Ok(()), None), (Ok(()), None)];
+            let mut results = vec![new_execution_result(Ok(())); 2];
             let _ = BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder);
             let (_bank, (entry, _tick_height)) = entry_receiver.recv().unwrap();
             assert_eq!(entry.transactions.len(), txs.len());
 
             // InstructionErrors should still be recorded
-            results[0] = (
-                Err(TransactionError::InstructionError(
-                    1,
-                    SystemError::ResultWithNegativeLamports.into(),
-                )),
-                None,
-            );
+            results[0] = new_execution_result(Err(TransactionError::InstructionError(
+                1,
+                SystemError::ResultWithNegativeLamports.into(),
+            )));
             let (res, retryable) =
                 BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder);
             res.unwrap();
@@ -2048,7 +2051,7 @@ mod tests {
             assert_eq!(entry.transactions.len(), txs.len());
 
             // Other TransactionErrors should not be recorded
-            results[0] = (Err(TransactionError::AccountNotFound), None);
+            results[0] = TransactionExecutionResult::NotExecuted(TransactionError::AccountNotFound);
             let (res, retryable) =
                 BankingStage::record_transactions(bank.slot(), &txs, &results, &recorder);
             res.unwrap();

ledger/src/blockstore_processor.rs

@@ -20,8 +20,8 @@ use {
         accounts_index::AccountSecondaryIndexes,
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         bank::{
-            Bank, ExecuteTimings, InnerInstructionsList, RentDebits, TransactionBalancesSet,
-            TransactionExecutionResult, TransactionLogMessages, TransactionResults,
+            Bank, ExecuteTimings, RentDebits, TransactionBalancesSet, TransactionExecutionResult,
+            TransactionResults,
         },
         bank_forks::BankForks,
         bank_utils,
@@ -175,15 +175,14 @@ fn execute_batch(
 
     let pre_process_units: u64 = aggregate_total_execution_units(timings);
 
-    let (tx_results, balances, inner_instructions, transaction_logs) =
-        batch.bank().load_execute_and_commit_transactions(
-            batch,
-            MAX_PROCESSING_AGE,
-            transaction_status_sender.is_some(),
-            transaction_status_sender.is_some(),
-            transaction_status_sender.is_some(),
-            timings,
-        );
+    let (tx_results, balances) = batch.bank().load_execute_and_commit_transactions(
+        batch,
+        MAX_PROCESSING_AGE,
+        transaction_status_sender.is_some(),
+        transaction_status_sender.is_some(),
+        transaction_status_sender.is_some(),
+        timings,
+    );
 
     if bank
         .feature_set
@@ -238,8 +237,6 @@ fn execute_batch(
             execution_results,
             balances,
             token_balances,
-            inner_instructions,
-            transaction_logs,
             rent_debits,
         );
     }
@@ -1399,11 +1396,9 @@ pub enum TransactionStatusMessage {
 pub struct TransactionStatusBatch {
     pub bank: Arc<Bank>,
     pub transactions: Vec<SanitizedTransaction>,
-    pub statuses: Vec<TransactionExecutionResult>,
+    pub execution_results: Vec<TransactionExecutionResult>,
     pub balances: TransactionBalancesSet,
     pub token_balances: TransactionTokenBalancesSet,
-    pub inner_instructions: Option<Vec<Option<InnerInstructionsList>>>,
-    pub transaction_logs: Option<Vec<Option<TransactionLogMessages>>>,
     pub rent_debits: Vec<RentDebits>,
 }
 
@@ -1418,29 +1413,28 @@ impl TransactionStatusSender {
         &self,
         bank: Arc<Bank>,
         transactions: Vec<SanitizedTransaction>,
-        statuses: Vec<TransactionExecutionResult>,
+        mut execution_results: Vec<TransactionExecutionResult>,
         balances: TransactionBalancesSet,
         token_balances: TransactionTokenBalancesSet,
-        inner_instructions: Vec<Option<InnerInstructionsList>>,
-        transaction_logs: Vec<Option<TransactionLogMessages>>,
         rent_debits: Vec<RentDebits>,
     ) {
         let slot = bank.slot();
-        let (inner_instructions, transaction_logs) = if !self.enable_cpi_and_log_storage {
-            (None, None)
-        } else {
-            (Some(inner_instructions), Some(transaction_logs))
-        };
+        if !self.enable_cpi_and_log_storage {
+            execution_results.iter_mut().for_each(|execution_result| {
+                if let TransactionExecutionResult::Executed(details) = execution_result {
+                    details.log_messages.take();
+                    details.inner_instructions.take();
+                }
+            });
+        }
         if let Err(e) = self
             .sender
             .send(TransactionStatusMessage::Batch(TransactionStatusBatch {
                 bank,
                 transactions,
-                statuses,
+                execution_results,
                 balances,
                 token_balances,
-                inner_instructions,
-                transaction_logs,
                 rent_debits,
             }))
         {
@@ -3483,8 +3477,6 @@ pub mod tests {
                 ..
             },
             _balances,
-            _inner_instructions,
-            _log_messages,
         ) = batch.bank().load_execute_and_commit_transactions(
             &batch,
             MAX_PROCESSING_AGE,

program-test/src/lib.rs

@@ -416,7 +416,7 @@ fn setup_fees(bank: Bank) -> Bank {
     bank.commit_transactions(
         &[],     // transactions
         &mut [], // loaded accounts
-        &[],     // transaction execution results
+        vec![],  // transaction execution results
         0,       // tx count
         1,       // signature count
         &mut ExecuteTimings::default(),