Fix rbate handling for case where reserve is feeAccounted but not entitledRebate (#1030)

* For split trade 50:50 with 2 reserves fee entitled. out of them one is entitled rebate.
	current: 1 entitled rebate reserve will recieve all rebate amount
	fix: entitled rebate reserve will recieve rebate amount according to its split in the trade

Co-authored-by: Ilan Doron <[email protected]>

Author: Anyhowclick

contracts/sol6/KyberNetwork.sol

@@ -62,8 +62,6 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
     /// @param networkFeeBps Network fee bps determined by kyberDao, or default value
     /// @param numEntitledRebateReserves No. of reserves that are eligible for rebates
     /// @param feeAccountedBps Proportion of this trade that fee is accounted to, in BPS. Up to 2 * BPS
-    /// @param entitledRebateBps Proportion of reserves entitled for rebate, in BPS. Up to 2 * BPS
-    /// @param rateWithNetworkFee src -> dest token rate, after accounting for only network fee
     struct TradeData {
         TradeInput input;
         ReservesData tokenToEth;
@@ -74,7 +72,6 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
         uint256 networkFeeBps;
         uint256 numEntitledRebateReserves;
         uint256 feeAccountedBps; // what part of this trade is fee paying. for token -> token - up to 200%
-        uint256 entitledRebateBps;
     }
 
     struct TradeInput {
@@ -979,7 +976,7 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
             rebatePercentBps,
             tradeData.tokenToEth,
             index,
-            tradeData.entitledRebateBps
+            tradeData.feeAccountedBps
         );
 
         // eth -> token
@@ -988,7 +985,7 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
             rebatePercentBps,
             tradeData.ethToToken,
             index,
-            tradeData.entitledRebateBps
+            tradeData.feeAccountedBps
         );
 
         rebateWallets = kyberStorage.getRebateWalletsFromIds(rebateReserveIds);
@@ -999,14 +996,14 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
         uint256[] memory rebatePercentBps,
         ReservesData memory reservesData,
         uint256 index,
-        uint256 entitledRebateBps
+        uint256 feeAccountedBps
     ) internal pure returns (uint256) {
         uint256 _index = index;
 
         for (uint256 i = 0; i < reservesData.isEntitledRebateFlags.length; i++) {
             if (reservesData.isEntitledRebateFlags[i]) {
                 rebateReserveIds[_index] = reservesData.ids[i];
-                rebatePercentBps[_index] = (reservesData.splitsBps[i] * BPS) / entitledRebateBps;
+                rebatePercentBps[_index] = (reservesData.splitsBps[i] * BPS) / feeAccountedBps;
                 _index++;
             }
         }
@@ -1130,7 +1127,6 @@ contract KyberNetwork is WithdrawableNoModifiers, Utils5, IKyberNetwork, Reentra
                 tradeData.feeAccountedBps += reservesData.splitsBps[i];
 
                 if (reservesData.isEntitledRebateFlags[i]) {
-                    tradeData.entitledRebateBps += reservesData.splitsBps[i];
                     tradeData.numEntitledRebateReserves++;
                 }
             }

solcOptimiserSettings.js

@@ -1,4 +1,4 @@
 module.exports = {
     enabled: true,
-    runs: 410
+    runs: 430
 }

test/sol6/kyberNetwork.js

@@ -2399,7 +2399,7 @@ contract('KyberNetwork', function(accounts) {
         });
     });
 
-    describe("test fee handler integrations with 1 mock and 1 apr", async() => {
+    describe("test fee handler integrations with 1 mock and 1 fpr", async() => {
         let platformFee = new BN(200);
         let reserveIdToWallet = [];
         let rebateWallets;
@@ -2461,22 +2461,17 @@ contract('KyberNetwork', function(accounts) {
             platformFeeWei = tradeWei.mul(platfromFeeBps).div(BPS);
             Helper.assertEqual(await feeHandler.feePerPlatformWallet(platformWallet), beforePlatformFee.add(platformFeeWei), "unexpected rebate value");
             networkFeeWei = tradeWei.mul(networkFeeBps).div(BPS).mul(feeAccountedBps).div(BPS);
-            let hasRebate = false;
+            rebateWei = zeroBN;
             for (const [rebateWallet, beforeBalance] of Object.entries(beforeRebate)) {
                 if (rebateWallet in rebatePerWallet) {
+                    rebateWei = rebateWei.add(rebatePerWallet[rebateWallet]);
                     Helper.assertApproximate(await feeHandler.rebatePerWallet(rebateWallet), beforeBalance.add(rebatePerWallet[rebateWallet]), "unexpected rebate value");
-                    hasRebate = true;
                 }else {
                     Helper.assertApproximate(await feeHandler.rebatePerWallet(rebateWallet), beforeBalance, "unexpected rebate value");
                 }
             }
-            if (hasRebate) {
-                totalPayout = platformFeeWei.add(networkFeeWei.mul(rewardInBPS).div(BPS)).add(networkFeeWei.mul(rebateInBPS).div(BPS));
-            } else {
-                totalPayout = platformFeeWei.add(networkFeeWei.mul(rewardInBPS).div(BPS));
-            }
+            totalPayout = platformFeeWei.add(networkFeeWei.mul(rewardInBPS).div(BPS)).add(rebateWei);
             Helper.assertApproximate(await feeHandler.totalPayoutBalance(), beforeTotalBalancePayout.add(totalPayout), "unexpected payout balance");
-            
         }
 
         it("e2t trade. see fee updated in fee handler.", async() => {
@@ -2516,7 +2511,7 @@ contract('KyberNetwork', function(accounts) {
             await assertFeeHandlerUpdate(tradeEventArgs.ethWeiValue, platformFee, BPS, rebatePerWallet);
         });
 
-        it("should have rebate given only to rebate entitled reserve.", async() => {
+        it("check that reserve rebate amount is correct", async() => {
             await storage.setFeeAccountedPerReserveType(true, true, true, true, true, true, {from: admin});
             // set rebate entitled true for FPR
             await storage.setEntitledRebatePerReserveType(true, false, false, false, false, false, {from: admin});
@@ -2532,11 +2527,15 @@ contract('KyberNetwork', function(accounts) {
                 srcToken.address, destToken.address, srcQty);
             txResult = await network.tradeWithHintAndFee(networkProxy, srcToken.address, srcQty, destToken.address, taker,
                 maxDestAmt, minConversionRate, platformWallet, platformFee, hint, {from: networkProxy});
+            
             // assert first rebateWallet is received enitled rebate value
             let tradeEventArgs = nwHelper.getTradeEventArgs(txResult);
-            let rebatePerWallet = {}
-            let expectedRebate = new BN(tradeEventArgs.ethWeiValue).mul(networkFeeBps).div(BPS).mul(new BN(2)).mul(rebateInBPS).div(BPS);
-            rebatePerWallet[rebateWallets[0]] = expectedRebate
+            let rebatePerWallet = {};
+            let feeAccountedBps = new BN(2).mul(BPS);
+            let entitledRebateBps = BPS;
+            let networkFeeWei = new BN(tradeEventArgs.ethWeiValue).mul(networkFeeBps).div(BPS).mul(new BN(2));
+            let expectedRebateWei = networkFeeWei.mul(entitledRebateBps).div(feeAccountedBps).mul(rebateInBPS).div(BPS);;
+            rebatePerWallet[rebateWallets[0]] = expectedRebateWei;
             await assertFeeHandlerUpdate(tradeEventArgs.ethWeiValue, platformFee, BPS.mul(new BN(2)), rebatePerWallet);
             // revert changes
             await storage.setEntitledRebatePerReserveType(true, true, true, true, true, true, {from: admin});