Skip to content

fix: skip DA cache restore for P2P-only nodes - #3408

Merged
tac0turtle merged 1 commit into
evstack:mainfrom
jgimeno:fix/p2p-only-da-cache-restore
Jul 27, 2026
Merged

fix: skip DA cache restore for P2P-only nodes#3408
tac0turtle merged 1 commit into
evstack:mainfrom
jgimeno:fix/p2p-only-da-cache-restore

Conversation

@jgimeno

@jgimeno jgimeno commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Overview

P2P-only sync nodes restored persisted DA inclusion snapshots left by a previous DA-enabled run. This happened because NewSyncComponents always constructed a cache manager that restored DA state, even when the node had no DA client or submitter.

This change:

  • adds a cache manager constructor that skips persisted DA inclusion restoration;
  • uses it for sync nodes whose DA client is nil;
  • preserves the existing restore behavior for DA-enabled nodes and the existing ClearCache behavior;
  • adds a disk-backed regression test covering a DA-enabled run followed by a P2P-only restart.

P2P-only nodes no longer load in-flight DA header and data inclusion entries from the previous DA-enabled configuration.

Validation

  • go test ./... -count=1
  • go vet ./block/...

Summary by CodeRabbit

  • Bug Fixes
    • Prevented P2P-only sync nodes from restoring stale data-availability cache state.
    • Ensured standard configurations continue restoring persisted cache information as expected.
    • Added coverage verifying caches start without previously stored inclusion state when data availability is disabled.

@coderabbitai

coderabbitai Bot commented Jul 27, 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: 5268b337-5e68-4c8b-897b-35d1454f3ec9

📥 Commits

Reviewing files that changed from the base of the PR and between ec029e0 and 17978c0.

📒 Files selected for processing (3)
  • block/components.go
  • block/components_test.go
  • block/internal/cache/manager.go

📝 Walkthrough

Walkthrough

NewSyncComponents now skips restoring persisted DA inclusion cache state when started without a DA client. Cache manager construction exposes this choice, and a test verifies P2P-only startup does not reload persisted inclusion markers.

Changes

DA cache restoration

Layer / File(s) Summary
Cache manager restoration control
block/internal/cache/manager.go
A shared constructor enables or skips persisted DA inclusion restoration based on an explicit flag.
Sync component cache selection
block/components.go, block/components_test.go
P2P-only startup uses the non-restoring cache manager, with a test confirming persisted inclusion markers remain absent after database reopen.

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

Possibly related PRs

Suggested reviewers: tac0turtle, julienrbrt

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, specific, and accurately summarizes the main change.
Description check ✅ Passed The description includes an Overview and Validation and explains the rationale and behavior change clearly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 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.

@jgimeno
jgimeno marked this pull request as ready for review July 27, 2026 08:25

@tac0turtle tac0turtle left a comment

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.

thank you

@tac0turtle
tac0turtle enabled auto-merge July 27, 2026 08:28
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.34%. Comparing base (ec029e0) to head (17978c0).

Files with missing lines Patch % Lines
block/internal/cache/manager.go 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3408      +/-   ##
==========================================
+ Coverage   62.33%   62.34%   +0.01%     
==========================================
  Files         120      120              
  Lines       13454    13461       +7     
==========================================
+ Hits         8386     8392       +6     
- Misses       4126     4128       +2     
+ Partials      942      941       -1     
Flag Coverage Δ
combined 62.34% <77.77%> (+0.01%) ⬆️

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.

@tac0turtle
tac0turtle added this pull request to the merge queue Jul 27, 2026
@tac0turtle
tac0turtle removed this pull request from the merge queue due to a manual request Jul 27, 2026
@tac0turtle
tac0turtle merged commit 2a19748 into evstack:main Jul 27, 2026
26 of 29 checks passed
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