diff --git a/apps/voting/contracts/Voting.sol b/apps/voting/contracts/Voting.sol index baa57f8cf0..1ede249a27 100644 --- a/apps/voting/contracts/Voting.sol +++ b/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; } } diff --git a/apps/voting/test/mocks/VotingMock.sol b/apps/voting/test/mocks/VotingMock.sol new file mode 100644 index 0000000000..d4a83e9915 --- /dev/null +++ b/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); + } +} diff --git a/apps/voting/test/voting.js b/apps/voting/test/voting.js index a044654f50..df76ccf6e8 100644 --- a/apps/voting/test/voting.js +++ b/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") + }) + }) })