Skip to content

fix(eth): serialize empty lists as [] instead of null#7214

Merged
sudo-shashank merged 19 commits into
mainfrom
shashank/fix-accessList
Jun 24, 2026
Merged

fix(eth): serialize empty lists as [] instead of null#7214
sudo-shashank merged 19 commits into
mainfrom
shashank/fix-accessList

Conversation

@sudo-shashank

@sudo-shashank sudo-shashank commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary of changes

Changes introduced in this pull request:

  • Use NotNullVec instead of Vec for empty lists so they are never null.

Reference issue to close (if applicable)

Closes #7205

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

Summary

  • Bug Fixes
    • Updated Ethereum JSON-RPC responses for eth_accounts, eth_getBlockReceipts*, and trace_* to always return non-null arrays, using [] for empty results instead of null.
    • Updated transaction accessList handling: typed transactions serialize an empty access list as [], while legacy transactions omit the field; deserialization treats both null and [] as “not present”.
  • Documentation
    • Updated OpenRPC specs (v0–v2) to match the non-null array and accessList requirements.
  • Tests
    • Extended RPC serialization/deserialization coverage for empty and omitted accessList cases.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)

Walkthrough

NotNullVec<T> gains Default and GetSize derives to enable proper empty-array and size-tracking semantics. Seven Ethereum RPC methods (EthAccounts, EthGetBlockReceipts, EthGetBlockReceiptsLimited, EthTraceBlock, EthTraceTransaction, EthTraceReplayBlockTransactions, EthTraceFilter) are converted from returning Vec<T> to NotNullVec<T>. ApiEthTx.access_list field is changed from Vec<EthHash> to Option<NotNullVec<EthHash>>, ensuring empty lists serialize as [] rather than null. OpenRPC schemas are updated across all versions to reflect these non-nullable array contracts. Unit tests validate serialization and deserialization behavior, and a changelog entry documents the fix.

Changes

ETH empty-list serialization fix

Layer / File(s) Summary
NotNullVec struct: Default and GetSize derives
src/lotus_json/vec.rs
Adds GetSize import and extends NotNullVec<T> derive list with Default and GetSize traits.
ApiEthTx.access_list field and transaction conversions
src/rpc/methods/eth.rs, src/rpc/methods/eth/eth_tx.rs
Updates ApiEthTx.access_list to Option<NotNullVec<EthHash>> with serde/schemars configuration to omit when None. Native-message and EIP-1559 transaction conversions now set access_list to Some(NotNullVec(vec![])) for empty lists. Comprehensive unit tests verify serialization (empty, populated, explicit None omission, legacy behavior) and deserialization (missing/null handling).
ETH RPC method return types switched to NotNullVec
src/rpc/methods/eth.rs
Imports NotNullVec and updates seven method return types (EthAccounts, EthGetBlockReceipts, EthGetBlockReceiptsLimited, EthTraceBlock, EthTraceTransaction, EthTraceReplayBlockTransactions, EthTraceFilter) from Vec<T> to NotNullVec<T>. Handlers wrap results with NotNullVec or .map(NotNullVec). Fixes trace_filter to iterate over inner vector via block_traces.0.
OpenRPC specification schemas across all versions
docs/openrpc-specs/v0.json, docs/openrpc-specs/v1.json, docs/openrpc-specs/v2.json
Updates method result schemas for Filecoin-prefixed and lowercase variants from optional/nullable arrays (required: false, type: [array, null]) to required non-nullable arrays (required: true, type: array). Updates ApiEthTx.accessList property from nullable to non-nullable array type across all spec versions.
Changelog and test snapshots
CHANGELOG.md, src/tool/subcommands/api_cmd/test_snapshots.txt
Adds changelog entry for PR #7214 documenting accessList serialization alignment with go-ethereum/reth. Updates regenerated test snapshot references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ChainSafe/forest#6731: Changes to Ethereum trace RPC handlers (trace_block, trace-related responses) to return non-null arrays overlap with this PR's trace handler refactoring in src/rpc/methods/eth.rs.

