CHANGELOG.md (1)

32-32: Use consistent “Tipset” casing across entries.

Elsewhere you use “Tipset” (e.g., ChainGetFinalizedTipset). Recommend: Filecoin.ChainGetTipset for consistency and grepability.

src/tool/subcommands/api_cmd/generate_test_snapshot.rs (1)

47-49: Also accept method aliases to avoid false “RPC method not found”.

Snapshots/tests might record an alias. Consider matching NAME or NAME_ALIAS before the API_PATHS check.

Apply this minimal change:

-            if test_dump.request.method_name.as_ref() == <$ty>::NAME
-                && <$ty>::API_PATHS.contains(test_dump.path)
+            let name = test_dump.request.method_name.as_ref();
+            if (name == <$ty>::NAME
+                || <$ty>::NAME_ALIAS.is_some_and(|a| a == name))
+                && <$ty>::API_PATHS.contains(test_dump.path)
             {

src/rpc/reflect/mod.rs (1)

166-172: Clarify path semantics and make formatting reusable.

The helper returns a path segment (no leading slash). Document this to avoid misuse and consider a Display impl for reuse.

You can add:

-    pub fn path(&self) -> &'static str {
+    /// Returns the relative path segment (without leading slash), e.g. `rpc/v1`.
+    pub fn path(&self) -> &'static str {
         match self {
             Self::V0 => "rpc/v0",
             Self::V1 => "rpc/v1",
             Self::V2 => "rpc/v2",
         }
     }

Optionally:

impl core::fmt::Display for ApiPaths {
    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
        f.write_str(self.path())
    }
}

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

6-17: Add a non-empty round‑trip case as well.

Great baseline. Add one with key/height/tag populated to catch serde attribute regressions.

src/rpc/request.rs (1)

46-52: Make ApiPaths precedence explicit via refactor for code clarity.

The current implementation using iter().max() works correctly because ApiPaths has explicit discriminants (V0=1, V1=2, V2=4), making the Ord comparison deterministic. However, relying on implicit enum Ord behavior is a maintainability risk. The suggested explicit precedence refactor guards against accidental reordering and clarifies intent:

-    pub fn max_api_path(api_paths: BitFlags<ApiPaths>) -> anyhow::Result<ApiPaths> {
-        api_paths.iter().max().context("No supported versions")
-    }
+    pub fn max_api_path(api_paths: BitFlags<ApiPaths>) -> anyhow::Result<ApiPaths> {
+        // Highest supported first to make precedence explicit.
+        if api_paths.contains(ApiPaths::V2) {
+            Ok(ApiPaths::V2)
+        } else if api_paths.contains(ApiPaths::V1) {
+            Ok(ApiPaths::V1)
+        } else if api_paths.contains(ApiPaths::V0) {
+            Ok(ApiPaths::V0)
+        } else {
+            anyhow::bail!("No supported versions")
+        }
+    }

Server capability negotiation is already in place: api_path() operates on self.api_paths (server-supported versions), and the result is passed to get_or_init_client() for correct URL routing.

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

476-476: Fix grammar in documentation.

The documentation reads "The nodes to test against is offline" but should be "The node to test against is offline" (singular) or "The nodes to test against are offline" (plural).

Apply this diff to fix the grammar:

-    /// The nodes to test against is offline, the chain is out of sync.
+    /// The node to test against is offline, the chain is out of sync.

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

1144-1144: Unreachable error case due to validation.

Line 1144 returns an error for "no tipset found for selector," but this should be unreachable because selector.validate() (line 1123) ensures exactly one of key, height, or tag is specified. The three preceding conditionals (lines 1125, 1130, 1140) should cover all valid cases.

Consider replacing with:

unreachable!("selector validation ensures one criterion is specified")

This makes the invariant explicit and will help catch bugs if the validation logic changes.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 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
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.

Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.

Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.

Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.

Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5944
File: src/chain/store/index.rs:0-0
Timestamp: 2025-08-18T03:09:47.932Z
Learning: In Forest's tipset_by_height caching implementation, hanabi1224 prefers performance-conscious solutions that leverage finality guarantees rather than expensive chain walking for fork detection. The approach of constraining cache lookups to finalized epochs (using CHECKPOINT_INTERVAL >= CHAIN_FINALITY) provides fork safety without the performance cost of ancestry verification.

src/tool/subcommands/api_cmd/api_compare_tests.rs (1)

462-474: Consider more explicit offline validation strategy.

The function always returns true in offline mode, which effectively skips validation. While this makes sense since F3 finality isn't available offline, consider adding a comment explaining why offline tests for tagged tipsets (Safe/Finalized) can't be properly validated, or consider testing that the offline server returns an appropriate error instead of skipping validation entirely.

Apply this diff to add clarifying documentation:

 fn validate_tagged_tipset_v2(req: rpc::Request<Option<Tipset>>, offline: bool) -> RpcTest {
     RpcTest::validate(req, move |forest, lotus| match (forest, lotus) {
         (None, None) => true,
         (Some(forest), Some(lotus)) => {
             if offline {
+                // Offline mode: F3 is unavailable, so we can't verify exact finality epochs.
+                // We accept any non-None result as valid for offline testing.
                 true
             } else {
                 (forest.epoch() - lotus.epoch()).abs() <= 2
             }
         }
         _ => false,
     })
 }

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

1116-1142: Consider making unreachable error more explicit.

The error at line 1141 should be unreachable if selector.validate() properly ensures at least one selection criterion is present. Consider using anyhow::bail! with a message indicating this is an internal invariant violation, or add a debug_assert to document this expectation.

Apply this diff:

         if let Some(tag) = &selector.tag {
             let ts = Self::get_tipset_by_tag(&ctx, *tag).await?;
             return Ok(ts);
         }
-        Err(anyhow::anyhow!("no tipset found for selector").into())
+        // Unreachable: validate() should ensure at least one criterion is set
+        debug_assert!(false, "selector.validate() should prevent reaching this point");
+        Err(anyhow::anyhow!("internal error: no valid selector criteria after validation").into())
     }
 }

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

1-100: Good baseline coverage, consider expanding edge cases.

The tests cover the fundamental validation rules well (single vs. multiple criteria). Consider adding tests for:

• Different TipsetTag variants (Safe, Latest, not just Finalized)
• TipsetHeight with anchor set to various values
• Boundary conditions for the at field in TipsetHeight

These would provide more comprehensive validation coverage.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 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
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.

Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6079
File: src/chain_sync/metrics.rs:6-7
Timestamp: 2025-09-16T12:55:26.955Z
Learning: HEAD_EPOCH references in shell scripts (like scripts/tests/calibnet_eth_mapping_check.sh) that extract data from `forest-cli info show` are unrelated to Rust metrics constants with the same name and should not be flagged when metrics cleanup is performed.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.

Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5944
File: src/chain/store/index.rs:0-0
Timestamp: 2025-08-18T03:09:47.932Z
Learning: In Forest's tipset_by_height caching implementation, hanabi1224 prefers performance-conscious solutions that leverage finality guarantees rather than expensive chain walking for fork detection. The approach of constraining cache lookups to finalized epochs (using CHECKPOINT_INTERVAL >= CHAIN_FINALITY) provides fork safety without the performance cost of ancestry verification.