more cleanup

ledger-tool/src/run.rs

@@ -15,7 +15,9 @@ use {
     },
     solana_program_runtime::{
         invoke_context::InvokeContext,
-        loaded_programs::{LoadProgramMetrics, LoadedProgram, LoadedProgramType},
+        loaded_programs::{
+            LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET,
+        },
         with_mock_invoke_context,
     },
     solana_rbpf::{
@@ -416,7 +418,9 @@ pub fn run(ledger_path: &Path, matches: &ArgMatches<'_>) {
         bank.get_builtin_programs()
     );
 
-    let mut loaded_programs = LoadedProgramsForTxBatch::default();
+    // Adding `DELAY_VISIBILITY_SLOT_OFFSET` to slots to accommodate for delay visibility of the program
+    let mut loaded_programs =
+        LoadedProgramsForTxBatch::new(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
     for key in cached_account_keys {
         let program = bank.load_program(&key, true).unwrap_or_else(|err| {
             // Create a tombstone for the program in the cache
@@ -429,7 +433,7 @@ pub fn run(ledger_path: &Path, matches: &ArgMatches<'_>) {
         debug!("Loaded program {}", key);
         loaded_programs.replenish(key, program);
     }
-    invoke_context.programs_loaded_for_tx_batch = Rc::new(RefCell::new(loaded_programs));
+    invoke_context.programs_loaded_for_tx_batch = Rc::new(loaded_programs);
 
     invoke_context
         .transaction_context

program-runtime/src/invoke_context.rs

@@ -159,7 +159,7 @@ pub struct InvokeContext<'a> {
     current_compute_budget: ComputeBudget,
     compute_meter: RefCell<u64>,
     accounts_data_meter: AccountsDataMeter,
-    pub programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+    pub programs_loaded_for_tx_batch: Rc<LoadedProgramsForTxBatch>,
     pub programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
     pub feature_set: Arc<FeatureSet>,
     pub timings: ExecuteDetailsTimings,
@@ -178,7 +178,7 @@ impl<'a> InvokeContext<'a> {
         sysvar_cache: &'a SysvarCache,
         log_collector: Option<Rc<RefCell<LogCollector>>>,
         compute_budget: ComputeBudget,
-        programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+        programs_loaded_for_tx_batch: Rc<LoadedProgramsForTxBatch>,
         programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
         feature_set: Arc<FeatureSet>,
         blockhash: Hash,
@@ -942,7 +942,7 @@ macro_rules! with_mock_invoke_context_and_builtin_programs {
             &sysvar_cache,
             Some(LogCollector::new_ref()),
             compute_budget,
-            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
+            Rc::new(LoadedProgramsForTxBatch::default()),
             Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
             Arc::new(FeatureSet::all_enabled()),
             Hash::default(),

program-runtime/src/loaded_programs.rs

@@ -25,7 +25,7 @@ use {
 };
 
 const MAX_LOADED_ENTRY_COUNT: usize = 256;
-const DELAY_VISIBILITY_SLOT_OFFSET: Slot = 1;
+pub const DELAY_VISIBILITY_SLOT_OFFSET: Slot = 1;
 
 /// Relationship between two fork IDs
 #[derive(Copy, Clone, PartialEq)]
@@ -318,6 +318,10 @@ impl LoadedProgramsForTxBatch {
     pub fn slot(&self) -> Slot {
         self.slot
     }
+
+    pub fn set_slot(&mut self, slot: Slot) {
+        self.slot = slot;
+    }
 }
 
 pub enum LoadedProgramMatchCriteria {

programs/bpf_loader/src/lib.rs

@@ -10,7 +10,9 @@ use {
         compute_budget::ComputeBudget,
         ic_logger_msg, ic_msg,
         invoke_context::{BpfAllocator, InvokeContext, SyscallContext},
-        loaded_programs::{LoadProgramMetrics, LoadedProgram, LoadedProgramType},
+        loaded_programs::{
+            LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET,
+        },
         log_collector::LogCollector,
         stable_log,
         sysvar_cache::get_sysvar_with_account_check,
@@ -91,7 +93,7 @@ pub fn load_program_from_bytes(
     register_syscalls_time.stop();
     load_program_metrics.register_syscalls_us = register_syscalls_time.as_us();
     let effective_slot = if feature_set.is_active(&delay_visibility_of_program_deployment::id()) {
-        deployment_slot.saturating_add(1)
+        deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET)
     } else {
         deployment_slot
     };
@@ -190,6 +192,19 @@ pub fn load_program_from_account(
     Ok((loaded_program, Some(load_program_metrics)))
 }
 
+fn find_program_in_cache(
+    invoke_context: &InvokeContext,
+    pubkey: &Pubkey,
+) -> Option<Arc<LoadedProgram>> {
+    // First lookup the cache of the programs modified by the current transaction. If not found, lookup
+    // the cache of the cache of the programs that are loaded for the transaction batch.
+    invoke_context
+        .programs_modified_by_tx
+        .borrow()
+        .find(pubkey)
+        .or_else(|| invoke_context.programs_loaded_for_tx_batch.find(pubkey))
+}
+
 macro_rules! deploy_program {
     ($invoke_context:expr, $program_id:expr, $loader_key:expr,
      $account_size:expr, $slot:expr, $drop:expr, $new_programdata:expr $(,)?) => {{
@@ -206,7 +221,7 @@ macro_rules! deploy_program {
             true, /* reject_deployment_of_broken_elfs */
             false, /* debugging_features */
         )?;
-        if let Some(old_entry) = $invoke_context.programs_loaded_for_tx_batch.borrow().find(&$program_id) {
+        if let Some(old_entry) = find_program_in_cache($invoke_context, &$program_id) {
             let usage_counter = old_entry.usage_counter.load(Ordering::Relaxed);
             executor.usage_counter.store(usage_counter, Ordering::Relaxed);
         }
@@ -553,18 +568,7 @@ fn process_instruction_inner(
     }
 
     let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time");
-    // First lookup the cache of the programs modified by the current transaction. If not found, lookup
-    // the cache of the cache of the programs that are loaded for the transaction batch.
-    let executor = invoke_context
-        .programs_modified_by_tx
-        .borrow()
-        .find(program_account.get_key())
-        .or_else(|| {
-            invoke_context
-                .programs_loaded_for_tx_batch
-                .borrow()
-                .find(program_account.get_key())
-        })
+    let executor = find_program_in_cache(invoke_context, program_account.get_key())
         .ok_or(InstructionError::InvalidAccountData)?;
 
     if executor.is_tombstone() {
@@ -1661,7 +1665,10 @@ fn execute<'a, 'b: 'a>(
 }
 
 pub mod test_utils {
-    use {super::*, solana_sdk::account::ReadableAccount};
+    use {
+        super::*, solana_program_runtime::loaded_programs::DELAY_VISIBILITY_SLOT_OFFSET,
+        solana_sdk::account::ReadableAccount,
+    };
 
     pub fn load_all_invoked_programs(invoke_context: &mut InvokeContext) {
         let num_accounts = invoke_context.transaction_context.get_number_of_accounts();
@@ -1693,7 +1700,8 @@ pub mod test_utils {
                     true,
                     false,
                 ) {
-                    let mut cache = invoke_context.programs_loaded_for_tx_batch.borrow_mut();
+                    let mut cache = invoke_context.programs_modified_by_tx.borrow_mut();
+                    cache.set_slot(DELAY_VISIBILITY_SLOT_OFFSET);
                     cache.replenish(*pubkey, Arc::new(loaded_program));
                 }
             }
@@ -4097,7 +4105,7 @@ mod tests {
             usage_counter: AtomicU64::new(100),
         };
         invoke_context
-            .programs_loaded_for_tx_batch
+            .programs_modified_by_tx
             .borrow_mut()
             .replenish(program_id, Arc::new(program));
 
@@ -4130,7 +4138,7 @@ mod tests {
             usage_counter: AtomicU64::new(100),
         };
         invoke_context
-            .programs_loaded_for_tx_batch
+            .programs_modified_by_tx
             .borrow_mut()
             .replenish(program_id, Arc::new(program));
 

programs/sbf/tests/programs.rs

@@ -379,7 +379,10 @@ fn test_program_sbf_loader_deprecated() {
         let bank = Bank::new_for_tests(&genesis_config);
         let program_id = create_program(&bank, &bpf_loader_deprecated::id(), program);
 
-        let bank_client = BankClient::new(bank);
+        let mut bank_client = BankClient::new(bank);
+        bank_client
+            .advance_slot(1, &Pubkey::default())
+            .expect("Failed to advance the slot");
         let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)];
         let instruction = Instruction::new_with_bytes(program_id, &[1], account_metas);
         let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
@@ -3908,7 +3911,10 @@ fn test_program_sbf_inner_instruction_alignment_checks() {
 
     // invoke unaligned program, which will call aligned program twice,
     // unaligned should be allowed once invoke completes
-    let bank_client = BankClient::new(bank);
+    let mut bank_client = BankClient::new(bank);
+    bank_client
+        .advance_slot(1, &Pubkey::default())
+        .expect("Failed to advance the slot");
     let mut instruction = Instruction::new_with_bytes(
         inner_instruction_alignment_check,
         &[0],

runtime/src/bank.rs

@@ -4264,7 +4264,7 @@ impl Bank {
         timings: &mut ExecuteTimings,
         error_counters: &mut TransactionErrorMetrics,
         log_messages_bytes_limit: Option<usize>,
-        programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+        programs_loaded_for_tx_batch: Rc<LoadedProgramsForTxBatch>,
     ) -> TransactionExecutionResult {
         let prev_accounts_data_len = self.load_accounts_data_size();
         let transaction_accounts = std::mem::take(&mut loaded_transaction.accounts);
@@ -4620,7 +4620,8 @@ impl Bank {
             &self.blockhash_queue.read().unwrap(),
         );
 
-        let programs_loaded_for_tx_batch = self.replenish_program_cache(&program_accounts_map);
+        let programs_loaded_for_tx_batch =
+            Rc::new(self.replenish_program_cache(&program_accounts_map));
 
         let mut load_time = Measure::start("accounts_load");
         let mut loaded_transactions = self.rc.accounts.load_accounts(
@@ -4638,7 +4639,6 @@ impl Bank {
         );
         load_time.stop();
 
-        let programs_loaded_for_tx_batch = Rc::new(RefCell::new(programs_loaded_for_tx_batch));
         let mut execution_time = Measure::start("execution_time");
         let mut signature_count: u64 = 0;
 

runtime/src/bank/tests.rs

@@ -38,7 +38,7 @@ use {
         declare_process_instruction,
         executor_cache::TransactionExecutorCache,
         invoke_context::mock_process_instruction,
-        loaded_programs::{LoadedProgram, LoadedProgramType},
+        loaded_programs::{LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET},
         prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType},
         timings::ExecuteTimings,
     },
@@ -7771,7 +7771,8 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
         Ok(()),
         solana_bpf_loader_program::process_instruction,
         |invoke_context| {
-            let mut cache = invoke_context.programs_loaded_for_tx_batch.borrow_mut();
+            let mut cache = invoke_context.programs_modified_by_tx.borrow_mut();
+            cache.set_slot(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
             cache.replenish(program_keypair.pubkey(), loaded_program.clone());
         },
         |_invoke_context| {},

runtime/src/message_processor.rs

@@ -58,7 +58,7 @@ impl MessageProcessor {
         transaction_context: &mut TransactionContext,
         rent: Rent,
         log_collector: Option<Rc<RefCell<LogCollector>>>,
-        programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+        programs_loaded_for_tx_batch: Rc<LoadedProgramsForTxBatch>,
         programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
         feature_set: Arc<FeatureSet>,
         compute_budget: ComputeBudget,
@@ -274,8 +274,7 @@ mod tests {
         let mut transaction_context =
             TransactionContext::new(accounts, Some(Rent::default()), 1, 3);
         let program_indices = vec![vec![2]];
-        let programs_loaded_for_tx_batch =
-            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default()));
+        let programs_loaded_for_tx_batch = Rc::new(LoadedProgramsForTxBatch::default());
         let account_keys = (0..transaction_context.get_number_of_accounts())
             .map(|index| {
                 *transaction_context
@@ -502,8 +501,7 @@ mod tests {
         let mut transaction_context =
             TransactionContext::new(accounts, Some(Rent::default()), 1, 3);
         let program_indices = vec![vec![2]];
-        let programs_loaded_for_tx_batch =
-            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default()));
+        let programs_loaded_for_tx_batch = Rc::new(LoadedProgramsForTxBatch::default());
         let account_metas = vec![
             AccountMeta::new(
                 *transaction_context.get_key_of_account_at_index(0).unwrap(),
@@ -678,7 +676,7 @@ mod tests {
             &mut transaction_context,
             RentCollector::default().rent,
             None,
-            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
+            Rc::new(LoadedProgramsForTxBatch::default()),
             Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
             Arc::new(FeatureSet::all_enabled()),
             ComputeBudget::default(),