diff --git a/Cargo.lock b/Cargo.lock index 4005d2f41b0079..778e7d26f8c9dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7805,6 +7805,7 @@ dependencies = [ name = "solana-svm" version = "2.1.0" dependencies = [ + "ahash 0.8.10", "assert_matches", "bincode", "itertools 0.12.1", diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 39e3dc1fd95d5c..e47b19c54ecfa9 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -915,55 +915,6 @@ mod tests { }, }; - fn execute_transactions_with_dummy_poh_service( - bank: Arc, - transactions: Vec, - ) -> ProcessTransactionsSummary { - let transactions = sanitize_transactions(transactions); - let ledger_path = get_tmp_ledger_path_auto_delete!(); - let blockstore = Blockstore::open(ledger_path.path()) - .expect("Expected to be able to open database ledger"); - let (poh_recorder, _entry_receiver, record_receiver) = PohRecorder::new( - bank.tick_height(), - bank.last_blockhash(), - bank.clone(), - Some((4, 4)), - bank.ticks_per_slot(), - Arc::new(blockstore), - &Arc::new(LeaderScheduleCache::new_from_bank(&bank)), - &PohConfig::default(), - Arc::new(AtomicBool::default()), - ); - let recorder = poh_recorder.new_recorder(); - let poh_recorder = Arc::new(RwLock::new(poh_recorder)); - - poh_recorder - .write() - .unwrap() - .set_bank_for_test(bank.clone()); - - let poh_simulator = simulate_poh(record_receiver, &poh_recorder); - - let (replay_vote_sender, _replay_vote_receiver) = unbounded(); - let committer = Committer::new( - None, - replay_vote_sender, - Arc::new(PrioritizationFeeCache::new(0u64)), - ); - let consumer = Consumer::new(committer, recorder, QosService::new(1), None); - let process_transactions_summary = - consumer.process_transactions(&bank, &Instant::now(), &transactions); - - poh_recorder - .read() - .unwrap() - .is_exited - .store(true, Ordering::Relaxed); - let _ = poh_simulator.join(); - - process_transactions_summary - } - fn generate_new_address_lookup_table( authority: Option, num_addresses: usize, @@ -1694,131 +1645,6 @@ mod tests { Blockstore::destroy(ledger_path.path()).unwrap(); } - #[test] - fn test_process_transactions_instruction_error() { - solana_logger::setup(); - let lamports = 10_000; - let GenesisConfigInfo { - genesis_config, - mint_keypair, - .. - } = create_slow_genesis_config(lamports); - let (bank, _bank_forks) = Bank::new_no_wallclock_throttle_for_tests(&genesis_config); - // set cost tracker limits to MAX so it will not filter out TXs - bank.write_cost_tracker() - .unwrap() - .set_limits(u64::MAX, u64::MAX, u64::MAX); - - // Transfer more than the balance of the mint keypair, should cause a - // InstructionError::InsufficientFunds that is then committed. Needs to be - // MAX_NUM_TRANSACTIONS_PER_BATCH at least so it doesn't conflict on account locks - // with the below transaction - let mut transactions = vec![ - system_transaction::transfer( - &mint_keypair, - &Pubkey::new_unique(), - lamports + 1, - genesis_config.hash(), - ); - TARGET_NUM_TRANSACTIONS_PER_BATCH - ]; - - // Make one transaction that will succeed. - transactions.push(system_transaction::transfer( - &mint_keypair, - &Pubkey::new_unique(), - 1, - genesis_config.hash(), - )); - - let transactions_len = transactions.len(); - let ProcessTransactionsSummary { - reached_max_poh_height, - transaction_counts, - retryable_transaction_indexes, - .. - } = execute_transactions_with_dummy_poh_service(bank, transactions); - - // All the transactions should have been replayed, but only 1 committed - assert!(!reached_max_poh_height); - assert_eq!( - transaction_counts, - CommittedTransactionsCounts { - attempted_processing_count: transactions_len as u64, - // Both transactions should have been committed, even though one was an error, - // because InstructionErrors are committed - committed_transactions_count: 2, - committed_transactions_with_successful_result_count: 1, - processed_but_failed_commit: 0, - } - ); - assert_eq!( - retryable_transaction_indexes, - (1..transactions_len - 1).collect::>() - ); - } - - #[test] - fn test_process_transactions_account_in_use() { - solana_logger::setup(); - let GenesisConfigInfo { - genesis_config, - mint_keypair, - .. - } = create_slow_genesis_config(10_000); - let (bank, _bank_forks) = Bank::new_no_wallclock_throttle_for_tests(&genesis_config); - // set cost tracker limits to MAX so it will not filter out TXs - bank.write_cost_tracker() - .unwrap() - .set_limits(u64::MAX, u64::MAX, u64::MAX); - - // Make all repetitive transactions that conflict on the `mint_keypair`, so only 1 should be executed - let mut transactions = vec![ - system_transaction::transfer( - &mint_keypair, - &Pubkey::new_unique(), - 1, - genesis_config.hash() - ); - TARGET_NUM_TRANSACTIONS_PER_BATCH - ]; - - // Make one more in separate batch that also conflicts, but because it's in a separate batch, it - // should be executed - transactions.push(system_transaction::transfer( - &mint_keypair, - &Pubkey::new_unique(), - 1, - genesis_config.hash(), - )); - - let transactions_len = transactions.len(); - let ProcessTransactionsSummary { - reached_max_poh_height, - transaction_counts, - retryable_transaction_indexes, - .. - } = execute_transactions_with_dummy_poh_service(bank, transactions); - - // All the transactions should have been replayed, but only 2 committed (first and last) - assert!(!reached_max_poh_height); - assert_eq!( - transaction_counts, - CommittedTransactionsCounts { - attempted_processing_count: transactions_len as u64, - committed_transactions_count: 2, - committed_transactions_with_successful_result_count: 2, - processed_but_failed_commit: 0, - } - ); - - // Everything except first and last index of the transactions failed and are last retryable - assert_eq!( - retryable_transaction_indexes, - (1..transactions_len - 1).collect::>() - ); - } - #[test] fn test_process_transactions_returns_unprocessed_txs() { solana_logger::setup(); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 5273b4601bfe33..54b03ede04fe14 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3827,16 +3827,11 @@ pub mod tests { &[&mint_keypair], bank.last_blockhash(), ); - // First process attempt will fail but still update status cache + // First process attempt will fail assert_eq!( bank.process_transaction(&tx), Err(TransactionError::ProgramAccountNotFound) ); - // Second attempt will be rejected since tx was already in status cache - assert_eq!( - bank.process_transaction(&tx), - Err(TransactionError::AlreadyProcessed) - ); // Make sure other errors don't update the signature cache let tx = system_transaction::transfer(&mint_keypair, &pubkey, 1000, Hash::default()); diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index d966d986fb8305..0aa6767d083b39 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -17,8 +17,11 @@ use { nonce_info::NonceInfo, transaction_error_metrics::TransactionErrorMetrics, }, + std::collections::HashSet, }; +const DEFAULT_NUM_TRANSACTIONS_PER_BATCH: usize = 64; + impl Bank { /// Checks a batch of sanitized transactions again bank for age and status pub fn check_transactions_with_forwarding_delay( @@ -61,7 +64,7 @@ impl Bank { self.check_status_cache(sanitized_txs, lock_results, error_counters) } - fn check_age( + pub fn check_age( &self, sanitized_txs: &[impl core::borrow::Borrow], lock_results: &[TransactionResult<()>], @@ -71,18 +74,26 @@ impl Bank { let hash_queue = self.blockhash_queue.read().unwrap(); let last_blockhash = hash_queue.last_hash(); let next_durable_nonce = DurableNonce::from_blockhash(&last_blockhash); + let mut entry_level_dedup_map = HashSet::with_capacity(DEFAULT_NUM_TRANSACTIONS_PER_BATCH); sanitized_txs .iter() .zip(lock_results) .map(|(tx, lock_res)| match lock_res { - Ok(()) => self.check_transaction_age( - tx.borrow(), - max_age, - &next_durable_nonce, - &hash_queue, - error_counters, - ), + Ok(()) => { + let msg_hash = tx.borrow().message_hash(); + if !entry_level_dedup_map.insert(msg_hash) { + return Err(TransactionError::AlreadyProcessed); + } + + self.check_transaction_age( + tx.borrow(), + max_age, + &next_durable_nonce, + &hash_queue, + error_counters, + ) + } Err(e) => Err(e.clone()), }) .collect() diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 56612762743f97..30cccb3f51c908 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -3053,6 +3053,44 @@ fn test_readonly_accounts() { assert_eq!(results[1], Err(TransactionError::AccountInUse)); } +#[test] +fn test_check_transaction_age() { + let (genesis_config, mint_keypair) = create_genesis_config(sol_to_lamports(1.)); + let bank = Bank::new_with_bank_forks_for_tests(&genesis_config).0; + let alice = Keypair::new(); + let amount = genesis_config.rent.minimum_balance(0); + + let tx1 = system_transaction::transfer( + &mint_keypair, + &alice.pubkey(), + amount, + genesis_config.hash(), + ); + + let tx2 = system_transaction::transfer( + &mint_keypair, + &alice.pubkey(), + amount, + genesis_config.hash(), + ); + let pay_alice = vec![tx1, tx2]; + + let batch = bank.prepare_batch_for_tests(pay_alice); + let mut error_counters = TransactionErrorMetrics::default(); + let lock_result = vec![Ok(()), Ok(())]; + let check_age_results = bank.check_age( + batch.sanitized_transactions(), + &lock_result, + MAX_PROCESSING_AGE, + &mut error_counters, + ); + + assert_eq!( + check_age_results[1], + Err(TransactionError::AlreadyProcessed) + ); //Duplicate transactions in check_age should give AlreadyProcessed error +} + #[test] fn test_interleaving_locks() { let (genesis_config, mint_keypair) = create_genesis_config(sol_to_lamports(1.)); @@ -3121,45 +3159,28 @@ fn test_load_and_execute_commit_transactions_fees_only(enable_fees_only_txs: boo genesis_config.epoch_schedule.get_first_slot_in_epoch(1), ); - // Use rent-paying fee payer to show that rent is not collected for fees - // only transactions even when they use a rent-paying account. - let rent_paying_fee_payer = Pubkey::new_unique(); - bank.store_account( - &rent_paying_fee_payer, - &AccountSharedData::new( - genesis_config.rent.minimum_balance(0) - 1, - 0, - &system_program::id(), - ), - ); + let source_keypair = Keypair::new(); + let source = source_keypair.pubkey(); + let destination = Pubkey::new_unique(); - // Use nonce to show that loaded account stats also included loaded - // nonce account size - let nonce_size = nonce::State::size(); - let nonce_balance = genesis_config.rent.minimum_balance(nonce_size); - let nonce_pubkey = Pubkey::new_unique(); - let nonce_authority = rent_paying_fee_payer; - let nonce_initial_hash = DurableNonce::from_blockhash(&Hash::new_unique()); - let nonce_data = nonce::state::Data::new(nonce_authority, nonce_initial_hash, 5000); - let nonce_account = AccountSharedData::new_data( - nonce_balance, - &nonce::state::Versions::new(nonce::State::Initialized(nonce_data.clone())), - &system_program::id(), - ) - .unwrap(); - bank.store_account(&nonce_pubkey, &nonce_account); + let mut source_data = AccountSharedData::default(); + let mut destination_data = AccountSharedData::default(); - // Invoke missing program to trigger load error in order to commit a - // fees-only transaction - let missing_program_id = Pubkey::new_unique(); - let transaction = Transaction::new_unsigned(Message::new_with_blockhash( - &[ - system_instruction::advance_nonce_account(&nonce_pubkey, &rent_paying_fee_payer), - Instruction::new_with_bincode(missing_program_id, &0, vec![]), - ], - Some(&rent_paying_fee_payer), - &nonce_data.blockhash(), - )); + let data = vec![5u8; 66 * 1024 * 1024]; + source_data.set_lamports(LAMPORTS_PER_SOL * 10); + // source_data.set_data(data.clone()); + destination_data.set_lamports(LAMPORTS_PER_SOL); + destination_data.set_data(data.clone()); + + bank.store_account(&source, &source_data); + bank.store_account(&destination, &destination_data); + + let transaction = system_transaction::transfer( + &source_keypair, + &destination, + LAMPORTS_PER_SOL, + genesis_config.hash(), + ); let batch = bank.prepare_batch_for_tests(vec![transaction]); let commit_results = bank @@ -3177,7 +3198,7 @@ fn test_load_and_execute_commit_transactions_fees_only(enable_fees_only_txs: boo assert_eq!( commit_results, vec![Ok(CommittedTransaction { - status: Err(TransactionError::ProgramAccountNotFound), + status: Err(TransactionError::MaxLoadedAccountsDataSizeExceeded), log_messages: None, inner_instructions: None, return_data: None, @@ -3185,15 +3206,15 @@ fn test_load_and_execute_commit_transactions_fees_only(enable_fees_only_txs: boo fee_details: FeeDetails::new(5000, 0, true), rent_debits: RentDebits::default(), loaded_account_stats: TransactionLoadedAccountsStats { - loaded_accounts_count: 2, - loaded_accounts_data_size: nonce_size as u32, + loaded_accounts_count: 1, + loaded_accounts_data_size: 0, }, })] ); } else { assert_eq!( commit_results, - vec![Err(TransactionError::ProgramAccountNotFound)] + vec![Err(TransactionError::MaxLoadedAccountsDataSizeExceeded)] ); } } @@ -5226,6 +5247,114 @@ fn test_nonce_transaction() { ); } +#[test] +fn test_two_nonce_transactions() { + let (mut bank, _mint_keypair, custodian_keypair, nonce_keypair, bank_forks) = + setup_nonce_with_bank( + 10_000_000, + |_| {}, + 5_000_000, + 250_000, + None, + FeatureSet::all_enabled(), + ) + .unwrap(); + let alice_keypair = Keypair::new(); + let alice_pubkey = alice_keypair.pubkey(); + let custodian_pubkey = custodian_keypair.pubkey(); + let nonce_pubkey = nonce_keypair.pubkey(); + + assert_eq!(bank.get_balance(&custodian_pubkey), 4_750_000); + assert_eq!(bank.get_balance(&nonce_pubkey), 250_000); + + /* Grab the hash stored in the nonce account */ + let nonce_hash = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); + + /* Kick nonce hash off the blockhash_queue */ + for _ in 0..MAX_RECENT_BLOCKHASHES + 1 { + goto_end_of_slot(bank.clone()); + bank = new_from_parent_with_fork_next_slot(bank, bank_forks.as_ref()); + } + + /* Expect a non-Nonce transfer to fail */ + assert_eq!( + bank.process_transaction(&system_transaction::transfer( + &custodian_keypair, + &alice_pubkey, + 100_000, + nonce_hash + ),), + Err(TransactionError::BlockhashNotFound), + ); + /* Check fee not charged */ + assert_eq!(bank.get_balance(&custodian_pubkey), 4_750_000); + + /* Nonce transfer */ + let nonce_tx = Transaction::new_signed_with_payer( + &[ + system_instruction::advance_nonce_account(&nonce_pubkey, &nonce_pubkey), + system_instruction::transfer(&custodian_pubkey, &alice_pubkey, 100_000), + ], + Some(&custodian_pubkey), + &[&custodian_keypair, &nonce_keypair], + nonce_hash, + ); + // assert_eq!(bank.process_transaction(&nonce_tx), Ok(())); + let mut all_transactions = Vec::new(); + all_transactions.push(nonce_tx.clone()); + + let nonce_tx2 = Transaction::new_signed_with_payer( + &[ + system_instruction::advance_nonce_account(&nonce_pubkey, &nonce_pubkey), + system_instruction::transfer(&custodian_pubkey, &alice_pubkey, 100_000), + ], + Some(&custodian_pubkey), + &[&custodian_keypair, &nonce_keypair], + nonce_hash, + ); + + all_transactions.push(nonce_tx2.clone()); + let _execution_results = bank.try_process_transactions(&mut all_transactions.iter()); + + /* Check balances */ + let mut recent_message = nonce_tx.message; + recent_message.recent_blockhash = bank.last_blockhash(); + let expected_balance = 4_650_000 + - bank + .get_fee_for_message(&new_sanitized_message(recent_message)) + .unwrap(); + assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); + assert_eq!(bank.get_balance(&nonce_pubkey), 250_000); + assert_eq!(bank.get_balance(&alice_pubkey), 100_000); + + /* Confirm stored nonce has advanced */ + let new_nonce = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); + assert_ne!(nonce_hash, new_nonce); + + // /* Nonce re-use fails */ + // let nonce_tx = Transaction::new_signed_with_payer( + // &[ + // system_instruction::advance_nonce_account(&nonce_pubkey, &nonce_pubkey), + // system_instruction::transfer(&custodian_pubkey, &alice_pubkey, 100_000), + // ], + // Some(&custodian_pubkey), + // &[&custodian_keypair, &nonce_keypair], + // nonce_hash, + // ); + // assert_eq!( + // execution_results., + // Err(TransactionError::BlockhashNotFound) + // ); + + // println!("results: {:?}", execution_results); + /* Check fee not charged and nonce not advanced */ + assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); + assert_eq!( + new_nonce, + get_nonce_blockhash(&bank, &nonce_pubkey).unwrap() + ); +} + #[test] fn test_nonce_transaction_with_tx_wide_caps() { let feature_set = FeatureSet::all_enabled(); diff --git a/svm/Cargo.toml b/svm/Cargo.toml index 9f0d80c4d1f5ad..e881ad51f9eaaf 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -10,6 +10,7 @@ license = { workspace = true } edition = { workspace = true } [dependencies] +ahash = { workspace = true } itertools = { workspace = true } log = { workspace = true } percentage = { workspace = true } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 0799ac8ba3cec7..6c2f8a229b67a6 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -30,12 +30,15 @@ use { solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, solana_svm_transaction::svm_message::SVMMessage, solana_system_program::{get_system_account_kind, SystemAccountKind}, - std::num::NonZeroU32, + std::{collections::HashMap, num::NonZeroU32}, }; // for the load instructions pub(crate) type TransactionRent = u64; pub(crate) type TransactionProgramIndices = Vec>; +pub type TransactionProgramIndicestResult = Result; +pub type TransactionLoadAccountResult = Result>; +pub type TransactionRentResult = Result; pub type TransactionCheckResult = Result; pub type TransactionValidationResult = Result; @@ -51,8 +54,24 @@ pub enum TransactionLoadResult { /// loaded NotLoaded(TransactionError), } +pub type UniqueLoadedAccounts = HashMap; + +#[derive(Clone)] +#[cfg_attr(feature = "dev-context-only-utils", derive(Default))] +pub struct RentDetails { + pub rent: TransactionRent, + pub rent_debits: RentDebits, + pub loaded_accounts_data_size: u32, +} #[derive(PartialEq, Eq, Debug, Clone)] +pub struct LoadedAccountDetails { + pub pubkey: Pubkey, + pub account_found: bool, +} + +#[derive(PartialEq, Eq, Debug, Clone)] +#[cfg_attr(feature = "dev-context-only-utils", derive(Default))] pub struct CheckedTransactionDetails { pub nonce: Option, pub lamports_per_signature: u64, @@ -94,6 +113,48 @@ pub struct FeesOnlyTransaction { pub rollback_accounts: RollbackAccounts, pub fee_details: FeeDetails, } +pub fn update_unique_loaded_accounts( + loaded_transaction: &LoadedTransaction, + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) { + loaded_transaction + .accounts + .iter() + .for_each(|(key, account)| { + unique_loaded_accounts.insert(*key, account.clone()); + }); +} + +pub fn limited_update_unique_loaded_accounts( + message: &impl SVMMessage, + loaded_transaction: &LoadedTransaction, + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) { + let fee_payer_address = message.fee_payer(); + match &loaded_transaction.rollback_accounts { + RollbackAccounts::FeePayerOnly { fee_payer_account } => { + if let Some(account) = unique_loaded_accounts.get_mut(fee_payer_address) { + *account = fee_payer_account.clone(); + } + } + RollbackAccounts::SameNonceAndFeePayer { nonce } => { + if let Some(account) = unique_loaded_accounts.get_mut(nonce.address()) { + *account = nonce.account().clone(); + } + } + RollbackAccounts::SeparateNonceAndFeePayer { + nonce, + fee_payer_account, + } => { + if let Some(account) = unique_loaded_accounts.get_mut(fee_payer_address) { + *account = fee_payer_account.clone(); + } + if let Some(account) = unique_loaded_accounts.get_mut(nonce.address()) { + *account = nonce.account().clone(); + } + } + } +} /// Collect rent from an account if rent is still enabled and regardless of /// whether rent is enabled, set the rent epoch to u64::MAX if the account is @@ -181,176 +242,62 @@ pub fn validate_fee_payer( ) } -/// Collect information about accounts used in txs transactions and -/// return vector of tuples, one for each transaction in the -/// batch. Each tuple contains struct of information about accounts as -/// its first element and an optional transaction nonce info as its -/// second element. -pub(crate) fn load_accounts( +pub(crate) fn calculate_program_indices( callbacks: &CB, txs: &[impl SVMMessage], - validation_results: Vec, + initial_load_results: &mut Vec, error_metrics: &mut TransactionErrorMetrics, - account_overrides: Option<&AccountOverrides>, - feature_set: &FeatureSet, - rent_collector: &dyn SVMRentCollector, - loaded_programs: &ProgramCacheForTxBatch, -) -> Vec { + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) -> Vec { txs.iter() - .zip(validation_results) - .map(|(transaction, validation_result)| { - load_transaction( - callbacks, - transaction, - validation_result, - error_metrics, - account_overrides, - feature_set, - rent_collector, - loaded_programs, - ) + .zip(initial_load_results) + .map(|etx| match etx { + (message, Ok(loaded_accounts)) => { + // load transaction indices + match load_transaction_indices( + callbacks, + message, + loaded_accounts, + error_metrics, + unique_loaded_accounts, + ) { + Ok(program_indices) => Ok(program_indices), + Err(e) => Err(e), + } + } + (_, Err(e)) => Err(e.clone()), }) .collect() } -fn load_transaction( - callbacks: &CB, - message: &impl SVMMessage, - validation_result: TransactionValidationResult, - error_metrics: &mut TransactionErrorMetrics, - account_overrides: Option<&AccountOverrides>, - feature_set: &FeatureSet, - rent_collector: &dyn SVMRentCollector, - loaded_programs: &ProgramCacheForTxBatch, -) -> TransactionLoadResult { - match validation_result { - Err(e) => TransactionLoadResult::NotLoaded(e), - Ok(tx_details) => { - let load_result = load_transaction_accounts( - callbacks, - message, - tx_details.loaded_fee_payer_account, - &tx_details.compute_budget_limits, - error_metrics, - account_overrides, - feature_set, - rent_collector, - loaded_programs, - ); - - match load_result { - Ok(loaded_tx_accounts) => TransactionLoadResult::Loaded(LoadedTransaction { - accounts: loaded_tx_accounts.accounts, - program_indices: loaded_tx_accounts.program_indices, - fee_details: tx_details.fee_details, - rent: loaded_tx_accounts.rent, - rent_debits: loaded_tx_accounts.rent_debits, - rollback_accounts: tx_details.rollback_accounts, - compute_budget_limits: tx_details.compute_budget_limits, - loaded_accounts_data_size: loaded_tx_accounts.loaded_accounts_data_size, - }), - Err(err) => TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: err, - fee_details: tx_details.fee_details, - rollback_accounts: tx_details.rollback_accounts, - }), - } - } - } -} - -#[derive(PartialEq, Eq, Debug, Clone)] -struct LoadedTransactionAccounts { - pub accounts: Vec, - pub program_indices: TransactionProgramIndices, - pub rent: TransactionRent, - pub rent_debits: RentDebits, - pub loaded_accounts_data_size: u32, -} - -fn load_transaction_accounts( +fn load_transaction_indices( callbacks: &CB, message: &impl SVMMessage, - loaded_fee_payer_account: LoadedTransactionAccount, - compute_budget_limits: &ComputeBudgetLimits, + accounts: &mut Vec, error_metrics: &mut TransactionErrorMetrics, - account_overrides: Option<&AccountOverrides>, - feature_set: &FeatureSet, - rent_collector: &dyn SVMRentCollector, - loaded_programs: &ProgramCacheForTxBatch, -) -> Result { - let mut tx_rent: TransactionRent = 0; - let account_keys = message.account_keys(); - let mut accounts = Vec::with_capacity(account_keys.len()); - let mut accounts_found = Vec::with_capacity(account_keys.len()); - let mut rent_debits = RentDebits::default(); - let mut accumulated_accounts_data_size: u32 = 0; - - let instruction_accounts = message - .instructions_iter() - .flat_map(|instruction| instruction.accounts) - .unique() - .collect::>(); - - let mut collect_loaded_account = |key, (loaded_account, found)| -> Result<()> { - let LoadedTransactionAccount { - account, - loaded_size, - rent_collected, - } = loaded_account; - - accumulate_and_check_loaded_account_data_size( - &mut accumulated_accounts_data_size, - loaded_size, - compute_budget_limits.loaded_accounts_bytes, - error_metrics, - )?; - - tx_rent += rent_collected; - rent_debits.insert(key, rent_collected, account.lamports()); - - accounts.push((*key, account)); - accounts_found.push(found); - Ok(()) - }; - - // Since the fee payer is always the first account, collect it first. Note - // that account overrides are already applied during fee payer validation so - // it's fine to use the fee payer directly here rather than checking account - // overrides again. - collect_loaded_account(message.fee_payer(), (loaded_fee_payer_account, true))?; - - // Attempt to load and collect remaining non-fee payer accounts - for (account_index, account_key) in account_keys.iter().enumerate().skip(1) { - let (loaded_account, account_found) = load_transaction_account( - callbacks, - message, - account_key, - account_index, - &instruction_accounts[..], - account_overrides, - feature_set, - rent_collector, - loaded_programs, - )?; - collect_loaded_account(account_key, (loaded_account, account_found))?; - } - + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) -> TransactionProgramIndicestResult { let builtins_start_index = accounts.len(); let program_indices = message .instructions_iter() .map(|instruction| { let mut account_indices = Vec::with_capacity(2); let program_index = instruction.program_id_index as usize; - // This command may never return error, because the transaction is sanitized - let (program_id, program_account) = accounts - .get(program_index) - .ok_or(TransactionError::ProgramAccountNotFound)?; - if native_loader::check_id(program_id) { + // This command should never return error because the transaction was already added in the unique_loaded_accounts + let (program_id, program_account) = + if let Some(loaded_account) = accounts.get(program_index) { + let account = unique_loaded_accounts.get(&loaded_account.pubkey).unwrap(); + (loaded_account.pubkey, account) + } else { + return Err(TransactionError::ProgramAccountNotFound); + }; + if native_loader::check_id(&program_id) { return Ok(account_indices); } - let account_found = accounts_found.get(program_index).unwrap_or(&true); + let account_found = accounts + .get(program_index) + .map_or(true, |loaded_account| loaded_account.account_found); if !account_found { error_metrics.account_not_found += 1; return Err(TransactionError::ProgramAccountNotFound); @@ -369,7 +316,7 @@ fn load_transaction_accounts( .get(builtins_start_index..) .ok_or(TransactionError::ProgramAccountNotFound)? .iter() - .any(|(key, _)| key == owner_id) + .any(|loaded_account| &loaded_account.pubkey == owner_id) { if let Some(owner_account) = callbacks.get_account_shared_data(owner_id) { if !native_loader::check_id(owner_account.owner()) @@ -378,13 +325,11 @@ fn load_transaction_accounts( error_metrics.invalid_program_for_execution += 1; return Err(TransactionError::InvalidProgramForExecution); } - accumulate_and_check_loaded_account_data_size( - &mut accumulated_accounts_data_size, - owner_account.data().len(), - compute_budget_limits.loaded_accounts_bytes, - error_metrics, - )?; - accounts.push((*owner_id, owner_account)); + accounts.push(LoadedAccountDetails { + pubkey: *owner_id, + account_found: true, + }); + unique_loaded_accounts.insert(*owner_id, owner_account); } else { error_metrics.account_not_found += 1; return Err(TransactionError::ProgramAccountNotFound); @@ -392,15 +337,83 @@ fn load_transaction_accounts( } Ok(account_indices) }) - .collect::>>>()?; - - Ok(LoadedTransactionAccounts { - accounts, - program_indices, - rent: tx_rent, - rent_debits, - loaded_accounts_data_size: accumulated_accounts_data_size, - }) + .collect::>>()?; + + Ok(program_indices) +} + +/// Collect information about accounts used in txs and +/// return vector of TransactionLoadAccountResult for each transaction in the +/// batch. Each struct contains public keys of accounts used in the transaction +/// and wheather the account was found or not in accounts-db. +/// Load unique accounts in `unique_loaded_accounts` and returns only loading error +pub(crate) fn load_accounts( + callbacks: &CB, + messages: &[impl SVMMessage], + check_results: &[TransactionCheckResult], + account_overrides: Option<&AccountOverrides>, + loaded_programs: &ProgramCacheForTxBatch, + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) -> Vec { + messages + .iter() + .zip(check_results.iter()) + .map(|etx| match etx { + (message, Ok(_)) => { + // load transactions + match load_transaction_accounts( + callbacks, + message, + account_overrides, + loaded_programs, + unique_loaded_accounts, + ) { + Ok(loaded_accounts) => Ok(loaded_accounts), + Err(e) => Err(e), + } + } + (_, Err(e)) => Err(e.clone()), + }) + .collect() +} + +fn load_transaction_accounts( + callbacks: &CB, + message: &impl SVMMessage, + account_overrides: Option<&AccountOverrides>, + loaded_programs: &ProgramCacheForTxBatch, + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) -> TransactionLoadAccountResult { + let account_keys = message.account_keys(); + let mut accounts: Vec = Vec::with_capacity(account_keys.len()); + + let instruction_accounts = message + .instructions_iter() + .flat_map(|instruction| instruction.accounts) + .unique() + .collect::>(); + + // Attempt to load and collect remaining non-fee payer accounts + for (account_index, account_key) in account_keys.iter().enumerate() { + let (account_found, account) = load_transaction_account( + callbacks, + message, + account_key, + account_index, + &instruction_accounts[..], + account_overrides, + loaded_programs, + unique_loaded_accounts, + )?; + + unique_loaded_accounts.insert(*account_key, account.clone()); + accounts.push(LoadedAccountDetails { + pubkey: *account_key, + account_found, + }); + } + + Ok(accounts) } fn load_transaction_account( @@ -410,12 +423,10 @@ fn load_transaction_account( account_index: usize, instruction_accounts: &[&u8], account_overrides: Option<&AccountOverrides>, - feature_set: &FeatureSet, - rent_collector: &dyn SVMRentCollector, loaded_programs: &ProgramCacheForTxBatch, -) -> Result<(LoadedTransactionAccount, bool)> { + unique_loaded_accounts: &mut UniqueLoadedAccounts, +) -> Result<(bool, AccountSharedData)> { let mut account_found = true; - let mut was_inspected = false; let is_instruction_account = u8::try_from(account_index) .map(|i| instruction_accounts.contains(&&i)) .unwrap_or(false); @@ -423,19 +434,11 @@ fn load_transaction_account( let loaded_account = if solana_sdk::sysvar::instructions::check_id(account_key) { // Since the instructions sysvar is constructed by the SVM and modified // for each transaction instruction, it cannot be overridden. - LoadedTransactionAccount { - loaded_size: 0, - account: construct_instructions_account(message), - rent_collected: 0, - } + (account_found, construct_instructions_account(message)) } else if let Some(account_override) = account_overrides.and_then(|overrides| overrides.get(account_key)) { - LoadedTransactionAccount { - loaded_size: account_override.data().len(), - account: account_override.clone(), - rent_collected: 0, - } + (account_found, account_override.clone()) } else if let Some(program) = (!is_instruction_account && !is_writable) .then_some(()) .and_then(|_| loaded_programs.find(account_key)) @@ -445,43 +448,11 @@ fn load_transaction_account( .ok_or(TransactionError::AccountNotFound)?; // Optimization to skip loading of accounts which are only used as // programs in top-level instructions and not passed as instruction accounts. - LoadedTransactionAccount { - loaded_size: program.account_size, - account: account_shared_data_from_program(&program), - rent_collected: 0, - } - } else { + (account_found, account_shared_data_from_program(&program)) + } else if !unique_loaded_accounts.contains_key(account_key) { callbacks .get_account_shared_data(account_key) - .map(|mut account| { - let rent_collected = if is_writable { - // Inspect the account prior to collecting rent, since - // rent collection can modify the account. - debug_assert!(!was_inspected); - callbacks.inspect_account( - account_key, - AccountState::Alive(&account), - is_writable, - ); - was_inspected = true; - - collect_rent_from_account( - feature_set, - rent_collector, - account_key, - &mut account, - ) - .rent_amount - } else { - 0 - }; - - LoadedTransactionAccount { - loaded_size: account.data().len(), - account, - rent_collected, - } - }) + .map(|account| (account_found, account)) .unwrap_or_else(|| { account_found = false; let mut default_account = AccountSharedData::default(); @@ -489,24 +460,33 @@ fn load_transaction_account( // Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account // with this field already set would allow us to skip rent collection for these accounts. default_account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - LoadedTransactionAccount { - loaded_size: default_account.data().len(), - account: default_account, - rent_collected: 0, - } + (account_found, default_account) }) + } else { + let account = unique_loaded_accounts + .get(account_key) + .cloned() + .unwrap_or_else(|| { + // this should never happen + account_found = false; + let mut default_account = AccountSharedData::default(); + // All new accounts must be rent-exempt (enforced in Bank::execute_loaded_transaction). + // Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account + // with this field already set would allow us to skip rent collection for these accounts. + default_account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + default_account + }); + (account_found, account) }; - if !was_inspected { - let account_state = if account_found { - AccountState::Alive(&loaded_account.account) - } else { - AccountState::Dead - }; - callbacks.inspect_account(account_key, account_state, is_writable); - } + let account_state = if account_found { + AccountState::Alive(&loaded_account.1) + } else { + AccountState::Dead + }; + callbacks.inspect_account(account_key, account_state, is_writable); - Ok((loaded_account, account_found)) + Ok(loaded_account) } fn account_shared_data_from_program(loaded_program: &ProgramCacheEntry) -> AccountSharedData { @@ -523,7 +503,7 @@ fn account_shared_data_from_program(loaded_program: &ProgramCacheEntry) -> Accou /// Returns TransactionErr::MaxLoadedAccountsDataSizeExceeded if /// `accumulated_accounts_data_size` exceeds /// `requested_loaded_accounts_data_size_limit`. -fn accumulate_and_check_loaded_account_data_size( +pub fn accumulate_and_check_loaded_account_data_size( accumulated_loaded_accounts_data_size: &mut u32, account_data_size: usize, requested_loaded_accounts_data_size_limit: NonZeroU32, @@ -580,13 +560,17 @@ mod tests { crate::{ transaction_account_state_info::TransactionAccountStateInfo, transaction_processing_callback::TransactionProcessingCallback, + transaction_processor::TransactionBatchProcessor, }, nonce::state::Versions as NonceVersions, solana_compute_budget::{compute_budget::ComputeBudget, compute_budget_limits}, - solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch}, + solana_program_runtime::loaded_programs::{ + BlockRelation, ForkGraph, ProgramCacheEntry, ProgramCacheForTxBatch, + }, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, bpf_loader_upgradeable, + clock::Slot, epoch_schedule::EpochSchedule, feature_set::FeatureSet, hash::Hash, @@ -601,7 +585,6 @@ mod tests { pubkey::Pubkey, rent::Rent, rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, - rent_debits::RentDebits, reserved_account_keys::ReservedAccountKeys, signature::{Keypair, Signature, Signer}, system_program, system_transaction, sysvar, @@ -611,6 +594,14 @@ mod tests { std::{borrow::Cow, cell::RefCell, collections::HashMap, sync::Arc}, }; + struct TestForkGraph {} + + impl ForkGraph for TestForkGraph { + fn relationship(&self, _a: Slot, _b: Slot) -> BlockRelation { + BlockRelation::Unknown + } + } + #[derive(Default)] struct TestCallbacks { accounts_map: HashMap, @@ -649,13 +640,10 @@ mod tests { fn load_accounts_with_features_and_rent( tx: Transaction, accounts: &[TransactionAccount], - rent_collector: &RentCollector, error_metrics: &mut TransactionErrorMetrics, - feature_set: &mut FeatureSet, ) -> Vec { - feature_set.deactivate(&feature_set::disable_rent_fees_collection::id()); - let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); let fee_payer_account = accounts[0].1.clone(); + let loaded_programs = ProgramCacheForTxBatch::default(); let mut accounts_map = HashMap::new(); for (pubkey, account) in accounts { accounts_map.insert(*pubkey, account.clone()); @@ -664,22 +652,51 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); + let load_result = load_transaction_accounts( &callbacks, - &[sanitized_tx], - vec![Ok(ValidatedTransactionDetails { - loaded_fee_payer_account: LoadedTransactionAccount { - account: fee_payer_account, - ..LoadedTransactionAccount::default() - }, - ..ValidatedTransactionDetails::default() - })], - error_metrics, + sanitized_tx.message(), None, - feature_set, - rent_collector, - &ProgramCacheForTxBatch::default(), - ) + &loaded_programs, + &mut unique_loaded_accounts, + ); + + let mut accounts = load_result.clone().unwrap(); + let program_indices = load_transaction_indices( + &callbacks, + sanitized_tx.message(), + &mut accounts, + error_metrics, + &mut unique_loaded_accounts, + ); + + let validation_result = ValidatedTransactionDetails { + loaded_fee_payer_account: LoadedTransactionAccount { + account: fee_payer_account, + ..LoadedTransactionAccount::default() + }, + ..ValidatedTransactionDetails::default() + }; + + if load_result.is_err() { + return vec![TransactionLoadResult::NotLoaded(load_result.unwrap_err())]; + } else if program_indices.is_err() { + return vec![TransactionLoadResult::NotLoaded( + program_indices.unwrap_err(), + )]; + } + let batch_processor = TransactionBatchProcessor::::default(); + let loaded_transaction = batch_processor.create_loaded_transaction( + &Ok(accounts), + &program_indices.unwrap(), + &Ok(validation_result), + &RentDetails::default(), + &mut unique_loaded_accounts, + ); + + vec![TransactionLoadResult::Loaded(loaded_transaction)] } /// get a feature set with all features activated @@ -704,28 +721,15 @@ mod tests { accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, ) -> Vec { - load_accounts_with_features_and_rent( - tx, - accounts, - &RentCollector::default(), - error_metrics, - &mut FeatureSet::all_enabled(), - ) + load_accounts_with_features_and_rent(tx, accounts, error_metrics) } fn load_accounts_with_excluded_features( tx: Transaction, accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, - exclude_features: Option<&[Pubkey]>, ) -> Vec { - load_accounts_with_features_and_rent( - tx, - accounts, - &RentCollector::default(), - error_metrics, - &mut all_features_except(exclude_features), - ) + load_accounts_with_features_and_rent(tx, accounts, error_metrics) } #[test] @@ -758,10 +762,7 @@ mod tests { assert_eq!(load_results.len(), 1); assert!(matches!( load_results[0], - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::ProgramAccountNotFound, - .. - }), + TransactionLoadResult::NotLoaded(TransactionError::ProgramAccountNotFound), )); } @@ -792,7 +793,7 @@ mod tests { ); let loaded_accounts = - load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics, None); + load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics); assert_eq!(error_metrics.account_not_found, 0); assert_eq!(loaded_accounts.len(), 1); @@ -840,10 +841,7 @@ mod tests { assert_eq!(load_results.len(), 1); assert!(matches!( load_results[0], - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::ProgramAccountNotFound, - .. - }), + TransactionLoadResult::NotLoaded(TransactionError::ProgramAccountNotFound), )); } @@ -877,10 +875,7 @@ mod tests { assert_eq!(load_results.len(), 1); assert!(matches!( load_results[0], - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::InvalidProgramForExecution, - .. - }), + TransactionLoadResult::NotLoaded(TransactionError::InvalidProgramForExecution), )); } @@ -923,7 +918,7 @@ mod tests { ); let loaded_accounts = - load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics, None); + load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics); assert_eq!(error_metrics.account_not_found, 0); assert_eq!(loaded_accounts.len(), 1); @@ -945,9 +940,10 @@ mod tests { tx: Transaction, account_overrides: Option<&AccountOverrides>, ) -> Vec { - let tx = SanitizedTransaction::from_transaction_for_tests(tx); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(tx); let mut error_metrics = TransactionErrorMetrics::default(); + let loaded_programs = ProgramCacheForTxBatch::default(); let mut accounts_map = HashMap::new(); for (pubkey, account) in accounts { accounts_map.insert(*pubkey, account.clone()); @@ -956,16 +952,43 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &callbacks, - &[tx], - vec![Ok(ValidatedTransactionDetails::default())], - &mut error_metrics, + sanitized_transaction.message(), account_overrides, - &FeatureSet::all_enabled(), - &RentCollector::default(), - &ProgramCacheForTxBatch::default(), - ) + &loaded_programs, + &mut unique_loaded_accounts, + ); + + let program_indices = load_transaction_indices( + &callbacks, + sanitized_transaction.message(), + &mut load_result.clone().unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); + + let validation_result = ValidatedTransactionDetails::default(); + + if load_result.is_err() { + return vec![TransactionLoadResult::NotLoaded(load_result.unwrap_err())]; + } else if program_indices.is_err() { + return vec![TransactionLoadResult::NotLoaded( + program_indices.unwrap_err(), + )]; + } + let batch_processor = TransactionBatchProcessor::::default(); + let loaded_transaction = batch_processor.create_loaded_transaction( + &load_result, + &program_indices.unwrap(), + &Ok(validation_result), + &RentDetails::default(), + &mut unique_loaded_accounts, + ); + + vec![TransactionLoadResult::Loaded(loaded_transaction)] } #[test] @@ -986,10 +1009,7 @@ mod tests { assert_eq!(load_results.len(), 1); assert!(matches!( load_results[0], - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::ProgramAccountNotFound, - .. - }), + TransactionLoadResult::NotLoaded(TransactionError::ProgramAccountNotFound), )); } @@ -1217,68 +1237,6 @@ mod tests { assert_eq!(shared_data, expected); } - #[test] - fn test_load_transaction_accounts_fee_payer() { - let fee_payer_address = Pubkey::new_unique(); - let message = Message { - account_keys: vec![fee_payer_address], - header: MessageHeader::default(), - instructions: vec![], - recent_blockhash: Hash::default(), - }; - - let sanitized_message = new_unchecked_sanitized_message(message); - let mut mock_bank = TestCallbacks::default(); - - let fee_payer_balance = 200; - let mut fee_payer_account = AccountSharedData::default(); - fee_payer_account.set_lamports(fee_payer_balance); - mock_bank - .accounts_map - .insert(fee_payer_address, fee_payer_account.clone()); - let fee_payer_rent_debit = 42; - - let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); - - let sanitized_transaction = SanitizedTransaction::new_for_tests( - sanitized_message, - vec![Signature::new_unique()], - false, - ); - let result = load_transaction_accounts( - &mock_bank, - sanitized_transaction.message(), - LoadedTransactionAccount { - loaded_size: fee_payer_account.data().len(), - account: fee_payer_account.clone(), - rent_collected: fee_payer_rent_debit, - }, - &ComputeBudgetLimits::default(), - &mut error_metrics, - None, - &FeatureSet::default(), - &RentCollector::default(), - &loaded_programs, - ); - - let expected_rent_debits = { - let mut rent_debits = RentDebits::default(); - rent_debits.insert(&fee_payer_address, fee_payer_rent_debit, fee_payer_balance); - rent_debits - }; - assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![(fee_payer_address, fee_payer_account)], - program_indices: vec![], - rent: fee_payer_rent_debit, - rent_debits: expected_rent_debits, - loaded_accounts_data_size: 0, - } - ); - } - #[test] fn test_load_transaction_accounts_native_loader() { let key1 = Keypair::new(); @@ -1304,7 +1262,6 @@ mod tests { .accounts_map .insert(key1.pubkey(), fee_payer_account.clone()); - let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( @@ -1312,36 +1269,33 @@ mod tests { vec![Signature::new_unique()], false, ); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); let result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount { - account: fee_payer_account.clone(), - ..LoadedTransactionAccount::default() - }, - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); + let result_keys: Vec = result + .unwrap() + .into_iter() + .map(|loaded_accounts| loaded_accounts.pubkey) + .collect(); + let expected_keys = vec![key1.pubkey(), native_loader::id()]; + assert_eq!(result_keys, expected_keys,); + assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![ - (key1.pubkey(), fee_payer_account), - ( - native_loader::id(), - mock_bank.accounts_map[&native_loader::id()].clone() - ) - ], - program_indices: vec![vec![]], - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } + fee_payer_account, + unique_loaded_accounts.get(&key1.pubkey()).unwrap().clone(), + ); + assert_eq!( + mock_bank.accounts_map[&native_loader::id()].clone(), + unique_loaded_accounts + .get(&native_loader::id()) + .unwrap() + .clone(), ); } @@ -1367,7 +1321,6 @@ mod tests { account_data.set_lamports(200); mock_bank.accounts_map.insert(key1.pubkey(), account_data); - let mut error_metrics = TransactionErrorMetrics::default(); let mut loaded_programs = ProgramCacheForTxBatch::default(); loaded_programs.replenish(key2.pubkey(), Arc::new(ProgramCacheEntry::default())); @@ -1376,16 +1329,13 @@ mod tests { vec![Signature::new_unique()], false, ); + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); let result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount::default(), - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); assert_eq!(result.err(), Some(TransactionError::AccountNotFound)); @@ -1411,26 +1361,29 @@ mod tests { let mut mock_bank = TestCallbacks::default(); let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + unique_loaded_accounts.insert(key1.pubkey(), account_data); + let mut accounts = vec![LoadedAccountDetails { + pubkey: key1.pubkey(), + account_found: false, + }]; + let result = load_transaction_indices( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount::default(), - &ComputeBudgetLimits::default(), + &mut accounts, &mut error_metrics, - None, - &FeatureSet::default(), - &RentCollector::default(), - &loaded_programs, + &mut unique_loaded_accounts, ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1456,26 +1409,35 @@ mod tests { let mut mock_bank = TestCallbacks::default(); let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + + let loaded_programs = ProgramCacheForTxBatch::default(); + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount::default(), - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, + ); + + let result = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, ); assert_eq!( @@ -1505,13 +1467,15 @@ mod tests { let mut account_data = AccountSharedData::default(); account_data.set_owner(native_loader::id()); account_data.set_executable(true); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); - let mut fee_payer_account = AccountSharedData::default(); - fee_payer_account.set_lamports(200); + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); mock_bank .accounts_map - .insert(key2.pubkey(), fee_payer_account.clone()); + .insert(key2.pubkey(), fee_payer_account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); @@ -1520,37 +1484,41 @@ mod tests { vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount { - account: fee_payer_account.clone(), - ..LoadedTransactionAccount::default() - }, - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); + + assert_eq!(unique_loaded_accounts.len(), 2); assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![ - (key2.pubkey(), fee_payer_account), - ( - key1.pubkey(), - mock_bank.accounts_map[&key1.pubkey()].clone() - ), - ], - program_indices: vec![vec![1]], - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } + unique_loaded_accounts + .get_key_value(&key2.pubkey()) + .unwrap(), + (&key2.pubkey(), &fee_payer_account_data) ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key1.pubkey()) + .unwrap(), + ( + &key1.pubkey(), + &mock_bank.accounts_map[&key1.pubkey()].clone() + ) + ); + + assert_eq!(program_indices.unwrap(), vec![vec![1]]); } #[test] @@ -1573,11 +1541,15 @@ mod tests { let mut mock_bank = TestCallbacks::default(); let mut account_data = AccountSharedData::default(); account_data.set_executable(true); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); - mock_bank.accounts_map.insert(key2.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key2.pubkey(), account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); @@ -1586,19 +1558,27 @@ mod tests { vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount::default(), - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); - assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); + + assert_eq!( + program_indices.err(), + Some(TransactionError::ProgramAccountNotFound) + ); } #[test] @@ -1623,11 +1603,15 @@ mod tests { let mut account_data = AccountSharedData::default(); account_data.set_executable(true); account_data.set_owner(key3.pubkey()); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); - mock_bank.accounts_map.insert(key2.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key2.pubkey(), account_data.clone()); mock_bank .accounts_map @@ -1640,20 +1624,25 @@ mod tests { vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount::default(), - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, + ); + + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, ); assert_eq!( - result.err(), + program_indices.err(), Some(TransactionError::InvalidProgramForExecution) ); } @@ -1677,21 +1666,26 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); let mut account_data = AccountSharedData::default(); account_data.set_executable(true); account_data.set_owner(key3.pubkey()); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key1.pubkey(), account_data.clone()); - let mut fee_payer_account = AccountSharedData::default(); - fee_payer_account.set_lamports(200); + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); mock_bank .accounts_map - .insert(key2.pubkey(), fee_payer_account.clone()); + .insert(key2.pubkey(), fee_payer_account_data.clone()); let mut account_data = AccountSharedData::default(); account_data.set_executable(true); account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key3.pubkey(), account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); @@ -1701,41 +1695,50 @@ mod tests { vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount { - account: fee_payer_account.clone(), - ..LoadedTransactionAccount::default() - }, - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); + + assert_eq!(unique_loaded_accounts.len(), 3); assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![ - (key2.pubkey(), fee_payer_account), - ( - key1.pubkey(), - mock_bank.accounts_map[&key1.pubkey()].clone() - ), - ( - key3.pubkey(), - mock_bank.accounts_map[&key3.pubkey()].clone() - ), - ], - program_indices: vec![vec![1]], - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } + unique_loaded_accounts + .get_key_value(&key2.pubkey()) + .unwrap(), + (&key2.pubkey(), &fee_payer_account_data) + ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key1.pubkey()) + .unwrap(), + ( + &key1.pubkey(), + &mock_bank.accounts_map[&key1.pubkey()].clone() + ) ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key3.pubkey()) + .unwrap(), + ( + &key3.pubkey(), + &mock_bank.accounts_map[&key3.pubkey()].clone() + ) + ); + + assert_eq!(program_indices.unwrap(), vec![vec![1]]); } #[test] @@ -1765,21 +1768,23 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); - let mut account_data = AccountSharedData::default(); - account_data.set_executable(true); - account_data.set_owner(key3.pubkey()); - mock_bank.accounts_map.insert(key1.pubkey(), account_data); + let mut account_data1 = AccountSharedData::default(); + account_data1.set_executable(true); + account_data1.set_owner(key3.pubkey()); + mock_bank.accounts_map.insert(key1.pubkey(), account_data1); - let mut fee_payer_account = AccountSharedData::default(); - fee_payer_account.set_lamports(200); + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); mock_bank .accounts_map - .insert(key2.pubkey(), fee_payer_account.clone()); + .insert(key2.pubkey(), fee_payer_account_data.clone()); - let mut account_data = AccountSharedData::default(); - account_data.set_executable(true); - account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + let mut account_data3 = AccountSharedData::default(); + account_data3.set_executable(true); + account_data3.set_owner(native_loader::id()); + mock_bank + .accounts_map + .insert(key3.pubkey(), account_data3.clone()); let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); @@ -1789,44 +1794,50 @@ mod tests { vec![Signature::new_unique()], false, ); - let result = load_transaction_accounts( + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, sanitized_transaction.message(), - LoadedTransactionAccount { - account: fee_payer_account.clone(), - ..LoadedTransactionAccount::default() - }, - &ComputeBudgetLimits::default(), - &mut error_metrics, None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, + ); + + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.clone().unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, ); - let mut account_data = AccountSharedData::default(); - account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![ - (key2.pubkey(), fee_payer_account), - ( - key1.pubkey(), - mock_bank.accounts_map[&key1.pubkey()].clone() - ), - (key4.pubkey(), account_data), - ( - key3.pubkey(), - mock_bank.accounts_map[&key3.pubkey()].clone() - ), - ], - program_indices: vec![vec![1], vec![1]], - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } + unique_loaded_accounts + .get_key_value(&key1.pubkey()) + .unwrap(), + ( + &key1.pubkey(), + &mock_bank.accounts_map[&key1.pubkey()].clone() + ) + ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key2.pubkey()) + .unwrap(), + (&key2.pubkey(), &fee_payer_account_data) ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key3.pubkey()) + .unwrap(), + ( + &key3.pubkey(), + &mock_bank.accounts_map[&key3.pubkey()].clone() + ) + ); + + assert_eq!(program_indices.unwrap(), vec![vec![1], vec![1]]); } #[test] @@ -1857,29 +1868,34 @@ mod tests { ); let num_accounts = tx.message().account_keys.len(); let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); - let mut error_metrics = TransactionErrorMetrics::default(); - let mut load_results = load_accounts( + let loaded_programs = ProgramCacheForTxBatch::default(); + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &bank, - &[sanitized_tx.clone()], - vec![Ok(ValidatedTransactionDetails::default())], - &mut error_metrics, + sanitized_tx.message(), None, - &FeatureSet::default(), - &RentCollector::default(), - &ProgramCacheForTxBatch::default(), + &loaded_programs, + &mut unique_loaded_accounts, ); - let TransactionLoadResult::Loaded(loaded_transaction) = load_results.swap_remove(0) else { - panic!("transaction loading failed"); - }; + let accounts: Vec<(Pubkey, AccountSharedData)> = load_result + .unwrap() + .iter() + .map(|loaded_account| { + let key = loaded_account.pubkey; + let account = unique_loaded_accounts.get(&key).unwrap().clone(); + (key, account) + }) + .collect(); let compute_budget = ComputeBudget::new(u64::from( compute_budget_limits::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, )); let rent_collector = RentCollector::default(); let transaction_context = TransactionContext::new( - loaded_transaction.accounts, - rent_collector.get_rent().clone(), + accounts, + Rent::default(), compute_budget.max_instruction_stack_depth, compute_budget.max_instruction_trace_length, ); @@ -1888,7 +1904,7 @@ mod tests { TransactionAccountStateInfo::new( &transaction_context, sanitized_tx.message(), - &rent_collector, + &rent_collector ) .len(), num_accounts, @@ -1896,7 +1912,7 @@ mod tests { } #[test] - fn test_load_accounts_success() { + fn test_load_accounts_and_indices_success() { let key1 = Keypair::new(); let key2 = Keypair::new(); let key3 = Keypair::new(); @@ -1927,16 +1943,18 @@ mod tests { account_data.set_owner(key3.pubkey()); mock_bank.accounts_map.insert(key1.pubkey(), account_data); - let mut fee_payer_account = AccountSharedData::default(); - fee_payer_account.set_lamports(200); + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); mock_bank .accounts_map - .insert(key2.pubkey(), fee_payer_account.clone()); + .insert(key2.pubkey(), fee_payer_account_data.clone()); let mut account_data = AccountSharedData::default(); account_data.set_executable(true); account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + mock_bank + .accounts_map + .insert(key3.pubkey(), account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); let loaded_programs = ProgramCacheForTxBatch::default(); @@ -1946,66 +1964,58 @@ mod tests { vec![Signature::new_unique()], false, ); - let validation_result = Ok(ValidatedTransactionDetails { - loaded_fee_payer_account: LoadedTransactionAccount { - account: fee_payer_account, - ..LoadedTransactionAccount::default() - }, - ..ValidatedTransactionDetails::default() - }); - let mut load_results = load_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, - &[sanitized_transaction], - vec![validation_result], - &mut error_metrics, + sanitized_transaction.message(), None, - &FeatureSet::default(), - &RentCollector::default(), &loaded_programs, + &mut unique_loaded_accounts, ); - let mut account_data = AccountSharedData::default(); - account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.clone().unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); - assert_eq!(load_results.len(), 1); - let TransactionLoadResult::Loaded(loaded_transaction) = load_results.swap_remove(0) else { - panic!("transaction loading failed"); - }; assert_eq!( - loaded_transaction, - LoadedTransaction { - accounts: vec![ - ( - key2.pubkey(), - mock_bank.accounts_map[&key2.pubkey()].clone() - ), - ( - key1.pubkey(), - mock_bank.accounts_map[&key1.pubkey()].clone() - ), - (key4.pubkey(), account_data), - ( - key3.pubkey(), - mock_bank.accounts_map[&key3.pubkey()].clone() - ), - ], - program_indices: vec![vec![1], vec![1]], - fee_details: FeeDetails::default(), - rollback_accounts: RollbackAccounts::default(), - compute_budget_limits: ComputeBudgetLimits::default(), - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } + unique_loaded_accounts + .get_key_value(&key1.pubkey()) + .unwrap(), + ( + &key1.pubkey(), + &mock_bank.accounts_map[&key1.pubkey()].clone() + ) + ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key2.pubkey()) + .unwrap(), + ( + &key2.pubkey(), + &mock_bank.accounts_map[&key2.pubkey()].clone() + ) + ); + assert_eq!( + unique_loaded_accounts + .get_key_value(&key3.pubkey()) + .unwrap(), + ( + &key3.pubkey(), + &mock_bank.accounts_map[&key3.pubkey()].clone() + ) ); + + assert_eq!(program_indices.unwrap(), vec![vec![1], vec![1]]); } #[test] - fn test_load_accounts_error() { + fn test_load_accounts_and_indices_error() { let mock_bank = TestCallbacks::default(); - let feature_set = FeatureSet::default(); - let rent_collector = RentCollector::default(); let message = Message { account_keys: vec![Pubkey::new_from_array([0; 32])], @@ -2025,43 +2035,45 @@ mod tests { false, ); - let validation_result = Ok(ValidatedTransactionDetails::default()); - let load_results = load_accounts( + let mut error_metrics = TransactionErrorMetrics::default(); + let loaded_programs = ProgramCacheForTxBatch::default(); + + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let load_result = load_transaction_accounts( &mock_bank, - &[sanitized_transaction.clone()], - vec![validation_result.clone()], - &mut TransactionErrorMetrics::default(), + sanitized_transaction.message(), None, - &feature_set, - &rent_collector, - &ProgramCacheForTxBatch::default(), + &loaded_programs, + &mut unique_loaded_accounts, ); - assert!(matches!( - load_results[0], - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::InvalidProgramForExecution, - .. - }), - )); + let program_indices = load_transaction_indices( + &mock_bank, + sanitized_transaction.message(), + &mut load_result.clone().unwrap(), + &mut error_metrics, + &mut unique_loaded_accounts, + ); - let validation_result = Err(TransactionError::InvalidWritableAccount); + assert_eq!( + program_indices, + Err(TransactionError::ProgramAccountNotFound) + ); + + let check_results: Vec = + vec![Err(TransactionError::InvalidWritableAccount)]; - let load_results = load_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let result = load_accounts( &mock_bank, - &[sanitized_transaction.clone()], - vec![validation_result], - &mut TransactionErrorMetrics::default(), + &vec![sanitized_transaction], + &check_results, None, - &feature_set, - &rent_collector, - &ProgramCacheForTxBatch::default(), + &loaded_programs, + &mut unique_loaded_accounts, ); - assert!(matches!( - load_results[0], - TransactionLoadResult::NotLoaded(TransactionError::InvalidWritableAccount), - )); + assert_eq!(result, vec![Err(TransactionError::InvalidWritableAccount)]); } #[test] @@ -2190,22 +2202,18 @@ mod tests { vec![Signature::new_unique()], false, ); - let validation_result = Ok(ValidatedTransactionDetails { - loaded_fee_payer_account: LoadedTransactionAccount { - account: account0.clone(), - ..LoadedTransactionAccount::default() - }, - ..ValidatedTransactionDetails::default() - }); + + let loaded_programs = ProgramCacheForTxBatch::default(); + let check_results: Vec = + vec![Ok(CheckedTransactionDetails::default())]; + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); let _load_results = load_accounts( &mock_bank, - &[sanitized_transaction], - vec![validation_result], - &mut TransactionErrorMetrics::default(), + &vec![sanitized_transaction], + &check_results, None, - &FeatureSet::default(), - &RentCollector::default(), - &ProgramCacheForTxBatch::default(), + &loaded_programs, + &mut unique_loaded_accounts, ); // ensure the loaded accounts are inspected @@ -2218,7 +2226,7 @@ mod tests { actual_inspected_accounts.sort_unstable_by(|a, b| a.0.cmp(&b.0)); let mut expected_inspected_accounts = vec![ - // *not* key0, since it is loaded during fee payer validation + (address0, vec![(Some(account0), true)]), (address1, vec![(Some(account1), true)]), (address2, vec![(None, true)]), (address3, vec![(Some(account3), false)]), diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index c6a5e43149949e..f004b5238c585d 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -3,10 +3,13 @@ use qualifier_attr::{field_qualifiers, qualifiers}; use { crate::{ account_loader::{ - collect_rent_from_account, load_accounts, validate_fee_payer, - CheckedTransactionDetails, LoadedTransaction, LoadedTransactionAccount, - TransactionCheckResult, TransactionLoadResult, TransactionValidationResult, - ValidatedTransactionDetails, + accumulate_and_check_loaded_account_data_size, calculate_program_indices, + collect_rent_from_account, limited_update_unique_loaded_accounts, load_accounts, + update_unique_loaded_accounts, validate_fee_payer, CheckedTransactionDetails, + FeesOnlyTransaction, LoadedTransaction, LoadedTransactionAccount, RentDetails, + TransactionCheckResult, TransactionLoadAccountResult, TransactionProgramIndices, + TransactionRent, TransactionRentResult, TransactionValidationResult, + UniqueLoadedAccounts, ValidatedTransactionDetails, }, account_overrides::AccountOverrides, message_processor::MessageProcessor, @@ -15,9 +18,10 @@ use { transaction_account_state_info::TransactionAccountStateInfo, transaction_error_metrics::TransactionErrorMetrics, transaction_execution_result::{ExecutedTransaction, TransactionExecutionDetails}, - transaction_processing_callback::{AccountState, TransactionProcessingCallback}, + transaction_processing_callback::TransactionProcessingCallback, transaction_processing_result::{ProcessedTransaction, TransactionProcessingResult}, }, + ahash::AHashSet, log::debug, percentage::Percentage, solana_bpf_loader_program::syscalls::{ @@ -36,7 +40,7 @@ use { }, solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, solana_sdk::{ - account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, + account::{AccountSharedData, ReadableAccount, WritableAccount, PROGRAM_OWNERS}, clock::{Epoch, Slot}, feature_set::{self, remove_rounding_in_fee_calculation, FeatureSet}, fee::{FeeBudgetLimits, FeeStructure}, @@ -45,6 +49,7 @@ use { instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, pubkey::Pubkey, rent_collector::RentCollector, + rent_debits::RentDebits, saturating_add_assign, transaction::{self, TransactionError}, transaction_context::{ExecutionRecord, TransactionContext}, @@ -64,6 +69,7 @@ use { /// A list of log messages emitted during a transaction pub type TransactionLogMessages = Vec; +pub const DEFAULT_TRANSACTIONS_PER_BATCH: usize = 64; /// The output of the transaction batch processor's /// `load_and_execute_sanitized_transactions` method. pub struct LoadAndExecuteSanitizedTransactionsOutput { @@ -237,26 +243,11 @@ impl TransactionBatchProcessor { let mut error_metrics = TransactionErrorMetrics::default(); let mut execute_timings = ExecuteTimings::default(); - let (validation_results, validate_fees_us) = measure_us!(self.validate_fees( - callbacks, - config.account_overrides, - sanitized_txs, - check_results, - &environment.feature_set, - environment - .fee_structure - .unwrap_or(&FeeStructure::default()), - environment - .rent_collector - .unwrap_or(&RentCollector::default()), - &mut error_metrics - )); - let (mut program_cache_for_tx_batch, program_cache_us) = measure_us!({ let mut program_accounts_map = Self::filter_executable_program_accounts( callbacks, sanitized_txs, - &validation_results, + &check_results, PROGRAM_OWNERS, ); for builtin_program in self.builtin_program_ids.read().unwrap().iter() { @@ -283,55 +274,142 @@ impl TransactionBatchProcessor { program_cache_for_tx_batch }); - let (loaded_transactions, load_accounts_us) = measure_us!(load_accounts( + let mut unique_loaded_accounts: UniqueLoadedAccounts = HashMap::default(); + let (mut initial_load_results, load_accounts_us) = measure_us!(load_accounts( callbacks, sanitized_txs, - validation_results, - &mut error_metrics, + &check_results, config.account_overrides, - &environment.feature_set, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), &program_cache_for_tx_batch, + &mut unique_loaded_accounts, )); + let (program_indexes_results, load_program_indices_us) = + measure_us!(calculate_program_indices( + callbacks, + sanitized_txs, + &mut initial_load_results, + &mut error_metrics, + &mut unique_loaded_accounts, + )); + let enable_transaction_loading_failure_fees = environment .feature_set .is_active(&feature_set::enable_transaction_loading_failure_fees::id()); + let mut dedup_nonce_lookup: AHashSet = AHashSet::default(); + let mut validate_fees_us = 0; let (processing_results, execution_us): (Vec, u64) = - measure_us!(loaded_transactions - .into_iter() - .zip(sanitized_txs.iter()) - .map(|(load_result, tx)| match load_result { - TransactionLoadResult::NotLoaded(err) => Err(err), - TransactionLoadResult::FeesOnly(fees_only_tx) => { - if enable_transaction_loading_failure_fees { - Ok(ProcessedTransaction::FeesOnly(Box::new(fees_only_tx))) - } else { - Err(fees_only_tx.load_error) - } - } - TransactionLoadResult::Loaded(loaded_transaction) => { - let executed_tx = self.execute_loaded_transaction( - tx, - loaded_transaction, - &mut execute_timings, - &mut error_metrics, - &mut program_cache_for_tx_batch, - environment, - config, - ); - - // Update batch specific cache of the loaded programs with the modifications - // made by the transaction, if it executed successfully. - if executed_tx.was_successful() { - program_cache_for_tx_batch.merge(&executed_tx.programs_modified_by_tx); - } + measure_us!(sanitized_txs + .iter() + .zip(check_results) + .zip(initial_load_results.iter()) + .zip(program_indexes_results.iter()) + .map( + |(((tx, check_results), accounts), program_indices)| match program_indices { + Err(e) => Err(e.clone()), + Ok(program_indices) => { + // validate fee payer + let (fee_validation_result, validate_fees_time) = measure_us!(self + .validate_transaction_fee_payer( + config.account_overrides, + tx, + check_results.unwrap(), + &environment.feature_set, + environment + .fee_structure + .unwrap_or(&FeeStructure::default()), + environment + .rent_collector + .unwrap_or(&RentCollector::default()), + &mut error_metrics, + &mut unique_loaded_accounts, + )); + validate_fees_us += validate_fees_time; + + let rent_collection_result = match &fee_validation_result { + Ok(fee_validation_result) => { + match self.collect_rent_and_validate_account_size( + tx, + fee_validation_result, + accounts, + &environment.feature_set, + environment + .rent_collector + .unwrap_or(&RentCollector::default()), + &mut error_metrics, + &mut unique_loaded_accounts, + ) { + Ok(rent_details) => rent_details, + Err(e) => { + if enable_transaction_loading_failure_fees { + let fees_only_tx = FeesOnlyTransaction { + load_error: e, + fee_details: fee_validation_result.fee_details, + rollback_accounts: fee_validation_result + .rollback_accounts + .clone(), + }; + return Ok(ProcessedTransaction::FeesOnly( + Box::new(fees_only_tx), + )); + } else { + return Err(e.clone()); + } + } + } + } + Err(e) => return Err(e.clone()), + }; + + // check for duplicate nonces at this point as beyond this point there will + // not be any non-recordable errors + Self::check_duplicate_nonces( + &fee_validation_result.as_ref().unwrap().rollback_accounts, + tx, + &mut dedup_nonce_lookup, + )?; + + let loaded_transaction = self.create_loaded_transaction( + accounts, + program_indices, + &fee_validation_result, + &rent_collection_result, + &mut unique_loaded_accounts, + ); + + let executed_tx = self.execute_loaded_transaction( + tx, + loaded_transaction, + &mut execute_timings, + &mut error_metrics, + &mut program_cache_for_tx_batch, + environment, + config, + ); + + if executed_tx.was_successful() { + // Update batch specific cache of the loaded programs with the modifications + // made by the transaction, if it executed successfully. + program_cache_for_tx_batch + .merge(&executed_tx.programs_modified_by_tx); + // store all accounts back into the map + update_unique_loaded_accounts( + &executed_tx.loaded_transaction, + &mut unique_loaded_accounts, + ); + } else { + // only update fee-payer, nonce. + limited_update_unique_loaded_accounts( + tx, + &executed_tx.loaded_transaction, + &mut unique_loaded_accounts, + ); + } - Ok(ProcessedTransaction::Executed(Box::new(executed_tx))) + Ok(ProcessedTransaction::Executed(Box::new(executed_tx))) + } } - }) + ) .collect()); // Skip eviction when there's no chance this particular tx batch has increased the size of @@ -361,6 +439,10 @@ impl TransactionBatchProcessor { execute_timings .saturating_add_in_place(ExecuteTimingType::ProgramCacheUs, program_cache_us); execute_timings.saturating_add_in_place(ExecuteTimingType::LoadUs, load_accounts_us); + execute_timings.saturating_add_in_place( + ExecuteTimingType::LoadProgramIndicesUs, + load_program_indices_us, + ); execute_timings.saturating_add_in_place(ExecuteTimingType::ExecuteUs, execution_us); LoadAndExecuteSanitizedTransactionsOutput { @@ -370,44 +452,32 @@ impl TransactionBatchProcessor { } } - fn validate_fees( - &self, - callbacks: &CB, - account_overrides: Option<&AccountOverrides>, - sanitized_txs: &[impl core::borrow::Borrow], - check_results: Vec, - feature_set: &FeatureSet, - fee_structure: &FeeStructure, - rent_collector: &dyn SVMRentCollector, - error_counters: &mut TransactionErrorMetrics, - ) -> Vec { - sanitized_txs - .iter() - .zip(check_results) - .map(|(sanitized_tx, check_result)| { - check_result.and_then(|checked_details| { - let message = sanitized_tx.borrow(); - self.validate_transaction_fee_payer( - callbacks, - account_overrides, - message, - checked_details, - feature_set, - fee_structure, - rent_collector, - error_counters, - ) - }) - }) - .collect() + fn check_duplicate_nonces( + rollback_accounts: &RollbackAccounts, + message: &impl SVMMessage, + dedup_nonce_lookup: &mut AHashSet, + ) -> Result<(), TransactionError> { + match rollback_accounts { + RollbackAccounts::SameNonceAndFeePayer { .. } + | RollbackAccounts::SeparateNonceAndFeePayer { .. } => { + let nonce_hash = message.recent_blockhash(); + if dedup_nonce_lookup.contains(nonce_hash) { + return Err(TransactionError::BlockhashNotFound); + } else { + dedup_nonce_lookup.insert(*nonce_hash); + return Ok(()); + } + } + _ => {} + } + Ok(()) } // Loads transaction fee payer, collects rent if necessary, then calculates // transaction fees, and deducts them from the fee payer balance. If the // account is not found or has insufficient funds, an error is returned. - fn validate_transaction_fee_payer( + fn validate_transaction_fee_payer( &self, - callbacks: &CB, account_overrides: Option<&AccountOverrides>, message: &impl SVMMessage, checked_details: CheckedTransactionDetails, @@ -415,6 +485,7 @@ impl TransactionBatchProcessor { fee_structure: &FeeStructure, rent_collector: &dyn SVMRentCollector, error_counters: &mut TransactionErrorMetrics, + unique_loaded_accounts: &mut UniqueLoadedAccounts, ) -> transaction::Result { let compute_budget_limits = process_compute_budget_instructions( message.program_instructions_iter(), @@ -424,22 +495,15 @@ impl TransactionBatchProcessor { })?; let fee_payer_address = message.fee_payer(); - let fee_payer_account = account_overrides .and_then(|overrides| overrides.get(fee_payer_address).cloned()) - .or_else(|| callbacks.get_account_shared_data(fee_payer_address)); + .or_else(|| unique_loaded_accounts.get(fee_payer_address).cloned()); let Some(mut fee_payer_account) = fee_payer_account else { error_counters.account_not_found += 1; return Err(TransactionError::AccountNotFound); }; - callbacks.inspect_account( - fee_payer_address, - AccountState::Alive(&fee_payer_account), - true, // <-- is_writable - ); - let fee_payer_loaded_rent_epoch = fee_payer_account.rent_epoch(); let fee_payer_rent_debit = collect_rent_from_account( feature_set, @@ -464,14 +528,25 @@ impl TransactionBatchProcessor { ); let fee_payer_index = 0; - validate_fee_payer( + match validate_fee_payer( fee_payer_address, &mut fee_payer_account, fee_payer_index, error_counters, rent_collector, fee_details.total_fee(), - )?; + ) { + Ok(_) => { + if let Some(fee_payer) = unique_loaded_accounts.get_mut(fee_payer_address) { + fee_payer.set_lamports(fee_payer_account.lamports()); + fee_payer.set_rent_epoch(fee_payer_account.rent_epoch()); + } + }, + Err(err) => { + error_counters.invalid_rent_paying_account += 1; + return Err(err) + }, + }; // Capture fee-subtracted fee payer account and original nonce account state // to rollback to if transaction execution fails. @@ -495,16 +570,116 @@ impl TransactionBatchProcessor { }) } + // Collect rent and validate account sizes + fn collect_rent_and_validate_account_size( + &self, + message: &impl SVMMessage, + fee_validation_result: &ValidatedTransactionDetails, + accounts: &TransactionLoadAccountResult, + feature_set: &FeatureSet, + rent_collector: &dyn SVMRentCollector, + error_metrics: &mut TransactionErrorMetrics, + unique_loaded_accounts: &mut UniqueLoadedAccounts, + ) -> TransactionRentResult { + let mut tx_rent: TransactionRent = 0; + let mut rent_debits = RentDebits::default(); + let mut accumulated_accounts_data_size: u32 = 0; + accounts + .as_ref() + .unwrap() + .iter() + .enumerate() + .map(|(i, loaded_account)| { + let key = loaded_account.pubkey; + let account = unique_loaded_accounts.get_mut(&key).unwrap(); + let is_fee_payer = i == 0; + let (account_size, rent) = if message.is_writable(i) && !is_fee_payer { + let rent_due = + collect_rent_from_account(feature_set, rent_collector, &key, account) + .rent_amount; + (account.data().len(), rent_due) + } else { + // if the account is fee payer + if is_fee_payer { + ( + account.data().len(), + fee_validation_result + .loaded_fee_payer_account + .rent_collected, + ) + } + // the account is readable + else { + (account.data().len(), 0) + } + }; + + tx_rent += rent; + rent_debits.insert(&key, rent, account.lamports()); + + accumulate_and_check_loaded_account_data_size( + &mut accumulated_accounts_data_size, + account_size, + fee_validation_result + .compute_budget_limits + .loaded_accounts_bytes, + error_metrics, + )?; + + Ok((account_size, rent)) + }) + .collect::, TransactionError>>()?; + + Ok(RentDetails { + rent: tx_rent, + rent_debits, + loaded_accounts_data_size: accumulated_accounts_data_size, + }) + } + + pub fn create_loaded_transaction( + &self, + accounts: &TransactionLoadAccountResult, + program_indices: &TransactionProgramIndices, + fee_validation_result: &TransactionValidationResult, + rent_collection_result: &RentDetails, + unique_loaded_accounts: &mut UniqueLoadedAccounts, + ) -> LoadedTransaction { + let accounts: Vec<(Pubkey, AccountSharedData)> = accounts + .as_ref() + .unwrap() + .iter() + .map(|loaded_account| { + let key = loaded_account.pubkey; + let account = unique_loaded_accounts.get(&key).unwrap().clone(); + (key, account) + }) + .collect(); + let tx_details = fee_validation_result.as_ref().unwrap().clone(); + let rent_details = rent_collection_result.clone(); + + LoadedTransaction { + accounts, + program_indices: program_indices.to_vec(), + fee_details: tx_details.fee_details, + rollback_accounts: tx_details.rollback_accounts, + compute_budget_limits: tx_details.compute_budget_limits, + rent: rent_collection_result.rent, + rent_debits: rent_details.rent_debits, + loaded_accounts_data_size: rent_details.loaded_accounts_data_size, + } + } + /// Returns a map from executable program accounts (all accounts owned by any loader) /// to their usage counters, for the transactions with a valid blockhash or nonce. fn filter_executable_program_accounts( callbacks: &CB, txs: &[impl SVMMessage], - validation_results: &[TransactionValidationResult], + check_results: &[TransactionCheckResult], program_owners: &[Pubkey], ) -> HashMap { let mut result: HashMap = HashMap::new(); - validation_results.iter().zip(txs).for_each(|etx| { + check_results.iter().zip(txs).for_each(|etx| { if let (Ok(_), tx) = etx { tx.account_keys() .iter() @@ -996,7 +1171,7 @@ mod tests { super::*, crate::{ account_loader::ValidatedTransactionDetails, nonce_info::NonceInfo, - rollback_accounts::RollbackAccounts, + rollback_accounts::RollbackAccounts, transaction_processing_callback::AccountState, }, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_program_runtime::loaded_programs::{BlockRelation, ProgramCacheEntryType}, @@ -1184,6 +1359,11 @@ mod tests { let mut processing_config = TransactionProcessingConfig::default(); processing_config.recording_config.enable_log_recording = true; + let mut unique_loaded_accounts: HashMap = HashMap::default(); + for acct in loaded_transaction.accounts.iter() { + unique_loaded_accounts.insert(acct.0, acct.1.clone()); + } + let executed_tx = batch_processor.execute_loaded_transaction( &sanitized_transaction, loaded_transaction.clone(), @@ -1418,17 +1598,17 @@ mod tests { sanitized_transaction_2.clone(), sanitized_transaction_1, ]; - let validation_results = vec![ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), - Err(TransactionError::ProgramAccountNotFound), + let check_results = vec![ + Ok(CheckedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), + Err(TransactionError::AccountInUse), ]; let owners = vec![owner1, owner2]; let result = TransactionBatchProcessor::::filter_executable_program_accounts( &mock_bank, &transactions, - &validation_results, + &check_results, &owners, ); @@ -1511,8 +1691,8 @@ mod tests { &bank, &[sanitized_tx1, sanitized_tx2], &[ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), ], owners, ); @@ -1603,15 +1783,15 @@ mod tests { let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2); let owners = &[program1_pubkey, program2_pubkey]; - let validation_results = vec![ - Ok(ValidatedTransactionDetails::default()), + let check_results = vec![ + Ok(CheckedTransactionDetails::default()), Err(TransactionError::BlockhashNotFound), ]; let programs = TransactionBatchProcessor::::filter_executable_program_accounts( &bank, &[sanitized_tx1, sanitized_tx2], - &validation_results, + &check_results, owners, ); @@ -1878,15 +2058,12 @@ mod tests { ); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -1897,6 +2074,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); let post_validation_fee_payer_account = { @@ -1956,15 +2134,12 @@ mod tests { let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -1975,6 +2150,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); let post_validation_fee_payer_account = { @@ -2011,11 +2187,10 @@ mod tests { let message = new_unchecked_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); - let mock_bank = MockBankCallback::default(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2026,6 +2201,7 @@ mod tests { &FeeStructure::default(), &RentCollector::default(), &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!(error_counters.account_not_found, 1); @@ -2041,15 +2217,12 @@ mod tests { let fee_payer_account = AccountSharedData::new(1, 0, &Pubkey::default()); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2060,6 +2233,7 @@ mod tests { &FeeStructure::default(), &RentCollector::default(), &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!(error_counters.insufficient_funds, 1); @@ -2079,15 +2253,12 @@ mod tests { let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default()); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2098,6 +2269,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!( @@ -2115,15 +2287,12 @@ mod tests { let fee_payer_account = AccountSharedData::new(1_000_000, 0, &Pubkey::new_unique()); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2134,6 +2303,7 @@ mod tests { &FeeStructure::default(), &RentCollector::default(), &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!(error_counters.invalid_account_for_fee, 1); @@ -2151,11 +2321,10 @@ mod tests { Some(&Pubkey::new_unique()), )); - let mock_bank = MockBankCallback::default(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2166,6 +2335,7 @@ mod tests { &FeeStructure::default(), &RentCollector::default(), &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!(error_counters.invalid_compute_budget, 1); @@ -2207,10 +2377,6 @@ mod tests { let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); @@ -2218,8 +2384,9 @@ mod tests { *fee_payer_address, fee_payer_account.clone(), )); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2230,6 +2397,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); let post_validation_fee_payer_account = { @@ -2273,15 +2441,12 @@ mod tests { let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, None, &message, CheckedTransactionDetails { @@ -2292,6 +2457,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); assert_eq!(error_counters.insufficient_funds, 1); @@ -2326,16 +2492,12 @@ mod tests { AccountSharedData::new(necessary_balance, 0, &Pubkey::default()); account_overrides.set_account(fee_payer_address, Some(fee_payer_account_override)); - let mock_bank = MockBankCallback { - account_shared_data: Arc::new(RwLock::new(mock_accounts)), - ..Default::default() - }; - let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); + let mut unique_loaded_accounts = UniqueLoadedAccounts::default(); + unique_loaded_accounts.insert(*fee_payer_address, fee_payer_account.clone()); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, Some(&account_overrides), &message, CheckedTransactionDetails { @@ -2346,6 +2508,7 @@ mod tests { &FeeStructure::default(), &rent_collector, &mut error_counters, + &mut unique_loaded_accounts, ); assert!( result.is_ok(), @@ -2353,61 +2516,4 @@ mod tests { result.err() ); } - - // Ensure `TransactionProcessingCallback::inspect_account()` is called when - // validating the fee payer, since that's when the fee payer account is loaded. - #[test] - fn test_inspect_account_fee_payer() { - let fee_payer_address = Pubkey::new_unique(); - let fee_payer_account = AccountSharedData::new_rent_epoch( - 123_000_000_000, - 0, - &Pubkey::default(), - RENT_EXEMPT_RENT_EPOCH, - ); - let mock_bank = MockBankCallback::default(); - mock_bank - .account_shared_data - .write() - .unwrap() - .insert(fee_payer_address, fee_payer_account.clone()); - - let message = new_unchecked_sanitized_message(Message::new_with_blockhash( - &[ - ComputeBudgetInstruction::set_compute_unit_limit(2000u32), - ComputeBudgetInstruction::set_compute_unit_price(1_000_000_000), - ], - Some(&fee_payer_address), - &Hash::new_unique(), - )); - let batch_processor = TransactionBatchProcessor::::default(); - batch_processor - .validate_transaction_fee_payer( - &mock_bank, - None, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature: 5000, - }, - &FeatureSet::default(), - &FeeStructure::default(), - &RentCollector::default(), - &mut TransactionErrorMetrics::default(), - ) - .unwrap(); - - // ensure the fee payer is an inspected account - let actual_inspected_accounts: Vec<_> = mock_bank - .inspected_accounts - .read() - .unwrap() - .iter() - .map(|(k, v)| (*k, v.clone())) - .collect(); - assert_eq!( - actual_inspected_accounts.as_slice(), - &[(fee_payer_address, vec![(Some(fee_payer_account), true)])], - ); - } } diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 08f6db09101e04..bab90a02af9802 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -432,6 +432,137 @@ fn program_medley() -> Vec { vec![test_entry] } +fn conflicting_tranfers() -> Vec { + let mut test_entry = SvmTestEntry::default(); + let transfer_amount = LAMPORTS_PER_SOL; + + // 2 transactions with same fee-payer + { + let source_keypair = Keypair::new(); + let source = source_keypair.pubkey(); + let destination = Pubkey::new_unique(); + + let mut source_data = AccountSharedData::default(); + let mut destination_data = AccountSharedData::default(); + + source_data.set_lamports(LAMPORTS_PER_SOL * 10); + test_entry.add_initial_account(source, &source_data); + + test_entry.push_transaction(system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + )); + + destination_data + .checked_add_lamports(transfer_amount) + .unwrap(); + test_entry.create_expected_account(destination, &destination_data); + + test_entry.decrease_expected_lamports(&source, transfer_amount + LAMPORTS_PER_SIGNATURE); + + // second transfer with the same fee payer + let destination = Pubkey::new_unique(); + let mut destination_data = AccountSharedData::default(); + + test_entry.push_transaction(system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + )); + + destination_data + .checked_add_lamports(transfer_amount) + .unwrap(); + test_entry.create_expected_account(destination, &destination_data); + + test_entry.decrease_expected_lamports(&source, transfer_amount + LAMPORTS_PER_SIGNATURE); + } + + // 2 transfers with same fee-payer, second tx will fail due to first + { + let source_keypair = Keypair::new(); + let source = source_keypair.pubkey(); + let destination = Pubkey::new_unique(); + + let mut source_data = AccountSharedData::default(); + let mut destination_data = AccountSharedData::default(); + + source_data.set_lamports(LAMPORTS_PER_SOL + LAMPORTS_PER_SIGNATURE); + test_entry.add_initial_account(source, &source_data); + + test_entry.push_transaction(system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + )); + + destination_data + .checked_add_lamports(transfer_amount) + .unwrap(); + test_entry.create_expected_account(destination, &destination_data); + + test_entry.decrease_expected_lamports(&source, transfer_amount + LAMPORTS_PER_SIGNATURE); + + // second transfer with the same fee payer which should fail + let destination = Pubkey::new_unique(); + test_entry.transaction_batch.push(TransactionBatchItem { + transaction: system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + ), + asserts: TransactionBatchItemAsserts::not_executed(), + ..TransactionBatchItem::default() + }); + } + + // 2 duplicate transactions + { + let source_keypair = Keypair::new(); + let source = source_keypair.pubkey(); + let destination = Pubkey::new_unique(); + + let mut source_data = AccountSharedData::default(); + let mut destination_data = AccountSharedData::default(); + + source_data.set_lamports(LAMPORTS_PER_SOL * 10); + test_entry.add_initial_account(source, &source_data); + + test_entry.push_transaction(system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + )); + + destination_data + .checked_add_lamports(transfer_amount) + .unwrap(); + test_entry.create_expected_account(destination, &destination_data); + + test_entry.decrease_expected_lamports(&source, transfer_amount + LAMPORTS_PER_SIGNATURE); + + // second duplicate transfer which should fail + test_entry.transaction_batch.push(TransactionBatchItem { + transaction: system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + ), + check_result: Err(TransactionError::AlreadyProcessed), + asserts: TransactionBatchItemAsserts::not_executed(), + }); + } + + vec![test_entry] +} + fn simple_transfer() -> Vec { let mut test_entry = SvmTestEntry::default(); let transfer_amount = LAMPORTS_PER_SOL; @@ -548,6 +679,7 @@ fn simple_transfer() -> Vec { #[test_case(program_medley())] #[test_case(simple_transfer())] +#[test_case(conflicting_tranfers())] fn svm_integration(test_entries: Vec) { for test_entry in test_entries { execute_test_entry(test_entry); diff --git a/timings/src/lib.rs b/timings/src/lib.rs index 46878559eb25cc..5a951d974e7328 100644 --- a/timings/src/lib.rs +++ b/timings/src/lib.rs @@ -47,6 +47,7 @@ pub enum ExecuteTimingType { CheckUs, ValidateFeesUs, LoadUs, + LoadProgramIndicesUs, ExecuteUs, StoreUs, UpdateStakesCacheUs, @@ -121,6 +122,13 @@ eager_macro_rules! { $eager_1 .index(ExecuteTimingType::LoadUs), i64 ), + ( + "load_program_indices_us", + *$self + .metrics + .index(ExecuteTimingType::LoadProgramIndicesUs), + i64 + ), ( "execute_us", *$self