-
Notifications
You must be signed in to change notification settings - Fork 182
feat(rpc): V2 RPC methods StateGetActor and StateGetID #6312
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
WalkthroughAdds 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
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-09-02T10:05:34.350ZApplied to files:
📚 Learning: 2025-08-08T12:11:55.266ZApplied to files:
🧬 Code graph analysis (1)src/rpc/methods/state.rs (1)
⏰ 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)
🔇 Additional comments (3)
Comment |
| // Copyright 2019-2025 ChainSafe Systems | ||
| // SPDX-License-Identifier: Apache-2.0, MIT | ||
|
|
||
| use super::*; |
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.
Moved from src/rpc/methods/chain/types.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: 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/StateGetIDentries wired throughTipsetSelector { key: tipset.key().into(), ..Default::default() }correctly mirror the existingStateGetActorcoverage at a concrete tipset.For
StateGetID, usingAddress::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 thatStateGetIDvalidates 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}onTipsetSelector, non‑negativeatplus a “not both key and tag” anchor constraint, andprevious→ResolveNullTipsetmapping—line up cleanly with howChainGetTipSetV2::get_tipsetinterprets 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.0field access for helper methods (e.g.,is_none()and a dedicatedis_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 betweenChainGetFinalizedTipsetand V2TipsetTag::Finalized.The new
ChainGetTipSetV2helpers look coherent overall: selector validation, anchor resolution, and safe/finalized tag handling all fit together nicely, andget_required_tipsetis a good central hook for state v2 methods.One subtle semantic difference to be aware of:
ChainGetFinalizedTipset::handlecomputes the EC finality epoch as(head.epoch() - chain_finality).max(0)and always returns some tipset, even whenhead.epoch() < chain_finality(it effectively treats genesis as “finalized” in that early window).get_ec_finalized_tipsetused byget_latest_finalized_tipsetinstead returnsNonewhenhead.epoch() < chain_finality, which meansTipsetTag::Finalizedin V2 can yieldNone(and thus an error viaget_required_tipset) in situations whereChainGetFinalizedTipsetwould still return a tipset.If the intent is that “finalized” via the tag and via
ChainGetFinalizedTipsetshould behave identically, consider either clampingec_finality_epochto0inget_ec_finalized_tipsetas well, or documenting this intentional difference so clients know that V2 selectors won’t report a finalized tipset until the chain is at leastchain_finalityepochs long.Also applies to: 1007-1103, 1105-1159
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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.mdsrc/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.rssrc/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_...andfilecoin_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‑placedThe new
Addedbullets forFilecoin.StateGetActorandFilecoin.StateGetIDunder 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 correctlyMaking
tipset_selectora private submodule and re‑exporting its contents fromrpc::typesaligns 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 macroRegistering
StateGetActorV2andStateGetIDinfor_each_rpc_method!is the right way to expose them through:
- server registration (
create_modules),- OpenRPC generation,
- the
preludere‑exports and permission metadata.Just ensure their
API_PATHSandNAME/NAME_ALIASinsrc/rpc/methods/state.rsgate them to the intended API version(s) and avoid alias collisions withStateGetActor.
LesnyRumcajs
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
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
Summary by CodeRabbit
New Features
Filecoin.StateGetActorAPI v2 with tipset selection.Filecoin.StateGetIDAPI v2 with tipset selection.Tests
✏️ Tip: You can customize this high-level summary in your review settings.