Skip to content

Add support for the Filecoin.EthGetBalance V2#6403

Merged
sudo-shashank merged 4 commits intomainfrom
shashank/eth-get-balance-v2
Jan 12, 2026
Merged

Add support for the Filecoin.EthGetBalance V2#6403
sudo-shashank merged 4 commits intomainfrom
shashank/eth-get-balance-v2

Conversation

@sudo-shashank
Copy link
Contributor

@sudo-shashank sudo-shashank commented Jan 12, 2026

Summary of changes

Changes introduced in this pull request:

  • Impl Filecoin.EthGetBalance V2 and added test.

Reference issue to close (if applicable)

Closes #6293

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.EthGetBalance API v2 endpoint for retrieving Ethereum-compatible account balances.
    • V2 supports flexible block identification (block numbers and block hashes).
  • Documentation

    • Changelog updated with the new v2 entry and descriptive return information.
  • Tests

    • Expanded test coverage and added a snapshot exercising various blockParam input modes for the v2 API.

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

@sudo-shashank sudo-shashank added the RPC requires calibnet RPC checks to run on CI label Jan 12, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

Walkthrough

Adds a V2 RPC method for Filecoin.EthGetBalance (accepting ExtBlockNumberOrHash) and refactors shared balance lookup into a new helper used by both v1 and v2. Registers the new method, expands tests for v2, and updates snapshots and changelog.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.md
Adds an Added-section entry documenting Filecoin.EthGetBalance for API v2.
RPC Method Implementation
src/rpc/methods/eth.rs
Adds EthGetBalanceV2 RPC method (NAME "Filecoin.EthGetBalance") with v2-specific params and handler; extracts common balance logic into new eth_get_balance helper; updates v1 EthGetBalance to call the helper; adds DESCRIPTION metadata.
RPC Registration
src/rpc/mod.rs
Registers EthGetBalanceV2 in the for_each_rpc_method macro (eth vertical).
Tests & Snapshots
src/tool/subcommands/api_cmd/api_compare_tests.rs, test_snapshots.txt
Adds multiple EthGetBalanceV2 test cases covering various ExtBlockNumberOrHash inputs and block-resolution modes; adds corresponding snapshot filename to test_snapshots.txt.

Sequence Diagram(s)

sequenceDiagram participant Client participant RPC_Server participant TipsetResolver participant StateStore participant ActorLookup Client->>RPC_Server: filecoin_eth_getBalance(address, blockParam) (v2) RPC_Server->>TipsetResolver: resolve blockParam (tipset_by_block_number_or_hash_v2) TipsetResolver-->>RPC_Server: tipset / block state RPC_Server->>StateStore: construct state at tipset StateStore->>ActorLookup: load actor by converted EthAddress ActorLookup-->>StateStore: actor (or none) StateStore-->>RPC_Server: balance (bigint) or zero RPC_Server-->>Client: EthBigInt(balance) 
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • akaladarshi
  • hanabi1224
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding V2 support for Filecoin.EthGetBalance RPC method.
Linked Issues check ✅ Passed The PR successfully implements Filecoin.EthGetBalance V2 with proper RPC method registration, helper function refactoring, test coverage, and changelog documentation as required by issue #6293.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing Filecoin.EthGetBalance V2: RPC method definition, helper function extraction, test cases, and changelog updates are all within scope of the linked issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

@sudo-shashank sudo-shashank marked this pull request as ready for review January 12, 2026 04:42
@sudo-shashank sudo-shashank requested a review from a team as a code owner January 12, 2026 04:42
@sudo-shashank sudo-shashank requested review from akaladarshi and hanabi1224 and removed request for a team January 12, 2026 04:42
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

🤖 Fix all issues with AI agents
In @src/tool/subcommands/api_cmd/api_compare_tests.rs: - Around line 1645-1715: Add an `offline` parameter to eth_tests_with_tipset and gate the ExtPredefined::Safe and ExtPredefined::Finalized EthGetBalanceV2 tests behind `if !offline`; specifically, update the eth_tests_with_tipset signature to accept the same offline boolean used by chain_tests_with_tipset, propagate the caller’s offline value into eth_tests_with_tipset, and wrap the RpcTest::basic requests that use ExtPredefined::Safe and ExtPredefined::Finalized so they are only pushed when `!offline` (similar to the gating in chain_tests_with_tipset); reference functions/types: eth_tests_with_tipset, chain_tests_with_tipset, EthGetBalanceV2::request, ExtPredefined::Safe, ExtPredefined::Finalized, and get_latest_finalized_tipset for consistency with existing finality handling. 
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5547983 and 2183a3c.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/rpc/methods/eth.rs
  • src/rpc/mod.rs
  • src/tool/subcommands/api_cmd/api_compare_tests.rs
  • src/tool/subcommands/api_cmd/test_snapshots.txt
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-01-05T12:54:40.850Z
Learnt from: hanabi1224 Repo: ChainSafe/forest PR: 6381 File: src/lotus_json/actors/states/cron_state.rs:8-8 Timestamp: 2026-01-05T12:54:40.850Z Learning: In Rust code reviews, do not derive Eq for a struct if any field does not implement Eq (e.g., types from external dependencies). If a type like CronStateLotusJson includes fields wrapping external dependencies that lack Eq, derive PartialEq (or implement PartialEq manually) but avoid deriving Eq. This ensures comparisons compile and reflect actual equivalence semantics. When needed, consider implementing custom PartialEq (and possibly Eq) only after ensuring all fields (or wrappers) implement Eq, or keep PartialEq-only if full equality semantics cannot be expressed. 

Applied to files:

  • src/rpc/mod.rs
  • src/rpc/methods/eth.rs
  • src/tool/subcommands/api_cmd/api_compare_tests.rs
📚 Learning: 2026-01-05T12:56:13.802Z
Learnt from: hanabi1224 Repo: ChainSafe/forest PR: 6381 File: src/lotus_json/actors/states/evm_state.rs:41-44 Timestamp: 2026-01-05T12:56:13.802Z Learning: In Rust codebases (e.g., Forest), do not add #[cfg(test)] to functions already annotated with #[test]. The #[test] attribute ensures the function is compiled only for tests, so a separate #[cfg(test)] is redundant and can be removed if present. Apply this check to all Rust files that contain #[test] functions. 

Applied to files:

  • src/rpc/mod.rs
  • src/rpc/methods/eth.rs
  • src/tool/subcommands/api_cmd/api_compare_tests.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-08-25T14:11:10.790Z
Learnt from: hanabi1224 Repo: ChainSafe/forest PR: 5978 File: .github/workflows/unit-tests.yml:86-96 Timestamp: 2025-08-25T14:11:10.790Z Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files. 

Applied to files:

  • src/tool/subcommands/api_cmd/test_snapshots.txt
📚 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/eth.rs
🧬 Code graph analysis (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (1)
src/rpc/methods/eth.rs (11)
  • from_str (285-287)
  • from_str (393-404)
  • from_str (453-472)
  • from_block_number (366-368)
  • from_block_number (426-428)
  • from_block_number_object (377-381)
  • from_block_number_object (437-441)
  • from_block_hash_object (386-391)
  • from_block_hash_object (446-451)
  • from_predefined (362-364)
  • from_predefined (422-424)
⏰ 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). (3)
  • GitHub Check: Coverage
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: tests-release
🔇 Additional comments (6)
CHANGELOG.md (1)

40-41: LGTM!

The changelog entry follows the established format and is correctly placed in the "Added" section alongside other API v2 implementations.

src/tool/subcommands/api_cmd/test_snapshots.txt (1)

62-62: LGTM!

The new snapshot file for EthGetBalanceV2 follows the established naming convention and is correctly placed in alphabetical order. Based on learnings, these snapshots are downloaded from the DigitalOcean bucket when needed for testing.

src/rpc/mod.rs (1)

112-113: LGTM!

The EthGetBalanceV2 method is correctly registered in the for_each_rpc_method macro, following the established pattern where V2 methods are placed immediately after their V1 counterparts. This is consistent with other V2 methods like EthCallV2, EthEstimateGasV2, EthFeeHistoryV2, and EthGetTransactionCountV2.

src/rpc/methods/eth.rs (3)

871-882: Nice dedup: EthGetBalance now delegates to shared helper.


885-905: EthGetBalanceV2 wiring looks consistent (V2 path + ExtBlockNumberOrHash + shared helper).


907-916: Please verify “missing account” semantics (error vs zero) match Lotus/Ethereum expectations.

eth_get_balance currently uses state.get_required_actor(&fil_addr)?;, so a non-existent actor becomes an error. If Lotus returns 0x0 for unknown accounts (Ethereum common behavior), Forest would diverge.

Possible adjustment (only if Lotus expects “0 for missing”)
 fn eth_get_balance<DB: Blockstore>( ctx: &Ctx<DB>, address: &EthAddress, ts: &Tipset, ) -> Result<EthBigInt> { let fil_addr = address.to_filecoin_address()?; let state = StateTree::new_from_root(ctx.store_owned(), ts.parent_state())?; - let actor = state.get_required_actor(&fil_addr)?; - Ok(EthBigInt(actor.balance.atto().clone())) + match state.get_actor(&fil_addr)? { + Some(actor) => Ok(EthBigInt(actor.balance.atto().clone())), + None => Ok(EthBigInt(BigInt::zero())), + } }
@codecov
Copy link

codecov bot commented Jan 12, 2026

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.51%. Comparing base (5547983) to head (158b2eb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/rpc/methods/eth.rs 71.42% 0 Missing and 6 partials ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/rpc/mod.rs 24.47% <ø> (ø)
src/rpc/methods/eth.rs 66.87% <71.42%> (+0.10%) ⬆️

... and 9 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 5547983...158b2eb. Read the comment docs.

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

@sudo-shashank One small question, should we specify in the description if the API is V2 or not, I'm not sure WDYT

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

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2183a3c and 158b2eb.

📒 Files selected for processing (1)
  • src/rpc/methods/eth.rs
🧰 Additional context used
🧠 Learnings (3)
📚 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/eth.rs
📚 Learning: 2026-01-05T12:54:40.850Z
Learnt from: hanabi1224 Repo: ChainSafe/forest PR: 6381 File: src/lotus_json/actors/states/cron_state.rs:8-8 Timestamp: 2026-01-05T12:54:40.850Z Learning: In Rust code reviews, do not derive Eq for a struct if any field does not implement Eq (e.g., types from external dependencies). If a type like CronStateLotusJson includes fields wrapping external dependencies that lack Eq, derive PartialEq (or implement PartialEq manually) but avoid deriving Eq. This ensures comparisons compile and reflect actual equivalence semantics. When needed, consider implementing custom PartialEq (and possibly Eq) only after ensuring all fields (or wrappers) implement Eq, or keep PartialEq-only if full equality semantics cannot be expressed. 

Applied to files:

  • src/rpc/methods/eth.rs
📚 Learning: 2026-01-05T12:56:13.802Z
Learnt from: hanabi1224 Repo: ChainSafe/forest PR: 6381 File: src/lotus_json/actors/states/evm_state.rs:41-44 Timestamp: 2026-01-05T12:56:13.802Z Learning: In Rust codebases (e.g., Forest), do not add #[cfg(test)] to functions already annotated with #[test]. The #[test] attribute ensures the function is compiled only for tests, so a separate #[cfg(test)] is redundant and can be removed if present. Apply this check to all Rust files that contain #[test] functions. 

Applied to files:

  • src/rpc/methods/eth.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: cargo-publish-dry-run
  • GitHub Check: rubocop
  • GitHub Check: Build Ubuntu
  • GitHub Check: Build MacOS
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: All lint checks
  • GitHub Check: Coverage
  • GitHub Check: tests-release
  • GitHub Check: Check
🔇 Additional comments (3)
src/rpc/methods/eth.rs (3)

867-868: LGTM!

The DESCRIPTION constant addition is appropriate and consistent with other RPC methods in the file.


882-883: LGTM!

Clean refactor to use the shared helper function, maintaining the same behavior while enabling code reuse with V2.


887-909: LGTM!

The V2 implementation correctly follows the established pattern for other V2 methods in this codebase:

  • Uses ExtBlockNumberOrHash which supports safe and finalized block tags
  • Uses the async tipset_by_block_number_or_hash_v2 for proper handling of extended predefined blocks
  • Shares the common eth_get_balance helper with V1
  • API paths correctly restricted to V2 only
@sudo-shashank
Copy link
Contributor Author

sudo-shashank commented Jan 12, 2026

@sudo-shashank One small question, should we specify in the description if the API is V2 or not, I'm not sure WDYT

Not required I feel, we can add it later if required for all the V2 API

@sudo-shashank sudo-shashank added this pull request to the merge queue Jan 12, 2026
Merged via the queue into main with commit 7d9b1b1 Jan 12, 2026
51 checks passed
@sudo-shashank sudo-shashank deleted the shashank/eth-get-balance-v2 branch January 12, 2026 07:25
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

2 participants