Add ability to specify takerAssetFillAmount and taker scenarios as part of a FillScenario

4 files changed, 153 insertions, 35 deletions

diff --git a/packages/contracts/src/utils/core_combinatorial_utils.ts b/packages/contracts/src/utils/core_combinatorial_utils.ts
index 6fbd8304e..dac4e9edd 100644
--- a/packages/contracts/src/utils/core_combinatorial_utils.ts
+++ b/packages/contracts/src/utils/core_combinatorial_utils.ts
@@ -7,7 +7,7 @@ import {
     OrderValidationUtils,
 } from '@0xproject/order-utils';
 import { AssetProxyId, Order, SignatureType, SignedOrder } from '@0xproject/types';
-import { BigNumber } from '@0xproject/utils';
+import { BigNumber, errorUtils } from '@0xproject/utils';
 import { Web3Wrapper } from '@0xproject/web3-wrapper';
 import * as chai from 'chai';
 import { BlockParamLiteral, LogWithDecodedArgs, Provider, TxData } from 'ethereum-types';
@@ -35,6 +35,9 @@ import {
     FeeRecipientAddressScenario,
     OrderAmountScenario,
     OrderScenario,
+    TakerAssetFillAmountScenario,
+    TakerScenario,
+    FillScenario,
 } from '../utils/types';
 
 chaiSetup.configure();
