Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slasher: False negative for surrounded votes #13591

Closed
nalepae opened this issue Feb 7, 2024 · 0 comments · Fixed by #13612
Closed

Slasher: False negative for surrounded votes #13591

nalepae opened this issue Feb 7, 2024 · 0 comments · Fixed by #13612
Assignees
Labels
Bug Something isn't working

Comments

@nalepae
Copy link
Contributor

nalepae commented Feb 7, 2024

Describe the bug

processQueuedAttestations it the following:

func (s *Service) processQueuedAttestations(ctx context.Context, slotTicker <-chan primitives.Slot) {
	defer s.wg.Done()

	for {
		select {
		case currentSlot := <-slotTicker:
			attestations := s.attsQueue.dequeue()
			currentEpoch := slots.ToEpoch(currentSlot)
   ...

If:

  • when we enter a first time in the case currentSlot := <-slotTicker, the queue contain an attestation A, and if,
  • when we enter a second time in case currentSlot := <-slotTicker, the queue contain an attestation B, and if,
  • B is surrounded by A

then no slashing offense will be detected ==> ❌

Additional notes:

If:

  • when we enter a first time in the case currentSlot := <-slotTicker, the queue contain two attestations A and B and if,
  • B is surrounded by A, or if
  • B surrounds by A

then the slashing offence will be detected. ==> ✅

If:

  • when we enter a first time in the case currentSlot := <-slotTicker, the queue contain an attestation A, and if,
  • when we enter a second time in case currentSlot := <-slotTicker, the queue contain an attestation B, and if,
  • B surrounds A

then the slashing offence will be detected. ==> ✅

Has this worked before in a previous version?

Probably not.

@nalepae nalepae added the Bug Something isn't working label Feb 7, 2024
@nalepae nalepae self-assigned this Feb 7, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 13, 2024
* `LastEpochWrittenForValidators`: Use golang if.

* `SaveLastEpochsWrittenForValidators`: Refactor.

* `SaveLastEpochsWrittenForValidators`: Fix when `epochByValIndex` > `batchSize`.

Before this commit, `TestStore_LastEpochWrittenForValidators` works if `validatorsCount <= 10000`
and stops working if `validatorsCount > 10000`.

* Slasher: Detect surrounded votes in multiple batches.

Fixes #13591.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant