diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index ffb965c48bab7f..655609ad2b3307 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -153,6 +153,8 @@ impl FeatureSet { increase_tx_account_lock_limit: self.is_active(&increase_tx_account_lock_limit::id()), disable_rent_fees_collection: self.is_active(&disable_rent_fees_collection::id()), enable_extend_program_checked: self.is_active(&enable_extend_program_checked::id()), + formalize_loaded_transaction_data_size: self + .is_active(&formalize_loaded_transaction_data_size::id()), } } } @@ -1095,6 +1097,10 @@ pub mod enable_extend_program_checked { solana_pubkey::declare_id!("97QCmR4QtfeQsAti9srfHFk5uMRFP95CvXG8EGr615HM"); } +pub mod formalize_loaded_transaction_data_size { + solana_pubkey::declare_id!("DeS7sR48ZcFTUmt5FFEVDr1v1bh73aAbZiZq3SYr8Eh8"); +} + pub static FEATURE_NAMES: LazyLock> = LazyLock::new(|| { [ (secp256k1_program_enabled::id(), "secp256k1 program"), @@ -1330,6 +1336,7 @@ pub static FEATURE_NAMES: LazyLock> = LazyLock::n (mask_out_rent_epoch_in_vm_serialization::id(), "SIMD-0267: Sets rent_epoch to a constant in the VM"), (enshrine_slashing_program::id(), "SIMD-0204: Slashable event verification"), (enable_extend_program_checked::id(), "Enable ExtendProgramChecked instruction"), + (formalize_loaded_transaction_data_size::id(), "SIMD-0186: Loaded transaction data size specification"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 8087e4b2f8ea78..9efcb3cd466f8d 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7177,11 +7177,15 @@ fn test_bank_load_program() { } #[allow(deprecated)] -#[test] -fn test_bpf_loader_upgradeable_deploy_with_max_len() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_bpf_loader_upgradeable_deploy_with_max_len(formalize_loaded_transaction_data_size: bool) { let (genesis_config, mint_keypair) = create_genesis_config_no_tx_fee(1_000_000_000); let mut bank = Bank::new_for_tests(&genesis_config); bank.feature_set = Arc::new(FeatureSet::all_enabled()); + if !formalize_loaded_transaction_data_size { + bank.deactivate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank.clone()); @@ -8376,10 +8380,15 @@ fn test_timestamp_fast() { } } -#[test] -fn test_program_is_native_loader() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_program_is_native_loader(formalize_loaded_transaction_data_size: bool) { let (genesis_config, mint_keypair) = create_genesis_config(50000); - let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); + let mut bank = Bank::new_for_tests(&genesis_config); + if formalize_loaded_transaction_data_size { + bank.activate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } + let (bank, _bank_forks) = bank.wrap_with_bank_forks_for_tests(); let tx = Transaction::new_signed_with_payer( &[Instruction::new_with_bincode( @@ -8391,20 +8400,29 @@ fn test_program_is_native_loader() { &[&mint_keypair], bank.last_blockhash(), ); - assert_eq!( - bank.process_transaction(&tx), - Err(TransactionError::InstructionError( - 0, - InstructionError::UnsupportedProgramId - )) - ); + + let err = bank.process_transaction(&tx).unwrap_err(); + if formalize_loaded_transaction_data_size { + assert_eq!(err, TransactionError::ProgramAccountNotFound); + } else { + assert_eq!( + err, + TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId) + ); + } } -#[test] -fn test_invoke_non_program_account_owned_by_a_builtin() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_invoke_non_program_account_owned_by_a_builtin( + formalize_loaded_transaction_data_size: bool, +) { let (genesis_config, mint_keypair) = create_genesis_config(10000000); let mut bank = Bank::new_for_tests(&genesis_config); bank.activate_feature(&feature_set::remove_accounts_executable_flag_checks::id()); + if formalize_loaded_transaction_data_size { + bank.activate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } let (bank, _bank_forks) = bank.wrap_with_bank_forks_for_tests(); let bogus_program = Pubkey::new_unique(); @@ -8431,13 +8449,12 @@ fn test_invoke_non_program_account_owned_by_a_builtin() { &[&mint_keypair, &created_account_keypair], bank.last_blockhash(), ); - assert_eq!( - bank.process_transaction(&tx), - Err(TransactionError::InstructionError( - 0, - InstructionError::UnsupportedProgramId - )) - ); + let expected_error = if formalize_loaded_transaction_data_size { + TransactionError::InvalidProgramForExecution + } else { + TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId) + }; + assert_eq!(bank.process_transaction(&tx), Err(expected_error),); } #[test] @@ -10563,8 +10580,9 @@ fn test_calculate_fee_secp256k1() { assert_eq!(calculate_test_fee(&message, 1, &fee_structure,), 11); } -#[test] -fn test_an_empty_instruction_without_program() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_an_empty_instruction_without_program(formalize_loaded_transaction_data_size: bool) { let (genesis_config, mint_keypair) = create_genesis_config_no_tx_fee_no_rent(1); let destination = solana_pubkey::new_rand(); let mut ix = system_instruction::transfer(&mint_keypair.pubkey(), &destination, 0); @@ -10572,11 +10590,21 @@ fn test_an_empty_instruction_without_program() { let message = Message::new(&[ix], Some(&mint_keypair.pubkey())); let tx = Transaction::new(&[&mint_keypair], message, genesis_config.hash()); - let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - assert_eq!( - bank.process_transaction(&tx).unwrap_err(), - TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId), - ); + let mut bank = Bank::new_for_tests(&genesis_config); + if !formalize_loaded_transaction_data_size { + bank.deactivate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } + let (bank, _bank_forks) = bank.wrap_with_bank_forks_for_tests(); + + let err = bank.process_transaction(&tx).unwrap_err(); + if formalize_loaded_transaction_data_size { + assert_eq!(err, TransactionError::ProgramAccountNotFound); + } else { + assert_eq!( + err, + TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId) + ); + } } #[test] @@ -12224,8 +12252,11 @@ fn test_is_in_slot_hashes_history() { assert!(!new_bank.is_in_slot_hashes_history(&0)); } -#[test] -fn test_feature_activation_loaded_programs_cache_preparation_phase() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_feature_activation_loaded_programs_cache_preparation_phase( + formalize_loaded_transaction_data_size: bool, +) { solana_logger::setup(); // Bank Setup @@ -12234,6 +12265,9 @@ fn test_feature_activation_loaded_programs_cache_preparation_phase() { let mut feature_set = FeatureSet::all_enabled(); feature_set.deactivate(&feature_set::disable_sbpf_v0_execution::id()); feature_set.deactivate(&feature_set::reenable_sbpf_v0_execution::id()); + if !formalize_loaded_transaction_data_size { + feature_set.deactivate(&feature_set::formalize_loaded_transaction_data_size::id()); + } bank.feature_set = Arc::new(feature_set); let (root_bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); @@ -13383,8 +13417,9 @@ fn test_deploy_last_epoch_slot() { assert_eq!(result_with_feature_enabled, Ok(())); } -#[test] -fn test_loader_v3_to_v4_migration() { +#[test_case(false; "informal_loaded_size")] +#[test_case(true; "simd186_loaded_size")] +fn test_loader_v3_to_v4_migration(formalize_loaded_transaction_data_size: bool) { solana_logger::setup(); // Bank Setup @@ -13395,6 +13430,9 @@ fn test_loader_v3_to_v4_migration() { ); let mut bank = Bank::new_for_tests(&genesis_config); bank.activate_feature(&feature_set::remove_accounts_executable_flag_checks::id()); + if formalize_loaded_transaction_data_size { + bank.activate_feature(&feature_set::formalize_loaded_transaction_data_size::id()); + } let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let fee_calculator = genesis_config.fee_rate_governor.create_fee_calculator(); let mut next_slot = 1; diff --git a/svm-feature-set/src/lib.rs b/svm-feature-set/src/lib.rs index d7cb6aa445a215..c421b8b9aac351 100644 --- a/svm-feature-set/src/lib.rs +++ b/svm-feature-set/src/lib.rs @@ -36,6 +36,7 @@ pub struct SVMFeatureSet { pub increase_tx_account_lock_limit: bool, pub disable_rent_fees_collection: bool, pub enable_extend_program_checked: bool, + pub formalize_loaded_transaction_data_size: bool, } impl SVMFeatureSet { @@ -77,6 +78,7 @@ impl SVMFeatureSet { increase_tx_account_lock_limit: true, disable_rent_fees_collection: true, enable_extend_program_checked: true, + formalize_loaded_transaction_data_size: true, } } } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index ea4f133e24c9fc..7b367e3aca4a90 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -8,11 +8,13 @@ use { }, ahash::{AHashMap, AHashSet}, solana_account::{ - Account, AccountSharedData, ReadableAccount, WritableAccount, PROGRAM_OWNERS, + state_traits::StateMut, Account, AccountSharedData, ReadableAccount, WritableAccount, + PROGRAM_OWNERS, }, solana_fee_structure::FeeDetails, solana_instruction::{BorrowedAccountMeta, BorrowedInstruction}, solana_instructions_sysvar::construct_instructions_data, + solana_loader_v3_interface::state::UpgradeableLoaderState, solana_nonce::state::State as NonceState, solana_nonce_account::{get_system_account_kind, SystemAccountKind}, solana_program_runtime::execution_budget::{ @@ -23,7 +25,7 @@ use { solana_rent_collector::{CollectedInfo, RENT_EXEMPT_RENT_EPOCH}, solana_rent_debits::RentDebits, solana_sdk_ids::{ - native_loader, + bpf_loader_upgradeable, native_loader, sysvar::{self, slot_history}, }, solana_svm_callback::{AccountState, TransactionProcessingCallback}, @@ -35,6 +37,14 @@ use { std::num::{NonZeroU32, Saturating}, }; +// Per SIMD-0186, all accounts are assigned a base size of 64 bytes to cover +// the storage cost of metadata. +pub(crate) const TRANSACTION_ACCOUNT_BASE_SIZE: usize = 64; + +// Per SIMD-0186, resolved address lookup tables are assigned a base size of 8248 +// bytes: 8192 bytes for the maximum table size plus 56 bytes for metadata. +const ADDRESS_LOOKUP_TABLE_BASE_SIZE: usize = 8248; + // for the load instructions pub(crate) type TransactionRent = u64; pub(crate) type TransactionProgramIndices = Vec>; @@ -194,6 +204,12 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> { account_key: &Pubkey, is_writable: bool, ) -> Option { + let base_account_size = if self.feature_set.formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let account = self.load_account(account_key); // Inspect prior to collecting rent, since rent collection can modify the account. @@ -208,7 +224,7 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> { ); account.map(|account| LoadedTransactionAccount { - loaded_size: account.data().len(), + loaded_size: base_account_size.saturating_add(account.data().len()), account, rent_collected: 0, }) @@ -477,6 +493,31 @@ struct LoadedTransactionAccounts { pub(crate) loaded_accounts_data_size: u32, } +impl LoadedTransactionAccounts { + fn increase_calculated_data_size( + &mut self, + data_size_delta: usize, + requested_loaded_accounts_data_size_limit: NonZeroU32, + error_metrics: &mut TransactionErrorMetrics, + ) -> Result<()> { + let Ok(data_size_delta) = u32::try_from(data_size_delta) else { + error_metrics.max_loaded_accounts_data_size_exceeded += 1; + return Err(TransactionError::MaxLoadedAccountsDataSizeExceeded); + }; + + self.loaded_accounts_data_size = self + .loaded_accounts_data_size + .saturating_add(data_size_delta); + + if self.loaded_accounts_data_size > requested_loaded_accounts_data_size_limit.get() { + error_metrics.max_loaded_accounts_data_size_exceeded += 1; + Err(TransactionError::MaxLoadedAccountsDataSizeExceeded) + } else { + Ok(()) + } + } +} + fn load_transaction_accounts( account_loader: &mut AccountLoader, message: &impl SVMMessage, @@ -484,6 +525,179 @@ fn load_transaction_accounts( loaded_accounts_bytes_limit: NonZeroU32, error_metrics: &mut TransactionErrorMetrics, rent_collector: &dyn SVMRentCollector, +) -> Result { + if account_loader + .feature_set + .formalize_loaded_transaction_data_size + { + load_transaction_accounts_simd186( + account_loader, + message, + loaded_fee_payer_account, + loaded_accounts_bytes_limit, + error_metrics, + rent_collector, + ) + } else { + load_transaction_accounts_old( + account_loader, + message, + loaded_fee_payer_account, + loaded_accounts_bytes_limit, + error_metrics, + rent_collector, + ) + } +} + +fn load_transaction_accounts_simd186( + account_loader: &mut AccountLoader, + message: &impl SVMMessage, + loaded_fee_payer_account: LoadedTransactionAccount, + loaded_accounts_bytes_limit: NonZeroU32, + error_metrics: &mut TransactionErrorMetrics, + rent_collector: &dyn SVMRentCollector, +) -> Result { + let account_keys = message.account_keys(); + let mut additional_loaded_accounts: AHashSet = AHashSet::new(); + + let mut loaded_transaction_accounts = LoadedTransactionAccounts { + accounts: Vec::with_capacity(account_keys.len()), + program_indices: Vec::with_capacity(message.num_instructions()), + rent: 0, + rent_debits: RentDebits::default(), + loaded_accounts_data_size: 0, + }; + + // 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), + loaded_accounts_bytes_limit, + error_metrics, + )?; + + let mut collect_loaded_account = + |account_loader: &mut AccountLoader, key, loaded_account| -> Result<()> { + let LoadedTransactionAccount { + account, + loaded_size, + rent_collected, + } = loaded_account; + + loaded_transaction_accounts.increase_calculated_data_size( + loaded_size, + loaded_accounts_bytes_limit, + error_metrics, + )?; + + loaded_transaction_accounts.rent = loaded_transaction_accounts + .rent + .saturating_add(rent_collected); + + loaded_transaction_accounts + .rent_debits + .insert(key, rent_collected, account.lamports()); + + // This has been annotated branch-by-branch because collapsing the logic is infeasible. + // Its purpose is to ensure programdata accounts are counted once and *only* once per + // transaction. By checking account_keys, we never double-count a programdata account + // that was explictly included in the transaction. We also use a hashset to gracefully + // handle cases that LoaderV3 presumably makes impossible, such as self-referential + // program accounts or multiply-referenced programdata accounts, for added safety. + // + // If in the future LoaderV3 programs are migrated to LoaderV4, this entire code block + // can be deleted. + // + // If this is a valid LoaderV3 program... + if bpf_loader_upgradeable::check_id(account.owner()) { + if let Ok(UpgradeableLoaderState::Program { + programdata_address, + }) = account.state() + { + // ...its programdata was not already counted and will not later be counted... + if !account_keys.iter().any(|key| programdata_address == *key) + && !additional_loaded_accounts.contains(&programdata_address) + { + // ...and the programdata account exists (if it doesn't, it is *not* a load failure)... + if let Some(programdata_account) = + account_loader.load_account(&programdata_address) + { + // ...count programdata toward this transaction's total size. + loaded_transaction_accounts.increase_calculated_data_size( + TRANSACTION_ACCOUNT_BASE_SIZE + .saturating_add(programdata_account.data().len()), + loaded_accounts_bytes_limit, + error_metrics, + )?; + additional_loaded_accounts.insert(programdata_address); + } + } + } + } + + loaded_transaction_accounts.accounts.push((*key, account)); + + Ok(()) + }; + + // Since the fee payer is always the first account, collect it first. + // We can use it directly because it was already loaded during validation. + collect_loaded_account( + account_loader, + message.fee_payer(), + loaded_fee_payer_account, + )?; + + // 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 = load_transaction_account( + account_loader, + message, + account_key, + account_index, + rent_collector, + ); + collect_loaded_account(account_loader, account_key, loaded_account)?; + } + + for (program_id, instruction) in message.program_instructions_iter() { + let Some(program_account) = account_loader.load_account(program_id) else { + error_metrics.account_not_found += 1; + return Err(TransactionError::ProgramAccountNotFound); + }; + + if !account_loader + .feature_set + .remove_accounts_executable_flag_checks + && !program_account.executable() + { + error_metrics.invalid_program_for_execution += 1; + return Err(TransactionError::InvalidProgramForExecution); + } + + let owner_id = program_account.owner(); + if !native_loader::check_id(owner_id) && !PROGRAM_OWNERS.contains(owner_id) { + error_metrics.invalid_program_for_execution += 1; + return Err(TransactionError::InvalidProgramForExecution); + } + + loaded_transaction_accounts + .program_indices + .push(vec![instruction.program_id_index as IndexOfAccount]); + } + + Ok(loaded_transaction_accounts) +} + +fn load_transaction_accounts_old( + account_loader: &mut AccountLoader, + message: &impl SVMMessage, + loaded_fee_payer_account: LoadedTransactionAccount, + loaded_accounts_bytes_limit: NonZeroU32, + error_metrics: &mut TransactionErrorMetrics, + rent_collector: &dyn SVMRentCollector, ) -> Result { let mut tx_rent: TransactionRent = 0; let account_keys = message.account_keys(); @@ -560,22 +774,6 @@ fn load_transaction_accounts( } if !validated_loaders.contains(owner_id) { - // NOTE there are several feature gate activations that affect this code: - // * `remove_accounts_executable_flag_checks`: this implicitly makes system, vote, stake, et al valid loaders - // it is impossible to mark an account executable and also have it be owned by one of them - // so, with the feature disabled, we always fail the executable check if they are a program id owner - // however, with the feature enabled, any account owned by an account owned by native loader is a "program" - // this is benign (any such transaction will fail at execution) but it affects which transactions pay fees - // * `enable_transaction_loading_failure_fees`: loading failures behave the same as execution failures - // at this point we can restrict valid loaders to those contained in `PROGRAM_OWNERS` - // since any other pseudo-loader owner is destined to fail at execution - // * SIMD-186: explicitly defines a sensible transaction data size algorithm - // at this point we stop counting loaders toward transaction data size entirely - // - // when _all three_ of `remove_accounts_executable_flag_checks`, `enable_transaction_loading_failure_fees`, - // and SIMD-186 are active, we do not need to load loaders at all to comply with consensus rules - // we may verify program ids are owned by `PROGRAM_OWNERS` purely as an optimization - // this could even be done before loading the rest of the accounts for a transaction if let Some(owner_account) = account_loader.load_account(owner_id) { if !native_loader::check_id(owner_account.owner()) || (!account_loader @@ -719,6 +917,7 @@ mod tests { super::*, crate::transaction_account_state_info::TransactionAccountStateInfo, agave_reserved_account_keys::ReservedAccountKeys, + rand0_7::prelude::*, solana_account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, solana_epoch_schedule::EpochSchedule, solana_hash::Hash, @@ -750,9 +949,10 @@ mod tests { solana_transaction_context::{TransactionAccount, TransactionContext}, solana_transaction_error::{TransactionError, TransactionResult as Result}, std::{borrow::Cow, cell::RefCell, collections::HashMap, fs::File, io::Read}, + test_case::test_case, }; - #[derive(Clone, Default)] + #[derive(Clone)] struct TestCallbacks { accounts_map: HashMap, #[allow(clippy::type_complexity)] @@ -761,6 +961,16 @@ mod tests { feature_set: SVMFeatureSet, } + impl Default for TestCallbacks { + fn default() -> Self { + Self { + accounts_map: HashMap::default(), + inspected_accounts: RefCell::default(), + feature_set: SVMFeatureSet::all_enabled(), + } + } + } + impl InvokeContextCallback for TestCallbacks {} impl TransactionProcessingCallback for TestCallbacks { @@ -843,22 +1053,9 @@ mod tests { )) } - fn load_accounts_aux_test( - tx: Transaction, - accounts: &[TransactionAccount], - error_metrics: &mut TransactionErrorMetrics, - ) -> TransactionLoadResult { - load_accounts_with_features_and_rent( - tx, - accounts, - &RentCollector::default(), - error_metrics, - SVMFeatureSet::all_enabled(), - ) - } - - #[test] - fn test_load_accounts_unknown_program_id() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_unknown_program_id(formalize_loaded_transaction_data_size: bool) { let mut accounts: Vec = Vec::new(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -881,7 +1078,16 @@ mod tests { instructions, ); - let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics); + let mut feature_set = SVMFeatureSet::all_enabled(); + feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; + + let load_results = load_accounts_with_features_and_rent( + tx, + &accounts, + &RentCollector::default(), + &mut error_metrics, + feature_set, + ); assert_eq!(error_metrics.account_not_found.0, 1); assert!(matches!( @@ -893,8 +1099,9 @@ mod tests { )); } - #[test] - fn test_load_accounts_no_loaders() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_no_loaders(formalize_loaded_transaction_data_size: bool) { let mut accounts: Vec = Vec::new(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -919,29 +1126,43 @@ mod tests { instructions, ); + let mut feature_set = SVMFeatureSet::all_enabled(); + feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; + let loaded_accounts = load_accounts_with_features_and_rent( tx, &accounts, &RentCollector::default(), &mut error_metrics, - SVMFeatureSet::all_enabled(), + feature_set, ); - assert_eq!(error_metrics.account_not_found.0, 0); match &loaded_accounts { - TransactionLoadResult::Loaded(loaded_transaction) => { + TransactionLoadResult::Loaded(loaded_transaction) + if !formalize_loaded_transaction_data_size => + { + assert_eq!(error_metrics.account_not_found.0, 0); assert_eq!(loaded_transaction.accounts.len(), 3); assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1); assert_eq!(loaded_transaction.program_indices.len(), 1); assert_eq!(loaded_transaction.program_indices[0].len(), 0); } - TransactionLoadResult::FeesOnly(fees_only_tx) => panic!("{}", fees_only_tx.load_error), - TransactionLoadResult::NotLoaded(e) => panic!("{e}"), + TransactionLoadResult::FeesOnly(fees_only_tx) + if formalize_loaded_transaction_data_size => + { + assert_eq!(error_metrics.account_not_found.0, 1); + assert_eq!( + fees_only_tx.load_error, + TransactionError::ProgramAccountNotFound, + ); + } + result => panic!("unexpected result: {:?}", result), } } - #[test] - fn test_load_accounts_bad_owner() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_bad_owner(formalize_loaded_transaction_data_size: bool) { let mut accounts: Vec = Vec::new(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -953,7 +1174,6 @@ mod tests { accounts.push((key0, account)); let mut account = AccountSharedData::new(40, 1, &Pubkey::default()); - account.set_owner(bpf_loader_upgradeable::id()); account.set_executable(true); accounts.push((key1, account)); @@ -966,20 +1186,41 @@ mod tests { instructions, ); - let load_results = load_accounts_aux_test(tx, &accounts, &mut error_metrics); + let mut feature_set = SVMFeatureSet::all_enabled(); + feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; - assert_eq!(error_metrics.account_not_found.0, 1); - assert!(matches!( - load_results, - TransactionLoadResult::FeesOnly(FeesOnlyTransaction { - load_error: TransactionError::ProgramAccountNotFound, - .. - }), - )); + let load_results = load_accounts_with_features_and_rent( + tx, + &accounts, + &RentCollector::default(), + &mut error_metrics, + feature_set, + ); + + if formalize_loaded_transaction_data_size { + assert_eq!(error_metrics.invalid_program_for_execution.0, 1); + assert!(matches!( + load_results, + TransactionLoadResult::FeesOnly(FeesOnlyTransaction { + load_error: TransactionError::InvalidProgramForExecution, + .. + }), + )); + } else { + assert_eq!(error_metrics.account_not_found.0, 1); + assert!(matches!( + load_results, + TransactionLoadResult::FeesOnly(FeesOnlyTransaction { + load_error: TransactionError::ProgramAccountNotFound, + .. + }), + )); + } } - #[test] - fn test_load_accounts_not_executable() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_not_executable(formalize_loaded_transaction_data_size: bool) { let mut accounts: Vec = Vec::new(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -1004,6 +1245,8 @@ mod tests { let mut feature_set = SVMFeatureSet::all_enabled(); feature_set.remove_accounts_executable_flag_checks = false; + feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; + let load_results = load_accounts_with_features_and_rent( tx, &accounts, @@ -1022,8 +1265,9 @@ mod tests { )); } - #[test] - fn test_load_accounts_multiple_loaders() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_multiple_loaders(formalize_loaded_transaction_data_size: bool) { let mut accounts: Vec = Vec::new(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -1060,12 +1304,15 @@ mod tests { instructions, ); + let mut feature_set = SVMFeatureSet::all_enabled(); + feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; + let loaded_accounts = load_accounts_with_features_and_rent( tx, &accounts, &RentCollector::default(), &mut error_metrics, - SVMFeatureSet::all_enabled(), + feature_set, ); assert_eq!(error_metrics.account_not_found.0, 0); @@ -1149,17 +1396,28 @@ mod tests { let keypair = Keypair::new(); let account = AccountSharedData::new(1_000_000, 0, &Pubkey::default()); + let mut program_account = AccountSharedData::default(); + program_account.set_lamports(1); + program_account.set_executable(true); + program_account.set_owner(native_loader::id()); + let instructions = vec![CompiledInstruction::new(2, &(), vec![0])]; let tx = Transaction::new_with_compiled_instructions( &[&keypair], &[slot_history_id], Hash::default(), - vec![native_loader::id()], + vec![bpf_loader::id()], instructions, ); - let loaded_accounts = - load_accounts_no_store(&[(keypair.pubkey(), account)], tx, Some(&account_overrides)); + let loaded_accounts = load_accounts_no_store( + &[ + (keypair.pubkey(), account), + (bpf_loader::id(), program_account), + ], + tx, + Some(&account_overrides), + ); match &loaded_accounts { TransactionLoadResult::Loaded(loaded_transaction) => { assert_eq!(loaded_transaction.accounts[0].0, keypair.pubkey()); @@ -1420,8 +1678,9 @@ mod tests { ); } - #[test] - fn test_load_transaction_accounts_native_loader() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_transaction_accounts_native_loader(formalize_loaded_transaction_data_size: bool) { let key1 = Keypair::new(); let message = Message { account_keys: vec![key1.pubkey(), native_loader::id()], @@ -1436,6 +1695,8 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; mock_bank .accounts_map .insert(native_loader::id(), AccountSharedData::default()); @@ -1453,11 +1714,19 @@ mod tests { vec![Signature::new_unique()], false, ); + + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let result = load_transaction_accounts( &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), + loaded_size: base_account_size, ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, @@ -1465,22 +1734,30 @@ mod tests { &RentCollector::default(), ); - assert_eq!( - result.unwrap(), - LoadedTransactionAccounts { - accounts: vec![ - (key1.pubkey(), fee_payer_account), - ( - native_loader::id(), - mock_bank.accounts_map[&native_loader::id()].clone() - ) - ], - program_indices: vec![vec![]], - rent: 0, - rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, - } - ); + if formalize_loaded_transaction_data_size { + assert_eq!( + result.unwrap_err(), + TransactionError::ProgramAccountNotFound, + ); + } else { + let loaded_accounts_data_size = base_account_size as u32 * 2; + assert_eq!( + result.unwrap(), + LoadedTransactionAccounts { + accounts: vec![ + (key1.pubkey(), fee_payer_account), + ( + native_loader::id(), + mock_bank.accounts_map[&native_loader::id()].clone() + ) + ], + program_indices: vec![vec![]], + rent: 0, + rent_debits: RentDebits::default(), + loaded_accounts_data_size, + } + ); + } } #[test] @@ -1570,8 +1847,11 @@ mod tests { ); } - #[test] - fn test_load_transaction_accounts_native_loader_owner() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_transaction_accounts_native_loader_owner( + formalize_loaded_transaction_data_size: bool, + ) { let key1 = Keypair::new(); let key2 = Keypair::new(); @@ -1588,6 +1868,8 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_data = AccountSharedData::default(); account_data.set_owner(native_loader::id()); account_data.set_lamports(1); @@ -1608,11 +1890,19 @@ mod tests { vec![Signature::new_unique()], false, ); + + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let result = load_transaction_accounts( &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), + loaded_size: base_account_size, ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, @@ -1620,6 +1910,8 @@ mod tests { &RentCollector::default(), ); + let loaded_accounts_data_size = base_account_size as u32 * 2; + assert_eq!( result.unwrap(), LoadedTransactionAccounts { @@ -1633,7 +1925,7 @@ mod tests { program_indices: vec![vec![1]], rent: 0, rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, + loaded_accounts_data_size, } ); } @@ -1739,11 +2031,13 @@ mod tests { ); } - #[test] - fn test_load_transaction_accounts_program_success_complete() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_transaction_accounts_program_success_complete( + formalize_loaded_transaction_data_size: bool, + ) { let key1 = Keypair::new(); let key2 = Keypair::new(); - let key3 = Keypair::new(); let message = Message { account_keys: vec![key2.pubkey(), key1.pubkey()], @@ -1758,10 +2052,12 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_data = AccountSharedData::default(); account_data.set_lamports(1); account_data.set_executable(true); - account_data.set_owner(key3.pubkey()); + account_data.set_owner(bpf_loader::id()); mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut fee_payer_account = AccountSharedData::default(); @@ -1774,7 +2070,9 @@ mod tests { account_data.set_lamports(1); account_data.set_executable(true); account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + mock_bank + .accounts_map + .insert(bpf_loader::id(), account_data); let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -1784,11 +2082,19 @@ mod tests { vec![Signature::new_unique()], false, ); + + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let result = load_transaction_accounts( &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), + loaded_size: base_account_size, ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, @@ -1796,6 +2102,8 @@ mod tests { &RentCollector::default(), ); + let loaded_accounts_data_size = base_account_size as u32 * 2; + assert_eq!( result.unwrap(), LoadedTransactionAccounts { @@ -1809,20 +2117,22 @@ mod tests { program_indices: vec![vec![1]], rent: 0, rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, + loaded_accounts_data_size, } ); } - #[test] - fn test_load_transaction_accounts_program_builtin_saturating_add() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_transaction_accounts_program_builtin_saturating_add( + formalize_loaded_transaction_data_size: bool, + ) { let key1 = Keypair::new(); let key2 = Keypair::new(); let key3 = Keypair::new(); - let key4 = Keypair::new(); let message = Message { - account_keys: vec![key2.pubkey(), key1.pubkey(), key4.pubkey()], + account_keys: vec![key2.pubkey(), key1.pubkey(), key3.pubkey()], header: MessageHeader::default(), instructions: vec![ CompiledInstruction { @@ -1841,10 +2151,12 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_data = AccountSharedData::default(); account_data.set_lamports(1); account_data.set_executable(true); - account_data.set_owner(key3.pubkey()); + account_data.set_owner(bpf_loader::id()); mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut fee_payer_account = AccountSharedData::default(); @@ -1857,7 +2169,9 @@ mod tests { account_data.set_lamports(1); account_data.set_executable(true); account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + mock_bank + .accounts_map + .insert(bpf_loader::id(), account_data); let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -1867,11 +2181,19 @@ mod tests { vec![Signature::new_unique()], false, ); + + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let result = load_transaction_accounts( &mut account_loader, sanitized_transaction.message(), LoadedTransactionAccount { account: fee_payer_account.clone(), + loaded_size: base_account_size, ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, @@ -1879,6 +2201,8 @@ mod tests { &RentCollector::default(), ); + let loaded_accounts_data_size = base_account_size as u32 * 2; + let mut account_data = AccountSharedData::default(); account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); assert_eq!( @@ -1890,12 +2214,12 @@ mod tests { key1.pubkey(), mock_bank.accounts_map[&key1.pubkey()].clone() ), - (key4.pubkey(), account_data), + (key3.pubkey(), account_data), ], program_indices: vec![vec![1], vec![1]], rent: 0, rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, + loaded_accounts_data_size, } ); } @@ -1965,15 +2289,15 @@ mod tests { ); } - #[test] - fn test_load_accounts_success() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_load_accounts_success(formalize_loaded_transaction_data_size: bool) { let key1 = Keypair::new(); let key2 = Keypair::new(); let key3 = Keypair::new(); - let key4 = Keypair::new(); let message = Message { - account_keys: vec![key2.pubkey(), key1.pubkey(), key4.pubkey()], + account_keys: vec![key2.pubkey(), key1.pubkey(), key3.pubkey()], header: MessageHeader::default(), instructions: vec![ CompiledInstruction { @@ -1992,10 +2316,12 @@ mod tests { let sanitized_message = new_unchecked_sanitized_message(message); let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_data = AccountSharedData::default(); account_data.set_lamports(1); account_data.set_executable(true); - account_data.set_owner(key3.pubkey()); + account_data.set_owner(bpf_loader::id()); mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut fee_payer_account = AccountSharedData::default(); @@ -2008,7 +2334,9 @@ mod tests { account_data.set_lamports(1); account_data.set_executable(true); account_data.set_owner(native_loader::id()); - mock_bank.accounts_map.insert(key3.pubkey(), account_data); + mock_bank + .accounts_map + .insert(bpf_loader::id(), account_data); let mut account_loader = (&mock_bank).into(); let mut error_metrics = TransactionErrorMetrics::default(); @@ -2018,9 +2346,17 @@ mod tests { vec![Signature::new_unique()], false, ); + + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + let validation_result = Ok(ValidatedTransactionDetails { loaded_fee_payer_account: LoadedTransactionAccount { account: fee_payer_account, + loaded_size: base_account_size, ..LoadedTransactionAccount::default() }, ..ValidatedTransactionDetails::default() @@ -2034,6 +2370,8 @@ mod tests { &RentCollector::default(), ); + let loaded_accounts_data_size = base_account_size as u32 * 2; + let mut account_data = AccountSharedData::default(); account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); @@ -2052,7 +2390,7 @@ mod tests { key1.pubkey(), mock_bank.accounts_map[&key1.pubkey()].clone() ), - (key4.pubkey(), account_data), + (key3.pubkey(), account_data), ], program_indices: vec![vec![1], vec![1]], fee_details: FeeDetails::default(), @@ -2060,7 +2398,7 @@ mod tests { compute_budget: SVMTransactionExecutionBudget::default(), rent: 0, rent_debits: RentDebits::default(), - loaded_accounts_data_size: 0, + loaded_accounts_data_size, } ); } @@ -2287,6 +2625,7 @@ mod tests { #[test] fn test_load_transaction_accounts_data_sizes() { let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.formalize_loaded_transaction_data_size = false; let loader_v2 = bpf_loader::id(); let loader_v3 = bpf_loader_upgradeable::id(); @@ -2368,10 +2707,13 @@ mod tests { let mut program_accounts = HashMap::new(); program_accounts.insert(program1, (&loader_v2, 0)); program_accounts.insert(program2, (&loader_v3, 0)); - let feature_set = SVMFeatureSet::default(); let test_transaction_data_size = |transaction, expected_size| { - let mut account_loader = - AccountLoader::new_with_loaded_accounts_capacity(None, &mock_bank, &feature_set, 0); + let mut account_loader = AccountLoader::new_with_loaded_accounts_capacity( + None, + &mock_bank, + &mock_bank.feature_set, + 0, + ); let loaded_transaction_accounts = load_transaction_accounts( &mut account_loader, @@ -2644,4 +2986,195 @@ mod tests { assert_eq!(account_loader.load_account(&fee_payer), None); assert_eq!(account_loader.get_account_shared_data(&fee_payer), None); } + + // note all magic numbers (how many accounts, how many instructions, how big to size buffers) are arbitrary + // other than trying not to swamp programs with blank accounts and keep transaction size below the 64mb limit + #[test_case(false; "executable_mandatory")] + #[test_case(true; "executable_optional")] + fn test_load_transaction_accounts_data_sizes_simd186( + remove_accounts_executable_flag_checks: bool, + ) { + let mut rng = rand0_7::thread_rng(); + let mut mock_bank = TestCallbacks::default(); + mock_bank.feature_set.remove_accounts_executable_flag_checks = + remove_accounts_executable_flag_checks; + + // arbitrary accounts + for _ in 0..128 { + let account = AccountSharedData::create( + 1, + vec![0; rng.gen_range(0, 128)], + Pubkey::new_unique(), + rng.gen(), + u64::MAX, + ); + mock_bank.accounts_map.insert(Pubkey::new_unique(), account); + } + + // fee-payers + let mut fee_payers = vec![]; + for _ in 0..8 { + let fee_payer = Pubkey::new_unique(); + let account = AccountSharedData::create( + LAMPORTS_PER_SOL, + vec![0; rng.gen_range(0, 32)], + system_program::id(), + rng.gen(), + u64::MAX, + ); + mock_bank.accounts_map.insert(fee_payer, account); + fee_payers.push(fee_payer); + } + + // programs + let mut loader_owned_accounts = vec![]; + let mut programdata_tracker = AHashMap::new(); + for loader in PROGRAM_OWNERS { + for _ in 0..16 { + let program_id = Pubkey::new_unique(); + let mut account = AccountSharedData::create( + 1, + vec![0; rng.gen_range(0, 512)], + *loader, + !remove_accounts_executable_flag_checks || rng.gen(), + u64::MAX, + ); + + // give half loaderv3 accounts (if theyre long enough) a valid programdata + // a quarter a dead pointer and a quarter nothing + // we set executable like a program because after the flag is disabled... + // ...programdata and buffer accounts can be used as program ids without aborting loading + // this will always fail at execution but we are merely testing the data size accounting here + if *loader == bpf_loader_upgradeable::id() && account.data().len() >= 64 { + let programdata_address = Pubkey::new_unique(); + let has_programdata = rng.gen(); + + if has_programdata { + let programdata_account = AccountSharedData::create( + 1, + vec![0; rng.gen_range(0, 512)], + *loader, + !remove_accounts_executable_flag_checks || rng.gen(), + u64::MAX, + ); + programdata_tracker.insert( + program_id, + (programdata_address, programdata_account.data().len()), + ); + mock_bank + .accounts_map + .insert(programdata_address, programdata_account); + loader_owned_accounts.push(programdata_address); + } + + if has_programdata || rng.gen() { + account + .set_state(&UpgradeableLoaderState::Program { + programdata_address, + }) + .unwrap(); + } + } + + mock_bank.accounts_map.insert(program_id, account); + loader_owned_accounts.push(program_id); + } + } + + let mut all_accounts = mock_bank.accounts_map.keys().copied().collect::>(); + + // append some to-be-created accounts + // this is to test that their size is 0 rather than 64 + for _ in 0..32 { + all_accounts.push(Pubkey::new_unique()); + } + + let mut account_loader = (&mock_bank).into(); + + // now generate arbitrary transactions using this accounts + // we ensure valid fee-payers and that all program ids are loader-owned + // otherwise any account can appear anywhere + // some edge cases we hope to hit (not necessarily all in every run): + // * programs used multiple times as program ids and/or normal accounts are counted once + // * loaderv3 programdata used explicitly zero one or multiple times is counted once + // * loaderv3 programs with missing programdata are allowed through + // * loaderv3 programdata used as program id does nothing weird + // * loaderv3 programdata used as a regular account does nothing weird + // * the programdata conditions hold regardless of ordering + for _ in 0..1024 { + let mut instructions = vec![]; + for _ in 0..rng.gen_range(1, 8) { + let mut accounts = vec![]; + for _ in 0..rng.gen_range(1, 16) { + all_accounts.shuffle(&mut rng); + let pubkey = all_accounts[0]; + + accounts.push(AccountMeta { + pubkey, + is_writable: rng.gen(), + is_signer: rng.gen() && rng.gen(), + }); + } + + loader_owned_accounts.shuffle(&mut rng); + let program_id = loader_owned_accounts[0]; + instructions.push(Instruction { + accounts, + program_id, + data: vec![], + }); + } + + fee_payers.shuffle(&mut rng); + let fee_payer = fee_payers[0]; + let fee_payer_account = mock_bank.accounts_map.get(&fee_payer).cloned().unwrap(); + + let transaction = SanitizedTransaction::from_transaction_for_tests( + Transaction::new_with_payer(&instructions, Some(&fee_payer)), + ); + + let mut expected_size = 0; + let mut counted_programdatas = transaction + .account_keys() + .iter() + .copied() + .collect::>(); + + for pubkey in transaction.account_keys().iter() { + if let Some(account) = mock_bank.accounts_map.get(pubkey) { + expected_size += TRANSACTION_ACCOUNT_BASE_SIZE + account.data().len(); + }; + + if let Some((programdata_address, programdata_size)) = + programdata_tracker.get(pubkey) + { + if counted_programdatas.get(programdata_address).is_none() { + expected_size += TRANSACTION_ACCOUNT_BASE_SIZE + programdata_size; + counted_programdatas.insert(*programdata_address); + } + } + } + + assert!(expected_size <= MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES.get() as usize); + + let loaded_transaction_accounts = load_transaction_accounts( + &mut account_loader, + &transaction, + LoadedTransactionAccount { + loaded_size: TRANSACTION_ACCOUNT_BASE_SIZE + fee_payer_account.data().len(), + account: fee_payer_account, + rent_collected: 0, + }, + MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, + &mut TransactionErrorMetrics::default(), + &RentCollector::default(), + ) + .unwrap(); + + assert_eq!( + loaded_transaction_accounts.loaded_accounts_data_size, + expected_size as u32, + ); + } + } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 6e7c4229c74800..7d00785a6fa492 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1099,7 +1099,10 @@ mod tests { use { super::*, crate::{ - account_loader::{LoadedTransactionAccount, ValidatedTransactionDetails}, + account_loader::{ + LoadedTransactionAccount, ValidatedTransactionDetails, + TRANSACTION_ACCOUNT_BASE_SIZE, + }, nonce_info::NonceInfo, rollback_accounts::RollbackAccounts, }, @@ -1147,7 +1150,7 @@ mod tests { } } - #[derive(Default, Clone)] + #[derive(Clone)] struct MockBankCallback { account_shared_data: Arc>>, #[allow(clippy::type_complexity)] @@ -1156,6 +1159,16 @@ mod tests { feature_set: SVMFeatureSet, } + impl Default for MockBankCallback { + fn default() -> Self { + Self { + account_shared_data: Arc::default(), + inspected_accounts: Arc::default(), + feature_set: SVMFeatureSet::all_enabled(), + } + } + } + impl InvokeContextCallback for MockBankCallback {} impl TransactionProcessingCallback for MockBankCallback { @@ -2036,8 +2049,11 @@ mod tests { assert_eq!(entry, Arc::new(program)); } - #[test] - fn test_validate_transaction_fee_payer_exact_balance() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_validate_transaction_fee_payer_exact_balance( + formalize_loaded_transaction_data_size: bool, + ) { let lamports_per_signature = 5000; let message = new_unchecked_sanitized_message(Message::new_with_blockhash( &[ @@ -2075,10 +2091,12 @@ mod tests { ); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { + let mut mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); @@ -2107,6 +2125,12 @@ mod tests { account }; + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + assert_eq!( result, Ok(ValidatedTransactionDetails { @@ -2122,7 +2146,7 @@ mod tests { .loaded_accounts_data_size_limit, fee_details: FeeDetails::new(transaction_fee, priority_fee), loaded_fee_payer_account: LoadedTransactionAccount { - loaded_size: fee_payer_account.data().len(), + loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, rent_collected: fee_payer_rent_debit, }, @@ -2130,8 +2154,11 @@ mod tests { ); } - #[test] - fn test_validate_transaction_fee_payer_rent_paying() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_validate_transaction_fee_payer_rent_paying( + formalize_loaded_transaction_data_size: bool, + ) { let lamports_per_signature = 5000; let message = new_unchecked_sanitized_message(Message::new_with_blockhash( &[], @@ -2156,10 +2183,13 @@ mod tests { let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { + let mut mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; + mock_bank.feature_set.disable_rent_fees_collection = false; let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); @@ -2183,6 +2213,12 @@ mod tests { account }; + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + assert_eq!( result, Ok(ValidatedTransactionDetails { @@ -2198,7 +2234,7 @@ mod tests { .loaded_accounts_data_size_limit, fee_details: FeeDetails::new(transaction_fee, 0), loaded_fee_payer_account: LoadedTransactionAccount { - loaded_size: fee_payer_account.data().len(), + loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, rent_collected: fee_payer_rent_debit, } @@ -2381,8 +2417,9 @@ mod tests { assert_eq!(result, Err(TransactionError::DuplicateInstruction(1u8))); } - #[test] - fn test_validate_transaction_fee_payer_is_nonce() { + #[test_case(false; "informal_loaded_size")] + #[test_case(true; "simd186_loaded_size")] + fn test_validate_transaction_fee_payer_is_nonce(formalize_loaded_transaction_data_size: bool) { let lamports_per_signature = 5000; let rent_collector = RentCollector::default(); let compute_unit_limit = 1000u64; @@ -2421,10 +2458,12 @@ mod tests { let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); - let mock_bank = MockBankCallback { + let mut mock_bank = MockBankCallback { account_shared_data: Arc::new(RwLock::new(mock_accounts)), ..Default::default() }; + mock_bank.feature_set.formalize_loaded_transaction_data_size = + formalize_loaded_transaction_data_size; let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); @@ -2457,6 +2496,12 @@ mod tests { account }; + let base_account_size = if formalize_loaded_transaction_data_size { + TRANSACTION_ACCOUNT_BASE_SIZE + } else { + 0 + }; + assert_eq!( result, Ok(ValidatedTransactionDetails { @@ -2472,7 +2517,7 @@ mod tests { .loaded_accounts_data_size_limit, fee_details: FeeDetails::new(transaction_fee, priority_fee), loaded_fee_payer_account: LoadedTransactionAccount { - loaded_size: fee_payer_account.data().len(), + loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, rent_collected: 0, } diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index d0eae11f560c3f..80768774e7624f 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -4,8 +4,8 @@ use { crate::mock_bank::{ create_custom_loader, 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, }, agave_feature_set::{self as feature_set, FeatureSet}, solana_account::{AccountSharedData, ReadableAccount, WritableAccount, PROGRAM_OWNERS}, @@ -2193,14 +2193,20 @@ fn simd83_fee_payer_deallocate() -> Vec { vec![test_entry] } -fn simd83_account_reallocate() -> Vec { +fn simd83_account_reallocate(formalize_loaded_transaction_data_size: 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(); common_test_entry.add_initial_program(program_name); + if !formalize_loaded_transaction_data_size { + common_test_entry + .disabled_features + .push(feature_set::formalize_loaded_transaction_data_size::id()); + } let fee_payer_keypair = Keypair::new(); let fee_payer = fee_payer_keypair.pubkey(); @@ -2223,11 +2229,20 @@ fn simd83_account_reallocate() -> Vec { let target_start_size = 100; common_test_entry.add_initial_account(target, &mk_target(target_start_size)); + // we set a budget that is enough pre-large-realloc but not enough post-large-realloc + // the relevant feature counts programdata size, so if enabled, we add breathing room + // this test has nothing to do with the feature + let size_budget = Some(if formalize_loaded_transaction_data_size { + (program_size + MAX_PERMITTED_DATA_INCREASE) as u32 + } else { + MAX_PERMITTED_DATA_INCREASE as u32 + }); + let print_transaction = WriteProgramInstruction::Print.create_transaction( program_id, &fee_payer_keypair, target, - Some(MAX_PERMITTED_DATA_INCREASE.try_into().unwrap()), + size_budget, ); common_test_entry.decrease_expected_lamports(&fee_payer, LAMPORTS_PER_SIGNATURE * 2); @@ -2338,7 +2353,8 @@ fn program_cache_update_tombstone() -> Vec { #[test_case(simd83_nonce_reuse(true))] #[test_case(simd83_account_deallocate())] #[test_case(simd83_fee_payer_deallocate())] -#[test_case(simd83_account_reallocate())] +#[test_case(simd83_account_reallocate(false))] +#[test_case(simd83_account_reallocate(true))] #[test_case(program_cache_update_tombstone())] fn svm_integration(test_entries: Vec) { for test_entry in test_entries {