Revert "some refactoring"

This reverts commit 377ce7fbfb.

beacon-chain/db/kv/attestations.go

@@ -18,6 +18,42 @@ import (
 	"go.opencensus.io/trace"
 )
 
+func toAttestations(ac *dbpb.AttestationContainer) []*ethpb.Attestation {
+	if ac == nil {
+		return nil
+	}
+
+	atts := make([]*ethpb.Attestation, len(ac.SignaturePairs))
+	for i, sp := range ac.SignaturePairs {
+		atts[i] = &ethpb.Attestation{
+			Data:            ac.Data,
+			AggregationBits: sp.AggregationBits,
+			Signature:       sp.Signature,
+			// TODO: Add custody bits in phase 1.
+		}
+	}
+	return atts
+}
+
+func insertAttestation(ac *dbpb.AttestationContainer, att *ethpb.Attestation) {
+	sigPairsNotEclipsed := make([]*dbpb.AttestationContainer_SignaturePair, 0, len(ac.SignaturePairs))
+	for _, sp := range ac.SignaturePairs {
+		// if att is fully contained in some existing bitfield, do nothing.
+		if sp.AggregationBits.Contains(att.AggregationBits) {
+			return
+		}
+		// filter any existing signature pairs that are fully contained within
+		// the new attestation.
+		if !att.AggregationBits.Contains(sp.AggregationBits) {
+			sigPairsNotEclipsed = append(sigPairsNotEclipsed, sp)
+		}
+	}
+	ac.SignaturePairs = append(sigPairsNotEclipsed, &dbpb.AttestationContainer_SignaturePair{
+		AggregationBits: att.AggregationBits,
+		Signature:       att.Signature,
+	})
+}
+
 // Attestation retrieval by attestation data root.
 func (k *Store) AttestationsByDataRoot(ctx context.Context, attDataRoot [32]byte) ([]*ethpb.Attestation, error) {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.Attestation")
@@ -33,7 +69,7 @@ func (k *Store) AttestationsByDataRoot(ctx context.Context, attDataRoot [32]byte
 		if err := proto.Unmarshal(enc, ac); err != nil {
 			return err
 		}
-		atts = ac.ToAttestations()
+		atts = toAttestations(ac)
 		return nil
 	})
 	if err != nil {
@@ -73,7 +109,7 @@ func (k *Store) Attestations(ctx context.Context, f *filters.QueryFilter) ([]*et
 			if err := proto.Unmarshal(encoded, ac); err != nil {
 				return err
 			}
-			atts = append(atts, ac.ToAttestations()...)
+			atts = append(atts, toAttestations(ac)...)
 		}
 		return nil
 	})
@@ -162,7 +198,7 @@ func (k *Store) SaveAttestation(ctx context.Context, att *ethpb.Attestation) err
 			}
 		}
 
-		ac.InsertAttestation(att)
+		insertAttestation(ac, att)
 
 		enc, err := proto.Marshal(ac)
 		if err != nil {

beacon-chain/operations/service.go

@@ -215,7 +215,7 @@ func (s *Service) AttestationPool(ctx context.Context, requestedSlot uint64) ([]
 	// TODO: Greed selection. Which attestations provide the most profit?
 	var validAttsCount uint64
 	for root, ac := range s.attestationPool {
-		for _, att := range ac.ToAttestations() {
+		for _, att := range dbpb.ToAttestations(ac) {
 			if s.recentAttestationBitlist.Contains(root, att.AggregationBits) {
 				// TODO: Delete attestation from container?
 				continue
@@ -247,7 +247,7 @@ func (s *Service) AttestationPoolNoVerify(ctx context.Context) ([]*ethpb.Attesta
 	atts := make([]*ethpb.Attestation, 0, len(s.attestationPool))
 
 	for _, ac := range s.attestationPool {
-		atts = append(atts, ac.ToAttestations()...)
+		atts = append(atts, dbpb.ToAttestations(ac)...)
 	}
 
 	return atts, nil
@@ -303,7 +303,7 @@ func (s *Service) HandleAttestation(ctx context.Context, message proto.Message)
 		return nil
 	}
 
-	beforeAggregation := append(ac.ToAttestations(), attestation)
+	beforeAggregation := append(dbpb.ToAttestations(ac), attestation)
 
 	// Filter any out attestation that is already fully included.
 	for i, att := range beforeAggregation {
@@ -391,7 +391,7 @@ func (s *Service) removeAttestationsFromPool(ctx context.Context, attestations [
 
 		ac, ok := s.attestationPool[root]
 		if ok {
-			atts := ac.ToAttestations()
+			atts := dbpb.ToAttestations(ac)
 			for i, att := range atts {
 				if s.recentAttestationBitlist.Contains(root, att.AggregationBits) {
 					log.Debug("deleting attestation")

beacon-chain/operations/service_test.go

@@ -251,7 +251,7 @@ func TestHandleAttestation_Aggregates_LargeNumValidators(t *testing.T) {
 
 	// We fetch the final attestation from the attestation pool, which should be an aggregation of
 	// all committee members effectively.
-	aggAtt := opsSrv.attestationPool[attDataRoot].ToAttestations()[0]
+	aggAtt := dbpb.ToAttestations(opsSrv.attestationPool[attDataRoot])[0]
 	b1 := aggAtt.AggregationBits.Bytes()
 	b2 := totalAggBits.Bytes()
 
@@ -403,7 +403,7 @@ func TestHandleAttestation_Skips_PreviouslyAggregatedAttestations(t *testing.T)
 	if err != nil {
 		t.Error(err)
 	}
-	dbAtt := service.attestationPool[attDataHash].ToAttestations()[0]
+	dbAtt := dbpb.ToAttestations(service.attestationPool[attDataHash])[0]
 
 	dbAttBits := dbAtt.AggregationBits.Bytes()
 	aggregatedBits := att1.AggregationBits.Or(att2.AggregationBits).Bytes()
@@ -418,7 +418,7 @@ func TestHandleAttestation_Skips_PreviouslyAggregatedAttestations(t *testing.T)
 	if err := service.HandleAttestation(context.Background(), att2); err != nil {
 		t.Error(err)
 	}
-	dbAtt = service.attestationPool[attDataHash].ToAttestations()[0]
+	dbAtt = dbpb.ToAttestations(service.attestationPool[attDataHash])[0]
 
 	dbAttBits = dbAtt.AggregationBits.Bytes()
 	if !bytes.Equal(dbAttBits, aggregatedBits) {
@@ -432,7 +432,7 @@ func TestHandleAttestation_Skips_PreviouslyAggregatedAttestations(t *testing.T)
 	if err := service.HandleAttestation(context.Background(), att3); err != nil {
 		t.Error(err)
 	}
-	dbAtt = service.attestationPool[attDataHash].ToAttestations()[0]
+	dbAtt = dbpb.ToAttestations(service.attestationPool[attDataHash])[0]
 
 	dbAttBits = dbAtt.AggregationBits.Bytes()
 	if !bytes.Equal(dbAttBits, aggregatedBits) {

proto/beacon/db/attestation_container_helper.go

@@ -31,7 +31,7 @@ func (ac *AttestationContainer) Contains(att *ethpb.Attestation) bool {
 	return false
 }
 
-func (ac *AttestationContainer) ToAttestations() []*ethpb.Attestation {
+func ToAttestations(ac *AttestationContainer) []*ethpb.Attestation {
 	if ac == nil {
 		return nil
 	}
@@ -49,22 +49,3 @@ func (ac *AttestationContainer) ToAttestations() []*ethpb.Attestation {
 	}
 	return atts
 }
-
-func (ac *AttestationContainer) InsertAttestation(att *ethpb.Attestation) {
-	sigPairsNotEclipsed := make([]*AttestationContainer_SignaturePair, 0, len(ac.SignaturePairs))
-	for _, sp := range ac.SignaturePairs {
-		// if att is fully contained in some existing bitfield, do nothing.
-		if sp.AggregationBits.Contains(att.AggregationBits) {
-			return
-		}
-		// filter any existing signature pairs that are fully contained within
-		// the new attestation.
-		if !att.AggregationBits.Contains(sp.AggregationBits) {
-			sigPairsNotEclipsed = append(sigPairsNotEclipsed, sp)
-		}
-	}
-	ac.SignaturePairs = append(sigPairsNotEclipsed, &AttestationContainer_SignaturePair{
-		AggregationBits: att.AggregationBits,
-		Signature:       att.Signature,
-	})
-}