Properly handle skip slots in fork choice #1579
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to the way epoch pairs are justified/finalized and how we handle skipped slots in the consensus, we have to be careful to ensure not only that the justified/finalized block is in the chain, but that it is in the chain at the precise justified/finalized epoch (accounting for skip slots in exceptional cases)
Previously I erroneously put in conditions such the following:
This just ensured that the block was in fact later than the finalized epoch, but not that the finalized root was in the chain (potentially at a skip slot) at that epoch.
To fix this, we modify
get_ancestor
to return the latest root at or before the queried slot in the chain in question. This mimics the behavior of how we save roots in state and how we vote on roots, and it very nicely fixes this corner case.Helps properly handle cases in which we justify/finalize blocks after skipped epochs like in the following:
Say block 64 at epoch 2 is finalized. Blocks that build off of block 64 at epoch 1 should not be considered in the fork choice. the modificaiton to
get_ancestor
protect against this case