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

160-466: GetPowerTable::compute now borrows Tipset directly; consider dropping redundant &ts in v12+ macro calls

The switch to ts: &Tipset aligns this helper with the rest of the state-manager APIs and removes the extra Arc layer; the internal uses (get_actor_state*, resolve_to_deterministic_address, logging, etc.) are all consistent and avoid additional cloning, which fits the PR goal.

One minor clean‑up: in the v12–v17 branches you still pass &ts into handle_miner_state_v12_on!, which makes $ts an &&Tipset while v8–v11 use ts directly. It’s harmless but slightly noisy and inconsistent with the earlier arms; you can simplify the call sites to take ts as an &Tipset:

-            power::State::V12(s) => {
-                handle_miner_state_v12_on!(
-                    v12,
-                    id_power_worker_mappings,
-                    &ts,
-                    s,
-                    &from_policy_v13_to_v12(&ctx.chain_config().policy)
-                );
-            }
+            power::State::V12(s) => {
+                handle_miner_state_v12_on!(
+                    v12,
+                    id_power_worker_mappings,
+                    ts,
+                    s,
+                    &from_policy_v13_to_v12(&ctx.chain_config().policy)
+                );
+            }
@@
-            power::State::V13(s) => {
+            power::State::V13(s) => {
                 handle_miner_state_v12_on!(
                     v13,
                     id_power_worker_mappings,
-                    &ts,
+                    ts,
                     s,
                     &ctx.chain_config().policy
                 );
             }
@@
-            power::State::V14(s) => {
+            power::State::V14(s) => {
                 handle_miner_state_v12_on!(
                     v14,
                     id_power_worker_mappings,
-                    &ts,
+                    ts,
                     s,
                     &from_policy_v13_to_v14(&ctx.chain_config().policy)
                 );
             }
@@
-            power::State::V15(s) => {
+            power::State::V15(s) => {
                 handle_miner_state_v12_on!(
                     v15,
                     id_power_worker_mappings,
-                    &ts,
+                    ts,
                     s,
                     &from_policy_v13_to_v15(&ctx.chain_config().policy)
                 );
             }
@@
-            power::State::V16(s) => {
+            power::State::V16(s) => {
                 handle_miner_state_v12_on!(
                     v16,
                     id_power_worker_mappings,
-                    &ts,
+                    ts,
                     s,
                     &from_policy_v13_to_v16(&ctx.chain_config().policy)
                 );
             }
@@
-            power::State::V17(s) => {
+            power::State::V17(s) => {
                 handle_miner_state_v12_on!(
                     v17,
                     id_power_worker_mappings,
-                    &ts,
+                    ts,
                     s,
                     &from_policy_v13_to_v17(&ctx.chain_config().policy)
                 );
             }

src/message_pool/msgpool/selection.rs (1)

9-29: Tipset-by-value run_head_change refactor looks sound; BorrowMut use is slightly overkill

Passing from/to as owned Tipset and cloning them into left_chain/right_chain preserves the previous behaviour while avoiding deep cloning, assuming Tipset stays internally Arc-backed. The epoch walk and LCA search logic are unchanged.

The new BorrowMut import and usage (rmsgs.borrow_mut()) work but are a bit unnecessary here: rmsgs is already a &mut HashMap<…>, which is exactly what remove_from_selected_msgs conceptually needs. If that helper is now generic over impl BorrowMut<_>, it’s more idiomatic to take the BorrowMut at the callee boundary and keep call sites passing &mut directly.

Not a blocker, but you could simplify by either:

• Letting remove_from_selected_msgs itself call .borrow_mut() on its argument, or
• Narrowing the parameter type back to &mut Pending and passing rmsgs without the trait call here.

Also applies to: 815-869

src/rpc/methods/eth/filter/mod.rs (1)

405-414: Remove unnecessary Arc wrapping.

Since collect_events now accepts &Tipset instead of &Arc<Tipset>, the Arc::new() wrapping at lines 407 and 412 is no longer necessary. Due to Deref coercion this still compiles, but it allocates an Arc unnecessarily.

 ParsedFilterTipsets::Hash(block_hash) => {
     let tipset = get_tipset_from_hash(ctx.chain_store(), block_hash)?;
-    let tipset = Arc::new(tipset);
     Self::collect_events(ctx, &tipset, Some(pf), skip_event, &mut collected_events)
         .await?;
 }
 ParsedFilterTipsets::Key(tsk) => {
-    let tipset = Arc::new(Tipset::load_required(ctx.store(), tsk)?);
+    let tipset = Tipset::load_required(ctx.store(), tsk)?;
     Self::collect_events(ctx, &tipset, Some(pf), skip_event, &mut collected_events)
         .await?;
 }

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

1505-1547: get_block_receipts still wraps Tipset into an Arc unnecessarily

The function:

• Resolves ts: Tipset, then immediately does let ts_ref = Arc::new(ts);.
• Passes &ts_ref into execute_tipset and new_eth_tx_receipt, which now both only require &Tipset.

This reintroduces an Arc<Tipset> layer in a hot path without need; a simple refactor to keep ts as a plain Tipset and pass &ts everywhere would reduce allocations and keep the API surface cleaner.

Example refactor:

-    let ts = tipset_by_block_number_or_hash(...)?;
-    ...
-    let ts_ref = Arc::new(ts);
-    let ts_key = ts_ref.key();
+    let ts = tipset_by_block_number_or_hash(...)?;
+    let ts_key = ts.key();
...
-    let (state_root, msgs_and_receipts) = execute_tipset(ctx, &ts_ref).await?;
+    let (state_root, msgs_and_receipts) = execute_tipset(ctx, &ts).await?;
...
-            ts_ref.epoch(),
-            &ts_key.cid()?,
+            ts.epoch(),
+            &ts_key.cid()?,
...
-        let receipt = new_eth_tx_receipt(ctx, &ts_ref, &tx, &receipt).await?;
+        let receipt = new_eth_tx_receipt(ctx, &ts, &tx, &receipt).await?;

This keeps behavior identical but avoids the extra Arc.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

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.

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: 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
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:461-487
Timestamp: 2025-08-08T12:11:55.266Z
Learning: Forest (src/ipld/util.rs, Rust): In UnorderedChainStream::poll_next, dropping `extract_sender` (when no more tipsets and the extract queue is empty) is the intended shutdown signal for workers. Any subsequent attempt to enqueue work after this drop is a logic error and should be treated as an error; do not change `send()` to ignore a missing sender.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 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.

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.

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: hanabi1224
Repo: ChainSafe/forest PR: 5946
File: src/rpc/methods/state.rs:1458-1458
Timestamp: 2025-08-19T11:17:35.177Z
Learning: FuturesOrdered in the futures crate has a push_front method that pushes futures to the front of the queue, making them next to be considered for completion, in addition to the standard push/push_back methods.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.

Learnt from: elmattic
Repo: ChainSafe/forest PR: 6166
File: src/auth/mod.rs:127-150
Timestamp: 2025-10-16T11:05:13.586Z
Learning: In Rust 2024, `std::env::set_var` and `std::env::remove_var` are unsafe functions. They require `unsafe` blocks because they are only safe in single-threaded programs (Windows is an exception). When these functions are used in tests, ensure proper serialization with `#[serial]` or similar mechanisms.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5841
File: src/utils/get_size/mod.rs:10-10
Timestamp: 2025-07-17T15:21:40.753Z
Learning: The get_size2 crate's GetSize trait provides default implementations: get_stack_size() uses std::mem::size_of, get_heap_size() returns 0, and get_size() returns their sum. An empty impl like `impl GetSize for MyType {}` is valid and uses these defaults, making it suitable for simple wrapper types that don't allocate heap memory.