Two-phase voting

apps/voting/contracts/Voting.sol

@@ -58,13 +58,16 @@ contract Voting is IForwarder, AragonApp {
     // We are mimicing an array, we use a mapping instead to make app upgrade more graceful
     mapping (uint256 => Vote) internal votes;
     uint256 public votesLength;
+    uint64 public objectionTime;
 
     event StartVote(uint256 indexed voteId, address indexed creator, string metadata);
     event CastVote(uint256 indexed voteId, address indexed voter, bool supports, uint256 stake);
+    event CastObjection(uint256 indexed voteId, address indexed voter, uint256 stake);
     event ExecuteVote(uint256 indexed voteId);
     event ChangeSupportRequired(uint64 supportRequiredPct);
     event ChangeMinQuorum(uint64 minAcceptQuorumPct);
     event ChangeVoteTime(uint64 voteTime);
+    event ChangeObjectionTime(uint64 objectionTime);
 
     modifier voteExists(uint256 _voteId) {
         require(_voteId < votesLength, ERROR_NO_VOTE);
@@ -76,9 +79,13 @@ contract Voting is IForwarder, AragonApp {
     * @param _token MiniMeToken Address that will be used as governance token
     * @param _supportRequiredPct Percentage of yeas in casted votes for a vote to succeed (expressed as a percentage of 10^18; eg. 10^16 = 1%, 10^18 = 100%)
     * @param _minAcceptQuorumPct Percentage of yeas in total possible votes for a vote to succeed (expressed as a percentage of 10^18; eg. 10^16 = 1%, 10^18 = 100%)
-    * @param _voteTime Seconds that a vote will be open for token holders to vote (unless enough yeas or nays have been cast to make an early decision)
+    * @param _voteTime The duration of the main vote phase, i.e. seconds that a vote will be open for token holders to both support and object to the outcome
+    * @param _objectionTime The duration of the objection vote phase, i.e. seconds that a vote will be open after the main vote phase ends for token holders to object to the outcome
     */
-    function initialize(MiniMeToken _token, uint64 _supportRequiredPct, uint64 _minAcceptQuorumPct, uint64 _voteTime) external onlyInit {
+    function initialize(MiniMeToken _token, uint64 _supportRequiredPct, uint64 _minAcceptQuorumPct, uint64 _voteTime, uint64 _objectionTime)
+        external
+        onlyInit
+    {
         initialized();
 
         require(_minAcceptQuorumPct <= _supportRequiredPct, ERROR_INIT_PCTS);
@@ -88,6 +95,7 @@ contract Voting is IForwarder, AragonApp {
         supportRequiredPct = _supportRequiredPct;
         minAcceptQuorumPct = _minAcceptQuorumPct;
         voteTime = _voteTime;
+        objectionTime = _objectionTime;
     }
 
     /**
@@ -125,50 +133,65 @@ contract Voting is IForwarder, AragonApp {
     */
     function unsafelyChangeVoteTime(uint64 _voteTime)
         external
-        authP(UNSAFELY_MODIFY_VOTE_TIME_ROLE, arr(uint256(_voteTime), uint256(voteTime)))
+        auth(UNSAFELY_MODIFY_VOTE_TIME_ROLE)
     {
         voteTime = _voteTime;
 
         emit ChangeVoteTime(_voteTime);
     }
 
+    /**
+    * @notice Change the objection phase duration to `_objectionTime` sec. The change affects all existing unexecuted votes, so be really careful with it
+    * @param _objectionTime New objection time
+    */
+    function unsafelyChangeObjectionTime(uint64 _objectionTime)
+        external
+        auth(UNSAFELY_MODIFY_VOTE_TIME_ROLE)
+    {
+        objectionTime = _objectionTime;
+
+        emit ChangeObjectionTime(_objectionTime);
+    }
+
     /**
     * @notice Create a new vote about "`_metadata`"
     * @param _executionScript EVM script to be executed on approval
     * @param _metadata Vote metadata
     * @return voteId Id for newly created vote
     */
     function newVote(bytes _executionScript, string _metadata) external auth(CREATE_VOTES_ROLE) returns (uint256 voteId) {
-        return _newVote(_executionScript, _metadata, true, true);
+        return _newVote(_executionScript, _metadata, true);
     }
 
     /**
     * @notice Create a new vote about "`_metadata`"
+    * @dev  _executesIfDecided was deprecated to introduce a proper lock period between decision and execution.
     * @param _executionScript EVM script to be executed on approval
     * @param _metadata Vote metadata
     * @param _castVote Whether to also cast newly created vote
-    * @param _executesIfDecided Whether to also immediately execute newly created vote if decided
+    * @param _executesIfDecided_deprecated Whether to also immediately execute newly created vote if decided
     * @return voteId id for newly created vote
     */
-    function newVote(bytes _executionScript, string _metadata, bool _castVote, bool _executesIfDecided)
+    function newVote(bytes _executionScript, string _metadata, bool _castVote, bool _executesIfDecided_deprecated)
         external
         auth(CREATE_VOTES_ROLE)
         returns (uint256 voteId)
     {
-        return _newVote(_executionScript, _metadata, _castVote, _executesIfDecided);
+        return _newVote(_executionScript, _metadata, _castVote);
     }
 
     /**
-    * @notice Vote `_supports ? 'yes' : 'no'` in vote #`_voteId`
+    * @notice Vote `_supports ? 'yes' : 'no'` in vote #`_voteId`. During objection phase one can only vote 'no'
     * @dev Initialization check is implicitly provided by `voteExists()` as new votes can only be
     *      created via `newVote(),` which requires initialization
+    * @dev  _executesIfDecided was deprecated to introduce a proper lock period between decision and execution.
     * @param _voteId Id for vote
     * @param _supports Whether voter supports the vote
-    * @param _executesIfDecided Whether the vote should execute its action if it becomes decided
+    * @param _executesIfDecided_deprecated Whether the vote should execute its action if it becomes decided
     */
-    function vote(uint256 _voteId, bool _supports, bool _executesIfDecided) external voteExists(_voteId) {
-        require(_canVote(_voteId, msg.sender), ERROR_CAN_NOT_VOTE);
-        _vote(_voteId, _supports, msg.sender, _executesIfDecided);
+    function vote(uint256 _voteId, bool _supports, bool _executesIfDecided_deprecated) external voteExists(_voteId) {
+        require(_canVote(_voteId, msg.sender) || (!_supports && _canObject(_voteId, msg.sender)), ERROR_CAN_NOT_VOTE);
+        _vote(_voteId, _supports, msg.sender);
     }
 
     /**
@@ -199,7 +222,7 @@ contract Voting is IForwarder, AragonApp {
     */
     function forward(bytes _evmScript) public {
         require(canForward(msg.sender, _evmScript), ERROR_CAN_NOT_FORWARD);
-        _newVote(_evmScript, "", true, true);
+        _newVote(_evmScript, "", true);
     }
 
     /**
@@ -226,19 +249,29 @@ contract Voting is IForwarder, AragonApp {
     }
 
     /**
-    * @notice Tells whether `_sender` can participate in the vote #`_voteId` or not
+    * @notice Tells whether `_sender` can participate in the main phase of the vote #`_voteId`
     * @dev Initialization check is implicitly provided by `voteExists()` as new votes can only be
     *      created via `newVote(),` which requires initialization
-    * @return True if the given voter can participate a certain vote, false otherwise
+    * @return True if the given voter can participate in the main phase of a certain vote, false otherwise
     */
     function canVote(uint256 _voteId, address _voter) public view voteExists(_voteId) returns (bool) {
         return _canVote(_voteId, _voter);
     }
 
+    /**
+    * @notice Tells whether `_sender` can participate in the objection phase of the vote #`_voteId`
+    * @dev Initialization check is implicitly provided by `voteExists()` as new votes can only be
+    *      created via `newVote(),` which requires initialization
+    * @return True if the given voter can participate in the objection phase of a certain vote, false otherwise
+    */
+    function canObject(uint256 _voteId, address _voter) public view voteExists(_voteId) returns (bool) {
+        return _canObject(_voteId, _voter);
+    }
+
     /**
     * @dev Return all information for a vote by its ID
     * @param _voteId Vote identifier
-    * @return Vote open status
+    * @return true if the vote open for both support and objection
     * @return Vote executed status
     * @return Vote start date
     * @return Vote snapshot block
@@ -248,6 +281,7 @@ contract Voting is IForwarder, AragonApp {
     * @return Vote nays amount
     * @return Vote power
     * @return Vote script
+    * @return true if the vote open for objection
     */
     function getVote(uint256 _voteId)
         public
@@ -263,7 +297,8 @@ contract Voting is IForwarder, AragonApp {
             uint256 yea,
             uint256 nay,
             uint256 votingPower,
-            bytes script
+            bytes script,
+            bool openForObjection
         )
     {
         Vote storage vote_ = votes[_voteId];
@@ -278,6 +313,7 @@ contract Voting is IForwarder, AragonApp {
         nay = vote_.nay;
         votingPower = vote_.votingPower;
         script = vote_.executionScript;
+        openForObjection = _isVoteOpenForObjection(vote_);
     }
 
     /**
@@ -295,7 +331,7 @@ contract Voting is IForwarder, AragonApp {
     * @dev Internal function to create a new vote
     * @return voteId id for newly created vote
     */
-    function _newVote(bytes _executionScript, string _metadata, bool _castVote, bool _executesIfDecided) internal returns (uint256 voteId) {
+    function _newVote(bytes _executionScript, string _metadata, bool _castVote) internal returns (uint256 voteId) {
         uint64 snapshotBlock = getBlockNumber64() - 1; // avoid double voting in this very block
         uint256 votingPower = token.totalSupplyAt(snapshotBlock);
         require(votingPower > 0, ERROR_NO_VOTING_POWER);
@@ -313,14 +349,15 @@ contract Voting is IForwarder, AragonApp {
         emit StartVote(voteId, msg.sender, _metadata);
 
         if (_castVote && _canVote(voteId, msg.sender)) {
-            _vote(voteId, true, msg.sender, _executesIfDecided);
+            _vote(voteId, true, msg.sender);
         }
     }
 
     /**
-    * @dev Internal function to cast a vote. It assumes the queried vote exists.
+    * @dev Internal function to cast a vote or object to.
+      @dev It assumes that voter can support or object to the vote
     */
-    function _vote(uint256 _voteId, bool _supports, address _voter, bool _executesIfDecided) internal {
+    function _vote(uint256 _voteId, bool _supports, address _voter) internal {
         Vote storage vote_ = votes[_voteId];
 
         // This could re-enter, though we can assume the governance token is not malicious
@@ -344,9 +381,8 @@ contract Voting is IForwarder, AragonApp {
 
         emit CastVote(_voteId, _voter, _supports, voterStake);
 
-        if (_executesIfDecided && _canExecute(_voteId)) {
-            // We've already checked if the vote can be executed with `_canExecute()`
-            _unsafeExecuteVote(_voteId);
+        if (!_isVoteOpen(vote_)) { // objection phase
+            emit CastObjection(_voteId, _voter, voterStake);
         }
     }
 
@@ -383,15 +419,16 @@ contract Voting is IForwarder, AragonApp {
             return false;
         }
 
-        // Voting is already decided
-        if (_isValuePct(vote_.yea, vote_.votingPower, vote_.supportRequiredPct)) {
-            return true;
-        }
-
         // Vote ended?
         if (_isVoteOpen(vote_)) {
             return false;
         }
+
+        // Objection time ended?
+        if (_isVoteOpenForObjection(vote_)) {
+            return false;
+        }
+
         // Has enough support?
         uint256 totalVotes = vote_.yea.add(vote_.nay);
         if (!_isValuePct(vote_.yea, totalVotes, vote_.supportRequiredPct)) {
@@ -414,14 +451,31 @@ contract Voting is IForwarder, AragonApp {
         return _isVoteOpen(vote_) && token.balanceOfAt(_voter, vote_.snapshotBlock) > 0;
     }
 
+    /**
+    * @dev Internal function to check if a voter can at least object to a vote outcome. It assumes the queried vote exists.
+    * @return True if the given voter can object to a certain vote outcome, false otherwise
+    */
+    function _canObject(uint256 _voteId, address _voter) internal view returns (bool) {
+        Vote storage vote_ = votes[_voteId];
+        return _isVoteOpenForObjection(vote_) && token.balanceOfAt(_voter, vote_.snapshotBlock) > 0;
+    }
+
     /**
     * @dev Internal function to check if a vote is still open
-    * @return True if the given vote is open, false otherwise
+    * @return True if less than voteTime has passed since the vote start
     */
     function _isVoteOpen(Vote storage vote_) internal view returns (bool) {
         return getTimestamp64() < vote_.startDate.add(voteTime) && !vote_.executed;
     }
 
+    /**
+    * @dev Internal function to check if a vote is still possible to object
+    * @return True if less than voteTime + objectionTime has passed since the vote start
+    */
+    function _isVoteOpenForObjection(Vote storage vote_) internal view returns (bool) {
+        return getTimestamp64() < vote_.startDate.add(voteTime).add(objectionTime) && !vote_.executed;
+    }
+
     /**
     * @dev Calculates whether `_value` is more than a percentage `_pct` of `_total`
     */

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

@@ -13,7 +13,7 @@ contract VotingMock is Voting, TimeHelpersMock {
         external
         returns (uint256 voteId)
     {
-        voteId = _newVote(_executionScript, _metadata, _castVote, _executesIfDecided);
+        voteId = _newVote(_executionScript, _metadata, _castVote);
         emit StartVote(voteId, msg.sender, _metadata);
     }
 
@@ -30,7 +30,7 @@ contract VotingMock is Voting, TimeHelpersMock {
         token.generateTokens(_holder, _tokenAmount);
 
         bytes memory noScript = new bytes(0);
-        voteId = _newVote(noScript, _metadata, false, false);
+        voteId = _newVote(noScript, _metadata, false);
         emit StartVote(voteId, msg.sender, _metadata);
     }
 }