Move transaction results data structures out of bank.rs (#32386)

* Move transaction results data structures out of bank.rs

* Fix CI check failures

banks-server/src/banks_server.rs

@@ -9,9 +9,10 @@ use {
     },
     solana_client::connection_cache::ConnectionCache,
     solana_runtime::{
-        bank::{Bank, TransactionExecutionResult, TransactionSimulationResult},
+        bank::{Bank, TransactionSimulationResult},
         bank_forks::BankForks,
         commitment::BlockCommitmentCache,
+        transaction_results::TransactionExecutionResult,
     },
     solana_sdk::{
         account::Account,

core/src/banking_stage/committer.rs

@@ -7,13 +7,11 @@ use {
     solana_measure::measure_us,
     solana_runtime::{
         accounts::TransactionLoadResult,
-        bank::{
+        bank::{Bank, CommitTransactionCounts, TransactionBalancesSet},
-            Bank, CommitTransactionCounts, TransactionBalancesSet, TransactionExecutionResult,
-            TransactionResults,
-        },
         bank_utils,
         prioritization_fee_cache::PrioritizationFeeCache,
         transaction_batch::TransactionBatch,
+        transaction_results::{TransactionExecutionResult, TransactionResults},
         vote_sender_types::ReplayVoteSender,
     },
     solana_sdk::{pubkey::Pubkey, saturating_add_assign},

core/src/banking_stage/consumer.rs

@@ -17,9 +17,10 @@ use {
     },
     solana_program_runtime::timings::ExecuteTimings,
     solana_runtime::{
-        bank::{Bank, LoadAndExecuteTransactionsOutput, TransactionCheckResult},
+        bank::{Bank, LoadAndExecuteTransactionsOutput},
         transaction_batch::TransactionBatch,
         transaction_error_metrics::TransactionErrorMetrics,
+        transaction_results::TransactionCheckResult,
     },
     solana_sdk::{
         clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE},

ledger/src/blockstore_processor.rs

@@ -28,10 +28,7 @@ use {
         accounts_db::{AccountShrinkThreshold, AccountsDbConfig},
         accounts_index::AccountSecondaryIndexes,
         accounts_update_notifier_interface::AccountsUpdateNotifier,
-        bank::{
+        bank::{Bank, TransactionBalancesSet},
-            Bank, TransactionBalancesSet, TransactionExecutionDetails, TransactionExecutionResult,
-            TransactionResults,
-        },
         bank_forks::BankForks,
         bank_utils,
         commitment::VOTE_THRESHOLD_SIZE,
@@ -41,6 +38,9 @@ use {
         rent_debits::RentDebits,
         runtime_config::RuntimeConfig,
         transaction_batch::TransactionBatch,
+        transaction_results::{
+            TransactionExecutionDetails, TransactionExecutionResult, TransactionResults,
+        },
         vote_account::VoteAccountsHashMap,
         vote_sender_types::ReplayVoteSender,
     },

programs/sbf/tests/programs.rs

@@ -19,15 +19,16 @@ use {
     solana_program_runtime::{compute_budget::ComputeBudget, timings::ExecuteTimings},
     solana_rbpf::vm::ContextObject,
     solana_runtime::{
-        bank::{
+        bank::TransactionBalancesSet,
-            DurableNonceFee, InnerInstruction, TransactionBalancesSet, TransactionExecutionDetails,
-            TransactionExecutionResult, TransactionResults,
-        },
         loader_utils::{
             create_program, load_and_finalize_program, load_program, load_program_from_file,
             load_upgradeable_buffer, load_upgradeable_program, set_upgrade_authority,
             upgrade_program,
         },
+        transaction_results::{
+            DurableNonceFee, InnerInstruction, TransactionExecutionDetails,
+            TransactionExecutionResult, TransactionResults,
+        },
     },
     solana_sbf_rust_invoke::instructions::*,
     solana_sbf_rust_realloc::instructions::*,

rpc/src/transaction_status_service.rs

@@ -6,7 +6,7 @@ use {
         blockstore::Blockstore,
         blockstore_processor::{TransactionStatusBatch, TransactionStatusMessage},
     },
-    solana_runtime::bank::{DurableNonceFee, TransactionExecutionDetails},
+    solana_runtime::transaction_results::{DurableNonceFee, TransactionExecutionDetails},
     solana_transaction_status::{
         extract_and_fmt_memos, InnerInstruction, InnerInstructions, Reward, TransactionStatusMeta,
     },

runtime/src/accounts.rs

@@ -13,13 +13,14 @@ use {
         },
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         ancestors::Ancestors,
-        bank::{Bank, TransactionCheckResult, TransactionExecutionResult},
+        bank::Bank,
         blockhash_queue::BlockhashQueue,
         nonce_info::{NonceFull, NonceInfo},
         rent_collector::RentCollector,
         rent_debits::RentDebits,
         storable_accounts::StorableAccounts,
         transaction_error_metrics::TransactionErrorMetrics,
+        transaction_results::{TransactionCheckResult, TransactionExecutionResult},
     },
     dashmap::DashMap,
     itertools::Itertools,
@@ -1481,8 +1482,8 @@ mod tests {
     use {
         super::*,
         crate::{
-            bank::{DurableNonceFee, TransactionExecutionDetails},
             rent_collector::RentCollector,
+            transaction_results::{DurableNonceFee, TransactionExecutionDetails},
         },
         assert_matches::assert_matches,
         solana_address_lookup_table_program::state::LookupTableMeta,

runtime/src/bank.rs

@@ -62,7 +62,7 @@ use {
         epoch_accounts_hash::{self, EpochAccountsHash},
         epoch_rewards_hasher::hash_rewards_into_partitions,
         epoch_stakes::{EpochStakes, NodeVoteAccounts},
-        nonce_info::{NonceFull, NonceInfo, NoncePartial},
+        nonce_info::{NonceInfo, NoncePartial},
         partitioned_rewards::PartitionedEpochRewardsConfig,
         rent_collector::{CollectedInfo, RentCollector},
         rent_debits::RentDebits,
@@ -82,6 +82,11 @@ use {
         storable_accounts::StorableAccounts,
         transaction_batch::TransactionBatch,
         transaction_error_metrics::TransactionErrorMetrics,
+        transaction_results::{
+            inner_instructions_list_from_instruction_trace, DurableNonceFee,
+            TransactionCheckResult, TransactionExecutionDetails, TransactionExecutionResult,
+            TransactionResults,
+        },
         vote_account::{VoteAccount, VoteAccounts, VoteAccountsHashMap},
     },
     byteorder::{ByteOrder, LittleEndian},
@@ -140,7 +145,6 @@ use {
         hash::{extend_and_hash, hashv, Hash},
         incinerator,
         inflation::Inflation,
-        instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT},
         lamports::LamportsError,
         loader_v4,
         message::{AccountKeys, SanitizedMessage},
@@ -291,77 +295,6 @@ impl BankRc {
     }
 }
 
-pub type TransactionCheckResult = (Result<()>, Option<NoncePartial>);
-
-pub struct TransactionResults {
-    pub fee_collection_results: Vec<Result<()>>,
-    pub execution_results: Vec<TransactionExecutionResult>,
-    pub rent_debits: Vec<RentDebits>,
-}
-
-#[derive(Debug, Clone)]
-pub struct TransactionExecutionDetails {
-    pub status: Result<()>,
-    pub log_messages: Option<Vec<String>>,
-    pub inner_instructions: Option<InnerInstructionsList>,
-    pub durable_nonce_fee: Option<DurableNonceFee>,
-    pub return_data: Option<TransactionReturnData>,
-    pub executed_units: u64,
-    /// The change in accounts data len for this transaction.
-    /// NOTE: This value is valid IFF `status` is `Ok`.
-    pub accounts_data_len_delta: i64,
-}
-
-/// 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<TransactionExecutionDetails, TransactionError>` 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 {
-        details: TransactionExecutionDetails,
-        programs_modified_by_tx: Box<LoadedProgramsForTxBatch>,
-        programs_updated_only_for_global_cache: Box<LoadedProgramsForTxBatch>,
-    },
-    NotExecuted(TransactionError),
-}
-
-impl TransactionExecutionResult {
-    pub fn was_executed_successfully(&self) -> bool {
-        match self {
-            Self::Executed { details, .. } => details.status.is_ok(),
-            Self::NotExecuted { .. } => false,
-        }
-    }
-
-    pub fn was_executed(&self) -> bool {
-        match self {
-            Self::Executed { .. } => true,
-            Self::NotExecuted(_) => false,
-        }
-    }
-
-    pub fn details(&self) -> Option<&TransactionExecutionDetails> {
-        match self {
-            Self::Executed { details, .. } => Some(details),
-            Self::NotExecuted(_) => None,
-        }
-    }
-
-    pub fn flattened_result(&self) -> Result<()> {
-        match self {
-            Self::Executed { details, .. } => details.status.clone(),
-            Self::NotExecuted(err) => Err(err.clone()),
-        }
-    }
-}
-
 pub struct LoadAndExecuteTransactionsOutput {
     pub loaded_transactions: Vec<TransactionLoadResult>,
     // Vector of results indicating whether a transaction was executed or could not
@@ -379,30 +312,6 @@ pub struct LoadAndExecuteTransactionsOutput {
     pub error_counters: TransactionErrorMetrics,
 }
 
-#[derive(Debug, Clone)]
-pub enum DurableNonceFee {
-    Valid(u64),
-    Invalid,
-}
-
-impl From<&NonceFull> for DurableNonceFee {
-    fn from(nonce: &NonceFull) -> Self {
-        match nonce.lamports_per_signature() {
-            Some(lamports_per_signature) => Self::Valid(lamports_per_signature),
-            None => Self::Invalid,
-        }
-    }
-}
-
-impl DurableNonceFee {
-    pub fn lamports_per_signature(&self) -> Option<u64> {
-        match self {
-            Self::Valid(lamports_per_signature) => Some(*lamports_per_signature),
-            Self::Invalid => None,
-        }
-    }
-}
-
 pub struct TransactionSimulationResult {
     pub result: Result<()>,
     pub logs: TransactionLogMessages,
@@ -426,74 +335,6 @@ impl TransactionBalancesSet {
 }
 pub type TransactionBalances = Vec<Vec<u64>>;
 
-/// An ordered list of compiled instructions that were invoked during a
-/// transaction instruction
-pub type InnerInstructions = Vec<InnerInstruction>;
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct InnerInstruction {
-    pub instruction: CompiledInstruction,
-    /// Invocation stack height of this instruction. Instruction stack height
-    /// starts at 1 for transaction instructions.
-    pub stack_height: u8,
-}
-
-/// A list of compiled instructions that were invoked during each instruction of
-/// a transaction
-pub type InnerInstructionsList = Vec<InnerInstructions>;
-
-/// Extract the InnerInstructionsList from a TransactionContext
-pub fn inner_instructions_list_from_instruction_trace(
-    transaction_context: &TransactionContext,
-) -> InnerInstructionsList {
-    debug_assert!(transaction_context
-        .get_instruction_context_at_index_in_trace(0)
-        .map(|instruction_context| instruction_context.get_stack_height()
-            == TRANSACTION_LEVEL_STACK_HEIGHT)
-        .unwrap_or(true));
-    let mut outer_instructions = Vec::new();
-    for index_in_trace in 0..transaction_context.get_instruction_trace_length() {
-        if let Ok(instruction_context) =
-            transaction_context.get_instruction_context_at_index_in_trace(index_in_trace)
-        {
-            let stack_height = instruction_context.get_stack_height();
-            if stack_height == TRANSACTION_LEVEL_STACK_HEIGHT {
-                outer_instructions.push(Vec::new());
-            } else if let Some(inner_instructions) = outer_instructions.last_mut() {
-                let stack_height = u8::try_from(stack_height).unwrap_or(u8::MAX);
-                let instruction = CompiledInstruction::new_from_raw_parts(
-                    instruction_context
-                        .get_index_of_program_account_in_transaction(
-                            instruction_context
-                                .get_number_of_program_accounts()
-                                .saturating_sub(1),
-                        )
-                        .unwrap_or_default() as u8,
-                    instruction_context.get_instruction_data().to_vec(),
-                    (0..instruction_context.get_number_of_instruction_accounts())
-                        .map(|instruction_account_index| {
-                            instruction_context
-                                .get_index_of_instruction_account_in_transaction(
-                                    instruction_account_index,
-                                )
-                                .unwrap_or_default() as u8
-                        })
-                        .collect(),
-                );
-                inner_instructions.push(InnerInstruction {
-                    instruction,
-                    stack_height,
-                });
-            } else {
-                debug_assert!(false);
-            }
-        } else {
-            debug_assert!(false);
-        }
-    }
-    outer_instructions
-}
-
 /// A list of log messages emitted during a transaction
 pub type TransactionLogMessages = Vec<String>;
 

runtime/src/bank/tests.rs

@@ -11267,50 +11267,6 @@ fn test_skip_rewrite() {
     }
 }
 
-#[test]
-fn test_inner_instructions_list_from_instruction_trace() {
-    let instruction_trace = [1, 2, 1, 1, 2, 3, 2];
-    let mut transaction_context = TransactionContext::new(vec![], None, 3, instruction_trace.len());
-    for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
-        while stack_height <= transaction_context.get_instruction_context_stack_height() {
-            transaction_context.pop().unwrap();
-        }
-        if stack_height > transaction_context.get_instruction_context_stack_height() {
-            transaction_context
-                .get_next_instruction_context()
-                .unwrap()
-                .configure(&[], &[], &[index_in_trace as u8]);
-            transaction_context.push().unwrap();
-        }
-    }
-    let inner_instructions = inner_instructions_list_from_instruction_trace(&transaction_context);
-
-    assert_eq!(
-        inner_instructions,
-        vec![
-            vec![InnerInstruction {
-                instruction: CompiledInstruction::new_from_raw_parts(0, vec![1], vec![]),
-                stack_height: 2,
-            }],
-            vec![],
-            vec![
-                InnerInstruction {
-                    instruction: CompiledInstruction::new_from_raw_parts(0, vec![4], vec![]),
-                    stack_height: 2,
-                },
-                InnerInstruction {
-                    instruction: CompiledInstruction::new_from_raw_parts(0, vec![5], vec![]),
-                    stack_height: 3,
-                },
-                InnerInstruction {
-                    instruction: CompiledInstruction::new_from_raw_parts(0, vec![6], vec![]),
-                    stack_height: 2,
-                },
-            ]
-        ]
-    );
-}
-
 #[derive(Serialize, Deserialize)]
 enum MockReallocInstruction {
     Realloc(usize, u64, Pubkey),

runtime/src/bank_utils.rs

@@ -1,7 +1,8 @@
 use {
     crate::{
-        bank::{Bank, TransactionResults},
+        bank::Bank,
         genesis_utils::{self, GenesisConfigInfo, ValidatorVoteKeypairs},
+        transaction_results::TransactionResults,
         vote_parser,
         vote_sender_types::ReplayVoteSender,
     },

runtime/src/lib.rs

@@ -83,6 +83,7 @@ pub mod transaction_batch;
 pub mod transaction_cost;
 pub mod transaction_error_metrics;
 pub mod transaction_priority_details;
+pub mod transaction_results;
 mod verify_accounts_hash_in_background;
 pub mod vote_account;
 pub mod vote_parser;

runtime/src/transaction_results.rs

@@ -0,0 +1,226 @@
+use {
+    crate::{
+        nonce_info::{NonceFull, NonceInfo, NoncePartial},
+        rent_debits::RentDebits,
+    },
+    solana_program_runtime::loaded_programs::LoadedProgramsForTxBatch,
+    solana_sdk::{
+        instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT},
+        transaction::{self, TransactionError},
+        transaction_context::{TransactionContext, TransactionReturnData},
+    },
+};
+
+pub type TransactionCheckResult = (transaction::Result<()>, Option<NoncePartial>);
+
+pub struct TransactionResults {
+    pub fee_collection_results: Vec<transaction::Result<()>>,
+    pub execution_results: Vec<TransactionExecutionResult>,
+    pub rent_debits: Vec<RentDebits>,
+}
+
+/// 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<TransactionExecutionDetails, TransactionError>` 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 {
+        details: TransactionExecutionDetails,
+        programs_modified_by_tx: Box<LoadedProgramsForTxBatch>,
+        programs_updated_only_for_global_cache: Box<LoadedProgramsForTxBatch>,
+    },
+    NotExecuted(TransactionError),
+}
+
+impl TransactionExecutionResult {
+    pub fn was_executed_successfully(&self) -> bool {
+        match self {
+            Self::Executed { details, .. } => details.status.is_ok(),
+            Self::NotExecuted { .. } => false,
+        }
+    }
+
+    pub fn was_executed(&self) -> bool {
+        match self {
+            Self::Executed { .. } => true,
+            Self::NotExecuted(_) => false,
+        }
+    }
+
+    pub fn details(&self) -> Option<&TransactionExecutionDetails> {
+        match self {
+            Self::Executed { details, .. } => Some(details),
+            Self::NotExecuted(_) => None,
+        }
+    }
+
+    pub fn flattened_result(&self) -> transaction::Result<()> {
+        match self {
+            Self::Executed { details, .. } => details.status.clone(),
+            Self::NotExecuted(err) => Err(err.clone()),
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct TransactionExecutionDetails {
+    pub status: transaction::Result<()>,
+    pub log_messages: Option<Vec<String>>,
+    pub inner_instructions: Option<InnerInstructionsList>,
+    pub durable_nonce_fee: Option<DurableNonceFee>,
+    pub return_data: Option<TransactionReturnData>,
+    pub executed_units: u64,
+    /// The change in accounts data len for this transaction.
+    /// NOTE: This value is valid IFF `status` is `Ok`.
+    pub accounts_data_len_delta: i64,
+}
+
+#[derive(Debug, Clone)]
+pub enum DurableNonceFee {
+    Valid(u64),
+    Invalid,
+}
+
+impl From<&NonceFull> for DurableNonceFee {
+    fn from(nonce: &NonceFull) -> Self {
+        match nonce.lamports_per_signature() {
+            Some(lamports_per_signature) => Self::Valid(lamports_per_signature),
+            None => Self::Invalid,
+        }
+    }
+}
+
+impl DurableNonceFee {
+    pub fn lamports_per_signature(&self) -> Option<u64> {
+        match self {
+            Self::Valid(lamports_per_signature) => Some(*lamports_per_signature),
+            Self::Invalid => None,
+        }
+    }
+}
+
+/// An ordered list of compiled instructions that were invoked during a
+/// transaction instruction
+pub type InnerInstructions = Vec<InnerInstruction>;
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct InnerInstruction {
+    pub instruction: CompiledInstruction,
+    /// Invocation stack height of this instruction. Instruction stack height
+    /// starts at 1 for transaction instructions.
+    pub stack_height: u8,
+}
+
+/// A list of compiled instructions that were invoked during each instruction of
+/// a transaction
+pub type InnerInstructionsList = Vec<InnerInstructions>;
+
+/// Extract the InnerInstructionsList from a TransactionContext
+pub fn inner_instructions_list_from_instruction_trace(
+    transaction_context: &TransactionContext,
+) -> InnerInstructionsList {
+    debug_assert!(transaction_context
+        .get_instruction_context_at_index_in_trace(0)
+        .map(|instruction_context| instruction_context.get_stack_height()
+            == TRANSACTION_LEVEL_STACK_HEIGHT)
+        .unwrap_or(true));
+    let mut outer_instructions = Vec::new();
+    for index_in_trace in 0..transaction_context.get_instruction_trace_length() {
+        if let Ok(instruction_context) =
+            transaction_context.get_instruction_context_at_index_in_trace(index_in_trace)
+        {
+            let stack_height = instruction_context.get_stack_height();
+            if stack_height == TRANSACTION_LEVEL_STACK_HEIGHT {
+                outer_instructions.push(Vec::new());
+            } else if let Some(inner_instructions) = outer_instructions.last_mut() {
+                let stack_height = u8::try_from(stack_height).unwrap_or(u8::MAX);
+                let instruction = CompiledInstruction::new_from_raw_parts(
+                    instruction_context
+                        .get_index_of_program_account_in_transaction(
+                            instruction_context
+                                .get_number_of_program_accounts()
+                                .saturating_sub(1),
+                        )
+                        .unwrap_or_default() as u8,
+                    instruction_context.get_instruction_data().to_vec(),
+                    (0..instruction_context.get_number_of_instruction_accounts())
+                        .map(|instruction_account_index| {
+                            instruction_context
+                                .get_index_of_instruction_account_in_transaction(
+                                    instruction_account_index,
+                                )
+                                .unwrap_or_default() as u8
+                        })
+                        .collect(),
+                );
+                inner_instructions.push(InnerInstruction {
+                    instruction,
+                    stack_height,
+                });
+            } else {
+                debug_assert!(false);
+            }
+        } else {
+            debug_assert!(false);
+        }
+    }
+    outer_instructions
+}
+
+#[cfg(test)]
+mod tests {
+    use {super::*, solana_sdk::transaction_context::TransactionContext};
+
+    #[test]
+    fn test_inner_instructions_list_from_instruction_trace() {
+        let instruction_trace = [1, 2, 1, 1, 2, 3, 2];
+        let mut transaction_context =
+            TransactionContext::new(vec![], None, 3, instruction_trace.len());
+        for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() {
+            while stack_height <= transaction_context.get_instruction_context_stack_height() {
+                transaction_context.pop().unwrap();
+            }
+            if stack_height > transaction_context.get_instruction_context_stack_height() {
+                transaction_context
+                    .get_next_instruction_context()
+                    .unwrap()
+                    .configure(&[], &[], &[index_in_trace as u8]);
+                transaction_context.push().unwrap();
+            }
+        }
+        let inner_instructions =
+            inner_instructions_list_from_instruction_trace(&transaction_context);
+
+        assert_eq!(
+            inner_instructions,
+            vec![
+                vec![InnerInstruction {
+                    instruction: CompiledInstruction::new_from_raw_parts(0, vec![1], vec![]),
+                    stack_height: 2,
+                }],
+                vec![],
+                vec![
+                    InnerInstruction {
+                        instruction: CompiledInstruction::new_from_raw_parts(0, vec![4], vec![]),
+                        stack_height: 2,
+                    },
+                    InnerInstruction {
+                        instruction: CompiledInstruction::new_from_raw_parts(0, vec![5], vec![]),
+                        stack_height: 3,
+                    },
+                    InnerInstruction {
+                        instruction: CompiledInstruction::new_from_raw_parts(0, vec![6], vec![]),
+                        stack_height: 2,
+                    },
+                ]
+            ]
+        );
+    }
+}