Integrate actor v18#6863
Merged
sudo-shashank merged 6 commits intomainfrom Apr 9, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughDependency versions for all Filecoin actor-state crates and Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RPC
participant Shim
participant ActorState as "Actor State / Storage"
participant LotusJson as "lotus_json"
Client->>RPC: Request (e.g., StateLookup / MinerInfo)
RPC->>Shim: load actor state (match ActorVersion::V18)
Shim->>ActorState: Deserialize v18 state from store
ActorState-->>Shim: v18 state struct
Shim->>LotusJson: convert State::V18 -> Lotus JSON
LotusJson-->>RPC: Lotus-JSON payload / typed result
RPC-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
sudo-shashank
force-pushed
the
shashank/add-v18-actor-support
branch
from
a0b46fb to
920e1aa
Compare
sudo-shashank
requested review from
akaladarshi and
hanabi1224
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
src/lotus_json/actors/states/market_state.rs (1)
175-175: Add a dedicatedState::V18round-trip case.This new arm is only exercised if
State::default_latest_version()already points at V18. Pinning one explicit V18 snapshot here would keep the branch covered even if the “latest” helper lags behind.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lotus_json/actors/states/market_state.rs` at line 175, Add an explicit test/arm that round-trips State::V18 instead of relying solely on State::default_latest_version(); locate the match/enum handling that currently uses v13_plus_market_state_fields for [V13, V14, V15, V16, V17, V18] and add a dedicated branch for State::V18 (or a dedicated V18 snapshot case in the test) that pins an explicit V18 snapshot so the V18 path is exercised even if State::default_latest_version() does not return V18; ensure the new case uses the same serialization/deserialization assertions as the other versions to validate round-trip behavior.src/lotus_json/actors/states/verified_reg_state.rs (1)
125-125: Add a dedicatedState::V18round-trip case.This arm is still covered only indirectly via
State::default_latest_version(). An explicit V18 snapshot would make the new mapping fail fast if the latest-version helper ever falls out of sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lotus_json/actors/states/verified_reg_state.rs` at line 125, Add an explicit round-trip test case for State::V18 instead of relying only on State::default_latest_version(); locate the verified registry state snapshot tests that use v9_to_latest_verified_reg_state_fields and add a dedicated arm/case that serializes and deserializes State::V18 (using the same field mapping helpers) to assert equality/round-trip correctness, so any future drift between the latest-version helper and the actual V18 mapping fails fast.src/lotus_json/actors/params/power_params.rs (1)
403-434: Collapse theMinerPowerParamsper-version modules into a macro.
impl_power_miner_power_params_v18is a byte-for-byte copy of the v16/v17 modules, and the section header above is now stale. A tiny macro here would keep future actor bumps to a one-line change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lotus_json/actors/params/power_params.rs` around lines 403 - 434, The per-version module impl_power_miner_power_params_v18 is a byte-for-byte duplicate of the v16/v17 blocks; create a small macro (e.g., define_miner_power_params_impl!) that takes a version token (like v16, v17, v18) and expands to the module with type alias T = fil_actor_power_state::VERSION::MinerPowerParams, the snapshots test, and the HasLotusJson impl mapping to MinerPowerParamsLotusJson (including snapshots(), into_lotus_json and from_lotus_json). Replace the three repeated modules with three macro invocations (one per version) and update the section header to be generic or removed; keep references to impl_power_miner_power_params_v{VER} and the HasLotusJson impl so reviewers can locate generated code.src/shim/actors/builtin/miner/ext/state.rs (1)
185-199: Add context to the new V18 sector-loading failures.This arm propagates raw errors from
load_sector_infos,Sectors::load, and the AMT iteration, which makes a bad V18 state much harder to diagnose. Please wrap those calls with.context(...)before bubbling them up.As per coding guidelines, "Use anyhow::Result for most operations and add context with `.context()` when errors occur".♻️ Suggested change
State::V18(st) => { if let Some(sectors) = sectors { Ok(st - .load_sector_infos(&store, sectors)? + .load_sector_infos(&store, sectors) + .context("loading miner v18 sector infos")? .into_iter() .map(From::from) .collect()) } else { - let sectors = fil_actor_miner_state::v18::Sectors::load(&store, &st.sectors)?; + let sectors = fil_actor_miner_state::v18::Sectors::load(&store, &st.sectors) + .context("loading miner v18 sectors")?; let mut infos = Vec::with_capacity(sectors.amt.count() as usize); sectors.amt.for_each(|_, info| { infos.push(info.clone().into()); Ok(()) - })?; + }) + .context("iterating miner v18 sectors")?; Ok(infos) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shim/actors/builtin/miner/ext/state.rs` around lines 185 - 199, State::V18 arm is propagating raw errors from load_sector_infos, fil_actor_miner_state::v18::Sectors::load, and sectors.amt.for_each which loses context; wrap each call with anyhow::Context via .context("...") describing the operation (e.g., "loading sector infos for V18 state", "loading V18 Sectors AMT", "iterating V18 sectors AMT") so failures include helpful messages; update calls within the State::V18 match arm referencing load_sector_infos, fil_actor_miner_state::v18::Sectors::load, and sectors.amt.for_each to append .context(...) before the ? is returned.src/lotus_json/actors/states/miner_state.rs (1)
154-170: Add a non-null V18 vesting-funds snapshot.This fixture still keeps
VestingFundsasnull, so the newly added V18 embedded round-trip on Lines 193-208 never gets exercised. A second snapshot with an actual embedded payload would cover the only new behavior in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lotus_json/actors/states/miner_state.rs` around lines 154 - 170, The test fixture uses fil_actor_miner_state::v18::VestingFunds(None) so the V18 embedded round-trip code path in State::default_latest_version is never exercised; add a second State::default_latest_version snapshot (or replace this one) that uses fil_actor_miner_state::v18::VestingFunds(Some(...)) with a real, minimal vesting-funds payload (matching the v18 struct shape expected by the round-trip) so the embedded V18 serialization/deserialization path is exercised; ensure the new snapshot mirrors the other default fields and uses the same State::default_latest_version constructor pattern so the only difference is a non-null V18 vesting-funds value.src/lotus_json/actors/params/miner_params.rs (1)
4013-4071: Consider centralizing these miner version lists.This block now carries several subtly different ranges (
8..=18,9..=18,13..=18,16..=18, plus intentionally capped pre-deprecation lists). Pulling the common sets behind a small helper macro or alias would make future actor bumps easier to audit and reduce accidental omissions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lotus_json/actors/params/miner_params.rs` around lines 4013 - 4071, The repeated literal version lists (e.g., 8..=18, 9..=18, 13..=18, 16..=18) used across macros like impl_lotus_json_for_miner_constructor_params!, impl_lotus_json_for_miner_change_worker_param!, impl_lotus_json_for_miner_prove_commit_sectors3_params!, impl_lotus_json_for_miner_max_termination_fee_params!, impl_miner_prove_commit_sector_params!, etc., should be centralized: create small helper macros or named aliases (e.g., V8_TO_18!, V9_TO_18!, V13_TO_18!, V16_TO_18!) that expand to the appropriate comma-separated version lists and replace each literal list with the corresponding helper macro invocation so all version ranges are defined in one place and future actor bumps are changed once.src/rpc/registry/actors/payment_channel.rs (1)
9-9: Minor: Update comment to reflect extended version coverage.The comment states methods are "consistent across all versions V8-V16" but the code now covers V8-V18.
📝 Suggested comment update
-// Payment channel methods are consistent across all versions V8-V16 +// Payment channel methods are consistent across all versions V8-V18🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/registry/actors/payment_channel.rs` at line 9, Update the top-line comment in payment_channel.rs that currently says "Payment channel methods are consistent across all versions V8-V16" to reflect the new coverage V8-V18; locate the comment in the payment_channel.rs file (the module handling PaymentChannel methods) and change the text to indicate V8-V18 so the comment matches the implemented support.src/rpc/registry/actors/reward.rs (1)
9-9: Consider renaming the macro to reflect extended version support.The macro
register_reward_version_11_to_16is now used for versions 11 through 18. Consider renaming it toregister_reward_version_11_plusor similar to avoid confusion as more versions are added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/registry/actors/reward.rs` at line 9, The macro register_reward_version_11_to_16 should be renamed to a name that reflects extended support (e.g., register_reward_version_11_plus); update the macro_rules! declaration name and replace all invocations of register_reward_version_11_to_16! with the new name, and adjust any comments/docstrings referencing the old name to avoid confusion when adding future versions (ensure symbol consistency for macro_rules! register_reward_version_11_plus and all call sites).src/rpc/methods/state.rs (1)
520-536:"Robust address not found"is not actually reachable here.
map.for_each(...)only errors on iteration failures, not on a miss, so this new v18 arm always falls through toOk(robust_addr). IfAddress::default()is the intended sentinel, the context is misleading and should be removed; otherwise, track afoundflag and return the not-found error explicitly.As per coding guidelines:
**/*.rs: Useanyhow::Result<T>for most operations and add context with.context()when errors occur`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/state.rs` around lines 520 - 536, The v18 arm in init::State::V18 uses fil_actor_init_state::v18::AddressMap::load and map.for_each to set robust_addr, but map.for_each does not produce an error when the address is missing so the trailing .context("Robust address not found") is misleading; either remove that context and accept Address::default() as the sentinel, or add a boolean found flag (set inside map.for_each when *v == id_addr_decoded) and after iteration return an explicit error (e.g., anyhow::bail! or Err(anyhow::anyhow!("Robust address not found"))) if not found—update the code around robust_addr and map.for_each accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/shim/actors/builtin/multisig/mod.rs`:
- Around line 176-183: The v18 match arm currently panics by calling
.expect(...) when loading fil_actor_multisig_state::v18::PendingTxnMap in
State::V18; instead propagate the error from PendingTxnMap::load through
get_pending_txn by using the anyhow Context trait (use anyhow::Context) and
replacing .expect(...) with a .context("loading pending txns for v18")? (or
similar) so the function returns an anyhow::Result with a helpful context;
reference PendingTxnMap::load, st.pending_txs,
fil_actor_multisig_state::v18::PENDING_TXN_CONFIG and get_pending_txn when
making the change.
---
Nitpick comments:
In `@src/lotus_json/actors/params/miner_params.rs`:
- Around line 4013-4071: The repeated literal version lists (e.g., 8..=18,
9..=18, 13..=18, 16..=18) used across macros like
impl_lotus_json_for_miner_constructor_params!,
impl_lotus_json_for_miner_change_worker_param!,
impl_lotus_json_for_miner_prove_commit_sectors3_params!,
impl_lotus_json_for_miner_max_termination_fee_params!,
impl_miner_prove_commit_sector_params!, etc., should be centralized: create
small helper macros or named aliases (e.g., V8_TO_18!, V9_TO_18!, V13_TO_18!,
V16_TO_18!) that expand to the appropriate comma-separated version lists and
replace each literal list with the corresponding helper macro invocation so all
version ranges are defined in one place and future actor bumps are changed once.
In `@src/lotus_json/actors/params/power_params.rs`:
- Around line 403-434: The per-version module impl_power_miner_power_params_v18
is a byte-for-byte duplicate of the v16/v17 blocks; create a small macro (e.g.,
define_miner_power_params_impl!) that takes a version token (like v16, v17, v18)
and expands to the module with type alias T =
fil_actor_power_state::VERSION::MinerPowerParams, the snapshots test, and the
HasLotusJson impl mapping to MinerPowerParamsLotusJson (including snapshots(),
into_lotus_json and from_lotus_json). Replace the three repeated modules with
three macro invocations (one per version) and update the section header to be
generic or removed; keep references to impl_power_miner_power_params_v{VER} and
the HasLotusJson impl so reviewers can locate generated code.
In `@src/lotus_json/actors/states/market_state.rs`:
- Line 175: Add an explicit test/arm that round-trips State::V18 instead of
relying solely on State::default_latest_version(); locate the match/enum
handling that currently uses v13_plus_market_state_fields for [V13, V14, V15,
V16, V17, V18] and add a dedicated branch for State::V18 (or a dedicated V18
snapshot case in the test) that pins an explicit V18 snapshot so the V18 path is
exercised even if State::default_latest_version() does not return V18; ensure
the new case uses the same serialization/deserialization assertions as the other
versions to validate round-trip behavior.
In `@src/lotus_json/actors/states/miner_state.rs`:
- Around line 154-170: The test fixture uses
fil_actor_miner_state::v18::VestingFunds(None) so the V18 embedded round-trip
code path in State::default_latest_version is never exercised; add a second
State::default_latest_version snapshot (or replace this one) that uses
fil_actor_miner_state::v18::VestingFunds(Some(...)) with a real, minimal
vesting-funds payload (matching the v18 struct shape expected by the round-trip)
so the embedded V18 serialization/deserialization path is exercised; ensure the
new snapshot mirrors the other default fields and uses the same
State::default_latest_version constructor pattern so the only difference is a
non-null V18 vesting-funds value.
In `@src/lotus_json/actors/states/verified_reg_state.rs`:
- Line 125: Add an explicit round-trip test case for State::V18 instead of
relying only on State::default_latest_version(); locate the verified registry
state snapshot tests that use v9_to_latest_verified_reg_state_fields and add a
dedicated arm/case that serializes and deserializes State::V18 (using the same
field mapping helpers) to assert equality/round-trip correctness, so any future
drift between the latest-version helper and the actual V18 mapping fails fast.
In `@src/rpc/methods/state.rs`:
- Around line 520-536: The v18 arm in init::State::V18 uses
fil_actor_init_state::v18::AddressMap::load and map.for_each to set robust_addr,
but map.for_each does not produce an error when the address is missing so the
trailing .context("Robust address not found") is misleading; either remove that
context and accept Address::default() as the sentinel, or add a boolean found
flag (set inside map.for_each when *v == id_addr_decoded) and after iteration
return an explicit error (e.g., anyhow::bail! or Err(anyhow::anyhow!("Robust
address not found"))) if not found—update the code around robust_addr and
map.for_each accordingly.
In `@src/rpc/registry/actors/payment_channel.rs`:
- Line 9: Update the top-line comment in payment_channel.rs that currently says
"Payment channel methods are consistent across all versions V8-V16" to reflect
the new coverage V8-V18; locate the comment in the payment_channel.rs file (the
module handling PaymentChannel methods) and change the text to indicate V8-V18
so the comment matches the implemented support.
In `@src/rpc/registry/actors/reward.rs`:
- Line 9: The macro register_reward_version_11_to_16 should be renamed to a name
that reflects extended support (e.g., register_reward_version_11_plus); update
the macro_rules! declaration name and replace all invocations of
register_reward_version_11_to_16! with the new name, and adjust any
comments/docstrings referencing the old name to avoid confusion when adding
future versions (ensure symbol consistency for macro_rules!
register_reward_version_11_plus and all call sites).
In `@src/shim/actors/builtin/miner/ext/state.rs`:
- Around line 185-199: State::V18 arm is propagating raw errors from
load_sector_infos, fil_actor_miner_state::v18::Sectors::load, and
sectors.amt.for_each which loses context; wrap each call with anyhow::Context
via .context("...") describing the operation (e.g., "loading sector infos for
V18 state", "loading V18 Sectors AMT", "iterating V18 sectors AMT") so failures
include helpful messages; update calls within the State::V18 match arm
referencing load_sector_infos, fil_actor_miner_state::v18::Sectors::load, and
sectors.amt.for_each to append .context(...) before the ? is returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2f861a0-e21c-46a7-9aed-e0c152299c3d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
Cargo.tomlsrc/lotus_json/actors/params/account_params.rssrc/lotus_json/actors/params/cron_params.rssrc/lotus_json/actors/params/datacap_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/miner_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.rssrc/lotus_json/actors/states/cron_state.rssrc/lotus_json/actors/states/datacap_state.rssrc/lotus_json/actors/states/evm_state.rssrc/lotus_json/actors/states/init_state.rssrc/lotus_json/actors/states/market_state.rssrc/lotus_json/actors/states/miner_state.rssrc/lotus_json/actors/states/multisig_state.rssrc/lotus_json/actors/states/payment_channel_state.rssrc/lotus_json/actors/states/power_states.rssrc/lotus_json/actors/states/reward_state.rssrc/lotus_json/actors/states/system_state.rssrc/lotus_json/actors/states/verified_reg_state.rssrc/lotus_json/entry.rssrc/lotus_json/tombstone.rssrc/lotus_json/transient_data.rssrc/rpc/methods/f3.rssrc/rpc/methods/state.rssrc/rpc/registry/actors/account.rssrc/rpc/registry/actors/cron.rssrc/rpc/registry/actors/datacap.rssrc/rpc/registry/actors/eam.rssrc/rpc/registry/actors/eth_account.rssrc/rpc/registry/actors/evm.rssrc/rpc/registry/actors/init.rssrc/rpc/registry/actors/market.rssrc/rpc/registry/actors/miner.rssrc/rpc/registry/actors/multisig.rssrc/rpc/registry/actors/payment_channel.rssrc/rpc/registry/actors/power.rssrc/rpc/registry/actors/reward.rssrc/rpc/registry/actors/system.rssrc/rpc/registry/actors/verified_reg.rssrc/rpc/types/sector_impl.rssrc/shim/actors/builtin/account/mod.rssrc/shim/actors/builtin/cron/mod.rssrc/shim/actors/builtin/datacap/mod.rssrc/shim/actors/builtin/evm/mod.rssrc/shim/actors/builtin/init/mod.rssrc/shim/actors/builtin/market/ext/balance_table.rssrc/shim/actors/builtin/market/ext/state.rssrc/shim/actors/builtin/market/mod.rssrc/shim/actors/builtin/miner/ext/deadline.rssrc/shim/actors/builtin/miner/ext/state.rssrc/shim/actors/builtin/miner/mod.rssrc/shim/actors/builtin/multisig/mod.rssrc/shim/actors/builtin/paymentchannel/mod.rssrc/shim/actors/builtin/power/ext.rssrc/shim/actors/builtin/power/mod.rssrc/shim/actors/builtin/reward/mod.rssrc/shim/actors/builtin/system/mod.rssrc/shim/actors/builtin/verifreg/ext/state.rssrc/shim/actors/builtin/verifreg/mod.rssrc/shim/actors/convert.rssrc/shim/actors/state_load.rssrc/shim/runtime.rs
hanabi1224
previously approved these changes
Apr 8, 2026
sudo-shashank
force-pushed
the
shashank/add-v18-actor-support
branch
from
91acce3 to
82812a1
Compare
hanabi1224
approved these changes
Apr 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
New Features
Dependencies