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

Opt-in to debug endpoint usage in Validators #100

Closed

Conversation

moshe-blox
Copy link
Contributor

@moshe-blox moshe-blox commented Jan 25, 2024

The Beacon API spec says that the debug endpoints are:

Set of endpoints to debug chain and shouldn't be exposed publicly.

For that reason, Prysm disables them by default, and since the Validators method leverages the debug endpoint to retrieve large validator sets, it's likely to fail in many setups.

I believe that go-eth2-client should either check that these endpoints are available as part of client creation, or allow the user to opt-in to using them.

This PR implements the latter, since I suspect that requesting the state at every connection may be too heavy.

@mcdee If you think opting-in at the client-level is preferable to method-level, I wouldn't mind refactoring that.

@mcdee
Copy link
Contributor

mcdee commented Jan 26, 2024

The text from the spec differs a little from reality nowadays, in that the state endpoint is generally recognized as useful. There was discussion about moving it outside of the debug namespace due to this, but it was ultimately decided to leave it there to avoid the potential confusion of moving the endpoint around (see ethereum/beacon-APIs#357 for details).

I would suggest that given the linked conversation it would make sense for prysm to default to enabling the debug endpoints. All other beacon nodes provide these without additional flags. Having what amounts to a specific flag for the current release of prysm isn't something I'm interested in adding to the client, especially given the performance savings when obtaining large numbers of validators via state.

@moshe-blox
Copy link
Contributor Author

moshe-blox commented Jan 28, 2024

@mcdee I agree regarding Prysm, though I think it would've been safer to wait until it's no longer a debug route before making it the default behavior.

With that said, after checking your claim, BeaconState is indeed so much faster! (~13s vs. ~55s with 5k pubkeys)

We're likely gonna add a preflight check by calling BeaconState on boot, so that our node fails early and alerts the user to enable the Prysm flag, so we don't need this PR anymore.

Thanks 🙏

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