fix(cache,da): drop finalized snapshot entries on restore and drain DA subscription channel

[tac0turtle]

Overview

Two memory fixes for long-running nodes, found while investigating a user report of sustained memory growth (+201 MiB/min) and OOM kills on syncing nodes.

1. Cache restore leaks stale placeholders across crash/restart cycles

The DA-inclusion snapshot is only persisted on graceful shutdown (Components.Stop). After a crash (e.g. OOM kill, SIGKILL), the node restores a snapshot from the last graceful shutdown, which can contain heights already at or below the persisted DAIncludedHeight watermark. Since:

• the inclusion loop only calls DeleteHeight for heights it advances through, and
• IsHeightDAIncluded short-circuits true below the watermark so those entries are never consulted,

restored placeholders below the watermark were never evicted, and were re-persisted on the next graceful save — so the snapshot grew monotonically across crash/restart cycles.

Cache.RestoreFromStore now takes the finalized height and skips entries at or below it. Skipped entries still contribute to maxDAHeight, so DaHeight() semantics are unchanged. manager.RestoreFromStore reads DAIncludedHeightKey and passes it through; a read error falls back to unfiltered restore instead of failing startup.

The pre-existing comment in TestManager_SaveAndRestoreFromStore already described this as the intended behavior ("The cache entry is not restored — this is correct and intentional") — the code just didn't do it. It now does, and the test asserts it.

2. DA subscription goroutine leak on reconnect

The client.Subscribe wrapper goroutine exited on ctx cancellation without draining the underlying jsonrpc channel. The go-jsonrpc delivery goroutine blocked on send into that channel never observed the cancellation and never closed its channel — leaking one goroutine per watchdog reconnect (observed in the field as Subscribe.func1 goroutines doubling). The wrapper now drains the raw channel on exit, matching what its own doc comment previously required of callers.

Testing

• New TestCache_RestoreFromStore_SkipsFinalizedEntries covering partial and fully-stale snapshots
• TestManager_SaveAndRestoreFromStore now asserts finalized heights are not restored
• go test ./block/..., go vet, golangci-lint all clean

Note: this does not address the primary OOM driver from the same investigation (unbounded pendingEvents buffering when DA catchup outpaces execution) — that needs a separate backpressure change.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Prevented finalized cache entries from being restored as active placeholders after restart.
  • Preserved the correct maximum data-availability height while removing obsolete entries.
  • Improved cache restoration when persisted metadata is unavailable.
  • Prevented subscription shutdown and reconnection scenarios from leaving message delivery blocked.

• Reliability

  • Improved consistency of restored header and data availability state across restarts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee127894-6385-4068-92d9-4d2930c23502

📥 Commits

Reviewing files that changed from the base of the PR and between 19d3a86 and 896871f.

📒 Files selected for processing (5)

• block/internal/cache/generic_cache.go
• block/internal/cache/generic_cache_test.go
• block/internal/cache/manager.go
• block/internal/cache/manager_test.go
• block/internal/da/client.go

---

📝 Walkthrough

Walkthrough

Cache restoration now accepts a finalized-height watermark, skips finalized placeholders while retaining DA height, and passes persisted metadata through manager restoration. DA subscription shutdown now drains the underlying JSON-RPC channel.

Changes

Watermarked cache restoration

Layer / File(s)	Summary
Finalized-aware snapshot restoration block/internal/cache/generic_cache.go, block/internal/cache/generic_cache_test.go	RestoreFromStore preserves the maximum decoded DA height while skipping snapshot entries at or below the finalized height; tests cover stale and partially finalized snapshots.
Manager integration and regression coverage block/internal/cache/manager.go, block/internal/cache/manager_test.go, block/internal/cache/generic_cache_test.go	Manager restoration reads the persisted DA-included height and passes it to both caches; restore tests use the updated signature and verify finalized entries are absent.

DA subscription shutdown

Layer / File(s)	Summary
Subscription channel draining block/internal/da/client.go	Subscribe drains the raw JSON-RPC channel in a deferred shutdown loop and updates its lifecycle documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CacheManager
  participant Store
  participant Cache
  CacheManager->>Store: read DAIncludedHeightKey
  CacheManager->>Cache: restore with finalized height
  Cache->>Cache: update DA height from snapshot
  Cache->>Cache: install only non-finalized placeholders

Loading

sequenceDiagram
  participant Caller
  participant Subscribe
  participant JSONRPC
  Caller->>Subscribe: cancel context
  Subscribe->>JSONRPC: drain raw channel
  JSONRPC-->>Subscribe: close channel

Loading

Possibly related PRs

• evstack/ev-node#3204: Also changes cache snapshot restoration and storage behavior.

Suggested reviewers: julienrbrt

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes both main fixes: skipping finalized cache entries on restore and draining the DA subscription channel.
Description check	✅ Passed	The description matches the template with a clear Overview, rationale, and testing section, and it explains both fixes in enough detail.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch marko/cache-restore-filter-sub-drain

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

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

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	⏩ skipped	Jul 16, 2026, 10:29 AM

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.32%. Comparing base (19d3a86) to head (896871f).

Files with missing lines	Patch %	Lines
block/internal/cache/manager.go	16.66%	2 Missing and 3 partials ⚠️
block/internal/da/client.go	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3385      +/-   ##
==========================================
- Coverage   62.33%   62.32%   -0.01%     
==========================================
  Files         120      120              
  Lines       13444    13452       +8     
==========================================
+ Hits         8380     8384       +4     
- Misses       4124     4126       +2     
- Partials      940      942       +2     

Flag	Coverage Δ
combined	62.32% <50.00%> (-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.