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

Eth1 data processing is "locally" ambiguous #555

Closed
jannikluhn opened this issue Feb 2, 2019 · 2 comments
Closed

Eth1 data processing is "locally" ambiguous #555

jannikluhn opened this issue Feb 2, 2019 · 2 comments

Comments

@jannikluhn
Copy link
Contributor

If block.eth1_data equals eth1_data_vote.eth1_data for some eth1_data_vote in state.eth1_data_votes, set eth1_data_vote.vote_count += 1.

On its own this sentence is ambiguous in the case that multiple eth1_data_votes match (should all be updated or only the first one?). I assume that this case is not possible, but this is not obvious from this section alone (state.eth1_data_votes is also updated at other places). So it might be worth clarifying, either by adding a note or by over-specifying what's supposed to be done if multiple votes match.

@djrtwo
Copy link
Contributor

djrtwo commented Feb 2, 2019

I see... eth1_data_votes is only set in one other place and it is the clear it out.

Is the following worth the extra verbosity?

  • Let matching_data_votes = [vote for vote in state.eth1_data_votes if vote == block.eth1_data_vote].
  • If len(matching_data_votes) > 0, set matching_data_votes[0].vote_count += 1.
  • Otherwise, append to state.eth1_data_votes a new Eth1DataVote(eth1_data=block.eth1_data, vote_count=1).

@jannikluhn
Copy link
Contributor Author

This would make it more clear, but I agree that it adds some verbosity only to fix this very minor issue. How about this?

  • If there is an eth1_data_vote in states.eth1_data_votes for which eth1_data_vote.eth1_data equals block.eth1_data (there will be at most one), set eth1_data_vote.vote_count += 1.

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

No branches or pull requests

2 participants