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.
What was wrong?
Addresses a number of issues in crosslink processing encountered during update to v0.5.1:
previous_crosslink
in the attestation outdated. This PR, also, adds a test to a spec that covers this case. Described in this issue Previous crosslink becomes outdated after epoch transition #845.get_winning_root_and_participants
returned empty participation lists for crosslinks of updated shards as this update madeprevious_crosslink
in attestations out of date.GENESIS_EPOCH
caused incorrect crosslink processing. Mentioned here: Missed filter condition in get_winning_root_and_participants #841 (comment).How it was fixed.
previous_crosslink
was fixed by breaking crosslinks down toprevious_epoch_crosslinks
andcurrent_epoch_crosslinks
. Now attestation is checked against both lists which prevents it from being spoiled. There was another solution of this issue, to process crosslinks only for previous epoch. But this approach adds a delay to a shard crosslinking, IMO, a speed of crosslinking process is invariant that is not desired to be broken.apply_reward
has been called during the epoch processing. It preserves an invariant of calculating voting balances before validator balances get changed.Crosslinks processing flow
This is how this PR affects epoch processing:
Complexity
This PR adds a certain degree of complexity to crosslink and epoch processing. This complexity has been added in order to preserve invariants that from my perspective are important for the beacon chain processing. It may be there are other ways of solving these issues, would be happy to discuss them here.
Naming and types
Disclaimer: I am not pythonist. In this PR you can meet bad namings, not that idiomatic function decomposition alongside with inappropriate type usage (esp.
Vector
).Testing
These changes have been tested in our simulator. It works well for ideal case when all attestations are included in the next coming block, but I believe those are good for sophisticated cases as well. Corresponding commit harmony-dev/beacon-chain-java@a1ee101
Misc
This PR a bit affects Phase 1 spec. There is also renaming of
previous_crosslink
in attestation data tosource_crosslink
; this is done becauseprevious_crosslink
name started to clash withprevious_epoch_crosslinks
. Not sure that I likesource_crosslink
but gave up on picking something else.Related issues and PRs
Addresses #845, supplants #841.