Skip to content

Conversation

@Mikelle
Copy link
Contributor

@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
Contributor 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
Contributor 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

@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
Contributor 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
Contributor 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
@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

3 participants