@@ -142,11 +145,12 @@ export class CoreCombinatorialUtils {
     public exchangeWrapper: ExchangeWrapper;
     public assetWrapper: AssetWrapper;
     public static generateOrderCombinations(): OrderScenario[] {
+        const takerScenarios = [TakerScenario.Unspecified];
         const feeRecipientScenarios = [FeeRecipientAddressScenario.EthUserAddress];
-        const makerAssetAmountScenario = [OrderAmountScenario.NonZero];
-        const takerAssetAmountScenario = [OrderAmountScenario.NonZero];
-        const makerFeeScenario = [OrderAmountScenario.NonZero];
-        const takerFeeScenario = [OrderAmountScenario.NonZero];
+        const makerAssetAmountScenario = [OrderAmountScenario.Large];
+        const takerAssetAmountScenario = [OrderAmountScenario.Large];
+        const makerFeeScenario = [OrderAmountScenario.Large];
+        const takerFeeScenario = [OrderAmountScenario.Large];
         const expirationTimeSecondsScenario = [ExpirationTimeSecondsScenario.InFuture];
         const makerAssetDataScenario = [
             AssetDataScenario.ERC20FiveDecimals,
@@ -161,6 +165,7 @@ export class CoreCombinatorialUtils {
             AssetDataScenario.ZRXFeeToken,
         ];
         const orderScenarioArrays = CoreCombinatorialUtils._allPossibleCases([
+            takerScenarios,
             feeRecipientScenarios,
             makerAssetAmountScenario,
             takerAssetAmountScenario,
@@ -173,14 +178,15 @@ export class CoreCombinatorialUtils {
 
         const orderScenarios = _.map(orderScenarioArrays, orderScenarioArray => {
             const orderScenario: OrderScenario = {
-                feeRecipientScenario: orderScenarioArray[0] as FeeRecipientAddressScenario,
-                makerAssetAmountScenario: orderScenarioArray[1] as OrderAmountScenario,
-                takerAssetAmountScenario: orderScenarioArray[2] as OrderAmountScenario,
-                makerFeeScenario: orderScenarioArray[3] as OrderAmountScenario,
-                takerFeeScenario: orderScenarioArray[4] as OrderAmountScenario,
-                expirationTimeSecondsScenario: orderScenarioArray[5] as ExpirationTimeSecondsScenario,
-                makerAssetDataScenario: orderScenarioArray[6] as AssetDataScenario,
-                takerAssetDataScenario: orderScenarioArray[7] as AssetDataScenario,
+                takerScenario: orderScenarioArray[0] as TakerScenario,
+                feeRecipientScenario: orderScenarioArray[1] as FeeRecipientAddressScenario,
+                makerAssetAmountScenario: orderScenarioArray[2] as OrderAmountScenario,
+                takerAssetAmountScenario: orderScenarioArray[3] as OrderAmountScenario,
+                makerFeeScenario: orderScenarioArray[4] as OrderAmountScenario,
+                takerFeeScenario: orderScenarioArray[5] as OrderAmountScenario,
+                expirationTimeSecondsScenario: orderScenarioArray[6] as ExpirationTimeSecondsScenario,
+                makerAssetDataScenario: orderScenarioArray[7] as AssetDataScenario,
+                takerAssetDataScenario: orderScenarioArray[8] as AssetDataScenario,
             };
             return orderScenario;
         });
@@ -225,7 +231,10 @@ export class CoreCombinatorialUtils {
         this.exchangeWrapper = exchangeWrapper;
         this.assetWrapper = assetWrapper;
     }
-    public async testFillOrderScenarioAsync(order: Order, provider: Provider): Promise<void> {
+    public async testFillOrderScenarioAsync(provider: Provider, fillScenario: FillScenario): Promise<void> {
+        // 1. Generate order
+        const order = this.orderFactory.generateOrder(fillScenario.orderScenario);
+
         // 2. Sign order
         const orderHashBuff = orderHashUtils.getOrderHashBuff(order);
         const signature = signingUtils.signMessage(orderHashBuff, this.makerPrivateKey, SignatureType.EthSign);
@@ -235,8 +244,9 @@ export class CoreCombinatorialUtils {
         };
 
         // 3. Permutate the maker and taker balance/allowance scenarios
+        // TODO(fabio)
 
-        // 4. Figure out fill amount OR error
+        // 4. Figure out fill amount
         const balanceAndProxyAllowanceFetcher = new SimpleAssetBalanceAndProxyAllowanceFetcher(this.assetWrapper);
         const orderFilledCancelledFetcher = new SimpleOrderFilledCancelledFetcher(
             this.exchangeWrapper,
@@ -249,14 +259,37 @@ export class CoreCombinatorialUtils {
             this.takerAddress,
         );
 
-        // If order is fillable, decide how much to fill
-        // TODO: Make this configurable
-        const takerAssetProxyId = assetProxyUtils.decodeAssetDataId(signedOrder.takerAssetData);
-        const makerAssetProxyId = assetProxyUtils.decodeAssetDataId(signedOrder.makerAssetData);
-        const isEitherAssetERC721 = takerAssetProxyId === ERC721_PROXY_ID || makerAssetProxyId === ERC721_PROXY_ID;
-        const takerAssetFillAmount = isEitherAssetERC721
-            ? fillableTakerAssetAmount
-            : fillableTakerAssetAmount.div(2).floor();
+        let takerAssetFillAmount;
+        const takerAssetFillAmountScenario = fillScenario.takerAssetFillAmountScenario;
+        switch (takerAssetFillAmountScenario) {
+            case TakerAssetFillAmountScenario.Zero:
+                takerAssetFillAmount = new BigNumber(0);
+                break;
+
+            case TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount:
+                takerAssetFillAmount = fillableTakerAssetAmount;
+                break;
+
+            case TakerAssetFillAmountScenario.GreaterThanRemainingFillableTakerAssetAmount:
+                takerAssetFillAmount = fillableTakerAssetAmount.add(1);
+                break;
+
+            case TakerAssetFillAmountScenario.LessThanRemainingFillableTakerAssetAmount:
+                const takerAssetProxyId = assetProxyUtils.decodeAssetDataId(signedOrder.takerAssetData);
+                const makerAssetProxyId = assetProxyUtils.decodeAssetDataId(signedOrder.makerAssetData);
+                const isEitherAssetERC721 =
+                    takerAssetProxyId === ERC721_PROXY_ID || makerAssetProxyId === ERC721_PROXY_ID;
+                if (isEitherAssetERC721) {
+                    throw new Error(
+                        'Cannot test `TakerAssetFillAmountScenario.LessThanRemainingFillableTakerAssetAmount` together with ERC721 assets since orders involving ERC721 must always be filled exactly.',
+                    );
+                }
+                takerAssetFillAmount = fillableTakerAssetAmount.div(2).floor();
+                break;
+
+            default:
+                throw errorUtils.spawnSwitchErr('TakerAssetFillAmountScenario', takerAssetFillAmountScenario);
+        }
 
         // 5. If I fill it by X, what are the resulting balances/allowances/filled amounts exp?
         const orderValidationUtils = new OrderValidationUtils(orderFilledCancelledFetcher);
diff --git a/packages/contracts/src/utils/new_order_factory.ts b/packages/contracts/src/utils/new_order_factory.ts
index 715e38d6d..7de7be3bd 100644
--- a/packages/contracts/src/utils/new_order_factory.ts
+++ b/packages/contracts/src/utils/new_order_factory.ts
@@ -12,11 +12,15 @@ import {
     FeeRecipientAddressScenario,
     OrderAmountScenario,
     OrderScenario,
+    TakerScenario,
 } from './types';
 
 const TEN_UNITS_EIGHTEEN_DECIMALS = new BigNumber(10000000000000000000);
+const FIVE_UNITS_EIGHTEEN_DECIMALS = new BigNumber(5000000000000000000);
 const POINT_ONE_UNITS_EIGHTEEN_DECIMALS = new BigNumber(100000000000000000);
+const POINT_ZERO_FIVE_UNITS_EIGHTEEN_DECIMALS = new BigNumber(50000000000000000);
 const TEN_UNITS_FIVE_DECIMALS = new BigNumber(1000000);
+const FIVE_UNITS_FIVE_DECIMALS = new BigNumber(500000);
 const ONE_NFT_UNIT = new BigNumber(1);
 
 export class NewOrderFactory {
@@ -46,7 +50,7 @@ export class NewOrderFactory {
     }
     public generateOrder(orderScenario: OrderScenario): Order {
         const makerAddress = this._userAddresses[1];
-        const takerAddress = this._userAddresses[2];
+        let takerAddress = this._userAddresses[2];
         const erc721MakerAssetIds = this._erc721Balances[makerAddress][this._erc721Token.address];
         const erc721TakerAssetIds = this._erc721Balances[takerAddress][this._erc721Token.address];
         let feeRecipientAddress;
@@ -114,7 +118,7 @@ export class NewOrderFactory {
         }
 
         switch (orderScenario.makerAssetAmountScenario) {
-            case OrderAmountScenario.NonZero:
+            case OrderAmountScenario.Large:
                 switch (orderScenario.makerAssetDataScenario) {
                     case AssetDataScenario.ZRXFeeToken:
                     case AssetDataScenario.ERC20NonZRXEighteenDecimals:
@@ -130,6 +134,22 @@ export class NewOrderFactory {
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.makerAssetDataScenario);
                 }
                 break;
+            case OrderAmountScenario.Small:
+                switch (orderScenario.makerAssetDataScenario) {
+                    case AssetDataScenario.ZRXFeeToken:
+                    case AssetDataScenario.ERC20NonZRXEighteenDecimals:
+                        makerAssetAmount = FIVE_UNITS_EIGHTEEN_DECIMALS;
+                        break;
+                    case AssetDataScenario.ERC20FiveDecimals:
+                        makerAssetAmount = FIVE_UNITS_FIVE_DECIMALS;
+                        break;
+                    case AssetDataScenario.ERC721:
+                        makerAssetAmount = ONE_NFT_UNIT;
+                        break;
+                    default:
+                        throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.makerAssetDataScenario);
+                }
+                break;
             case OrderAmountScenario.Zero:
                 makerAssetAmount = new BigNumber(0);
                 break;
@@ -138,7 +158,7 @@ export class NewOrderFactory {
         }
 
         switch (orderScenario.takerAssetAmountScenario) {
-            case OrderAmountScenario.NonZero:
+            case OrderAmountScenario.Large:
                 switch (orderScenario.takerAssetDataScenario) {
                     case AssetDataScenario.ERC20NonZRXEighteenDecimals:
                     case AssetDataScenario.ZRXFeeToken:
@@ -154,6 +174,22 @@ export class NewOrderFactory {
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.takerAssetDataScenario);
                 }
                 break;
+            case OrderAmountScenario.Small:
+                switch (orderScenario.takerAssetDataScenario) {
+                    case AssetDataScenario.ERC20NonZRXEighteenDecimals:
+                    case AssetDataScenario.ZRXFeeToken:
+                        takerAssetAmount = FIVE_UNITS_EIGHTEEN_DECIMALS;
+                        break;
+                    case AssetDataScenario.ERC20FiveDecimals:
+                        takerAssetAmount = FIVE_UNITS_FIVE_DECIMALS;
+                        break;
+                    case AssetDataScenario.ERC721:
+                        takerAssetAmount = ONE_NFT_UNIT;
+                        break;
+                    default:
+                        throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.takerAssetDataScenario);
+                }
+                break;
             case OrderAmountScenario.Zero:
                 takerAssetAmount = new BigNumber(0);
                 break;
@@ -162,9 +198,12 @@ export class NewOrderFactory {
         }
 
         switch (orderScenario.makerFeeScenario) {
-            case OrderAmountScenario.NonZero:
+            case OrderAmountScenario.Large:
                 makerFee = POINT_ONE_UNITS_EIGHTEEN_DECIMALS;
                 break;
+            case OrderAmountScenario.Small:
+                makerFee = POINT_ZERO_FIVE_UNITS_EIGHTEEN_DECIMALS;
+                break;
             case OrderAmountScenario.Zero:
                 makerFee = new BigNumber(0);
                 break;
@@ -173,9 +212,12 @@ export class NewOrderFactory {
         }
 
         switch (orderScenario.takerFeeScenario) {
-            case OrderAmountScenario.NonZero:
+            case OrderAmountScenario.Large:
                 takerFee = POINT_ONE_UNITS_EIGHTEEN_DECIMALS;
                 break;
+            case OrderAmountScenario.Small:
+                takerFee = POINT_ZERO_FIVE_UNITS_EIGHTEEN_DECIMALS;
+                break;
             case OrderAmountScenario.Zero:
                 takerFee = new BigNumber(0);
                 break;
@@ -197,6 +239,23 @@ export class NewOrderFactory {
                 );
         }
 
+        switch (orderScenario.takerScenario) {
+            case TakerScenario.CorrectlySpecified:
+                break; // noop since takerAddress is already specified
+
+            case TakerScenario.IncorrectlySpecified:
+                const notTaker = this._userAddresses[3];
+                takerAddress = notTaker;
+                break;
+
+            case TakerScenario.Unspecified:
+                takerAddress = constants.NULL_ADDRESS;
+                break;
+
+            default:
+                throw errorUtils.spawnSwitchErr('TakerScenario', orderScenario.takerScenario);
+        }
+
         const order: Order = {
             senderAddress: constants.NULL_ADDRESS,
             makerAddress,
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 77e8d867e..273e8c2d6 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -160,7 +160,14 @@ export enum FeeRecipientAddressScenario {
 
 export enum OrderAmountScenario {
     Zero = 'ZERO',
-    NonZero = 'NON_ZERO',
+    Large = 'LARGE',
+    Small = 'SMALL',
+}
+
+export enum TakerScenario {
+    CorrectlySpecified = 'CORRECTLY_SPECFIED',
+    IncorrectlySpecified = 'INCORRECTLY_SPECFIED',
+    Unspecified = 'UNSPECIFIED',
 }
 
 export enum ExpirationTimeSecondsScenario {
@@ -175,7 +182,15 @@ export enum AssetDataScenario {
     ERC20NonZRXEighteenDecimals = 'ERC20_NON_ZRX_EIGHTEEN_DECIMALS',
 }
 
+export enum TakerAssetFillAmountScenario {
+    Zero = 'ZERO',
+    GreaterThanRemainingFillableTakerAssetAmount = 'GREATER_THAN_REMAINING_FILLABLE_TAKER_ASSET_AMOUNT',
+    LessThanRemainingFillableTakerAssetAmount = 'GREATER_THAN_REMAINING_FILLABLE_TAKER_ASSET_AMOUNT',
+    ExactlyRemainingFillableTakerAssetAmount = 'GREATER_THAN_REMAINING_FILLABLE_TAKER_ASSET_AMOUNT',
+}
+
 export interface OrderScenario {
+    takerScenario: TakerScenario;
     feeRecipientScenario: FeeRecipientAddressScenario;
     makerAssetAmountScenario: OrderAmountScenario;
     takerAssetAmountScenario: OrderAmountScenario;
@@ -185,3 +200,8 @@ export interface OrderScenario {
     makerAssetDataScenario: AssetDataScenario;
     takerAssetDataScenario: AssetDataScenario;
 }
+
+export interface FillScenario {
+    orderScenario: OrderScenario;
+    takerAssetFillAmountScenario: TakerAssetFillAmountScenario;
+}
diff --git a/packages/contracts/test/exchange/combinatorial_tests.ts b/packages/contracts/test/exchange/combinatorial_tests.ts
index 2870a22ed..fd9193d69 100644
--- a/packages/contracts/test/exchange/combinatorial_tests.ts
+++ b/packages/contracts/test/exchange/combinatorial_tests.ts
@@ -5,7 +5,7 @@ import { chaiSetup } from '../../src/utils/chai_setup';
 import { CoreCombinatorialUtils, coreCombinatorialUtilsFactoryAsync } from '../../src/utils/core_combinatorial_utils';
 import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
 
-import { OrderScenario } from '../../src/utils/types';
+import { FillScenario, OrderScenario, TakerAssetFillAmountScenario } from '../../src/utils/types';
 
 chaiSetup.configure();
 const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
@@ -26,8 +26,9 @@ describe('Combinatorial tests', () => {
     afterEach(async () => {
         await blockchainLifecycle.revertAsync();
     });
-    const test = (orderScenarios: OrderScenario[]) => {
-        _.forEach(orderScenarios, orderScenario => {
+    const test = (fillScenarios: FillScenario[]) => {
+        _.forEach(fillScenarios, fillScenario => {
+            const orderScenario = fillScenario.orderScenario;
             const description = `Combinatorial OrderFill: ${orderScenario.feeRecipientScenario} ${
                 orderScenario.makerAssetAmountScenario
             } ${orderScenario.takerAssetAmountScenario} ${orderScenario.makerFeeScenario} ${
@@ -36,13 +37,18 @@ describe('Combinatorial tests', () => {
                 orderScenario.takerAssetDataScenario
             }`;
             it(description, async () => {
-                const order = coreCombinatorialUtils.orderFactory.generateOrder(orderScenario);
-                await coreCombinatorialUtils.testFillOrderScenarioAsync(order, provider);
+                await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario);
             });
         });
     };
 
     const allOrderScenarios = CoreCombinatorialUtils.generateOrderCombinations();
+    const allFillScenarios = _.map(allOrderScenarios, orderScenario => {
+        return {
+            orderScenario,
+            takerAssetFillAmountScenario: TakerAssetFillAmountScenario.LessThanRemainingFillableTakerAssetAmount,
+        };
+    });
 
-    describe.only('Fills orders', () => test(allOrderScenarios));
+    describe('Fills orders', () => test(allFillScenarios));
 });