chore(syncing): avoid sending duplicate events to channel by julienrbrt · Pull Request #3207 · evstack/ev-node

julienrbrt · 2026-03-30T09:28:03Z

Seeing a lot of DBG height already processed in the logs.
This skips sending the events earlier.

Summary by CodeRabbit

Bug Fixes
- Prevented duplicate height events from being enqueued and processed, avoiding redundant work and improving reliability.
- Enhanced debug/error logs for height processing to include the event source, making issue diagnosis and tracing easier.

github-actions · 2026-03-30T09:28:15Z

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`⏩ skipped`	`✅ passed`	`✅ passed`	Mar 30, 2026, 11:50 AM

claude · 2026-03-30T09:28:18Z

Claude finished @julienrbrt's task in 1m 24s —— View job

Code Review

Gather context
Read changed files
Review code quality, correctness, and architecture
Post findings

The change is minimal, correct, and addresses a real issue. Here's my analysis:

Summary

The PR adds a deduplication guard in PipeEvent (syncer.go:519-523) that short-circuits before enqueuing events already tracked in s.cache.IsHeaderSeen. Previously, duplicate DA/P2P events for the same header would always enter the heightInCh channel, and only be filtered in processHeightEvent — causing repeated DBG height already processed log noise and unnecessary channel pressure. The fix moves this check upstream.

Correctness

Early dedup is sound. IsHeaderSeen is marked true after a header is committed. There's a small TOCTOU window where two concurrent calls to PipeEvent for the same header could both pass the new guard and enqueue. This is fine — processHeightEvent already has the same guard at syncer.go:555 as a second line of defense, so no duplicate processing occurs.

SetPendingEvent interaction is correct. Previously, if the channel was full (default branch), a duplicate event would still be stored via SetPendingEvent. With the new early return, duplicates no longer get re-queued as pending — which is the desired behavior since there's nothing useful left to process.

da_follower.go callers handle nil return correctly. Both call sites at da_follower.go:112 and da_follower.go:171 treat a nil error return as success and continue normally, so the early return nil integrates cleanly.

Observability improvement

Adding Str("source", ...) to the debug log at syncer.go:558 and the error log at syncer.go:668 is a nice improvement — it's much easier to distinguish DA vs. P2P events in traces without this fix.

Minor points

Test coverage: Codecov flags 0% patch coverage on the 2 new lines. A unit test calling PipeEvent twice with the same event (verifying the second call returns nil without enqueueing) would lock in this behavior. Not blocking, but worth adding.
nil header concern (from CodeRabbit): Correctly dismissed by @julienrbrt — all three call sites (raft_retriever.go:142, da_follower.go:112, da_follower.go:171) construct DAHeightEvent with non-nil headers internally.

Verdict

LGTM. The fix is clean and correct with no architectural concerns.

coderabbitai · 2026-03-30T09:28:21Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72c65876-6e7d-4116-8f24-4160f834645e

📥 Commits

Reviewing files that changed from the base of the PR and between 1c75ef5 and fa31d6a.

📒 Files selected for processing (1)

block/internal/syncing/syncer.go

🚧 Files skipped from review as they are similar to previous changes (1)

block/internal/syncing/syncer.go

📝 Walkthrough

Walkthrough

Syncer.PipeEvent now returns early when a header hash is already marked seen to avoid enqueueing duplicates; logging in processHeightEvent and its error paths was extended to include the event Source as a structured field.

Changes

Cohort / File(s)	Summary
Deduplication & Logging `block/internal/syncing/syncer.go`	Added an early-return guard in `PipeEvent` using `s.cache.IsHeaderSeen(event.Header.Hash().String())` to skip already-seen headers. Extended structured logs in `processHeightEvent` and related error paths to include `string(event.Source)`.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 I checked each hash before the race,
No twin events crowding the place,
I log where they came from, clear and bright,
One hop, one note, then off into night 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description is minimal and lacks structure. It mentions the problem (repeated logs) and the solution approach, but is missing explicit sections like Overview, Goals, or Context as suggested by the template.	Expand the description to follow the template more closely: add an explicit Overview section, clarify the goal and rationale, and link related issues if applicable.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: avoiding duplicate events in the channel, which directly relates to the core fix of checking cache before enqueueing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch julien/sync

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@block/internal/syncing/syncer.go`:
- Around line 520-523: The code calls event.Header.Hash().String()
unconditionally before checking for nil, which can panic if event.Header is nil;
modify the logic in the syncer (where s.cache.IsHeaderSeen is used) to first
guard event.Header != nil and handle the malformed event (e.g., return an error
or skip processing) before calling event.Header.Hash().String(), ensuring no nil
dereference occurs when evaluating s.cache.IsHeaderSeen.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 938af7d1-7520-475d-bcbb-ebf448bd8b93

📥 Commits

Reviewing files that changed from the base of the PR and between 889da9a and 1c75ef5.

📒 Files selected for processing (1)

block/internal/syncing/syncer.go

block/internal/syncing/syncer.go

codecov · 2026-03-30T09:32:29Z

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.43%. Comparing base (889da9a) to head (fa31d6a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
block/internal/syncing/syncer.go	25.00%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3207      +/-   ##
==========================================
- Coverage   61.46%   61.43%   -0.04%     
==========================================
  Files         120      120              
  Lines       12462    12469       +7     
==========================================
  Hits         7660     7660              
- Misses       3944     3949       +5     
- Partials      858      860       +2

Flag	Coverage Δ
combined	`61.43% <25.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

julienrbrt added 2 commits March 30, 2026 11:14

chore(syncing): avoid sending duplicate events to channel

5bddb0d

add comment

1c75ef5

github-actions bot assigned julienrbrt Mar 30, 2026

coderabbitai bot reviewed Mar 30, 2026

View reviewed changes

block/internal/syncing/syncer.go Outdated Show resolved Hide resolved

julienrbrt requested a review from tac0turtle March 30, 2026 11:17

tac0turtle approved these changes Mar 30, 2026

View reviewed changes

improve comment

fa31d6a

julienrbrt enabled auto-merge March 30, 2026 11:51

julienrbrt added this pull request to the merge queue Mar 30, 2026

Merged via the queue into main with commit a4484f5 Mar 30, 2026
31 checks passed

julienrbrt deleted the julien/sync branch March 30, 2026 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(syncing): avoid sending duplicate events to channel#3207

chore(syncing): avoid sending duplicate events to channel#3207
julienrbrt merged 3 commits intomainfrom
julien/sync

julienrbrt commented Mar 30, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 30, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

codecov bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

julienrbrt commented Mar 30, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

github-actions bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Summary

Correctness

Observability improvement

Minor points

Verdict

Uh oh!

coderabbitai bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

julienrbrt commented Mar 30, 2026 •

edited by coderabbitai bot

Loading

github-actions bot commented Mar 30, 2026 •

edited

Loading

claude bot commented Mar 30, 2026 •

edited

Loading

coderabbitai bot commented Mar 30, 2026 •

edited

Loading

codecov bot commented Mar 30, 2026 •

edited

Loading