Add /eth/v2/validator/aggregate_attestation
#14481
Conversation
| return nil | ||
| } | ||
| } | ||
| if match == nil { |
There was a problem hiding this comment.
this should be moved inside the first match check right?
There was a problem hiding this comment.
doesn't matter too much because we assign to an existing variable
| func matchingAtt(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte) (ethpbalpha.Att, error) { | ||
| // GetAggregateAttestationV2 aggregates all attestations matching the given attestation data root and slot, returning the aggregated result. | ||
| func (s *Server) GetAggregateAttestationV2(w http.ResponseWriter, r *http.Request) { | ||
| _, span := trace.StartSpan(r.Context(), "validator.GetAggregateAttestation") |
There was a problem hiding this comment.
maybe should update validator.GetAggregateAttestationV2
| if !ok { | ||
| return | ||
| } | ||
|
|
| if !ok { | ||
| return | ||
| } | ||
|
|
| return nil | ||
| } | ||
| } | ||
| if match == nil { |
There was a problem hiding this comment.
doesn't matter too much because we assign to an existing variable
|
I just noticed that this endpoint never worked correctly. The description says
This means that if we don't have a matching aggregated attestation, but we have a matching unaggregated one, we have to aggregate these attestations. I think this is a good moment to fix this. |
| @@ -52,7 +53,12 @@ func (c *beaconApiValidatorClient) submitAggregateSelectionProof( | |||
| return nil, err | |||
| } | |||
|
|
|||
| aggregatedAttestation, err := convertAttestationToProto(aggregateAttestationResponse.Data) | |||
| var attData *ethpb.Attestation // Replace with your appropriate struct | |||
There was a problem hiding this comment.
You should unmarshal into the API type, not the protobuf one
There was a problem hiding this comment.
Not sure how to get it working with the API type
| _, attDataRoot, ok := shared.HexFromQuery(w, r, "attestation_data_root", fieldparams.RootLength, true) | ||
| if !ok { | ||
| return | ||
| } | ||
| _, slot, ok := shared.UintFromQuery(w, r, "slot", true) | ||
| if !ok { | ||
| return | ||
| } | ||
| _, index, ok := shared.UintFromQuery(w, r, "committee_index", true) | ||
| if !ok { | ||
| return | ||
| } |
There was a problem hiding this comment.
These should return a 400 / bad request since they are required fields.
There was a problem hiding this comment.
They already do, within shared.UintFromQuery we have ValidateUint which handles the http error, same thing applies to shared.HexFromQuery
| } | ||
|
|
||
| func matchingAtt(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte) (ethpbalpha.Att, error) { | ||
| func matchingAtt(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte, index string) (ethpbalpha.Att, error) { |
There was a problem hiding this comment.
If there are multiple that match, return the best one by most CommitteeBitsVal().Count(), assuming they cannot be further aggregated.
Or consider returning all matching atts.
func matchingAtts(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte, index string) ([]ethpbalpha.Att, error)| httputil.HandleError(w, "No matching attestation found", http.StatusNotFound) | ||
| return nil | ||
| } | ||
| _, err = attestations.Aggregate([]ethpbalpha.Att{match}) |
There was a problem hiding this comment.
| _, err = attestations.Aggregate([]ethpbalpha.Att{match}) | |
| agg, err = attestations.Aggregate([]ethpbalpha.Att{matches}) | |
| ... | |
| return agg[0] |
If they cannot be aggregate to just one (len(agg) == 1), then return best by committee bits count.
| } | ||
|
|
||
| func matchingAtt(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte) (ethpbalpha.Att, error) { | ||
| func matchingAtts(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byte, index primitives.CommitteeIndex) ([]ethpbalpha.Att, error) { | ||
| if len(atts) == 0 { |
There was a problem hiding this comment.
So that we don't panic in the declaration of postElectra
| } | ||
| typedAgg, ok := agg.(*ethpbalpha.Attestation) | ||
| if !ok { | ||
| httputil.HandleError(w, fmt.Sprintf("Attestation is not of type %T", ðpbalpha.Attestation{}), http.StatusInternalServerError) |
There was a problem hiding this comment.
%T with an empty struct is nicer in my opinion because if the type name changes, you will get a compiler error
|
|
||
| match, err := matchingAtts(s.AttestationsPool.AggregatedAttestations(), slot, attDataRoot, index) | ||
| if err != nil { | ||
| httputil.HandleError(w, "Could not get matching attestation: "+err.Error(), http.StatusInternalServerError) |
There was a problem hiding this comment.
Nit
| httputil.HandleError(w, "Could not get matching attestation: "+err.Error(), http.StatusInternalServerError) | |
| httputil.HandleError(w, "Could not get matching attestations: "+err.Error(), http.StatusInternalServerError) |
What type of PR is this?
Other
What does this PR do? Why is it needed?
Beacon API Electra alignment, add missing endpoint for
/eth/v2/validator/aggregate_attestation.As described in the spec https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/getAggregatedAttestationV2
Which issues(s) does this PR fix?
Part of #14476
Other notes for review
Acknowledgements