address review comments

program-runtime/src/executor_cache.rs

@@ -1,5 +1,5 @@
 use {
-    crate::loaded_programs::{InvalidProgramReason, LoadedProgram},
+    crate::loaded_programs::LoadedProgram,
     log::*,
     rand::Rng,
     solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch},
@@ -44,10 +44,7 @@ impl TransactionExecutorCache {
     pub fn set_tombstone(&mut self, key: Pubkey, slot: Slot) {
         self.visible.insert(
             key,
-            Arc::new(LoadedProgram::new_tombstone(
-                slot,
-                InvalidProgramReason::Closed,
-            )),
+            Arc::new(LoadedProgram::new_tombstone_program_closed(slot)),
         );
     }
 
@@ -65,10 +62,7 @@ impl TransactionExecutorCache {
                 // we don't load the new version from the database as it should remain invisible
                 self.visible.insert(
                     key,
-                    Arc::new(LoadedProgram::new_tombstone(
-                        current_slot,
-                        InvalidProgramReason::DelayVisibility,
-                    )),
+                    Arc::new(LoadedProgram::new_tombstone_delay_visibility(current_slot)),
                 );
             } else {
                 self.visible.insert(key, executor.clone());

program-runtime/src/loaded_programs.rs

@@ -55,32 +55,26 @@ pub trait WorkingSlot {
 }
 
 #[derive(Default)]
-pub enum InvalidProgramReason {
+pub enum LoadedProgramType {
+    /// Tombstone for undeployed, closed or unloadable programs
     #[default]
-    FailedToCompile,
+    FailedVerification,
     Closed,
     DelayVisibility,
-}
-
-pub enum LoadedProgramType {
-    /// Tombstone for undeployed, closed or unloadable programs
-    Invalid(InvalidProgramReason),
     LegacyV0(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     LegacyV1(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     Typed(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     BuiltIn(BuiltInProgram<InvokeContext<'static>>),
 }
 
-impl Default for LoadedProgramType {
-    fn default() -> Self {
-        LoadedProgramType::Invalid(InvalidProgramReason::FailedToCompile)
-    }
-}
-
 impl Debug for LoadedProgramType {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self {
-            LoadedProgramType::Invalid(_) => write!(f, "LoadedProgramType::Invalid"),
+            LoadedProgramType::FailedVerification => {
+                write!(f, "LoadedProgramType::FailedVerification")
+            }
+            LoadedProgramType::Closed => write!(f, "LoadedProgramType::Closed"),
+            LoadedProgramType::DelayVisibility => write!(f, "LoadedProgramType::DelayVisibility"),
             LoadedProgramType::LegacyV0(_) => write!(f, "LoadedProgramType::LegacyV0"),
             LoadedProgramType::LegacyV1(_) => write!(f, "LoadedProgramType::LegacyV1"),
             LoadedProgramType::Typed(_) => write!(f, "LoadedProgramType::Typed"),
@@ -202,9 +196,29 @@ impl LoadedProgram {
         }
     }
 
-    pub fn new_tombstone(slot: Slot, reason: InvalidProgramReason) -> Self {
+    pub fn new_tombstone_verification_failed(slot: Slot) -> Self {
         Self {
-            program: LoadedProgramType::Invalid(reason),
+            program: LoadedProgramType::FailedVerification,
+            account_size: 0,
+            deployment_slot: slot,
+            effective_slot: slot,
+            usage_counter: AtomicU64::default(),
+        }
+    }
+
+    pub fn new_tombstone_program_closed(slot: Slot) -> Self {
+        Self {
+            program: LoadedProgramType::Closed,
+            account_size: 0,
+            deployment_slot: slot,
+            effective_slot: slot,
+            usage_counter: AtomicU64::default(),
+        }
+    }
+
+    pub fn new_tombstone_delay_visibility(slot: Slot) -> Self {
+        Self {
+            program: LoadedProgramType::DelayVisibility,
             account_size: 0,
             deployment_slot: slot,
             effective_slot: slot,
@@ -213,7 +227,12 @@ impl LoadedProgram {
     }
 
     pub fn is_tombstone(&self) -> bool {
-        matches!(self.program, LoadedProgramType::Invalid(_))
+        match self.program {
+            LoadedProgramType::FailedVerification
+            | LoadedProgramType::Closed
+            | LoadedProgramType::DelayVisibility => true,
+            _ => false,
+        }
     }
 }
 
@@ -381,8 +400,8 @@ impl LoadedPrograms {
 mod tests {
     use {
         crate::loaded_programs::{
-            BlockRelation, ForkGraph, InvalidProgramReason, LoadedProgram, LoadedProgramEntry,
-            LoadedProgramType, LoadedPrograms, WorkingSlot,
+            BlockRelation, ForkGraph, LoadedProgram, LoadedProgramEntry, LoadedProgramType,
+            LoadedPrograms, WorkingSlot,
         },
         solana_rbpf::vm::BuiltInProgram,
         solana_sdk::{clock::Slot, pubkey::Pubkey},
@@ -409,10 +428,7 @@ mod tests {
     fn set_tombstone(cache: &mut LoadedPrograms, key: Pubkey, slot: Slot) -> Arc<LoadedProgram> {
         cache.assign_program(
             key,
-            Arc::new(LoadedProgram::new_tombstone(
-                slot,
-                InvalidProgramReason::FailedToCompile,
-            )),
+            Arc::new(LoadedProgram::new_tombstone_verification_failed(slot)),
         )
     }
 
@@ -655,20 +671,17 @@ mod tests {
 
     #[test]
     fn test_tombstone() {
-        let tombstone = LoadedProgram::new_tombstone(0, InvalidProgramReason::FailedToCompile);
+        let tombstone = LoadedProgram::new_tombstone_verification_failed(0);
         assert!(matches!(
             tombstone.program,
-            LoadedProgramType::Invalid(InvalidProgramReason::FailedToCompile)
+            LoadedProgramType::FailedVerification
         ));
         assert!(tombstone.is_tombstone());
         assert_eq!(tombstone.deployment_slot, 0);
         assert_eq!(tombstone.effective_slot, 0);
 
-        let tombstone = LoadedProgram::new_tombstone(100, InvalidProgramReason::Closed);
-        assert!(matches!(
-            tombstone.program,
-            LoadedProgramType::Invalid(InvalidProgramReason::Closed)
-        ));
+        let tombstone = LoadedProgram::new_tombstone_program_closed(100);
+        assert!(matches!(tombstone.program, LoadedProgramType::Closed));
         assert!(tombstone.is_tombstone());
         assert_eq!(tombstone.deployment_slot, 100);
         assert_eq!(tombstone.effective_slot, 100);
@@ -859,7 +872,7 @@ mod tests {
         usage_counter: AtomicU64,
     ) -> Arc<LoadedProgram> {
         Arc::new(LoadedProgram {
-            program: LoadedProgramType::Invalid(InvalidProgramReason::FailedToCompile),
+            program: LoadedProgramType::FailedVerification,
             account_size: 0,
             deployment_slot,
             effective_slot,

programs/bpf_loader/src/lib.rs

@@ -185,7 +185,6 @@ pub fn load_program_from_account(
         if let Some(loaded_program) = tx_executor_cache.get(program.get_key()) {
             if loaded_program.is_tombstone() {
                 // We cached that the Executor does not exist, abort
-                // This case can only happen once delay_visibility_of_program_deployment is active.
                 return Err(InstructionError::InvalidAccountData);
             }
             // Executor exists and is cached, use it
@@ -593,7 +592,9 @@ fn process_instruction_common(
 
     executor.usage_counter.fetch_add(1, Ordering::Relaxed);
     match &executor.program {
-        LoadedProgramType::Invalid(_) => Err(InstructionError::InvalidAccountData),
+        LoadedProgramType::FailedVerification
+        | LoadedProgramType::Closed
+        | LoadedProgramType::DelayVisibility => Err(InstructionError::InvalidAccountData),
         LoadedProgramType::LegacyV0(executable) => execute(executable, invoke_context),
         LoadedProgramType::LegacyV1(executable) => execute(executable, invoke_context),
         _ => Err(InstructionError::IncorrectProgramId),

programs/loader-v3/src/lib.rs

@@ -590,7 +590,9 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins
         drop(program);
         loaded_program.usage_counter.fetch_add(1, Ordering::Relaxed);
         match &loaded_program.program {
-            LoadedProgramType::Invalid(_) => Err(InstructionError::InvalidAccountData),
+            LoadedProgramType::FailedVerification
+            | LoadedProgramType::Closed
+            | LoadedProgramType::DelayVisibility => Err(InstructionError::InvalidAccountData),
             LoadedProgramType::Typed(executable) => execute(invoke_context, executable),
             _ => Err(InstructionError::IncorrectProgramId),
         }

runtime/src/accounts.rs

@@ -29,7 +29,7 @@ use {
     solana_address_lookup_table_program::{error::AddressLookupError, state::AddressLookupTable},
     solana_program_runtime::{
         compute_budget::{self, ComputeBudget},
-        loaded_programs::{InvalidProgramReason, LoadedProgram, LoadedProgramType},
+        loaded_programs::{LoadedProgram, LoadedProgramType},
     },
     solana_sdk::{
         account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
@@ -300,20 +300,17 @@ impl Accounts {
         program_accounts: &HashMap<Pubkey, &Pubkey>,
     ) -> Result<AccountSharedData> {
         // Check for tombstone
-        if let LoadedProgramType::Invalid(reason) = &program.program {
-            match reason {
-                InvalidProgramReason::FailedToCompile => {
-                    Err(TransactionError::InstructionError(0, InvalidAccountData))
-                }
-                InvalidProgramReason::Closed => Err(TransactionError::InvalidProgramForExecution),
-                InvalidProgramReason::DelayVisibility => {
-                    debug_assert!(
-                        feature_set.is_active(&delay_visibility_of_program_deployment::id())
-                    );
-                    Err(TransactionError::InstructionError(0, InvalidAccountData))
-                }
-            }?;
-        }
+        match &program.program {
+            LoadedProgramType::FailedVerification => {
+                Err(TransactionError::InstructionError(0, InvalidAccountData))
+            }
+            LoadedProgramType::Closed => Err(TransactionError::InvalidProgramForExecution),
+            LoadedProgramType::DelayVisibility => {
+                debug_assert!(feature_set.is_active(&delay_visibility_of_program_deployment::id()));
+                Err(TransactionError::InvalidProgramForExecution)
+            }
+            _ => Ok(()),
+        }?;
         // 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.

runtime/src/bank.rs

@@ -95,9 +95,7 @@ use {
         compute_budget::{self, ComputeBudget},
         executor_cache::{BankExecutorCache, TransactionExecutorCache, MAX_CACHED_EXECUTORS},
         invoke_context::{BuiltinProgram, ProcessInstructionWithContext},
-        loaded_programs::{
-            InvalidProgramReason, LoadedProgram, LoadedProgramEntry, LoadedPrograms, WorkingSlot,
-        },
+        loaded_programs::{LoadedProgram, LoadedProgramEntry, LoadedPrograms, WorkingSlot},
         log_collector::LogCollector,
         sysvar_cache::SysvarCache,
         timings::{ExecuteTimingType, ExecuteTimings},
@@ -4437,10 +4435,7 @@ impl Bank {
                     debug!("Failed to load program {}, error {:?}", pubkey, e);
                     let tombstone = self.loaded_programs_cache.write().unwrap().assign_program(
                         *pubkey,
-                        Arc::new(LoadedProgram::new_tombstone(
-                            self.slot,
-                            InvalidProgramReason::FailedToCompile,
-                        )),
+                        Arc::new(LoadedProgram::new_tombstone_verification_failed(self.slot)),
                     );
                     loaded_programs_for_txs.insert(*pubkey, tombstone);
                 }
@@ -4496,10 +4491,8 @@ impl Bank {
                 }
                 // Create a tombstone for the programs that failed to load
                 Err(_) => {
-                    let tombstone = Arc::new(LoadedProgram::new_tombstone(
-                        self.slot,
-                        InvalidProgramReason::FailedToCompile,
-                    ));
+                    let tombstone =
+                        Arc::new(LoadedProgram::new_tombstone_verification_failed(self.slot));
                     loaded_programs_for_txs.insert(**pubkey, tombstone.clone());
                     (**pubkey, tombstone)
                 }