From 0c74e095861f84d2e2bf6ed5555f78b1f52fc0a2 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 12 Feb 2024 16:00:15 +0100 Subject: [PATCH 1/4] `LastEpochWrittenForValidators`: Use golang if. --- beacon-chain/db/slasherkv/slasher.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/beacon-chain/db/slasherkv/slasher.go b/beacon-chain/db/slasherkv/slasher.go index 91225ab8059f..df57e6057fd1 100644 --- a/beacon-chain/db/slasherkv/slasher.go +++ b/beacon-chain/db/slasherkv/slasher.go @@ -46,8 +46,7 @@ func (s *Store) LastEpochWrittenForValidators( for i, encodedIndex := range encodedIndexes { var epoch primitives.Epoch - epochBytes := bkt.Get(encodedIndex) - if epochBytes != nil { + if epochBytes := bkt.Get(encodedIndex); epochBytes != nil { if err := epoch.UnmarshalSSZ(epochBytes); err != nil { return err } From 7383e71dc3fbca2d1e19a47b59f5a6a02be6f45a Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 12 Feb 2024 16:02:57 +0100 Subject: [PATCH 2/4] `SaveLastEpochsWrittenForValidators`: Refactor. --- beacon-chain/db/slasherkv/slasher.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon-chain/db/slasherkv/slasher.go b/beacon-chain/db/slasherkv/slasher.go index df57e6057fd1..7023333d8cd6 100644 --- a/beacon-chain/db/slasherkv/slasher.go +++ b/beacon-chain/db/slasherkv/slasher.go @@ -82,12 +82,14 @@ func (s *Store) SaveLastEpochsWrittenForValidators( return ctx.Err() } + encodedIndex := encodeValidatorIndex(valIndex) + encodedEpoch, err := epoch.MarshalSSZ() if err != nil { return err } - encodedIndexes = append(encodedIndexes, encodeValidatorIndex(valIndex)) + encodedIndexes = append(encodedIndexes, encodedIndex) encodedEpochs = append(encodedEpochs, encodedEpoch) } From 58074395f2a131241ec9d477a4eeef3a403a729e Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 12 Feb 2024 16:58:19 +0100 Subject: [PATCH 3/4] `SaveLastEpochsWrittenForValidators`: Fix when `epochByValIndex` > `batchSize`. Before this commit, `TestStore_LastEpochWrittenForValidators` works if `validatorsCount <= 10000` and stops working if `validatorsCount > 10000`. --- beacon-chain/db/slasherkv/slasher.go | 19 +++++++++---------- beacon-chain/db/slasherkv/slasher_test.go | 23 ++++++++++++++++------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/beacon-chain/db/slasherkv/slasher.go b/beacon-chain/db/slasherkv/slasher.go index 7023333d8cd6..3fb9c98cf0bc 100644 --- a/beacon-chain/db/slasherkv/slasher.go +++ b/beacon-chain/db/slasherkv/slasher.go @@ -52,10 +52,12 @@ func (s *Store) LastEpochWrittenForValidators( } } - attestedEpochs = append(attestedEpochs, &slashertypes.AttestedEpochForValidator{ + attestedEpoch := &slashertypes.AttestedEpochForValidator{ ValidatorIndex: validatorIndexes[i], Epoch: epoch, - }) + } + + attestedEpochs = append(attestedEpochs, attestedEpoch) } return nil @@ -95,7 +97,7 @@ func (s *Store) SaveLastEpochsWrittenForValidators( // The list of validators might be too massive for boltdb to handle in a single transaction, // so instead we split it into batches and write each batch. - for i := 0; i < len(encodedIndexes); i += batchSize { + for start := 0; start < len(encodedIndexes); start += batchSize { if ctx.Err() != nil { return ctx.Err() } @@ -106,17 +108,14 @@ func (s *Store) SaveLastEpochsWrittenForValidators( } bkt := tx.Bucket(attestedEpochsByValidator) + end := min(start+batchSize, len(encodedIndexes)) - minimum := i + batchSize - if minimum > len(encodedIndexes) { - minimum = len(encodedIndexes) - } - - for j, encodedIndex := range encodedIndexes[i:minimum] { + for j, encodedIndex := range encodedIndexes[start:end] { if ctx.Err() != nil { return ctx.Err() } - if err := bkt.Put(encodedIndex, encodedEpochs[j]); err != nil { + + if err := bkt.Put(encodedIndex, encodedEpochs[j+start]); err != nil { return err } } diff --git a/beacon-chain/db/slasherkv/slasher_test.go b/beacon-chain/db/slasherkv/slasher_test.go index cbf6a52252b0..da3070a12b6b 100644 --- a/beacon-chain/db/slasherkv/slasher_test.go +++ b/beacon-chain/db/slasherkv/slasher_test.go @@ -46,21 +46,29 @@ func TestStore_AttestationRecordForValidator_SaveRetrieve(t *testing.T) { func TestStore_LastEpochWrittenForValidators(t *testing.T) { ctx := context.Background() beaconDB := setupDB(t) - indices := []primitives.ValidatorIndex{1, 2, 3} - epoch := primitives.Epoch(5) + + validatorsCount := 11000 + indices := make([]primitives.ValidatorIndex, validatorsCount) + epochs := make([]primitives.Epoch, validatorsCount) + + for i := 0; i < validatorsCount; i++ { + indices[i] = primitives.ValidatorIndex(i) + epochs[i] = primitives.Epoch(i) + } attestedEpochs, err := beaconDB.LastEpochWrittenForValidators(ctx, indices) require.NoError(t, err) require.Equal(t, true, len(attestedEpochs) == len(indices)) + for _, item := range attestedEpochs { require.Equal(t, primitives.Epoch(0), item.Epoch) } - epochsByValidator := map[primitives.ValidatorIndex]primitives.Epoch{ - 1: epoch, - 2: epoch, - 3: epoch, + epochsByValidator := make(map[primitives.ValidatorIndex]primitives.Epoch, validatorsCount) + for i := 0; i < validatorsCount; i++ { + epochsByValidator[indices[i]] = epochs[i] } + err = beaconDB.SaveLastEpochsWrittenForValidators(ctx, epochsByValidator) require.NoError(t, err) @@ -70,9 +78,10 @@ func TestStore_LastEpochWrittenForValidators(t *testing.T) { for i, retrievedEpoch := range retrievedEpochs { want := &slashertypes.AttestedEpochForValidator{ - Epoch: epoch, + Epoch: epochs[i], ValidatorIndex: indices[i], } + require.DeepEqual(t, want, retrievedEpoch) } } From c8b1324854bfcd6f0f5bfcc5731a8cfb428443a4 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 13 Feb 2024 10:26:49 +0100 Subject: [PATCH 4/4] Slasher: Detect surrounded votes in multiple batches. Fixes https://github.com/prysmaticlabs/prysm/issues/13591. --- beacon-chain/slasher/detect_attestations.go | 32 +++++++----- .../slasher/detect_attestations_test.go | 51 +++++++++---------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index a9ae996aa667..a0a51d5da371 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -54,9 +54,7 @@ func (s *Service) checkSlashableAttestations( surroundStart := time.Now() for validatorChunkIndex, attestations := range groupedByValidatorChunkIndexAtts { - // The fact that we use always slashertypes.MinSpan is probably the root cause of - // https://github.com/prysmaticlabs/prysm/issues/13591 - surroundSlashings, err := s.checkSurrounds(ctx, attestations, slashertypes.MinSpan, currentEpoch, validatorChunkIndex) + surroundSlashings, err := s.checkSurrounds(ctx, attestations, currentEpoch, validatorChunkIndex) if err != nil { return nil, err } @@ -107,12 +105,12 @@ func (s *Service) checkSlashableAttestations( func (s *Service) checkSurrounds( ctx context.Context, attestations []*slashertypes.IndexedAttestationWrapper, - chunkKind slashertypes.ChunkKind, currentEpoch primitives.Epoch, validatorChunkIndex uint64, ) (map[[fieldparams.RootLength]byte]*ethpb.AttesterSlashing, error) { // Map of updated chunks by chunk index, which will be saved at the end. - updatedChunks := make(map[uint64]Chunker) + updatedMinChunks, updatedMaxChunks := map[uint64]Chunker{}, map[uint64]Chunker{} + groupedByChunkIndexAtts := s.groupByChunkIndex(attestations) validatorIndexes := s.params.validatorIndexesInChunk(validatorChunkIndex) @@ -120,14 +118,19 @@ func (s *Service) checkSurrounds( // Update epoch for validators. for _, validatorIndex := range validatorIndexes { - // This function modifies `updatedChunks` in place. - if err := s.epochUpdateForValidator(ctx, updatedChunks, validatorChunkIndex, chunkKind, currentEpoch, validatorIndex); err != nil { - return nil, errors.Wrapf(err, "could not update validator index chunks %d", validatorIndex) + // This function modifies `updatedMinChunks` in place. + if err := s.epochUpdateForValidator(ctx, updatedMinChunks, validatorChunkIndex, slashertypes.MinSpan, currentEpoch, validatorIndex); err != nil { + return nil, errors.Wrapf(err, "could not update validator index for min chunks %d", validatorIndex) + } + + // This function modifies `updatedMaxChunks` in place. + if err := s.epochUpdateForValidator(ctx, updatedMaxChunks, validatorChunkIndex, slashertypes.MaxSpan, currentEpoch, validatorIndex); err != nil { + return nil, errors.Wrapf(err, "could not update validator index for max chunks %d", validatorIndex) } } // Check for surrounding votes. - surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, groupedByChunkIndexAtts, slashertypes.MinSpan, validatorChunkIndex, currentEpoch) + surroundingSlashings, err := s.updateSpans(ctx, updatedMinChunks, groupedByChunkIndexAtts, slashertypes.MinSpan, validatorChunkIndex, currentEpoch) if err != nil { return nil, errors.Wrapf(err, "could not update min attestation spans for validator chunk index %d", validatorChunkIndex) } @@ -137,7 +140,7 @@ func (s *Service) checkSurrounds( } // Check for surrounded votes. - surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, groupedByChunkIndexAtts, slashertypes.MaxSpan, validatorChunkIndex, currentEpoch) + surroundedSlashings, err := s.updateSpans(ctx, updatedMaxChunks, groupedByChunkIndexAtts, slashertypes.MaxSpan, validatorChunkIndex, currentEpoch) if err != nil { return nil, errors.Wrapf(err, "could not update max attestation spans for validator chunk index %d", validatorChunkIndex) } @@ -146,8 +149,13 @@ func (s *Service) checkSurrounds( slashings[root] = slashing } - if err := s.saveUpdatedChunks(ctx, updatedChunks, chunkKind, validatorChunkIndex); err != nil { - return nil, err + // Save updated chunks into the database. + if err := s.saveUpdatedChunks(ctx, updatedMinChunks, slashertypes.MinSpan, validatorChunkIndex); err != nil { + return nil, errors.Wrap(err, "could not save chunks for min spans") + } + + if err := s.saveUpdatedChunks(ctx, updatedMaxChunks, slashertypes.MaxSpan, validatorChunkIndex); err != nil { + return nil, errors.Wrap(err, "could not save chunks for max spans") } return slashings, nil diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 76cf7bd67ecb..342a5f4a5999 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -171,7 +171,6 @@ func Test_processAttestations(t *testing.T) { name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000) - single step", steps: []*step{ { - currentEpoch: 1000, attestationsInfo: []*attestationInfo{ {source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, @@ -190,7 +189,6 @@ func Test_processAttestations(t *testing.T) { name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000) - two steps", steps: []*step{ { - currentEpoch: 1000, attestationsInfo: []*attestationInfo{ {source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, @@ -229,31 +227,30 @@ func Test_processAttestations(t *testing.T) { }, }, }, - // Uncomment when https://github.com/prysmaticlabs/prysm/issues/13591 is fixed - // { - // name: "Detects surrounded vote (source 0, target 3), (source 1, target 2) - two steps", - // steps: []*step{ - // { - // currentEpoch: 4, - // attestationsInfo: []*attestationInfo{ - // {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - // }, - // expectedSlashingsInfo: nil, - // }, - // { - // currentEpoch: 4, - // attestationsInfo: []*attestationInfo{ - // {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - // }, - // expectedSlashingsInfo: []*slashingInfo{ - // { - // attestationInfo_1: &attestationInfo{source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - // attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - // }, - // }, - // }, - // }, - // }, + { + name: "Detects surrounded vote (source 0, target 3), (source 1, target 2) - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + }, + }, + }, + }, { name: "Detects double vote, (source 1, target 2), (source 0, target 2) - single step", steps: []*step{