From c051e11a497e1b95febdfdf48683853fc2cc8d2a Mon Sep 17 00:00:00 2001
From: "F. Eugene Aumson" <feuGeneA@users.noreply.github.com>
Date: Mon, 30 Sep 2019 17:53:21 -0400
Subject: [PATCH] OrderValidationUtils: don't parse fee asset data when there
 are no fees specified (#2218)

* exhibit bug: empty fee asset data induces revert

* fix bug: don't parse fee asset data unless fee>0

* Update DevUtils Kovan contract address
---
 .../contracts/src/OrderValidationUtils.sol    |  6 ++---
 .../dev-utils/test/order_validation_utils.ts  | 22 +++++++++++++++++++
 packages/contract-addresses/src/index.ts      |  2 +-
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/contracts/dev-utils/contracts/src/OrderValidationUtils.sol b/contracts/dev-utils/contracts/src/OrderValidationUtils.sol
index 72e9242655..b7e218a4c0 100644
--- a/contracts/dev-utils/contracts/src/OrderValidationUtils.sol
+++ b/contracts/dev-utils/contracts/src/OrderValidationUtils.sol
@@ -86,9 +86,6 @@ contract OrderValidationUtils is
                 takerAssetAmount
             );
         } else {
-            // Get the transferable amount of the `makerFeeAsset`
-            uint256 transferableMakerFeeAssetAmount = getTransferableAssetAmount(makerAddress, order.makerFeeAssetData);
-
             // If `makerFee` is 0, the % that can be filled is (transferableMakerAssetAmount / makerAssetAmount)
             if (makerFee == 0) {
                 transferableTakerAssetAmount = LibMath.getPartialAmountFloor(
@@ -100,6 +97,9 @@ contract OrderValidationUtils is
             // If `makerAsset` does not equal `makerFeeAsset`, the % that can be filled is the lower of
             // (transferableMakerAssetAmount / makerAssetAmount) and (transferableMakerAssetFeeAmount / makerFee)
             } else {
+                // Get the transferable amount of the `makerFeeAsset`
+                uint256 transferableMakerFeeAssetAmount = getTransferableAssetAmount(makerAddress, order.makerFeeAssetData);
+
                 uint256 transferableMakerToTakerAmount = LibMath.getPartialAmountFloor(
                     transferableMakerAssetAmount,
                     order.makerAssetAmount,
diff --git a/contracts/dev-utils/test/order_validation_utils.ts b/contracts/dev-utils/test/order_validation_utils.ts
index 26a9787cf9..c222131a01 100644
--- a/contracts/dev-utils/test/order_validation_utils.ts
+++ b/contracts/dev-utils/test/order_validation_utils.ts
@@ -410,6 +410,28 @@ describe('OrderValidationUtils/OrderTransferSimulatorUtils', () => {
                 signedOrder.takerAssetAmount.minus(takerAssetFillAmount),
             );
         });
+        it('should return correct info even when there are no fees specified', async () => {
+            signedOrder = await orderFactory.newSignedOrderAsync({
+                makerFee: new BigNumber(0),
+                takerFee: new BigNumber(0),
+                makerFeeAssetData: '0x',
+                takerFeeAssetData: '0x',
+            });
+            await erc20Token.setBalance.awaitTransactionSuccessAsync(makerAddress, signedOrder.makerAssetAmount);
+            await erc20Token.approve.awaitTransactionSuccessAsync(erc20Proxy.address, signedOrder.makerAssetAmount, {
+                from: makerAddress,
+            });
+            const [
+                orderInfo,
+                fillableTakerAssetAmount,
+                isValidSignature,
+            ] = await devUtils.getOrderRelevantState.callAsync(signedOrder, signedOrder.signature);
+            expect(orderInfo.orderHash).to.equal(orderHashUtils.getOrderHashHex(signedOrder));
+            expect(orderInfo.orderStatus).to.equal(OrderStatus.Fillable);
+            expect(orderInfo.orderTakerAssetFilledAmount).to.bignumber.equal(constants.ZERO_AMOUNT);
+            expect(fillableTakerAssetAmount).to.bignumber.equal(signedOrder.takerAssetAmount);
+            expect(isValidSignature).to.equal(true);
+        });
     });
     describe('getOrderRelevantStates', async () => {
         it('should return the correct information for multiple orders', async () => {
diff --git a/packages/contract-addresses/src/index.ts b/packages/contract-addresses/src/index.ts
index 5aa083268b..61320fcb0c 100644
--- a/packages/contract-addresses/src/index.ts
+++ b/packages/contract-addresses/src/index.ts
@@ -95,7 +95,7 @@ const networkToAddresses: { [networkId: number]: ContractAddresses } = {
         multiAssetProxy: '0xf6313a772c222f51c28f2304c0703b8cf5428fd8',
         staticCallProxy: '0x48e94bdb9033640d45ea7c721e25f380f8bffa43',
         erc1155Proxy: '0x64517fa2b480ba3678a2a3c0cf08ef7fd4fad36f',
-        devUtils: '0xa5f985e8df08e17e88001dd85724707bcb80e413',
+        devUtils: '0xb1863ac46ae23ec55d6eeb8ecc8815655ee638a8',
     },
     // NetworkId 50 represents our Ganache snapshot generated from migrations.
     50: {