Make Aggregating In Parallel The Permanent Default (#13407)

* make it the permanent default

* gaz

beacon-chain/operations/attestations/BUILD.bazel

@@ -47,7 +47,6 @@ go_test(
     deps = [
         "//async:go_default_library",
         "//beacon-chain/operations/attestations/kv:go_default_library",
-        "//config/features:go_default_library",
         "//config/fieldparams:go_default_library",
         "//config/params:go_default_library",
         "//crypto/bls:go_default_library",

beacon-chain/operations/attestations/kv/BUILD.bazel

@@ -14,7 +14,6 @@ go_library(
     visibility = ["//beacon-chain:__subpackages__"],
     deps = [
         "//beacon-chain/core/helpers:go_default_library",
-        "//config/features:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types/primitives:go_default_library",
         "//crypto/hash:go_default_library",
@@ -39,7 +38,6 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
-        "//config/features:go_default_library",
         "//config/fieldparams:go_default_library",
         "//crypto/bls:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",

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

@@ -7,7 +7,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/core/helpers"
-	"github.com/prysmaticlabs/prysm/v4/config/features"
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
 	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	attaggregation "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1/attestation/aggregation/attestations"
@@ -46,30 +45,7 @@ func (c *AttCaches) aggregateUnaggregatedAtts(ctx context.Context, unaggregatedA
 	// Track the unaggregated attestations that aren't able to aggregate.
 	leftOverUnaggregatedAtt := make(map[[32]byte]bool)
 
-	if features.Get().AggregateParallel {
+	leftOverUnaggregatedAtt = c.aggregateParallel(attsByDataRoot, leftOverUnaggregatedAtt)
-		leftOverUnaggregatedAtt = c.aggregateParallel(attsByDataRoot, leftOverUnaggregatedAtt)
-	} else {
-		for _, atts := range attsByDataRoot {
-			aggregated, err := attaggregation.AggregateDisjointOneBitAtts(atts)
-			if err != nil {
-				return errors.Wrap(err, "could not aggregate unaggregated attestations")
-			}
-			if aggregated == nil {
-				return errors.New("could not aggregate unaggregated attestations")
-			}
-			if helpers.IsAggregated(aggregated) {
-				if err := c.SaveAggregatedAttestations([]*ethpb.Attestation{aggregated}); err != nil {
-					return err
-				}
-			} else {
-				h, err := hashFn(aggregated)
-				if err != nil {
-					return err
-				}
-				leftOverUnaggregatedAtt[h] = true
-			}
-		}
-	}
 
 	// Remove the unaggregated attestations from the pool that were successfully aggregated.
 	for _, att := range unaggregatedAtts {

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

@@ -9,7 +9,6 @@ import (
 	"github.com/pkg/errors"
 	fssz "github.com/prysmaticlabs/fastssz"
 	"github.com/prysmaticlabs/go-bitfield"
-	"github.com/prysmaticlabs/prysm/v4/config/features"
 	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
 	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
@@ -18,11 +17,6 @@ import (
 )
 
 func TestKV_Aggregated_AggregateUnaggregatedAttestations(t *testing.T) {
-	resetFn := features.InitWithReset(&features.Flags{
-		AggregateParallel: true,
-	})
-	defer resetFn()
-
 	cache := NewAttCaches()
 	priv, err := bls.RandKey()
 	require.NoError(t, err)

beacon-chain/operations/attestations/prepare_forkchoice_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/prysmaticlabs/go-bitfield"
-	"github.com/prysmaticlabs/prysm/v4/config/features"
 	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
 	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	attaggregation "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1/attestation/aggregation/attestations"
@@ -18,11 +17,6 @@ import (
 )
 
 func TestBatchAttestations_Multiple(t *testing.T) {
-	resetFn := features.InitWithReset(&features.Flags{
-		AggregateParallel: true,
-	})
-	defer resetFn()
-
 	s, err := NewService(context.Background(), &Config{Pool: NewPool()})
 	require.NoError(t, err)
 

config/features/config.go

@@ -68,8 +68,6 @@ type Flags struct {
 
 	PrepareAllPayloads bool // PrepareAllPayloads informs the engine to prepare a block on every slot.
 
-	AggregateParallel bool // AggregateParallel aggregates attestations in parallel.
-
 	// KeystoreImportDebounceInterval specifies the time duration the validator waits to reload new keys if they have
 	// changed on disk. This feature is for advanced use cases only.
 	KeystoreImportDebounceInterval time.Duration
@@ -231,11 +229,6 @@ func ConfigureBeaconChain(ctx *cli.Context) error {
 		logEnabled(prepareAllPayloads)
 		cfg.PrepareAllPayloads = true
 	}
-	cfg.AggregateParallel = true
-	if ctx.IsSet(disableAggregateParallel.Name) {
-		logEnabled(disableAggregateParallel)
-		cfg.AggregateParallel = false
-	}
 	if ctx.IsSet(disableResourceManager.Name) {
 		logEnabled(disableResourceManager)
 		cfg.DisableResourceManager = true

config/features/deprecated_flags.go

@@ -63,6 +63,11 @@ var (
 		Usage:  deprecatedUsage,
 		Hidden: true,
 	}
+	deprecatedDisableAggregateParallel = &cli.BoolFlag{
+		Name:   "disable-aggregate-parallel",
+		Usage:  deprecatedUsage,
+		Hidden: true,
+	}
 )
 
 // Deprecated flags for both the beacon node and validator client.
@@ -78,6 +83,7 @@ var deprecatedFlags = []cli.Flag{
 	deprecatedDisableBuildBlockParallel,
 	deprecatedDisableReorgLateBlocks,
 	deprecatedDisableOptionalEngineMethods,
+	deprecatedDisableAggregateParallel,
 }
 
 // deprecatedBeaconFlags contains flags that are still used by other components

config/features/flags.go

@@ -150,11 +150,6 @@ var (
 		Name:  "disable-registration-cache",
 		Usage: "Temporary flag for disabling the validator registration cache instead of using the DB. Note: registrations do not clear on restart while using the DB.",
 	}
-
-	disableAggregateParallel = &cli.BoolFlag{
-		Name:  "disable-aggregate-parallel",
-		Usage: "Disables parallel aggregation of attestations.",
-	}
 )
 
 // devModeFlags holds list of flags that are set when development mode is on.
@@ -209,7 +204,6 @@ var BeaconChainFlags = append(deprecatedBeaconFlags, append(deprecatedFlags, []c
 	EnableEIP4881,
 	disableResourceManager,
 	DisableRegistrationCache,
-	disableAggregateParallel,
 }...)...)
 
 // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E.