src/cli_shared/logger/mod.rs (1)

166-170: Good call reducing noisy crates to warn in tool mode

These directives will materially quieten forest-tool without hiding errors. One minor maintainability thought: the same four directives now live in both default_env_filter() and default_tool_filter(). Consider extracting them into a shared const COMMON_WARN_DIRECTIVES: &[&str] and building each list by chaining, to avoid future drift between the two.

src/state_manager/mod.rs (1)

1730-1731: Preserve error chains while improving the top-level message

Switching to map_err(|e| anyhow!("...: {e}")) makes the display nicer but discards the original error as the source, weakening backtraces/causes. Prefer attaching context while keeping the chain, and print the chain at the call site.

Suggested change here:

-            )
-            .map_err(|e| anyhow::anyhow!("couldn't compute tipset state: {e}"))?;
+            )
+            .context("couldn't compute tipset state")?;

And in the CLI (see comment in snapshot_cmd.rs), print {e:#} (pretty Display) to surface the source error text and context chain.

src/tool/subcommands/snapshot_cmd.rs (2)

434-450: Surface full error context to users

Now that validate_tipsets attaches context, you can print the full cause chain in the spinner message so users see the underlying error too. This keeps rich diagnostics while preserving the progress UI.

-        Err(e) => {
-            pb.abandon_with_message(format!("❌ Transaction result differs from Lotus: {e}"));
-            anyhow::bail!(e);
-        }
+        Err(e) => {
+            // `{e:#}` prints the context chain; typically single-line unless sources include newlines.
+            pb.abandon_with_message(format!("❌ Transaction result differs from Lotus: {e:#}"));
+            anyhow::bail!(e);
+        }

If multi-line messages prove noisy in practice, join the chain manually: let msg = e.chain().map(ToString::to_string).collect::<Vec<_>>().join(": "); and print msg.

---

408-429: Clarify semantics of --check-stateroots help text

The off-by-one change (ts.epoch() – epochs as i64 + 1) is correct and ensures exactly epochs tipsets are validated (yielding epochs – 1 transitions). However, the CLI doc comment for --check-stateroots still reads “Number of recent epochs to scan for bad messages/transactions,” which is misleading:

• It actually controls state‐root validation (not message/transaction scans).
• The number of transitions checked is one less than the provided value.

Please update both occurrences in src/tool/subcommands/snapshot_cmd.rs (around lines 57–59 and 74–76) to clarify:

-    /// Number of recent epochs to scan for bad messages/transactions
+    /// Number of recent state roots to validate (transitions checked = this value – 1)
     #[arg(long, default_value_t = 60)]
     check_stateroots: u32,

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

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

406-407: Spinner lifecycle is now explicit — nice

Moving away from .with_finish(...) here is cleaner since you’re explicitly finishing/abandoning in the success/error paths below. No action needed.

src/ipld/util.rs (4)

104-107: Make IterateType self-describing (derive Debug) and centralize formatting via Display.

Deriving Debug helps troubleshooting. Implementing Display avoids ad-hoc string construction at call sites.

Apply:

-enum IterateType {
+#[derive(Debug)]
+enum IterateType {
     Message(Cid),
     StateRoot(Cid),
 }
+
+impl std::fmt::Display for IterateType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            IterateType::Message(c) => write!(f, "message {c}"),
+            IterateType::StateRoot(c) => write!(f, "state root {c}"),
+        }
+    }
+}

---

208-239: Avoid variable shadowing and reuse IterateType’s Display for formatting.

Using a local string named ty shadows the enum binding and adds noise. Prefer a distinct binding and delegate formatting to Display.

Apply:

