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

feat: add validator api #310

Merged
merged 16 commits into from
Aug 7, 2024
Merged

feat: add validator api #310

merged 16 commits into from
Aug 7, 2024

Conversation

Mikelle
Copy link
Member

@Mikelle Mikelle commented Aug 2, 2024

Describe your changes

Add validator API

Issue ticket number and link

Fixes #309

Checklist before requesting a review

  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have tested this code by deploying the infrastructure and ensuring that commitments are being settled

@Mikelle Mikelle self-assigned this Aug 2, 2024
@Mikelle Mikelle marked this pull request as ready for review August 6, 2024 17:37
Copy link
Contributor

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

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

Did querying the router contract end up working? ie. this is working e2e?

Name: "validator-router-contract",
Usage: "address of the validator router contract",
EnvVars: []string{"MEV_COMMIT_VALIDATOR_ROUTER_ADDR"},
Action: func(ctx *cli.Context, s string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we supply a default value for deployment on Holesky? If it exists

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't do it for oracle, so I didn't add default variable here (as it also comes with private key for the alchemy)

$ref: '#/definitions/googlerpcStatus'
/v1/validator/get_validators:
get:
summary: GetValidators
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we separate this out from GetEpoch? Ie. why not just have a single endpoint that returns the validators of the latest epoch?

Reasoning here should prob be in our docs

Copy link
Member Author

@Mikelle Mikelle Aug 6, 2024

Choose a reason for hiding this comment

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

Asked Evan, he said that it's sufficient for GetEpoch be done automatically in GetValidators request

p2p/pkg/rpc/validator/service.go Outdated Show resolved Hide resolved
p2p/pkg/rpc/validator/service.go Outdated Show resolved Hide resolved
p2p/pkg/rpc/validator/service.go Outdated Show resolved Hide resolved
p2p/rpc/validatorapi/v1/validatorapi.proto Outdated Show resolved Hide resolved
p2p/pkg/rpc/validator/service.go Outdated Show resolved Hide resolved
@shaspitz
Copy link
Contributor

shaspitz commented Aug 6, 2024

We'll want to add some docs on how bidders can utilize these new endpoints to have better UX, along with any timing considerations

@Mikelle
Copy link
Member Author

Mikelle commented Aug 6, 2024

Thank you for the review! I definitely agree on the necessity of documentation. I wanted to add it once we have the latest smart contract set and that branch finalized and merged.

@Mikelle Mikelle requested a review from shaspitz August 6, 2024 23:15
@Mikelle
Copy link
Member Author

Mikelle commented Aug 6, 2024

Did querying the router contract end up working? ie. this is working e2e?

Router is working with fake ValidatorRegistry (which is always returns true)

Copy link
Contributor

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

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

Looks great!

@Mikelle Mikelle merged commit 6c78079 into main Aug 7, 2024
6 checks passed
@Mikelle Mikelle deleted the feat/309/validator-api branch August 7, 2024 00:45
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.

Add validator API
2 participants