From 38de1c9a315aec7f43b08d372dd2360c09bd0acd Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 13 Jun 2018 13:16:22 +0200 Subject: Remove comment since we have added the tests --- packages/order-utils/src/order_validation_utils.ts | 5 ----- 1 file changed, 5 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index 3a6704f26..ab471b455 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -14,11 +14,6 @@ import { utils } from './utils'; export class OrderValidationUtils { private _exchangeContract: ExchangeContract; - // TODO: Write some tests for the function - // const numerator = new BigNumber(20); - // const denominator = new BigNumber(999); - // const target = new BigNumber(50); - // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% public static isRoundingError(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { // Solidity's mulmod() in JS // Source: https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions -- cgit v1.2.3 From 946e6c16442ce434e160fa47a87cd705c5274038 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 13 Jun 2018 13:20:13 +0200 Subject: Remaining refactors of order-utils components for V2 --- .../abstract_order_filled_cancelled_fetcher.ts | 2 +- packages/order-utils/src/index.ts | 1 + packages/order-utils/src/order_validation_utils.ts | 62 +++++++++++----------- .../balance_and_proxy_allowance_lazy_store.ts | 4 +- .../test/exchange_transfer_simulator_test.ts | 25 ++++----- 5 files changed, 48 insertions(+), 46 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts index ec398a11e..865ea4e43 100644 --- a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts +++ b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts @@ -3,5 +3,5 @@ import { BigNumber } from '@0xproject/utils'; export abstract class AbstractOrderFilledCancelledFetcher { public abstract async getFilledTakerAmountAsync(orderHash: string): Promise; public abstract async isOrderCancelledAsync(orderHash: string): Promise; - public abstract getZRXTokenAddress(): string; + public abstract getZRXAssetData(): string; } diff --git a/packages/order-utils/src/index.ts b/packages/order-utils/src/index.ts index f9b37df82..592b9b7f6 100644 --- a/packages/order-utils/src/index.ts +++ b/packages/order-utils/src/index.ts @@ -16,6 +16,7 @@ export { generatePseudoRandomSalt } from './salt'; export { OrderError, MessagePrefixType, MessagePrefixOpts } from './types'; export { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher'; export { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher'; +export { BalanceAndProxyAllowanceLazyStore } from './store/balance_and_proxy_allowance_lazy_store'; export { RemainingFillableCalculator } from './remaining_fillable_calculator'; export { OrderStateUtils } from './order_state_utils'; export { assetProxyUtils } from './asset_proxy_utils'; diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index ab471b455..fb96502f2 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -5,15 +5,15 @@ import * as _ from 'lodash'; import { OrderError, TradeSide, TransferType } from './types'; +import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher'; import { constants } from './constants'; import { ExchangeTransferSimulator } from './exchange_transfer_simulator'; -import { ExchangeContract } from './generated_contract_wrappers/exchange'; import { orderHashUtils } from './order_hash'; import { isValidSignatureAsync } from './signature_utils'; import { utils } from './utils'; export class OrderValidationUtils { - private _exchangeContract: ExchangeContract; + private _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher; public static isRoundingError(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { // Solidity's mulmod() in JS // Source: https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions @@ -50,12 +50,12 @@ export class OrderValidationUtils { public static async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( exchangeTradeEmulator: ExchangeTransferSimulator, signedOrder: SignedOrder, - fillTakerTokenAmount: BigNumber, + fillTakerAssetAmount: BigNumber, senderAddress: string, - zrxTokenAddress: string, + zrxAssetData: string, ): Promise { const fillMakerTokenAmount = OrderValidationUtils._getPartialAmount( - fillTakerTokenAmount, + fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, ); @@ -71,17 +71,17 @@ export class OrderValidationUtils { signedOrder.takerAssetData, senderAddress, signedOrder.makerAddress, - fillTakerTokenAmount, + fillTakerAssetAmount, TradeSide.Taker, TransferType.Trade, ); const makerFeeAmount = OrderValidationUtils._getPartialAmount( - fillTakerTokenAmount, + fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerFee, ); await exchangeTradeEmulator.transferFromAsync( - zrxTokenAddress, + zrxAssetData, signedOrder.makerAddress, signedOrder.feeRecipientAddress, makerFeeAmount, @@ -89,12 +89,12 @@ export class OrderValidationUtils { TransferType.Fee, ); const takerFeeAmount = OrderValidationUtils._getPartialAmount( - fillTakerTokenAmount, + fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.takerFee, ); await exchangeTradeEmulator.transferFromAsync( - zrxTokenAddress, + zrxAssetData, senderAddress, signedOrder.feeRecipientAddress, takerFeeAmount, @@ -123,43 +123,43 @@ export class OrderValidationUtils { .round(0); return fillMakerTokenAmount; } - constructor(exchangeContract: ExchangeContract) { - this._exchangeContract = exchangeContract; + constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { + this._orderFilledCancelledFetcher = orderFilledCancelledFetcher; } public async validateOrderFillableOrThrowAsync( exchangeTradeEmulator: ExchangeTransferSimulator, signedOrder: SignedOrder, - zrxTokenAddress: string, + zrxAssetData: string, expectedFillTakerTokenAmount?: BigNumber, ): Promise { const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const filledTakerTokenAmount = await this._exchangeContract.filled.callAsync(orderHash); + const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow( signedOrder.takerAssetAmount, filledTakerTokenAmount, ); OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); - let fillTakerTokenAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); + let fillTakerAssetAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); if (!_.isUndefined(expectedFillTakerTokenAmount)) { - fillTakerTokenAmount = expectedFillTakerTokenAmount; + fillTakerAssetAmount = expectedFillTakerTokenAmount; } await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( exchangeTradeEmulator, signedOrder, - fillTakerTokenAmount, + fillTakerAssetAmount, signedOrder.takerAddress, - zrxTokenAddress, + zrxAssetData, ); } public async validateFillOrderThrowIfInvalidAsync( exchangeTradeEmulator: ExchangeTransferSimulator, provider: Provider, signedOrder: SignedOrder, - fillTakerTokenAmount: BigNumber, + fillTakerAssetAmount: BigNumber, takerAddress: string, - zrxTokenAddress: string, + zrxAssetData: string, ): Promise { - if (fillTakerTokenAmount.eq(0)) { + if (fillTakerAssetAmount.eq(0)) { throw new Error(ExchangeContractErrs.OrderFillAmountZero); } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); @@ -172,7 +172,7 @@ export class OrderValidationUtils { if (!isValid) { throw new Error(OrderError.InvalidSignature); } - const filledTakerTokenAmount = await this._exchangeContract.filled.callAsync(orderHash); + const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow( signedOrder.takerAssetAmount, filledTakerTokenAmount, @@ -182,19 +182,19 @@ export class OrderValidationUtils { } OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); const remainingTakerTokenAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); - const desiredFillTakerTokenAmount = remainingTakerTokenAmount.lessThan(fillTakerTokenAmount) + const desiredFillTakerTokenAmount = remainingTakerTokenAmount.lessThan(fillTakerAssetAmount) ? remainingTakerTokenAmount - : fillTakerTokenAmount; + : fillTakerAssetAmount; await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( exchangeTradeEmulator, signedOrder, desiredFillTakerTokenAmount, takerAddress, - zrxTokenAddress, + zrxAssetData, ); const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingError( - filledTakerTokenAmount, + desiredFillTakerTokenAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, ); @@ -207,19 +207,19 @@ export class OrderValidationUtils { exchangeTradeEmulator: ExchangeTransferSimulator, provider: Provider, signedOrder: SignedOrder, - fillTakerTokenAmount: BigNumber, + fillTakerAssetAmount: BigNumber, takerAddress: string, - zrxTokenAddress: string, + zrxAssetData: string, ): Promise { const filledTakerTokenAmount = await this.validateFillOrderThrowIfInvalidAsync( exchangeTradeEmulator, provider, signedOrder, - fillTakerTokenAmount, + fillTakerAssetAmount, takerAddress, - zrxTokenAddress, + zrxAssetData, ); - if (filledTakerTokenAmount !== fillTakerTokenAmount) { + if (filledTakerTokenAmount !== fillTakerAssetAmount) { throw new Error(ExchangeContractErrs.InsufficientRemainingFillAmount); } } diff --git a/packages/order-utils/src/store/balance_and_proxy_allowance_lazy_store.ts b/packages/order-utils/src/store/balance_and_proxy_allowance_lazy_store.ts index 08d50b924..e7352119d 100644 --- a/packages/order-utils/src/store/balance_and_proxy_allowance_lazy_store.ts +++ b/packages/order-utils/src/store/balance_and_proxy_allowance_lazy_store.ts @@ -19,8 +19,8 @@ export class BalanceAndProxyAllowanceLazyStore implements AbstractBalanceAndProx [userAddress: string]: BigNumber; }; }; - constructor(token: AbstractBalanceAndProxyAllowanceFetcher) { - this._balanceAndProxyAllowanceFetcher = token; + constructor(balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher) { + this._balanceAndProxyAllowanceFetcher = balanceAndProxyAllowanceFetcher; this._balance = {}; this._proxyAllowance = {}; } diff --git a/packages/order-utils/test/exchange_transfer_simulator_test.ts b/packages/order-utils/test/exchange_transfer_simulator_test.ts index eeae42698..c8fc9fbf3 100644 --- a/packages/order-utils/test/exchange_transfer_simulator_test.ts +++ b/packages/order-utils/test/exchange_transfer_simulator_test.ts @@ -5,6 +5,7 @@ import * as chai from 'chai'; import 'make-promises-safe'; import { artifacts } from '../src/artifacts'; +import { assetProxyUtils } from '../src/asset_proxy_utils'; import { constants } from '../src/constants'; import { ExchangeTransferSimulator } from '../src/exchange_transfer_simulator'; import { DummyERC20TokenContract } from '../src/generated_contract_wrappers/dummy_e_r_c20_token'; @@ -28,7 +29,7 @@ describe('ExchangeTransferSimulator', async () => { let coinbase: string; let sender: string; let recipient: string; - let exampleTokenAddress: string; + let exampleAssetData: string; let exchangeTransferSimulator: ExchangeTransferSimulator; let txHash: string; let erc20ProxyAddress: string; @@ -66,7 +67,7 @@ describe('ExchangeTransferSimulator', async () => { totalSupply, ); - exampleTokenAddress = dummyERC20Token.address; + exampleAssetData = assetProxyUtils.encodeERC20AssetData(dummyERC20Token.address); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -92,7 +93,7 @@ describe('ExchangeTransferSimulator', async () => { it("throws if the user doesn't have enough allowance", async () => { return expect( exchangeTransferSimulator.transferFromAsync( - exampleTokenAddress, + exampleAssetData, sender, recipient, transferAmount, @@ -108,7 +109,7 @@ describe('ExchangeTransferSimulator', async () => { await web3Wrapper.awaitTransactionSuccessAsync(txHash); return expect( exchangeTransferSimulator.transferFromAsync( - exampleTokenAddress, + exampleAssetData, sender, recipient, transferAmount, @@ -129,7 +130,7 @@ describe('ExchangeTransferSimulator', async () => { await web3Wrapper.awaitTransactionSuccessAsync(txHash); await exchangeTransferSimulator.transferFromAsync( - exampleTokenAddress, + exampleAssetData, sender, recipient, transferAmount, @@ -137,9 +138,9 @@ describe('ExchangeTransferSimulator', async () => { TransferType.Trade, ); const store = (exchangeTransferSimulator as any)._store; - const senderBalance = await store.getBalanceAsync(exampleTokenAddress, sender); - const recipientBalance = await store.getBalanceAsync(exampleTokenAddress, recipient); - const senderProxyAllowance = await store.getProxyAllowanceAsync(exampleTokenAddress, sender); + const senderBalance = await store.getBalanceAsync(exampleAssetData, sender); + const recipientBalance = await store.getBalanceAsync(exampleAssetData, recipient); + const senderProxyAllowance = await store.getProxyAllowanceAsync(exampleAssetData, sender); expect(senderBalance).to.be.bignumber.equal(0); expect(recipientBalance).to.be.bignumber.equal(transferAmount); expect(senderProxyAllowance).to.be.bignumber.equal(0); @@ -158,7 +159,7 @@ describe('ExchangeTransferSimulator', async () => { ); await web3Wrapper.awaitTransactionSuccessAsync(txHash); await exchangeTransferSimulator.transferFromAsync( - exampleTokenAddress, + exampleAssetData, sender, recipient, transferAmount, @@ -166,9 +167,9 @@ describe('ExchangeTransferSimulator', async () => { TransferType.Trade, ); const store = (exchangeTransferSimulator as any)._store; - const senderBalance = await store.getBalanceAsync(exampleTokenAddress, sender); - const recipientBalance = await store.getBalanceAsync(exampleTokenAddress, recipient); - const senderProxyAllowance = await store.getProxyAllowanceAsync(exampleTokenAddress, sender); + const senderBalance = await store.getBalanceAsync(exampleAssetData, sender); + const recipientBalance = await store.getBalanceAsync(exampleAssetData, recipient); + const senderProxyAllowance = await store.getProxyAllowanceAsync(exampleAssetData, sender); expect(senderBalance).to.be.bignumber.equal(0); expect(recipientBalance).to.be.bignumber.equal(transferAmount); expect(senderProxyAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); -- cgit v1.2.3 From 61243b418e4d962cd8d8a1d7a49f04510b3c1c7f Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 13 Jun 2018 16:09:04 +0200 Subject: Implement initial set of orderFill combinatorial tests --- .../order-utils/src/exchange_transfer_simulator.ts | 10 +- packages/order-utils/src/order_state_utils.ts | 262 ++++++++++++++++----- 2 files changed, 209 insertions(+), 63 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/exchange_transfer_simulator.ts b/packages/order-utils/src/exchange_transfer_simulator.ts index 32d53d6a2..cac4af243 100644 --- a/packages/order-utils/src/exchange_transfer_simulator.ts +++ b/packages/order-utils/src/exchange_transfer_simulator.ts @@ -1,7 +1,8 @@ -import { ExchangeContractErrs } from '@0xproject/types'; +import { AssetProxyId, ExchangeContractErrs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { AbstractBalanceAndProxyAllowanceLazyStore } from './abstract/abstract_balance_and_proxy_allowance_lazy_store'; +import { assetProxyUtils } from './asset_proxy_utils'; import { constants } from './constants'; import { TradeSide, TransferType } from './types'; @@ -89,8 +90,13 @@ export class ExchangeTransferSimulator { userAddress: string, amountInBaseUnits: BigNumber, ): Promise { + const assetProxyId = assetProxyUtils.decodeAssetDataId(assetData); + const isERC721Asset = assetProxyId === AssetProxyId.ERC721; const proxyAllowance = await this._store.getProxyAllowanceAsync(assetData, userAddress); - if (!proxyAllowance.eq(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS)) { + // HACK: This code assumes that all tokens with an UNLIMITED_ALLOWANCE_IN_BASE_UNITS set, + // are UnlimitedAllowanceTokens. This is however not true, it just so happens that all + // DummyERC20Tokens we use in tests are. + if (!proxyAllowance.eq(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS) && !isERC721Asset) { this._store.setProxyAllowance(assetData, userAddress, proxyAllowance.minus(amountInBaseUnits)); } } diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index 40f235da7..dbb65de59 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -7,43 +7,85 @@ import { SignedOrder, } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import * as _ from 'lodash'; import { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher'; import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher'; -import { assetProxyUtils } from './asset_proxy_utils'; +import { constants } from './constants'; import { orderHashUtils } from './order_hash'; import { RemainingFillableCalculator } from './remaining_fillable_calculator'; +interface SidedOrderRelevantState { + isMakerSide: boolean; + traderBalance: BigNumber; + traderProxyAllowance: BigNumber; + traderFeeBalance: BigNumber; + traderFeeProxyAllowance: BigNumber; + filledTakerAssetAmount: BigNumber; + remainingFillableAssetAmount: BigNumber; +} + const ACCEPTABLE_RELATIVE_ROUNDING_ERROR = 0.0001; export class OrderStateUtils { private _balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher; private _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher; - private static _validateIfOrderIsValid(signedOrder: SignedOrder, orderRelevantState: OrderRelevantState): void { - const availableTakerAssetAmount = signedOrder.takerAssetAmount.minus(orderRelevantState.filledTakerAssetAmount); + private static _validateIfOrderIsValid( + signedOrder: SignedOrder, + sidedOrderRelevantState: SidedOrderRelevantState, + ): void { + const isMakerSide = sidedOrderRelevantState.isMakerSide; + const availableTakerAssetAmount = signedOrder.takerAssetAmount.minus( + sidedOrderRelevantState.filledTakerAssetAmount, + ); if (availableTakerAssetAmount.eq(0)) { throw new Error(ExchangeContractErrs.OrderRemainingFillAmountZero); } - if (orderRelevantState.makerBalance.eq(0)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + if (sidedOrderRelevantState.traderBalance.eq(0)) { + throw new Error( + isMakerSide + ? ExchangeContractErrs.InsufficientMakerBalance + : ExchangeContractErrs.InsufficientTakerBalance, + ); } - if (orderRelevantState.makerProxyAllowance.eq(0)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + if (sidedOrderRelevantState.traderProxyAllowance.eq(0)) { + throw new Error( + isMakerSide + ? ExchangeContractErrs.InsufficientMakerAllowance + : ExchangeContractErrs.InsufficientTakerAllowance, + ); } if (!signedOrder.makerFee.eq(0)) { - if (orderRelevantState.makerFeeBalance.eq(0)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); + if (sidedOrderRelevantState.traderFeeBalance.eq(0)) { + throw new Error( + isMakerSide + ? ExchangeContractErrs.InsufficientMakerFeeBalance + : ExchangeContractErrs.InsufficientTakerFeeBalance, + ); } - if (orderRelevantState.makerFeeProxyAllowance.eq(0)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); + if (sidedOrderRelevantState.traderFeeProxyAllowance.eq(0)) { + throw new Error( + isMakerSide + ? ExchangeContractErrs.InsufficientMakerFeeAllowance + : ExchangeContractErrs.InsufficientTakerFeeAllowance, + ); } } - const minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.takerAssetAmount - .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR) - .dividedBy(signedOrder.makerAssetAmount); + + let minFillableTakerAssetAmountWithinNoRoundingErrorRange; + if (isMakerSide) { + minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.takerAssetAmount + .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR) + .dividedBy(signedOrder.makerAssetAmount); + } else { + minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.makerAssetAmount + .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR) + .dividedBy(signedOrder.takerAssetAmount); + } + if ( - orderRelevantState.remainingFillableTakerAssetAmount.lessThan( + sidedOrderRelevantState.remainingFillableAssetAmount.lessThan( minFillableTakerAssetAmountWithinNoRoundingErrorRange, ) ) { @@ -57,11 +99,20 @@ export class OrderStateUtils { this._balanceAndProxyAllowanceFetcher = balanceAndProxyAllowanceFetcher; this._orderFilledCancelledFetcher = orderFilledCancelledFetcher; } - public async getOrderStateAsync(signedOrder: SignedOrder): Promise { - const orderRelevantState = await this.getOrderRelevantStateAsync(signedOrder); + public async getOpenOrderStateAsync(signedOrder: SignedOrder): Promise { + const orderRelevantState = await this.getOpenOrderRelevantStateAsync(signedOrder); const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const sidedOrderRelevantState = { + isMakerSide: true, + traderBalance: orderRelevantState.makerBalance, + traderProxyAllowance: orderRelevantState.makerProxyAllowance, + traderFeeBalance: orderRelevantState.makerFeeBalance, + traderFeeProxyAllowance: orderRelevantState.makerFeeProxyAllowance, + filledTakerAssetAmount: orderRelevantState.filledTakerAssetAmount, + remainingFillableAssetAmount: orderRelevantState.remainingFillableMakerAssetAmount, + }; try { - OrderStateUtils._validateIfOrderIsValid(signedOrder, orderRelevantState); + OrderStateUtils._validateIfOrderIsValid(signedOrder, sidedOrderRelevantState); const orderState: OrderStateValid = { isValid: true, orderHash, @@ -77,63 +128,152 @@ export class OrderStateUtils { return orderState; } } - public async getOrderRelevantStateAsync(signedOrder: SignedOrder): Promise { - const zrxTokenAddress = this._orderFilledCancelledFetcher.getZRXTokenAddress(); - const makerProxyData = assetProxyUtils.decodeERC20AssetData(signedOrder.makerAssetData); - const makerAssetAddress = makerProxyData.tokenAddress; - const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const makerBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( - makerAssetAddress, - signedOrder.makerAddress, + public async getOpenOrderRelevantStateAsync(signedOrder: SignedOrder): Promise { + const isMaker = true; + const sidedOrderRelevantState = await this._getSidedOrderRelevantStateAsync( + isMaker, + signedOrder, + signedOrder.takerAddress, + ); + const remainingFillableTakerAssetAmount = sidedOrderRelevantState.remainingFillableAssetAmount + .times(signedOrder.takerAssetAmount) + .dividedToIntegerBy(signedOrder.makerAssetAmount); + + const orderRelevantState = { + makerBalance: sidedOrderRelevantState.traderBalance, + makerProxyAllowance: sidedOrderRelevantState.traderProxyAllowance, + makerFeeBalance: sidedOrderRelevantState.traderFeeBalance, + makerFeeProxyAllowance: sidedOrderRelevantState.traderFeeProxyAllowance, + filledTakerAssetAmount: sidedOrderRelevantState.filledTakerAssetAmount, + remainingFillableMakerAssetAmount: sidedOrderRelevantState.remainingFillableAssetAmount, + remainingFillableTakerAssetAmount, + }; + return orderRelevantState; + } + public async getMaxFillableTakerAssetAmountAsync( + signedOrder: SignedOrder, + takerAddress: string, + ): Promise { + // Get max fillable amount for an order, considering the makers ability to fill + let isMaker = true; + const orderRelevantMakerState = await this._getSidedOrderRelevantStateAsync( + isMaker, + signedOrder, + signedOrder.takerAddress, + ); + const remainingFillableTakerAssetAmountGivenMakersStatus = orderRelevantMakerState.remainingFillableAssetAmount; + + // Get max fillable amount for an order, considering the takers ability to fill + isMaker = false; + const orderRelevantTakerState = await this._getSidedOrderRelevantStateAsync(isMaker, signedOrder, takerAddress); + const remainingFillableTakerAssetAmountGivenTakersStatus = orderRelevantTakerState.remainingFillableAssetAmount; + + // The min of these two in the actualy max fillable by either party + const fillableTakerAssetAmount = BigNumber.min([ + remainingFillableTakerAssetAmountGivenMakersStatus, + remainingFillableTakerAssetAmountGivenTakersStatus, + ]); + + return fillableTakerAssetAmount; + } + public async getMaxFillableTakerAssetAmountForFailingOrderAsync( + signedOrder: SignedOrder, + takerAddress: string, + ): Promise { + // Get min of taker balance & allowance + const takerAssetBalanceOfTaker = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( + signedOrder.takerAssetData, + takerAddress, ); - const makerProxyAllowance = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( - makerAssetAddress, - signedOrder.makerAddress, + const takerAssetAllowanceOfTaker = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( + signedOrder.takerAssetData, + takerAddress, ); - const makerFeeBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( - zrxTokenAddress, - signedOrder.makerAddress, + const minTakerAssetAmount = BigNumber.min([takerAssetBalanceOfTaker, takerAssetAllowanceOfTaker]); + + // get remainingFillAmount + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const filledTakerAssetAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); + const remainingFillTakerAssetAmount = signedOrder.takerAssetAmount.minus(filledTakerAssetAmount); + + if (minTakerAssetAmount.gte(remainingFillTakerAssetAmount)) { + return remainingFillTakerAssetAmount; + } else { + return minTakerAssetAmount; + } + } + private async _getSidedOrderRelevantStateAsync( + isMakerSide: boolean, + signedOrder: SignedOrder, + takerAddress: string, + ): Promise { + let traderAddress; + let assetData; + let assetAmount; + let feeAmount; + if (isMakerSide) { + traderAddress = signedOrder.makerAddress; + assetData = signedOrder.makerAssetData; + assetAmount = signedOrder.makerAssetAmount; + feeAmount = signedOrder.makerFee; + } else { + traderAddress = takerAddress; + assetData = signedOrder.takerAssetData; + assetAmount = signedOrder.takerAssetAmount; + feeAmount = signedOrder.takerFee; + } + const zrxAssetData = this._orderFilledCancelledFetcher.getZRXAssetData(); + const isAssetZRX = assetData === zrxAssetData; + + const traderBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync(assetData, traderAddress); + const traderProxyAllowance = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( + assetData, + traderAddress, + ); + const traderFeeBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( + zrxAssetData, + traderAddress, ); - const makerFeeProxyAllowance = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( - zrxTokenAddress, - signedOrder.makerAddress, + const traderFeeProxyAllowance = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( + zrxAssetData, + traderAddress, ); + + const transferrableTraderAssetAmount = BigNumber.min([traderProxyAllowance, traderBalance]); + const transferrableFeeAssetAmount = BigNumber.min([traderFeeProxyAllowance, traderFeeBalance]); + + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const filledTakerAssetAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); - const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); const totalMakerAssetAmount = signedOrder.makerAssetAmount; const totalTakerAssetAmount = signedOrder.takerAssetAmount; + const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); const remainingTakerAssetAmount = isOrderCancelled ? new BigNumber(0) : totalTakerAssetAmount.minus(filledTakerAssetAmount); - const remainingMakerAssetAmount = remainingTakerAssetAmount - .times(totalMakerAssetAmount) - .dividedToIntegerBy(totalTakerAssetAmount); - const transferrableMakerAssetAmount = BigNumber.min([makerProxyAllowance, makerBalance]); - const transferrableFeeAssetAmount = BigNumber.min([makerFeeProxyAllowance, makerFeeBalance]); - - const zrxAssetData = assetProxyUtils.encodeERC20AssetData(zrxTokenAddress); - const isMakerAssetZRX = signedOrder.makerAssetData === zrxAssetData; + const remainingMakerAssetAmount = remainingTakerAssetAmount.eq(0) + ? new BigNumber(0) + : remainingTakerAssetAmount.times(totalMakerAssetAmount).dividedToIntegerBy(totalTakerAssetAmount); + const remainingAssetAmount = isMakerSide ? remainingMakerAssetAmount : remainingTakerAssetAmount; + const remainingFillableCalculator = new RemainingFillableCalculator( - signedOrder.makerFee, - signedOrder.makerAssetAmount, - isMakerAssetZRX, - transferrableMakerAssetAmount, + feeAmount, + assetAmount, + isAssetZRX, + transferrableTraderAssetAmount, transferrableFeeAssetAmount, - remainingMakerAssetAmount, + remainingAssetAmount, ); - const remainingFillableMakerAssetAmount = remainingFillableCalculator.computeRemainingFillable(); - const remainingFillableTakerAssetAmount = remainingFillableMakerAssetAmount - .times(signedOrder.takerAssetAmount) - .dividedToIntegerBy(signedOrder.makerAssetAmount); - const orderRelevantState = { - makerBalance, - makerProxyAllowance, - makerFeeBalance, - makerFeeProxyAllowance, + const remainingFillableAssetAmount = remainingFillableCalculator.computeRemainingFillable(); + + const sidedOrderRelevantState = { + isMakerSide, + traderBalance, + traderProxyAllowance, + traderFeeBalance, + traderFeeProxyAllowance, filledTakerAssetAmount, - remainingFillableMakerAssetAmount, - remainingFillableTakerAssetAmount, + remainingFillableAssetAmount, }; - return orderRelevantState; + return sidedOrderRelevantState; } } -- cgit v1.2.3 From 6239686afaf733707ad0c4687c535969c2d53205 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 13 Jun 2018 18:36:30 +0200 Subject: Fix divide by zero bug --- packages/order-utils/src/remaining_fillable_calculator.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/remaining_fillable_calculator.ts b/packages/order-utils/src/remaining_fillable_calculator.ts index bc146e931..29e19e5ab 100644 --- a/packages/order-utils/src/remaining_fillable_calculator.ts +++ b/packages/order-utils/src/remaining_fillable_calculator.ts @@ -23,9 +23,9 @@ export class RemainingFillableCalculator { this._transferrableAssetAmount = transferrableAssetAmount; this._transferrableFeeAmount = transferrableFeeAmount; this._remainingOrderAssetAmount = remainingOrderAssetAmount; - this._remainingOrderFeeAmount = remainingOrderAssetAmount - .times(this._orderFee) - .dividedToIntegerBy(this._orderAssetAmount); + this._remainingOrderFeeAmount = orderAssetAmount.eq(0) + ? new BigNumber(0) + : remainingOrderAssetAmount.times(orderFee).dividedToIntegerBy(orderAssetAmount); } public computeRemainingFillable(): BigNumber { if (this._hasSufficientFundsForFeeAndTransferAmount()) { -- cgit v1.2.3 From eea86757d5cdd63de4b0b06f7dd76fc2c2ddbd36 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 15 Jun 2018 00:00:02 +0200 Subject: - Refactor assetWrapper to contain more of the normalizing logic instead of erc20Wrapper and erc721Wrapper - Add burn method to DummyERC721Token - Add additional methods to assetWrapper to set balance/allowances on ERC20 and ERC721 tokens - Use approve instead of approveAll for ERC721 tokens --- packages/order-utils/src/exchange_transfer_simulator.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/exchange_transfer_simulator.ts b/packages/order-utils/src/exchange_transfer_simulator.ts index cac4af243..a70259fcd 100644 --- a/packages/order-utils/src/exchange_transfer_simulator.ts +++ b/packages/order-utils/src/exchange_transfer_simulator.ts @@ -91,12 +91,11 @@ export class ExchangeTransferSimulator { amountInBaseUnits: BigNumber, ): Promise { const assetProxyId = assetProxyUtils.decodeAssetDataId(assetData); - const isERC721Asset = assetProxyId === AssetProxyId.ERC721; const proxyAllowance = await this._store.getProxyAllowanceAsync(assetData, userAddress); // HACK: This code assumes that all tokens with an UNLIMITED_ALLOWANCE_IN_BASE_UNITS set, // are UnlimitedAllowanceTokens. This is however not true, it just so happens that all // DummyERC20Tokens we use in tests are. - if (!proxyAllowance.eq(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS) && !isERC721Asset) { + if (!proxyAllowance.eq(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS)) { this._store.setProxyAllowance(assetData, userAddress, proxyAllowance.minus(amountInBaseUnits)); } } -- cgit v1.2.3 From 247f8c8557a501ea5081615d9b031c9e4b7aeec7 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 20 Jun 2018 19:13:30 +0200 Subject: Fix orderValidationUtils where was confusing a makerAssetAmount as a takerAssetAmount --- packages/order-utils/src/order_state_utils.ts | 9 ++++++++- packages/order-utils/src/order_validation_utils.ts | 13 +++---------- packages/order-utils/src/utils.ts | 7 +++++++ 3 files changed, 18 insertions(+), 11 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index dbb65de59..3752fdadb 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -14,6 +14,7 @@ import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_f import { constants } from './constants'; import { orderHashUtils } from './order_hash'; import { RemainingFillableCalculator } from './remaining_fillable_calculator'; +import { utils } from './utils'; interface SidedOrderRelevantState { isMakerSide: boolean; @@ -161,7 +162,13 @@ export class OrderStateUtils { signedOrder, signedOrder.takerAddress, ); - const remainingFillableTakerAssetAmountGivenMakersStatus = orderRelevantMakerState.remainingFillableAssetAmount; + const remainingFillableTakerAssetAmountGivenMakersStatus = signedOrder.makerAssetAmount.eq(0) + ? new BigNumber(0) + : utils.getPartialAmount( + orderRelevantMakerState.remainingFillableAssetAmount, + signedOrder.makerAssetAmount, + signedOrder.takerAssetAmount, + ); // Get max fillable amount for an order, considering the takers ability to fill isMaker = false; diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index fb96502f2..da5b37456 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -54,7 +54,7 @@ export class OrderValidationUtils { senderAddress: string, zrxAssetData: string, ): Promise { - const fillMakerTokenAmount = OrderValidationUtils._getPartialAmount( + const fillMakerTokenAmount = utils.getPartialAmount( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, @@ -75,7 +75,7 @@ export class OrderValidationUtils { TradeSide.Taker, TransferType.Trade, ); - const makerFeeAmount = OrderValidationUtils._getPartialAmount( + const makerFeeAmount = utils.getPartialAmount( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerFee, @@ -88,7 +88,7 @@ export class OrderValidationUtils { TradeSide.Maker, TransferType.Fee, ); - const takerFeeAmount = OrderValidationUtils._getPartialAmount( + const takerFeeAmount = utils.getPartialAmount( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.takerFee, @@ -116,13 +116,6 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.OrderFillExpired); } } - private static _getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { - const fillMakerTokenAmount = numerator - .mul(target) - .div(denominator) - .round(0); - return fillMakerTokenAmount; - } constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { this._orderFilledCancelledFetcher = orderFilledCancelledFetcher; } diff --git a/packages/order-utils/src/utils.ts b/packages/order-utils/src/utils.ts index 6149316f6..7aaaf0609 100644 --- a/packages/order-utils/src/utils.ts +++ b/packages/order-utils/src/utils.ts @@ -12,4 +12,11 @@ export const utils = { const milisecondsInSecond = 1000; return new BigNumber(Date.now() / milisecondsInSecond).round(); }, + getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + const fillMakerTokenAmount = numerator + .mul(target) + .div(denominator) + .round(0); + return fillMakerTokenAmount; + }, }; -- cgit v1.2.3 From ad67a6add54cc31f4b888dbbfe70fed75b743745 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 13:51:00 +0200 Subject: Check revert reasons in declarative and combinatorial tests --- packages/order-utils/src/order_validation_utils.ts | 113 +++++++++++---------- 1 file changed, 59 insertions(+), 54 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index da5b37456..d2449b183 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -1,4 +1,5 @@ -import { ExchangeContractErrs, Order, SignedOrder } from '@0xproject/types'; +import { Order, SignedOrder } from '@0xproject/types'; +import { RevertReasons } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; @@ -54,66 +55,70 @@ export class OrderValidationUtils { senderAddress: string, zrxAssetData: string, ): Promise { - const fillMakerTokenAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.makerAssetAmount, - ); - await exchangeTradeEmulator.transferFromAsync( - signedOrder.makerAssetData, - signedOrder.makerAddress, - senderAddress, - fillMakerTokenAmount, - TradeSide.Maker, - TransferType.Trade, - ); - await exchangeTradeEmulator.transferFromAsync( - signedOrder.takerAssetData, - senderAddress, - signedOrder.makerAddress, - fillTakerAssetAmount, - TradeSide.Taker, - TransferType.Trade, - ); - const makerFeeAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.makerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - signedOrder.makerAddress, - signedOrder.feeRecipientAddress, - makerFeeAmount, - TradeSide.Maker, - TransferType.Fee, - ); - const takerFeeAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.takerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - senderAddress, - signedOrder.feeRecipientAddress, - takerFeeAmount, - TradeSide.Taker, - TransferType.Fee, - ); + try { + const fillMakerTokenAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.makerAssetAmount, + ); + await exchangeTradeEmulator.transferFromAsync( + signedOrder.makerAssetData, + signedOrder.makerAddress, + senderAddress, + fillMakerTokenAmount, + TradeSide.Maker, + TransferType.Trade, + ); + await exchangeTradeEmulator.transferFromAsync( + signedOrder.takerAssetData, + senderAddress, + signedOrder.makerAddress, + fillTakerAssetAmount, + TradeSide.Taker, + TransferType.Trade, + ); + const makerFeeAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.makerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + signedOrder.makerAddress, + signedOrder.feeRecipientAddress, + makerFeeAmount, + TradeSide.Maker, + TransferType.Fee, + ); + const takerFeeAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.takerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + senderAddress, + signedOrder.feeRecipientAddress, + takerFeeAmount, + TradeSide.Taker, + TransferType.Fee, + ); + } catch (err) { + throw new Error(RevertReasons.TransferFailed); + } } private static _validateRemainingFillAmountNotZeroOrThrow( takerAssetAmount: BigNumber, filledTakerTokenAmount: BigNumber, ): void { if (takerAssetAmount.eq(filledTakerTokenAmount)) { - throw new Error(ExchangeContractErrs.OrderRemainingFillAmountZero); + throw new Error(RevertReasons.OrderUnfillable); } } private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void { const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec(); if (expirationTimeSeconds.lessThan(currentUnixTimestampSec)) { - throw new Error(ExchangeContractErrs.OrderFillExpired); + throw new Error(RevertReasons.OrderUnfillable); } } constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { @@ -153,7 +158,7 @@ export class OrderValidationUtils { zrxAssetData: string, ): Promise { if (fillTakerAssetAmount.eq(0)) { - throw new Error(ExchangeContractErrs.OrderFillAmountZero); + throw new Error(RevertReasons.TakerAssetFillAmount); } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const isValid = await isValidSignatureAsync( @@ -171,7 +176,7 @@ export class OrderValidationUtils { filledTakerTokenAmount, ); if (signedOrder.takerAddress !== constants.NULL_ADDRESS && signedOrder.takerAddress !== takerAddress) { - throw new Error(ExchangeContractErrs.TransactionSenderIsNotFillOrderTaker); + throw new Error(RevertReasons.InvalidTaker); } OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); const remainingTakerTokenAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); @@ -192,7 +197,7 @@ export class OrderValidationUtils { signedOrder.makerAssetAmount, ); if (wouldRoundingErrorOccur) { - throw new Error(ExchangeContractErrs.OrderFillRoundingError); + throw new Error(RevertReasons.RoundingError); } return filledTakerTokenAmount; } @@ -213,7 +218,7 @@ export class OrderValidationUtils { zrxAssetData, ); if (filledTakerTokenAmount !== fillTakerAssetAmount) { - throw new Error(ExchangeContractErrs.InsufficientRemainingFillAmount); + throw new Error(RevertReasons.OrderUnfillable); } } } -- cgit v1.2.3 From 7b0f7c2e5c11ca03927eeeeada6d05994774fe86 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 13:51:18 +0200 Subject: Remove no longer used method --- packages/order-utils/src/order_validation_utils.ts | 16 ---------------- 1 file changed, 16 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index d2449b183..ba7050ac7 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -32,22 +32,6 @@ export class OrderValidationUtils { const isError = errPercentageTimes1000000.gt(1000); return isError; } - public static validateCancelOrderThrowIfInvalid( - order: Order, - cancelTakerTokenAmount: BigNumber, - filledTakerTokenAmount: BigNumber, - ): void { - if (cancelTakerTokenAmount.eq(0)) { - throw new Error(ExchangeContractErrs.OrderCancelAmountZero); - } - if (order.takerAssetAmount.eq(filledTakerTokenAmount)) { - throw new Error(ExchangeContractErrs.OrderAlreadyCancelledOrFilled); - } - const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec(); - if (order.expirationTimeSeconds.lessThan(currentUnixTimestampSec)) { - throw new Error(ExchangeContractErrs.OrderCancelExpired); - } - } public static async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( exchangeTradeEmulator: ExchangeTransferSimulator, signedOrder: SignedOrder, -- cgit v1.2.3 From 71c9b98896f454945e41661aca514946004f2346 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 14:23:35 +0200 Subject: Fix revertReason --- packages/order-utils/src/order_validation_utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index ba7050ac7..11a7a36c5 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -142,7 +142,7 @@ export class OrderValidationUtils { zrxAssetData: string, ): Promise { if (fillTakerAssetAmount.eq(0)) { - throw new Error(RevertReasons.TakerAssetFillAmount); + throw new Error(RevertReasons.InvalidTakerAmount); } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const isValid = await isValidSignatureAsync( -- cgit v1.2.3 From 178676ef8c5a32cdb557d04a0658d0967b4b8a75 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 20:08:00 +0200 Subject: Fix lint errors --- packages/order-utils/src/exchange_transfer_simulator.ts | 3 +-- packages/order-utils/src/order_state_utils.ts | 1 - packages/order-utils/src/order_validation_utils.ts | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/exchange_transfer_simulator.ts b/packages/order-utils/src/exchange_transfer_simulator.ts index a70259fcd..58d122ee4 100644 --- a/packages/order-utils/src/exchange_transfer_simulator.ts +++ b/packages/order-utils/src/exchange_transfer_simulator.ts @@ -1,4 +1,4 @@ -import { AssetProxyId, ExchangeContractErrs } from '@0xproject/types'; +import { ExchangeContractErrs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { AbstractBalanceAndProxyAllowanceLazyStore } from './abstract/abstract_balance_and_proxy_allowance_lazy_store'; @@ -90,7 +90,6 @@ export class ExchangeTransferSimulator { userAddress: string, amountInBaseUnits: BigNumber, ): Promise { - const assetProxyId = assetProxyUtils.decodeAssetDataId(assetData); const proxyAllowance = await this._store.getProxyAllowanceAsync(assetData, userAddress); // HACK: This code assumes that all tokens with an UNLIMITED_ALLOWANCE_IN_BASE_UNITS set, // are UnlimitedAllowanceTokens. This is however not true, it just so happens that all diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index 3752fdadb..e3423ccf2 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -7,7 +7,6 @@ import { SignedOrder, } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; -import * as _ from 'lodash'; import { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher'; import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher'; diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index 11a7a36c5..778556d6a 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -1,5 +1,4 @@ -import { Order, SignedOrder } from '@0xproject/types'; -import { RevertReasons } from '@0xproject/types'; +import { RevertReasons, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; -- cgit v1.2.3 From 1134ff107542d9014b195b6dc6ce6040752c92e3 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 20:17:00 +0200 Subject: Fix tslint issues --- packages/order-utils/src/exchange_transfer_simulator.ts | 1 - packages/order-utils/src/order_state_utils.ts | 1 - 2 files changed, 2 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/exchange_transfer_simulator.ts b/packages/order-utils/src/exchange_transfer_simulator.ts index 58d122ee4..72ed85406 100644 --- a/packages/order-utils/src/exchange_transfer_simulator.ts +++ b/packages/order-utils/src/exchange_transfer_simulator.ts @@ -2,7 +2,6 @@ import { ExchangeContractErrs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { AbstractBalanceAndProxyAllowanceLazyStore } from './abstract/abstract_balance_and_proxy_allowance_lazy_store'; -import { assetProxyUtils } from './asset_proxy_utils'; import { constants } from './constants'; import { TradeSide, TransferType } from './types'; diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index e3423ccf2..dd45a1298 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -10,7 +10,6 @@ import { BigNumber } from '@0xproject/utils'; import { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher'; import { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher'; -import { constants } from './constants'; import { orderHashUtils } from './order_hash'; import { RemainingFillableCalculator } from './remaining_fillable_calculator'; import { utils } from './utils'; -- cgit v1.2.3 From fb03003b3a9e7beb1054a50bad4320e60ca52e04 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Tue, 26 Jun 2018 07:50:24 +0200 Subject: Fix inconsistency between contract and reference implementation of order validation logic --- packages/order-utils/src/order_validation_utils.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index 778556d6a..fb5143d03 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -143,6 +143,9 @@ export class OrderValidationUtils { if (fillTakerAssetAmount.eq(0)) { throw new Error(RevertReasons.InvalidTakerAmount); } + if (signedOrder.makerAssetAmount.eq(0) || signedOrder.takerAssetAmount.eq(0)) { + throw new Error(RevertReasons.OrderUnfillable); + } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const isValid = await isValidSignatureAsync( provider, -- cgit v1.2.3 From d66ca7b5cbb5dc90e888c0466326825341d5fda4 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Tue, 26 Jun 2018 23:17:20 +0200 Subject: Fix enum name --- packages/order-utils/src/order_validation_utils.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index fb5143d03..54428f77d 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -1,4 +1,4 @@ -import { RevertReasons, SignedOrder } from '@0xproject/types'; +import { RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; @@ -87,7 +87,7 @@ export class OrderValidationUtils { TransferType.Fee, ); } catch (err) { - throw new Error(RevertReasons.TransferFailed); + throw new Error(RevertReason.TransferFailed); } } private static _validateRemainingFillAmountNotZeroOrThrow( @@ -95,13 +95,13 @@ export class OrderValidationUtils { filledTakerTokenAmount: BigNumber, ): void { if (takerAssetAmount.eq(filledTakerTokenAmount)) { - throw new Error(RevertReasons.OrderUnfillable); + throw new Error(RevertReason.OrderUnfillable); } } private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void { const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec(); if (expirationTimeSeconds.lessThan(currentUnixTimestampSec)) { - throw new Error(RevertReasons.OrderUnfillable); + throw new Error(RevertReason.OrderUnfillable); } } constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { @@ -141,10 +141,10 @@ export class OrderValidationUtils { zrxAssetData: string, ): Promise { if (fillTakerAssetAmount.eq(0)) { - throw new Error(RevertReasons.InvalidTakerAmount); + throw new Error(RevertReason.InvalidTakerAmount); } if (signedOrder.makerAssetAmount.eq(0) || signedOrder.takerAssetAmount.eq(0)) { - throw new Error(RevertReasons.OrderUnfillable); + throw new Error(RevertReason.OrderUnfillable); } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const isValid = await isValidSignatureAsync( @@ -162,7 +162,7 @@ export class OrderValidationUtils { filledTakerTokenAmount, ); if (signedOrder.takerAddress !== constants.NULL_ADDRESS && signedOrder.takerAddress !== takerAddress) { - throw new Error(RevertReasons.InvalidTaker); + throw new Error(RevertReason.InvalidTaker); } OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); const remainingTakerTokenAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); @@ -183,7 +183,7 @@ export class OrderValidationUtils { signedOrder.makerAssetAmount, ); if (wouldRoundingErrorOccur) { - throw new Error(RevertReasons.RoundingError); + throw new Error(RevertReason.RoundingError); } return filledTakerTokenAmount; } @@ -204,7 +204,7 @@ export class OrderValidationUtils { zrxAssetData, ); if (filledTakerTokenAmount !== fillTakerAssetAmount) { - throw new Error(RevertReasons.OrderUnfillable); + throw new Error(RevertReason.OrderUnfillable); } } } -- cgit v1.2.3