Merge pull request #300 from aragon/voting_minor_improvements

Voting minor improvements

apps/voting/contracts/Voting.sol

@@ -277,20 +277,19 @@ contract Voting is IForwarder, AragonApp {
     }
 
     function _isVoteOpen(Vote storage vote) internal view returns (bool) {
-        return uint64(now) < (vote.startDate.add(voteTime)) && !vote.executed;
+        return uint64(now) < vote.startDate.add(voteTime) && !vote.executed;
     }
 
     /**
     * @dev Calculates whether `_value` is at least a percentage `_pct` of `_total`
     */
     function _isValuePct(uint256 _value, uint256 _total, uint256 _pct) internal pure returns (bool) {
-        if (_value == 0 && _total > 0)
+        if (_total == 0) {
             return false;
+        }
 
-        uint256 m = _total.mul(_pct);
-        uint256 v = m / PCT_BASE;
+        uint256 computedPct = _value.mul(PCT_BASE) / _total;
 
-        // If division is exact, allow same value, otherwise require value to be greater
-        return m % PCT_BASE == 0 ? _value >= v : _value > v;
+        return computedPct >= _pct;
     }
 }

apps/voting/test/mocks/VotingMock.sol

@@ -0,0 +1,11 @@
+pragma solidity 0.4.18;
+
+import "../../contracts/Voting.sol";
+
+
+contract VotingMock is Voting {
+    // _isValuePct public wrapper
+    function isValuePct(uint256 _value, uint256 _total, uint256 _pct) external pure returns (bool) {
+        return _isValuePct(_value, _total, _pct);
+    }
+}

apps/voting/test/voting.js

@@ -371,4 +371,26 @@ contract('Voting App', accounts => {
             })
         })
     })
+
+    context('isValuePct unit test', async () => {
+        let votingMock
+
+        before(async () => {
+            votingMock = await getContract('VotingMock').new()
+        })
+
+        it('tests total = 0', async () => {
+            const result1 = await votingMock.isValuePct(0, 0, pct16(50))
+            assert.equal(result1, false, "total 0 should always return false")
+            const result2 = await votingMock.isValuePct(1, 0, pct16(50))
+            assert.equal(result2, false, "total 0 should always return false")
+        })
+
+        it('tests value = 0', async () => {
+            const result1 = await votingMock.isValuePct(0, 10, pct16(50))
+            assert.equal(result1, false, "value 0 should false if pct is non-zero")
+            const result2 = await votingMock.isValuePct(0, 10, 0)
+            assert.equal(result2, true, "value 0 should return true if pct is zero")
+        })
+    })
 })