Skip to content

Commit

Permalink
TransactionBatch: generic over transaction type (#2836)
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge authored Sep 12, 2024
1 parent c8c6d2a commit 2f8f910
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 35 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions core/src/banking_stage/committer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -67,7 +67,7 @@ impl Committer {

pub(super) fn commit_transactions(
&self,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
processing_results: Vec<TransactionProcessingResult>,
starting_transaction_index: Option<usize>,
bank: &Arc<Bank>,
Expand Down Expand Up @@ -129,7 +129,7 @@ impl Committer {
&self,
commit_results: Vec<TransactionCommitResult>,
bank: &Arc<Bank>,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
pre_balance_info: &mut PreBalanceInfo,
starting_transaction_index: Option<usize>,
) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ impl Consumer {
fn execute_and_commit_transactions_locked(
&self,
bank: &Arc<Bank>,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
) -> ExecuteAndCommitTransactionsOutput {
let transaction_status_sender_enabled = self.committer.transaction_status_sender_enabled();
let mut execute_and_commit_timings = LeaderExecuteAndCommitTimings::default();
Expand Down
1 change: 1 addition & 0 deletions ledger/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
21 changes: 11 additions & 10 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<usize>,
}

Expand All @@ -98,7 +99,7 @@ fn first_err(results: &[Result<()>]) -> Result<()> {

// Includes transaction signature for unit-testing
fn get_first_error(
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
commit_results: &[TransactionCommitResult],
) -> Option<(Result<()>, Signature)> {
let mut first_err = None;
Expand Down Expand Up @@ -133,7 +134,7 @@ fn create_thread_pool(num_threads: usize) -> ThreadPool {
}

pub fn execute_batch(
batch: &TransactionBatchWithIndexes,
batch: &TransactionBatchWithIndexes<SanitizedTransaction>,
bank: &Arc<Bank>,
transaction_status_sender: Option<&TransactionStatusSender>,
replay_vote_sender: Option<&ReplayVoteSender>,
Expand Down Expand Up @@ -282,7 +283,7 @@ impl ExecuteBatchesInternalMetrics {
fn execute_batches_internal(
bank: &Arc<Bank>,
replay_tx_thread_pool: &ThreadPool,
batches: &[TransactionBatchWithIndexes],
batches: &[TransactionBatchWithIndexes<SanitizedTransaction>],
transaction_status_sender: Option<&TransactionStatusSender>,
replay_vote_sender: Option<&ReplayVoteSender>,
log_messages_bytes_limit: Option<usize>,
Expand Down Expand Up @@ -360,7 +361,7 @@ fn execute_batches_internal(
fn process_batches(
bank: &BankWithScheduler,
replay_tx_thread_pool: &ThreadPool,
batches: &[TransactionBatchWithIndexes],
batches: &[TransactionBatchWithIndexes<SanitizedTransaction>],
transaction_status_sender: Option<&TransactionStatusSender>,
replay_vote_sender: Option<&ReplayVoteSender>,
batch_execution_timing: &mut BatchExecutionTiming,
Expand Down Expand Up @@ -415,7 +416,7 @@ fn process_batches(

fn schedule_batches_for_execution(
bank: &BankWithScheduler,
batches: &[TransactionBatchWithIndexes],
batches: &[TransactionBatchWithIndexes<SanitizedTransaction>],
) -> Result<()> {
for TransactionBatchWithIndexes {
batch,
Expand All @@ -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));
Expand All @@ -455,7 +456,7 @@ fn rebatch_transactions<'a>(
fn rebatch_and_execute_batches(
bank: &Arc<Bank>,
replay_tx_thread_pool: &ThreadPool,
batches: &[TransactionBatchWithIndexes],
batches: &[TransactionBatchWithIndexes<SanitizedTransaction>],
transaction_status_sender: Option<&TransactionStatusSender>,
replay_vote_sender: Option<&ReplayVoteSender>,
timing: &mut BatchExecutionTiming,
Expand Down Expand Up @@ -494,7 +495,7 @@ fn rebatch_and_execute_batches(

let target_batch_count = get_thread_count() as u64;

let mut tx_batches: Vec<TransactionBatchWithIndexes> = vec![];
let mut tx_batches: Vec<TransactionBatchWithIndexes<SanitizedTransaction>> = 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;
Expand Down
4 changes: 2 additions & 2 deletions ledger/src/token_balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -37,7 +37,7 @@ fn get_mint_decimals(bank: &Bank, mint: &Pubkey) -> Option<u8> {

pub fn collect_token_balances(
bank: &Bank,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
mint_decimals: &mut HashMap<Pubkey, u8>,
) -> TransactionTokenBalances {
let mut balances: TransactionTokenBalances = vec![];
Expand Down
1 change: 1 addition & 0 deletions programs/sbf/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 21 additions & 9 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<VersionedTransaction>) -> Result<TransactionBatch> {
pub fn prepare_entry_batch(
&self,
txs: Vec<VersionedTransaction>,
) -> Result<TransactionBatch<SanitizedTransaction>> {
let sanitized_txs = txs
.into_iter()
.map(|tx| {
Expand Down Expand Up @@ -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
Expand All @@ -3361,7 +3364,7 @@ impl Bank {
&'a self,
transactions: &'b [SanitizedTransaction],
transaction_results: impl Iterator<Item = Result<()>>,
) -> 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(
Expand All @@ -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);
Expand Down Expand Up @@ -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<SanitizedTransaction>,
) -> TransactionBalances {
let mut balances: TransactionBalances = vec![];
for transaction in batch.sanitized_transactions() {
let mut transaction_balances: Vec<u64> = vec![];
Expand All @@ -3548,7 +3554,7 @@ impl Bank {

pub fn load_and_execute_transactions(
&self,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
max_age: usize,
timings: &mut ExecuteTimings,
error_counters: &mut TransactionErrorMetrics,
Expand Down Expand Up @@ -4682,7 +4688,7 @@ impl Bank {
#[must_use]
pub fn load_execute_and_commit_transactions(
&self,
batch: &TransactionBatch,
batch: &TransactionBatch<SanitizedTransaction>,
max_age: usize,
collect_balances: bool,
recording_config: ExecutionRecordingConfig,
Expand Down Expand Up @@ -4788,7 +4794,10 @@ impl Bank {
}

#[must_use]
fn process_transaction_batch(&self, batch: &TransactionBatch) -> Vec<Result<()>> {
fn process_transaction_batch(
&self,
batch: &TransactionBatch<SanitizedTransaction>,
) -> Vec<Result<()>> {
self.load_execute_and_commit_transactions(
batch,
MAX_PROCESSING_AGE,
Expand Down Expand Up @@ -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<Transaction>) -> TransactionBatch {
pub fn prepare_batch_for_tests(
&self,
txs: Vec<Transaction>,
) -> TransactionBatch<SanitizedTransaction> {
let transaction_account_lock_limit = self.get_transaction_account_lock_limit();
let sanitized_txs = txs
.into_iter()
Expand Down
23 changes: 13 additions & 10 deletions runtime/src/transaction_batch.rs
Original file line number Diff line number Diff line change
@@ -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<Result<()>>,
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<Result<()>>,
bank: &'a Bank,
sanitized_txs: Cow<'b, [SanitizedTransaction]>,
sanitized_txs: Cow<'b, [Tx]>,
) -> Self {
assert_eq!(lock_results.len(), sanitized_txs.len());
Self {
Expand All @@ -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
}

Expand Down Expand Up @@ -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);
Expand All @@ -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]
Expand Down

0 comments on commit 2f8f910

Please sign in to comment.