Merge branch 'development' into dev-tools-pages

8 files changed, 84 insertions, 52 deletions

diff --git a/packages/asset-buyer/CHANGELOG.json b/packages/asset-buyer/CHANGELOG.json
index aa0d28879..0e4623d05 100644
--- a/packages/asset-buyer/CHANGELOG.json
+++ b/packages/asset-buyer/CHANGELOG.json
@@ -1,5 +1,24 @@
 [
     {
+        "version": "3.0.0",
+        "changes": [
+            {
+                "note": "update `getBuyQuoteAsync` to return eth spent on assets instead of per unit amount",
+                "pr": 1252
+            }
+        ],
+        "timestamp": 1542208198
+    },
+    {
+        "timestamp": 1542134075,
+        "version": "2.2.2",
+        "changes": [
+            {
+                "note": "Dependencies updated"
+            }
+        ]
+    },
+    {
         "timestamp": 1542028948,
         "version": "2.2.1",
         "changes": [
diff --git a/packages/asset-buyer/CHANGELOG.md b/packages/asset-buyer/CHANGELOG.md
index 331e4df9d..d6013e53e 100644
--- a/packages/asset-buyer/CHANGELOG.md
+++ b/packages/asset-buyer/CHANGELOG.md
@@ -5,6 +5,14 @@ Edit the package's CHANGELOG.json file only.
 
 CHANGELOG
 
+## v3.0.0 - _November 14, 2018_
+
+    * update `getBuyQuoteAsync` to return eth spent on assets instead of per unit amount (#1252)
+
+## v2.2.2 - _November 13, 2018_
+
+    * Dependencies updated
+
 ## v2.2.1 - _November 12, 2018_
 
     * Dependencies updated
diff --git a/packages/asset-buyer/package.json b/packages/asset-buyer/package.json
index 58daac16e..fad33476b 100644
--- a/packages/asset-buyer/package.json
+++ b/packages/asset-buyer/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@0x/asset-buyer",
-    "version": "2.2.1",
+    "version": "3.0.0",
     "engines": {
         "node": ">=6.12"
     },
@@ -36,16 +36,16 @@
     },
     "homepage": "https://github.com/0xProject/0x-monorepo/packages/asset-buyer/README.md",
     "dependencies": {
-        "@0x/assert": "^1.0.16",
-        "@0x/connect": "^3.0.4",
-        "@0x/contract-wrappers": "^4.0.0",
-        "@0x/json-schemas": "^2.1.0",
-        "@0x/order-utils": "^3.0.0",
-        "@0x/subproviders": "^2.1.2",
+        "@0x/assert": "^1.0.17",
+        "@0x/connect": "^3.0.6",
+        "@0x/contract-wrappers": "^4.0.2",
+        "@0x/json-schemas": "^2.1.1",
+        "@0x/order-utils": "^3.0.2",
+        "@0x/subproviders": "^2.1.4",
         "@0x/types": "^1.2.1",
         "@0x/typescript-typings": "^3.0.4",
-        "@0x/utils": "^2.0.4",
-        "@0x/web3-wrapper": "^3.1.2",
+        "@0x/utils": "^2.0.5",
+        "@0x/web3-wrapper": "^3.1.4",
         "ethereum-types": "^1.1.2",
         "lodash": "^4.17.10"
     },
diff --git a/packages/asset-buyer/src/types.ts b/packages/asset-buyer/src/types.ts
index 3f1e6ff21..3b573edca 100644
--- a/packages/asset-buyer/src/types.ts
+++ b/packages/asset-buyer/src/types.ts
@@ -54,12 +54,12 @@ export interface BuyQuote {
 }
 
 /**
- * ethPerAssetPrice: The price of one unit of the desired asset in ETH
+ * assetEthAmount: The amount of eth required to pay for the requested asset.
  * feeEthAmount: The amount of eth required to pay the affiliate fee.
- * totalEthAmount: the total amount of eth required to complete the buy. (Filling orders, feeOrders, and paying affiliate fee)
+ * totalEthAmount: The total amount of eth required to complete the buy (filling orders, feeOrders, and paying affiliate fee).
  */
 export interface BuyQuoteInfo {
-    ethPerAssetPrice: BigNumber;
+    assetEthAmount: BigNumber;
     feeEthAmount: BigNumber;
     totalEthAmount: BigNumber;
 }
diff --git a/packages/asset-buyer/src/utils/assert.ts b/packages/asset-buyer/src/utils/assert.ts
index 2466f53a4..fcf9b0d0e 100644
--- a/packages/asset-buyer/src/utils/assert.ts
+++ b/packages/asset-buyer/src/utils/assert.ts
@@ -18,7 +18,7 @@ export const assert = {
         }
     },
     isValidBuyQuoteInfo(variableName: string, buyQuoteInfo: BuyQuoteInfo): void {
-        sharedAssert.isBigNumber(`${variableName}.ethPerAssetPrice`, buyQuoteInfo.ethPerAssetPrice);
+        sharedAssert.isBigNumber(`${variableName}.assetEthAmount`, buyQuoteInfo.assetEthAmount);
         sharedAssert.isBigNumber(`${variableName}.feeEthAmount`, buyQuoteInfo.feeEthAmount);
         sharedAssert.isBigNumber(`${variableName}.totalEthAmount`, buyQuoteInfo.totalEthAmount);
     },
diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts
index 6a67ed1ed..b15b880c2 100644
--- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts
+++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts
@@ -106,28 +106,28 @@ function calculateQuoteInfo(
     isMakerAssetZrxToken: boolean,
 ): BuyQuoteInfo {
     // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right
-    let ethAmountToBuyAsset = constants.ZERO_AMOUNT;
-    let ethAmountToBuyZrx = constants.ZERO_AMOUNT;
+    let assetEthAmount = constants.ZERO_AMOUNT;
+    let zrxEthAmount = constants.ZERO_AMOUNT;
     if (isMakerAssetZrxToken) {
-        ethAmountToBuyAsset = findEthAmountNeededToBuyZrx(ordersAndFillableAmounts, assetBuyAmount);
+        assetEthAmount = findEthAmountNeededToBuyZrx(ordersAndFillableAmounts, assetBuyAmount);
     } else {
         // find eth and zrx amounts needed to buy
         const ethAndZrxAmountToBuyAsset = findEthAndZrxAmountNeededToBuyAsset(ordersAndFillableAmounts, assetBuyAmount);
-        ethAmountToBuyAsset = ethAndZrxAmountToBuyAsset[0];
+        assetEthAmount = ethAndZrxAmountToBuyAsset[0];
         const zrxAmountToBuyAsset = ethAndZrxAmountToBuyAsset[1];
         // find eth amount needed to buy zrx
-        ethAmountToBuyZrx = findEthAmountNeededToBuyZrx(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset);
+        zrxEthAmount = findEthAmountNeededToBuyZrx(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset);
     }
-    /// find the eth amount needed to buy the affiliate fee
-    const ethAmountToBuyAffiliateFee = ethAmountToBuyAsset.mul(feePercentage).ceil();
-    const totalEthAmountWithoutAffiliateFee = ethAmountToBuyAsset.plus(ethAmountToBuyZrx);
-    const ethAmountTotal = totalEthAmountWithoutAffiliateFee.plus(ethAmountToBuyAffiliateFee);
-    // divide into the assetBuyAmount in order to find rate of makerAsset / WETH
-    const ethPerAssetPrice = totalEthAmountWithoutAffiliateFee.div(assetBuyAmount);
+    // eth amount needed to buy the affiliate fee
+    const affiliateFeeEthAmount = assetEthAmount.mul(feePercentage).ceil();
+    // eth amount needed for fees is the sum of affiliate fee and zrx fee
+    const feeEthAmount = affiliateFeeEthAmount.plus(zrxEthAmount);
+    // eth amount needed in total is the sum of the amount needed for the asset and the amount needed for fees
+    const totalEthAmount = assetEthAmount.plus(feeEthAmount);
     return {
-        totalEthAmount: ethAmountTotal,
-        feeEthAmount: ethAmountToBuyAffiliateFee,
-        ethPerAssetPrice,
+        assetEthAmount,
+        feeEthAmount,
+        totalEthAmount,
     };
 }
 
diff --git a/packages/asset-buyer/src/utils/order_provider_response_processor.ts b/packages/asset-buyer/src/utils/order_provider_response_processor.ts
index 28f684f3c..25e85b2cc 100644
--- a/packages/asset-buyer/src/utils/order_provider_response_processor.ts
+++ b/packages/asset-buyer/src/utils/order_provider_response_processor.ts
@@ -44,19 +44,24 @@ export const orderProviderResponseProcessor = {
         let unsortedOrders = filteredOrders;
         // if an orderValidator is provided, use on chain information to calculate remaining fillable makerAsset amounts
         if (!_.isUndefined(orderValidator)) {
-            // TODO(bmillman): improvement
-            // try/catch this request and throw a more domain specific error
             const takerAddresses = _.map(filteredOrders, () => constants.NULL_ADDRESS);
-            const ordersAndTradersInfo = await orderValidator.getOrdersAndTradersInfoAsync(
-                filteredOrders,
-                takerAddresses,
-            );
-            // take orders + on chain information and find the valid orders and remaining fillable maker asset amounts
-            unsortedOrders = getValidOrdersWithRemainingFillableMakerAssetAmountsFromOnChain(
-                filteredOrders,
-                ordersAndTradersInfo,
-                isMakerAssetZrxToken,
-            );
+            try {
+                const ordersAndTradersInfo = await orderValidator.getOrdersAndTradersInfoAsync(
+                    filteredOrders,
+                    takerAddresses,
+                );
+                // take orders + on chain information and find the valid orders and remaining fillable maker asset amounts
+                unsortedOrders = getValidOrdersWithRemainingFillableMakerAssetAmountsFromOnChain(
+                    filteredOrders,
+                    ordersAndTradersInfo,
+                    isMakerAssetZrxToken,
+                );
+            } catch (err) {
+                // Sometimes we observe this call to orderValidator fail with response `0x`
+                // Because of differences in Parity / Geth implementations, its very hard to tell if this response is a "system error"
+                // or a revert. In this case we just swallow these errors and fallback to partial fill information from the SRA.
+                // TODO(bmillman): report these errors so we have an idea of how often we're getting these failures.
+            }
         }
         // sort orders by rate
         // TODO(bmillman): optimization
diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts
index 0ea371982..a30017b72 100644
--- a/packages/asset-buyer/test/buy_quote_calculator_test.ts
+++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts
@@ -108,17 +108,17 @@ describe('buyQuoteCalculator', () => {
             // 50 eth to fill the first order + 100 eth for fees
             const expectedEthAmountForAsset = new BigNumber(50);
             const expectedEthAmountForZrxFees = new BigNumber(100);
-            const expectedFillEthAmount = expectedEthAmountForAsset.plus(expectedEthAmountForZrxFees);
-            const expectedFeeEthAmount = expectedEthAmountForAsset.mul(feePercentage);
+            const expectedFillEthAmount = expectedEthAmountForAsset;
+            const expectedAffiliateFeeEthAmount = expectedEthAmountForAsset.mul(feePercentage);
+            const expectedFeeEthAmount = expectedAffiliateFeeEthAmount.plus(expectedEthAmountForZrxFees);
             const expectedTotalEthAmount = expectedFillEthAmount.plus(expectedFeeEthAmount);
-            const expectedEthPerAssetPrice = expectedFillEthAmount.div(assetBuyAmount);
+            expect(buyQuote.bestCaseQuoteInfo.assetEthAmount).to.bignumber.equal(expectedFillEthAmount);
             expect(buyQuote.bestCaseQuoteInfo.feeEthAmount).to.bignumber.equal(expectedFeeEthAmount);
             expect(buyQuote.bestCaseQuoteInfo.totalEthAmount).to.bignumber.equal(expectedTotalEthAmount);
-            expect(buyQuote.bestCaseQuoteInfo.ethPerAssetPrice).to.bignumber.equal(expectedEthPerAssetPrice);
             // because we have no slippage protection, minRate is equal to maxRate
+            expect(buyQuote.worstCaseQuoteInfo.assetEthAmount).to.bignumber.equal(expectedFillEthAmount);
             expect(buyQuote.worstCaseQuoteInfo.feeEthAmount).to.bignumber.equal(expectedFeeEthAmount);
             expect(buyQuote.worstCaseQuoteInfo.totalEthAmount).to.bignumber.equal(expectedTotalEthAmount);
-            expect(buyQuote.worstCaseQuoteInfo.ethPerAssetPrice).to.bignumber.equal(expectedEthPerAssetPrice);
             // test if feePercentage gets passed through
             expect(buyQuote.feePercentage).to.equal(feePercentage);
         });
@@ -146,23 +146,23 @@ describe('buyQuoteCalculator', () => {
             // 50 eth to fill the first order + 100 eth for fees
             const expectedEthAmountForAsset = new BigNumber(50);
             const expectedEthAmountForZrxFees = new BigNumber(100);
-            const expectedFillEthAmount = expectedEthAmountForAsset.plus(expectedEthAmountForZrxFees);
-            const expectedFeeEthAmount = expectedEthAmountForAsset.mul(feePercentage);
+            const expectedFillEthAmount = expectedEthAmountForAsset;
+            const expectedAffiliateFeeEthAmount = expectedEthAmountForAsset.mul(feePercentage);
+            const expectedFeeEthAmount = expectedAffiliateFeeEthAmount.plus(expectedEthAmountForZrxFees);
             const expectedTotalEthAmount = expectedFillEthAmount.plus(expectedFeeEthAmount);
-            const expectedEthPerAssetPrice = expectedFillEthAmount.div(assetBuyAmount);
+            expect(buyQuote.bestCaseQuoteInfo.assetEthAmount).to.bignumber.equal(expectedFillEthAmount);
             expect(buyQuote.bestCaseQuoteInfo.feeEthAmount).to.bignumber.equal(expectedFeeEthAmount);
             expect(buyQuote.bestCaseQuoteInfo.totalEthAmount).to.bignumber.equal(expectedTotalEthAmount);
-            expect(buyQuote.bestCaseQuoteInfo.ethPerAssetPrice).to.bignumber.equal(expectedEthPerAssetPrice);
             // 100 eth to fill the first order + 208 eth for fees
             const expectedWorstEthAmountForAsset = new BigNumber(100);
             const expectedWorstEthAmountForZrxFees = new BigNumber(208);
-            const expectedWorstFillEthAmount = expectedWorstEthAmountForAsset.plus(expectedWorstEthAmountForZrxFees);
-            const expectedWorstFeeEthAmount = expectedWorstEthAmountForAsset.mul(feePercentage);
+            const expectedWorstFillEthAmount = expectedWorstEthAmountForAsset;
+            const expectedWorstAffiliateFeeEthAmount = expectedWorstEthAmountForAsset.mul(feePercentage);
+            const expectedWorstFeeEthAmount = expectedWorstAffiliateFeeEthAmount.plus(expectedWorstEthAmountForZrxFees);
             const expectedWorstTotalEthAmount = expectedWorstFillEthAmount.plus(expectedWorstFeeEthAmount);
-            const expectedWorstEthPerAssetPrice = expectedWorstFillEthAmount.div(assetBuyAmount);
+            expect(buyQuote.worstCaseQuoteInfo.assetEthAmount).to.bignumber.equal(expectedWorstFillEthAmount);
             expect(buyQuote.worstCaseQuoteInfo.feeEthAmount).to.bignumber.equal(expectedWorstFeeEthAmount);
             expect(buyQuote.worstCaseQuoteInfo.totalEthAmount).to.bignumber.equal(expectedWorstTotalEthAmount);
-            expect(buyQuote.worstCaseQuoteInfo.ethPerAssetPrice).to.bignumber.equal(expectedWorstEthPerAssetPrice);
             // test if feePercentage gets passed through
             expect(buyQuote.feePercentage).to.equal(feePercentage);
         });