Switch programs activation to whole-set based gating

genesis-programs/src/lib.rs

@@ -49,102 +49,86 @@ enum Program {
     BuiltinLoader((String, Pubkey, ProcessInstructionWithContext)),
 }
 
-fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Option<Vec<Program>> {
+// given operating_mode and epoch, return the entire set of enabled programs
+fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec<Program> {
+    let mut programs = vec![];
+
     match operating_mode {
         OperatingMode::Development => {
-            if epoch == 0 {
-                // Programs used for testing
-                Some(vec![
-                    Program::BuiltinLoader(solana_bpf_loader_program!()),
-                    Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()),
-                    Program::Native(solana_vest_program!()),
-                    Program::Native(solana_budget_program!()),
-                    Program::Native(solana_exchange_program!()),
-                ])
-            } else if epoch == std::u64::MAX {
+            // Programs used for testing
+            programs.extend(vec![
+                Program::BuiltinLoader(solana_bpf_loader_program!()),
+                Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()),
+                Program::Native(solana_vest_program!()),
+                Program::Native(solana_budget_program!()),
+                Program::Native(solana_exchange_program!()),
+            ]);
+
+            #[allow(clippy::absurd_extreme_comparisons)]
+            if epoch >= std::u64::MAX {
                 // The epoch of std::u64::MAX is a placeholder and is expected
                 // to be reduced in a future network update.
-                Some(vec![Program::BuiltinLoader(solana_bpf_loader_program!())])
-            } else {
-                None
+                programs.extend(vec![Program::BuiltinLoader(solana_bpf_loader_program!())]);
             }
         }
-        OperatingMode::Stable => {
-            if epoch == std::u64::MAX {
+        OperatingMode::Preview => {
+            #[allow(clippy::absurd_extreme_comparisons)]
+            if epoch >= std::u64::MAX {
                 // The epoch of std::u64::MAX is a placeholder and is expected
                 // to be reduced in a future network update.
-                Some(vec![
+                programs.extend(vec![
                     Program::BuiltinLoader(solana_bpf_loader_program!()),
                     Program::Native(solana_vest_program!()),
                 ])
-            } else {
-                None
             }
         }
-        OperatingMode::Preview => {
-            if epoch == std::u64::MAX {
+        OperatingMode::Stable => {
+            // at which epoch, bpf_loader_program is enabled??
+            #[allow(clippy::absurd_extreme_comparisons)]
+            if epoch >= std::u64::MAX {
                 // The epoch of std::u64::MAX is a placeholder and is expected
                 // to be reduced in a future network update.
-                Some(vec![
+                programs.extend(vec![
                     Program::BuiltinLoader(solana_bpf_loader_program!()),
                     Program::Native(solana_vest_program!()),
-                ])
-            } else {
-                None
+                ]);
             }
         }
-    }
+    };
+
+    programs
 }
 
-pub fn get_native_programs(
-    operating_mode: OperatingMode,
-    epoch: Epoch,
-) -> Option<Vec<(String, Pubkey)>> {
-    match get_programs(operating_mode, epoch) {
-        Some(programs) => {
-            let mut native_programs = vec![];
-            for program in programs {
-                if let Program::Native((string, key)) = program {
-                    native_programs.push((string, key));
-                }
-            }
-            Some(native_programs)
+pub fn get_native_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec<(String, Pubkey)> {
+    let mut native_programs = vec![];
+    for program in get_programs(operating_mode, epoch) {
+        if let Program::Native((string, key)) = program {
+            native_programs.push((string, key));
         }
-        None => None,
     }
+    native_programs
 }
 
 pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpochCallback {
     Box::new(move |bank: &mut Bank| {
+        // Be careful to add arbitrary logic here; this should be idempotent and can be called
+        // at arbitrary point in an epoch not only epoch boundaries.
+        // This is because this closure need to be executed immediately after snapshot restoration,
+        // in addition to usual epoch boundaries
         if let Some(inflation) = get_inflation(operating_mode, bank.epoch()) {
             info!("Entering new epoch with inflation {:?}", inflation);
             bank.set_inflation(inflation);
         }
-        if let Some(programs) = get_programs(operating_mode, bank.epoch()) {
-            for program in programs {
-                match program {
-                    Program::Native((name, program_id)) => {
-                        bank.add_native_program(&name, &program_id);
-                    }
-                    Program::BuiltinLoader((
-                        name,
-                        program_id,
-                        process_instruction_with_context,
-                    )) => {
-                        bank.add_builtin_loader(
-                            &name,
-                            program_id,
-                            process_instruction_with_context,
-                        );
-                    }
+        for program in get_programs(operating_mode, bank.epoch()) {
+            match program {
+                Program::Native((name, program_id)) => {
+                    bank.add_native_program(&name, &program_id);
+                }
+                Program::BuiltinLoader((name, program_id, process_instruction_with_context)) => {
+                    bank.add_builtin_loader(&name, program_id, process_instruction_with_context);
                 }
             }
         }
-        if OperatingMode::Stable == operating_mode {
-            bank.set_cross_program_support(bank.epoch() >= 63);
-        } else {
-            bank.set_cross_program_support(true);
-        }
     })
 }
 
@@ -156,7 +140,7 @@ mod tests {
     #[test]
     fn test_id_uniqueness() {
         let mut unique = HashSet::new();
-        let programs = get_programs(OperatingMode::Development, 0).unwrap();
+        let programs = get_programs(OperatingMode::Development, 0);
         for program in programs {
             match program {
                 Program::Native((name, id)) => assert!(unique.insert((name, id))),
@@ -176,22 +160,14 @@ mod tests {
 
     #[test]
     fn test_development_programs() {
-        assert_eq!(
-            get_programs(OperatingMode::Development, 0).unwrap().len(),
-            5
-        );
-        assert!(get_programs(OperatingMode::Development, 1).is_none());
+        assert_eq!(get_programs(OperatingMode::Development, 0).len(), 5);
+        assert_eq!(get_programs(OperatingMode::Development, 1).len(), 5);
     }
 
     #[test]
     fn test_native_development_programs() {
-        assert_eq!(
-            get_native_programs(OperatingMode::Development, 0)
-                .unwrap()
-                .len(),
-            3
-        );
-        assert!(get_native_programs(OperatingMode::Development, 1).is_none());
+        assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3);
+        assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3);
     }
 
     #[test]
@@ -209,7 +185,7 @@ mod tests {
 
     #[test]
     fn test_softlaunch_programs() {
-        assert!(get_programs(OperatingMode::Stable, 1).is_none());
-        assert!(get_programs(OperatingMode::Stable, std::u64::MAX).is_some());
+        assert!(get_programs(OperatingMode::Stable, 1).is_empty());
+        assert!(!get_programs(OperatingMode::Stable, std::u64::MAX).is_empty());
     }
 }

genesis/src/main.rs

@@ -468,7 +468,7 @@ fn main() -> Result<(), Box<dyn error::Error>> {
     );
 
     let native_instruction_processors =
-        solana_genesis_programs::get_native_programs(operating_mode, 0).unwrap_or_else(Vec::new);
+        solana_genesis_programs::get_native_programs(operating_mode, 0);
     let inflation = solana_genesis_programs::get_inflation(operating_mode, 0).unwrap();
 
     let mut genesis_config = GenesisConfig {

local-cluster/src/local_cluster.rs

@@ -173,7 +173,6 @@ impl LocalCluster {
             OperatingMode::Stable | OperatingMode::Preview => {
                 genesis_config.native_instruction_processors =
                     solana_genesis_programs::get_native_programs(genesis_config.operating_mode, 0)
-                        .unwrap_or_default()
             }
             _ => (),
         }

runtime/src/bank.rs

@@ -10,7 +10,7 @@ use crate::{
     accounts_db::{ErrorCounters, SnapshotStorages},
     accounts_index::Ancestors,
     blockhash_queue::BlockhashQueue,
-    builtins::{get_builtins, get_epoch_activated_builtins},
+    builtins::get_builtins,
     epoch_stakes::{EpochStakes, NodeVoteAccounts},
     log_collector::LogCollector,
     message_processor::MessageProcessor,
@@ -557,17 +557,7 @@ impl Bank {
 
         let leader_schedule_epoch = epoch_schedule.get_leader_schedule_epoch(slot);
         if parent.epoch() < new.epoch() {
-            if let Some(entered_epoch_callback) =
-                parent.entered_epoch_callback.read().unwrap().as_ref()
-            {
-                entered_epoch_callback(&mut new)
-            }
-
-            if let Some(builtins) = get_epoch_activated_builtins(new.operating_mode(), new.epoch) {
-                for program in builtins.iter() {
-                    new.add_builtin(&program.name, program.id, program.entrypoint);
-                }
-            }
+            new.refresh_programs_and_inflation();
         }
 
         new.update_epoch_stakes(leader_schedule_epoch);
@@ -2598,10 +2588,7 @@ impl Bank {
     }
 
     pub fn finish_init(&mut self) {
-        let builtins = get_builtins();
-        for program in builtins.iter() {
-            self.add_builtin(&program.name, program.id, program.entrypoint);
-        }
+        self.refresh_programs_and_inflation();
     }
 
     pub fn set_parent(&mut self, parent: &Arc<Bank>) {
@@ -3178,6 +3165,30 @@ impl Bank {
         consumed_budget.saturating_sub(budget_recovery_delta)
     }
 
+    // This is called from snapshot restore and for each epoch boundary
+    // The entire code path herein must be idempotent
+    pub fn refresh_programs_and_inflation(&mut self) {
+        if let Some(entered_epoch_callback) =
+            self.entered_epoch_callback.clone().read().unwrap().as_ref()
+        {
+            entered_epoch_callback(self)
+        }
+
+        for program in get_builtins(self.operating_mode(), self.epoch()) {
+            self.add_builtin(&program.name, program.id, program.entrypoint);
+        }
+
+        self.recheck_cross_program_support();
+    }
+
+    fn recheck_cross_program_support(self: &mut Bank) {
+        if OperatingMode::Stable == self.operating_mode() {
+            self.set_cross_program_support(self.epoch() >= 63);
+        } else {
+            self.set_cross_program_support(true);
+        }
+    }
+
     fn fix_recent_blockhashes_sysvar_delay(&self) -> bool {
         let activation_slot = match self.operating_mode() {
             OperatingMode::Development => 0,
@@ -8091,4 +8102,17 @@ mod tests {
         );
         assert_eq!(bank.get_balance(&mint_keypair.pubkey()), 496); // no transaction fee charged
     }
+
+    #[test]
+    fn test_finish_init() {
+        let (genesis_config, _mint_keypair) = create_genesis_config(100_000);
+        let mut bank = Bank::new(&genesis_config);
+        bank.message_processor = MessageProcessor::default();
+        bank.message_processor.set_cross_program_support(false);
+
+        // simulate bank is just after deserialized from snapshot
+        bank.finish_init();
+
+        assert_eq!(bank.message_processor.get_cross_program_support(), true);
+    }
 }

runtime/src/builtins.rs

@@ -4,9 +4,11 @@ use crate::{
 };
 use solana_sdk::{clock::Epoch, genesis_config::OperatingMode, system_program};
 
-/// All builtin programs that should be active at the given (operating_mode, epoch)
-pub fn get_builtins() -> Vec<Builtin> {
-    vec![
+/// The entire set of available builtin programs that should be active at the given (operating_mode, epoch)
+pub fn get_builtins(_operating_mode: OperatingMode, _epoch: Epoch) -> Vec<Builtin> {
+    let mut builtins = vec![];
+
+    builtins.extend(vec![
         Builtin::new(
             "system_program",
             system_program::id(),
@@ -27,13 +29,10 @@ pub fn get_builtins() -> Vec<Builtin> {
             solana_vote_program::id(),
             Entrypoint::Program(solana_vote_program::vote_instruction::process_instruction),
         ),
-    ]
-}
+    ]);
+
+    // if we ever add gated builtins, add here like this
+    // if _epoch >= 10 { builtins.extend(....) }
 
-/// Builtin programs that activate at the given (operating_mode, epoch)
-pub fn get_epoch_activated_builtins(
-    _operating_mode: OperatingMode,
-    _epoch: Epoch,
-) -> Option<Vec<Builtin>> {
-    None
+    builtins
 }