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

HTTP endpoint GetIndividualVotes #14198

Merged
merged 17 commits into from
Jul 19, 2024
Merged

HTTP endpoint GetIndividualVotes #14198

merged 17 commits into from
Jul 19, 2024

Conversation

saolyn
Copy link
Contributor

@saolyn saolyn commented Jul 9, 2024

What type of PR is this?

Other

What does this PR do? Why is it needed?

This PR adds the HTTP endpoint for GetIndividualVotes

Other notes for review

@saolyn saolyn requested a review from a team as a code owner July 9, 2024 16:54
@saolyn saolyn requested review from potuz, terencechain and rkapka July 9, 2024 16:54
@saolyn saolyn added the API Api related tasks label Jul 10, 2024
Copy link
Contributor Author

@saolyn saolyn left a comment

Choose a reason for hiding this comment

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

Should I just remove GetIndividualVotes endpoint from v1alpha1 since the proto object is deprecated?

@rkapka
Copy link
Contributor

rkapka commented Jul 12, 2024

Should I just remove GetIndividualVotes endpoint from v1alpha1 since the proto object is deprecated?

No, we only ever remove stuff in major releases

prestonvanloon and others added 5 commits July 17, 2024 13:19
* Electra: EIP-7251 Update `process_voluntary_exit`

* Add unit test for VerifyExitAndSignature EIP-7251

* @potuz peer feedback
* Add Current Changes

* add back check

* Avoid a Panic
* Implement Initial Logic

* Include check in main.go

* Add tests for multiple flags

* remove usage of append

* remove config/features dependency

* Move ValidateNetworkFlags to config/features

* Nit

* removed NetworkFlags from cmd

* remove usage of empty string literal

* add comment

* add flag validation to prysctl validator-exit

---------

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
beacon-chain/rpc/core/validator.go Outdated Show resolved Hide resolved
api/server/structs/endpoints_beacon.go Show resolved Hide resolved
defer span.End()

var req structs.GetIndividualVotesRequest
if r.Body != http.NoBody {
Copy link
Contributor

@rkapka rkapka Jul 17, 2024

Choose a reason for hiding this comment

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

  1. You made the endpoint a GET, which doesn't have a body in the request. We can either put the epoch, public keys and indices in query params or make the endpoint a POST. Given that the length of a query string can be limited by some servers, the latter is a much better option.
  2. What if there is no body? The function will panic when reading req.Epoch. I see we have the same pattern in GetValidatorPerformance, but this is a bug. We should return a BadRequest error in this case. What we usually do:
err = json.NewDecoder(r.Body).Decode(&req)
		switch {
		case errors.Is(err, io.EOF):
			httputil.HandleError(w, "No data submitted", http.StatusBadRequest)
			return
		case err != nil:
			httputil.HandleError(w, "Could not decode request body: "+err.Error(), http.StatusBadRequest)
			return
		}

beacon-chain/rpc/endpoints.go Outdated Show resolved Hide resolved
@saolyn saolyn force-pushed the get-individual-votes-http branch from b893d07 to 25e517a Compare July 18, 2024 16:01
@rkapka rkapka enabled auto-merge July 19, 2024 11:56
@rkapka rkapka added this pull request to the merge queue Jul 19, 2024
Merged via the queue into develop with commit 57ffc12 Jul 19, 2024
16 of 17 checks passed
@rkapka rkapka deleted the get-individual-votes-http branch July 19, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants