chore: missed feedback from #3204

[julienrbrt]

ref: #3204 (comment)

Summary by CodeRabbit

• Refactor

  • Optimized cache locking mechanism for improved read performance.

• Documentation

  • Corrected typo in database path configuration description.

[claude]

Claude finished @julienrbrt's task in 49s —— View job

---

Code Review

• Gather context and read changed files
• Review block/internal/cache/generic_cache.go
• Review pkg/config/config.go
• Post review feedback

---

This PR addresses two small pieces of missed feedback from #3204. Both changes are correct and clean.

block/internal/cache/generic_cache.go — RWMutex upgrade

Overall: LGTM. The upgrade from sync.Mutex to sync.RWMutex is correct and idiomatic. All read-only operations now use RLock/RUnlock:

Method	Operation	Lock type
isSeen	map read	RLock ✓
getDAIncluded	map read	RLock ✓
getDAIncludedByHeight	map read	RLock ✓
daIncludedLen	len()	RLock ✓
persistSnapshot	iterate maps, copy to local slice	RLock ✓
All write methods	map mutations	Lock ✓

One subtle but correct pattern in persistSnapshot (line 153–162): the RLock covers only the map iteration/copy, and the lock is released before the store write. This avoids holding the mutex during potentially slow I/O and is the right approach for this use case.

One observation worth noting: setMaxDAHeight (line 117–126) is called from within setDAIncluded while mu.Lock() is already held (line 103). This is safe because setMaxDAHeight exclusively uses the lock-free atomic.Uint64 and never attempts to re-acquire mu, so there is no deadlock risk.

pkg/config/config.go — typo fix

Removes the duplicate "for" in the FlagDBPath flag description: "path for for node database" → "path for node database". Correct and straightforward.

---

No issues found. Both changes are correct, minimal, and improve the codebase.

[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	Mar 29, 2026, 6:56 AM

[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: 3f1c5f85-2520-46ea-a9d9-2c3192642ab0

📥 Commits

Reviewing files that changed from the base of the PR and between 763e8c6 and 5fbb461.

📒 Files selected for processing (2)

• block/internal/cache/generic_cache.go
• pkg/config/config.go

---

📝 Walkthrough

Walkthrough

The cache implementation's synchronization mechanism is optimized by switching from an exclusive mutex to a read-write mutex, allowing concurrent reads in accessor methods while maintaining exclusive locks for write operations. A configuration flag description typo is also corrected.

Changes

Cohort / File(s)	Summary
Cache synchronization optimization block/internal/cache/generic_cache.go	Replaced sync.Mutex with sync.RWMutex and updated read-only accessors (isSeen, getDAIncluded, getDAIncludedByHeight, daIncludedLen, and the read portion of persistSnapshot) to use RLock/RUnlock. Write paths remain on exclusive Lock/Unlock.
Configuration documentation fix pkg/config/config.go	Corrected typo in flag description from "path for for node database" to "path for node database".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through locks so bright,
Read and write now share the light,
Concurrent whiskers twitch with glee,
As typos flee and mutex's free!
Performance bounds, no more despair, 🔒✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is minimal, providing only a reference link without explaining the changes or their rationale as required by the template.	Add an overview section explaining what feedback was addressed, why these changes matter, and how they improve the code (e.g., performance benefits of RWMutex, typo correction).
Title check	❓ Inconclusive	The title refers to feedback from PR #3204 but doesn't clearly describe the specific changes made (RWMutex conversion and typo fix).	Consider a more descriptive title like 'refactor: use RWMutex for read-heavy cache operations' or 'chore: address review feedback on cache locking and documentation'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

---

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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.45%. Comparing base (763e8c6) to head (5fbb461).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3206      +/-   ##
==========================================
- Coverage   61.46%   61.45%   -0.02%     
==========================================
  Files         120      120              
  Lines       12462    12462              
==========================================
- Hits         7660     7658       -2     
- Misses       3943     3945       +2     
  Partials      859      859              

Flag	Coverage Δ
combined	61.45% <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 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.