Buyers buying at higher than declared dollar price due to rounding issue

[adamkolar]

Because solidity rounds down during division, calculating jcrEthRate in this function:

  function receiveEthPrice(uint ethUsdPrice) external onlyEthPriceProvider {
    require(ethUsdPrice > 0);
    jcrEthRate = ethUsdPrice.div(jcrUsdRate);
  }

effectively strips down cents from current ETH/USD rate, so 300.99 becomes 300. This means that in the worst case, buyers can lose out on about 0.33% tokens at the current ETH price. If the price of ETH goes down, situation gets worse, at 200 ETH/USD the loss can be 0.5% at 100 ETH/USD, it becomes 1% and so on. Or to put it in other terms, a buyer who invests 1000 ETH can lose out on $1000 worth of tokens due to unfortunate timing. To mitigate this I propose the following fixes. Store ethUsdPrice in cents upon receiving it:

  function receiveEthPrice(uint ethUsdPrice) external onlyEthPriceProvider {
    require(ethUsdPrice > 0);
    ethUsdPriceStorage = ethUsdPrice;
  }

and move the ETH/USD conversion into the doPurchase function:

    uint tokens = msg.value.mul(ethUsdPriceStorage).div(jcrUsdRate);

in this way, the maximal loss due to rounding is limited to $1 per buy.

[artemii235]

Hello @adamkolar!

Thank you for opening an issue. It's good suggestion, we will implement it in next PR.