Enable attestation cache flag by default, deprecate feature flag (#4873)

* Enable attester flag by default

beacon-chain/cache/attestation_data.go

@@ -11,7 +11,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"k8s.io/client-go/tools/cache"
 )
 
@@ -59,12 +58,6 @@ func NewAttestationCache() *AttestationCache {
 // Get waits for any in progress calculation to complete before returning a
 // cached response, if any.
 func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRequest) (*ethpb.AttestationData, error) {
-	if !featureconfig.Get().EnableAttestationCache {
-		// Return a miss result if cache is not enabled.
-		attestationCacheMiss.Inc()
-		return nil, nil
-	}
-
 	if req == nil {
 		return nil, errors.New("nil attestation data request")
 	}
@@ -113,10 +106,6 @@ func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRe
 // MarkInProgress a request so that any other similar requests will block on
 // Get until MarkNotInProgress is called.
 func (c *AttestationCache) MarkInProgress(req *ethpb.AttestationDataRequest) error {
-	if !featureconfig.Get().EnableAttestationCache {
-		return nil
-	}
-
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	s, e := reqToKey(req)
@@ -126,19 +115,13 @@ func (c *AttestationCache) MarkInProgress(req *ethpb.AttestationDataRequest) err
 	if c.inProgress[s] {
 		return ErrAlreadyInProgress
 	}
-	if featureconfig.Get().EnableAttestationCache {
-		c.inProgress[s] = true
-	}
+	c.inProgress[s] = true
 	return nil
 }
 
 // MarkNotInProgress will release the lock on a given request. This should be
 // called after put.
 func (c *AttestationCache) MarkNotInProgress(req *ethpb.AttestationDataRequest) error {
-	if !featureconfig.Get().EnableAttestationCache {
-		return nil
-	}
-
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	s, e := reqToKey(req)
@@ -151,10 +134,6 @@ func (c *AttestationCache) MarkNotInProgress(req *ethpb.AttestationDataRequest)
 
 // Put the response in the cache.
 func (c *AttestationCache) Put(ctx context.Context, req *ethpb.AttestationDataRequest, res *ethpb.AttestationData) error {
-	if !featureconfig.Get().EnableAttestationCache {
-		return nil
-	}
-
 	data := &attestationReqResWrapper{
 		req,
 		res,

beacon-chain/cache/feature_flag_test.go

@@ -4,7 +4,6 @@ import "github.com/prysmaticlabs/prysm/shared/featureconfig"
 
 func init() {
 	featureconfig.Init(&featureconfig.Flags{
-		EnableAttestationCache:  true,
 		EnableEth1DataVoteCache: true,
 	})
 }

beacon-chain/core/state/benchmarks_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/shared/benchutil"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
@@ -52,10 +51,6 @@ func BenchmarkExecuteStateTransition_FullBlock(b *testing.B) {
 }
 
 func BenchmarkExecuteStateTransition_WithCache(b *testing.B) {
-	config := &featureconfig.Flags{
-		EnableAttestationCache: true,
-	}
-	featureconfig.Init(config)
 	benchutil.SetBenchmarkConfig()
 
 	beaconState, err := benchutil.PreGenState1Epoch()
@@ -91,10 +86,6 @@ func BenchmarkExecuteStateTransition_WithCache(b *testing.B) {
 }
 
 func BenchmarkProcessEpoch_2FullEpochs(b *testing.B) {
-	config := &featureconfig.Flags{
-		EnableAttestationCache: true,
-	}
-	featureconfig.Init(config)
 	benchutil.SetBenchmarkConfig()
 	beaconState, err := benchutil.PreGenState2FullEpochs()
 	if err != nil {

beacon-chain/rpc/validator/BUILD.bazel

@@ -88,7 +88,6 @@ go_test(
         "//shared/bls:go_default_library",
         "//shared/bytesutil:go_default_library",
         "//shared/event:go_default_library",
-        "//shared/featureconfig:go_default_library",
         "//shared/hashutil:go_default_library",
         "//shared/params:go_default_library",
         "//shared/testutil:go_default_library",

beacon-chain/rpc/validator/attester_test.go

@@ -20,7 +20,6 @@ import (
 	mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing"
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/bls"
-	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
@@ -310,14 +309,6 @@ func TestAttestationDataAtSlot_HandlesFarAwayJustifiedEpoch(t *testing.T) {
 }
 
 func TestAttestationDataSlot_handlesInProgressRequest(t *testing.T) {
-	// Cache toggled by feature flag for now. See https://github.com/prysmaticlabs/prysm/issues/3106.
-	featureconfig.Init(&featureconfig.Flags{
-		EnableAttestationCache: true,
-	})
-	defer func() {
-		featureconfig.Init(nil)
-	}()
-
 	s := &pbp2p.BeaconState{Slot: 100}
 	state, _ := beaconstate.InitializeFromProto(s)
 	ctx := context.Background()

shared/featureconfig/config.go

@@ -49,7 +49,6 @@ type Flags struct {
 	DisableForkChoice bool
 
 	// Cache toggles.
-	EnableAttestationCache  bool // EnableAttestationCache; see https://github.com/prysmaticlabs/prysm/issues/3106.
 	EnableSSZCache          bool // EnableSSZCache see https://github.com/prysmaticlabs/prysm/pull/4558.
 	EnableEth1DataVoteCache bool // EnableEth1DataVoteCache; see https://github.com/prysmaticlabs/prysm/issues/3106.
 	EnableSkipSlotsCache    bool // EnableSkipSlotsCache caches the state in skipped slots.
@@ -94,10 +93,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("UNSAFE: Disabled fork choice for updating chain head")
 		cfg.DisableForkChoice = true
 	}
-	if ctx.GlobalBool(enableAttestationCacheFlag.Name) {
-		log.Warn("Enabled unsafe attestation cache")
-		cfg.EnableAttestationCache = true
-	}
 	if ctx.GlobalBool(enableSSZCache.Name) {
 		log.Warn("Enabled unsafe ssz cache")
 		cfg.EnableSSZCache = true

shared/featureconfig/flags.go

@@ -22,10 +22,6 @@ var (
 		Usage: "UNSAFE: disable fork choice for determining head of the beacon chain.",
 	}
 	// enableAttestationCacheFlag see https://github.com/prysmaticlabs/prysm/issues/3106.
-	enableAttestationCacheFlag = cli.BoolFlag{
-		Name:  "enable-attestation-cache",
-		Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106",
-	}
 	// enableSSZCache see https://github.com/prysmaticlabs/prysm/pull/4558.
 	enableSSZCache = cli.BoolFlag{
 		Name:  "enable-ssz-cache",
@@ -200,6 +196,11 @@ var (
 		Usage:  deprecatedUsage,
 		Hidden: true,
 	}
+	deprecatedEnableAttestationCacheFlag = cli.BoolFlag{
+		Name:  "enable-attestation-cache",
+		Usage: deprecatedUsage,
+		Hidden: true,
+	}
 )
 
 var deprecatedFlags = []cli.Flag{
@@ -222,6 +223,7 @@ var deprecatedFlags = []cli.Flag{
 	deprecatedCacheProposerIndicesFlag,
 	deprecatedprotoArrayForkChoice,
 	deprecatedForkchoiceAggregateAttestations,
+	deprecatedEnableAttestationCacheFlag,
 }
 
 // ValidatorFlags contains a list of all the feature flags that apply to the validator client.
@@ -244,7 +246,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 	writeSSZStateTransitionsFlag,
 	disableForkChoiceUnsafeFlag,
 	enableSSZCache,
-	enableAttestationCacheFlag,
 	enableEth1DataVoteCacheFlag,
 	initSyncVerifyEverythingFlag,
 	initSyncCacheStateFlag,
@@ -261,7 +262,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
 // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E.
 var E2EBeaconChainFlags = []string{
 	"--enable-ssz-cache",
-	"--enable-attestation-cache",
 	"--cache-proposer-indices",
 	"--cache-filtered-block-tree",
 	"--enable-skip-slots-cache",