Skip to content

Synchronize and reset the DVT middleware connection flag - #49

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/dvt-middleware-flag-race
Open

Synchronize and reset the DVT middleware connection flag#49
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/dvt-middleware-flag-race

Conversation

@damilolaedwards

Copy link
Copy Markdown

Summary

connectedToDVTMiddleware was written from checkDVT without holding any lock, while Proposal and BlindedProposal read it unlocked to decide whether to skip the RANDAO reveal check. It was also never cleared once set, so a client that connected through DVT middleware once kept skipping that check for the rest of its life, even after reconnecting to a plain node later on.

This guards the flag with the existing connection mutex, adds an accessor for the read sites to use, and resets the flag whenever the connection goes inactive so it gets re-evaluated fresh on every reconnect.

Test plan

  • New test confirming the flag sets on a Charon-flagged node, clears once the connection goes inactive, and stays clear after reconnecting to a plain node
  • New test exercising concurrent connection-state transitions against concurrent flag reads under the race detector, no race reported
  • Full repo build and vet pass, all packages still compile

connectedToDVTMiddleware was written from checkDVT without holding
any lock, while Proposal and BlindedProposal read it unlocked to
decide whether to skip the RANDAO reveal check. It was also never
cleared once set, so a client that ever connected through DVT
middleware kept skipping that check for the rest of its life, even
after reconnecting to a plain node.

Guard the flag with the existing connection mutex, add an accessor
for the read sites to use, and reset the flag whenever the connection
goes inactive so it gets re-evaluated fresh on every reconnect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant