Skip to content

Conversation

@hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Dec 5, 2025

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes #6311
Closes #6310

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

  • New Features

    • Added Filecoin.StateGetActor API v2 with tipset selection.
    • Added Filecoin.StateGetID API v2 with tipset selection.
    • Introduced a flexible tipset selector (key, height, or tags: Latest/Finalized/Safe) for V2 calls.
  • Tests

    • Added new RPC snapshot tests covering the V2 state queries.

✏️ Tip: You can customize this high-level summary in your review settings.

@hanabi1224 hanabi1224 added the RPC requires calibnet RPC checks to run on CI label Dec 5, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

Adds RPC v2 support: new Filecoin.StateGetActor and Filecoin.StateGetID handlers that accept a TipsetSelector; moves tipset selector types into a dedicated module; extracts ChainGetTipSetV2 helper functions to centralize tipset-resolution logic; updates tests and changelog entries.

Changes

Cohort / File(s) Summary
Tipset selector types (new module)
src/rpc/types/tipset_selector.rs, src/rpc/types/mod.rs
Added TipsetSelector, TipsetHeight, TipsetAnchor, TipsetTag with validation and policy resolution; registered and re-exported the new module.
Removed types from chain types
src/rpc/methods/chain/types.rs
Removed previously defined TipsetSelector/TipsetHeight/TipsetAnchor/TipsetTag (migrated to new module).
Chain tipset helpers extraction
src/rpc/methods/chain.rs
Extracted multiple public helper methods on ChainGetTipSetV2 (get_tipset, get_required_tipset, get_tipset_by_anchor, get_tipset_by_tag, get_latest_safe_tipset, get_latest_finalized_tipset, get_ec_finalized_tipset) and refactored the RpcMethod handler to delegate to them.
New RPC v2 methods
src/rpc/methods/state.rs, src/rpc/mod.rs
Added StateGetActorV2 and StateGetID RPC v2 implementations and registered them in the RPC registry; both resolve tipsets via ChainGetTipSetV2 before performing state lookups.
Tests & snapshots
src/tool/subcommands/api_cmd/api_compare_tests.rs, src/tool/subcommands/api_cmd/test_snapshots.txt
Added calls to StateGetActorV2 and StateGetID in API comparison tests; added two new snapshot files.
Changelog
CHANGELOG.md
Documented the addition of Filecoin.StateGetActor and Filecoin.StateGetID for API v2 (entries referencing PR #6312).

Sequence Diagram(s)

mermaid
sequenceDiagram
autonumber
participant Client
participant RPC_Layer as RPC (Filecoin.StateGetActor / StateGetID v2)
participant ChainHelper as ChainGetTipSetV2
participant StateMgr as StateManager
participant Blockstore
Note over Client,RPC_Layer: Client calls Filecoin.StateGetActor/StateGetID with TipsetSelector
Client->>RPC_Layer: request(address, TipsetSelector)
RPC_Layer->>ChainHelper: resolve tipset (get_tipset)
alt selector by key
ChainHelper->>Blockstore: lookup tipset by key
else selector by height/tag/anchor
ChainHelper->>ChainHelper: apply height/tag/anchor logic
ChainHelper->>Blockstore: fetch required tipset(s)
end
ChainHelper-->>RPC_Layer: resolved parent tipset
RPC_Layer->>StateMgr: query actor or id at resolved tipset
StateMgr->>Blockstore: load actor state / address mapping
StateMgr-->>RPC_Layer: actor state or ID
RPC_Layer-->>Client: return result

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to correctness of moved types and everywhere they're referenced after migration.
  • Validate TipsetSelector validation and null-tipset policy logic.
  • Review ChainGetTipSetV2 helper error handling and concurrency/generic bounds.
  • Confirm RPC registration and parameter names/types for v2 methods.
  • Check test updates and snapshot additions for expected outputs.

Possibly related PRs

Suggested reviewers

  • akaladarshi
  • sudo-shashank

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues check ❓ Inconclusive Both linked issues (#6310, #6311) lack detailed completion criteria, making it impossible to validate whether code changes meet specific requirements. However, the PR implements V2 RPC methods for StateGetActor and StateGetID as indicated by issue titles. Add detailed completion criteria to GitHub issues #6310 and #6311, such as method signatures, parameter requirements, and expected behavior, to enable proper validation of implementation completeness.
Out of Scope Changes check ❓ Inconclusive The PR includes refactoring of tipset selector logic in chain.rs and relocates TipsetSelector types from chain/types.rs to a new rpc/types/tipset_selector.rs module, which appear to support the V2 RPC methods but extend beyond the immediate scope of implementing StateGetActor and StateGetID. Clarify whether the tipset selector refactoring and module reorganization are necessary for the V2 RPC methods or represent separate infrastructure improvements that should be tracked as separate issues.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: introducing V2 RPC methods StateGetActor and StateGetID, matching the changeset's primary objectives.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hm/state-get-id

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4897c3 and 8a2acb7.

📒 Files selected for processing (1)
  • src/rpc/methods/state.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/rpc/registry/actors/miner.rs:221-223
Timestamp: 2025-09-02T10:05:34.350Z
Learning: For miner actor ChangeOwnerAddress and ChangeOwnerAddressExported methods: versions 8-10 use bare Address as parameter type, while versions 11+ use ChangeOwnerAddressParams. This reflects the evolution of the Filecoin miner actor parameter structures across versions.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-09-02T10:05:34.350Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/rpc/registry/actors/miner.rs:221-223
Timestamp: 2025-09-02T10:05:34.350Z
Learning: For miner actor ChangeOwnerAddress and ChangeOwnerAddressExported methods: versions 8-10 use bare Address as parameter type, while versions 11+ use ChangeOwnerAddressParams. This reflects the evolution of the Filecoin miner actor parameter structures across versions.

Applied to files:

  • src/rpc/methods/state.rs
📚 Learning: 2025-08-08T12:11:55.266Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:461-487
Timestamp: 2025-08-08T12:11:55.266Z
Learning: Forest (src/ipld/util.rs, Rust): In UnorderedChainStream::poll_next, dropping `extract_sender` (when no more tipsets and the extract queue is empty) is the intended shutdown signal for workers. Any subsequent attempt to enqueue work after this drop is a logic error and should be treated as an error; do not change `send()` to ignore a missing sender.

Applied to files:

  • src/rpc/methods/state.rs
🧬 Code graph analysis (1)
src/rpc/methods/state.rs (1)
src/rpc/methods/chain.rs (3)
  • chain (444-444)
  • chain (482-482)
  • get_required_tipset (1133-1140)
⏰ 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). (8)
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Build MacOS
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: Build Ubuntu
  • GitHub Check: tests-release
  • GitHub Check: tests
  • GitHub Check: Check
  • GitHub Check: All lint checks
🔇 Additional comments (3)
src/rpc/methods/state.rs (3)

8-8: LGTM! Import needed for V2 tipset resolution.

The import of ChainGetTipSetV2 is correctly used by the new V2 RPC methods to resolve tipsets from TipsetSelector parameters.


305-325: LGTM! V2 variant correctly implemented.

StateGetActorV2 properly implements the V2 version of StateGetActor:

  • Uses TipsetSelector parameter and ChainGetTipSetV2::get_required_tipset for V2 tipset resolution
  • Restricted to V2 API path only
  • Description accurately reflects the return type (Option<ActorState>)
  • Logic matches the V1 method with appropriate V2 adaptations

327-347: LGTM! New V2 method correctly implemented.

StateGetID is properly implemented as a new V2 RPC method:

  • Uses TipsetSelector parameter and ChainGetTipSetV2::get_required_tipset for V2 tipset resolution
  • Restricted to V2 API path only
  • Description accurately reflects the functionality (retrieving ID address)
  • Logic follows the same pattern as StateLookupID but with V2 conventions
  • Proper error handling with async/await

Comment @coderabbitai help to get the list of available commands and usage tips.

@hanabi1224 hanabi1224 marked this pull request as ready for review December 5, 2025 14:22
@hanabi1224 hanabi1224 requested a review from a team as a code owner December 5, 2025 14:22
@hanabi1224 hanabi1224 requested review from akaladarshi and sudo-shashank and removed request for a team December 5, 2025 14:22
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

use super::*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from src/rpc/methods/chain/types.rs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/tool/subcommands/api_cmd/api_compare_tests.rs (1)

22-22: V2 state RPC tests look good; consider exercising non‑ID address for StateGetID.

The new StateGetActorV2 / StateGetID entries wired through TipsetSelector { key: tipset.key().into(), ..Default::default() } correctly mirror the existing StateGetActor coverage at a concrete tipset.

For StateGetID, using Address::SYSTEM_ACTOR (already an ID address) doesn’t actually exercise address‑to‑ID resolution differences between Lotus and Forest. If you want a slightly stronger parity check, consider adding a second case with a robust/BLS/SECP address whose ID resolution is stable in the snapshot, so that StateGetID validates the selector plumbing and the lookup path together.

Also applies to: 812-842

src/rpc/types/tipset_selector.rs (1)

10-50: Selector/height/anchor validation is well‑scoped and matches V2 selection semantics.

The invariants you enforce here—exactly one of {key, tag, height} on TipsetSelector, non‑negative at plus a “not both key and tag” anchor constraint, and previousResolveNullTipset mapping—line up cleanly with how ChainGetTipSetV2::get_tipset interprets selectors. I don’t see any edge‑case correctness issues in this module.

If you later change ApiTipsetKey’s internals, it might be worth swapping the direct .0 field access for helper methods (e.g., is_none() and a dedicated is_some()), but that’s purely a future‑proofing tweak, not a blocker.

Also applies to: 53-81, 83-109, 111-129

src/rpc/methods/chain.rs (1)

22-22: Clarify / align finalized-tipset semantics between ChainGetFinalizedTipset and V2 TipsetTag::Finalized.

The new ChainGetTipSetV2 helpers look coherent overall: selector validation, anchor resolution, and safe/finalized tag handling all fit together nicely, and get_required_tipset is a good central hook for state v2 methods.

One subtle semantic difference to be aware of:

  • ChainGetFinalizedTipset::handle computes the EC finality epoch as (head.epoch() - chain_finality).max(0) and always returns some tipset, even when head.epoch() < chain_finality (it effectively treats genesis as “finalized” in that early window).
  • get_ec_finalized_tipset used by get_latest_finalized_tipset instead returns None when head.epoch() < chain_finality, which means TipsetTag::Finalized in V2 can yield None (and thus an error via get_required_tipset) in situations where ChainGetFinalizedTipset would still return a tipset.

If the intent is that “finalized” via the tag and via ChainGetFinalizedTipset should behave identically, consider either clamping ec_finality_epoch to 0 in get_ec_finalized_tipset as well, or documenting this intentional difference so clients know that V2 selectors won’t report a finalized tipset until the chain is at least chain_finality epochs long.

Also applies to: 1007-1103, 1105-1159

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07cbdaf and d4897c3.

📒 Files selected for processing (9)
  • CHANGELOG.md (1 hunks)
  • src/rpc/methods/chain.rs (4 hunks)
  • src/rpc/methods/chain/types.rs (0 hunks)
  • src/rpc/methods/state.rs (2 hunks)
  • src/rpc/mod.rs (1 hunks)
  • src/rpc/types/mod.rs (1 hunks)
  • src/rpc/types/tipset_selector.rs (1 hunks)
  • src/tool/subcommands/api_cmd/api_compare_tests.rs (2 hunks)
  • src/tool/subcommands/api_cmd/test_snapshots.txt (2 hunks)
💤 Files with no reviewable changes (1)
  • src/rpc/methods/chain/types.rs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/rpc/registry/actors/miner.rs:221-223
Timestamp: 2025-09-02T10:05:34.350Z
Learning: For miner actor ChangeOwnerAddress and ChangeOwnerAddressExported methods: versions 8-10 use bare Address as parameter type, while versions 11+ use ChangeOwnerAddressParams. This reflects the evolution of the Filecoin miner actor parameter structures across versions.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-09-11T16:03:14.328Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.

Applied to files:

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

Applied to files:

  • CHANGELOG.md
  • src/rpc/methods/state.rs
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.

Applied to files:

  • src/tool/subcommands/api_cmd/test_snapshots.txt
📚 Learning: 2025-09-10T10:32:54.333Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/rpc/registry/actors/miner.rs:388-392
Timestamp: 2025-09-10T10:32:54.333Z
Learning: In miner actor v17, the methods ExtendSectorExpiration, ProveCommitAggregate, and ProveReplicaUpdates have been deprecated and are intentionally not registered, unlike in v16 where they are still supported.

Applied to files:

  • src/rpc/methods/state.rs
📚 Learning: 2025-08-08T12:11:55.266Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:461-487
Timestamp: 2025-08-08T12:11:55.266Z
Learning: Forest (src/ipld/util.rs, Rust): In UnorderedChainStream::poll_next, dropping `extract_sender` (when no more tipsets and the extract queue is empty) is the intended shutdown signal for workers. Any subsequent attempt to enqueue work after this drop is a logic error and should be treated as an error; do not change `send()` to ignore a missing sender.

Applied to files:

  • src/rpc/methods/state.rs
  • src/rpc/methods/chain.rs
🧬 Code graph analysis (4)
src/rpc/mod.rs (2)
src/state_manager/chain_rand.rs (4)
  • state (191-191)
  • state (194-194)
  • state (209-209)
  • state (211-211)
src/rpc/methods/sync/types.rs (1)
  • state (105-107)
src/rpc/methods/state.rs (1)
src/rpc/methods/chain.rs (3)
  • chain (444-444)
  • chain (482-482)
  • get_required_tipset (1133-1140)
src/rpc/types/tipset_selector.rs (1)
src/rpc/types/mod.rs (1)
  • is_none (168-170)
src/rpc/methods/chain.rs (1)
src/rpc/methods/state.rs (16)
  • handle (110-115)
  • handle (133-139)
  • handle (152-158)
  • handle (173-179)
  • handle (197-206)
  • handle (224-230)
  • handle (247-254)
  • handle (271-279)
  • handle (295-302)
  • handle (318-324)
  • handle (340-346)
  • handle (376-506)
  • handle (525-533)
  • handle (549-579)
  • handle (596-602)
  • handle (618-638)
⏰ 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). (20)
  • GitHub Check: Bootstrap checks - Lotus
  • GitHub Check: Devnet checks
  • GitHub Check: Calibnet eth mapping check
  • GitHub Check: Calibnet kademlia checks
  • GitHub Check: Diff snapshot export checks
  • GitHub Check: Calibnet RPC checks
  • GitHub Check: db-migration-checks
  • GitHub Check: Bootstrap checks - Forest
  • GitHub Check: Calibnet api test-stateful check
  • GitHub Check: Calibnet no discovery checks
  • GitHub Check: State migrations
  • GitHub Check: Wallet tests
  • GitHub Check: V2 snapshot export checks
  • GitHub Check: Forest CLI checks
  • GitHub Check: Calibnet stateless mode check
  • GitHub Check: Calibnet stateless RPC check
  • GitHub Check: Calibnet check
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: tests
  • GitHub Check: tests-release
🔇 Additional comments (4)
src/tool/subcommands/api_cmd/test_snapshots.txt (1)

215-215: New State snapshot entries look consistent*

The added filecoin_stategetactor_... and filecoin_stategetid_... snapshots follow existing naming/grouping conventions and fit alongside the other State* snapshots.

Also applies to: 225-225

CHANGELOG.md (1)

34-36: Changelog entries are accurate and well‑placed

The new Added bullets for Filecoin.StateGetActor and Filecoin.StateGetID under Forest unreleased correctly document the API v2 additions and match existing style.

src/rpc/types/mod.rs (1)

8-10: Tipset selector module exposure is wired correctly

Making tipset_selector a private submodule and re‑exporting its contents from rpc::types aligns with the existing pattern for shared RPC types and keeps the public surface tidy.

src/rpc/mod.rs (1)

200-201: New state RPC methods are correctly registered in the macro

Registering StateGetActorV2 and StateGetID in for_each_rpc_method! is the right way to expose them through:

  • server registration (create_modules),
  • OpenRPC generation,
  • the prelude re‑exports and permission metadata.

Just ensure their API_PATHS and NAME/NAME_ALIAS in src/rpc/methods/state.rs gate them to the intended API version(s) and avoid alias collisions with StateGetActor.

Copy link
Member

@LesnyRumcajs LesnyRumcajs left a comment

Choose a reason for hiding this comment

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

LGTM

@hanabi1224 hanabi1224 enabled auto-merge December 5, 2025 15:56
@hanabi1224 hanabi1224 added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit 522e65a Dec 5, 2025
51 checks passed
@hanabi1224 hanabi1224 deleted the hm/state-get-id branch December 5, 2025 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC requires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RPC v2] Filecoin.StateGetID [RPC v2] Filecoin.StateGetActor

3 participants