Restrict access to Bank's HardForks (#32180)

* Restrict access to Bank's HardForks

Callers could previously obtain a a lock to read/write HardForks from
any Bank. This would allow any caller to modify, and creates the
opportunity for inconsistent handling of what is considered a valid hard
fork (ie too old).

This PR adds a function to Bank so consistent sanity checks can be
applied; the caller will already have a Bank as that is where they would
have obtained the HardForks from in the first place. Additionally,
change the getter to return a copy of HardForks (simple Vec).

* Allow hard fork at bank slot if bank is not yet frozen

core/src/validator.rs

@@ -665,19 +665,15 @@ impl Validator {
             entry_notifier,
             Some(poh_timing_point_sender.clone()),
         )?;
+        let hard_forks = bank_forks.read().unwrap().root_bank().hard_forks();
+        if !hard_forks.is_empty() {
+            info!("Hard forks: {:?}", hard_forks);
+        }
 
         node.info.set_wallclock(timestamp());
         node.info.set_shred_version(compute_shred_version(
             &genesis_config.hash(),
-            Some(
-                &bank_forks
-                    .read()
-                    .unwrap()
-                    .working_bank()
-                    .hard_forks()
-                    .read()
-                    .unwrap(),
-            ),
+            Some(&hard_forks),
         ));
 
         Self::print_node_info(&node);
