From dd3737fb1bc820838f1213c7f6cce83e2b4786d1 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 6 Apr 2020 17:50:27 -0400 Subject: [PATCH 1/5] Change how default values are set --- validator/db/attestation_history.go | 6 ++++++ validator/db/db.go | 19 +------------------ validator/db/proposal_history.go | 2 +- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/validator/db/attestation_history.go b/validator/db/attestation_history.go index 572b531058d..e9e657d0531 100644 --- 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/db.go b/validator/db/db.go index 7416a8b5c94..37b84ac71cc 100644 --- a/validator/db/db.go +++ b/validator/db/db.go @@ -92,27 +92,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 index 19b980b5c10..c02b20f0b85 100644 --- 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 From 6b8ec3e03123a28a4d1f492752a9b227a80a359a Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 6 Apr 2020 17:55:26 -0400 Subject: [PATCH 2/5] Remove unused imports --- validator/db/db.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/validator/db/db.go b/validator/db/db.go index 37b84ac71cc..e043d9cac8b 100644 --- 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" From 5b112e21d060abcfd6430ec43e027bdf1c319ac5 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 6 Apr 2020 17:59:21 -0400 Subject: [PATCH 3/5] Remove wasteful db call --- validator/client/validator_attest.go | 11 ++--------- validator/client/validator_propose.go | 12 +++--------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/validator/client/validator_attest.go b/validator/client/validator_attest.go index 7e05e35f31b..e5046a4b51b 100644 --- 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 index 26e7bbb5fb8..0420e62ec68 100644 --- 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") From 05044baa3e7b28cfe8058117727fdf051a2ab04e Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 6 Apr 2020 18:19:28 -0400 Subject: [PATCH 4/5] Fix db tests --- validator/db/attestation_history_test.go | 27 +++++++----------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/validator/db/attestation_history_test.go b/validator/db/attestation_history_test.go index 9747edbf71a..4f06fe41c00 100644 --- 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: newMap, + } + if !reflect.DeepEqual(savedHistory, clean) { + t.Fatalf("Expected attestation history to be clean, received %v", savedHistory) } } From be3bec41af352beafb3abc936f7c97c9d48a877f Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 6 Apr 2020 18:30:37 -0400 Subject: [PATCH 5/5] Fix db test --- validator/db/attestation_history_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator/db/attestation_history_test.go b/validator/db/attestation_history_test.go index 4f06fe41c00..006060edde2 100644 --- a/validator/db/attestation_history_test.go +++ b/validator/db/attestation_history_test.go @@ -174,9 +174,9 @@ func TestDeleteAttestationHistory_OK(t *testing.T) { cleanMap := make(map[uint64]uint64) cleanMap[0] = params.BeaconConfig().FarFutureEpoch clean := &slashpb.AttestationHistory{ - TargetToSource: newMap, + TargetToSource: cleanMap, } if !reflect.DeepEqual(savedHistory, clean) { - t.Fatalf("Expected attestation history to be clean, received %v", savedHistory) + t.Fatalf("Expected attestation history to be %v, received %v", clean, savedHistory) } }