-                    Iterate(epoch, block_cid, ty, cid_vec) => {
+                    Iterate(epoch, block_cid, src, cid_vec) => {
...
-                                } else if fail_on_dead_links {
-                                    let ty = match ty {
-                                        IterateType::Message(c) => {
-                                            format!("message {c}")
-                                        }
-                                        IterateType::StateRoot(c) => {
-                                            format!("state root {c}")
-                                        }
-                                    };
-                                    return Poll::Ready(Some(Err(anyhow::anyhow!(
-                                        "[Iterate] missing key: {cid} from {ty} in block {block_cid} at epoch {epoch}"
-                                    ))));
+                                } else if fail_on_dead_links {
+                                    return Poll::Ready(Some(Err(anyhow::anyhow!(
+                                        "[Iterate] missing key: {cid} from {src} in block {block_cid} at epoch {epoch}"
+                                    ))));
                                 }

---

451-452: Align unordered worker error text with the new taxonomy.

Use the “[Iterate]” prefix here for consistency. Consider propagating block/epoch/source in the unordered path in a follow-up (larger refactor).

-                                        .send_async(Err(anyhow::anyhow!("missing key: {cid}")))
+                                        .send_async(Err(anyhow::anyhow!("[Iterate] missing key: {cid}")))

---

520-522: Match unordered Emit error with ordered path.

Prefix the unordered-queue miss with “[Emit]” for parity.

-                    return Poll::Ready(Some(Err(anyhow::anyhow!("missing key: {cid}"))));
+                    return Poll::Ready(Some(Err(anyhow::anyhow!("[Emit] missing key: {cid}"))));

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/ipld/util.rs (5)

113-114: Good call: enriching Iterate with epoch, block CID, and source improves diagnostics.

The additional context should materially reduce triage time when a dead link is hit.

---

203-204: Clearer error taxonomy for Emit path.

Prefixing with “[Emit]” makes it obvious which stage failed.

---

266-273: Capturing message-traversal context is spot on.

Carrying epoch, block CID, and the specific message root will make downstream errors actionable.

---

282-288: State-root traversal context looks good.

Consistent with the message path and respects the stateroot_limit logic and the epoch==0 special case.

---

200-239: No lingering error message references found

Search across the codebase for the old “[Emit] missing key:” and “[Iterate] missing key:” patterns returned only the two occurrences in src/ipld/util.rs (lines 203 and 237), which you’ve already updated. There are no matches in tests, documentation, or other modules—everything is safe to merge.

src/ipld/util.rs (5)

104-108: Derive common traits on IterateType for easier debugging and ergonomics.

Add Copy, Clone, and Debug—Cid is Copy in this codebase, so deriving these is cheap and improves logs/tests.

+#[derive(Copy, Clone, Debug)]
 enum IterateType {
     Message(Cid),
     StateRoot(Cid),
 }

---

208-239: Great contextualization of dead-link errors in Iterate path.

Including epoch, block CID, and whether the origin was messages vs state root will materially speed up operator troubleshooting.

Two very minor nits you may consider:

• Since the variable is used, naming it origin (or kind) instead of _type avoids the underscore convention for “intentionally unused” bindings.
• Consider normalizing the wording to “state-root” (hyphenated) for consistency with other docs, e.g., “from state-root {c}”.

---

448-453: Unify error taxonomy in the unordered stream with the ordered stream.

Ordered stream uses “[Emit] …” and “[Iterate] …” prefixes; unordered still reports a bare “missing key: …”. Prefixing unordered emit-path errors with “[Emit]” aligns tooling and tests that parse logs.

Apply both diffs:

--- a/src/ipld/util.rs
+++ b/src/ipld/util.rs
@@
-                                    let _ = block_sender
-                                        .send_async(Err(anyhow::anyhow!("missing key: {cid}")))
+                                    let _ = block_sender
+                                        .send_async(Err(anyhow::anyhow!("[Emit] missing key: {cid}")))
                                         .await;

--- a/src/ipld/util.rs
+++ b/src/ipld/util.rs
@@
-                } else if fail_on_dead_links {
-                    return Poll::Ready(Some(Err(anyhow::anyhow!("missing key: {cid}"))));
+                } else if fail_on_dead_links {
+                    return Poll::Ready(Some(Err(anyhow::anyhow!("[Emit] missing key: {cid}"))));
                 }

Also applies to: 520-522

---

481-509: Minor: surface send() errors with more context.

The error includes the inner value; if you decide to keep enriching diagnostics, consider adding a short prefix like “extract queue:” to disambiguate from ordered-stream errors in downstream logs.

---

180-201: Note for future: consider adding a Display impl for IterateType.

Would let you avoid building a temporary String for type_display and keep formatting terse: “[Iterate] missing key: {cid} from {origin} in block …”.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.