-
Notifications
You must be signed in to change notification settings - Fork 999
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
Add GET /eth/v2/beacon/pool/attestations
endpoint
#14560
base: develop
Are you sure you want to change the base?
Conversation
/eth/v2/beacon/pool/attestations
endpoint
/eth/v2/beacon/pool/attestations
endpointGET /eth/v2/beacon/pool/attestations
endpoint
c43e5ba
to
33a2948
Compare
case *eth.AttestationElectra: | ||
allAtts[i] = structs.AttElectraFromConsensus(a) | ||
if !versionSet { | ||
firstVersion = a.Version() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of doing this first version set etc, can't we get the fork from the slot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same with the other function
} else { | ||
bothDefined := rawSlot != "" && rawCommitteeIndex != "" | ||
filteredAtts := make([]interface{}, 0, len(attestations)) | ||
for _, att := range attestations { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be a way to merge these two loops and just append based on these checks right?
committeeIndexMatch := rawCommitteeIndex != "" && att.GetData().CommitteeIndex == primitives.CommitteeIndex(committeeIndex) | ||
slotMatch := rawSlot != "" && att.GetData().Slot == primitives.Slot(slot) | ||
shouldAppend := (bothDefined && committeeIndexMatch && slotMatch) || (!bothDefined && (committeeIndexMatch || slotMatch)) | ||
if shouldAppend { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified like this:
committeeIndexMatch := true
slotMatch := true
if rawCommitteeIndex != "" && att.GetData().CommitteeIndex != primitives.CommitteeIndex(committeeIndex) {
committeeIndexMatch = false
}
if rawSlot != "" && att.GetData().Slot != primitives.Slot(slot) {
slotMatch = false
}
if committeeIndexMatch && slotMatch {
} else { | ||
httputil.HandleError(w, fmt.Sprintf("unable to convert attestations of type %T", att), http.StatusInternalServerError) | ||
if !ok { | ||
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestations of type %T", att), http.StatusInternalServerError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestations of type %T", att), http.StatusInternalServerError) | |
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestation of type %T", att), http.StatusInternalServerError) |
if shouldAppend { | ||
a, ok := att.(*eth.Attestation) | ||
if !ok { | ||
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestations of type %T", att), http.StatusInternalServerError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestations of type %T", att), http.StatusInternalServerError) | |
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestation of type %T", att), http.StatusInternalServerError) |
} | ||
httputil.WriteJson(w, &structs.ListAttestationsResponse{Data: filteredAtts}) | ||
w.Header().Set(api.VersionHeader, version.String(firstVersion)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same idea as for slashings: we should use the head state or current slot to determine the version and only return attestations for that version.
What type of PR is this?
Other
What does this PR do? Why is it needed?
Beacon API Electra alignment, add missing endpoint for GET
/eth/v2/beacon/pool/attestations
.As described in the spec https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getPoolAttestationsV2
Which issues(s) does this PR fix?
Part of #14476
Other notes for review
Acknowledgements