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

Issue in validator spec attester responsibility #628

Closed
rauljordan opened this issue Feb 14, 2019 · 1 comment
Closed

Issue in validator spec attester responsibility #628

rauljordan opened this issue Feb 14, 2019 · 1 comment
Labels
scope:v-guide Validator guide

Comments

@rauljordan
Copy link
Contributor

In the current honest validator spec's attester section, we see the following:

Epoch boundary root

Set attestation_data.epoch_boundary_root = hash_tree_root(epoch_boundary) where epoch_boundary is the block at the most recent epoch boundary in the chain defined by head -- i.e. the BeaconBlock where block.slot == get_epoch_start_slot(head.slot).

Note: This can be looked up in the state using get_block_root(state, get_epoch_start_slot(head.slot)).

Justified block root

Set attestation_data.justified_block_root = hash_tree_root(justified_block) where justified_block is the block at state.justified_epoch in the chain defined by head.

Note: This can be looked up in the state using get_block_root(state, justified_epoch).

The above does not make much sense, as get_epoch_start_slot is defined in terms of epochs, not slots, as epoch * EPOCH_LENGTH, but want we want here instead is to get the start slot of the epoch boundary defined by the canonical head, so instead we can do get_epoch_start_slot(closest_epoch_boundary(head.Slot)). Similarly for the justified block root, it should be get_epoch_start_slot(state.justified_epoch) instead.

See our Github gist for our implementation of this logic in Prysm.

https://gist.github.com/rauljordan/d4e57afb63e8caea4333ebe3e4d3588c

@djrtwo
Copy link
Contributor

djrtwo commented Feb 18, 2019

Right, we changed get_epoch_start_slot to take an epoch instead of a slot when we moved epochs to being more important. Looks like this can be fixed by ensuring we get the epoch of the slot in question and pass it to get_epoch_start_slot in both cases, correct?

djrtwo added a commit that referenced this issue Feb 21, 2019
@djrtwo djrtwo closed this as completed Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:v-guide Validator guide
Projects
None yet
Development

No branches or pull requests

3 participants