-
Notifications
You must be signed in to change notification settings - Fork 146
Implement light client friendliness changes #234
Conversation
8aac335
to
10d4d37
Compare
10d4d37
to
8bd7d3c
Compare
@@ -154,8 +154,9 @@ def get_initial_beacon_state(*, | |||
state = activate_validator( | |||
state, | |||
validator_index, | |||
genesis=True, | |||
is_genesis=True, |
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.
👍
@@ -88,13 +127,21 @@ def process_validator_registry(state: BeaconState, | |||
state = state.copy( | |||
current_epoch_calculation_slot=state.slot, | |||
) | |||
|
|||
# `helpers.generate_seed` path for mocking tests |
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.
I think know what this comment is indicating but could we maybe make it a bit more explicit. Here's what I think it is trying to say.
The
helpers.generate_seed
function is only present to provide an entry point for mocking this out in tests.
|
||
# TODO: chanege to hash_tree_root | ||
index_root = hash_eth2( | ||
bytes( |
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.
Is it guaranteed that get_active_validator_indices()
will only return values in the range 0-255
? A glance at the implementation doesn't appear it has that guarantee. If not, then this call will fail if any of the indices are outside of this range.
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.
ohhh good point!!!!
(Note that it's a workaround before we add tree hashing.) I'm changing it to:
# TODO: chanege to hash_tree_root
index_root = hash_eth2(
b''.join(
[
index.to_bytes(32, 'big')
for index in get_active_validator_indices(
state.validator_registry,
# TODO: change to `per-epoch` version
state.slot,
)
]
)
)
(p.s. The upper-bound of the size of ValidatorIndex
is uint64.)
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.
Hrm, a uint64
should fit within 8 bytes right?
result_state = _update_latest_index_roots(state, config) | ||
|
||
index_root = hash_eth2( | ||
bytes( |
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.
Same here WRT to having any indices that are outside of 0-255
What was wrong?
Implement ethereum/consensus-specs#476
Rationale: ethereum/consensus-specs#459
Note that it's this version of the spec: https://github.com/ethereum/eth2.0-specs/blob/85d39af1cad0e4869944be8bdcbfb75b7c3c64f6/specs/core/0_beacon-chain.md and it doesn't reflect #235 yet.
How was it fixed?
ValidatorRegistryDeltaBlock
ValidatorRegistryDeltaFlag
BeaconState.validator_registry_delta_chain_tip
BeaconState.previous_epoch_randao_mix
->BeaconState.previous_epoch_seed
BeaconState.current_epoch_randao_mix
->BeaconState.current_epoch_seed
BeaconState.latest_index_roots
get_active_index_root
get_entry_exit_effect_slot
generate_seed
: note that it was added by other spec PR, but added the "slot" version here since it's helpful to refactor the code. Will have to change to "epoch" version for Make epochs first class citizens and epoch trans at end of epoch #235 in other PR.eth2/beacon/on_startup.py
: update the state fields.validator_status_helpers.py
: removevalidator_registry_delta_chain_tip
updating.Cute Animal Picture
source