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

Rework Shard transition processing #1856

Merged
merged 2 commits into from
Jun 1, 2020
Merged

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented May 28, 2020

Pending on #1854

  1. Separate New Attestation processin section and Shard transition processing section
  2. Combine process_crosslinks and verify_empty_shard_transition into process_shard_transitions
  3. Make verify_empty_shard_transition return bool as other verify_* functions

/cc @terencechain

@hwwhww hwwhww added phase1 general:presentation Presentation (as opposed to content) labels May 28, 2020
@hwwhww hwwhww force-pushed the hwwhww/verify_empty_shard_transition branch from 5695cb9 to 0aec204 Compare May 28, 2020 19:26
###### Updated `process_attestation`

```python
def process_attestation(state: BeaconState, attestation: Attestation) -> None:
Copy link
Contributor Author

@hwwhww hwwhww May 28, 2020

Choose a reason for hiding this comment

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

To reviewers: only moving the function block to here. Nothing was changed in this function.

@hwwhww hwwhww requested a review from djrtwo May 28, 2020 19:31
@@ -695,7 +696,7 @@ def process_operations(state: BeaconState, body: BeaconBlockBody) -> None:
# See custody game spec.
process_custody_game_operations(state, body)

process_crosslinks(state, body.shard_transitions, body.attestations)
process_shard_transitions(state, body.shard_transitions, body.attestations)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be neat to convert to:
for_ops(body.shard_transitions, body.attestations, process_shard_transitions)
But it doesn't look feasible today 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But for_ops is mainly for executing operation one-by-one, and we need all body.shard_transitions and body.attestations to process crosslinks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!

@hwwhww hwwhww force-pushed the hwwhww/verify_empty_shard_transition branch from 0aec204 to 327deb4 Compare May 29, 2020 19:15
@hwwhww hwwhww changed the base branch from hwwhww/phase1_refactor to dev June 1, 2020 09:46
@hwwhww hwwhww merged commit 8e5db1b into dev Jun 1, 2020
@hwwhww hwwhww deleted the hwwhww/verify_empty_shard_transition branch June 1, 2020 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:presentation Presentation (as opposed to content) phase1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants