Change attester protection to return default if DB is empty (#5323)

* Change how default values are set * Remove unused imports * Remove wasteful db call * Fix db tests * Fix db test
prysmaticlabs · Apr 6, 2020 · 894508f · 894508f
1 parent 91b19b9
commit 894508f
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 59 deletions.
diff --git a/validator/client/validator_attest.go b/validator/client/validator_attest.go
@@ -83,8 +83,9 @@ 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[:])
+		history, err = v.db.AttestationHistory(ctx, pubKey[:])
 		if err != nil {
 			log.Errorf("Could not get attestation history from DB: %v", err)
 			if v.emitAccountMetrics {
@@ -148,14 +149,6 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot uint64, pubKey [
 	}
 
 	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
-		}
 		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)

diff --git a/validator/client/validator_propose.go b/validator/client/validator_propose.go
@@ -9,6 +9,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
+	"github.com/prysmaticlabs/go-bitfield"
 	"github.com/prysmaticlabs/go-ssz"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/shared/bls"
@@ -85,8 +86,9 @@ func (v *validator) ProposeBlock(ctx context.Context, slot uint64, pubKey [48]by
 		return
 	}
 
+	var slotBits bitfield.Bitlist
 	if featureconfig.Get().ProtectProposer {
-		slotBits, err := v.db.ProposalHistoryForEpoch(ctx, pubKey[:], epoch)
+		slotBits, err = v.db.ProposalHistoryForEpoch(ctx, pubKey[:], epoch)
 		if err != nil {
 			log.WithError(err).Error("Failed to get proposal history")
 			if v.emitAccountMetrics {
@@ -130,14 +132,6 @@ func (v *validator) ProposeBlock(ctx context.Context, slot uint64, pubKey [48]by
 	}
 
 	if featureconfig.Get().ProtectProposer {
-		slotBits, err := v.db.ProposalHistoryForEpoch(ctx, pubKey[:], epoch)
-		if err != nil {
-			log.WithError(err).Error("Failed to get proposal history")
-			if v.emitAccountMetrics {
-				validatorProposeFailVec.WithLabelValues(fmtKey).Inc()
-			}
-			return
-		}
 		slotBits.SetBitAt(slot%params.BeaconConfig().SlotsPerEpoch, true)
 		if err := v.db.SaveProposalHistoryForEpoch(ctx, pubKey[:], epoch, slotBits); err != nil {
 			log.WithError(err).Error("Failed to save updated proposal history")

diff --git a/validator/db/attestation_history.go b/validator/db/attestation_history.go
@@ -6,6 +6,7 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 	slashpb "github.com/prysmaticlabs/prysm/proto/slashing"
+	"github.com/prysmaticlabs/prysm/shared/params"
 	bolt "go.etcd.io/bbolt"
 	"go.opencensus.io/trace"
 )
@@ -31,6 +32,11 @@ func (db *Store) AttestationHistory(ctx context.Context, publicKey []byte) (*sla
 		bucket := tx.Bucket(historicAttestationsBucket)
 		enc := bucket.Get(publicKey)
 		if enc == nil {
+			newMap := make(map[uint64]uint64)
+			newMap[0] = params.BeaconConfig().FarFutureEpoch
+			attestationHistory = &slashpb.AttestationHistory{
+				TargetToSource: newMap,
+			}
 			return nil
 		}
 		attestationHistory, err = unmarshalAttestationHistory(enc)

diff --git a/validator/db/attestation_history_test.go b/validator/db/attestation_history_test.go
@@ -9,7 +9,7 @@ import (
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
 
-func TestAttestationHistory_InitializesNewPubKeys(t *testing.T) {
+func TestAttestationHistory_EmptyVal(t *testing.T) {
 	pubkeys := [][48]byte{{30}, {25}, {20}}
 	db := SetupDB(t, pubkeys)
 	defer TeardownDB(t, db)
@@ -31,22 +31,6 @@ func TestAttestationHistory_InitializesNewPubKeys(t *testing.T) {
 	}
 }
 
-func TestAttestationHistory_NilDB(t *testing.T) {
-	db := SetupDB(t, [][48]byte{})
-	defer TeardownDB(t, db)
-
-	valPubkey := []byte{1, 2, 3}
-
-	attestationHistory, err := db.AttestationHistory(context.Background(), valPubkey)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if attestationHistory != nil {
-		t.Fatalf("Expected attestation history to be nil, received: %v", attestationHistory)
-	}
-}
-
 func TestSaveAttestationHistory_OK(t *testing.T) {
 	db := SetupDB(t, [][48]byte{})
 	defer TeardownDB(t, db)
@@ -187,7 +171,12 @@ func TestDeleteAttestationHistory_OK(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to get attestation history: %v", err)
 	}
-	if savedHistory != nil {
-		t.Fatalf("Expected attestation history to be nil, received %v", savedHistory)
+	cleanMap := make(map[uint64]uint64)
+	cleanMap[0] = params.BeaconConfig().FarFutureEpoch
+	clean := &slashpb.AttestationHistory{
+		TargetToSource: cleanMap,
+	}
+	if !reflect.DeepEqual(savedHistory, clean) {
+		t.Fatalf("Expected attestation history to be %v, received %v", clean, savedHistory)
 	}
 }
diff --git a/validator/db/db.go b/validator/db/db.go
@@ -1,14 +1,11 @@
 package db
 
 import (
-	"context"
 	"os"
 	"path/filepath"
 	"time"
 
 	"github.com/pkg/errors"
-	slashpb "github.com/prysmaticlabs/prysm/proto/slashing"
-	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/validator/db/iface"
 	"github.com/sirupsen/logrus"
 	bolt "go.etcd.io/bbolt"
@@ -92,27 +89,10 @@ func NewKVStore(dirPath string, pubKeys [][48]byte) (*Store, error) {
 		return nil, err
 	}
 
-	// Initialize the required pubKeys into the DB to ensure they're not empty.
+	// Initialize the required public keys into the DB to ensure they're not empty.
 	if err := kv.initializeSubBuckets(pubKeys); err != nil {
 		return nil, err
 	}
-	for _, pubkey := range pubKeys {
-		attHistory, err := kv.AttestationHistory(context.Background(), pubkey[:])
-		if err != nil {
-			return nil, err
-		}
-		if attHistory == nil {
-			newMap := make(map[uint64]uint64)
-			newMap[0] = params.BeaconConfig().FarFutureEpoch
-			cleanHistory := &slashpb.AttestationHistory{
-				TargetToSource: newMap,
-			}
-			if err := kv.SaveAttestationHistory(context.Background(), pubkey[:], cleanHistory); err != nil {
-				return nil, err
-			}
-		}
-
-	}
 
 	return kv, err
 }

diff --git a/validator/db/proposal_history.go b/validator/db/proposal_history.go
@@ -97,7 +97,7 @@ func (db *Store) initializeSubBuckets(pubKeys [][48]byte) error {
 		bucket := tx.Bucket(historicProposalsBucket)
 		for _, pubKey := range pubKeys {
 			if _, err := bucket.CreateBucketIfNotExists(pubKey[:]); err != nil {
-				return errors.Wrap(err, "failed to delete the proposal history")
+				return errors.Wrap(err, "failed to create proposal history bucket")
 			}
 		}
 		return nil