Remove redundant bounds check from getBlock and getBlockTime

rpc/src/rpc.rs

@@ -1002,26 +1002,36 @@ impl JsonRpcRequestProcessor {
         })
     }
 
-    fn check_blockstore_root<T>(
+    // 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>(
         &self,
         result: &std::result::Result<T, BlockstoreError>,
         slot: Slot,
     ) -> Result<()> {
-        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());
-            }
-            if self.blockstore.is_skipped(slot) {
-                return Err(RpcCustomError::SlotSkipped { slot }.into());
+        match result {
+            // The slot was found, all good
+            Ok(Some(_)) => 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
+            Ok(_) => {
+                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 {
+                    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())
             }
+            _ => Ok(()),
         }
-        Ok(())
     }
 
     fn check_slot_cleaned_up<T>(
@@ -1098,7 +1108,7 @@ impl JsonRpcRequestProcessor {
             {
                 self.check_blockstore_writes_complete(slot)?;
                 let result = self.blockstore.get_rooted_block(slot, true);
-                self.check_blockstore_root(&result, slot)?;
+                self.check_blockstore_bounds(&result, slot)?;
                 let encode_block = |confirmed_block: ConfirmedBlock| -> Result<UiConfirmedBlock> {
                     let mut encoded_block = confirmed_block
                         .encode_with_options(encoding, encoding_options)
@@ -1322,7 +1332,7 @@ impl JsonRpcRequestProcessor {
                 .highest_super_majority_root()
         {
             let result = self.blockstore.get_rooted_block_time(slot);
-            self.check_blockstore_root(&result, slot)?;
+            self.check_blockstore_bounds(&result, slot)?;
             if result.is_err() || matches!(result, Ok(None)) {
                 if let Some(bigtable_ledger_storage) = &self.bigtable_ledger_storage {
                     let bigtable_result = bigtable_ledger_storage.get_confirmed_block(slot).await;