Skip to content

Conversation

@hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Dec 12, 2025

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

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

  • Refactor
    • Modernized the internal execution framework for forest-tool and forest-wallet to enhance application responsiveness and optimize resource utilization through improved asynchronous processing patterns.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Walkthrough

This pull request converts multiple synchronous entry points to asynchronous ones, leveraging Tokio's async runtime. The binary entry points adopt the #[tokio::main(flavor = "multi_thread")] attribute, while the internal library main functions become async without explicit runtime construction, relying on async/await patterns instead of block_on.

Changes

Cohort / File(s) Summary
Binary Entry Points
src/bin/forest-tool.rs, src/bin/forest-wallet.rs
Converted synchronous main functions to async with #[tokio::main(flavor = "multi_thread")] attribute. Updated function signatures and call sites to use .await when invoking the inner async functions.
Library Main Functions
src/tool/main.rs, src/wallet/main.rs
Changed from synchronous to async function signatures. Removed explicit Tokio runtime construction and block_on usage. Updated subcommand dispatch to directly await async operations instead of blocking.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Changes follow a consistent pattern: converting sync entry points to async via #[tokio::main] attribute or async function signatures
  • All modifications are mechanical (signature updates, attribute addition, .await insertion)
  • No complex logic changes or control flow restructuring
  • Repetitive refactoring across similar files reduces reasoning burden per file

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting forest-tool and forest-wallet binaries to use Tokio's #[tokio::main] macro instead of manual runtime setup.
✨ 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/tokio-rt-macro

📜 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 8093469 and b7c6fed.

📒 Files selected for processing (4)
  • src/bin/forest-tool.rs (1 hunks)
  • src/bin/forest-wallet.rs (1 hunks)
  • src/tool/main.rs (2 hunks)
  • src/wallet/main.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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-08-28T12:52:46.927Z
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.

Applied to files:

  • src/wallet/main.rs
  • src/bin/forest-tool.rs
  • src/tool/main.rs
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 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-tool.rs
  • src/bin/forest-wallet.rs
📚 Learning: 2025-08-04T13:36:22.993Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.

Applied to files:

  • src/bin/forest-tool.rs
  • src/bin/forest-wallet.rs
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.

Applied to files:

  • src/bin/forest-tool.rs
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.

Applied to files:

  • src/bin/forest-tool.rs
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.

Applied to files:

  • src/bin/forest-tool.rs
🧬 Code graph analysis (4)
src/wallet/main.rs (2)
src/cli/main.rs (1)
  • main (14-50)
src/shim/address.rs (1)
  • set_global (66-69)
src/bin/forest-tool.rs (5)
src/bin/forest-wallet.rs (1)
  • main (5-7)
src/tool/main.rs (1)
  • main (11-38)
src/wallet/main.rs (1)
  • main (13-34)
src/bin/forest-cli.rs (1)
  • main (5-7)
src/bin/forest.rs (1)
  • main (4-6)
src/tool/main.rs (5)
src/bin/forest-tool.rs (1)
  • main (5-7)
src/bin/forest-wallet.rs (1)
  • main (5-7)
src/wallet/main.rs (1)
  • main (13-34)
src/cli/main.rs (1)
  • main (14-50)
src/daemon/main.rs (1)
  • main (26-71)
src/bin/forest-wallet.rs (6)
src/bin/forest-tool.rs (1)
  • main (5-7)
src/tool/main.rs (1)
  • main (11-38)
src/wallet/main.rs (1)
  • main (13-34)
src/cli/main.rs (1)
  • main (14-50)
src/bin/forest-cli.rs (1)
  • main (5-7)
src/bin/forest.rs (1)
  • main (4-6)
⏰ 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: tests-release
  • GitHub Check: tests
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Coverage
  • GitHub Check: All lint checks
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: Build Ubuntu
  • GitHub Check: Build MacOS
🔇 Additional comments (4)
src/wallet/main.rs (1)

13-33: Async conversion is clean and properly awaited at the call site. The binary entry point in src/bin/forest-wallet.rs correctly uses #[tokio::main] and awaits the call to forest_wallet_main() on line 6.

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

4-6: The implementation is correct; no action needed. Tokio is configured with the full feature set (which includes macros and rt-multi-thread), and forest::forest_tool_main is properly defined as pub async fn main in src/tool/main.rs:11. The #[tokio::main(flavor = "multi_thread")] macro and .await call will compile and work as intended.

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

4-6: No issues found. The tokio macro entrypoint is correct—forest_wallet_main is async and tokio features ('full') are enabled in Cargo.toml, which includes the required macros feature for the #[tokio::main] attribute.

src/tool/main.rs (1)

11-37: All async dispatch signatures verified as correct. Backup and Completion are properly non-async, returning anyhow::Result<()> synchronously, while all other variants are async and correctly awaited in the main dispatch. Return types are consistently anyhow::Result<()> across all variants.


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

@hanabi1224 hanabi1224 marked this pull request as ready for review December 12, 2025 15:54
@hanabi1224 hanabi1224 requested a review from a team as a code owner December 12, 2025 15:54
@hanabi1224 hanabi1224 requested review from LesnyRumcajs and sudo-shashank and removed request for a team December 12, 2025 15:54
@hanabi1224 hanabi1224 enabled auto-merge December 12, 2025 16:02
@hanabi1224 hanabi1224 added this pull request to the merge queue Dec 12, 2025
@codecov
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.10%. Comparing base (8093469) to head (b7c6fed).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/tool/main.rs 43.75% 9 Missing ⚠️
src/wallet/main.rs 0.00% 7 Missing ⚠️
src/bin/forest-wallet.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/bin/forest-tool.rs 100.00% <100.00%> (ø)
src/bin/forest-wallet.rs 0.00% <0.00%> (ø)
src/wallet/main.rs 0.00% <0.00%> (ø)
src/tool/main.rs 56.52% <43.75%> (-2.74%) ⬇️

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8093469...b7c6fed. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Merged via the queue into main with commit 6a29d19 Dec 12, 2025
50 checks passed
@hanabi1224 hanabi1224 deleted the hm/tokio-rt-macro branch December 12, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants