Voting: fix snapshot block for new vote

apps/voting/contracts/Voting.sol

@@ -269,13 +269,14 @@ contract Voting is IForwarder, AragonApp {
         internal
         returns (uint256 voteId)
     {
-        uint256 votingPower = token.totalSupplyAt(vote_.snapshotBlock);
+        uint64 snapshotBlock = getBlockNumber64() - 1; // avoid double voting in this very block
+        uint256 votingPower = token.totalSupplyAt(snapshotBlock);
         require(votingPower > 0, ERROR_NO_VOTING_POWER);
 
         voteId = votesLength++;
         Vote storage vote_ = votes[voteId];
         vote_.startDate = getTimestamp64();
-        vote_.snapshotBlock = getBlockNumber64() - 1; // avoid double voting in this very block
+        vote_.snapshotBlock = snapshotBlock;
         vote_.supportRequiredPct = supportRequiredPct;
         vote_.minAcceptQuorumPct = minAcceptQuorumPct;
         vote_.votingPower = votingPower;

apps/voting/contracts/test/mocks/VotingMock.sol

@@ -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);
     }
+
+    // Mint a token and create a vote in the same transaction to test snapshot block values are correct
+    function newTokenAndVote(address _holder, uint256 _tokenAmount, string _metadata)
+        external
+        returns (uint256 voteId)
+    {
+        token.generateTokens(_holder, _tokenAmount);
+
+        bytes memory noScript = new bytes(0);
+        voteId = _newVote(noScript, _metadata, false, false);
+        emit StartVote(voteId, msg.sender, _metadata);
+    }
 }

apps/voting/package.json

@@ -40,9 +40,9 @@
   "license": "(GPL-3.0-or-later OR AGPL-3.0-or-later)",
   "description": "",
   "devDependencies": {
+    "@aragon/apps-shared-migrations": "1.0.0",
     "@aragon/cli": "^5.2.0-beta.1",
     "@aragon/test-helpers": "^1.0.1",
-    "@aragon/apps-shared-migrations": "1.0.0",
     "eth-gas-reporter": "^0.1.5",
     "ganache-cli": "^6.1.0",
     "solidity-coverage": "0.5.11",

apps/voting/test/voting.js

@@ -223,12 +223,12 @@ contract('Voting App', accounts => {
                     assert.isTrue(isOpen, 'vote should be open')
                     assert.isFalse(isExecuted, 'vote should not be executed')
                     assert.equal(creator, holder51, 'creator should be correct')
-                    assert.equal(snapshotBlock, await getBlockNumber() - 1, 'snapshot block should be correct')
-                    assert.equal(supportRequired.toNumber(), neededSupport.toNumber(), 'required support should be app required support')
-                    assert.equal(minQuorum.toNumber(), minimumAcceptanceQuorum.toNumber(), 'min quorum should be app min quorum')
+                    assert.equal(snapshotBlock.toString(), await getBlockNumber() - 1, 'snapshot block should be correct')
+                    assert.equal(supportRequired.toString(), neededSupport.toString(), 'required support should be app required support')
+                    assert.equal(minQuorum.toString(), minimumAcceptanceQuorum.toString(), 'min quorum should be app min quorum')
                     assert.equal(y, 0, 'initial yea should be 0')
                     assert.equal(n, 0, 'initial nay should be 0')
-                    assert.equal(votingPower.toString(), bigExp(100, decimals).toString(), 'total voters should be 100')
+                    assert.equal(votingPower.toString(), bigExp(100, decimals).toString(), 'voting power should be 100')
                     assert.equal(execScript, script, 'script should be correct')
                     assert.equal(metadata, 'metadata', 'should have returned correct metadata')
                     assert.equal(await voting.getVoterState(voteId, nonHolder), VOTER_STATE.ABSENT, 'nonHolder should not have voted')
@@ -253,7 +253,7 @@ contract('Voting App', accounts => {
                     await timeTravel(votingTime + 1)
 
                     const state = await voting.getVote(voteId)
-                    assert.equal(state[4].toNumber(), neededSupport.toNumber(), 'required support in vote should stay equal')
+                    assert.equal(state[4].toString(), neededSupport.toString(), 'required support in vote should stay equal')
                     await voting.executeVote(voteId) // exec doesn't fail
                 })
 
@@ -268,7 +268,7 @@ contract('Voting App', accounts => {
                     await timeTravel(votingTime + 1)
 
                     const state = await voting.getVote(voteId)
-                    assert.equal(state[5].toNumber(), minimumAcceptanceQuorum.toNumber(), 'acceptance quorum in vote should stay equal')
+                    assert.equal(state[5].toString(), minimumAcceptanceQuorum.toString(), 'acceptance quorum in vote should stay equal')
                     await voting.executeVote(voteId) // exec doesn't fail
                 })
 
@@ -495,6 +495,49 @@ contract('Voting App', accounts => {
         })
     })
 
+    context('changing token supply', () => {
+        const holder1 = accounts[1]
+        const holder2 = accounts[2]
+
+        const neededSupport = pct16(50)
+        const minimumAcceptanceQuorum = pct16(20)
+
+        beforeEach(async () => {
+            token = await MiniMeToken.new(NULL_ADDRESS, NULL_ADDRESS, 0, 'n', 0, 'n', true) // empty parameters minime
+
+            await token.generateTokens(holder1, 1)
+            await token.generateTokens(holder2, 1)
+
+            await voting.initialize(token.address, neededSupport, minimumAcceptanceQuorum, votingTime)
+        })
+
+        it('uses the correct snapshot value if tokens are minted afterwards', async () => {
+            // Create vote and afterwards generate some tokens
+            const voteId = createdVoteId(await voting.newVote(EMPTY_SCRIPT, 'metadata'))
+            await token.generateTokens(holder2, 1)
+
+            const [isOpen, isExecuted, startDate, snapshotBlock, supportRequired, minQuorum, y, n, votingPower, execScript] = await voting.getVote(voteId)
+
+            // Generating tokens advanced the block by one
+            assert.equal(snapshotBlock.toString(), await getBlockNumber() - 2, 'snapshot block should be correct')
+            assert.equal(votingPower.toString(), (await token.totalSupplyAt(snapshotBlock)).toString(), 'voting power should match snapshot supply')
+            assert.equal(votingPower.toString(), 2, 'voting power should be correct')
+        })
+
+        it('uses the correct snapshot value if tokens are minted in the same block', async () => {
+            // Create vote and generate some tokens in the same transaction
+            // Requires the voting mock to be the token's owner
+            await token.changeController(voting.address)
+            const voteId = createdVoteId(await voting.newTokenAndVote(holder2, 1, 'metadata'))
+
+            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(), 'voting power should match snapshot supply')
+            assert.equal(votingPower.toString(), 2, 'voting power should be correct')
+        })
+    })
+
     context('before init', () => {
         it('fails creating a vote before initialization', async () => {
             return assertRevert(async () => {