diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index 011e21d4cc6..97c7c1a6eb5 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -581,6 +581,7 @@ mod tests { rollback_accounts: RollbackAccounts::FeePayerOnly { fee_payer: (from_address, from_account_pre.clone()), }, + loaded_accounts_data_size: 0, // unused }, )))]; let max_collected_accounts = max_number_of_accounts_to_collect(&txs, &processing_results); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2ab65f181d4..b9e55f14b4c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3172,7 +3172,7 @@ impl Bank { None, None, 0, - fees_only_tx.rollback_accounts.data_size() as u32, + fees_only_tx.loaded_accounts_data_size, ), }, Err(error) => (vec![], Err(error), None, None, None, 0, 0), diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 814ca766051..6d13b9a9355 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -92,7 +92,7 @@ use { }, solana_stake_program::stake_state, solana_svm::{ - account_loader::{FeesOnlyTransaction, LoadedTransaction}, + account_loader::{FeesOnlyTransaction, LoadedTransaction, TRANSACTION_ACCOUNT_BASE_SIZE}, rollback_accounts::RollbackAccounts, transaction_commit_result::TransactionCommitResultExtensions, transaction_execution_result::ExecutedTransaction, @@ -1872,19 +1872,25 @@ fn test_interleaving_locks() { .is_ok()); } -#[test] -fn test_load_and_execute_commit_transactions_fees_only() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_load_and_execute_commit_transactions_fees_only( + formalize_loaded_transaction_data_size: bool, +) { let GenesisConfigInfo { mut genesis_config, .. } = genesis_utils::create_genesis_config(100 * LAMPORTS_PER_SOL); genesis_config.rent = Rent::default(); genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0); let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - let bank = Bank::new_from_parent( + let mut bank = Bank::new_from_parent( bank, &Pubkey::new_unique(), genesis_config.epoch_schedule.get_first_slot_in_epoch(1), ); + if !formalize_loaded_transaction_data_size { + bank.deactivate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } // Use rent-paying fee payer to show that rent is not collected for fees // only transactions even when they use a rent-paying account. @@ -1937,6 +1943,13 @@ fn test_load_and_execute_commit_transactions_fees_only() { ) .0; + // Loaded account size is correctly calculated via RollbackAccounts + let loaded_accounts_data_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE * 2 + nonce_size + } else { + nonce_size + } as u32; + assert_eq!( commit_results, vec![Ok(CommittedTransaction { @@ -1948,7 +1961,7 @@ fn test_load_and_execute_commit_transactions_fees_only() { fee_details: FeeDetails::new(5000, 0), loaded_account_stats: TransactionLoadedAccountsStats { loaded_accounts_count: 2, - loaded_accounts_data_size: nonce_size as u32, + loaded_accounts_data_size, }, })] ); @@ -11382,6 +11395,7 @@ fn test_filter_program_errors_and_collect_fee_details() { load_error: TransactionError::InvalidProgramForExecution, rollback_accounts: RollbackAccounts::default(), fee_details, + loaded_accounts_data_size: 0, // unused }, ))), new_executed_processing_result( diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index abd9ba46970..d98fec8157f 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -147,6 +147,7 @@ pub struct FeesOnlyTransaction { pub load_error: TransactionError, pub rollback_accounts: RollbackAccounts, pub fee_details: FeeDetails, + pub loaded_accounts_data_size: u32, } // This is an internal SVM type that tracks account changes throughout a @@ -443,11 +444,28 @@ pub(crate) fn load_transaction( compute_budget: tx_details.compute_budget, loaded_accounts_data_size: loaded_tx_accounts.loaded_accounts_data_size, }), - Err(err) => TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: err, - fee_details: tx_details.fee_details, - rollback_accounts: tx_details.rollback_accounts, - }), + Err(err) => { + let formalize_loaded_transaction_data_size = account_loader + .feature_set + .formalize_loaded_transaction_data_size; + + let loaded_accounts_data_size = address_lookup_table_data_size( + message, + formalize_loaded_transaction_data_size, + ) + .saturating_add( + tx_details + .rollback_accounts + .data_size(formalize_loaded_transaction_data_size), + ) as u32; + + TransactionLoadResult::FeesOnly(FeesOnlyTransaction { + load_error: err, + fee_details: tx_details.fee_details, + rollback_accounts: tx_details.rollback_accounts, + loaded_accounts_data_size, + }) + } } } } @@ -485,6 +503,19 @@ impl LoadedTransactionAccounts { } } +fn address_lookup_table_data_size( + message: &impl SVMMessage, + formalize_loaded_transaction_data_size: bool, +) -> usize { + if formalize_loaded_transaction_data_size { + message + .num_lookup_tables() + .saturating_mul(ADDRESS_LOOKUP_TABLE_BASE_SIZE) + } else { + 0 + } +} + fn load_transaction_accounts( account_loader: &mut AccountLoader, message: &impl SVMMessage, @@ -536,9 +567,9 @@ fn load_transaction_accounts_simd186( // Transactions pay a base fee per address lookup table. loaded_transaction_accounts.increase_calculated_data_size( - message - .num_lookup_tables() - .saturating_mul(ADDRESS_LOOKUP_TABLE_BASE_SIZE), + address_lookup_table_data_size( + message, true, /* formalize_loaded_transaction_data_size */ + ), loaded_accounts_bytes_limit, error_metrics, )?; diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index 2fb2ca3837d..063c27bb6fd 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -1,5 +1,5 @@ use { - crate::nonce_info::NonceInfo, + crate::{account_loader::TRANSACTION_ACCOUNT_BASE_SIZE, nonce_info::NonceInfo}, solana_account::{AccountSharedData, ReadableAccount, WritableAccount}, solana_clock::Epoch, solana_pubkey::Pubkey, @@ -127,9 +127,12 @@ impl RollbackAccounts { /// Size of accounts tracked for rollback, used when calculating the actual /// cost of transaction processing in the cost model. - pub fn data_size(&self) -> usize { + pub(crate) fn data_size(&self, formalize_loaded_transaction_data_size: bool) -> usize { let mut total_size: usize = 0; for (_, account) in self.iter() { + if formalize_loaded_transaction_data_size { + total_size = total_size.saturating_add(TRANSACTION_ACCOUNT_BASE_SIZE); + } total_size = total_size.saturating_add(account.data().len()); } total_size diff --git a/svm/src/transaction_processing_result.rs b/svm/src/transaction_processing_result.rs index 76724a67b63..4805f48fea5 100644 --- a/svm/src/transaction_processing_result.rs +++ b/svm/src/transaction_processing_result.rs @@ -94,7 +94,7 @@ impl ProcessedTransaction { pub fn loaded_accounts_data_size(&self) -> u32 { match self { Self::Executed(context) => context.loaded_transaction.loaded_accounts_data_size, - Self::FeesOnly(details) => details.rollback_accounts.data_size() as u32, + Self::FeesOnly(details) => details.loaded_accounts_data_size, } } }