From 25216d2c33489c3c00b08f312600a3f80597acbe Mon Sep 17 00:00:00 2001 From: HaoranYi Date: Wed, 24 Apr 2024 20:32:35 +0000 Subject: [PATCH 1/3] reorder load_account and program_cache replenish --- runtime/src/bank.rs | 11 + .../bank/builtins/core_bpf_migration/mod.rs | 8 +- runtime/src/bank/tests.rs | 16 +- svm/src/account_loader.rs | 478 +++++++++++++++--- svm/src/program_loader.rs | 23 +- svm/src/transaction_processing_callback.rs | 10 +- svm/src/transaction_processor.rs | 366 ++------------ svm/tests/integration_test.rs | 4 + svm/tests/mock_bank.rs | 9 + 9 files changed, 511 insertions(+), 414 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b73a16696f8fcf..fe7c6d50b7fa50 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6735,6 +6735,17 @@ impl TransactionProcessingCallback for Bank { .ok() } + fn load_account_with( + &self, + pubkey: &Pubkey, + callback: impl for<'a> Fn(&'a AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)> { + self.rc + .accounts + .accounts_db + .load_account_with(&self.ancestors, pubkey, callback) + } + fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.rc .accounts diff --git a/runtime/src/bank/builtins/core_bpf_migration/mod.rs b/runtime/src/bank/builtins/core_bpf_migration/mod.rs index 871b173e5fa772..3969f2ef8fc355 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/mod.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/mod.rs @@ -242,7 +242,7 @@ impl Bank { let source = SourceBuffer::new_checked(self, &config.source_buffer_address)?; // Attempt serialization first before modifying the bank. - let new_target_program_account = self.new_target_program_account(&target)?; + let mut new_target_program_account = self.new_target_program_account(&target)?; let new_target_program_data_account = self.new_target_program_data_account(&source, config.upgrade_authority_address)?; @@ -268,6 +268,12 @@ impl Bank { new_target_program_data_account.data(), )?; + // After successfully deployed the `Builtin` program, we need to mark + // the builtin program account as executable. This is required for + // transaction account loading, when "fake program account" optimization + // is disabled. + new_target_program_account.set_executable(true); + // Calculate the lamports to burn. // The target program account will be replaced, so burn its lamports. // The source buffer account will be cleared, so burn its lamports. diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index c4a771e4ad0dda..09c7e5f9b4572d 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7135,12 +7135,10 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { invocation_message.clone(), bank.last_blockhash(), ); + // See comments below for Test buffer invocation. assert_eq!( bank.process_transaction(&transaction), - Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidAccountData - )), + Err(TransactionError::InvalidProgramForExecution), ); { let program_cache = bank.transaction_processor.program_cache.read().unwrap(); @@ -7159,12 +7157,14 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { let instruction = Instruction::new_with_bytes(buffer_address, &[], Vec::new()); let message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); let transaction = Transaction::new(&[&binding], message, bank.last_blockhash()); + // This is a slightly change of behavior for this PR. In the old code, we + // short-circuit buffer account to fake it and mark it executable. So it + // passed program execution check but failed with `Err(InstructionError(0, InvalidAccountData))`. + // With this PR, we don't do that. The buffer account will NOT be executable. A different error is throw `Err(InvalidProgramForExecution)`. + // This will NOT break consensus fortunately! Changing to `Err(InvalidProgramForExecution)` won't affect bank hash! assert_eq!( bank.process_transaction(&transaction), - Err(TransactionError::InstructionError( - 0, - InstructionError::InvalidAccountData, - )), + Err(TransactionError::InvalidProgramForExecution), ); { let program_cache = bank.transaction_processor.program_cache.read().unwrap(); diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index c62963c09e0934..950e33df499327 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -5,11 +5,9 @@ use { transaction_error_metrics::TransactionErrorMetrics, transaction_processing_callback::TransactionProcessingCallback, }, - itertools::Itertools, solana_compute_budget::compute_budget_processor::{ process_compute_budget_instructions, ComputeBudgetLimits, }, - solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch}, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, feature_set::{self, FeatureSet}, @@ -27,7 +25,10 @@ use { transaction_context::{IndexOfAccount, TransactionAccount}, }, solana_system_program::{get_system_account_kind, SystemAccountKind}, - std::num::NonZeroUsize, + std::{ + collections::{hash_map::Entry, HashMap}, + num::NonZeroUsize, + }, }; // for the load instructions @@ -154,6 +155,7 @@ pub fn validate_fee_payer( /// batch. Each tuple contains struct of information about accounts as /// its first element and an optional transaction nonce info as its /// second element. +/// This function also populate program_accounts map. pub(crate) fn load_accounts( callbacks: &CB, txs: &[SanitizedTransaction], @@ -162,7 +164,8 @@ pub(crate) fn load_accounts( account_overrides: Option<&AccountOverrides>, feature_set: &FeatureSet, rent_collector: &RentCollector, - loaded_programs: &ProgramCacheForTxBatch, + program_owners: &[Pubkey], + program_accounts: &mut HashMap, ) -> Vec { txs.iter() .zip(validation_results) @@ -179,7 +182,8 @@ pub(crate) fn load_accounts( account_overrides, feature_set, rent_collector, - loaded_programs, + program_owners, + program_accounts, ) } (_, Err(e)) => Err(e), @@ -195,7 +199,8 @@ fn load_transaction_accounts( account_overrides: Option<&AccountOverrides>, feature_set: &FeatureSet, rent_collector: &RentCollector, - loaded_programs: &ProgramCacheForTxBatch, + program_owners: &[Pubkey], + program_accounts: &mut HashMap, ) -> Result { let mut tx_rent: TransactionRent = 0; let account_keys = message.account_keys(); @@ -206,13 +211,6 @@ fn load_transaction_accounts( get_requested_loaded_accounts_data_size_limit(message)?; let mut accumulated_accounts_data_size: usize = 0; - let instruction_accounts = message - .instructions() - .iter() - .flat_map(|instruction| &instruction.accounts) - .unique() - .collect::>(); - let mut accounts = account_keys .iter() .enumerate() @@ -223,9 +221,6 @@ fn load_transaction_accounts( construct_instructions_account(message) } else { let is_fee_payer = i == 0; - let instruction_account = u8::try_from(i) - .map(|i| instruction_accounts.contains(&&i)) - .unwrap_or(false); let (account_size, account, rent) = if is_fee_payer { ( tx_details.fee_payer_account.data().len(), @@ -236,22 +231,26 @@ fn load_transaction_accounts( account_overrides.and_then(|overrides| overrides.get(key)) { (account_override.data().len(), account_override.clone(), 0) - } else if let Some(program) = (!instruction_account && !message.is_writable(i)) - .then_some(()) - .and_then(|_| loaded_programs.find(key)) - { - callbacks - .get_account_shared_data(key) - .ok_or(TransactionError::AccountNotFound)?; - // Optimization to skip loading of accounts which are only used as - // programs in top-level instructions and not passed as instruction accounts. - let program_account = account_shared_data_from_program(&program); - (program.account_size, program_account, 0) } else { + let is_account_writable = message.is_writable(i); callbacks - .get_account_shared_data(key) - .map(|mut account| { - if message.is_writable(i) { + .load_account_with(key, |_account| { + // cache the account in read-only cache + true + }) + .map(|(mut account, _slot)| { + if program_owners.contains(account.owner()) { + match program_accounts.entry(*key) { + Entry::Vacant(entry) => { + entry.insert(1); + } + Entry::Occupied(mut entry) => { + let count = entry.get_mut(); + saturating_add_assign!(*count, 1); + } + } + } + if is_account_writable { let rent_due = collect_rent_from_account( feature_set, rent_collector, @@ -391,16 +390,6 @@ fn get_requested_loaded_accounts_data_size_limit( ) } -fn account_shared_data_from_program(loaded_program: &ProgramCacheEntry) -> AccountSharedData { - // It's an executable program account. The program is already loaded in the cache. - // So the account data is not needed. Return a dummy AccountSharedData with meta - // information. - let mut program_account = AccountSharedData::default(); - program_account.set_owner(loaded_program.account_owner()); - program_account.set_executable(true); - program_account -} - /// Accumulate loaded account data size into `accumulated_accounts_data_size`. /// Returns TransactionErr::MaxLoadedAccountsDataSizeExceeded if /// `requested_loaded_accounts_data_size_limit` is specified and @@ -442,10 +431,10 @@ mod tests { }, nonce::state::Versions as NonceVersions, solana_compute_budget::{compute_budget::ComputeBudget, compute_budget_processor}, - solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch}, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, bpf_loader_upgradeable, + clock::Slot, epoch_schedule::EpochSchedule, feature_set::FeatureSet, hash::Hash, @@ -467,7 +456,7 @@ mod tests { transaction::{Result, SanitizedTransaction, Transaction, TransactionError}, transaction_context::{TransactionAccount, TransactionContext}, }, - std::{borrow::Cow, collections::HashMap, convert::TryFrom, sync::Arc}, + std::{borrow::Cow, collections::HashMap, convert::TryFrom}, }; #[derive(Default)] @@ -480,6 +469,15 @@ mod tests { None } + fn load_account_with( + &self, + pubkey: &Pubkey, + _callback: impl for<'a> Fn(&'a AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)> { + let account = self.accounts_map.get(pubkey).cloned()?; + Some((account, 100)) + } + fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.accounts_map.get(pubkey).cloned() } @@ -500,6 +498,7 @@ mod tests { accounts_map.insert(*pubkey, account.clone()); } let callbacks = TestCallbacks { accounts_map }; + let mut program_accounts = HashMap::default(); load_accounts( &callbacks, &[sanitized_tx], @@ -511,7 +510,8 @@ mod tests { None, feature_set, rent_collector, - &ProgramCacheForTxBatch::default(), + &[], + &mut program_accounts, ) } @@ -788,6 +788,8 @@ mod tests { accounts_map.insert(*pubkey, account.clone()); } let callbacks = TestCallbacks { accounts_map }; + + let mut program_accounts = HashMap::default(); load_accounts( &callbacks, &[tx], @@ -796,7 +798,8 @@ mod tests { account_overrides, &FeatureSet::all_enabled(), &RentCollector::default(), - &ProgramCacheForTxBatch::default(), + &[], + &mut program_accounts, ) } @@ -1202,7 +1205,6 @@ mod tests { .insert(key1.pubkey(), fee_payer_account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1220,7 +1222,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1267,8 +1270,6 @@ mod tests { mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let mut loaded_programs = ProgramCacheForTxBatch::default(); - loaded_programs.replenish(key2.pubkey(), Arc::new(ProgramCacheEntry::default())); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1283,10 +1284,17 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); - assert_eq!(result.err(), Some(TransactionError::AccountNotFound)); + // The original code throws "AccountNotFound" error. With this PR, it + // throws "ProgramAccountNotFound" error. But I don't think this is a + // concern for breaking consensus. As the original "AccountNotFound" + // error should not be triggered in real world. When we can find "key" + // in program_cache, which is after replenish, then it must be in + // accounts-db. + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); } #[test] @@ -1312,7 +1320,6 @@ mod tests { mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1327,7 +1334,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1356,7 +1364,6 @@ mod tests { mock_bank.accounts_map.insert(key1.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1371,7 +1378,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1409,7 +1417,6 @@ mod tests { .accounts_map .insert(key2.pubkey(), fee_payer_account_data.clone()); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1427,7 +1434,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1477,7 +1485,6 @@ mod tests { account_data.set_lamports(200); mock_bank.accounts_map.insert(key2.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1492,7 +1499,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1530,7 +1538,6 @@ mod tests { .accounts_map .insert(key3.pubkey(), AccountSharedData::default()); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1545,7 +1552,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1590,7 +1598,6 @@ mod tests { mock_bank.accounts_map.insert(key3.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1608,7 +1615,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1680,7 +1688,6 @@ mod tests { mock_bank.accounts_map.insert(key3.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1698,7 +1705,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); let mut account_data = AccountSharedData::default(); @@ -1766,7 +1774,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &ProgramCacheForTxBatch::default(), + &[], + &mut HashMap::default(), ); let compute_budget = ComputeBudget::new(u64::from( @@ -1834,7 +1843,6 @@ mod tests { mock_bank.accounts_map.insert(key3.pubkey(), account_data); let mut error_metrics = TransactionErrorMetrics::default(); - let loaded_programs = ProgramCacheForTxBatch::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( sanitized_message, @@ -1854,7 +1862,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); let mut account_data = AccountSharedData::default(); @@ -1924,7 +1933,8 @@ mod tests { None, &feature_set, &rent_collector, - &ProgramCacheForTxBatch::default(), + &[], + &mut HashMap::default(), ); assert_eq!( @@ -1942,7 +1952,8 @@ mod tests { None, &feature_set, &rent_collector, - &ProgramCacheForTxBatch::default(), + &[], + &mut HashMap::default(), ); assert_eq!(result, vec![Err(TransactionError::InvalidWritableAccount)]); @@ -2018,4 +2029,333 @@ mod tests { assert_eq!(account.rent_epoch(), 0); assert_eq!(account.lamports(), 0); } + + #[test] + fn test_load_transaction_accounts_populate_program_accounts_with_filter_executable_program_account( + ) { + let mut mock_bank = TestCallbacks::default(); + + let fee_keypair = Keypair::new(); + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); + mock_bank + .accounts_map + .insert(fee_keypair.pubkey(), fee_payer_account_data.clone()); + let fee_key = fee_keypair.pubkey(); + + let key1 = Pubkey::new_unique(); + let owner1 = Pubkey::new_unique(); + + let mut data = AccountSharedData::default(); + data.set_owner(owner1); + data.set_lamports(93); + mock_bank.accounts_map.insert(key1, data); + + let message = Message { + account_keys: vec![fee_key, key1], + header: MessageHeader::default(), + instructions: vec![CompiledInstruction { + program_id_index: 0, + accounts: vec![], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + let sanitized_message = new_unchecked_sanitized_message(message); + + let sanitized_transaction_1 = SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + ); + + let key2 = Pubkey::new_unique(); + let owner2 = Pubkey::new_unique(); + + let mut account_data = AccountSharedData::default(); + account_data.set_owner(owner2); + account_data.set_lamports(250); + mock_bank.accounts_map.insert(key2, account_data); + + let message = Message { + account_keys: vec![fee_key, key1, key2], + header: MessageHeader::default(), + instructions: vec![CompiledInstruction { + program_id_index: 0, + accounts: vec![], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + let sanitized_message = new_unchecked_sanitized_message(message); + + let sanitized_transaction_2 = SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + ); + + let transactions = vec![ + sanitized_transaction_1.clone(), + sanitized_transaction_2.clone(), + sanitized_transaction_2, + sanitized_transaction_1, + ]; + let validated_results = vec![ + Ok(ValidatedTransactionDetails::default()), + Ok(ValidatedTransactionDetails::default()), + Err(TransactionError::BlockhashNotFound), + Err(TransactionError::ProgramAccountNotFound), + ]; + let owners = vec![owner1, owner2]; + + let mut program_accounts = HashMap::default(); + let mut error_metrics = TransactionErrorMetrics::default(); + + let results = load_accounts( + &mock_bank, + &transactions, + validated_results, + &mut error_metrics, + None, + &FeatureSet::default(), + &RentCollector::default(), + &owners, + &mut program_accounts, + ); + + assert_eq!(results[2], Err(TransactionError::BlockhashNotFound)); + assert_eq!(results[3], Err(TransactionError::ProgramAccountNotFound)); + + assert_eq!(program_accounts.len(), 2); + assert_eq!(program_accounts[&key1], 2); + assert_eq!(program_accounts[&key2], 1); + } + + #[test] + fn test_load_transaction_accounts_populate_program_account_with_filter_executable_program_account_no_errors( + ) { + let mut mock_bank = TestCallbacks::default(); + + let keypair1 = Keypair::new(); + let keypair2 = Keypair::new(); + + let non_program_pubkey1 = Pubkey::new_unique(); + let non_program_pubkey2 = Pubkey::new_unique(); + let program1_pubkey = Pubkey::new_unique(); + let program2_pubkey = Pubkey::new_unique(); + let account1_pubkey = Pubkey::new_unique(); + let account2_pubkey = Pubkey::new_unique(); + let account3_pubkey = Pubkey::new_unique(); + let account4_pubkey = Pubkey::new_unique(); + let account5_pubkey = Pubkey::new_unique(); + + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); + mock_bank + .accounts_map + .insert(keypair1.pubkey(), fee_payer_account_data.clone()); + + mock_bank + .accounts_map + .insert(keypair2.pubkey(), fee_payer_account_data.clone()); + + mock_bank.accounts_map.insert( + non_program_pubkey1, + AccountSharedData::new(1, 10, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + non_program_pubkey2, + AccountSharedData::new(1, 10, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + program1_pubkey, + AccountSharedData::new(40, 1, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + program2_pubkey, + AccountSharedData::new(40, 1, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + account1_pubkey, + AccountSharedData::new(1, 10, &non_program_pubkey1), + ); + mock_bank.accounts_map.insert( + account2_pubkey, + AccountSharedData::new(1, 10, &non_program_pubkey2), + ); + mock_bank.accounts_map.insert( + account3_pubkey, + AccountSharedData::new(40, 1, &program1_pubkey), + ); + mock_bank.accounts_map.insert( + account4_pubkey, + AccountSharedData::new(40, 1, &program2_pubkey), + ); + + let tx1 = Transaction::new_with_compiled_instructions( + &[&keypair1], + &[non_program_pubkey1], + Hash::new_unique(), + vec![account1_pubkey, account2_pubkey, account3_pubkey], + vec![CompiledInstruction::new(1, &(), vec![0])], + ); + let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1); + + let tx2 = Transaction::new_with_compiled_instructions( + &[&keypair2], + &[non_program_pubkey2], + Hash::new_unique(), + vec![account4_pubkey, account3_pubkey, account2_pubkey], + vec![CompiledInstruction::new(1, &(), vec![0])], + ); + let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2); + + let owners = [program1_pubkey, program2_pubkey]; + + let mut program_accounts = HashMap::default(); + let mut error_metrics = TransactionErrorMetrics::default(); + + let _results = load_accounts( + &mock_bank, + &[sanitized_tx1, sanitized_tx2], + vec![ + Ok(ValidatedTransactionDetails::default()), + Ok(ValidatedTransactionDetails::default()), + ], + &mut error_metrics, + None, + &FeatureSet::default(), + &RentCollector::default(), + &owners, + &mut program_accounts, + ); + + // The result should contain only account3_pubkey, and account4_pubkey as the program accounts + assert_eq!(program_accounts.len(), 2); + assert_eq!( + program_accounts + .get(&account3_pubkey) + .expect("failed to find the program account"), + &2 + ); + assert_eq!( + program_accounts + .get(&account4_pubkey) + .expect("failed to find the program account"), + &1 + ); + } + + #[test] + fn test_load_transaction_accounts_filter_executable_program_account_invalid_blockhash() { + let mut mock_bank = TestCallbacks::default(); + + let keypair1 = Keypair::new(); + let keypair2 = Keypair::new(); + + let non_program_pubkey1 = Pubkey::new_unique(); + let non_program_pubkey2 = Pubkey::new_unique(); + let program1_pubkey = Pubkey::new_unique(); + let program2_pubkey = Pubkey::new_unique(); + let account1_pubkey = Pubkey::new_unique(); + let account2_pubkey = Pubkey::new_unique(); + let account3_pubkey = Pubkey::new_unique(); + let account4_pubkey = Pubkey::new_unique(); + let account5_pubkey = Pubkey::new_unique(); + + let mut fee_payer_account_data = AccountSharedData::default(); + fee_payer_account_data.set_lamports(200); + mock_bank + .accounts_map + .insert(keypair1.pubkey(), fee_payer_account_data.clone()); + + mock_bank + .accounts_map + .insert(keypair2.pubkey(), fee_payer_account_data.clone()); + + mock_bank.accounts_map.insert( + non_program_pubkey1, + AccountSharedData::new(1, 10, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + non_program_pubkey2, + AccountSharedData::new(1, 10, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + program1_pubkey, + AccountSharedData::new(40, 1, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + program2_pubkey, + AccountSharedData::new(40, 1, &account5_pubkey), + ); + mock_bank.accounts_map.insert( + account1_pubkey, + AccountSharedData::new(1, 10, &non_program_pubkey1), + ); + mock_bank.accounts_map.insert( + account2_pubkey, + AccountSharedData::new(1, 10, &non_program_pubkey2), + ); + mock_bank.accounts_map.insert( + account3_pubkey, + AccountSharedData::new(40, 1, &program1_pubkey), + ); + mock_bank.accounts_map.insert( + account4_pubkey, + AccountSharedData::new(40, 1, &program2_pubkey), + ); + + let tx1 = Transaction::new_with_compiled_instructions( + &[&keypair1], + &[non_program_pubkey1], + Hash::new_unique(), + vec![account1_pubkey, account2_pubkey, account3_pubkey], + vec![CompiledInstruction::new(1, &(), vec![0])], + ); + let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1); + + let tx2 = Transaction::new_with_compiled_instructions( + &[&keypair2], + &[non_program_pubkey2], + Hash::new_unique(), + vec![account4_pubkey, account3_pubkey, account2_pubkey], + vec![CompiledInstruction::new(1, &(), vec![0])], + ); + // Let's not register blockhash from tx2. This should cause the tx2 to fail + let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2); + + let owners = [program1_pubkey, program2_pubkey]; + + let mut program_accounts = HashMap::default(); + let mut error_metrics = TransactionErrorMetrics::default(); + + let results = load_accounts( + &mock_bank, + &[sanitized_tx1, sanitized_tx2], + vec![ + Ok(ValidatedTransactionDetails::default()), + Err(TransactionError::BlockhashNotFound), + ], + &mut error_metrics, + None, + &FeatureSet::default(), + &RentCollector::default(), + &owners, + &mut program_accounts, + ); + + // The result should contain only account3_pubkey as the program accounts + assert_eq!(program_accounts.len(), 1); + assert_eq!( + program_accounts + .get(&account3_pubkey) + .expect("failed to find the program account"), + &1 + ); + assert_eq!(results[1], Err(TransactionError::BlockhashNotFound)); + } } diff --git a/svm/src/program_loader.rs b/svm/src/program_loader.rs index f77780fc9fecfa..2e0fd85fdd034b 100644 --- a/svm/src/program_loader.rs +++ b/svm/src/program_loader.rs @@ -69,8 +69,8 @@ pub(crate) fn load_program_accounts( callbacks: &CB, pubkey: &Pubkey, ) -> Option { - let program_account = callbacks.get_account_shared_data(pubkey)?; - + // load program account by `pubkey` and don't insert the program account into read cache. + let (program_account, _slot) = callbacks.load_account_with(pubkey, |_| false)?; if loader_v4::check_id(program_account.owner()) { return Some( solana_loader_v4_program::get_state(program_account.data()) @@ -93,11 +93,19 @@ pub(crate) fn load_program_accounts( return Some(ProgramAccountLoadResult::ProgramOfLoaderV2(program_account)); } + assert!( + bpf_loader_upgradeable::check_id(program_account.owner()), + "unexpected program account owner {}", + program_account.owner(), + ); + if let Ok(UpgradeableLoaderState::Program { programdata_address, }) = program_account.state() { - if let Some(programdata_account) = callbacks.get_account_shared_data(&programdata_address) { + if let Some((programdata_account, _slot)) = + callbacks.load_account_with(&programdata_address, |_| false) + { if let Ok(UpgradeableLoaderState::ProgramData { slot, upgrade_authority_address: _, @@ -300,6 +308,15 @@ mod tests { } } + fn load_account_with( + &self, + pubkey: &Pubkey, + _callback: impl FnMut(&AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)> { + let account = self.account_shared_data.borrow().get(pubkey).cloned()?; + Some((account, 100)) + } + fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.account_shared_data.borrow().get(pubkey).cloned() } diff --git a/svm/src/transaction_processing_callback.rs b/svm/src/transaction_processing_callback.rs index 760a6606568798..2aaa70e9c4bde0 100644 --- a/svm/src/transaction_processing_callback.rs +++ b/svm/src/transaction_processing_callback.rs @@ -1,4 +1,4 @@ -use solana_sdk::{account::AccountSharedData, pubkey::Pubkey}; +use solana_sdk::{account::AccountSharedData, clock::Slot, pubkey::Pubkey}; /// Runtime callbacks for transaction processing. pub trait TransactionProcessingCallback { @@ -6,5 +6,13 @@ pub trait TransactionProcessingCallback { fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option; + // If `callback` returns `true`, the account will be stored into read cache after loading. + // Otherwise, it will skip read cache. + fn load_account_with( + &self, + pubkey: &Pubkey, + callback: impl for<'a> Fn(&'a AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)>; + fn add_builtin_account(&self, _name: &str, _program_id: &Pubkey) {} } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 73afc8cac772db..3cf6e191d23c6a 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -55,7 +55,7 @@ use { solana_vote::vote_account::VoteAccountsHashMap, std::{ cell::RefCell, - collections::{hash_map::Entry, HashMap, HashSet}, + collections::{HashMap, HashSet}, fmt::{Debug, Formatter}, rc::Rc, }, @@ -248,17 +248,28 @@ impl TransactionBatchProcessor { &mut error_metrics )); - let mut program_cache_time = Measure::start("program_cache"); - let mut program_accounts_map = Self::filter_executable_program_accounts( + let mut load_time = Measure::start("accounts_load"); + let mut program_accounts_map: HashMap = HashMap::new(); + let mut loaded_transactions = load_accounts( callbacks, sanitized_txs, - &validation_results, + validation_results, + &mut error_metrics, + config.account_overrides, + &environment.feature_set, + environment + .rent_collector + .unwrap_or(&RentCollector::default()), PROGRAM_OWNERS, + &mut program_accounts_map, ); + load_time.stop(); + for builtin_program in self.builtin_program_ids.read().unwrap().iter() { program_accounts_map.insert(*builtin_program, 0); } + let mut program_cache_time = Measure::start("program_cache"); let program_cache_for_tx_batch = Rc::new(RefCell::new(self.replenish_program_cache( callbacks, &program_accounts_map, @@ -280,21 +291,6 @@ impl TransactionBatchProcessor { } program_cache_time.stop(); - let mut load_time = Measure::start("accounts_load"); - let mut loaded_transactions = load_accounts( - callbacks, - sanitized_txs, - validation_results, - &mut error_metrics, - config.account_overrides, - &environment.feature_set, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), - &program_cache_for_tx_batch.borrow(), - ); - load_time.stop(); - let mut execution_time = Measure::start("execution_time"); let execution_results: Vec = loaded_transactions @@ -488,39 +484,6 @@ impl TransactionBatchProcessor { }) } - /// Returns a map from executable program accounts (all accounts owned by any loader) - /// to their usage counters, for the transactions with a valid blockhash or nonce. - fn filter_executable_program_accounts( - callbacks: &CB, - txs: &[SanitizedTransaction], - validation_results: &[TransactionValidationResult], - program_owners: &[Pubkey], - ) -> HashMap { - let mut result: HashMap = HashMap::new(); - validation_results.iter().zip(txs).for_each(|etx| { - if let (Ok(_), tx) = etx { - tx.message() - .account_keys() - .iter() - .for_each(|key| match result.entry(*key) { - Entry::Occupied(mut entry) => { - let count = entry.get_mut(); - saturating_add_assign!(*count, 1); - } - Entry::Vacant(entry) => { - if callbacks - .account_matches_owners(key, program_owners) - .is_some() - { - entry.insert(1); - } - } - }); - } - }); - result - } - fn replenish_program_cache( &self, callback: &CB, @@ -567,6 +530,7 @@ impl TransactionBatchProcessor { let program_to_store = program_to_load.map(|(key, count)| { // Load, verify and compile one program. + let program = load_program_with_pubkey( callback, &program_cache.get_environments_for_epoch(self.epoch), @@ -1006,10 +970,10 @@ mod tests { rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, rent_debits::RentDebits, reserved_account_keys::ReservedAccountKeys, - signature::{Keypair, Signature}, + signature::Signature, system_program, sysvar::{self, rent::Rent}, - transaction::{SanitizedTransaction, Transaction, TransactionError}, + transaction::SanitizedTransaction, transaction_context::TransactionContext, }, std::{ @@ -1053,6 +1017,20 @@ mod tests { } } + fn load_account_with( + &self, + pubkey: &Pubkey, + _callback: impl FnMut(&AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)> { + let account = self + .account_shared_data + .read() + .unwrap() + .get(pubkey) + .cloned()?; + Some((account, 100)) + } + fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.account_shared_data .read() @@ -1313,7 +1291,8 @@ mod tests { let mut account_maps: HashMap = HashMap::new(); account_maps.insert(key, 4); - batch_processor.replenish_program_cache(&mock_bank, &account_maps, false, true); + let _cache = + batch_processor.replenish_program_cache(&mock_bank, &account_maps, false, true); } #[test] @@ -1352,283 +1331,6 @@ mod tests { } } - #[test] - fn test_filter_executable_program_accounts() { - let mock_bank = MockBankCallback::default(); - let key1 = Pubkey::new_unique(); - let owner1 = Pubkey::new_unique(); - - let mut data = AccountSharedData::default(); - data.set_owner(owner1); - data.set_lamports(93); - mock_bank - .account_shared_data - .write() - .unwrap() - .insert(key1, data); - - let message = Message { - account_keys: vec![key1], - header: MessageHeader::default(), - instructions: vec![CompiledInstruction { - program_id_index: 0, - accounts: vec![], - data: vec![], - }], - recent_blockhash: Hash::default(), - }; - - let sanitized_message = new_unchecked_sanitized_message(message); - - let sanitized_transaction_1 = SanitizedTransaction::new_for_tests( - sanitized_message, - vec![Signature::new_unique()], - false, - ); - - let key2 = Pubkey::new_unique(); - let owner2 = Pubkey::new_unique(); - - let mut account_data = AccountSharedData::default(); - account_data.set_owner(owner2); - account_data.set_lamports(90); - mock_bank - .account_shared_data - .write() - .unwrap() - .insert(key2, account_data); - - let message = Message { - account_keys: vec![key1, key2], - header: MessageHeader::default(), - instructions: vec![CompiledInstruction { - program_id_index: 0, - accounts: vec![], - data: vec![], - }], - recent_blockhash: Hash::default(), - }; - - let sanitized_message = new_unchecked_sanitized_message(message); - - let sanitized_transaction_2 = SanitizedTransaction::new_for_tests( - sanitized_message, - vec![Signature::new_unique()], - false, - ); - - let transactions = vec![ - sanitized_transaction_1.clone(), - sanitized_transaction_2.clone(), - sanitized_transaction_1, - ]; - let validation_results = vec![ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), - Err(TransactionError::ProgramAccountNotFound), - ]; - let owners = vec![owner1, owner2]; - - let result = TransactionBatchProcessor::::filter_executable_program_accounts( - &mock_bank, - &transactions, - &validation_results, - &owners, - ); - - assert_eq!(result.len(), 2); - assert_eq!(result[&key1], 2); - assert_eq!(result[&key2], 1); - } - - #[test] - fn test_filter_executable_program_accounts_no_errors() { - let keypair1 = Keypair::new(); - let keypair2 = Keypair::new(); - - let non_program_pubkey1 = Pubkey::new_unique(); - let non_program_pubkey2 = Pubkey::new_unique(); - let program1_pubkey = Pubkey::new_unique(); - let program2_pubkey = Pubkey::new_unique(); - let account1_pubkey = Pubkey::new_unique(); - let account2_pubkey = Pubkey::new_unique(); - let account3_pubkey = Pubkey::new_unique(); - let account4_pubkey = Pubkey::new_unique(); - - let account5_pubkey = Pubkey::new_unique(); - - let bank = MockBankCallback::default(); - bank.account_shared_data.write().unwrap().insert( - non_program_pubkey1, - AccountSharedData::new(1, 10, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - non_program_pubkey2, - AccountSharedData::new(1, 10, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - program1_pubkey, - AccountSharedData::new(40, 1, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - program2_pubkey, - AccountSharedData::new(40, 1, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - account1_pubkey, - AccountSharedData::new(1, 10, &non_program_pubkey1), - ); - bank.account_shared_data.write().unwrap().insert( - account2_pubkey, - AccountSharedData::new(1, 10, &non_program_pubkey2), - ); - bank.account_shared_data.write().unwrap().insert( - account3_pubkey, - AccountSharedData::new(40, 1, &program1_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - account4_pubkey, - AccountSharedData::new(40, 1, &program2_pubkey), - ); - - let tx1 = Transaction::new_with_compiled_instructions( - &[&keypair1], - &[non_program_pubkey1], - Hash::new_unique(), - vec![account1_pubkey, account2_pubkey, account3_pubkey], - vec![CompiledInstruction::new(1, &(), vec![0])], - ); - let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1); - - let tx2 = Transaction::new_with_compiled_instructions( - &[&keypair2], - &[non_program_pubkey2], - Hash::new_unique(), - vec![account4_pubkey, account3_pubkey, account2_pubkey], - vec![CompiledInstruction::new(1, &(), vec![0])], - ); - let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2); - - let owners = &[program1_pubkey, program2_pubkey]; - let programs = - TransactionBatchProcessor::::filter_executable_program_accounts( - &bank, - &[sanitized_tx1, sanitized_tx2], - &[ - Ok(ValidatedTransactionDetails::default()), - Ok(ValidatedTransactionDetails::default()), - ], - owners, - ); - - // The result should contain only account3_pubkey, and account4_pubkey as the program accounts - assert_eq!(programs.len(), 2); - assert_eq!( - programs - .get(&account3_pubkey) - .expect("failed to find the program account"), - &2 - ); - assert_eq!( - programs - .get(&account4_pubkey) - .expect("failed to find the program account"), - &1 - ); - } - - #[test] - fn test_filter_executable_program_accounts_invalid_blockhash() { - let keypair1 = Keypair::new(); - let keypair2 = Keypair::new(); - - let non_program_pubkey1 = Pubkey::new_unique(); - let non_program_pubkey2 = Pubkey::new_unique(); - let program1_pubkey = Pubkey::new_unique(); - let program2_pubkey = Pubkey::new_unique(); - let account1_pubkey = Pubkey::new_unique(); - let account2_pubkey = Pubkey::new_unique(); - let account3_pubkey = Pubkey::new_unique(); - let account4_pubkey = Pubkey::new_unique(); - - let account5_pubkey = Pubkey::new_unique(); - - let bank = MockBankCallback::default(); - bank.account_shared_data.write().unwrap().insert( - non_program_pubkey1, - AccountSharedData::new(1, 10, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - non_program_pubkey2, - AccountSharedData::new(1, 10, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - program1_pubkey, - AccountSharedData::new(40, 1, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - program2_pubkey, - AccountSharedData::new(40, 1, &account5_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - account1_pubkey, - AccountSharedData::new(1, 10, &non_program_pubkey1), - ); - bank.account_shared_data.write().unwrap().insert( - account2_pubkey, - AccountSharedData::new(1, 10, &non_program_pubkey2), - ); - bank.account_shared_data.write().unwrap().insert( - account3_pubkey, - AccountSharedData::new(40, 1, &program1_pubkey), - ); - bank.account_shared_data.write().unwrap().insert( - account4_pubkey, - AccountSharedData::new(40, 1, &program2_pubkey), - ); - - let tx1 = Transaction::new_with_compiled_instructions( - &[&keypair1], - &[non_program_pubkey1], - Hash::new_unique(), - vec![account1_pubkey, account2_pubkey, account3_pubkey], - vec![CompiledInstruction::new(1, &(), vec![0])], - ); - let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1); - - let tx2 = Transaction::new_with_compiled_instructions( - &[&keypair2], - &[non_program_pubkey2], - Hash::new_unique(), - vec![account4_pubkey, account3_pubkey, account2_pubkey], - vec![CompiledInstruction::new(1, &(), vec![0])], - ); - // Let's not register blockhash from tx2. This should cause the tx2 to fail - let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2); - - let owners = &[program1_pubkey, program2_pubkey]; - let validation_results = vec![ - Ok(ValidatedTransactionDetails::default()), - Err(TransactionError::BlockhashNotFound), - ]; - let programs = - TransactionBatchProcessor::::filter_executable_program_accounts( - &bank, - &[sanitized_tx1, sanitized_tx2], - &validation_results, - owners, - ); - - // The result should contain only account3_pubkey as the program accounts - assert_eq!(programs.len(), 1); - assert_eq!( - programs - .get(&account3_pubkey) - .expect("failed to find the program account"), - &1 - ); - } - #[test] #[allow(deprecated)] fn test_sysvar_cache_initialization1() { diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 5c5d74f984c8ec..f70085fedb990c 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -192,6 +192,10 @@ fn deploy_program(name: String, mock_bank: &mut MockBankCallback) -> Pubkey { account_data.set_data(bincode::serialize(&state).unwrap()); account_data.set_lamports(25); account_data.set_owner(bpf_loader_upgradeable::id()); + // We are no longer faking executable accounts. Executable accounts will be + // loaded from accounts-db, which would require executable flag to be + // marked. + account_data.set_executable(true); mock_bank .account_shared_data .borrow_mut() diff --git a/svm/tests/mock_bank.rs b/svm/tests/mock_bank.rs index f67b63346e95b7..022db0943d08a9 100644 --- a/svm/tests/mock_bank.rs +++ b/svm/tests/mock_bank.rs @@ -47,6 +47,15 @@ impl TransactionProcessingCallback for MockBankCallback { } } + fn load_account_with( + &self, + pubkey: &Pubkey, + _callback: impl for<'a> Fn(&'a AccountSharedData) -> bool, + ) -> Option<(AccountSharedData, Slot)> { + let account = self.account_shared_data.borrow().get(pubkey).cloned()?; + Some((account, 100)) + } + fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { self.account_shared_data.borrow().get(pubkey).cloned() } From 07aea8f7b70b811714eb0c4ae1cbfa86eb676c2f Mon Sep 17 00:00:00 2001 From: HaoranYi Date: Mon, 17 Jun 2024 20:25:00 +0000 Subject: [PATCH 2/3] fix a test --- svm/src/program_loader.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/svm/src/program_loader.rs b/svm/src/program_loader.rs index 2e0fd85fdd034b..130a28a00226c2 100644 --- a/svm/src/program_loader.rs +++ b/svm/src/program_loader.rs @@ -11,7 +11,7 @@ use { account::{AccountSharedData, ReadableAccount}, account_utils::StateMut, bpf_loader, bpf_loader_deprecated, - bpf_loader_upgradeable::{self, UpgradeableLoaderState}, + bpf_loader_upgradeable::UpgradeableLoaderState, clock::Slot, instruction::InstructionError, loader_v4::{self, LoaderV4State, LoaderV4Status}, @@ -93,12 +93,6 @@ pub(crate) fn load_program_accounts( return Some(ProgramAccountLoadResult::ProgramOfLoaderV2(program_account)); } - assert!( - bpf_loader_upgradeable::check_id(program_account.owner()), - "unexpected program account owner {}", - program_account.owner(), - ); - if let Ok(UpgradeableLoaderState::Program { programdata_address, }) = program_account.state() From 0ac1a615ef80395320f3f41dd6fa3eb37cbabb02 Mon Sep 17 00:00:00 2001 From: HaoranYi Date: Fri, 21 Jun 2024 14:21:15 +0000 Subject: [PATCH 3/3] rebase fixes --- svm/src/account_loader.rs | 4 ++-- svm/src/program_loader.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 950e33df499327..9010a0f5112839 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -1137,7 +1137,6 @@ mod tests { 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, @@ -1156,7 +1155,8 @@ mod tests { None, &FeatureSet::default(), &RentCollector::default(), - &loaded_programs, + &[], + &mut HashMap::default(), ); let expected_rent_debits = { diff --git a/svm/src/program_loader.rs b/svm/src/program_loader.rs index 130a28a00226c2..06cb70a9b55e0c 100644 --- a/svm/src/program_loader.rs +++ b/svm/src/program_loader.rs @@ -11,7 +11,7 @@ use { account::{AccountSharedData, ReadableAccount}, account_utils::StateMut, bpf_loader, bpf_loader_deprecated, - bpf_loader_upgradeable::UpgradeableLoaderState, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Slot, instruction::InstructionError, loader_v4::{self, LoaderV4State, LoaderV4Status},