From 55c05c5ea555ad52a059de9f5992a451c4fd5331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 5 Apr 2024 13:03:18 +0200 Subject: [PATCH] Fix - `FailedVerification` and `Closed` tombstones (#419) * Only the verifier can cause FailedVerification, everything else is Closed * Removes the environments parameter from load_program_accounts(). * cargo fmt * Simplify invocation of deployed program * Attempt to invoke a program before it is deployed * Attempt to invoke a buffer before it is used in a deployment * Escalates Option return value of load_program_accounts() to load_program_with_pubkey(). * Review feedback --- ledger-tool/src/program.rs | 6 +- program-runtime/src/loaded_programs.rs | 25 +- runtime/src/bank.rs | 25 +- runtime/src/bank/tests.rs | 82 ++++--- svm/src/transaction_processor.rs | 318 ++++++++++++------------- 5 files changed, 244 insertions(+), 212 deletions(-) diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index d4ecb0d469..7a5f5cc649 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -522,7 +522,11 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { let mut loaded_programs = bank.new_program_cache_for_tx_batch_for_slot(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET); for key in cached_account_keys { - loaded_programs.replenish(key, bank.load_program(&key, false, bank.epoch())); + loaded_programs.replenish( + key, + bank.load_program(&key, false, bank.epoch()) + .expect("Couldn't find program account"), + ); debug!("Loaded program {}", key); } invoke_context.programs_loaded_for_tx_batch = &loaded_programs; diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 99c77bf943..419ae7330b 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -63,11 +63,11 @@ pub trait ForkGraph { /// Actual payload of [LoadedProgram]. #[derive(Default)] pub enum LoadedProgramType { - /// Tombstone for programs which did not pass the verifier. - /// - /// These can potentially come back alive if the environment changes. + /// Tombstone for programs which currently do not pass the verifier but could if the feature set changed. FailedVerification(ProgramRuntimeEnvironment), - /// Tombstone for programs which were explicitly undeployed / closed. + /// Tombstone for programs that were either explicitly closed or never deployed. + /// + /// It's also used for accounts belonging to program loaders, that don't actually contain program code (e.g. buffer accounts for LoaderV3 programs). #[default] Closed, /// Tombstone for programs which have recently been modified but the new version is not visible yet. @@ -776,12 +776,17 @@ impl ProgramCache { Ok(index) => { let existing = slot_versions.get_mut(index).unwrap(); match (&existing.program, &entry.program) { + // Add test for Closed => Loaded transition in same slot (LoadedProgramType::Builtin(_), LoadedProgramType::Builtin(_)) + | (LoadedProgramType::Closed, LoadedProgramType::LegacyV0(_)) + | (LoadedProgramType::Closed, LoadedProgramType::LegacyV1(_)) + | (LoadedProgramType::Closed, LoadedProgramType::Typed(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::LegacyV0(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::LegacyV1(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::Typed(_)) => {} #[cfg(test)] - (LoadedProgramType::Unloaded(_), LoadedProgramType::TestLoaded(_)) => {} + (LoadedProgramType::Closed, LoadedProgramType::TestLoaded(_)) + | (LoadedProgramType::Unloaded(_), LoadedProgramType::TestLoaded(_)) => {} _ => { // Something is wrong, I can feel it ... error!("ProgramCache::assign_program() failed key={:?} existing={:?} entry={:?}", key, slot_versions, entry); @@ -1680,7 +1685,6 @@ mod tests { #[test_matrix( ( LoadedProgramType::FailedVerification(Arc::new(BuiltinProgram::new_mock())), - LoadedProgramType::Closed, LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())), ), ( @@ -1692,7 +1696,10 @@ mod tests { ) )] #[test_matrix( - (LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())),), + ( + LoadedProgramType::Closed, + LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())), + ), ( LoadedProgramType::FailedVerification(Arc::new(BuiltinProgram::new_mock())), LoadedProgramType::Closed, @@ -1739,6 +1746,10 @@ mod tests { ); } + #[test_case( + LoadedProgramType::Closed, + LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())) + )] #[test_case( LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())), LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1571ff11e1..a05cf04bf4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1253,16 +1253,19 @@ impl Bank { { let effective_epoch = program_cache.latest_root_epoch.saturating_add(1); drop(program_cache); - let recompiled = new.load_program(&key, false, effective_epoch); - recompiled - .tx_usage_counter - .fetch_add(program_to_recompile.tx_usage_counter.load(Relaxed), Relaxed); - recompiled - .ix_usage_counter - .fetch_add(program_to_recompile.ix_usage_counter.load(Relaxed), Relaxed); - let mut program_cache = - new.transaction_processor.program_cache.write().unwrap(); - program_cache.assign_program(key, recompiled); + if let Some(recompiled) = new.load_program(&key, false, effective_epoch) { + recompiled.tx_usage_counter.fetch_add( + program_to_recompile.tx_usage_counter.load(Relaxed), + Relaxed, + ); + recompiled.ix_usage_counter.fetch_add( + program_to_recompile.ix_usage_counter.load(Relaxed), + Relaxed, + ); + let mut program_cache = + new.transaction_processor.program_cache.write().unwrap(); + program_cache.assign_program(key, recompiled); + } } } else if new.epoch() != program_cache.latest_root_epoch || slot_index.saturating_add(slots_in_recompilation_phase) >= slots_in_epoch @@ -6886,7 +6889,7 @@ impl Bank { pubkey: &Pubkey, reload: bool, effective_epoch: Epoch, - ) -> Arc { + ) -> Option> { self.transaction_processor .load_program_with_pubkey(self, pubkey, reload, effective_epoch) } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 31e189e079..9e8b44ffbe 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -39,11 +39,7 @@ use { compute_budget::ComputeBudget, compute_budget_processor::{self, MAX_COMPUTE_UNIT_LIMIT}, declare_process_instruction, - invoke_context::mock_process_instruction, - loaded_programs::{ - LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch, - DELAY_VISIBILITY_SLOT_OFFSET, - }, + loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch}, prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType}, timings::ExecuteTimings, }, @@ -7141,7 +7137,7 @@ fn test_bank_load_program() { programdata_account.set_rent_epoch(1); bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); - let program = bank.load_program(&key1, false, bank.epoch()); + let program = bank.load_program(&key1, false, bank.epoch()).unwrap(); assert_matches!(program.program, LoadedProgramType::LegacyV1(_)); assert_eq!( program.account_size, @@ -7167,6 +7163,26 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { ); let upgrade_authority_keypair = Keypair::new(); + // Invoke not yet deployed program + let instruction = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); + let invocation_message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); + let binding = mint_keypair.insecure_clone(); + let transaction = Transaction::new( + &[&binding], + invocation_message.clone(), + bank.last_blockhash(), + ); + assert_eq!( + bank.process_transaction(&transaction), + Err(TransactionError::ProgramAccountNotFound), + ); + { + // Make sure it is not in the cache because the account owner is not a loader + let program_cache = bank.transaction_processor.program_cache.read().unwrap(); + let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); + assert!(slot_versions.is_empty()); + } + // Load program file let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so") .expect("file open failed"); @@ -7214,6 +7230,28 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { &bpf_loader_upgradeable::id(), ); + // Test buffer invocation + bank.store_account(&buffer_address, &buffer_account); + 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()); + assert_eq!( + bank.process_transaction(&transaction), + Err(TransactionError::InstructionError( + 0, + InstructionError::InvalidAccountData, + )), + ); + { + let program_cache = bank.transaction_processor.program_cache.read().unwrap(); + let slot_versions = program_cache.get_slot_versions_for_tests(&buffer_address); + assert_eq!(slot_versions.len(), 1); + assert!(matches!( + slot_versions[0].program, + LoadedProgramType::Closed, + )); + } + // Test successful deploy let payer_base_balance = LAMPORTS_PER_SOL; let deploy_fees = { @@ -7231,7 +7269,6 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { &system_program::id(), ), ); - bank.store_account(&buffer_address, &buffer_account); bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default()); bank.store_account(&programdata_address, &AccountSharedData::default()); let message = Message::new( @@ -7296,30 +7333,15 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { assert_eq!(*elf.get(i).unwrap(), *byte); } - let loaded_program = bank.load_program(&program_keypair.pubkey(), false, bank.epoch()); + // Advance the bank so that the program becomes effective + goto_end_of_slot(bank.clone()); + let bank = bank_client + .advance_slot(1, bank_forks.as_ref(), &mint_keypair.pubkey()) + .unwrap(); - // Invoke deployed program - mock_process_instruction( - &bpf_loader_upgradeable::id(), - vec![0, 1], - &[], - vec![ - (programdata_address, post_programdata_account), - (program_keypair.pubkey(), post_program_account), - ], - Vec::new(), - Ok(()), - solana_bpf_loader_program::Entrypoint::vm, - |invoke_context| { - invoke_context - .programs_modified_by_tx - .set_slot_for_tests(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET); - invoke_context - .programs_modified_by_tx - .replenish(program_keypair.pubkey(), loaded_program.clone()); - }, - |_invoke_context| {}, - ); + // Invoke the deployed program + let transaction = Transaction::new(&[&binding], invocation_message, bank.last_blockhash()); + assert!(bank.process_transaction(&transaction).is_ok()); // Test initialized program account bank.clear_signatures(); diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index cc84347fa0..d827695e15 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -20,7 +20,7 @@ use { loaded_programs::{ ForkGraph, LoadProgramMetrics, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, LoadedProgramsForTxBatch, ProgramCache, ProgramRuntimeEnvironment, - ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET, + DELAY_VISIBILITY_SLOT_OFFSET, }, log_collector::LogCollector, runtime_config::RuntimeConfig, @@ -112,8 +112,7 @@ pub trait TransactionProcessingCallback { #[derive(Debug)] enum ProgramAccountLoadResult { - AccountNotFound, - InvalidAccountData(ProgramRuntimeEnvironment), + InvalidAccountData, ProgramOfLoaderV1orV2(AccountSharedData), ProgramOfLoaderV3(AccountSharedData, AccountSharedData, Slot), ProgramOfLoaderV4(AccountSharedData, Slot), @@ -385,15 +384,18 @@ impl TransactionBatchProcessor { result } - /// Load program with a specific pubkey from program cache, and - /// update the program's access slot as a side-effect. + /// Loads the program with the given pubkey. + /// + /// If the account doesn't exist it returns `None`. If the account does exist, it must be a program + /// account (belong to one of the program loaders). Returns `Some(InvalidAccountData)` if the program + /// account is `Closed`, contains invalid data or any of the programdata accounts are invalid. pub fn load_program_with_pubkey( &self, callbacks: &CB, pubkey: &Pubkey, reload: bool, effective_epoch: Epoch, - ) -> Arc { + ) -> Option> { let program_cache = self.program_cache.read().unwrap(); let environments = program_cache.get_environments_for_epoch(effective_epoch); let mut load_program_metrics = LoadProgramMetrics { @@ -401,74 +403,69 @@ impl TransactionBatchProcessor { ..LoadProgramMetrics::default() }; - let mut loaded_program = - match self.load_program_accounts(callbacks, pubkey, environments) { - ProgramAccountLoadResult::AccountNotFound => Ok(LoadedProgram::new_tombstone( - self.slot, - LoadedProgramType::Closed, - )), + let mut loaded_program = match self.load_program_accounts(callbacks, pubkey)? { + ProgramAccountLoadResult::InvalidAccountData => Ok(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::Closed, + )), - ProgramAccountLoadResult::InvalidAccountData(env) => Err((self.slot, env)), + ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => { + Self::load_program_from_bytes( + &mut load_program_metrics, + program_account.data(), + program_account.owner(), + program_account.data().len(), + 0, + environments.program_runtime_v1.clone(), + reload, + ) + .map_err(|_| (0, environments.program_runtime_v1.clone())) + } - ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => { + ProgramAccountLoadResult::ProgramOfLoaderV3( + program_account, + programdata_account, + slot, + ) => programdata_account + .data() + .get(UpgradeableLoaderState::size_of_programdata_metadata()..) + .ok_or(Box::new(InstructionError::InvalidAccountData).into()) + .and_then(|programdata| { Self::load_program_from_bytes( &mut load_program_metrics, - program_account.data(), + programdata, program_account.owner(), - program_account.data().len(), - 0, + program_account + .data() + .len() + .saturating_add(programdata_account.data().len()), + slot, environments.program_runtime_v1.clone(), reload, ) - .map_err(|_| (0, environments.program_runtime_v1.clone())) - } + }) + .map_err(|_| (slot, environments.program_runtime_v1.clone())), - ProgramAccountLoadResult::ProgramOfLoaderV3( - program_account, - programdata_account, - slot, - ) => programdata_account - .data() - .get(UpgradeableLoaderState::size_of_programdata_metadata()..) - .ok_or(Box::new(InstructionError::InvalidAccountData).into()) - .and_then(|programdata| { - Self::load_program_from_bytes( - &mut load_program_metrics, - programdata, - program_account.owner(), - program_account - .data() - .len() - .saturating_add(programdata_account.data().len()), - slot, - environments.program_runtime_v1.clone(), - reload, - ) - }) - .map_err(|_| (slot, environments.program_runtime_v1.clone())), - - ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot) => { - program_account - .data() - .get(LoaderV4State::program_data_offset()..) - .ok_or(Box::new(InstructionError::InvalidAccountData).into()) - .and_then(|elf_bytes| { - Self::load_program_from_bytes( - &mut load_program_metrics, - elf_bytes, - &loader_v4::id(), - program_account.data().len(), - slot, - environments.program_runtime_v2.clone(), - reload, - ) - }) - .map_err(|_| (slot, environments.program_runtime_v2.clone())) - } - } - .unwrap_or_else(|(slot, env)| { - LoadedProgram::new_tombstone(slot, LoadedProgramType::FailedVerification(env)) - }); + ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot) => program_account + .data() + .get(LoaderV4State::program_data_offset()..) + .ok_or(Box::new(InstructionError::InvalidAccountData).into()) + .and_then(|elf_bytes| { + Self::load_program_from_bytes( + &mut load_program_metrics, + elf_bytes, + &loader_v4::id(), + program_account.data().len(), + slot, + environments.program_runtime_v2.clone(), + reload, + ) + }) + .map_err(|_| (slot, environments.program_runtime_v2.clone())), + } + .unwrap_or_else(|(slot, env)| { + LoadedProgram::new_tombstone(slot, LoadedProgramType::FailedVerification(env)) + }); let mut timings = ExecuteDetailsTimings::default(); load_program_metrics.submit_datapoint(&mut timings); @@ -488,7 +485,7 @@ impl TransactionBatchProcessor { .max(self.epoch_schedule.get_first_slot_in_epoch(effective_epoch)); } loaded_program.update_access_slot(self.slot); - Arc::new(loaded_program) + Some(Arc::new(loaded_program)) } fn replenish_program_cache( @@ -553,7 +550,9 @@ impl TransactionBatchProcessor { if let Some((key, count)) = program_to_load { // Load, verify and compile one program. - let program = self.load_program_with_pubkey(callback, &key, false, self.epoch); + let program = self + .load_program_with_pubkey(callback, &key, false, self.epoch) + .expect("called load_program_with_pubkey() with nonexistent account"); program.tx_usage_counter.store(count, Ordering::Relaxed); program_to_store = Some((key, program)); } else if missing_programs.is_empty() { @@ -839,56 +838,48 @@ impl TransactionBatchProcessor { &self, callbacks: &CB, pubkey: &Pubkey, - environments: &ProgramRuntimeEnvironments, - ) -> ProgramAccountLoadResult { - let program_account = match callbacks.get_account_shared_data(pubkey) { - None => return ProgramAccountLoadResult::AccountNotFound, - Some(account) => account, - }; - - debug_assert!(solana_bpf_loader_program::check_loader_id( - program_account.owner() - )); + ) -> Option { + let program_account = callbacks.get_account_shared_data(pubkey)?; if loader_v4::check_id(program_account.owner()) { - return solana_loader_v4_program::get_state(program_account.data()) - .ok() - .and_then(|state| { - (!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot) - }) - .map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot)) - .unwrap_or(ProgramAccountLoadResult::InvalidAccountData( - environments.program_runtime_v2.clone(), - )); + return Some( + solana_loader_v4_program::get_state(program_account.data()) + .ok() + .and_then(|state| { + (!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot) + }) + .map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot)) + .unwrap_or(ProgramAccountLoadResult::InvalidAccountData), + ); } if !bpf_loader_upgradeable::check_id(program_account.owner()) { - return ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account); + return Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2( + program_account, + )); } if let Ok(UpgradeableLoaderState::Program { programdata_address, }) = program_account.state() { - let programdata_account = match callbacks.get_account_shared_data(&programdata_address) + if let Some(programdata_account) = + callbacks.get_account_shared_data(&programdata_address) { - None => return ProgramAccountLoadResult::AccountNotFound, - Some(account) => account, - }; - - if let Ok(UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address: _, - }) = programdata_account.state() - { - return ProgramAccountLoadResult::ProgramOfLoaderV3( - program_account, - programdata_account, + if let Ok(UpgradeableLoaderState::ProgramData { slot, - ); + upgrade_authority_address: _, + }) = programdata_account.state() + { + return Some(ProgramAccountLoadResult::ProgramOfLoaderV3( + program_account, + programdata_account, + slot, + )); + } } } - ProgramAccountLoadResult::InvalidAccountData(environments.program_runtime_v1.clone()) + Some(ProgramAccountLoadResult::InvalidAccountData) } /// Extract the InnerInstructionsList from a TransactionContext @@ -970,7 +961,8 @@ mod tests { use { super::*, solana_program_runtime::{ - loaded_programs::BlockRelation, solana_rbpf::program::BuiltinProgram, + loaded_programs::{BlockRelation, ProgramRuntimeEnvironments}, + solana_rbpf::program::BuiltinProgram, }, solana_sdk::{ account::{create_account_shared_data_for_test, WritableAccount}, @@ -1087,12 +1079,10 @@ mod tests { fn test_load_program_accounts_account_not_found() { let mut mock_bank = MockBankCallback::default(); let key = Pubkey::new_unique(); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); - - assert!(matches!(result, ProgramAccountLoadResult::AccountNotFound)); + let result = batch_processor.load_program_accounts(&mock_bank, &key); + assert!(result.is_none()); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader_upgradeable::id()); @@ -1104,17 +1094,20 @@ mod tests { .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); - assert!(matches!(result, ProgramAccountLoadResult::AccountNotFound)); + let result = batch_processor.load_program_accounts(&mock_bank, &key); + assert!(matches!( + result, + Some(ProgramAccountLoadResult::InvalidAccountData) + )); account_data.set_data(Vec::new()); mock_bank.account_shared_data.insert(key, account_data); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); } @@ -1124,26 +1117,25 @@ mod tests { let mut mock_bank = MockBankCallback::default(); let mut account_data = AccountSharedData::default(); account_data.set_owner(loader_v4::id()); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); account_data.set_data(vec![0; 64]); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); let loader_data = LoaderV4State { @@ -1161,10 +1153,10 @@ mod tests { .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); match result { - ProgramAccountLoadResult::ProgramOfLoaderV4(data, slot) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV4(data, slot)) => { assert_eq!(data, account_data); assert_eq!(slot, 25); } @@ -1179,15 +1171,14 @@ mod tests { let mut mock_bank = MockBankCallback::default(); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader::id()); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); match result { - ProgramAccountLoadResult::ProgramOfLoaderV1orV2(data) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2(data)) => { assert_eq!(data, account_data); } _ => panic!("Invalid result"), @@ -1199,7 +1190,6 @@ mod tests { let key1 = Pubkey::new_unique(); let key2 = Pubkey::new_unique(); let mut mock_bank = MockBankCallback::default(); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); let mut account_data = AccountSharedData::default(); @@ -1223,10 +1213,10 @@ mod tests { .account_shared_data .insert(key2, account_data2.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key1, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key1); match result { - ProgramAccountLoadResult::ProgramOfLoaderV3(data1, data2, slot) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV3(data1, data2, slot)) => { assert_eq!(data1, account_data); assert_eq!(data2, account_data2); assert_eq!(slot, 25); @@ -1291,9 +1281,7 @@ mod tests { let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 50); - - let loaded_program = LoadedProgram::new_tombstone(0, LoadedProgramType::Closed); - assert_eq!(result, Arc::new(loaded_program)); + assert!(result.is_none()); } #[test] @@ -1321,7 +1309,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); } #[test] @@ -1349,7 +1337,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let buffer = load_test_program(); account_data.set_data(buffer); @@ -1371,7 +1359,7 @@ mod tests { false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1416,7 +1404,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let mut buffer = load_test_program(); let mut header = bincode::serialize(&state).unwrap(); @@ -1451,7 +1439,7 @@ mod tests { environments.program_runtime_v1.clone(), false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1490,7 +1478,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let mut header = account_data.data().to_vec(); let mut complement = @@ -1523,7 +1511,7 @@ mod tests { environments.program_runtime_v1.clone(), false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1546,7 +1534,7 @@ mod tests { let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 20); let slot = batch_processor.epoch_schedule.get_first_slot_in_epoch(20); - assert_eq!(result.effective_slot, slot); + assert_eq!(result.unwrap().effective_slot, slot); } #[test] @@ -1825,47 +1813,51 @@ mod tests { assert_eq!(error_metrics.instruction_error, 1); } + #[test] + #[should_panic = "called load_program_with_pubkey() with nonexistent account"] + fn test_replenish_program_cache_with_nonexistent_accounts() { + let mock_bank = MockBankCallback::default(); + let batch_processor = TransactionBatchProcessor::::default(); + batch_processor.program_cache.write().unwrap().fork_graph = + Some(Arc::new(RwLock::new(TestForkGraph {}))); + let key = Pubkey::new_unique(); + let owner = Pubkey::new_unique(); + + let mut account_maps: HashMap = HashMap::new(); + account_maps.insert(key, (&owner, 4)); + + batch_processor.replenish_program_cache(&mock_bank, &account_maps, true); + } + #[test] fn test_replenish_program_cache() { - // Case 1 let mut mock_bank = MockBankCallback::default(); let batch_processor = TransactionBatchProcessor::::default(); batch_processor.program_cache.write().unwrap().fork_graph = Some(Arc::new(RwLock::new(TestForkGraph {}))); - let key1 = Pubkey::new_unique(); - let key2 = Pubkey::new_unique(); + let key = Pubkey::new_unique(); let owner = Pubkey::new_unique(); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader::id()); - mock_bank.account_shared_data.insert(key2, account_data); + mock_bank.account_shared_data.insert(key, account_data); let mut account_maps: HashMap = HashMap::new(); - account_maps.insert(key1, (&owner, 2)); + account_maps.insert(key, (&owner, 4)); - account_maps.insert(key2, (&owner, 4)); - let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, false); - - let program1 = result.find(&key1).unwrap(); - assert!(matches!(program1.program, LoadedProgramType::Closed)); - assert!(!result.hit_max_limit); - let program2 = result.find(&key2).unwrap(); - assert!(matches!( - program2.program, - LoadedProgramType::FailedVerification(_) - )); - - // Case 2 - let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true); - - let program1 = result.find(&key1).unwrap(); - assert!(matches!(program1.program, LoadedProgramType::Closed)); - assert!(!result.hit_max_limit); - let program2 = result.find(&key2).unwrap(); - assert!(matches!( - program2.program, - LoadedProgramType::FailedVerification(_) - )); + for limit_to_load_programs in [false, true] { + let result = batch_processor.replenish_program_cache( + &mock_bank, + &account_maps, + limit_to_load_programs, + ); + assert!(!result.hit_max_limit); + let program = result.find(&key).unwrap(); + assert!(matches!( + program.program, + LoadedProgramType::FailedVerification(_) + )); + } } #[test]