Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix websocket from_height #1853

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Fix websocket from_height #1853

merged 1 commit into from
Sep 13, 2024

Conversation

codchen
Copy link
Collaborator

@codchen codchen commented Sep 12, 2024

Describe your changes and provide context

To fix #1850

There is a bug in how fromHeight is set during the websocket handler loop. Specifically each iteration will only look at the latest height if we don't set filter.FromBlock to the last height handled by the last iteration, so in the case where multiple heights have passed between two websocket iterations, events would be lost.

Testing performed to validate your change

difficult to simulate latest height passing in unit test. Since this is an RPC-only change, we can apply it to the RPC node the bug reporter is using and see if the error is resolved

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.81%. Comparing base (e8e4b3b) to head (5cda988).
Report is 165 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1853      +/-   ##
==========================================
- Coverage   61.64%   60.81%   -0.84%     
==========================================
  Files         365      260     -105     
  Lines       26178    22869    -3309     
==========================================
- Hits        16138    13908    -2230     
+ Misses       8967     7976     -991     
+ Partials     1073      985      -88     
Files with missing lines Coverage Δ
evmrpc/subscribe.go 63.40% <100.00%> (-0.81%) ⬇️

... and 192 files with indirect coverage changes

@codchen codchen merged commit aeecb5f into main Sep 13, 2024
50 checks passed
@codchen codchen deleted the fix-ws-events branch September 13, 2024 02:58
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.

Websocket eth filter system not reporting events correctly
2 participants