fix(debug-client): fix off-by-one in block hash buffer lookup (#21628)

Co-authored-by: Amp <amp@ampcode.com>

crates/consensus/debug-client/src/client.rs

@@ -38,12 +38,8 @@ pub trait BlockProvider: Send + Sync + 'static {
         offset: usize,
     ) -> impl Future<Output = eyre::Result<B256>> + Send {
         async move {
-            let stored_hash = previous_block_hashes
-                .len()
-                .checked_sub(offset)
-                .and_then(|index| previous_block_hashes.get(index));
-            if let Some(hash) = stored_hash {
-                return Ok(*hash);
+            if let Some(hash) = get_hash_at_offset(previous_block_hashes, offset) {
+                return Ok(hash);
             }
 
             // Return zero hash if the chain isn't long enough to have the block at the offset.
@@ -83,7 +79,7 @@ where
     /// Spawn the client to start sending FCUs and new payloads by periodically fetching recent
     /// blocks.
     pub async fn run(self) {
-        let mut previous_block_hashes = AllocRingBuffer::new(64);
+        let mut previous_block_hashes = AllocRingBuffer::new(65);
         let mut block_stream = {
             let (tx, rx) = mpsc::channel::<P::Block>(64);
             let block_provider = self.block_provider.clone();
@@ -142,3 +138,60 @@ where
         }
     }
 }
+
+/// Looks up a block hash from the ring buffer at the given offset from the most recent entry.
+///
+/// Returns `None` if the buffer doesn't have enough entries to satisfy the offset.
+fn get_hash_at_offset(buffer: &AllocRingBuffer<B256>, offset: usize) -> Option<B256> {
+    buffer.len().checked_sub(offset + 1).and_then(|index| buffer.get(index).copied())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_get_hash_at_offset() {
+        let mut buffer: AllocRingBuffer<B256> = AllocRingBuffer::new(65);
+
+        // Empty buffer returns None for any offset
+        assert_eq!(get_hash_at_offset(&buffer, 0), None);
+        assert_eq!(get_hash_at_offset(&buffer, 1), None);
+
+        // Push hashes 0..65
+        for i in 0..65u8 {
+            buffer.push(B256::with_last_byte(i));
+        }
+
+        // offset=0 should return the most recent (64)
+        assert_eq!(get_hash_at_offset(&buffer, 0), Some(B256::with_last_byte(64)));
+
+        // offset=32 (safe block) should return hash 32
+        assert_eq!(get_hash_at_offset(&buffer, 32), Some(B256::with_last_byte(32)));
+
+        // offset=64 (finalized block) should return hash 0 (the oldest)
+        assert_eq!(get_hash_at_offset(&buffer, 64), Some(B256::with_last_byte(0)));
+
+        // offset=65 exceeds buffer, should return None
+        assert_eq!(get_hash_at_offset(&buffer, 65), None);
+    }
+
+    #[test]
+    fn test_get_hash_at_offset_insufficient_entries() {
+        let mut buffer: AllocRingBuffer<B256> = AllocRingBuffer::new(65);
+
+        // With only 1 entry, only offset=0 works
+        buffer.push(B256::with_last_byte(1));
+        assert_eq!(get_hash_at_offset(&buffer, 0), Some(B256::with_last_byte(1)));
+        assert_eq!(get_hash_at_offset(&buffer, 1), None);
+        assert_eq!(get_hash_at_offset(&buffer, 32), None);
+        assert_eq!(get_hash_at_offset(&buffer, 64), None);
+
+        // With 33 entries, offset=32 works but offset=64 doesn't
+        for i in 2..=33u8 {
+            buffer.push(B256::with_last_byte(i));
+        }
+        assert_eq!(get_hash_at_offset(&buffer, 32), Some(B256::with_last_byte(1)));
+        assert_eq!(get_hash_at_offset(&buffer, 64), None);
+    }
+}