Revert " Remove redundant bounds check from getBlock and getBlockTime… (#33996)

Revert " Remove redundant bounds check from getBlock and getBlockTime (#33901)"

This reverts commit 03a456e7bb.

ledger/src/blockstore.rs

@@ -3104,6 +3104,21 @@ impl Blockstore {
         matches!(self.db.get::<cf::Root>(slot), Ok(Some(true)))
     }
 
+    /// Returns true if a slot is between the rooted slot bounds of the ledger, but has not itself
+    /// been rooted. This is either because the slot was skipped, or due to a gap in ledger data,
+    /// as when booting from a newer snapshot.
+    pub fn is_skipped(&self, slot: Slot) -> bool {
+        let lowest_root = self
+            .rooted_slot_iterator(0)
+            .ok()
+            .and_then(|mut iter| iter.next())
+            .unwrap_or_default();
+        match self.db.get::<cf::Root>(slot).ok().flatten() {
+            Some(_) => false,
+            None => slot < self.max_root() && slot > lowest_root,
+        }
+    }
+
     pub fn insert_bank_hash(&self, slot: Slot, frozen_hash: Hash, is_duplicate_confirmed: bool) {
         if let Some(prev_value) = self.bank_hash_cf.get(slot).unwrap() {
             if prev_value.frozen_hash() == frozen_hash && prev_value.is_duplicate_confirmed() {
@@ -6853,6 +6868,22 @@ pub mod tests {
         }
     }
 
+    #[test]
+    fn test_is_skipped() {
+        let ledger_path = get_tmp_ledger_path_auto_delete!();
+        let blockstore = Blockstore::open(ledger_path.path()).unwrap();
+        let roots = [2, 4, 7, 12, 15];
+        blockstore.set_roots(roots.iter()).unwrap();
+
+        for i in 0..20 {
+            if i < 2 || roots.contains(&i) || i > 15 {
+                assert!(!blockstore.is_skipped(i));
+            } else {
+                assert!(blockstore.is_skipped(i));
+            }
+        }
+    }
+
     #[test]
     fn test_iter_bounds() {
         let ledger_path = get_tmp_ledger_path_auto_delete!();

rpc/src/rpc.rs

@@ -1002,37 +1002,26 @@ impl JsonRpcRequestProcessor {
         })
     }
 
-    // Check if the given `slot` is within the blockstore bounds. This function assumes that
-    // `result` is from a blockstore fetch, and that the fetch:
-    // 1) Checked if `slot` is above the lowest cleanup slot (and errored if not)
-    // 2) Checked if `slot` is a root
-    fn check_blockstore_bounds<T>(
+    fn check_blockstore_root<T>(
         &self,
         result: &std::result::Result<T, BlockstoreError>,
         slot: Slot,
     ) -> Result<()> {
-        match result {
-            // The slot was found, all good
-            Ok(_) => Ok(()),
-            // The slot was cleaned up, return Ok() for now to allow fallback to bigtable
-            Err(BlockstoreError::SlotCleanedUp) => Ok(()),
-            // The slot was not cleaned up but also not found
-            Err(BlockstoreError::SlotNotRooted) => {
-                let max_root = self.blockstore.max_root();
-                debug!("check_blockstore_bounds, slot: {slot}, max root: {max_root}");
-                // Our node hasn't seen this slot yet, error out
-                if slot >= max_root {
+        if let Err(err) = result {
+            debug!(
+                "check_blockstore_root, slot: {:?}, max root: {:?}, err: {:?}",
+                slot,
+                self.blockstore.max_root(),
+                err
+            );
+            if slot >= self.blockstore.max_root() {
                 return Err(RpcCustomError::BlockNotAvailable { slot }.into());
             }
-                // The slot is within the bounds of the blockstore as the lookup that yielded
-                // `result` checked that `slot` was greater than the blockstore's lowest
-                // cleanup slot and we just checked that `slot` was less than the blockstore's
-                // largest root. Thus, the slot must have been skipped and we can error out.
-                Err(RpcCustomError::SlotSkipped { slot }.into())
+            if self.blockstore.is_skipped(slot) {
+                return Err(RpcCustomError::SlotSkipped { slot }.into());
             }
-            // Some other Blockstore error, ignore for now
-            _ => Ok(()),
         }
+        Ok(())
     }
 
     fn check_slot_cleaned_up<T>(
@@ -1109,7 +1098,7 @@ impl JsonRpcRequestProcessor {
             {
                 self.check_blockstore_writes_complete(slot)?;
                 let result = self.blockstore.get_rooted_block(slot, true);
-                self.check_blockstore_bounds(&result, slot)?;
+                self.check_blockstore_root(&result, slot)?;
                 let encode_block = |confirmed_block: ConfirmedBlock| -> Result<UiConfirmedBlock> {
                     let mut encoded_block = confirmed_block
                         .encode_with_options(encoding, encoding_options)
@@ -1333,7 +1322,7 @@ impl JsonRpcRequestProcessor {
                 .highest_super_majority_root()
         {
             let result = self.blockstore.get_rooted_block_time(slot);
-            self.check_blockstore_bounds(&result, slot)?;
+            self.check_blockstore_root(&result, slot)?;
             if result.is_err() {
                 if let Some(bigtable_ledger_storage) = &self.bigtable_ledger_storage {
                     let bigtable_result = bigtable_ledger_storage.get_confirmed_block(slot).await;