src/shim/sector.rs (1)

138-142: Make invalid() a const (and consider crate-private).

Const enables usage in statics and avoids runtime cost. If this is only for internals/tests, prefer pub(crate) to reduce accidental use.

-impl RegisteredSealProof {
-    pub fn invalid() -> Self {
-        RegisteredSealProof(RegisteredSealProofV4::Invalid(0))
-    }
-}
+impl RegisteredSealProof {
+    pub const fn invalid() -> Self {
+        RegisteredSealProof(RegisteredSealProofV4::Invalid(0))
+    }
+}

src/rpc/registry/methods_reg.rs (1)

97-97: Add Market to the supported-actors test to prevent regressions.

Extend the test matrix so we catch gaps if future refactors miss Market method registration.

         let supported_actors = vec![
             BuiltinActor::Account,
             BuiltinActor::Miner,
             BuiltinActor::EVM,
             BuiltinActor::Cron,
             BuiltinActor::DataCap,
+            BuiltinActor::Market,
         ];

src/lotus_json/padded_piece_size.rs (2)

7-10: Consider marking the wrapper as serde-transparent.

Add #[serde(transparent)] to guarantee newtype passthrough semantics and avoid any accidental nesting if serde derives change in the future.

-#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, JsonSchema)]
+#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, JsonSchema)]
+#[serde(transparent)]
 pub struct PaddedPieceSizeLotusJson(#[schemars(with = "u64")] PaddedPieceSize);

---

14-17: Add minimal snapshots to lock in JSON as a plain number.

Right now snapshots() returns an empty vec. Add at least one sample (e.g., 2048) to catch regressions in encoding/decoding.

     #[cfg(test)]
     fn snapshots() -> Vec<(serde_json::Value, Self)> {
-        vec![]
+        vec![
+            (json!(0u64), PaddedPieceSize(0)),
+            (json!(2048u64), PaddedPieceSize(2048)),
+        ]
     }

src/rpc/registry/actors/market.rs (1)

177-190: Dispatch covers v8–v16; consider logging for unknown versions.

Match arms look correct. Optionally log a trace on the default branch to aid debugging when encountering unexpected versions.

-        _ => {}
+        _ => {
+            tracing::trace!(version, "market: no methods registered for unknown actor version");
+        }

src/lotus_json/actor_states/methods/miner_actor_params.rs (2)

3713-3741: Optional: add snapshots for PieceChange to validate CID/size/payload.

Providing at least one non-empty example will guard against regressions in PaddedPieceSize mapping and RawBytes handling.

                     #[cfg(test)]
                     fn snapshots() -> Vec<(serde_json::Value, Self)> {
-                        vec![
-                        ]
+                        let cid = Cid::try_from("bafy2bzacedu6q2cpk2t7c6gk7m2wkfpfb4v6qzjvkg3gq6m7k6c3r5h7lxy").unwrap();
+                        vec![(
+                            json!({
+                                "Data": "bafy2bzacedu6q2cpk2t7c6gk7m2wkfpfb4v6qzjvkg3gq6m7k6c3r5h7lxy",
+                                "Size": 2048,
+                                "Payload": "AQID"
+                            }),
+                            Self {
+                                data: cid,
+                                size: fvm_shared4::piece::PaddedPieceSize(2048).into(),
+                                payload: RawBytes::new(vec![1,2,3]),
+                            },
+                        )]
                     }

---

3766-3790: Optional: add snapshots for SectorChanges/SectorContentChangedParams.

One end-to-end example (with a single added PieceChange) would exercise nested mappings and the new PaddedPieceSize LotusJson wrapper.

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

3432-3434: Rename misleading var: it’s used by both AddBalanceExported and GetBalanceExported.

market_actor_get_balance_exported_params holds a generic Address and is used for AddBalanceExported too. Rename to avoid confusion.

-    let market_actor_get_balance_exported_params = Address::new_id(1000);
+    let market_actor_address_param = Address::new_id(1000);
@@
-            to_vec(&market_actor_get_balance_exported_params)?,
+            to_vec(&market_actor_address_param)?,
@@
-            to_vec(&market_actor_get_balance_exported_params)?,
+            to_vec(&market_actor_address_param)?,

Also applies to: 3508-3512, 3526-3530

---

3330-3373: Minor: prefer u64 literals for atto amounts to avoid implicit narrowing.

TokenAmount::from_atto(10u8) works but reads odd. Use 10u64 for clarity/consistency across the file.

---

3471-3494: Document the TODOs with upstream issue links in code comments.

You already reference go-state-types issues—add a brief rationale inline so future readers know why tests are gated.

Also applies to: 3496-3506

src/lotus_json/actor_states/methods/miner_change_worker_params.rs (1)

76-88: Schema/serde consistency: payload uses RawBytes here (vs Vec in market).

Assuming miner PieceChange truly uses RawBytes (and market’s ext uses Vec), this divergence is correct. If intentional, consider a brief comment to prevent future “deduplicate” attempts.

src/lotus_json/actor_states/methods/market_actor_params.rs (4)

674-689: Lotus field naming: DealIDs vs DealIds.

You’ve correctly used DealIDs here (and DealIds elsewhere where Lotus uses that spelling). Add a short comment noting Lotus’ inconsistency to avoid “fixing” it later.

---

1450-1454: BitField snapshot is inconsistent with constructed value.

Snapshot JSON shows [0] but the constructed BitField::new() is empty. Align them to avoid brittle tests.

-                                serde_json::json!([0]),
-                                Self { deal_ids: BitField::new() }
+                                serde_json::json!([]),
+                                Self { deal_ids: BitField::new() }

Or, if you want [0] in JSON, set the bit:

+                                let mut bf = BitField::new();
+                                bf.set(0);
-                                Self { deal_ids: BitField::new() }
+                                Self { deal_ids: bf }

Also applies to: 1463-1481

---

301-327: Signature type per version looks right; add a unit snapshot for BLS too.

You cover Secp256k1; consider adding a BLS snapshot to catch cross-version serde differences early.

Also applies to: 328-405

---

1300-1336: Nit: Consistent JSON keys for SectorDataSpec and ComputeDataCommitmentParams.

You intentionally use “DealIds” here (PascalCase) vs “DealIDs” elsewhere. Add inline comments to document Lotus API expectations to deter accidental renames.

Also applies to: 1354-1398

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.937Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.

Learnt from: akaladarshi
PR: ChainSafe/forest#5896
File: src/lotus_json/actor_states/methods/verified_reg_actor.rs:133-137
Timestamp: 2025-08-18T12:25:29.183Z
Learning: In Lotus JSON for verified registry actor, the SectorAllocationClaimsLotusJson struct uses "SectorExpiry" as the field name for the expiry field, not "Expiry". The current code with #[serde(rename = "SectorExpiry")] is correct.

Learnt from: akaladarshi
PR: ChainSafe/forest#5923
File: src/rpc/registry/actors/miner.rs:221-223
Timestamp: 2025-09-02T10:05:34.350Z
Learning: For miner actor ChangeOwnerAddress and ChangeOwnerAddressExported methods: versions 8-10 use bare Address as parameter type, while versions 11+ use ChangeOwnerAddressParams. This reflects the evolution of the Filecoin miner actor parameter structures across versions.

Learnt from: akaladarshi
PR: ChainSafe/forest#5923
File: src/rpc/registry/actors/miner.rs:0-0
Timestamp: 2025-08-14T06:46:08.056Z
Learning: ProveCommitSectors3 method is available in fil_actor_miner_state::v13 and should be mapped to ProveCommitSectors3Params, not ProveCommitSectorParams.

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

3330-3373: Helper builders: OK; minor nit on tiny TokenAmount literals

Using TokenAmount::from_atto(10u8) works but is a bit odd next to other u64 literals. Consider u64 for consistency.

- fvm_shared4::econ::TokenAmount::from_atto(10u8),
+ fvm_shared4::econ::TokenAmount::from_atto(10u64),

---

3597-3602: BitField setup for SettleDealPaymentsExported

Using a BitField with bit 42 set is fine. Consider adding another case with multiple ranges to catch encoding edge cases later.

src/lotus_json/actor_states/methods/market_actor_params.rs (1)

1517-1579: SectorContentChangedParams mappings

The ext::miner wrapper looks consistent. Consider adding a non-empty snapshot with multiple PieceChange entries later to harden coverage.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.937Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.

Learnt from: akaladarshi
PR: ChainSafe/forest#5896
File: src/lotus_json/actor_states/methods/verified_reg_actor.rs:133-137
Timestamp: 2025-08-18T12:25:29.183Z
Learning: In Lotus JSON for verified registry actor, the SectorAllocationClaimsLotusJson struct uses "SectorExpiry" as the field name for the expiry field, not "Expiry". The current code with #[serde(rename = "SectorExpiry")] is correct.

src/rpc/registry/actors/market.rs (3)

4-4: Alias the macro to avoid name confusion with the local fn of the same identifier.

Having both a macro and a function named register_actor_methods in scope is legal but easy to misread. Alias the macro and invoke the alias throughout this module.

Apply:

-use crate::rpc::registry::methods_reg::{MethodRegistry, register_actor_methods};
+use crate::rpc::registry::methods_reg::{MethodRegistry, register_actor_methods as register_methods};

Then replace all invocations:

-        register_actor_methods!(
+        register_methods!(

(Repeat for all occurrences in this file.)

---

89-109: Slight import clarity tweak for nested ext::miner path.

Split the nested import to make the scope explicit and reduce chances of future merge conflicts.

-        use $market_state_version::{
-            GetDealSectorParams, Method, SettleDealPaymentsParams,
-            ext::miner::SectorContentChangedParams,
-        };
+        use $market_state_version::{GetDealSectorParams, Method, SettleDealPaymentsParams};
+        use $market_state_version::ext::miner::SectorContentChangedParams;

---

177-191: Log on unknown/unsupported version to aid diagnostics.

A silent no-op on _ => {} can hide misconfigurations. Emit a low-level log.

-        _ => {}
+        _ => {
+            tracing::debug!(%version, actor_code = %cid, "Market actor version not handled; no method registrations performed");
+        }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/lotus_json/actor_states/methods/market_actor_params.rs (2)

14-18: Fix: bring serde_json::json! into scope (compilation blocker).

Unqualified json! is used in several snapshots but the macro isn’t imported. Add the import (or fully-qualify each call).

Apply at the top of the file:

 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
+use serde_json::json;
 use std::fmt::Debug;

Also applies to: 81-85, 422-447, 504-526, 586-610

---

1424-1431: Empty BitField snapshot should be [] (not [0]).

[0] encodes bit 0 set; an empty BitField should serialize to an empty array.

-                            (
-                                serde_json::json!([0]),
-                                Self { deal_ids: BitField::new() }
-                            ),
+                            (
+                                serde_json::json!([]),
+                                Self { deal_ids: BitField::new() }
+                            ),

src/lotus_json/actor_states/methods/market_actor_params.rs (1)

411-491: Optional: deduplicate PublishStorageDeals snapshots.

The V2/V3/V4 snapshot blocks share most JSON; only the signature type differs. Consider extracting a tiny helper to build the common JSON/value pair to reduce duplication and future drift.

I can propose a small local function under #[cfg(test)] returning the shared tuple, parameterized by the signature.

Also applies to: 493-573, 575-655

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: akaladarshi
PR: ChainSafe/forest#5896
File: src/lotus_json/actor_states/methods/verified_reg_actor.rs:133-137
Timestamp: 2025-08-18T12:25:29.183Z
Learning: In Lotus JSON for verified registry actor, the SectorAllocationClaimsLotusJson struct uses "SectorExpiry" as the field name for the expiry field, not "Expiry". The current code with #[serde(rename = "SectorExpiry")] is correct.

Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:56-58
Timestamp: 2025-09-02T08:44:08.346Z
Learning: In Lotus JSON for payment channel SignedVoucher, the secret_pre_image field should use serde rename to "SecretHash" (not "SecretPreImage") for Lotus API compatibility, even though the internal struct field is named SecretPreimage. This matches the actual JSON format used by Lotus.

Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.937Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.