fix(database): BlockHashCache incorrectly returns zero for block 0#3319
Merged
rakita merged 3 commits intobluealloy:mainfrom Jan 16, 2026
Merged
fix(database): BlockHashCache incorrectly returns zero for block 0#3319rakita merged 3 commits intobluealloy:mainfrom
rakita merged 3 commits intobluealloy:mainfrom
Conversation
The ring buffer cache introduced in bluealloy#3299 was initialized with `(0, B256::ZERO)` for all entries. This caused `get(0)` to incorrectly match the default entry and return `B256::ZERO` instead of `None`, preventing the database fallback from being invoked. This is a consensus bug for any chain at low block numbers (< 256) where BLOCKHASH(0) should return the genesis block hash. Fix: Use `Option<B256>` instead of `B256` to explicitly distinguish between "not cached" (`None`) and "cached with value" (`Some(hash)`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
bshastry
force-pushed
the
fix/block-hash-cache-block-zero
branch
from
de59c02 to
1463bd8
Compare
rakita
reviewed
Jan 16, 2026
| /// The reason we store block number alongside its hash is to handle the case where it wraps around, | ||
| /// so we can verify the block number. Uses `Option<B256>` to distinguish between "not cached" | ||
| /// and "cached with value" - this is important because block 0 is a valid block number. | ||
| hashes: Box<[(u64, Option<B256>); BLOCK_HASH_HISTORY_USIZE]>, |
Member
There was a problem hiding this comment.
A slightly better approach would be to have Option as block number is used to deduce if block hash is up to date
| /// Inserts a block hash for the given block number. | ||
| #[inline] | ||
| pub const fn insert(&mut self, block_number: u64, block_hash: B256) { | ||
| pub fn insert(&mut self, block_number: u64, block_hash: B256) { |
Member
There was a problem hiding this comment.
No need to remove consts, right?
Address review feedback: - Use Option<u64> for block number instead of Option<B256> for hash - Keep const on insert and get functions
rakita
reviewed
Jan 16, 2026
| Some(stored_hash) | ||
| } else { | ||
| None | ||
| if let Some(stored) = stored_block_number { |
Member
There was a problem hiding this comment.
Can be simplified with if Some(block_number) == stored_block_number {
Use direct Option comparison as suggested in review.
rakita
approved these changes
Jan 16, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes regression caused by #3299
The ring buffer cache introduced in #3299 was initialized with
(0, B256::ZERO)for all entries. This causedget(0)to incorrectly match the default entry and returnB256::ZEROinstead ofNone, preventing the database fallback from being invoked.Fix: Use
Option<B256>instead ofB256to explicitly distinguish between "not cached" (None) and "cached with value" (Some(hash)).🤖 Generated with Claude Code