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.

Sign up for GitHub

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

Jump to bottom

Fetch unaggregated atts in GetAggregateAttestation #13533

Merged
merged 2 commits into from
Jan 26, 2024
Merged

Fetch unaggregated atts in GetAggregateAttestation #13533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccb0ccc
Fetch unaggregated atts in `GetAggregateAttestation`
rkapka Jan 26, 2024
728b65f
Merge branch 'develop' into fix-att-aggregation
rkapka Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 42 additions & 28 deletions beacon-chain/rpc/eth/validator/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -51,53 +51,67 @@ func (s *Server) GetAggregateAttestation(w http.ResponseWriter, r *http.Request)
return
}

if err := s.AttestationsPool.AggregateUnaggregatedAttestations(ctx); err != nil {
Copy link
Contributor

@james-prysm james-prysm Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

note: found out this was just wrong before in the gRPC implementation of GetAggregateAttestation, instead we should be looking at the business logic of SubmitAggregateSelectionProof in v1alpha1

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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extracted function doesn't have the bits check because they can't be different - otherwise the root wouldn't match.

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")
Expand Down
78 changes: 52 additions & 26 deletions beacon-chain/rpc/eth/validator/handlers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestGetAggregateAttestation(t *testing.T) {
AggregationBits: []byte{0, 1, 1},
Data: &ethpbalpha.AttestationData{
Slot: 2,
CommitteeIndex: 2,
CommitteeIndex: 1,
BeaconBlockRoot: root21,
Source: &ethpbalpha.Checkpoint{
Epoch: 1,
Expand All @@ -90,7 +90,7 @@ func TestGetAggregateAttestation(t *testing.T) {
AggregationBits: []byte{0, 1, 1, 1},
Data: &ethpbalpha.AttestationData{
Slot: 2,
CommitteeIndex: 3,
CommitteeIndex: 1,
BeaconBlockRoot: root22,
Source: &ethpbalpha.Checkpoint{
Epoch: 1,
Expand All @@ -103,33 +103,56 @@ func TestGetAggregateAttestation(t *testing.T) {
},
Signature: sig22,
}
root33 := bytesutil.PadTo([]byte("root3_3"), 32)
sig33 := bls.NewAggregateSignature().Marshal()
attslot33 := &ethpbalpha.Attestation{
AggregationBits: []byte{1, 0, 0, 1},
root31 := bytesutil.PadTo([]byte("root3_1"), 32)
sig31 := bls.NewAggregateSignature().Marshal()
attslot31 := &ethpbalpha.Attestation{
AggregationBits: []byte{1, 0},
Data: &ethpbalpha.AttestationData{
Slot: 2,
CommitteeIndex: 3,
BeaconBlockRoot: root33,
Slot: 3,
CommitteeIndex: 1,
BeaconBlockRoot: root31,
Source: &ethpbalpha.Checkpoint{
Epoch: 1,
Root: root31,
},
Target: &ethpbalpha.Checkpoint{
Epoch: 1,
Root: root31,
},
},
Signature: sig31,
}
root32 := bytesutil.PadTo([]byte("root3_2"), 32)
sig32 := bls.NewAggregateSignature().Marshal()
attslot32 := &ethpbalpha.Attestation{
AggregationBits: []byte{0, 1},
Data: &ethpbalpha.AttestationData{
Slot: 3,
CommitteeIndex: 1,
BeaconBlockRoot: root32,
Source: &ethpbalpha.Checkpoint{
Epoch: 1,
Root: root33,
Root: root32,
},
Target: &ethpbalpha.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[:])
Expand All @@ -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)
Expand All @@ -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{}
Expand All @@ -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))
Expand Down
Loading