From 98c1952956d87adf5e73a42a1b78b8bf8b4119d2 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 2 Oct 2018 15:00:35 -0400 Subject: fix: persist artifacts with only relevant sources https://github.com/0xProject/0x-monorepo/pull/1108 https://app.asana.com/0/684263176955174/842516551768097/f --- packages/sol-compiler/src/compiler.ts | 57 ++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 5 deletions(-) (limited to 'packages') diff --git a/packages/sol-compiler/src/compiler.ts b/packages/sol-compiler/src/compiler.ts index a29367485..350a5a125 100644 --- a/packages/sol-compiler/src/compiler.ts +++ b/packages/sol-compiler/src/compiler.ts @@ -296,13 +296,12 @@ export class Compiler { compilerOutput: solc.StandardOutput, ): Promise { const compiledContract = compilerOutput.contracts[contractPath][contractName]; - const sourceCodes = _.mapValues( - compilerOutput.sources, - (_1, sourceFilePath) => this._resolver.resolve(sourceFilePath).source, - ); + + const { sourceCodes, sources } = this._getSourcesWithDependencies(contractPath); + const contractVersion: ContractVersionData = { compilerOutput: compiledContract, - sources: compilerOutput.sources, + sources, sourceCodes, sourceTreeHashHex, compiler: { @@ -333,6 +332,54 @@ export class Compiler { await fsWrapper.writeFileAsync(currentArtifactPath, artifactString); logUtils.warn(`${contractName} artifact saved!`); } + private _getSourcesWithDependencies( + contractPath: string, + ): { sourceCodes: { [sourceName: string]: string }; sources: { [sourceName: string]: { id: number } } } { + const sources = { [contractPath]: { id: 0 } }; + const sourceCodes = { [contractPath]: this._resolver.resolve(contractPath).source }; + this._recursivelyGatherDependencySources(contractPath, sourceCodes[contractPath], sources, sourceCodes, 1); + return { sourceCodes, sources }; + } + private _recursivelyGatherDependencySources( + contractPath: string, + contractSource: string, + sourcesToAppendTo: { [sourceName: string]: { id: number } }, + sourceCodesToAppendTo: { [sourceName: string]: string }, + nextId: number, + ): number { + let nextId_ = nextId; + + const importStatementMatches = contractSource.match(/import[^;]*;/g); + if (importStatementMatches === null) { + return nextId_; + } + for (const importStatementMatch of importStatementMatches) { + const importPathMatches = importStatementMatch.match(/\"([^\"]*)\"/); + if (importPathMatches === null || importPathMatches.length === 0) { + continue; + } + + let importPath = importPathMatches[1]; + const lastPathSeparatorPos = contractPath.lastIndexOf('/'); + const importFolder = lastPathSeparatorPos === -1 ? '' : contractPath.slice(0, lastPathSeparatorPos + 1); + importPath = importPath.slice(0, 2) === './' ? importPath.replace(/^.\//, importFolder) : importPath; + + if (_.isUndefined(sourcesToAppendTo[importPath])) { + sourcesToAppendTo[importPath] = { id: nextId_ }; + sourceCodesToAppendTo[importPath] = this._resolver.resolve(importPath).source; + nextId_ += 1; + + nextId_ = this._recursivelyGatherDependencySources( + importPath, + this._resolver.resolve(importPath).source, + sourcesToAppendTo, + sourceCodesToAppendTo, + nextId_, + ); + } + } + return nextId_; + } private _compile(solcInstance: solc.SolcInstance, standardInput: solc.StandardInput): solc.StandardOutput { const compiled: solc.StandardOutput = JSON.parse( solcInstance.compileStandardWrapper(JSON.stringify(standardInput), importPath => { -- cgit v1.2.3 From 37c55302e74d6a6706a390fe03432c0d3f119510 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 2 Oct 2018 17:29:47 -0700 Subject: Fix some small bugs in compiler.ts --- packages/sol-compiler/src/compiler.ts | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'packages') diff --git a/packages/sol-compiler/src/compiler.ts b/packages/sol-compiler/src/compiler.ts index 350a5a125..ea080c312 100644 --- a/packages/sol-compiler/src/compiler.ts +++ b/packages/sol-compiler/src/compiler.ts @@ -349,7 +349,7 @@ export class Compiler { ): number { let nextId_ = nextId; - const importStatementMatches = contractSource.match(/import[^;]*;/g); + const importStatementMatches = contractSource.match(/\nimport[^;]*;/g); if (importStatementMatches === null) { return nextId_; } @@ -360,9 +360,24 @@ export class Compiler { } let importPath = importPathMatches[1]; + // HACK(ablrow): We have, e.g.: + // + // importPath = "../../utils/LibBytes/LibBytes.sol" + // contractPath = "2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol" + // + // Resolver doesn't understand "../" so we want to pass + // "2.0.0/utils/LibBytes/LibBytes.sol" to resolver. + // + // This hack involves using path.resolve. But path.resolve returns + // absolute directories by default. We trick it into thinking that + // contractPath is a root directory by prepending a '/' and then + // removing the '/' the end. + // + // path.resolve("/a/b/c", ""../../d/e") === "/a/d/e" + // const lastPathSeparatorPos = contractPath.lastIndexOf('/'); - const importFolder = lastPathSeparatorPos === -1 ? '' : contractPath.slice(0, lastPathSeparatorPos + 1); - importPath = importPath.slice(0, 2) === './' ? importPath.replace(/^.\//, importFolder) : importPath; + const contractFolder = lastPathSeparatorPos === -1 ? '' : contractPath.slice(0, lastPathSeparatorPos + 1); + importPath = path.resolve('/' + contractFolder, importPath).replace('/', ''); if (_.isUndefined(sourcesToAppendTo[importPath])) { sourcesToAppendTo[importPath] = { id: nextId_ }; -- cgit v1.2.3 From 39a336ca6d6879ad6413cee714d929cb99bcd968 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 3 Oct 2018 10:44:19 -0400 Subject: fix: use original source ID's from compiler output Previously, was generating fresh source ID's but per @LogvinovLeon 's comment (cited below) that will likely break existing source code mappings. Changed to use the original source code mapping ID's that were generated by the compiler https://app.asana.com/0/684263176955174/842516551768097/f https://github.com/0xProject/0x-monorepo/pull/1108 https://github.com/0xProject/0x-monorepo/pull/1108#pullrequestreview-161059063 --- packages/sol-compiler/src/compiler.ts | 37 +++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 13 deletions(-) (limited to 'packages') diff --git a/packages/sol-compiler/src/compiler.ts b/packages/sol-compiler/src/compiler.ts index ea080c312..620728ed2 100644 --- a/packages/sol-compiler/src/compiler.ts +++ b/packages/sol-compiler/src/compiler.ts @@ -297,7 +297,7 @@ export class Compiler { ): Promise { const compiledContract = compilerOutput.contracts[contractPath][contractName]; - const { sourceCodes, sources } = this._getSourcesWithDependencies(contractPath); + const { sourceCodes, sources } = this._getSourcesWithDependencies(contractPath, compilerOutput.sources); const contractVersion: ContractVersionData = { compilerOutput: compiledContract, @@ -332,26 +332,39 @@ export class Compiler { await fsWrapper.writeFileAsync(currentArtifactPath, artifactString); logUtils.warn(`${contractName} artifact saved!`); } + /** + * For the given @param contractPath, populates JSON objects to be used in the ContractArtifact interface's + * properties `sources` (source code file names mapped to ID numbers) and `sourceCodes` (source code content of + * contracts) for that contract. The source code pointed to by contractPath is read and parsed directly (via + * `this._resolver.resolve().source`), as are its imports, recursively. The ID numbers for @return `sources` are + * taken from the corresponding ID's in @param fullSources, and the content for @return sourceCodes is read from + * disk (via the aforementioned `resolver.source`). + */ private _getSourcesWithDependencies( contractPath: string, + fullSources: { [sourceName: string]: { id: number } }, ): { sourceCodes: { [sourceName: string]: string }; sources: { [sourceName: string]: { id: number } } } { - const sources = { [contractPath]: { id: 0 } }; + const sources = { [contractPath]: { id: fullSources[contractPath].id } }; const sourceCodes = { [contractPath]: this._resolver.resolve(contractPath).source }; - this._recursivelyGatherDependencySources(contractPath, sourceCodes[contractPath], sources, sourceCodes, 1); + this._recursivelyGatherDependencySources( + contractPath, + sourceCodes[contractPath], + fullSources, + sources, + sourceCodes, + ); return { sourceCodes, sources }; } private _recursivelyGatherDependencySources( contractPath: string, contractSource: string, + fullSources: { [sourceName: string]: { id: number } }, sourcesToAppendTo: { [sourceName: string]: { id: number } }, sourceCodesToAppendTo: { [sourceName: string]: string }, - nextId: number, - ): number { - let nextId_ = nextId; - + ): void { const importStatementMatches = contractSource.match(/\nimport[^;]*;/g); if (importStatementMatches === null) { - return nextId_; + return; } for (const importStatementMatch of importStatementMatches) { const importPathMatches = importStatementMatch.match(/\"([^\"]*)\"/); @@ -380,20 +393,18 @@ export class Compiler { importPath = path.resolve('/' + contractFolder, importPath).replace('/', ''); if (_.isUndefined(sourcesToAppendTo[importPath])) { - sourcesToAppendTo[importPath] = { id: nextId_ }; + sourcesToAppendTo[importPath] = { id: fullSources[importPath].id }; sourceCodesToAppendTo[importPath] = this._resolver.resolve(importPath).source; - nextId_ += 1; - nextId_ = this._recursivelyGatherDependencySources( + this._recursivelyGatherDependencySources( importPath, this._resolver.resolve(importPath).source, + fullSources, sourcesToAppendTo, sourceCodesToAppendTo, - nextId_, ); } } - return nextId_; } private _compile(solcInstance: solc.SolcInstance, standardInput: solc.StandardInput): solc.StandardOutput { const compiled: solc.StandardOutput = JSON.parse( -- cgit v1.2.3 From f614a2425f79c0ba4982b0868ed6edd9c491ef1b Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 3 Oct 2018 10:57:37 -0400 Subject: fix: comment need for sourceCodes pruning --- packages/sol-compiler/src/compiler.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/sol-compiler/src/compiler.ts b/packages/sol-compiler/src/compiler.ts index 620728ed2..7eefc1474 100644 --- a/packages/sol-compiler/src/compiler.ts +++ b/packages/sol-compiler/src/compiler.ts @@ -297,6 +297,10 @@ export class Compiler { ): Promise { const compiledContract = compilerOutput.contracts[contractPath][contractName]; + // need to gather sourceCodes for this artifact, but compilerOutput.sources (the list of contract modules) + // contains listings for for every contract compiled during the compiler invocation that compiled the contract + // to be persisted, which could include many that are irrelevant to the contract at hand. So, gather up only + // the relevant sources: const { sourceCodes, sources } = this._getSourcesWithDependencies(contractPath, compilerOutput.sources); const contractVersion: ContractVersionData = { @@ -333,7 +337,7 @@ export class Compiler { logUtils.warn(`${contractName} artifact saved!`); } /** - * For the given @param contractPath, populates JSON objects to be used in the ContractArtifact interface's + * For the given @param contractPath, populates JSON objects to be used in the ContractVersionData interface's * properties `sources` (source code file names mapped to ID numbers) and `sourceCodes` (source code content of * contracts) for that contract. The source code pointed to by contractPath is read and parsed directly (via * `this._resolver.resolve().source`), as are its imports, recursively. The ID numbers for @return `sources` are -- cgit v1.2.3 From 57b4396193b731d97f73f4e3648a8dbdc1f589ac Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 13:23:48 -0700 Subject: Add buy_quote_calculator_test --- .../asset-buyer/src/utils/buy_quote_calculator.ts | 147 ++++++++++++++++----- .../asset-buyer/test/buy_quote_calculator_test.ts | 130 ++++++++++++++++++ 2 files changed, 246 insertions(+), 31 deletions(-) create mode 100644 packages/asset-buyer/test/buy_quote_calculator_test.ts (limited to 'packages') diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index b706ea143..53f2228e9 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -1,4 +1,4 @@ -import { marketUtils } from '@0xproject/order-utils'; +import { marketUtils, rateUtils } from '@0xproject/order-utils'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; @@ -21,6 +21,7 @@ export const buyQuoteCalculator = { const feeOrders = feeOrdersAndFillableAmounts.orders; const remainingFillableFeeAmounts = feeOrdersAndFillableAmounts.remainingFillableMakerAssetAmounts; const slippageBufferAmount = assetBuyAmount.mul(slippagePercentage).round(); + // find the orders that cover the desired assetBuyAmount (with slippage) const { resultOrders, remainingFillAmount, @@ -29,9 +30,11 @@ export const buyQuoteCalculator = { remainingFillableMakerAssetAmounts, slippageBufferAmount, }); + // if we do not have enough orders to cover the desired assetBuyAmount, throw if (remainingFillAmount.gt(constants.ZERO_AMOUNT)) { throw new Error(AssetBuyerError.InsufficientAssetLiquidity); } + // given the orders calculated above, find the fee-orders that cover the desired assetBuyAmount (with slippage) // TODO(bmillman): optimization // update this logic to find the minimum amount of feeOrders to cover the worst case as opposed to // finding order that cover all fees, this will help with estimating ETH and minimizing gas usage @@ -40,49 +43,131 @@ export const buyQuoteCalculator = { remainingFeeAmount, feeOrdersRemainingFillableMakerAssetAmounts, } = marketUtils.findFeeOrdersThatCoverFeesForTargetOrders(resultOrders, feeOrders, { - remainingFillableMakerAssetAmounts, + remainingFillableMakerAssetAmounts: ordersRemainingFillableMakerAssetAmounts, remainingFillableFeeAmounts, }); + // if we do not have enough feeOrders to cover the fees, throw if (remainingFeeAmount.gt(constants.ZERO_AMOUNT)) { throw new Error(AssetBuyerError.InsufficientZrxLiquidity); } + // assetData information for the result const assetData = orders[0].makerAssetData; - - // calculate minRate and maxRate by calculating min and max eth usage and then dividing into - // assetBuyAmount to get assetData / WETH, needs to take into account feePercentage as well - // minEthAmount = (sum(takerAssetAmount[i]) until sum(makerAssetAmount[i]) >= assetBuyAmount ) * (1 + feePercentage) - // maxEthAmount = (sum(takerAssetAmount[i]) until i == orders.length) * (1 + feePercentage) - const allOrders = _.concat(resultOrders, resultFeeOrders); - const allRemainingAmounts = _.concat( - ordersRemainingFillableMakerAssetAmounts, - feeOrdersRemainingFillableMakerAssetAmounts, + // compile the resulting trimmed set of orders for makerAsset and feeOrders that are needed for assetBuyAmount + const trimmedOrdersAndFillableAmounts: OrdersAndFillableAmounts = { + orders: resultOrders, + remainingFillableMakerAssetAmounts: ordersRemainingFillableMakerAssetAmounts, + }; + const trimmedFeeOrdersAndFillableAmounts: OrdersAndFillableAmounts = { + orders: resultFeeOrders, + remainingFillableMakerAssetAmounts: feeOrdersRemainingFillableMakerAssetAmounts, + }; + const minRate = calculateRate( + trimmedOrdersAndFillableAmounts, + trimmedFeeOrdersAndFillableAmounts, + assetBuyAmount, + feePercentage, + ); + // in order to calculate the maxRate, reverse the ordersAndFillableAmounts such that they are sorted from worst rate to best rate + const maxRate = calculateRate( + reverseOrdersAndFillableAmounts(trimmedOrdersAndFillableAmounts), + reverseOrdersAndFillableAmounts(trimmedFeeOrdersAndFillableAmounts), + assetBuyAmount, + feePercentage, ); - let minEthAmount = constants.ZERO_AMOUNT; - let maxEthAmount = constants.ZERO_AMOUNT; - let cumulativeMakerAmount = constants.ZERO_AMOUNT; - _.forEach(allOrders, (order, index) => { - const remainingFillableMakerAssetAmount = allRemainingAmounts[index]; - const claimableTakerAssetAmount = orderUtils.calculateRemainingTakerAssetAmount( - order, - remainingFillableMakerAssetAmount, - ); - // taker asset is always assumed to be WETH - maxEthAmount = maxEthAmount.plus(claimableTakerAssetAmount); - if (cumulativeMakerAmount.lessThan(assetBuyAmount)) { - minEthAmount = minEthAmount.plus(claimableTakerAssetAmount); - } - cumulativeMakerAmount = cumulativeMakerAmount.plus(remainingFillableMakerAssetAmount); - }); - const feeAdjustedMinRate = minEthAmount.mul(feePercentage + 1).div(assetBuyAmount); - const feeAdjustedMaxRate = minEthAmount.mul(feePercentage + 1).div(assetBuyAmount); return { assetData, orders: resultOrders, feeOrders: resultFeeOrders, - minRate: feeAdjustedMinRate, - maxRate: feeAdjustedMaxRate, + minRate, + maxRate, assetBuyAmount, feePercentage, }; }, }; + +function calculateRate( + ordersAndFillableAmounts: OrdersAndFillableAmounts, + feeOrdersAndFillableAmounts: OrdersAndFillableAmounts, + assetBuyAmount: BigNumber, + feePercentage: number, +): BigNumber { + // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right (best rate to worst rate) + const [minEthAmountToBuyAsset, minZrxAmountToBuyAsset] = findEthAndZrxAmountNeededToBuyAsset( + ordersAndFillableAmounts, + assetBuyAmount, + ); + // find the total eth needed to buy fees + const minEthAmountToBuyFees = findEthAmountNeededToBuyFees(feeOrdersAndFillableAmounts, minZrxAmountToBuyAsset); + const finalMinEthAmount = minEthAmountToBuyAsset.plus(minEthAmountToBuyFees).mul(feePercentage + 1); + // divide into the assetBuyAmount in order to find rate of makerAsset / WETH + const result = assetBuyAmount.div(finalMinEthAmount); + return result; +} + +// given an OrdersAndFillableAmounts, reverse the orders and remainingFillableMakerAssetAmounts properties +function reverseOrdersAndFillableAmounts(ordersAndFillableAmounts: OrdersAndFillableAmounts): OrdersAndFillableAmounts { + const ordersCopy = _.clone(ordersAndFillableAmounts.orders); + const remainingFillableMakerAssetAmountsCopy = _.clone(ordersAndFillableAmounts.remainingFillableMakerAssetAmounts); + return { + orders: ordersCopy.reverse(), + remainingFillableMakerAssetAmounts: remainingFillableMakerAssetAmountsCopy.reverse(), + }; +} + +function findEthAmountNeededToBuyFees( + feeOrdersAndFillableAmounts: OrdersAndFillableAmounts, + feeAmount: BigNumber, +): BigNumber { + const { orders, remainingFillableMakerAssetAmounts } = feeOrdersAndFillableAmounts; + const result = _.reduce( + orders, + (acc, order, index) => { + const remainingFillableMakerAssetAmount = remainingFillableMakerAssetAmounts[index]; + const amountToFill = BigNumber.min(acc.remainingFeeAmount, remainingFillableMakerAssetAmount); + const feeAdjustedRate = rateUtils.getFeeAdjustedRateOfFeeOrder(order); + const ethAmountForThisOrder = feeAdjustedRate.mul(amountToFill); + return { + ethAmount: acc.ethAmount.plus(ethAmountForThisOrder), + remainingFeeAmount: BigNumber.max(constants.ZERO_AMOUNT, acc.remainingFeeAmount.minus(amountToFill)), + }; + }, + { + ethAmount: constants.ZERO_AMOUNT, + remainingFeeAmount: feeAmount, + }, + ); + return result.ethAmount; +} + +function findEthAndZrxAmountNeededToBuyAsset( + ordersAndFillableAmounts: OrdersAndFillableAmounts, + assetBuyAmount: BigNumber, +): [BigNumber, BigNumber] { + const { orders, remainingFillableMakerAssetAmounts } = ordersAndFillableAmounts; + const result = _.reduce( + orders, + (acc, order, index) => { + const remainingFillableMakerAssetAmount = remainingFillableMakerAssetAmounts[index]; + const amountToFill = BigNumber.min(acc.remainingAssetBuyAmount, remainingFillableMakerAssetAmount); + const ethAmountForThisOrder = amountToFill + .mul(order.takerAssetAmount) + .dividedToIntegerBy(order.makerAssetAmount); + const zrxAmountForThisOrder = amountToFill.mul(order.takerFee).dividedToIntegerBy(order.makerAssetAmount); + return { + ethAmount: acc.ethAmount.plus(ethAmountForThisOrder), + zrxAmount: acc.ethAmount.plus(zrxAmountForThisOrder), + remainingAssetBuyAmount: BigNumber.max( + constants.ZERO_AMOUNT, + acc.remainingAssetBuyAmount.minus(amountToFill), + ), + }; + }, + { + ethAmount: constants.ZERO_AMOUNT, + zrxAmount: constants.ZERO_AMOUNT, + remainingAssetBuyAmount: assetBuyAmount, + }, + ); + return [result.ethAmount, result.zrxAmount]; +} diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts new file mode 100644 index 000000000..2cad1ab05 --- /dev/null +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -0,0 +1,130 @@ +import { orderFactory } from '@0xproject/order-utils/lib/src/order_factory'; +import { BigNumber } from '@0xproject/utils'; +import * as chai from 'chai'; +import * as _ from 'lodash'; +import 'mocha'; + +import { AssetBuyerError, OrdersAndFillableAmounts } from '../src/types'; +import { buyQuoteCalculator } from '../src/utils/buy_quote_calculator'; + +import { chaiSetup } from './utils/chai_setup'; + +chaiSetup.configure(); +const expect = chai.expect; + +const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; +const NULL_BYTES = '0x'; + +// tslint:disable:custom-no-magic-numbers +describe('buyQuoteCalculator', () => { + describe('#calculate', () => { + let ordersAndFillableAmounts: OrdersAndFillableAmounts; + let feeOrdersAndFillableAmounts: OrdersAndFillableAmounts; + beforeEach(() => { + // generate two orders for our desired maker asset + // the first order has a rate of 4 makerAsset / WETH with a takerFee of 200 ZRX and has only 200 / 400 makerAsset units left to fill (half fillable) + // the second order has a rate of 2 makerAsset / WETH with a takerFee of 100 ZRX and has 200 / 200 makerAsset units left to fill (completely fillable) + // generate one order for fees + // the fee order has a rate of 1 ZRX / WETH with no taker fee and has 100 ZRX left to fill (completely fillable) + const firstOrder = orderFactory.createOrder( + NULL_ADDRESS, + new BigNumber(400), + NULL_BYTES, + new BigNumber(100), + NULL_BYTES, + NULL_ADDRESS, + { + takerFee: new BigNumber(200), + }, + ); + const firstRemainingFillAmount = new BigNumber(200); + const secondOrder = orderFactory.createOrder( + NULL_ADDRESS, + new BigNumber(200), + NULL_BYTES, + new BigNumber(100), + NULL_BYTES, + NULL_ADDRESS, + { + takerFee: new BigNumber(100), + }, + ); + const secondRemainingFillAmount = secondOrder.makerAssetAmount; + const signedOrders = _.map([firstOrder, secondOrder], order => { + return { + ...order, + signature: NULL_BYTES, + }; + }); + ordersAndFillableAmounts = { + orders: signedOrders, + remainingFillableMakerAssetAmounts: [firstRemainingFillAmount, secondRemainingFillAmount], + }; + const feeOrder = orderFactory.createOrder( + NULL_ADDRESS, + new BigNumber(100), + NULL_BYTES, + new BigNumber(100), + NULL_BYTES, + NULL_ADDRESS, + ); + const signedFeeOrder = { + ...feeOrder, + signature: NULL_BYTES, + }; + feeOrdersAndFillableAmounts = { + orders: [signedFeeOrder], + remainingFillableMakerAssetAmounts: [signedFeeOrder.makerAssetAmount], + }; + }); + it('should throw if not enough maker asset liquidity', () => { + // we have 400 makerAsset units available to fill but attempt to calculate a quote for 500 makerAsset units + expect(() => + buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + feeOrdersAndFillableAmounts, + new BigNumber(500), + 0, + 0, + ), + ).to.throw(AssetBuyerError.InsufficientAssetLiquidity); + }); + it('should throw if not enough ZRX liquidity', () => { + // we request 300 makerAsset units but the ZRX order is only enough to fill the first order, which only has 200 makerAssetUnits available + expect(() => + buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + feeOrdersAndFillableAmounts, + new BigNumber(300), + 0, + 0, + ), + ).to.throw(AssetBuyerError.InsufficientZrxLiquidity); + }); + it('calculates a correct buyQuote', () => { + // we request 200 makerAsset units which can be filled using the first order + // the first order requires a fee of 100 ZRX from the taker which can be filled by the feeOrder + const assetBuyAmount = new BigNumber(200); + const feePercentage = 0.02; + const slippagePercentage = 0; + const buyQuote = buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + feeOrdersAndFillableAmounts, + assetBuyAmount, + feePercentage, + slippagePercentage, + ); + // test if orders are correct + expect(buyQuote.orders).to.deep.equal([ordersAndFillableAmounts.orders[0]]); + expect(buyQuote.feeOrders).to.deep.equal([feeOrdersAndFillableAmounts.orders[0]]); + // test if rates are correct + const expectedMinEthToFill = new BigNumber(150); + const expectedMinRate = assetBuyAmount.div(expectedMinEthToFill.mul(feePercentage + 1)); + expect(buyQuote.minRate).to.bignumber.equal(expectedMinRate); + // because we have no slippage protection, minRate is equal to maxRate + expect(buyQuote.maxRate).to.bignumber.equal(expectedMinRate); + // test if feePercentage gets passed through + expect(buyQuote.feePercentage).to.equal(feePercentage); + }); + }); +}); -- cgit v1.2.3 From 260db053fe45ae8d5973207b43d762d905b28cd8 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 14:50:59 -0700 Subject: Add additional test for slippage --- .../asset-buyer/src/utils/buy_quote_calculator.ts | 12 ++-- .../asset-buyer/test/buy_quote_calculator_test.ts | 76 ++++++++++++++++++---- 2 files changed, 70 insertions(+), 18 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index 53f2228e9..5a531e000 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -92,16 +92,16 @@ function calculateRate( assetBuyAmount: BigNumber, feePercentage: number, ): BigNumber { - // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right (best rate to worst rate) - const [minEthAmountToBuyAsset, minZrxAmountToBuyAsset] = findEthAndZrxAmountNeededToBuyAsset( + // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right + const [ethAmountToBuyAsset, zrxAmountToBuyAsset] = findEthAndZrxAmountNeededToBuyAsset( ordersAndFillableAmounts, assetBuyAmount, ); // find the total eth needed to buy fees - const minEthAmountToBuyFees = findEthAmountNeededToBuyFees(feeOrdersAndFillableAmounts, minZrxAmountToBuyAsset); - const finalMinEthAmount = minEthAmountToBuyAsset.plus(minEthAmountToBuyFees).mul(feePercentage + 1); + const ethAmountToBuyFees = findEthAmountNeededToBuyFees(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset); + const ethAmount = ethAmountToBuyAsset.plus(ethAmountToBuyFees).mul(feePercentage + 1); // divide into the assetBuyAmount in order to find rate of makerAsset / WETH - const result = assetBuyAmount.div(finalMinEthAmount); + const result = assetBuyAmount.div(ethAmount); return result; } @@ -156,7 +156,7 @@ function findEthAndZrxAmountNeededToBuyAsset( const zrxAmountForThisOrder = amountToFill.mul(order.takerFee).dividedToIntegerBy(order.makerAssetAmount); return { ethAmount: acc.ethAmount.plus(ethAmountForThisOrder), - zrxAmount: acc.ethAmount.plus(zrxAmountForThisOrder), + zrxAmount: acc.zrxAmount.plus(zrxAmountForThisOrder), remainingAssetBuyAmount: BigNumber.max( constants.ZERO_AMOUNT, acc.remainingAssetBuyAmount.minus(amountToFill), diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 2cad1ab05..37a429531 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -19,7 +19,8 @@ const NULL_BYTES = '0x'; describe('buyQuoteCalculator', () => { describe('#calculate', () => { let ordersAndFillableAmounts: OrdersAndFillableAmounts; - let feeOrdersAndFillableAmounts: OrdersAndFillableAmounts; + let smallFeeOrderAndFillableAmount: OrdersAndFillableAmounts; + let allFeeOrdersAndFillableAmounts: OrdersAndFillableAmounts; beforeEach(() => { // generate two orders for our desired maker asset // the first order has a rate of 4 makerAsset / WETH with a takerFee of 200 ZRX and has only 200 / 400 makerAsset units left to fill (half fillable) @@ -60,7 +61,7 @@ describe('buyQuoteCalculator', () => { orders: signedOrders, remainingFillableMakerAssetAmounts: [firstRemainingFillAmount, secondRemainingFillAmount], }; - const feeOrder = orderFactory.createOrder( + const smallFeeOrder = orderFactory.createOrder( NULL_ADDRESS, new BigNumber(100), NULL_BYTES, @@ -68,13 +69,32 @@ describe('buyQuoteCalculator', () => { NULL_BYTES, NULL_ADDRESS, ); - const signedFeeOrder = { - ...feeOrder, + const signedSmallFeeOrder = { + ...smallFeeOrder, signature: NULL_BYTES, }; - feeOrdersAndFillableAmounts = { - orders: [signedFeeOrder], - remainingFillableMakerAssetAmounts: [signedFeeOrder.makerAssetAmount], + smallFeeOrderAndFillableAmount = { + orders: [signedSmallFeeOrder], + remainingFillableMakerAssetAmounts: [signedSmallFeeOrder.makerAssetAmount], + }; + const largeFeeOrder = orderFactory.createOrder( + NULL_ADDRESS, + new BigNumber(100), + NULL_BYTES, + new BigNumber(200), + NULL_BYTES, + NULL_ADDRESS, + ); + const signedLargeFeeOrder = { + ...largeFeeOrder, + signature: NULL_BYTES, + }; + allFeeOrdersAndFillableAmounts = { + orders: [signedSmallFeeOrder, signedLargeFeeOrder], + remainingFillableMakerAssetAmounts: [ + signedSmallFeeOrder.makerAssetAmount, + largeFeeOrder.makerAssetAmount, + ], }; }); it('should throw if not enough maker asset liquidity', () => { @@ -82,7 +102,7 @@ describe('buyQuoteCalculator', () => { expect(() => buyQuoteCalculator.calculate( ordersAndFillableAmounts, - feeOrdersAndFillableAmounts, + smallFeeOrderAndFillableAmount, new BigNumber(500), 0, 0, @@ -94,14 +114,14 @@ describe('buyQuoteCalculator', () => { expect(() => buyQuoteCalculator.calculate( ordersAndFillableAmounts, - feeOrdersAndFillableAmounts, + smallFeeOrderAndFillableAmount, new BigNumber(300), 0, 0, ), ).to.throw(AssetBuyerError.InsufficientZrxLiquidity); }); - it('calculates a correct buyQuote', () => { + it('calculates a correct buyQuote with no slippage', () => { // we request 200 makerAsset units which can be filled using the first order // the first order requires a fee of 100 ZRX from the taker which can be filled by the feeOrder const assetBuyAmount = new BigNumber(200); @@ -109,15 +129,16 @@ describe('buyQuoteCalculator', () => { const slippagePercentage = 0; const buyQuote = buyQuoteCalculator.calculate( ordersAndFillableAmounts, - feeOrdersAndFillableAmounts, + smallFeeOrderAndFillableAmount, assetBuyAmount, feePercentage, slippagePercentage, ); // test if orders are correct expect(buyQuote.orders).to.deep.equal([ordersAndFillableAmounts.orders[0]]); - expect(buyQuote.feeOrders).to.deep.equal([feeOrdersAndFillableAmounts.orders[0]]); + expect(buyQuote.feeOrders).to.deep.equal([smallFeeOrderAndFillableAmount.orders[0]]); // test if rates are correct + // 50 eth to fill the first order + 100 eth for fees const expectedMinEthToFill = new BigNumber(150); const expectedMinRate = assetBuyAmount.div(expectedMinEthToFill.mul(feePercentage + 1)); expect(buyQuote.minRate).to.bignumber.equal(expectedMinRate); @@ -126,5 +147,36 @@ describe('buyQuoteCalculator', () => { // test if feePercentage gets passed through expect(buyQuote.feePercentage).to.equal(feePercentage); }); + it('calculates a correct buyQuote with with slippage', () => { + // we request 200 makerAsset units which can be filled using the first order + // however with 50% slippage we are protecting the buy with 100 extra makerAssetUnits + // so we need enough orders to fill 300 makerAssetUnits + // 300 makerAssetUnits can only be filled using both orders + // the first order requires a fee of 100 ZRX from the taker which can be filled by the feeOrder + const assetBuyAmount = new BigNumber(200); + const feePercentage = 0.02; + const slippagePercentage = 0.5; + const buyQuote = buyQuoteCalculator.calculate( + ordersAndFillableAmounts, + allFeeOrdersAndFillableAmounts, + assetBuyAmount, + feePercentage, + slippagePercentage, + ); + // test if orders are correct + expect(buyQuote.orders).to.deep.equal(ordersAndFillableAmounts.orders); + expect(buyQuote.feeOrders).to.deep.equal(allFeeOrdersAndFillableAmounts.orders); + // test if rates are correct + // 50 eth to fill the first order + 100 eth for fees + const expectedMinEthToFill = new BigNumber(150); + const expectedMinRate = assetBuyAmount.div(expectedMinEthToFill.mul(feePercentage + 1)); + expect(buyQuote.minRate).to.bignumber.equal(expectedMinRate); + // 100 eth to fill the first order + 200 eth for fees + const expectedMaxEthToFill = new BigNumber(300); + const expectedMaxRate = assetBuyAmount.div(expectedMaxEthToFill.mul(feePercentage + 1)); + expect(buyQuote.maxRate).to.bignumber.equal(expectedMaxRate); + // test if feePercentage gets passed through + expect(buyQuote.feePercentage).to.equal(feePercentage); + }); }); }); -- cgit v1.2.3 From 93736c15675973f053e7b9b072c1d029b9dd385d Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 14:52:44 -0700 Subject: Fix linter --- packages/asset-buyer/src/utils/buy_quote_calculator.ts | 2 -- 1 file changed, 2 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index 5a531e000..9ccaa7933 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -5,8 +5,6 @@ import * as _ from 'lodash'; import { constants } from '../constants'; import { AssetBuyerError, BuyQuote, OrdersAndFillableAmounts } from '../types'; -import { orderUtils } from './order_utils'; - // Calculates a buy quote for orders that have WETH as the takerAsset export const buyQuoteCalculator = { calculate( -- cgit v1.2.3 From 10f54893ef483d40de4d72f752ae921c62c9ac62 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 14:59:06 -0700 Subject: Update CHANGELOG --- packages/asset-buyer/CHANGELOG.json | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'packages') diff --git a/packages/asset-buyer/CHANGELOG.json b/packages/asset-buyer/CHANGELOG.json index 8c2c7a8d2..df67059cb 100644 --- a/packages/asset-buyer/CHANGELOG.json +++ b/packages/asset-buyer/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Expand AssetBuyer to work with multiple assets at once", "pr": 1086 + }, + { + "note": "Fix minRate and maxRate calculation", + "pr": 1113 } ] }, -- cgit v1.2.3 From 250a9a480940ae4fca48109aae97ee0323d57a52 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 22:54:32 -0700 Subject: Add comments about buy quote calculation --- packages/asset-buyer/src/utils/buy_quote_calculator.ts | 2 ++ 1 file changed, 2 insertions(+) (limited to 'packages') diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index 9ccaa7933..78666356c 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -148,9 +148,11 @@ function findEthAndZrxAmountNeededToBuyAsset( (acc, order, index) => { const remainingFillableMakerAssetAmount = remainingFillableMakerAssetAmounts[index]; const amountToFill = BigNumber.min(acc.remainingAssetBuyAmount, remainingFillableMakerAssetAmount); + // find the amount of eth required to fill amountToFill (amountToFill / makerAssetAmount) * takerAssetAmount const ethAmountForThisOrder = amountToFill .mul(order.takerAssetAmount) .dividedToIntegerBy(order.makerAssetAmount); + // find the amount of zrx required to fill fees for amountToFill (amountToFill / makerAssetAmount) * takerFee const zrxAmountForThisOrder = amountToFill.mul(order.takerFee).dividedToIntegerBy(order.makerAssetAmount); return { ethAmount: acc.ethAmount.plus(ethAmountForThisOrder), -- cgit v1.2.3 From 059162a90a3d26e5fdfefd8553bb1f721a3116fc Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 23:14:55 -0700 Subject: Add additional order factory methods and refactor test to use them --- .../asset-buyer/test/buy_quote_calculator_test.ts | 83 ++++++---------------- packages/order-utils/src/order_factory.ts | 39 +++++++++- 2 files changed, 61 insertions(+), 61 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 37a429531..3bd0f8d4e 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -27,74 +27,37 @@ describe('buyQuoteCalculator', () => { // the second order has a rate of 2 makerAsset / WETH with a takerFee of 100 ZRX and has 200 / 200 makerAsset units left to fill (completely fillable) // generate one order for fees // the fee order has a rate of 1 ZRX / WETH with no taker fee and has 100 ZRX left to fill (completely fillable) - const firstOrder = orderFactory.createOrder( - NULL_ADDRESS, - new BigNumber(400), - NULL_BYTES, - new BigNumber(100), - NULL_BYTES, - NULL_ADDRESS, - { - takerFee: new BigNumber(200), - }, - ); + const firstOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(400), + takerAssetAmount: new BigNumber(100), + takerFee: new BigNumber(200), + }); const firstRemainingFillAmount = new BigNumber(200); - const secondOrder = orderFactory.createOrder( - NULL_ADDRESS, - new BigNumber(200), - NULL_BYTES, - new BigNumber(100), - NULL_BYTES, - NULL_ADDRESS, - { - takerFee: new BigNumber(100), - }, - ); - const secondRemainingFillAmount = secondOrder.makerAssetAmount; - const signedOrders = _.map([firstOrder, secondOrder], order => { - return { - ...order, - signature: NULL_BYTES, - }; + const secondOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(200), + takerAssetAmount: new BigNumber(100), + takerFee: new BigNumber(100), }); + const secondRemainingFillAmount = secondOrder.makerAssetAmount; ordersAndFillableAmounts = { - orders: signedOrders, + orders: [firstOrder, secondOrder], remainingFillableMakerAssetAmounts: [firstRemainingFillAmount, secondRemainingFillAmount], }; - const smallFeeOrder = orderFactory.createOrder( - NULL_ADDRESS, - new BigNumber(100), - NULL_BYTES, - new BigNumber(100), - NULL_BYTES, - NULL_ADDRESS, - ); - const signedSmallFeeOrder = { - ...smallFeeOrder, - signature: NULL_BYTES, - }; + const smallFeeOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(100), + takerAssetAmount: new BigNumber(100), + }); smallFeeOrderAndFillableAmount = { - orders: [signedSmallFeeOrder], - remainingFillableMakerAssetAmounts: [signedSmallFeeOrder.makerAssetAmount], - }; - const largeFeeOrder = orderFactory.createOrder( - NULL_ADDRESS, - new BigNumber(100), - NULL_BYTES, - new BigNumber(200), - NULL_BYTES, - NULL_ADDRESS, - ); - const signedLargeFeeOrder = { - ...largeFeeOrder, - signature: NULL_BYTES, + orders: [smallFeeOrder], + remainingFillableMakerAssetAmounts: [smallFeeOrder.makerAssetAmount], }; + const largeFeeOrder = orderFactory.createSignedOrderFromPartial({ + makerAssetAmount: new BigNumber(100), + takerAssetAmount: new BigNumber(200), + }); allFeeOrdersAndFillableAmounts = { - orders: [signedSmallFeeOrder, signedLargeFeeOrder], - remainingFillableMakerAssetAmounts: [ - signedSmallFeeOrder.makerAssetAmount, - largeFeeOrder.makerAssetAmount, - ], + orders: [smallFeeOrder, largeFeeOrder], + remainingFillableMakerAssetAmounts: [smallFeeOrder.makerAssetAmount, largeFeeOrder.makerAssetAmount], }; }); it('should throw if not enough maker asset liquidity', () => { diff --git a/packages/order-utils/src/order_factory.ts b/packages/order-utils/src/order_factory.ts index 46a69ae4d..b1292903a 100644 --- a/packages/order-utils/src/order_factory.ts +++ b/packages/order-utils/src/order_factory.ts @@ -8,8 +8,21 @@ import { orderHashUtils } from './order_hash'; import { generatePseudoRandomSalt } from './salt'; import { signatureUtils } from './signature_utils'; import { CreateOrderOpts } from './types'; - export const orderFactory = { + createOrderFromPartial(partialOrder: Partial): Order { + const defaultOrder = generateEmptyOrder(); + return { + ...defaultOrder, + ...partialOrder, + }; + }, + createSignedOrderFromPartial(partialSignedOrder: Partial): SignedOrder { + const defaultOrder = generateEmptySignedOrder(); + return { + ...defaultOrder, + ...partialSignedOrder, + }; + }, createOrder( makerAddress: string, makerAssetAmount: BigNumber, @@ -69,6 +82,30 @@ export const orderFactory = { }, }; +function generateEmptySignedOrder(): SignedOrder { + return { + ...generateEmptyOrder(), + signature: constants.NULL_BYTES, + }; +} +function generateEmptyOrder(): Order { + return { + senderAddress: constants.NULL_ADDRESS, + makerAddress: constants.NULL_ADDRESS, + takerAddress: constants.NULL_ADDRESS, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + makerAssetAmount: constants.ZERO_AMOUNT, + takerAssetAmount: constants.ZERO_AMOUNT, + makerAssetData: constants.NULL_BYTES, + takerAssetData: constants.NULL_BYTES, + salt: generatePseudoRandomSalt(), + exchangeAddress: constants.NULL_ADDRESS, + feeRecipientAddress: constants.NULL_ADDRESS, + expirationTimeSeconds: constants.INFINITE_TIMESTAMP_SEC, + }; +} + function generateDefaultCreateOrderOpts(): { takerAddress: string; senderAddress: string; -- cgit v1.2.3 From 24e0fbd7b9debcfd193f9174f0254d7d5ae22c12 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 23:22:38 -0700 Subject: Add fee order with a takerFee --- packages/asset-buyer/test/buy_quote_calculator_test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 3bd0f8d4e..0108dfcc6 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -52,12 +52,16 @@ describe('buyQuoteCalculator', () => { remainingFillableMakerAssetAmounts: [smallFeeOrder.makerAssetAmount], }; const largeFeeOrder = orderFactory.createSignedOrderFromPartial({ - makerAssetAmount: new BigNumber(100), + makerAssetAmount: new BigNumber(110), takerAssetAmount: new BigNumber(200), + takerFee: new BigNumber(10), }); allFeeOrdersAndFillableAmounts = { orders: [smallFeeOrder, largeFeeOrder], - remainingFillableMakerAssetAmounts: [smallFeeOrder.makerAssetAmount, largeFeeOrder.makerAssetAmount], + remainingFillableMakerAssetAmounts: [ + smallFeeOrder.makerAssetAmount, + largeFeeOrder.makerAssetAmount.minus(largeFeeOrder.takerFee), + ], }; }); it('should throw if not enough maker asset liquidity', () => { -- cgit v1.2.3 From a7a007435cfefc8146246c0db66efdee05c809b8 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 23:23:53 -0700 Subject: Remove unused constants --- packages/asset-buyer/test/buy_quote_calculator_test.ts | 3 --- 1 file changed, 3 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 0108dfcc6..667dec051 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -12,9 +12,6 @@ import { chaiSetup } from './utils/chai_setup'; chaiSetup.configure(); const expect = chai.expect; -const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; -const NULL_BYTES = '0x'; - // tslint:disable:custom-no-magic-numbers describe('buyQuoteCalculator', () => { describe('#calculate', () => { -- cgit v1.2.3 From 4394036e341e9755a57a29680f5b14f3c92fef9c Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 3 Oct 2018 23:46:07 -0700 Subject: Add missing default options --- packages/asset-buyer/src/asset_buyer.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/src/asset_buyer.ts b/packages/asset-buyer/src/asset_buyer.ts index 0bb757f52..a56bc01d5 100644 --- a/packages/asset-buyer/src/asset_buyer.ts +++ b/packages/asset-buyer/src/asset_buyer.ts @@ -123,7 +123,7 @@ export class AssetBuyer { public async getBuyQuoteAsync( assetData: string, assetBuyAmount: BigNumber, - options: Partial, + options: Partial = {}, ): Promise { const { feePercentage, shouldForceOrderRefresh, slippagePercentage } = { ...constants.DEFAULT_BUY_QUOTE_REQUEST_OPTS, @@ -164,7 +164,7 @@ export class AssetBuyer { public async getBuyQuoteForERC20TokenAddressAsync( tokenAddress: string, assetBuyAmount: BigNumber, - options: Partial, + options: Partial = {}, ): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isBigNumber('assetBuyAmount', assetBuyAmount); @@ -179,7 +179,10 @@ export class AssetBuyer { * * @return A promise of the txHash. */ - public async executeBuyQuoteAsync(buyQuote: BuyQuote, options: Partial): Promise { + public async executeBuyQuoteAsync( + buyQuote: BuyQuote, + options: Partial = {}, + ): Promise { const { rate, takerAddress, feeRecipient } = { ...constants.DEFAULT_BUY_QUOTE_EXECUTION_OPTS, ...options, -- cgit v1.2.3 From c613b6741d7eff069c93ef1a800b67ddfb618c51 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 4 Oct 2018 15:47:43 +0100 Subject: Upgrade webpack --- packages/0x.js/package.json | 7 ++- packages/0x.js/webpack.config.js | 31 +++++------ packages/contract-wrappers/package.json | 1 - packages/order-watcher/package.json | 2 - packages/subproviders/package.json | 2 +- packages/testnet-faucets/package.json | 6 +-- packages/website/package.json | 28 +++++----- packages/website/ts/index.tsx | 30 +++++------ packages/website/ts/lazy_component.tsx | 2 +- packages/website/webpack.config.js | 95 +++++++++++++++++---------------- 10 files changed, 99 insertions(+), 105 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/package.json b/packages/0x.js/package.json index c89a3e613..268972a4f 100644 --- a/packages/0x.js/package.json +++ b/packages/0x.js/package.json @@ -52,13 +52,12 @@ "@types/node": "*", "@types/sinon": "^2.2.2", "@types/web3-provider-engine": "^14.0.0", - "awesome-typescript-loader": "^3.1.3", + "awesome-typescript-loader": "^5.2.1", "chai": "^4.0.1", "chai-as-promised": "^7.1.0", "chai-bignumber": "^2.0.1", "copyfiles": "^2.0.0", "dirty-chai": "^2.0.1", - "json-loader": "^0.5.4", "make-promises-safe": "^1.1.0", "mocha": "^4.1.0", "npm-run-all": "^4.1.2", @@ -70,8 +69,8 @@ "tslint": "5.11.0", "typedoc": "0.12.0", "typescript": "3.0.1", - "uglifyjs-webpack-plugin": "^1.3.0", - "webpack": "^3.1.0" + "uglifyjs-webpack-plugin": "^2.0.1", + "webpack": "^4.20.2" }, "dependencies": { "@0xproject/assert": "^1.0.12", diff --git a/packages/0x.js/webpack.config.js b/packages/0x.js/webpack.config.js index 1ad0a79ec..397faa76f 100644 --- a/packages/0x.js/webpack.config.js +++ b/packages/0x.js/webpack.config.js @@ -2,8 +2,7 @@ * This is to generate the umd bundle only */ const _ = require('lodash'); -const webpack = require('webpack'); -const UglifyJsPlugin = require('uglifyjs-webpack-plugin'); +const TerserPlugin = require('terser-webpack-plugin'); const path = require('path'); const production = process.env.NODE_ENV === 'production'; @@ -16,6 +15,7 @@ if (production) { module.exports = { entry, + mode: 'production', output: { path: path.resolve(__dirname, '_bundles'), filename: '[name].js', @@ -27,19 +27,18 @@ module.exports = { extensions: ['.ts', '.js', '.json'], }, devtool: 'source-map', - plugins: [ - // TODO: Revert to webpack bundled version with webpack v4. - // The v3 series bundled version does not support ES6 and - // fails to build. - new UglifyJsPlugin({ - sourceMap: true, - uglifyOptions: { - mangle: { - reserved: ['BigNumber'], + optimization: { + minimizer: [ + new TerserPlugin({ + sourceMap: true, + terserOptions: { + mangle: { + reserved: ['BigNumber'], + }, }, - }, - }), - ], + }), + ], + }, module: { rules: [ { @@ -59,10 +58,6 @@ module.exports = { ], exclude: /node_modules/, }, - { - test: /\.json$/, - loader: 'json-loader', - }, ], }, }; diff --git a/packages/contract-wrappers/package.json b/packages/contract-wrappers/package.json index 22dd6521c..c345d0a6a 100644 --- a/packages/contract-wrappers/package.json +++ b/packages/contract-wrappers/package.json @@ -53,7 +53,6 @@ "@types/sinon": "^2.2.2", "@types/uuid": "^3.4.2", "@types/web3-provider-engine": "^14.0.0", - "awesome-typescript-loader": "^3.1.3", "chai": "^4.0.1", "chai-as-promised": "^7.1.0", "chai-bignumber": "^2.0.1", diff --git a/packages/order-watcher/package.json b/packages/order-watcher/package.json index 9f57dd6fd..de5f7ba09 100644 --- a/packages/order-watcher/package.json +++ b/packages/order-watcher/package.json @@ -52,13 +52,11 @@ "@types/mocha": "^2.2.42", "@types/node": "*", "@types/sinon": "^2.2.2", - "awesome-typescript-loader": "^3.1.3", "chai": "^4.0.1", "chai-as-promised": "^7.1.0", "chai-bignumber": "^2.0.1", "copyfiles": "^2.0.0", "dirty-chai": "^2.0.1", - "json-loader": "^0.5.4", "make-promises-safe": "^1.1.0", "mocha": "^4.1.0", "npm-run-all": "^4.1.2", diff --git a/packages/subproviders/package.json b/packages/subproviders/package.json index f9063228f..491f079d2 100644 --- a/packages/subproviders/package.json +++ b/packages/subproviders/package.json @@ -76,7 +76,7 @@ "tslint": "5.11.0", "typedoc": "0.12.0", "typescript": "3.0.1", - "webpack": "^3.1.0" + "webpack": "^4.20.2" }, "optionalDependencies": { "@ledgerhq/hw-transport-node-hid": "^4.3.0" diff --git a/packages/testnet-faucets/package.json b/packages/testnet-faucets/package.json index 33f557538..8213f896f 100644 --- a/packages/testnet-faucets/package.json +++ b/packages/testnet-faucets/package.json @@ -36,15 +36,15 @@ "@types/body-parser": "^1.16.1", "@types/express": "^4.0.35", "@types/lodash": "4.14.104", - "awesome-typescript-loader": "^3.1.3", + "awesome-typescript-loader": "^5.2.1", "gulp": "^3.9.1", "make-promises-safe": "^1.1.0", "nodemon": "^1.11.0", "shx": "^0.2.2", - "source-map-loader": "^0.1.6", + "source-map-loader": "^0.2.4", "tslint": "5.11.0", "typescript": "3.0.1", - "webpack": "^3.1.0", + "webpack": "^4.20.2", "webpack-node-externals": "^1.6.0" } } diff --git a/packages/website/package.json b/packages/website/package.json index 8c115d8a0..530f71677 100644 --- a/packages/website/package.json +++ b/packages/website/package.json @@ -7,11 +7,12 @@ "private": true, "description": "Website and 0x portal dapp", "scripts": { - "build": "NODE_ENV=production node --max_old_space_size=8192 ../../node_modules/.bin/webpack", + "build": "node --max_old_space_size=8192 ../../node_modules/.bin/webpack --mode production", + "build:dev": "../../node_modules/.bin/webpack --mode development", "build:ci": "yarn build", "clean": "shx rm -f public/bundle*", "lint": "tslint --project . 'ts/**/*.ts' 'ts/**/*.tsx'", - "watch_without_deps": "webpack-dev-server --content-base public --https", + "dev": "webpack-dev-server --mode development --content-base public --https", "deploy_dogfood": "npm run build; aws s3 sync ./public/. s3://dogfood.0xproject.com --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers", "deploy_staging": "npm run build; aws s3 sync ./public/. s3://staging-0xproject --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers", "deploy_live": "DEPLOY_ROLLBAR_SOURCEMAPS=true npm run build; aws s3 sync ./public/. s3://0xproject.com --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --exclude *.map.js" @@ -87,26 +88,23 @@ "@types/react-tap-event-plugin": "0.0.30", "@types/redux": "^3.6.0", "@types/web3-provider-engine": "^14.0.0", - "awesome-typescript-loader": "^3.1.3", - "copy-webpack-plugin": "^4.0.1", + "awesome-typescript-loader": "^5.2.1", "copyfiles": "^2.0.0", "css-loader": "0.23.x", - "exports-loader": "0.6.x", - "imports-loader": "0.6.x", - "json-loader": "^0.5.4", - "less-loader": "^2.2.3", + "less-loader": "^4.1.0", "make-promises-safe": "^1.1.0", "raw-loader": "^0.5.1", - "rollbar-sourcemap-webpack-plugin": "^2.3.0", + "rollbar-sourcemap-webpack-plugin": "^2.4.0", "shx": "^0.2.2", - "source-map-loader": "^0.1.6", - "style-loader": "0.13.x", + "source-map-loader": "^0.2.4", + "style-loader": "0.23.x", + "terser-webpack-plugin": "^1.1.0", "tslint": "5.11.0", "tslint-config-0xproject": "^0.0.2", "typescript": "3.0.1", - "uglifyjs-webpack-plugin": "^1.2.5", - "webpack": "^3.1.0", - "webpack-dev-middleware": "^1.10.0", - "webpack-dev-server": "^2.5.0" + "uglifyjs-webpack-plugin": "^2.0.1", + "webpack": "^4.20.2", + "webpack-cli": "3.1.2", + "webpack-dev-server": "^3.1.9" } } diff --git a/packages/website/ts/index.tsx b/packages/website/ts/index.tsx index 9e59b00ac..d4a79cc4f 100644 --- a/packages/website/ts/index.tsx +++ b/packages/website/ts/index.tsx @@ -26,47 +26,47 @@ import 'less/all.less'; // We pass modulePromise returning lambda instead of module promise, // cause we only want to import the module when the user navigates to the page. -// At the same time webpack statically parses for System.import() to determine bundle chunk split points -// so each lazy import needs it's own `System.import()` declaration. +// At the same time webpack statically parses for import() to determine bundle chunk split points +// so each lazy import needs it's own `import()` declaration. const LazyPortal = createLazyComponent('Portal', async () => - System.import(/* webpackChunkName: "portal" */ 'ts/containers/portal'), + import(/* webpackChunkName: "portal" */ 'ts/containers/portal'), ); const LazyZeroExJSDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "zeroExDocs" */ 'ts/containers/zero_ex_js_documentation'), + import(/* webpackChunkName: "zeroExDocs" */ 'ts/containers/zero_ex_js_documentation'), ); const LazyContractWrappersDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "contractWrapperDocs" */ 'ts/containers/contract_wrappers_documentation'), + import(/* webpackChunkName: "contractWrapperDocs" */ 'ts/containers/contract_wrappers_documentation'), ); const LazyOrderWatcherDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "orderWatcherDocs" */ 'ts/containers/order_watcher_documentation'), + import(/* webpackChunkName: "orderWatcherDocs" */ 'ts/containers/order_watcher_documentation'), ); const LazySmartContractsDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "smartContractDocs" */ 'ts/containers/smart_contracts_documentation'), + import(/* webpackChunkName: "smartContractDocs" */ 'ts/containers/smart_contracts_documentation'), ); const LazyConnectDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "connectDocs" */ 'ts/containers/connect_documentation'), + import(/* webpackChunkName: "connectDocs" */ 'ts/containers/connect_documentation'), ); const LazyWeb3WrapperDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "web3WrapperDocs" */ 'ts/containers/web3_wrapper_documentation'), + import(/* webpackChunkName: "web3WrapperDocs" */ 'ts/containers/web3_wrapper_documentation'), ); const LazySolCompilerDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "solCompilerDocs" */ 'ts/containers/sol_compiler_documentation'), + import(/* webpackChunkName: "solCompilerDocs" */ 'ts/containers/sol_compiler_documentation'), ); const LazyJSONSchemasDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "jsonSchemasDocs" */ 'ts/containers/json_schemas_documentation'), + import(/* webpackChunkName: "jsonSchemasDocs" */ 'ts/containers/json_schemas_documentation'), ); const LazySolCovDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "solCovDocs" */ 'ts/containers/sol_cov_documentation'), + import(/* webpackChunkName: "solCovDocs" */ 'ts/containers/sol_cov_documentation'), ); const LazySubprovidersDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "subproviderDocs" */ 'ts/containers/subproviders_documentation'), + import(/* webpackChunkName: "subproviderDocs" */ 'ts/containers/subproviders_documentation'), ); const LazyOrderUtilsDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "orderUtilsDocs" */ 'ts/containers/order_utils_documentation'), + import(/* webpackChunkName: "orderUtilsDocs" */ 'ts/containers/order_utils_documentation'), ); const LazyEthereumTypesDocumentation = createLazyComponent('Documentation', async () => - System.import(/* webpackChunkName: "ethereumTypesDocs" */ 'ts/containers/ethereum_types_documentation'), + import(/* webpackChunkName: "ethereumTypesDocs" */ 'ts/containers/ethereum_types_documentation'), ); const DOCUMENT_TITLE = '0x: The Protocol for Trading Tokens'; diff --git a/packages/website/ts/lazy_component.tsx b/packages/website/ts/lazy_component.tsx index dce06ed8d..9d3b9944a 100644 --- a/packages/website/ts/lazy_component.tsx +++ b/packages/website/ts/lazy_component.tsx @@ -49,7 +49,7 @@ export class LazyComponent extends React.Component System.import('ts/containers/portal'));`` + * @example `const LazyPortal = createLazyComponent('Portal', () => import('ts/containers/portal'));`` */ export const createLazyComponent = (componentName: string, lazyImport: () => Promise) => { return (props: any) => { diff --git a/packages/website/webpack.config.js b/packages/website/webpack.config.js index 8653196a6..1b213b339 100644 --- a/packages/website/webpack.config.js +++ b/packages/website/webpack.config.js @@ -1,6 +1,6 @@ const path = require('path'); const webpack = require('webpack'); -const UglifyJsPlugin = require('uglifyjs-webpack-plugin'); +const TerserPlugin = require('terser-webpack-plugin'); const RollbarSourceMapPlugin = require('rollbar-sourcemap-webpack-plugin'); const childProcess = require('child_process'); @@ -9,44 +9,7 @@ const GIT_SHA = childProcess .toString() .trim(); -const generatePlugins = () => { - let plugins = []; - if (process.env.NODE_ENV === 'production') { - plugins = plugins.concat([ - // Since we do not use moment's locale feature, we exclude them from the bundle. - // This reduces the bundle size by 0.4MB. - new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/), - new webpack.DefinePlugin({ - 'process.env': { - NODE_ENV: JSON.stringify(process.env.NODE_ENV), - GIT_SHA: JSON.stringify(GIT_SHA), - }, - }), - // TODO: Revert to webpack bundled version with webpack v4. - // The v3 series bundled version does not support ES6 and - // fails to build. - new UglifyJsPlugin({ - sourceMap: true, - uglifyOptions: { - mangle: { - reserved: ['BigNumber'], - }, - }, - }), - ]); - if (process.env.DEPLOY_ROLLBAR_SOURCEMAPS === 'true') { - plugins = plugins.concat([ - new RollbarSourceMapPlugin({ - accessToken: '32c39bfa4bb6440faedc1612a9c13d28', - version: GIT_SHA, - publicPath: 'https://0xproject.com/', - }), - ]); - } - } - return plugins; -}; -module.exports = { +const config = { entry: ['./ts/index.tsx'], output: { path: path.join(__dirname, '/public'), @@ -54,7 +17,6 @@ module.exports = { chunkFilename: 'bundle-[name].js', publicPath: '/', }, - devtool: 'source-map', resolve: { modules: [path.join(__dirname, '/ts'), 'node_modules'], extensions: ['.ts', '.tsx', '.js', '.jsx', '.json', '.md'], @@ -92,10 +54,18 @@ module.exports = { test: /\.css$/, loaders: ['style-loader', 'css-loader'], }, - { - test: /\.json$/, - loader: 'json-loader', - }, + ], + }, + optimization: { + minimizer: [ + new TerserPlugin({ + sourceMap: true, + terserOptions: { + mangle: { + reserved: ['BigNumber'], + }, + }, + }), ], }, devServer: { @@ -115,5 +85,40 @@ module.exports = { }, disableHostCheck: true, }, - plugins: generatePlugins(), +}; + +module.exports = (_env, argv) => { + let plugins = []; + if (argv.mode === 'development') { + config.devtool = 'source-map'; + config.mode = 'development'; + } else { + config.mode = 'production'; + plugins = plugins.concat([ + // Since we do not use moment's locale feature, we exclude them from the bundle. + // This reduces the bundle size by 0.4MB. + new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/), + new webpack.DefinePlugin({ + 'process.env': { + NODE_ENV: JSON.stringify(process.env.NODE_ENV), + GIT_SHA: JSON.stringify(GIT_SHA), + }, + }), + ]); + if (process.env.DEPLOY_ROLLBAR_SOURCEMAPS === 'true') { + plugins = plugins.concat([ + new RollbarSourceMapPlugin({ + accessToken: '32c39bfa4bb6440faedc1612a9c13d28', + version: GIT_SHA, + publicPath: 'https://0xproject.com/', + }), + ]); + } + } + console.log('i ｢atl｣: Mode: ', config.mode); + + config.plugins = plugins; + console.log('i ｢atl｣: Plugin Count: ', config.plugins.length); + + return config; }; -- cgit v1.2.3 From d5379ab3202030a74fb6e0947b2cf3c99e162df3 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 4 Oct 2018 15:58:50 +0100 Subject: Add back sourceMap support for both dev/prod --- packages/website/webpack.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/website/webpack.config.js b/packages/website/webpack.config.js index 1b213b339..ec265be93 100644 --- a/packages/website/webpack.config.js +++ b/packages/website/webpack.config.js @@ -17,6 +17,7 @@ const config = { chunkFilename: 'bundle-[name].js', publicPath: '/', }, + devtool: 'source-map', resolve: { modules: [path.join(__dirname, '/ts'), 'node_modules'], extensions: ['.ts', '.tsx', '.js', '.jsx', '.json', '.md'], @@ -90,7 +91,6 @@ const config = { module.exports = (_env, argv) => { let plugins = []; if (argv.mode === 'development') { - config.devtool = 'source-map'; config.mode = 'development'; } else { config.mode = 'production'; -- cgit v1.2.3 From 08ba4ffd61e17155b4fcbf0a9d7385ba2af0c725 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 4 Oct 2018 16:35:30 +0100 Subject: Build website in parallel with other tests since no other test relies on it being built to run --- packages/website/package.json | 1 - 1 file changed, 1 deletion(-) (limited to 'packages') diff --git a/packages/website/package.json b/packages/website/package.json index 530f71677..3cf243645 100644 --- a/packages/website/package.json +++ b/packages/website/package.json @@ -9,7 +9,6 @@ "scripts": { "build": "node --max_old_space_size=8192 ../../node_modules/.bin/webpack --mode production", "build:dev": "../../node_modules/.bin/webpack --mode development", - "build:ci": "yarn build", "clean": "shx rm -f public/bundle*", "lint": "tslint --project . 'ts/**/*.ts' 'ts/**/*.tsx'", "dev": "webpack-dev-server --mode development --content-base public --https", -- cgit v1.2.3 From 98db4b154343127bdecbbb1e72f8ef1bb49b494e Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 4 Oct 2018 16:46:12 +0100 Subject: force re-build --- packages/website/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/website/README.md b/packages/website/README.md index 7115a3b5c..2edc17cd9 100644 --- a/packages/website/README.md +++ b/packages/website/README.md @@ -14,7 +14,7 @@ Please read our [contribution guidelines](../../CONTRIBUTING.md) before getting ## Local Dev Setup -Requires Node version 6.9.5 or higher. +Requires Node version 6.9.5 or higher Add the following to your `/etc/hosts` file: -- cgit v1.2.3 From 63d97f4c8837dcf8d17afc494dcd8b1ba10fee16 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 4 Oct 2018 14:33:07 -0700 Subject: Update BuyQuote interface --- packages/asset-buyer/src/asset_buyer.ts | 15 ++++------ packages/asset-buyer/src/types.ts | 27 ++++++++++++----- packages/asset-buyer/src/utils/assert.ts | 11 +++++-- .../asset-buyer/src/utils/buy_quote_calculator.ts | 26 ++++++++++------- .../asset-buyer/test/buy_quote_calculator_test.ts | 34 +++++++++++++++------- 5 files changed, 73 insertions(+), 40 deletions(-) (limited to 'packages') diff --git a/packages/asset-buyer/src/asset_buyer.ts b/packages/asset-buyer/src/asset_buyer.ts index a56bc01d5..7ec39e012 100644 --- a/packages/asset-buyer/src/asset_buyer.ts +++ b/packages/asset-buyer/src/asset_buyer.ts @@ -183,19 +183,19 @@ export class AssetBuyer { buyQuote: BuyQuote, options: Partial = {}, ): Promise { - const { rate, takerAddress, feeRecipient } = { + const { ethAmount, takerAddress, feeRecipient } = { ...constants.DEFAULT_BUY_QUOTE_EXECUTION_OPTS, ...options, }; assert.isValidBuyQuote('buyQuote', buyQuote); - if (!_.isUndefined(rate)) { - assert.isBigNumber('rate', rate); + if (!_.isUndefined(ethAmount)) { + assert.isBigNumber('ethAmount', ethAmount); } if (!_.isUndefined(takerAddress)) { assert.isETHAddressHex('takerAddress', takerAddress); } assert.isETHAddressHex('feeRecipient', feeRecipient); - const { orders, feeOrders, feePercentage, assetBuyAmount, maxRate } = buyQuote; + const { orders, feeOrders, feePercentage, assetBuyAmount, worstCaseQuoteInfo } = buyQuote; // if no takerAddress is provided, try to get one from the provider let finalTakerAddress; if (!_.isUndefined(takerAddress)) { @@ -210,15 +210,12 @@ export class AssetBuyer { throw new Error(AssetBuyerError.NoAddressAvailable); } } - // if no rate is provided, default to the maxRate from buyQuote - const desiredRate = rate || maxRate; - // calculate how much eth is required to buy assetBuyAmount at the desired rate - const ethAmount = assetBuyAmount.dividedToIntegerBy(desiredRate); + // if no ethAmount is provided, default to the worst ethAmount from buyQuote const txHash = await this._contractWrappers.forwarder.marketBuyOrdersWithEthAsync( orders, assetBuyAmount, finalTakerAddress, - ethAmount, + ethAmount || worstCaseQuoteInfo.totalEthAmount, feeOrders, feePercentage, feeRecipient, diff --git a/packages/asset-buyer/src/types.ts b/packages/asset-buyer/src/types.ts index 8d3dcbfe6..b96795bb6 100644 --- a/packages/asset-buyer/src/types.ts +++ b/packages/asset-buyer/src/types.ts @@ -35,21 +35,32 @@ export interface OrderProvider { /** * assetData: String that represents a specific asset (for more info: https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md). + * assetBuyAmount: The amount of asset to buy. * orders: An array of objects conforming to SignedOrder. These orders can be used to cover the requested assetBuyAmount plus slippage. * feeOrders: An array of objects conforming to SignedOrder. These orders can be used to cover the fees for the orders param above. - * minRate: Min rate that needs to be paid in order to execute the buy. - * maxRate: Max rate that can be paid in order to execute the buy. - * assetBuyAmount: The amount of asset to buy. * feePercentage: Optional affiliate fee percentage used to calculate the eth amounts above. + * bestCaseQuoteInfo: Info about the best case price for the asset. + * worstCaseQuoteInfo: Info about the worst case price for the asset. */ export interface BuyQuote { assetData: string; + assetBuyAmount: BigNumber; orders: SignedOrder[]; feeOrders: SignedOrder[]; - minRate: BigNumber; - maxRate: BigNumber; - assetBuyAmount: BigNumber; feePercentage?: number; + bestCaseQuoteInfo: BuyQuoteInfo; + worstCaseQuoteInfo: BuyQuoteInfo; +} + +/** + * ethPerAssetPrice: The price of one unit of the desired asset in ETH + * 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) + */ +export interface BuyQuoteInfo { + ethPerAssetPrice: BigNumber; + feeEthAmount: BigNumber; + totalEthAmount: BigNumber; } /** @@ -64,12 +75,12 @@ export interface BuyQuoteRequestOpts { } /** - * rate: The desired rate to execute the buy at. Affects the amount of ETH sent with the transaction, defaults to buyQuote.maxRate. + * ethAmount: The desired amount of eth to spend. Defaults to buyQuote.worstCaseQuoteInfo.totalEthAmount. * takerAddress: The address to perform the buy. Defaults to the first available address from the provider. * feeRecipient: The address where affiliate fees are sent. Defaults to null address (0x000...000). */ export interface BuyQuoteExecutionOpts { - rate?: BigNumber; + ethAmount?: BigNumber; takerAddress?: string; feeRecipient: string; } diff --git a/packages/asset-buyer/src/utils/assert.ts b/packages/asset-buyer/src/utils/assert.ts index 04f425237..d43b71fee 100644 --- a/packages/asset-buyer/src/utils/assert.ts +++ b/packages/asset-buyer/src/utils/assert.ts @@ -3,7 +3,7 @@ import { schemas } from '@0xproject/json-schemas'; import { SignedOrder } from '@0xproject/types'; import * as _ from 'lodash'; -import { BuyQuote, OrderProvider, OrderProviderRequest } from '../types'; +import { BuyQuote, BuyQuoteInfo, OrderProvider, OrderProviderRequest } from '../types'; export const assert = { ...sharedAssert, @@ -11,13 +11,18 @@ export const assert = { sharedAssert.isHexString(`${variableName}.assetData`, buyQuote.assetData); sharedAssert.doesConformToSchema(`${variableName}.orders`, buyQuote.orders, schemas.signedOrdersSchema); sharedAssert.doesConformToSchema(`${variableName}.feeOrders`, buyQuote.feeOrders, schemas.signedOrdersSchema); - sharedAssert.isBigNumber(`${variableName}.minRate`, buyQuote.minRate); - sharedAssert.isBigNumber(`${variableName}.maxRate`, buyQuote.maxRate); + assert.isValidBuyQuoteInfo(`${variableName}.bestCaseQuoteInfo`, buyQuote.bestCaseQuoteInfo); + assert.isValidBuyQuoteInfo(`${variableName}.worstCaseQuoteInfo`, buyQuote.worstCaseQuoteInfo); sharedAssert.isBigNumber(`${variableName}.assetBuyAmount`, buyQuote.assetBuyAmount); if (!_.isUndefined(buyQuote.feePercentage)) { sharedAssert.isNumber(`${variableName}.feePercentage`, buyQuote.feePercentage); } }, + isValidBuyQuoteInfo(variableName: string, buyQuoteInfo: BuyQuoteInfo): void { + sharedAssert.isBigNumber(`${variableName}.ethPerAssetPrice`, buyQuoteInfo.ethPerAssetPrice); + sharedAssert.isBigNumber(`${variableName}.feeEthAmount`, buyQuoteInfo.feeEthAmount); + sharedAssert.isBigNumber(`${variableName}.totalEthAmount`, buyQuoteInfo.totalEthAmount); + }, isValidOrderProvider(variableName: string, orderFetcher: OrderProvider): void { sharedAssert.isFunction(`${variableName}.getOrdersAsync`, orderFetcher.getOrdersAsync); }, diff --git a/packages/asset-buyer/src/utils/buy_quote_calculator.ts b/packages/asset-buyer/src/utils/buy_quote_calculator.ts index 78666356c..cb0fd128c 100644 --- a/packages/asset-buyer/src/utils/buy_quote_calculator.ts +++ b/packages/asset-buyer/src/utils/buy_quote_calculator.ts @@ -3,7 +3,7 @@ import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; import { constants } from '../constants'; -import { AssetBuyerError, BuyQuote, OrdersAndFillableAmounts } from '../types'; +import { AssetBuyerError, BuyQuote, BuyQuoteInfo, OrdersAndFillableAmounts } from '../types'; // Calculates a buy quote for orders that have WETH as the takerAsset export const buyQuoteCalculator = { @@ -59,37 +59,38 @@ export const buyQuoteCalculator = { orders: resultFeeOrders, remainingFillableMakerAssetAmounts: feeOrdersRemainingFillableMakerAssetAmounts, }; - const minRate = calculateRate( + const bestCaseQuoteInfo = calculateQuoteInfo( trimmedOrdersAndFillableAmounts, trimmedFeeOrdersAndFillableAmounts, assetBuyAmount, feePercentage, ); // in order to calculate the maxRate, reverse the ordersAndFillableAmounts such that they are sorted from worst rate to best rate - const maxRate = calculateRate( + const worstCaseQuoteInfo = calculateQuoteInfo( reverseOrdersAndFillableAmounts(trimmedOrdersAndFillableAmounts), reverseOrdersAndFillableAmounts(trimmedFeeOrdersAndFillableAmounts), assetBuyAmount, feePercentage, ); + return { assetData, orders: resultOrders, feeOrders: resultFeeOrders, - minRate, - maxRate, + bestCaseQuoteInfo, + worstCaseQuoteInfo, assetBuyAmount, feePercentage, }; }, }; -function calculateRate( +function calculateQuoteInfo( ordersAndFillableAmounts: OrdersAndFillableAmounts, feeOrdersAndFillableAmounts: OrdersAndFillableAmounts, assetBuyAmount: BigNumber, feePercentage: number, -): BigNumber { +): BuyQuoteInfo { // find the total eth and zrx needed to buy assetAmount from the resultOrders from left to right const [ethAmountToBuyAsset, zrxAmountToBuyAsset] = findEthAndZrxAmountNeededToBuyAsset( ordersAndFillableAmounts, @@ -97,10 +98,15 @@ function calculateRate( ); // find the total eth needed to buy fees const ethAmountToBuyFees = findEthAmountNeededToBuyFees(feeOrdersAndFillableAmounts, zrxAmountToBuyAsset); - const ethAmount = ethAmountToBuyAsset.plus(ethAmountToBuyFees).mul(feePercentage + 1); + const ethAmountBeforeAffiliateFee = ethAmountToBuyAsset.plus(ethAmountToBuyFees); + const totalEthAmount = ethAmountBeforeAffiliateFee.mul(feePercentage + 1); // divide into the assetBuyAmount in order to find rate of makerAsset / WETH - const result = assetBuyAmount.div(ethAmount); - return result; + const ethPerAssetPrice = ethAmountBeforeAffiliateFee.div(assetBuyAmount); + return { + totalEthAmount, + feeEthAmount: totalEthAmount.minus(ethAmountBeforeAffiliateFee), + ethPerAssetPrice, + }; } // given an OrdersAndFillableAmounts, reverse the orders and remainingFillableMakerAssetAmounts properties diff --git a/packages/asset-buyer/test/buy_quote_calculator_test.ts b/packages/asset-buyer/test/buy_quote_calculator_test.ts index 667dec051..b987b45a8 100644 --- a/packages/asset-buyer/test/buy_quote_calculator_test.ts +++ b/packages/asset-buyer/test/buy_quote_calculator_test.ts @@ -103,11 +103,17 @@ describe('buyQuoteCalculator', () => { expect(buyQuote.feeOrders).to.deep.equal([smallFeeOrderAndFillableAmount.orders[0]]); // test if rates are correct // 50 eth to fill the first order + 100 eth for fees - const expectedMinEthToFill = new BigNumber(150); - const expectedMinRate = assetBuyAmount.div(expectedMinEthToFill.mul(feePercentage + 1)); - expect(buyQuote.minRate).to.bignumber.equal(expectedMinRate); + const expectedFillEthAmount = new BigNumber(150); + const expectedTotalEthAmount = expectedFillEthAmount.mul(feePercentage + 1); + const expectedFeeEthAmount = expectedTotalEthAmount.minus(expectedFillEthAmount); + const expectedEthPerAssetPrice = expectedFillEthAmount.div(assetBuyAmount); + 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.maxRate).to.bignumber.equal(expectedMinRate); + 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); }); @@ -132,13 +138,21 @@ describe('buyQuoteCalculator', () => { expect(buyQuote.feeOrders).to.deep.equal(allFeeOrdersAndFillableAmounts.orders); // test if rates are correct // 50 eth to fill the first order + 100 eth for fees - const expectedMinEthToFill = new BigNumber(150); - const expectedMinRate = assetBuyAmount.div(expectedMinEthToFill.mul(feePercentage + 1)); - expect(buyQuote.minRate).to.bignumber.equal(expectedMinRate); + const expectedFillEthAmount = new BigNumber(150); + const expectedTotalEthAmount = expectedFillEthAmount.mul(feePercentage + 1); + const expectedFeeEthAmount = expectedTotalEthAmount.minus(expectedFillEthAmount); + const expectedEthPerAssetPrice = expectedFillEthAmount.div(assetBuyAmount); + 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 + 200 eth for fees - const expectedMaxEthToFill = new BigNumber(300); - const expectedMaxRate = assetBuyAmount.div(expectedMaxEthToFill.mul(feePercentage + 1)); - expect(buyQuote.maxRate).to.bignumber.equal(expectedMaxRate); + const expectedWorstFillEthAmount = new BigNumber(300); + const expectedWorstTotalEthAmount = expectedWorstFillEthAmount.mul(feePercentage + 1); + const expectedWorstFeeEthAmount = expectedWorstTotalEthAmount.minus(expectedWorstFillEthAmount); + const expectedWorstEthPerAssetPrice = expectedWorstFillEthAmount.div(assetBuyAmount); + 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); }); -- cgit v1.2.3