Skip to content

chore: use U256 for EthBigInt#7201

Merged
LesnyRumcajs merged 1 commit into
mainfrom
eth-big-int-type
Jun 19, 2026
Merged

chore: use U256 for EthBigInt#7201
LesnyRumcajs merged 1 commit into
mainfrom
eth-big-int-type

Conversation

@LesnyRumcajs

@LesnyRumcajs LesnyRumcajs commented Jun 19, 2026

Copy link
Copy Markdown
Member

Summary of changes

Changes introduced in this pull request:

  • use U256 as an underlying type for EthBigInt - this is better because there's no heap allocation involved there (U256 vs sign + vec)

Reference issue to close (if applicable)

Closes #4436

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.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

  • Refactor

    • Updated internal representation of RPC integer types for improved compatibility and consistency across financial value responses (base fees, gas prices, balances, and transaction fees).
  • Bug Fixes

    • Refined gas price validation logic to ensure accurate detection of non-zero values in RPC responses.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner June 19, 2026 14:28
@LesnyRumcajs LesnyRumcajs requested review from hanabi1224 and sudo-shashank and removed request for a team June 19, 2026 14:28
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

EthBigInt is migrated from a num_bigint::BigInt-backed wrapper to an ethereum_types::U256-backed wrapper. Bidirectional From conversions between EthBigInt, BigInt, and TokenAmount are added. All construction sites across eth RPC handlers and trace submodules are updated to use the new From/into() API.

Changes

EthBigInt U256 migration

Layer / File(s) Summary
EthBigInt type redefinition and From impls
src/rpc/methods/eth.rs
Removes the BigInt-backed EthBigInt definition with custom serde/get-size logic, replaces it with a U256-backed wrapper, and adds From<BigInt>, From<&BigInt>, From<u64>, From<EthBigInt> for BigInt, and From<EthBigInt> for TokenAmount impls. Removes the big_int_heap_size_helper import and TODO comment.
RPC handler construction sites
src/rpc/methods/eth.rs, src/rpc/methods/eth/types.rs, src/tool/subcommands/api_cmd/api_compare_tests.rs
Updates eth_baseFee, BaseFeeByHeight, eth_gasPrice, eth_getBalance, receipt gas fee/cap, reward-percentile premium, and eth_maxPriorityFeePerGas to use .into() or EthBigInt::from(...). Updates EthCallMessageMessage value conversion and changes the EthGasPrice API test to check non-zero instead of positive.
RPC handler serde test
src/rpc/methods/eth.rs
Updates gas_price_result_serde_roundtrip to construct EthBigInt via U256::from(i) and assert equality after encode/decode.
Geth trace module migration
src/rpc/methods/eth/trace/geth.rs
Updates build_account_snapshot_from_entries and all prestate/diff-mode balance assertions in tests to use EthBigInt::from(...). Removes unused num::BigInt test import.
State diff module migration
src/rpc/methods/eth/trace/state_diff.rs
Updates build_account_diff and all balance diff assertions in unit and EVM actor scenario tests to use EthBigInt::from(...). Removes num::BigInt test import.
Trace types test migration
src/rpc/methods/eth/trace/types.rs
Migrates all EthBigInt construction sites in AccountState, AccountDiff, StateDiff, and EthTraceGethCallFrame tests/helpers to EthBigInt::from(...). Removes num_bigint::BigInt test import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • ChainSafe/forest#6289: Directly touches EthBigInt in src/rpc/methods/eth.rs, adding GetSize impls that are now removed by this PR's migration to U256.
  • ChainSafe/forest#6412: Modifies the same src/rpc/methods/eth.rs and src/rpc/methods/eth/trace/* files, adding Ethereum trace call result types that depend on the EthBigInt representation changed here.

Suggested labels

RPC

Suggested reviewers

  • 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
Title check ✅ Passed The title accurately summarizes the main change: refactoring EthBigInt to use U256 instead of BigInt.
Linked Issues check ✅ Passed The PR successfully addresses issue #4436 by replacing rpc::eth::BigInt with ethereum_types::U256 as the underlying type.
Out of Scope Changes check ✅ Passed All changes are scoped to updating EthBigInt implementations and related test updates; no unrelated modifications detected.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch eth-big-int-type
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch eth-big-int-type

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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

@LesnyRumcajs LesnyRumcajs added the RPC requires calibnet RPC checks to run on CI label Jun 19, 2026
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.40%. Comparing base (ead4de9) to head (683f2bf).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/rpc/methods/eth.rs 88.88% 2 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/rpc/methods/eth/trace/geth.rs 81.18% <100.00%> (ø)
src/rpc/methods/eth/trace/state_diff.rs 80.74% <100.00%> (ø)
src/rpc/methods/eth/trace/types.rs 93.60% <100.00%> (-0.02%) ⬇️
src/rpc/methods/eth/types.rs 65.71% <100.00%> (ø)
src/rpc/methods/eth.rs 65.09% <88.88%> (+0.26%) ⬆️

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Jun 19, 2026
Merged via the queue into main with commit 49bc678 Jun 19, 2026
34 of 56 checks passed
@LesnyRumcajs LesnyRumcajs deleted the eth-big-int-type branch June 19, 2026 16:20
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.

remove rpc::eth::BigInt

2 participants