Skip to content

Commit

Permalink
More feature flags removal (#7526)
Browse files Browse the repository at this point in the history
* Remove disable domain cache

* Remove don't verify att sig flag

* Remove batch verify and attester copies

* Remove batch verify usage

* Update tests

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
terencechain and prylabs-bulldozer[bot] authored Oct 14, 2020
1 parent 0a00738 commit 8f04c55
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 154 deletions.
11 changes: 2 additions & 9 deletions beacon-chain/cache/attestation_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/prometheus/client_golang/prometheus/promauto"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"k8s.io/client-go/tools/cache"
)

Expand Down Expand Up @@ -99,10 +98,7 @@ func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRe

if exists && item != nil && item.(*attestationReqResWrapper).res != nil {
attestationCacheHit.Inc()
if featureconfig.Get().ReduceAttesterStateCopy {
return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil
}
return item.(*attestationReqResWrapper).res, nil
return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil
}
attestationCacheMiss.Inc()
return nil, nil
Expand Down Expand Up @@ -167,10 +163,7 @@ func wrapperToKey(i interface{}) (string, error) {
}

func reqToKey(req *ethpb.AttestationDataRequest) (string, error) {
if featureconfig.Get().ReduceAttesterStateCopy {
return fmt.Sprintf("%d", req.Slot), nil
}
return fmt.Sprintf("%d-%d", req.CommitteeIndex, req.Slot), nil
return fmt.Sprintf("%d", req.Slot), nil
}

type attestationReqResWrapper struct {
Expand Down
9 changes: 2 additions & 7 deletions beacon-chain/rpc/validator/attester.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/shared/bls"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"github.com/prysmaticlabs/prysm/shared/params"
"go.opencensus.io/trace"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -44,9 +43,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
return nil, status.Errorf(codes.Internal, "Could not retrieve data from attestation cache: %v", err)
}
if res != nil {
if featureconfig.Get().ReduceAttesterStateCopy {
res.CommitteeIndex = req.CommitteeIndex
}
res.CommitteeIndex = req.CommitteeIndex
return res, nil
}

Expand All @@ -59,9 +56,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
if res == nil {
return nil, status.Error(codes.DataLoss, "A request was in progress and resolved to nil")
}
if featureconfig.Get().ReduceAttesterStateCopy {
res.CommitteeIndex = req.CommitteeIndex
}
res.CommitteeIndex = req.CommitteeIndex
return res, nil
}
return nil, status.Errorf(codes.Internal, "Could not mark attestation as in-progress: %v", err)
Expand Down
3 changes: 2 additions & 1 deletion beacon-chain/rpc/validator/attester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ func TestAttestationDataSlot_handlesInProgressRequest(t *testing.T) {
}

res := &ethpb.AttestationData{
Target: &ethpb.Checkpoint{Epoch: 55, Root: make([]byte, 32)},
CommitteeIndex: 1,
Target: &ethpb.Checkpoint{Epoch: 55, Root: make([]byte, 32)},
}

require.NoError(t, server.AttestationCache.MarkInProgress(req))
Expand Down
1 change: 0 additions & 1 deletion beacon-chain/state/stategen/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ go_library(
"//beacon-chain/state:go_default_library",
"//proto/beacon/p2p/v1:go_default_library",
"//shared/bytesutil:go_default_library",
"//shared/featureconfig:go_default_library",
"//shared/params:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@com_github_prometheus_client_golang//prometheus:go_default_library",
Expand Down
27 changes: 6 additions & 21 deletions beacon-chain/state/stategen/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/db/filters"
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"go.opencensus.io/trace"
)

Expand All @@ -30,32 +29,18 @@ func (s *State) ReplayBlocks(ctx context.Context, state *stateTrie.BeaconState,
if state.Slot() >= signed[i].Block.Slot {
continue
}
if featureconfig.Get().EnableStateGenSigVerify {
state, err = transition.ExecuteStateTransition(ctx, state, signed[i])
if err != nil {
return nil, err
}
} else {
state, err = executeStateTransitionStateGen(ctx, state, signed[i])
if err != nil {
return nil, err
}
state, err = executeStateTransitionStateGen(ctx, state, signed[i])
if err != nil {
return nil, err
}
}
}

// If there is skip slots at the end.
if targetSlot > state.Slot() {
if featureconfig.Get().EnableStateGenSigVerify {
state, err = transition.ProcessSlots(ctx, state, targetSlot)
if err != nil {
return nil, err
}
} else {
state, err = processSlotsStateGen(ctx, state, targetSlot)
if err != nil {
return nil, err
}
state, err = processSlotsStateGen(ctx, state, targetSlot)
if err != nil {
return nil, err
}
}

Expand Down
1 change: 0 additions & 1 deletion beacon-chain/sync/initial-sync/initial_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ func TestMain(m *testing.M) {
logrus.SetOutput(ioutil.Discard)

resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{
BatchBlockVerify: true,
EnablePeerScorer: true,
})
defer resetCfg()
Expand Down
16 changes: 2 additions & 14 deletions beacon-chain/sync/initial-sync/round_robin.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,8 @@ func (s *Service) processFetchedData(
defer s.updatePeerScorerStats(data.pid, startSlot)

// Use Batch Block Verify to process and verify batches directly.
if featureconfig.Get().BatchBlockVerify {
batchReceiver := s.chain.ReceiveBlockBatch
if err := s.processBatchedBlocks(ctx, genesis, data.blocks, batchReceiver); err != nil {
log.WithError(err).Debug("Batch is not processed")
}
return
}

blockReceiver := s.chain.ReceiveBlockInitialSync
for _, blk := range data.blocks {
if err := s.processBlock(ctx, genesis, blk, blockReceiver); err != nil {
log.WithError(err).Debug("Block is not processed")
continue
}
if err := s.processBatchedBlocks(ctx, genesis, data.blocks, s.chain.ReceiveBlockBatch); err != nil {
log.WithError(err).Debug("Batch is not processed")
}
}

Expand Down
8 changes: 3 additions & 5 deletions beacon-chain/sync/validate_aggregate_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,9 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe
}

// Verify aggregated attestation has a valid signature.
if !featureconfig.Get().DisableStrictAttestationPubsubVerification {
if err := blocks.VerifyAttestationSignature(ctx, bs, signed.Message.Aggregate); err != nil {
traceutil.AnnotateError(span, err)
return pubsub.ValidationReject
}
if err := blocks.VerifyAttestationSignature(ctx, bs, signed.Message.Aggregate); err != nil {
traceutil.AnnotateError(span, err)
return pubsub.ValidationReject
}

return pubsub.ValidationAccept
Expand Down
11 changes: 4 additions & 7 deletions beacon-chain/sync/validate_beacon_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,10 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p
return pubsub.ValidationReject
}

// Attestation's signature is a valid BLS signature and belongs to correct public key..
if !featureconfig.Get().DisableStrictAttestationPubsubVerification {
if err := blocks.VerifyAttestationSignature(ctx, preState, att); err != nil {
log.WithError(err).Error("Could not verify attestation")
traceutil.AnnotateError(span, err)
return pubsub.ValidationReject
}
if err := blocks.VerifyAttestationSignature(ctx, preState, att); err != nil {
log.WithError(err).Error("Could not verify attestation")
traceutil.AnnotateError(span, err)
return pubsub.ValidationReject
}

s.setSeenCommitteeIndicesSlot(att.Data.Slot, att.Data.CommitteeIndex, att.AggregationBits)
Expand Down
70 changes: 21 additions & 49 deletions shared/featureconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,32 +41,27 @@ type Flags struct {
ZinkenTestnet bool // ZinkenTestnet defines the flag through which we can enable the node to run on the Zinken testnet.

// Feature related flags.
WriteSSZStateTransitions bool // WriteSSZStateTransitions to tmp directory.
DisableDynamicCommitteeSubnets bool // Disables dynamic attestation committee subnets via p2p.
SkipBLSVerify bool // Skips BLS verification across the runtime.
EnableBlst bool // Enables new BLS library from supranational.
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.
LocalProtection bool // LocalProtection prevents the validator client from signing any messages that would be considered a slashable offense from the validators view.
SlasherProtection bool // SlasherProtection protects validator fron sending over a slashable offense over the network using external slasher.
DisableStrictAttestationPubsubVerification bool // DisableStrictAttestationPubsubVerification will disabling strict signature verification in pubsub.
DisableUpdateHeadPerAttestation bool // DisableUpdateHeadPerAttestation will disabling update head on per attestation basis.
EnableDomainDataCache bool // EnableDomainDataCache caches validator calls to DomainData per epoch.
EnableStateGenSigVerify bool // EnableStateGenSigVerify verifies proposer and randao signatures during state gen.
CheckHeadState bool // CheckHeadState checks the current headstate before retrieving the desired state from the db.
EnableNoise bool // EnableNoise enables the beacon node to use NOISE instead of SECIO when performing a handshake with another peer.
DontPruneStateStartUp bool // DontPruneStateStartUp disables pruning state upon beacon node start up.
WaitForSynced bool // WaitForSynced uses WaitForSynced in validator startup to ensure it can communicate with the beacon node as soon as possible.
ReduceAttesterStateCopy bool // ReduceAttesterStateCopy reduces head state copies for attester rpc.
EnableAccountsV2 bool // EnableAccountsV2 for Prysm validator clients.
BatchBlockVerify bool // BatchBlockVerify performs batched verification of block batches that we receive when syncing.
InitSyncVerbose bool // InitSyncVerbose logs every processed block during initial syncing.
EnableFinalizedDepositsCache bool // EnableFinalizedDepositsCache enables utilization of cached finalized deposits.
EnableEth1DataMajorityVote bool // EnableEth1DataMajorityVote uses the Voting With The Majority algorithm to vote for eth1data.
EnableAttBroadcastDiscoveryAttempts bool // EnableAttBroadcastDiscoveryAttempts allows the p2p service to attempt to ensure a subnet peer is present before broadcasting an attestation.
EnablePeerScorer bool // EnablePeerScorer enables experimental peer scoring in p2p.
EnablePruningDepositProofs bool // EnablePruningDepositProofs enables pruning deposit proofs which significantly reduces the size of a deposit
WriteSSZStateTransitions bool // WriteSSZStateTransitions to tmp directory.
DisableDynamicCommitteeSubnets bool // Disables dynamic attestation committee subnets via p2p.
SkipBLSVerify bool // Skips BLS verification across the runtime.
EnableBlst bool // Enables new BLS library from supranational.
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.
LocalProtection bool // LocalProtection prevents the validator client from signing any messages that would be considered a slashable offense from the validators view.
SlasherProtection bool // SlasherProtection protects validator fron sending over a slashable offense over the network using external slasher.
DisableUpdateHeadPerAttestation bool // DisableUpdateHeadPerAttestation will disabling update head on per attestation basis.
CheckHeadState bool // CheckHeadState checks the current headstate before retrieving the desired state from the db.
EnableNoise bool // EnableNoise enables the beacon node to use NOISE instead of SECIO when performing a handshake with another peer.
DontPruneStateStartUp bool // DontPruneStateStartUp disables pruning state upon beacon node start up.
WaitForSynced bool // WaitForSynced uses WaitForSynced in validator startup to ensure it can communicate with the beacon node as soon as possible.
EnableAccountsV2 bool // EnableAccountsV2 for Prysm validator clients.
InitSyncVerbose bool // InitSyncVerbose logs every processed block during initial syncing.
EnableFinalizedDepositsCache bool // EnableFinalizedDepositsCache enables utilization of cached finalized deposits.
EnableEth1DataMajorityVote bool // EnableEth1DataMajorityVote uses the Voting With The Majority algorithm to vote for eth1data.
EnableAttBroadcastDiscoveryAttempts bool // EnableAttBroadcastDiscoveryAttempts allows the p2p service to attempt to ensure a subnet peer is present before broadcasting an attestation.
EnablePeerScorer bool // EnablePeerScorer enables experimental peer scoring in p2p.
EnablePruningDepositProofs bool // EnablePruningDepositProofs enables pruning deposit proofs which significantly reduces the size of a deposit

// DisableForkChoice disables using LMD-GHOST fork choice to update
// the head of the chain based on attestations and instead accepts any valid received block
Expand Down Expand Up @@ -195,18 +190,10 @@ func ConfigureBeaconChain(ctx *cli.Context) {
log.Warn("Enabled filtered block tree cache for fork choice.")
cfg.EnableBlockTreeCache = true
}
if ctx.Bool(disableStrictAttestationPubsubVerificationFlag.Name) {
log.Warn("Disabled strict attestation signature verification in pubsub")
cfg.DisableStrictAttestationPubsubVerification = true
}
if ctx.Bool(disableUpdateHeadPerAttestation.Name) {
log.Warn("Disabled update head on per attestation basis")
cfg.DisableUpdateHeadPerAttestation = true
}
if ctx.Bool(enableStateGenSigVerify.Name) {
log.Warn("Enabling sig verify for state gen")
cfg.EnableStateGenSigVerify = true
}
if ctx.Bool(checkHeadState.Name) {
log.Warn("Enabling check head state for chainservice")
cfg.CheckHeadState = true
Expand All @@ -224,11 +211,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
log.Warn("Disabling slashing broadcasting to p2p network")
cfg.DisableBroadcastSlashings = true
}
cfg.ReduceAttesterStateCopy = true
if ctx.Bool(disableReduceAttesterStateCopy.Name) {
log.Warn("Disabling reducing attester state copy")
cfg.ReduceAttesterStateCopy = false
}
if ctx.IsSet(disableGRPCConnectionLogging.Name) {
cfg.DisableGRPCConnectionLogs = true
}
Expand All @@ -240,11 +222,6 @@ func ConfigureBeaconChain(ctx *cli.Context) {
log.Warn("Disabling new beacon state locks")
cfg.NewBeaconStateLocks = false
}
cfg.BatchBlockVerify = true
if ctx.Bool(disableBatchBlockVerify.Name) {
log.Warn("Disabling batch block verification when syncing.")
cfg.BatchBlockVerify = false
}
if ctx.Bool(initSyncVerbose.Name) {
log.Warn("Logging every processed block during initial syncing.")
cfg.InitSyncVerbose = true
Expand Down Expand Up @@ -320,11 +297,6 @@ func ConfigureValidator(ctx *cli.Context) {
log.Warn("Enabled validator attestation and block slashing protection using an external slasher.")
cfg.SlasherProtection = true
}
cfg.EnableDomainDataCache = true
if ctx.Bool(disableDomainDataCacheFlag.Name) {
log.Warn("Disabled domain data cache.")
cfg.EnableDomainDataCache = false
}
Init(cfg)
}

Expand Down
28 changes: 0 additions & 28 deletions shared/featureconfig/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,24 +77,10 @@ var (
Usage: "Enables the validator to connect to external slasher to prevent it from " +
"transmitting a slashable offence over the network.",
}
disableStrictAttestationPubsubVerificationFlag = &cli.BoolFlag{
Name: "disable-strict-attestation-pubsub-verification",
Usage: "Disable strict signature verification of attestations in pubsub. See PR 4782 for details.",
}
disableUpdateHeadPerAttestation = &cli.BoolFlag{
Name: "disable-update-head-attestation",
Usage: "Disable update fork choice head on per attestation. See PR 4802 for details.",
}
disableDomainDataCacheFlag = &cli.BoolFlag{
Name: "disable-domain-data-cache",
Usage: "Disable caching of domain data requests per epoch. This feature reduces the total " +
"calls to the beacon node for each assignment.",
}
enableStateGenSigVerify = &cli.BoolFlag{
Name: "enable-state-gen-sig-verify",
Usage: "Enable signature verification for state gen. This feature increases the cost to generate a historical state," +
"the resulting state is signature verified.",
}
checkHeadState = &cli.BoolFlag{
Name: "check-head-state",
Usage: "Enables the checking of head state in chainservice first before retrieving the desired state from the db.",
Expand All @@ -116,10 +102,6 @@ var (
Name: "disable-lookback",
Usage: "Disables use of the lookback feature and updates attestation history for validators from head to epoch 0",
}
disableReduceAttesterStateCopy = &cli.BoolFlag{
Name: "disable-reduce-attester-state-copy",
Usage: "Disables the feature to reduce the amount of state copies for attester rpc",
}
disableGRPCConnectionLogging = &cli.BoolFlag{
Name: "disable-grpc-connection-logging",
Usage: "Disables displaying logs for newly connected grpc clients",
Expand All @@ -133,10 +115,6 @@ var (
Name: "disable-new-beacon-state-locks",
Usage: "Disable new beacon state locking",
}
disableBatchBlockVerify = &cli.BoolFlag{
Name: "disable-batch-block-verify",
Usage: "Disable full signature verification of blocks in batches instead of singularly.",
}
initSyncVerbose = &cli.BoolFlag{
Name: "init-sync-verbose",
Usage: "Enable logging every processed block during initial syncing.",
Expand Down Expand Up @@ -188,7 +166,6 @@ var devModeFlags = []cli.Flag{
var ValidatorFlags = append(deprecatedFlags, []cli.Flag{
enableLocalProtectionFlag,
enableExternalSlasherProtectionFlag,
disableDomainDataCacheFlag,
waitForSyncedFlag,
AltonaTestnet,
OnyxTestnet,
Expand Down Expand Up @@ -223,15 +200,12 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
kafkaBootstrapServersFlag,
enableBackupWebhookFlag,
cacheFilteredBlockTreeFlag,
disableStrictAttestationPubsubVerificationFlag,
disableUpdateHeadPerAttestation,
enableStateGenSigVerify,
checkHeadState,
disableNoiseHandshake,
dontPruneStateStartUp,
disableBroadcastSlashingFlag,
waitForSyncedFlag,
disableReduceAttesterStateCopy,
disableGRPCConnectionLogging,
attestationAggregationStrategy,
disableNewBeaconStateLocks,
Expand All @@ -240,7 +214,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
MedallaTestnet,
SpadinaTestnet,
ZinkenTestnet,
disableBatchBlockVerify,
initSyncVerbose,
disableFinalizedDepositsCache,
enableBlst,
Expand All @@ -254,7 +227,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{
"--cache-filtered-block-tree",
"--enable-state-gen-sig-verify",
"--check-head-state",
"--attestation-aggregation-strategy=max_cover",
"--dev",
Expand Down
Loading

0 comments on commit 8f04c55

Please sign in to comment.