add mutex for validator.highestValidSlot

[infosecual]

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

This bug fixes a potential race condition (read/write) in the validator process. The area of contention is the validator struct's highestValidSlot field.

Explanation

validator.ReceiveBlocks() starts a loop that will wait to receive blocks from the bn and processes them as they arrive. This loop makes a write to v.highestValidSlot (validator/client/validator.go:361) which can be read at the same time by validator.waitOneThirdOrValidBlock() (validator/client/attest.go:257). This read can be triggered when a validator publishes a regular attestation or a sync committee contribution.

Fix

I added a simple mutex around access of validator.highestValidSlot. I used a vanilla Mutex (vs a RWMutex) since there are only two reads of the field and only one of them is in a different critical region from conflicting write.

Other notes for review

This was detected while running a validator client instrumented with ThreadSanitizer (compiled with go build -race) on mainnet-shadowfork-5. Here is the TSAN detection output for reference:

==================
WARNING: DATA RACE
Write at 0x00c000158960 by goroutine 112:
  github.com/prysmaticlabs/prysm/validator/client.(*validator).ReceiveBlocks()
      /go/src/prysm/validator/client/validator.go:361 +0x7a4
  github.com/prysmaticlabs/prysm/validator/client.run.func4()
      /go/src/prysm/validator/client/runner.go:107 +0x75

Previous read at 0x00c000158960 by goroutine 115:
  github.com/prysmaticlabs/prysm/validator/client.(*validator).waitOneThirdOrValidBlock()
      /go/src/prysm/validator/client/attest.go:257 +0x111
  github.com/prysmaticlabs/prysm/validator/client.(*validator).SubmitAttestation()
      /go/src/prysm/validator/client/attest.go:40 +0x2a4
  github.com/prysmaticlabs/prysm/validator/client.run.func1()
      /go/src/prysm/validator/client/runner.go:207 +0x2c1
  github.com/prysmaticlabs/prysm/validator/client.run.func7()
      /go/src/prysm/validator/client/runner.go:221 +0xa1

Goroutine 112 (running) created at:
  github.com/prysmaticlabs/prysm/validator/client.run()
      /go/src/prysm/validator/client/runner.go:107 +0xda5
  github.com/prysmaticlabs/prysm/validator/client.(*ValidatorService).Start.func1()
      /go/src/prysm/validator/client/service.go:223 +0x64

Goroutine 115 (running) created at:
  github.com/prysmaticlabs/prysm/validator/client.run()
      /go/src/prysm/validator/client/runner.go:203 +0x28f5
  github.com/prysmaticlabs/prysm/validator/client.(*ValidatorService).Start.func1()
      /go/src/prysm/validator/client/service.go:223 +0x64
==================

[potuz]

Thank you for this

[CLAassistant]

All committers have signed the CLA.

[terencechain]

lgtm!

[prestonvanloon]

Amazing thanks!