diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index c18a9639129b34..97890f540d1ed4 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -6,7 +6,6 @@ use { transaction_error_metrics::TransactionErrorMetrics, transaction_processing_callback::{AccountState, TransactionProcessingCallback}, }, - itertools::Itertools, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_feature_set::{self as feature_set, FeatureSet}, solana_program_runtime::loaded_programs::ProgramCacheForTxBatch, @@ -30,7 +29,7 @@ use { solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, solana_svm_transaction::svm_message::SVMMessage, solana_system_program::{get_system_account_kind, SystemAccountKind}, - std::{collections::HashMap, num::NonZeroU32}, + std::{collections::HashMap, num::NonZeroU32, sync::Arc}, }; // for the load instructions @@ -53,6 +52,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, @@ -95,6 +95,102 @@ pub struct FeesOnlyTransaction { pub fee_details: FeeDetails, } +#[cfg_attr(feature = "dev-context-only-utils", derive(Clone))] +pub(crate) struct AccountLoader<'a, CB: TransactionProcessingCallback> { + account_overrides: Option<&'a AccountOverrides>, + pub(crate) program_cache: ProgramCacheForTxBatch, + program_accounts: HashMap, + callbacks: &'a CB, + pub(crate) feature_set: Arc, +} +impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> { + pub fn new( + account_overrides: Option<&'a AccountOverrides>, + program_cache: ProgramCacheForTxBatch, + program_accounts: HashMap, + callbacks: &'a CB, + feature_set: Arc, + ) -> AccountLoader<'a, CB> { + Self { + program_cache, + account_overrides, + callbacks, + program_accounts, + feature_set, + } + } + + pub fn load_account( + &mut self, + account_key: &Pubkey, + usage_pattern: AccountUsagePattern, + ) -> Option { + let is_writable = usage_pattern == AccountUsagePattern::Writable; + let is_invisible_read = usage_pattern == AccountUsagePattern::ReadOnlyInvisible; + let use_program_cache = !self + .feature_set + .is_active(&feature_set::disable_account_loader_special_case::id()); + + if let Some(account_override) = self + .account_overrides + .and_then(|overrides| overrides.get(account_key)) + { + Some(LoadedTransactionAccount { + loaded_size: account_override.data().len(), + account: account_override.clone(), + rent_collected: 0, + }) + } else if let Some(program) = (use_program_cache && is_invisible_read) + .then_some(()) + .and_then(|_| self.program_cache.find(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(account_key, &self.program_accounts) + .ok()?, + 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); + + Some(LoadedTransactionAccount { + loaded_size: account.data().len(), + account, + rent_collected: 0, + }) + } else { + self.callbacks + .inspect_account(account_key, AccountState::Dead, is_writable); + + None + } + } +} + +#[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, + } + } +} + /// 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,65 +277,23 @@ 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, - program_accounts: &HashMap, - 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, - program_accounts, - 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, - program_accounts: &HashMap, - 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, - program_accounts, - loaded_programs, ); match load_result { @@ -272,18 +326,13 @@ struct LoadedTransactionAccounts { pub loaded_accounts_data_size: u32, } -#[allow(clippy::too_many_arguments)] 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, - program_accounts: &HashMap, - loaded_programs: &ProgramCacheForTxBatch, ) -> Result { let mut tx_rent: TransactionRent = 0; let account_keys = message.account_keys(); @@ -292,12 +341,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, @@ -329,16 +372,11 @@ 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, - program_accounts, - loaded_programs, )?; collect_loaded_account(account_key, (loaded_account, account_found))?; } @@ -363,7 +401,9 @@ fn load_transaction_accounts( return Err(TransactionError::ProgramAccountNotFound); } - if !feature_set.is_active(&feature_set::remove_accounts_executable_flag_checks::id()) + if !account_loader + .feature_set + .is_active(&feature_set::remove_accounts_executable_flag_checks::id()) && !program_account.executable() { error_metrics.invalid_program_for_execution += 1; @@ -380,9 +420,15 @@ 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()) - || (!feature_set + || (!account_loader + .feature_set .is_active(&feature_set::remove_accounts_executable_flag_checks::id()) && !owner_account.executable()) { @@ -414,96 +460,51 @@ fn load_transaction_accounts( }) } -#[allow(clippy::too_many_arguments)] 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, - program_accounts: &HashMap, - loaded_programs: &ProgramCacheForTxBatch, ) -> Result<(LoadedTransactionAccount, bool)> { let mut account_found = true; - let disable_account_loader_special_case = - feature_set.is_active(&feature_set::disable_account_loader_special_case::id()); - 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( + &account_loader.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) = - (!disable_account_loader_special_case && !is_instruction_account && !is_writable) - .then_some(()) - .and_then(|_| loaded_programs.find(account_key)) - { - // 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(account_key, program_accounts)?, + 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)) @@ -622,7 +623,7 @@ mod tests { std::{borrow::Cow, cell::RefCell, collections::HashMap, fs::File, io::Read, sync::Arc}, }; - #[derive(Default)] + #[derive(Clone, Default)] struct TestCallbacks { accounts_map: HashMap, #[allow(clippy::type_complexity)] @@ -657,13 +658,25 @@ mod tests { } } + impl<'a> From<&'a TestCallbacks> for AccountLoader<'a, TestCallbacks> { + fn from(callbacks: &'a TestCallbacks) -> AccountLoader<'a, TestCallbacks> { + AccountLoader::new( + None, + ProgramCacheForTxBatch::default(), + HashMap::default(), + callbacks, + Arc::::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(); @@ -675,22 +688,20 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( - &callbacks, - &[sanitized_tx], - vec![Ok(ValidatedTransactionDetails { + let mut account_loader: AccountLoader = (&callbacks).into(); + account_loader.feature_set = Arc::new(feature_set.clone()); + 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, - &HashMap::new(), - &ProgramCacheForTxBatch::default(), ) } @@ -737,7 +748,7 @@ mod tests { tx: Transaction, accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, - ) -> Vec { + ) -> TransactionLoadResult { load_accounts_with_features_and_rent( tx, accounts, @@ -752,7 +763,7 @@ mod tests { accounts: &[TransactionAccount], error_metrics: &mut TransactionErrorMetrics, exclude_features: Option<&[Pubkey]>, - ) -> Vec { + ) -> TransactionLoadResult { load_accounts_with_features_and_rent( tx, accounts, @@ -789,9 +800,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, .. @@ -829,8 +839,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); @@ -871,9 +880,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, .. @@ -916,9 +924,8 @@ mod tests { ); 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, .. @@ -968,8 +975,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); @@ -986,7 +992,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(); @@ -998,16 +1004,19 @@ mod tests { accounts_map, ..Default::default() }; - load_accounts( + let mut account_loader = AccountLoader::new( + account_overrides, + ProgramCacheForTxBatch::default(), + HashMap::default(), &callbacks, - &[tx], - vec![Ok(ValidatedTransactionDetails::default())], + Arc::new(FeatureSet::all_enabled()), + ); + load_transaction( + &mut account_loader, + &tx, + Ok(ValidatedTransactionDetails::default()), &mut error_metrics, - account_overrides, - &FeatureSet::all_enabled(), &RentCollector::default(), - &HashMap::new(), - &ProgramCacheForTxBatch::default(), ) } @@ -1026,9 +1035,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, .. @@ -1058,8 +1066,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); @@ -1279,10 +1286,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, @@ -1290,7 +1297,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(), @@ -1299,11 +1306,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &HashMap::new(), - &loaded_programs, ); let expected_rent_debits = { @@ -1347,9 +1350,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, @@ -1357,7 +1360,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1365,11 +1368,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &HashMap::new(), - &loaded_programs, ); assert_eq!( @@ -1452,9 +1451,17 @@ mod tests { )), ); + let mut account_loader = AccountLoader::new( + None, + loaded_programs, + program_accounts, + &mock_bank, + Arc::::default(), + ); + let mut error_metrics = TransactionErrorMetrics::default(); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1462,11 +1469,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &program_accounts, - &loaded_programs, ); // Executable flag is bypassed @@ -1502,7 +1505,7 @@ mod tests { )); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1510,11 +1513,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &program_accounts, - &loaded_programs, ); // including program as instruction account bypasses executable bypass @@ -1529,7 +1528,7 @@ mod tests { ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, transaction.message(), LoadedTransactionAccount { account: account_data.clone(), @@ -1537,11 +1536,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &program_accounts, - &loaded_programs, ); // including program as writable bypasses executable bypass @@ -1572,9 +1567,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, @@ -1582,16 +1577,12 @@ 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(), - &HashMap::new(), - &loaded_programs, ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1618,9 +1609,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, @@ -1628,16 +1619,12 @@ 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(), - &HashMap::new(), - &loaded_programs, ); assert_eq!( @@ -1674,8 +1661,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, @@ -1683,7 +1671,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1691,11 +1679,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &HashMap::new(), - &loaded_programs, ); assert_eq!( @@ -1741,8 +1725,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, @@ -1750,16 +1735,12 @@ 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(), - &HashMap::new(), - &loaded_programs, ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1792,12 +1773,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, @@ -1805,16 +1786,12 @@ 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(), - &HashMap::new(), - &loaded_programs, ); assert_eq!( @@ -1857,9 +1834,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, @@ -1867,7 +1844,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1875,11 +1852,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &HashMap::new(), - &loaded_programs, ); assert_eq!( @@ -1946,9 +1919,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, @@ -1956,7 +1929,7 @@ mod tests { false, ); let result = load_transaction_accounts( - &mock_bank, + &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -1964,11 +1937,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut error_metrics, - None, - &FeatureSet::default(), &RentCollector::default(), - &HashMap::new(), - &loaded_programs, ); let mut account_data = AccountSharedData::default(); @@ -2012,9 +1981,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, @@ -2025,19 +1994,15 @@ 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(), - &HashMap::new(), - &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"); }; @@ -2105,9 +2070,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, @@ -2122,23 +2087,18 @@ 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(), - &HashMap::new(), - &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!( @@ -2173,7 +2133,7 @@ mod tests { #[test] fn test_load_accounts_error() { let mock_bank = TestCallbacks::default(); - let feature_set = FeatureSet::default(); + let mut account_loader = (&mock_bank).into(); let rent_collector = RentCollector::default(); let message = Message { @@ -2195,20 +2155,16 @@ 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, - &HashMap::new(), - &ProgramCacheForTxBatch::default(), ); assert!(matches!( - load_results[0], + load_result, TransactionLoadResult::FeesOnly(FeesOnlyTransaction { load_error: TransactionError::InvalidProgramForExecution, .. @@ -2217,20 +2173,16 @@ 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, - &HashMap::new(), - &ProgramCacheForTxBatch::default(), ); assert!(matches!( - load_results[0], + load_result, TransactionLoadResult::NotLoaded(TransactionError::InvalidWritableAccount), )); } @@ -2332,6 +2284,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], @@ -2368,16 +2321,12 @@ 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(), - &HashMap::new(), - &ProgramCacheForTxBatch::default(), ); // ensure the loaded accounts are inspected @@ -2501,8 +2450,16 @@ mod tests { program_cache.replenish(program2, Arc::new(program2_entry)); let test_transaction_data_size = |transaction, expected_size| { - let loaded_transaction_accounts = load_transaction_accounts( + let mut account_loader = AccountLoader::new( + None, + program_cache.clone(), + program_accounts.clone(), &mock_bank, + Arc::::default(), + ); + + let loaded_transaction_accounts = load_transaction_accounts( + &mut account_loader, &transaction, LoadedTransactionAccount { account: fee_payer_account.clone(), @@ -2511,11 +2468,7 @@ mod tests { }, &ComputeBudgetLimits::default(), &mut TransactionErrorMetrics::default(), - None, - &FeatureSet::default(), &RentCollector::default(), - &program_accounts, - &program_cache, ) .unwrap(); diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index fcd8117351f2d0..2862f3661ef641 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -3,19 +3,19 @@ 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, + nonce_info::NonceInfo, program_loader::{get_program_modification_slot, load_program_with_pubkey}, rollback_accounts::RollbackAccounts, 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}, }, log::debug, @@ -45,15 +45,17 @@ 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}, native_loader, + 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}, }, @@ -350,26 +352,14 @@ impl TransactionBatchProcessor { // Initialize metrics. let mut error_metrics = TransactionErrorMetrics::default(); let mut execute_timings = ExecuteTimings::default(); - - let (validation_results, validate_fees_us) = measure_us!(self.validate_fees( - callbacks, - config.account_overrides, - sanitized_txs, - check_results, - &environment.feature_set, - environment.fee_lamports_per_signature, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), - &mut error_metrics - )); + let mut processing_results = Vec::with_capacity(sanitized_txs.len()); let native_loader = native_loader::id(); let (program_accounts_map, filter_executable_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() { @@ -378,7 +368,7 @@ impl TransactionBatchProcessor { program_accounts_map }); - let (mut program_cache_for_tx_batch, program_cache_us) = measure_us!({ + let (program_cache_for_tx_batch, program_cache_us) = measure_us!({ let program_cache_for_tx_batch = self.replenish_program_cache( callbacks, &program_accounts_map, @@ -399,63 +389,95 @@ impl TransactionBatchProcessor { program_cache_for_tx_batch }); - let (loaded_transactions, load_accounts_us) = measure_us!(load_accounts( - callbacks, - sanitized_txs, - validation_results, - &mut error_metrics, + let mut account_loader = AccountLoader::new( config.account_overrides, - &environment.feature_set, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), - &program_accounts_map, - &program_cache_for_tx_batch, - )); + program_cache_for_tx_batch, + program_accounts_map, + callbacks, + environment.feature_set.clone(), + ); 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| { + Self::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + tx, + tx_details, + &environment.blockhash, + environment.fee_lamports_per_signature, + 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 + .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 { + 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()); + + Ok(ProcessedTransaction::Executed(Box::new(executed_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() @@ -468,7 +490,7 @@ impl TransactionBatchProcessor { debug!( "load: {}us execute: {}us txs_len={}", - load_accounts_us, + load_us, execution_us, sanitized_txs.len(), ); @@ -479,7 +501,7 @@ impl TransactionBatchProcessor { .saturating_add_in_place(ExecuteTimingType::FilterExecutableUs, filter_executable_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 { @@ -489,48 +511,52 @@ impl TransactionBatchProcessor { } } - fn validate_fees( - &self, - callbacks: &CB, - account_overrides: Option<&AccountOverrides>, - sanitized_txs: &[impl core::borrow::Borrow], - check_results: Vec, - feature_set: &FeatureSet, + fn validate_transaction_nonce_and_fee_payer( + account_loader: &mut AccountLoader, + message: &impl SVMMessage, + checked_details: CheckedTransactionDetails, + environment_blockhash: &Hash, fee_lamports_per_signature: u64, 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_lamports_per_signature, - rent_collector, - error_counters, - ) - }) - }) - .collect() + ) -> transaction::Result { + // If this is a nonce transaction, validate the nonce info. + // This must be done for every transaction to support SIMD83 because + // it may have changed due to use, authorization, or deallocation. + // This function is a successful no-op if given a blockhash transaction. + if let CheckedTransactionDetails { + nonce: Some(ref nonce_info), + lamports_per_signature: _, + } = checked_details + { + let next_durable_nonce = DurableNonce::from_blockhash(environment_blockhash); + Self::validate_transaction_nonce( + account_loader, + message, + nonce_info, + &next_durable_nonce, + error_counters, + )?; + } + + // Now validate the fee-payer for the transaction unconditionally. + Self::validate_transaction_fee_payer( + account_loader, + message, + checked_details, + fee_lamports_per_signature, + rent_collector, + error_counters, + ) } // 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, fee_lamports_per_signature: u64, rent_collector: &dyn SVMRentCollector, error_counters: &mut TransactionErrorMetrics, @@ -543,30 +569,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( - feature_set, + let fee_payer_loaded_rent_epoch = loaded_fee_payer.account.rent_epoch(); + loaded_fee_payer.rent_collected = collect_rent_from_account( + &account_loader.feature_set, rent_collector, fee_payer_address, - &mut fee_payer_account, + &mut loaded_fee_payer.account, ) .rent_amount; @@ -581,13 +597,15 @@ impl TransactionBatchProcessor { lamports_per_signature == 0, fee_lamports_per_signature, fee_budget_limits.prioritization_fee, - feature_set.is_active(&remove_rounding_in_fee_calculation::id()), + account_loader + .feature_set + .is_active(&remove_rounding_in_fee_calculation::id()), ); 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, @@ -599,8 +617,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, ); @@ -608,24 +626,78 @@ 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, + nonce_info: &NonceInfo, + next_durable_nonce: &DurableNonce, + error_counters: &mut TransactionErrorMetrics, + ) -> transaction::Result<()> { + // When SIMD83 is enabled, 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. + 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 = + ¤t_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(()), + } + } + /// 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<'a, CB: TransactionProcessingCallback>( callbacks: &CB, txs: &[impl SVMMessage], - validation_results: &[TransactionValidationResult], + check_results: &[TransactionCheckResult], program_owners: &'a [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() @@ -1119,8 +1191,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, @@ -1219,6 +1293,18 @@ mod tests { } } + impl<'a> From<&'a MockBankCallback> for AccountLoader<'a, MockBankCallback> { + fn from(callbacks: &'a MockBankCallback) -> AccountLoader<'a, MockBankCallback> { + AccountLoader::new( + None, + ProgramCacheForTxBatch::default(), + HashMap::default(), + callbacks, + Arc::::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")] @@ -1588,9 +1674,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]; @@ -1598,7 +1684,7 @@ mod tests { let result = TransactionBatchProcessor::::filter_executable_program_accounts( &mock_bank, &transactions, - &validation_results, + &check_results, &owners, ); @@ -1681,8 +1767,8 @@ mod tests { &bank, &[sanitized_tx1, sanitized_tx2], &[ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), + Ok(CheckedTransactionDetails::default()), ], owners, ); @@ -1773,15 +1859,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, ); @@ -2051,22 +2137,22 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &rent_collector, - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &rent_collector, + &mut error_counters, + ); let post_validation_fee_payer_account = { let mut account = fee_payer_account.clone(); @@ -2129,22 +2215,22 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &rent_collector, - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &rent_collector, + &mut error_counters, + ); let post_validation_fee_payer_account = { let mut account = fee_payer_account.clone(); @@ -2181,21 +2267,21 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &RentCollector::default(), - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &RentCollector::default(), + &mut error_counters, + ); assert_eq!(error_counters.account_not_found, 1); assert_eq!(result, Err(TransactionError::AccountNotFound)); @@ -2214,22 +2300,22 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &RentCollector::default(), - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &RentCollector::default(), + &mut error_counters, + ); assert_eq!(error_counters.insufficient_funds, 1); assert_eq!(result, Err(TransactionError::InsufficientFundsForFee)); @@ -2252,22 +2338,22 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &rent_collector, - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &rent_collector, + &mut error_counters, + ); assert_eq!( result, @@ -2288,22 +2374,22 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &RentCollector::default(), - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &RentCollector::default(), + &mut error_counters, + ); assert_eq!(error_counters.invalid_account_for_fee, 1); assert_eq!(result, Err(TransactionError::InvalidAccountForFee)); @@ -2321,21 +2407,21 @@ 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, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &RentCollector::default(), - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &RentCollector::default(), + &mut error_counters, + ); assert_eq!(error_counters.invalid_compute_budget, 1); assert_eq!(result, Err(TransactionError::DuplicateInstruction(1u8))); @@ -2343,7 +2429,6 @@ mod tests { #[test] fn test_validate_transaction_fee_payer_is_nonce() { - let feature_set = FeatureSet::default(); let lamports_per_signature = 5000; let rent_collector = RentCollector::default(); let compute_unit_limit = 2 * solana_compute_budget_program::DEFAULT_COMPUTE_UNITS; @@ -2368,9 +2453,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(); @@ -2381,24 +2468,27 @@ 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 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(next_durable_nonce, lamports_per_signature) + .unwrap(); - let result = batch_processor.validate_transaction_fee_payer( - &mock_bank, - None, + let tx_details = CheckedTransactionDetails { + nonce: Some(future_nonce.clone()), + lamports_per_signature, + }; + + let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, &message, - CheckedTransactionDetails { - nonce: nonce.clone(), - lamports_per_signature, - }, - &feature_set, + tx_details, + &environment_blockhash, FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, @@ -2415,7 +2505,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 @@ -2449,18 +2539,17 @@ 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, + let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, &message, CheckedTransactionDetails { nonce: None, lamports_per_signature, }, - &feature_set, + &Hash::default(), FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, @@ -2502,23 +2591,29 @@ mod tests { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + let mut account_loader = AccountLoader::new( + Some(&account_overrides), + ProgramCacheForTxBatch::default(), + HashMap::default(), + &mock_bank, + Arc::::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), - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &rent_collector, - &mut error_counters, - ); + let result = + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &rent_collector, + &mut error_counters, + ); assert!( result.is_ok(), "test_account_override_used: {:?}", @@ -2543,6 +2638,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( &[ @@ -2552,22 +2648,19 @@ mod tests { Some(&fee_payer_address), &Hash::new_unique(), )); - let batch_processor = TransactionBatchProcessor::::default(); - batch_processor - .validate_transaction_fee_payer( - &mock_bank, - None, - &message, - CheckedTransactionDetails { - nonce: None, - lamports_per_signature: 5000, - }, - &FeatureSet::default(), - FeeStructure::default().lamports_per_signature, - &RentCollector::default(), - &mut TransactionErrorMetrics::default(), - ) - .unwrap(); + TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( + &mut account_loader, + &message, + CheckedTransactionDetails { + nonce: None, + lamports_per_signature: 5000, + }, + &Hash::default(), + FeeStructure::default().lamports_per_signature, + &RentCollector::default(), + &mut TransactionErrorMetrics::default(), + ) + .unwrap(); // ensure the fee payer is an inspected account let actual_inspected_accounts: Vec<_> = mock_bank