Conversation
|
|
||
| if block.SealingBlockInfo() != nil { | ||
| // are we the highest validator | ||
| highestEpoch, highestValidatorSet := n.epochs.highestEpoch() |
There was a problem hiding this comment.
highestEpoch == md.Seq {
So this basically means we have replicated the sealing block of the last epoch?
And the reason this works is that we replicate backwards?
There was a problem hiding this comment.
If so, can we add some comments?
There was a problem hiding this comment.
yep, if we have an epoch in the epochs map, that means we have verified it's accepted by enough validators. Therefore, if the block we are indexing the sealing block that is part of the highest epoch we should check if we need to transition.
will add a comment
There was a problem hiding this comment.
I'm not sure this is good enough, on second thought.
If we start the non-validator we initialize this map with the last thing we have in the ledger and then if we commit a sealing block we will transition, won't we?
I think we should peek at the P-chain and compare the validator set and only if they're the same, then trigger it.
Thoughts?
There was a problem hiding this comment.
I think we should peek at the P-chain and compare the validator set and only if they're the same, then trigger it.
Hmm not sure this works either, because what if we are in the middle of a transition? and the highest pchain validator set is different than the current epoch. Maybe our validator is down, and it needs to catch up in order to sign off on the approval that will transition the epoch.
There was a problem hiding this comment.
ok so i think as long as we do #530 we should be able to know for sure that we have the latest epoch for real, thoughts?
| e.broadcast(finalizationMsg) | ||
|
|
||
| e.epochSealed.Store(true) | ||
| if e.OnSealingBlockIndex != nil { |
There was a problem hiding this comment.
Why do we need to add this? This can be implemented in instance.go, that's what it's for.
2 participants
The main feature of this PR is the callback in the
EpochandNonValidatorstructs that signal to parent components a transition is required. This fixes the non-validator bug of restarting in-between epochs when it shouldn't be.In the
Epochstruct, when a sealing block is indexed we will now call theOnSealingBlockIndexwhich will notify the instance of an epoch change. Similarly, theNonValidatorhas aTransitionToValidatorconfig that notifies theInstancewhen it should transition an epoch change.The main benefit is now the non-validator can signal to the
Instancewhen it is ready to transition to a validator. Before, since the Storage component was handling this change, the Storage component needed to re-call into the non-validator to confirm whether the sealing block actually meant a transition was needed. Since the non-validator has all the info, that logic should stay within the non-validator. This also helps avoiding deadlock since we don't need to be concerned about re-grabbing the non-validator lock.This PR also simplifies and refactors some of the logic for transitioning an epoch.
nodeRoleiotatransitionEpochNonValidatorandtransitionEpochValidator