Use helper to aggregate attestations in pool (#4794)

* Aggregate attestations in pool
* test
* clarify test
* fix test

beacon-chain/operations/attestations/kv/aggregated.go

@@ -29,13 +29,10 @@ func (p *AttCaches) SaveAggregatedAttestation(att *ethpb.Attestation) error {
 		}
 	}
 
-	// Ensure that this attestation is not already fully contained in an existing attestation.
-	for _, a := range atts {
-		if a.AggregationBits.Contains(att.AggregationBits) {
-			return nil
-		}
+	atts, err = helpers.AggregateAttestations(append(atts, att))
+	if err != nil {
+		return err
 	}
-	atts = append(atts, att)
 
 	// DefaultExpiration is set to what was given to New(). In this case
 	// it's one epoch.

beacon-chain/operations/attestations/kv/aggregated_test.go

@@ -279,3 +279,24 @@ func TestKV_HasAggregatedAttestation(t *testing.T) {
 		})
 	}
 }
+
+func TestKV_Aggregated_AggregatesAttestations(t *testing.T) {
+	cache := NewAttCaches()
+
+	att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1101}}
+	att2 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1111}}
+	atts := []*ethpb.Attestation{att1, att2}
+
+	for _, att := range atts {
+		if err := cache.SaveAggregatedAttestation(att); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	returned := cache.AggregatedAttestations()
+
+	// It should have only returned att2.
+	if !reflect.DeepEqual(att2, returned[0]) || len(returned) != 1 {
+		t.Error("Did not receive correct aggregated atts")
+	}
+}

beacon-chain/rpc/validator/proposer_test.go

@@ -1366,8 +1366,9 @@ func TestDeleteAttsInPool_Aggregated(t *testing.T) {
 		AttPool: attestations.NewPool(),
 	}
 
-	aggregatedAtts := []*ethpb.Attestation{{AggregationBits: bitfield.Bitlist{0b10101}}, {AggregationBits: bitfield.Bitlist{0b11010}}}
-	unaggregatedAtts := []*ethpb.Attestation{{AggregationBits: bitfield.Bitlist{0b1001}}, {AggregationBits: bitfield.Bitlist{0b0001}}}
+	sig := bls.RandKey().Sign([]byte("foo"), 0).Marshal()
+	aggregatedAtts := []*ethpb.Attestation{{AggregationBits: bitfield.Bitlist{0b10101}, Signature: sig}, {AggregationBits: bitfield.Bitlist{0b11010}, Signature: sig}}
+	unaggregatedAtts := []*ethpb.Attestation{{AggregationBits: bitfield.Bitlist{0b1001}, Signature: sig}, {AggregationBits: bitfield.Bitlist{0b0001}, Signature: sig}}
 
 	if err := s.AttPool.SaveAggregatedAttestations(aggregatedAtts); err != nil {
 		t.Fatal(err)
@@ -1376,7 +1377,11 @@ func TestDeleteAttsInPool_Aggregated(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := s.deleteAttsInPool(append(aggregatedAtts, unaggregatedAtts...)); err != nil {
+	aa, err := helpers.AggregateAttestations(aggregatedAtts)
+	if err != nil {
+		t.Error(err)
+	}
+	if err := s.deleteAttsInPool(append(aa, unaggregatedAtts...)); err != nil {
 		t.Fatal(err)
 	}
 	if len(s.AttPool.AggregatedAttestations()) != 0 {