Suggested reviewers

  • hanabi1224
  • LesnyRumcajs
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing empty list serialization to use [] instead of null in Ethereum RPC responses.
Linked Issues check ✅ Passed The PR addresses all three completion criteria from #7205: verifies correct behavior against reth/go-ethereum standards, implements the fix for the accessList field serialization, and adds comprehensive regression tests.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the accessList serialization issue and aligning with Ethereum standards; no unrelated or out-of-scope modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch shashank/fix-accessList
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch shashank/fix-accessList

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

@sudo-shashank sudo-shashank added the RPC requires calibnet RPC checks to run on CI label Jun 22, 2026
@sudo-shashank sudo-shashank marked this pull request as ready for review June 22, 2026 06:34
@sudo-shashank sudo-shashank requested a review from a team as a code owner June 22, 2026 06:34
@sudo-shashank sudo-shashank requested review from LesnyRumcajs and hanabi1224 and removed request for a team June 22, 2026 06:34
@sudo-shashank sudo-shashank marked this pull request as draft June 22, 2026 06:50
@sudo-shashank sudo-shashank marked this pull request as ready for review June 22, 2026 12:38
Comment thread src/rpc/methods/eth.rs Outdated
@sudo-shashank sudo-shashank marked this pull request as draft June 22, 2026 14:20
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.06%. Comparing base (d3ce944) to head (6cebe06).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/lotus_json/vec.rs 90.00% 0 Missing and 1 partial ⚠️
src/rpc/methods/eth.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/rpc/methods/eth/eth_tx.rs 100.00% <100.00%> (+31.81%) ⬆️
src/lotus_json/vec.rs 77.77% <90.00%> (+16.23%) ⬆️
src/rpc/methods/eth.rs 65.04% <92.30%> (+0.07%) ⬆️

... and 9 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 d3ce944...6cebe06. 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

Copy link
Copy Markdown
Member

API compare tests failed in | Filecoin.EthGetTransactionByBlockNumberAndIndex (20) | 18/20 | 18/20 | ⚠️ Mixed Results (CustomCheckFailed) |. This must be addressed.

@sudo-shashank

Copy link
Copy Markdown
Contributor Author

API compare tests failed in | Filecoin.EthGetTransactionByBlockNumberAndIndex (20) | 18/20 | 18/20 | ⚠️ Mixed Results (CustomCheckFailed) |. This must be addressed.

filtering these methods until lotus fix them

@hanabi1224

Copy link
Copy Markdown
Contributor

filtering these methods until lotus fix them

Is there a Lotus PR fixing this? If so, I'd suggest we use the commit tag once it's merged instead of disabling the tests

@sudo-shashank

Copy link
Copy Markdown
Contributor Author

filtering these methods until lotus fix them

Is there a Lotus PR fixing this? If so, I'd suggest we use the commit tag once it's merged instead of disabling the tests

@hanabi1224 we reported it in #fil-lotus-dev also the tracking issue #12214 is long due to be fixed and i don't see any open PR

Comment thread src/rpc/methods/eth.rs
Comment thread scripts/tests/api_compare/filter-list Outdated
@sudo-shashank

Copy link
Copy Markdown
Contributor Author

filtering these methods until lotus fix them

Is there a Lotus PR fixing this? If so, I'd suggest we use the commit tag once it's merged instead of disabling the tests

@hanabi1224 no more disabling the test for any method

@sudo-shashank sudo-shashank enabled auto-merge June 24, 2026 14:01
Comment thread src/rpc/methods/eth.rs
@sudo-shashank sudo-shashank requested a review from hanabi1224 June 24, 2026 14:54
@sudo-shashank sudo-shashank added this pull request to the merge queue Jun 24, 2026
Merged via the queue into main with commit cfb1049 Jun 24, 2026
44 checks passed
@sudo-shashank sudo-shashank deleted the shashank/fix-accessList branch June 24, 2026 15:47
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.

eth_getblockByNumber possible discrepancy on accessList

3 participants