diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index d9c59bdc5..c49166310 100644 --- a/banks-server/src/banks_server.rs +++ b/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, diff --git a/core/src/banking_stage/committer.rs b/core/src/banking_stage/committer.rs index 8fa26533b..1f82be37d 100644 --- a/core/src/banking_stage/committer.rs +++ b/core/src/banking_stage/committer.rs @@ -7,13 +7,11 @@ use { solana_measure::measure_us, solana_runtime::{ accounts::TransactionLoadResult, - bank::{ - Bank, CommitTransactionCounts, TransactionBalancesSet, TransactionExecutionResult, - TransactionResults, - }, + bank::{Bank, CommitTransactionCounts, TransactionBalancesSet}, bank_utils, prioritization_fee_cache::PrioritizationFeeCache, transaction_batch::TransactionBatch, + transaction_results::{TransactionExecutionResult, TransactionResults}, vote_sender_types::ReplayVoteSender, }, solana_sdk::{pubkey::Pubkey, saturating_add_assign}, diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 08bea0b55..19302bc55 100644 --- a/core/src/banking_stage/consumer.rs +++ b/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}, diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 8a8c328d4..a0b7803c9 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -28,10 +28,7 @@ use { accounts_db::{AccountShrinkThreshold, AccountsDbConfig}, accounts_index::AccountSecondaryIndexes, accounts_update_notifier_interface::AccountsUpdateNotifier, - bank::{ - Bank, TransactionBalancesSet, TransactionExecutionDetails, TransactionExecutionResult, - TransactionResults, - }, + bank::{Bank, TransactionBalancesSet}, 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, }, diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 9a3910f4f..7894ee4ba 100644 --- a/programs/sbf/tests/programs.rs +++ b/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::{ - DurableNonceFee, InnerInstruction, TransactionBalancesSet, TransactionExecutionDetails, - TransactionExecutionResult, TransactionResults, - }, + bank::TransactionBalancesSet, 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::*, diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index d591c329a..8fc20fa66 100644 --- a/rpc/src/transaction_status_service.rs +++ b/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, }, diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index c5aad8207..25ee74e87 100644 --- a/runtime/src/accounts.rs +++ b/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, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 429d75ca9..b96742247 100644 --- a/runtime/src/bank.rs +++ b/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); - -pub struct TransactionResults { - pub fee_collection_results: Vec>, - pub execution_results: Vec, - pub rent_debits: Vec, -} - -#[derive(Debug, Clone)] -pub struct TransactionExecutionDetails { - pub status: Result<()>, - pub log_messages: Option>, - pub inner_instructions: Option, - pub durable_nonce_fee: Option, - pub return_data: Option, - 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` 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, - programs_updated_only_for_global_cache: Box, - }, - 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, // 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 { - 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>; -/// An ordered list of compiled instructions that were invoked during a -/// transaction instruction -pub type InnerInstructions = Vec; - -#[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; - -/// 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; diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 58538f090..fb761686e 100644 --- a/runtime/src/bank/tests.rs +++ b/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), diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index ba9d20d2e..38a92ef01 100644 --- a/runtime/src/bank_utils.rs +++ b/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, }, diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 0f7947889..9d8a92e54 100644 --- a/runtime/src/lib.rs +++ b/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; diff --git a/runtime/src/transaction_results.rs b/runtime/src/transaction_results.rs new file mode 100644 index 000000000..f7228dfff --- /dev/null +++ b/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); + +pub struct TransactionResults { + pub fee_collection_results: Vec>, + pub execution_results: Vec, + pub rent_debits: Vec, +} + +/// 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 { + details: TransactionExecutionDetails, + programs_modified_by_tx: Box, + programs_updated_only_for_global_cache: Box, + }, + 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>, + pub inner_instructions: Option, + pub durable_nonce_fee: Option, + pub return_data: Option, + 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 { + 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; + +#[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; + +/// 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, + }, + ] + ] + ); + } +}