refactor: committed tx may not have been executed

core/src/banking_stage/committer.rs

@@ -101,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,

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,
                     }
                 })

ledger/src/blockstore_processor.rs

@@ -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,7 @@ pub mod tests {
         },
         solana_svm::{
             transaction_commit_result::CommittedTransaction,
-            transaction_execution_result::{
-                TransactionExecutionDetails, TransactionLoadedAccountsStats,
-            },
+            transaction_execution_result::TransactionLoadedAccountsStats,
             transaction_processor::ExecutionRecordingConfig,
         },
         solana_vote::vote_account::VoteAccount,
@@ -5079,20 +5077,17 @@ pub mod tests {
         let txs = vec![tx.clone(), tx];
         let commit_results = vec![
             Ok(CommittedTransaction {
+                status: Ok(()),
+                log_messages: None,
+                inner_instructions: None,
+                return_data: None,
+                executed_units: actual_execution_cu,
+                fee_details: FeeDetails::default(),
+                rent_debits: RentDebits::default(),
                 loaded_account_stats: TransactionLoadedAccountsStats {
                     loaded_accounts_data_size: actual_loaded_accounts_data_size,
                     loaded_accounts_count: 2,
                 },
-                execution_details: TransactionExecutionDetails {
-                    status: Ok(()),
-                    log_messages: None,
-                    inner_instructions: None,
-                    return_data: None,
-                    executed_units: actual_execution_cu,
-                    accounts_data_len_delta: 0,
-                },
-                fee_details: FeeDetails::default(),
-                rent_debits: RentDebits::default(),
             }),
             Err(TransactionError::AccountNotFound),
         ];

programs/sbf/tests/programs.rs

@@ -63,7 +63,7 @@ use {
     },
     solana_svm::{
         transaction_commit_result::CommittedTransaction,
-        transaction_execution_result::{InnerInstruction, TransactionExecutionDetails},
+        transaction_execution_result::InnerInstruction,
         transaction_processor::ExecutionRecordingConfig,
     },
     solana_timings::ExecuteTimings,
@@ -107,12 +107,12 @@ fn process_transaction_and_record_inner(
             None,
         )
         .0;
-    let TransactionExecutionDetails {
+    let CommittedTransaction {
         inner_instructions,
         log_messages,
         status,
         ..
-    } = commit_results.swap_remove(0).unwrap().execution_details;
+    } = commit_results.swap_remove(0).unwrap();
     let inner_instructions = inner_instructions.expect("cpi recording should be enabled");
     let log_messages = log_messages.expect("log recording should be enabled");
     (status, inner_instructions, log_messages)
@@ -163,16 +163,12 @@ fn execute_transactions(
         )| {
             commit_result.map(|committed_tx| {
                 let CommittedTransaction {
+                    status,
+                    log_messages,
+                    inner_instructions,
+                    return_data,
+                    executed_units,
                     fee_details,
-                    execution_details:
-                        TransactionExecutionDetails {
-                            status,
-                            log_messages,
-                            inner_instructions,
-                            return_data,
-                            executed_units,
-                            ..
-                        },
                     ..
                 } = committed_tx;
 
@@ -5244,7 +5240,6 @@ fn test_function_call_args() {
     let return_data = &result[0]
         .as_ref()
         .unwrap()
-        .execution_details
         .return_data
         .as_ref()
         .unwrap()

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,
     },
@@ -99,15 +96,11 @@ impl TransactionStatusService {
                     };
 
                     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,
                         ..
@@ -333,17 +326,14 @@ pub(crate) mod tests {
         rent_debits.insert(&pubkey, 123, 456);
 
         let commit_result = Ok(CommittedTransaction {
-            loaded_account_stats: TransactionLoadedAccountsStats::default(),
-            execution_details: TransactionExecutionDetails {
-                status: Ok(()),
-                log_messages: None,
-                inner_instructions: None,
-                return_data: None,
-                executed_units: 0,
-                accounts_data_len_delta: 0,
-            },
+            status: Ok(()),
+            log_messages: None,
+            inner_instructions: None,
+            return_data: None,
+            executed_units: 0,
             fee_details: FeeDetails::default(),
             rent_debits,
+            loaded_account_stats: TransactionLoadedAccountsStats::default(),
         });
 
         let balances = TransactionBalancesSet {

runtime/src/bank.rs

@@ -149,7 +149,7 @@ use {
         stake_state::StakeStateV2,
     },
     solana_svm::{
-        account_loader::collect_rent_from_account,
+        account_loader::{collect_rent_from_account, LoadedTransaction},
         account_overrides::AccountOverrides,
         account_saver::collect_accounts_to_store,
         transaction_commit_result::{CommittedTransaction, TransactionCommitResult},
@@ -3859,29 +3859,37 @@ impl Bank {
     ) -> Vec<TransactionCommitResult> {
         processing_results
             .into_iter()
-            .map(|processing_result| match processing_result {
-                Ok(processed_tx) => {
-                    let loaded_tx = &processed_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 processed_tx.was_successful() {
-                        processed_tx.loaded_transaction.rent_debits
-                    } else {
-                        RentDebits::default()
-                    };
+            .map(|processing_result| {
+                let processed_tx = processing_result?;
+                let execution_details = processed_tx.execution_details;
+                let LoadedTransaction {
+                    rent_debits,
+                    accounts: loaded_accounts,
+                    loaded_accounts_data_size,
+                    fee_details,
+                    ..
+                } = processed_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 {
-                        loaded_account_stats,
-                        execution_details: processed_tx.execution_details,
-                        fee_details: processed_tx.loaded_transaction.fee_details,
-                        rent_debits,
-                    })
-                }
-                Err(err) => Err(err),
+                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,
+                    },
+                })
             })
             .collect()
     }
@@ -4615,7 +4623,7 @@ impl Bank {
     pub fn process_transaction_with_metadata(
         &self,
         tx: impl Into<VersionedTransaction>,
-    ) -> Result<TransactionExecutionDetails> {
+    ) -> Result<CommittedTransaction> {
         let txs = vec![tx.into()];
         let batch = self.prepare_entry_batch(txs)?;
 
@@ -4632,8 +4640,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.

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<String>,
     pub instructions: Vec<UiInstruction>,
     pub is_simple_vote_tx: bool,
-    pub execution_results: Option<TransactionExecutionDetails>,
+    pub commit_details: Option<TransactionCommitDetails>,
+}
+
+#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+pub struct TransactionCommitDetails {
+    pub status: TransactionResult<()>,
+    pub log_messages: Option<Vec<String>>,
+    pub inner_instructions: Option<InnerInstructionsList>,
+    pub return_data: Option<TransactionReturnData>,
+    pub executed_units: u64,
+    pub fee_details: FeeDetails,
+}
+
+impl From<CommittedTransaction> 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.

runtime/src/bank/tests.rs

@@ -5732,7 +5732,7 @@ fn test_pre_post_transaction_balances() {
     // Failed transactions still produce balance sets
     // This is an InstructionError - fees charged
     assert_eq!(
-        commit_results[2].transaction_result(),
+        commit_results[2].as_ref().unwrap().status,
         Err(TransactionError::InstructionError(
             0,
             InstructionError::Custom(1),
@@ -9090,7 +9090,6 @@ fn test_tx_log_order() {
     assert!(commit_results[0]
         .as_ref()
         .unwrap()
-        .execution_details
         .log_messages
         .as_ref()
         .unwrap()[1]
@@ -9099,7 +9098,6 @@ fn test_tx_log_order() {
     assert!(commit_results[1]
         .as_ref()
         .unwrap()
-        .execution_details
         .log_messages
         .as_ref()
         .unwrap()[2]
@@ -9193,12 +9191,7 @@ fn test_tx_return_data() {
                 None,
             )
             .0;
-        let return_data = commit_results[0]
-            .as_ref()
-            .unwrap()
-            .execution_details
-            .return_data
-            .clone();
+        let return_data = commit_results[0].as_ref().unwrap().return_data.clone();
         if let Some(index) = index {
             let return_data = return_data.unwrap();
             assert_eq!(return_data.program_id, mock_program_id);