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

@@ -776,17 +776,12 @@ impl<FG: ForkGraph> ProgramCache<FG> {
             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()))

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();