Voting: Allow to modify required support

apps/voting/contracts/Voting.sol

@@ -18,6 +18,7 @@ contract Voting is IForwarder, AragonApp {
     using SafeMath64 for uint64;
 
     bytes32 public constant CREATE_VOTES_ROLE = keccak256("CREATE_VOTES_ROLE");
+    bytes32 public constant MODIFY_SUPPORT_ROLE = keccak256("MODIFY_SUPPORT_ROLE");
     bytes32 public constant MODIFY_QUORUM_ROLE = keccak256("MODIFY_QUORUM_ROLE");
 
     uint256 public constant PCT_BASE = 10 ** 18; // 0% = 0; 1% = 10^16; 100% = 10^18
@@ -28,6 +29,7 @@ contract Voting is IForwarder, AragonApp {
         address creator;
         uint64 startDate;
         uint256 snapshotBlock;
+        uint256 supportRequiredPct;
         uint256 minAcceptQuorumPct;
         uint256 yea;
         uint256 nay;
@@ -50,6 +52,7 @@ contract Voting is IForwarder, AragonApp {
     event StartVote(uint256 indexed voteId);
     event CastVote(uint256 indexed voteId, address indexed voter, bool supports, uint256 stake);
     event ExecuteVote(uint256 indexed voteId);
+    event ChangeSupportRequired(uint256 supportRequiredPct);
     event ChangeMinQuorum(uint256 minAcceptQuorumPct);
 
     modifier voteExists(uint256 _voteId) {
@@ -85,6 +88,21 @@ contract Voting is IForwarder, AragonApp {
         voteTime = _voteTime;
     }
 
+    /**
+    * @notice Change required support to `(_supportRequiredPct - _supportRequiredPct % 10^16) / 10^14`%
+    * @param _supportRequiredPct New required support
+    */
+    function changeSupportRequiredPct(uint256 _supportRequiredPct)
+        external
+        authP(MODIFY_SUPPORT_ROLE, arr(_supportRequiredPct, supportRequiredPct))
+    {
+        require(minAcceptQuorumPct <= _supportRequiredPct);
+        require(_supportRequiredPct <= PCT_BASE);
+        supportRequiredPct = _supportRequiredPct;
+
+        emit ChangeSupportRequired(_supportRequiredPct);
+    }
+
     /**
     * @notice Change minimum acceptance quorum to `(_minAcceptQuorumPct - _minAcceptQuorumPct % 10^16) / 10^14`%
     * @param _minAcceptQuorumPct New acceptance quorum
@@ -178,7 +196,7 @@ contract Voting is IForwarder, AragonApp {
         }
 
         // Voting is already decided
-        if (_isValuePct(vote_.yea, vote_.totalVoters, supportRequiredPct)) {
+        if (_isValuePct(vote_.yea, vote_.totalVoters, vote_.supportRequiredPct)) {
             return true;
         }
 
@@ -189,7 +207,7 @@ contract Voting is IForwarder, AragonApp {
             return false;
         }
         // Has enough support?
-        if (!_isValuePct(vote_.yea, totalVotes, supportRequiredPct)) {
+        if (!_isValuePct(vote_.yea, totalVotes, vote_.supportRequiredPct)) {
             return false;
         }
         // Has min quorum?
@@ -210,6 +228,7 @@ contract Voting is IForwarder, AragonApp {
             address creator,
             uint64 startDate,
             uint256 snapshotBlock,
+            uint256 supportRequired,
             uint256 minAcceptQuorum,
             uint256 yea,
             uint256 nay,
@@ -224,6 +243,7 @@ contract Voting is IForwarder, AragonApp {
         creator = vote_.creator;
         startDate = vote_.startDate;
         snapshotBlock = vote_.snapshotBlock;
+        supportRequired = vote_.supportRequiredPct;
         minAcceptQuorum = vote_.minAcceptQuorumPct;
         yea = vote_.yea;
         nay = vote_.nay;
@@ -251,6 +271,7 @@ contract Voting is IForwarder, AragonApp {
         vote_.metadata = _metadata;
         vote_.snapshotBlock = getBlockNumber() - 1; // avoid double voting in this very block
         vote_.totalVoters = token.totalSupplyAt(vote_.snapshotBlock);
+        vote_.supportRequiredPct = supportRequiredPct;
         vote_.minAcceptQuorumPct = minAcceptQuorumPct;
 
         emit StartVote(voteId);

apps/voting/test/voting.js

@@ -32,7 +32,7 @@ contract('Voting App', accounts => {
     let votingBase, daoFact, voting, token, executionTarget
 
     let APP_MANAGER_ROLE
-    let CREATE_VOTES_ROLE, MODIFY_QUORUM_ROLE
+    let CREATE_VOTES_ROLE, MODIFY_SUPPORT_ROLE, MODIFY_QUORUM_ROLE
 
     const votingTime = 1000
     const root = accounts[0]
@@ -47,6 +47,7 @@ contract('Voting App', accounts => {
         // Setup constants
         APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
         CREATE_VOTES_ROLE = await votingBase.CREATE_VOTES_ROLE()
+        MODIFY_SUPPORT_ROLE = await votingBase.MODIFY_SUPPORT_ROLE()
         MODIFY_QUORUM_ROLE = await votingBase.MODIFY_QUORUM_ROLE()
     })
 
@@ -61,6 +62,7 @@ contract('Voting App', accounts => {
         voting = Voting.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
 
         await acl.createPermission(ANY_ADDR, voting.address, CREATE_VOTES_ROLE, root, { from: root })
+        await acl.createPermission(ANY_ADDR, voting.address, MODIFY_SUPPORT_ROLE, root, { from: root })
         await acl.createPermission(ANY_ADDR, voting.address, MODIFY_QUORUM_ROLE, root, { from: root })
     })
 
@@ -144,12 +146,32 @@ contract('Voting App', accounts => {
             assert.equal(voteId, 0, 'voting should have been created')
         })
 
+        it('can change required support', async () => {
+            const receipt = await voting.changeSupportRequiredPct(neededSupport.add(1))
+            const events = receipt.logs.filter(x => x.event == 'ChangeSupportRequired')
+
+            assert.equal(events.length, 1, 'should have emitted ChangeSupportRequired event')
+            assert.equal((await voting.supportRequiredPct()).toString(), neededSupport.add(1).toString(), 'should have changed required support')
+        })
+
+        it('fails changing required support lower than minimum acceptance quorum', async () => {
+            return assertRevert(async () => {
+                await voting.changeSupportRequiredPct(minimumAcceptanceQuorum.minus(1))
+            })
+        })
+
+        it('fails changing required support to greater than 100%', async () => {
+            return assertRevert(async () => {
+                await voting.changeSupportRequiredPct(pct16(101))
+            })
+        })
+
         it('can change minimum acceptance quorum', async () => {
             const receipt = await voting.changeMinAcceptQuorumPct(1)
             const events = receipt.logs.filter(x => x.event == 'ChangeMinQuorum')
 
             assert.equal(events.length, 1, 'should have emitted ChangeMinQuorum event')
-            assert.equal(await voting.minAcceptQuorumPct(), 1, 'should have change acceptance quorum')
+            assert.equal(await voting.minAcceptQuorumPct(), 1, 'should have changed acceptance quorum')
         })
 
         it('fails changing minimum acceptance quorum to zero', async () => {
@@ -174,13 +196,14 @@ contract('Voting App', accounts => {
             })
 
             it('has correct state', async () => {
-                const [isOpen, isExecuted, creator, startDate, snapshotBlock, minQuorum, y, n, totalVoters, execScript] = await voting.getVote(voteId)
+                const [isOpen, isExecuted, creator, startDate, snapshotBlock, supportRequired, minQuorum, y, n, totalVoters, execScript] = await voting.getVote(voteId)
 
                 assert.isTrue(isOpen, 'vote should be open')
                 assert.isFalse(isExecuted, 'vote should not be executed')
                 assert.equal(creator, holder50, 'creator should be correct')
                 assert.equal(snapshotBlock, await getBlockNumber() - 1, 'snapshot block should be correct')
-                assert.deepEqual(minQuorum, minimumAcceptanceQuorum, 'min quorum should be app min quorum')
+                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(y, 0, 'initial yea should be 0')
                 assert.equal(n, 0, 'initial nay should be 0')
                 assert.equal(totalVoters, 100, 'total voters should be 100')
@@ -195,19 +218,35 @@ contract('Voting App', accounts => {
                 })
             })
 
+          it('changing required support does not affect vote required support', async () => {
+                await voting.changeSupportRequiredPct(pct16(70))
+
+                // With previous required support at 50%, vote should be approved
+                // with new quorum at 70% it shouldn't have, but since min quorum is snapshotted
+                // it will succeed
+
+                await voting.vote(voteId, true, false, { from: holder50 })
+                await voting.vote(voteId, true, false, { from: holder19 })
+                await voting.vote(voteId, false, false, { from: holder31 })
+                await timeTravel(votingTime + 1)
+
+                const state = await voting.getVote(voteId)
+                assert.equal(state[5].toNumber(), neededSupport.toNumber(), 'required support in vote should stay equal')
+                await voting.executeVote(voteId) // exec doesn't fail
+            })
+
             it('changing min quorum doesnt affect vote min quorum', async () => {
                 await voting.changeMinAcceptQuorumPct(pct16(50))
 
                 // With previous min acceptance quorum at 20%, vote should be approved
                 // with new quorum at 50% it shouldn't have, but since min quorum is snapshotted
                 // it will succeed
 
-
                 await voting.vote(voteId, true, true, { from: holder31 })
                 await timeTravel(votingTime + 1)
 
                 const state = await voting.getVote(voteId)
-                assert.deepEqual(state[5], minimumAcceptanceQuorum, 'acceptance quorum in vote should stay equal')
+                assert.equal(state[6].toNumber(), minimumAcceptanceQuorum.toNumber(), 'acceptance quorum in vote should stay equal')
                 await voting.executeVote(voteId) // exec doesn't fail
             })
 
@@ -216,7 +255,7 @@ contract('Voting App', accounts => {
                 const state = await voting.getVote(voteId)
                 const voterState = await voting.getVoterState(voteId, holder31)
 
-                assert.equal(state[7], 31, 'nay vote should have been counted')
+                assert.equal(state[8], 31, 'nay vote should have been counted')
                 assert.equal(voterState, VOTER_STATE.NAY, 'holder31 should have nay voter status')
             })
 
@@ -226,8 +265,8 @@ contract('Voting App', accounts => {
                 await voting.vote(voteId, true, true, { from: holder31 })
                 const state = await voting.getVote(voteId)
 
-                assert.equal(state[6], 31, 'yea vote should have been counted')
-                assert.equal(state[7], 0, 'nay vote should have been removed')
+                assert.equal(state[7], 31, 'yea vote should have been counted')
+                assert.equal(state[8], 0, 'nay vote should have been removed')
             })
 
             it('token transfers dont affect voting', async () => {
@@ -236,7 +275,7 @@ contract('Voting App', accounts => {
                 await voting.vote(voteId, true, true, { from: holder31 })
                 const state = await voting.getVote(voteId)
 
-                assert.equal(state[6], 31, 'yea vote should have been counted')
+                assert.equal(state[7], 31, 'yea vote should have been counted')
                 assert.equal(await token.balanceOf(holder31), 0, 'balance should be 0 at current block')
             })