Skip to content

fix(test): JSON snapshot tests for actor states #6381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hanabi1224 merged 1 commit into from
Jan 6, 2026
Merged

fix(test): JSON snapshot tests for actor states #6381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/lotus_json/actors/states/account_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
// SPDX-License-Identifier: Apache-2.0, MIT

use super::*;
use crate::shim::{actors::account, address::Address};
use crate::shim::{actors::account::State, address::Address};
use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
pub struct AccountStateLotusJson {
#[schemars(with = "LotusJson<Address>")]
#[serde(with = "crate::lotus_json")]
address: Address,
}

impl HasLotusJson for account::State {
impl HasLotusJson for State {
type LotusJson = AccountStateLotusJson;

#[cfg(test)]
Expand All @@ -38,3 +38,4 @@ impl HasLotusJson for account::State {
Self::default_latest_version(lotus_json.address.into())
}
}
crate::test_snapshots!(State);
3 changes: 2 additions & 1 deletion src/lotus_json/actors/states/cron_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;
use crate::shim::actors::cron::{Entry, State};
use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, PartialEq, Serialize, Deserialize, JsonSchema)]
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Missing Eq derive creates inconsistency.

CronStateLotusJson derives only PartialEq, not Eq, which is inconsistent with other state structs in this PR (InitStateLotusJson, TombstoneLotusJson, PaymentChannelStateLotusJson all derive both). This happens because Entry (from cron/mod.rs) doesn't derive Eq. Once Entry is updated to derive Eq, this struct should also be updated for consistency.

🔎 Proposed fix (apply after fixing Entry) 
-#[derive(Debug, PartialEq, Serialize, Deserialize, JsonSchema)]
+#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
 #[serde(rename_all = "PascalCase")]
 pub struct CronStateLotusJson {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In @src/lotus_json/actors/states/cron_state.rs around line 8, CronStateLotusJson
currently derives PartialEq but not Eq (inconsistency with other state structs)
because its field type Entry lacks Eq; after updating Entry in cron/mod.rs to
derive Eq, update the CronStateLotusJson derive list to include Eq (i.e., change
#[derive(Debug, PartialEq, Serialize, Deserialize, JsonSchema)] to include Eq)
so the struct matches InitStateLotusJson, TombstoneLotusJson, and
PaymentChannelStateLotusJson.

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eq is not applicable for external dependencies fil_actor_cron_state::v8::Entry

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanabi1224, thank you for the clarification! You're absolutely right—since Entry wraps external types from fil_actor_cron_state that don't implement Eq, it's not possible for CronStateLotusJson to derive Eq either. Deriving only PartialEq is the correct approach here given this constraint.

The inconsistency with other state structs that derive both Eq and PartialEq is justified and unavoidable in this case.

✏️ Learnings added 
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/cron_state.rs:8-8
Timestamp: 2026-01-05T12:54:40.850Z
Learning: In the Forest codebase, CronStateLotusJson in src/lotus_json/actors/states/cron_state.rs derives only PartialEq (not Eq) because it contains Entry types that wrap external dependencies from fil_actor_cron_state, which don't implement Eq. This constraint from external dependencies prevents deriving Eq.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 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: 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.

#[serde(rename_all = "PascalCase")]
pub struct CronStateLotusJson {
#[schemars(with = "LotusJson<Vec<Entry>>")]
Expand Down Expand Up @@ -75,3 +75,4 @@ impl HasLotusJson for State {
State::default_latest_version_from_entries(entries)
}
}
crate::test_snapshots!(State);
15 changes: 8 additions & 7 deletions src/lotus_json/actors/states/datacap_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::shim::actors::datacap::State;
use crate::shim::address::Address;
use fil_actors_shared::frc46_token::token::state::TokenState;

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
#[schemars(rename = "DatacapState")]
pub struct DatacapStateLotusJson {
Expand All @@ -26,12 +26,12 @@ impl HasLotusJson for State {
fn snapshots() -> Vec<(serde_json::Value, Self)> {
vec![(
json!({
"governor": "t00",
"token": {
"supply": "0",
"balances": {"/":"baeaaaaa"},
"allowances": {"/":"baeaaaaa"},
"hamt_bit_width": 0
"Governor": "f00",
"Token": {
"Supply": "0",
"Balances": {"/":"baeaaaaa"},
"Allowances": {"/":"baeaaaaa"},
"HamtBitWidth": 0
}
}),
State::default_latest_version(
Expand Down Expand Up @@ -70,3 +70,4 @@ impl HasLotusJson for State {
State::default_latest_version(lotus_json.governor.into(), lotus_json.token)
}
}
crate::test_snapshots!(State);
232 changes: 124 additions & 108 deletions src/lotus_json/actors/states/evm_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::shim::actors::evm::{State, TombstoneState};
use ::cid::Cid;
use pastey::paste;

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
#[schemars(rename = "EVMState")]
pub struct EVMStateLotusJson {
Expand Down Expand Up @@ -35,60 +35,67 @@ macro_rules! impl_evm_state_lotus_json {
(no_transient_data: $($version:literal),+) => {
$(
paste! {
impl HasLotusJson for fil_actor_evm_state::[<v $version>]::State {
type LotusJson = EVMStateLotusJson;

#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
vec![(
json!({
"Bytecode": {"/":"baeaaaaa"},
"BytecodeHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"ContractState": {"/":"baeaaaaa"},
"Nonce": 0,
"Tombstone": null,
"TransientData": null
}),
Self {
bytecode: Cid::default(),
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from([0; 32]),
contract_state: Cid::default(),
nonce: 0,
tombstone: None,
},
)]
mod [<impl_evm_state_lotus_json_ $version>] {
use super::*;
type T = fil_actor_evm_state::[<v $version>]::State;
#[test]
fn snapshots() {
crate::lotus_json::assert_all_snapshots::<T>();
}
Comment on lines +41 to 44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing #[cfg(test)] on test functions may cause compilation issues.

Similar to the issue in vesting_funds.rs, the snapshots test functions inside the macro-generated modules are missing #[cfg(test)] attributes. This could cause compilation errors in release mode if crate::lotus_json::assert_all_snapshots is only available in test builds.

🔎 Proposed fix 
             mod [<impl_evm_state_lotus_json_ $version>] {
                 use super::*;
                 type T = fil_actor_evm_state::[<v $version>]::State;
+                #[cfg(test)]
                 #[test]
                 fn snapshots() {
                     crate::lotus_json::assert_all_snapshots::<T>();
                 }

Apply to both macro arms (no_transient_data and with_transient_data).

Also applies to: 112-115

🤖 Prompt for AI Agents 
In @src/lotus_json/actors/states/evm_state.rs around lines 41-44, The test
functions named snapshots that call
crate::lotus_json::assert_all_snapshots::<T>() need a #[cfg(test)] attribute so
they are compiled only in test builds; add #[cfg(test)] above each snapshots fn
in both macro arms (no_transient_data and with_transient_data) — apply the same
change to the other occurrence (the second snapshots function) to avoid
release-mode compilation errors when assert_all_snapshots is test-only.

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI proves the code compiles fine

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanabi1224, you're absolutely right! I apologize for the incorrect suggestion. Functions marked with #[test] are already only compiled during test builds by Rust's built-in test framework, so the additional #[cfg(test)] attribute is unnecessary. The code compiles correctly as-is, and CI confirms this.

Thank you for the clarification!

✏️ Learnings added 
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/evm_state.rs:41-44
Timestamp: 2026-01-05T12:56:13.802Z
Learning: In the Forest codebase, functions marked with #[test] do not require an additional #[cfg(test)] attribute, as the #[test] attribute already ensures they are only compiled during test builds. Do not suggest adding #[cfg(test)] to functions that already have #[test].

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/cron_state.rs:8-8
Timestamp: 2026-01-05T12:54:40.850Z
Learning: In the Forest codebase, CronStateLotusJson in src/lotus_json/actors/states/cron_state.rs derives only PartialEq (not Eq) because it contains Entry types that wrap external dependencies from fil_actor_cron_state, which don't implement Eq. This constraint from external dependencies prevents deriving Eq.

Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 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: hanabi1224
Repo: ChainSafe/forest PR: 6074
File: src/rpc/methods/chain.rs:55-56
Timestamp: 2025-09-17T11:32:44.185Z
Learning: In the Forest codebase, hanabi1224 prefers that CodeRabbit should not warn about potential compilation issues (such as Send bounds, async/await compatibility, etc.) since they are already guarded by CI tests. Focus should be on higher-level design and logic issues instead.

Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.

Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5946
File: src/rpc/methods/state.rs:1459-1463
Timestamp: 2025-08-19T11:25:56.710Z
Learning: hanabi1224 prefers that CodeRabbit should not warn about compilation errors in the Forest codebase since they are already guarded by CI tests. Focus should be on higher-level design and logic issues instead.

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.

impl HasLotusJson for T {
type LotusJson = EVMStateLotusJson;
#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
vec![(
json!({
"Bytecode": {"/":"baeaaaaa"},
"BytecodeHash": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],
"ContractState": {"/":"baeaaaaa"},
"Nonce": 0,
"Tombstone": null,
"TransientData": null
}),
Self {
bytecode: Cid::default(),
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from([0; 32]),
contract_state: Cid::default(),
nonce: 0,
tombstone: None,
},
)]
}

fn into_lotus_json(self) -> Self::LotusJson {
let bytecode_hash_data: [u8; 32] = self.bytecode_hash.into();

EVMStateLotusJson {
bytecode: self.bytecode,
bytecode_hash: bytecode_hash_data,
contract_state: self.contract_state,
nonce: self.nonce,
tombstone: self.tombstone.map(|t| {
let tombstone_state = TombstoneState::[<V $version>](t);
tombstone_state.into_lotus_json()
}),
transient_data: None,
fn into_lotus_json(self) -> Self::LotusJson {
let bytecode_hash_data: [u8; 32] = self.bytecode_hash.into();

EVMStateLotusJson {
bytecode: self.bytecode,
bytecode_hash: bytecode_hash_data,
contract_state: self.contract_state,
nonce: self.nonce,
tombstone: self.tombstone.map(|t| {
let tombstone_state = TombstoneState::[<V $version>](t);
tombstone_state.into_lotus_json()
}),
transient_data: None,
}
}
}

fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
let tombstone = lotus_json.tombstone.map(|tombstone_lotus| {
fil_actor_evm_state::[<v $version>]::Tombstone {
origin: tombstone_lotus.origin.into(),
nonce: tombstone_lotus.nonce,
fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
let tombstone = lotus_json.tombstone.map(|tombstone_lotus| {
fil_actor_evm_state::[<v $version>]::Tombstone {
origin: tombstone_lotus.origin.into(),
nonce: tombstone_lotus.nonce,
}
});

Self {
bytecode: lotus_json.bytecode,
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from(lotus_json.bytecode_hash),
contract_state: lotus_json.contract_state,
nonce: lotus_json.nonce,
tombstone,
}
});

Self {
bytecode: lotus_json.bytecode,
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from(lotus_json.bytecode_hash),
contract_state: lotus_json.contract_state,
nonce: lotus_json.nonce,
tombstone,
}
}
}
Expand All @@ -99,70 +106,78 @@ macro_rules! impl_evm_state_lotus_json {
(with_transient_data: $($version:literal),+) => {
$(
paste! {
impl HasLotusJson for fil_actor_evm_state::[<v $version>]::State {
type LotusJson = EVMStateLotusJson;

#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
vec![(
json!({
"Bytecode": {"/":"baeaaaaa"},
"BytecodeHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"ContractState": {"/":"baeaaaaa"},
"Nonce": 0,
"Tombstone": null,
"TransientData": null
}),
Self {
bytecode: Cid::default(),
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from([0; 32]),
contract_state: Cid::default(),
nonce: 0,
tombstone: None,
transient_data: None,
},
)]
mod [<impl_evm_state_lotus_json_ $version>] {
use super::*;
type T = fil_actor_evm_state::[<v $version>]::State;
#[test]
fn snapshots() {
crate::lotus_json::assert_all_snapshots::<T>();
}

fn into_lotus_json(self) -> Self::LotusJson {
let bytecode_hash_data: [u8; 32] = self.bytecode_hash.into();

EVMStateLotusJson {
bytecode: self.bytecode,
bytecode_hash: bytecode_hash_data,
contract_state: self.contract_state,
nonce: self.nonce,
tombstone: self.tombstone.map(|t| {
let tombstone_state = TombstoneState::[<V $version>](t);
tombstone_state.into_lotus_json()
}),
transient_data: Some(self.transient_data.map(|td| {
td.into_lotus_json()
})),
impl HasLotusJson for T {
type LotusJson = EVMStateLotusJson;

#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
vec![(
json!({
"Bytecode": {"/":"baeaaaaa"},
"BytecodeHash": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],
"ContractState": {"/":"baeaaaaa"},
"Nonce": 0,
"Tombstone": null,
"TransientData": null
}),
Self {
bytecode: Cid::default(),
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from([0; 32]),
contract_state: Cid::default(),
nonce: 0,
tombstone: None,
transient_data: None,
},
)]
}
}

fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
let tombstone = lotus_json.tombstone.map(|tombstone_lotus| {
fil_actor_evm_state::[<v $version>]::Tombstone {
origin: tombstone_lotus.origin.into(),
nonce: tombstone_lotus.nonce,
fn into_lotus_json(self) -> Self::LotusJson {
let bytecode_hash_data: [u8; 32] = self.bytecode_hash.into();

EVMStateLotusJson {
bytecode: self.bytecode,
bytecode_hash: bytecode_hash_data,
contract_state: self.contract_state,
nonce: self.nonce,
tombstone: self.tombstone.map(|t| {
let tombstone_state = TombstoneState::[<V $version>](t);
tombstone_state.into_lotus_json()
}),
transient_data: Some(self.transient_data.map(|td| {
td.into_lotus_json()
})),
}
});
}

let transient_data = lotus_json.transient_data
.and_then(|outer_option| outer_option)
.map(|transient_data_lotus| {
fil_actor_evm_state::[<v $version>]::TransientData::from_lotus_json(transient_data_lotus)
fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
let tombstone = lotus_json.tombstone.map(|tombstone_lotus| {
fil_actor_evm_state::[<v $version>]::Tombstone {
origin: tombstone_lotus.origin.into(),
nonce: tombstone_lotus.nonce,
}
});

Self {
bytecode: lotus_json.bytecode,
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from(lotus_json.bytecode_hash),
contract_state: lotus_json.contract_state,
nonce: lotus_json.nonce,
tombstone,
transient_data,
let transient_data = lotus_json.transient_data
.and_then(|outer_option| outer_option)
.map(|transient_data_lotus| {
fil_actor_evm_state::[<v $version>]::TransientData::from_lotus_json(transient_data_lotus)
});

Self {
bytecode: lotus_json.bytecode,
bytecode_hash: fil_actor_evm_state::[<v $version>]::BytecodeHash::from(lotus_json.bytecode_hash),
contract_state: lotus_json.contract_state,
nonce: lotus_json.nonce,
tombstone,
transient_data,
}
}
}
}
Expand All @@ -179,7 +194,7 @@ impl HasLotusJson for State {
vec![(
json!({
"Bytecode": {"/":"baeaaaaa"},
"BytecodeHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"BytecodeHash": [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],
"ContractState": {"/":"baeaaaaa"},
"Nonce": 0,
"Tombstone": null,
Expand Down Expand Up @@ -216,6 +231,7 @@ impl HasLotusJson for State {
)
}
}
crate::test_snapshots!(State);

// Implement for versions without transient_data (v10-v15)
impl_evm_state_lotus_json!(no_transient_data: 10, 11, 12, 13, 14, 15);
Expand Down
3 changes: 2 additions & 1 deletion src/lotus_json/actors/states/init_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;
use crate::shim::actors::init::State;
use ::cid::Cid;

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
#[schemars(rename = "InitState")]
pub struct InitStateLotusJson {
Expand Down Expand Up @@ -59,3 +59,4 @@ impl HasLotusJson for State {
)
}
}
crate::test_snapshots!(State);
5 changes: 3 additions & 2 deletions src/lotus_json/actors/states/market_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::shim::{clock::ChainEpoch, econ::TokenAmount};
use ::cid::Cid;
use fvm_shared4::deal::DealID;

#[derive(Debug, Serialize, Deserialize, JsonSchema)]
#[derive(Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
#[schemars(rename = "MarketState")]
pub struct MarketStateLotusJson {
Expand Down Expand Up @@ -131,7 +131,7 @@ impl HasLotusJson for State {
"TotalClientLockedCollateral": "0",
"TotalProviderLockedCollateral": "0",
"TotalClientStorageFee": "0",
"PendingDealAllocationIDs": {"/":"baeaaaaa"},
"PendingDealAllocationIds": {"/":"baeaaaaa"},
"ProviderSectors": {"/":"baeaaaaa"}
Comment on lines +134 to 135
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the market_state.rs file to understand the full context
fd -t f "market_state.rs" | head -5

Repository: ChainSafe/forest

Length of output: 104

🏁 Script executed:

# Read the market_state.rs file to see the struct definition and serde attributes
cat -n src/lotus_json/actors/states/market_state.rs | head -100

Repository: ChainSafe/forest

Length of output: 4143

🏁 Script executed:

# Search for PendingDealAllocation references in the codebase
rg -n "PendingDealAllocation" --type rust -C 2

Repository: ChainSafe/forest

Length of output: 534

🏁 Script executed:

# Check for NextID patterns and other explicit renames in the same file
rg -n "rename = " src/lotus_json/actors/states/market_state.rs

Repository: ChainSafe/forest

Length of output: 161

🏁 Script executed:

# Search for test files related to market_state
fd -t f "market_state" --type rust | grep -i test

Repository: ChainSafe/forest

Length of output: 231

🏁 Script executed:

# Look for JSON examples or test data that might show the expected format
rg -l "PendingDealAllocationIds\|PendingDealAllocationIDs" --type rust

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Check if there are any Lotus-related documentation or comments about the format
rg -n "Lotus\|lotus\|API" src/lotus_json/actors/states/market_state.rs -C 2

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Search for references in tests to see how this field is used
rg -n "pending_deal_allocation" --type rust -C 3

Repository: ChainSafe/forest

Length of output: 13223

🏁 Script executed:

# Look for test JSON data in the repo
find . -name "*.json" -type f | head -20

Repository: ChainSafe/forest

Length of output: 811

🏁 Script executed:

# Search for references to Lotus documentation or API specs
rg -n "lotus\|Lotus" src/lotus_json/actors/states/market_state.rs -A 2 -B 2

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Look for comparison with NextID pattern - check other ID fields
rg -n "rename = \".*ID\"" src/lotus_json/ --type rust

Repository: ChainSafe/forest

Length of output: 1552

🏁 Script executed:

# Check git history or comments around this field
git log -p --follow -S "pending_deal_allocation" -- src/lotus_json/actors/states/market_state.rs 2>/dev/null | head -100

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Look for test files that might contain market state JSON examples
find . -path ./node_modules -prune -o -name "*market*" -type f | grep -E "\.(json|txt|toml)" | head -20

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Search for test functions that serialize/deserialize MarketStateLotusJson
rg -n "MarketStateLotusJson" --type rust -A 5 -B 2 | head -80

Repository: ChainSafe/forest

Length of output: 3886

🏁 Script executed:

# Check if there's any documentation about the field name format
rg -n "PendingDealAllocationIDs\|AllocationIds" --type rust

Repository: ChainSafe/forest

Length of output: 42

🏁 Script executed:

# Get the test snapshot JSON to see the actual expected format
sed -n '119,170p' src/lotus_json/actors/states/market_state.rs

Repository: ChainSafe/forest

Length of output: 1828

Add explicit rename to preserve uppercase "IDs" in field name.

The field should use rename = "PendingDealAllocationIDs" to match the Lotus API format and be consistent with the next_id field (line 35) which explicitly renames to "NextID". The current reliance on rename_all = "PascalCase" produces "PendingDealAllocationIds", but all similar ID fields in the codebase use explicit renames to preserve uppercase "IDs" for API compatibility.

🤖 Prompt for AI Agents 
In @src/lotus_json/actors/states/market_state.rs around lines 134-135, The JSON
field for pending deal allocation IDs must preserve uppercase "IDs": add an
explicit serde rename for the struct field pending_deal_allocation_ids (the
field that currently serializes to "PendingDealAllocationIds") by annotating it
with rename = "PendingDealAllocationIDs" (same style as the NextID explicit
rename used for next_id) so the serialized/deserialized key matches the Lotus
API exactly.

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akaladarshi does this comment make sense?

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Collaborator

@akaladarshi akaladarshi Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanabi1224 It would have been relevant if the lotus was consistent with their naming convention, but right now Lotus is using PendingDealAllocationIds so we have to manually write these.

If the naming was consistent we could just use rename as the comment suggest but sadly it's not.

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! So the current field key is correct and no changes are needed.

Copy link
Copy Markdown
Collaborator

@akaladarshi akaladarshi Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

}),
State::default_latest_version(
Expand Down Expand Up @@ -195,3 +195,4 @@ impl HasLotusJson for State {
)
}
}
crate::test_snapshots!(State);
Loading
Loading