Skip to content

fix(syncing): clean up already processed DA data - #3413

Merged
tac0turtle merged 1 commit into
evstack:mainfrom
jgimeno:fix/cleanup-processed-da-data
Jul 31, 2026
Merged

fix(syncing): clean up already processed DA data#3413
tac0turtle merged 1 commit into
evstack:mainfrom
jgimeno:fix/cleanup-processed-da-data

Conversation

@jgimeno

@jgimenojgimeno commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Overview

Clean up DA retriever data when the syncer discards an event for a height that has already been processed.

processBlobs keeps matched data pending until the syncer accepts the candidate block. When the node had already reached that block height, processHeightEvent returned early without removing the retained data. Replaying older DA heights after catching up through another source could therefore make pendingData grow with blocks the node no longer needed.

This change:

  • removes only the pending DA data associated with the discarded height;
  • reuses the same cleanup path after successful DA synchronization;
  • leaves P2P events and data above the current height untouched;
  • adds a regression test covering multiple already-processed heights and a future pending height.

Testing

  • go test ./block/internal/syncing -run '^TestProcessHeightEvent_AlreadyProcessedDADataDoesNotAccumulate$' -count=1
  • go test ./block/... -count=1

Summary by CodeRabbit

  • Bug Fixes
    • Prevented stale data from accumulating when duplicate or already-processed data is received.
    • Improved cleanup of pending data after successful synchronization.
  • Tests
    • Added coverage to verify that processed data is removed promptly while future pending data remains available.

@coderabbitai

coderabbitaiBot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 578a2a34-e11c-49a1-9136-d9a97405a8a9

📥 Commits

Reviewing files that changed from the base of the PR and between 1f8b332 and c5c5ef7.

📒 Files selected for processing (2)
  • block/internal/syncing/syncer.go
  • block/internal/syncing/syncer_test.go

📝 Walkthrough

Walkthrough

Syncer now removes pending DA data when duplicate or already-processed events are encountered, using a shared cleanup helper after successful synchronization. A test verifies processed heights are removed while future-height data remains pending.

Changes

DA Pending Data Cleanup

Layer / File(s)Summary
Process DA events and remove pending entries
block/internal/syncing/syncer.go, block/internal/syncing/syncer_test.go
Pending DA data is cleaned up for already-processed events and after successful synchronization through a shared helper. Tests verify processed heights are removed while future pending data remains.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers:julienrbrt, tac0turtle

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title is concise, semantic-commit styled, and accurately summarizes the main syncing cleanup change.
Description check✅ PassedThe description fills the required Overview section and includes context, rationale, behavior changes, and testing.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov

codecovBot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.34%. Comparing base (1f8b332) to head (c5c5ef7).

Additional details and impacted files
@@ Coverage Diff @@## main #3413 +/- ##
==========================================
+ Coverage 62.32% 62.34% +0.02% 
==========================================
Files 120 120 Lines 13461 13465 +4 ==========================================
+ Hits 8389 8395 +6 + Misses 4130 4128 -2 
Partials 942 942 
FlagCoverage Δ
combined62.34% <100.00%> (+0.02%)⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

@jgimeno
jgimeno marked this pull request as ready for review July 30, 2026 22:17
@tac0turtle
tac0turtle merged commit f2e2f97 into evstack:mainJul 31, 2026
26 of 29 checks passed
Sign up for freeto 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

@jgimeno@tac0turtle