From ccc6a6bf6fab3dccb12677153e41f28b486d0ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 2 Mar 2024 15:55:53 +0100 Subject: [PATCH] Fix - `test_feature_activation_loaded_programs_recompilation_phase()` (#35299) * Fixes test_feature_activation_loaded_programs_recompilation_phase() to trigger the recompilation phase before the epoch boundary. * Adds a direct check of the cached entries around recompilation. --- program-runtime/src/loaded_programs.rs | 8 +++ runtime/src/bank/tests.rs | 88 ++++++++++++++++++-------- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 8e3e670469c45c..926d1179837380 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1079,6 +1079,14 @@ impl LoadedPrograms { } } + /// Returns the `slot_versions` of the second level for the given program id. + pub fn get_slot_versions_for_tests(&self, key: &Pubkey) -> &[Arc] { + self.entries + .get(key) + .map(|second_level| second_level.slot_versions.as_ref()) + .unwrap_or(&[]) + } + /// This function removes the given entry for the given program from the cache. /// The function expects that the program and entry exists in the cache. Otherwise it'll panic. fn unload_program_entry(&mut self, program: &Pubkey, remove_entry: &Arc) { diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 2283899d3ca30d..753116ff878e18 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -11884,12 +11884,6 @@ fn test_feature_activation_loaded_programs_recompilation_phase() { .remove(&feature_set::reject_callx_r10::id()); let (root_bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - // Test a basic transfer - let amount = genesis_config.rent.minimum_balance(0); - let pubkey = solana_sdk::pubkey::new_rand(); - root_bank.transfer(amount, &mint_keypair, &pubkey).unwrap(); - assert_eq!(root_bank.get_balance(&pubkey), amount); - // Program Setup let program_keypair = Keypair::new(); let program_data = @@ -11903,26 +11897,19 @@ fn test_feature_activation_loaded_programs_recompilation_phase() { }); root_bank.store_account(&program_keypair.pubkey(), &program_account); - // Compose instruction using the desired program - let instruction1 = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); - let message1 = Message::new(&[instruction1], Some(&mint_keypair.pubkey())); - let binding1 = mint_keypair.insecure_clone(); - let signers1 = vec![&binding1]; - let transaction1 = Transaction::new(&signers1, message1, root_bank.last_blockhash()); + // Compose message using the desired program. + let instruction = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); + let message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); + let binding = mint_keypair.insecure_clone(); + let signers = vec![&binding]; - // Advance the bank so the next transaction can be submitted. + // Advance the bank so that the program becomes effective. goto_end_of_slot(root_bank.clone()); let bank = new_from_parent_with_fork_next_slot(root_bank, bank_forks.as_ref()); - // Compose second instruction using the same program with a different block hash - let instruction2 = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); - let message2 = Message::new(&[instruction2], Some(&mint_keypair.pubkey())); - let binding2 = mint_keypair.insecure_clone(); - let signers2 = vec![&binding2]; - let transaction2 = Transaction::new(&signers2, message2, bank.last_blockhash()); - - // Execute before feature is enabled to get program into the cache. - let result_without_feature_enabled = bank.process_transaction(&transaction1); + // Load the program with the old environment. + let transaction = Transaction::new(&signers, message.clone(), bank.last_blockhash()); + let result_without_feature_enabled = bank.process_transaction(&transaction); assert_eq!( result_without_feature_enabled, Err(TransactionError::InstructionError( @@ -11931,7 +11918,7 @@ fn test_feature_activation_loaded_programs_recompilation_phase() { )) ); - // Activate feature + // Schedule feature activation to trigger a change of environment at the epoch boundary. let feature_account_balance = std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1); bank.store_account( @@ -11939,12 +11926,59 @@ fn test_feature_activation_loaded_programs_recompilation_phase() { &feature::create_account(&Feature { activated_at: None }, feature_account_balance), ); + // Advance the bank to middle of epoch to start the recompilation phase. + goto_end_of_slot(bank.clone()); + let bank = new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), 16); + let current_env = bank + .loaded_programs_cache + .read() + .unwrap() + .get_environments_for_epoch(0) + .program_runtime_v1 + .clone(); + let upcoming_env = bank + .loaded_programs_cache + .read() + .unwrap() + .get_environments_for_epoch(1) + .program_runtime_v1 + .clone(); + + // Advance the bank to recompile the program. + { + let loaded_programs_cache = bank.loaded_programs_cache.read().unwrap(); + let slot_versions = + loaded_programs_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); + assert_eq!(slot_versions.len(), 1); + assert!(Arc::ptr_eq( + slot_versions[0].program.get_environment().unwrap(), + ¤t_env + )); + } + goto_end_of_slot(bank.clone()); + let bank = new_from_parent_with_fork_next_slot(bank, bank_forks.as_ref()); + { + let loaded_programs_cache = bank.loaded_programs_cache.read().unwrap(); + let slot_versions = + loaded_programs_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); + assert_eq!(slot_versions.len(), 2); + assert!(Arc::ptr_eq( + slot_versions[0].program.get_environment().unwrap(), + ¤t_env + )); + assert!(Arc::ptr_eq( + slot_versions[1].program.get_environment().unwrap(), + &upcoming_env + )); + } + + // Advance the bank to cross the epoch boundary and activate the feature. goto_end_of_slot(bank.clone()); - // Advance to next epoch, which starts the recompilation phase - let bank = new_from_parent_next_epoch(bank, bank_forks.as_ref(), 1); + let bank = new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), 33); - // Execute after feature is enabled to check it was filtered out and reverified. - let result_with_feature_enabled = bank.process_transaction(&transaction2); + // Load the program with the new environment. + let transaction = Transaction::new(&signers, message, bank.last_blockhash()); + let result_with_feature_enabled = bank.process_transaction(&transaction); assert_eq!( result_with_feature_enabled, Err(TransactionError::InstructionError(