From 98405a39dbb870c1eb9f562afca4539602917c67 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 14 Jun 2018 10:40:17 +0200 Subject: Add ability to specify takerAssetFillAmount and taker scenarios as part of a FillScenario --- .../src/utils/core_combinatorial_utils.ts | 79 +++++++++++++++------- packages/contracts/src/utils/new_order_factory.ts | 69 +++++++++++++++++-- packages/contracts/src/utils/types.ts | 22 +++++- .../contracts/test/exchange/combinatorial_tests.ts | 18 +++-- 4 files changed, 153 insertions(+), 35 deletions(-) (limited to 'packages') 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 { + public async testFillOrderScenarioAsync(provider: Provider, fillScenario: FillScenario): Promise { + // 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)); }); -- cgit v1.2.3