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

fix filter for next round period 0 votes #62

Merged
merged 5 commits into from Jun 20, 2019

Conversation

@Vervious
Copy link
Contributor

Vervious commented Jun 19, 2019

Summary

This fixes a deviation from the spec. Currently, the code (accidentally) filters all votePresent and voteVerified events from the next round, if myPlayer.Period is large. Instead, whether we filter votes from the next round should not be a function of current period. As specified in the spec, we allow votes with vote.Round = player.Round + 1 and vote.Period == 0 and vote.Step = {propose,soft,cert,next}. We continue to filter all FPR votes.

Test Plan

Added a unit test for this scenario

Vervious added 3 commits Jun 13, 2019
This fixes a deviation from the spec. Currently, the code
(accidentally) filters all votePresent and voteVerified events
from the next round, if myPlayer.Period is large. Instead,
whether we filter votes from the next round should not be a
function of current period. As specified in the spec, we allow
votes with vote.Round = player.Round + 1 and vote.Period == 0
and vote.Step = {propose,soft,cert,next}
@Vervious Vervious self-assigned this Jun 19, 2019
@Vervious Vervious requested review from algoradam and derbear Jun 19, 2019
@Vervious

This comment has been minimized.

Copy link
Contributor Author

Vervious commented Jun 19, 2019

Fixes #63

agreement/voteAggregator.go Outdated Show resolved Hide resolved
Copy link
Collaborator

derbear left a comment

(See comments)

@Vervious

This comment has been minimized.

Copy link
Contributor Author

Vervious commented Jun 20, 2019

See algorandfoundation/specs#1 for spec update

@Vervious Vervious requested a review from derbear Jun 20, 2019
@Vervious Vervious dismissed derbear’s stale review Jun 20, 2019

updated

@Vervious Vervious merged commit c05b79d into algorand:master Jun 20, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
license/cla Contributor License Agreement is signed.
Details
@Vervious Vervious deleted the Vervious:nextvotefiltering branch Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.