daostack / infra

GNU General Public License v3.0
25 stars 22 forks source link

AbsoluteVote doesnt work when total reputation<100 #89

Closed sirpy closed 4 years ago

sirpy commented 4 years ago

in the method _execute you do DIV before MUL if(votes > totalReputation/100*precReq) this of course becomes 0 when totalReputation<100 always mul before div

orenyodfat commented 4 years ago

this done in purpose to prevent overflow https://github.com/daostack/infra/blob/master/contracts/votingMachines/GenesisProtocolLogic.sol#L511 . yes, if total rep < 100 it will not work , though normally, and in the very most of the cases, total rep will be higher than 100. reputation is in wei units.

sirpy commented 4 years ago

i would say that the overflow multiplying by 0-100 is the extreme case. but ok. in any case dont you think a require is needed in order to check for overflow/underflow? like using SafeMath? or force DAO to have at least 100 rep?

orenyodfat commented 4 years ago

overflow of multiple total reputation in wei uints 1 rep = 10^18 is not so extreme case. adding a require for the very edge case of total rep<100 is redundant and not worth the addition gas for the check on each call. divided by 100 and then multiple by a value which is capped by 100 (params.queuedVoteRequiredPercentage) will do the work.