src/chain_sync/mod.rs (1)

6-6: Avoid widening the public API surface; re-export the needed items instead.

Making the whole chain_muxer module public expands the crate’s API unnecessarily. Prefer keeping the module private (or pub(crate)) and re-export only what you need (SyncConfig, DEFAULT_RECENT_STATE_ROOTS). This avoids committing to additional symbols long-term.

Apply:

- pub mod chain_muxer;
+ mod chain_muxer;

And re-export explicitly:

 pub use self::{
     bad_block_cache::BadBlockCache,
     chain_follower::{ChainFollower, load_full_tipset},
-    chain_muxer::SyncConfig,
+    chain_muxer::{SyncConfig, DEFAULT_RECENT_STATE_ROOTS},
     consensus::collect_errs,
     sync_status::{ForkSyncInfo, ForkSyncStage, NodeSyncStatus, SyncStatusReport},
     validation::{TipsetValidationError, TipsetValidator},
 };

src/chain_sync/chain_muxer.rs (1)

6-6: Public constant LGTM; consider documenting non-negativity invariant.

Exposing DEFAULT_RECENT_STATE_ROOTS is fine. Add a brief comment stating consumers should treat this as a non-negative count, and consider a future typed wrapper if we keep passing it across RPC boundaries.

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

5-5: Import via re-export to avoid depending on a public submodule path.

If you keep chain_muxer non-public (as suggested), switch the import to the re-export on chain_sync.

- use crate::chain_sync::chain_muxer::DEFAULT_RECENT_STATE_ROOTS;
+ use crate::chain_sync::DEFAULT_RECENT_STATE_ROOTS;

---

86-93: Tipset lookup timeout may still be too short for deep heights; consider making it configurable or longer.

15 minutes can be tight on cold nodes far from head. Either increase it (e.g., 30–60 minutes) or make it a hidden/advanced CLI flag.

-                            .with_timeout(Duration::from_secs(60 * 15)),
+                            .with_timeout(Duration::from_secs(60 * 30)),

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

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.

Learnt from: hanabi1224
PR: ChainSafe/forest#5969
File: src/tool/subcommands/snapshot_cmd.rs:412-412
Timestamp: 2025-08-25T13:35:24.230Z
Learning: In src/tool/subcommands/snapshot_cmd.rs, the +1 in `last_epoch = ts.epoch() - epochs as i64 + 1` fixes an off-by-1 bug where specifying --check-stateroots=N would validate N+1 epochs instead of N epochs, causing out-of-bounds errors when the snapshot contains only N recent state roots.

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

73-76: Validation correctly implemented.

The depth validation properly handles the spine-only case by allowing 0 while rejecting negative values, which prevents invalid parameters from flowing to the export logic.

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

93-103: Verify the 15-minute timeout is sufficient for deep chain traversals.

The timeout change from infinite wait to 15 minutes is reasonable for most cases, but may be insufficient when requesting epochs very far behind the chain head on large networks like mainnet.

Consider making the timeout configurable or increasing it for production use cases where users might legitimately need to export from very old epochs.
Based on the search results, I can see that Filecoin considers 120 blocks (1 hour) a conservative number of confirmations and messages typically take around 30 seconds (1 blocktime). However, I need more specific information about block/epoch timing to better assess the 15-minute timeout.Perfect! Now I have confirmed that Filecoin has a block time of 30 seconds, and 120 blocks (1 hour) is considered a conservative number of confirmations for high-value transfers.

Given this information:

• Each epoch = 30 seconds
• 15 minutes = 900 seconds = 30 epochs
• For very old epochs (like months or years back), the client would need to traverse potentially hundreds of thousands of epochs

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

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.

Learnt from: hanabi1224
PR: ChainSafe/forest#5969
File: src/tool/subcommands/snapshot_cmd.rs:412-412
Timestamp: 2025-08-25T13:35:24.230Z
Learning: In src/tool/subcommands/snapshot_cmd.rs, the +1 in `last_epoch = ts.epoch() - epochs as i64 + 1` fixes an off-by-1 bug where specifying --check-stateroots=N would validate N+1 epochs instead of N epochs, causing out-of-bounds errors when the snapshot contains only N recent state roots.