Release flag to aggregate attestations in fork choice. (#4820)

* Release flag to aggregate attestations in fork choice.
* Merge refs/heads/master into aggregate-atts-in-fc
* fix test
* Merge branch 'aggregate-atts-in-fc' of github.com:prysmaticlabs/prysm into aggregate-atts-in-fc
* Merge refs/heads/master into aggregate-atts-in-fc

beacon-chain/operations/attestations/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/operations/attestations/kv:go_default_library",
         "//beacon-chain/state:go_default_library",
-        "//shared/featureconfig:go_default_library",
         "//shared/hashutil:go_default_library",
         "//shared/params:go_default_library",
         "@com_github_dgraph_io_ristretto//:go_default_library",
@@ -40,7 +39,6 @@ go_test(
         "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/operations/attestations/kv:go_default_library",
         "//shared/bls:go_default_library",
-        "//shared/featureconfig:go_default_library",
         "@com_github_gogo_protobuf//proto:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@com_github_prysmaticlabs_go_bitfield//:go_default_library",

beacon-chain/operations/attestations/aggregate_test.go

@@ -11,15 +11,8 @@ import (
 	"github.com/prysmaticlabs/go-bitfield"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/shared/bls"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 )
 
-func init() {
-	fc := featureconfig.Get()
-	fc.ForkchoiceAggregateAttestations = true
-	featureconfig.Init(fc)
-}
-
 func TestAggregateAttestations_SingleAttestation(t *testing.T) {
 	s, err := NewService(context.Background(), &Config{Pool: NewPool()})
 	if err != nil {

beacon-chain/operations/attestations/prepare_forkchoice.go

@@ -10,7 +10,6 @@ import (
 	"github.com/prysmaticlabs/go-ssz"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"go.opencensus.io/trace"
@@ -51,30 +50,24 @@ func (s *Service) batchForkChoiceAtts(ctx context.Context) error {
 	atts = append(atts, s.pool.ForkchoiceAttestations()...)
 
 	// Consolidate attestations by aggregating them by similar data root.
-	if featureconfig.Get().ForkchoiceAggregateAttestations {
-		for _, att := range atts {
-			seen, err := s.seen(att)
-			if err != nil {
-				return err
-			}
-			if seen {
-				continue
-			}
-
-			attDataRoot, err := ssz.HashTreeRoot(att.Data)
-			if err != nil {
-				return err
-			}
-			attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
+	for _, att := range atts {
+		seen, err := s.seen(att)
+		if err != nil {
+			return err
+		}
+		if seen {
+			continue
 		}
 
-		for _, atts := range attsByDataRoot {
-			if err := s.aggregateAndSaveForkChoiceAtts(atts); err != nil {
-				return err
-			}
+		attDataRoot, err := ssz.HashTreeRoot(att.Data)
+		if err != nil {
+			return err
 		}
-	} else {
-		if err := s.pool.SaveForkchoiceAttestations(atts); err != nil {
+		attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
+	}
+
+	for _, atts := range attsByDataRoot {
+		if err := s.aggregateAndSaveForkChoiceAtts(atts); err != nil {
 			return err
 		}
 	}

shared/featureconfig/config.go

@@ -40,7 +40,6 @@ type Flags struct {
 	KafkaBootstrapServers                      string // KafkaBootstrapServers to find kafka servers to stream blocks, attestations, etc.
 	ProtectProposer                            bool   // ProtectProposer prevents the validator client from signing any proposals that would be considered a slashable offense.
 	ProtectAttester                            bool   // ProtectAttester prevents the validator client from signing any attestations that would be considered a slashable offense.
-	ForkchoiceAggregateAttestations            bool   // ForkchoiceAggregateAttestations attempts to aggregate attestations before processing in fork choice.
 	DisableStrictAttestationPubsubVerification bool   // DisableStrictAttestationPubsubVerification will disabling strict signature verification in pubsub.
 	DisableUpdateHeadPerAttestation            bool   // DisableUpdateHeadPerAttestation will disabling update head on per attestation basis.
 
@@ -141,10 +140,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("Enabled filtered block tree cache for fork choice.")
 		cfg.EnableBlockTreeCache = true
 	}
-	if ctx.GlobalBool(forkchoiceAggregateAttestations.Name) {
-		log.Warn("Enabled fork choice aggregation pre-processing of attestations")
-		cfg.ForkchoiceAggregateAttestations = true
-	}
 	if ctx.GlobalBool(disableStrictAttestationPubsubVerificationFlag.Name) {
 		log.Warn("Disabled strict attestation signature verification in pubsub")
 		cfg.DisableStrictAttestationPubsubVerification = true

shared/featureconfig/flags.go

@@ -90,10 +90,6 @@ var (
 		Usage: "Prevent the validator client from signing and broadcasting 2 any slashable attestations. " +
 			"Protects from slashing.",
 	}
-	forkchoiceAggregateAttestations = cli.BoolFlag{
-		Name:  "forkchoice-aggregate-attestations",
-		Usage: "Preprocess attestations by aggregation before running fork choice.",
-	}
 	disableStrictAttestationPubsubVerificationFlag = cli.BoolFlag{
 		Name:  "disable-strict-attestation-pubsub-verification",
 		Usage: "Disable strict signature verification of attestations in pubsub. See PR 4782 for details.",
@@ -197,7 +193,13 @@ var (
 	deprecatedprotoArrayForkChoice = cli.BoolFlag{
 		Name:   "proto-array-forkchoice",
 		Usage:  deprecatedUsage,
-		Hidden: true}
+		Hidden: true,
+	}
+	deprecatedForkchoiceAggregateAttestations = cli.BoolFlag{
+		Name:  "forkchoice-aggregate-attestations",
+		Usage:  deprecatedUsage,
+		Hidden: true,
+	}
 )
 
 var deprecatedFlags = []cli.Flag{
@@ -219,6 +221,7 @@ var deprecatedFlags = []cli.Flag{
 	deprecatedSaveDepositDataFlag,
 	deprecatedCacheProposerIndicesFlag,
 	deprecatedprotoArrayForkChoice,
+	deprecatedForkchoiceAggregateAttestations,
 }
 
 // ValidatorFlags contains a list of all the feature flags that apply to the validator client.
@@ -251,7 +254,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 	enableSkipSlotsCacheFlag,
 	enableSlasherFlag,
 	cacheFilteredBlockTreeFlag,
-	forkchoiceAggregateAttestations,
 	disableStrictAttestationPubsubVerificationFlag,
 	disableUpdateHeadPerAttestation,
 }...)