From 2f8f910a5cae585417a3f01f63542f5a8c4320e9 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Thu, 12 Sep 2024 13:59:39 -0500 Subject: [PATCH] TransactionBatch: generic over transaction type (#2836) --- Cargo.lock | 1 + core/src/banking_stage/committer.rs | 6 +++--- core/src/banking_stage/consumer.rs | 2 +- ledger/Cargo.toml | 1 + ledger/src/blockstore_processor.rs | 21 ++++++++++---------- ledger/src/token_balances.rs | 4 ++-- programs/sbf/Cargo.lock | 1 + runtime/src/bank.rs | 30 ++++++++++++++++++++--------- runtime/src/transaction_batch.rs | 23 ++++++++++++---------- 9 files changed, 54 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 187aba8daa5d97..6c06b529821ce7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6729,6 +6729,7 @@ dependencies = [ "solana-storage-bigtable", "solana-storage-proto", "solana-svm", + "solana-svm-transaction", "solana-timings", "solana-transaction-status", "solana-vote", diff --git a/core/src/banking_stage/committer.rs b/core/src/banking_stage/committer.rs index ff27104251759e..ed718bbdee95fd 100644 --- a/core/src/banking_stage/committer.rs +++ b/core/src/banking_stage/committer.rs @@ -12,7 +12,7 @@ use { transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, }, - solana_sdk::{pubkey::Pubkey, saturating_add_assign}, + solana_sdk::{pubkey::Pubkey, saturating_add_assign, transaction::SanitizedTransaction}, solana_svm::{ transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions}, transaction_processing_result::{ @@ -67,7 +67,7 @@ impl Committer { pub(super) fn commit_transactions( &self, - batch: &TransactionBatch, + batch: &TransactionBatch, processing_results: Vec, starting_transaction_index: Option, bank: &Arc, @@ -129,7 +129,7 @@ impl Committer { &self, commit_results: Vec, bank: &Arc, - batch: &TransactionBatch, + batch: &TransactionBatch, pre_balance_info: &mut PreBalanceInfo, starting_transaction_index: Option, ) { diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index b8cd383a896634..4bd6f50cd18b65 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -550,7 +550,7 @@ impl Consumer { fn execute_and_commit_transactions_locked( &self, bank: &Arc, - batch: &TransactionBatch, + batch: &TransactionBatch, ) -> ExecuteAndCommitTransactionsOutput { let transaction_status_sender_enabled = self.committer.transaction_status_sender_enabled(); let mut execute_and_commit_timings = LeaderExecuteAndCommitTimings::default(); diff --git a/ledger/Cargo.toml b/ledger/Cargo.toml index 44367a30fd5ec7..b12fcddf2b9b41 100644 --- a/ledger/Cargo.toml +++ b/ledger/Cargo.toml @@ -59,6 +59,7 @@ solana-stake-program = { workspace = true } solana-storage-bigtable = { workspace = true } solana-storage-proto = { workspace = true } solana-svm = { workspace = true } +solana-svm-transaction = { workspace = true } solana-timings = { workspace = true } solana-transaction-status = { workspace = true } solana-vote = { workspace = true } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 89a013531f407c..28b1b09b025dea 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -56,6 +56,7 @@ use { transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions}, transaction_processor::ExecutionRecordingConfig, }, + solana_svm_transaction::svm_transaction::SVMTransaction, solana_timings::{report_execute_timings, ExecuteTimingType, ExecuteTimings}, solana_transaction_status::token_balances::TransactionTokenBalancesSet, solana_vote::vote_account::VoteAccountsHashMap, @@ -77,8 +78,8 @@ use { #[cfg(feature = "dev-context-only-utils")] use {qualifier_attr::qualifiers, solana_runtime::bank::HashOverrides}; -pub struct TransactionBatchWithIndexes<'a, 'b> { - pub batch: TransactionBatch<'a, 'b>, +pub struct TransactionBatchWithIndexes<'a, 'b, Tx: SVMTransaction + Clone> { + pub batch: TransactionBatch<'a, 'b, Tx>, pub transaction_indexes: Vec, } @@ -98,7 +99,7 @@ fn first_err(results: &[Result<()>]) -> Result<()> { // Includes transaction signature for unit-testing fn get_first_error( - batch: &TransactionBatch, + batch: &TransactionBatch, commit_results: &[TransactionCommitResult], ) -> Option<(Result<()>, Signature)> { let mut first_err = None; @@ -133,7 +134,7 @@ fn create_thread_pool(num_threads: usize) -> ThreadPool { } pub fn execute_batch( - batch: &TransactionBatchWithIndexes, + batch: &TransactionBatchWithIndexes, bank: &Arc, transaction_status_sender: Option<&TransactionStatusSender>, replay_vote_sender: Option<&ReplayVoteSender>, @@ -282,7 +283,7 @@ impl ExecuteBatchesInternalMetrics { fn execute_batches_internal( bank: &Arc, replay_tx_thread_pool: &ThreadPool, - batches: &[TransactionBatchWithIndexes], + batches: &[TransactionBatchWithIndexes], transaction_status_sender: Option<&TransactionStatusSender>, replay_vote_sender: Option<&ReplayVoteSender>, log_messages_bytes_limit: Option, @@ -360,7 +361,7 @@ fn execute_batches_internal( fn process_batches( bank: &BankWithScheduler, replay_tx_thread_pool: &ThreadPool, - batches: &[TransactionBatchWithIndexes], + batches: &[TransactionBatchWithIndexes], transaction_status_sender: Option<&TransactionStatusSender>, replay_vote_sender: Option<&ReplayVoteSender>, batch_execution_timing: &mut BatchExecutionTiming, @@ -415,7 +416,7 @@ fn process_batches( fn schedule_batches_for_execution( bank: &BankWithScheduler, - batches: &[TransactionBatchWithIndexes], + batches: &[TransactionBatchWithIndexes], ) -> Result<()> { for TransactionBatchWithIndexes { batch, @@ -439,7 +440,7 @@ fn rebatch_transactions<'a>( start: usize, end: usize, transaction_indexes: &'a [usize], -) -> TransactionBatchWithIndexes<'a, 'a> { +) -> TransactionBatchWithIndexes<'a, 'a, SanitizedTransaction> { let txs = &sanitized_txs[start..=end]; let results = &lock_results[start..=end]; let mut tx_batch = TransactionBatch::new(results.to_vec(), bank, Cow::from(txs)); @@ -455,7 +456,7 @@ fn rebatch_transactions<'a>( fn rebatch_and_execute_batches( bank: &Arc, replay_tx_thread_pool: &ThreadPool, - batches: &[TransactionBatchWithIndexes], + batches: &[TransactionBatchWithIndexes], transaction_status_sender: Option<&TransactionStatusSender>, replay_vote_sender: Option<&ReplayVoteSender>, timing: &mut BatchExecutionTiming, @@ -494,7 +495,7 @@ fn rebatch_and_execute_batches( let target_batch_count = get_thread_count() as u64; - let mut tx_batches: Vec = vec![]; + let mut tx_batches: Vec> = vec![]; let rebatched_txs = if total_cost > target_batch_count.saturating_mul(minimal_tx_cost) { let target_batch_cost = total_cost / target_batch_count; let mut batch_cost: u64 = 0; diff --git a/ledger/src/token_balances.rs b/ledger/src/token_balances.rs index cc074dfcc0da0a..6190f8f1d20423 100644 --- a/ledger/src/token_balances.rs +++ b/ledger/src/token_balances.rs @@ -6,7 +6,7 @@ use { solana_measure::measure::Measure, solana_metrics::datapoint_debug, solana_runtime::{bank::Bank, transaction_batch::TransactionBatch}, - solana_sdk::{account::ReadableAccount, pubkey::Pubkey}, + solana_sdk::{account::ReadableAccount, pubkey::Pubkey, transaction::SanitizedTransaction}, solana_transaction_status::{ token_balances::TransactionTokenBalances, TransactionTokenBalance, }, @@ -37,7 +37,7 @@ fn get_mint_decimals(bank: &Bank, mint: &Pubkey) -> Option { pub fn collect_token_balances( bank: &Bank, - batch: &TransactionBatch, + batch: &TransactionBatch, mint_decimals: &mut HashMap, ) -> TransactionTokenBalances { let mut balances: TransactionTokenBalances = vec![]; diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 622c921d0512ea..c6c664274fa9a0 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5273,6 +5273,7 @@ dependencies = [ "solana-storage-bigtable", "solana-storage-proto", "solana-svm", + "solana-svm-transaction", "solana-timings", "solana-transaction-status", "solana-vote", diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ee6e785a1e6b68..d4e78bf5180ae4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3317,7 +3317,10 @@ impl Bank { /// Prepare a transaction batch from a list of versioned transactions from /// an entry. Used for tests only. - pub fn prepare_entry_batch(&self, txs: Vec) -> Result { + pub fn prepare_entry_batch( + &self, + txs: Vec, + ) -> Result> { let sanitized_txs = txs .into_iter() .map(|tx| { @@ -3346,7 +3349,7 @@ impl Bank { pub fn prepare_sanitized_batch<'a, 'b>( &'a self, txs: &'b [SanitizedTransaction], - ) -> TransactionBatch<'a, 'b> { + ) -> TransactionBatch<'a, 'b, SanitizedTransaction> { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_results = self .rc @@ -3361,7 +3364,7 @@ impl Bank { &'a self, transactions: &'b [SanitizedTransaction], transaction_results: impl Iterator>, - ) -> TransactionBatch<'a, 'b> { + ) -> TransactionBatch<'a, 'b, SanitizedTransaction> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_results = self.rc.accounts.lock_accounts_with_results( @@ -3376,7 +3379,7 @@ impl Bank { pub fn prepare_unlocked_batch_from_single_tx<'a>( &'a self, transaction: &'a SanitizedTransaction, - ) -> TransactionBatch<'_, '_> { + ) -> TransactionBatch<'_, '_, SanitizedTransaction> { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_result = validate_account_locks(transaction.message().account_keys(), tx_account_lock_limit); @@ -3534,7 +3537,10 @@ impl Bank { .is_hash_valid_for_age(hash, max_age) } - pub fn collect_balances(&self, batch: &TransactionBatch) -> TransactionBalances { + pub fn collect_balances( + &self, + batch: &TransactionBatch, + ) -> TransactionBalances { let mut balances: TransactionBalances = vec![]; for transaction in batch.sanitized_transactions() { let mut transaction_balances: Vec = vec![]; @@ -3548,7 +3554,7 @@ impl Bank { pub fn load_and_execute_transactions( &self, - batch: &TransactionBatch, + batch: &TransactionBatch, max_age: usize, timings: &mut ExecuteTimings, error_counters: &mut TransactionErrorMetrics, @@ -4682,7 +4688,7 @@ impl Bank { #[must_use] pub fn load_execute_and_commit_transactions( &self, - batch: &TransactionBatch, + batch: &TransactionBatch, max_age: usize, collect_balances: bool, recording_config: ExecutionRecordingConfig, @@ -4788,7 +4794,10 @@ impl Bank { } #[must_use] - fn process_transaction_batch(&self, batch: &TransactionBatch) -> Vec> { + fn process_transaction_batch( + &self, + batch: &TransactionBatch, + ) -> Vec> { self.load_execute_and_commit_transactions( batch, MAX_PROCESSING_AGE, @@ -6796,7 +6805,10 @@ impl Bank { } /// Prepare a transaction batch from a list of legacy transactions. Used for tests only. - pub fn prepare_batch_for_tests(&self, txs: Vec) -> TransactionBatch { + pub fn prepare_batch_for_tests( + &self, + txs: Vec, + ) -> TransactionBatch { let transaction_account_lock_limit = self.get_transaction_account_lock_limit(); let sanitized_txs = txs .into_iter() diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index ecec27e02e93aa..1d33be2c57c591 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -1,22 +1,21 @@ use { - crate::bank::Bank, - solana_sdk::transaction::{Result, SanitizedTransaction}, - std::borrow::Cow, + crate::bank::Bank, solana_sdk::transaction::Result, + solana_svm_transaction::svm_transaction::SVMTransaction, std::borrow::Cow, }; // Represents the results of trying to lock a set of accounts -pub struct TransactionBatch<'a, 'b> { +pub struct TransactionBatch<'a, 'b, Tx: SVMTransaction + Clone> { lock_results: Vec>, bank: &'a Bank, - sanitized_txs: Cow<'b, [SanitizedTransaction]>, + sanitized_txs: Cow<'b, [Tx]>, needs_unlock: bool, } -impl<'a, 'b> TransactionBatch<'a, 'b> { +impl<'a, 'b, Tx: SVMTransaction + Clone> TransactionBatch<'a, 'b, Tx> { pub fn new( lock_results: Vec>, bank: &'a Bank, - sanitized_txs: Cow<'b, [SanitizedTransaction]>, + sanitized_txs: Cow<'b, [Tx]>, ) -> Self { assert_eq!(lock_results.len(), sanitized_txs.len()); Self { @@ -31,7 +30,7 @@ impl<'a, 'b> TransactionBatch<'a, 'b> { &self.lock_results } - pub fn sanitized_transactions(&self) -> &[SanitizedTransaction] { + pub fn sanitized_transactions(&self) -> &[Tx] { &self.sanitized_txs } @@ -82,7 +81,7 @@ impl<'a, 'b> TransactionBatch<'a, 'b> { } // Unlock all locked accounts in destructor. -impl<'a, 'b> Drop for TransactionBatch<'a, 'b> { +impl<'a, 'b, Tx: SVMTransaction + Clone> Drop for TransactionBatch<'a, 'b, Tx> { fn drop(&mut self) { if self.needs_unlock() { self.set_needs_unlock(false); @@ -100,7 +99,11 @@ mod tests { use { super::*, crate::genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, - solana_sdk::{signature::Keypair, system_transaction, transaction::TransactionError}, + solana_sdk::{ + signature::Keypair, + system_transaction, + transaction::{SanitizedTransaction, TransactionError}, + }, }; #[test]