From d448c4e5f062c447717df9f7ebc2def8bd2d4c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 12 Apr 2024 14:59:51 +0200 Subject: [PATCH] Fix - `ProgramCache::assign_program()` Closed => Loaded in same slot (#673) * Refactors test_add_builtin() to use LoadedProgram::new_builtin() and ProgramCache::extract(). * Revert Closed => Loaded transition * Adds more loaded programs deployment tests --- program-runtime/src/loaded_programs.rs | 13 ++----- runtime/src/bank/tests.rs | 47 +++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 419ae7330..7807ef447 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -776,17 +776,12 @@ 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::Closed, LoadedProgramType::TestLoaded(_)) - | (LoadedProgramType::Unloaded(_), 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); @@ -1684,6 +1679,7 @@ mod tests { #[test_matrix( ( + LoadedProgramType::Closed, LoadedProgramType::FailedVerification(Arc::new(BuiltinProgram::new_mock())), LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())), ), @@ -1697,7 +1693,6 @@ mod tests { )] #[test_matrix( ( - LoadedProgramType::Closed, LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())), ), ( @@ -1746,10 +1741,6 @@ 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/tests.rs b/runtime/src/bank/tests.rs index 43e7280d9..f472ac0a9 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7126,7 +7126,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { ); let upgrade_authority_keypair = Keypair::new(); - // Invoke not yet deployed program + // Test nonexistent program invocation 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(); @@ -7193,6 +7193,32 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { &bpf_loader_upgradeable::id(), ); + // Test uninitialized program invocation + bank.store_account(&program_keypair.pubkey(), &program_account); + let transaction = Transaction::new( + &[&binding], + invocation_message.clone(), + 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(&program_keypair.pubkey()); + assert_eq!(slot_versions.len(), 1); + assert_eq!(slot_versions[0].deployment_slot, 0); + assert_eq!(slot_versions[0].effective_slot, 0); + assert!(matches!( + slot_versions[0].program, + LoadedProgramType::Closed, + )); + } + // Test buffer invocation bank.store_account(&buffer_address, &buffer_account); let instruction = Instruction::new_with_bytes(buffer_address, &[], Vec::new()); @@ -7209,6 +7235,8 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { 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_eq!(slot_versions[0].deployment_slot, 0); + assert_eq!(slot_versions[0].effective_slot, 0); assert!(matches!( slot_versions[0].program, LoadedProgramType::Closed, @@ -7305,6 +7333,23 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { // Invoke the deployed program let transaction = Transaction::new(&[&binding], invocation_message, bank.last_blockhash()); assert!(bank.process_transaction(&transaction).is_ok()); + { + let program_cache = bank.transaction_processor.program_cache.read().unwrap(); + let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); + assert_eq!(slot_versions.len(), 2); + assert_eq!(slot_versions[0].deployment_slot, 0); + assert_eq!(slot_versions[0].effective_slot, 0); + assert!(matches!( + slot_versions[0].program, + LoadedProgramType::Closed, + )); + assert_eq!(slot_versions[1].deployment_slot, 0); + assert_eq!(slot_versions[1].effective_slot, 1); + assert!(matches!( + slot_versions[1].program, + LoadedProgramType::LegacyV1(_), + )); + } // Test initialized program account bank.clear_signatures();