Move code to check_program_modification_slot out of SVM (#329)

* Move code to check_program_modification_slot out of SVM

* add documentation for the public function

Author: pgarg66

runtime/src/bank.rs

@@ -99,7 +99,8 @@ use {
         compute_budget_processor::process_compute_budget_instructions,
         invoke_context::BuiltinFunctionWithContext,
         loaded_programs::{
-            LoadedProgram, LoadedProgramType, LoadedPrograms, ProgramRuntimeEnvironments,
+            LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, LoadedPrograms,
+            ProgramRuntimeEnvironments,
         },
         runtime_config::RuntimeConfig,
         timings::{ExecuteTimingType, ExecuteTimings},
@@ -168,7 +169,8 @@ use {
         account_overrides::AccountOverrides,
         transaction_error_metrics::TransactionErrorMetrics,
         transaction_processor::{
-            TransactionBatchProcessor, TransactionLogMessages, TransactionProcessingCallback,
+            ExecutionRecordingConfig, TransactionBatchProcessor, TransactionLogMessages,
+            TransactionProcessingCallback,
         },
         transaction_results::{
             TransactionExecutionDetails, TransactionExecutionResult, TransactionResults,
@@ -271,7 +273,6 @@ pub struct BankRc {
 
 #[cfg(RUSTC_WITH_SPECIALIZATION)]
 use solana_frozen_abi::abi_example::AbiExample;
-use solana_svm::transaction_processor::ExecutionRecordingConfig;
 
 #[cfg(RUSTC_WITH_SPECIALIZATION)]
 impl AbiExample for BankRc {
@@ -550,6 +551,7 @@ impl PartialEq for Bank {
             loaded_programs_cache: _,
             epoch_reward_status: _,
             transaction_processor: _,
+            check_program_modification_slot: _,
             // Ignore new fields explicitly if they do not impact PartialEq.
             // Adding ".." will remove compile-time checks that if a new field
             // is added to the struct, this PartialEq is accordingly updated.
@@ -810,6 +812,8 @@ pub struct Bank {
     epoch_reward_status: EpochRewardStatus,
 
     transaction_processor: TransactionBatchProcessor<BankForks>,
+
+    check_program_modification_slot: bool,
 }
 
 struct VoteWithStakeDelegations {
@@ -996,6 +1000,7 @@ impl Bank {
             ))),
             epoch_reward_status: EpochRewardStatus::default(),
             transaction_processor: TransactionBatchProcessor::default(),
+            check_program_modification_slot: false,
         };
 
         bank.transaction_processor = TransactionBatchProcessor::new(
@@ -1314,6 +1319,7 @@ impl Bank {
             loaded_programs_cache: parent.loaded_programs_cache.clone(),
             epoch_reward_status: parent.epoch_reward_status.clone(),
             transaction_processor: TransactionBatchProcessor::default(),
+            check_program_modification_slot: false,
         };
 
         new.transaction_processor = TransactionBatchProcessor::new(
@@ -1864,6 +1870,7 @@ impl Bank {
             ))),
             epoch_reward_status: fields.epoch_reward_status,
             transaction_processor: TransactionBatchProcessor::default(),
+            check_program_modification_slot: false,
         };
 
         bank.transaction_processor = TransactionBatchProcessor::new(
@@ -7517,7 +7524,7 @@ impl Bank {
     }
 
     pub fn check_program_modification_slot(&mut self) {
-        self.transaction_processor.check_program_modification_slot = true;
+        self.check_program_modification_slot = true;
     }
 
     pub fn load_program(
@@ -7579,6 +7586,18 @@ impl TransactionProcessingCallback for Bank {
             Ok(())
         }
     }
+
+    fn get_program_match_criteria(&self, program: &Pubkey) -> LoadedProgramMatchCriteria {
+        if self.check_program_modification_slot {
+            self.transaction_processor
+                .program_modification_slot(self, program)
+                .map_or(LoadedProgramMatchCriteria::Tombstone, |slot| {
+                    LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(slot)
+                })
+        } else {
+            LoadedProgramMatchCriteria::NoCriteria
+        }
+    }
 }
 
 #[cfg(feature = "dev-context-only-utils")]

svm/src/transaction_processor.rs

@@ -103,6 +103,10 @@ pub trait TransactionProcessingCallback {
     ) -> transaction::Result<()> {
         Ok(())
     }
+
+    fn get_program_match_criteria(&self, _program: &Pubkey) -> LoadedProgramMatchCriteria {
+        LoadedProgramMatchCriteria::NoCriteria
+    }
 }
 
 #[derive(Debug)]
@@ -128,8 +132,6 @@ pub struct TransactionBatchProcessor<FG: ForkGraph> {
     /// Transaction fee structure
     fee_structure: FeeStructure,
 
-    pub check_program_modification_slot: bool,
-
     /// Optional config parameters that can override runtime behavior
     runtime_config: Arc<RuntimeConfig>,
 
@@ -145,10 +147,6 @@ impl<FG: ForkGraph> Debug for TransactionBatchProcessor<FG> {
             .field("epoch", &self.epoch)
             .field("epoch_schedule", &self.epoch_schedule)
             .field("fee_structure", &self.fee_structure)
-            .field(
-                "check_program_modification_slot",
-                &self.check_program_modification_slot,
-            )
             .field("runtime_config", &self.runtime_config)
             .field("sysvar_cache", &self.sysvar_cache)
             .field("loaded_programs_cache", &self.loaded_programs_cache)
@@ -163,7 +161,6 @@ impl<FG: ForkGraph> Default for TransactionBatchProcessor<FG> {
             epoch: Epoch::default(),
             epoch_schedule: EpochSchedule::default(),
             fee_structure: FeeStructure::default(),
-            check_program_modification_slot: false,
             runtime_config: Arc::<RuntimeConfig>::default(),
             sysvar_cache: RwLock::<SysvarCache>::default(),
             loaded_programs_cache: Arc::new(RwLock::new(LoadedPrograms::new(
@@ -188,7 +185,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             epoch,
             epoch_schedule,
             fee_structure,
-            check_program_modification_slot: false,
             runtime_config,
             sysvar_cache: RwLock::<SysvarCache>::default(),
             loaded_programs_cache,
@@ -491,30 +487,15 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         limit_to_load_programs: bool,
     ) -> LoadedProgramsForTxBatch {
         let mut missing_programs: Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))> =
-            if self.check_program_modification_slot {
-                program_accounts_map
-                    .iter()
-                    .map(|(pubkey, (_, count))| {
-                        (
-                            *pubkey,
-                            (
-                                self.program_modification_slot(callback, pubkey)
-                                    .map_or(LoadedProgramMatchCriteria::Tombstone, |slot| {
-                                        LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(slot)
-                                    }),
-                                *count,
-                            ),
-                        )
-                    })
-                    .collect()
-            } else {
-                program_accounts_map
-                    .iter()
-                    .map(|(pubkey, (_, count))| {
-                        (*pubkey, (LoadedProgramMatchCriteria::NoCriteria, *count))
-                    })
-                    .collect()
-            };
+            program_accounts_map
+                .iter()
+                .map(|(pubkey, (_, count))| {
+                    (
+                        *pubkey,
+                        (callback.get_program_match_criteria(pubkey), *count),
+                    )
+                })
+                .collect();
 
         let mut loaded_programs_for_txs = None;
         let mut program_to_store = None;
@@ -763,7 +744,11 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         }
     }
 
-    fn program_modification_slot<CB: TransactionProcessingCallback>(
+    /// Find the slot in which the program was most recently modified.
+    /// Returns slot 0 for programs deployed with v1/v2 loaders, since programs deployed
+    /// with those loaders do not retain deployment slot information.
+    /// Returns an error if the program's account state can not be found or parsed.
+    pub fn program_modification_slot<CB: TransactionProcessingCallback>(
         &self,
         callbacks: &CB,
         pubkey: &Pubkey,
@@ -1815,10 +1800,7 @@ mod tests {
     fn test_replenish_program_cache() {
         // Case 1
         let mut mock_bank = MockBankCallback::default();
-        let mut batch_processor = TransactionBatchProcessor::<TestForkGraph> {
-            check_program_modification_slot: true,
-            ..TransactionBatchProcessor::default()
-        };
+        let batch_processor = TransactionBatchProcessor::<TestForkGraph>::default();
         batch_processor
             .loaded_programs_cache
             .write()
@@ -1848,8 +1830,6 @@ mod tests {
         ));
 
         // Case 2
-        batch_processor.check_program_modification_slot = false;
-
         let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true);
 
         let program1 = result.find(&key1).unwrap();