fix(test): JSON snapshot tests for rest of the actor params #6386
Conversation
akaladarshi
requested review from
hanabi1224 and
sudo-shashank
and removed request for
a team
WalkthroughRefactors HasLotusJson impls across lotus_json actor param files: moves per-version trait impls into private modules with a local type alias Changes
Sequence Diagram(s)(omitted — change is an internal refactor without new multi-component control flow) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom Pre-merge Checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-08-18T12:25:29.183ZApplied to files:
📚 Learning: 2025-09-10T12:07:10.578ZApplied to files:
📚 Learning: 2026-01-05T12:54:40.850ZApplied to files:
📚 Learning: 2025-09-02T08:46:04.937ZApplied to files:
📚 Learning: 2026-01-05T12:56:13.802ZApplied to files:
📚 Learning: 2025-09-02T10:05:34.350ZApplied to files:
📚 Learning: 2025-09-02T08:44:08.346ZApplied to files:
📚 Learning: 2026-01-05T13:02:14.604ZApplied to files:
📚 Learning: 2025-09-11T16:03:14.328ZApplied to files:
🧬 Code graph analysis (2)src/lotus_json/actors/params/verified_reg_params.rs (3)
src/lotus_json/actors/params/power_params.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lotus_json/actors/params/verified_reg_params.rs (1)
556-586: Fix serde rename attribute for UniversalReceiverParams.type_ field.The struct at line 251 has
#[serde(rename = "Type_")]but should be#[serde(rename = "Type")]. The snapshot correctly expects the JSON field name to be"Type", which aligns with standard Lotus JSON naming conventions (as seen in other actor parameters like "SectorExpiry" and "SecretHash"). The current serde attribute will cause deserialization failures when Lotus sends the "Type" field.Change line 251 from
#[serde(rename = "Type_")]to#[serde(rename = "Type")].
🤖 Fix all issues with AI agents
In @src/lotus_json/actors/params/power_params.rs:
- Around line 94-113: The snapshot's JSON "Multiaddrs" contains base64 entries
"Ag==" and "Aw==" (which decode to byte vectors [2] and [3]) but the snapshots()
function's Self value sets multiaddrs: vec![]; make these consistent by updating
the Self value in snapshots() to multiaddrs: vec![vec![2], vec![3]] (or
alternatively change the JSON to an empty array) so the Multiaddrs field in the
JSON and the multiaddrs field in snapshots()/Self match.
🧹 Nitpick comments (1)
src/lotus_json/actors/params/paych_params.rs (1)
316-327: Consider usingunwrap_or_default()for cleaner merges deserialization.The current pattern at lines 316-323 can be simplified:
merges: lotus_json.sv.merges.unwrap_or_default().into_iter().map(|m| ...).collect()However, this is a minor style suggestion and the current implementation is correct.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/eam_params.rssrc/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/init_params.rssrc/lotus_json/actors/params/market_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/reward_params.rssrc/lotus_json/actors/params/verified_reg_params.rs
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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: akaladarshi
Repo: ChainSafe/forest PR: 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
Repo: ChainSafe/forest PR: 6000
File: src/lotus_json/actors/params/init_params.rs:34-47
Timestamp: 2025-09-10T12:07:10.578Z
Learning: In Lotus JSON for init actor Exec4Params, the InitExec4ParamsLotusJson struct uses "sub_address" as the field name which serializes to "SubAddress" in JSON via PascalCase conversion. This correctly maps to the internal fil_actor_init_state subaddress field through the conversion methods, maintaining Lotus API compatibility.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/cron_state.rs:8-8
Timestamp: 2026-01-05T12:54:51.873Z
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.
📚 Learning: 2025-08-18T12:25:29.183Z
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.
Applied to files:
src/lotus_json/actors/params/verified_reg_params.rssrc/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/reward_params.rssrc/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/init_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2025-09-10T12:07:10.578Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/lotus_json/actors/params/init_params.rs:34-47
Timestamp: 2025-09-10T12:07:10.578Z
Learning: In Lotus JSON for init actor Exec4Params, the InitExec4ParamsLotusJson struct uses "sub_address" as the field name which serializes to "SubAddress" in JSON via PascalCase conversion. This correctly maps to the internal fil_actor_init_state subaddress field through the conversion methods, maintaining Lotus API compatibility.
Applied to files:
src/lotus_json/actors/params/verified_reg_params.rssrc/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/reward_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/init_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2026-01-05T12:54:40.850Z
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 Rust code reviews, do not derive Eq for a struct if any field does not implement Eq (e.g., types from external dependencies). If a type like CronStateLotusJson includes fields wrapping external dependencies that lack Eq, derive PartialEq (or implement PartialEq manually) but avoid deriving Eq. This ensures comparisons compile and reflect actual equivalence semantics. When needed, consider implementing custom PartialEq (and possibly Eq) only after ensuring all fields (or wrappers) implement Eq, or keep PartialEq-only if full equality semantics cannot be expressed.
Applied to files:
src/lotus_json/actors/params/verified_reg_params.rssrc/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/reward_params.rssrc/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/init_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2026-01-05T12:56:13.802Z
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 Rust codebases (e.g., Forest), do not add #[cfg(test)] to functions already annotated with #[test]. The #[test] attribute ensures the function is compiled only for tests, so a separate #[cfg(test)] is redundant and can be removed if present. Apply this check to all Rust files that contain #[test] functions.
Applied to files:
src/lotus_json/actors/params/verified_reg_params.rssrc/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/reward_params.rssrc/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/init_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2025-09-11T16:03:14.328Z
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.
Applied to files:
src/lotus_json/actors/params/evm_params.rssrc/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2026-01-05T13:02:14.604Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/shim/actors/builtin/cron/mod.rs:47-47
Timestamp: 2026-01-05T13:02:14.604Z
Learning: The Entry enum in src/shim/actors/builtin/cron/mod.rs cannot derive Eq because it wraps external fil_actor_cron_state::vX::Entry types (v8-v17) that don't implement Eq. Only PartialEq can be derived for this enum due to this external dependency constraint.
Applied to files:
src/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/power_params.rs
📚 Learning: 2025-09-02T10:05:34.350Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 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.
Applied to files:
src/lotus_json/actors/params/power_params.rssrc/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/multisig_params.rs
📚 Learning: 2025-09-02T08:46:04.937Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 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.
Applied to files:
src/lotus_json/actors/params/paych_params.rssrc/lotus_json/actors/params/multisig_params.rssrc/lotus_json/actors/params/eam_params.rs
📚 Learning: 2025-09-02T08:44:08.346Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 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.
Applied to files:
src/lotus_json/actors/params/paych_params.rs
🧬 Code graph analysis (7)
src/lotus_json/actors/params/verified_reg_params.rs (2)
src/lotus_json/mod.rs (18)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)snapshots(636-638)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)from_lotus_json(149-149)from_lotus_json(587-589)from_lotus_json(601-606)from_lotus_json(622-628)from_lotus_json(647-654)src/lotus_json/actors/params/miner_params.rs (19)
snapshots(844-858)snapshots(1229-1262)snapshots(1362-1373)snapshots(1501-1514)snapshots(1643-1656)into_lotus_json(860-866)into_lotus_json(1264-1283)into_lotus_json(1375-1380)into_lotus_json(1516-1522)into_lotus_json(1658-1664)into_lotus_json(1825-1833)into_lotus_json(1902-1909)from_lotus_json(868-874)from_lotus_json(1285-1307)from_lotus_json(1382-1387)from_lotus_json(1524-1530)from_lotus_json(1666-1672)from_lotus_json(1835-1840)from_lotus_json(1911-1915)
src/lotus_json/actors/params/evm_params.rs (1)
src/lotus_json/mod.rs (18)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)snapshots(636-638)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)from_lotus_json(149-149)from_lotus_json(587-589)from_lotus_json(601-606)from_lotus_json(622-628)from_lotus_json(647-654)
src/lotus_json/actors/params/reward_params.rs (1)
src/lotus_json/mod.rs (17)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)from_lotus_json(149-149)from_lotus_json(587-589)from_lotus_json(601-606)from_lotus_json(622-628)from_lotus_json(647-654)
src/lotus_json/actors/params/cron_params.rs (2)
src/lotus_json/mod.rs (15)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)assert_all_snapshots(286-296)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)from_lotus_json(149-149)from_lotus_json(587-589)from_lotus_json(601-606)from_lotus_json(622-628)from_lotus_json(647-654)src/shim/address.rs (1)
new_id(142-144)
src/lotus_json/actors/params/power_params.rs (1)
src/lotus_json/mod.rs (13)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)snapshots(636-638)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)
src/lotus_json/actors/params/multisig_params.rs (2)
src/lotus_json/mod.rs (12)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)src/shim/address.rs (1)
new_id(142-144)
src/lotus_json/actors/params/eam_params.rs (2)
src/lotus_json/actors/params/evm_params.rs (3)
snapshots(161-168)into_lotus_json(170-174)from_lotus_json(176-179)src/lotus_json/mod.rs (17)
snapshots(147-147)snapshots(581-583)snapshots(595-597)snapshots(612-614)assert_all_snapshots(286-296)serde_json(316-316)serde_json(340-340)into_lotus_json(148-148)into_lotus_json(584-586)into_lotus_json(598-600)into_lotus_json(615-621)into_lotus_json(639-646)from_lotus_json(149-149)from_lotus_json(587-589)from_lotus_json(601-606)from_lotus_json(622-628)from_lotus_json(647-654)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: tests-release
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
- GitHub Check: Coverage
🔇 Additional comments (40)
src/lotus_json/actors/params/evm_params.rs (3)
26-67: LGTM! Per-version module pattern correctly implemented for EVM ConstructorParams.The module wrapping with local type alias
T, the#[test] fn snapshots()callingassert_all_snapshots::<T>(), and theHasLotusJsonimplementation are all correctly structured. The snapshot test data and conversion logic look correct.
91-149: LGTM! Consistent pattern applied to DelegateCallParams.The per-version module structure is correctly implemented with appropriate snapshot test data covering all fields (code, input, caller, value).
160-181: LGTM! GetStorageAtParams uses the test_snapshots! macro appropriately.The snapshot data correctly tests a 32-byte storage key, and the
test_snapshots!macro invocation at line 181 follows the established pattern for non-versioned types.src/lotus_json/actors/params/verified_reg_params.rs (7)
269-310: LGTM! ConstructorParams module pattern correctly implemented.The per-version module structure with local type alias
Tand centralized snapshot testing is correctly applied.
312-359: LGTM! VerifierParams implementation is correct.The snapshot test data and conversion logic properly handle the address and allowance fields.
660-752: Verify the Signature field name consistency across V2/V3/V4.The V2 snapshot at lines 684-687 uses
"Signature"but V3/V4 snapshots at lines 777/870 use"VerifierSignature". The struct definitions at lines 61 and 84 show V2 doesn't have the rename while V3/V4 have#[serde(rename = "VerifierSignature")]. This appears correct but should be verified for Lotus API compatibility.
940-1049: LGTM! ClaimAllocationsParams v12+ correctly uses nested SectorAllocationClaims structure.The snapshot correctly tests the nested structure with Sectors containing Claims arrays.
1051-1140: LGTM! ClaimAllocationsParams v11 uses the flat SectorAllocationClaim structure.The v9-v11 flat structure is correctly handled separately from the v12+ nested structure. Based on learnings, the
SectorExpiryfield naming is correct.
1259-1299: LGTM! BytesParams v8 implementation is correctly isolated.The v8-specific BytesParams for UseBytes/RestoreBytes methods is properly handled in its own module.
1383-1399: LGTM! Version ranges for macro invocations are correctly specified.The version ranges align with the API evolution:
- v8/v9 use V2 signature types
- v10/v11 use V3 signature types
- v12+ use V4 signature types
src/lotus_json/actors/params/reward_params.rs (4)
48-91: LGTM! RewardConstructorParams correctly handles optional power field.The snapshot tests appropriately cover both
nulland non-null cases for the optionalpowerfield, and theBigIntDewrapper is correctly used for version-specific serialization.
98-151: LGTM! AwardBlockRewardParams implementation is correct.The conversion logic properly handles Address, TokenAmount, and win_count fields.
158-201: LGTM! UpdateNetworkKPIParams mirrors the ConstructorParams pattern.Both optional BigInt fields (power and curr_realized_power) are handled consistently.
203-207: LGTM! Version ranges correctly specified.ConstructorParams and UpdateNetworkKPIParams are available from v11+, while AwardBlockRewardParams is available from v8+.
src/lotus_json/actors/params/cron_params.rs (2)
10-16: LGTM! Added PartialEq derive for assert_all_snapshots compatibility.The
PartialEqderive is required becauseassert_all_snapshots<T>requires<T as HasLotusJson>::LotusJson: PartialEq + std::fmt::Debug. Based on learnings,Eqis intentionally not derived since the wrappedEntrytypes from external dependencies don't implementEq.
22-89: LGTM! Cron ConstructorParams with version-aware Entry handling.The
from_lotus_jsonimplementation at lines 70-81 correctly handles the case where theEntryvariant might not match the expected version by falling back to field-by-field conversion. This ensures robustness when deserializing entries across versions.src/lotus_json/actors/params/init_params.rs (4)
53-94: LGTM! InitConstructorParams implementation is correct.Simple network_name field mapping is correctly implemented.
100-147: LGTM! InitExecParams correctly handles CID and RawBytes fields.The snapshot test data uses base64-encoded bytes and CID format consistent with Lotus JSON.
153-204: LGTM! Exec4Params field mapping is correct.The
subaddressfield (internal) maps tosub_addressin LotusJson (lines 188, 196), which serializes to"SubAddress"via PascalCase. This maintains Lotus API compatibility as per learnings.
206-208: LGTM! Version ranges are correctly specified.Exec4Params is only available from v10+ (EIP-4844 support), while ConstructorParams and ExecParams are available from v8+.
src/lotus_json/actors/params/paych_params.rs (4)
167-212: LGTM! ConstructorParams implementation is straightforward.Simple from/to address mapping is correctly implemented.
219-334: LGTM! UpdateChannelStateParams V2 correctly handles all SignedVoucher fields.Key Lotus compatibility features are correctly implemented:
- Empty merges serialize as
null(not[]) at lines 286-293, per learningsSecretHashfield rename (line 57) is correct per learnings- Signature uses fvm_shared2 types appropriately
340-455: LGTM! UpdateChannelStateParams V3 follows the same pattern as V2.The implementation correctly uses fvm_shared3 signature types while maintaining the same serialization logic.
461-576: LGTM! UpdateChannelStateParams V4 correctly uses fvm_shared4 types.The implementation is consistent with V2/V3 while using the appropriate fvm_shared4 signature types for v12+ actor versions.
src/lotus_json/actors/params/eam_params.rs (4)
23-68: LGTM! EAMCreateParams implementation is correct.The
initcodefield correctly converts betweenVec<u8>andRawBytes, and the nonce field is passed through directly.
83-128: LGTM! EAMCreate2Params correctly handles the 32-byte salt array.The salt field is a fixed-size
[u8; 32]array that serializes as a JSON array of integers, which is the correct Lotus JSON format.
142-175: LGTM! CreateExternalParams is a simple newtype wrapper.The tuple struct correctly wraps initcode bytes and the LotusJson representation is transparent base64-encoded bytes.
177-179: LGTM! All EAM params correctly start from v10.The EAM (Ethereum Address Manager) actor was introduced in network version 18, which corresponds to actor version 10.
src/lotus_json/actors/params/power_params.rs (4)
142-191: LGTM!The
UpdateClaimedPowerParamsimplementation correctly mapsraw_byte_deltaandquality_adjusted_deltafields between the actor type and LotusJson representation.
194-243: LGTM!The
EnrollCronEventParamsimplementation correctly handles theevent_epochandpayloadfields with proper RawBytes handling.
338-402: LGTM!The manual implementations for
MinerPowerParamsv16 and v17 are correctly structured with proper field mappings. Using explicit modules rather than a macro is appropriate since this param type only exists in v16+.
57-67: Verify whether the snapshot tests for these tuple structs actually pass.
UpdatePledgeTotalParamsLotusJsonandMinerRawPowerParamsLotusJsonare tuple structs that expect to serialize as objects ({"PledgeDelta": ...}and{"Miner": ...}) according to their snapshot tests. However, standard Rust serde behavior for single-field tuple structs is to serialize as just the inner value, not as an object.
UpdatePledgeTotalParamsLotusJsonhas a field-level#[serde(with = "crate::lotus_json")]attribute which applies custom serialization, butMinerRawPowerParamsLotusJsonhas no field-level serde attributes—only the struct-level#[serde(rename_all = "PascalCase")], which should not affect tuple field serialization. The struct-levelrename_allattribute applies only to named fields.The test framework (
assert_one_snapshot) validates that serializing theLotusJsontype produces the expected snapshot JSON. Confirm these tests pass and the serialization produces the expected object format, or refactor these structs to use named fields if object-format JSON is required.src/lotus_json/actors/params/multisig_params.rs (8)
118-174: LGTM!The
ConstructorParamsimplementation correctly maps all fields (signers, num_approvals_threshold, unlock_duration, start_epoch) with proper Address conversions.
176-232: LGTM!The
ProposeParamsimplementation correctly handles the to, value, method, and params fields with appropriate type conversions for Address, TokenAmount, and RawBytes.
234-282: LGTM!The
TxnIDParamsimplementation correctly wraps/unwraps the version-specificTxnIDtype viafil_actor_multisig_state::[<v $version>]::TxnID(...).
284-332: LGTM!The
AddSignerParamsimplementation correctly maps the signer address and increase flag.
334-382: LGTM!The
RemoveSignerParamsimplementation correctly maps the signer address and decrease flag.
384-432: LGTM!The
SwapSignerParamsimplementation correctly maps the from and to addresses.
434-476: LGTM!The
ChangeNumApprovalsThresholdParamsimplementation correctly maps the new_threshold field.
478-530: LGTM!The
LockBalanceParamsimplementation correctly handles start_epoch, unlock_duration, and amount fields with proper TokenAmount conversion.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Part of #6365
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.