Add and use get_indices_for_slot and get_block_hash
#62
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mratsim
added a commit
to status-im/nimbus-eth2
that referenced
this pull request
Aug 21, 2018
|
|
||
| sback = current_block.slot_number - cycle_length * 2 | ||
| assert sback <= slot < sback + cycle_length * 2 | ||
| return active_state.recent_block_hashes[slot - sback] |
There was a problem hiding this comment.
have you considered the case where sback could be negative?
Example:
current_block.slot_number = 10
sback = 10 - 64 * 2 = -118
then it'll return active_state.recent_block_hashes[slot + 118]
There was a problem hiding this comment.
I think it's okay since practically:
active_state.recent_block_hasheswas initialized to a list ofCYCLE_LENGTH * 2zero hashes, and then updated viaget_new_recent_block_hashesblock by block.- The domain of
slot - sbackwas guaranteed by the assertionsback <= slot < sback + cycle_length * 2, that said,0 <= slot - sback < CYCLE_LENGTH * 2.
I will add a test of the scenario you mentioned, thank you! 👍
hwwhww
force-pushed
the
get_indices_for_slot
branch
from
b638683
to
8ec0903
Compare
beacon_chain/state/helpers.py
Outdated
| cycle_length = config['cycle_length'] | ||
|
|
||
| ifh_start = crystallized_state.last_state_recalc - cycle_length | ||
| assert ifh_start <= slot < ifh_start + cycle_length * 2 |
There was a problem hiding this comment.
What does ifh stand for?
There was a problem hiding this comment.
Oh, indices_for_height, but it was renamed to indices_for_slot. I will update it. Thanks.
ChihChengLiang
approved these changes
Aug 23, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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?
Fixes #59
How was it fixed?
indices_for_heightstoindices_for_slotsget_indices_for_slotandget_block_hashhelper functions.get_signed_parent_hashes()andget_attestation_indicesto use those functions.I think if we use the old
get_signed_parent_hashes()+ add some boundary cases checks, it would be faster and efficient than the new one.TODO: Add more tests and refactor.