Skip to content

Add support for the Filecoin.EthGetBlockByNumber V2#6404

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

Add support for the Filecoin.EthGetBlockByNumber V2#6404
sudo-shashank merged 4 commits intomainfrom
shashank/eth-get-block-by-number-v2

Conversation

@sudo-shashank
Copy link
Copy Markdown
Contributor

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

Summary of changes

Changes introduced in this pull request:

  • Impl Filecoin.EthGetBlockByNumber V2 and added test.

Reference issue to close (if applicable)

Closes #6294

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

    • Introduced Filecoin.EthGetBlockByNumber / eth_getBlockByNumber endpoint for API v2 to fetch blocks by number or hash with optional full transaction info.
  • Tests

    • Added comprehensive tests covering v2 behavior for various block identifiers and parameter combinations.
  • Changelog

    • Added an unreleased entry noting the new API v2 endpoint and included updated test snapshot.

✏️ 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
Copy Markdown
Contributor

coderabbitai Bot commented Jan 12, 2026

Walkthrough

Implements a new EthGetBlockByNumberV2 RPC method variant under API v2, registers it with the RPC macro, adds tests and snapshot, and records the change in the changelog.

Changes

Cohort / File(s) Summary
Core RPC implementation
src/rpc/methods/eth.rs, src/rpc/mod.rs
Add EthGetBlockByNumberV2 implementing RpcMethod<2> (same public API as v1) using ApiPaths::V2 and tipset_by_block_number_or_hash_v2; register method in RPC macro.
Tests & snapshots
src/tool/subcommands/api_cmd/api_compare_tests.rs, src/tool/subcommands/api_cmd/test_snapshots.txt
Add identity and basic tests for EthGetBlockByNumberV2 (various block tags and fullTxInfo flags) and insert corresponding snapshot file reference.
Changelog
CHANGELOG.md
Add entry under Forest unreleased: Implemented Filecoin.EthGetBlockByNumber for API v2.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RPC as RPC Server
    participant Tipset as TipsetResolverV2
    participant Builder as BlockBuilder

    Client->>RPC: Filecoin.EthGetBlockByNumber (V2) request
    RPC->>Tipset: tipset_by_block_number_or_hash_v2(params)
    Tipset-->>RPC: Tipset (or not found / error)
    RPC->>Builder: block_from_filecoin_tipset(tipset, fullTxInfo)
    Builder-->>RPC: Block (Ethereum-style)
    RPC-->>Client: JSON-RPC response with Block
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • LesnyRumcajs
  • akaladarshi
🚥 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 accurately describes the main change: adding V2 support for Filecoin.EthGetBlockByNumber RPC method, matching the actual code modifications across multiple files.
Linked Issues check ✅ Passed The PR implements Filecoin.EthGetBlockByNumber for RPC v2 as required by issue #6294, adding the method implementation, registration, tests, and documentation.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing EthGetBlockByNumberV2: method implementation, registration, tests, test snapshots, and changelog updates—no unrelated modifications detected.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8447dcc and 95d9651.

📒 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
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/tool/subcommands/api_cmd/test_snapshots.txt
  • src/rpc/mod.rs
  • src/tool/subcommands/api_cmd/api_compare_tests.rs
🧰 Additional context used
🧠 Learnings (2)
📚 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). (8)
  • GitHub Check: Coverage
  • GitHub Check: rubocop
  • GitHub Check: tests-release
  • GitHub Check: Build Ubuntu
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Build MacOS
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: All lint checks
🔇 Additional comments (2)
CHANGELOG.md (1)

42-43: LGTM!

The changelog entry is well-formatted and correctly documents the implementation of Filecoin.EthGetBlockByNumber for API v2. It follows the established pattern of similar entries and is appropriately placed in chronological order within the "Added" section.

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

1623-1643: LGTM!

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

  • Uses tipset_by_block_number_or_hash_v2 which properly handles Safe and Finalized predefined blocks through ChainGetTipSetV2 async methods
  • API paths correctly restricted to V2 with make_bitflags!(ApiPaths::V2)
  • Parameter and return types match the non-V2 version appropriately
  • Consistent with other V2 implementations (EthGetBalanceV2, EthCallV2, EthEstimateGasV2, EthFeeHistoryV2, EthGetTransactionCountV2)

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 05:39
@sudo-shashank sudo-shashank requested a review from a team as a code owner January 12, 2026 05:39
@sudo-shashank sudo-shashank requested review from LesnyRumcajs and hanabi1224 and removed request for a team January 12, 2026 05:39
Copy link
Copy Markdown
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 5547983 and 8447dcc.

📒 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 (3)
📚 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: 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/tool/subcommands/api_cmd/api_compare_tests.rs
  • 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/mod.rs
  • src/tool/subcommands/api_cmd/api_compare_tests.rs
  • src/rpc/methods/eth.rs
🧬 Code graph analysis (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (5)
src/rpc/reflect/mod.rs (1)
  • request (288-298)
src/rpc/client.rs (1)
  • request (258-271)
src/rpc/methods/eth.rs (4)
  • from_block_number (366-368)
  • from_block_number (426-428)
  • from_predefined (362-364)
  • from_predefined (422-424)
src/networks/mod.rs (1)
  • epoch (509-522)
src/blocks/tipset.rs (2)
  • epoch (310-312)
  • epoch (525-527)
⏰ 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: tests-release
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Coverage
🔇 Additional comments (4)
CHANGELOG.md (1)

40-41: LGTM!

The changelog entry follows the established format and is correctly placed under the "Added" section, consistent with other API v2 implementations.

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

64-64: LGTM!

The snapshot entry follows the established naming convention for v2 methods and is correctly placed in alphabetical order after the v1 variant.

src/rpc/mod.rs (1)

115-115: Looks good! The registration of EthGetBlockByNumberV2 follows the established pattern, placing the V2 method immediately after its V1 counterpart. The type is properly defined as a public enum in src/rpc/methods/eth.rs.

src/tool/subcommands/api_cmd/api_compare_tests.rs (1)

1695-1744: Policy choices are consistent with existing V1 tests—no refactoring needed.

The added EthGetBlockByNumberV2 cases correctly mirror the V1 EthGetBlockByNumber tests, including identical policy choices for Safe/Finalized (using basic). V1 tests already use this same pattern without special gating, and there are no open issues or comments suggesting Safe/Finalized is environment-dependent or F3-exclusive. The F3-related gating in the file applies only to F3-specific RPC methods, not to block selection via Safe/Finalized tags. The policy is appropriate and follows established patterns.

Comment thread src/rpc/methods/eth.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 12, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.53%. Comparing base (7d9b1b1) to head (95d9651).
⚠️ Report is 1 commits behind head on main.

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

... and 6 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 7d9b1b1...95d9651. Read the comment docs.

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

@sudo-shashank sudo-shashank added this pull request to the merge queue Jan 12, 2026
Merged via the queue into main with commit 90579f9 Jan 12, 2026
51 checks passed
@sudo-shashank sudo-shashank deleted the shashank/eth-get-block-by-number-v2 branch January 12, 2026 09:33
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RPC v2] Filecoin.EthGetBlockByNumber

2 participants