Skip to content

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

Open
amir-deris wants to merge 1 commit intomainfrom
amir/plt-282-evmrpc-null-id-treated-as-notification
Open

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

Conversation

@amir-deris
Copy link
Copy Markdown
Contributor

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

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
Copy link
Copy Markdown

github-actions Bot commented Apr 22, 2026

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
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.31%. Comparing base (63f25b1) to head (355d2c5).

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.

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.

1 participant