src/db/car/any.rs (1)

141-146: LGTM with an optional idiom note.

The implementation correctly delegates to the existing TryFrom<&Path> via as_path(). This provides convenient conversion for callers holding &PathBuf.

Note: Rust API guidelines typically favor accepting &Path over &PathBuf since PathBuf derefs to Path, making &Path more flexible. However, as a trait implementation (rather than a function parameter), this may simplify specific call sites. If this isn't used frequently, consider whether callers could use .as_ref() or .as_path() with the existing TryFrom<&Path> instead.

src/tool/subcommands/state_compute_cmd.rs (3)

81-81: Avoid unwraps on fallible constructors; add context

Replace unwrap() with ? plus anyhow::Context for clearer errors.

+use anyhow::Context as _;

 ...
-            )
-            .unwrap(),
+            )
+            .context("creating ChainStore")?,
 ...
-        let state_manager = Arc::new(StateManager::new(chain_store.clone()).unwrap());
+        let state_manager = Arc::new(
+            StateManager::new(chain_store.clone()).context("creating StateManager")?
+        );
 ...
-            )
-            .unwrap(),
+            )
+            .context("creating ChainStore (replay)")?,
 ...
-        let state_manager = Arc::new(StateManager::new(chain_store.clone()).unwrap());
+        let state_manager = Arc::new(
+            StateManager::new(chain_store.clone()).context("creating StateManager (replay)")?
+        );

Also applies to: 90-90, 145-145

---

100-107: Stream snapshot to file when exporting to avoid double buffering

Currently you always build a full in-memory Vec<u8> then optionally write it to disk. When --export-db-to is set, write directly to the file; fall back to in-memory only when not exporting.

-        let mut db_snapshot = vec![];
-        db.export_forest_car(&mut db_snapshot).await?;
-        println!(
-            "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
-            human_bytes::human_bytes(db_snapshot.len() as f64)
-        );
-        if let Some(export_db_to) = export_db_to {
-            std::fs::write(export_db_to, db_snapshot)?;
-        }
+        if let Some(export_db_to) = export_db_to {
+            let mut file = std::fs::File::create(&export_db_to)?;
+            db.export_forest_car(&mut file).await?;
+            let size = file.metadata()?.len();
+            println!(
+                "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
+                human_bytes::human_bytes(size as u128)
+            );
+        } else {
+            let mut db_snapshot = Vec::new();
+            db.export_forest_car(&mut db_snapshot).await?;
+            println!(
+                "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
+                human_bytes::human_bytes(db_snapshot.len() as u128)
+            );
+        }

---

153-164: Minor: consistent byte/time formatting across commands

You’re using humantime and human_bytes—good. Consider adding the same byte-size line to replay runs (e.g., snapshot size once before the loop) for parity with compute. Optional.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/tool/subcommands/state_compute_cmd.rs (4)

61-66: Add error context when resolving DB path/config.

Wrap fallible ops with anyhow::Context for actionable CLI errors.

Apply within the selected lines:

-            let (_, config) = read_config(None, Some(chain.clone()))?;
-            db_root(&chain_path(&config))?
+            let (_, config) = read_config(None, Some(chain.clone()))
+                .context("reading forest config")?;
+            db_root(&chain_path(&config))
+                .context("resolving database root path")?

Outside the selected lines, add the import near the other uses:

+use anyhow::Context;

---

72-82: Avoid unwraps; propagate with context.

Prefer bubbling errors with context over panicking in a CLI tool.

-        let chain_store = Arc::new(
-            ChainStore::new(
+        let chain_store = Arc::new(
+            ChainStore::new(
                 db.clone(),
                 db.clone(),
                 db.clone(),
                 db.clone(),
                 chain_config,
                 genesis_header,
-            )
-            .unwrap(),
+            )
+            .context("initializing ChainStore")?,
         );
-        let state_manager = Arc::new(StateManager::new(chain_store.clone()).unwrap());
+        let state_manager = Arc::new(
+            StateManager::new(chain_store.clone()).context("initializing StateManager")?,
+        );

-        let chain_store = Arc::new(
-            ChainStore::new(
+        let chain_store = Arc::new(
+            ChainStore::new(
                 db.clone(),
                 db.clone(),
                 db.clone(),
                 db.clone(),
                 chain_config,
                 genesis_header,
-            )
-            .unwrap(),
+            )
+            .context("initializing ChainStore")?,
         );
-        let state_manager = Arc::new(StateManager::new(chain_store.clone()).unwrap());
+        let state_manager = Arc::new(
+            StateManager::new(chain_store.clone()).context("initializing StateManager")?,
+        );

Also applies to: 90-90, 146-146, 148-148

---

89-89: Keep writing HEAD to the tracker; add a brief comment to document intent.

This is correct for exporting minimal snapshots from ReadOpsTrackingStore. Add a note to prevent future “wrong store” regressions.

-        SettingsStoreExt::write_obj(&db.tracker, crate::db::setting_keys::HEAD_KEY, ts.key())?;
+        // Write HEAD to the tracker MemoryDB so export_forest_car() uses the accessed subset.
+        // This intentionally does NOT mutate the underlying on-disk DB.
+        SettingsStoreExt::write_obj(&db.tracker, crate::db::setting_keys::HEAD_KEY, ts.key())?;

Based on learnings

---

149-166: Print iteration index to ease benchmarking output parsing.

Small UX win when running -n > 1.

-        for _ in 0..n.get() {
+        for i in 1..=n.get() {
             let start = Instant::now();
@@
-            println!(
-                "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, took {}.",
-                humantime::format_duration(start.elapsed())
-            );
+            println!(
+                "run {}/{}: epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, took {}.",
+                i,
+                n.get(),
+                humantime::format_duration(start.elapsed())
+            );

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.

Learnt from: hanabi1224
PR: ChainSafe/forest#6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.

src/tool/subcommands/state_compute_cmd.rs (1)

119-120: Default for n is fine

NonZeroUsize::new(1).unwrap() is infallible here; keeping it is acceptable.

src/tool/subcommands/state_compute_cmd.rs (4)

86-86: Writing HEAD to tracker is correct for minimal CAR export; add a clarifying comment and avoid an extra alloc

This HEAD write must target the tracker MemoryDB so export_forest_car roots the snapshot correctly. Add a short comment to prevent regressions, and pass a reference to avoid an extra allocation.

Based on learnings

-        SettingsStoreExt::write_obj(&db.tracker, crate::db::setting_keys::HEAD_KEY, ts.key())?;
+        // Export reads from the tracker MemoryDB; seed HEAD so the CAR roots correctly.
+        SettingsStoreExt::write_obj(&db.tracker, crate::db::setting_keys::HEAD_KEY, &ts.key())?;

---

96-104: Avoid buffering entire snapshot in memory when writing to a file

Exporting to Vec allocates the full CAR in memory even when --export-db-to is provided. Stream directly to the file and derive size from metadata; fall back to Vec only when no path is given.

-        let mut db_snapshot = vec![];
-        db.export_forest_car(&mut db_snapshot).await?;
-        println!(
-            "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
-            human_bytes::human_bytes(db_snapshot.len() as f64)
-        );
-        if let Some(export_db_to) = export_db_to {
-            std::fs::write(export_db_to, db_snapshot)?;
-        }
+        if let Some(path) = export_db_to {
+            use std::io::Write;
+            let file = std::fs::File::create(&path)?;
+            let mut writer = std::io::BufWriter::new(file);
+            db.export_forest_car(&mut writer).await?;
+            writer.flush()?;
+            let size = std::fs::metadata(&path)?.len();
+            println!(
+                "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
+                human_bytes::human_bytes(size as f64)
+            );
+        } else {
+            let mut buf = Vec::new();
+            db.export_forest_car(&mut buf).await?;
+            println!(
+                "epoch: {epoch}, state_root: {state_root}, receipt_root: {receipt_root}, db_snapshot_size: {}",
+                human_bytes::human_bytes(buf.len() as f64)
+            );
+        }

---

45-51: CLI UX: add ValueHint for path args

Helps shell completions and tooling.

-    #[arg(long)]
+    #[arg(long, value_hint = clap::ValueHint::DirPath)]
     db: Option<PathBuf>,
-    #[arg(long)]
+    #[arg(long, value_hint = clap::ValueHint::FilePath)]
     export_db_to: Option<PathBuf>,

---

112-121: Snapshot path: add ValueHint

Small UX win for positional arg.

 pub struct ReplayComputeCommand {
     /// Path to the database snapshot `CAR` file generated by `forest-tool state compute`
-    snapshot: PathBuf,
+    #[arg(value_hint = clap::ValueHint::FilePath)]
+    snapshot: PathBuf,

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.

Learnt from: hanabi1224
PR: ChainSafe/forest#6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.