doppelganger detection walltime refactor #2656
Conversation
beacon_chain/spec/datatypes/base.nim
Outdated
| @@ -472,6 +472,7 @@ type | |||
|
|
|||
| DoppelgangerProtection* = object | |||
| broadcastStartEpoch*: Epoch | |||
| nodeLaunchSlot*: Slot | |||
There was a problem hiding this comment.
with this change, we can remove broadcastStartEpoch and simply use nodeLaunchSlot.epoch + 2 - this also appropriately moves the init logic to eth2_processor, rather than the main beacon node file, which also means it's easier to write tests for the doppelganger code.
also, the object type itself can be moved to eth2_processor, I'm not sure why it's here?
There was a problem hiding this comment.
This accommodates gossip turning on and off due, e.g,. to sufficiently long network outages. broadcastStartEpoch is set during gossip setup, while nodeLaunchSlot is set up during node initialization. This is, in principle, the correct distinction to make: when one node isn't gossiping, it's okay for another node to gossip instead, and interleave. It might be worth using the simplified nodeLaunchSlot.epoch + 2 model, but it is less flexible.
As far as the object type, 556d06f.
There was a problem hiding this comment.
fair enough - I suspect this will need revisiting when incorporating the slashing protection scheme, let's document this flow here for now?
| attesterIndices: openArray[ValidatorIndex]) = | ||
| # Only check for attestations after node launch, not potential attestations | ||
| # bouncing around from up to several minutes prior. | ||
| if attestation.data.slot <= self.doppelgangerDetection.nodeLaunchSlot: |
There was a problem hiding this comment.
actually, this comparison achieves the same thing as the rounding suggested in per #2650 (comment) - the comment could be more precise however, in that we're aiming to allow any attestation that this instance created (ie it's not about minutes)
…nd allow for one-slot overlap to avoid false positives on intra-slot restarts
tersec
changed the title
* strawman doppelganger detection walltime refactor * move DoppelgangerProtection to Eth2Processor * increase comment precision * document difference between broadcastStartEpoch and nodeLaunchSlot, and allow for one-slot overlap to avoid false positives on intra-slot restarts
In response to #2650