Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Implement per-block processing - eth1 data #253

Merged
merged 2 commits into from
Feb 4, 2019

Conversation

jannikluhn
Copy link
Contributor

What was wrong?

eth1 data processing was missing

How was it fixed?

Implemented it

Cute Animal Picture

put a cute animal picture link inside the parentheses

@hwwhww hwwhww changed the title Implement eth1 data processing Implement per-block processing - eth1 data Feb 3, 2019
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

I changed the title to per-block since there's also another Per-epoch processing - eth1 data section needs to be implemented in other PR.

Looks good, just one optimization comment. 👍

(index, eth1_data_vote)
for index, eth1_data_vote in enumerate(state.eth1_data_votes)
if block.eth1_data == eth1_data_vote.eth1_data
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this try, except StopIteration, else pattern looks interesting to me! But since ethereum/consensus-specs#561 clarified ethereum/consensus-specs#555 and using first makes us lose the advantage of that we can break the for-loop earlier once we find block.eth1_data == eth1_data_vote.eth1_data. IMHO I think the formal boring for-if-break is better in this case now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First will also break early as it's given a generator instead of a list.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh yes!

@jannikluhn jannikluhn merged commit f0e5bd4 into ethereum:master Feb 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants