Issues with calculateBonus() method

 function calculateBonus(uint tokens) internal constant returns (uint bonus) {
    if (msg.value < 100 * (1 ether)) {
      return 0;
    }

    if (msg.value >= 100 * (1 ether) && msg.value < 250 * (1 ether)) {
      return tokens.div(100).mul(5);
    }

    if (msg.value >= 250 * (1 ether) && msg.value < 500 * (1 ether)) {
      return tokens.div(100).mul(7);
    }

    if (msg.value >= 500 * (1 ether) && msg.value < 1000 * (1 ether)) {
      return tokens.div(100).mul(10);
    }

    if (msg.value >= 1000 * (1 ether) && msg.value < 2000 * (1 ether)) {
      return tokens.div(1000).mul(125);
    }

    if (msg.value >= 2000 * (1 ether) && msg.value < 5000 * (1 ether)) {
      return tokens.div(100).mul(15);
    }

    if (msg.value >= 5000 * (1 ether)) {
      return tokens.div(100).mul(20);
    }
  }

Multiplication before division for more precision in the calculated result. See the following sample calculation.

pragma solidity ^0.4.16;

contract Test {
    uint public hardCap = 57142e18;
    uint public calc1;
    uint public calc2;

    function Test() public {
        // Result 57142000000000000000000
        calc1 = ((hardCap / 1000) * 999);
        // Result 57084858000000000000000
        calc2 = hardCap * 999 / 1000;
    }
}

The whole function can be refactored to require less computation. My suggestion:

function calculateBonus(uint tokens) internal constant returns (uint bonus) {
uint weiAmount = msg.value;

if (weiAmount >= 5000 ether) {
  return tokens.mul(20).div(100);
}

if (weiAmount >= 2000 ether) {
  return tokens.mul(15).div(100);
}

if (weiAmount >= 1000 ether) {
  return tokens.mul(125).div(1000);
}

if (weiAmount >= 500 ether) {
  return tokens.mul(10).div(100);
}

if (weiAmount >= 250 ether) {
  return tokens.mul(7).div(100);
}

if (weiAmount >= 100 ether) {
  return tokens.mul(5).div(100);
}

return 0;
}

JincorTech / ico

Issues with calculateBonus() method #45