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

Forward validator registrations without decoding #733

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

jtraglia
Copy link
Collaborator

@jtraglia jtraglia commented Feb 5, 2025

📝 Summary

  • Move functionality to new register_validator.go file.
  • Forward validator registrations to each relay without decoding the message.
    • If the node sends validator registrations in SSZ, we inherit that.
  • Add a wrapUserAgent method.

✅ I have run these commands

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

return errNoSuccessfulRelayResponse
}

func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(log *logrus.Entry, regBytes []byte, header http.Header) {
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 the relay monitor code paths have been deprecated; and if not, likely should be

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. I would like to remove this in a separate PR.

ralexstokes
ralexstokes previously approved these changes Feb 11, 2025
Copy link
Collaborator

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

the code generally looks good. some notes:

  • there are no tests for this AFACIT, we should test somehow before putting into a release
  • there is a change in behavior here where mev-boost no longer verifies the validator registrations deserialize correctly; it should be fine (the relay will still do this, etc) but is somewhat of a notable change
  • im not going all of the go APIs with a fine-toothed comb, so could be missing something with error handling, request processing, etc

@jtraglia
Copy link
Collaborator Author

there are no tests for this AFACIT, we should test somehow before putting into a release

Agreed. I've pushed some basic tests to ensure the forwarding works as expected.

there is a change in behavior here where mev-boost no longer verifies the validator registrations deserialize correctly; it should be fine (the relay will still do this, etc) but is somewhat of a notable change

Also agreed. But I feel that this is fine. The builder/relay should check this. Keep in mind it's possible for clients to connect directly to a single builder/relay and skip the mev-boost middleman. Meaning they need to handle bad inputs too.

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.

2 participants