Disable forkchoice pre-processing of attestations (#4774)

* Disable forkchoice pre-processing of attestations
* space
* gaz

beacon-chain/operations/attestations/BUILD.bazel

@@ -15,6 +15,7 @@ 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",
@@ -39,6 +40,7 @@ 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

@@ -2,17 +2,24 @@ package attestations
 
 import (
 	"context"
-	"github.com/gogo/protobuf/proto"
 	"reflect"
 	"sort"
 	"testing"
 
+	"github.com/gogo/protobuf/proto"
+	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/go-bitfield"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/shared/bls"
 )
 
+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,6 +10,7 @@ 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"
@@ -49,24 +50,31 @@ func (s *Service) batchForkChoiceAtts(ctx context.Context) error {
 	atts = append(atts, s.pool.BlockAttestations()...)
 	atts = append(atts, s.pool.ForkchoiceAttestations()...)
 
-	for _, att := range atts {
-		seen, err := s.seen(att)
-		if err != nil {
-			return err
-		}
-		if seen {
-			continue
-		}
+	// 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
+			attDataRoot, err := ssz.HashTreeRoot(att.Data)
+			if err != nil {
+				return err
+			}
+			attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
 		}
-		attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
-	}
 
-	for _, atts := range attsByDataRoot {
-		if err := s.aggregateAndSaveForkChoiceAtts(atts); err != nil {
+		for _, atts := range attsByDataRoot {
+			if err := s.aggregateAndSaveForkChoiceAtts(atts); err != nil {
+				return err
+			}
+		}
+	} else {
+		if err := s.pool.SaveForkchoiceAttestations(atts); err != nil {
 			return err
 		}
 	}

shared/featureconfig/config.go

@@ -28,19 +28,20 @@ var log = logrus.WithField("prefix", "flags")
 
 // Flags is a struct to represent which features the client will perform on runtime.
 type Flags struct {
-	CustomGenesisDelay        uint64 // CustomGenesisDelay signals how long of a delay to set to start the chain.
-	MinimalConfig             bool   // MinimalConfig as defined in the spec.
-	WriteSSZStateTransitions  bool   // WriteSSZStateTransitions to tmp directory.
-	InitSyncNoVerify          bool   // InitSyncNoVerify when initial syncing w/o verifying block's contents.
-	SkipBLSVerify             bool   // Skips BLS verification across the runtime.
-	EnableBackupWebhook       bool   // EnableBackupWebhook to allow database backups to trigger from monitoring port /db/backup.
-	PruneEpochBoundaryStates  bool   // PruneEpochBoundaryStates prunes the epoch boundary state before last finalized check point.
-	EnableSnappyDBCompression bool   // EnableSnappyDBCompression in the database.
-	InitSyncCacheState        bool   // InitSyncCacheState caches state during initial sync.
-	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.
-	ProtoArrayForkChoice      bool   // ProtoArrayForkChoice enables proto array fork choice. Significant improvements over the spec version.
+	CustomGenesisDelay              uint64 // CustomGenesisDelay signals how long of a delay to set to start the chain.
+	MinimalConfig                   bool   // MinimalConfig as defined in the spec.
+	WriteSSZStateTransitions        bool   // WriteSSZStateTransitions to tmp directory.
+	InitSyncNoVerify                bool   // InitSyncNoVerify when initial syncing w/o verifying block's contents.
+	SkipBLSVerify                   bool   // Skips BLS verification across the runtime.
+	EnableBackupWebhook             bool   // EnableBackupWebhook to allow database backups to trigger from monitoring port /db/backup.
+	PruneEpochBoundaryStates        bool   // PruneEpochBoundaryStates prunes the epoch boundary state before last finalized check point.
+	EnableSnappyDBCompression       bool   // EnableSnappyDBCompression in the database.
+	InitSyncCacheState              bool   // InitSyncCacheState caches state during initial sync.
+	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.
+	ProtoArrayForkChoice            bool   // ProtoArrayForkChoice enables proto array fork choice. Significant improvements over the spec version.
+	ForkchoiceAggregateAttestations bool   // ForkchoiceAggregateAttestations attempts to aggregate attestations before processing in fork choice.
 
 	// DisableForkChoice disables using LMD-GHOST fork choice to update
 	// the head of the chain based on attestations and instead accepts any valid received block
@@ -143,6 +144,10 @@ func ConfigureBeaconChain(ctx *cli.Context) {
 		log.Warn("Enabled using proto array fork choice over spec fork choice.")
 		cfg.ProtoArrayForkChoice = true
 	}
+	if ctx.GlobalBool(forkchoiceAggregateAttestations.Name) {
+		log.Warn("Enabled fork choice aggregation pre-processing of attestations")
+		cfg.ForkchoiceAggregateAttestations = true
+	}
 	Init(cfg)
 }
 

shared/featureconfig/flags.go

@@ -94,6 +94,11 @@ var (
 		Name:  "proto-array-forkchoice",
 		Usage: "Uses proto array fork choice over the naive spec fork choice. Better implementation in terms of mem usage and speed. ",
 	}
+	forkchoiceAggregateAttestations = cli.BoolFlag{
+		Name: "forkchoice-aggregate-attestations",
+		Usage: "Preprocess attestations by aggregation before running fork choice.",
+	}
+
 )
 
 // Deprecated flags list.