Fix comments from chain security 2nd audit (Draft)

- remove comments describing reserve Id first byte as being reserve type.
- remove unused file import in kyberDao.
- move setDecimals call to avoid un necessary trigger.
- remove rateWithNetworkFee from TradeData struct. since its is unused 
- fix revert message for reward + rebate limit
- remove unused 'bool add' from event AddReserveToStorage
- align casting for reserveId 0. from '0' --> 'bytes32(0)'

Author: ilanDoron

contracts/sol6/Dao/KyberDao.sol

@@ -7,8 +7,6 @@ import "../IERC20.sol";
 import "../IKyberDao.sol";
 import "../utils/zeppelin/ReentrancyGuard.sol";
 import "../utils/Utils5.sol";
-import "../IKyberFeeHandler.sol";
-
 
 /**
  * @notice  This contract is using SafeMath for uint, which is inherited from EpochUtils
@@ -719,7 +717,7 @@ contract KyberDao is IKyberDao, EpochUtils, ReentrancyGuard, Utils5, DaoOperator
                     getRebateAndRewardFromData(options[i]);
                 require(
                     rewardInBps.add(rebateInBps) <= BPS,
-                    "validateParams: rebate + reward must be smaller then BPS"
+                    "validateParams: rebate + reward can't be bigger than BPS"
                 );
             }
         }

contracts/sol6/KyberMatchingEngine.sol

@@ -59,7 +59,7 @@ contract KyberMatchingEngine is KyberHintHandler, IKyberMatchingEngine, Withdraw
     /// @param dest Destination token
     /// @param isTokenToToken Whether the trade is token -> token
     /// @param hint Advanced instructions for running the trade
-    /// @return reserveIds Array of reserve IDs for the trade, each being 32 bytes. 1st byte is reserve type
+    /// @return reserveIds Array of reserve IDs for the trade, each being 32 bytes
     /// @return splitValuesBps Array of split values (in basis points) for the trade
     /// @return processWithRate Enum ProcessWithRate, whether extra processing is required or not
     function getTradingReserves(

contracts/sol6/KyberNetwork.sol

@@ -75,7 +75,6 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
         uint256 numEntitledRebateReserves;
         uint256 feeAccountedBps; // what part of this trade is fee paying. for token -> token - up to 200%
         uint256 entitledRebateBps;
-        uint256 rateWithNetworkFee;
     }
 
     struct TradeInput {
@@ -227,10 +226,10 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
 
         if (add) {
             token.safeApprove(reserve, 2**255);
+            setDecimals(token);
         } else {
             token.safeApprove(reserve, 0);
         }
-        setDecimals(token);
     }
 
     /// @notice Can be called only by operator
@@ -266,6 +265,7 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
         for(uint i = 0; i < reserves.length; i++) {
             if (add) {
                 token.safeApprove(reserves[i], 2**255);
+                setDecimals(token);
             } else {
                 token.safeApprove(reserves[i], 0);
             }

contracts/sol6/KyberStorage.sol

@@ -69,8 +69,7 @@ contract KyberStorage is IKyberStorage, PermissionGroupsNoModifiers, Utils5 {
         address indexed reserve,
         bytes32 indexed reserveId,
         IKyberStorage.ReserveType reserveType,
-        address indexed rebateWallet,
-        bool add
+        address indexed rebateWallet
     );
 
     event ReserveRebateWalletSet(
@@ -126,7 +125,7 @@ contract KyberStorage is IKyberStorage, PermissionGroupsNoModifiers, Utils5 {
     /// @notice Can be called only by operator
     /// @dev Adds a reserve to the storage
     /// @param reserve The reserve address
-    /// @param reserveId The reserve ID in 32 bytes. 1st byte is reserve type
+    /// @param reserveId The reserve ID in 32 bytes.
     /// @param resType Type of the reserve out of enum ReserveType
     /// @param rebateWallet Rebate wallet address for this reserve
     function addReserve(
@@ -137,7 +136,7 @@ contract KyberStorage is IKyberStorage, PermissionGroupsNoModifiers, Utils5 {
     ) external {
         onlyOperator();
         require(reserveAddressToId[reserve] == bytes32(0), "reserve has id");
-        require(reserveId != 0, "reserveId = 0");
+        require(reserveId != bytes32(0), "reserveId = 0");
         require(
             (resType != ReserveType.NONE) && (uint256(resType) < uint256(ReserveType.LAST)),
             "bad reserve type"
@@ -160,7 +159,7 @@ contract KyberStorage is IKyberStorage, PermissionGroupsNoModifiers, Utils5 {
         reserveAddressToId[reserve] = reserveId;
         reserveType[reserveId] = uint256(resType);
 
-        emit AddReserveToStorage(reserve, reserveId, resType, rebateWallet, true);
+        emit AddReserveToStorage(reserve, reserveId, resType, rebateWallet);
         emit ReserveRebateWalletSet(reserveId, rebateWallet);
     }
 
@@ -500,7 +499,7 @@ contract KyberStorage is IKyberStorage, PermissionGroupsNoModifiers, Utils5 {
     }
 
     /// @notice Returns information about a reserve given its reserve ID
-    /// @return reserveId The reserve ID in 32 bytes. 1st byte is reserve type
+    /// @return reserveId The reserve ID in 32 bytes.
     /// @return rebateWallet address of rebate wallet of this reserve
     /// @return resType Reserve type from enum ReserveType
     /// @return isFeeAccountedFlag Whether fees are to be charged for the trade for this reserve

test/sol6/kyberDao.js

@@ -1390,14 +1390,14 @@ contract('KyberDao', function(accounts) {
           2, currentBlock + 15, currentBlock + 15 + minCampPeriod, minPercentageInPrecision, cInPrecision, tInPrecision,
           [1, getDataFromRebateAndReward(100, 10001 - 100), 2, 3], '0x', {from: daoOperator}
         ),
-        "validateParams: rebate + reward must be smaller then BPS"
+        "validateParams: rebate + reward can't be bigger than BPS"
       )
       await expectRevert(
         submitNewCampaign(kyberDao,
           2, currentBlock + 17, currentBlock + 17 + minCampPeriod, minPercentageInPrecision, cInPrecision, tInPrecision,
           [1, 2, getDataFromRebateAndReward(20, 10000)], '0x', {from: daoOperator}
         ),
-        "validateParams: rebate + reward must be smaller then BPS"
+        "validateParams: rebate + reward can't be bigger than BPS"
       )
       await submitNewCampaign(kyberDao,
         2, currentBlock + 19, currentBlock + 19 + minCampPeriod, minPercentageInPrecision, cInPrecision, tInPrecision,

test/sol6/kyberStorage.js

@@ -275,8 +275,7 @@ contract('KyberStorage', function(accounts) {
                 reserve: mockReserve.address,
                 reserveId: nwHelper.genReserveID(MOCK_ID, mockReserve.address).toLowerCase(),
                 reserveType: new BN(ReserveType.FPR),
-                rebateWallet: anyWallet,
-                add: true
+                rebateWallet: anyWallet
             });
             expectEvent(txResult, 'ReserveRebateWalletSet', {
                 reserveId: nwHelper.genReserveID(MOCK_ID, mockReserve.address).toLowerCase(),