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

Voting: fix snapshot block for new vote #648

Merged
merged 2 commits into from Feb 11, 2019
Merged

Voting: fix snapshot block for new vote #648

merged 2 commits into from Feb 11, 2019

Conversation

@sohkai
Copy link
Member

sohkai commented Feb 6, 2019

Testing this is a bit contrived, but it does fail the previous contract version.

@bingen
bingen approved these changes Feb 6, 2019
Copy link
Member

bingen left a comment

LGTM. Nice catch and nice tests!!

@@ -20,4 +20,16 @@ contract VotingMock is Voting {
function isValuePct(uint256 _value, uint256 _total, uint256 _pct) external pure returns (bool) {
return _isValuePct(_value, _total, _pct);
}

// Create a token and a vote in the same transaction to test snapshot block valuess are correct

This comment has been minimized.

Copy link
@bingen

bingen Feb 6, 2019

Member

Besides the minor typo in "valuess", "Create a token" sounds confusing to me, I'd rather say something like "Mint tokens" (and create a vote)

const [isOpen, isExecuted, startDate, snapshotBlock, supportRequired, minQuorum, y, n, votingPower, execScript] = await voting.getVote(voteId)

assert.equal(snapshotBlock.toString(), await getBlockNumber() - 1, 'snapshot block should be correct')
assert.equal(votingPower.toString(), (await token.totalSupplyAt(snapshotBlock)).toString(), 'total voters should be correct')

This comment has been minimized.

Copy link
@bingen

bingen Feb 6, 2019

Member

We could even check that those values match the expected total number of tokens (3, I guess)

This comment has been minimized.

Copy link
@sohkai

sohkai Feb 7, 2019

Author Member

Actually 2, since it's the block before, but 👍.

@sohkai sohkai added this to the A1 Sprint: 4.1 milestone Feb 11, 2019
@izqui
izqui approved these changes Feb 11, 2019
@sohkai sohkai merged commit 2c17479 into next Feb 11, 2019
5 checks passed
5 checks passed
License Compliance All checks passed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls First build on fix-new-vote at 96.124%
Details
license/cla Contributor License Agreement is signed.
Details
@sohkai sohkai deleted the fix-new-vote branch Feb 11, 2019
sohkai added a commit that referenced this pull request Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.