Skip to content

fix: detect reorged sidechain blocks by canonical hash - #352

Closed
YoGhurt111 wants to merge 6 commits into
OpenZeppelin:mainfrom
YoGhurt111:fix-reorg-canonical-hash
Closed

fix: detect reorged sidechain blocks by canonical hash#352
YoGhurt111 wants to merge 6 commits into
OpenZeppelin:mainfrom
YoGhurt111:fix-reorg-canonical-hash

Conversation

@YoGhurt111

Copy link
Copy Markdown
Contributor

Summary

  • Detect reorgs by comparing a previously scanned block against the current canonical block at the same height.
  • Require buffered common-ancestor candidates to still be canonical before accepting them.
  • Add a regression test for providers that keep reorged sidechain blocks queryable by hash.

Root Cause

Some execution clients can still return a reorged sidechain block from eth_getBlockByHash after the canonical block at the same height has changed. The previous reorg check treated a successful hash lookup as proof that the buffered/scanned block was still canonical, so same-height replacements could be missed and the affected range would not be replayed.

Testing

  • cargo test --features test-utils detects_reorg_when_old_hash_is_available_but_not_canonical
  • cargo check --features test-utils

Note: cargo fmt --check on this machine reports unrelated formatting diffs in existing files (src/block_range_scanner/common.rs and src/event_scanner/block_range_handler.rs) due to the stable rustfmt handling of the repository rustfmt config. Those unrelated files were not changed in this PR.

@YoGhurt111
YoGhurt111 marked this pull request as ready for review May 25, 2026 12:43
@0xNeshi

0xNeshi commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Confirmed the finding that Geth can return a non-canonical block by hash, if the block is still in the node's DB.

Sources:

@YoGhurt111 feel free to list other clients that do the same

Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
@0xNeshi 0xNeshi added priority: high Important for MVP/core features type: bug Something isn't working labels May 27, 2026
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
Comment thread src/block_range_scanner/reorg_handler.rs Outdated
0xNeshi added 3 commits May 28, 2026 17:04
Co-authored-by: Nenad <xinef.it@gmail.com>
Co-authored-by: Nenad <xinef.it@gmail.com>

@0xNeshi 0xNeshi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@0xNeshi

0xNeshi commented May 29, 2026

Copy link
Copy Markdown
Collaborator

We have a signed commit requirement, so closing this PR with unsigned commits in favor of #354

@0xNeshi 0xNeshi closed this May 29, 2026
0xNeshi pushed a commit that referenced this pull request Jun 1, 2026
## Summary
- Detect reorgs by comparing a previously scanned block against the
current canonical block at the same height.
- Store buffered block numbers alongside hashes so common ancestor
lookup uses a single canonical block-by-number request.
- Add regression coverage for providers that keep reorged sidechain
blocks queryable by hash.

## Root Cause
Some execution clients can still return a reorged sidechain block from
`eth_getBlockByHash` after the canonical block at the same height has
changed. Treating a successful hash lookup as proof that a
buffered/scanned block is still canonical can miss same-height
replacements, so the affected range is not replayed.

## Fix
This change compares the saved block hash with the canonical block
returned by `eth_getBlockByNumber` at the saved height. The reorg buffer
stores `(BlockNumber, BlockHash)`, allowing common ancestor detection to
validate canonicality with one RPC call per buffered candidate.

## Testing
- `cargo test --features test-utils
detects_reorg_when_old_hash_is_available_but_not_canonical`
- `cargo check --features test-utils`

This PR replaces #352 with a single signed commit.
0xNeshi pushed a commit that referenced this pull request Jun 1, 2026
## Summary
- Detect reorgs by comparing a previously scanned block against the
current canonical block at the same height.
- Store buffered block numbers alongside hashes so common ancestor
lookup uses a single canonical block-by-number request.
- Add regression coverage for providers that keep reorged sidechain
blocks queryable by hash.

## Root Cause
Some execution clients can still return a reorged sidechain block from
`eth_getBlockByHash` after the canonical block at the same height has
changed. Treating a successful hash lookup as proof that a
buffered/scanned block is still canonical can miss same-height
replacements, so the affected range is not replayed.

## Fix
This change compares the saved block hash with the canonical block
returned by `eth_getBlockByNumber` at the saved height. The reorg buffer
stores `(BlockNumber, BlockHash)`, allowing common ancestor detection to
validate canonicality with one RPC call per buffered candidate.

## Testing
- `cargo test --features test-utils
detects_reorg_when_old_hash_is_available_but_not_canonical`
- `cargo check --features test-utils`

This PR replaces #352 with a single signed commit.

(cherry picked from commit 62de185)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: high Important for MVP/core features type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants