-
Notifications
You must be signed in to change notification settings - Fork 182
Clean up StateDecodeParams API changes
#6000
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.
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
Conversation
WalkthroughRefactors lotus_json into an actors/params layout, moves many Lotus JSON adapters and tests under per-actor modules, expands adapter coverage to v17, converts RPC registries from numeric u64 to ActorVersion enum, adds per-actor StateDecodeParams test generators, removes legacy adapter files, and bumps actor dependency versions and CHANGELOG. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant RPC as Forest RPC
participant AR as ActorRegistry
participant MR as MethodRegistry
participant LJ as LotusJson adapters
Client->>RPC: StateDecodeParams(code_cid, method_num, params, tipset)
RPC->>AR: get_actor_details_from_code(code_cid)
AR-->>RPC: (BuiltinActor, ActorVersion)
RPC->>MR: lookup_decoder(BuiltinActor, ActorVersion, method_num)
alt decoder found
RPC->>LJ: deserialize params via HasLotusJson for actor/version
LJ-->>RPC: JSON params
RPC-->>Client: Decoded JSON
else decoder missing / Type::Placeholder
RPC-->>Client: Error - no deserializer registered
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Comment |
a7d1e1e to
7c30481
Compare
7c30481 to
fbbff8b
Compare
fbbff8b to
c96ab46
Compare
…ode-param-cleanup
…ode-param-cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
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/miner_params.rs (1)
3616-3621: from_lotus_json drops require_activation_success (always false)from_lotus_json for ProveCommitSectorsNIParams hard-codes require_activation_success = false, breaking roundtrip and behavior.
Apply this fix:
- require_activation_success: false, + require_activation_success: lotus_json.require_activation_success,
🧹 Nitpick comments (25)
src/rpc/registry/actors/eth_account.rs (1)
24-24: Nit: clarify why V8/V9 are no-opsAdd a brief comment to explain the absence of EthAccount before V10.
Apply this diff:
- ActorVersion::V8 | ActorVersion::V9 => {} + // EthAccount actor was introduced in V10; V8/V9 have no registrations. + ActorVersion::V8 | ActorVersion::V9 => {}src/rpc/registry/actors/datacap.rs (1)
126-142: Exhaustive ActorVersion dispatch looks correct (V8–V17).No wildcard; V8 is a no-op (Datacap absent pre-V9), and newer versions map to the right modules.
A short comment on Line 132 clarifying “Datacap introduced in V9” would help future readers.
src/rpc/registry/methods_reg.rs (4)
11-11: Avoid version-pinned Type import; match Placeholder via BuiltinActor for consistency.Prevents coupling to v11 and keeps the match uniform with other arms.
Apply:
-use fil_actors_shared::v11::runtime::builtins::Type; @@ - Type::Placeholder => {} + BuiltinActor::Placeholder => {}If
BuiltinActor::Placeholderisn’t available in your shim, consider re-exporting it there rather than importing a versionedType.Also applies to: 104-105
66-70: Include code_cid in the “no deserializer registered” error.Improves debugging when multiple bundles or versions are present.
- bail!( - "No deserializer registered for actor type {actor_type:?} ({version}), method {method_num}" - ); + bail!( + "No deserializer registered for actor type {actor_type:?} ({version}), method {method_num}, code_cid {code_cid}" + );
131-134: Drop unnecessary as_str() when encoding base64.Slightly cleaner and avoids a transient &str.
- Ok(serde_json::json!(BASE64_STANDARD.encode(bytes).as_str())) + Ok(serde_json::json!(BASE64_STANDARD.encode(bytes)))
355-359: Remove redundant hard-coded base64 assert in test.You already assert equality to the computed expected_base64.
- assert_eq!(json_value, json!(expected_base64)); - assert_eq!(json_value.as_str().unwrap(), "ghgqRBI0Vng="); + assert_eq!(json_value, json!(expected_base64));src/rpc/registry/actors/reward.rs (1)
9-28: Rename macro to reflect V17 usage for clarity.Macro name “11_to_16” is misleading now that it’s used for V17 too.
-macro_rules! register_reward_version_11_to_16 { +macro_rules! register_reward_version_11_plus { @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v11) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v11) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v12) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v12) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v13) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v13) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v14) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v14) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v15) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v15) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v16) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v16) @@ - register_reward_version_11_to_16!(registry, cid, fil_actor_reward_state::v17) + register_reward_version_11_plus!(registry, cid, fil_actor_reward_state::v17)Also applies to: 71-90
src/rpc/registry/actors/init.rs (2)
9-23: Macro name/comment drift: used for V17 but labeled “10–16”Rename to reflect actual coverage and avoid future drift.
-// Macro for versions 10-16 that have Exec4 -macro_rules! register_init_versions_10_to_16 { +// Macro for versions 10+ that have Exec4 +macro_rules! register_init_versions_10_plus { ($registry:expr, $code_cid:expr, $state_version:path) => {{ use $state_version::{ConstructorParams, Exec4Params, ExecParams, Method}; register_actor_methods!( $registry, $code_cid, [ (Method::Constructor, ConstructorParams), (Method::Exec, ExecParams), (Method::Exec4, Exec4Params) ] ); }}; }And update call sites below accordingly (see next comment).
Also applies to: 25-40
54-77: Apply the new macro name at all call sitesTo keep naming consistent after the rename above.
- register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v10) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v10) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v11) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v11) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v12) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v12) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v13) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v13) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v14) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v14) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v15) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v15) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v16) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v16) - register_init_versions_10_to_16!(registry, cid, fil_actor_init_state::v17) + register_init_versions_10_plus!(registry, cid, fil_actor_init_state::v17)src/rpc/registry/actors/power.rs (1)
110-139: Optional: de-duplicate V16/V17 registrationThe param’ed method sets are identical; consider a single “16+” macro to reduce duplication.
-macro_rules! register_power_version_17 { +// Same as v16; keep one shared macro for 16+ +macro_rules! register_power_version_16_plus { @@ } @@ - ActorVersion::V16 => register_power_version_16!(registry, cid, fil_actor_power_state::v16), - ActorVersion::V17 => register_power_version_17!(registry, cid, fil_actor_power_state::v17), + ActorVersion::V16 => register_power_version_16_plus!(registry, cid, fil_actor_power_state::v16), + ActorVersion::V17 => register_power_version_16_plus!(registry, cid, fil_actor_power_state::v17),src/rpc/registry/actors/account.rs (3)
28-35: Confirm v9 UniversalReceiverHook on Account actor.Is
Method::UniversalReceiverHookactually exposed by the v9 Account actor? If not, this registration will surface a non-existent method.If verification shows it’s absent, drop it:
register_actor_methods!( registry, cid, [ (Method::PubkeyAddress, empty), - (Method::UniversalReceiverHook, empty) ] );
57-61: Doc nit: include v17 in the “with_types” comment.The macro is used for V15–V17; the comment mentions only v15, v16.
- // For versions that use types module (v15, v16) + // For versions that use types module (v15–v17)
10-55: Minor duplication across v8/v9/v10 helpers.You can reduce repetition by introducing a small helper/macro for {Constructor(Address), PubkeyAddress} and param-only deltas.
src/rpc/registry/actors_reg.rs (2)
30-37: Unknown major versions are silently skipped. Add tracing.If bundles contain a newer major version, entries are dropped without visibility. Emit a warn/debug to aid diagnostics.
- if let Ok(version_u64) = metadata.actor_major_version() { - if let Some(version) = ActorVersion::from_repr(version_u64 as u8) { + if let Ok(version_u64) = metadata.actor_major_version() { + if let Some(version) = ActorVersion::from_repr(version_u64 as u8) { for (actor_type, cid) in metadata.manifest.builtin_actors() { map.insert(cid, (actor_type, version)); } - } + } else { + tracing::warn!("Skipping actors with unknown major version: {version_u64}"); + } }
70-79: Consider returning version from load path if needed later.You ignore the resolved ActorVersion here; if future state decoding needs version-specific handling, thread it through now.
src/lotus_json/actors/params/account_params.rs (4)
6-6: Wrong Serialize import; prefer serde or drop it.
#[derive(Serialize)]targetsserde::Serialize. Thejsonrpsee::core::Serializeimport is misleading.-use jsonrpsee::core::Serialize; +use serde::Serialize;Or remove the line entirely.
11-17: Schema parity: add schemars(transparent).To mirror
serde(transparent)in the OpenAPI/JSON Schema, add#[schemars(transparent)].#[derive(Serialize, Deserialize, JsonSchema, Debug, Clone, PartialEq)] #[serde(transparent)] +#[schemars(transparent)] pub struct AccountConstructorParamsLotusJson {
41-44: Fully qualify json! macro in snapshots.
json!isn’t imported in this module; useserde_json::json!to avoid scope surprises.- json!({ + serde_json::json!({- json!("f01234"), + serde_json::json!("f01234"),Also applies to: 83-83
30-69: Snapshot defaults: null vs base64 for Vec.LotusJson for
Vec<u8>typically encodes bytes as base64 strings. Usingnullmay not reflect real RPC payloads and could hide regressions.Consider a non-empty example (e.g.,
"SGVsbG8=") for both fields to match production encoding.src/rpc/registry/actors/payment_channel.rs (1)
9-11: Update comment to match supported versionsThe code handles V8–V17; the comment says V8–V16.
-// Payment channel methods are consistent across all versions V8-V16 +// Payment channel methods are consistent across all versions V8–V17src/lotus_json/actors/params/power_params.rs (1)
317-339: Deduplicate v16/v17MinerPowerParamsimplsThe two impl blocks are identical; consider a small macro to reduce duplication and keep future bumps trivial.
Example:
macro_rules! impl_lotus_json_for_power_miner_power_params { ($($ver:literal),+ $(,)?) => {$( paste! { impl HasLotusJson for fil_actor_power_state::[<v $ver>]::MinerPowerParams { type LotusJson = MinerPowerParamsLotusJson; #[cfg(test)] fn snapshots() -> Vec<(serde_json::Value, Self)> { vec![(json!({"Miner": 1002}), Self { miner: 1002 })] } fn into_lotus_json(self) -> Self::LotusJson { MinerPowerParamsLotusJson { miner: self.miner } } fn from_lotus_json(lotus_json: Self::LotusJson) -> Self { Self { miner: lotus_json.miner } } } } )+} }Then:
impl_lotus_json_for_power_miner_power_params!(16, 17);src/rpc/registry/actors/miner.rs (2)
335-349: Minor naming nit: singular “version”Function name reads “versions_14” but registers a single version; consider renaming for consistency with others.
-fn register_miner_versions_14(registry: &mut MethodRegistry, cid: Cid) { +fn register_miner_version_14(registry: &mut MethodRegistry, cid: Cid) {And update the call site in the match.
4-4: Potential unused import
MethodNumisn’t referenced in this module; if not needed by macros, consider removing to keep imports lean.src/lotus_json/actors/params/init_params.rs (1)
7-11: Trim unused/incorrect imports
jsonrpsee::core::Serialize,serde::Deserialize, andstd::fmt::Debugaren’t needed for derives and can trigger unused import warnings.-use jsonrpsee::core::Serialize; @@ -use serde::Deserialize; -use std::fmt::Debug;src/lotus_json/actors/params/miner_params.rs (1)
3713-3715: Macro name vs. usage mismatch (includes v8 in a v9+ macro)impl_lotus_json_for_miner_terminate_sectors_params_v9_and_above! is invoked with 8 as well. If v8 really shares the same struct semantics, consider renaming the macro; otherwise, drop v8 here and keep a dedicated v8 impl.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
CHANGELOG.md(1 hunks)Cargo.toml(1 hunks)src/lotus_json/actor_states/methods/account_constructor_params.rs(0 hunks)src/lotus_json/actor_states/methods/init_constructor_params.rs(0 hunks)src/lotus_json/actor_states/methods/init_exec4_params.rs(0 hunks)src/lotus_json/actor_states/methods/init_exec_params.rs(0 hunks)src/lotus_json/actor_states/methods/mod.rs(0 hunks)src/lotus_json/actors/mod.rs(1 hunks)src/lotus_json/actors/params/account_params.rs(2 hunks)src/lotus_json/actors/params/cron_params.rs(1 hunks)src/lotus_json/actors/params/datacap_params.rs(4 hunks)src/lotus_json/actors/params/eam_params.rs(1 hunks)src/lotus_json/actors/params/evm_params.rs(2 hunks)src/lotus_json/actors/params/init_params.rs(1 hunks)src/lotus_json/actors/params/miner_params.rs(1 hunks)src/lotus_json/actors/params/mod.rs(1 hunks)src/lotus_json/actors/params/multisig_params.rs(1 hunks)src/lotus_json/actors/params/paych_params.rs(1 hunks)src/lotus_json/actors/params/power_params.rs(2 hunks)src/lotus_json/actors/params/reward_params.rs(1 hunks)src/lotus_json/actors/params/verified_reg_params.rs(1 hunks)src/lotus_json/actors/states/mod.rs(1 hunks)src/lotus_json/mod.rs(1 hunks)src/rpc/registry/actors/account.rs(3 hunks)src/rpc/registry/actors/cron.rs(2 hunks)src/rpc/registry/actors/datacap.rs(2 hunks)src/rpc/registry/actors/eam.rs(2 hunks)src/rpc/registry/actors/eth_account.rs(2 hunks)src/rpc/registry/actors/evm.rs(2 hunks)src/rpc/registry/actors/init.rs(3 hunks)src/rpc/registry/actors/market.rs(2 hunks)src/rpc/registry/actors/miner.rs(14 hunks)src/rpc/registry/actors/multisig.rs(2 hunks)src/rpc/registry/actors/payment_channel.rs(2 hunks)src/rpc/registry/actors/power.rs(2 hunks)src/rpc/registry/actors/reward.rs(2 hunks)src/rpc/registry/actors/system.rs(2 hunks)src/rpc/registry/actors/verified_reg.rs(2 hunks)src/rpc/registry/actors_reg.rs(1 hunks)src/rpc/registry/methods_reg.rs(8 hunks)
💤 Files with no reviewable changes (5)
- src/lotus_json/actor_states/methods/init_exec_params.rs
- src/lotus_json/actor_states/methods/mod.rs
- src/lotus_json/actor_states/methods/init_exec4_params.rs
- src/lotus_json/actor_states/methods/init_constructor_params.rs
- src/lotus_json/actor_states/methods/account_constructor_params.rs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-02T10:05:34.350Z
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.
Applied to files:
src/rpc/registry/actors/eth_account.rssrc/rpc/registry/actors/power.rssrc/rpc/registry/actors/multisig.rssrc/rpc/registry/actors/evm.rssrc/rpc/registry/actors/account.rssrc/lotus_json/actors/params/power_params.rssrc/rpc/registry/actors/miner.rssrc/lotus_json/actors/params/miner_params.rs
📚 Learning: 2025-08-18T12:25:29.183Z
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.
Applied to files:
src/lotus_json/mod.rs
📚 Learning: 2025-08-14T06:46:08.056Z
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.
Applied to files:
src/rpc/registry/actors/miner.rs
🧬 Code graph analysis (18)
src/lotus_json/actors/mod.rs (1)
src/shim/actors/builtin/market/mod.rs (1)
states(165-218)
src/rpc/registry/actors/cron.rs (3)
src/rpc/registry/actors/eam.rs (1)
register_actor_methods(28-46)src/rpc/registry/actors/power.rs (1)
register_actor_methods(141-174)src/rpc/registry/actors/system.rs (1)
register_actor_methods(23-40)
src/rpc/registry/actors/eam.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eth_account.rs (1)
register_actor_methods(18-50)
src/rpc/registry/methods_reg.rs (1)
src/rpc/registry/actors_reg.rs (1)
get_real_actor_cid(138-144)
src/rpc/registry/actors/eth_account.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eam.rs (1)
register_actor_methods(28-46)
src/rpc/registry/actors/reward.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/power.rs (1)
register_actor_methods(141-174)
src/rpc/registry/actors/system.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/power.rs (1)
register_actor_methods(141-174)
src/rpc/registry/actors/init.rs (8)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eam.rs (1)
register_actor_methods(28-46)src/rpc/registry/actors/eth_account.rs (1)
register_actor_methods(18-50)src/rpc/registry/actors/multisig.rs (1)
register_actor_methods(76-109)src/rpc/registry/actors/power.rs (1)
register_actor_methods(141-174)src/rpc/registry/actors/reward.rs (1)
register_actor_methods(50-92)src/rpc/registry/actors/system.rs (1)
register_actor_methods(23-40)src/rpc/registry/actors/verified_reg.rs (1)
register_actor_methods(239-268)
src/rpc/registry/actors/verified_reg.rs (3)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eam.rs (1)
register_actor_methods(28-46)src/rpc/registry/actors/init.rs (1)
register_actor_methods(42-79)
src/rpc/registry/actors/power.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/init.rs (1)
register_actor_methods(42-79)
src/lotus_json/actors/params/init_params.rs (1)
src/lotus_json/actors/params/datacap_params.rs (24)
snapshots(218-231)snapshots(271-286)snapshots(322-333)snapshots(365-376)snapshots(405-414)snapshots(441-450)snapshots(480-491)snapshots(523-534)into_lotus_json(233-239)into_lotus_json(288-295)into_lotus_json(335-340)into_lotus_json(378-383)into_lotus_json(416-420)into_lotus_json(452-456)into_lotus_json(493-498)into_lotus_json(536-541)from_lotus_json(241-247)from_lotus_json(297-304)from_lotus_json(342-347)from_lotus_json(385-390)from_lotus_json(422-426)from_lotus_json(458-462)from_lotus_json(500-505)from_lotus_json(543-548)
src/rpc/registry/actors/multisig.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/init.rs (1)
register_actor_methods(42-79)
src/rpc/registry/actors/evm.rs (1)
src/rpc/methods/state.rs (16)
fil_actors_shared(2098-2101)fil_actors_shared(2110-2113)fil_actors_shared(2122-2125)fil_actors_shared(2134-2137)fil_actors_shared(2146-2149)fil_actors_shared(2158-2161)fil_actors_shared(2240-2243)fil_actors_shared(2252-2255)fil_actors_shared(2264-2267)fil_actors_shared(2276-2279)fil_actors_shared(2288-2291)fil_actors_shared(2300-2303)fil_actors_shared(2727-2727)fil_actors_shared(2735-2735)fil_actors_shared(2742-2742)fil_actors_shared(2752-2752)
src/rpc/registry/actors/account.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eth_account.rs (1)
register_actor_methods(18-50)
src/lotus_json/actors/params/power_params.rs (3)
src/lotus_json/actors/params/datacap_params.rs (8)
snapshots(218-231)snapshots(271-286)snapshots(322-333)snapshots(365-376)snapshots(405-414)snapshots(441-450)snapshots(480-491)snapshots(523-534)src/lotus_json/actors/params/evm_params.rs (1)
snapshots(139-146)src/lotus_json/actors/params/miner_params.rs (7)
snapshots(797-811)snapshots(1152-1186)snapshots(1279-1290)snapshots(1403-1417)snapshots(1532-1546)snapshots(1679-1691)snapshots(1752-1762)
src/rpc/registry/actors/payment_channel.rs (1)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)
src/rpc/registry/actors/miner.rs (2)
src/rpc/registry/actors/cron.rs (1)
register_actor_methods(24-41)src/rpc/registry/actors/eam.rs (1)
register_actor_methods(28-46)
src/lotus_json/actors/params/account_params.rs (3)
src/shim/address.rs (2)
with(72-77)new_id(142-144)src/lotus_json/actors/params/datacap_params.rs (8)
snapshots(218-231)snapshots(271-286)snapshots(322-333)snapshots(365-376)snapshots(405-414)snapshots(441-450)snapshots(480-491)snapshots(523-534)src/lotus_json/actor_states/methods/account_constructor_params.rs (1)
AccountConstructorParamsLotusJson(10-14)
⏰ 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: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: All lint checks
🔇 Additional comments (40)
src/lotus_json/actors/params/evm_params.rs (1)
64-64: v17 coverage for EVM params — LGTMBoth macro invocations correctly extend support through version 17 in line with the crate bumps. No further issues spotted here.
Also applies to: 127-127
src/lotus_json/actors/states/mod.rs (1)
3-3: Minor formatting change — OKNo functional impact.
Cargo.toml (1)
71-86: Actor state crates bumped to 24.1.2 — LGTMVersions are in lockstep across all fil_actor_* crates and fil_actors_shared, matching the added V17 support in this PR.
src/lotus_json/actors/mod.rs (1)
4-6: Module split into params/states — LGTMPrivate submodules align with the new actors namespace. No issues.
CHANGELOG.md (1)
33-33: Changelog entry added — LGTMEntry is concise and correctly references PR #6000.
src/lotus_json/actors/params/cron_params.rs (1)
82-82: Add Cron constructor LotusJson for V17 — LGTMMacro arity extension is correct; snapshots and conversions remain consistent.
src/lotus_json/actors/params/multisig_params.rs (1)
460-467: Extend multisig params to V17 — LGTMAll multisig param macros consistently include 17. No API surface change.
src/rpc/registry/actors/market.rs (1)
8-8: Switch to ActorVersion enum with exhaustive mapping — solid improvementType-safe dispatch without a catch-all is preferable. Mappings for V8–V17 route to the correct registration macros.
Also applies to: 178-214
src/lotus_json/actors/params/eam_params.rs (1)
150-152: V17 EAM params coverage — LGTMExtending all three EAM param impl macros to include 17 is consistent with the registry changes and keeps conversions symmetric.
src/rpc/registry/actors/eth_account.rs (1)
18-49: Type-safe version dispatch with ActorVersion + V17 — LGTMSignature change and exhaustive matching per ActorVersion look correct; registrations for V10–V17 are wired to the right state modules.
src/lotus_json/actors/params/paych_params.rs (2)
543-543: Constructor params now include V17 — LGTMMacro arity update aligns constructor Lotus JSON bridging with supported versions.
546-546: UpdateChannelState v4 extended to V17 — LGTMExtending the v4 adapter through V17 matches the fvm_shared4 signature usage in this file and other actors in the PR.
src/rpc/registry/actors/datacap.rs (2)
87-124: Generic macro for v11+ is tidy and consistent.Shared FRC46 param types + BalanceParams/ConstructorParams align with exported methods.
50-50: Ignore v10 ConstructorParams suggestion; Address is correct
The v10 Datacap actor’s Constructor parameter type is Address, so the existing import and mapping are already correct.Likely an incorrect or invalid review comment.
src/rpc/registry/actors/cron.rs (1)
24-40: Enum-based versioning + V17 support: LGTM.Clean switch to ActorVersion with exhaustive match and unchanged method surface via macro.
src/rpc/registry/actors/system.rs (1)
23-39: System actor registry update is correct.Constructor mapped to empty across V8–V17 via macro; exhaustive match is good.
src/rpc/registry/actors/eam.rs (1)
28-45: Enum-based version dispatch for EAM looks correct (V10–V17) with explicit V8/V9 no-ops.Consistent with other actor modules; constructor and params registration via the macro is clear and maintainable.
src/rpc/registry/actors/reward.rs (1)
50-91: LGTM: ActorVersion dispatch (V8–V17) and method registration are correct.Reuse of the 11–16 macro for V17 is fine given identical signatures.
src/rpc/registry/actors/init.rs (2)
7-7: LGTM: switch to ActorVersionImport and enum-based dispatch align with the repo-wide migration.
42-78: Exhaustive match over ActorVersion: goodExplicit arms V8–V17 with no fallback improves safety.
src/rpc/registry/actors/verified_reg.rs (2)
8-8: LGTM: ActorVersion importConsistent with other actor registries.
239-267: LGTM: enum-based dispatcherPer-version registrars are clear and exhaustive for V8–V17.
src/rpc/registry/actors/power.rs (3)
7-7: LGTM: ActorVersion importMatches the project-wide API change.
141-173: LGTM: exhaustive matchVersion routing mirrors other actors and includes V17.
101-108: ```shell
#!/bin/bashClone the Filecoin builtin-actors repo for version analysis
if [ ! -d builtin-actors ]; then
git clone https://github.com/filecoin-project/builtin-actors.git
fi
cd builtin-actorsList tags for v10 to v17
tags=$(git tag -l | grep -E "^v(10|11|12|13|14|15|16|17)(\.|$)" | sort -V)
echo "Analyzing tags: $tags"
for tag in $tags; do
echo "=== $tag ==="
git checkout -f $tag
echo "- NetworkRawPowerExported:"
rg -n "NetworkRawPowerExported" -g "actors/power/src/" || echo " (not present)"
echo "- MinerCountExported:"
rg -n "MinerCountExported" -g "actors/power/src/" || echo " (not present)"
echo "- MinerConsensusCountExported:"
rg -n "MinerConsensusCountExported" -g "actors/power/src/**" || echo " (not present)"
echo
done</blockquote></details> <details> <summary>src/rpc/registry/actors/account.rs (1)</summary><blockquote> `77-95`: **Double-check “AuthenticateMessageExported” for v11–v14.** Some versions might still expose `AuthenticateMessage` (not `...Exported`). Please confirm against fil_actor_account_state to avoid mismatched method numbers. </blockquote></details> <details> <summary>src/rpc/registry/actors/payment_channel.rs (1)</summary><blockquote> `33-69`: **Enum-based dispatch across V8–V17 looks solid** Explicit per-variant registration is clear and consistent with other actors. </blockquote></details> <details> <summary>src/lotus_json/actors/params/power_params.rs (1)</summary><blockquote> `341-345`: **v17 coverage for power params macros — LGTM** Macro expansions now include v17 consistently. </blockquote></details> <details> <summary>src/rpc/registry/actors/miner.rs (2)</summary><blockquote> `136-151`: **v16+ common methods block — LGTM** Separate macro for v16 onwards keeps new exported calls tidy. --- `374-386`: **v16 block — LGTM** Correctly registers ExtendSectorExpiration/ProveCommitAggregate/ProveReplicaUpdates for v16. </blockquote></details> <details> <summary>src/lotus_json/actors/params/init_params.rs (2)</summary><blockquote> `98-106`: **Sanity-check default CID snapshot** The snapshot uses `"/": "baeaaaaa"` while the struct uses `Cid::default()`. Please confirm that your `LotusJson<Cid>` encoder renders `Cid::default()` as `"baeaaaaa"`; otherwise tests will fail. Run to audit usage consistency across the repo: ```shell #!/bin/bash # Check how often "CodeCid" vs "CodeCID" appears in Lotus JSON snapshots/structs. rg -nP --type=rs -C2 '"CodeCid"|CodeCID|Subaddress|SubAddress' src/lotus_json # Optional: list places where Cid::default() is used in snapshots to compare expected string form. rg -nP --type=rs -C2 'Cid::default\(\).*json!' src | sed -n '1,200p'Also applies to: 142-151
21-30: Fix JSON field names to match Lotus conventions ("CodeCid", "Subaddress")The struct-level serde config would yield "CodeCid" and "Subaddress". Overriding to "CodeCID" and using "sub_address" will break round-trips and the snapshots. Align the field names with Lotus and your snapshots.
Apply:
@@ pub struct InitExecParamsLotusJson { #[schemars(with = "LotusJson<Cid>")] #[serde(with = "crate::lotus_json")] - #[serde(rename = "CodeCID")] pub code_cid: Cid, @@ pub struct InitExec4ParamsLotusJson { #[schemars(with = "LotusJson<Cid>")] #[serde(with = "crate::lotus_json")] - #[serde(rename = "CodeCID")] pub code_cid: Cid, @@ - #[schemars(with = "LotusJson<RawBytes>")] - #[serde(with = "crate::lotus_json")] - pub sub_address: RawBytes, + #[schemars(with = "LotusJson<RawBytes>")] + #[serde(with = "crate::lotus_json")] + pub subaddress: RawBytes, }Also applies to: 32-47
⛔ Skipped due to learnings
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.src/rpc/registry/actors/multisig.rs (1)
76-108: Enum-based dispatch LGTMClean switch to ActorVersion with exhaustive matching. V8 correctly excludes UniversalReceiverHook; V9+ includes it via the dedicated macro. Looks consistent with other actor registries.
src/lotus_json/actors/params/datacap_params.rs (1)
51-51: v17 support added correctlyExtending the macro invocations to include v17 for balance, constructor, destroy, and mint params is consistent with the rest of the PR. No signature mismatches apparent.
Also applies to: 88-88, 138-138, 198-198
src/lotus_json/actors/params/reward_params.rs (1)
176-176: Reward param adapters extended to v17 — looks goodConstructor, AwardBlockReward, and UpdateNetworkKPI now cover 17; types remain aligned with fvm_shared4 bigint paths.
Also applies to: 178-180
src/lotus_json/actors/params/verified_reg_params.rs (1)
1239-1255: Verified-reg v17 extensions are coherentAll affected macros now include 17, matching the cross-repo expansion. No structural changes beyond widening the sets.
src/lotus_json/actors/params/miner_params.rs (1)
3741-3741: ExtendSectorExpirationParams missing v17Versions 9–16 are covered; v17 should be included for parity with other miner params.
-impl_miner_extend_sector_expiration_params_v9_onwards!(9, 10, 11, 12, 13, 14, 15, 16); +impl_miner_extend_sector_expiration_params_v9_onwards!(9, 10, 11, 12, 13, 14, 15, 16, 17);⛔ Skipped due to learnings
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/lotus_json/mod.rs (1)
221-221: Module rename aligns with new layoutSwitching to mod actors; matches the folder reorg to actors/{params,states}. No issues.
src/lotus_json/actors/params/mod.rs (1)
4-17: New params module index looks goodPrivate submodules keep surface area tight and align with the new actors/* organization.
src/rpc/registry/actors/evm.rs (1)
43-58: ActorVersion-based EVM registration LGTMEnum match is exhaustive; V8/V9 no-op and V10–V17 wired through register_evm_version! consistently.
sudo-shashank
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d7a4cb2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/tool/subcommands/api_cmd/state_decode_params_tests/init.rs (1)
24-43: Good: replaced magic method numbers with typedMethodconstants.This aligns with the PR objective and improves readability.
🧹 Nitpick comments (34)
src/state_migration/nv27/mod.rs (1)
15-15: Add trailing comma for cleaner diffs and rustfmt friendlinessMinor style nit; keeps future edits minimal.
- fil_actor_system_state::v17::State + fil_actor_system_state::v17::State,src/tool/subcommands/api_cmd/state_decode_params_tests/power.rs (4)
10-16: Consider more realistic peer and multiaddrs to exercise decoding paths.
peer: b"miner".to_vec()and emptymultiaddrsdecode fine but don’t stress non-trivial inputs. A sample PeerId (bytes) and at least one multiaddr would provide better coverage.
28-31: Use a non-empty payload for EnrollCronEvent to cover non-zero RawBytes.An empty payload is valid but misses the non-empty decode path. Consider a small byte payload to increase coverage.
39-120: Reduce repetition: cache tipset key and/or add a tiny helper for request construction.You repeat
tipset.key().into()and theStateDecodeParams::requestwrapper many times. Caching the TSK and/or a small helper cuts noise and future churn.Example minimal change:
pub fn create_tests(tipset: &Tipset) -> Result<Vec<RpcTest>> { + let tsk = tipset.key().to_owned().into(); … - Ok(vec![ - RpcTest::identity(StateDecodeParams::request(( - Address::POWER_ACTOR, Method::CreateMiner as u64, to_vec(&power_create_miner_params)?, tipset.key().into(), - ))?), + Ok(vec![ + RpcTest::identity(StateDecodeParams::request(( + Address::POWER_ACTOR, Method::CreateMiner as u64, to_vec(&power_create_miner_params)?, tsk.clone(), + ))?), … ]) }Or introduce a local
req(method, params)closure to DRY it up.
76-83: Keep TODOs synced with upstream status.The Lotus/go-state-types note is helpful. Consider linking a short comment on when to re-enable the test (e.g., feature flag or version check) to avoid permanent dead code.
src/tool/subcommands/api_cmd/state_decode_params_tests/init.rs (4)
5-6: Prefer explicit imports to avoid wildcard pollution.Make the imported items clear and reduce namespace collisions.
-use fil_actor_init_state::v17::*; +use fil_actor_init_state::v17::{ConstructorParams, Exec4Params, ExecParams, Method};
13-17: Use a realistic 20-byte subaddress for Exec4.Some decoders validate subaddress length; 3 bytes may be rejected.
let init_exec4_params = Exec4Params { code_cid: Cid::default(), constructor_params: fvm_ipld_encoding::RawBytes::new(vec![0x12, 0x34, 0x56]), // dummy bytecode - subaddress: fvm_ipld_encoding::RawBytes::new(vec![0x12, 0x34, 0x56]), // dummy bytecode + subaddress: fvm_ipld_encoding::RawBytes::new(vec![0u8; 20]), // dummy 20-byte subaddress };If
Exec4Params::subaddressisfvm_shared::address::Subaddressin v17, switch to that type accordingly.
13-22: DRY: factor out repeated dummy constructor params.Reduces duplication and keeps tests tidy.
- let init_exec4_params = Exec4Params { + let dummy_constructor = fvm_ipld_encoding::RawBytes::new(vec![0x12, 0x34, 0x56]); + let init_exec4_params = Exec4Params { code_cid: Cid::default(), - constructor_params: fvm_ipld_encoding::RawBytes::new(vec![0x12, 0x34, 0x56]), // dummy bytecode + constructor_params: dummy_constructor.clone(), // dummy bytecode subaddress: fvm_ipld_encoding::RawBytes::new(vec![0u8; 20]), // dummy bytecode }; - let init_exec_params = ExecParams { + let init_exec_params = ExecParams { code_cid: Cid::default(), - constructor_params: fvm_ipld_encoding::RawBytes::new(vec![0x12, 0x34, 0x56]), // dummy bytecode + constructor_params: dummy_constructor, // dummy bytecode };
7-8: Clarify doc to note version coverage.Tiny doc tweak to reflect v17 focus and methods under test.
-/// Creates state decode params tests for the Init actor. +/// Creates state decode params tests for the Init actor (v17): Constructor, Exec, Exec4.src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs (1)
5-5: Narrow the import to what’s used.Prefer an explicit import to avoid pulling the whole module into scope.
Apply this diff:
-use fil_actor_ethaccount_state::v17::*; +use fil_actor_ethaccount_state::v17::Method;src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (5)
115-117: Avoid.unwrap()on Base64 decode; propagate errors instead.Use
?to keep the function infallible-free and consistent with nearby?uses.@@ - Ok(vec![ + let urh_payload = BASE64_STANDARD.decode("ghgqRBI0Vng=")?; + Ok(vec![ @@ - Method::UniversalReceiverHook as u64, - BASE64_STANDARD.decode("ghgqRBI0Vng=").unwrap(), + Method::UniversalReceiverHook as u64, + urh_payload,
101-112: Duplicate LockBalance test entry — intentional?Two identical LockBalance cases add no signal and slow the suite. If unintentional, remove one.
@@ RpcTest::identity(StateDecodeParams::request(( Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, Method::LockBalance as u64, to_vec(&multisig_lock_bal_params)?, tipset.key().into(), ))?), - RpcTest::identity(StateDecodeParams::request(( - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, - Method::LockBalance as u64, - to_vec(&multisig_lock_bal_params)?, - tipset.key().into(), - ))?),
17-22: Replace magic number0inProposeParams.methodwith theMETHOD_SENDconstant.This aligns with the PR goal to avoid magic method numbers.
@@ -use base64::{Engine, prelude::BASE64_STANDARD}; +use base64::{Engine, prelude::BASE64_STANDARD}; +use fvm_shared::METHOD_SEND; @@ - method: 0, + method: METHOD_SEND,
52-118: DRY up repeated target address and tipset key.Reduce repetition and chance of copy/paste drift by factoring these once.
@@ - Ok(vec![ + let target = Address::new_id(18101); + let tsk = tipset.key().into(); + Ok(vec![ @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), // https://calibration.filscan.io/en/address/t018101/ @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(), @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + target.clone(), @@ - tipset.key().into(), + tsk.clone(),
24-27: Confirm intended semantics ofproposal_hash.You set
proposal_hashto a single zero byte. If an “empty/none” hash was intended, considerVec::new()instead. If a non-empty placeholder is required, add a brief comment.src/tool/subcommands/api_cmd/state_decode_params_tests/market.rs (4)
81-83: Use a single Address type for params (avoid unnecessary conversions).WithdrawBalanceParams already expects an fvm_shared4 Address; using the shim Address then
.into()adds noise. Match the style used for AddBalanceParams above.- let market_actor_withdraw_balance_params = WithdrawBalanceParams { - provider_or_client: Address::new_id(1000).into(), + let market_actor_withdraw_balance_params = WithdrawBalanceParams { + provider_or_client: fvm_shared4::address::Address::new_id(1000), amount: TokenAmount::default().into(), };
107-107: Prefer fvm_shared4 Address for exported call params as well.Keeps Address usage consistent across all params and mirrors other initializations in this file.
- let market_actor_get_balance_exported_params = Address::new_id(1000); + let market_actor_get_balance_exported_params = fvm_shared4::address::Address::new_id(1000);
38-45: Use distinct client and provider IDs for clearer test intent.Having both set to the same ID is harmless for decode, but using different values avoids accidental conflation in future assertions or logs.
- fvm_shared4::address::Address::new_id(1000), + fvm_shared4::address::Address::new_id(2000),
48-51: Use a 96-byte BLS signature to avoid potential length checks.Some decoders or future validations may enforce BLS length. A 96-byte placeholder keeps the test resilient.
- client_signature: fvm_shared4::crypto::signature::Signature::new_bls( - b"test_signature".to_vec(), - ), + client_signature: fvm_shared4::crypto::signature::Signature::new_bls(vec![0u8; 96]),src/tool/subcommands/api_cmd/state_decode_params_tests/datacap.rs (4)
74-171: Use the canonical Datacap actor address constant and DRY it up."Address::DATACAP_TOKEN_ACTOR" may not exist (in fvm_shared constants it's typically DATACAP_TOKEN_ACTOR_ADDR). Define a single local constant and reuse it to avoid future renames and reduce repetition. Please verify which constant your codebase exports and adjust accordingly.
@@ -use super::*; -use fil_actor_datacap_state::v17::*; +use super::*; +use fil_actor_datacap_state::v17::*; +use fvm_shared::address::DATACAP_TOKEN_ACTOR_ADDR; + +// Single source of truth for the Datacap actor address used below. +const DATACAP: Address = DATACAP_TOKEN_ACTOR_ADDR; @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::Constructor as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::MintExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::DestroyExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::NameExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::SymbolExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::TotalSupplyExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::BalanceExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::GranularityExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::TransferExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::TransferFromExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::IncreaseAllowanceExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::DecreaseAllowanceExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::RevokeAllowanceExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::BurnExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::BurnFromExported as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::DATACAP_TOKEN_ACTOR, + RpcTest::identity(StateDecodeParams::request(( + DATACAP, Method::AllowanceExported as u64,
5-6: Prefer explicit imports over wildcard.Using v17::* and super::* makes API drift harder to spot during upgrades. Import only the items used (ConstructorParams, MintParams, DestroyParams, BalanceParams, Method, TokenAmount, Address, StateDecodeParams, RpcTest, Tipset, to_vec).
9-27: Consider non-zero TokenAmounts for better coverage.Zero amounts serialize fine, but using small non-zero values can exercise more code paths in encoders/decoders across implementations.
9-72: Unify type sources where possible.You mix v17-specific params (Constructor, Mint, Destroy, Balance) with shared FRC-46 types elsewhere. If shared equivalents exist for these, prefer them to reduce version-coupling. Keep v17-only types where no shared equivalent exists.
src/tool/subcommands/api_cmd/state_decode_params_tests/verified_reg.rs (4)
96-193: Qualify Method to avoid shadowing and make the actor/version explicitUsing a bare
Method::...after a glob import risks name clashes and reduces clarity across versions. Qualify it with the crate path.- Method::Constructor as u64, + fil_actor_verifreg_state::v17::Method::Constructor as u64, @@ - Method::AddVerifier as u64, + fil_actor_verifreg_state::v17::Method::AddVerifier as u64, @@ - Method::RemoveVerifier as u64, + fil_actor_verifreg_state::v17::Method::RemoveVerifier as u64, @@ - Method::AddVerifiedClient as u64, + fil_actor_verifreg_state::v17::Method::AddVerifiedClient as u64, @@ - Method::RemoveVerifiedClientDataCap as u64, + fil_actor_verifreg_state::v17::Method::RemoveVerifiedClientDataCap as u64, @@ - Method::RemoveExpiredAllocations as u64, + fil_actor_verifreg_state::v17::Method::RemoveExpiredAllocations as u64, @@ - Method::ClaimAllocations as u64, + fil_actor_verifreg_state::v17::Method::ClaimAllocations as u64, @@ - Method::GetClaims as u64, + fil_actor_verifreg_state::v17::Method::GetClaims as u64, @@ - Method::ExtendClaimTerms as u64, + fil_actor_verifreg_state::v17::Method::ExtendClaimTerms as u64, @@ - Method::RemoveExpiredClaims as u64, + fil_actor_verifreg_state::v17::Method::RemoveExpiredClaims as u64, @@ - Method::AddVerifiedClientExported as u64, + fil_actor_verifreg_state::v17::Method::AddVerifiedClientExported as u64, @@ - Method::RemoveExpiredAllocationsExported as u64, + fil_actor_verifreg_state::v17::Method::RemoveExpiredAllocationsExported as u64, @@ - Method::GetClaimsExported as u64, + fil_actor_verifreg_state::v17::Method::GetClaimsExported as u64, @@ - Method::ExtendClaimTermsExported as u64, + fil_actor_verifreg_state::v17::Method::ExtendClaimTermsExported as u64, @@ - Method::RemoveExpiredClaimsExported as u64, + fil_actor_verifreg_state::v17::Method::RemoveExpiredClaimsExported as u64, @@ - Method::UniversalReceiverHook as u64, + fil_actor_verifreg_state::v17::Method::UniversalReceiverHook as u64,
33-41: Use realistic signature sizes to prevent future validation flakinessIf upstream starts validating lengths, these test vectors could fail. Prefer canonical sizes: 96 bytes (BLS) and 65 bytes (secp256k1).
- signature: fvm_shared4::crypto::signature::Signature::new_bls( - b"test_signature_1".to_vec(), - ), + signature: fvm_shared4::crypto::signature::Signature::new_bls(vec![0u8; 96]), @@ - signature: fvm_shared4::crypto::signature::Signature::new_secp256k1( - b"test_signature_2".to_vec(), - ), + signature: fvm_shared4::crypto::signature::Signature::new_secp256k1(vec![1u8; 65]),
16-16: Improve readability of byte sizesUse bit shifts for IEC units.
- allowance: StoragePower::from(1048576u64), // 1MB + allowance: StoragePower::from(1u64 << 20), // 1 MiB @@ - allowance: types::DataCap::from(2097152u64), // 2MB + allowance: types::DataCap::from(2u64 << 20), // 2 MiB @@ - data_cap_amount_to_remove: types::DataCap::from(1048576u64), + data_cap_amount_to_remove: types::DataCap::from(1u64 << 20),Also applies to: 25-26, 30-30
14-17: Use types::DataCap for AddVerifierParams.allowance (v17)v17 declares AddVerifierParams.allowance as types::DataCap (alias for abi::StoragePower). Replace StoragePower::from(1048576u64) with types::DataCap::from(1048576u64) to mirror the client path and keep type consistency — applies to src/tool/subcommands/api_cmd/state_decode_params_tests/verified_reg.rs lines 14–17, 23–26, 29–31.
src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs (4)
56-56: Use 32-byte randomness to match expected PoSt commitment size.Future validation could enforce length; use a 32-byte vector now.
- chain_commit_rand: Randomness(vec![1, 22, 43]), + chain_commit_rand: Randomness([0u8; 32].to_vec()),
153-153: Use valid padded piece size values.23 is not a valid padded piece size. Prefer powers of two (e.g., 2048).
- size: fvm_shared4::piece::PaddedPieceSize(23), + size: fvm_shared4::piece::PaddedPieceSize(2048),
173-173: Ditto: use a valid padded piece size.12 → 512 (or another valid size).
- size: fvm_shared4::piece::PaddedPieceSize(12), + size: fvm_shared4::piece::PaddedPieceSize(512),
221-505: Reduce boilerplate with a tiny helper for test construction.A helper keeps the method casting and tipset plumbing in one place, making additions less error-prone.
Add near the top of the module:
fn mk(to: Address, method: Method, params: Vec<u8>, tipset: &Tipset) -> Result<RpcTest> { RpcTest::identity(StateDecodeParams::request((to, method as u64, params, tipset.key().into()))?) }Then replace repetitive blocks like:
RpcTest::identity(StateDecodeParams::request(( MINER_ADDRESS, Method::ControlAddresses as u64, vec![], tipset.key().into(), ))?),with:
mk(MINER_ADDRESS, Method::ControlAddresses, vec![], tipset)?,src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs (3)
33-75: Avoid unwraps; parse the address once and reuse.Prevents panic on bad input and removes repeated parsing work.
Apply:
pub fn create_tests(tipset: &Tipset) -> Result<Vec<RpcTest>> { + let evm_address = Address::from_str(EVM_ADDRESS)?; @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::Constructor as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::Resurrect as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::GetBytecode as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::GetBytecodeHash as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::InvokeContract as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address.clone(), Method::InvokeContractDelegate as u64, @@ - RpcTest::identity(StateDecodeParams::request(( - Address::from_str(EVM_ADDRESS).unwrap(), + RpcTest::identity(StateDecodeParams::request(( + evm_address, Method::GetStorageAt as u64,Additionally, please verify that
EVM_ADDRESSresolves to an actual EVM-actor instance on all chains/snapshots you run against; otherwiseStateDecodeParamsmay fail to resolve actor code for decoding. If not guaranteed, consider making the address configurable or discovering an EVM actor address from the state attipset.
23-28: Double-check DelegateCallParams.value type and units.Ensure
value: TokenAmount::default().into()matches the expected type (e.g.,Option<TokenAmount>or a plainTokenAmount) and unit (attoFIL). If it’s aTokenAmount, prefer an explicit zero to avoid ambiguity.Possible tweak:
- value: TokenAmount::default().into(), + value: TokenAmount::from_atto(0).into(),Also consider explicitly importing the type for clarity:
+use fvm_shared::econ::TokenAmount;
13-75: Gate these tests on actor version availability.This module hard-codes v17 types. If the supplied
tipsetpredates actors v17, decoding can diverge. Consider skipping (returningOk(vec![])) unless v17 is active attipset.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
Cargo.toml(1 hunks)src/state_migration/nv27/mod.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/account.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/cron.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/datacap.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/eam.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/init.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/market.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/paych.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/power.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/reward.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/system.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/verified_reg.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/tool/subcommands/api_cmd/state_decode_params_tests/reward.rs
- src/tool/subcommands/api_cmd/state_decode_params_tests/system.rs
- Cargo.toml
- src/tool/subcommands/api_cmd/state_decode_params_tests/paych.rs
- src/tool/subcommands/api_cmd/state_decode_params_tests/eam.rs
- src/tool/subcommands/api_cmd/state_decode_params_tests/account.rs
- src/tool/subcommands/api_cmd/state_decode_params_tests/cron.rs
🧰 Additional context used
🧬 Code graph analysis (9)
src/tool/subcommands/api_cmd/state_decode_params_tests/power.rs (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
identity(330-332)request(263-263)request(288-288)power_actor_state_decode_params_tests(2190-2302)
src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs (2)
src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
create_tests(2099-2132)identity(330-332)request(263-263)request(288-288)src/rpc/methods/eth/types.rs (1)
serialize_params(83-87)
src/tool/subcommands/api_cmd/state_decode_params_tests/verified_reg.rs (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
identity(330-332)request(263-263)request(288-288)verified_reg_actor_state_decode_params_tests(3138-3328)
src/tool/subcommands/api_cmd/state_decode_params_tests/datacap.rs (2)
src/tool/subcommands/api_cmd/state_decode_params_tests/account.rs (1)
create_tests(8-33)src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
create_tests(2099-2132)identity(330-332)request(263-263)request(288-288)
src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (5)
create_tests(2099-2132)identity(330-332)request(263-263)request(288-288)miner_actor_state_decode_params_tests(2472-3022)
src/tool/subcommands/api_cmd/state_decode_params_tests/market.rs (4)
src/tool/subcommands/api_cmd/api_compare_tests.rs (3)
identity(330-332)request(263-263)request(288-288)src/blocks/chain4u.rs (1)
tipset(183-185)src/shim/econ.rs (1)
from_atto(106-108)src/shim/address.rs (1)
new_id(142-144)
src/tool/subcommands/api_cmd/state_decode_params_tests/init.rs (2)
src/tool/subcommands/api_cmd/state_decode_params_tests/account.rs (1)
create_tests(8-33)src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs (1)
create_tests(13-76)
src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
identity(330-332)request(263-263)request(288-288)multisig_actor_state_decode_params_tests(3024-3136)
src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs (2)
src/tool/subcommands/api_cmd/state_decode_params_tests/eam.rs (1)
create_tests(8-47)src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
create_tests(2099-2132)identity(330-332)request(263-263)request(288-288)
⏰ 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: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
🔇 Additional comments (16)
src/state_migration/nv27/mod.rs (2)
13-16: v17 system state added for NV27 migration — LGTMThis removes the duplicate v16 entry and correctly introduces v17. Ordering v16 -> v17 looks right for NV27.
13-16: NV27 → Actors v17 mapping verifiedRPC registries, shim, NV27 migration, and state_decode_params tests reference ActorVersion::V17 / fil_actor_system_state::v17 — mapping is consistent and no changes required.
Relevant locations: src/state_migration/nv27/mod.rs, src/rpc/registry/actors/system.rs, src/shim/actors/builtin/system/mod.rs, src/tool/subcommands/api_cmd/state_decode_params_tests/*
src/tool/subcommands/api_cmd/state_decode_params_tests/power.rs (2)
13-16: Correct Window PoSt proof variant for CreateMiner — good fix.Using
RegisteredPoStProof::StackedDRGWindow32GiBV1P1(Window, not Winning) matches CreateMiner semantics. LGTM.
23-26: Good use of typed deltas (StoragePower) for UpdateClaimedPower.Typed values avoid unit confusion and match v17 definitions. LGTM.
src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs (2)
5-5: Good use of v17 types and actor-specific method constants.Importing the v17 module and using
Method::Constructoravoids magic numbers and aligns with the PR’s objectives.Also applies to: 11-11
10-10: Verify decode works when the delegated address doesn’t exist at the given tipset.If
StateDecodeParamsresolves the actor code by looking uptoin state, a synthetic f4 (EAM namespace + 20 zero bytes) may not exist and could cause the RPC to fail. Confirm that both Forest and Lotus can decode based purely on the delegated-address namespace without requiring the actor to be present attipset. If not, consider:
- Supplying a known existing EthAccount f4 at
tipset, or- Adding a code-CID–based decode path (if supported), or
- Marking this test “conditional/skip” when the actor is absent.
src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (1)
8-51: Good coverage and structure for v17 Multisig params.Param construction looks correct and matches v17 types; this sets up solid parity tests.
src/tool/subcommands/api_cmd/state_decode_params_tests/market.rs (1)
125-275: Good use of typed Method constants instead of magic numbers.Casting enum variants to u64 aligns with the request API while keeping readability and version-scoping via v17::Method.
src/tool/subcommands/api_cmd/state_decode_params_tests/datacap.rs (3)
28-41: LGTM: FRC-46 transfer params usage is correct.The shared FRC-46 types and operator_data via RawBytes look good.
42-58: LGTM: Allowance operations are well-covered.Increase/Decrease/Revoke params match FRC-46 expectations.
59-67: LGTM: Burn/BurnFrom params are correct.Shapes align with FRC-46; using owner on BurnFrom is appropriate.
src/tool/subcommands/api_cmd/state_decode_params_tests/verified_reg.rs (2)
8-94: LGTM overall — v17 coverage and method constants look goodTest vectors are comprehensive and align with the PR goals (actor-specific method constants, v17 coverage).
158-186: No action — v17 exported method variants are present.
Confirmed fil_actor_verifreg_state v17 exposes AddVerifiedClientExported, RemoveExpiredAllocationsExported, GetClaimsExported, ExtendClaimTermsExported, RemoveExpiredClaimsExported.src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs (1)
220-220: Keepconst—Address::new_idis a const fn.
src/shim/address.rs definespub const fn new_id(id: u64) -> Self(line 142), soconst MINER_ADDRESS: Address = Address::new_id(78216);is valid; no change required.Likely an incorrect or invalid review comment.
src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs (2)
30-31: No change needed — GetStorageAtParams::new accepts shorter inputs and left-pads to 32 bytesGetStorageAtParams::new left-pads a supplied Vec into a [u8; 32] (GET_STORAGE_AT_PARAMS_ARRAY_LENGTH = 32), so vec![0xa] is valid and the existing test/serialization is correct (see src/rpc/methods/eth/types.rs).
Likely an incorrect or invalid review comment.
39-44: Resurrect uses ConstructorParams — no change required.
ResurrectParams is a type alias for ConstructorParams, so passing ConstructorParams (to_vec(&evm_constructor_params)) matches the actor ABI. (docs.rs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (4)
101-112: Duplicate LockBalance test entry — likely accidental.The two
LockBalancetests are identical. If duplication wasn’t intended, drop one.Apply:
RpcTest::identity(StateDecodeParams::request(( Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, Method::LockBalance as u64, to_vec(&multisig_lock_bal_params)?, tipset.key().into(), ))?), - RpcTest::identity(StateDecodeParams::request(( - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, - Method::LockBalance as u64, - to_vec(&multisig_lock_bal_params)?, - tipset.key().into(), - ))?),
54-115: DRY: factor the multisig address into a const.Avoid repeating
Address::new_id(18101)10×; using a single const makes future updates trivial.@@ - Ok(vec![ + const MSIG_ID: u64 = 18_101; + Ok(vec![ RpcTest::identity(StateDecodeParams::request(( - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), // https://calibration.filscan.io/en/address/t018101/ Method::Constructor as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::Propose as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::Approve as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::Cancel as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::AddSigner as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::RemoveSigner as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::SwapSigner as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::ChangeNumApprovalsThreshold as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::LockBalance as u64, @@ - Address::new_id(18101), // https://calibration.filscan.io/en/address/t018101/, + Address::new_id(MSIG_ID), Method::UniversalReceiverHook as u64,
24-27: Prefer explicit element type for readability.
vec![Default::default()]relies on inference;vec![0u8]is clearer and avoids accidental type changes.let multisig_tx_id_params = TxnIDParams { id: Default::default(), - proposal_hash: vec![Default::default()], + proposal_hash: vec![0u8], };
20-21: Confirm:ProposeParams.method = 0.Is
0intentional for the proposed call? If you meant a specific method, consider setting that method’s number explicitly for a more representative test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs(1 hunks)src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tool/subcommands/api_cmd/state_decode_params_tests/evm.rs
- src/tool/subcommands/api_cmd/state_decode_params_tests/ethaccount.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
identity(330-332)request(263-263)request(288-288)multisig_actor_state_decode_params_tests(3024-3136)
⏰ 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: Build Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: cargo-publish-dry-run
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
🔇 Additional comments (2)
src/tool/subcommands/api_cmd/state_decode_params_tests/multisig.rs (2)
6-7: Good: upgraded to v17 API and actor method constants.Using
fil_actor_multisig_state::v17::*andMethod::* as u64satisfies the “no magic numbers” goal and aligns with the v17 expansion.
115-117: Good: fallible base64 decode with?instead ofunwrap().This removes a potential panic path in tests.
Summary of changes
Changes introduced in this pull request:
lotus_json/actorsstateandparamsfil_actor_shared::ActorVersioninstead of u64 to track actor versionsExec4in v10 forinitactorReference issue to close (if applicable)
Closes #5937
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Refactor
Tests
Chores
Documentation