fix(cache): reduce tx cache retention

[julienrbrt]

Overview

Extract and tweak some useful improvements from #3290

Summary by CodeRabbit

• Bug Fixes
  • Reduced transaction cache retention period and optimized cleanup frequency to prevent out-of-memory conditions during heavy transaction load, improving system stability.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR reduces transaction cache retention from 24 hours to 30 minutes to prevent out-of-memory conditions during heavy transaction load, and adjusts the cleanup frequency to run at one-tenth the retention interval with a 15-second minimum.

Changes

Cohort / File(s)	Summary
Documentation CHANGELOG.md	Added changelog entry documenting the transaction cache retention reduction.
Cache Configuration block/internal/cache/manager.go	Reduced DefaultTxCacheRetention constant from 24 hours to 30 minutes to limit memory usage.
Cleanup Scheduling block/internal/reaping/reaper.go	Changed cleanup ticker from fixed 1-hour interval to dynamic interval calculated as DefaultTxCacheRetention/10 with a 15-second minimum bound.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• tac0turtle

Poem

🐰 Cache shrinks from day to night,
Thirty minutes feels just right!
Memory breathes, no overflow,
Cleanup dances, swift and low. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is incomplete. It references an external PR but lacks specific details about changes made, rationale for reduction, and impact on system behavior.	Expand the description to explain why cache retention is being reduced (OOM prevention), what the new retention period is, and how the cleanup interval was adjusted.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(cache): reduce tx cache retention' directly and clearly describes the main change: reducing transaction cache retention time to prevent OOM conditions.
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.

_{✏️ 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/cache-mem

---

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 and usage tips.}

[claude]

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

---

Code Review

• Gather context and understand the PR
• Read changed files
• Analyze code quality, architecture, and correctness
• Post review feedback

---

This is a small, focused fix to reduce memory pressure from the transaction seen-hash cache. The changes are clean. A few observations:

---

block/internal/cache/manager.go

DefaultTxCacheRetention: 24h → 30 min

The reduction is drastic (48×). The fix is correct for the OOM problem, but the tradeoff should be called out explicitly: the deduplication window is now 30 minutes. If a node restarts or is offline for >30 minutes and has already submitted transactions to the sequencer that haven't been included in a block yet, those txs can be re-submitted, potentially producing duplicates in the sequencer queue. This is likely an acceptable tradeoff (duplicates are better than OOMs), but it's worth documenting whether the sequencer handles idempotency, or whether this window should be configurable.

---

block/internal/reaping/reaper.go

CleanupInterval = max(cache.DefaultTxCacheRetention/10, 15*time.Second)

This is a nice improvement over the previous hardcoded 1h — it creates a proportional invariant (cleanup runs ~10× per retention window). With 30 min retention, this evaluates to 3 min, which is much more responsive than the old 1 hour.

The 15*time.Second floor is a sensible safeguard. One minor note: this constant is evaluated at compile time using a const-level expression involving max over time.Duration values — this is valid Go 1.21+ and works correctly.

Cleanup call in drainMempool

// block/internal/reaping/reaper.go:161
removed := r.cache.CleanupOldTxs(cache.DefaultTxCacheRetention)

This explicitly passes cache.DefaultTxCacheRetention to CleanupOldTxs, even though CleanupOldTxs already defaults to DefaultTxCacheRetention when given ≤ 0. This is fine, but using 0 (or a named constant like cache.DefaultTxCacheRetention) vs explicit passing is a style choice — being explicit is arguably clearer here. Fix this →

---

Testing

No tests were added or updated. Given the change reduces an important operational parameter by 48×, it would be worth having at least one test asserting that entries older than DefaultTxCacheRetention are evicted by CleanupOldTxs, and entries newer are not. This guards against regressions if the constant is changed again. Fix this →

---

Summary

The changes are correct, minimal, and well-reasoned. The CleanupInterval formula is a nice touch. The main design question is whether 30 minutes is the right floor for all deployments or whether this should be made configurable. For now, the constant approach is pragmatic and solves the immediate issue.

[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	✅ passed	Apr 28, 2026, 6:12 PM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.50%. Comparing base (389e904) to head (0311231).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3299      +/-   ##
==========================================
+ Coverage   62.48%   62.50%   +0.02%     
==========================================
  Files         122      122              
  Lines       13047    13047              
==========================================
+ Hits         8152     8155       +3     
+ Misses       4009     4008       -1     
+ Partials      886      884       -2     

Flag	Coverage Δ
combined	62.50% <ø> (+0.02%)	⬆️

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.

[coderabbitai]

🧹 Nitpick comments (1)

block/internal/cache/manager.go (1)

29-30: Consider making tx retention configurable instead of fixed.
At Line 30, a single hardcoded retention may be too aggressive or too lenient across different networks/workloads. Keeping 30m as default but exposing config would give operators safer tuning control.

🤖 Prompt for AI Agents

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

In `@block/internal/cache/manager.go` around lines 29 - 30, Replace the hardcoded
DefaultTxCacheRetention with a configurable retention by keeping
DefaultTxCacheRetention as the default value but adding a configurable
field/parameter (e.g., TxCacheRetention) to the cache manager config or
constructor; update the cache manager initialization (where New/Init for the
manager is called) to accept an optional retention value (read from a config
struct or env flag) and fall back to DefaultTxCacheRetention when unset, and
ensure any uses of DefaultTxCacheRetention (symbol DefaultTxCacheRetention) are
switched to reference the configured retention field so operators can tune
retention per deployment.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@block/internal/cache/manager.go`:
- Around line 29-30: Replace the hardcoded DefaultTxCacheRetention with a
configurable retention by keeping DefaultTxCacheRetention as the default value
but adding a configurable field/parameter (e.g., TxCacheRetention) to the cache
manager config or constructor; update the cache manager initialization (where
New/Init for the manager is called) to accept an optional retention value (read
from a config struct or env flag) and fall back to DefaultTxCacheRetention when
unset, and ensure any uses of DefaultTxCacheRetention (symbol
DefaultTxCacheRetention) are switched to reference the configured retention
field so operators can tune retention per deployment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5c5a916-8385-4d53-aecb-fa45a37152db

📥 Commits

Reviewing files that changed from the base of the PR and between 389e904 and 0311231.

📒 Files selected for processing (3)

• CHANGELOG.md
• block/internal/cache/manager.go
• block/internal/reaping/reaper.go