Skip to content

Conversation

@jtraglia
Copy link
Collaborator

@jtraglia jtraglia commented Feb 3, 2025

📝 Summary

I think "functionality" is a bit too vague. Splitting things up makes sense IMO:

  • register_validator.go -- implements registerValidator
  • get_header.go -- implements getHeader
  • get_payload.go -- implements getPayload

Also does some intermediate refactoring, particularly in getPayload.

✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 76.68998% with 100 lines in your changes missing coverage. Please review.

Project coverage is 43.67%. Comparing base (74a8ecb) to head (1eaf793).
Report is 41 commits behind head on develop.

Files with missing lines Patch % Lines
server/get_payload.go 60.94% 46 Missing and 20 partials ⚠️
server/get_header.go 79.59% 21 Missing and 9 partials ⚠️
server/utils.go 95.08% 3 Missing ⚠️
server/service.go 96.15% 0 Missing and 1 partial ⚠️

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

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #729      +/-   ##
===========================================
- Coverage    43.84%   43.67%   -0.17%     
===========================================
  Files           15       18       +3     
  Lines         1608     1708     +100     
===========================================
+ Hits           705      746      +41     
- Misses         847      904      +57     
- Partials        56       58       +2     
Flag Coverage Δ
unittests 43.67% <76.68%> (-0.17%) ⬇️

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.

Comment on lines 330 to +332
func (m *BoostService) respondPayload(w http.ResponseWriter, log *logrus.Entry, result *builderApi.VersionedSubmitBlindedBlockResponse, originalBid bidResp) {
// If no payload has been received from relay, log loudly about withholding!
if result == nil || getPayloadResponseIsEmpty(result) {
if result == nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a notable change. I've removed getPayloadResponseIsEmpty (only exists in verifyPayload now). I do not believe it's necessary to check this here but I could be wrong. If the response is empty, verifyPayload will return a nil result and this will respond with an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this prevents us from accessing nil fields. I don't know if those are set during the unmarshalling. If the are, I think we can remove this check

@jtraglia
Copy link
Collaborator Author

jtraglia commented Feb 4, 2025

I'm going to close this & break it up into a few PRs.

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