Fix asset-swapper bugs and misc improvements. (#2406)

* `@0x/contracts-erc20-bridge-sampler`: Add gas limits to external quote calls.
`@0x/contract-addresses`: Point `erc20BridgeSampler` to new version.

* `@0x/asset-swapper`: Ignore zero sample results from the sampler contract.
`@0x/asset-swapper`: Allow skipping Uniswap when dealing with low precision amounts with `minUniswapDecimals` option.
`@0x/asset-swapper`: Increase default `runLimit` from `1024` to `4096`.
`@0x/asset-swapper`: Increase default `numSamples` from `8` to `10`
`@0x/asset-swapper`: Fix ordering of optimized orders.
`@0x/asset-swapper`: Fix best and worst quotes being reversed sometimes.
`@0x/asset-swapper`: Fix rounding of quoted asset amounts.

* `@0x/contracts-utils`: Add kovan addresses to `DeploymentConstants`.
`@0x/contract-addresses`: Add kovan `ERC20BridgeSampler` address.

* `@0x/asset-swapper`: Change default `minUniswapDecimals` option from 8 to 7.

* `@0x/contracts-erc20-bridge-sampler`: Fix changelog.

* `@0x/asset-swapper`: Revert uniswap decimals fix.

* `@0x/asset-swapper`: Undo bridge slippage when computing best case quote.

* `@0x/asset-swapper`: Take asset data from input orders instead of output orders in quote result calculation.

* `@0x/asset-swapper`: Move `SAMPLER_CONTRACT_GAS_LIMIT` constant to `market_operation_utils/constants`.

* Compare equivalent asset data

* Fix redundant zero check

* Update CHANGELOG

* Set fee amount in fillable amounts test

Co-authored-by: Jacob Evans <dekz@dekz.net>

packages/asset-swapper/CHANGELOG.json

@@ -1,4 +1,41 @@
 [
+    {
+        "version": "3.0.3",
+        "changes": [
+            {
+                "note": "Ignore zero sample results from the sampler contract.",
+                "pr": 2406
+            },
+            {
+                "note": "Increase default `runLimit` from `1024` to `4096`.",
+                "pr": 2406
+            },
+            {
+                "note": "Increase default `numSamples` from `8` to `10`",
+                "pr": 2406
+            },
+            {
+                "note": "Fix ordering of optimized orders.",
+                "pr": 2406
+            },
+            {
+                "note": "Fix best and worst quotes being reversed sometimes.",
+                "pr": 2406
+            },
+            {
+                "note": "Fix rounding of quoted asset amounts.",
+                "pr": 2406
+            },
+            {
+                "note": "Undo bridge slippage in best case quote calculation.",
+                "pr": 2406
+            },
+            {
+                "note": "Compare equivalent asset data when validating quotes and checking fee asset data.",
+                "pr": 2421
+            }
+        ]
+    },
     {
         "version": "3.0.2",
         "changes": [

packages/asset-swapper/src/constants.ts

@@ -83,4 +83,5 @@ export const constants = {
     NULL_ERC20_ASSET_DATA,
     PROTOCOL_FEE_UTILS_POLLING_INTERVAL_IN_MS,
     MARKET_UTILS_AMOUNT_BUFFER_PERCENTAGE,
+    BRIDGE_ASSET_DATA_PREFIX: '0xdc1600f3',
 };

packages/asset-swapper/src/swap_quoter.ts

@@ -22,6 +22,7 @@ import {
 import { assert } from './utils/assert';
 import { calculateLiquidity } from './utils/calculate_liquidity';
 import { MarketOperationUtils } from './utils/market_operation_utils';
+import { constants as marketOperationConstants } from './utils/market_operation_utils/constants';
 import { dummyOrderUtils } from './utils/market_operation_utils/dummy_order_utils';
 import { orderPrunerUtils } from './utils/order_prune_utils';
 import { OrderStateUtils } from './utils/order_state_utils';
@@ -165,6 +166,7 @@ export class SwapQuoter {
         const samplerContract = new IERC20BridgeSamplerContract(
             this._contractAddresses.erc20BridgeSampler,
             this.provider,
+            { gas: marketOperationConstants.SAMPLER_CONTRACT_GAS_LIMIT },
         );
         this._marketOperationUtils = new MarketOperationUtils(samplerContract, this._contractAddresses, {
             chainId,

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

@@ -36,13 +36,13 @@ export const assert = {
     ): void {
         _.every(orders, (order: SignedOrder, index: number) => {
             assert.assert(
-                order.takerAssetData === takerAssetData,
+                utils.isAssetDataEquivalent(takerAssetData, order.takerAssetData),
                 `Expected ${variableName}[${index}].takerAssetData to be ${takerAssetData} but found ${
                     order.takerAssetData
                 }`,
             );
             assert.assert(
-                order.makerAssetData === makerAssetData,
+                utils.isAssetDataEquivalent(makerAssetData, order.makerAssetData),
                 `Expected ${variableName}[${index}].makerAssetData to be ${makerAssetData} but found ${
                     order.makerAssetData
                 }`,
@@ -72,7 +72,7 @@ export const assert = {
     },
     isValidForwarderSignedOrder(variableName: string, order: SignedOrder, wethAssetData: string): void {
         assert.assert(
-            order.takerAssetData === wethAssetData,
+            utils.isExactAssetData(order.takerAssetData, wethAssetData),
             `Expected ${variableName} to have takerAssetData set as ${wethAssetData}, but is ${order.takerAssetData}`,
         );
     },

packages/asset-swapper/src/utils/market_operation_utils/constants.ts

@@ -24,11 +24,11 @@ export const SELL_SOURCES = [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Da
 export const BUY_SOURCES = [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Dai];
 
 export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = {
-    runLimit: 1024,
+    runLimit: 4096,
     excludedSources: [],
     bridgeSlippage: 0.0005,
     dustFractionThreshold: 0.01,
-    numSamples: 8,
+    numSamples: 10,
     noConflicts: true,
 };
 
@@ -40,4 +40,5 @@ export const constants = {
     DEFAULT_GET_MARKET_ORDERS_OPTS,
     ERC20_PROXY_ID: '0xf47261b0',
     WALLET_SIGNATURE: '0x04',
+    SAMPLER_CONTRACT_GAS_LIMIT: 10e6,
 };

packages/asset-swapper/src/utils/market_operation_utils/create_order.ts

@@ -4,6 +4,7 @@ import { AbiEncoder, BigNumber } from '@0x/utils';
 
 import { constants } from '../../constants';
 import { SignedOrderWithFillableAmounts } from '../../types';
+import { sortingUtils } from '../../utils/sorting_utils';
 
 import { constants as marketOperationUtilConstants } from './constants';
 import { AggregationError, ERC20BridgeSource, Fill, FillData, NativeFillData, OrderDomain } from './types';
@@ -45,7 +46,7 @@ export class CreateOrderUtils {
                 );
             }
         }
-        return orders;
+        return sortingUtils.sortOrders(orders);
     }
 
     // Convert buy fills into orders.
@@ -76,7 +77,7 @@ export class CreateOrderUtils {
                 );
             }
         }
-        return orders;
+        return sortingUtils.sortOrders(orders);
     }
 
     private _getBridgeAddressFromSource(source: ERC20BridgeSource): string {
@@ -135,7 +136,7 @@ function createCommonOrderFields(
 ): CommonOrderFields {
     const makerAssetAmountAdjustedWithSlippage = isBuy
         ? makerAssetAmount
-        : makerAssetAmount.times(1 - slippage).integerValue(BigNumber.ROUND_UP);
+        : makerAssetAmount.times(1 - slippage).integerValue(BigNumber.ROUND_DOWN);
     const takerAssetAmountAdjustedWithSlippage = isBuy
         ? takerAssetAmount.times(slippage + 1).integerValue(BigNumber.ROUND_UP)
         : takerAssetAmount;

packages/asset-swapper/src/utils/market_operation_utils/index.ts

@@ -137,7 +137,6 @@ export class MarketOperationUtils {
             DexOrderSampler.getSampleAmounts(makerAmount, _opts.numSamples),
             difference(BUY_SOURCES, _opts.excludedSources),
         );
-
         const nativeOrdersWithFillableAmounts = createSignedOrdersWithFillableAmounts(
             nativeOrders,
             fillableAmounts,
@@ -266,20 +265,27 @@ function createPathsFromDexQuotes(dexQuotes: DexSample[][], noConflicts: boolean
     for (const quote of dexQuotes) {
         // Native orders can be filled in any order, so they're all root nodes.
         const path: Fill[] = [];
-        paths.push(path);
+        let prevSample: DexSample | undefined;
         // tslint:disable-next-line: prefer-for-of
         for (let i = 0; i < quote.length; i++) {
             const sample = quote[i];
-            const prev = i !== 0 ? quote[i - 1] : undefined;
+            if (sample.output.eq(0) || (prevSample && prevSample.output.gte(sample.output))) {
-            const parent = i !== 0 ? path[path.length - 1] : undefined;
+                // Stop if the output is zero or does not increase.
+                break;
+            }
             path.push({
-                parent,
+                parent: path.length !== 0 ? path[path.length - 1] : undefined,
                 flags: sourceToFillFlags(sample.source),
                 exclusionMask: noConflicts ? sourceToExclusionMask(sample.source) : 0,
-                input: sample.input.minus(prev ? prev.input : 0),
+                input: sample.input.minus(prevSample ? prevSample.input : 0),
-                output: sample.output.minus(prev ? prev.output : 0),
+                output: sample.output.minus(prevSample ? prevSample.output : 0),
                 fillData: { source: sample.source },
             });
+            prevSample = quote[i];
+        }
+        if (path.length > 0) {
+            // Don't push empty paths.
+            paths.push(path);
         }
     }
     return paths;

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

@@ -21,8 +21,10 @@ export class ProtocolFeeUtils {
         return constants.PROTOCOL_FEE_MULTIPLIER;
     }
 
-    // tslint:disable-next-line: prefer-function-over-method
     public async getGasPriceEstimationOrThrowAsync(shouldHardRefresh?: boolean): Promise<BigNumber> {
+        if (this.gasPriceEstimation.eq(constants.ZERO_AMOUNT)) {
+            return this._getGasPriceFromGasStationOrThrowAsync();
+        }
         if (shouldHardRefresh) {
             return this._getGasPriceFromGasStationOrThrowAsync();
         } else {

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

@@ -91,16 +91,17 @@ export class SwapQuoteCalculator {
         }
 
         // assetData information for the result
-        const takerAssetData = resultOrders[0].takerAssetData;
+        const takerAssetData = prunedOrders[0].takerAssetData;
-        const makerAssetData = resultOrders[0].makerAssetData;
+        const makerAssetData = prunedOrders[0].makerAssetData;
 
         const bestCaseQuoteInfo = await this._calculateQuoteInfoAsync(
-            resultOrders,
+            createBestCaseOrders(resultOrders, operation, opts.bridgeSlippage),
             assetFillAmount,
             gasPrice,
             operation,
         );
-        // in order to calculate the maxRate, reverse the ordersAndFillableAmounts such that they are sorted from worst rate to best rate
+        // in order to calculate the maxRate, reverse the ordersAndFillableAmounts
+        // such that they are sorted from worst rate to best rate
         const worstCaseQuoteInfo = await this._calculateQuoteInfoAsync(
             _.reverse(resultOrders.slice()),
             assetFillAmount,
@@ -176,7 +177,7 @@ export class SwapQuoteCalculator {
                 const makerAssetAmount = takerAssetAmountWithFees
                     .div(adjustedFillableTakerAssetAmount)
                     .multipliedBy(adjustedFillableMakerAssetAmount)
-                    .integerValue(BigNumber.ROUND_CEIL);
+                    .integerValue(BigNumber.ROUND_DOWN);
                 return {
                     totalMakerAssetAmount: totalMakerAssetAmount.plus(makerAssetAmount),
                     totalTakerAssetAmount: totalTakerAssetAmount.plus(takerAssetAmount),
@@ -230,7 +231,7 @@ export class SwapQuoteCalculator {
                 const takerAssetAmountWithFees = makerFillAmount
                     .div(adjustedFillableMakerAssetAmount)
                     .multipliedBy(adjustedFillableTakerAssetAmount)
-                    .integerValue(BigNumber.ROUND_CEIL);
+                    .integerValue(BigNumber.ROUND_UP);
 
                 const { takerAssetAmount, feeTakerAssetAmount } = getTakerAssetAmountBreakDown(
                     order,
@@ -291,7 +292,7 @@ function getTakerAssetAmountBreakDown(
         const takerAssetAmount = takerFeeAmount
             .div(makerAssetFillAmount)
             .multipliedBy(takerAssetAmountWithFees)
-            .integerValue(BigNumber.ROUND_CEIL);
+            .integerValue(BigNumber.ROUND_UP);
         return {
             takerAssetAmount,
             feeTakerAssetAmount: takerAssetAmountWithFees.minus(takerAssetAmount),
@@ -302,3 +303,35 @@ function getTakerAssetAmountBreakDown(
         takerAssetAmount: takerAssetAmountWithFees,
     };
 }
+
+function createBestCaseOrders(
+    orders: SignedOrderWithFillableAmounts[],
+    operation: MarketOperation,
+    bridgeSlippage: number,
+): SignedOrderWithFillableAmounts[] {
+    // Scale the asset amounts to undo the bridge slippage applied to
+    // bridge orders.
+    const bestCaseOrders: SignedOrderWithFillableAmounts[] = [];
+    for (const order of orders) {
+        if (!order.makerAssetData.startsWith(constants.BRIDGE_ASSET_DATA_PREFIX)) {
+            bestCaseOrders.push(order);
+            continue;
+        }
+        bestCaseOrders.push({
+            ...order,
+            ...(operation === MarketOperation.Sell
+                ? {
+                      makerAssetAmount: order.makerAssetAmount
+                          .dividedBy(1 - bridgeSlippage)
+                          .integerValue(BigNumber.ROUND_DOWN),
+                  }
+                : // Buy operation
+                  {
+                      takerAssetAmount: order.takerAssetAmount
+                          .dividedBy(bridgeSlippage + 1)
+                          .integerValue(BigNumber.ROUND_UP),
+                  }),
+        });
+    }
+    return bestCaseOrders;
+}

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

@@ -15,6 +15,7 @@ import {
     SwapQuoteConsumerError,
     SwapQuoteExecutionOpts,
 } from '../types';
+import { utils } from '../utils/utils';
 
 import { assert } from './assert';
 
@@ -65,7 +66,7 @@ export const swapQuoteConsumerUtils = {
         return _.every(orders, order => swapQuoteConsumerUtils.isValidForwarderSignedOrder(order, wethAssetData));
     },
     isValidForwarderSignedOrder(order: SignedOrder, wethAssetData: string): boolean {
-        return order.takerAssetData === wethAssetData;
+        return utils.isExactAssetData(order.takerAssetData, wethAssetData);
     },
     async getExtensionContractTypeForSwapQuoteAsync(
         quote: SwapQuote,

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

@@ -1,5 +1,6 @@
-import { Order } from '@0x/types';
+import { assetDataUtils } from '@0x/order-utils';
-import { BigNumber } from '@0x/utils';
+import { AssetData, ERC20AssetData, ERC20BridgeAssetData, Order } from '@0x/types';
+import { BigNumber, NULL_BYTES } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
 import { AbiDefinition, ContractAbi, MethodAbi } from 'ethereum-types';
 import * as _ from 'lodash';
@@ -27,10 +28,10 @@ export const utils = {
         ) as MethodAbi | undefined;
     },
     isOrderTakerFeePayableWithMakerAsset<T extends Order>(order: T): boolean {
-        return order.takerFeeAssetData === order.makerAssetData;
+        return !order.takerFee.isZero() && utils.isAssetDataEquivalent(order.takerFeeAssetData, order.makerAssetData);
     },
     isOrderTakerFeePayableWithTakerAsset<T extends Order>(order: T): boolean {
-        return order.takerFeeAssetData === order.takerAssetData;
+        return !order.takerFee.isZero() && utils.isAssetDataEquivalent(order.takerFeeAssetData, order.takerAssetData);
     },
     getAdjustedMakerAndTakerAmountsFromTakerFees<T extends Order>(order: T): [BigNumber, BigNumber] {
         const adjustedMakerAssetAmount = utils.isOrderTakerFeePayableWithMakerAsset(order)
@@ -41,4 +42,66 @@ export const utils = {
             : order.takerAssetAmount;
         return [adjustedMakerAssetAmount, adjustedTakerAssetAmount];
     },
+    isExactAssetData(expectedAssetData: string, actualAssetData: string): boolean {
+        return expectedAssetData === actualAssetData;
+    },
+    /**
+     * Compare the Asset Data for equivalency. Expected is the asset data the user provided (wanted),
+     * actual is the asset data found or created.
+     */
+    isAssetDataEquivalent(expectedAssetData: string, actualAssetData: string): boolean {
+        if (utils.isExactAssetData(expectedAssetData, actualAssetData)) {
+            return true;
+        }
+        const decodedExpectedAssetData = assetDataUtils.decodeAssetDataOrThrow(expectedAssetData);
+        const decodedActualAssetData = assetDataUtils.decodeAssetDataOrThrow(actualAssetData);
+        // ERC20 === ERC20, ERC20 === ERC20Bridge
+        if (
+            utils.isERC20EquivalentAssetData(decodedExpectedAssetData) &&
+            utils.isERC20EquivalentAssetData(decodedActualAssetData)
+        ) {
+            const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress;
+            return doesTokenAddressMatch;
+        }
+        // ERC1155 === ERC1155
+        if (
+            assetDataUtils.isERC1155TokenAssetData(decodedExpectedAssetData) &&
+            assetDataUtils.isERC1155TokenAssetData(decodedActualAssetData)
+        ) {
+            const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress;
+            // IDs may be out of order yet still equivalent
+            // i.e (["a", "b"], [1,2]) === (["b", "a"], [2, 1])
+            //     (["a", "b"], [2,1]) !== (["b", "a"], [2, 1])
+            const hasAllIds = decodedExpectedAssetData.tokenIds.every(
+                id => decodedActualAssetData.tokenIds.findIndex(v => id.eq(v)) !== -1,
+            );
+            const hasAllValues = decodedExpectedAssetData.tokenIds.every((id, i) =>
+                decodedExpectedAssetData.tokenValues[i].eq(
+                    decodedActualAssetData.tokenValues[decodedActualAssetData.tokenIds.findIndex(v => id.eq(v))],
+                ),
+            );
+            // If expected contains callback data, ensure it is present
+            // if actual has callbackdata and expected provided none then ignore it
+            const hasEquivalentCallback =
+                decodedExpectedAssetData.callbackData === NULL_BYTES ||
+                decodedExpectedAssetData.callbackData === decodedActualAssetData.callbackData;
+            return doesTokenAddressMatch && hasAllIds && hasAllValues && hasEquivalentCallback;
+        }
+        // ERC721 === ERC721
+        if (
+            assetDataUtils.isERC721TokenAssetData(decodedExpectedAssetData) ||
+            assetDataUtils.isERC721TokenAssetData(decodedActualAssetData)
+        ) {
+            // Asset Data should exactly match for ERC721
+            return utils.isExactAssetData(expectedAssetData, actualAssetData);
+        }
+
+        // TODO(dekz): Unsupported cases
+        // ERCXX(token) === MAP(token, staticCall)
+        // MAP(a, b) === MAP(b, a) === MAP(b, a, staticCall)
+        return false;
+    },
+    isERC20EquivalentAssetData(assetData: AssetData): assetData is ERC20AssetData | ERC20BridgeAssetData {
+        return assetDataUtils.isERC20TokenAssetData(assetData) || assetDataUtils.isERC20BridgeAssetData(assetData);
+    },
 };

packages/asset-swapper/test/fillable_amounts_utils_test.ts

@@ -12,13 +12,14 @@ chaiSetup.configure();
 const expect = chai.expect;
 
 // tslint:disable:custom-no-magic-numbers
-const FAKE_ERC20_TAKER_ASSET_DATA = '0xf47261b22222222222222222222222222222222222222222222222222222222222222222';
+const FAKE_ERC20_TAKER_ASSET_DATA = '0xf47261b02222222222222222222222222222222222222222222222222222222222222222';
-const FAKE_ERC20_MAKER_ASSET_DATA = '0xf47261b11111111111111111111111111111111111111111111111111111111111111111';
+const FAKE_ERC20_MAKER_ASSET_DATA = '0xf47261b01111111111111111111111111111111111111111111111111111111111111111';
 
 const TAKER_ASSET_DENOMINATED_TAKER_FEE_ORDER = testOrderFactory.generateTestSignedOrderWithFillableAmounts({
     takerAssetData: FAKE_ERC20_TAKER_ASSET_DATA,
     makerAssetData: FAKE_ERC20_MAKER_ASSET_DATA,
     takerFeeAssetData: FAKE_ERC20_TAKER_ASSET_DATA,
+    takerFee: baseUnitAmount(2),
     fillableMakerAssetAmount: baseUnitAmount(5),
     fillableTakerAssetAmount: baseUnitAmount(10),
     fillableTakerFeeAmount: baseUnitAmount(2),
@@ -28,6 +29,7 @@ const MAKER_ASSET_DENOMINATED_TAKER_FEE_ORDER = testOrderFactory.generateTestSig
     takerAssetData: FAKE_ERC20_TAKER_ASSET_DATA,
     makerAssetData: FAKE_ERC20_MAKER_ASSET_DATA,
     takerFeeAssetData: FAKE_ERC20_MAKER_ASSET_DATA,
+    takerFee: baseUnitAmount(2),
     fillableMakerAssetAmount: baseUnitAmount(10),
     fillableTakerAssetAmount: baseUnitAmount(5),
     fillableTakerFeeAmount: baseUnitAmount(2),

packages/asset-swapper/test/market_operation_utils_test.ts

@@ -53,12 +53,12 @@ describe('MarketOperationUtils tests', () => {
             expirationTimeSeconds: getRandomInteger(0, 2 ** 64),
             makerAssetData: MAKER_ASSET_DATA,
             takerAssetData: TAKER_ASSET_DATA,
-            makerFeeAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()),
+            makerFeeAssetData: constants.NULL_BYTES,
-            takerFeeAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()),
+            takerFeeAssetData: constants.NULL_BYTES,
             makerAssetAmount: getRandomInteger(1, 1e18),
             takerAssetAmount: getRandomInteger(1, 1e18),
-            makerFee: getRandomInteger(1, 1e17),
+            makerFee: constants.ZERO_AMOUNT,
-            takerFee: getRandomInteger(1, 1e17),
+            takerFee: constants.ZERO_AMOUNT,
             signature: hexUtils.random(),
             ...overrides,
         };
@@ -99,7 +99,7 @@ describe('MarketOperationUtils tests', () => {
         const singleTakerAssetAmount = takerAssetAmount.div(rates.length).integerValue(BigNumber.ROUND_UP);
         return rates.map(r =>
             createOrder({
-                makerAssetAmount: singleTakerAssetAmount.times(r),
+                makerAssetAmount: singleTakerAssetAmount.times(r).integerValue(),
                 takerAssetAmount: singleTakerAssetAmount,
             }),
         );
@@ -110,7 +110,7 @@ describe('MarketOperationUtils tests', () => {
         return (rates as any).map((r: Numberish) =>
             createOrder({
                 makerAssetAmount: singleMakerAssetAmount,
-                takerAssetAmount: singleMakerAssetAmount.div(r),
+                takerAssetAmount: singleMakerAssetAmount.div(r).integerValue(),
             }),
         );
     }
@@ -479,22 +479,22 @@ describe('MarketOperationUtils tests', () => {
                     { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: false },
                 );
                 expect(improvedOrders).to.be.length(4);
-                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData)).sort();
+                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData));
                 const expectedSources = [
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Uniswap,
-                    ERC20BridgeSource.Eth2Dai,
                     ERC20BridgeSource.Kyber,
-                ].sort();
+                    ERC20BridgeSource.Eth2Dai,
+                    ERC20BridgeSource.Uniswap,
+                    ERC20BridgeSource.Native,
+                ];
                 expect(orderSources).to.deep.eq(expectedSources);
             });
 
             it('excludes Kyber when `noConflicts` enabled and Uniswap or Eth2Dai are used first', async () => {
                 const rates: RatesBySource = {};
-                rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.2, 0.1];
+                rates[ERC20BridgeSource.Native] = [0.3, 0.2, 0.1, 0.05];
                 rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05];
                 rates[ERC20BridgeSource.Eth2Dai] = [0.6, 0.05, 0.05, 0.05];
-                rates[ERC20BridgeSource.Kyber] = [0.7, 0.05, 0.05, 0.05];
+                rates[ERC20BridgeSource.Kyber] = [0.4, 0.05, 0.05, 0.05];
                 const marketOperationUtils = new MarketOperationUtils(
                     createSamplerFromSellRates(rates),
                     contractAddresses,
@@ -506,19 +506,19 @@ describe('MarketOperationUtils tests', () => {
                     { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: true },
                 );
                 expect(improvedOrders).to.be.length(4);
-                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData)).sort();
+                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData));
                 const expectedSources = [
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Uniswap,
                     ERC20BridgeSource.Eth2Dai,
-                ].sort();
+                    ERC20BridgeSource.Uniswap,
+                    ERC20BridgeSource.Native,
+                    ERC20BridgeSource.Native,
+                ];
                 expect(orderSources).to.deep.eq(expectedSources);
             });
 
             it('excludes Uniswap and Eth2Dai when `noConflicts` enabled and Kyber is used first', async () => {
                 const rates: RatesBySource = {};
-                rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.2, 0.1];
+                rates[ERC20BridgeSource.Native] = [0.1, 0.05, 0.05, 0.05];
                 rates[ERC20BridgeSource.Uniswap] = [0.15, 0.05, 0.05, 0.05];
                 rates[ERC20BridgeSource.Eth2Dai] = [0.15, 0.05, 0.05, 0.05];
                 rates[ERC20BridgeSource.Kyber] = [0.7, 0.05, 0.05, 0.05];
@@ -533,13 +533,13 @@ describe('MarketOperationUtils tests', () => {
                     { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: true },
                 );
                 expect(improvedOrders).to.be.length(4);
-                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData)).sort();
+                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData));
                 const expectedSources = [
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Native,
                     ERC20BridgeSource.Kyber,
-                ].sort();
+                    ERC20BridgeSource.Native,
+                    ERC20BridgeSource.Native,
+                    ERC20BridgeSource.Native,
+                ];
                 expect(orderSources).to.deep.eq(expectedSources);
             });
         });
@@ -729,13 +729,13 @@ describe('MarketOperationUtils tests', () => {
                     { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512 },
                 );
                 expect(improvedOrders).to.be.length(4);
-                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData)).sort();
+                const orderSources = improvedOrders.map(o => getSourceFromAssetData(o.makerAssetData));
                 const expectedSources = [
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Native,
-                    ERC20BridgeSource.Uniswap,
                     ERC20BridgeSource.Eth2Dai,
-                ].sort();
+                    ERC20BridgeSource.Uniswap,
+                    ERC20BridgeSource.Native,
+                    ERC20BridgeSource.Native,
+                ];
                 expect(orderSources).to.deep.eq(expectedSources);
             });
         });

packages/asset-swapper/test/sorting_utils_test.ts

@@ -10,8 +10,8 @@ import { testOrderFactory } from './utils/test_order_factory';
 chaiSetup.configure();
 const expect = chai.expect;
 
-const FAKE_ERC20_TAKER_ASSET_DATA = '0xf47261b22222222222222222222222222222222222222222222222222222222222222222';
+const FAKE_ERC20_TAKER_ASSET_DATA = '0xf47261b02222222222222222222222222222222222222222222222222222222222222222';
-const FAKE_ERC20_MAKER_ASSET_DATA = '0xf47261b11111111111111111111111111111111111111111111111111111111111111111';
+const FAKE_ERC20_MAKER_ASSET_DATA = '0xf47261b01111111111111111111111111111111111111111111111111111111111111111';
 
 describe('sortingUtils', () => {
     describe('#sortOrders', () => {

packages/asset-swapper/test/swap_quote_calculator_test.ts

@@ -450,20 +450,20 @@ describe('swapQuoteCalculator', () => {
             );
             // test if orders are correct
             expect(swapQuote.orders).to.deep.equal([
-                testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0],
                 testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[1],
+                testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0],
             ]);
             expect(swapQuote.takerAssetFillAmount).to.bignumber.equal(assetSellAmount);
             // test if rates are correct
             // 50 takerAsset units to fill the first order + 100 takerAsset units for fees
-            expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({
+            expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({
                 feeTakerAssetAmount: baseUnitAmount(2),
                 takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)),
                 totalTakerAssetAmount: assetSellAmount,
                 makerAssetAmount: baseUnitAmount(0.8),
                 protocolFeeInWeiAmount: baseUnitAmount(30, 4),
             });
-            expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({
+            expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({
                 feeTakerAssetAmount: baseUnitAmount(2),
                 takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)),
                 totalTakerAssetAmount: assetSellAmount,

packages/asset-swapper/test/utils_test.ts

@@ -0,0 +1,144 @@
+import { tokenUtils } from '@0x/dev-utils';
+import { assetDataUtils } from '@0x/order-utils';
+import { BigNumber, NULL_ADDRESS, NULL_BYTES } from '@0x/utils';
+import * as chai from 'chai';
+import 'mocha';
+
+import { utils } from '../src/utils/utils';
+
+import { chaiSetup } from './utils/chai_setup';
+
+chaiSetup.configure();
+const expect = chai.expect;
+
+describe('utils', () => {
+    describe('isAssetDataEquivalent', () => {
+        describe('ERC20', () => {
+            const [tokenA, tokenB] = tokenUtils.getDummyERC20TokenAddresses();
+            it('should succeed ERC20 to be ERC20Bridge', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                    assetDataUtils.encodeERC20BridgeAssetData(tokenA, NULL_ADDRESS, NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should succeed ERC20Bridge to be ERC20', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC20BridgeAssetData(tokenA, NULL_ADDRESS, NULL_BYTES),
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should succeed ERC20 to be ERC20', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should fail if ERC20Bridge is not the same ERC20 token', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                    assetDataUtils.encodeERC20BridgeAssetData(tokenB, NULL_ADDRESS, NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+            it('should fail if ERC20 is not the same ERC20 token', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                    assetDataUtils.encodeERC20AssetData(tokenB),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+        });
+        describe('ERC721', () => {
+            const [tokenA, tokenB] = tokenUtils.getDummyERC20TokenAddresses();
+            const tokenIdA = new BigNumber(1);
+            const tokenIdB = new BigNumber(2);
+            it('should succeed if ERC721 the same ERC721 token and id', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA),
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should fail if ERC721 is not the same ERC721 token', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA),
+                    assetDataUtils.encodeERC721AssetData(tokenB, tokenIdA),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+            it('should fail if ERC721 is not the same ERC721 id', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA),
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdB),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+            it('should fail if ERC721 is compared with ERC20', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA),
+                    assetDataUtils.encodeERC20AssetData(tokenA),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+        });
+        describe('ERC1155', () => {
+            const [tokenA, tokenB] = tokenUtils.getDummyERC20TokenAddresses();
+            const tokenIdA = new BigNumber(1);
+            const tokenIdB = new BigNumber(2);
+            const valueA = new BigNumber(1);
+            const valueB = new BigNumber(2);
+            it('should succeed if ERC1155 is the same ERC1155 token and id', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA], [valueA], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA], [valueA], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should succeed if ERC1155 is the same ERC1155 token and ids', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should succeed if ERC1155 is the same ERC1155 token and ids in different orders', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should succeed if ERC1155 is the same ERC1155 token and ids with different callback data', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], tokenA),
+                );
+                expect(isEquivalent).to.be.true();
+            });
+            it('should fail if ERC1155 contains different ids', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenB, [tokenIdA], [valueB], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+            it('should fail if ERC1155 is a different ERC1155 token', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES),
+                    assetDataUtils.encodeERC1155AssetData(tokenB, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+            it('should fail if expected ERC1155 has different callback data', () => {
+                const isEquivalent = utils.isAssetDataEquivalent(
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], tokenA),
+                    assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES),
+                );
+                expect(isEquivalent).to.be.false();
+            });
+        });
+    });
+});

packages/order-utils/CHANGELOG.json

@@ -1,4 +1,13 @@
 [
+    {
+        "version": "10.1.0",
+        "changes": [
+            {
+                "note": "Export `isERC20AssetData`, `isERC20BridgeAssetData` and other equivalents.",
+                "pr": 2421
+            }
+        ]
+    },
     {
         "timestamp": 1576540892,
         "version": "10.0.1",

packages/order-utils/src/asset_data_utils.ts

@@ -2,9 +2,14 @@ import { IAssetDataContract } from '@0x/contract-wrappers';
 import {
     AssetData,
     AssetProxyId,
+    ERC1155AssetData,
+    ERC20AssetData,
+    ERC20BridgeAssetData,
+    ERC721AssetData,
     MultiAssetData,
     MultiAssetDataWithRecursiveDecoding,
     SingleAssetData,
+    StaticCallAssetData,
 } from '@0x/types';
 import { BigNumber, hexUtils, NULL_ADDRESS } from '@0x/utils';
 import * as _ from 'lodash';
@@ -162,4 +167,22 @@ export const assetDataUtils = {
             nestedAssetData: flattenedDecodedNestedAssetData as SingleAssetData[],
         };
     },
+    isERC20TokenAssetData(assetData: AssetData): assetData is ERC20AssetData {
+        return assetData.assetProxyId === AssetProxyId.ERC20;
+    },
+    isERC20BridgeAssetData(assetData: AssetData): assetData is ERC20BridgeAssetData {
+        return assetData.assetProxyId === AssetProxyId.ERC20Bridge;
+    },
+    isERC1155TokenAssetData(assetData: AssetData): assetData is ERC1155AssetData {
+        return assetData.assetProxyId === AssetProxyId.ERC1155;
+    },
+    isERC721TokenAssetData(assetData: AssetData): assetData is ERC721AssetData {
+        return assetData.assetProxyId === AssetProxyId.ERC721;
+    },
+    isMultiAssetData(assetData: AssetData): assetData is MultiAssetData {
+        return assetData.assetProxyId === AssetProxyId.MultiAsset;
+    },
+    isStaticCallAssetData(assetData: AssetData): assetData is StaticCallAssetData {
+        return assetData.assetProxyId === AssetProxyId.StaticCall;
+    },
 };