@@ -1674,21 +1670,6 @@ fn load_blockstore(
     // is processing the dropped banks from the `pruned_banks_receiver` channel.
     let pruned_banks_receiver =
         AccountsBackgroundService::setup_bank_drop_callback(bank_forks.clone());
-    {
-        let hard_forks: Vec<_> = bank_forks
-            .read()
-            .unwrap()
-            .working_bank()
-            .hard_forks()
-            .read()
-            .unwrap()
-            .iter()
-            .copied()
-            .collect();
-        if !hard_forks.is_empty() {
-            info!("Hard forks: {:?}", hard_forks);
-        }
-    }
 
     leader_schedule_cache.set_fixed_leader_schedule(config.fixed_leader_schedule.clone());
     {

ledger-tool/src/main.rs

@@ -2243,15 +2243,7 @@ fn main() {
                             "{}",
                             compute_shred_version(
                                 &genesis_config.hash(),
-                                Some(
-                                    &bank_forks
-                                        .read()
-                                        .unwrap()
-                                        .working_bank()
-                                        .hard_forks()
-                                        .read()
-                                        .unwrap()
-                                )
+                                Some(&bank_forks.read().unwrap().working_bank().hard_forks())
                             )
                         );
                     }
@@ -3120,10 +3112,7 @@ fn main() {
 
                         println!(
                             "Shred version: {}",
-                            compute_shred_version(
-                                &genesis_config.hash(),
-                                Some(&bank.hard_forks().read().unwrap())
-                            )
+                            compute_shred_version(&genesis_config.hash(), Some(&bank.hard_forks()))
                         );
                     }
                     Err(err) => {

ledger/src/bank_forks_utils.rs

@@ -172,18 +172,9 @@ pub fn load_bank_forks(
 
     if let Some(ref new_hard_forks) = process_options.new_hard_forks {
         let root_bank = bank_forks.read().unwrap().root_bank();
-        let hard_forks = root_bank.hard_forks();
-
-        for hard_fork_slot in new_hard_forks.iter() {
-            if *hard_fork_slot > root_bank.slot() {
-                hard_forks.write().unwrap().register(*hard_fork_slot);
-            } else {
-                warn!(
-                    "Hard fork at {} ignored, --hard-fork option can be removed.",
-                    hard_fork_slot
-                );
-            }
-        }
+        new_hard_forks
+            .iter()
+            .for_each(|hard_fork_slot| root_bank.register_hard_fork(*hard_fork_slot));
     }
 
     (bank_forks, leader_schedule_cache, starting_snapshot_hashes)

program-test/src/lib.rs

@@ -1211,9 +1211,10 @@ impl ProgramTestContext {
 
     /// record a hard fork slot in working bank; should be in the past
     pub fn register_hard_fork(&mut self, hard_fork_slot: Slot) {
-        let bank_forks = self.bank_forks.write().unwrap();
-        let hard_forks = bank_forks.working_bank().hard_forks();
-        let mut write = hard_forks.write().unwrap();
-        write.register(hard_fork_slot);
+        self.bank_forks
+            .read()
+            .unwrap()
+            .working_bank()
+            .register_hard_fork(hard_fork_slot)
     }
 }

program-test/tests/sysvar_last_restart_slot.rs

@@ -92,13 +92,13 @@ async fn get_sysvar_last_restart_slot() {
     context.register_hard_fork(40 as Slot);
     context.warp_to_slot(45).unwrap();
     check_with_program(&mut context, program_id, 41).await;
-    context.register_hard_fork(43 as Slot);
     context.register_hard_fork(47 as Slot);
+    context.register_hard_fork(48 as Slot);
     context.warp_to_slot(46).unwrap();
-    check_with_program(&mut context, program_id, 43).await;
+    check_with_program(&mut context, program_id, 41).await;
     context.register_hard_fork(50 as Slot);
     context.warp_to_slot(48).unwrap();
-    check_with_program(&mut context, program_id, 47).await;
+    check_with_program(&mut context, program_id, 48).await;
     context.warp_to_slot(50).unwrap();
     check_with_program(&mut context, program_id, 50).await;
 }

runtime/src/bank.rs

@@ -2426,8 +2426,7 @@ impl Bank {
         if feature_flag {
             let last_restart_slot = {
                 let slot = self.slot;
-                let hard_forks = self.hard_forks();
-                let hard_forks_r = hard_forks.read().unwrap();
+                let hard_forks_r = self.hard_forks.read().unwrap();
 
                 // Only consider hard forks <= this bank's slot to avoid prematurely applying
                 // a hard fork that is set to occur in the future.
@@ -7309,8 +7308,33 @@ impl Bank {
         *self.inflation.write().unwrap() = inflation;
     }
 
-    pub fn hard_forks(&self) -> Arc<RwLock<HardForks>> {
-        self.hard_forks.clone()
+    /// Get a snapshot of the current set of hard forks
+    pub fn hard_forks(&self) -> HardForks {
+        self.hard_forks.read().unwrap().clone()
+    }
+
+    pub fn register_hard_fork(&self, new_hard_fork_slot: Slot) {
+        let bank_slot = self.slot();
+
+        let lock = self.freeze_lock();
+        let bank_frozen = *lock != Hash::default();
+        if new_hard_fork_slot < bank_slot {
+            warn!(
+                "Hard fork at slot {new_hard_fork_slot} ignored, the hard fork is older \
+                than the bank at slot {bank_slot} that attempted to register it."
+            );
+        } else if (new_hard_fork_slot == bank_slot) && bank_frozen {
+            warn!(
+                "Hard fork at slot {new_hard_fork_slot} ignored, the hard fork is the same \
+                slot as the bank at slot {bank_slot} that attempted to register it, but that \
+                bank is already frozen."
+            );
+        } else {
+            self.hard_forks
+                .write()
+                .unwrap()
+                .register(new_hard_fork_slot);
+        }
     }
 
     // Hi! leaky abstraction here....

sdk/src/hard_forks.rs

@@ -33,6 +33,11 @@ impl HardForks {
         self.hard_forks.iter()
     }
 
+    // Returns `true` is there are currently no registered hard forks
+    pub fn is_empty(&self) -> bool {
+        self.hard_forks.is_empty()
+    }
+
     // Returns data to include in the bank hash for the given slot if a hard fork is scheduled
     pub fn get_hash_data(&self, slot: Slot, parent_slot: Slot) -> Option<[u8; 8]> {
         // The expected number of hard forks in a cluster is small.