fix: make validated block cache bounded

[hanabi1224]

Summary of changes

Changes introduced in this pull request:

# HELP cache_validated_blocks_size_bytes Size of cache validated_blocks in bytes
# TYPE cache_validated_blocks_size_bytes gauge
# UNIT cache_validated_blocks_size_bytes bytes
cache_validated_blocks_size_bytes 6624
# HELP cache_validated_blocks_len Length of cache validated_blocks
# TYPE cache_validated_blocks_len gauge
cache_validated_blocks_len 69
# HELP cache_validated_blocks_cap Capacity of cache validated_blocks
# TYPE cache_validated_blocks_cap gauge
cache_validated_blocks_cap 14400
# HELP cache_validated_blocks_hits Cache hits of validated_blocks
# TYPE cache_validated_blocks_hits counter
cache_validated_blocks_hits_total 60
# HELP cache_validated_blocks_misses Cache misses of validated_blocks
# TYPE cache_validated_blocks_misses counter
cache_validated_blocks_misses_total 122

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

• I have performed a self-review of my own code,
• I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
• I have added tests that prove my fix is effective or that my feature works (if possible),
• I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

• I have read and agree to the CONTRIBUTING document.
• I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

• Refactor
  • Optimized internal block validation tracking mechanism for improved performance.
  • Updated chain synchronization reset process.

[coderabbitai]

Walkthrough

ChainStore replaces validated-blocks tracking from Mutex-guarded HashSet<Cid> to SizeTrackingCache<CidWrapper, ()>, updating imports, introducing a cache capacity constant, modifying the field type, initializing the cache with metrics, and adapting query and mutation methods. ChainFollower::reset is updated to clear the cache via direct access instead of acquiring a lock.

Changes

Validated Blocks Cache Migration

Layer / File(s)	Summary
ChainStore cache implementation src/chain/store/chain_store.rs	Imports are updated to remove HashSet and Mutex, new VALIDATED_BLOCKS_CACHE_SIZE constant is added, ChainStore::validated_blocks field type changes from Arc<Mutex<HashSet<Cid>>> to SizeTrackingCache<CidWrapper, ()>, initialization in ChainStore::new switches to SizeTrackingCache::new_with_metrics, and is_block_validated, mark_block_as_validated, and unmark_block_as_validated adapt from mutex-protected set operations to cache API calls.
ChainFollower reset adaptation src/chain_sync/chain_follower.rs	ChainFollower::reset removes the explicit .lock() call and clears validated_blocks via direct cache field access on chain_store() instead of acquiring a mutex.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• ChainSafe/forest#7131: Touches validated-blocks tracking logic and internal hash map clearing/size metrics in related files.

Suggested reviewers

• LesnyRumcajs
• sudo-shashank
• akaladarshi

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main change: replacing the unbounded validated blocks HashSet with a bounded SizeTrackingCache.
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 hm/bound-validated-block-cache

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch hm/bound-validated-block-cache

---

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

[coderabbitai]

🧹 Nitpick comments (2)

src/chain/store/chain_store.rs (1)

154-157: ⚡ Quick win

Add a regression test for the bounded behavior.

This constructor is the core behavior change in the PR, but the visible coverage still only exercises insert/lookup. A small-capacity test that overfills the cache and asserts eviction or cap enforcement would make the “bounded” part much harder to regress.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chain/store/chain_store.rs` around lines 154 - 157, Add a regression test
that verifies the bounded eviction behavior of the validated_blocks cache
created via SizeTrackingCache::new_with_metrics (the constructor change in this
PR): instantiate a SizeTrackingCache with a very small capacity (override
VALIDATED_BLOCKS_CACHE_SIZE or construct directly with a small cap), insert more
entries than the capacity, then assert the cache enforces the bound (e.g.,
size() <= capacity) and that older/expected entries have been evicted (lookup
for the earliest-inserted keys returns miss while recent keys hit); place the
test near the chain_store tests and name it something like
validated_blocks_bounded_eviction_test so future regressions are obvious.

src/chain_sync/chain_follower.rs (1)

153-157: ⚡ Quick win

Hide validated-block clearing behind ChainStore.

reset() now depends on the concrete validated_blocks field and SizeTrackingCache API. A small ChainStore::clear_validated_blocks() helper would keep the sync layer insulated from future storage changes.

♻️ Suggested refactor

// src/chain/store/chain_store.rs
+    pub(crate) fn clear_validated_blocks(&self) {
+        self.validated_blocks.clear();
+    }

// src/chain_sync/chain_follower.rs
-        self.state_manager.chain_store().validated_blocks.clear();
+        self.state_manager.chain_store().clear_validated_blocks();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chain_sync/chain_follower.rs` around lines 153 - 157, The reset logic in
ChainFollower is reaching into ChainStore internals by clearing validated_blocks
directly, so add a ChainStore::clear_validated_blocks() helper and call that
from ChainFollower::reset() instead. Keep the sync layer using the ChainStore
API only, and move any SizeTrackingCache-specific clearing behavior behind that
new helper.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/chain_sync/chain_follower.rs`:
- Around line 153-157: The reset logic in ChainFollower is reaching into
ChainStore internals by clearing validated_blocks directly, so add a
ChainStore::clear_validated_blocks() helper and call that from
ChainFollower::reset() instead. Keep the sync layer using the ChainStore API
only, and move any SizeTrackingCache-specific clearing behavior behind that new
helper.

In `@src/chain/store/chain_store.rs`:
- Around line 154-157: Add a regression test that verifies the bounded eviction
behavior of the validated_blocks cache created via
SizeTrackingCache::new_with_metrics (the constructor change in this PR):
instantiate a SizeTrackingCache with a very small capacity (override
VALIDATED_BLOCKS_CACHE_SIZE or construct directly with a small cap), insert more
entries than the capacity, then assert the cache enforces the bound (e.g.,
size() <= capacity) and that older/expected entries have been evicted (lookup
for the earliest-inserted keys returns miss while recent keys hit); place the
test near the chain_store tests and name it something like
validated_blocks_bounded_eviction_test so future regressions are obvious.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7447ddb8-00b1-48b8-bc5b-eb0a3453a861

📥 Commits

Reviewing files that changed from the base of the PR and between 39a6969 and 348da86.

📒 Files selected for processing (2)

• src/chain/store/chain_store.rs
• src/chain_sync/chain_follower.rs

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.34%. Comparing base (39a6969) to head (348da86).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/chain/store/chain_store.rs	85.71%	1 Missing ⚠️
src/chain_sync/chain_follower.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/chain/store/chain_store.rs	69.29% <85.71%> (+0.27%)	⬆️
src/chain_sync/chain_follower.rs	33.53% <0.00%> (+0.16%)	⬆️

... and 13 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39a6969...348da86. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.