Skip to content

fix(evmrpc): treat id=null as a valid request, not a JSON-RPC notification#3303

Merged
amir-deris merged 1 commit into
mainfrom
amir/plt-282-evmrpc-null-id-treated-as-notification
Apr 27, 2026
Merged

fix(evmrpc): treat id=null as a valid request, not a JSON-RPC notification#3303
amir-deris merged 1 commit into
mainfrom
amir/plt-282-evmrpc-null-id-treated-as-notification

Conversation

@amir-deris

@amir-deris amir-deris commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Summary

JSON-RPC 2.0 defines a Notification as a request object with no id member. A request with "id": null is discouraged by the spec but is still a valid call and must receive a response.

The previous implementation of isJSONRPCNotificationID returned true for both a missing id and an explicit "id": null, causing requests with null ids to be silently dropped from batch responses.

Changes

  • isJSONRPCNotificationID now returns true only when the raw id field is absent (len(id) == 0), not when it is null
  • Added TestWrapSeiLegacyHTTP_BatchNullIDIsNotNotification to verify that a batch containing "id": null forwards the request and includes the response

Test plan

  • go test ./evmrpc/... -run TestWrapSeiLegacyHTTP_Batch

@amir-deris amir-deris self-assigned this Apr 22, 2026
@amir-deris amir-deris changed the title Excluded id=null as notification, added test. fix(evmrpc): treat id=null as a valid request, not a JSON-RPC notification Apr 22, 2026
@github-actions

github-actions Bot commented Apr 22, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 22, 2026, 11:47 PM

@codecov

codecov Bot commented Apr 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.31%. Comparing base (63f25b1) to head (355d2c5).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3303      +/-   ##
==========================================
- Coverage   58.31%   58.31%   -0.01%     
==========================================
  Files        2085     2085              
  Lines      209061   209058       -3     
==========================================
- Hits       121913   121910       -3     
  Misses      78355    78355              
  Partials     8793     8793              
Flag Coverage Δ
sei-chain-pr 62.60% <100.00%> (?)
sei-db 69.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
evmrpc/sei_legacy_http.go 77.55% <100.00%> (-0.23%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amir-deris amir-deris added this pull request to the merge queue Apr 23, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 23, 2026
@amir-deris amir-deris added this pull request to the merge queue Apr 27, 2026
Merged via the queue into main with commit d1c8ac7 Apr 27, 2026
44 checks passed
@amir-deris amir-deris deleted the amir/plt-282-evmrpc-null-id-treated-as-notification branch April 27, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants