feat: strict checks for address args in forest-cli

[LesnyRumcajs]

Summary of changes

Changes introduced in this pull request:

• stricter checks for the address arguments in the forest-cli.
• simplified some async logic

Reference issue to close (if applicable)

Closes#6006

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

• Improvements

  • Stricter address validation across CLI subcommands, reducing invalid input errors.
  • Updated mpool pending filters (to/from) and nonce to require validated addresses.
  • State read-state now accepts validated addresses directly for more reliable queries.
  • Automatic network detection at startup for better default behavior.

• Documentation

  • Added changelog entry noting stricter checks for address arguments in forest-cli subcommands.

[coderabbitai]

Walkthrough

Converts the CLI to async: forest-cli entrypoint uses Tokio multi-thread runtime; src/cli/main.rs becomes async, moves logger setup earlier, creates a tokenless RPC client, fetches network name before parsing CLI, and dispatches subcommands via await. Updates CLI subcommands to use StrictAddress instead of String/Address for address params. Adds changelog entry.

Changes

Cohort / File(s)	Summary
Async CLI entrypoint and initialization src/bin/forest-cli.rs, src/cli/main.rs	Switched to #[tokio::main(flavor = "multi_thread")] and async fn main; removed manual runtime/block_on; early logger setup; instantiate RPC client and fetch network name pre-parse; direct async command dispatch.
Mpool CLI address strictness src/cli/subcommands/mpool_cmd.rs	Replaced String/Address params with StrictAddress for Pending { to, from } and Nonce { address }; updated filtering, RPC calls, and tests to convert StrictAddress via .into().
State CLI address strictness src/cli/subcommands/state_cmd.rs	Changed ReadState { actor_address } from String to StrictAddress; removed manual parsing/validation; pass .into() at request call site; trimmed unused imports.
Changelog CHANGELOG.md	Added unreleased note referencing stricter address checks in forest-cli subcommands (#6006).

Sequence Diagram(s)

sequenceDiagram
autonumber
participant OS as OS
participant CLI as forest-cli (bin)
participant Main as cli::main
participant Log as logger
participant RPC as rpc::Client
participant Node as Lotus RPC
OS->>CLI: start
rect rgba(200,220,255,0.25)
note right of CLI: Tokio multi_thread runtime
CLI->>Main: await main(args)
end
Main->>Log: setup_logger(...)
Main->>RPC: default_or_from_env(None)
Main->>Node: StateNetworkName.call()
Node-->>Main: NetworkName
Main->>Main: set CurrentNetwork if not Mainnet
Main->>Main: Cli::parse_from(args)
alt Subcommand: mpool pending/nonce
Main->>RPC: execute with StrictAddress (.into())
else Subcommand: state read-state
Main->>RPC: StateReadState.request(StrictAddress.into(), tipset)
else Subcommand: config
Main->>Main: run(&mut stdout)
end
Main-->>CLI: Result<()>
CLI-->>OS: exit code

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Replace String with Address type in mpool subcommands (to/from, nonce) [#6006]	✅
Replace String with Address type in state read-state actor_address [#6006]	✅
Audit all forest-cli subcommands for String address params [#6006]	❓	Only mpool and state commands are changed here; broader audit not evidenced.
Update related parsing/conversion logic [#6006]	✅
Ensure tests are updated accordingly [#6006]	✅

Assessment against linked issues: Out-of-scope changes

Code Change	Explanation
Convert binary entrypoint to Tokio async runtime (src/bin/forest-cli.rs: main function)	Runtime change is not specified in #6006, which focuses on address parameter types.
Early logger initialization and reordering (src/cli/main.rs: logger::setup_logger moved to start)	Logging setup changes are unrelated to replacing String with Address types.
Pre-parse network detection via tokenless RPC client (src/cli/main.rs: create client and call StateNetworkName before Cli::parse_from)	Network detection before CLI parse is outside #6006 scope.
Removal of manual runtime/block_on and restructuring control flow (src/cli/main.rs: async flow)	Structural async refactor is not part of address type replacements.

Possibly related PRs

• feat: add forest-cli mpool nonce + add RPC snapshot test #5999 — Introduces mpool nonce command and address handling; this PR modifies the same areas to use StrictAddress.
• feat: add --n-epochs to forest-cli state compute #5946 — Touches state_cmd.rs and actor address handling; closely related to the shift from String to typed addresses.

Suggested reviewers

• elmattic
• akaladarshi
• hanabi1224

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 36d1470 and 11ba58c.

📒 Files selected for processing (1)

• src/cli/main.rs (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/cli/main.rs

⏰ 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). (9)

• GitHub Check: Build Ubuntu
• GitHub Check: Build MacOS
• 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
• GitHub Check: Analyze (rust)
• GitHub Check: Analyze (go)

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch use-strict-address-in-cli

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[LesnyRumcajs]

@coderabbitai create a corresponding issue for the forest-wallet binary

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find forest-wallet related files to understand its functionality
echo "=== Searching for forest-wallet files ==="
fd -t f forest-wallet
echo -e "\n=== Searching for forest-wallet mentions in code ==="
rg -n --type rust "forest.wallet" -C 2
echo -e "\n=== Looking for wallet-related CLI or binary code ==="
rg -n --type rust "wallet.*cli\|cli.*wallet" -C 2
echo -e "\n=== Check if there's a separate wallet binary ==="
rg -n "bin.*wallet\|wallet.*bin" Cargo.toml -A 3 -B 3

Length of output: 2902

---

🏁 Script executed:

#!/bin/bash
# Let's examine the forest-wallet binary and its subcommands to understand what address-related functionality it has
echo "=== Examining forest-wallet binary ==="
cat src/bin/forest-wallet.rs
echo -e "\n=== Examining wallet subcommands structure ==="
cat src/wallet/subcommands/mod.rs
echo -e "\n=== Looking for wallet subcommands that handle addresses ==="
fd -t f . src/wallet/subcommands/ -x basename {} \; | head -20
echo -e "\n=== Checking wallet subcommands for address handling ==="
rg -n --type rust "address\|Address" src/wallet/ -C 1
echo -e "\n=== Checking Cargo.toml for forest-wallet binary definition ==="
rg -n "forest-wallet" Cargo.toml -A 5 -B 5

Length of output: 2044

---

🏁 Script executed:

#!/bin/bash
# Let's examine the actual wallet commands to see what address functionality exists
echo "=== Examining wallet commands ==="
cat src/wallet/subcommands/wallet_cmd.rs
echo -e "\n=== Looking for any other wallet-related files ==="
fd -t f . src/wallet/ | head -20
echo -e "\n=== Check the main wallet module ==="
ls -la src/wallet/
echo -e "\n=== Look at the wallet main function to understand the structure ==="
rg -n --type rust "mod.*wallet\|wallet.*mod" src/lib.rs -C 2
echo -e "\n=== Find wallet main implementation ==="
rg -n --type rust "forest_wallet_main" src/ -C 2

Length of output: 23115

---

🏁 Script executed:

#!/bin/bash
# Now let's look at the wallet main implementation to understand the current structure
echo "=== Examining wallet main implementation ==="
cat src/wallet/main.rs
echo -e "\n=== Check the wallet mod.rs ==="
cat src/wallet/mod.rs
echo -e "\n=== Look at the changes made to forest-cli for context ==="
rg -n --type rust "StrictAddress" src/cli/ -C 1 | head -30
echo -e "\n=== Check Cargo.toml for forest-wallet binary ==="
rg -n "\[\[bin\]\]" Cargo.toml -A 5 -B 1

Length of output: 3524

---

🏁 Script executed:

#!/bin/bash
# Let's examine the issue #6006 that this PR closes to understand the context better
echo "=== Looking for issue references or TODO comments about wallet address validation ==="
rg -n "6006\|address.*valid\|strict.*address" --type rust -C 2
echo -e "\n=== Let's check what the current forest-wallet address handling looks like ==="
# Looking at the key commands that handle addresses in wallet
echo "Key address handling in wallet commands:"
echo "- Balance: uses String -> StrictAddress::from_str"
echo "- Export: uses String -> StrictAddress::from_str" echo "- Has: uses String -> StrictAddress::from_str"
echo "- Delete: uses String -> StrictAddress::from_str"
echo "- SetDefault: uses String -> StrictAddress::from_str"
echo "- Sign: uses String -> StrictAddress::from_str"
echo "- Verify: uses String -> StrictAddress::from_str"
echo "- Send: uses String -> StrictAddress::from_str for both from and target_address"
echo -e "\n=== Let's also check what changes were made to forest-cli to understand the pattern ==="
rg -n "StrictAddress" src/cli/ --type rust | head -10

Length of output: 2410

---

🏁 Script executed:

#!/bin/bash
# Now I understand the pattern. Let's create the GitHub issue for forest-wallet
echo "=== Creating GitHub issue for forest-wallet strict address validation ==="
gh issue create \
--title "feat: implement strict address validation in forest-wallet CLI" \
--body "## Summary
Similar to the improvements made to \`forest-cli\` in PR #6011, the \`forest-wallet\` binary should implement stricter address validation to improve user experience and prevent invalid address inputs.
## Background
PR #6011 implemented stricter checks for address arguments in \`forest-cli\` by:
- Using typed addresses (\`StrictAddress\`) instead of raw strings for safer input handling
- Adding early network detection before CLI parsing to ensure correct network context during validation
- Migrating CLI to an asynchronous runtime for improved responsiveness
## Current State
The \`forest-wallet\` binary currently handles addresses in the following commands but uses runtime string-to-address conversion instead of compile-time type safety:
- \`balance\`: address parameter
- \`export\`: address parameter - \`has\`: key parameter (address)
- \`delete\`: address parameter
- \`set-default\`: key parameter (address)
- \`sign\`: address parameter
- \`verify\`: address parameter
- \`send\`: from and target_address parameters
All these currently accept \`String\` parameters and convert using \`StrictAddress::from_str()\` at runtime.
## Proposed Changes
1. **Update command definitions**: Change string parameters to \`StrictAddress\` types for all address-handling commands
2. **Add early network detection**: Implement similar network detection logic as in \`forest-cli\` before CLI parsing
3. **Migrate to async runtime**: Follow the same pattern as \`forest-cli\` for consistent architecture
## Commands to Update
- \`Balance { address: StrictAddress }\`
- \`Export { address: StrictAddress }\`
- \`Has { key: StrictAddress }\`
- \`Delete { address: StrictAddress }\`
- \`SetDefault { key: StrictAddress }\`
- \`Sign { address: StrictAddress }\`
- \`Verify { address: StrictAddress }\`
- \`Send { from: Option<StrictAddress>, target_address: StrictAddress }\`
## Benefits
- Early validation of address arguments before command execution
- Better error messages for invalid addresses
- Consistent user experience across Forest CLI tools
- Reduced runtime errors due to invalid address formats
## References
- Related PR: #6011
- Original issue: #6006
- Requester: @LesnyRumcajs" \
--assignee LesnyRumcajs

Length of output: 2490

---

@LesnyRumcajs I've successfully created issue #6012 for implementing strict address validation in the forest-wallet binary.

The issue documents the need to apply the same stricter address validation improvements that were implemented in forest-cli (PR #6011) to the forest-wallet binary. This includes:

• 8 commands that currently use String parameters for addresses and need to be updated to use StrictAddress types
• Early network detection before CLI parsing for proper validation context
• Async runtime migration for consistency with forest-cli

The issue references this PR (#6011) and the original issue (#6006) for context, and I've assigned it to you as requested.

🐰✨

---

🧠 Learnings used

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.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (5)

CHANGELOG.md (1)

32-32: Reword entry for clarity and consistency.

Prefer “Stricter …” and reference the actual change (StrictAddress), aligning tone with nearby entries.

-- [#6006](https://github.com/ChainSafe/forest/issues/6006) More strict checks for the address arguments in the `forest-cli` subcommands.+- [#6006](https://github.com/ChainSafe/forest/issues/6006) Stricter validation for address arguments in `forest-cli` subcommands (use `StrictAddress`).

src/cli/subcommands/state_cmd.rs (2)

36-39: Public CLI surface change: document/test network-prefix expectations.

actor_address: StrictAddress now depends on CurrentNetwork. Ensure help text or docs mention expected prefixes (f… for mainnet, t… for testnets) and add a test that fails on mismatched prefixes.

---

76-82: Timeout choice: reconsider Duration::MAX.

An unbounded read-state can hang indefinitely. Consider a reasonable default (e.g., env-configurable) with a clear error on timeout.

src/cli/subcommands/mpool_cmd.rs (2)

49-70: Precompute filter keys and simplify predicates.

Avoid repeated conversions inside the hot filter loop and use map_or for readability.

 fn filter_messages(
messages: Vec<SignedMessage>,
local_addrs: Option<HashSet<Address>>,
- to: &Option<StrictAddress>,- from: &Option<StrictAddress>,+ to: &Option<StrictAddress>,+ from: &Option<StrictAddress>,
) -> anyhow::Result<Vec<SignedMessage>> {
use crate::message::Message;
- let filtered = messages+ // Precompute once outside the loop+ let to_addr: Option<Address> = to.as_ref().map(|a| (*a).into());+ let from_addr: Option<Address> = from.as_ref().map(|a| (*a).into());++ let filtered = messages
.into_iter()
.filter(|msg| {
local_addrs
.as_ref()
.map(|addrs| addrs.contains(&msg.from()))
.unwrap_or(true)
- && to.map(|addr| msg.to() == addr.into()).unwrap_or(true)- && from.map(|addr| msg.from() == addr.into()).unwrap_or(true)+ && to_addr.map_or(true, |addr| msg.to() == addr)+ && from_addr.map_or(true, |addr| msg.from() == addr)
})
.collect();
Ok(filtered)
}

---

425-432: Minor: prefer mainnet/testnet-agnostic fixtures.

Using t… literals is fine for unit tests, but consider constructing via CurrentNetwork to avoid coupling to a default. Not blocking.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 15cdbb3 and 3726d55.

📒 Files selected for processing (5)

• CHANGELOG.md (1 hunks)
• src/bin/forest-cli.rs (1 hunks)
• src/cli/main.rs (1 hunks)
• src/cli/subcommands/mpool_cmd.rs (6 hunks)
• src/cli/subcommands/state_cmd.rs (3 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 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.

📚 Learning: 2025-08-08T12:10:45.218Z

Learnt from: hanabi1224
PR: ChainSafe/forest#5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.

Applied to files:

• src/bin/forest-cli.rs

🧬 Code graph analysis (3)

src/bin/forest-cli.rs (1)

src/cli/main.rs (1)

• main (14-51)

src/cli/subcommands/mpool_cmd.rs (3)

src/message/chain_message.rs (1)

• to (53-58)

src/message/signed_message.rs (1)

• to (107-109)

src/shim/address.rs (13)

• from (311-313)
• from (317-319)
• from (323-325)
• from (329-331)
• from (335-337)
• from (341-343)
• from (347-353)
• from (357-359)
• from (363-369)
• from (373-375)
• from (379-381)
• from (385-387)
• from (391-395)

src/cli/main.rs (5)

src/bin/forest-cli.rs (1)

• main (5-7)

src/wallet/main.rs (1)

• main (13-39)

src/rpc/client.rs (3)

• client (114-114)
• default_or_from_env (46-77)
• call (92-151)

src/shim/address.rs (4)

• from_str (178-183)
• from_str (299-302)
• set_global (66-69)
• default (122-124)

src/cli_shared/logger/mod.rs (1)

• setup_logger (21-115)

⏰ 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). (9)

• GitHub Check: cargo-publish-dry-run
• GitHub Check: Build Ubuntu
• GitHub Check: tests-release
• GitHub Check: Build MacOS
• GitHub Check: tests
• GitHub Check: Build forest binaries on Linux AMD64
• GitHub Check: All lint checks
• GitHub Check: Analyze (rust)
• GitHub Check: Analyze (go)

🔇 Additional comments (5)

src/bin/forest-cli.rs (1)

4-6: Async entrypoint migration looks good.

Top-level Tokio runtime + await on forest_main avoids nested runtimes. No issues spotted.

src/cli/main.rs (1)

32-50: Consider honoring user-provided logger options.

Passing CliOpts::default() ignores any user-specified logging options. If CLI exposes such options, parse and pass them; otherwise, confirm this is intentional.

src/cli/subcommands/state_cmd.rs (1)

7-7: Import reduction and StrictAddress usage are appropriate.

Switching to StrictAddress tightens validation at parse-time and simplifies downstream conversions.

src/cli/subcommands/mpool_cmd.rs (2)

28-37: CLI types tightened to StrictAddress — good upgrade.

This enforces address validity at parse time and removes ad-hoc parsing.

---

271-273: Nonce call conversion is correct.

Passing address.into() keeps RPC surface unchanged while enforcing strict parsing at the CLI boundary.