Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple justification and finalization processing #925

Merged
merged 8 commits into from
Apr 16, 2019

Conversation

JustinDrake
Copy link
Collaborator

More readable and saves a few lines.

@JustinDrake JustinDrake added the general:presentation Presentation (as opposed to content) label Apr 15, 2019
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Show resolved Hide resolved
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving but I hope someone can figure out another name than previous_previous_justified_epoch. 😆

@JustinDrake
Copy link
Collaborator Author

I hope someone can figure out another name than previous_previous_justified_epoch. 😆

I guess antepenultimate_justified_epoch is slightly better :)

@JustinDrake JustinDrake merged commit f84818f into dev Apr 16, 2019
@hwwhww hwwhww mentioned this pull request Apr 17, 2019
state.finalized_epoch = antepenultimate_justified_epoch
state.finalized_root = get_block_root(state, get_epoch_start_slot(state.finalized_epoch))
# The 1st/2nd/3rd most recent epochs are justified, the 1st using the 3rd as source
if (bitfield >> 0) % 8 == 0b111 and state.previous_justified_root == current_epoch - 2:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug. making new PR

@djrtwo
Copy link
Contributor

djrtwo commented Apr 17, 2019

I'm pretty sure we used to do it in a similar way but this was seen as difficult to analyze because we were changing things around in the state before running the comparisons..

For example parsing what state.previous_justified_epoch == current_epoch - 2 means is tough because previous_justified_epoch was already shifted. So it's something like "does the current_justified_epoch that was just moved into the previous_justified_epoch equal to the actual current_epoch minus 2?"

@djrtwo djrtwo deleted the JustinDrake-patch-12 branch April 17, 2019 14:53
@djrtwo djrtwo mentioned this pull request Apr 17, 2019
JustinDrake added a commit that referenced this pull request Apr 18, 2019
JustinDrake added a commit that referenced this pull request Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:presentation Presentation (as opposed to content)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants