Stop loading program accounts if program exists in cache

program-runtime/src/executor_cache.rs

@@ -1,5 +1,5 @@
 use {
-    crate::loaded_programs::LoadedProgram,
+    crate::loaded_programs::{LoadedProgram, LoadedProgramType},
     log::*,
     rand::Rng,
     solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch},
@@ -42,8 +42,13 @@ impl TransactionExecutorCache {
     }
 
     pub fn set_tombstone(&mut self, key: Pubkey, slot: Slot) {
-        self.visible
-            .insert(key, Arc::new(LoadedProgram::new_tombstone(slot)));
+        self.visible.insert(
+            key,
+            Arc::new(LoadedProgram::new_tombstone(
+                slot,
+                LoadedProgramType::Closed,
+            )),
+        );
     }
 
     pub fn set(
@@ -58,7 +63,13 @@ impl TransactionExecutorCache {
             if delay_visibility_of_program_deployment {
                 // Place a tombstone in the cache so that
                 // we don't load the new version from the database as it should remain invisible
-                self.set_tombstone(key, current_slot);
+                self.visible.insert(
+                    key,
+                    Arc::new(LoadedProgram::new_tombstone(
+                        current_slot,
+                        LoadedProgramType::DelayVisibility,
+                    )),
+                );
             } else {
                 self.visible.insert(key, executor.clone());
             }

program-runtime/src/invoke_context.rs

@@ -14,7 +14,6 @@ use {
     solana_rbpf::vm::ContextObject,
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount},
-        bpf_loader_upgradeable::{self, UpgradeableLoaderState},
         feature_set::{
             enable_early_verification_of_account_modifications, native_programs_consume_cu,
             FeatureSet,
@@ -624,37 +623,11 @@ impl<'a> InvokeContext<'a> {
             ic_msg!(self, "Account {} is not executable", callee_program_id);
             return Err(InstructionError::AccountNotExecutable);
         }
-        let mut program_indices = vec![];
-        if borrowed_program_account.get_owner() == &bpf_loader_upgradeable::id() {
-            if let UpgradeableLoaderState::Program {
-                programdata_address,
-            } = borrowed_program_account.get_state()?
-            {
-                if let Some(programdata_account_index) = self
-                    .transaction_context
-                    .find_index_of_program_account(&programdata_address)
-                {
-                    program_indices.push(programdata_account_index);
-                } else {
-                    ic_msg!(
-                        self,
-                        "Unknown upgradeable programdata account {}",
-                        programdata_address,
-                    );
-                    return Err(InstructionError::MissingAccount);
-                }
-            } else {
-                ic_msg!(
-                    self,
-                    "Invalid upgradeable program account {}",
-                    callee_program_id,
-                );
-                return Err(InstructionError::MissingAccount);
-            }
-        }
-        program_indices.push(borrowed_program_account.get_index_in_transaction());
 
-        Ok((instruction_accounts, program_indices))
+        Ok((
+            instruction_accounts,
+            vec![borrowed_program_account.get_index_in_transaction()],
+        ))
     }
 
     /// Processes an instruction and returns how many compute units were used

program-runtime/src/loaded_programs.rs

@@ -58,7 +58,9 @@ pub trait WorkingSlot {
 pub enum LoadedProgramType {
     /// Tombstone for undeployed, closed or unloadable programs
     #[default]
-    Invalid,
+    FailedVerification,
+    Closed,
+    DelayVisibility,
     LegacyV0(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     LegacyV1(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     Typed(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
@@ -68,7 +70,11 @@ pub enum LoadedProgramType {
 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"),
@@ -190,18 +196,25 @@ impl LoadedProgram {
         }
     }
 
-    pub fn new_tombstone(slot: Slot) -> Self {
-        Self {
-            program: LoadedProgramType::Invalid,
+    pub fn new_tombstone(slot: Slot, reason: LoadedProgramType) -> Self {
+        let tombstone = Self {
+            program: reason,
             account_size: 0,
             deployment_slot: slot,
             effective_slot: slot,
             usage_counter: AtomicU64::default(),
-        }
+        };
+        debug_assert!(tombstone.is_tombstone());
+        tombstone
     }
 
     pub fn is_tombstone(&self) -> bool {
-        matches!(self.program, LoadedProgramType::Invalid)
+        matches!(
+            self.program,
+            LoadedProgramType::FailedVerification
+                | LoadedProgramType::Closed
+                | LoadedProgramType::DelayVisibility
+        )
     }
 }
 
@@ -266,7 +279,15 @@ impl LoadedPrograms {
             let existing = second_level
                 .get(index)
                 .expect("Missing entry, even though position was found");
-            assert!(
+            if existing.is_tombstone()
+                && entry.is_tombstone()
+                && existing.deployment_slot == entry.deployment_slot
+            {
+                // If there's already a tombstone for the program at the given slot, let's return
+                // the existing entry instead of adding another.
+                return existing.clone();
+            }
+            debug_assert!(
                 existing.deployment_slot != entry.deployment_slot
                     || existing.effective_slot != entry.effective_slot
             );
@@ -395,7 +416,13 @@ mod tests {
     }
 
     fn set_tombstone(cache: &mut LoadedPrograms, key: Pubkey, slot: Slot) -> Arc<LoadedProgram> {
-        cache.assign_program(key, Arc::new(LoadedProgram::new_tombstone(slot)))
+        cache.assign_program(
+            key,
+            Arc::new(LoadedProgram::new_tombstone(
+                slot,
+                LoadedProgramType::FailedVerification,
+            )),
+        )
     }
 
     #[test]
@@ -637,14 +664,17 @@ mod tests {
 
     #[test]
     fn test_tombstone() {
-        let tombstone = LoadedProgram::new_tombstone(0);
-        assert!(matches!(tombstone.program, LoadedProgramType::Invalid));
+        let tombstone = LoadedProgram::new_tombstone(0, LoadedProgramType::FailedVerification);
+        assert!(matches!(
+            tombstone.program,
+            LoadedProgramType::FailedVerification
+        ));
         assert!(tombstone.is_tombstone());
         assert_eq!(tombstone.deployment_slot, 0);
         assert_eq!(tombstone.effective_slot, 0);
 
-        let tombstone = LoadedProgram::new_tombstone(100);
-        assert!(matches!(tombstone.program, LoadedProgramType::Invalid));
+        let tombstone = LoadedProgram::new_tombstone(100, LoadedProgramType::Closed);
+        assert!(matches!(tombstone.program, LoadedProgramType::Closed));
         assert!(tombstone.is_tombstone());
         assert_eq!(tombstone.deployment_slot, 100);
         assert_eq!(tombstone.effective_slot, 100);
@@ -835,7 +865,7 @@ mod tests {
         usage_counter: AtomicU64,
     ) -> Arc<LoadedProgram> {
         Arc::new(LoadedProgram {
-            program: LoadedProgramType::Invalid,
+            program: LoadedProgramType::FailedVerification,
             account_size: 0,
             deployment_slot,
             effective_slot,

programs/bpf_loader/src/lib.rs

@@ -135,7 +135,7 @@ pub fn load_program_from_bytes(
     Ok(loaded_program)
 }
 
-fn get_programdata_offset_and_depoyment_offset(
+fn get_programdata_offset_and_deployment_offset(
     log_collector: &Option<Rc<RefCell<LogCollector>>>,
     program: &BorrowedAccount,
     programdata: &BorrowedAccount,
@@ -181,21 +181,20 @@ pub fn load_program_from_account(
         return Err(InstructionError::IncorrectProgramId);
     }
 
-    let (programdata_offset, deployment_slot) =
-        get_programdata_offset_and_depoyment_offset(&log_collector, program, programdata)?;
-
     if let Some(ref tx_executor_cache) = tx_executor_cache {
         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
             return Ok((loaded_program, None));
         }
     }
 
+    let (programdata_offset, deployment_slot) =
+        get_programdata_offset_and_deployment_offset(&log_collector, program, programdata)?;
+
     let programdata_size = if programdata_offset != 0 {
         programdata.get_data().len()
     } else {
@@ -556,14 +555,16 @@ fn process_instruction_common(
         ic_logger_msg!(log_collector, "Program is not executable");
         return Err(InstructionError::IncorrectProgramId);
     }
+
     let programdata_account = if bpf_loader_upgradeable::check_id(program_account.get_owner()) {
-        let programdata_account = instruction_context.try_borrow_program_account(
-            transaction_context,
-            instruction_context
-                .get_number_of_program_accounts()
-                .saturating_sub(2),
-        )?;
-        Some(programdata_account)
+        instruction_context
+            .try_borrow_program_account(
+                transaction_context,
+                instruction_context
+                    .get_number_of_program_accounts()
+                    .saturating_sub(2),
+            )
+            .ok()
     } else {
         None
     };
@@ -591,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),
         }