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

Add logging and validation improvements #589

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

avalonche
Copy link
Collaborator

@avalonche avalonche commented Mar 8, 2024

📝 Summary

Some logging improvements and validation:

  • Logging error only if the bid was eligible and payload was not found. failed getting execution payload (2/2) - payload not found, but found bid in database - this error log occurred often due to race conditions in which a submission passed checkFloorBidValue earlier in the request and was simulated. However a higher value bid was simulated and updated in redis before the block submission finished simulation, resulting in a bid below the floor when SaveBidAndUpdateTopBid is reached. The bid is saved but the payload was not.
  • Demote the 202 response code to a warning. This response code was returned from lighthouse when a duplicate block was gossiped, see [Merged by Bors] - Send success code for duplicate blocks on HTTP sigp/lighthouse#4655
  • Changes the data api parameters to int64. bigint in postgres is represented as a int64 value which was not properly validated in the request parameters and uint64 values could be inputted.

⛱ Motivation and Context

Reduces error logging noise in the relay.

📚 References


✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy
  • I have seen and agree to CONTRIBUTING.md

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 30.41%. Comparing base (108d5e3) to head (82adb09).

Files Patch % Lines
services/api/service.go 0.00% 13 Missing ⚠️
beaconclient/multi_beacon_client.go 0.00% 4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
- Coverage   30.44%   30.41%   -0.03%     
==========================================
  Files          41       41              
  Lines        6268     6273       +5     
==========================================
  Hits         1908     1908              
- Misses       4113     4118       +5     
  Partials      247      247              
Flag Coverage Δ
unittests 30.41% <0.00%> (-0.03%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metachris
metachris previously approved these changes Mar 11, 2024
Copy link
Collaborator

@metachris metachris left a comment

Choose a reason for hiding this comment

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

lgtm. left a small comment and not sure why changing uint to int?

Co-authored-by: Chris Hager <chris@linuxuser.at>
@avalonche
Copy link
Collaborator Author

postgres bigint uses int64, and the error occurs when user input larger than int64 is entered. Changing from uint64 to int64 would bound the values to int64, the same representation as Postgres and the error will be handled in ParseInt instead of checking for the bound ourselves

@metachris metachris merged commit 9d6a4b7 into main Mar 12, 2024
6 checks passed
@metachris metachris deleted the logging-improvements branch March 12, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants