From 1b70d2b5663edaa4f26e44eaff94011646314dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Fri, 26 Jan 2024 18:08:58 +0100 Subject: [PATCH] Fetch unaggregated atts in `GetAggregateAttestation` (#13533) --- beacon-chain/rpc/eth/validator/handlers.go | 70 ++++++++++------- .../rpc/eth/validator/handlers_test.go | 78 ++++++++++++------- 2 files changed, 94 insertions(+), 54 deletions(-) diff --git a/beacon-chain/rpc/eth/validator/handlers.go b/beacon-chain/rpc/eth/validator/handlers.go index cecd6b8f37bc..b94fa289e6ff 100644 --- a/beacon-chain/rpc/eth/validator/handlers.go +++ b/beacon-chain/rpc/eth/validator/handlers.go @@ -38,7 +38,7 @@ import ( // GetAggregateAttestation aggregates all attestations matching the given attestation data root and slot, returning the aggregated result. func (s *Server) GetAggregateAttestation(w http.ResponseWriter, r *http.Request) { - ctx, span := trace.StartSpan(r.Context(), "validator.GetAggregateAttestation") + _, span := trace.StartSpan(r.Context(), "validator.GetAggregateAttestation") defer span.End() _, attDataRoot, ok := shared.HexFromQuery(w, r, "attestation_data_root", fieldparams.RootLength, true) @@ -51,53 +51,67 @@ func (s *Server) GetAggregateAttestation(w http.ResponseWriter, r *http.Request) return } - if err := s.AttestationsPool.AggregateUnaggregatedAttestations(ctx); err != nil { - httputil.HandleError(w, "Could not aggregate unaggregated attestations: "+err.Error(), http.StatusBadRequest) + var match *ethpbalpha.Attestation + var err error + + match, err = matchingAtt(s.AttestationsPool.AggregatedAttestations(), primitives.Slot(slot), attDataRoot) + if err != nil { + httputil.HandleError(w, "Could not get matching attestation: "+err.Error(), http.StatusInternalServerError) return } - - allAtts := s.AttestationsPool.AggregatedAttestations() - var bestMatchingAtt *ethpbalpha.Attestation - for _, att := range allAtts { - if att.Data.Slot == primitives.Slot(slot) { - root, err := att.Data.HashTreeRoot() - if err != nil { - httputil.HandleError(w, "Could not get attestation data root: "+err.Error(), http.StatusInternalServerError) - return - } - if bytes.Equal(root[:], attDataRoot) { - if bestMatchingAtt == nil || len(att.AggregationBits) > len(bestMatchingAtt.AggregationBits) { - bestMatchingAtt = att - } - } + if match == nil { + atts, err := s.AttestationsPool.UnaggregatedAttestations() + if err != nil { + httputil.HandleError(w, "Could not get unaggregated attestations: "+err.Error(), http.StatusInternalServerError) + return + } + match, err = matchingAtt(atts, primitives.Slot(slot), attDataRoot) + if err != nil { + httputil.HandleError(w, "Could not get matching attestation: "+err.Error(), http.StatusInternalServerError) + return } } - if bestMatchingAtt == nil { + if match == nil { httputil.HandleError(w, "No matching attestation found", http.StatusNotFound) return } response := &AggregateAttestationResponse{ Data: &shared.Attestation{ - AggregationBits: hexutil.Encode(bestMatchingAtt.AggregationBits), + AggregationBits: hexutil.Encode(match.AggregationBits), Data: &shared.AttestationData{ - Slot: strconv.FormatUint(uint64(bestMatchingAtt.Data.Slot), 10), - CommitteeIndex: strconv.FormatUint(uint64(bestMatchingAtt.Data.CommitteeIndex), 10), - BeaconBlockRoot: hexutil.Encode(bestMatchingAtt.Data.BeaconBlockRoot), + Slot: strconv.FormatUint(uint64(match.Data.Slot), 10), + CommitteeIndex: strconv.FormatUint(uint64(match.Data.CommitteeIndex), 10), + BeaconBlockRoot: hexutil.Encode(match.Data.BeaconBlockRoot), Source: &shared.Checkpoint{ - Epoch: strconv.FormatUint(uint64(bestMatchingAtt.Data.Source.Epoch), 10), - Root: hexutil.Encode(bestMatchingAtt.Data.Source.Root), + Epoch: strconv.FormatUint(uint64(match.Data.Source.Epoch), 10), + Root: hexutil.Encode(match.Data.Source.Root), }, Target: &shared.Checkpoint{ - Epoch: strconv.FormatUint(uint64(bestMatchingAtt.Data.Target.Epoch), 10), - Root: hexutil.Encode(bestMatchingAtt.Data.Target.Root), + Epoch: strconv.FormatUint(uint64(match.Data.Target.Epoch), 10), + Root: hexutil.Encode(match.Data.Target.Root), }, }, - Signature: hexutil.Encode(bestMatchingAtt.Signature), + Signature: hexutil.Encode(match.Signature), }} httputil.WriteJson(w, response) } +func matchingAtt(atts []*ethpbalpha.Attestation, slot primitives.Slot, attDataRoot []byte) (*ethpbalpha.Attestation, error) { + for _, att := range atts { + if att.Data.Slot == slot { + root, err := att.Data.HashTreeRoot() + if err != nil { + return nil, errors.Wrap(err, "could not get attestation data root") + } + if bytes.Equal(root[:], attDataRoot) { + return att, nil + } + } + } + return nil, nil +} + // SubmitContributionAndProofs publishes multiple signed sync committee contribution and proofs. func (s *Server) SubmitContributionAndProofs(w http.ResponseWriter, r *http.Request) { ctx, span := trace.StartSpan(r.Context(), "validator.SubmitContributionAndProofs") diff --git a/beacon-chain/rpc/eth/validator/handlers_test.go b/beacon-chain/rpc/eth/validator/handlers_test.go index f7c885b9e60f..50aca2d6d261 100644 --- a/beacon-chain/rpc/eth/validator/handlers_test.go +++ b/beacon-chain/rpc/eth/validator/handlers_test.go @@ -71,7 +71,7 @@ func TestGetAggregateAttestation(t *testing.T) { AggregationBits: []byte{0, 1, 1}, Data: ðpbalpha.AttestationData{ Slot: 2, - CommitteeIndex: 2, + CommitteeIndex: 1, BeaconBlockRoot: root21, Source: ðpbalpha.Checkpoint{ Epoch: 1, @@ -90,7 +90,7 @@ func TestGetAggregateAttestation(t *testing.T) { AggregationBits: []byte{0, 1, 1, 1}, Data: ðpbalpha.AttestationData{ Slot: 2, - CommitteeIndex: 3, + CommitteeIndex: 1, BeaconBlockRoot: root22, Source: ðpbalpha.Checkpoint{ Epoch: 1, @@ -103,33 +103,56 @@ func TestGetAggregateAttestation(t *testing.T) { }, Signature: sig22, } - root33 := bytesutil.PadTo([]byte("root3_3"), 32) - sig33 := bls.NewAggregateSignature().Marshal() - attslot33 := ðpbalpha.Attestation{ - AggregationBits: []byte{1, 0, 0, 1}, + root31 := bytesutil.PadTo([]byte("root3_1"), 32) + sig31 := bls.NewAggregateSignature().Marshal() + attslot31 := ðpbalpha.Attestation{ + AggregationBits: []byte{1, 0}, Data: ðpbalpha.AttestationData{ - Slot: 2, - CommitteeIndex: 3, - BeaconBlockRoot: root33, + Slot: 3, + CommitteeIndex: 1, + BeaconBlockRoot: root31, + Source: ðpbalpha.Checkpoint{ + Epoch: 1, + Root: root31, + }, + Target: ðpbalpha.Checkpoint{ + Epoch: 1, + Root: root31, + }, + }, + Signature: sig31, + } + root32 := bytesutil.PadTo([]byte("root3_2"), 32) + sig32 := bls.NewAggregateSignature().Marshal() + attslot32 := ðpbalpha.Attestation{ + AggregationBits: []byte{0, 1}, + Data: ðpbalpha.AttestationData{ + Slot: 3, + CommitteeIndex: 1, + BeaconBlockRoot: root32, Source: ðpbalpha.Checkpoint{ Epoch: 1, - Root: root33, + Root: root32, }, Target: ðpbalpha.Checkpoint{ Epoch: 1, - Root: root33, + Root: root32, }, }, - Signature: sig33, + Signature: sig32, } + pool := attestations.NewPool() err := pool.SaveAggregatedAttestations([]*ethpbalpha.Attestation{attSlot1, attslot21, attslot22}) assert.NoError(t, err) + err = pool.SaveUnaggregatedAttestations([]*ethpbalpha.Attestation{attslot31, attslot32}) + assert.NoError(t, err) + s := &Server{ AttestationsPool: pool, } - t.Run("ok", func(t *testing.T) { + t.Run("matching aggregated att", func(t *testing.T) { reqRoot, err := attslot22.Data.HashTreeRoot() require.NoError(t, err) attDataRoot := hexutil.Encode(reqRoot[:]) @@ -147,7 +170,7 @@ func TestGetAggregateAttestation(t *testing.T) { assert.DeepEqual(t, "0x00010101", resp.Data.AggregationBits) assert.DeepEqual(t, hexutil.Encode(sig22), resp.Data.Signature) assert.Equal(t, "2", resp.Data.Data.Slot) - assert.Equal(t, "3", resp.Data.Data.CommitteeIndex) + assert.Equal(t, "1", resp.Data.Data.CommitteeIndex) assert.DeepEqual(t, hexutil.Encode(root22), resp.Data.Data.BeaconBlockRoot) require.NotNil(t, resp.Data.Data.Source) assert.Equal(t, "1", resp.Data.Data.Source.Epoch) @@ -156,19 +179,11 @@ func TestGetAggregateAttestation(t *testing.T) { assert.Equal(t, "1", resp.Data.Data.Target.Epoch) assert.DeepEqual(t, hexutil.Encode(root22), resp.Data.Data.Target.Root) }) - - t.Run("aggregate beforehand", func(t *testing.T) { - err = s.AttestationsPool.SaveUnaggregatedAttestation(attslot33) - require.NoError(t, err) - newAtt := ethpbalpha.CopyAttestation(attslot33) - newAtt.AggregationBits = []byte{0, 1, 0, 1} - err = s.AttestationsPool.SaveUnaggregatedAttestation(newAtt) - require.NoError(t, err) - - reqRoot, err := attslot33.Data.HashTreeRoot() + t.Run("matching unaggregated att", func(t *testing.T) { + reqRoot, err := attslot32.Data.HashTreeRoot() require.NoError(t, err) attDataRoot := hexutil.Encode(reqRoot[:]) - url := "http://example.com?attestation_data_root=" + attDataRoot + "&slot=2" + url := "http://example.com?attestation_data_root=" + attDataRoot + "&slot=3" request := httptest.NewRequest(http.MethodGet, url, nil) writer := httptest.NewRecorder() writer.Body = &bytes.Buffer{} @@ -178,7 +193,18 @@ func TestGetAggregateAttestation(t *testing.T) { resp := &AggregateAttestationResponse{} require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp)) require.NotNil(t, resp) - assert.DeepEqual(t, "0x01010001", resp.Data.AggregationBits) + require.NotNil(t, resp.Data) + assert.DeepEqual(t, "0x0001", resp.Data.AggregationBits) + assert.DeepEqual(t, hexutil.Encode(sig32), resp.Data.Signature) + assert.Equal(t, "3", resp.Data.Data.Slot) + assert.Equal(t, "1", resp.Data.Data.CommitteeIndex) + assert.DeepEqual(t, hexutil.Encode(root32), resp.Data.Data.BeaconBlockRoot) + require.NotNil(t, resp.Data.Data.Source) + assert.Equal(t, "1", resp.Data.Data.Source.Epoch) + assert.DeepEqual(t, hexutil.Encode(root32), resp.Data.Data.Source.Root) + require.NotNil(t, resp.Data.Data.Target) + assert.Equal(t, "1", resp.Data.Data.Target.Epoch) + assert.DeepEqual(t, hexutil.Encode(root32), resp.Data.Data.Target.Root) }) t.Run("no matching attestation", func(t *testing.T) { attDataRoot := hexutil.Encode(bytesutil.PadTo([]byte("foo"), 32))