Skip to content

init data-gatherer#426

Merged
ninaiiad merged 4 commits intodevelopfrom
ng/data-gatherer
Mar 27, 2026
Merged

init data-gatherer#426
ninaiiad merged 4 commits intodevelopfrom
ng/data-gatherer

Conversation

@ninaiiad
Copy link
Copy Markdown
Collaborator

@ninaiiad ninaiiad commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

View 6 additional findings in Devin Review.

Open in Devin Review

Comment on lines +127 to +130
if let Some((slot, block_hash, builder_pubkey)) =
Self::extract_block_hash_and_pubkey(bid.header.encoding, payload, is_mergeable)
{
max_slot = max_slot.max(slot);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 SSZ byte extraction from compressed payloads produces garbage data and corrupts slot tracking

The extract_block_hash_and_pubkey function reads raw bytes at fixed SSZ offsets (tile.rs:88-109) without checking the submission's compression (bid.header.compression is available but unused). For compressed SSZ payloads (Zstd via TCP, Gzip/Zstd via HTTP), the function reads compressed bytes and interprets them as slot/block_hash/builder_pubkey, returning Some(...) with garbage values.

This has cascading effects: the garbage slot (random bytes interpreted as u64) is used to update max_slot at line 130, which triggers on_new_slot(garbage_slot) at line 184. This sets self.current_slot to a near-u64::MAX value and prematurely flushes all ClickHouse entries via publish_snapshot. After that, no legitimate slot transition will satisfy max_slot > self.current_slot, permanently breaking the ClickHouse data pipeline for the remainder of the process lifetime.

Contrast with the JSON path which handles this gracefully

The JSON path (tile.rs:63-86) calls serde_json::from_slice on the compressed bytes, which fails and returns None via .ok()?. This just logs an error without corrupting state. The SSZ path lacks equivalent validation.

Prompt for agents
In crates/relay/src/data_gatherer/tile.rs, line 128 calls Self::extract_block_hash_and_pubkey(bid.header.encoding, payload, is_mergeable) without checking compression. The function at lines 57-112 reads raw bytes which may be compressed (Zstd for TCP, Gzip/Zstd for HTTP).

Fix option 1 (skip compressed): Before calling extract_block_hash_and_pubkey at line 128, check if bid.header.compression is not Compression::None, and if so, skip the ClickHouse extraction (still do S3 upload). This is the simplest fix.

Fix option 2 (pass compression): Add Compression parameter to extract_block_hash_and_pubkey, decompress before parsing, or return None for compressed payloads.

The fix should ensure the garbage slot value never propagates to max_slot at line 130.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +100 to +104
let rows = self
.map
.extract_if(|_, v| v.slot < new_slot)
.map(|(hash, info)| BlockInfoRow::from(self.instance_id, hash, info))
.collect::<Vec<BlockInfoRow>>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 publish_snapshot returns Some with empty rows, sending unnecessary empty inserts to ClickHouse

publish_snapshot at clickhouse.rs:96 returns None only if self.map.is_empty(). When the map is non-empty but extract_if at line 102 matches zero entries (all entries have slot >= new_slot), the method still returns Some(future) with an empty rows vec. The spawned future then calls insert_rows which creates a ClickHouse insert, writes zero rows, and calls end() — sending an unnecessary network request. This also logs "inserted 0 rows" which can be misleading.

Suggested change
let rows = self
.map
.extract_if(|_, v| v.slot < new_slot)
.map(|(hash, info)| BlockInfoRow::from(self.instance_id, hash, info))
.collect::<Vec<BlockInfoRow>>();
let rows = self
.map
.extract_if(|_, v| v.slot < new_slot)
.map(|(hash, info)| BlockInfoRow::from(self.instance_id, hash, info))
.collect::<Vec<BlockInfoRow>>();
if rows.is_empty() {
return None;
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@ninaiiad ninaiiad merged commit 3e78329 into develop Mar 27, 2026
3 checks passed
@ninaiiad ninaiiad deleted the ng/data-gatherer branch March 27, 2026 17:59
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