From 90674d9038b45644e82545558d9937b9eec498f9 Mon Sep 17 00:00:00 2001
From: Brandon Millman <brandon.millman@gmail.com>
Date: Fri, 14 Sep 2018 13:42:37 +0200
Subject: Various clean up

---
 packages/forwarder-helper/src/forwarder_helper_factory.ts | 8 +++++---
 packages/forwarder-helper/src/utils/order_utils.ts        | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/packages/forwarder-helper/src/forwarder_helper_factory.ts b/packages/forwarder-helper/src/forwarder_helper_factory.ts
index be38f2bd5..2b37ac98f 100644
--- a/packages/forwarder-helper/src/forwarder_helper_factory.ts
+++ b/packages/forwarder-helper/src/forwarder_helper_factory.ts
@@ -24,6 +24,7 @@ export const forwarderHelperFactory = {
     getForwarderHelperForOrders(orders: SignedOrder[], feeOrders: SignedOrder[] = []): ForwarderHelper {
         assert.doesConformToSchema('orders', orders, schemas.signedOrdersSchema);
         assert.doesConformToSchema('feeOrders', orders, schemas.signedOrdersSchema);
+        // TODO: Add assertion here for orders all having the same makerAsset and takerAsset
         const config: ForwarderHelperImplConfig = {
             orders,
             feeOrders,
@@ -96,6 +97,7 @@ export const forwarderHelperFactory = {
             const ordersFromSra = getOpenAsksFromOrderbook(makerAssetOrderbook);
             const feeOrdersFromSra = getOpenAsksFromOrderbook(zrxOrderbook);
             // TODO: try catch these requests and throw a more domain specific error
+            // TODO: optimization, reduce this to once RPC call buy combining orders into one array and then splitting up the response
             const [makerAssetOrdersAndTradersInfo, feeOrdersAndTradersInfo] = await Promise.all(
                 _.map([ordersFromSra, feeOrdersFromSra], ordersToBeValidated => {
                     const takerAddresses = _.map(ordersToBeValidated, () => constants.NULL_ADDRESS);
@@ -159,7 +161,7 @@ function getValidOrdersAndRemainingFillableMakerAssetAmountsFromApi(
 ): OrdersAndRemainingFillableMakerAssetAmounts {
     const result = _.reduce(
         apiOrders,
-        (acc, apiOrder, index) => {
+        (acc, apiOrder) => {
             // get current accumulations
             const { orders, remainingFillableMakerAssetAmounts } = acc;
             // get order and metadata
@@ -168,7 +170,7 @@ function getValidOrdersAndRemainingFillableMakerAssetAmountsFromApi(
             if (orderUtils.isOrderExpired(order) || !orderUtils.isOpenOrder(order)) {
                 return acc;
             }
-            // calculate remainingFillableMakerAssetAmount from api metadata
+            // calculate remainingFillableMakerAssetAmount from api metadata, else assume order is completely fillable
             const remainingFillableTakerAssetAmount = _.get(
                 metaData,
                 'remainingTakerAssetAmount',
@@ -215,7 +217,7 @@ function getValidOrdersAndRemainingFillableMakerAssetAmountsFromOnChain(
             const { orders, remainingFillableMakerAssetAmounts } = acc;
             // get corresponding on-chain state for the order
             const { orderInfo, traderInfo } = ordersAndTradersInfo[index];
-            // if the order IS NOT fillable, do not add anything and continue iterating
+            // if the order IS NOT fillable, do not add anything to the accumulations and continue iterating
             if (orderInfo.orderStatus !== OrderStatus.FILLABLE) {
                 return acc;
             }
diff --git a/packages/forwarder-helper/src/utils/order_utils.ts b/packages/forwarder-helper/src/utils/order_utils.ts
index 482006bf1..bb0bdb80f 100644
--- a/packages/forwarder-helper/src/utils/order_utils.ts
+++ b/packages/forwarder-helper/src/utils/order_utils.ts
@@ -11,7 +11,7 @@ export const orderUtils = {
     },
     calculateRemainingMakerAssetAmount(order: SignedOrder, remainingTakerAssetAmount: BigNumber): BigNumber {
         const result = remainingTakerAssetAmount.eq(0)
-            ? new BigNumber(0)
+            ? constants.ZERO_AMOUNT
             : remainingTakerAssetAmount.times(order.makerAssetAmount).dividedToIntegerBy(order.takerAssetAmount);
         return result;
     },
-- 
cgit v1.2.3