From 89a87d7b57c52e4cd0eec7ba93eb9b295d92ee6d Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Thu, 10 Oct 2024 07:05:40 -0700 Subject: [PATCH 1/7] svm: allow conflicting transactions in entries --- Cargo.lock | 1 + programs/sbf/Cargo.lock | 1 + svm/Cargo.toml | 1 + svm/src/account_loader.rs | 418 ++++++++++++++++++++++--------- svm/src/transaction_processor.rs | 332 ++++++++++++++---------- 5 files changed, 502 insertions(+), 251 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d522ff3580112..a92b577e9bf478 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8355,6 +8355,7 @@ dependencies = [ name = "solana-svm" version = "2.2.0" dependencies = [ + "ahash 0.8.11", "assert_matches", "base64 0.22.1", "bincode", diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 17ca6b9ee419b9..eb41ba5210077c 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -7041,6 +7041,7 @@ dependencies = [ name = "solana-svm" version = "2.2.0" dependencies = [ + "ahash 0.8.11", "itertools 0.12.1", "log", "percentage", diff --git a/svm/Cargo.toml b/svm/Cargo.toml index a6edc55fc1fca2..74983966537320 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 2e47957d7f31dd..9beda9a1c47247 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -5,13 +5,15 @@ use { rollback_accounts::RollbackAccounts, transaction_error_metrics::TransactionErrorMetrics, transaction_processing_callback::{AccountState, TransactionProcessingCallback}, + transaction_processing_result::ProcessedTransaction, }, + ahash::AHashMap, itertools::Itertools, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_feature_set::{self as feature_set, FeatureSet}, solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch}, solana_sdk::{ - account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, + account::{Account, AccountSharedData, ReadableAccount, WritableAccount, PROGRAM_OWNERS}, fee::FeeDetails, native_loader, nonce::State as NonceState, @@ -95,6 +97,260 @@ pub struct FeesOnlyTransaction { pub fee_details: FeeDetails, } +#[cfg_attr(feature = "dev-context-only-utils", derive(Clone))] +pub(crate) struct AccountLoader<'a, CB: TransactionProcessingCallback> { + pub program_cache: ProgramCacheForTxBatch, + account_overrides: Option<&'a AccountOverrides>, + account_cache: AHashMap, + callbacks: &'a CB, +} +impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> { + pub fn new_with_account_cache_capacity( + callbacks: &'a CB, + account_overrides: Option<&'a AccountOverrides>, + program_cache: ProgramCacheForTxBatch, + capacity: usize, + ) -> AccountLoader<'a, CB> { + Self { + program_cache, + account_overrides, + account_cache: AHashMap::with_capacity(capacity), + callbacks, + } + } + + pub fn load_account( + &mut self, + account_key: &Pubkey, + usage_pattern: AccountUsagePattern, + ) -> Option { + let is_writable = usage_pattern == AccountUsagePattern::Writable; + let is_invisible = usage_pattern == AccountUsagePattern::ReadOnlyInvisible; + + if let Some(cache_item) = self.account_cache.get_mut(account_key) { + // Non-builtin accounts owned by NativeLoader are never loaded into the cache. + let is_owned_by_loader = PROGRAM_OWNERS.iter().contains(cache_item.account.owner()); + + if !cache_item.inspected_as_writable { + // Inspecting a read-only account is harmless. Inspecting a writable + // account must be done before rent collection, and must never be done + // again in-batch, because inspection is intended to preserve the + // state prior to any writes. + self.callbacks.inspect_account( + account_key, + AccountState::Alive(&cache_item.account), + is_writable, + ); + cache_item.inspected_as_writable = cache_item.inspected_as_writable || is_writable; + } + + if cache_item.account.lamports() == 0 { + None + } else if let Some(program) = (is_invisible && is_owned_by_loader) + .then_some(()) + .and_then(|_| self.program_cache.find(account_key)) + { + // NOTE if this account could be found in the program cache, we must + // substitute its size here, to perserve an oddity with transaction size + // calculations. However, we CANNOT skip the accounts cache check, + // in case the account was non-executable and modified in-batch. + // + // We must also check the current owner, in case this account was closed + // and reopened, in which case we ignore the stale program cache entry. + // + // When we properly check if program cache entries are executable, + // we can go to the program cache first and remove this branch, because + // executable accounts are immutable. + Some(LoadedTransactionAccount { + loaded_size: program.account_size, + account: account_shared_data_from_program(&program), + rent_collected: 0, + }) + } else { + Some(LoadedTransactionAccount { + loaded_size: cache_item.account.data().len(), + account: cache_item.account.clone(), + rent_collected: 0, + }) + } + } else if let Some(account_override) = self + .account_overrides + .and_then(|overrides| overrides.get(account_key)) + { + // We mark `inspected_as_writable` because overrides are never inspected. + self.account_cache.insert( + *account_key, + AccountCacheItem { + account: account_override.clone(), + inspected_as_writable: true, + }, + ); + + Some(LoadedTransactionAccount { + loaded_size: account_override.data().len(), + account: account_override.clone(), + rent_collected: 0, + }) + } else if let Some(program) = is_invisible + .then_some(()) + .and_then(|_| self.program_cache.find(account_key)) + { + self.callbacks.get_account_shared_data(account_key)?; + // Optimization to skip loading of accounts which are only used as + // programs in top-level instructions and not passed as instruction accounts. + Some(LoadedTransactionAccount { + loaded_size: program.account_size, + account: account_shared_data_from_program(&program), + rent_collected: 0, + }) + } else if let Some(account) = self.callbacks.get_account_shared_data(account_key) { + // Inspect the account prior to collecting rent, since + // rent collection can modify the account. + self.callbacks + .inspect_account(account_key, AccountState::Alive(&account), is_writable); + + self.account_cache.insert( + *account_key, + AccountCacheItem { + account: account.clone(), + inspected_as_writable: is_writable, + }, + ); + + Some(LoadedTransactionAccount { + loaded_size: account.data().len(), + account, + rent_collected: 0, + }) + } else { + self.callbacks + .inspect_account(account_key, AccountState::Dead, is_writable); + + None + } + } + + pub fn update_accounts_for_executed_tx( + &mut self, + message: &impl SVMMessage, + processed_transaction: &ProcessedTransaction, + ) { + match processed_transaction { + ProcessedTransaction::Executed(executed_tx) => { + if executed_tx.execution_details.status.is_ok() { + self.update_accounts_for_successful_tx( + message, + &executed_tx.loaded_transaction.accounts, + ); + } else { + self.update_accounts_for_failed_tx( + message, + &executed_tx.loaded_transaction.rollback_accounts, + ); + } + } + ProcessedTransaction::FeesOnly(fees_only_tx) => { + self.update_accounts_for_failed_tx(message, &fees_only_tx.rollback_accounts); + } + } + } + + pub fn update_accounts_for_failed_tx( + &mut self, + message: &impl SVMMessage, + rollback_accounts: &RollbackAccounts, + ) { + let fee_payer_address = message.fee_payer(); + match rollback_accounts { + RollbackAccounts::FeePayerOnly { fee_payer_account } => { + self.update_account(fee_payer_address, fee_payer_account); + } + RollbackAccounts::SameNonceAndFeePayer { nonce } => { + self.update_account(nonce.address(), nonce.account()); + } + RollbackAccounts::SeparateNonceAndFeePayer { + nonce, + fee_payer_account, + } => { + self.update_account(nonce.address(), nonce.account()); + self.update_account(fee_payer_address, fee_payer_account); + } + } + } + + fn update_accounts_for_successful_tx( + &mut self, + message: &impl SVMMessage, + transaction_accounts: &[TransactionAccount], + ) { + for (i, (address, account)) in (0..message.account_keys().len()).zip(transaction_accounts) { + if !message.is_writable(i) { + continue; + } + + // Accounts that are invoked and also not passed as an instruction + // account to a program don't need to be stored because it's assumed + // to be impossible for a committable transaction to modify an + // invoked account if said account isn't passed to some program. + if message.is_invoked(i) && !message.is_instruction_account(i) { + continue; + } + + self.update_account(address, account); + } + } + + fn update_account(&mut self, account_key: &Pubkey, account: &AccountSharedData) { + // If an account has been dropped, we must hide the data from any future transactions. + // We NEVER remove items from cache, or else we will fetch stale data from accounts-db. + // Because `update_account()` is only called on writable accounts using the same rules + // as commit, this (correctly) does not shadow rent-paying zero-lamport read-only accounts. + let account = if account.lamports() == 0 { + &AccountSharedData::default() + } else { + account + }; + + // Accounts are inspected during loading, and we only update writable accounts. + // Ergo, `inspected_as_writable` is always true. + self.account_cache + .entry(*account_key) + .and_modify(|cache_item| { + debug_assert!(cache_item.inspected_as_writable); + cache_item.account = account.clone(); + }) + .or_insert_with(|| AccountCacheItem { + account: account.clone(), + inspected_as_writable: true, + }); + } +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(crate) enum AccountUsagePattern { + Writable, + ReadOnlyInstruction, + ReadOnlyInvisible, +} +impl AccountUsagePattern { + pub fn new(message: &impl SVMMessage, account_index: usize) -> Self { + let is_writable = message.is_writable(account_index); + let is_instruction_account = message.is_instruction_account(account_index); + + match (is_writable, is_instruction_account) { + (true, _) => Self::Writable, + (false, true) => Self::ReadOnlyInstruction, + (false, false) => Self::ReadOnlyInvisible, + } + } +} + +#[derive(Debug, Clone)] +struct AccountCacheItem { + account: AccountSharedData, + inspected_as_writable: bool, +} + /// 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 /// rent exempt. @@ -181,61 +437,25 @@ 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( - callbacks: &CB, - txs: &[impl SVMMessage], - validation_results: Vec, - error_metrics: &mut TransactionErrorMetrics, - account_overrides: Option<&AccountOverrides>, - feature_set: &FeatureSet, - rent_collector: &dyn SVMRentCollector, - loaded_programs: &ProgramCacheForTxBatch, -) -> 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, - ) - }) - .collect() -} - -fn load_transaction( - callbacks: &CB, +pub(crate) fn load_transaction( + account_loader: &mut AccountLoader, 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, + account_loader, 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 { @@ -269,15 +489,13 @@ struct LoadedTransactionAccounts { } fn load_transaction_accounts( - callbacks: &CB, + account_loader: &mut AccountLoader, message: &impl SVMMessage, loaded_fee_payer_account: LoadedTransactionAccount, compute_budget_limits: &ComputeBudgetLimits, 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(); @@ -286,12 +504,6 @@ fn load_transaction_accounts( 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, @@ -323,15 +535,12 @@ fn load_transaction_accounts( // 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, + account_loader, message, account_key, account_index, - &instruction_accounts[..], - account_overrides, feature_set, rent_collector, - loaded_programs, )?; collect_loaded_account(account_key, (loaded_account, account_found))?; } @@ -371,7 +580,12 @@ fn load_transaction_accounts( .iter() .any(|(key, _)| key == owner_id) { - if let Some(owner_account) = callbacks.get_account_shared_data(owner_id) { + // NOTE this will be changed to a `load_account()` call in a PR immediately following this one + // we want to stop pushing loaders on the accounts vec, but tests need to change if we do that + // and this PR is complex enough that we want to code review any new behaviors separately + if let Some(owner_account) = + account_loader.callbacks.get_account_shared_data(owner_id) + { if !native_loader::check_id(owner_account.owner()) || !owner_account.executable() { @@ -404,93 +618,51 @@ fn load_transaction_accounts( } fn load_transaction_account( - callbacks: &CB, + account_loader: &mut AccountLoader, message: &impl SVMMessage, account_key: &Pubkey, account_index: usize, - instruction_accounts: &[&u8], - account_overrides: Option<&AccountOverrides>, feature_set: &FeatureSet, rent_collector: &dyn SVMRentCollector, - loaded_programs: &ProgramCacheForTxBatch, ) -> Result<(LoadedTransactionAccount, bool)> { let mut account_found = true; - let is_instruction_account = u8::try_from(account_index) - .map(|i| instruction_accounts.contains(&&i)) - .unwrap_or(false); - let is_writable = message.is_writable(account_index); + let usage_pattern = AccountUsagePattern::new(message, account_index); + 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. + // for each transaction instruction, it cannot be loaded. LoadedTransactionAccount { loaded_size: 0, account: construct_instructions_account(message), rent_collected: 0, } - } else if let Some(account_override) = - account_overrides.and_then(|overrides| overrides.get(account_key)) + } else if let Some(mut loaded_account) = account_loader.load_account(account_key, usage_pattern) { + loaded_account.rent_collected = if usage_pattern == AccountUsagePattern::Writable { + collect_rent_from_account( + feature_set, + rent_collector, + account_key, + &mut loaded_account.account, + ) + .rent_amount + } else { + 0 + }; + + loaded_account + } else { + 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); LoadedTransactionAccount { - loaded_size: account_override.data().len(), - account: account_override.clone(), - rent_collected: 0, - } - } else if let Some(program) = (!is_instruction_account && !is_writable) - .then_some(()) - .and_then(|_| loaded_programs.find(account_key)) - { - callbacks - .get_account_shared_data(account_key) - .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), + loaded_size: default_account.data().len(), + account: default_account, rent_collected: 0, } - } else { - callbacks - .get_account_shared_data(account_key) - .map(|mut account| { - // Inspect the account prior to collecting rent, since - // rent collection can modify the account. - callbacks.inspect_account(account_key, AccountState::Alive(&account), is_writable); - - let rent_collected = if is_writable { - 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, - } - }) - .unwrap_or_else(|| { - callbacks.inspect_account(account_key, AccountState::Dead, is_writable); - - 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); - LoadedTransactionAccount { - loaded_size: default_account.data().len(), - account: default_account, - rent_collected: 0, - } - }) }; Ok((loaded_account, account_found)) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 7b54dfbf878297..542300e0e3425b 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -3,10 +3,9 @@ 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, + collect_rent_from_account, load_transaction, validate_fee_payer, AccountLoader, + AccountUsagePattern, CheckedTransactionDetails, LoadedTransaction, + TransactionCheckResult, TransactionLoadResult, ValidatedTransactionDetails, }, account_overrides::AccountOverrides, message_processor::MessageProcessor, @@ -15,9 +14,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}, }, + itertools::Itertools, log::debug, percentage::Percentage, solana_bpf_loader_program::syscalls::{ @@ -40,14 +40,16 @@ use { solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, solana_sdk::{ account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, + account_utils::StateMut, clock::{Epoch, Slot}, fee::{FeeBudgetLimits, FeeStructure}, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, + nonce::state::{DurableNonce, State as NonceState, Versions as NonceVersions}, pubkey::Pubkey, rent_collector::RentCollector, - saturating_add_assign, + saturating_add_assign, system_program, transaction::{self, TransactionError}, transaction_context::{ExecutionRecord, TransactionContext}, }, @@ -262,27 +264,13 @@ impl TransactionBatchProcessor { // Initialize metrics. let mut error_metrics = TransactionErrorMetrics::default(); let mut execute_timings = ExecuteTimings::default(); + let mut processing_results = Vec::with_capacity(sanitized_txs.len()); - 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 (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() { @@ -309,62 +297,128 @@ impl TransactionBatchProcessor { program_cache_for_tx_batch }); - let (loaded_transactions, load_accounts_us) = measure_us!(load_accounts( + let account_keys_in_batch = sanitized_txs + .iter() + .flat_map(|tx| tx.account_keys().iter()) + .sorted() + .dedup() + .count(); + + let mut account_loader = AccountLoader::new_with_account_cache_capacity( callbacks, - sanitized_txs, - validation_results, - &mut error_metrics, config.account_overrides, - &environment.feature_set, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), - &program_cache_for_tx_batch, - )); + program_cache_for_tx_batch, + account_keys_in_batch, + ); let enable_transaction_loading_failure_fees = environment .feature_set .is_active(&enable_transaction_loading_failure_fees::id()); - 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) - } + + let (mut validate_fees_us, mut load_us, mut execution_us): (u64, u64, u64) = (0, 0, 0); + + // Validate, execute, and collect results from each transaction in order. + // With SIMD83, transactions must be executed in order, because transactions + // in the same batch may modify the same accounts. Transaction order is + // preserved within entries written to the ledger. + for (tx, check_result) in sanitized_txs.iter().zip(check_results) { + let (validate_result, single_validate_fees_us) = measure_us!(check_result + .and_then(|tx_details| { + // If this is a nonce transaction, validate the nonce info. + // This must be done for every transaction to supprt SIMD83 because + // it may have changed due to use, authorization, or deallocation. + // This function is a successful no-op if given a blockhash transaction. + Self::validate_transaction_nonce( + &mut account_loader, + tx, + &tx_details, + &environment.blockhash, + &mut error_metrics, + )?; + + Ok(tx_details) + }) + .and_then(|tx_details| { + // Now validate the fee-payer for all transactions. + Self::validate_transaction_fee_payer( + &mut account_loader, + tx, + tx_details, + &environment.feature_set, + environment + .fee_structure + .unwrap_or(&FeeStructure::default()), + environment + .rent_collector + .unwrap_or(&RentCollector::default()), + &mut error_metrics, + ) + })); + validate_fees_us = validate_fees_us.saturating_add(single_validate_fees_us); + + let (load_result, single_load_us) = measure_us!(load_transaction( + &mut account_loader, + tx, + validate_result, + &mut error_metrics, + &environment.feature_set, + environment + .rent_collector + .unwrap_or(&RentCollector::default()), + )); + load_us = load_us.saturating_add(single_load_us); + + let (processing_result, single_execution_us) = measure_us!(match load_result { + TransactionLoadResult::NotLoaded(err) => Err(err), + TransactionLoadResult::FeesOnly(fees_only_tx) => { + if enable_transaction_loading_failure_fees { + // Update loaded accounts cache with nonce and fee-payer + account_loader + .update_accounts_for_failed_tx(tx, &fees_only_tx.rollback_accounts); + + 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); - } + } + TransactionLoadResult::Loaded(loaded_transaction) => { + let executed_tx = self.execute_loaded_transaction( + tx, + loaded_transaction, + &mut execute_timings, + &mut error_metrics, + &mut account_loader.program_cache, + environment, + config, + ); - Ok(ProcessedTransaction::Executed(Box::new(executed_tx))) + // Update batch specific cache of the loaded programs with the modifications + // made by the transaction, if it executed successfully. + if executed_tx.was_successful() { + account_loader + .program_cache + .merge(&executed_tx.programs_modified_by_tx); } - }) - .collect()); + + // Update loaded accounts cache with account states which might have changed + let processed_tx = ProcessedTransaction::Executed(Box::new(executed_tx)); + account_loader.update_accounts_for_executed_tx(tx, &processed_tx); + + Ok(processed_tx) + } + }); + execution_us = execution_us.saturating_add(single_execution_us); + + processing_results.push(processing_result); + } // Skip eviction when there's no chance this particular tx batch has increased the size of // ProgramCache entries. Note that loaded_missing is deliberately defined, so that there's // still at least one other batch, which will evict the program cache, even after the // occurrences of cooperative loading. - if program_cache_for_tx_batch.loaded_missing || program_cache_for_tx_batch.merged_modified { + if account_loader.program_cache.loaded_missing + || account_loader.program_cache.merged_modified + { const SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE: u8 = 90; self.program_cache .write() @@ -377,7 +431,7 @@ impl TransactionBatchProcessor { debug!( "load: {}us execute: {}us txs_len={}", - load_accounts_us, + load_us, execution_us, sanitized_txs.len(), ); @@ -386,7 +440,7 @@ impl TransactionBatchProcessor { .saturating_add_in_place(ExecuteTimingType::ValidateFeesUs, validate_fees_us); 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::LoadUs, load_us); execute_timings.saturating_add_in_place(ExecuteTimingType::ExecuteUs, execution_us); LoadAndExecuteSanitizedTransactionsOutput { @@ -396,45 +450,11 @@ 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() - } - // 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( - &self, - callbacks: &CB, - account_overrides: Option<&AccountOverrides>, + account_loader: &mut AccountLoader, message: &impl SVMMessage, checked_details: CheckedTransactionDetails, feature_set: &FeatureSet, @@ -450,30 +470,20 @@ impl TransactionBatchProcessor { })?; let fee_payer_address = message.fee_payer(); - let mut fee_payer_account = if let Some(fee_payer_account) = - account_overrides.and_then(|overrides| overrides.get(fee_payer_address).cloned()) - { - fee_payer_account - } else if let Some(fee_payer_account) = callbacks.get_account_shared_data(fee_payer_address) - { - callbacks.inspect_account( - fee_payer_address, - AccountState::Alive(&fee_payer_account), - true, // <-- is_writable - ); - fee_payer_account - } else { + let Some(mut loaded_fee_payer) = + account_loader.load_account(fee_payer_address, AccountUsagePattern::Writable) + else { error_counters.account_not_found += 1; return Err(TransactionError::AccountNotFound); }; - let fee_payer_loaded_rent_epoch = fee_payer_account.rent_epoch(); - let fee_payer_rent_debit = collect_rent_from_account( + let fee_payer_loaded_rent_epoch = loaded_fee_payer.account.rent_epoch(); + loaded_fee_payer.rent_collected = collect_rent_from_account( feature_set, rent_collector, fee_payer_address, - &mut fee_payer_account, + &mut loaded_fee_payer.account, ) .rent_amount; @@ -494,7 +504,7 @@ impl TransactionBatchProcessor { let fee_payer_index = 0; validate_fee_payer( fee_payer_address, - &mut fee_payer_account, + &mut loaded_fee_payer.account, fee_payer_index, error_counters, rent_collector, @@ -506,8 +516,8 @@ impl TransactionBatchProcessor { let rollback_accounts = RollbackAccounts::new( nonce, *fee_payer_address, - fee_payer_account.clone(), - fee_payer_rent_debit, + loaded_fee_payer.account.clone(), + loaded_fee_payer.rent_collected, fee_payer_loaded_rent_epoch, ); @@ -515,24 +525,90 @@ impl TransactionBatchProcessor { fee_details, rollback_accounts, compute_budget_limits, - loaded_fee_payer_account: LoadedTransactionAccount { - loaded_size: fee_payer_account.data().len(), - account: fee_payer_account, - rent_collected: fee_payer_rent_debit, - }, + loaded_fee_payer_account: loaded_fee_payer, }) } + fn validate_transaction_nonce( + account_loader: &mut AccountLoader, + message: &impl SVMMessage, + checked_details: &CheckedTransactionDetails, + environment_blockhash: &Hash, + error_counters: &mut TransactionErrorMetrics, + ) -> transaction::Result<()> { + if let CheckedTransactionDetails { + nonce: Some(nonce_info), + lamports_per_signature: _, + } = checked_details + { + let next_durable_nonce = DurableNonce::from_blockhash(environment_blockhash); + + // If the nonce has been used in this batch already, we must drop the transaction. + // This is the same as if it was used in different batches in the same slot. + // If the nonce account was closed in the batch, we error as if the blockhash didn't validate. + // We must validate the account in case it was reopened, either as a normal system account, + // or a fake nonce account. We must also check the signer in case the authority was changed. + // + // Note these checks are *not* obviated by fee-only transactions. Instead, when we relax + // account locks for SIMD83, we can remove most batch-level checks and rely solely on these. + let nonce_is_valid = account_loader + .load_account(nonce_info.address(), AccountUsagePattern::Writable) + .and_then(|loaded_nonce| { + let current_nonce_account = &loaded_nonce.account; + system_program::check_id(current_nonce_account.owner()).then_some(())?; + StateMut::::state(current_nonce_account).ok() + }) + .and_then( + |current_nonce_versions| match current_nonce_versions.state() { + NonceState::Initialized(ref current_nonce_data) => { + let nonce_can_be_advanced = + current_nonce_data.durable_nonce != next_durable_nonce; + + let nonce_authority_is_valid = message + .account_keys() + .iter() + .enumerate() + .any(|(i, address)| { + address == ¤t_nonce_data.authority && message.is_signer(i) + }); + + if nonce_authority_is_valid { + Some(nonce_can_be_advanced) + } else { + None + } + } + _ => None, + }, + ); + + match nonce_is_valid { + None => { + error_counters.blockhash_not_found += 1; + Err(TransactionError::BlockhashNotFound) + } + Some(false) => { + error_counters.account_not_found += 1; + Err(TransactionError::AccountNotFound) + } + Some(true) => Ok(()), + } + } else { + // not a nonce transaction, nothing to do + Ok(()) + } + } + /// 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() From 3350328b40aed1582c5abc7613b78d3e28e8ed0e Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Fri, 11 Oct 2024 12:01:36 -0700 Subject: [PATCH 2/7] fix tx processor unit tests --- svm/src/transaction_processor.rs | 98 +++++++++++++++++++------------- 1 file changed, 60 insertions(+), 38 deletions(-) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 542300e0e3425b..4743dd915c3f06 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1099,8 +1099,10 @@ mod tests { use { super::*, crate::{ - account_loader::ValidatedTransactionDetails, nonce_info::NonceInfo, + account_loader::{LoadedTransactionAccount, ValidatedTransactionDetails}, + nonce_info::NonceInfo, rollback_accounts::RollbackAccounts, + transaction_processing_callback::AccountState, }, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_feature_set::FeatureSet, @@ -1199,6 +1201,12 @@ mod tests { } } + impl<'a> From<&'a MockBankCallback> for AccountLoader<'a, MockBankCallback> { + fn from(callbacks: &'a MockBankCallback) -> AccountLoader<'a, MockBankCallback> { + AccountLoader::new(callbacks, None, ProgramCacheForTxBatch::default()) + } + } + #[test_case(1; "Check results too small")] #[test_case(3; "Check results too large")] #[should_panic(expected = "Length of check_results does not match length of sanitized_txs")] @@ -1568,9 +1576,9 @@ mod tests { sanitized_transaction_2.clone(), sanitized_transaction_1, ]; - let validation_results = vec![ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), + let check_results = vec![ + Ok(CheckedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), Err(TransactionError::ProgramAccountNotFound), ]; let owners = vec![owner1, owner2]; @@ -1578,7 +1586,7 @@ mod tests { let result = TransactionBatchProcessor::::filter_executable_program_accounts( &mock_bank, &transactions, - &validation_results, + &check_results, &owners, ); @@ -1661,8 +1669,8 @@ mod tests { &bank, &[sanitized_tx1, sanitized_tx2], &[ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), ], owners, ); @@ -1753,15 +1761,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, ); @@ -2032,17 +2040,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2110,17 +2119,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2162,16 +2172,17 @@ mod tests { new_unchecked_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); let mock_bank = MockBankCallback::default(); + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2195,17 +2206,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2233,17 +2245,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2269,17 +2282,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2302,16 +2316,17 @@ mod tests { )); let mock_bank = MockBankCallback::default(); + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2362,23 +2377,23 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); - let nonce = Some(NonceInfo::new( - *fee_payer_address, - fee_payer_account.clone(), - )); + let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); + let mut future_nonce = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); + future_nonce.try_advance_nonce(durable_nonce, 0).unwrap(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { - nonce: nonce.clone(), + nonce: Some(future_nonce.clone()), lamports_per_signature, }, + &durable_nonce, &feature_set, &FeeStructure::default(), &rent_collector, @@ -2396,7 +2411,7 @@ mod tests { result, Ok(ValidatedTransactionDetails { rollback_accounts: RollbackAccounts::new( - nonce, + Some(future_nonce), *fee_payer_address, post_validation_fee_payer_account.clone(), 0, // fee_payer_rent_debit @@ -2430,17 +2445,18 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &feature_set, &FeeStructure::default(), &rent_collector, @@ -2483,18 +2499,23 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = AccountLoader::new( + &mock_bank, + Some(&account_overrides), + ProgramCacheForTxBatch::default(), + ); let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - Some(&account_overrides), + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2524,6 +2545,7 @@ mod tests { .write() .unwrap() .insert(fee_payer_address, fee_payer_account.clone()); + let mut account_loader = (&mock_bank).into(); let message = new_unchecked_sanitized_message(Message::new_with_blockhash( &[ @@ -2536,13 +2558,13 @@ mod tests { let batch_processor = TransactionBatchProcessor::::default(); batch_processor .validate_transaction_fee_payer( - &mock_bank, - None, + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature: 5000, }, + &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), From 1fb89e271c063c194cc4ab5b1f34c43bc8f9125b Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Fri, 11 Oct 2024 12:35:26 -0700 Subject: [PATCH 3/7] fix account loader unit tests --- svm/src/account_loader.rs | 357 +++++++++++++++++++------------------- 1 file changed, 178 insertions(+), 179 deletions(-) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 9beda9a1c47247..40ece47fe57c55 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -55,6 +55,7 @@ pub enum TransactionLoadResult { } #[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, @@ -807,13 +808,19 @@ mod tests { } } + impl<'a> From<&'a TestCallbacks> for AccountLoader<'a, TestCallbacks> { + fn from(callbacks: &'a TestCallbacks) -> AccountLoader<'a, TestCallbacks> { + AccountLoader::new(callbacks, None, ProgramCacheForTxBatch::default()) + } + } + fn load_accounts_with_features_and_rent( tx: Transaction, accounts: &[TransactionAccount], rent_collector: &RentCollector, error_metrics: &mut TransactionErrorMetrics, feature_set: &mut FeatureSet, - ) -> Vec { + ) -> TransactionLoadResult { 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(); @@ -825,21 +832,20 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( - &callbacks, - &[sanitized_tx], - vec![Ok(ValidatedTransactionDetails { + let mut account_loader = (&callbacks).into(); + load_transaction( + &mut account_loader, + &sanitized_tx, + Ok(ValidatedTransactionDetails { loaded_fee_payer_account: LoadedTransactionAccount { account: fee_payer_account, ..LoadedTransactionAccount::default() }, ..ValidatedTransactionDetails::default() - })], + }), error_metrics, - None, feature_set, rent_collector, - &ProgramCacheForTxBatch::default(), ) } @@ -886,7 +892,7 @@ mod tests { tx: Transaction, accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, - ) -> Vec { + ) -> TransactionLoadResult { load_accounts_with_features_and_rent( tx, accounts, @@ -901,7 +907,7 @@ mod tests { accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, exclude_features: Option<&[Pubkey]>, - ) -> Vec { + ) -> TransactionLoadResult { load_accounts_with_features_and_rent( tx, accounts, @@ -938,9 +944,8 @@ mod tests { let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics); assert_eq!(error_metrics.account_not_found, 1); - assert_eq!(load_results.len(), 1); assert!(matches!( - load_results[0], + load_results, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::ProgramAccountNotFound, .. @@ -978,8 +983,7 @@ mod tests { load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics, None); assert_eq!(error_metrics.account_not_found, 0); - assert_eq!(loaded_accounts.len(), 1); - match &loaded_accounts[0] { + match &loaded_accounts { TransactionLoadResult::Loaded(loaded_transaction) => { assert_eq!(loaded_transaction.accounts.len(), 3); assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1); @@ -1020,9 +1024,8 @@ mod tests { let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics); assert_eq!(error_metrics.account_not_found, 1); - assert_eq!(load_results.len(), 1); assert!(matches!( - load_results[0], + load_results, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::ProgramAccountNotFound, .. @@ -1057,9 +1060,8 @@ mod tests { let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics); assert_eq!(error_metrics.invalid_program_for_execution, 1); - assert_eq!(load_results.len(), 1); assert!(matches!( - load_results[0], + load_results, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::InvalidProgramForExecution, .. @@ -1109,8 +1111,7 @@ mod tests { load_accounts_with_excluded_features(tx, &accounts, &mut error_metrics, None); assert_eq!(error_metrics.account_not_found, 0); - assert_eq!(loaded_accounts.len(), 1); - match &loaded_accounts[0] { + match &loaded_accounts { TransactionLoadResult::Loaded(loaded_transaction) => { assert_eq!(loaded_transaction.accounts.len(), 4); assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1); @@ -1127,7 +1128,7 @@ mod tests { accounts: &[TransactionAccount], tx: Transaction, account_overrides: Option<&AccountOverrides>, - ) -> Vec { + ) -> TransactionLoadResult { let tx = SanitizedTransaction::from_transaction_for_tests(tx); let mut error_metrics = TransactionErrorMetrics::default(); @@ -1139,15 +1140,18 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( + let mut account_loader = AccountLoader::new( &callbacks, - &[tx], - vec![Ok(ValidatedTransactionDetails::default())], - &mut error_metrics, account_overrides, + ProgramCacheForTxBatch::default(), + ); + load_transaction( + &mut account_loader, + &tx, + Ok(ValidatedTransactionDetails::default()), + &mut error_metrics, &FeatureSet::all_enabled(), &RentCollector::default(), - &ProgramCacheForTxBatch::default(), ) } @@ -1166,9 +1170,8 @@ mod tests { ); let load_results = load_accounts_no_store(&[], tx, None); - assert_eq!(load_results.len(), 1); assert!(matches!( - load_results[0], + load_results, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::ProgramAccountNotFound, .. @@ -1198,8 +1201,7 @@ mod tests { let loaded_accounts = load_accounts_no_store(&[(keypair.pubkey(), account)], tx, Some(&account_overrides)); - assert_eq!(loaded_accounts.len(), 1); - match &loaded_accounts[0] { + match &loaded_accounts { TransactionLoadResult::Loaded(loaded_transaction) => { assert_eq!(loaded_transaction.accounts[0].0, keypair.pubkey()); assert_eq!(loaded_transaction.accounts[1].0, slot_history_id); @@ -1419,10 +1421,10 @@ mod tests { mock_bank .accounts_map .insert(fee_payer_address, fee_payer_account.clone()); + let mut account_loader = (&mock_bank).into(); 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, @@ -1430,7 +1432,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { loaded_size: fee_payer_account.data().len(), @@ -1439,10 +1441,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); let expected_rent_debits = { @@ -1486,9 +1486,9 @@ mod tests { mock_bank .accounts_map .insert(key1.pubkey(), fee_payer_account.clone()); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1496,7 +1496,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1504,10 +1504,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!( @@ -1554,24 +1552,24 @@ mod tests { let mut loaded_programs = ProgramCacheForTxBatch::default(); loaded_programs.replenish(key2.pubkey(), Arc::new(ProgramCacheEntry::default())); + let mut account_loader = AccountLoader::new(&mock_bank, None, loaded_programs); + let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, vec![Signature::new_unique()], false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount::default(), &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); - assert_eq!(result.err(), Some(TransactionError::AccountNotFound)); + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); } #[test] @@ -1610,10 +1608,11 @@ mod tests { mock_bank .accounts_map .insert(native_loader::id(), loader_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let mut loaded_programs = ProgramCacheForTxBatch::default(); + // without cache, program is invalid let transaction = SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( &[Instruction::new_with_bytes( @@ -1627,7 +1626,7 @@ mod tests { )); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1635,25 +1634,23 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); - // without cache, program is invalid assert_eq!( result.err(), Some(TransactionError::InvalidProgramForExecution) ); - loaded_programs.replenish( + // with cache, executable flag is bypassed + account_loader.program_cache.replenish( program_keypair.pubkey(), Arc::new(ProgramCacheEntry::default()), ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1661,13 +1658,10 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); - // with cache, executable flag is bypassed let mut cached_program = AccountSharedData::default(); cached_program.set_owner(native_loader::id()); cached_program.set_executable(true); @@ -1686,6 +1680,7 @@ mod tests { } ); + // including program as instruction account bypasses executable bypass let transaction = SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( &[Instruction::new_with_bytes( @@ -1699,7 +1694,7 @@ mod tests { )); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1707,13 +1702,10 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); - // including program as instruction account bypasses executable bypass assert_eq!( result.err(), Some(TransactionError::InvalidProgramForExecution) @@ -1725,7 +1717,7 @@ mod tests { ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1733,10 +1725,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); // including program as writable bypasses executable bypass @@ -1767,9 +1757,9 @@ mod tests { let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); mock_bank.accounts_map.insert(key1.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1777,15 +1767,13 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount::default(), &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1812,9 +1800,9 @@ mod tests { let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); mock_bank.accounts_map.insert(key1.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1822,15 +1810,13 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount::default(), &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!( @@ -1867,8 +1853,9 @@ mod tests { mock_bank .accounts_map .insert(key2.pubkey(), fee_payer_account.clone()); + let mut account_loader = (&mock_bank).into(); + let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1876,7 +1863,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1884,10 +1871,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!( @@ -1933,8 +1918,9 @@ mod tests { let mut account_data = AccountSharedData::default(); account_data.set_lamports(200); mock_bank.accounts_map.insert(key2.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); + let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1942,15 +1928,13 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount::default(), &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1983,12 +1967,12 @@ mod tests { 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(key3.pubkey(), AccountSharedData::default()); + let mut account_loader = (&mock_bank).into(); + let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1996,15 +1980,13 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount::default(), &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!( @@ -2047,9 +2029,9 @@ mod tests { account_data.set_executable(true); account_data.set_owner(native_loader::id()); mock_bank.accounts_map.insert(key3.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -2057,7 +2039,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -2065,10 +2047,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); assert_eq!( @@ -2135,9 +2115,9 @@ mod tests { account_data.set_executable(true); account_data.set_owner(native_loader::id()); mock_bank.accounts_map.insert(key3.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -2145,7 +2125,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -2153,10 +2133,8 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); let mut account_data = AccountSharedData::default(); @@ -2200,9 +2178,9 @@ mod tests { let mut mint_data = AccountSharedData::default(); mint_data.set_lamports(2); bank.accounts_map.insert(mint_keypair.pubkey(), mint_data); - bank.accounts_map .insert(recipient, AccountSharedData::default()); + let mut account_loader = (&bank).into(); let tx = system_transaction::transfer( &mint_keypair, @@ -2213,18 +2191,16 @@ 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( - &bank, - &[sanitized_tx.clone()], - vec![Ok(ValidatedTransactionDetails::default())], + let load_result = load_transaction( + &mut account_loader, + &sanitized_tx.clone(), + Ok(ValidatedTransactionDetails::default()), &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &ProgramCacheForTxBatch::default(), ); - let TransactionLoadResult::Loaded(loaded_transaction) = load_results.swap_remove(0) else { + let TransactionLoadResult::Loaded(loaded_transaction) = load_result else { panic!("transaction loading failed"); }; @@ -2292,9 +2268,9 @@ mod tests { account_data.set_executable(true); account_data.set_owner(native_loader::id()); mock_bank.accounts_map.insert(key3.pubkey(), account_data); + let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -2309,22 +2285,19 @@ mod tests { ..ValidatedTransactionDetails::default() }); - let mut load_results = load_accounts( - &mock_bank, - &[sanitized_transaction], - vec![validation_result], + let load_result = load_transaction( + &mut account_loader, + &sanitized_transaction, + validation_result, &mut error_metrics, - None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, ); let mut account_data = AccountSharedData::default(); account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - assert_eq!(load_results.len(), 1); - let TransactionLoadResult::Loaded(loaded_transaction) = load_results.swap_remove(0) else { + let TransactionLoadResult::Loaded(loaded_transaction) = load_result else { panic!("transaction loading failed"); }; assert_eq!( @@ -2359,6 +2332,7 @@ mod tests { #[test] fn test_load_accounts_error() { let mock_bank = TestCallbacks::default(); + let mut account_loader = (&mock_bank).into(); let feature_set = FeatureSet::default(); let rent_collector = RentCollector::default(); @@ -2381,19 +2355,17 @@ mod tests { ); let validation_result = Ok(ValidatedTransactionDetails::default()); - let load_results = load_accounts( - &mock_bank, - &[sanitized_transaction.clone()], - vec![validation_result.clone()], + let load_result = load_transaction( + &mut account_loader, + &sanitized_transaction, + validation_result.clone(), &mut TransactionErrorMetrics::default(), - None, &feature_set, &rent_collector, - &ProgramCacheForTxBatch::default(), ); assert!(matches!( - load_results[0], + load_result, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::InvalidProgramForExecution, .. @@ -2402,19 +2374,17 @@ mod tests { let validation_result = Err(TransactionError::InvalidWritableAccount); - let load_results = load_accounts( - &mock_bank, - &[sanitized_transaction.clone()], - vec![validation_result], + let load_result = load_transaction( + &mut account_loader, + &sanitized_transaction, + validation_result, &mut TransactionErrorMetrics::default(), - None, &feature_set, &rent_collector, - &ProgramCacheForTxBatch::default(), ); assert!(matches!( - load_results[0], + load_result, TransactionLoadResult::NotLoaded(TransactionError::InvalidWritableAccount), )); } @@ -2516,6 +2486,7 @@ mod tests { account3.set_executable(true); account3.set_owner(native_loader::id()); mock_bank.accounts_map.insert(address3, account3.clone()); + let mut account_loader = (&mock_bank).into(); let message = Message { account_keys: vec![address0, address1, address2, address3], @@ -2552,15 +2523,13 @@ mod tests { }, ..ValidatedTransactionDetails::default() }); - let _load_results = load_accounts( - &mock_bank, - &[sanitized_transaction], - vec![validation_result], + let _load_results = load_transaction( + &mut account_loader, + &sanitized_transaction, + validation_result, &mut TransactionErrorMetrics::default(), - None, &FeatureSet::default(), &RentCollector::default(), - &ProgramCacheForTxBatch::default(), ); // ensure the loaded accounts are inspected @@ -2634,31 +2603,32 @@ mod tests { let account2 = Pubkey::new_unique(); let (account2_size, _) = make_account(account2, program2, false); - let test_transaction_data_size_with_cache = |transaction, cache, expected_size| { - let loaded_transaction_accounts = load_transaction_accounts( - &mock_bank, - &transaction, - LoadedTransactionAccount { - account: fee_payer_account.clone(), - loaded_size: fee_payer_size as usize, - rent_collected: 0, - }, - &ComputeBudgetLimits::default(), - &mut TransactionErrorMetrics::default(), - None, - &FeatureSet::default(), - &RentCollector::default(), - &cache, - ) - .unwrap(); + let test_transaction_data_size = + |account_loader: &mut AccountLoader, transaction, expected_size| { + let loaded_transaction_accounts = load_transaction_accounts( + account_loader, + &transaction, + LoadedTransactionAccount { + account: fee_payer_account.clone(), + loaded_size: fee_payer_size as usize, + rent_collected: 0, + }, + &ComputeBudgetLimits::default(), + &mut TransactionErrorMetrics::default(), + &FeatureSet::default(), + &RentCollector::default(), + ) + .unwrap(); - assert_eq!( - loaded_transaction_accounts.loaded_accounts_data_size, - expected_size - ); - }; + assert_eq!( + loaded_transaction_accounts.loaded_accounts_data_size, + expected_size + ); + }; - let test_data_size_with_cache = |instructions: Vec<_>, cache, expected_size| { + let test_data_size = |account_loader: &mut AccountLoader, + instructions: Vec<_>, + expected_size| { let transaction = SanitizedTransaction::from_transaction_for_tests( Transaction::new_signed_with_payer( &instructions, @@ -2668,21 +2638,19 @@ mod tests { ), ); - test_transaction_data_size_with_cache(transaction, cache, expected_size) + test_transaction_data_size(account_loader, transaction, expected_size) }; for account_meta in [AccountMeta::new, AccountMeta::new_readonly] { - let test_data_size = |instructions, expected_size| { - test_data_size_with_cache( - instructions, - ProgramCacheForTxBatch::default(), - expected_size, - ) - }; + let mut account_loader = (&mock_bank).into(); // one program plus loader let ixns = vec![Instruction::new_with_bytes(program1, &[], vec![])]; - test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + program1_size + bpf_loader_size + fee_payer_size, + ); // two programs, two loaders, two accounts let ixns = vec![ @@ -2690,6 +2658,7 @@ mod tests { Instruction::new_with_bytes(program2, &[], vec![account_meta(account2, false)]), ]; test_data_size( + &mut account_loader, ixns, account1_size + account2_size @@ -2707,6 +2676,7 @@ mod tests { vec![account_meta(account2, false)], )]; test_data_size( + &mut account_loader, ixns, account2_size + program1_size + bpf_loader_size + fee_payer_size, ); @@ -2716,11 +2686,15 @@ mod tests { Instruction::new_with_bytes(program1, &[], vec![]), Instruction::new_with_bytes(program1, &[], vec![]), ]; - test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + program1_size + bpf_loader_size + fee_payer_size, + ); // native loader not counted if loader let ixns = vec![Instruction::new_with_bytes(bpf_loader::id(), &[], vec![])]; - test_data_size(ixns, bpf_loader_size + fee_payer_size); + test_data_size(&mut account_loader, ixns, bpf_loader_size + fee_payer_size); // native loader counted if instruction let ixns = vec![Instruction::new_with_bytes( @@ -2728,7 +2702,11 @@ mod tests { &[], vec![account_meta(native_loader::id(), false)], )]; - test_data_size(ixns, bpf_loader_size + native_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + bpf_loader_size + native_loader_size + fee_payer_size, + ); // native loader counted if invoked let ixns = vec![Instruction::new_with_bytes( @@ -2736,7 +2714,11 @@ mod tests { &[], vec![], )]; - test_data_size(ixns, native_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + native_loader_size + fee_payer_size, + ); // native loader counted once if invoked and instruction let ixns = vec![Instruction::new_with_bytes( @@ -2744,7 +2726,11 @@ mod tests { &[], vec![account_meta(native_loader::id(), false)], )]; - test_data_size(ixns, native_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + native_loader_size + fee_payer_size, + ); // loader counted twice if included in instruction let ixns = vec![Instruction::new_with_bytes( @@ -2752,7 +2738,11 @@ mod tests { &[], vec![account_meta(bpf_loader::id(), false)], )]; - test_data_size(ixns, program1_size + bpf_loader_size * 2 + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + program1_size + bpf_loader_size * 2 + fee_payer_size, + ); // cover that case with multiple loaders to be sure let ixns = vec![ @@ -2772,6 +2762,7 @@ mod tests { ), ]; test_data_size( + &mut account_loader, ixns, account1_size + program1_size @@ -2786,7 +2777,11 @@ mod tests { Instruction::new_with_bytes(bpf_loader::id(), &[], vec![]), Instruction::new_with_bytes(program1, &[], vec![]), ]; - test_data_size(ixns, program1_size + bpf_loader_size * 2 + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + program1_size + bpf_loader_size * 2 + fee_payer_size, + ); // fee-payer counted once let ixns = vec![Instruction::new_with_bytes( @@ -2794,23 +2789,27 @@ mod tests { &[], vec![account_meta(fee_payer, false)], )]; - test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size); + test_data_size( + &mut account_loader, + ixns, + program1_size + bpf_loader_size + fee_payer_size, + ); // edge cases involving program cache - let mut program_cache = ProgramCacheForTxBatch::default(); - let program2_entry = ProgramCacheEntry { account_size: (program2_size + programdata2_size) as usize, account_owner: ProgramCacheEntryOwner::LoaderV3, ..ProgramCacheEntry::default() }; - program_cache.replenish(program2, Arc::new(program2_entry)); + account_loader + .program_cache + .replenish(program2, Arc::new(program2_entry)); - // normal function call uses the combined cache size + // normal invocation uses the combined cache size let ixns = vec![Instruction::new_with_bytes(program2, &[], vec![])]; - test_data_size_with_cache( + test_data_size( + &mut account_loader, ixns, - program_cache.clone(), program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size, ); @@ -2820,9 +2819,9 @@ mod tests { &[], vec![account_meta(program2, false)], )]; - test_data_size_with_cache( + test_data_size( + &mut account_loader, ixns, - program_cache.clone(), program2_size + upgradeable_loader_size + fee_payer_size, ); @@ -2832,9 +2831,9 @@ mod tests { &[], vec![account_meta(programdata2, false)], )]; - test_data_size_with_cache( + test_data_size( + &mut account_loader, ixns, - program_cache.clone(), program2_size + programdata2_size * 2 + upgradeable_loader_size + fee_payer_size, ); @@ -2847,17 +2846,17 @@ mod tests { account_meta(programdata2, false), ], )]; - test_data_size_with_cache( + test_data_size( + &mut account_loader, ixns, - program_cache.clone(), program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size, ); // writable program bypasses the cache let tx = new_unchecked_sanitized_transaction_with_writable_program(program2, fee_payer); - test_transaction_data_size_with_cache( + test_transaction_data_size( + &mut account_loader, tx, - program_cache.clone(), program2_size + upgradeable_loader_size + fee_payer_size, ); From 5405ceb3826d9efc540d78200083ce3b28aa135b Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Thu, 10 Oct 2024 03:16:29 -0700 Subject: [PATCH 4/7] add integration tests from old branch --- svm/tests/integration_test.rs | 1038 ++++++++++++++++++++++++++++++++- 1 file changed, 1034 insertions(+), 4 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 32802953b8374c..96adbb8975e99e 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -4,13 +4,14 @@ use { crate::mock_bank::{ create_executable_environment, deploy_program_with_upgrade_authority, program_address, - register_builtins, MockBankCallback, MockForkGraph, EXECUTION_EPOCH, EXECUTION_SLOT, - WALLCLOCK_TIME, + program_data_size, register_builtins, MockBankCallback, MockForkGraph, EXECUTION_EPOCH, + EXECUTION_SLOT, WALLCLOCK_TIME, }, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, clock::Slot, compute_budget::ComputeBudgetInstruction, + entrypoint::MAX_PERMITTED_DATA_INCREASE, feature_set::{self, FeatureSet}, hash::Hash, instruction::{AccountMeta, Instruction}, @@ -1085,7 +1086,692 @@ fn simple_nonce(enable_fee_only_transactions: bool, fee_paying_nonce: bool) -> V vec![test_entry] } -#[allow(unused)] +fn intrabatch_account_reuse(enable_fee_only_transactions: bool) -> Vec { + let mut test_entries = vec![]; + let transfer_amount = LAMPORTS_PER_SOL; + let wallet_rent = Rent::default().minimum_balance(0); + + // batch 0: two successful transfers from the same source + { + let mut test_entry = SvmTestEntry::default(); + + let source_keypair = Keypair::new(); + let source = source_keypair.pubkey(); + let destination1 = Pubkey::new_unique(); + let destination2 = Pubkey::new_unique(); + + let mut source_data = AccountSharedData::default(); + let destination1_data = AccountSharedData::default(); + let destination2_data = AccountSharedData::default(); + + source_data.set_lamports(LAMPORTS_PER_SOL * 10); + test_entry.add_initial_account(source, &source_data); + + for (destination, mut destination_data) in [ + (destination1, destination1_data), + (destination2, destination2_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); + } + + test_entries.push(test_entry); + } + + // batch 1: + // * successful transfer, source left with rent-exempt minimum + // * non-processable transfer due to underfunded fee-payer + { + let mut test_entry = SvmTestEntry::default(); + + 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(transfer_amount + LAMPORTS_PER_SIGNATURE + wallet_rent); + 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); + + test_entry.push_transaction_with_status( + system_transaction::transfer( + &source_keypair, + &destination, + transfer_amount, + Hash::default(), + ), + ExecutionStatus::Discarded, + ); + + test_entries.push(test_entry); + } + + // batch 2: + // * successful transfer to a previously unfunded account + // * successful transfer using the new account as a fee-payer in the same batch + { + let mut test_entry = SvmTestEntry::default(); + let first_transfer_amount = transfer_amount + LAMPORTS_PER_SIGNATURE + wallet_rent; + let second_transfer_amount = transfer_amount; + + let grandparent_keypair = Keypair::new(); + let grandparent = grandparent_keypair.pubkey(); + let parent_keypair = Keypair::new(); + let parent = parent_keypair.pubkey(); + let child = Pubkey::new_unique(); + + let mut grandparent_data = AccountSharedData::default(); + let mut parent_data = AccountSharedData::default(); + let mut child_data = AccountSharedData::default(); + + grandparent_data.set_lamports(LAMPORTS_PER_SOL * 10); + test_entry.add_initial_account(grandparent, &grandparent_data); + + test_entry.push_transaction(system_transaction::transfer( + &grandparent_keypair, + &parent, + first_transfer_amount, + Hash::default(), + )); + + parent_data + .checked_add_lamports(first_transfer_amount) + .unwrap(); + test_entry.create_expected_account(parent, &parent_data); + + test_entry.decrease_expected_lamports( + &grandparent, + first_transfer_amount + LAMPORTS_PER_SIGNATURE, + ); + + test_entry.push_transaction(system_transaction::transfer( + &parent_keypair, + &child, + second_transfer_amount, + Hash::default(), + )); + + child_data + .checked_add_lamports(second_transfer_amount) + .unwrap(); + test_entry.create_expected_account(child, &child_data); + + test_entry + .decrease_expected_lamports(&parent, second_transfer_amount + LAMPORTS_PER_SIGNATURE); + + test_entries.push(test_entry); + } + + // batch 3: + // * non-processable transfer due to underfunded fee-payer (two signatures) + // * successful transfer with the same fee-payer (one signature) + { + let mut test_entry = SvmTestEntry::default(); + + let feepayer_keypair = Keypair::new(); + let feepayer = feepayer_keypair.pubkey(); + let separate_source_keypair = Keypair::new(); + let separate_source = separate_source_keypair.pubkey(); + let destination = Pubkey::new_unique(); + + let mut feepayer_data = AccountSharedData::default(); + let mut separate_source_data = AccountSharedData::default(); + let mut destination_data = AccountSharedData::default(); + + feepayer_data.set_lamports(1 + LAMPORTS_PER_SIGNATURE + wallet_rent); + test_entry.add_initial_account(feepayer, &feepayer_data); + + separate_source_data.set_lamports(LAMPORTS_PER_SOL * 10); + test_entry.add_initial_account(separate_source, &separate_source_data); + + test_entry.push_transaction_with_status( + Transaction::new_signed_with_payer( + &[system_instruction::transfer( + &separate_source, + &destination, + 1, + )], + Some(&feepayer), + &[&feepayer_keypair, &separate_source_keypair], + Hash::default(), + ), + ExecutionStatus::Discarded, + ); + + test_entry.push_transaction(system_transaction::transfer( + &feepayer_keypair, + &destination, + 1, + Hash::default(), + )); + + destination_data.checked_add_lamports(1).unwrap(); + test_entry.create_expected_account(destination, &destination_data); + + test_entry.decrease_expected_lamports(&feepayer, 1 + LAMPORTS_PER_SIGNATURE); + } + + // batch 4: + // * processable non-executable transaction + // * successful transfer + // this confirms we update the AccountsMap from RollbackAccounts intrabatch + if enable_fee_only_transactions { + let mut test_entry = SvmTestEntry::default(); + + 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); + + let mut load_program_fail_instruction = + system_instruction::transfer(&source, &Pubkey::new_unique(), transfer_amount); + load_program_fail_instruction.program_id = Pubkey::new_unique(); + + test_entry.push_transaction_with_status( + Transaction::new_signed_with_payer( + &[load_program_fail_instruction], + Some(&source), + &[&source_keypair], + Hash::default(), + ), + ExecutionStatus::ProcessedFailed, + ); + + 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); + + test_entries.push(test_entry); + } + + if enable_fee_only_transactions { + for test_entry in &mut test_entries { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + } + + test_entries +} + +fn nonce_reuse(enable_fee_only_transactions: bool, fee_paying_nonce: bool) -> Vec { + let mut test_entries = vec![]; + + let program_name = "hello-solana"; + let program_id = program_address(program_name); + + let fee_payer_keypair = Keypair::new(); + let non_fee_nonce_keypair = Keypair::new(); + let fee_payer = fee_payer_keypair.pubkey(); + let nonce_pubkey = if fee_paying_nonce { + fee_payer + } else { + non_fee_nonce_keypair.pubkey() + }; + + let nonce_size = nonce::State::size(); + let initial_durable = DurableNonce::from_blockhash(&Hash::new_unique()); + let initial_nonce_data = + nonce::state::Data::new(fee_payer, initial_durable, LAMPORTS_PER_SIGNATURE); + let initial_nonce_account = AccountSharedData::new_data( + LAMPORTS_PER_SOL, + &nonce::state::Versions::new(nonce::State::Initialized(initial_nonce_data.clone())), + &system_program::id(), + ) + .unwrap(); + let initial_nonce_info = NonceInfo::new(nonce_pubkey, initial_nonce_account.clone()); + + let advanced_durable = DurableNonce::from_blockhash(&LAST_BLOCKHASH); + let mut advanced_nonce_info = initial_nonce_info.clone(); + advanced_nonce_info + .try_advance_nonce(advanced_durable, LAMPORTS_PER_SIGNATURE) + .unwrap(); + + let advance_instruction = system_instruction::advance_nonce_account(&nonce_pubkey, &fee_payer); + let withdraw_instruction = system_instruction::withdraw_nonce_account( + &nonce_pubkey, + &fee_payer, + &fee_payer, + LAMPORTS_PER_SOL, + ); + + let successful_noop_instruction = Instruction::new_with_bytes(program_id, &[], vec![]); + let failing_noop_instruction = Instruction::new_with_bytes(system_program::id(), &[], vec![]); + let fee_only_noop_instruction = Instruction::new_with_bytes(Pubkey::new_unique(), &[], vec![]); + + let second_transaction = Transaction::new_signed_with_payer( + &[ + advance_instruction.clone(), + successful_noop_instruction.clone(), + ], + Some(&fee_payer), + &[&fee_payer_keypair], + *advanced_durable.as_hash(), + ); + + let mut common_test_entry = SvmTestEntry::default(); + + common_test_entry.add_initial_account(nonce_pubkey, &initial_nonce_account); + + if !fee_paying_nonce { + let mut fee_payer_data = AccountSharedData::default(); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + common_test_entry.add_initial_account(fee_payer, &fee_payer_data); + } + + common_test_entry + .final_accounts + .get_mut(&nonce_pubkey) + .unwrap() + .data_as_mut_slice() + .copy_from_slice(advanced_nonce_info.account().data()); + + common_test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + let common_test_entry = common_test_entry; + + // batch 0: one transaction that advances the nonce twice + { + let mut test_entry = common_test_entry.clone(); + + let transaction = Transaction::new_signed_with_payer( + &[advance_instruction.clone(), advance_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + *initial_durable.as_hash(), + ); + + test_entry.push_nonce_transaction_with_status( + transaction, + initial_nonce_info.clone(), + ExecutionStatus::ExecutedFailed, + ); + + test_entries.push(test_entry); + } + + // batch 1: + // * a successful nonce transaction + // * a nonce transaction that reuses the same nonce; this transaction must be dropped + { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[ + advance_instruction.clone(), + successful_noop_instruction.clone(), + ], + Some(&fee_payer), + &[&fee_payer_keypair], + *initial_durable.as_hash(), + ); + + test_entry.push_nonce_transaction(first_transaction, initial_nonce_info.clone()); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entries.push(test_entry); + } + + // batch 2: + // * an executable failed nonce transaction + // * a nonce transaction that reuses the same nonce; this transaction must be dropped + { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[advance_instruction.clone(), failing_noop_instruction], + Some(&fee_payer), + &[&fee_payer_keypair], + *initial_durable.as_hash(), + ); + + test_entry.push_nonce_transaction_with_status( + first_transaction, + initial_nonce_info.clone(), + ExecutionStatus::ExecutedFailed, + ); + + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entries.push(test_entry); + } + + // batch 3: + // * a processable non-executable nonce transaction, if fee-only transactions are enabled + // * a nonce transaction that reuses the same nonce; this transaction must be dropped + if enable_fee_only_transactions { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[advance_instruction.clone(), fee_only_noop_instruction], + Some(&fee_payer), + &[&fee_payer_keypair], + *initial_durable.as_hash(), + ); + + test_entry.push_nonce_transaction_with_status( + first_transaction, + initial_nonce_info.clone(), + ExecutionStatus::ProcessedFailed, + ); + + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + // if the nonce account pays fees, it keeps its new rent epoch, otherwise it resets + if !fee_paying_nonce { + test_entry + .final_accounts + .get_mut(&nonce_pubkey) + .unwrap() + .set_rent_epoch(0); + } + + test_entries.push(test_entry); + } + + // batch 4: + // * a successful blockhash transaction that also advances the nonce + // * a nonce transaction that reuses the same nonce; this transaction must be dropped + { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[ + successful_noop_instruction.clone(), + advance_instruction.clone(), + ], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + test_entry.push_nonce_transaction(first_transaction, initial_nonce_info.clone()); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entries.push(test_entry); + } + + for test_entry in &mut test_entries { + test_entry.add_initial_program(program_name); + + if enable_fee_only_transactions { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + } + + // batch 5: + // * a successful blockhash transaction that closes the nonce + // * a nonce transaction that uses the nonce; this transaction must be dropped + // * a successful blockhash noop transaction that touches the nonce, convenience to see state update + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[withdraw_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + test_entry.push_transaction(Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes( + program_id, + &[], + vec![AccountMeta::new_readonly(nonce_pubkey, false)], + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + )); + + test_entry + .increase_expected_lamports(&fee_payer, LAMPORTS_PER_SOL - LAMPORTS_PER_SIGNATURE); + + test_entry.update_expected_account_data(nonce_pubkey, &AccountSharedData::default()); + + test_entries.push(test_entry); + } + + // batch 6: + // * a successful blockhash transaction that closes the nonce + // * a successful blockhash transaction that funds the closed account + // * a nonce transaction that uses the account; this transaction must be dropped + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[withdraw_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + let middle_transaction = system_transaction::transfer( + &fee_payer_keypair, + &nonce_pubkey, + LAMPORTS_PER_SOL, + Hash::default(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_transaction(middle_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + let mut new_nonce_state = AccountSharedData::default(); + new_nonce_state.set_lamports(LAMPORTS_PER_SOL); + + test_entry.update_expected_account_data(nonce_pubkey, &new_nonce_state); + + test_entries.push(test_entry); + } + + // batch 7: + // * a successful blockhash transaction that closes the nonce + // * a successful blockhash transaction that reopens the account with proper nonce size + // * a nonce transaction that uses the account; this transaction must be dropped + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[withdraw_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + let middle_transaction = system_transaction::create_account( + &fee_payer_keypair, + &non_fee_nonce_keypair, + Hash::default(), + LAMPORTS_PER_SOL, + nonce_size as u64, + &system_program::id(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_transaction(middle_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); + + let new_nonce_state = AccountSharedData::create( + LAMPORTS_PER_SOL, + vec![0; nonce_size], + system_program::id(), + false, + u64::MAX, + ); + + test_entry.update_expected_account_data(nonce_pubkey, &new_nonce_state); + + test_entries.push(test_entry); + } + + // batch 8: + // * a successful blockhash transaction that closes the nonce + // * a successful blockhash transaction that reopens the nonce + // * a nonce transaction that uses the nonce; this transaction must be dropped + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let first_transaction = Transaction::new_signed_with_payer( + &[withdraw_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + let create_instructions = system_instruction::create_nonce_account( + &fee_payer, + &nonce_pubkey, + &fee_payer, + LAMPORTS_PER_SOL, + ); + + let middle_transaction = Transaction::new_signed_with_payer( + &create_instructions, + Some(&fee_payer), + &[&fee_payer_keypair, &non_fee_nonce_keypair], + Hash::default(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_transaction(middle_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); + + test_entries.push(test_entry); + } + + // batch 9: + // * a successful blockhash noop transaction + // * a nonce transaction that uses a spoofed nonce account; this transaction must be dropped + // check_age would never let such a transaction through validation + // this simulates the case where someone closes a nonce account, then reuses the address in the same batch + // but as a non-system account that parses as an initialized nonce account + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + test_entry.initial_accounts.remove(&nonce_pubkey); + test_entry.final_accounts.remove(&nonce_pubkey); + + let mut fake_nonce_account = initial_nonce_account.clone(); + fake_nonce_account.set_rent_epoch(u64::MAX); + fake_nonce_account.set_owner(Pubkey::new_unique()); + test_entry.add_initial_account(nonce_pubkey, &fake_nonce_account); + + let first_transaction = Transaction::new_signed_with_payer( + &[successful_noop_instruction.clone()], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + test_entries.push(test_entry); + } + + for test_entry in &mut test_entries { + test_entry.add_initial_program(program_name); + + if enable_fee_only_transactions { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + } + + test_entries +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum WriteProgramInstruction { Print, @@ -1094,7 +1780,7 @@ enum WriteProgramInstruction { Realloc(usize), } impl WriteProgramInstruction { - fn _create_transaction( + fn create_transaction( self, program_id: Pubkey, fee_payer: &Keypair, @@ -1139,6 +1825,339 @@ impl WriteProgramInstruction { } } +fn account_deallocate() -> Vec { + let mut test_entries = vec![]; + + // batch 0: sanity check, the program actually sets data + // batch 1: removing lamports from account hides it from subsequent in-batch transactions + for remove_lamports in [false, true] { + let mut test_entry = SvmTestEntry::default(); + + let program_name = "write-to-account"; + let program_id = program_address(program_name); + test_entry.add_initial_program(program_name); + + let fee_payer_keypair = Keypair::new(); + let fee_payer = fee_payer_keypair.pubkey(); + + let mut fee_payer_data = AccountSharedData::default(); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + test_entry.add_initial_account(fee_payer, &fee_payer_data); + + let target = Pubkey::new_unique(); + + let mut target_data = AccountSharedData::create( + Rent::default().minimum_balance(1), + vec![0], + program_id, + false, + u64::MAX, + ); + test_entry.add_initial_account(target, &target_data); + + let set_data_transaction = WriteProgramInstruction::Set.create_transaction( + program_id, + &fee_payer_keypair, + target, + None, + ); + test_entry.push_transaction(set_data_transaction); + + target_data.data_as_mut_slice()[0] = 100; + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + test_entry.update_expected_account_data(target, &target_data); + + if remove_lamports { + let dealloc_transaction = WriteProgramInstruction::Dealloc.create_transaction( + program_id, + &fee_payer_keypair, + target, + None, + ); + test_entry.push_transaction(dealloc_transaction); + + let print_transaction = WriteProgramInstruction::Print.create_transaction( + program_id, + &fee_payer_keypair, + target, + None, + ); + test_entry.push_transaction(print_transaction); + test_entry.transaction_batch[2] + .asserts + .logs + .push("Program log: account size 0".to_string()); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); + + test_entry.update_expected_account_data(target, &AccountSharedData::default()); + } + + test_entries.push(test_entry); + } + + test_entries +} + +fn fee_payer_deallocate(enable_fee_only_transactions: bool) -> Vec { + let mut test_entry = SvmTestEntry::default(); + if enable_fee_only_transactions { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + + let program_name = "hello-solana"; + let real_program_id = program_address(program_name); + test_entry.add_initial_program(program_name); + + // 0/1: a rent-paying fee-payer goes to zero lamports on an executed transaction, the batch sees it as deallocated + // 2/3: the same, except if fee-only transactions are enabled, it goes to zero lamports from a a fee-only transaction + for do_fee_only_transaction in if enable_fee_only_transactions { + vec![false, true] + } else { + vec![false] + } { + let dealloc_fee_payer_keypair = Keypair::new(); + let dealloc_fee_payer = dealloc_fee_payer_keypair.pubkey(); + + let mut dealloc_fee_payer_data = AccountSharedData::default(); + dealloc_fee_payer_data.set_lamports(LAMPORTS_PER_SIGNATURE); + dealloc_fee_payer_data.set_rent_epoch(u64::MAX - 1); + test_entry.add_initial_account(dealloc_fee_payer, &dealloc_fee_payer_data); + + let stable_fee_payer_keypair = Keypair::new(); + let stable_fee_payer = stable_fee_payer_keypair.pubkey(); + + let mut stable_fee_payer_data = AccountSharedData::default(); + stable_fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + test_entry.add_initial_account(stable_fee_payer, &stable_fee_payer_data); + + // transaction which drains a fee-payer + let instruction = Instruction::new_with_bytes( + if do_fee_only_transaction { + Pubkey::new_unique() + } else { + real_program_id + }, + &[], + vec![], + ); + + let transaction = Transaction::new_signed_with_payer( + &[instruction], + Some(&dealloc_fee_payer), + &[&dealloc_fee_payer_keypair], + Hash::default(), + ); + + test_entry.push_transaction_with_status( + transaction, + if do_fee_only_transaction { + ExecutionStatus::ProcessedFailed + } else { + ExecutionStatus::Succeeded + }, + ); + + test_entry.decrease_expected_lamports(&dealloc_fee_payer, LAMPORTS_PER_SIGNATURE); + + // as noted in `account_deallocate()` we must touch the account to see if anything actually happened + let instruction = Instruction::new_with_bytes( + real_program_id, + &[], + vec![AccountMeta::new_readonly(dealloc_fee_payer, false)], + ); + test_entry.push_transaction(Transaction::new_signed_with_payer( + &[instruction], + Some(&stable_fee_payer), + &[&stable_fee_payer_keypair], + Hash::default(), + )); + + test_entry.decrease_expected_lamports(&stable_fee_payer, LAMPORTS_PER_SIGNATURE); + + test_entry.update_expected_account_data(dealloc_fee_payer, &AccountSharedData::default()); + } + + // 4: a rent-paying non-nonce fee-payer goes to zero on a fee-only nonce transaction, the batch sees it as deallocated + // we test in `simple_nonce()` that nonce fee-payers cannot as a rule be brought below rent-exemption + if enable_fee_only_transactions { + let dealloc_fee_payer_keypair = Keypair::new(); + let dealloc_fee_payer = dealloc_fee_payer_keypair.pubkey(); + + let mut dealloc_fee_payer_data = AccountSharedData::default(); + dealloc_fee_payer_data.set_lamports(LAMPORTS_PER_SIGNATURE); + dealloc_fee_payer_data.set_rent_epoch(u64::MAX - 1); + test_entry.add_initial_account(dealloc_fee_payer, &dealloc_fee_payer_data); + + let stable_fee_payer_keypair = Keypair::new(); + let stable_fee_payer = stable_fee_payer_keypair.pubkey(); + + let mut stable_fee_payer_data = AccountSharedData::default(); + stable_fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + test_entry.add_initial_account(stable_fee_payer, &stable_fee_payer_data); + + let nonce_pubkey = Pubkey::new_unique(); + let initial_durable = DurableNonce::from_blockhash(&Hash::new_unique()); + let initial_nonce_data = + nonce::state::Data::new(dealloc_fee_payer, initial_durable, LAMPORTS_PER_SIGNATURE); + let initial_nonce_account = AccountSharedData::new_data( + LAMPORTS_PER_SOL, + &nonce::state::Versions::new(nonce::State::Initialized(initial_nonce_data.clone())), + &system_program::id(), + ) + .unwrap(); + let initial_nonce_info = NonceInfo::new(nonce_pubkey, initial_nonce_account.clone()); + + let advanced_durable = DurableNonce::from_blockhash(&LAST_BLOCKHASH); + let mut advanced_nonce_info = initial_nonce_info.clone(); + advanced_nonce_info + .try_advance_nonce(advanced_durable, LAMPORTS_PER_SIGNATURE) + .unwrap(); + + let advance_instruction = + system_instruction::advance_nonce_account(&nonce_pubkey, &dealloc_fee_payer); + let fee_only_noop_instruction = + Instruction::new_with_bytes(Pubkey::new_unique(), &[], vec![]); + + // fee-only nonce transaction which drains a fee-payer + let transaction = Transaction::new_signed_with_payer( + &[advance_instruction, fee_only_noop_instruction], + Some(&dealloc_fee_payer), + &[&dealloc_fee_payer_keypair], + Hash::default(), + ); + test_entry.push_transaction_with_status(transaction, ExecutionStatus::ProcessedFailed); + + test_entry.decrease_expected_lamports(&dealloc_fee_payer, LAMPORTS_PER_SIGNATURE); + + // as noted in `account_deallocate()` we must touch the account to see if anything actually happened + let instruction = Instruction::new_with_bytes( + real_program_id, + &[], + vec![AccountMeta::new_readonly(dealloc_fee_payer, false)], + ); + test_entry.push_transaction(Transaction::new_signed_with_payer( + &[instruction], + Some(&stable_fee_payer), + &[&stable_fee_payer_keypair], + Hash::default(), + )); + + test_entry.decrease_expected_lamports(&stable_fee_payer, LAMPORTS_PER_SIGNATURE); + + test_entry.update_expected_account_data(dealloc_fee_payer, &AccountSharedData::default()); + } + + vec![test_entry] +} + +fn account_reallocate(enable_fee_only_transactions: bool) -> Vec { + let mut test_entries = vec![]; + + let program_name = "write-to-account"; + let program_id = program_address(program_name); + let program_size = program_data_size(program_name); + + let mut common_test_entry = SvmTestEntry::default(); + + let fee_payer_keypair = Keypair::new(); + let fee_payer = fee_payer_keypair.pubkey(); + + let mut fee_payer_data = AccountSharedData::default(); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + common_test_entry.add_initial_account(fee_payer, &fee_payer_data); + + let mk_target = |size| { + AccountSharedData::create( + LAMPORTS_PER_SOL * 10, + vec![0; size], + program_id, + false, + u64::MAX, + ) + }; + + let target = Pubkey::new_unique(); + let target_start_size = 100; + common_test_entry.add_initial_account(target, &mk_target(target_start_size)); + + let print_transaction = WriteProgramInstruction::Print.create_transaction( + program_id, + &fee_payer_keypair, + target, + Some( + (program_size + MAX_PERMITTED_DATA_INCREASE) + .try_into() + .unwrap(), + ), + ); + + common_test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); + + let common_test_entry = common_test_entry; + + // batch 0/1: + // * successful realloc up/down + // * change reflected in same batch + for new_target_size in [target_start_size + 1, target_start_size - 1] { + let mut test_entry = common_test_entry.clone(); + + let realloc_transaction = WriteProgramInstruction::Realloc(new_target_size) + .create_transaction(program_id, &fee_payer_keypair, target, None); + test_entry.push_transaction(realloc_transaction); + + test_entry.push_transaction(print_transaction.clone()); + test_entry.transaction_batch[1] + .asserts + .logs + .push(format!("Program log: account size {}", new_target_size)); + + test_entry.update_expected_account_data(target, &mk_target(new_target_size)); + + test_entries.push(test_entry); + } + + // batch 2: + // * successful large realloc up + // * transaction is aborted based on the new transaction data size post-realloc + { + let mut test_entry = common_test_entry.clone(); + + let new_target_size = target_start_size + MAX_PERMITTED_DATA_INCREASE; + let expected_print_status = if enable_fee_only_transactions { + ExecutionStatus::ProcessedFailed + } else { + test_entry.increase_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + ExecutionStatus::Discarded + }; + + let realloc_transaction = WriteProgramInstruction::Realloc(new_target_size) + .create_transaction(program_id, &fee_payer_keypair, target, None); + test_entry.push_transaction(realloc_transaction); + + test_entry.push_transaction_with_status(print_transaction.clone(), expected_print_status); + + test_entry.update_expected_account_data(target, &mk_target(new_target_size)); + + test_entries.push(test_entry); + } + + for test_entry in &mut test_entries { + test_entry.add_initial_program(program_name); + + if enable_fee_only_transactions { + test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } + } + + test_entries +} + #[test_case(program_medley())] #[test_case(simple_transfer(false))] #[test_case(simple_transfer(true))] @@ -1146,6 +2165,17 @@ impl WriteProgramInstruction { #[test_case(simple_nonce(true, false))] #[test_case(simple_nonce(false, true))] #[test_case(simple_nonce(true, true))] +#[test_case(intrabatch_account_reuse(false))] +#[test_case(intrabatch_account_reuse(true))] +#[test_case(nonce_reuse(false, false))] +#[test_case(nonce_reuse(true, false))] +#[test_case(nonce_reuse(false, true))] +#[test_case(nonce_reuse(true, true))] +#[test_case(account_deallocate())] +#[test_case(fee_payer_deallocate(false))] +#[test_case(fee_payer_deallocate(true))] +#[test_case(account_reallocate(false))] +#[test_case(account_reallocate(true))] fn svm_integration(test_entries: Vec) { for test_entry in test_entries { let env = SvmTestEnvironment::create(test_entry); From 2bc8322fd4ecf5858e8ae3842d2f77c4b757ea59 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 15 Oct 2024 08:57:06 -0700 Subject: [PATCH 5/7] more integration test work --- svm/tests/integration_test.rs | 131 ++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 7 deletions(-) diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 96adbb8975e99e..6242aad7562f9f 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -2062,6 +2062,12 @@ fn account_reallocate(enable_fee_only_transactions: bool) -> Vec { let program_size = program_data_size(program_name); let mut common_test_entry = SvmTestEntry::default(); + common_test_entry.add_initial_program(program_name); + if enable_fee_only_transactions { + common_test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } let fee_payer_keypair = Keypair::new(); let fee_payer = fee_payer_keypair.pubkey(); @@ -2145,14 +2151,124 @@ fn account_reallocate(enable_fee_only_transactions: bool) -> Vec { test_entries.push(test_entry); } - for test_entry in &mut test_entries { - test_entry.add_initial_program(program_name); + test_entries +} - if enable_fee_only_transactions { - test_entry - .enabled_features - .push(feature_set::enable_transaction_loading_failure_fees::id()); - } +fn rent_delinquent() -> Vec { + let mut test_entries = vec![]; + let mut common_test_entry = SvmTestEntry::default(); + + let program_name = "write-to-account"; + let program_id = program_address(program_name); + common_test_entry.add_initial_program(program_name); + + let fee_payer_keypair = Keypair::new(); + let fee_payer = fee_payer_keypair.pubkey(); + + let mut fee_payer_data = AccountSharedData::default(); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL * 2); + common_test_entry.add_initial_account(fee_payer, &fee_payer_data); + + let target = Pubkey::new_unique(); + + let target_data_before = AccountSharedData::create(0, vec![1], program_id, false, 0); + common_test_entry.add_initial_account(target, &target_data_before); + + let push_expected_size_log = |common_test_entry: &mut SvmTestEntry, expected_size| { + common_test_entry + .transaction_batch + .last_mut() + .unwrap() + .asserts + .logs + .push(format!("Program log: account size {}", expected_size)); + }; + + let print_transaction_read_only = WriteProgramInstruction::Print.create_transaction( + program_id, + &fee_payer_keypair, + target, + None, + ); + + // read-only rent-delinquent account exists + common_test_entry.push_transaction(print_transaction_read_only.clone()); + push_expected_size_log(&mut common_test_entry, 1); + + // account was not shadowed or deallocated by previous transaction + common_test_entry.push_transaction(print_transaction_read_only.clone()); + push_expected_size_log(&mut common_test_entry, 1); + + // transfer that fails and uses the rent-delinquent account as writable + common_test_entry.push_transaction_with_status( + system_transaction::transfer( + &fee_payer_keypair, + &target, + LAMPORTS_PER_SOL * 10, + Hash::default(), + ), + ExecutionStatus::ExecutedFailed, + ); + + // account was not shadowed or deallocated by previous transaction + common_test_entry.push_transaction(print_transaction_read_only.clone()); + push_expected_size_log(&mut common_test_entry, 1); + + let mut print_transaction_writable = print_transaction_read_only.clone(); + print_transaction_writable + .message + .header + .num_readonly_unsigned_accounts -= 1; + let print_transaction_writable = print_transaction_writable; + + let common_test_entry = common_test_entry; + + // batch 0: + // * all transactions above + // * transfer that brings the account to rent-exemption + // * account has not been deallocated (using the writable print transaction to update rent epoch) + { + let mut test_entry = common_test_entry.clone(); + + test_entry.push_transaction(system_transaction::transfer( + &fee_payer_keypair, + &target, + LAMPORTS_PER_SOL, + Hash::default(), + )); + + test_entry.push_transaction(print_transaction_writable.clone()); + push_expected_size_log(&mut test_entry, 1); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SOL); + test_entry.increase_expected_lamports(&target, LAMPORTS_PER_SOL); + + test_entries.push(test_entry); + } + + // batch 1: + // * all transactions above + // * successful transaction that takes the rent-delinquent account as writable + // * account has been deallocated + { + let mut test_entry = common_test_entry.clone(); + + test_entry.push_transaction(print_transaction_writable); + push_expected_size_log(&mut test_entry, 1); + + test_entry.push_transaction(print_transaction_read_only); + push_expected_size_log(&mut test_entry, 0); + + test_entry.update_expected_account_data(target, &AccountSharedData::default()); + + test_entries.push(test_entry); + } + + for test_entry in &mut test_entries { + test_entry.decrease_expected_lamports( + &fee_payer, + LAMPORTS_PER_SIGNATURE * test_entry.transaction_batch.len() as u64, + ); } test_entries @@ -2176,6 +2292,7 @@ fn account_reallocate(enable_fee_only_transactions: bool) -> Vec { #[test_case(fee_payer_deallocate(true))] #[test_case(account_reallocate(false))] #[test_case(account_reallocate(true))] +#[test_case(rent_delinquent())] fn svm_integration(test_entries: Vec) { for test_entry in test_entries { let env = SvmTestEnvironment::create(test_entry); From 9092569eb6cb039b196239f9973cf89a4fe34a95 Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Wed, 16 Oct 2024 03:59:12 -0700 Subject: [PATCH 6/7] improve loader cache size tests --- svm/src/account_loader.rs | 139 ++++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 49 deletions(-) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 40ece47fe57c55..b42c884bdea0ad 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -773,7 +773,7 @@ mod tests { std::{borrow::Cow, cell::RefCell, collections::HashMap, sync::Arc}, }; - #[derive(Default)] + #[derive(Clone, Default)] struct TestCallbacks { accounts_map: HashMap, #[allow(clippy::type_complexity)] @@ -2796,72 +2796,113 @@ mod tests { ); // edge cases involving program cache + + // we set up a clean loader first + // the account loader checks its own internal cache prior to the program cache + // this means we must test every case against a loader that has, and has not, seen the relevant account + // NOTE when program cache usage is fixed, we will go to the program cache first + // so this will no longer be necessary + drop(account_loader); + let mut clean_loader: AccountLoader<_> = (&mock_bank).into(); let program2_entry = ProgramCacheEntry { account_size: (program2_size + programdata2_size) as usize, account_owner: ProgramCacheEntryOwner::LoaderV3, ..ProgramCacheEntry::default() }; - account_loader + clean_loader .program_cache .replenish(program2, Arc::new(program2_entry)); - // normal invocation uses the combined cache size - let ixns = vec![Instruction::new_with_bytes(program2, &[], vec![])]; - test_data_size( - &mut account_loader, - ixns, - program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size, + let mut cache_test_txs = vec![]; + + // normal program invocation uses the combined cache size + let normal_tx = SanitizedTransaction::from_transaction_for_tests( + Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes(program2, &[], vec![])], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ), ); + let normal_tx_size = + program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size; + cache_test_txs.push((normal_tx, normal_tx_size)); + + // program as writable, non-instruction account, bypasses the cache + let writable_tx = + new_unchecked_sanitized_transaction_with_writable_program(program2, fee_payer); + let writable_tx_size = program2_size + upgradeable_loader_size + fee_payer_size; + cache_test_txs.push((writable_tx, writable_tx_size)); // program as instruction account bypasses the cache - let ixns = vec![Instruction::new_with_bytes( - program2, - &[], - vec![account_meta(program2, false)], - )]; - test_data_size( - &mut account_loader, - ixns, - program2_size + upgradeable_loader_size + fee_payer_size, + let instruction_tx = SanitizedTransaction::from_transaction_for_tests( + Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes( + program2, + &[], + vec![account_meta(program2, false)], + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ), ); + let instruction_tx_size = program2_size + upgradeable_loader_size + fee_payer_size; + cache_test_txs.push((instruction_tx, instruction_tx_size)); // programdata as instruction account double-counts it - let ixns = vec![Instruction::new_with_bytes( - program2, - &[], - vec![account_meta(programdata2, false)], - )]; - test_data_size( - &mut account_loader, - ixns, - program2_size + programdata2_size * 2 + upgradeable_loader_size + fee_payer_size, + let programdata_tx = SanitizedTransaction::from_transaction_for_tests( + Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes( + program2, + &[], + vec![account_meta(programdata2, false)], + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ), ); - - // both as instruction accounts, for completeness - let ixns = vec![Instruction::new_with_bytes( - program2, - &[], - vec![ - account_meta(program2, false), - account_meta(programdata2, false), - ], - )]; - test_data_size( - &mut account_loader, - ixns, - program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size, + let programdata_tx_size = + program2_size + programdata2_size * 2 + upgradeable_loader_size + fee_payer_size; + cache_test_txs.push((programdata_tx, programdata_tx_size)); + + // program and programdata as instruction accounts + // loading one account can never affect the loading of a different account + // we only include it for completeness + let double_instruction_tx = SanitizedTransaction::from_transaction_for_tests( + Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes( + program2, + &[], + vec![ + account_meta(program2, false), + account_meta(programdata2, false), + ], + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ), ); + let double_instruction_tx_size = + program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size; + cache_test_txs.push((double_instruction_tx, double_instruction_tx_size)); - // writable program bypasses the cache - let tx = new_unchecked_sanitized_transaction_with_writable_program(program2, fee_payer); - test_transaction_data_size( - &mut account_loader, - tx, - program2_size + upgradeable_loader_size + fee_payer_size, - ); + // first test all cases against a clean loader + for (tx, size) in cache_test_txs.clone() { + test_transaction_data_size(&mut clean_loader.clone(), tx, size); + } - // NOTE for the new loader we *must* also test arbitrary permutations of the cache transactions - // to ensure that the batched loading is overridden on a tx-per-tx basis + // next test all cases against a dirty loader + // we loop twice to ensure every transaction runs against a fully populated internal cache + // this ensures there is no difference in behavior regardless of where the account is loaded from + let mut dirty_loader = clean_loader.clone(); + for _ in 0..1 { + for (tx, size) in cache_test_txs.clone() { + test_transaction_data_size(&mut dirty_loader, tx, size); + } + } } } } From 9ec457bb8aca187a788f40e6806b0511d40bfbdc Mon Sep 17 00:00:00 2001 From: hanako mumei <81144685+2501babe@users.noreply.github.com> Date: Tue, 22 Oct 2024 08:30:38 -0700 Subject: [PATCH 7/7] more tests for code review --- svm/src/account_loader.rs | 73 +++-- svm/src/transaction_processor.rs | 115 ++++---- svm/tests/concurrent_tests.rs | 1 + svm/tests/integration_test.rs | 445 ++++++++++++++++++++++++++----- 4 files changed, 489 insertions(+), 145 deletions(-) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index b42c884bdea0ad..2e0e3c4ba247b9 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -771,6 +771,7 @@ mod tests { transaction_context::{TransactionAccount, TransactionContext}, }, std::{borrow::Cow, cell::RefCell, collections::HashMap, sync::Arc}, + test_case::test_matrix, }; #[derive(Clone, Default)] @@ -810,7 +811,12 @@ mod tests { impl<'a> From<&'a TestCallbacks> for AccountLoader<'a, TestCallbacks> { fn from(callbacks: &'a TestCallbacks) -> AccountLoader<'a, TestCallbacks> { - AccountLoader::new(callbacks, None, ProgramCacheForTxBatch::default()) + AccountLoader::new_with_account_cache_capacity( + callbacks, + None, + ProgramCacheForTxBatch::default(), + callbacks.accounts_map.len(), + ) } } @@ -1140,10 +1146,11 @@ mod tests { accounts_map, ..Default::default() }; - let mut account_loader = AccountLoader::new( + let mut account_loader = AccountLoader::new_with_account_cache_capacity( &callbacks, account_overrides, ProgramCacheForTxBatch::default(), + accounts.len(), ); load_transaction( &mut account_loader, @@ -1552,7 +1559,8 @@ mod tests { let mut loaded_programs = ProgramCacheForTxBatch::default(); loaded_programs.replenish(key2.pubkey(), Arc::new(ProgramCacheEntry::default())); - let mut account_loader = AccountLoader::new(&mock_bank, None, loaded_programs); + let mut account_loader = + AccountLoader::new_with_account_cache_capacity(&mock_bank, None, loaded_programs, 1); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1572,15 +1580,21 @@ mod tests { assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); } - #[test] - fn test_load_transaction_accounts_program_account_executable_bypass() { - // currently, the account loader retrieves read-only non-instruction accounts from the program cache - // it creates a mock AccountSharedData with the executable flag set to true - // however, it does not check whether these accounts are actually executable before doing so - // this affects consensus: a transaction that uses a cached non-executable program executes and fails - // but if the transaction gets the program from accounts-db, it will be dropped during account loading - // this test enforces the current behavior, so that future account loader changes do not break consensus - + // currently, the account loader retrieves read-only non-instruction accounts from the program cache + // it creates a mock AccountSharedData with the executable flag set to true + // however, it does not check whether these accounts are actually executable before doing so + // this affects consensus: a transaction that uses a cached non-executable program executes and fails + // but if the transaction gets the program from accounts-db, it will be dropped during account loading + // this test enforces the current behavior, so that future account loader changes do not break consensus + // + // account cache has its own code path that accesses program cache, so we test hit and miss + // we also test bpf_loader and bpf_loader_upgradeable, because accounts owned by the latter can behave strangely + // all cases should produce the same results + #[test_matrix([bpf_loader::id(), bpf_loader_upgradeable::id()], [false, true])] + fn test_load_transaction_accounts_program_account_executable_bypass( + program_owner: Pubkey, + clear_account_cache: bool, + ) { let mut mock_bank = TestCallbacks::default(); let account_keypair = Keypair::new(); let program_keypair = Keypair::new(); @@ -1593,7 +1607,7 @@ mod tests { let mut program_data = AccountSharedData::default(); program_data.set_lamports(200); - program_data.set_owner(bpf_loader::id()); + program_data.set_owner(program_owner); mock_bank .accounts_map .insert(program_keypair.pubkey(), program_data); @@ -1604,7 +1618,7 @@ mod tests { loader_data.set_owner(native_loader::id()); mock_bank .accounts_map - .insert(bpf_loader::id(), loader_data.clone()); + .insert(program_owner, loader_data.clone()); mock_bank .accounts_map .insert(native_loader::id(), loader_data); @@ -1649,6 +1663,10 @@ mod tests { Arc::new(ProgramCacheEntry::default()), ); + if clear_account_cache { + account_loader.account_cache = AHashMap::default(); + } + let result = load_transaction_accounts( &mut account_loader, transaction.message(), @@ -1693,6 +1711,10 @@ mod tests { Hash::default(), )); + if clear_account_cache { + account_loader.account_cache = AHashMap::default(); + } + let result = load_transaction_accounts( &mut account_loader, transaction.message(), @@ -1716,6 +1738,10 @@ mod tests { account_keypair.pubkey(), ); + if clear_account_cache { + account_loader.account_cache = AHashMap::default(); + } + let result = load_transaction_accounts( &mut account_loader, transaction.message(), @@ -2460,6 +2486,23 @@ mod tests { assert_eq!(account.lamports(), 0); } + #[test] + fn test_load_account_dropped() { + let mock_bank = TestCallbacks::default(); + let mut account_loader: AccountLoader = (&mock_bank).into(); + + let address = Pubkey::new_unique(); + let account = AccountSharedData::default(); + let cache_item = AccountCacheItem { + account, + inspected_as_writable: false, + }; + account_loader.account_cache.insert(address, cache_item); + + let result = account_loader.load_account(&address, AccountUsagePattern::ReadOnlyInvisible); + assert!(result.is_none()); + } + // Ensure `TransactionProcessingCallback::inspect_account()` is called when // loading accounts for transaction processing. #[test] @@ -2898,7 +2941,7 @@ mod tests { // we loop twice to ensure every transaction runs against a fully populated internal cache // this ensures there is no difference in behavior regardless of where the account is loaded from let mut dirty_loader = clean_loader.clone(); - for _ in 0..1 { + for _ in 0..=1 { for (tx, size) in cache_test_txs.clone() { test_transaction_data_size(&mut dirty_loader, tx, size); } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 4743dd915c3f06..6d024d170e39ca 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1203,7 +1203,13 @@ mod tests { impl<'a> From<&'a MockBankCallback> for AccountLoader<'a, MockBankCallback> { fn from(callbacks: &'a MockBankCallback) -> AccountLoader<'a, MockBankCallback> { - AccountLoader::new(callbacks, None, ProgramCacheForTxBatch::default()) + let capacity = callbacks.account_shared_data.read().unwrap().len(); + AccountLoader::new_with_account_cache_capacity( + callbacks, + None, + ProgramCacheForTxBatch::default(), + capacity, + ) } } @@ -2043,15 +2049,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2122,15 +2126,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2174,15 +2176,13 @@ mod tests { let mock_bank = MockBankCallback::default(); let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2209,15 +2209,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2248,15 +2246,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2285,15 +2281,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2318,15 +2312,13 @@ mod tests { let mock_bank = MockBankCallback::default(); let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &RentCollector::default(), @@ -2364,9 +2356,11 @@ mod tests { { let fee_payer_account = AccountSharedData::new_data( min_balance + transaction_fee + priority_fee, - &nonce::state::Versions::new(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new(nonce::State::Initialized(nonce::state::Data::new( + *fee_payer_address, + DurableNonce::default(), + lamports_per_signature, + ))), &system_program::id(), ) .unwrap(); @@ -2380,20 +2374,33 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); + let environment_blockhash = Hash::new_unique(); + let next_durable_nonce = DurableNonce::from_blockhash(&environment_blockhash); let mut future_nonce = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); - future_nonce.try_advance_nonce(durable_nonce, 0).unwrap(); + future_nonce + .try_advance_nonce(next_durable_nonce, lamports_per_signature) + .unwrap(); + + let tx_details = CheckedTransactionDetails { + nonce: Some(future_nonce.clone()), + lamports_per_signature, + }; - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_nonce( &mut account_loader, &message, - CheckedTransactionDetails { - nonce: Some(future_nonce.clone()), - lamports_per_signature, - }, - &durable_nonce, + &tx_details, + &environment_blockhash, + &mut error_counters, + ); + + assert_eq!(result, Ok(())); + + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( + &mut account_loader, + &message, + tx_details, &feature_set, &FeeStructure::default(), &rent_collector, @@ -2448,15 +2455,13 @@ mod tests { let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &feature_set, &FeeStructure::default(), &rent_collector, @@ -2499,23 +2504,22 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; - let mut account_loader = AccountLoader::new( + let mut account_loader = AccountLoader::new_with_account_cache_capacity( &mock_bank, Some(&account_overrides), ProgramCacheForTxBatch::default(), + 2, ); let mut error_counters = TransactionErrorMetrics::default(); - let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.validate_transaction_fee_payer( + let result = TransactionBatchProcessor::::validate_transaction_fee_payer( &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &DurableNonce::default(), &FeatureSet::default(), &FeeStructure::default(), &rent_collector, @@ -2555,22 +2559,19 @@ mod tests { Some(&fee_payer_address), &Hash::new_unique(), )); - let batch_processor = TransactionBatchProcessor::::default(); - batch_processor - .validate_transaction_fee_payer( - &mut account_loader, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature: 5000, - }, - &DurableNonce::default(), - &FeatureSet::default(), - &FeeStructure::default(), - &RentCollector::default(), - &mut TransactionErrorMetrics::default(), - ) - .unwrap(); + TransactionBatchProcessor::::validate_transaction_fee_payer( + &mut account_loader, + &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 diff --git a/svm/tests/concurrent_tests.rs b/svm/tests/concurrent_tests.rs index 4d0a500b845bc7..a76f2f692fab65 100644 --- a/svm/tests/concurrent_tests.rs +++ b/svm/tests/concurrent_tests.rs @@ -152,6 +152,7 @@ fn svm_concurrent() { let mut check_data = vec![Vec::new(); THREADS]; let read_account = Pubkey::new_unique(); let mut account_data = AccountSharedData::default(); + account_data.set_lamports(BALANCE); account_data.set_data(AMOUNT.to_le_bytes().to_vec()); mock_bank .account_shared_data diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 6242aad7562f9f..a65cc79743ae91 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -8,13 +8,15 @@ use { EXECUTION_SLOT, WALLCLOCK_TIME, }, solana_sdk::{ - account::{AccountSharedData, ReadableAccount, WritableAccount}, + account::{AccountSharedData, ReadableAccount, WritableAccount, PROGRAM_OWNERS}, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Slot, compute_budget::ComputeBudgetInstruction, entrypoint::MAX_PERMITTED_DATA_INCREASE, feature_set::{self, FeatureSet}, hash::Hash, instruction::{AccountMeta, Instruction}, + native_loader, native_token::LAMPORTS_PER_SOL, nonce::{self, state::DurableNonce}, pubkey::Pubkey, @@ -151,7 +153,12 @@ impl SvmTestEnvironment<'_> { for (pubkey, account_data) in executed_transaction.loaded_transaction.accounts.clone() { - final_accounts_actual.insert(pubkey, account_data); + // by excluding executable, we skip program cache dummy accounts + // the real account loader is smarter and skips read-only accounts + // this can improved if complex multi-batch tests require it in the future + if !account_data.executable() { + final_accounts_actual.insert(pubkey, account_data); + } } } Ok(ProcessedTransaction::FeesOnly(fees_only_transaction)) => { @@ -219,8 +226,9 @@ impl SvmTestEnvironment<'_> { } } + // merge new account states into the bank for multi-batch tests let mut mock_bank_accounts = self.mock_bank.account_shared_data.write().unwrap(); - *mock_bank_accounts = final_accounts_actual; + mock_bank_accounts.extend(final_accounts_actual); } } @@ -1759,6 +1767,95 @@ fn nonce_reuse(enable_fee_only_transactions: bool, fee_paying_nonce: bool) -> Ve test_entries.push(test_entry); } + // batch 10: + // * a successful blockhash transaction that changes the nonce authority + // * a nonce transaction that uses the nonce with the old authority; this transaction must be dropped + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let new_authority = Pubkey::new_unique(); + + let first_transaction = Transaction::new_signed_with_payer( + &[system_instruction::authorize_nonce_account( + &nonce_pubkey, + &fee_payer, + &new_authority, + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_nonce_transaction_with_status( + second_transaction.clone(), + advanced_nonce_info.clone(), + ExecutionStatus::Discarded, + ); + + let final_nonce_data = + nonce::state::Data::new(new_authority, initial_durable, LAMPORTS_PER_SIGNATURE); + let final_nonce_account = AccountSharedData::new_data( + LAMPORTS_PER_SOL, + &nonce::state::Versions::new(nonce::State::Initialized(final_nonce_data)), + &system_program::id(), + ) + .unwrap(); + + test_entry.update_expected_account_data(nonce_pubkey, &final_nonce_account); + + test_entries.push(test_entry); + } + + // batch 11: + // * a successful blockhash transaction that changes the nonce authority + // * a nonce transaction that uses the nonce with the new authority; this transaction succeeds + if !fee_paying_nonce { + let mut test_entry = common_test_entry.clone(); + + let new_authority_keypair = Keypair::new(); + let new_authority = new_authority_keypair.pubkey(); + + let first_transaction = Transaction::new_signed_with_payer( + &[system_instruction::authorize_nonce_account( + &nonce_pubkey, + &fee_payer, + &new_authority, + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); + + let second_transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::advance_nonce_account(&nonce_pubkey, &new_authority), + successful_noop_instruction.clone(), + ], + Some(&fee_payer), + &[&fee_payer_keypair, &new_authority_keypair], + *advanced_durable.as_hash(), + ); + + test_entry.push_transaction(first_transaction); + test_entry.push_nonce_transaction(second_transaction.clone(), advanced_nonce_info.clone()); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); + + let final_nonce_data = + nonce::state::Data::new(new_authority, advanced_durable, LAMPORTS_PER_SIGNATURE); + let final_nonce_account = AccountSharedData::new_data( + LAMPORTS_PER_SOL, + &nonce::state::Versions::new(nonce::State::Initialized(final_nonce_data)), + &system_program::id(), + ) + .unwrap(); + + test_entry.update_expected_account_data(nonce_pubkey, &final_nonce_account); + + test_entries.push(test_entry); + } + for test_entry in &mut test_entries { test_entry.add_initial_program(program_name); @@ -2154,9 +2251,14 @@ fn account_reallocate(enable_fee_only_transactions: bool) -> Vec { test_entries } -fn rent_delinquent() -> Vec { +fn bpf_loader_buffer(enable_fee_only_transactions: bool) -> Vec { let mut test_entries = vec![]; let mut common_test_entry = SvmTestEntry::default(); + if enable_fee_only_transactions { + common_test_entry + .enabled_features + .push(feature_set::enable_transaction_loading_failure_fees::id()); + } let program_name = "write-to-account"; let program_id = program_address(program_name); @@ -2166,109 +2268,305 @@ fn rent_delinquent() -> Vec { let fee_payer = fee_payer_keypair.pubkey(); let mut fee_payer_data = AccountSharedData::default(); - fee_payer_data.set_lamports(LAMPORTS_PER_SOL * 2); + fee_payer_data.set_lamports(LAMPORTS_PER_SOL); + fee_payer_data.set_rent_epoch(u64::MAX); common_test_entry.add_initial_account(fee_payer, &fee_payer_data); - let target = Pubkey::new_unique(); + let buffer_keypair = Keypair::new(); + let buffer = buffer_keypair.pubkey(); - let target_data_before = AccountSharedData::create(0, vec![1], program_id, false, 0); - common_test_entry.add_initial_account(target, &target_data_before); + let mut buffer_bytes = vec![0; MAX_PERMITTED_DATA_INCREASE]; + bincode::serialize_into( + &mut buffer_bytes, + &UpgradeableLoaderState::Buffer { + authority_address: Some(fee_payer), + }, + ) + .unwrap(); - let push_expected_size_log = |common_test_entry: &mut SvmTestEntry, expected_size| { - common_test_entry - .transaction_batch - .last_mut() - .unwrap() - .asserts - .logs - .push(format!("Program log: account size {}", expected_size)); - }; + // this is used to test non-buffer accounts for all loaders + let mut throwaway_account_owners = vec![None]; + for program_owner in PROGRAM_OWNERS { + throwaway_account_owners.push(Some(*program_owner)); + } - let print_transaction_read_only = WriteProgramInstruction::Print.create_transaction( - program_id, - &fee_payer_keypair, - target, - None, + let initial_buffer_data = AccountSharedData::create( + LAMPORTS_PER_SOL, + buffer_bytes.clone(), + bpf_loader_upgradeable::id(), + false, + u64::MAX, ); + common_test_entry.add_initial_account(buffer, &initial_buffer_data); - // read-only rent-delinquent account exists - common_test_entry.push_transaction(print_transaction_read_only.clone()); - push_expected_size_log(&mut common_test_entry, 1); - - // account was not shadowed or deallocated by previous transaction - common_test_entry.push_transaction(print_transaction_read_only.clone()); - push_expected_size_log(&mut common_test_entry, 1); + // bpf_loader_upgradeable does not reassign closed accounts to system + // so reopening a closed buffer under a new owner can only be done in a subsequent transaction + let close_transaction = Transaction::new_signed_with_payer( + &[bpf_loader_upgradeable::close( + &buffer, &fee_payer, &fee_payer, + )], + Some(&fee_payer), + &[&fee_payer_keypair], + Hash::default(), + ); - // transfer that fails and uses the rent-delinquent account as writable - common_test_entry.push_transaction_with_status( - system_transaction::transfer( - &fee_payer_keypair, - &target, - LAMPORTS_PER_SOL * 10, + // make a transaction to reopen an account with a given size + let reopen = |new_size: usize| { + let reopen_transaction = Transaction::new_signed_with_payer( + &[system_instruction::create_account( + &fee_payer, + &buffer, + LAMPORTS_PER_SOL, + new_size as u64, + &system_program::id(), + )], + Some(&fee_payer), + &[&fee_payer_keypair, &buffer_keypair], Hash::default(), - ), - ExecutionStatus::ExecutedFailed, - ); + ); + + let reopened_buffer_data = AccountSharedData::create( + LAMPORTS_PER_SOL, + vec![0; new_size], + system_program::id(), + false, + u64::MAX, + ); - // account was not shadowed or deallocated by previous transaction - common_test_entry.push_transaction(print_transaction_read_only.clone()); - push_expected_size_log(&mut common_test_entry, 1); + (reopen_transaction, reopened_buffer_data) + }; - let mut print_transaction_writable = print_transaction_read_only.clone(); - print_transaction_writable - .message - .header - .num_readonly_unsigned_accounts -= 1; - let print_transaction_writable = print_transaction_writable; + let print_transaction = WriteProgramInstruction::Print.create_transaction( + program_id, + &fee_payer_keypair, + buffer, + None, + ); let common_test_entry = common_test_entry; // batch 0: - // * all transactions above - // * transfer that brings the account to rent-exemption - // * account has not been deallocated (using the writable print transaction to update rent epoch) + // * check buffer size (must be account size) { let mut test_entry = common_test_entry.clone(); - test_entry.push_transaction(system_transaction::transfer( - &fee_payer_keypair, - &target, - LAMPORTS_PER_SOL, - Hash::default(), - )); + test_entry.push_transaction(print_transaction.clone()); + test_entry.transaction_batch[0] + .asserts + .logs + .push(format!("Program log: account size {}", buffer_bytes.len(),)); + + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + + test_entries.push(test_entry); + } + + // batch 1: + // * close buffer + // * check buffer size (must be zero) + { + let mut test_entry = common_test_entry.clone(); - test_entry.push_transaction(print_transaction_writable.clone()); - push_expected_size_log(&mut test_entry, 1); + test_entry.push_transaction(close_transaction.clone()); + test_entry.push_transaction(print_transaction.clone()); + test_entry.transaction_batch[1] + .asserts + .logs + .push("Program log: account size 0".to_string()); - test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SOL); - test_entry.increase_expected_lamports(&target, LAMPORTS_PER_SOL); + test_entry + .increase_expected_lamports(&fee_payer, LAMPORTS_PER_SOL - LAMPORTS_PER_SIGNATURE * 2); + test_entry.update_expected_account_data(buffer, &AccountSharedData::default()); test_entries.push(test_entry); } // batch 1: - // * all transactions above - // * successful transaction that takes the rent-delinquent account as writable - // * account has been deallocated + // * close buffer + // * reopen as system account of large size + // * check buffer size (must be new size) { let mut test_entry = common_test_entry.clone(); - test_entry.push_transaction(print_transaction_writable); - push_expected_size_log(&mut test_entry, 1); + let (reopen_transaction, reopened_buffer_data) = reopen(MAX_PERMITTED_DATA_INCREASE); - test_entry.push_transaction(print_transaction_read_only); - push_expected_size_log(&mut test_entry, 0); + test_entry.push_transaction(close_transaction.clone()); + test_entry.push_transaction(reopen_transaction); + test_entry.push_transaction(print_transaction); + test_entry.transaction_batch[2].asserts.logs.push(format!( + "Program log: account size {}", + MAX_PERMITTED_DATA_INCREASE, + )); - test_entry.update_expected_account_data(target, &AccountSharedData::default()); + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 4); + test_entry.update_expected_account_data(buffer, &reopened_buffer_data); test_entries.push(test_entry); } - for test_entry in &mut test_entries { - test_entry.decrease_expected_lamports( - &fee_payer, - LAMPORTS_PER_SIGNATURE * test_entry.transaction_batch.len() as u64, + // batch 3/4: + // * close buffer + // * (only batch 3) reopen as large system account + // * include account as invisible and use compute budget to check size + // + // batch 2 is a sanity check that compute budget failure depends only on the system account size + // it succeeds because we create a 1 byte account with a 10kb transaction data size limit + // + // in batch 3 we expect the third transaction to see a 10kb account + // this fails given a 10kb limit because there are some other accounts of negligible size + // this codepath will find it in the accounts cache, then find it in the program cache + // the program cache check normally substitutes a different transaction size + // this is so read-only non-instruction upgradeable program sizes include programdata + // in this case, however, we do *not* use the program cache size (which is always 0) + // because the program cache does not evict entries, even if the account was closed + // + // the test account must be read-only non-instruction, so we append it to account keys + // we use a system program transfer for our no-op to simplify size math because its only 14 bytes + for sanity_check in [true, false] { + let mut test_entry = common_test_entry.clone(); + + let new_account_size = if sanity_check { + 1 + } else { + MAX_PERMITTED_DATA_INCREASE + }; + + let (reopen_transaction, reopened_buffer_data) = reopen(new_account_size); + + let mut noop_transaction = Transaction::new_with_payer( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit( + MAX_PERMITTED_DATA_INCREASE as u32, + ), + system_instruction::transfer(&fee_payer, &fee_payer, 0), + ], + Some(&fee_payer), ); + + noop_transaction.message.account_keys.push(buffer); + noop_transaction + .message + .header + .num_readonly_unsigned_accounts += 1; + noop_transaction.sign(&[&fee_payer_keypair], Hash::default()); + + let expected_status = if sanity_check { + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 4); + ExecutionStatus::Succeeded + } else if enable_fee_only_transactions { + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 4); + ExecutionStatus::ProcessedFailed + } else { + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 3); + ExecutionStatus::Discarded + }; + + test_entry.push_transaction(close_transaction.clone()); + test_entry.push_transaction(reopen_transaction); + test_entry.push_transaction_with_status(noop_transaction, expected_status); + + test_entry.update_expected_account_data(buffer, &reopened_buffer_data); + + test_entries.push(test_entry); + } + + // batch 5: buffer from program cache has zero size + // batch 6: same as above but force it into account cache + // batches 7-14: both of above, with an account owned by each loader + for throwaway_account_owner in throwaway_account_owners { + for transaction_count in 1..=2 { + let mut test_entry = common_test_entry.clone(); + + let target = match throwaway_account_owner { + None => buffer, + Some(loader_id) => { + let target = Pubkey::new_unique(); + let throwaway_account_data = AccountSharedData::create( + LAMPORTS_PER_SOL, + vec![0; MAX_PERMITTED_DATA_INCREASE], + loader_id, + false, + u64::MAX, + ); + test_entry.add_initial_account(target, &throwaway_account_data); + + target + } + }; + + // 10kb budget is a comfortable margin to ensure the calculated size of the target is zero + // if the real account size is used, the program sizes push us over this threshold + let mut noop_transaction = Transaction::new_with_payer( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit( + MAX_PERMITTED_DATA_INCREASE as u32, + ), + system_instruction::transfer(&fee_payer, &fee_payer, 0), + ], + Some(&fee_payer), + ); + + noop_transaction.message.account_keys.push(target); + noop_transaction + .message + .header + .num_readonly_unsigned_accounts += 1; + noop_transaction.sign(&[&fee_payer_keypair], Hash::default()); + + for _ in 0..transaction_count { + test_entry.push_transaction(noop_transaction.clone()); + } + + test_entry + .decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * transaction_count); + + test_entries.push(test_entry); + } + } + + // batch 15: non-builtin accounts owned by NativeLoader are never added to the cache + { + let mut test_entry = common_test_entry.clone(); + + let target = Pubkey::new_unique(); + + let throwaway_account_data = AccountSharedData::create( + LAMPORTS_PER_SOL, + vec![0; MAX_PERMITTED_DATA_INCREASE], + native_loader::id(), + false, + u64::MAX, + ); + + test_entry.add_initial_account(target, &throwaway_account_data); + + let mut noop_transaction = Transaction::new_with_payer( + &[ + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit( + MAX_PERMITTED_DATA_INCREASE as u32 / 2, + ), + system_instruction::transfer(&fee_payer, &fee_payer, 0), + ], + Some(&fee_payer), + ); + + noop_transaction.message.account_keys.push(target); + noop_transaction + .message + .header + .num_readonly_unsigned_accounts += 1; + noop_transaction.sign(&[&fee_payer_keypair], Hash::default()); + + let expected_status = if enable_fee_only_transactions { + test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE); + ExecutionStatus::ProcessedFailed + } else { + ExecutionStatus::Discarded + }; + + test_entry.push_transaction_with_status(noop_transaction, expected_status); + + test_entries.push(test_entry); } test_entries @@ -2292,7 +2590,8 @@ fn rent_delinquent() -> Vec { #[test_case(fee_payer_deallocate(true))] #[test_case(account_reallocate(false))] #[test_case(account_reallocate(true))] -#[test_case(rent_delinquent())] +#[test_case(bpf_loader_buffer(false))] +#[test_case(bpf_loader_buffer(true))] fn svm_integration(test_entries: Vec) { for test_entry in test_entries { let env = SvmTestEnvironment::create(test_entry);