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

Merkleisation friendly pending attestations #697

Closed
wants to merge 2 commits into from

Conversation

JustinDrake
Copy link
Collaborator

Also cleaner presentation and conceptually

Also cleaner presentation and conceptually
@@ -1789,7 +1793,7 @@ The steps below happen when `(state.slot + 1) % SLOTS_PER_EPOCH == 0`.

* Let `previous_total_balance = get_total_balance(state, get_active_validator_indices(state.validator_registry, previous_epoch))`.
* Validators that made an attestation during the previous epoch, targeting the previous justified slot:
* Let `previous_epoch_attestations = [a for a in state.latest_attestations if previous_epoch == slot_to_epoch(a.data.slot)]`. (Note: Each of these attestations votes for the previous justified epoch/block root because of the [attestation block validity rules](#attestations-1).)
* Let `previous_epoch_attestations = state.previous_epoch_attestations`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just get rid of this line and use state.previous_epoch_attestations everywhere? A 6 character reduction is not normally worth a helper variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I left it as is because of the nice helper variable symmetry:

  • Let previous_epoch_attestations...
  • Let previous_epoch_attester_indices...
  • Let previous_epoch_attesting_balance...
  • Let previous_epoch_boundary_attestations...
  • Let previous_epoch_boundary_attester_indices...
  • Let previous_epoch_boundary_attesting_balance...
  • Let previous_epoch_head_attestations...
  • Let previous_epoch_head_attester_indices...
  • Let previous_epoch_head_attesting_balance...

@@ -1779,7 +1783,7 @@ The steps below happen when `(state.slot + 1) % SLOTS_PER_EPOCH == 0`.
[Validators](#dfn-Validator) attesting during the current epoch:

* Let `current_total_balance = get_total_balance(state, get_active_validator_indices(state.validator_registry, current_epoch))`.
* Let `current_epoch_attestations = [a for a in state.latest_attestations if current_epoch == slot_to_epoch(a.data.slot)]`. (Note: Each of these attestations votes for the current justified epoch/block root because of the [attestation block validity rules](#attestations-1).)
* Let `current_epoch_attestations = state.current_epoch_attestations`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as with prev attestations below.

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

I like this.

As I mentioned in the comment, I think we are safe changing get_previous_epoch to use - 1 in all cases.

vbuterin added a commit that referenced this pull request Mar 3, 2019
@vbuterin
Copy link
Contributor

vbuterin commented Mar 3, 2019

Assimilated into #711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants