Skip to content

Conversation

@estensen
Copy link
Contributor

…xclusion

Introduces orphaned_l1_hashes table and writer support, adds L1 anti- reorg filter to ClickHouse reader, and wires L1 reorg detection in the processor to record orphaned L1 block hashes (one-block and traditional reorgs). Moves schema changes into a new forward migration to avoid modifying initial schema.

…xclusion

Introduces `orphaned_l1_hashes` table and writer support, adds L1 anti-
reorg filter to ClickHouse reader, and wires L1 reorg detection in the
processor to record orphaned L1 block hashes (one-block and traditional
reorgs). Moves schema changes into a new forward migration to avoid
modifying initial schema.
@estensen estensen requested a review from Copilot August 11, 2025 15:23
@vercel
Copy link

vercel bot commented Aug 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
taikoscope ✅ Ready (Inspect) Visit Preview Aug 11, 2025 3:24pm

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces L1 reorg filtering functionality to handle L1 blockchain reorganizations by tracking orphaned L1 block hashes and excluding them from queries.

Key changes:

  • Adds L1 reorg detection using a dedicated ReorgDetector instance
  • Creates orphaned_l1_hashes table to store orphaned L1 block hashes
  • Implements query-side filtering to exclude orphaned L1 blocks from results

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/driver/src/processor.rs Adds L1 reorg detection logic and orphaned hash recording
crates/clickhouse/src/writer.rs Implements writer method for inserting orphaned L1 hashes
crates/clickhouse/src/schema.rs Adds orphaned_l1_hashes table schema definition
crates/clickhouse/src/reader/client.rs Implements L1 reorg filtering in queries and hash lookup method
crates/clickhouse/src/models.rs Defines OrphanedL1HashRow data model
crates/clickhouse/migrations/019_create_orphaned_l1_hashes.sql Creates orphaned_l1_hashes table with bloom filter index
crates/clickhouse/migrations/016_add_data_skipping_indices.sql Adds extra blank line
crates/clickhouse/migrations/001_create_tables.sql Removes trailing blank line

"Failed to insert orphaned L1 hash"
);
} else {
info!(block_number = header.number, orphaned_hash = ?hash, "Inserted orphaned L1 hash");
Copy link

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

Using info! for this logging is inconsistent with the error case above which uses tracing::error!. Consider using tracing::info! for consistency.

Suggested change
info!(block_number = header.number, orphaned_hash = ?hash, "Inserted orphaned L1 hash");
tracing::info!(block_number = header.number, orphaned_hash = ?hash, "Inserted orphaned L1 hash");

Copilot uses AI. Check for mistakes.
"Failed to insert orphaned L1 hashes"
);
} else {
info!(
Copy link

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

Using info! for this logging is inconsistent with the error case above which uses tracing::error!. Consider using tracing::info! for consistency.

Suggested change
info!(
tracing::info!(

Copilot uses AI. Check for mistakes.
db = self.db_name,
);

let rows = self.execute::<HashRow>(&query).await?;
Copy link

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

Direct string interpolation of user-provided block numbers into SQL query creates potential SQL injection vulnerability. Consider using parameterized queries or proper escaping.

Suggested change
let rows = self.execute::<HashRow>(&query).await?;
let query = format!(
"SELECT block_hash, l1_block_number \
FROM (\
SELECT block_hash, l1_block_number, \
ROW_NUMBER() OVER (PARTITION BY l1_block_number ORDER BY inserted_at DESC) as rn \
FROM {db}.l1_head_events \
WHERE l1_block_number IN {{block_numbers:Array(UInt64)}}\
) ranked \
WHERE rn = 1 \
ORDER BY l1_block_number",
db = self.db_name,
);
let mut query_builder = self.client.query(&query);
query_builder.bind("block_numbers", block_numbers);
let rows = query_builder
.fetch_all::<HashRow>()
.await
.wrap_err("Failed to fetch latest l1 hashes for blocks")?;

Copilot uses AI. Check for mistakes.
if depth > 0 {
if let Some(reader) = clickhouse_reader {
let new_head = header.number;
let orphaned_start = new_head.saturating_add(1);
Copy link

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

The logic for calculating orphaned block range appears incorrect. For a reorg where new_head < old_head, the orphaned blocks should be from new_head + 1 to old_head, but new_head.saturating_add(1) will be greater than old_head, making the range invalid.

Suggested change
let orphaned_start = new_head.saturating_add(1);
let orphaned_start = new_head + 1;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants