fix: remove write_cache from ForestCar and PlainCar #5860

hanabi1224 · 2025-07-23T11:54:03Z

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

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.

Summary by CodeRabbit

Refactor
- Removed all write and caching support from ForestCar and PlainCar blockstores, making them strictly read-only.
- Updated error messages to clearly indicate that writes are unsupported and suggest using ManyCar for write operations.
- Adjusted snapshot validation and chain store tests to use a layered blockstore combining in-memory and read-only CAR file storage.
- Removed PersistentStore trait implementation from AnyCar to streamline blockstore behavior.
Chores
- Cleaned up unused imports and suppressed warnings about unused code in specific modules.

coderabbitai · 2025-07-23T11:54:09Z

Walkthrough

The changes remove all support for write caching and persistent storage from the ForestCar and PlainCar CAR blockstore implementations, making them strictly read-only. The PersistentStore trait and related implementations are deleted. The snapshot validation tool and test setups now wrap AnyCar in a ManyCar with an in-memory backend, reflecting the new read-only design. Additionally, #[allow(dead_code)] attributes were added to suppress unused code warnings in hash_map.rs.

Changes

File(s)	Change Summary
src/cid_collections/hash_map.rs	Added `#[allow(dead_code)]` attributes to suppress unused code warnings in `CidHashMap` and related types/methods.
src/db/car/any.rs	Removed `PersistentStore` import and its implementation for `AnyCar<ReaderT>`.
src/db/car/forest.rs	Removed `write_cache` field, all write logic, and `PersistentStore` trait. `put_keyed` now panics as unsupported (read-only).
src/db/car/plain.rs	Removed `write_cache` field, all write logic, and cache helpers. `put_keyed` now panics as unsupported (read-only).
src/tool/subcommands/snapshot_cmd.rs	Changed snapshot validation to use `ManyCar<MemoryDB>` with `AnyCar` as a read-only layer; updated imports accordingly.
src/rpc/methods/chain.rs (test module)	Updated tests to use `ManyCar` wrapping `AnyCar` instead of `PlainCar` for `ChainStore` initialization.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SnapshotCmd
    participant MemoryDB
    participant AnyCar
    participant ManyCar
    participant CARFile

    User->>SnapshotCmd: Run Validate subcommand
    SnapshotCmd->>MemoryDB: Create in-memory DB
    SnapshotCmd->>AnyCar: Open CAR file as read-only
    SnapshotCmd->>ManyCar: Create ManyCar with MemoryDB
    SnapshotCmd->>ManyCar: Add AnyCar as read-only layer
    SnapshotCmd->>ManyCar: Perform validation (reads only)
    ManyCar->>AnyCar: Read block
    AnyCar->>CARFile: Read block data

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~45 minutes

Suggested reviewers

elmattic

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c14b494 and 86d76aa.

📒 Files selected for processing (1)

src/cid_collections/hash_map.rs (8 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/cid_collections/hash_map.rs

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch hm/no-write-cache-in-forest-car

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/rpc/methods/chain.rs (1)

1073-1078: Replace the double unwrap() with descriptive expect() calls

Using plain unwrap() in tests is acceptable, but expect() gives clearer failure context without extra code.

-                ManyCar::new(MemoryDB::default())
-                    .with_read_only(AnyCar::new(genesis_car).unwrap())
-                    .unwrap(),
+                ManyCar::new(MemoryDB::default())
+                    .with_read_only(
+                        AnyCar::new(genesis_car)
+                            .expect("failed to open embedded genesis CAR"),
+                    )
+                    .expect("failed to create layered ManyCar"),

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7028201 and 05ed155.

📒 Files selected for processing (3)

src/db/car/forest.rs (2 hunks)
src/db/car/plain.rs (3 hunks)
src/rpc/methods/chain.rs (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

src/db/car/forest.rs
src/db/car/plain.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: tests-release
GitHub Check: tests
GitHub Check: Build forest binaries on Linux AMD64
GitHub Check: Build Ubuntu
GitHub Check: cargo-publish-dry-run
GitHub Check: Build MacOS
GitHub Check: All lint checks

🔇 Additional comments (1)

src/rpc/methods/chain.rs (1)

958-961: Imports look correct

New MemoryDB, AnyCar, and ManyCar re-exports are required by the test code and are used below. No further action needed.

LesnyRumcajs · 2025-07-23T14:11:46Z

src/cid_collections/hash_map.rs

+#![allow(dead_code)]
+


Suggested change

#![allow(dead_code)]

why is it needed?

All hash_map::Entry usages have been removed.

I'd mark individual items, not the entire file, then.

LesnyRumcajs · 2025-07-23T14:12:51Z

src/db/car/forest.rs

+    /// Not supported, use [`super::ManyCar`] instead.
+    fn put_keyed(&self, _: &Cid, _: &[u8]) -> anyhow::Result<()> {
+        anyhow::bail!("ForestCar is read-only, use ManyCar instead");


I think we should panic at this point, as it's an implementation issue if the code goes here.

Same in some other places. Probably we could use unimplemented! macro.

Or even better, unreachable!

…forest-car

fix: remove write_cache from ForestCar and PlainCar

7028201

hanabi1224 marked this pull request as ready for review July 23, 2025 12:25

hanabi1224 requested a review from a team as a code owner July 23, 2025 12:25

hanabi1224 requested review from akaladarshi and sudo-shashank and removed request for a team July 23, 2025 12:25

fix unit tests

05ed155

hanabi1224 requested a review from LesnyRumcajs July 23, 2025 13:59

coderabbitai bot reviewed Jul 23, 2025

View reviewed changes

Merge branch 'main' into hm/no-write-cache-in-forest-car

50ef0ad

LesnyRumcajs reviewed Jul 23, 2025

View reviewed changes

hanabi1224 added 3 commits July 23, 2025 22:43

panic in put_keyed

c14b494

Merge remote-tracking branch 'origin/main' into hm/no-write-cache-in-…

8681872

…forest-car

allow dead_code for functions

86d76aa

LesnyRumcajs approved these changes Jul 24, 2025

View reviewed changes

sudo-shashank approved these changes Jul 24, 2025

View reviewed changes

hanabi1224 added this pull request to the merge queue Jul 24, 2025

Merged via the queue into main with commit e7b3fed Jul 24, 2025
36 checks passed

hanabi1224 deleted the hm/no-write-cache-in-forest-car branch July 24, 2025 13:58

coderabbitai bot mentioned this pull request Aug 12, 2025

fix: use hashlink::LruCache in SizeTrackingLruCache #5922

Merged

4 tasks

This was referenced Oct 10, 2025

Extend cargo-spellcheck to cover entire repository including .github directory #6062

Open

feat: forest-tool db import #6161

Merged

coderabbitai bot mentioned this pull request Oct 22, 2025

bench: tipset validation #6181

Merged

4 tasks

fix: remove write_cache from ForestCar and PlainCar #5860

fix: remove write_cache from ForestCar and PlainCar #5860

Uh oh!

Conversation

hanabi1224 commented Jul 23, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reference issue to close (if applicable)

Other information and links

Change checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hanabi1224 commented Jul 23, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 23, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)