Align to ssv-spec v0.3.3
#1109
Align to ssv-spec v0.3.3
#1109
Conversation
moshe-blox
changed the title
ssv-spec to branch drop-blinded-block-rejectionssv-spec to latest
| func (n *p2pNetwork) SyncHighestDecided(mid spectypes.MessageID) error { | ||
| ctx := context.TODO() // TODO: pass context to SyncHighestDecided | ||
|
|
||
| return n.syncer.SyncHighestDecided(ctx, n.interfaceLogger, mid, func(msg *queue.DecodedSSVMessage) { | ||
| n.msgRouter.Route(ctx, msg) | ||
| }) | ||
| } | ||
|
|
||
| func (n *p2pNetwork) SyncDecidedByRange(mid spectypes.MessageID, from, to qbft.Height) { | ||
| ctx := context.TODO() // TODO: pass context to SyncDecidedByRange |
There was a problem hiding this comment.
I thought we said we wanted to leave sync for fullnodes and not just removed entirely. wouldn't just deleting like this stop fullnode/explorer functionality?
There was a problem hiding this comment.
Yes, but since the history sync was disabled anyway, and we want to redesign the syncing, then we preferred to fully align to spec here by removing SyncDecidedByRange as well.
| return c.UponFutureMsg(logger, msg) | ||
| } else { | ||
| return c.UponExistingInstanceMsg(logger, msg) | ||
| return nil, fmt.Errorf("future msg from height, could not process") |
There was a problem hiding this comment.
The spec doesn't concern itself with future messages.
Note that in practice, if there is for example a one second drift between different operators clocks, it can potentially cause duties to be missed.
Not sure how common this case is. Also it should effect at least 2 operators in a committee of 4 to have an impact. I understood engineering is not concerned about it.
However, I am writing this comment here for the record
|
|
||
| func (b *BaseRunner) ShouldProcessDuty(duty *spectypes.Duty) error { | ||
| if b.QBFTController.Height >= specqbft.Height(duty.Slot) { | ||
| return errors.Errorf("duty for slot %d already passed. Current height is %d", duty.Slot, |
There was a problem hiding this comment.
I think this is my bad here...
The bootstrap case isn't covered here (height == 0)
I think I need to create a PR in spec for it.
It isn't a big deal for mainnet but still a bug
There was a problem hiding this comment.
There was a problem hiding this comment.
We will address it in the next spec aligned version
it's not needed, just removed it |
|
" VoluntaryExit support (partially implemented to pass tests, not yet enabled, delayed for a future PR)" |
Changes
tests.json(State comparison json ssv-spec#275)Instance.BroadcastandStartNewDuty)golang-ci-lintversionRecommended QA tests:
StartNewDutywas modified, confirm that missed attestations aren't due to duties refusing to start.ValidatorRegistrationRunner: Change a fee recipient and check that the relays have received the updated address.Recommended changes in a followup PR: