Merge pull request #2157 from 0xProject/feature/orderbook-removing-smart-routing-logic

Refactored asset-swapper smart logic and renamed options

packages/asset-swapper/src/index.ts

@@ -33,10 +33,11 @@ export {
     SwapQuote,
     SwapQuoteConsumerOpts,
     CalldataInfo,
-    ConsumerType,
+    ExtensionContractType,
     SwapQuoteGetOutputOpts,
     SwapQuoteExecutionOpts,
     SwapQuoteInfo,
+    GetExtensionContractTypeOpts,
     SwapQuoteExecutionOptsBase,
     SwapQuoteGetOutputOptsBase,
     ForwarderSwapQuoteExecutionOpts,

packages/asset-swapper/src/quote_consumers/swap_quote_consumer.ts

@@ -6,7 +6,8 @@ import * as _ from 'lodash';
 import { constants } from '../constants';
 import {
     CalldataInfo,
-    ConsumerType,
+    ExtensionContractType,
+    GetExtensionContractTypeOpts,
     SmartContractParams,
     SmartContractParamsInfo,
     SwapQuote,
@@ -25,9 +26,9 @@ export class SwapQuoteConsumer implements SwapQuoteConsumerBase<SmartContractPar
     public readonly provider: ZeroExProvider;
     public readonly networkId: number;
 
-    private readonly _contractWrappers: ContractWrappers;
     private readonly _exchangeConsumer: ExchangeSwapQuoteConsumer;
     private readonly _forwarderConsumer: ForwarderSwapQuoteConsumer;
+    private readonly _contractWrappers: ContractWrappers;
 
     constructor(supportedProvider: SupportedProvider, options: Partial<SwapQuoteConsumerOpts> = {}) {
         const { networkId } = _.merge({}, constants.DEFAULT_SWAP_QUOTER_OPTS, options);
@@ -36,12 +37,12 @@ export class SwapQuoteConsumer implements SwapQuoteConsumerBase<SmartContractPar
         const provider = providerUtils.standardizeOrThrow(supportedProvider);
         this.provider = provider;
         this.networkId = networkId;
-        this._contractWrappers = new ContractWrappers(this.provider, {
-            networkId,
-        });
 
         this._exchangeConsumer = new ExchangeSwapQuoteConsumer(supportedProvider, options);
         this._forwarderConsumer = new ForwarderSwapQuoteConsumer(supportedProvider, options);
+        this._contractWrappers = new ContractWrappers(this.provider, {
+            networkId,
+        });
     }
 
     /**
@@ -54,7 +55,7 @@ export class SwapQuoteConsumer implements SwapQuoteConsumerBase<SmartContractPar
         opts: Partial<SwapQuoteGetOutputOpts> = {},
     ): Promise<CalldataInfo> {
         assert.isValidSwapQuote('quote', quote);
-        const consumer = await this._getConsumerForSwapQuoteAsync(quote, opts);
+        const consumer = await this._getConsumerForSwapQuoteAsync(opts);
         return consumer.getCalldataOrThrowAsync(quote, opts);
     }
 
@@ -68,7 +69,7 @@ export class SwapQuoteConsumer implements SwapQuoteConsumerBase<SmartContractPar
         opts: Partial<SwapQuoteGetOutputOpts> = {},
     ): Promise<SmartContractParamsInfo<SmartContractParams>> {
         assert.isValidSwapQuote('quote', quote);
-        const consumer = await this._getConsumerForSwapQuoteAsync(quote, opts);
+        const consumer = await this._getConsumerForSwapQuoteAsync(opts);
         return consumer.getSmartContractParamsOrThrowAsync(quote, opts);
     }
 
@@ -82,25 +83,26 @@ export class SwapQuoteConsumer implements SwapQuoteConsumerBase<SmartContractPar
         opts: Partial<SwapQuoteExecutionOpts> = {},
     ): Promise<string> {
         assert.isValidSwapQuote('quote', quote);
-        const consumer = await this._getConsumerForSwapQuoteAsync(quote, opts);
+        const consumer = await this._getConsumerForSwapQuoteAsync(opts);
         return consumer.executeSwapQuoteOrThrowAsync(quote, opts);
     }
 
-    private async _getConsumerForSwapQuoteAsync(
+    public async getOptimalExtensionContractTypeAsync(
         quote: SwapQuote,
-        opts: Partial<SwapQuoteGetOutputOpts>,
+        opts: Partial<GetExtensionContractTypeOpts> = {},
-    ): Promise<SwapQuoteConsumerBase<SmartContractParams>> {
+    ): Promise<ExtensionContractType> {
-        const useConsumerType =
+        return swapQuoteConsumerUtils.getExtensionContractTypeForSwapQuoteAsync(
-            opts.useConsumerType ||
-            (await swapQuoteConsumerUtils.getConsumerTypeForSwapQuoteAsync(
             quote,
             this._contractWrappers,
             this.provider,
             opts,
-            ));
+        );
-        if (useConsumerType === ConsumerType.Exchange) {
+    }
-            return this._exchangeConsumer;
+
-        } else if (useConsumerType === ConsumerType.Forwarder) {
+    private async _getConsumerForSwapQuoteAsync(
+        opts: Partial<SwapQuoteGetOutputOpts>,
+    ): Promise<SwapQuoteConsumerBase<SmartContractParams>> {
+        if (opts.useExtensionContract === ExtensionContractType.Forwarder) {
             return this._forwarderConsumer;
         }
         return this._exchangeConsumer;

packages/asset-swapper/src/types.ts

@@ -86,9 +86,9 @@ export interface ExchangeMarketSellSmartContractParams extends SmartContractPara
 /**
  * Represents the varying smart contracts that can consume a valid swap quote
  */
-export enum ConsumerType {
+export enum ExtensionContractType {
     Forwarder = 'FORWARDER',
-    Exchange = 'EXCHANGE',
+    None = 'NONE',
 }
 
 /**
@@ -183,13 +183,17 @@ export interface ForwarderSwapQuoteGetOutputOpts extends SwapQuoteGetOutputOptsB
 
 export type SwapQuote = MarketBuySwapQuote | MarketSellSwapQuote;
 
+export interface GetExtensionContractTypeOpts {
+    takerAddress?: string;
+    ethAmount?: BigNumber;
+}
+
 /**
  * takerAddress: The address to perform the buy. Defaults to the first available address from the provider.
  * useConsumerType: If provided, defaults the SwapQuoteConsumer to create output consumed by ConsumerType.
  */
 export interface SwapQuoteGetOutputOpts extends ForwarderSwapQuoteGetOutputOpts {
-    takerAddress?: string;
+    useExtensionContract: ExtensionContractType;
-    useConsumerType?: ConsumerType;
 }
 
 export interface ForwarderSwapQuoteExecutionOpts extends ForwarderSwapQuoteGetOutputOpts, SwapQuoteExecutionOptsBase {}

packages/asset-swapper/src/utils/swap_quote_consumer_utils.ts

@@ -8,11 +8,11 @@ import * as _ from 'lodash';
 
 import { constants } from '../constants';
 import {
-    ConsumerType,
+    ExtensionContractType,
+    GetExtensionContractTypeOpts,
     SwapQuote,
     SwapQuoteConsumerError,
     SwapQuoteExecutionOpts,
-    SwapQuoteGetOutputOpts,
 } from '../types';
 
 import { assert } from './assert';
@@ -79,12 +79,12 @@ export const swapQuoteConsumerUtils = {
             return optimizedOrder;
         });
     },
-    async getConsumerTypeForSwapQuoteAsync(
+    async getExtensionContractTypeForSwapQuoteAsync(
         quote: SwapQuote,
         contractWrappers: ContractWrappers,
         provider: Provider,
-        opts: Partial<SwapQuoteGetOutputOpts>,
+        opts: Partial<GetExtensionContractTypeOpts>,
-    ): Promise<ConsumerType> {
+    ): Promise<ExtensionContractType> {
         const wethAssetData = assetDataUtils.encodeERC20AssetData(contractWrappers.contractAddresses.etherToken);
         if (swapQuoteConsumerUtils.isValidForwarderSwapQuote(quote, wethAssetData)) {
             if (opts.takerAddress !== undefined) {
@@ -102,14 +102,14 @@ export const swapQuoteConsumerUtils = {
             );
             if (isEnoughEthAndWethBalance[1]) {
                 // should be more gas efficient to use exchange consumer, so if possible use it.
-                return ConsumerType.Exchange;
+                return ExtensionContractType.None;
             } else if (isEnoughEthAndWethBalance[0] && !isEnoughEthAndWethBalance[1]) {
-                return ConsumerType.Forwarder;
+                return ExtensionContractType.Forwarder;
             }
             // Note: defaulting to forwarderConsumer if takerAddress is null or not enough balance of either wEth or Eth
-            return ConsumerType.Forwarder;
+            return ExtensionContractType.Forwarder;
         } else {
-            return ConsumerType.Exchange;
+            return ExtensionContractType.None;
         }
     },
 };

packages/asset-swapper/test/swap_quote_consumer_test.ts

@@ -7,7 +7,7 @@ import * as chai from 'chai';
 import 'mocha';
 
 import { SwapQuote, SwapQuoteConsumer } from '../src';
-import { ConsumerType } from '../src/types';
+import { ExtensionContractType } from '../src/types';
 
 import { chaiSetup } from './utils/chai_setup';
 import { migrateOnceAsync } from './utils/migrate';
@@ -116,18 +116,18 @@ describe('SwapQuoteConsumer', () => {
     //      * Testing that SwapQuoteConsumer logic correctly performs a execution (doesn't throw or revert)
     //      * Does not test the validity of the state change performed by the forwarder smart contract
     //      */
-    //     it('should perform an asset swap with Forwarder contract when provided corresponding useConsumerType option', async () => {
+    //     it('should perform an asset swap with Forwarder contract when provided corresponding useExtensionContract option', async () => {
     //         let makerBalance = await erc20TokenContract.balanceOf.callAsync(makerAddress);
     //         let takerBalance = await erc20TokenContract.balanceOf.callAsync(takerAddress);
     //         expect(makerBalance).to.bignumber.equal(new BigNumber(10).multipliedBy(ONE_ETH_IN_WEI));
     //         expect(takerBalance).to.bignumber.equal(constants.ZERO_AMOUNT);
-    //         await swapQuoteConsumer.executeSwapQuoteOrThrowAsync(marketSellSwapQuote, { takerAddress, useConsumerType: ConsumerType.Forwarder });
+    //         await swapQuoteConsumer.executeSwapQuoteOrThrowAsync(marketSellSwapQuote, { takerAddress, useExtensionContract: ConsumerType.Forwarder });
     //         makerBalance = await erc20TokenContract.balanceOf.callAsync(makerAddress);
     //         takerBalance = await erc20TokenContract.balanceOf.callAsync(takerAddress);
     //         expect(takerBalance).to.bignumber.equal(new BigNumber(10).multipliedBy(ONE_ETH_IN_WEI));
     //         expect(makerBalance).to.bignumber.equal(constants.ZERO_AMOUNT);
     //     });
-    //     it('should perform an asset swap with Exchange contract when provided corresponding useConsumerType option', async () => {
+    //     it('should perform an asset swap with Exchange contract when provided corresponding useExtensionContract option', async () => {
     //         let makerBalance = await erc20TokenContract.balanceOf.callAsync(makerAddress);
     //         let takerBalance = await erc20TokenContract.balanceOf.callAsync(takerAddress);
     //         expect(makerBalance).to.bignumber.equal(new BigNumber(10).multipliedBy(ONE_ETH_IN_WEI));
@@ -143,15 +143,15 @@ describe('SwapQuoteConsumer', () => {
     describe('getSmartContractParamsOrThrow', () => {
         describe('valid swap quote', async () => {
             // TODO(david) Check for valid MethodAbi
-            it('should provide correct and optimized smart contract params for Forwarder contract when provided corresponding useConsumerType option', async () => {
+            it('should provide correct and optimized smart contract params for Forwarder contract when provided corresponding useExtensionContract option', async () => {
                 const { toAddress } = await swapQuoteConsumer.getSmartContractParamsOrThrowAsync(marketSellSwapQuote, {
-                    useConsumerType: ConsumerType.Forwarder,
+                    useExtensionContract: ExtensionContractType.Forwarder,
                 });
                 expect(toAddress).to.deep.equal(contractWrappers.forwarder.address);
             });
-            it('should provide correct and optimized smart contract params for Exchange contract when provided corresponding useConsumerType option', async () => {
+            it('should provide correct and optimized smart contract params for Exchange contract when provided corresponding useExtensionContract option', async () => {
                 const { toAddress } = await swapQuoteConsumer.getSmartContractParamsOrThrowAsync(marketSellSwapQuote, {
-                    useConsumerType: ConsumerType.Exchange,
+                    useExtensionContract: ExtensionContractType.None,
                 });
                 expect(toAddress).to.deep.equal(contractWrappers.exchange.address);
             });
@@ -160,15 +160,15 @@ describe('SwapQuoteConsumer', () => {
 
     describe('getCalldataOrThrow', () => {
         describe('valid swap quote', async () => {
-            it('should provide correct and optimized calldata options for Forwarder contract when provided corresponding useConsumerType option', async () => {
+            it('should provide correct and optimized calldata options for Forwarder contract when provided corresponding useExtensionContract option', async () => {
                 const { toAddress } = await swapQuoteConsumer.getCalldataOrThrowAsync(marketSellSwapQuote, {
-                    useConsumerType: ConsumerType.Forwarder,
+                    useExtensionContract: ExtensionContractType.Forwarder,
                 });
                 expect(toAddress).to.deep.equal(contractWrappers.forwarder.address);
             });
-            it('should provide correct and optimized smart contract params for Exchange contract when provided corresponding useConsumerType option', async () => {
+            it('should provide correct and optimized smart contract params for Exchange contract when provided corresponding useExtensionContract option', async () => {
                 const { toAddress } = await swapQuoteConsumer.getCalldataOrThrowAsync(marketSellSwapQuote, {
-                    useConsumerType: ConsumerType.Exchange,
+                    useExtensionContract: ExtensionContractType.None,
                 });
                 expect(toAddress).to.deep.equal(contractWrappers.exchange.address);
             });

packages/asset-swapper/test/swap_quote_consumer_utils_test.ts

@@ -6,9 +6,8 @@ import { BigNumber } from '@0x/utils';
 import * as chai from 'chai';
 import 'mocha';
 
-import { SwapQuote } from '../src';
+import { SwapQuote, SwapQuoteConsumer } from '../src';
-import { ConsumerType } from '../src/types';
+import { ExtensionContractType } from '../src/types';
-import { swapQuoteConsumerUtils } from '../src/utils/swap_quote_consumer_utils';
 
 import { chaiSetup } from './utils/chai_setup';
 import { migrateOnceAsync } from './utils/migrate';
@@ -39,6 +38,7 @@ describe('swapQuoteConsumerUtils', () => {
     let takerAssetData: string;
     let wethAssetData: string;
     let contractAddresses: ContractAddresses;
+    let swapQuoteConsumer: SwapQuoteConsumer;
 
     const networkId = TESTRPC_NETWORK_ID;
     before(async () => {
@@ -57,6 +57,10 @@ describe('swapQuoteConsumerUtils', () => {
             assetDataUtils.encodeERC20AssetData(takerTokenAddress),
             assetDataUtils.encodeERC20AssetData(contractAddresses.etherToken),
         ];
+
+        swapQuoteConsumer = new SwapQuoteConsumer(provider, {
+            networkId,
+        });
     });
     after(async () => {
         await blockchainLifecycle.revertAsync();
@@ -127,44 +131,36 @@ describe('swapQuoteConsumerUtils', () => {
         });
 
         it('should return exchange consumer if takerAsset is not wEth', async () => {
-            const consumerType = await swapQuoteConsumerUtils.getConsumerTypeForSwapQuoteAsync(
+            const extensionContractType = await swapQuoteConsumer.getOptimalExtensionContractTypeAsync(
                 exchangeSwapQuote,
-                contractWrappers,
-                provider,
                 { takerAddress },
             );
-            expect(consumerType).to.equal(ConsumerType.Exchange);
+            expect(extensionContractType).to.equal(ExtensionContractType.None);
         });
         it('should return forwarder consumer if takerAsset is wEth and have enough eth balance', async () => {
-            const consumerType = await swapQuoteConsumerUtils.getConsumerTypeForSwapQuoteAsync(
+            const extensionContractType = await swapQuoteConsumer.getOptimalExtensionContractTypeAsync(
                 forwarderSwapQuote,
-                contractWrappers,
-                provider,
                 { takerAddress },
             );
-            expect(consumerType).to.equal(ConsumerType.Forwarder);
+            expect(extensionContractType).to.equal(ExtensionContractType.Forwarder);
         });
         it('should return exchange consumer if takerAsset is wEth and taker has enough weth', async () => {
             const etherInWei = new BigNumber(20).multipliedBy(ONE_ETH_IN_WEI);
             await contractWrappers.weth9.deposit.sendTransactionAsync({ value: etherInWei, from: takerAddress });
-            const consumerType = await swapQuoteConsumerUtils.getConsumerTypeForSwapQuoteAsync(
+            const extensionContractType = await swapQuoteConsumer.getOptimalExtensionContractTypeAsync(
                 forwarderSwapQuote,
-                contractWrappers,
-                provider,
                 { takerAddress },
             );
-            expect(consumerType).to.equal(ConsumerType.Exchange);
+            expect(extensionContractType).to.equal(ExtensionContractType.None);
         });
         it('should return forwarder consumer if takerAsset is wEth and takerAddress has no available balance in either weth or eth (defaulting behavior)', async () => {
             const etherInWei = new BigNumber(50).multipliedBy(ONE_ETH_IN_WEI);
             await contractWrappers.weth9.deposit.sendTransactionAsync({ value: etherInWei, from: takerAddress });
-            const consumerType = await swapQuoteConsumerUtils.getConsumerTypeForSwapQuoteAsync(
+            const extensionContractType = await swapQuoteConsumer.getOptimalExtensionContractTypeAsync(
                 largeForwarderSwapQuote,
-                contractWrappers,
-                provider,
                 { takerAddress },
             );
-            expect(consumerType).to.equal(ConsumerType.Forwarder);
+            expect(extensionContractType).to.equal(ExtensionContractType.Forwarder);
         });
     });
 });