From 69054d85e80f9e41200015a9b0eef2a9fe00f439 Mon Sep 17 00:00:00 2001 From: Steve Klebanoff Date: Fri, 14 Dec 2018 15:18:20 -0800 Subject: Only send in amountAvailableToFill if it's a non-zero amount, add additional tests and nest, and put error into its own file --- packages/asset-buyer/src/errors.ts | 16 ++ packages/asset-buyer/src/index.ts | 2 +- packages/asset-buyer/src/types.ts | 12 +- .../asset-buyer/src/utils/buy_quote_calculator.ts | 16 +- .../asset-buyer/test/buy_quote_calculator_test.ts | 196 ++++++++++++--------- packages/asset-buyer/test/utils/test_helpers.ts | 10 +- packages/instant/src/util/asset.ts | 6 +- 7 files changed, 154 insertions(+), 104 deletions(-) create mode 100644 packages/asset-buyer/src/errors.ts (limited to 'packages') diff --git a/packages/asset-buyer/src/errors.ts b/packages/asset-buyer/src/errors.ts new file mode 100644 index 000000000..ef05a5d0a --- /dev/null +++ b/packages/asset-buyer/src/errors.ts @@ -0,0 +1,16 @@ +import { BigNumber } from '@0x/utils'; + +import { AssetBuyerError } from './types'; + +/** + * Error class representing insufficient asset liquidity + */ +export class InsufficientAssetLiquidityError extends Error { + public amountAvailableToFill?: BigNumber; + constructor(amountAvailableToFill?: BigNumber) { + super(AssetBuyerError.InsufficientAssetLiquidity); + this.amountAvailableToFill = amountAvailableToFill; + // Setting prototype so instanceof works. See https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work + Object.setPrototypeOf(this, InsufficientAssetLiquidityError.prototype); + } +} diff --git a/packages/asset-buyer/src/index.ts b/packages/asset-buyer/src/index.ts index 59515e24f..a42d7e272 100644 --- a/packages/asset-buyer/src/index.ts +++ b/packages/asset-buyer/src/index.ts @@ -9,6 +9,7 @@ export { SignedOrder } from '@0x/types'; export { BigNumber } from '@0x/utils'; export { AssetBuyer } from './asset_buyer'; +export { InsufficientAssetLiquidityError } from './errors'; export { BasicOrderProvider } from './order_providers/basic_order_provider'; export { StandardRelayerAPIOrderProvider } from './order_providers/standard_relayer_api_order_provider'; export { @@ -18,7 +19,6 @@ export { BuyQuoteExecutionOpts, BuyQuoteInfo, BuyQuoteRequestOpts, - InsufficientAssetLiquidityError, OrderProvider, OrderProviderRequest, OrderProviderResponse, diff --git a/packages/asset-buyer/src/types.ts b/packages/asset-buyer/src/types.ts index d435f337e..d5d6be695 100644 --- a/packages/asset-buyer/src/types.ts +++ b/packages/asset-buyer/src/types.ts @@ -102,7 +102,7 @@ export interface AssetBuyerOpts { } /** - * Possible errors thrown by an AssetBuyer instance or associated static methods. + * Possible error messages thrown by an AssetBuyer instance or associated static methods. */ export enum AssetBuyerError { NoEtherTokenContractFound = 'NO_ETHER_TOKEN_CONTRACT_FOUND', @@ -117,16 +117,6 @@ export enum AssetBuyerError { TransactionValueTooLow = 'TRANSACTION_VALUE_TOO_LOW', } -export class InsufficientAssetLiquidityError extends Error { - public amountAvailableToFill: BigNumber; - constructor(amountAvailableToFill: BigNumber) { - super(AssetBuyerError.InsufficientAssetLiquidity); - this.amountAvailableToFill = amountAvailableToFill; - // Setting prototype so instanceof works. See https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work - Object.setPrototypeOf(this, InsufficientAssetLiquidityError.prototype); - } -} - export interface OrdersAndFillableAmounts { orders: SignedOrder[]; remainingFillableMakerAssetAmounts: BigNumber[]; diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index 23d3e9b24..59293d1b7 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -1,16 +1,10 @@ import { marketUtils, SignedOrder } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; -import { Web3Wrapper } from '@0x/web3-wrapper'; import * as _ from 'lodash'; import { constants } from '../constants'; -import { - AssetBuyerError, - BuyQuote, - BuyQuoteInfo, - InsufficientAssetLiquidityError, - OrdersAndFillableAmounts, -} from '../types'; +import { InsufficientAssetLiquidityError } from '../errors'; +import { AssetBuyerError, BuyQuote, BuyQuoteInfo, OrdersAndFillableAmounts } from '../types'; import { orderUtils } from './order_utils'; @@ -53,7 +47,11 @@ export const buyQuoteCalculator = { .div(multiplerNeededWithSlippage) .round(0, BigNumber.ROUND_DOWN); - throw new InsufficientAssetLiquidityError(amountAvailableToFillConsideringSlippage); + throw new InsufficientAssetLiquidityError( + amountAvailableToFillConsideringSlippage.gt(constants.ZERO_AMOUNT) + ? amountAvailableToFillConsideringSlippage + : undefined, + ); } // if we are not buying ZRX: // given the orders calculated above, find the fee-orders that cover the desired assetBuyAmount (with slippage) diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 98458cd5b..fdc17ef25 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -67,87 +67,125 @@ describe('buyQuoteCalculator', () => { ], }; }); - it('should throw if not enough maker asset liquidity (multiple orders)', () => { - // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units - const errorFunction = () => { - buyQuoteCalculator.calculate( - ordersAndFillableAmounts, - smallFeeOrderAndFillableAmount, - new BigNumber(500), - 0, - 0, - false, - ); - }; - testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(400)); - }); - it('should throw if not enough maker asset liquidity (multiple orders with 20% slippage)', () => { - // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units - const errorFunction = () => { - buyQuoteCalculator.calculate( - ordersAndFillableAmounts, - smallFeeOrderAndFillableAmount, - new BigNumber(500), - 0, - 0.2, - false, - ); - }; - testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(333)); - }); - it('should throw if not enough maker asset liquidity (multiple orders with 5% slippage)', () => { - // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units - const errorFunction = () => { - buyQuoteCalculator.calculate( - ordersAndFillableAmounts, - smallFeeOrderAndFillableAmount, - new BigNumber(600), - 0, - 0.05, - false, - ); - }; - testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(380)); - }); - it('should throw if not enough maker asset liquidity (partially filled order)', () => { - const firstOrderAndFillableAmount: OrdersAndFillableAmounts = { - orders: [firstOrder], - remainingFillableMakerAssetAmounts: [firstRemainingFillAmount], - }; + describe('InsufficientLiquidityError', () => { + it('should throw if not enough maker asset liquidity (multiple orders)', () => { + // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units + const errorFunction = () => { + buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + smallFeeOrderAndFillableAmount, + new BigNumber(500), + 0, + 0, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(400)); + }); + it('should throw if not enough maker asset liquidity (multiple orders with 20% slippage)', () => { + // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units + const errorFunction = () => { + buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + smallFeeOrderAndFillableAmount, + new BigNumber(500), + 0, + 0.2, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(333)); + }); + it('should throw if not enough maker asset liquidity (multiple orders with 5% slippage)', () => { + // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units + const errorFunction = () => { + buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + smallFeeOrderAndFillableAmount, + new BigNumber(600), + 0, + 0.05, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(380)); + }); + it('should throw if not enough maker asset liquidity (partially filled order)', () => { + const firstOrderAndFillableAmount: OrdersAndFillableAmounts = { + orders: [firstOrder], + remainingFillableMakerAssetAmounts: [firstRemainingFillAmount], + }; - const errorFunction = () => { - buyQuoteCalculator.calculate( - firstOrderAndFillableAmount, - smallFeeOrderAndFillableAmount, - new BigNumber(201), - 0, - 0, - false, - ); - }; - testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(200)); - }); - it('should throw if not enough maker asset liquidity (completely fillable order)', () => { - const completelyFillableOrder = orderFactory.createSignedOrderFromPartial({ - makerAssetAmount: new BigNumber(123), - takerAssetAmount: new BigNumber(100), - takerFee: new BigNumber(200), + const errorFunction = () => { + buyQuoteCalculator.calculate( + firstOrderAndFillableAmount, + smallFeeOrderAndFillableAmount, + new BigNumber(201), + 0, + 0, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(200)); + }); + it('should throw if not enough maker asset liquidity (completely fillable order)', () => { + const completelyFillableOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(123), + takerAssetAmount: new BigNumber(100), + takerFee: new BigNumber(200), + }); + const completelyFillableOrdersAndFillableAmount: OrdersAndFillableAmounts = { + orders: [completelyFillableOrder], + remainingFillableMakerAssetAmounts: [completelyFillableOrder.makerAssetAmount], + }; + const errorFunction = () => { + buyQuoteCalculator.calculate( + completelyFillableOrdersAndFillableAmount, + smallFeeOrderAndFillableAmount, + new BigNumber(124), + 0, + 0, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(123)); + }); + it('should throw with 1 amount available if no slippage', () => { + const smallOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(1), + takerAssetAmount: new BigNumber(1), + takerFee: new BigNumber(0), + }); + const errorFunction = () => { + buyQuoteCalculator.calculate( + { orders: [smallOrder], remainingFillableMakerAssetAmounts: [smallOrder.makerAssetAmount] }, + smallFeeOrderAndFillableAmount, + new BigNumber(600), + 0, + 0, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(1)); + }); + it('should throw without amount available to fill if amount rounds to 0', () => { + const smallOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(1), + takerAssetAmount: new BigNumber(1), + takerFee: new BigNumber(0), + }); + const errorFunction = () => { + buyQuoteCalculator.calculate( + { orders: [smallOrder], remainingFillableMakerAssetAmounts: [smallOrder.makerAssetAmount] }, + smallFeeOrderAndFillableAmount, + new BigNumber(600), + 0, + 0.2, + false, + ); + }; + testHelpers.expectInsufficientLiquidityError(expect, errorFunction, undefined); }); - const completelyFillableOrdersAndFillableAmount: OrdersAndFillableAmounts = { - orders: [completelyFillableOrder], - remainingFillableMakerAssetAmounts: [completelyFillableOrder.makerAssetAmount], - }; - const errorFunction = () => { - buyQuoteCalculator.calculate( - completelyFillableOrdersAndFillableAmount, - smallFeeOrderAndFillableAmount, - new BigNumber(124), - 0, - 0, - false, - ); - }; - testHelpers.expectInsufficientLiquidityError(expect, errorFunction, new BigNumber(123)); }); it('should not throw if order is fillable', () => { expect(() => diff --git a/packages/asset-buyer/test/utils/test_helpers.ts b/packages/asset-buyer/test/utils/test_helpers.ts index b99906792..d746316f8 100644 --- a/packages/asset-buyer/test/utils/test_helpers.ts +++ b/packages/asset-buyer/test/utils/test_helpers.ts @@ -1,12 +1,12 @@ import { BigNumber } from '@0x/utils'; -import { InsufficientAssetLiquidityError } from '../../src/types'; +import { InsufficientAssetLiquidityError } from '../../src/errors'; export const testHelpers = { expectInsufficientLiquidityError: ( expect: Chai.ExpectStatic, functionWhichTriggersError: () => void, - expectedAmountAvailableToFill: BigNumber, + expectedAmountAvailableToFill?: BigNumber, ): void => { let errorThrown = false; try { @@ -14,7 +14,11 @@ export const testHelpers = { } catch (e) { errorThrown = true; expect(e).to.be.instanceOf(InsufficientAssetLiquidityError); - expect(e.amountAvailableToFill).to.be.bignumber.equal(expectedAmountAvailableToFill); + if (expectedAmountAvailableToFill) { + expect(e.amountAvailableToFill).to.be.bignumber.equal(expectedAmountAvailableToFill); + } else { + expect(e.amountAvailableToFill).to.be.undefined(); + } } expect(errorThrown).to.be.true(); diff --git a/packages/instant/src/util/asset.ts b/packages/instant/src/util/asset.ts index 5e83e7e6d..756194f1f 100644 --- a/packages/instant/src/util/asset.ts +++ b/packages/instant/src/util/asset.ts @@ -113,7 +113,11 @@ export const assetUtils = { assetBuyerErrorMessage: (asset: ERC20Asset, error: Error): string | undefined => { if (error.message === AssetBuyerError.InsufficientAssetLiquidity) { const assetName = assetUtils.bestNameForAsset(asset, 'of this asset'); - if (error instanceof InsufficientAssetLiquidityError) { + if ( + error instanceof InsufficientAssetLiquidityError && + error.amountAvailableToFill && + error.amountAvailableToFill.greaterThan(BIG_NUMBER_ZERO) + ) { const unitAmountAvailableToFill = Web3Wrapper.toUnitAmount( error.amountAvailableToFill, asset.metaData.decimals, -- cgit v1.2.3