Skip to content

fix(evmrpc): use synthetic IDs in slow-path batch to handle duplicate and null request IDs#3331

Merged
masih merged 3 commits into
mainfrom
amir/plt-292-282-feedback-mergeSeiLegacyHTTPBatch-error
Apr 29, 2026
Merged

fix(evmrpc): use synthetic IDs in slow-path batch to handle duplicate and null request IDs#3331
masih merged 3 commits into
mainfrom
amir/plt-292-282-feedback-mergeSeiLegacyHTTPBatch-error

Conversation

@amir-deris

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

Copy link
Copy Markdown
Contributor

Problem

In the slow-path (gated) handleBatch flow, mergeSeiLegacyHTTPBatch built idToIdx keyed on the original request ID echoed back by the inner server. When a batch contained multiple requests sharing the same ID — including multiple "id":null entries — the old code detected the ID collision and fell through to appendMergeFailure, replacing all responses in the batch with internal errors.

Fix

Assign a unique sequential synthetic integer ID (0, 1, 2, …) to each forwarded non-notification request before dispatching to the inner server. The inner server echoes these synthetic IDs back, so idToIdx is always collision-free regardless of duplicate or null original IDs. Original IDs are restored in the output via patchJSONRPCResponseIDIfNeeded.

Changes:

  • Merge the blocked-check into the same parse loop to avoid a second pass over msgs
  • Replace appendMergeFailure on duplicate inner-ID with a best-effort continue (the synthetic-ID scheme makes this unreachable in normal operation, but it's safer than a hard failure)
  • Drop redundant json.RawMessage(...) casts on already-[]byte return values

Tests

  • Added TestWrapSeiLegacyHTTP_BatchTwoNullIDsDifferentResults: regression test covering two "id":null requests + a notification + a unique-ID request + a blocked sei_ method in one batch — verifies correct per-slot responses and that the notification is omitted from output
  • Updated existing tests to expect synthetic IDs (0, 1, …) instead of original IDs in the forwarded body

Linear

Closes PLT-282, PLT-292

@amir-deris amir-deris self-assigned this Apr 28, 2026
@amir-deris amir-deris changed the title Using synthetic ID to track rpc requests with duplicate id including … fix(evmrpc): use synthetic IDs in slow-path batch to handle duplicate and null request IDs Apr 28, 2026
@github-actions

github-actions Bot commented Apr 28, 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 29, 2026, 9:32 AM

@codecov

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.17%. Comparing base (740b2dd) to head (b03f9d1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
evmrpc/sei_legacy_http.go 85.71% 3 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3331      +/-   ##
==========================================
+ Coverage   59.16%   59.17%   +0.01%     
==========================================
  Files        2097     2097              
  Lines      172485   172496      +11     
==========================================
+ Hits       102053   102078      +25     
+ Misses      61570    61562       -8     
+ Partials     8862     8856       -6     
Flag Coverage Δ
sei-chain-pr 64.56% <85.71%> (?)
sei-db 70.41% <ø> (ø)

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 80.91% <85.71%> (+0.83%) ⬆️

... and 28 files with indirect coverage changes

🚀 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 requested review from bdchatham and masih April 28, 2026 23:46
@sei-will sei-will self-requested a review April 29, 2026 08:22
@masih masih added the backport release/v6.4 Backport to release v6.4 label Apr 29, 2026
@masih masih enabled auto-merge April 29, 2026 09:31
@masih masih added this pull request to the merge queue Apr 29, 2026
Merged via the queue into main with commit e47d7a2 Apr 29, 2026
40 checks passed
@masih masih deleted the amir/plt-292-282-feedback-mergeSeiLegacyHTTPBatch-error branch April 29, 2026 09:46
@seidroid

seidroid Bot commented Apr 29, 2026

Copy link
Copy Markdown

Created backport PR for release/v6.4:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-3331-to-release/v6.4
git worktree add --checkout .worktree/backport-3331-to-release/v6.4 backport-3331-to-release/v6.4
cd .worktree/backport-3331-to-release/v6.4
git reset --hard HEAD^
git cherry-pick -x e47d7a2ba3c0234a84baaa8adc1a0af04665af79
git push --force-with-lease

masih pushed a commit that referenced this pull request Apr 29, 2026
…batch to handle duplicate and null request IDs (#3333)

Backport of #3331 to `release/v6.4`.

---------

Co-authored-by: Amir <amir@seinetwork.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants