feat: add forest-cli state actor-cids#6061
Conversation
WalkthroughAdds a new RPC method Forest.StateActorInfo exposing builtin actor bundle/manifest CIDs and actor code CIDs for the current head; registers the RPC; adds a CLI subcommand Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as forest-cli
participant RPC as Forest RPC
participant Chain as Chain/Tipset
participant State as StateTree
participant Meta as ACTOR_BUNDLES_METADATA
User->>CLI: forest-cli state actor-cids --format json/text
CLI->>RPC: Forest.StateActorInfo()
RPC->>Chain: Load heaviest tipset
RPC->>State: Build state, read system builtin_actors CID
RPC->>Meta: Lookup bundle/manifest for network
RPC->>RPC: Load BuiltinActorManifest, map actor names→CIDs
RPC-->>CLI: StateActorCodeCidsOutput (manifest, bundle, actor_cids, versions)
alt format = json
CLI-->>User: Pretty JSON
else format = text
CLI-->>User: Human-readable lines
end
sequenceDiagram
autonumber
participant Script as calibnet_other_check.sh
participant CLI as forest-cli
participant jq as jq
participant FS as build/manifest.json
Script->>CLI: state actor-cids --format json
CLI-->>Script: JSON output
Script->>jq: extract .bundle["/"]
jq-->>Script: bundle_cid
Script->>FS: grep bundle_cid
alt Found
Script-->>Script: continue
else Not found
Script-->>Script: print error & exit 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
LesnyRumcajs
force-pushed
the
actor-info-cmd
branch
from
e998345 to
6fbf7a7
Compare
LesnyRumcajs
requested review from
hanabi1224 and
sudo-shashank
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/shim/machine/manifest.rs (1)
27-28: Avoid leaking internal field; rely on the accessor instead.
actor_list_cidalready has a public accessor (source_cid()), keeping the field private preserves invariants and allows future internal changes without API breakage.Apply this diff:
- /// The CID that this manifest was built from - pub actor_list_cid: Cid, + /// The CID that this manifest was built from + actor_list_cid: Cid,src/cli/subcommands/state_cmd.rs (1)
46-51: Clarify help text to reflect full output (manifest, bundle, and per-actor code CIDs).Align the CLI help with the actual output to reduce ambiguity.
Apply this diff:
- /// Returns the built-in actor bundle CIDs for the current network + /// Shows built-in actor code CIDs for the current head (heaviest tipset), + /// including the manifest and bundle CIDs.src/rpc/methods/state.rs (4)
3174-3182: Use fully-qualified LotusJson path in schemars for consistency.
Elsewhere in this file (e.g., NetworkParams) we usecrate::lotus_json::LotusJson<T>. Aligning avoids import/path surprises in schema generation.Apply:
- #[schemars(with = "LotusJson<Cid>")] + #[schemars(with = "crate::lotus_json::LotusJson<Cid>")] pub manifest: Cid, - #[schemars(with = "LotusJson<Cid>")] + #[schemars(with = "crate::lotus_json::LotusJson<Cid>")] pub bundle: Cid, - #[schemars(with = "LotusJson<HashMap<String, Cid>>")] + #[schemars(with = "crate::lotus_json::LotusJson<HashMap<String, Cid>>")] pub actor_cids: HashMap<String, Cid>,
3186-3204: Make CLI text output deterministic by sorting actor names.
Iteration over HashMap is non-deterministic; sorting improves readability and diffs.writeln!(f, "Actor CIDs:")?; let longest_name = self .actor_cids .keys() .map(|name| name.len()) .max() .unwrap_or(0); - for (name, cid) in &self.actor_cids { - writeln!(f, " {:width$} : {}", name, cid, width = longest_name)?; - } + let mut entries: Vec<_> = self.actor_cids.iter().collect(); + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + for (name, cid) in entries { + writeln!(f, " {:width$} : {}", name, cid, width = longest_name)?; + } Ok(())
3235-3241: Fix error message: it’s matching by manifest (actor list) CID, not bundle CID.
This avoids confusion when troubleshooting.- .with_context(|| { - format!( - "No actor bundle found for network {} with bundle CID {}", - ctx.chain_config().network, - actors - ) - })?; + .with_context(|| { + format!( + "No actor bundle metadata found for network {} with manifest (actor list) CID {}", + ctx.chain_config().network, + actors + ) + })?;
3252-3252: Replace chain_config().network_version(ts.epoch() - 1) with state_manager.get_network_version(ts.epoch()). Matches other RPC methods (e.g. StateNetworkVersion) and removes the off-by-one risk at epoch 0.- let network_version = ctx.chain_config().network_version(ts.epoch() - 1); + let network_version = ctx.state_manager.get_network_version(ts.epoch());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)scripts/tests/calibnet_other_check.sh(1 hunks)src/cli/subcommands/state_cmd.rs(3 hunks)src/rpc/methods/state.rs(4 hunks)src/rpc/mod.rs(1 hunks)src/shim/actors/builtin/system/mod.rs(2 hunks)src/shim/machine/manifest.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
PR: ChainSafe/forest#5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
🧬 Code graph analysis (2)
src/cli/subcommands/state_cmd.rs (2)
src/rpc/client.rs (2)
client(114-114)request(272-285)src/rpc/reflect/mod.rs (1)
request(250-260)
src/rpc/methods/state.rs (5)
src/daemon/context.rs (1)
chain_config(65-67)src/networks/mod.rs (2)
network_version(416-425)chain_config(718-723)src/shim/state_tree.rs (1)
new_from_tipset(185-187)src/state_manager/mod.rs (1)
chain_config(244-246)src/shim/machine/manifest.rs (1)
load_v1_actor_list(132-160)
⏰ 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: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: All lint checks
🔇 Additional comments (8)
src/shim/actors/builtin/system/mod.rs (1)
30-46: LGTM: uniform accessor across state versions.The new
builtin_actors_cid()method cleanly abstracts version differences and avoids duplication at call sites.src/rpc/mod.rs (1)
233-236: Registering StateActorInfo is correct and in the right vertical.Placement after
StateNetworkVersionis sensible; this will include it in routing and OpenRPC generation.src/cli/subcommands/state_cmd.rs (2)
15-19: LGTM: simple, clear output format enum.
clap::ValueEnumwith a default is appropriate here.
98-107: JSON printing path looks fine.Assuming
StateActorCodeCidsOutputderives/uses Lotus-compatible serde (as indicated bylotus_json_with_self!),serde_json::to_string_pretty(&info)will produce the expected JSON. No change needed.src/rpc/methods/state.rs (4)
14-15: LGTM: imports for lotus_json and networks metadata are appropriate.
These are required for the new output type and bundle lookup.
23-23: LGTM: init/system actor imports are necessary for state access.
Used in StateLookupRobustAddress and the new StateActorInfo endpoint.
35-35: LGTM: BuiltinActorManifest import is correct.
Needed for manifest loading/verification.
53-53: LGTM: anyhow::Context import is used properly for richer errors.
LesnyRumcajs
force-pushed
the
actor-info-cmd
branch
from
ec23c4f to
961ce4b
Compare
LesnyRumcajs
force-pushed
the
actor-info-cmd
branch
from
961ce4b to
26f4144
Compare
LesnyRumcajs
force-pushed
the
actor-info-cmd
branch
from
6604c70 to
d687893
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/rpc/methods/state.rs (2)
3187-3210: Deterministic text output: sort actor names before printingCurrent iteration over HashMap yields nondeterministic order. Sort to make the CLI’s text output stable.
Apply this diff:
- for (name, cid) in &self.actor_cids { - writeln!(f, " {:width$} : {}", name, cid, width = longest_name)?; - } + let mut entries: Vec<_> = self.actor_cids.iter().collect(); + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + for (name, cid) in entries { + writeln!(f, " {:width$} : {}", name, cid, width = longest_name)?; + }
3237-3242: Improve mismatch error with actionable detailsInclude both CIDs to ease diagnostics when manifests differ.
Apply this diff:
- if current_manifest != bundle.manifest { - return Err(anyhow::anyhow!("Actor bundle manifest does not match the manifest in the state tree. This indicates that the node is misconfigured or is running an unsupported network.") - .into()); - } + if current_manifest != bundle.manifest { + return Err(anyhow::anyhow!( + "Actor bundle manifest mismatch: state_actor_list_cid={}, bundle_actor_list_cid={}. \ +This indicates that the node is misconfigured or is running an unsupported network.", + current_manifest.actor_list_cid, + bundle.manifest.actor_list_cid + ).into()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)scripts/tests/calibnet_other_check.sh(1 hunks)src/cli/subcommands/state_cmd.rs(3 hunks)src/rpc/methods/state.rs(4 hunks)src/rpc/mod.rs(1 hunks)src/shim/actors/builtin/system/mod.rs(2 hunks)src/shim/machine/manifest.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots_ignored.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/shim/machine/manifest.rs
- src/cli/subcommands/state_cmd.rs
- src/shim/actors/builtin/system/mod.rs
- src/rpc/mod.rs
- scripts/tests/calibnet_other_check.sh
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
PR: ChainSafe/forest#5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
🧬 Code graph analysis (1)
src/rpc/methods/state.rs (4)
src/rpc/registry/actors_reg.rs (1)
load_and_serialize_actor_state(70-127)src/networks/mod.rs (3)
network_version(427-432)network_version(745-755)network_version_revision(436-444)src/shim/state_tree.rs (1)
new_from_tipset(190-192)src/shim/machine/manifest.rs (1)
load_v1_actor_list(132-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: All lint checks
🔇 Additional comments (1)
src/tool/subcommands/api_cmd/test_snapshots_ignored.txt (1)
84-84: Add StateActorInfo to snapshot ignore list — LGTMAppropriate to ignore since output depends on live network/bundle state and ordering.
3 participants
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Forest:
Change checklist
Summary by CodeRabbit
New Features
Tests
Documentation