-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Pythonize epoch transitions #711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aw wow! I hope that will clarify a lot! 😊
Will take another look.
tip: "Add suggestion to batch" 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So much more readable. 😍
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and get_previous_epoch_attestations
is undefined. 😅
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
Co-Authored-By: vbuterin <v@buterin.com>
base_reward -> get_base_reward Co-Authored-By: djrtwo <dannyjryan@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standardises terminology across transfers:
to
-> recipient
For some reason from
was changed to sender
for transfers earlier in this PR. I like this new terminology.
Co-Authored-By: djrtwo <dannyjryan@gmail.com>
There's currently 3 committee count functions:
|
Yeah, this is reasonable. I want to do any further changes outside of this PR. I'm working on a detailed changelog for implementers and it's already quite large :) |
FYI: I'm building an executable Go version of the spec, based on the spec from this PR, and really appreciate the new structuring. And yes, the PR is large, but it's worth it 100% 💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nitpicks.
Awesome work! 👍
Co-Authored-By: djrtwo <dannyjryan@gmail.com>
* Let `previous_active_validator_indices = get_active_validator_indices(state.validator_registry, previous_epoch)` | ||
* Let `epochs_since_finality = next_epoch - state.finalized_epoch`. | ||
adjusted_quotient = integer_squareroot(get_previous_total_balance(state)) // BASE_REWARD_QUOTIENT | ||
return get_effective_balance(state, index) // adjusted_quotient // 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to do some constant folding upon the final parameter tuning for these numbers? same below w/ INACTIVITY_PENALTY_QUOTIENT // 2
in get_inactivity_penalty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
This is first and foremost a refactor of the epoch transition to define it entirely using python code.
To make the code executable, a number of substantive changes were also included. See below for a detailed change log of such changes.
Substantive changes
latest_attestations
fromBeaconState
previous_epoch_attestations
andcurrent_epoch_attestations
toBeaconState
attestation.data.slot
is from previous or current epochstate.previous_epoch_attestations = state.current_epoch_attestations
.state.current_epoch_attestations = []
.(state, block) -> state
but there was an implicit third parameter --parent_root
latest_state_roots
to state which is also a nice historic feature in general.Proposal
withBeaconBlockHeader
. Removeshard
field.hash_tree_root(block) == hash_tree_root(block_header)
parent_root
toprevious_block_root
inBeaconBlock
eth1_data
andrandao_reveal
intoBeaconBlockBody
latest_state_roots
toBeaconState
batched_block_roots
tohistorical_roots
and use this to batch(block, state)
rather than justblock
. Do batching at end of epoch transition.get_temporary_block_header
helper that produces a block header from a block with aZERO_HASH
for state rootget_genesis_beacon_state
to handle modified state fieldsget_previous_epoch
returnscurrent_epoch - 1
in all caseswinning_root
favors lexicographically higher hashes rather than lowerget_base_reward
when no previous balance (02e8e89) (bugfix)