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: eth_getBlockByHash is using invalid cache record #3254

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

natanasow
Copy link
Collaborator

@natanasow natanasow commented Nov 13, 2024

Description:

We are getting alerts on intermittent eth_getTransactionReceipt calls with a 500 response. Tracing through the logs we can see that the getBlockByHash call is missing a valid hash, right after the call to the mirror node on the contracts/results endpoint.

Steps to reproduce:

  1. Look through the logs for an eth_getTransactionReceipt call with a 500 response.
  2. Note the request ID and trace through the logs the path of the request.
  3. Notice the call to getBlockByHash is missing a valid hash.
[34mDEBUG[39m (rpc-server/84 on mainnet-hashio-85fd789b7-twzsv): [36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] Validating method parameters for eth_getTransactionReceipt, params: ["0x411f2d52652ad237296f85041e382194d3fd5a7fb006e269975dede4652b70fc"][39m"
timestamp: "2024-11-11T15:30:19.138608570Z"

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] getTransactionReceipt(0x411f2d52652ad237296f85041e382194d3fd5a7fb006e269975dede4652b70fc)

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] [GET] contracts/results/0x411f2d52652ad237296f85041e382194d3fd5a7fb006e269975dede4652b70fc 200 4 ms

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] [GET] contracts/results/0x411f2d52652ad237296f85041e382194d3fd5a7fb006e269975dede4652b70fc 200 2 ms

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] getBlockByHash(hash=0x, showDetails=false)

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] [GET] blocks/NaN 400 status

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] Failed to retrieve block for hash 0x

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] Error invoking RPC: Invalid parameter: hashOrNumber

[36m[Request ID: 51841ec6-3dc9-4dad-af6a-07c4c3f01e02] [POST]: eth_getTransactionReceipt 500 20 ms

Related issue(s):

Fixes #3250

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@natanasow natanasow self-assigned this Nov 13, 2024
@natanasow natanasow added the bug Something isn't working label Nov 13, 2024
@natanasow natanasow added this to the 0.61.0 milestone Nov 13, 2024
Signed-off-by: nikolay <[email protected]>
Copy link

sonarcloud bot commented Nov 13, 2024

@natanasow natanasow changed the title chore: fix mirror node client fix: eth_getBlockByHash is using invalid cache key Nov 13, 2024
Copy link

github-actions bot commented Nov 13, 2024

Test Results

 17 files  ±0  230 suites  ±0   29m 20s ⏱️ +18s
607 tests ±0  603 ✅ ±0  4 💤 ±0  0 ❌ ±0 
623 runs  ±0  619 ✅ ±0  4 💤 ±0  0 ❌ ±0 

Results for commit 764079b. ± Comparison against base commit 3dd36f6.

♻️ This comment has been updated with latest results.

@natanasow natanasow marked this pull request as ready for review November 13, 2024 10:40
@natanasow natanasow changed the title fix: eth_getBlockByHash is using invalid cache key fix: eth_getBlockByHash is using invalid cache record Nov 13, 2024
Copy link
Contributor

@ebadiere ebadiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG.

@ebadiere ebadiere merged commit 967270f into main Nov 13, 2024
50 of 51 checks passed
@ebadiere ebadiere deleted the 3250-getBlockByHash-is-using-wrong-cache-key branch November 13, 2024 14:22
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.84%. Comparing base (3dd36f6) to head (764079b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3254      +/-   ##
==========================================
- Coverage   78.80%   77.84%   -0.96%     
==========================================
  Files          48       66      +18     
  Lines        3595     4460     +865     
  Branches      836     1000     +164     
==========================================
+ Hits         2833     3472     +639     
- Misses        423      613     +190     
- Partials      339      375      +36     
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 78.58% <100.00%> (+0.07%) ⬆️
server 83.28% <ø> (?)
ws-server 36.66% <ø> (?)

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

Files with missing lines Coverage Δ
packages/relay/src/lib/clients/mirrorNodeClient.ts 86.16% <100.00%> (+0.09%) ⬆️

... and 19 files with indirect coverage changes

quiet-node pushed a commit that referenced this pull request Nov 13, 2024
* chore: fix mirror node client

Signed-off-by: nikolay <[email protected]>

* chore: remove .only

Signed-off-by: nikolay <[email protected]>

---------

Signed-off-by: nikolay <[email protected]>
quiet-node pushed a commit that referenced this pull request Nov 13, 2024
* chore: fix mirror node client

Signed-off-by: nikolay <[email protected]>

* chore: remove .only

Signed-off-by: nikolay <[email protected]>

---------

Signed-off-by: nikolay <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
quiet-node added a commit that referenced this pull request Nov 13, 2024
fix: `eth_getBlockByHash` is using invalid cache record (#3254)

* chore: fix mirror node client



* chore: remove .only



---------

Signed-off-by: nikolay <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Co-authored-by: Nikolay Atanasow <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getBlockByHash in the eth_getTransactionReceipt flow is using the wrong cache keys
3 participants