From 7dfbb3c8b1c7775497c9d9221c9844e6126fc6c5 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 12:56:12 +0100 Subject: [PATCH 01/38] `helpers.go`: Improve naming consistency. --- beacon-chain/slasher/helpers.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/beacon-chain/slasher/helpers.go b/beacon-chain/slasher/helpers.go index 4b2f5b4021de..073911a49aee 100644 --- a/beacon-chain/slasher/helpers.go +++ b/beacon-chain/slasher/helpers.go @@ -21,19 +21,23 @@ func (s *Service) groupByValidatorChunkIndex( attestations []*slashertypes.IndexedAttestationWrapper, ) map[uint64][]*slashertypes.IndexedAttestationWrapper { groupedAttestations := make(map[uint64][]*slashertypes.IndexedAttestationWrapper) - for _, att := range attestations { - validatorChunkIndices := make(map[uint64]bool) - for _, validatorIdx := range att.IndexedAttestation.AttestingIndices { - validatorChunkIndex := s.params.validatorChunkIndex(primitives.ValidatorIndex(validatorIdx)) - validatorChunkIndices[validatorChunkIndex] = true + + for _, attestation := range attestations { + validatorChunkIndexes := make(map[uint64]bool) + + for _, validatorIndex := range attestation.IndexedAttestation.AttestingIndices { + validatorChunkIndex := s.params.validatorChunkIndex(primitives.ValidatorIndex(validatorIndex)) + validatorChunkIndexes[validatorChunkIndex] = true } - for validatorChunkIndex := range validatorChunkIndices { + + for validatorChunkIndex := range validatorChunkIndexes { groupedAttestations[validatorChunkIndex] = append( groupedAttestations[validatorChunkIndex], - att, + attestation, ) } } + return groupedAttestations } @@ -42,10 +46,12 @@ func (s *Service) groupByChunkIndex( attestations []*slashertypes.IndexedAttestationWrapper, ) map[uint64][]*slashertypes.IndexedAttestationWrapper { attestationsByChunkIndex := make(map[uint64][]*slashertypes.IndexedAttestationWrapper) - for _, att := range attestations { - chunkIdx := s.params.chunkIndex(att.IndexedAttestation.Data.Source.Epoch) - attestationsByChunkIndex[chunkIdx] = append(attestationsByChunkIndex[chunkIdx], att) + + for _, attestation := range attestations { + chunkIndex := s.params.chunkIndex(attestation.IndexedAttestation.Data.Source.Epoch) + attestationsByChunkIndex[chunkIndex] = append(attestationsByChunkIndex[chunkIndex], attestation) } + return attestationsByChunkIndex } From c0f524cdd601b3cf19f55dd292c8a46ecb1b9977 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 12:58:27 +0100 Subject: [PATCH 02/38] `detect_attestations.go`: Improve readability. --- beacon-chain/slasher/detect_attestations.go | 50 ++++++++++++--------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 0a1b222bfe82..d41a69a387fc 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -98,16 +98,12 @@ func (s *Service) checkSurrounds( // Map of updated chunks by chunk index, which will be saved at the end. updatedChunks := make(map[uint64]Chunker) groupedAtts := s.groupByChunkIndex(attestations) - validatorIndices := s.params.validatorIndicesInChunk(args.validatorChunkIndex) + validatorIndexes := s.params.validatorIndicesInChunk(args.validatorChunkIndex) // Update the min/max span chunks for the change of current epoch. - for _, validatorIndex := range validatorIndices { + for _, validatorIndex := range validatorIndexes { if err := s.epochUpdateForValidator(ctx, args, updatedChunks, validatorIndex); err != nil { - return nil, errors.Wrapf( - err, - "could not update validator index chunks %d", - validatorIndex, - ) + return nil, errors.Wrapf(err, "could not update validator index chunks %d", validatorIndex) } } @@ -185,6 +181,7 @@ func (s *Service) epochUpdateForValidator( if epoch == 0 { return nil } + for epoch <= args.currentEpoch { chunkIdx := s.params.chunkIndex(epoch) currentChunk, err := s.getChunk(ctx, args, updatedChunks, chunkIdx) @@ -370,18 +367,21 @@ func (s *Service) getChunk( chunksByChunkIdx map[uint64]Chunker, chunkIdx uint64, ) (Chunker, error) { - chunk, ok := chunksByChunkIdx[chunkIdx] - if ok { + // If the chunk exists in the map, we return it. + if chunk, ok := chunksByChunkIdx[chunkIdx]; ok { return chunk, nil } + // We can ensure we load the appropriate chunk we need by fetching from the DB. diskChunks, err := s.loadChunks(ctx, args, []uint64{chunkIdx}) if err != nil { return nil, errors.Wrapf(err, "could not load chunk at index %d", chunkIdx) } + if chunk, ok := diskChunks[chunkIdx]; ok { return chunk, nil } + return nil, fmt.Errorf("could not retrieve chunk at chunk index %d from disk", chunkIdx) } @@ -395,41 +395,51 @@ func (s *Service) loadChunks( ) (map[uint64]Chunker, error) { ctx, span := trace.StartSpan(ctx, "Slasher.loadChunks") defer span.End() + chunkKeys := make([][]byte, 0, len(chunkIndices)) for _, chunkIdx := range chunkIndices { chunkKeys = append(chunkKeys, s.params.flatSliceID(args.validatorChunkIndex, chunkIdx)) } + rawChunks, chunksExist, err := s.serviceCfg.Database.LoadSlasherChunks(ctx, args.kind, chunkKeys) if err != nil { - return nil, errors.Wrapf( - err, - "could not load slasher chunk index", - ) + return nil, errors.Wrapf(err, "could not load slasher chunk index") } + chunksByChunkIdx := make(map[uint64]Chunker, len(rawChunks)) for i := 0; i < len(rawChunks); i++ { // If the chunk exists in the database, we initialize it from the raw bytes data. // If it does not exist, we initialize an empty chunk. - var chunk Chunker + var ( + chunk Chunker + err error + ) + + chunkExists := chunksExist[i] + switch args.kind { case slashertypes.MinSpan: - if chunksExist[i] { + if chunkExists { chunk, err = MinChunkSpansSliceFrom(s.params, rawChunks[i]) - } else { - chunk = EmptyMinSpanChunksSlice(s.params) + break } + chunk = EmptyMinSpanChunksSlice(s.params) + case slashertypes.MaxSpan: - if chunksExist[i] { + if chunkExists { chunk, err = MaxChunkSpansSliceFrom(s.params, rawChunks[i]) - } else { - chunk = EmptyMaxSpanChunksSlice(s.params) + break } + chunk = EmptyMaxSpanChunksSlice(s.params) } + if err != nil { return nil, errors.Wrap(err, "could not initialize chunk") } + chunksByChunkIdx[chunkIndices[i]] = chunk } + return chunksByChunkIdx, nil } From cc4f2ecfad8dad36d2b63c120caf0d434cc2ae07 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 13:03:37 +0100 Subject: [PATCH 03/38] `receive.go`: Add `attsQueueSize` in log message. --- beacon-chain/slasher/receive.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beacon-chain/slasher/receive.go b/beacon-chain/slasher/receive.go index eb4daa0beb63..2db40b82ca3a 100644 --- a/beacon-chain/slasher/receive.go +++ b/beacon-chain/slasher/receive.go @@ -103,6 +103,7 @@ func (s *Service) processQueuedAttestations(ctx context.Context, slotTicker <-ch // We add back those attestations that are valid in the future to the queue. s.attsQueue.extend(validInFuture) + attsQueueSize := s.attsQueue.size() log.WithFields(logrus.Fields{ "currentSlot": currentSlot, @@ -110,6 +111,7 @@ func (s *Service) processQueuedAttestations(ctx context.Context, slotTicker <-ch "numValidAtts": len(validAtts), "numDeferredAtts": len(validInFuture), "numDroppedAtts": numDropped, + "attsQueueSize": attsQueueSize, }).Info("Processing queued attestations for slashing detection") // Save the attestation records to our database. From 1d88c767a56b7ba0964fb1f7b6be3411e1b49716 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 13:14:52 +0100 Subject: [PATCH 04/38] `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. --- beacon-chain/slasher/detect_attestations.go | 34 ++++++++++++--------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index d41a69a387fc..562a645a0797 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -27,14 +27,17 @@ func (s *Service) checkSlashableAttestations( if err != nil { return nil, errors.Wrap(err, "could not check slashable double votes") } + log.WithField("elapsed", time.Since(start)).Debug("Done checking double votes") + slashings = append(slashings, doubleVoteSlashings...) // Surrounding / surrounded votes groupedAtts := s.groupByValidatorChunkIndex(atts) log.WithField("numBatches", len(groupedAtts)).Debug("Batching attestations by validator chunk index") - start = time.Now() - batchTimes := make([]time.Duration, 0, len(groupedAtts)) + grouppedAttsCount := len(groupedAtts) + + batchDurations := make([]time.Duration, 0, len(groupedAtts)) for validatorChunkIdx, batch := range groupedAtts { innerStart := time.Now() @@ -54,24 +57,27 @@ func (s *Service) checkSlashableAttestations( s.latestEpochWrittenForValidator[idx] = currentEpoch } - batchTimes = append(batchTimes, time.Since(innerStart)) + batchDurations = append(batchDurations, time.Since(innerStart)) } - avgProcessingTimePerBatch := time.Duration(0) - for _, dur := range batchTimes { - avgProcessingTimePerBatch += dur + // Elapsed time computation + totalBatchDuration := time.Duration(0) + for _, batchDuration := range batchDurations { + totalBatchDuration += batchDuration } - if avgProcessingTimePerBatch != time.Duration(0) { - avgProcessingTimePerBatch = avgProcessingTimePerBatch / time.Duration(len(batchTimes)) + fields := logrus.Fields{ + "numAttestations": len(atts), + "numBatchesByValidatorChunkIndex": grouppedAttsCount, + "elapsed": totalBatchDuration, } - log.WithFields(logrus.Fields{ - "numAttestations": len(atts), - "numBatchesByValidatorChunkIndex": len(groupedAtts), - "elapsed": time.Since(start), - "avgBatchProcessingTime": avgProcessingTimePerBatch, - }).Info("Done checking slashable attestations") + if grouppedAttsCount > 0 { + avgProcessingTimePerBatch := totalBatchDuration / time.Duration(grouppedAttsCount) + fields["avgBatchProcessingTime"] = avgProcessingTimePerBatch + } + + log.WithFields(fields).Info("Done checking slashable attestations") if len(slashings) > 0 { log.WithField("numSlashings", len(slashings)).Warn("Slashable attestation offenses found") From c87bb066d02969293816e2478447dd646406248a Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 13:33:18 +0100 Subject: [PATCH 05/38] `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. --- beacon-chain/slasher/detect_attestations.go | 14 +++++++++----- beacon-chain/slasher/detect_attestations_test.go | 10 ++-------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 562a645a0797..762e20f0e9b0 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -190,10 +190,12 @@ func (s *Service) epochUpdateForValidator( for epoch <= args.currentEpoch { chunkIdx := s.params.chunkIndex(epoch) + currentChunk, err := s.getChunk(ctx, args, updatedChunks, chunkIdx) if err != nil { return err } + for s.params.chunkIndex(epoch) == chunkIdx && epoch <= args.currentEpoch { if err := setChunkRawDistance( s.params, @@ -208,6 +210,7 @@ func (s *Service) epochUpdateForValidator( epoch++ } } + return nil } @@ -379,7 +382,7 @@ func (s *Service) getChunk( } // We can ensure we load the appropriate chunk we need by fetching from the DB. - diskChunks, err := s.loadChunks(ctx, args, []uint64{chunkIdx}) + diskChunks, err := s.loadChunks(ctx, args.validatorChunkIndex, args.kind, []uint64{chunkIdx}) if err != nil { return nil, errors.Wrapf(err, "could not load chunk at index %d", chunkIdx) } @@ -396,7 +399,8 @@ func (s *Service) getChunk( // an empty chunk, add it to our map, and then return it to the caller. func (s *Service) loadChunks( ctx context.Context, - args *chunkUpdateArgs, + validatorChunkIndex uint64, + chunkKind slashertypes.ChunkKind, chunkIndices []uint64, ) (map[uint64]Chunker, error) { ctx, span := trace.StartSpan(ctx, "Slasher.loadChunks") @@ -404,10 +408,10 @@ func (s *Service) loadChunks( chunkKeys := make([][]byte, 0, len(chunkIndices)) for _, chunkIdx := range chunkIndices { - chunkKeys = append(chunkKeys, s.params.flatSliceID(args.validatorChunkIndex, chunkIdx)) + chunkKeys = append(chunkKeys, s.params.flatSliceID(validatorChunkIndex, chunkIdx)) } - rawChunks, chunksExist, err := s.serviceCfg.Database.LoadSlasherChunks(ctx, args.kind, chunkKeys) + rawChunks, chunksExist, err := s.serviceCfg.Database.LoadSlasherChunks(ctx, chunkKind, chunkKeys) if err != nil { return nil, errors.Wrapf(err, "could not load slasher chunk index") } @@ -423,7 +427,7 @@ func (s *Service) loadChunks( chunkExists := chunksExist[i] - switch args.kind { + switch chunkKind { case slashertypes.MinSpan: if chunkExists { chunk, err = MinChunkSpansSliceFrom(s.params, rawChunks[i]) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index c09b435cf90f..0b7711d391eb 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -695,10 +695,7 @@ func testLoadChunks(t *testing.T, kind slashertypes.ChunkKind) { emptyChunk = EmptyMaxSpanChunksSlice(defaultParams) } chunkIdx := uint64(2) - received, err := s.loadChunks(ctx, &chunkUpdateArgs{ - validatorChunkIndex: 0, - kind: kind, - }, []uint64{chunkIdx}) + received, err := s.loadChunks(ctx, 0, kind, []uint64{chunkIdx}) require.NoError(t, err) wanted := map[uint64]Chunker{ chunkIdx: emptyChunk, @@ -740,10 +737,7 @@ func testLoadChunks(t *testing.T, kind slashertypes.ChunkKind) { ) require.NoError(t, err) // Check if the retrieved chunks match what we just saved to disk. - received, err = s.loadChunks(ctx, &chunkUpdateArgs{ - validatorChunkIndex: 0, - kind: kind, - }, []uint64{2, 4, 6}) + received, err = s.loadChunks(ctx, 0, kind, []uint64{2, 4, 6}) require.NoError(t, err) require.DeepEqual(t, updatedChunks, received) } From aa2600b4bb85fa04a497e6f789d7df34424879a8 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 13:35:24 +0100 Subject: [PATCH 06/38] `getChunk`: Use specific `chunkIndex` and `chunkKind`. --- beacon-chain/slasher/detect_attestations.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 762e20f0e9b0..ebf8737c7b4e 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -191,7 +191,7 @@ func (s *Service) epochUpdateForValidator( for epoch <= args.currentEpoch { chunkIdx := s.params.chunkIndex(epoch) - currentChunk, err := s.getChunk(ctx, args, updatedChunks, chunkIdx) + currentChunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, updatedChunks, chunkIdx) if err != nil { return err } @@ -296,7 +296,7 @@ func (s *Service) applyAttestationForValidator( attestationDistance.Observe(float64(targetEpoch) - float64(sourceEpoch)) chunkIdx := s.params.chunkIndex(sourceEpoch) - chunk, err := s.getChunk(ctx, args, chunksByChunkIdx, chunkIdx) + chunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, chunksByChunkIdx, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } @@ -334,7 +334,7 @@ func (s *Service) applyAttestationForValidator( // keep updating chunks. for { chunkIdx = s.params.chunkIndex(startEpoch) - chunk, err := s.getChunk(ctx, args, chunksByChunkIdx, chunkIdx) + chunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, chunksByChunkIdx, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } @@ -372,7 +372,8 @@ func (s *Service) applyAttestationForValidator( // that span multiple chunk indices), then we fallback to fetching from disk. func (s *Service) getChunk( ctx context.Context, - args *chunkUpdateArgs, + validatorChunkIndex uint64, + chunkKind slashertypes.ChunkKind, chunksByChunkIdx map[uint64]Chunker, chunkIdx uint64, ) (Chunker, error) { @@ -382,7 +383,7 @@ func (s *Service) getChunk( } // We can ensure we load the appropriate chunk we need by fetching from the DB. - diskChunks, err := s.loadChunks(ctx, args.validatorChunkIndex, args.kind, []uint64{chunkIdx}) + diskChunks, err := s.loadChunks(ctx, validatorChunkIndex, chunkKind, []uint64{chunkIdx}) if err != nil { return nil, errors.Wrapf(err, "could not load chunk at index %d", chunkIdx) } From a44670897aed621c68b38beee4ab87a83ea12d76 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 13:45:25 +0100 Subject: [PATCH 07/38] `validatorIndicesInChunk` -> `validatorIndexesInChunk`. --- beacon-chain/slasher/detect_attestations.go | 4 ++-- beacon-chain/slasher/params.go | 8 +++++--- beacon-chain/slasher/params_test.go | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index ebf8737c7b4e..264251426776 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -52,7 +52,7 @@ func (s *Service) checkSlashableAttestations( slashings = append(slashings, attSlashings...) - indices := s.params.validatorIndicesInChunk(validatorChunkIdx) + indices := s.params.validatorIndexesInChunk(validatorChunkIdx) for _, idx := range indices { s.latestEpochWrittenForValidator[idx] = currentEpoch } @@ -104,7 +104,7 @@ func (s *Service) checkSurrounds( // Map of updated chunks by chunk index, which will be saved at the end. updatedChunks := make(map[uint64]Chunker) groupedAtts := s.groupByChunkIndex(attestations) - validatorIndexes := s.params.validatorIndicesInChunk(args.validatorChunkIndex) + validatorIndexes := s.params.validatorIndexesInChunk(args.validatorChunkIndex) // Update the min/max span chunks for the change of current epoch. for _, validatorIndex := range validatorIndexes { diff --git a/beacon-chain/slasher/params.go b/beacon-chain/slasher/params.go index ff5be1b40edb..ccd67df441b8 100644 --- a/beacon-chain/slasher/params.go +++ b/beacon-chain/slasher/params.go @@ -141,12 +141,14 @@ func (p *Parameters) flatSliceID(validatorChunkIndex, chunkIndex uint64) []byte // Given a validator chunk index, we determine all of the validator // indices that will belong in that chunk. -func (p *Parameters) validatorIndicesInChunk(validatorChunkIdx uint64) []primitives.ValidatorIndex { +func (p *Parameters) validatorIndexesInChunk(validatorChunkIndex uint64) []primitives.ValidatorIndex { validatorIndices := make([]primitives.ValidatorIndex, 0) - low := validatorChunkIdx * p.validatorChunkSize - high := (validatorChunkIdx + 1) * p.validatorChunkSize + low := validatorChunkIndex * p.validatorChunkSize + high := (validatorChunkIndex + 1) * p.validatorChunkSize + for i := low; i < high; i++ { validatorIndices = append(validatorIndices, primitives.ValidatorIndex(i)) } + return validatorIndices } diff --git a/beacon-chain/slasher/params_test.go b/beacon-chain/slasher/params_test.go index 56232c2652a2..7d8bcd23ee09 100644 --- a/beacon-chain/slasher/params_test.go +++ b/beacon-chain/slasher/params_test.go @@ -468,7 +468,7 @@ func TestParams_validatorIndicesInChunk(t *testing.T) { c := &Parameters{ validatorChunkSize: tt.fields.validatorChunkSize, } - if got := c.validatorIndicesInChunk(tt.validatorChunkIdx); !reflect.DeepEqual(got, tt.want) { + if got := c.validatorIndexesInChunk(tt.validatorChunkIdx); !reflect.DeepEqual(got, tt.want) { t.Errorf("validatorIndicesInChunk() = %v, want %v", got, tt.want) } }) From 3c31ad7dab4940a0dcc5ec06348f75597470823e Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 14:33:35 +0100 Subject: [PATCH 08/38] `epochUpdateForValidator`: Use explicit arguments. --- beacon-chain/slasher/detect_attestations.go | 16 +++++++++------- beacon-chain/slasher/detect_attestations_test.go | 12 ++++++------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 264251426776..c6c1bf583fe5 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -108,7 +108,7 @@ func (s *Service) checkSurrounds( // Update the min/max span chunks for the change of current epoch. for _, validatorIndex := range validatorIndexes { - if err := s.epochUpdateForValidator(ctx, args, updatedChunks, validatorIndex); err != nil { + if err := s.epochUpdateForValidator(ctx, args.validatorChunkIndex, args.kind, args.currentEpoch, updatedChunks, validatorIndex); err != nil { return nil, errors.Wrapf(err, "could not update validator index chunks %d", validatorIndex) } } @@ -179,7 +179,9 @@ func (s *Service) checkDoubleVotes( // map used as a cache for further processing and minimizing database reads later on. func (s *Service) epochUpdateForValidator( ctx context.Context, - args *chunkUpdateArgs, + validatorChunkIndex uint64, + chunkKind slashertypes.ChunkKind, + currentEpoch primitives.Epoch, updatedChunks map[uint64]Chunker, validatorIndex primitives.ValidatorIndex, ) error { @@ -188,15 +190,15 @@ func (s *Service) epochUpdateForValidator( return nil } - for epoch <= args.currentEpoch { - chunkIdx := s.params.chunkIndex(epoch) + for epoch <= currentEpoch { + chunkIndex := s.params.chunkIndex(epoch) - currentChunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, updatedChunks, chunkIdx) + currentChunk, err := s.getChunk(ctx, validatorChunkIndex, chunkKind, updatedChunks, chunkIndex) if err != nil { return err } - for s.params.chunkIndex(epoch) == chunkIdx && epoch <= args.currentEpoch { + for s.params.chunkIndex(epoch) == chunkIndex && epoch <= currentEpoch { if err := setChunkRawDistance( s.params, currentChunk.Chunk(), @@ -206,7 +208,7 @@ func (s *Service) epochUpdateForValidator( ); err != nil { return err } - updatedChunks[chunkIdx] = currentChunk + updatedChunks[chunkIndex] = currentChunk epoch++ } } diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 0b7711d391eb..fff361848b8e 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -453,9 +453,9 @@ func Test_epochUpdateForValidators(t *testing.T) { for _, valIdx := range validators { err := s.epochUpdateForValidator( ctx, - &chunkUpdateArgs{ - currentEpoch: currentEpoch, - }, + 0, // validatorChunkIndex + slashertypes.MinSpan, + currentEpoch, updatedChunks, valIdx, ) @@ -484,9 +484,9 @@ func Test_epochUpdateForValidators(t *testing.T) { for _, valIdx := range validators { err := s.epochUpdateForValidator( ctx, - &chunkUpdateArgs{ - currentEpoch: currentEpoch, - }, + 0, // validatorChunkIndex, + slashertypes.MinSpan, + currentEpoch, updatedChunks, valIdx, ) From 5e0b85179c5ce369e6c851dd99a71b9630760c7c Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 16:03:51 +0100 Subject: [PATCH 09/38] `getChunk`: Change order of arguments. --- beacon-chain/slasher/detect_attestations.go | 43 +++++++++++---------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index c6c1bf583fe5..211c6b7dc8aa 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -185,31 +185,31 @@ func (s *Service) epochUpdateForValidator( updatedChunks map[uint64]Chunker, validatorIndex primitives.ValidatorIndex, ) error { - epoch := s.latestEpochWrittenForValidator[validatorIndex] - if epoch == 0 { + latestEpochWritten := s.latestEpochWrittenForValidator[validatorIndex] + if latestEpochWritten == 0 { return nil } - for epoch <= currentEpoch { - chunkIndex := s.params.chunkIndex(epoch) + for latestEpochWritten <= currentEpoch { + chunkIndex := s.params.chunkIndex(latestEpochWritten) - currentChunk, err := s.getChunk(ctx, validatorChunkIndex, chunkKind, updatedChunks, chunkIndex) + currentChunk, err := s.getChunk(ctx, updatedChunks, chunkKind, validatorChunkIndex, chunkIndex) if err != nil { return err } - for s.params.chunkIndex(epoch) == chunkIndex && epoch <= currentEpoch { + for s.params.chunkIndex(latestEpochWritten) == chunkIndex && latestEpochWritten <= currentEpoch { if err := setChunkRawDistance( s.params, currentChunk.Chunk(), validatorIndex, - epoch, + latestEpochWritten, currentChunk.NeutralElement(), ); err != nil { return err } updatedChunks[chunkIndex] = currentChunk - epoch++ + latestEpochWritten++ } } @@ -298,7 +298,7 @@ func (s *Service) applyAttestationForValidator( attestationDistance.Observe(float64(targetEpoch) - float64(sourceEpoch)) chunkIdx := s.params.chunkIndex(sourceEpoch) - chunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, chunksByChunkIdx, chunkIdx) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, args.kind, args.validatorChunkIndex, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } @@ -336,7 +336,7 @@ func (s *Service) applyAttestationForValidator( // keep updating chunks. for { chunkIdx = s.params.chunkIndex(startEpoch) - chunk, err := s.getChunk(ctx, args.validatorChunkIndex, args.kind, chunksByChunkIdx, chunkIdx) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, args.kind, args.validatorChunkIndex, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } @@ -369,32 +369,33 @@ func (s *Service) applyAttestationForValidator( return nil, nil } -// Retrieves a chunk at a chunk index from a map. If such chunk does not exist, which -// should be rare (occurring when we receive an attestation with source and target epochs -// that span multiple chunk indices), then we fallback to fetching from disk. +// Retrieves a chunk from `chunksByChunkIndex` using a chunk index. +// If such chunk does not exist, which should be rare +// (occurring when we receive an attestation with source and target epochs +// that span multiple chunk indices), then fallbacks to fetching from database. func (s *Service) getChunk( ctx context.Context, - validatorChunkIndex uint64, + chunksByChunkIndex map[uint64]Chunker, chunkKind slashertypes.ChunkKind, - chunksByChunkIdx map[uint64]Chunker, - chunkIdx uint64, + validatorChunkIndex uint64, + chunkIndex uint64, ) (Chunker, error) { // If the chunk exists in the map, we return it. - if chunk, ok := chunksByChunkIdx[chunkIdx]; ok { + if chunk, ok := chunksByChunkIndex[chunkIndex]; ok { return chunk, nil } // We can ensure we load the appropriate chunk we need by fetching from the DB. - diskChunks, err := s.loadChunks(ctx, validatorChunkIndex, chunkKind, []uint64{chunkIdx}) + diskChunks, err := s.loadChunks(ctx, validatorChunkIndex, chunkKind, []uint64{chunkIndex}) if err != nil { - return nil, errors.Wrapf(err, "could not load chunk at index %d", chunkIdx) + return nil, errors.Wrapf(err, "could not load chunk at index %d", chunkIndex) } - if chunk, ok := diskChunks[chunkIdx]; ok { + if chunk, ok := diskChunks[chunkIndex]; ok { return chunk, nil } - return nil, fmt.Errorf("could not retrieve chunk at chunk index %d from disk", chunkIdx) + return nil, fmt.Errorf("could not retrieve chunk at chunk index %d from disk", chunkIndex) } // Load chunks for a specified list of chunk indices. We attempt to load it from the database. From 5d46108aeea2b9b6f6c8607e8c3a8bccd66d8608 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 16:07:40 +0100 Subject: [PATCH 10/38] `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. --- beacon-chain/slasher/detect_attestations.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 211c6b7dc8aa..f46d9b89cf7c 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -185,8 +185,8 @@ func (s *Service) epochUpdateForValidator( updatedChunks map[uint64]Chunker, validatorIndex primitives.ValidatorIndex, ) error { - latestEpochWritten := s.latestEpochWrittenForValidator[validatorIndex] - if latestEpochWritten == 0 { + latestEpochWritten, ok := s.latestEpochWrittenForValidator[validatorIndex] + if !ok { return nil } From 24e1be7fde153c6a59db783da9fe927757abffeb Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 17:08:15 +0100 Subject: [PATCH 11/38] `applyAttestationForValidator`: Use explicit arguments. --- beacon-chain/slasher/detect_attestations.go | 26 ++++++++------- .../slasher/detect_attestations_test.go | 32 ++++++++++++------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index f46d9b89cf7c..eb6ca81f61ad 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -256,10 +256,12 @@ func (s *Service) updateSpans( } slashing, err := s.applyAttestationForValidator( ctx, - args, - validatorIndex, - updatedChunks, att, + updatedChunks, + args.kind, + args.validatorChunkIndex, + validatorIndex, + args.currentEpoch, ) if err != nil { return nil, errors.Wrapf( @@ -285,10 +287,12 @@ func (s *Service) updateSpans( // source epoch up to its target. func (s *Service) applyAttestationForValidator( ctx context.Context, - args *chunkUpdateArgs, - validatorIndex primitives.ValidatorIndex, - chunksByChunkIdx map[uint64]Chunker, attestation *slashertypes.IndexedAttestationWrapper, + chunksByChunkIdx map[uint64]Chunker, + chunkKind slashertypes.ChunkKind, + validatorChunkIndex uint64, + validatorIndex primitives.ValidatorIndex, + currentEpoch primitives.Epoch, ) (*ethpb.AttesterSlashing, error) { ctx, span := trace.StartSpan(ctx, "Slasher.applyAttestationForValidator") defer span.End() @@ -298,7 +302,7 @@ func (s *Service) applyAttestationForValidator( attestationDistance.Observe(float64(targetEpoch) - float64(sourceEpoch)) chunkIdx := s.params.chunkIndex(sourceEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, args.kind, args.validatorChunkIndex, chunkIdx) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } @@ -323,7 +327,7 @@ func (s *Service) applyAttestationForValidator( // Get the first start epoch for the chunk. If it does not exist or // is not possible based on the input arguments, do not continue with the update. - startEpoch, exists := chunk.StartEpoch(sourceEpoch, args.currentEpoch) + startEpoch, exists := chunk.StartEpoch(sourceEpoch, currentEpoch) if !exists { return nil, nil } @@ -336,14 +340,14 @@ func (s *Service) applyAttestationForValidator( // keep updating chunks. for { chunkIdx = s.params.chunkIndex(startEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, args.kind, args.validatorChunkIndex, chunkIdx) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIdx) if err != nil { return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) } keepGoing, err := chunk.Update( &chunkUpdateArgs{ chunkIndex: chunkIdx, - currentEpoch: args.currentEpoch, + currentEpoch: currentEpoch, }, validatorIndex, startEpoch, @@ -355,7 +359,7 @@ func (s *Service) applyAttestationForValidator( "could not update chunk at chunk index %d for validator index %d and current epoch %d", chunkIdx, validatorIndex, - args.currentEpoch, + currentEpoch, ) } // We update the chunksByChunkIdx map with the chunk we just updated. diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index fff361848b8e..3652cabc4fe6 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -529,10 +529,12 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { att := createAttestationWrapper(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, - args, - validatorIdx, - chunksByChunkIdx, att, + chunksByChunkIdx, + args.kind, + args.validatorChunkIndex, + validatorIdx, + args.currentEpoch, ) require.NoError(t, err) require.Equal(t, true, slashing == nil) @@ -550,10 +552,12 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { slashableAtt := createAttestationWrapper(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, - args, - validatorIdx, - chunksByChunkIdx, slashableAtt, + chunksByChunkIdx, + args.kind, + args.validatorChunkIndex, + validatorIdx, + args.currentEpoch, ) require.NoError(t, err) require.NotNil(t, slashing) @@ -590,10 +594,12 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { att := createAttestationWrapper(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, - args, - validatorIdx, - chunksByChunkIdx, att, + chunksByChunkIdx, + args.kind, + args.validatorChunkIndex, + validatorIdx, + args.currentEpoch, ) require.NoError(t, err) require.Equal(t, true, slashing == nil) @@ -611,10 +617,12 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { slashableAtt := createAttestationWrapper(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, - args, - validatorIdx, - chunksByChunkIdx, slashableAtt, + chunksByChunkIdx, + args.kind, + args.validatorChunkIndex, + validatorIdx, + args.currentEpoch, ) require.NoError(t, err) require.NotNil(t, slashing) From 06853c48785a1c91cf8b1a43ab42ee6f4fa24c20 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 17:18:32 +0100 Subject: [PATCH 12/38] `updateSpans`: Use explicit arguments. --- beacon-chain/slasher/detect_attestations.go | 36 +++++++-------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index eb6ca81f61ad..e249afed5600 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -114,30 +114,14 @@ func (s *Service) checkSurrounds( } // Update min and max spans and retrieve any detected slashable offenses. - surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, &chunkUpdateArgs{ - kind: slashertypes.MinSpan, - validatorChunkIndex: args.validatorChunkIndex, - currentEpoch: args.currentEpoch, - }, groupedAtts) + surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MinSpan, args.validatorChunkIndex, args.currentEpoch) if err != nil { - return nil, errors.Wrapf( - err, - "could not update min attestation spans for validator chunk index %d", - args.validatorChunkIndex, - ) + return nil, errors.Wrapf(err, "could not update min attestation spans for validator chunk index %d", args.validatorChunkIndex) } - surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, &chunkUpdateArgs{ - kind: slashertypes.MaxSpan, - validatorChunkIndex: args.validatorChunkIndex, - currentEpoch: args.currentEpoch, - }, groupedAtts) + surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MaxSpan, args.validatorChunkIndex, args.currentEpoch) if err != nil { - return nil, errors.Wrapf( - err, - "could not update max attestation spans for validator chunk index %d", - args.validatorChunkIndex, - ) + return nil, errors.Wrapf(err, "could not update max attestation spans for validator chunk index %d", args.validatorChunkIndex) } slashings := make([]*ethpb.AttesterSlashing, 0, len(surroundingSlashings)+len(surroundedSlashings)) @@ -227,8 +211,10 @@ func (s *Service) epochUpdateForValidator( func (s *Service) updateSpans( ctx context.Context, updatedChunks map[uint64]Chunker, - args *chunkUpdateArgs, attestationsByChunkIdx map[uint64][]*slashertypes.IndexedAttestationWrapper, + kind slashertypes.ChunkKind, + validatorChunkIndex uint64, + currentEpoch primitives.Epoch, ) ([]*ethpb.AttesterSlashing, error) { ctx, span := trace.StartSpan(ctx, "Slasher.updateSpans") defer span.End() @@ -251,17 +237,17 @@ func (s *Service) updateSpans( // If we see an attestation with attesting indices [3, 4, 5] and we are updating // chunks for validator chunk index 0, only validator index 3 should make // it past this line. - if args.validatorChunkIndex != computedValidatorChunkIdx { + if validatorChunkIndex != computedValidatorChunkIdx { continue } slashing, err := s.applyAttestationForValidator( ctx, att, updatedChunks, - args.kind, - args.validatorChunkIndex, + kind, + validatorChunkIndex, validatorIndex, - args.currentEpoch, + currentEpoch, ) if err != nil { return nil, errors.Wrapf( From fedb1ef57563b8f09d0c92ba254c89c80d9a3908 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 17:24:59 +0100 Subject: [PATCH 13/38] `saveUpdatedChunks`: Use explicit arguments. --- beacon-chain/slasher/detect_attestations.go | 9 +++++---- beacon-chain/slasher/detect_attestations_test.go | 6 ++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index e249afed5600..23556fa3c856 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -127,7 +127,7 @@ func (s *Service) checkSurrounds( slashings := make([]*ethpb.AttesterSlashing, 0, len(surroundingSlashings)+len(surroundedSlashings)) slashings = append(slashings, surroundingSlashings...) slashings = append(slashings, surroundedSlashings...) - if err := s.saveUpdatedChunks(ctx, args, updatedChunks); err != nil { + if err := s.saveUpdatedChunks(ctx, updatedChunks, args.kind, args.validatorChunkIndex); err != nil { return nil, err } return slashings, nil @@ -450,17 +450,18 @@ func (s *Service) loadChunks( // Saves updated chunks to disk given the required database schema. func (s *Service) saveUpdatedChunks( ctx context.Context, - args *chunkUpdateArgs, updatedChunksByChunkIdx map[uint64]Chunker, + chunkKind slashertypes.ChunkKind, + validatorChunkIndex uint64, ) error { ctx, span := trace.StartSpan(ctx, "Slasher.saveUpdatedChunks") defer span.End() chunkKeys := make([][]byte, 0, len(updatedChunksByChunkIdx)) chunks := make([][]uint16, 0, len(updatedChunksByChunkIdx)) for chunkIdx, chunk := range updatedChunksByChunkIdx { - chunkKeys = append(chunkKeys, s.params.flatSliceID(args.validatorChunkIndex, chunkIdx)) + chunkKeys = append(chunkKeys, s.params.flatSliceID(validatorChunkIndex, chunkIdx)) chunks = append(chunks, chunk.Chunk()) } chunksSavedTotal.Add(float64(len(chunks))) - return s.serviceCfg.Database.SaveSlasherChunks(ctx, args.kind, chunkKeys, chunks) + return s.serviceCfg.Database.SaveSlasherChunks(ctx, chunkKind, chunkKeys, chunks) } diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 3652cabc4fe6..de79260eae3f 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -737,11 +737,9 @@ func testLoadChunks(t *testing.T, kind slashertypes.ChunkKind) { } err = s.saveUpdatedChunks( ctx, - &chunkUpdateArgs{ - validatorChunkIndex: 0, - kind: kind, - }, updatedChunks, + kind, + 0, // validatorChunkIndex ) require.NoError(t, err) // Check if the retrieved chunks match what we just saved to disk. From 75d36f119c2d70de964012e9ef46eef2d0830dfa Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 17:33:34 +0100 Subject: [PATCH 14/38] `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. --- beacon-chain/slasher/detect_attestations.go | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 23556fa3c856..62c0a964009d 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -42,10 +42,9 @@ func (s *Service) checkSlashableAttestations( for validatorChunkIdx, batch := range groupedAtts { innerStart := time.Now() - attSlashings, err := s.checkSurrounds(ctx, &chunkUpdateArgs{ - validatorChunkIndex: validatorChunkIdx, - currentEpoch: currentEpoch, - }, batch) + // The fact that we use always slashertypes.MinSpan is probably the root cause of + // https://github.com/prysmaticlabs/prysm/issues/13591 + attSlashings, err := s.checkSurrounds(ctx, batch, slashertypes.MinSpan, currentEpoch, validatorChunkIdx) if err != nil { return nil, err } @@ -98,36 +97,38 @@ func (s *Service) checkSlashableAttestations( // This function performs a lot of critical actions and is split into smaller helpers for cleanliness. func (s *Service) checkSurrounds( ctx context.Context, - args *chunkUpdateArgs, attestations []*slashertypes.IndexedAttestationWrapper, + chunkKind slashertypes.ChunkKind, + currentEpoch primitives.Epoch, + validatorChunkIndex uint64, ) ([]*ethpb.AttesterSlashing, error) { // Map of updated chunks by chunk index, which will be saved at the end. updatedChunks := make(map[uint64]Chunker) groupedAtts := s.groupByChunkIndex(attestations) - validatorIndexes := s.params.validatorIndexesInChunk(args.validatorChunkIndex) + validatorIndexes := s.params.validatorIndexesInChunk(validatorChunkIndex) // Update the min/max span chunks for the change of current epoch. for _, validatorIndex := range validatorIndexes { - if err := s.epochUpdateForValidator(ctx, args.validatorChunkIndex, args.kind, args.currentEpoch, updatedChunks, validatorIndex); err != nil { + if err := s.epochUpdateForValidator(ctx, validatorChunkIndex, chunkKind, currentEpoch, updatedChunks, validatorIndex); err != nil { return nil, errors.Wrapf(err, "could not update validator index chunks %d", validatorIndex) } } // Update min and max spans and retrieve any detected slashable offenses. - surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MinSpan, args.validatorChunkIndex, args.currentEpoch) + surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MinSpan, validatorChunkIndex, currentEpoch) if err != nil { - return nil, errors.Wrapf(err, "could not update min attestation spans for validator chunk index %d", args.validatorChunkIndex) + return nil, errors.Wrapf(err, "could not update min attestation spans for validator chunk index %d", validatorChunkIndex) } - surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MaxSpan, args.validatorChunkIndex, args.currentEpoch) + surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MaxSpan, validatorChunkIndex, currentEpoch) if err != nil { - return nil, errors.Wrapf(err, "could not update max attestation spans for validator chunk index %d", args.validatorChunkIndex) + return nil, errors.Wrapf(err, "could not update max attestation spans for validator chunk index %d", validatorChunkIndex) } slashings := make([]*ethpb.AttesterSlashing, 0, len(surroundingSlashings)+len(surroundedSlashings)) slashings = append(slashings, surroundingSlashings...) slashings = append(slashings, surroundedSlashings...) - if err := s.saveUpdatedChunks(ctx, updatedChunks, args.kind, args.validatorChunkIndex); err != nil { + if err := s.saveUpdatedChunks(ctx, updatedChunks, chunkKind, validatorChunkIndex); err != nil { return nil, err } return slashings, nil From 3377420f407a66a9cdeaf881ce1098baa1a5cc6b Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 18:23:35 +0100 Subject: [PATCH 15/38] `epochUpdateForValidator`: Set modified by the function argument first. --- beacon-chain/slasher/detect_attestations.go | 19 +++++++++++-------- .../slasher/detect_attestations_test.go | 4 ++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 62c0a964009d..41db4e866a0a 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -109,7 +109,8 @@ func (s *Service) checkSurrounds( // Update the min/max span chunks for the change of current epoch. for _, validatorIndex := range validatorIndexes { - if err := s.epochUpdateForValidator(ctx, validatorChunkIndex, chunkKind, currentEpoch, updatedChunks, validatorIndex); err != nil { + // 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) } } @@ -157,17 +158,18 @@ func (s *Service) checkDoubleVotes( return doubleVoteSlashings, nil } -// This function updates the slashing spans for a given validator for a change in epoch -// since the last epoch we have recorded for the validator. For example, if the last epoch a validator -// has written is N, and the current epoch is N+5, we update entries in the slashing spans -// with their neutral element for epochs N+1 to N+4. This also puts any loaded chunks in a -// map used as a cache for further processing and minimizing database reads later on. +// This function updates `updatedChunks`, representing the slashing spans for a given validator for +// a change in epoch since the last epoch we have recorded for the validator. +// For example, if the last epoch a validator has written is N, and the current epoch is N+5, +// we update entries in the slashing spans with their neutral element for epochs N+1 to N+4. +// This also puts any loaded chunks in a map used as a cache for further processing and minimizing +// database reads later on. func (s *Service) epochUpdateForValidator( ctx context.Context, + updatedChunks map[uint64]Chunker, validatorChunkIndex uint64, chunkKind slashertypes.ChunkKind, currentEpoch primitives.Epoch, - updatedChunks map[uint64]Chunker, validatorIndex primitives.ValidatorIndex, ) error { latestEpochWritten, ok := s.latestEpochWrittenForValidator[validatorIndex] @@ -193,6 +195,7 @@ func (s *Service) epochUpdateForValidator( ); err != nil { return err } + updatedChunks[chunkIndex] = currentChunk latestEpochWritten++ } @@ -230,7 +233,7 @@ func (s *Service) updateSpans( computedValidatorChunkIdx := s.params.validatorChunkIndex(validatorIndex) // Every validator chunk index represents a range of validators. - // If it possible that the validator index in this loop iteration is + // It is possible that the validator index in this loop iteration is // not part of the validator chunk index we are updating chunks for. // // For example, if there are 4 validators per validator chunk index, diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index de79260eae3f..5f470d34e48f 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -453,10 +453,10 @@ func Test_epochUpdateForValidators(t *testing.T) { for _, valIdx := range validators { err := s.epochUpdateForValidator( ctx, + updatedChunks, 0, // validatorChunkIndex slashertypes.MinSpan, currentEpoch, - updatedChunks, valIdx, ) require.NoError(t, err) @@ -484,10 +484,10 @@ func Test_epochUpdateForValidators(t *testing.T) { for _, valIdx := range validators { err := s.epochUpdateForValidator( ctx, + updatedChunks, 0, // validatorChunkIndex, slashertypes.MinSpan, currentEpoch, - updatedChunks, valIdx, ) require.NoError(t, err) From bbbe4daebce681354acf7ba3b4d389a7665cf851 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 21:31:58 +0100 Subject: [PATCH 16/38] `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. --- beacon-chain/slasher/detect_attestations.go | 31 +++++++++---------- .../slasher/detect_attestations_test.go | 8 ++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 41db4e866a0a..cafc815ad78b 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -33,25 +33,24 @@ func (s *Service) checkSlashableAttestations( slashings = append(slashings, doubleVoteSlashings...) // Surrounding / surrounded votes - groupedAtts := s.groupByValidatorChunkIndex(atts) - log.WithField("numBatches", len(groupedAtts)).Debug("Batching attestations by validator chunk index") - grouppedAttsCount := len(groupedAtts) + groupedByValidatorChunkIndexAtts := s.groupByValidatorChunkIndex(atts) + log.WithField("numBatches", len(groupedByValidatorChunkIndexAtts)).Debug("Batching attestations by validator chunk index") + groupsCount := len(groupedByValidatorChunkIndexAtts) + batchDurations := make([]time.Duration, 0, len(groupedByValidatorChunkIndexAtts)) - batchDurations := make([]time.Duration, 0, len(groupedAtts)) - - for validatorChunkIdx, batch := range groupedAtts { + for validatorChunkIndex, attestations := range groupedByValidatorChunkIndexAtts { innerStart := time.Now() // The fact that we use always slashertypes.MinSpan is probably the root cause of // https://github.com/prysmaticlabs/prysm/issues/13591 - attSlashings, err := s.checkSurrounds(ctx, batch, slashertypes.MinSpan, currentEpoch, validatorChunkIdx) + attSlashings, err := s.checkSurrounds(ctx, attestations, slashertypes.MinSpan, currentEpoch, validatorChunkIndex) if err != nil { return nil, err } slashings = append(slashings, attSlashings...) - indices := s.params.validatorIndexesInChunk(validatorChunkIdx) + indices := s.params.validatorIndexesInChunk(validatorChunkIndex) for _, idx := range indices { s.latestEpochWrittenForValidator[idx] = currentEpoch } @@ -67,12 +66,12 @@ func (s *Service) checkSlashableAttestations( fields := logrus.Fields{ "numAttestations": len(atts), - "numBatchesByValidatorChunkIndex": grouppedAttsCount, + "numBatchesByValidatorChunkIndex": groupsCount, "elapsed": totalBatchDuration, } - if grouppedAttsCount > 0 { - avgProcessingTimePerBatch := totalBatchDuration / time.Duration(grouppedAttsCount) + if groupsCount > 0 { + avgProcessingTimePerBatch := totalBatchDuration / time.Duration(groupsCount) fields["avgBatchProcessingTime"] = avgProcessingTimePerBatch } @@ -226,9 +225,9 @@ func (s *Service) updateSpans( // Apply the attestations to the related chunks and find any // slashings along the way. slashings := make([]*ethpb.AttesterSlashing, 0) - for _, attestationBatch := range attestationsByChunkIdx { - for _, att := range attestationBatch { - for _, validatorIdx := range att.IndexedAttestation.AttestingIndices { + for _, attestations := range attestationsByChunkIdx { + for _, attestation := range attestations { + for _, validatorIdx := range attestation.IndexedAttestation.AttestingIndices { validatorIndex := primitives.ValidatorIndex(validatorIdx) computedValidatorChunkIdx := s.params.validatorChunkIndex(validatorIndex) @@ -246,8 +245,8 @@ func (s *Service) updateSpans( } slashing, err := s.applyAttestationForValidator( ctx, - att, updatedChunks, + attestation, kind, validatorChunkIndex, validatorIndex, @@ -277,8 +276,8 @@ func (s *Service) updateSpans( // source epoch up to its target. func (s *Service) applyAttestationForValidator( ctx context.Context, - attestation *slashertypes.IndexedAttestationWrapper, chunksByChunkIdx map[uint64]Chunker, + attestation *slashertypes.IndexedAttestationWrapper, chunkKind slashertypes.ChunkKind, validatorChunkIndex uint64, validatorIndex primitives.ValidatorIndex, diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 5f470d34e48f..fe52ae7b12ec 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -529,8 +529,8 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { att := createAttestationWrapper(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, - att, chunksByChunkIdx, + att, args.kind, args.validatorChunkIndex, validatorIdx, @@ -552,8 +552,8 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { slashableAtt := createAttestationWrapper(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, - slashableAtt, chunksByChunkIdx, + slashableAtt, args.kind, args.validatorChunkIndex, validatorIdx, @@ -594,8 +594,8 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { att := createAttestationWrapper(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, - att, chunksByChunkIdx, + att, args.kind, args.validatorChunkIndex, validatorIdx, @@ -617,8 +617,8 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { slashableAtt := createAttestationWrapper(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, - slashableAtt, chunksByChunkIdx, + slashableAtt, args.kind, args.validatorChunkIndex, validatorIdx, From 0646d11c15910e33daec25f05477e4fb91e2eecd Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 21:51:04 +0100 Subject: [PATCH 17/38] `applyAttestationForValidator`: Rename variables. --- beacon-chain/slasher/detect_attestations.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index cafc815ad78b..88a76ced3184 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -285,15 +285,16 @@ func (s *Service) applyAttestationForValidator( ) (*ethpb.AttesterSlashing, error) { ctx, span := trace.StartSpan(ctx, "Slasher.applyAttestationForValidator") defer span.End() + sourceEpoch := attestation.IndexedAttestation.Data.Source.Epoch targetEpoch := attestation.IndexedAttestation.Data.Target.Epoch attestationDistance.Observe(float64(targetEpoch) - float64(sourceEpoch)) - chunkIdx := s.params.chunkIndex(sourceEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIdx) + chunkIndex := s.params.chunkIndex(sourceEpoch) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIndex) if err != nil { - return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) + return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) } // Check slashable, if so, return the slashing. @@ -328,14 +329,14 @@ func (s *Service) applyAttestationForValidator( // the start epoch of the next chunk. We exit once no longer need to // keep updating chunks. for { - chunkIdx = s.params.chunkIndex(startEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIdx) + chunkIndex = s.params.chunkIndex(startEpoch) + chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIndex) if err != nil { - return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIdx) + return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) } keepGoing, err := chunk.Update( &chunkUpdateArgs{ - chunkIndex: chunkIdx, + chunkIndex: chunkIndex, currentEpoch: currentEpoch, }, validatorIndex, @@ -346,13 +347,13 @@ func (s *Service) applyAttestationForValidator( return nil, errors.Wrapf( err, "could not update chunk at chunk index %d for validator index %d and current epoch %d", - chunkIdx, + chunkIndex, validatorIndex, currentEpoch, ) } // We update the chunksByChunkIdx map with the chunk we just updated. - chunksByChunkIdx[chunkIdx] = chunk + chunksByChunkIdx[chunkIndex] = chunk if !keepGoing { break } From c7e6aa95ee7322a6f25684fd4d59aa558262bd48 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Fri, 2 Feb 2024 21:52:23 +0100 Subject: [PATCH 18/38] `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. --- beacon-chain/slasher/detect_attestations_test.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index fe52ae7b12ec..085e3f64bd42 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -126,14 +126,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ + createAttestationWrapper(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), createAttestationWrapper(t, 0, 3, []uint64{0}, nil), - createAttestationWrapper( - t, - 1, - 2, - []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, - nil, - ), }, currentEpoch: 4, }, @@ -144,13 +138,7 @@ func Test_processQueuedAttestations(t *testing.T) { args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ createAttestationWrapper(t, 0, 3, []uint64{0}, nil), - createAttestationWrapper( - t, - 1, - 2, - []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, - nil, - ), + createAttestationWrapper(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), }, currentEpoch: 4, }, From fdcfe27dba0392113215b8212dec1f27639a18ba Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Sat, 3 Feb 2024 00:13:56 +0100 Subject: [PATCH 19/38] `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. --- beacon-chain/slasher/detect_attestations.go | 23 ++++++++------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 88a76ced3184..9cee268ed2bc 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -243,25 +243,20 @@ func (s *Service) updateSpans( if validatorChunkIndex != computedValidatorChunkIdx { continue } + slashing, err := s.applyAttestationForValidator( - ctx, - updatedChunks, - attestation, - kind, - validatorChunkIndex, - validatorIndex, - currentEpoch, + ctx, updatedChunks, attestation, kind, validatorChunkIndex, validatorIndex, currentEpoch, ) + if err != nil { - return nil, errors.Wrapf( - err, - "could not apply attestation for validator index %d", - validatorIndex, - ) + return nil, errors.Wrapf(err, "could not apply attestation for validator index %d", validatorIndex) } - if slashing != nil { - slashings = append(slashings, slashing) + + if slashing == nil { + continue } + + slashings = append(slashings, slashing) } } } From 41af050604f9cdf5ee6ee5531ce48c6fd8e9593a Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Sat, 3 Feb 2024 00:14:35 +0100 Subject: [PATCH 20/38] `checkSurrounds`: Rename variable. --- beacon-chain/slasher/detect_attestations.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index 9cee268ed2bc..ece2d3509e21 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -103,7 +103,7 @@ func (s *Service) checkSurrounds( ) ([]*ethpb.AttesterSlashing, error) { // Map of updated chunks by chunk index, which will be saved at the end. updatedChunks := make(map[uint64]Chunker) - groupedAtts := s.groupByChunkIndex(attestations) + groupedByChunkIndexAtts := s.groupByChunkIndex(attestations) validatorIndexes := s.params.validatorIndexesInChunk(validatorChunkIndex) // Update the min/max span chunks for the change of current epoch. @@ -115,12 +115,12 @@ func (s *Service) checkSurrounds( } // Update min and max spans and retrieve any detected slashable offenses. - surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MinSpan, validatorChunkIndex, currentEpoch) + surroundingSlashings, err := s.updateSpans(ctx, updatedChunks, 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) } - surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, groupedAtts, slashertypes.MaxSpan, validatorChunkIndex, currentEpoch) + surroundedSlashings, err := s.updateSpans(ctx, updatedChunks, 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) } From 36adda9c07f44e6a41a1d359f9f7747789dcd8ec Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Sat, 3 Feb 2024 00:37:09 +0100 Subject: [PATCH 21/38] `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. --- beacon-chain/slasher/detect_attestations.go | 50 ++++++++++++--------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index ece2d3509e21..d492d923b877 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -171,6 +171,8 @@ func (s *Service) epochUpdateForValidator( currentEpoch primitives.Epoch, validatorIndex primitives.ValidatorIndex, ) error { + var err error + latestEpochWritten, ok := s.latestEpochWrittenForValidator[validatorIndex] if !ok { return nil @@ -179,9 +181,12 @@ func (s *Service) epochUpdateForValidator( for latestEpochWritten <= currentEpoch { chunkIndex := s.params.chunkIndex(latestEpochWritten) - currentChunk, err := s.getChunk(ctx, updatedChunks, chunkKind, validatorChunkIndex, chunkIndex) - if err != nil { - return err + currentChunk, ok := updatedChunks[chunkIndex] + if !ok { + currentChunk, err = s.getChunk(ctx, chunkKind, validatorChunkIndex, chunkIndex) + if err != nil { + return errors.Wrap(err, "could not get chunk") + } } for s.params.chunkIndex(latestEpochWritten) == chunkIndex && latestEpochWritten <= currentEpoch { @@ -281,15 +286,20 @@ func (s *Service) applyAttestationForValidator( ctx, span := trace.StartSpan(ctx, "Slasher.applyAttestationForValidator") defer span.End() + var err error + sourceEpoch := attestation.IndexedAttestation.Data.Source.Epoch targetEpoch := attestation.IndexedAttestation.Data.Target.Epoch attestationDistance.Observe(float64(targetEpoch) - float64(sourceEpoch)) - chunkIndex := s.params.chunkIndex(sourceEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIndex) - if err != nil { - return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) + + chunk, ok := chunksByChunkIdx[chunkIndex] + if !ok { + chunk, err = s.getChunk(ctx, chunkKind, validatorChunkIndex, chunkIndex) + if err != nil { + return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) + } } // Check slashable, if so, return the slashing. @@ -325,10 +335,15 @@ func (s *Service) applyAttestationForValidator( // keep updating chunks. for { chunkIndex = s.params.chunkIndex(startEpoch) - chunk, err := s.getChunk(ctx, chunksByChunkIdx, chunkKind, validatorChunkIndex, chunkIndex) - if err != nil { - return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) + + chunk, ok := chunksByChunkIdx[chunkIndex] + if !ok { + chunk, err = s.getChunk(ctx, chunkKind, validatorChunkIndex, chunkIndex) + if err != nil { + return nil, errors.Wrapf(err, "could not get chunk at index %d", chunkIndex) + } } + keepGoing, err := chunk.Update( &chunkUpdateArgs{ chunkIndex: chunkIndex, @@ -338,6 +353,7 @@ func (s *Service) applyAttestationForValidator( startEpoch, targetEpoch, ) + if err != nil { return nil, errors.Wrapf( err, @@ -347,33 +363,27 @@ func (s *Service) applyAttestationForValidator( currentEpoch, ) } + // We update the chunksByChunkIdx map with the chunk we just updated. chunksByChunkIdx[chunkIndex] = chunk if !keepGoing { break } + // Move to first epoch of next chunk if needed. startEpoch = chunk.NextChunkStartEpoch(startEpoch) } + return nil, nil } -// Retrieves a chunk from `chunksByChunkIndex` using a chunk index. -// If such chunk does not exist, which should be rare -// (occurring when we receive an attestation with source and target epochs -// that span multiple chunk indices), then fallbacks to fetching from database. +// Retrieve a chunk from database from database. func (s *Service) getChunk( ctx context.Context, - chunksByChunkIndex map[uint64]Chunker, chunkKind slashertypes.ChunkKind, validatorChunkIndex uint64, chunkIndex uint64, ) (Chunker, error) { - // If the chunk exists in the map, we return it. - if chunk, ok := chunksByChunkIndex[chunkIndex]; ok { - return chunk, nil - } - // We can ensure we load the appropriate chunk we need by fetching from the DB. diskChunks, err := s.loadChunks(ctx, validatorChunkIndex, chunkKind, []uint64{chunkIndex}) if err != nil { From d091142dfc76ce34061d445dd4ef980e762c0d93 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Sat, 3 Feb 2024 01:52:06 +0100 Subject: [PATCH 22/38] `CheckSlashable`: Flatten. --- beacon-chain/slasher/chunks.go | 91 +++++++++++-------- .../slasher/detect_attestations_test.go | 4 +- 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/beacon-chain/slasher/chunks.go b/beacon-chain/slasher/chunks.go index 3f8afed47f41..be4a743a3ea1 100644 --- a/beacon-chain/slasher/chunks.go +++ b/beacon-chain/slasher/chunks.go @@ -199,30 +199,35 @@ func (m *MinSpanChunksSlice) CheckSlashable( ) } - if targetEpoch > minTarget { - existingAttRecord, err := slasherDB.AttestationRecordForValidator( - ctx, validatorIdx, minTarget, - ) - if err != nil { - return nil, errors.Wrapf( - err, "could not get existing attestation record at target %d", minTarget, - ) - } + if targetEpoch <= minTarget { + // The incoming attestation does not surround any existing ones. + return nil, nil + } - if existingAttRecord == nil { - return nil, nil - } + // The incoming attestation surrounds an existing one. + existingAttRecord, err := slasherDB.AttestationRecordForValidator(ctx, validatorIdx, minTarget) + if err != nil { + return nil, errors.Wrapf(err, "could not get existing attestation record at target %d", minTarget) + } - if sourceEpoch < existingAttRecord.IndexedAttestation.Data.Source.Epoch { - surroundingVotesTotal.Inc() - return ðpb.AttesterSlashing{ - Attestation_1: attestation.IndexedAttestation, - Attestation_2: existingAttRecord.IndexedAttestation, - }, nil - } + if existingAttRecord == nil { + return nil, nil + } + + if existingAttRecord.IndexedAttestation.Data.Source.Epoch <= sourceEpoch { + // This case should normally not happen, since if we have targetEpoch > minTarget, + // then there is at least one attestation we surround. + // However, it can happens if we have multiple attestation with the same target + // but with a different source. In this case, we have both a double vote AND a surround vote. + // The validator will be slashed for the double vote, and the surround vote will be ignored. + return nil, nil } - return nil, nil + surroundingVotesTotal.Inc() + return ðpb.AttesterSlashing{ + Attestation_1: attestation.IndexedAttestation, + Attestation_2: existingAttRecord.IndexedAttestation, + }, nil } // CheckSlashable takes in a validator index and an incoming attestation @@ -252,29 +257,35 @@ func (m *MaxSpanChunksSlice) CheckSlashable( ) } - if targetEpoch < maxTarget { - existingAttRecord, err := slasherDB.AttestationRecordForValidator( - ctx, validatorIdx, maxTarget, - ) - if err != nil { - return nil, errors.Wrapf( - err, "could not get existing attestation record at target %d", maxTarget, - ) - } + if targetEpoch >= maxTarget { + // The incoming attestation is not surrounded by any existing ones. + return nil, nil + } - if existingAttRecord == nil { - return nil, nil - } + // The incoming attestation is surrounded by an existing one. + existingAttRecord, err := slasherDB.AttestationRecordForValidator(ctx, validatorIdx, maxTarget) + if err != nil { + return nil, errors.Wrapf(err, "could not get existing attestation record at target %d", maxTarget) + } - if existingAttRecord.IndexedAttestation.Data.Source.Epoch < sourceEpoch { - surroundedVotesTotal.Inc() - return ðpb.AttesterSlashing{ - Attestation_1: existingAttRecord.IndexedAttestation, - Attestation_2: attestation.IndexedAttestation, - }, nil - } + if existingAttRecord == nil { + return nil, errors.Wrap(err, "no existing attestation record found") + } + + if existingAttRecord.IndexedAttestation.Data.Source.Epoch >= sourceEpoch { + // This case should normally not happen, since if we have targetEpoch < maxTarget, + // then there is at least one attestation that surrounds us. + // However, it can happens if we have multiple attestation with the same target + // but with a different source. In this case, we have both a double vote AND a surround vote. + // The validator will be slashed for the double vote, and the surround vote will be ignored. + return nil, nil } - return nil, nil + + surroundedVotesTotal.Inc() + return ðpb.AttesterSlashing{ + Attestation_1: existingAttRecord.IndexedAttestation, + Attestation_2: attestation.IndexedAttestation, + }, nil } // Update a min span chunk for a validator index starting at the current epoch, e_c, then updating diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 085e3f64bd42..af6a3db7bbc0 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -525,7 +525,7 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { args.currentEpoch, ) require.NoError(t, err) - require.Equal(t, true, slashing == nil) + require.IsNil(t, slashing) att.IndexedAttestation.AttestingIndices = []uint64{uint64(validatorIdx)} err = slasherDB.SaveAttestationRecordsForValidators( ctx, @@ -878,10 +878,12 @@ func createAttestationWrapper(t testing.TB, source, target primitives.Epoch, ind Root: params.BeaconConfig().ZeroHash[:], }, } + signRoot, err := data.HashTreeRoot() if err != nil { t.Fatal(err) } + return &slashertypes.IndexedAttestationWrapper{ IndexedAttestation: ðpb.IndexedAttestation{ AttestingIndices: indices, From e28858ec954bfeeed4daa8ed7e622c73717ccedd Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Sat, 3 Feb 2024 02:38:52 +0100 Subject: [PATCH 23/38] `detect_attestations_test.go`: Simplify. --- beacon-chain/slasher/chunks.go | 10 ++-- .../slasher/detect_attestations_test.go | 48 +++++++------------ 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/beacon-chain/slasher/chunks.go b/beacon-chain/slasher/chunks.go index be4a743a3ea1..fac47da4447a 100644 --- a/beacon-chain/slasher/chunks.go +++ b/beacon-chain/slasher/chunks.go @@ -16,10 +16,8 @@ import ( // functions used for attester slashing detection and // loading, saving, and updating min/max span chunks. type chunkUpdateArgs struct { - kind slashertypes.ChunkKind - chunkIndex uint64 - validatorChunkIndex uint64 - currentEpoch primitives.Epoch + chunkIndex uint64 + currentEpoch primitives.Epoch } // Chunker defines a struct which represents a slice containing a chunk for K different validator's @@ -503,12 +501,12 @@ func (m *MinSpanChunksSlice) NextChunkStartEpoch(startEpoch primitives.Epoch) pr // max_spans_val_i = [[-, -, -], [-, -, -], [-, -, -]] // // If C = chunkSize is 3 epochs per chunk, and we input start epoch of chunk 1 which is 3. The next start -// epoch is the start epoch of chunk 2, which is epoch 4. This is computed as: +// epoch is the start epoch of chunk 2, which is epoch 6. This is computed as: // // first_epoch(chunkIndex(startEpoch)+1) // first_epoch(chunkIndex(3)+1) // first_epoch(1 + 1) -// first_epoch(2) +// first_epoch(6) // 4 func (m *MaxSpanChunksSlice) NextChunkStartEpoch(startEpoch primitives.Epoch) primitives.Epoch { return m.params.firstEpoch(m.params.chunkIndex(startEpoch) + 1) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index af6a3db7bbc0..b5a0870dcce2 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -489,7 +489,6 @@ func Test_epochUpdateForValidators(t *testing.T) { func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { ctx := context.Background() slasherDB := dbtest.SetupSlasherDB(t) - defaultParams := DefaultParams() srv, err := New(context.Background(), &ServiceConfig{ Database: slasherDB, @@ -499,17 +498,10 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { require.NoError(t, err) // We initialize an empty chunks slice. - chunk := EmptyMinSpanChunksSlice(defaultParams) - chunkIdx := uint64(0) currentEpoch := primitives.Epoch(3) + validatorChunkIndex := uint64(0) validatorIdx := primitives.ValidatorIndex(0) - args := &chunkUpdateArgs{ - chunkIndex: chunkIdx, - currentEpoch: currentEpoch, - } - chunksByChunkIdx := map[uint64]Chunker{ - chunkIdx: chunk, - } + chunksByChunkIdx := map[uint64]Chunker{} // We apply attestation with (source 1, target 2) for our validator. source := primitives.Epoch(1) @@ -519,10 +511,10 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { ctx, chunksByChunkIdx, att, - args.kind, - args.validatorChunkIndex, + slashertypes.MinSpan, + validatorChunkIndex, validatorIdx, - args.currentEpoch, + currentEpoch, ) require.NoError(t, err) require.IsNil(t, slashing) @@ -542,10 +534,10 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { ctx, chunksByChunkIdx, slashableAtt, - args.kind, - args.validatorChunkIndex, + slashertypes.MinSpan, + validatorChunkIndex, validatorIdx, - args.currentEpoch, + currentEpoch, ) require.NoError(t, err) require.NotNil(t, slashing) @@ -554,7 +546,6 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { ctx := context.Background() slasherDB := dbtest.SetupSlasherDB(t) - defaultParams := DefaultParams() srv, err := New(context.Background(), &ServiceConfig{ Database: slasherDB, @@ -564,17 +555,10 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { require.NoError(t, err) // We initialize an empty chunks slice. - chunk := EmptyMaxSpanChunksSlice(defaultParams) - chunkIdx := uint64(0) currentEpoch := primitives.Epoch(3) + validatorChunkIndex := uint64(0) validatorIdx := primitives.ValidatorIndex(0) - args := &chunkUpdateArgs{ - chunkIndex: chunkIdx, - currentEpoch: currentEpoch, - } - chunksByChunkIdx := map[uint64]Chunker{ - chunkIdx: chunk, - } + chunksByChunkIdx := map[uint64]Chunker{} // We apply attestation with (source 0, target 3) for our validator. source := primitives.Epoch(0) @@ -584,10 +568,10 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { ctx, chunksByChunkIdx, att, - args.kind, - args.validatorChunkIndex, + slashertypes.MaxSpan, + validatorChunkIndex, validatorIdx, - args.currentEpoch, + currentEpoch, ) require.NoError(t, err) require.Equal(t, true, slashing == nil) @@ -607,10 +591,10 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { ctx, chunksByChunkIdx, slashableAtt, - args.kind, - args.validatorChunkIndex, + slashertypes.MaxSpan, + validatorChunkIndex, validatorIdx, - args.currentEpoch, + currentEpoch, ) require.NoError(t, err) require.NotNil(t, slashing) From 7b136b9d9d6f2c97a708434fa9d05cc1d24c856c Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 5 Feb 2024 12:52:27 +0100 Subject: [PATCH 24/38] `CheckSlashable`: Add error log in case of missing attestation. --- beacon-chain/slasher/chunks.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/beacon-chain/slasher/chunks.go b/beacon-chain/slasher/chunks.go index fac47da4447a..71b29a8816e9 100644 --- a/beacon-chain/slasher/chunks.go +++ b/beacon-chain/slasher/chunks.go @@ -209,6 +209,14 @@ func (m *MinSpanChunksSlice) CheckSlashable( } if existingAttRecord == nil { + // This case should normally not happen. If this happen, it means we previously + // recorded in our min/max DB an distance corresponding to an attestaiton, but WITHOUT + // recording the attestation itself. As a consequence, we say there is no surrounding vote, + // but we log an error. + log.Errorf("No existing attestation record found for validator %d at target %d, while a surrounding vote was detected.", + validatorIdx, minTarget, + ) + return nil, nil } @@ -267,7 +275,14 @@ func (m *MaxSpanChunksSlice) CheckSlashable( } if existingAttRecord == nil { - return nil, errors.Wrap(err, "no existing attestation record found") + // This case should normally not happen. If this happen, it means we previously + // recorded in our min/max DB an distance corresponding to an attestaiton, but WITHOUT + // recording the attestation itself. As a consequence, we say there is no surrounded vote, + // but we log an error. + log.Errorf("No existing attestation record found for validator %d at target %d, while a surrounded vote was detected.", + validatorIdx, maxTarget, + ) + return nil, nil } if existingAttRecord.IndexedAttestation.Data.Source.Epoch >= sourceEpoch { From caeb94741bfa0a272ed2a487559ec14f2857eedb Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 5 Feb 2024 17:56:30 +0100 Subject: [PATCH 25/38] `processQueuedAttestations`: Extract a sub function. So testing will be easier. --- beacon-chain/slasher/receive.go | 102 ++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 43 deletions(-) diff --git a/beacon-chain/slasher/receive.go b/beacon-chain/slasher/receive.go index 2db40b82ca3a..4bdb32a6268b 100644 --- a/beacon-chain/slasher/receive.go +++ b/beacon-chain/slasher/receive.go @@ -92,56 +92,72 @@ func (s *Service) processQueuedAttestations(ctx context.Context, slotTicker <-ch for { select { case currentSlot := <-slotTicker: + // Retrieve all attestations from the queue. attestations := s.attsQueue.dequeue() - currentEpoch := slots.ToEpoch(currentSlot) - // We take all the attestations in the queue and filter out - // those which are valid now and valid in the future. - validAtts, validInFuture, numDropped := s.filterAttestations(attestations, currentEpoch) - deferredAttestationsTotal.Add(float64(len(validInFuture))) - droppedAttestationsTotal.Add(float64(numDropped)) + // Process the retrieved attestations. + s.processAttestations(ctx, attestations, currentSlot) + case <-ctx.Done(): + return + } + } +} - // We add back those attestations that are valid in the future to the queue. - s.attsQueue.extend(validInFuture) - attsQueueSize := s.attsQueue.size() +func (s *Service) processAttestations( + ctx context.Context, + attestations []*slashertypes.IndexedAttestationWrapper, + currentSlot primitives.Slot, +) { + // Get the current epoch from the current slot. + currentEpoch := slots.ToEpoch(currentSlot) - log.WithFields(logrus.Fields{ - "currentSlot": currentSlot, - "currentEpoch": currentEpoch, - "numValidAtts": len(validAtts), - "numDeferredAtts": len(validInFuture), - "numDroppedAtts": numDropped, - "attsQueueSize": attsQueueSize, - }).Info("Processing queued attestations for slashing detection") - - // Save the attestation records to our database. - // If multiple attestations are provided for the same validator index + target epoch combination, - // then last (validator index + target epoch) => signing root) link is kept into the database. - if err := s.serviceCfg.Database.SaveAttestationRecordsForValidators( - ctx, validAtts, - ); err != nil { - log.WithError(err).Error(couldNotSaveAttRecord) - continue - } + // Take all the attestations in the queue and filter out + // those which are valid now and valid in the future. + validAttestations, validInFutureAttestations, numDropped := s.filterAttestations(attestations, currentEpoch) - // Check for slashings. - slashings, err := s.checkSlashableAttestations(ctx, currentEpoch, validAtts) - if err != nil { - log.WithError(err).Error(couldNotCheckSlashableAtt) - continue - } + // Increase corresponding prometheus metrics. + deferredAttestationsTotal.Add(float64(len(validInFutureAttestations))) + droppedAttestationsTotal.Add(float64(numDropped)) + processedAttestationsTotal.Add(float64(len(validAttestations))) - // Process attester slashings by verifying their signatures, submitting - // to the beacon node's operations pool, and logging them. - if err := s.processAttesterSlashings(ctx, slashings); err != nil { - log.WithError(err).Error(couldNotProcessAttesterSlashings) - continue - } + // We add back those attestations that are valid in the future to the queue. + s.attsQueue.extend(validInFutureAttestations) - processedAttestationsTotal.Add(float64(len(validAtts))) - case <-ctx.Done(): - return - } + // Compute some counts. + queuedAttestationsCount := s.attsQueue.size() + validAttestationsCount := len(validAttestations) + validInFutureAttestationsCount := len(validInFutureAttestations) + + // Log useful infrormation + log.WithFields(logrus.Fields{ + "currentSlot": currentSlot, + "currentEpoch": currentEpoch, + "numValidAtts": validAttestationsCount, + "numDeferredAtts": validInFutureAttestationsCount, + "numDroppedAtts": numDropped, + "attsQueueSize": queuedAttestationsCount, + }).Info("Processing queued attestations for slashing detection") + + // Save the attestation records to our database. + // If multiple attestations are provided for the same validator index + target epoch combination, + // then the first (validator index + target epoch) => signing root) link is kept into the database. + if err := s.serviceCfg.Database.SaveAttestationRecordsForValidators(ctx, validAttestations); err != nil { + log.WithError(err).Error(couldNotSaveAttRecord) + return + } + + // Check for attestatinos slashings (double, sourrounding, surrounded votes). + slashings, err := s.checkSlashableAttestations(ctx, currentEpoch, validAttestations) + if err != nil { + log.WithError(err).Error(couldNotCheckSlashableAtt) + return + } + + // Process attester slashings by verifying their signatures, submitting + // to the beacon node's operations pool, and logging them. + if err := s.processAttesterSlashings(ctx, slashings); err != nil { + log.WithError(err).Error(couldNotProcessAttesterSlashings) + return } } From f7d47257aa61004e2194d449ffd0494d95837561 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 5 Feb 2024 18:24:49 +0100 Subject: [PATCH 26/38] `processAttesterSlashings` and `processProposerSlashings`: Improve. --- beacon-chain/slasher/process_slashings.go | 79 ++++++++++++++--------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/beacon-chain/slasher/process_slashings.go b/beacon-chain/slasher/process_slashings.go index 9e7f02fb168a..8d73263b9582 100644 --- a/beacon-chain/slasher/process_slashings.go +++ b/beacon-chain/slasher/process_slashings.go @@ -3,8 +3,8 @@ package slasher import ( "context" + "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/v4/beacon-chain/core/blocks" - "github.com/prysmaticlabs/prysm/v4/beacon-chain/state" "github.com/prysmaticlabs/prysm/v4/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" ) @@ -12,69 +12,86 @@ import ( // Verifies attester slashings, logs them, and submits them to the slashing operations pool // in the beacon node if they pass validation. func (s *Service) processAttesterSlashings(ctx context.Context, slashings []*ethpb.AttesterSlashing) error { - var beaconState state.BeaconState - var err error - if len(slashings) > 0 { - beaconState, err = s.serviceCfg.HeadStateFetcher.HeadState(ctx) - if err != nil { - return err - } + // If no slashings, return early. + if len(slashings) == 0 { + return nil } - for _, sl := range slashings { - if err := s.verifyAttSignature(ctx, sl.Attestation_1); err != nil { - log.WithError(err).WithField("a", sl.Attestation_1).Warn( + + // Get the head state. + beaconState, err := s.serviceCfg.HeadStateFetcher.HeadState(ctx) + if err != nil { + return nil, errors.Wrap(err, "could not get head state") + } + + for _, slashing := range slashings { + // Verify the signature of the first attestation. + if err := s.verifyAttSignature(ctx, slashing.Attestation_1); err != nil { + log.WithError(err).WithField("a", slashing.Attestation_1).Warn( "Invalid signature for attestation in detected slashing offense", ) + continue } - if err := s.verifyAttSignature(ctx, sl.Attestation_2); err != nil { - log.WithError(err).WithField("b", sl.Attestation_2).Warn( + + // Verify the signature of the second attestation. + if err := s.verifyAttSignature(ctx, slashing.Attestation_2); err != nil { + log.WithError(err).WithField("b", slashing.Attestation_2).Warn( "Invalid signature for attestation in detected slashing offense", ) + continue } // Log the slashing event and insert into the beacon node's operations pool. - logAttesterSlashing(sl) - if err := s.serviceCfg.SlashingPoolInserter.InsertAttesterSlashing( - ctx, beaconState, sl, - ); err != nil { + logAttesterSlashing(slashing) + if err := s.serviceCfg.SlashingPoolInserter.InsertAttesterSlashing(ctx, beaconState, slashing); err != nil { log.WithError(err).Error("Could not insert attester slashing into operations pool") } } + return nil } // Verifies proposer slashings, logs them, and submits them to the slashing operations pool // in the beacon node if they pass validation. func (s *Service) processProposerSlashings(ctx context.Context, slashings []*ethpb.ProposerSlashing) error { - var beaconState state.BeaconState - var err error - if len(slashings) > 0 { - beaconState, err = s.serviceCfg.HeadStateFetcher.HeadState(ctx) - if err != nil { - return err - } + // If no slashings, return early. + if len(slashings) == 0 { + return nil } - for _, sl := range slashings { - if err := s.verifyBlockSignature(ctx, sl.Header_1); err != nil { - log.WithError(err).WithField("a", sl.Header_1).Warn( + + // Get the head state. + beaconState, err := s.serviceCfg.HeadStateFetcher.HeadState(ctx) + if err != nil { + return err + } + + for _, slashing := range slashings { + // Verify the signature of the first block. + if err := s.verifyBlockSignature(ctx, slashing.Header_1); err != nil { + log.WithError(err).WithField("a", slashing.Header_1).Warn( "Invalid signature for block header in detected slashing offense", ) + continue } - if err := s.verifyBlockSignature(ctx, sl.Header_2); err != nil { - log.WithError(err).WithField("b", sl.Header_2).Warn( + + // Verify the signature of the second block. + if err := s.verifyBlockSignature(ctx, slashing.Header_2); err != nil { + log.WithError(err).WithField("b", slashing.Header_2).Warn( "Invalid signature for block header in detected slashing offense", ) + continue } + // Log the slashing event and insert into the beacon node's operations pool. - logProposerSlashing(sl) - if err := s.serviceCfg.SlashingPoolInserter.InsertProposerSlashing(ctx, beaconState, sl); err != nil { + logProposerSlashing(slashing) + if err := s.serviceCfg.SlashingPoolInserter.InsertProposerSlashing(ctx, beaconState, slashing); err != nil { log.WithError(err).Error("Could not insert proposer slashing into operations pool") } } + return nil } From 0aeb405d7610bd7a443f13bf7f27307e5be29fca Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 5 Feb 2024 18:32:26 +0100 Subject: [PATCH 27/38] `processAttesterSlashings`: Return processed slashings. --- beacon-chain/slasher/process_slashings.go | 10 +++++++--- beacon-chain/slasher/process_slashings_test.go | 6 +++--- beacon-chain/slasher/receive.go | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/beacon-chain/slasher/process_slashings.go b/beacon-chain/slasher/process_slashings.go index 8d73263b9582..f3b18a86ebf0 100644 --- a/beacon-chain/slasher/process_slashings.go +++ b/beacon-chain/slasher/process_slashings.go @@ -11,10 +11,12 @@ import ( // Verifies attester slashings, logs them, and submits them to the slashing operations pool // in the beacon node if they pass validation. -func (s *Service) processAttesterSlashings(ctx context.Context, slashings []*ethpb.AttesterSlashing) error { +func (s *Service) processAttesterSlashings(ctx context.Context, slashings []*ethpb.AttesterSlashing) ([]*ethpb.AttesterSlashing, error) { + processedSlashings := make([]*ethpb.AttesterSlashing, 0, len(slashings)) + // If no slashings, return early. if len(slashings) == 0 { - return nil + return processedSlashings, nil } // Get the head state. @@ -47,9 +49,11 @@ func (s *Service) processAttesterSlashings(ctx context.Context, slashings []*eth if err := s.serviceCfg.SlashingPoolInserter.InsertAttesterSlashing(ctx, beaconState, slashing); err != nil { log.WithError(err).Error("Could not insert attester slashing into operations pool") } + + processedSlashings = append(processedSlashings, slashing) } - return nil + return processedSlashings, nil } // Verifies proposer slashings, logs them, and submits them to the slashing operations pool diff --git a/beacon-chain/slasher/process_slashings_test.go b/beacon-chain/slasher/process_slashings_test.go index 5afda6f1df98..64b1f8e50759 100644 --- a/beacon-chain/slasher/process_slashings_test.go +++ b/beacon-chain/slasher/process_slashings_test.go @@ -82,7 +82,7 @@ func TestService_processAttesterSlashings(t *testing.T) { }, } - err = s.processAttesterSlashings(ctx, slashings) + _, err = s.processAttesterSlashings(ctx, slashings) require.NoError(tt, err) require.LogsContain(tt, hook, "Invalid signature") }) @@ -101,7 +101,7 @@ func TestService_processAttesterSlashings(t *testing.T) { }, } - err = s.processAttesterSlashings(ctx, slashings) + _, err = s.processAttesterSlashings(ctx, slashings) require.NoError(tt, err) require.LogsContain(tt, hook, "Invalid signature") }) @@ -120,7 +120,7 @@ func TestService_processAttesterSlashings(t *testing.T) { }, } - err = s.processAttesterSlashings(ctx, slashings) + _, err = s.processAttesterSlashings(ctx, slashings) require.NoError(tt, err) require.LogsDoNotContain(tt, hook, "Invalid signature") }) diff --git a/beacon-chain/slasher/receive.go b/beacon-chain/slasher/receive.go index 4bdb32a6268b..1bfb4d9dcbda 100644 --- a/beacon-chain/slasher/receive.go +++ b/beacon-chain/slasher/receive.go @@ -155,7 +155,7 @@ func (s *Service) processAttestations( // Process attester slashings by verifying their signatures, submitting // to the beacon node's operations pool, and logging them. - if err := s.processAttesterSlashings(ctx, slashings); err != nil { + if _, err := s.processAttesterSlashings(ctx, slashings); err != nil { log.WithError(err).Error(couldNotProcessAttesterSlashings) return } From 54c13c5c105247f9b729a12384eaca276d3e518d Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 12:33:02 +0100 Subject: [PATCH 28/38] `createAttestationWrapper`: Rename variables. --- .../slasher/detect_attestations_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index b5a0870dcce2..6eb0f1909d08 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -850,9 +850,14 @@ func runAttestationsBenchmark(b *testing.B, s *Service, numAtts, numValidators u } } -func createAttestationWrapper(t testing.TB, source, target primitives.Epoch, indices []uint64, signingRoot []byte) *slashertypes.IndexedAttestationWrapper { +func createAttestationWrapper( + t testing.TB, + source, target primitives.Epoch, + indices []uint64, + beaconBlockRoot []byte, +) *slashertypes.IndexedAttestationWrapper { data := ðpb.AttestationData{ - BeaconBlockRoot: bytesutil.PadTo(signingRoot, 32), + BeaconBlockRoot: bytesutil.PadTo(beaconBlockRoot, 32), Source: ðpb.Checkpoint{ Epoch: source, Root: params.BeaconConfig().ZeroHash[:], @@ -863,10 +868,8 @@ func createAttestationWrapper(t testing.TB, source, target primitives.Epoch, ind }, } - signRoot, err := data.HashTreeRoot() - if err != nil { - t.Fatal(err) - } + signingRoot, err := data.HashTreeRoot() + require.NoError(t, err) return &slashertypes.IndexedAttestationWrapper{ IndexedAttestation: ðpb.IndexedAttestation{ @@ -874,6 +877,6 @@ func createAttestationWrapper(t testing.TB, source, target primitives.Epoch, ind Data: data, Signature: params.BeaconConfig().EmptySignature[:], }, - SigningRoot: signRoot, + SigningRoot: signingRoot, } } From fd3c5acfd8b22ea99c79725f460dc56ed436620a Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 13:07:06 +0100 Subject: [PATCH 29/38] `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. --- beacon-chain/db/slasherkv/slasher.go | 47 +++++++++---------- beacon-chain/db/slasherkv/slasher_test.go | 24 +++++----- .../slasher/detect_attestations_test.go | 4 +- beacon-chain/slasher/detect_blocks.go | 2 +- beacon-chain/slasher/detect_blocks_test.go | 4 +- beacon-chain/slasher/receive.go | 8 ++-- beacon-chain/slasher/types/types.go | 8 ++-- 7 files changed, 49 insertions(+), 48 deletions(-) diff --git a/beacon-chain/db/slasherkv/slasher.go b/beacon-chain/db/slasherkv/slasher.go index edca7d011e11..b672d8cb2bad 100644 --- a/beacon-chain/db/slasherkv/slasher.go +++ b/beacon-chain/db/slasherkv/slasher.go @@ -21,9 +21,8 @@ import ( ) const ( - // Signing root (32 bytes) attestationRecordKeySize = 32 // Bytes. - signingRootSize = 32 // Bytes. + rootSize = 32 // Bytes. ) // LastEpochWrittenForValidators given a list of validator indices returns the latest @@ -170,15 +169,15 @@ func (s *Store) CheckAttesterDoubleVotes( continue } - // Retrieve the attestation record corresponding to the signing root + // Retrieve the attestation record corresponding to the data root // from the database. encExistingAttRecord := attRecordsBkt.Get(attRecordsKey) if encExistingAttRecord == nil { continue } - existingSigningRoot := bytesutil.ToBytes32(attRecordsKey[:signingRootSize]) - if existingSigningRoot == attToProcess.SigningRoot { + existingDataRoot := bytesutil.ToBytes32(attRecordsKey[:rootSize]) + if existingDataRoot == attToProcess.DataRoot { continue } @@ -281,12 +280,12 @@ func (s *Store) SaveAttestationRecordsForValidators( return s.db.Update(func(tx *bolt.Tx) error { attRecordsBkt := tx.Bucket(attestationRecordsBucket) - signingRootsBkt := tx.Bucket(attestationDataRootsBucket) + dataRootsBkt := tx.Bucket(attestationDataRootsBucket) for i := len(attestations) - 1; i >= 0; i-- { attestation := attestations[i] - if err := attRecordsBkt.Put(attestation.SigningRoot[:], encodedRecords[i]); err != nil { + if err := attRecordsBkt.Put(attestation.DataRoot[:], encodedRecords[i]); err != nil { return err } @@ -294,7 +293,7 @@ func (s *Store) SaveAttestationRecordsForValidators( encIdx := encodeValidatorIndex(primitives.ValidatorIndex(valIdx)) key := append(encodedTargetEpoch[i], encIdx...) - if err := signingRootsBkt.Put(key, attestation.SigningRoot[:]); err != nil { + if err := dataRootsBkt.Put(key, attestation.DataRoot[:]); err != nil { return err } } @@ -396,14 +395,14 @@ func (s *Store) CheckDoubleBlockProposals( // If there is no existing proposal record (empty result), then there is no double proposal. // We can continue to the next proposal. - if len(encExistingProposalWrapper) < signingRootSize { + if len(encExistingProposalWrapper) < rootSize { continue } // Compare the proposal signing root in the DB with the incoming proposal signing root. // If they differ, we have a double proposal. - existingSigningRoot := bytesutil.ToBytes32(encExistingProposalWrapper[:signingRootSize]) - if existingSigningRoot != incomingProposal.SigningRoot { + existingRoot := bytesutil.ToBytes32(encExistingProposalWrapper[:rootSize]) + if existingRoot != incomingProposal.HeaderRoot { existingProposalWrapper, err := decodeProposalRecord(encExistingProposalWrapper) if err != nil { return err @@ -610,18 +609,18 @@ func encodeAttestationRecord(att *slashertypes.IndexedAttestationWrapper) ([]byt // Compress attestation. compressedAtt := snappy.Encode(nil, encodedAtt) - return append(att.SigningRoot[:], compressedAtt...), nil + return append(att.DataRoot[:], compressedAtt...), nil } // Decode attestation record from bytes. // The input encoded attestation record consists in the signing root concatened with the compressed attestation record. func decodeAttestationRecord(encoded []byte) (*slashertypes.IndexedAttestationWrapper, error) { - if len(encoded) < signingRootSize { - return nil, fmt.Errorf("wrong length for encoded attestation record, want minimum %d, got %d", signingRootSize, len(encoded)) + if len(encoded) < rootSize { + return nil, fmt.Errorf("wrong length for encoded attestation record, want minimum %d, got %d", rootSize, len(encoded)) } // Decompress attestation. - decodedAttBytes, err := snappy.Decode(nil, encoded[signingRootSize:]) + decodedAttBytes, err := snappy.Decode(nil, encoded[rootSize:]) if err != nil { return nil, err } @@ -633,13 +632,13 @@ func decodeAttestationRecord(encoded []byte) (*slashertypes.IndexedAttestationWr } // Decode signing root. - signingRootBytes := encoded[:signingRootSize] - signingRoot := bytesutil.ToBytes32(signingRootBytes) + dataRootBytes := encoded[:rootSize] + dataRoot := bytesutil.ToBytes32(dataRootBytes) // Return decoded attestation. attestation := &slashertypes.IndexedAttestationWrapper{ IndexedAttestation: decodedAtt, - SigningRoot: signingRoot, + DataRoot: dataRoot, } return attestation, nil @@ -654,18 +653,18 @@ func encodeProposalRecord(blkHdr *slashertypes.SignedBlockHeaderWrapper) ([]byte return nil, err } compressedHdr := snappy.Encode(nil, encodedHdr) - return append(blkHdr.SigningRoot[:], compressedHdr...), nil + return append(blkHdr.HeaderRoot[:], compressedHdr...), nil } func decodeProposalRecord(encoded []byte) (*slashertypes.SignedBlockHeaderWrapper, error) { - if len(encoded) < signingRootSize { + if len(encoded) < rootSize { return nil, fmt.Errorf( - "wrong length for encoded proposal record, want %d, got %d", signingRootSize, len(encoded), + "wrong length for encoded proposal record, want %d, got %d", rootSize, len(encoded), ) } - signingRoot := encoded[:signingRootSize] + dataRoot := encoded[:rootSize] decodedBlkHdr := ðpb.SignedBeaconBlockHeader{} - decodedHdrBytes, err := snappy.Decode(nil, encoded[signingRootSize:]) + decodedHdrBytes, err := snappy.Decode(nil, encoded[rootSize:]) if err != nil { return nil, err } @@ -674,7 +673,7 @@ func decodeProposalRecord(encoded []byte) (*slashertypes.SignedBlockHeaderWrappe } return &slashertypes.SignedBlockHeaderWrapper{ SignedBeaconBlockHeader: decodedBlkHdr, - SigningRoot: bytesutil.ToBytes32(signingRoot), + HeaderRoot: bytesutil.ToBytes32(dataRoot), }, nil } diff --git a/beacon-chain/db/slasherkv/slasher_test.go b/beacon-chain/db/slasherkv/slasher_test.go index 9ff873d7b8d4..1c67630bd730 100644 --- a/beacon-chain/db/slasherkv/slasher_test.go +++ b/beacon-chain/db/slasherkv/slasher_test.go @@ -40,7 +40,7 @@ func TestStore_AttestationRecordForValidator_SaveRetrieve(t *testing.T) { require.NoError(t, err) assert.DeepEqual(t, target, attRecord.IndexedAttestation.Data.Target.Epoch) assert.DeepEqual(t, source, attRecord.IndexedAttestation.Data.Source.Epoch) - assert.DeepEqual(t, sr, attRecord.SigningRoot) + assert.DeepEqual(t, sr, attRecord.DataRoot) } func TestStore_LastEpochWrittenForValidators(t *testing.T) { @@ -224,9 +224,9 @@ func TestStore_ExistingBlockProposals(t *testing.T) { // Second time checking same proposals but all with different signing root should // return all double proposals. - proposals[0].SigningRoot = bytesutil.ToBytes32([]byte{2}) - proposals[1].SigningRoot = bytesutil.ToBytes32([]byte{2}) - proposals[2].SigningRoot = bytesutil.ToBytes32([]byte{2}) + proposals[0].HeaderRoot = bytesutil.ToBytes32([]byte{2}) + proposals[1].HeaderRoot = bytesutil.ToBytes32([]byte{2}) + proposals[2].HeaderRoot = bytesutil.ToBytes32([]byte{2}) doubleProposals, err = beaconDB.CheckDoubleBlockProposals(ctx, proposals) require.NoError(t, err) @@ -515,7 +515,7 @@ func createProposalWrapper(t *testing.T, slot primitives.Slot, proposerIndex pri StateRoot: bytesutil.PadTo(signingRoot, 32), BodyRoot: params.BeaconConfig().ZeroHash[:], } - signRoot, err := header.HashTreeRoot() + headerRoot, err := header.HashTreeRoot() if err != nil { t.Fatal(err) } @@ -524,15 +524,16 @@ func createProposalWrapper(t *testing.T, slot primitives.Slot, proposerIndex pri Header: header, Signature: params.BeaconConfig().EmptySignature[:], }, - SigningRoot: signRoot, + HeaderRoot: headerRoot, } } -func createAttestationWrapper(source, target primitives.Epoch, indices []uint64, signingRoot []byte) *slashertypes.IndexedAttestationWrapper { - signRoot := bytesutil.ToBytes32(signingRoot) - if signingRoot == nil { - signRoot = params.BeaconConfig().ZeroHash +func createAttestationWrapper(source, target primitives.Epoch, indices []uint64, dataRootBytes []byte) *slashertypes.IndexedAttestationWrapper { + dataRoot := bytesutil.ToBytes32(dataRootBytes) + if dataRootBytes == nil { + dataRoot = params.BeaconConfig().ZeroHash } + data := ðpb.AttestationData{ BeaconBlockRoot: params.BeaconConfig().ZeroHash[:], Source: ðpb.Checkpoint{ @@ -544,13 +545,14 @@ func createAttestationWrapper(source, target primitives.Epoch, indices []uint64, Root: params.BeaconConfig().ZeroHash[:], }, } + return &slashertypes.IndexedAttestationWrapper{ IndexedAttestation: ðpb.IndexedAttestation{ AttestingIndices: indices, Data: data, Signature: params.BeaconConfig().EmptySignature[:], }, - SigningRoot: signRoot, + DataRoot: dataRoot, } } diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 6eb0f1909d08..67f16b820a52 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -868,7 +868,7 @@ func createAttestationWrapper( }, } - signingRoot, err := data.HashTreeRoot() + dataRoot, err := data.HashTreeRoot() require.NoError(t, err) return &slashertypes.IndexedAttestationWrapper{ @@ -877,6 +877,6 @@ func createAttestationWrapper( Data: data, Signature: params.BeaconConfig().EmptySignature[:], }, - SigningRoot: signingRoot, + DataRoot: dataRoot, } } diff --git a/beacon-chain/slasher/detect_blocks.go b/beacon-chain/slasher/detect_blocks.go index acd5e55a8d87..552c383ffdd5 100644 --- a/beacon-chain/slasher/detect_blocks.go +++ b/beacon-chain/slasher/detect_blocks.go @@ -37,7 +37,7 @@ func (s *Service) detectProposerSlashings( } // If we have seen this proposal before, we check if it is a double proposal. - if isDoubleProposal(incomingProposal.SigningRoot, existingProposal.SigningRoot) { + if isDoubleProposal(incomingProposal.HeaderRoot, existingProposal.HeaderRoot) { doubleProposalsTotal.Inc() slashing := ðpb.ProposerSlashing{ diff --git a/beacon-chain/slasher/detect_blocks_test.go b/beacon-chain/slasher/detect_blocks_test.go index 8fbc2bfacb5e..d2da8399f3d6 100644 --- a/beacon-chain/slasher/detect_blocks_test.go +++ b/beacon-chain/slasher/detect_blocks_test.go @@ -204,7 +204,7 @@ func createProposalWrapper(t *testing.T, slot primitives.Slot, proposerIndex pri StateRoot: bytesutil.PadTo(signingRoot, 32), BodyRoot: params.BeaconConfig().ZeroHash[:], } - signRoot, err := header.HashTreeRoot() + headerRoot, err := header.HashTreeRoot() require.NoError(t, err) fakeSig := make([]byte, 96) copy(fakeSig, "hello") @@ -213,6 +213,6 @@ func createProposalWrapper(t *testing.T, slot primitives.Slot, proposerIndex pri Header: header, Signature: fakeSig, }, - SigningRoot: signRoot, + HeaderRoot: headerRoot, } } diff --git a/beacon-chain/slasher/receive.go b/beacon-chain/slasher/receive.go index 1bfb4d9dcbda..8df5af3f53af 100644 --- a/beacon-chain/slasher/receive.go +++ b/beacon-chain/slasher/receive.go @@ -32,14 +32,14 @@ func (s *Service) receiveAttestations(ctx context.Context, indexedAttsChan chan if !validateAttestationIntegrity(att) { continue } - signingRoot, err := att.Data.HashTreeRoot() + dataRoot, err := att.Data.HashTreeRoot() if err != nil { log.WithError(err).Error("Could not get hash tree root of attestation") continue } attWrapper := &slashertypes.IndexedAttestationWrapper{ IndexedAttestation: att, - SigningRoot: signingRoot, + DataRoot: dataRoot, } s.attsQueue.push(attWrapper) case err := <-sub.Err(): @@ -63,14 +63,14 @@ func (s *Service) receiveBlocks(ctx context.Context, beaconBlockHeadersChan chan if !validateBlockHeaderIntegrity(blockHeader) { continue } - signingRoot, err := blockHeader.Header.HashTreeRoot() + headerRoot, err := blockHeader.Header.HashTreeRoot() if err != nil { log.WithError(err).Error("Could not get hash tree root of signed block header") continue } wrappedProposal := &slashertypes.SignedBlockHeaderWrapper{ SignedBeaconBlockHeader: blockHeader, - SigningRoot: signingRoot, + HeaderRoot: headerRoot, } s.blksQueue.push(wrappedProposal) case err := <-sub.Err(): diff --git a/beacon-chain/slasher/types/types.go b/beacon-chain/slasher/types/types.go index a622676eb6f4..c02fc9b6b49b 100644 --- a/beacon-chain/slasher/types/types.go +++ b/beacon-chain/slasher/types/types.go @@ -15,10 +15,10 @@ const ( ) // IndexedAttestationWrapper contains an indexed attestation with its -// signing root to reduce duplicated computation. +// data root to reduce duplicated computation. type IndexedAttestationWrapper struct { IndexedAttestation *ethpb.IndexedAttestation - SigningRoot [32]byte + DataRoot [32]byte } // AttesterDoubleVote represents a double vote instance @@ -39,10 +39,10 @@ type DoubleBlockProposal struct { } // SignedBlockHeaderWrapper contains an signed beacon block header with its -// signing root to reduce duplicated computation. +// header root to reduce duplicated computation. type SignedBlockHeaderWrapper struct { SignedBeaconBlockHeader *ethpb.SignedBeaconBlockHeader - SigningRoot [32]byte + HeaderRoot [32]byte } // AttestedEpochForValidator encapsulates a previously attested epoch From af8a6c6931a3bbd8d1d4a6537c219a3d6bf24604 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 14:05:11 +0100 Subject: [PATCH 30/38] `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. --- beacon-chain/slasher/chunks_test.go | 16 ++-- .../slasher/detect_attestations_test.go | 94 ++++++++++--------- beacon-chain/slasher/helpers_test.go | 62 ++++++------ beacon-chain/slasher/queue_test.go | 8 +- beacon-chain/slasher/receive_test.go | 26 ++--- 5 files changed, 105 insertions(+), 101 deletions(-) diff --git a/beacon-chain/slasher/chunks_test.go b/beacon-chain/slasher/chunks_test.go index 9963edb94704..790fb2368cf3 100644 --- a/beacon-chain/slasher/chunks_test.go +++ b/beacon-chain/slasher/chunks_test.go @@ -91,7 +91,7 @@ func TestMinSpanChunksSlice_CheckSlashable(t *testing.T) { validatorIdx := primitives.ValidatorIndex(1) source := primitives.Epoch(1) target := primitives.Epoch(2) - att := createAttestationWrapper(t, source, target, nil, nil) + att := createAttestationWrapperEmptySig(t, source, target, nil, nil) // A faulty chunk should lead to error. chunk := &MinSpanChunksSlice{ @@ -126,7 +126,7 @@ func TestMinSpanChunksSlice_CheckSlashable(t *testing.T) { chunk = EmptyMinSpanChunksSlice(params) source = primitives.Epoch(1) target = primitives.Epoch(2) - att = createAttestationWrapper(t, source, target, nil, nil) + att = createAttestationWrapperEmptySig(t, source, target, nil, nil) chunkIdx := uint64(0) startEpoch := target currentEpoch := target @@ -141,7 +141,7 @@ func TestMinSpanChunksSlice_CheckSlashable(t *testing.T) { // because we DO NOT have an existing attestation record in our database at the min target epoch. source = primitives.Epoch(0) target = primitives.Epoch(3) - surroundingVote := createAttestationWrapper(t, source, target, nil, nil) + surroundingVote := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err = chunk.CheckSlashable(ctx, slasherDB, validatorIdx, surroundingVote) require.NoError(t, err) @@ -150,7 +150,7 @@ func TestMinSpanChunksSlice_CheckSlashable(t *testing.T) { // Next up, we save the old attestation record, then check if the // surrounding vote is indeed slashable. attData := att.IndexedAttestation.Data - attRecord := createAttestationWrapper(t, attData.Source.Epoch, attData.Target.Epoch, []uint64{uint64(validatorIdx)}, []byte{1}) + attRecord := createAttestationWrapperEmptySig(t, attData.Source.Epoch, attData.Target.Epoch, []uint64{uint64(validatorIdx)}, []byte{1}) err = slasherDB.SaveAttestationRecordsForValidators( ctx, []*slashertypes.IndexedAttestationWrapper{attRecord}, @@ -173,7 +173,7 @@ func TestMaxSpanChunksSlice_CheckSlashable(t *testing.T) { validatorIdx := primitives.ValidatorIndex(1) source := primitives.Epoch(1) target := primitives.Epoch(2) - att := createAttestationWrapper(t, source, target, nil, nil) + att := createAttestationWrapperEmptySig(t, source, target, nil, nil) // A faulty chunk should lead to error. chunk := &MaxSpanChunksSlice{ @@ -208,7 +208,7 @@ func TestMaxSpanChunksSlice_CheckSlashable(t *testing.T) { chunk = EmptyMaxSpanChunksSlice(params) source = primitives.Epoch(0) target = primitives.Epoch(3) - att = createAttestationWrapper(t, source, target, nil, nil) + att = createAttestationWrapperEmptySig(t, source, target, nil, nil) chunkIdx := uint64(0) startEpoch := source currentEpoch := target @@ -223,7 +223,7 @@ func TestMaxSpanChunksSlice_CheckSlashable(t *testing.T) { // because we DO NOT have an existing attestation record in our database at the max target epoch. source = primitives.Epoch(1) target = primitives.Epoch(2) - surroundedVote := createAttestationWrapper(t, source, target, nil, nil) + surroundedVote := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err = chunk.CheckSlashable(ctx, slasherDB, validatorIdx, surroundedVote) require.NoError(t, err) @@ -233,7 +233,7 @@ func TestMaxSpanChunksSlice_CheckSlashable(t *testing.T) { // surroundedVote vote is indeed slashable. attData := att.IndexedAttestation.Data signingRoot := [32]byte{1} - attRecord := createAttestationWrapper( + attRecord := createAttestationWrapperEmptySig( t, attData.Source.Epoch, attData.Target.Epoch, []uint64{uint64(validatorIdx)}, signingRoot[:], ) err = slasherDB.SaveAttestationRecordsForValidators( diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 67f16b820a52..107b4c7dccc6 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -38,8 +38,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Same target with different signing roots", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, []byte{1}), - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, []byte{2}), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{2}), }, currentEpoch: 4, }, @@ -49,8 +49,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Same target with same signing roots", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, []byte{1}), - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, []byte{1}), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), }, currentEpoch: 4, }, @@ -60,8 +60,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Detects surrounding vote (source 1, target 2), (source 0, target 3)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -71,8 +71,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 50, 51, []uint64{0}, nil), - createAttestationWrapper(t, 0, 1000, []uint64{0}, nil), + createAttestationWrapperEmptySig(t, 50, 51, []uint64{0}, nil), + createAttestationWrapperEmptySig(t, 0, 1000, []uint64{0}, nil), }, currentEpoch: 1000, }, @@ -82,8 +82,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Detects surrounded vote (source 0, target 3), (source 1, target 2)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -93,8 +93,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Detects double vote, (source 1, target 2), (source 0, target 2)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -104,8 +104,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0}, nil), - createAttestationWrapper(t, 0, 3, []uint64{1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{1}, nil), }, currentEpoch: 4, }, @@ -115,8 +115,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapper(t, 1, 2, []uint64{2, 3}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{2, 3}, nil), }, currentEpoch: 4, }, @@ -126,8 +126,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), - createAttestationWrapper(t, 0, 3, []uint64{0}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0}, nil), }, currentEpoch: 4, }, @@ -137,8 +137,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 3, []uint64{0}, nil), - createAttestationWrapper(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), }, currentEpoch: 4, }, @@ -148,8 +148,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, (source 1, target 2), (source 2, target 3)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapper(t, 2, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 2, 3, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -159,8 +159,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, (source 0, target 3), (source 2, target 4)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapper(t, 2, 4, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 2, 4, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -170,8 +170,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, (source 0, target 2), (source 0, target 3)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 2, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -181,8 +181,8 @@ func Test_processQueuedAttestations(t *testing.T) { name: "Not slashable, (source 0, target 3), (source 0, target 2)", args: args{ attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 2, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), }, currentEpoch: 4, }, @@ -335,7 +335,7 @@ func Test_processQueuedAttestations_MultipleChunkIndices(t *testing.T) { } var sr [32]byte copy(sr[:], fmt.Sprintf("%d", i)) - att := createAttestationWrapper(t, source, target, []uint64{0}, sr[:]) + att := createAttestationWrapperEmptySig(t, source, target, []uint64{0}, sr[:]) s.attsQueue = newAttestationsQueue() s.attsQueue.push(att) slot, err := slots.EpochStart(i) @@ -392,8 +392,8 @@ func Test_processQueuedAttestations_OverlappingChunkIndices(t *testing.T) { }() // We create two attestations fully spanning chunk indices 0 and chunk 1 - att1 := createAttestationWrapper(t, primitives.Epoch(slasherParams.chunkSize-2), primitives.Epoch(slasherParams.chunkSize), []uint64{0, 1}, nil) - att2 := createAttestationWrapper(t, primitives.Epoch(slasherParams.chunkSize-1), primitives.Epoch(slasherParams.chunkSize+1), []uint64{0, 1}, nil) + att1 := createAttestationWrapperEmptySig(t, primitives.Epoch(slasherParams.chunkSize-2), primitives.Epoch(slasherParams.chunkSize), []uint64{0, 1}, nil) + att2 := createAttestationWrapperEmptySig(t, primitives.Epoch(slasherParams.chunkSize-1), primitives.Epoch(slasherParams.chunkSize+1), []uint64{0, 1}, nil) // We attempt to process the batch. s.attsQueue = newAttestationsQueue() @@ -506,7 +506,7 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { // We apply attestation with (source 1, target 2) for our validator. source := primitives.Epoch(1) target := primitives.Epoch(2) - att := createAttestationWrapper(t, source, target, nil, nil) + att := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, chunksByChunkIdx, @@ -529,7 +529,7 @@ func Test_applyAttestationForValidator_MinSpanChunk(t *testing.T) { // expect a slashable offense to be returned. source = primitives.Epoch(0) target = primitives.Epoch(3) - slashableAtt := createAttestationWrapper(t, source, target, nil, nil) + slashableAtt := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, chunksByChunkIdx, @@ -563,7 +563,7 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { // We apply attestation with (source 0, target 3) for our validator. source := primitives.Epoch(0) target := primitives.Epoch(3) - att := createAttestationWrapper(t, source, target, nil, nil) + att := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err := srv.applyAttestationForValidator( ctx, chunksByChunkIdx, @@ -586,7 +586,7 @@ func Test_applyAttestationForValidator_MaxSpanChunk(t *testing.T) { // expect a slashable offense to be returned. source = primitives.Epoch(1) target = primitives.Epoch(2) - slashableAtt := createAttestationWrapper(t, source, target, nil, nil) + slashableAtt := createAttestationWrapperEmptySig(t, source, target, nil, nil) slashing, err = srv.applyAttestationForValidator( ctx, chunksByChunkIdx, @@ -607,8 +607,8 @@ func Test_checkDoubleVotes_SlashableAttestationsOnDisk(t *testing.T) { // indeed check there could exist a double vote offense // within the list with respect to previous entries in the db. prevAtts := []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{1, 2}, []byte{1}), - createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{1}), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1, 2}, []byte{1}), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{1}), } srv, err := New(context.Background(), &ServiceConfig{ @@ -621,10 +621,10 @@ func Test_checkDoubleVotes_SlashableAttestationsOnDisk(t *testing.T) { err = slasherDB.SaveAttestationRecordsForValidators(ctx, prevAtts) require.NoError(t, err) - prev1 := createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{1}) - cur1 := createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{2}) - prev2 := createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{1}) - cur2 := createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{2}) + prev1 := createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{1}) + cur1 := createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{2}) + prev2 := createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{1}) + cur2 := createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{2}) wanted := []*ethpb.AttesterSlashing{ { Attestation_1: prev1.IndexedAttestation, @@ -636,7 +636,7 @@ func Test_checkDoubleVotes_SlashableAttestationsOnDisk(t *testing.T) { }, } newAtts := []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 2, []uint64{1, 2}, []byte{2}), // Different signing root. + createAttestationWrapperEmptySig(t, 0, 2, []uint64{1, 2}, []byte{2}), // Different signing root. } slashings, err := srv.checkDoubleVotes(ctx, newAtts) require.NoError(t, err) @@ -744,7 +744,7 @@ func TestService_processQueuedAttestations(t *testing.T) { require.NoError(t, err) s.attsQueue.extend([]*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{0, 1} /* indices */, nil /* signingRoot */), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{0, 1} /* indices */, nil /* signingRoot */), }) ctx, cancel := context.WithCancel(context.Background()) tickerChan := make(chan primitives.Slot) @@ -830,7 +830,7 @@ func runAttestationsBenchmark(b *testing.B, s *Service, numAtts, numValidators u target := primitives.Epoch(i + 1) var signingRoot [32]byte copy(signingRoot[:], fmt.Sprintf("%d", i)) - atts[i] = createAttestationWrapper( + atts[i] = createAttestationWrapperEmptySig( b, source, target, /* target */ @@ -850,7 +850,11 @@ func runAttestationsBenchmark(b *testing.B, s *Service, numAtts, numValidators u } } -func createAttestationWrapper( +// createAttestationWrapperEmptySig creates an attestation wrapper with source and target, +// for validators with indices, and a beacon block root (corresponding to the head vote). +// For source and target epochs, the corresponding root is null. +// The signature of the attestation is empty. +func createAttestationWrapperEmptySig( t testing.TB, source, target primitives.Epoch, indices []uint64, diff --git a/beacon-chain/slasher/helpers_test.go b/beacon-chain/slasher/helpers_test.go index 97e26b084d04..e54306df5ae7 100644 --- a/beacon-chain/slasher/helpers_test.go +++ b/beacon-chain/slasher/helpers_test.go @@ -32,13 +32,13 @@ func TestService_groupByValidatorChunkIndex(t *testing.T) { validatorChunkSize: 2, }, atts: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 0, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 1}, nil), }, want: map[uint64][]*slashertypes.IndexedAttestationWrapper{ 0: { - createAttestationWrapper(t, 0, 0, []uint64{0, 1}, nil), - createAttestationWrapper(t, 0, 0, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 1}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 1}, nil), }, }, }, @@ -48,17 +48,17 @@ func TestService_groupByValidatorChunkIndex(t *testing.T) { validatorChunkSize: 2, }, atts: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, []uint64{0, 2, 4}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 2, 4}, nil), }, want: map[uint64][]*slashertypes.IndexedAttestationWrapper{ 0: { - createAttestationWrapper(t, 0, 0, []uint64{0, 2, 4}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 2, 4}, nil), }, 1: { - createAttestationWrapper(t, 0, 0, []uint64{0, 2, 4}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 2, 4}, nil), }, 2: { - createAttestationWrapper(t, 0, 0, []uint64{0, 2, 4}, nil), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0, 2, 4}, nil), }, }, }, @@ -95,13 +95,13 @@ func TestService_groupByChunkIndex(t *testing.T) { historyLength: 3, }, atts: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, nil, nil), - createAttestationWrapper(t, 1, 0, nil, nil), + createAttestationWrapperEmptySig(t, 0, 0, nil, nil), + createAttestationWrapperEmptySig(t, 1, 0, nil, nil), }, want: map[uint64][]*slashertypes.IndexedAttestationWrapper{ 0: { - createAttestationWrapper(t, 0, 0, nil, nil), - createAttestationWrapper(t, 1, 0, nil, nil), + createAttestationWrapperEmptySig(t, 0, 0, nil, nil), + createAttestationWrapperEmptySig(t, 1, 0, nil, nil), }, }, }, @@ -112,17 +112,17 @@ func TestService_groupByChunkIndex(t *testing.T) { historyLength: 3, }, atts: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, nil, nil), - createAttestationWrapper(t, 1, 0, nil, nil), - createAttestationWrapper(t, 2, 0, nil, nil), + createAttestationWrapperEmptySig(t, 0, 0, nil, nil), + createAttestationWrapperEmptySig(t, 1, 0, nil, nil), + createAttestationWrapperEmptySig(t, 2, 0, nil, nil), }, want: map[uint64][]*slashertypes.IndexedAttestationWrapper{ 0: { - createAttestationWrapper(t, 0, 0, nil, nil), - createAttestationWrapper(t, 1, 0, nil, nil), + createAttestationWrapperEmptySig(t, 0, 0, nil, nil), + createAttestationWrapperEmptySig(t, 1, 0, nil, nil), }, 1: { - createAttestationWrapper(t, 2, 0, nil, nil), + createAttestationWrapperEmptySig(t, 2, 0, nil, nil), }, }, }, @@ -207,7 +207,7 @@ func TestService_filterAttestations(t *testing.T) { { name: "Source > target gets dropped", input: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 1, 0, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 1, 0, []uint64{1}, make([]byte, 32)), }, inputEpoch: 0, wantedDropped: 1, @@ -215,33 +215,33 @@ func TestService_filterAttestations(t *testing.T) { { name: "Source < target is valid", input: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1}, make([]byte, 32)), }, inputEpoch: 1, wantedValid: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1}, make([]byte, 32)), }, wantedDropped: 0, }, { name: "Source == target is valid", input: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{1}, make([]byte, 32)), }, inputEpoch: 1, wantedValid: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{1}, make([]byte, 32)), }, wantedDropped: 0, }, { name: "Attestation from the future is deferred", input: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 2, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{1}, make([]byte, 32)), }, inputEpoch: 1, wantedDeferred: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 2, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{1}, make([]byte, 32)), }, wantedDropped: 0, }, @@ -271,22 +271,22 @@ func Test_logSlashingEvent(t *testing.T) { { name: "Surrounding vote", slashing: ðpb.AttesterSlashing{ - Attestation_1: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, - Attestation_2: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_1: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_2: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, }, }, { name: "Surrounded vote", slashing: ðpb.AttesterSlashing{ - Attestation_1: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, - Attestation_2: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_1: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_2: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, }, }, { name: "Double vote", slashing: ðpb.AttesterSlashing{ - Attestation_1: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, - Attestation_2: createAttestationWrapper(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_1: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, + Attestation_2: createAttestationWrapperEmptySig(t, 0, 0, nil, nil).IndexedAttestation, }, }, } diff --git a/beacon-chain/slasher/queue_test.go b/beacon-chain/slasher/queue_test.go index 98b9b4eb2f11..4e82f2f81611 100644 --- a/beacon-chain/slasher/queue_test.go +++ b/beacon-chain/slasher/queue_test.go @@ -12,8 +12,8 @@ func Test_attestationsQueue(t *testing.T) { t.Run("push_and_dequeue", func(tt *testing.T) { attQueue := newAttestationsQueue() wantedAtts := []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{1}, make([]byte, 32)), - createAttestationWrapper(t, 1, 2, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{1}, make([]byte, 32)), } attQueue.push(wantedAtts[0]) attQueue.push(wantedAtts[1]) @@ -27,8 +27,8 @@ func Test_attestationsQueue(t *testing.T) { t.Run("extend_and_dequeue", func(tt *testing.T) { attQueue := newAttestationsQueue() wantedAtts := []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 1, []uint64{1}, make([]byte, 32)), - createAttestationWrapper(t, 1, 2, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1}, make([]byte, 32)), + createAttestationWrapperEmptySig(t, 1, 2, []uint64{1}, make([]byte, 32)), } attQueue.extend(wantedAtts) require.DeepEqual(t, 2, attQueue.size()) diff --git a/beacon-chain/slasher/receive_test.go b/beacon-chain/slasher/receive_test.go index 49e65ccf42df..49017878e735 100644 --- a/beacon-chain/slasher/receive_test.go +++ b/beacon-chain/slasher/receive_test.go @@ -38,8 +38,8 @@ func TestSlasher_receiveAttestations_OK(t *testing.T) { }() firstIndices := []uint64{1, 2, 3} secondIndices := []uint64{4, 5, 6} - att1 := createAttestationWrapper(t, 1, 2, firstIndices, nil) - att2 := createAttestationWrapper(t, 1, 2, secondIndices, nil) + att1 := createAttestationWrapperEmptySig(t, 1, 2, firstIndices, nil) + att2 := createAttestationWrapperEmptySig(t, 1, 2, secondIndices, nil) indexedAttsChan <- att1.IndexedAttestation indexedAttsChan <- att2.IndexedAttestation cancel() @@ -65,14 +65,14 @@ func TestService_pruneSlasherDataWithinSlidingWindow_AttestationsPruned(t *testi // Setup attestations for 2 validators at each epoch for epochs 0, 1, 2, 3. err := slasherDB.SaveAttestationRecordsForValidators(ctx, []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 0, []uint64{0}, bytesutil.PadTo([]byte("0a"), 32)), - createAttestationWrapper(t, 0, 0, []uint64{1}, bytesutil.PadTo([]byte("0b"), 32)), - createAttestationWrapper(t, 0, 1, []uint64{0}, bytesutil.PadTo([]byte("1a"), 32)), - createAttestationWrapper(t, 0, 1, []uint64{1}, bytesutil.PadTo([]byte("1b"), 32)), - createAttestationWrapper(t, 0, 2, []uint64{0}, bytesutil.PadTo([]byte("2a"), 32)), - createAttestationWrapper(t, 0, 2, []uint64{1}, bytesutil.PadTo([]byte("2b"), 32)), - createAttestationWrapper(t, 0, 3, []uint64{0}, bytesutil.PadTo([]byte("3a"), 32)), - createAttestationWrapper(t, 0, 3, []uint64{1}, bytesutil.PadTo([]byte("3b"), 32)), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{0}, bytesutil.PadTo([]byte("0a"), 32)), + createAttestationWrapperEmptySig(t, 0, 0, []uint64{1}, bytesutil.PadTo([]byte("0b"), 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{0}, bytesutil.PadTo([]byte("1a"), 32)), + createAttestationWrapperEmptySig(t, 0, 1, []uint64{1}, bytesutil.PadTo([]byte("1b"), 32)), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{0}, bytesutil.PadTo([]byte("2a"), 32)), + createAttestationWrapperEmptySig(t, 0, 2, []uint64{1}, bytesutil.PadTo([]byte("2b"), 32)), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{0}, bytesutil.PadTo([]byte("3a"), 32)), + createAttestationWrapperEmptySig(t, 0, 3, []uint64{1}, bytesutil.PadTo([]byte("3b"), 32)), }) require.NoError(t, err) @@ -93,8 +93,8 @@ func TestService_pruneSlasherDataWithinSlidingWindow_AttestationsPruned(t *testi // Setup attestations for 2 validators at epoch 4. err = slasherDB.SaveAttestationRecordsForValidators(ctx, []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapper(t, 0, 4, []uint64{0}, bytesutil.PadTo([]byte("4a"), 32)), - createAttestationWrapper(t, 0, 4, []uint64{1}, bytesutil.PadTo([]byte("4b"), 32)), + createAttestationWrapperEmptySig(t, 0, 4, []uint64{0}, bytesutil.PadTo([]byte("4a"), 32)), + createAttestationWrapperEmptySig(t, 0, 4, []uint64{1}, bytesutil.PadTo([]byte("4b"), 32)), }) require.NoError(t, err) @@ -222,7 +222,7 @@ func TestSlasher_receiveAttestations_OnlyValidAttestations(t *testing.T) { firstIndices := []uint64{1, 2, 3} secondIndices := []uint64{4, 5, 6} // Add a valid attestation. - validAtt := createAttestationWrapper(t, 1, 2, firstIndices, nil) + validAtt := createAttestationWrapperEmptySig(t, 1, 2, firstIndices, nil) indexedAttsChan <- validAtt.IndexedAttestation // Send an invalid, bad attestation which will not // pass integrity checks at it has invalid attestation data. From 4561cd50c7bc42a6f6ea452caf0ca4aa73243db3 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 14:48:56 +0100 Subject: [PATCH 31/38] Implement `createAttestationWrapper`. --- .../slasher/detect_attestations_test.go | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 107b4c7dccc6..18d0e99f9741 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -884,3 +884,66 @@ func createAttestationWrapperEmptySig( DataRoot: dataRoot, } } + +// createAttestationWrapper creates an attestation wrapper with source and target, +// for validators with indices, and a beacon block root (corresponding to the head vote). +// For source and target epochs, the corresponding root is null. +// if validatorIndice = indices[i], then the corresponding private key is privateKeys[validatorIndice]. +func createAttestationWrapper( + t testing.TB, + domain []byte, + privateKeys []common.SecretKey, + source, target primitives.Epoch, + indices []uint64, + beaconBlockRoot []byte, +) *slashertypes.IndexedAttestationWrapper { + // Create attestation data. + attestationData := ðpb.AttestationData{ + BeaconBlockRoot: bytesutil.PadTo(beaconBlockRoot, 32), + Source: ðpb.Checkpoint{ + Epoch: source, + Root: params.BeaconConfig().ZeroHash[:], + }, + Target: ðpb.Checkpoint{ + Epoch: target, + Root: params.BeaconConfig().ZeroHash[:], + }, + } + + // Compute attestation data root. + attestationDataRoot, err := attestationData.HashTreeRoot() + require.NoError(t, err) + + // Create valid signatures for all input attestations in the test. + signingRoot, err := signing.ComputeSigningRoot(attestationData, domain) + require.NoError(t, err) + + // For each attesting indice in the indexed attestation, create a signature. + signatures := make([]bls.Signature, 0, len(indices)) + for _, indice := range indices { + // Check that the indice is within the range of private keys. + require.Equal(t, true, indice < uint64(len(privateKeys))) + + // Retrieve the corresponding private key. + privateKey := privateKeys[indice] + + // Sign the signing root. + signature := privateKey.Sign(signingRoot[:]) + + // Append the signature to the signatures list. + signatures = append(signatures, signature) + } + + // Compute the aggregated signature. + signature := bls.AggregateSignatures(signatures).Marshal() + + // Create the attestation wrapper. + return &slashertypes.IndexedAttestationWrapper{ + IndexedAttestation: ðpb.IndexedAttestation{ + AttestingIndices: indices, + Data: attestationData, + Signature: signature, + }, + DataRoot: attestationDataRoot, + } +} From 9bee736b54e56da3b5bf12307b0695cce6706491 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 16:07:51 +0100 Subject: [PATCH 32/38] `processAttestations`: Return processed attester slashings. --- beacon-chain/slasher/receive.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/beacon-chain/slasher/receive.go b/beacon-chain/slasher/receive.go index 8df5af3f53af..766cc5fb90bd 100644 --- a/beacon-chain/slasher/receive.go +++ b/beacon-chain/slasher/receive.go @@ -107,7 +107,7 @@ func (s *Service) processAttestations( ctx context.Context, attestations []*slashertypes.IndexedAttestationWrapper, currentSlot primitives.Slot, -) { +) []*ethpb.AttesterSlashing { // Get the current epoch from the current slot. currentEpoch := slots.ToEpoch(currentSlot) @@ -143,22 +143,25 @@ func (s *Service) processAttestations( // then the first (validator index + target epoch) => signing root) link is kept into the database. if err := s.serviceCfg.Database.SaveAttestationRecordsForValidators(ctx, validAttestations); err != nil { log.WithError(err).Error(couldNotSaveAttRecord) - return + return nil } // Check for attestatinos slashings (double, sourrounding, surrounded votes). slashings, err := s.checkSlashableAttestations(ctx, currentEpoch, validAttestations) if err != nil { log.WithError(err).Error(couldNotCheckSlashableAtt) - return + return nil } // Process attester slashings by verifying their signatures, submitting // to the beacon node's operations pool, and logging them. - if _, err := s.processAttesterSlashings(ctx, slashings); err != nil { + processedAttesterSlashings, err := s.processAttesterSlashings(ctx, slashings) + if err != nil { log.WithError(err).Error(couldNotProcessAttesterSlashings) - return + return nil } + + return processedAttesterSlashings } // Process queued blocks every time an epoch ticker fires. We retrieve From cb51e7442d0af8859a4e00905368c4bd1258f834 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 18:02:04 +0100 Subject: [PATCH 33/38] Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. --- beacon-chain/slasher/BUILD.bazel | 2 +- .../slasher/detect_attestations_test.go | 446 +++++++++++------- 2 files changed, 273 insertions(+), 175 deletions(-) diff --git a/beacon-chain/slasher/BUILD.bazel b/beacon-chain/slasher/BUILD.bazel index 0c9cbd196ca9..e87f6519a323 100644 --- a/beacon-chain/slasher/BUILD.bazel +++ b/beacon-chain/slasher/BUILD.bazel @@ -30,7 +30,6 @@ go_library( "//beacon-chain/operations/slashings:go_default_library", "//beacon-chain/slasher/types:go_default_library", "//beacon-chain/startup:go_default_library", - "//beacon-chain/state:go_default_library", "//beacon-chain/state/stategen:go_default_library", "//beacon-chain/sync:go_default_library", "//config/fieldparams:go_default_library", @@ -78,6 +77,7 @@ go_test( "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//crypto/bls:go_default_library", + "//crypto/bls/common:go_default_library", "//encoding/bytesutil:go_default_library", "//proto/prysm/v1alpha1:go_default_library", "//testing/assert:go_default_library", diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 18d0e99f9741..03781171328b 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -15,6 +15,7 @@ import ( "github.com/prysmaticlabs/prysm/v4/config/params" "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v4/crypto/bls" + "github.com/prysmaticlabs/prysm/v4/crypto/bls/common" "github.com/prysmaticlabs/prysm/v4/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v4/testing/assert" @@ -24,208 +25,291 @@ import ( logTest "github.com/sirupsen/logrus/hooks/test" ) -func Test_processQueuedAttestations(t *testing.T) { - type args struct { - attestationQueue []*slashertypes.IndexedAttestationWrapper - currentEpoch primitives.Epoch - } +func Test_processAttestations(t *testing.T) { + type ( + attestationInfo struct { + source primitives.Epoch + target primitives.Epoch + indices []uint64 + beaconBlockRoot []byte + } + + slashingInfo struct { + attestationInfo_1 *attestationInfo + attestationInfo_2 *attestationInfo + } + ) + tests := []struct { - name string - args args - shouldBeSlashable bool + name string + currentEpoch primitives.Epoch + attestationsInfo []*attestationInfo + expectedSlashingsInfo []*slashingInfo }{ { - name: "Same target with different signing roots", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{2}), + name: "Same target with different signing roots", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + }, + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, }, - currentEpoch: 4, }, - shouldBeSlashable: true, }, { - name: "Same target with same signing roots", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, []byte{1}), - }, - currentEpoch: 4, + name: "Same target with same signing roots", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Detects surrounding vote (source 1, target 2), (source 0, target 3)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), + name: "Detects surrounding vote (source 1, target 2), (source 0, target 3)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 3, 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}, + }, + { + 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}, + }, + { + 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}, + }, + { + 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}, }, - currentEpoch: 4, }, - shouldBeSlashable: true, }, { - name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 50, 51, []uint64{0}, nil), - createAttestationWrapperEmptySig(t, 0, 1000, []uint64{0}, nil), + name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000)", + currentEpoch: 1000, + attestationsInfo: []*attestationInfo{ + {source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + { + attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, }, - currentEpoch: 1000, }, - shouldBeSlashable: true, }, { - name: "Detects surrounded vote (source 0, target 3), (source 1, target 2)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), + name: "Detects surrounded vote (source 0, target 3), (source 1, target 2)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {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}, + }, + { + 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}, }, - currentEpoch: 4, }, - shouldBeSlashable: true, }, { - name: "Detects double vote, (source 1, target 2), (source 0, target 2)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), + name: "Detects surrounded vote (source 0, target 3), (source 1, target 2)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {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}, + }, + { + 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}, }, - currentEpoch: 4, }, - shouldBeSlashable: true, }, { - name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0}, nil), - createAttestationWrapperEmptySig(t, 0, 3, []uint64{1}, nil), + name: "Detects double vote, (source 1, target 2), (source 0, target 2)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, 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: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, }, - currentEpoch: 4, }, - shouldBeSlashable: false, }, { - name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 1, 2, []uint64{2, 3}, nil), - }, - currentEpoch: 4, + name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 0, target: 3, indices: []uint64{1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0}, nil), - }, - currentEpoch: 4, + name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{2, 3}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0}, nil), - createAttestationWrapperEmptySig(t, 1, 2, []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, nil), - }, - currentEpoch: 4, + name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 1, target 2), (source 2, target 3)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 1, 2, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 2, 3, []uint64{0, 1}, nil), - }, - currentEpoch: 4, + name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 3), (source 2, target 4)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 2, 4, []uint64{0, 1}, nil), - }, - currentEpoch: 4, + name: "Not slashable, (source 1, target 2), (source 2, target 3)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 2, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 2), (source 0, target 3)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), - }, - currentEpoch: 4, + name: "Not slashable, (source 0, target 3), (source 2, target 4)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 2, target: 4, indices: []uint64{0, 1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 3), (source 0, target 2)", - args: args{ - attestationQueue: []*slashertypes.IndexedAttestationWrapper{ - createAttestationWrapperEmptySig(t, 0, 3, []uint64{0, 1}, nil), - createAttestationWrapperEmptySig(t, 0, 2, []uint64{0, 1}, nil), - }, - currentEpoch: 4, + name: "Not slashable, (source 0, target 2), (source 0, target 3)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + name: "Not slashable, (source 0, target 3), (source 0, target 2)", + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, }, - shouldBeSlashable: false, + expectedSlashingsInfo: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Create context. + ctx := context.Background() + + // Configure logging. hook := logTest.NewGlobal() defer hook.Reset() - slasherDB := dbtest.SetupSlasherDB(t) - ctx, cancel := context.WithCancel(context.Background()) - currentTime := time.Now() - totalSlots := uint64(tt.args.currentEpoch) * uint64(params.BeaconConfig().SlotsPerEpoch) - secondsSinceGenesis := time.Duration(totalSlots * params.BeaconConfig().SecondsPerSlot) - genesisTime := currentTime.Add(-secondsSinceGenesis * time.Second) + // Configure the slasher database. + slasherDB := dbtest.SetupSlasherDB(t) + // Configure the beacon state. beaconState, err := util.NewBeaconState() require.NoError(t, err) - slot, err := slots.EpochStart(tt.args.currentEpoch) + + // Get the currentSlot for the current epoch. + currentSlot, err := slots.EpochStart(tt.currentEpoch) require.NoError(t, err) - require.NoError(t, beaconState.SetSlot(slot)) - mockChain := &mock.ChainService{ - State: beaconState, - Slot: &slot, + + // Create the mock chain service. + mockChain := &mock.ChainService{State: beaconState} + + // Create the mock slashing pool inserter. + mockSlashingPoolInserter := &slashingsmock.PoolMock{} + + // Create the service configuration. + serviceConfig := &ServiceConfig{ + Database: slasherDB, + HeadStateFetcher: mockChain, + AttestationStateFetcher: mockChain, + SlashingPoolInserter: mockSlashingPoolInserter, } + // Create the slasher service. + slasherService, err := New(context.Background(), serviceConfig) + require.NoError(t, err) + // Initialize validators in the state. numVals := params.BeaconConfig().MinGenesisActiveValidatorCount validators := make([]*ethpb.Validator, numVals) - privKeys := make([]bls.SecretKey, numVals) - for i := range validators { - privKey, err := bls.RandKey() + privateKeys := make([]bls.SecretKey, numVals) + + for i := uint64(0); i < numVals; i++ { + // Create a random private key. + privateKey, err := bls.RandKey() require.NoError(t, err) - privKeys[i] = privKey - validators[i] = ðpb.Validator{ - PublicKey: privKey.PublicKey().Marshal(), - WithdrawalCredentials: make([]byte, 32), - } + + // Add the private key to the list. + privateKeys[i] = privateKey + + // Derive the public key from the private key. + publicKey := privateKey.PublicKey().Marshal() + + // Initialize the validator. + validator := ðpb.Validator{PublicKey: publicKey} + + // Add the validator to the list. + validators[i] = validator } + + // Set the validators into the state. err = beaconState.SetValidators(validators) require.NoError(t, err) + + // Compute the signing domain. domain, err := signing.Domain( beaconState.Fork(), 0, @@ -234,50 +318,64 @@ func Test_processQueuedAttestations(t *testing.T) { ) require.NoError(t, err) - // Create valid signatures for all input attestations in the test. - for _, attestationWrapper := range tt.args.attestationQueue { - signingRoot, err := signing.ComputeSigningRoot(attestationWrapper.IndexedAttestation.Data, domain) - require.NoError(t, err) - attestingIndices := attestationWrapper.IndexedAttestation.AttestingIndices - sigs := make([]bls.Signature, len(attestingIndices)) - for i, validatorIndex := range attestingIndices { - privKey := privKeys[validatorIndex] - sigs[i] = privKey.Sign(signingRoot[:]) - } - attestationWrapper.IndexedAttestation.Signature = bls.AggregateSignatures(sigs).Marshal() + // Build attestation wrappers. + attestationsCount := len(tt.attestationsInfo) + attestationWrappers := make([]*slashertypes.IndexedAttestationWrapper, 0, attestationsCount) + for _, attestationInfo := range tt.attestationsInfo { + // Create a wrapped attestation. + attestationWrapper := createAttestationWrapper( + t, + domain, + privateKeys, + attestationInfo.source, + attestationInfo.target, + attestationInfo.indices, + attestationInfo.beaconBlockRoot, + ) + + // Add the wrapped attestation to the list. + attestationWrappers = append(attestationWrappers, attestationWrapper) } - s, err := New(context.Background(), - &ServiceConfig{ - Database: slasherDB, - StateNotifier: &mock.MockStateNotifier{}, - HeadStateFetcher: mockChain, - AttestationStateFetcher: mockChain, - SlashingPoolInserter: &slashingsmock.PoolMock{}, - ClockWaiter: startup.NewClockSynchronizer(), - }) - require.NoError(t, err) - s.genesisTime = genesisTime - - currentSlotChan := make(chan primitives.Slot) - s.wg.Add(1) - go func() { - s.processQueuedAttestations(ctx, currentSlotChan) - }() - s.attsQueue.extend(tt.args.attestationQueue) - currentSlotChan <- slot - time.Sleep(time.Millisecond * 200) - cancel() - s.wg.Wait() - if tt.shouldBeSlashable { - require.LogsContain(t, hook, "Attester slashing detected") - } else { - require.LogsDoNotContain(t, hook, "Attester slashing detected") + // Build expected attester slashings. + expectedSlashings := make([]*ethpb.AttesterSlashing, 0, len(tt.expectedSlashingsInfo)) + for _, slashingInfo := range tt.expectedSlashingsInfo { + // Create attestations. + attestation_1 := createAttestationWrapper( + t, + domain, + privateKeys, + slashingInfo.attestationInfo_1.source, + slashingInfo.attestationInfo_1.target, + slashingInfo.attestationInfo_1.indices, + slashingInfo.attestationInfo_1.beaconBlockRoot, + ) + + attestation_2 := createAttestationWrapper( + t, + domain, + privateKeys, + slashingInfo.attestationInfo_2.source, + slashingInfo.attestationInfo_2.target, + slashingInfo.attestationInfo_2.indices, + slashingInfo.attestationInfo_2.beaconBlockRoot, + ) + + // Create the attester slashing. + attesterSlashing := ðpb.AttesterSlashing{ + Attestation_1: attestation_1.IndexedAttestation, + Attestation_2: attestation_2.IndexedAttestation, + } + + // Add the attester slashing to the list. + expectedSlashings = append(expectedSlashings, attesterSlashing) } - require.LogsDoNotContain(t, hook, couldNotSaveAttRecord) - require.LogsDoNotContain(t, hook, couldNotCheckSlashableAtt) - require.LogsDoNotContain(t, hook, couldNotProcessAttesterSlashings) + // Process the attestations. + processedSlashings := slasherService.processAttestations(ctx, attestationWrappers, currentSlot) + + // Check the processed slashings correspond to the expected slashings. + assert.DeepSSZEqual(t, expectedSlashings, processedSlashings) }) } } @@ -853,7 +951,7 @@ func runAttestationsBenchmark(b *testing.B, s *Service, numAtts, numValidators u // createAttestationWrapperEmptySig creates an attestation wrapper with source and target, // for validators with indices, and a beacon block root (corresponding to the head vote). // For source and target epochs, the corresponding root is null. -// The signature of the attestation is empty. +// The signature of the returned wrapped attestation is empty. func createAttestationWrapperEmptySig( t testing.TB, source, target primitives.Epoch, From a9450042739f4221206a1c894dd07866e5c85f26 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 6 Feb 2024 21:30:29 +0100 Subject: [PATCH 34/38] `Test_processAttestations`: Allow multiple steps. --- .../slasher/detect_attestations_test.go | 469 ++++++++++-------- 1 file changed, 268 insertions(+), 201 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 03781171328b..9df342278853 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -38,210 +38,275 @@ func Test_processAttestations(t *testing.T) { attestationInfo_1 *attestationInfo attestationInfo_2 *attestationInfo } + + step struct { + currentEpoch primitives.Epoch + attestationsInfo []*attestationInfo + expectedSlashingsInfo []*slashingInfo + } ) tests := []struct { - name string - currentEpoch primitives.Epoch - attestationsInfo []*attestationInfo - expectedSlashingsInfo []*slashingInfo + name string + steps []*step }{ { - name: "Same target with different signing roots", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, - }, - expectedSlashingsInfo: []*slashingInfo{ + name: "Same target with different signing roots - single step", + steps: []*step{ { - attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, - attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, - }, - { - attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, - attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + }, + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + }, + }, }, }, }, { - name: "Same target with same signing roots", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + name: "Same target with same signing roots - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Detects surrounding vote (source 1, target 2), (source 0, target 3)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 0, target: 3, 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 surrounding vote (source 1, target 2), (source 0, target 3) - single step", + steps: []*step{ { - 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}, - }, - { - 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}, - }, - { - 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}, + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 3, 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}, + }, + { + 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}, + }, + { + 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}, + }, + { + 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 surrounding vote (source 50, target 51), (source 0, target 1000)", - currentEpoch: 1000, - attestationsInfo: []*attestationInfo{ - {source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, - {source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, - }, - expectedSlashingsInfo: []*slashingInfo{ - { - attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, - attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, - }, + name: "Detects surrounding vote (source 50, target 51), (source 0, target 1000) - single step", + steps: []*step{ { - attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, - attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + + currentEpoch: 1000, + attestationsInfo: []*attestationInfo{ + {source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + { + attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + }, }, }, }, { - name: "Detects surrounded vote (source 0, target 3), (source 1, target 2)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - }, - expectedSlashingsInfo: []*slashingInfo{ + name: "Detects surrounded vote (source 0, target 3), (source 1, target 2) - single step", + steps: []*step{ { - 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}, - }, - { - 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}, + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {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}, + }, + { + 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)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - }, - expectedSlashingsInfo: []*slashingInfo{ + name: "Detects surrounded vote (source 0, target 3), (source 1, target 2) - single step", + steps: []*step{ { - 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}, - }, - { - 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}, + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {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}, + }, + { + 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 double vote, (source 1, target 2), (source 0, target 2)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - }, - expectedSlashingsInfo: []*slashingInfo{ + name: "Detects double vote, (source 1, target 2), (source 0, target 2) - single step", + steps: []*step{ { - attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - attestationInfo_2: &attestationInfo{source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - }, - { - attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - attestationInfo_2: &attestationInfo{source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, 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: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + { + attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + }, }, }, }, { - name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0}, beaconBlockRoot: nil}, - {source: 0, target: 3, indices: []uint64{1}, beaconBlockRoot: nil}, + name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 0, target: 3, indices: []uint64{1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 1, target: 2, indices: []uint64{2, 3}, beaconBlockRoot: nil}, + name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{2, 3}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, - {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, - {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 1, target 2), (source 2, target 3)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 2, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + name: "Not slashable, (source 1, target 2), (source 2, target 3) - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 2, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 3), (source 2, target 4)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 2, target: 4, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + name: "Not slashable, (source 0, target 3), (source 2, target 4) - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 2, target: 4, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 2), (source 0, target 3)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + name: "Not slashable, (source 0, target 2), (source 0, target 3) - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, { - name: "Not slashable, (source 0, target 3), (source 0, target 2)", - currentEpoch: 4, - attestationsInfo: []*attestationInfo{ - {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, - {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + name: "Not slashable, (source 0, target 3), (source 0, target 2) - single step", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, }, - expectedSlashingsInfo: nil, }, } for _, tt := range tests { @@ -260,10 +325,6 @@ func Test_processAttestations(t *testing.T) { beaconState, err := util.NewBeaconState() require.NoError(t, err) - // Get the currentSlot for the current epoch. - currentSlot, err := slots.EpochStart(tt.currentEpoch) - require.NoError(t, err) - // Create the mock chain service. mockChain := &mock.ChainService{State: beaconState} @@ -318,64 +379,70 @@ func Test_processAttestations(t *testing.T) { ) require.NoError(t, err) - // Build attestation wrappers. - attestationsCount := len(tt.attestationsInfo) - attestationWrappers := make([]*slashertypes.IndexedAttestationWrapper, 0, attestationsCount) - for _, attestationInfo := range tt.attestationsInfo { - // Create a wrapped attestation. - attestationWrapper := createAttestationWrapper( - t, - domain, - privateKeys, - attestationInfo.source, - attestationInfo.target, - attestationInfo.indices, - attestationInfo.beaconBlockRoot, - ) - - // Add the wrapped attestation to the list. - attestationWrappers = append(attestationWrappers, attestationWrapper) - } + for _, step := range tt.steps { + // Build attestation wrappers. + attestationsCount := len(step.attestationsInfo) + attestationWrappers := make([]*slashertypes.IndexedAttestationWrapper, 0, attestationsCount) + for _, attestationInfo := range step.attestationsInfo { + // Create a wrapped attestation. + attestationWrapper := createAttestationWrapper( + t, + domain, + privateKeys, + attestationInfo.source, + attestationInfo.target, + attestationInfo.indices, + attestationInfo.beaconBlockRoot, + ) + + // Add the wrapped attestation to the list. + attestationWrappers = append(attestationWrappers, attestationWrapper) + } - // Build expected attester slashings. - expectedSlashings := make([]*ethpb.AttesterSlashing, 0, len(tt.expectedSlashingsInfo)) - for _, slashingInfo := range tt.expectedSlashingsInfo { - // Create attestations. - attestation_1 := createAttestationWrapper( - t, - domain, - privateKeys, - slashingInfo.attestationInfo_1.source, - slashingInfo.attestationInfo_1.target, - slashingInfo.attestationInfo_1.indices, - slashingInfo.attestationInfo_1.beaconBlockRoot, - ) - - attestation_2 := createAttestationWrapper( - t, - domain, - privateKeys, - slashingInfo.attestationInfo_2.source, - slashingInfo.attestationInfo_2.target, - slashingInfo.attestationInfo_2.indices, - slashingInfo.attestationInfo_2.beaconBlockRoot, - ) - - // Create the attester slashing. - attesterSlashing := ðpb.AttesterSlashing{ - Attestation_1: attestation_1.IndexedAttestation, - Attestation_2: attestation_2.IndexedAttestation, + // Build expected attester slashings. + expectedSlashings := make([]*ethpb.AttesterSlashing, 0, len(step.expectedSlashingsInfo)) + for _, slashingInfo := range step.expectedSlashingsInfo { + // Create attestations. + attestation_1 := createAttestationWrapper( + t, + domain, + privateKeys, + slashingInfo.attestationInfo_1.source, + slashingInfo.attestationInfo_1.target, + slashingInfo.attestationInfo_1.indices, + slashingInfo.attestationInfo_1.beaconBlockRoot, + ) + + attestation_2 := createAttestationWrapper( + t, + domain, + privateKeys, + slashingInfo.attestationInfo_2.source, + slashingInfo.attestationInfo_2.target, + slashingInfo.attestationInfo_2.indices, + slashingInfo.attestationInfo_2.beaconBlockRoot, + ) + + // Create the attester slashing. + attesterSlashing := ðpb.AttesterSlashing{ + Attestation_1: attestation_1.IndexedAttestation, + Attestation_2: attestation_2.IndexedAttestation, + } + + // Add the attester slashing to the list. + expectedSlashings = append(expectedSlashings, attesterSlashing) } - // Add the attester slashing to the list. - expectedSlashings = append(expectedSlashings, attesterSlashing) - } + // Get the currentSlot for the current epoch. + currentSlot, err := slots.EpochStart(step.currentEpoch) + require.NoError(t, err) - // Process the attestations. - processedSlashings := slasherService.processAttestations(ctx, attestationWrappers, currentSlot) + // Process the attestations. + processedSlashings := slasherService.processAttestations(ctx, attestationWrappers, currentSlot) - // Check the processed slashings correspond to the expected slashings. - assert.DeepSSZEqual(t, expectedSlashings, processedSlashings) + // Check the processed slashings correspond to the expected slashings. + assert.DeepSSZEqual(t, expectedSlashings, processedSlashings) + } }) } } From 6ea4e41f3513bab2d817c0ce65e5a1ad749deac9 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Wed, 7 Feb 2024 14:55:13 +0100 Subject: [PATCH 35/38] `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. --- .../slasher/detect_attestations_test.go | 348 ++++++++++++++++++ 1 file changed, 348 insertions(+) diff --git a/beacon-chain/slasher/detect_attestations_test.go b/beacon-chain/slasher/detect_attestations_test.go index 9df342278853..e22ae47a06e9 100644 --- a/beacon-chain/slasher/detect_attestations_test.go +++ b/beacon-chain/slasher/detect_attestations_test.go @@ -72,6 +72,35 @@ func Test_processAttestations(t *testing.T) { }, }, }, + // Uncomment when https://github.com/prysmaticlabs/prysm/issues/13590 is fixed + // { + // name: "Same target with different signing roots - two steps", + // steps: []*step{ + // { + // currentEpoch: 4, + // attestationsInfo: []*attestationInfo{ + // {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + // }, + // expectedSlashingsInfo: nil, + // }, + // { + // currentEpoch: 4, + // attestationsInfo: []*attestationInfo{ + // {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + // }, + // expectedSlashingsInfo: []*slashingInfo{ + // { + // attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + // attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + // }, + // { + // attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + // attestationInfo_2: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{2}}, + // }, + // }, + // }, + // }, + // }, { name: "Same target with same signing roots - single step", steps: []*step{ @@ -85,6 +114,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Same target with same signing roots - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: []byte{1}}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Detects surrounding vote (source 1, target 2), (source 0, target 3) - single step", steps: []*step{ @@ -115,6 +163,42 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Detects surrounding vote (source 1, target 2), (source 0, target 3) - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, 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}, + }, + { + 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}, + }, + { + 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}, + }, + { + 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 surrounding vote (source 50, target 51), (source 0, target 1000) - single step", steps: []*step{ @@ -138,6 +222,31 @@ 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}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 1000, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: []*slashingInfo{ + { + attestationInfo_1: &attestationInfo{source: 0, target: 1000, indices: []uint64{0}, beaconBlockRoot: nil}, + attestationInfo_2: &attestationInfo{source: 50, target: 51, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + }, + }, + }, + }, { name: "Detects surrounded vote (source 0, target 3), (source 1, target 2) - single step", steps: []*step{ @@ -160,6 +269,35 @@ 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}, + // }, + // { + // 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) - single step", steps: []*step{ @@ -182,6 +320,35 @@ 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}, + // }, + // { + // 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 double vote, (source 1, target 2), (source 0, target 2) - single step", steps: []*step{ @@ -204,6 +371,35 @@ func Test_processAttestations(t *testing.T) { }, }, }, + // Uncomment when https://github.com/prysmaticlabs/prysm/issues/13590 is fixed + // { + // name: "Detects double vote, (source 1, target 2), (source 0, target 2) - two steps", + // steps: []*step{ + // { + // currentEpoch: 4, + // attestationsInfo: []*attestationInfo{ + // {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + // }, + // expectedSlashingsInfo: nil, + // }, + // { + // currentEpoch: 4, + // attestationsInfo: []*attestationInfo{ + // {source: 0, 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: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + // }, + // { + // attestationInfo_1: &attestationInfo{source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + // attestationInfo_2: &attestationInfo{source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + // }, + // }, + // }, + // }, + // }, { name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index - single step", steps: []*step{ @@ -217,6 +413,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, surrounding but non-overlapping attesting indices within same validator chunk index - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index - single step", steps: []*step{ @@ -230,6 +445,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, surrounded but non-overlapping attesting indices within same validator chunk index - 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{2, 3}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index - single step", steps: []*step{ @@ -243,6 +477,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, surrounding but non-overlapping attesting indices in different validator chunk index - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index - single step", steps: []*step{ @@ -256,6 +509,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, surrounded but non-overlapping attesting indices in different validator chunk index - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{params.BeaconConfig().MinGenesisActiveValidatorCount - 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, (source 1, target 2), (source 2, target 3) - single step", steps: []*step{ @@ -269,6 +541,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, (source 1, target 2), (source 2, target 3) - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 1, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 2, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, (source 0, target 3), (source 2, target 4) - single step", steps: []*step{ @@ -282,6 +573,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, (source 0, target 3), (source 2, target 4) - 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: 2, target: 4, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, (source 0, target 2), (source 0, target 3) - single step", steps: []*step{ @@ -295,6 +605,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, (source 0, target 2), (source 0, target 3) - two steps", + steps: []*step{ + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + { + currentEpoch: 4, + attestationsInfo: []*attestationInfo{ + {source: 0, target: 3, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, { name: "Not slashable, (source 0, target 3), (source 0, target 2) - single step", steps: []*step{ @@ -308,6 +637,25 @@ func Test_processAttestations(t *testing.T) { }, }, }, + { + name: "Not slashable, (source 0, target 3), (source 0, 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: 0, target: 2, indices: []uint64{0, 1}, beaconBlockRoot: nil}, + }, + expectedSlashingsInfo: nil, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From d1665e84939f87b96fe3f3df5b1baa392ed5f076 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Thu, 8 Feb 2024 16:38:31 +0100 Subject: [PATCH 36/38] `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon --- beacon-chain/slasher/chunks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon-chain/slasher/chunks.go b/beacon-chain/slasher/chunks.go index 71b29a8816e9..5d8c23e33f23 100644 --- a/beacon-chain/slasher/chunks.go +++ b/beacon-chain/slasher/chunks.go @@ -521,8 +521,8 @@ func (m *MinSpanChunksSlice) NextChunkStartEpoch(startEpoch primitives.Epoch) pr // first_epoch(chunkIndex(startEpoch)+1) // first_epoch(chunkIndex(3)+1) // first_epoch(1 + 1) -// first_epoch(6) -// 4 +// first_epoch(2) +// 6 func (m *MaxSpanChunksSlice) NextChunkStartEpoch(startEpoch primitives.Epoch) primitives.Epoch { return m.params.firstEpoch(m.params.chunkIndex(startEpoch) + 1) } From e41c2814004ddeeb24ed0de8ba6a02c2bdbf6ce8 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Thu, 8 Feb 2024 16:25:10 +0100 Subject: [PATCH 37/38] `chunks.go`: Avoid templating log messages. --- beacon-chain/slasher/chunks.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/beacon-chain/slasher/chunks.go b/beacon-chain/slasher/chunks.go index 5d8c23e33f23..dd972b2f3c3a 100644 --- a/beacon-chain/slasher/chunks.go +++ b/beacon-chain/slasher/chunks.go @@ -10,6 +10,7 @@ import ( slashertypes "github.com/prysmaticlabs/prysm/v4/beacon-chain/slasher/types" "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" + "github.com/sirupsen/logrus" ) // A struct encapsulating input arguments to @@ -213,10 +214,12 @@ func (m *MinSpanChunksSlice) CheckSlashable( // recorded in our min/max DB an distance corresponding to an attestaiton, but WITHOUT // recording the attestation itself. As a consequence, we say there is no surrounding vote, // but we log an error. - log.Errorf("No existing attestation record found for validator %d at target %d, while a surrounding vote was detected.", - validatorIdx, minTarget, - ) + fields := logrus.Fields{ + "validatorIndex": validatorIdx, + "targetEpoch": minTarget, + } + log.WithFields(fields).Error("No existing attestation record found while a surrounding vote was detected.") return nil, nil } @@ -279,9 +282,12 @@ func (m *MaxSpanChunksSlice) CheckSlashable( // recorded in our min/max DB an distance corresponding to an attestaiton, but WITHOUT // recording the attestation itself. As a consequence, we say there is no surrounded vote, // but we log an error. - log.Errorf("No existing attestation record found for validator %d at target %d, while a surrounded vote was detected.", - validatorIdx, maxTarget, - ) + fields := logrus.Fields{ + "validatorIndex": validatorIdx, + "targetEpoch": maxTarget, + } + + log.WithFields(fields).Error("No existing attestation record found while a surrounded vote was detected.") return nil, nil } From 2ffff9c829aa0425a618686907a3b5106a482d64 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Thu, 8 Feb 2024 16:34:08 +0100 Subject: [PATCH 38/38] `checkSlashableAttestations`: Simplify duration computation. --- beacon-chain/slasher/detect_attestations.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/beacon-chain/slasher/detect_attestations.go b/beacon-chain/slasher/detect_attestations.go index d492d923b877..dff721fd6ac4 100644 --- a/beacon-chain/slasher/detect_attestations.go +++ b/beacon-chain/slasher/detect_attestations.go @@ -18,6 +18,8 @@ import ( func (s *Service) checkSlashableAttestations( ctx context.Context, currentEpoch primitives.Epoch, atts []*slashertypes.IndexedAttestationWrapper, ) ([]*ethpb.AttesterSlashing, error) { + totalStart := time.Now() + slashings := make([]*ethpb.AttesterSlashing, 0) // Double votes @@ -36,11 +38,10 @@ func (s *Service) checkSlashableAttestations( groupedByValidatorChunkIndexAtts := s.groupByValidatorChunkIndex(atts) log.WithField("numBatches", len(groupedByValidatorChunkIndexAtts)).Debug("Batching attestations by validator chunk index") groupsCount := len(groupedByValidatorChunkIndexAtts) - batchDurations := make([]time.Duration, 0, len(groupedByValidatorChunkIndexAtts)) - for validatorChunkIndex, attestations := range groupedByValidatorChunkIndexAtts { - innerStart := time.Now() + 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 attSlashings, err := s.checkSurrounds(ctx, attestations, slashertypes.MinSpan, currentEpoch, validatorChunkIndex) @@ -54,24 +55,19 @@ func (s *Service) checkSlashableAttestations( for _, idx := range indices { s.latestEpochWrittenForValidator[idx] = currentEpoch } - - batchDurations = append(batchDurations, time.Since(innerStart)) } - // Elapsed time computation - totalBatchDuration := time.Duration(0) - for _, batchDuration := range batchDurations { - totalBatchDuration += batchDuration - } + surroundElapsed := time.Since(surroundStart) + totalElapsed := time.Since(totalStart) fields := logrus.Fields{ "numAttestations": len(atts), "numBatchesByValidatorChunkIndex": groupsCount, - "elapsed": totalBatchDuration, + "elapsed": totalElapsed, } if groupsCount > 0 { - avgProcessingTimePerBatch := totalBatchDuration / time.Duration(groupsCount) + avgProcessingTimePerBatch := surroundElapsed / time.Duration(groupsCount) fields["avgBatchProcessingTime"] = avgProcessingTimePerBatch }