Refactor validator protection for attesters (#5695)

* Finish refactor for attestation protection

* Finish refactor for proposal history

* Revert "Finish refactor for proposal history"

This reverts commit 2f13720.

* Fix tests

* Implement UpdateProtections

* Implement refactor

* Fixes

* fix

* Undo proposer changes

* Fix test

* Final look through

* Add tests for protections function

* Add lock

* Add flag in front of attester protection code

* Add proper rwlocks and fix flags

* fix

* fix build

* Fix atestation tests

* Fix deprecated flags

* Add protect attester to standard attesting test

* Remove comment

* gofmt

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

shared/featureconfig/config.go

@@ -233,15 +233,13 @@ func ConfigureValidator(ctx *cli.Context) {
 	complainOnDeprecatedFlags(ctx)
 	cfg := &Flags{}
 	cfg = configureConfig(ctx, cfg)
-	cfg.ProtectProposer = true
-	if ctx.Bool(disableProtectProposerFlag.Name) {
-		log.Warn("Disabled validator proposal slashing protection.")
-		cfg.ProtectProposer = false
-	}
-	cfg.ProtectAttester = true
-	if ctx.Bool(disableProtectAttesterFlag.Name) {
-		log.Warn("Disabled validator attestation slashing protection.")
-		cfg.ProtectAttester = false
+	if ctx.Bool(enableProtectProposerFlag.Name) {
+		log.Warn("Enabled validator proposal slashing protection.")
+		cfg.ProtectProposer = true
+	}
+	if ctx.Bool(enableProtectAttesterFlag.Name) {
+		log.Warn("Enabled validator attestation slashing protection.")
+		cfg.ProtectAttester = true
 	}
 	if ctx.Bool(enableDomainDataCacheFlag.Name) {
 		log.Warn("Enabled domain data cache.")

shared/featureconfig/flags.go

@@ -75,17 +75,15 @@ var (
 		Usage: "Cache filtered block tree by maintaining it rather than continually recalculating on the fly, " +
 			"this is used for fork choice.",
 	}
-	disableProtectProposerFlag = &cli.BoolFlag{
-		Name: "disable-protect-proposer",
-		Usage: "Disables functionality to prevent the validator client from signing and " +
+	enableProtectProposerFlag = &cli.BoolFlag{
+		Name: "enable-protect-proposer",
+		Usage: "Enables functionality to prevent the validator client from signing and " +
 			"broadcasting 2 different block proposals in the same epoch. Protects from slashing.",
-		Value: true,
 	}
-	disableProtectAttesterFlag = &cli.BoolFlag{
-		Name: "disable-protect-attester",
-		Usage: "Disables functionality to prevent the validator client from signing and " +
+	enableProtectAttesterFlag = &cli.BoolFlag{
+		Name: "enable-protect-attester",
+		Usage: "Enables functionality to prevent the validator client from signing and " +
 			"broadcasting 2 any slashable attestations.",
-		Value: true,
 	}
 	disableStrictAttestationPubsubVerificationFlag = &cli.BoolFlag{
 		Name:  "disable-strict-attestation-pubsub-verification",
@@ -315,6 +313,16 @@ var (
 		Usage:  deprecatedUsage,
 		Hidden: true,
 	}
+	deprecatedDisableProtectProposerFlag = &cli.BoolFlag{
+		Name:   "disable-protect-proposer",
+		Usage:  deprecatedUsage,
+		Hidden: true,
+	}
+	deprecatedDisableProtectAttesterFlag = &cli.BoolFlag{
+		Name:   "disable-protect-attester",
+		Usage:  deprecatedUsage,
+		Hidden: true,
+	}
 	deprecatedDisableInitSyncQueueFlag = &cli.BoolFlag{
 		Name:   "disable-init-sync-queue",
 		Usage:  deprecatedUsage,
@@ -353,14 +361,16 @@ var deprecatedFlags = []cli.Flag{
 	deprecatedDiscv5Flag,
 	deprecatedEnableSSZCache,
 	deprecatedUseSpanCacheFlag,
+	deprecatedDisableProtectProposerFlag,
+	deprecatedDisableProtectAttesterFlag,
 	deprecatedDisableInitSyncQueueFlag,
 }
 
 // ValidatorFlags contains a list of all the feature flags that apply to the validator client.
 var ValidatorFlags = append(deprecatedFlags, []cli.Flag{
 	minimalConfigFlag,
-	disableProtectAttesterFlag,
-	disableProtectProposerFlag,
+	enableProtectAttesterFlag,
+	enableProtectProposerFlag,
 	enableDomainDataCacheFlag,
 	waitForSyncedFlag,
 }...)
@@ -374,6 +384,8 @@ var SlasherFlags = append(deprecatedFlags, []cli.Flag{
 var E2EValidatorFlags = []string{
 	"--enable-domain-data-cache",
 	"--wait-for-synced",
+	"--enable-protect-attester",
+	"--enable-protect-proposer",
 }
 
 // BeaconChainFlags contains a list of all the feature flags that apply to the beacon-chain client.

validator/client/fake_validator_test.go

@@ -16,10 +16,12 @@ type fakeValidator struct {
 	NextSlotCalled                   bool
 	CanonicalHeadSlotCalled          bool
 	UpdateDutiesCalled               bool
+	UpdateProtectionsCalled          bool
 	RoleAtCalled                     bool
 	AttestToBlockHeadCalled          bool
 	ProposeBlockCalled               bool
 	LogValidatorGainsAndLossesCalled bool
+	SaveProtectionsCalled            bool
 	SlotDeadlineCalled               bool
 	ProposeBlockArg1                 uint64
 	AttestToBlockHeadArg1            uint64
@@ -76,11 +78,21 @@ func (fv *fakeValidator) UpdateDuties(_ context.Context, slot uint64) error {
 	return fv.UpdateDutiesRet
 }
 
+func (fv *fakeValidator) UpdateProtections(_ context.Context, slot uint64) error {
+	fv.UpdateProtectionsCalled = true
+	return nil
+}
+
 func (fv *fakeValidator) LogValidatorGainsAndLosses(_ context.Context, slot uint64) error {
 	fv.LogValidatorGainsAndLossesCalled = true
 	return nil
 }
 
+func (fv *fakeValidator) SaveProtections(_ context.Context) error {
+	fv.SaveProtectionsCalled = true
+	return nil
+}
+
 func (fv *fakeValidator) RolesAt(_ context.Context, slot uint64) (map[[48]byte][]validatorRole, error) {
 	fv.RoleAtCalled = true
 	fv.RoleAtArg1 = slot

validator/client/runner.go

@@ -27,11 +27,13 @@ type Validator interface {
 	SlotDeadline(slot uint64) time.Time
 	LogValidatorGainsAndLosses(ctx context.Context, slot uint64) error
 	UpdateDuties(ctx context.Context, slot uint64) error
+	UpdateProtections(ctx context.Context, slot uint64) error
 	RolesAt(ctx context.Context, slot uint64) (map[[48]byte][]validatorRole, error) // validator pubKey -> roles
 	SubmitAttestation(ctx context.Context, slot uint64, pubKey [48]byte)
 	ProposeBlock(ctx context.Context, slot uint64, pubKey [48]byte)
 	SubmitAggregateAndProof(ctx context.Context, slot uint64, pubKey [48]byte)
 	LogAttestationsSubmitted()
+	SaveProtections(ctx context.Context) error
 	UpdateDomainDataCaches(ctx context.Context, slot uint64)
 }
 
@@ -96,6 +98,12 @@ func run(ctx context.Context, v Validator) {
 				continue
 			}
 
+			if featureconfig.Get().ProtectAttester {
+				if err := v.UpdateProtections(ctx, slot); err != nil {
+					log.WithError(err).Error("Could not update validator protection")
+				}
+			}
+
 			// Start fetching domain data for the next epoch.
 			if helpers.IsEpochEnd(slot) {
 				go v.UpdateDomainDataCaches(ctx, slot+1)
@@ -132,6 +140,11 @@ func run(ctx context.Context, v Validator) {
 			go func() {
 				wg.Wait()
 				v.LogAttestationsSubmitted()
+				if featureconfig.Get().ProtectAttester {
+					if err := v.SaveProtections(ctx); err != nil {
+						log.WithError(err).Error("Could not save validator protection")
+					}
+				}
 				span.End()
 			}()
 		}

validator/client/validator.go

@@ -21,6 +21,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
+	slashpb "github.com/prysmaticlabs/prysm/proto/slashing"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
@@ -60,6 +61,8 @@ type validator struct {
 	domainDataCache                    *ristretto.Cache
 	aggregatedSlotCommitteeIDCache     *lru.Cache
 	aggregatedSlotCommitteeIDCacheLock sync.Mutex
+	attesterHistoryByPubKey            map[[48]byte]*slashpb.AttestationHistory
+	attesterHistoryByPubKeyLock        sync.RWMutex
 }
 
 var validatorStatusesGaugeVec = promauto.NewGaugeVec(
@@ -441,6 +444,35 @@ func (v *validator) RolesAt(ctx context.Context, slot uint64) (map[[48]byte][]va
 	return rolesAt, nil
 }
 
+// UpdateProtections goes through the duties of the given slot and fetches the required validator history,
+// assigning it in validator.
+func (v *validator) UpdateProtections(ctx context.Context, slot uint64) error {
+	attestingPubKeys := make([][48]byte, 0, len(v.duties.Duties))
+	for _, duty := range v.duties.Duties {
+		if duty == nil {
+			continue
+		}
+		if duty.AttesterSlot == slot {
+			attestingPubKeys = append(attestingPubKeys, bytesutil.ToBytes48(duty.PublicKey))
+		}
+	}
+	attHistoryByPubKey, err := v.db.AttestationHistoryForPubKeys(ctx, attestingPubKeys)
+	if err != nil {
+		return errors.Wrap(err, "could not get attester history")
+	}
+	v.attesterHistoryByPubKey = attHistoryByPubKey
+	return nil
+}
+
+// SaveProtections saves the attestation information currently in validator state.
+func (v *validator) SaveProtections(ctx context.Context) error {
+	if err := v.db.SaveAttestationHistoryForPubKeys(ctx, v.attesterHistoryByPubKey); err != nil {
+		return errors.Wrap(err, "could not save attester history to DB")
+	}
+	v.attesterHistoryByPubKey = make(map[[48]byte]*slashpb.AttestationHistory)
+	return nil
+}
+
 // isAggregator checks if a validator is an aggregator of a given slot, it uses the selection algorithm outlined in:
 // https://github.com/ethereum/eth2.0-specs/blob/v0.9.3/specs/validator/0_beacon-chain-validator.md#aggregation-selection
 func (v *validator) isAggregator(ctx context.Context, committee []uint64, slot uint64, pubKey [48]byte) (bool, error) {

validator/client/validator_attest.go

@@ -71,6 +71,9 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot uint64, pubKey [
 		log.Debug("Empty committee for validator duty, not attesting")
 		return
 	}
+	v.attesterHistoryByPubKeyLock.RLock()
+	attesterHistory := v.attesterHistoryByPubKey[pubKey]
+	v.attesterHistoryByPubKeyLock.RUnlock()
 
 	v.waitToSlotOneThird(ctx, slot)
 
@@ -87,17 +90,8 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot uint64, pubKey [
 		return
 	}
 
-	var history *slashpb.AttestationHistory
 	if featureconfig.Get().ProtectAttester {
-		history, err = v.db.AttestationHistory(ctx, pubKey[:])
-		if err != nil {
-			log.Errorf("Could not get attestation history from DB: %v", err)
-			if v.emitAccountMetrics {
-				validatorAttestFailVec.WithLabelValues(fmtKey).Inc()
-			}
-			return
-		}
-		if isNewAttSlashable(history, data.Source.Epoch, data.Target.Epoch) {
+		if isNewAttSlashable(attesterHistory, data.Source.Epoch, data.Target.Epoch) {
 			log.WithFields(logrus.Fields{
 				"sourceEpoch": data.Source.Epoch,
 				"targetEpoch": data.Target.Epoch,
@@ -152,17 +146,6 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot uint64, pubKey [
 		return
 	}
 
-	if featureconfig.Get().ProtectAttester {
-		history = markAttestationForTargetEpoch(history, data.Source.Epoch, data.Target.Epoch)
-		if err := v.db.SaveAttestationHistory(ctx, pubKey[:], history); err != nil {
-			log.Errorf("Could not save attestation history to DB: %v", err)
-			if v.emitAccountMetrics {
-				validatorAttestFailVec.WithLabelValues(fmtKey).Inc()
-			}
-			return
-		}
-	}
-
 	if err := v.saveAttesterIndexToData(data, duty.ValidatorIndex); err != nil {
 		log.WithError(err).Error("Could not save validator index for logging")
 		if v.emitAccountMetrics {
@@ -171,6 +154,13 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot uint64, pubKey [
 		return
 	}
 
+	if featureconfig.Get().ProtectAttester {
+		attesterHistory = markAttestationForTargetEpoch(attesterHistory, data.Source.Epoch, data.Target.Epoch)
+		v.attesterHistoryByPubKeyLock.Lock()
+		v.attesterHistoryByPubKey[pubKey] = attesterHistory
+		v.attesterHistoryByPubKeyLock.Unlock()
+	}
+
 	if v.emitAccountMetrics {
 		validatorAttestSuccessVec.WithLabelValues(fmtKey).Inc()
 	}