From 88c99396a2d1b880bffb21ef19507d02b474ba9b Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 15 Aug 2018 13:03:31 +1000 Subject: Rename OrderAlreadyCancelledOrFilled -> OrderCancelled. Remove try catch of throwing errors in favour of returning the Errors in a OrderValidationResult --- packages/order-utils/src/order_state_utils.ts | 61 ++++++++++++---------- .../order-utils/test/order_state_utils_test.ts | 4 +- packages/order-watcher/test/order_watcher_test.ts | 2 +- packages/types/src/index.ts | 3 +- packages/website/ts/utils/utils.ts | 7 +-- 5 files changed, 40 insertions(+), 37 deletions(-) diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index 7974d5d0b..18fc18bf6 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -25,6 +25,14 @@ interface SidedOrderRelevantState { remainingFillableAssetAmount: BigNumber; isOrderCancelled: boolean; } +interface OrderValidResult { + isValid: true; +} +interface OrderInvalidResult { + isValid: false; + error: ExchangeContractErrs; +} +type OrderValidationResult = OrderValidResult | OrderInvalidResult; export class OrderStateUtils { private readonly _balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher; @@ -32,46 +40,42 @@ export class OrderStateUtils { private static _validateIfOrderIsValid( signedOrder: SignedOrder, sidedOrderRelevantState: SidedOrderRelevantState, - ): void { + ): OrderValidationResult { const isMakerSide = sidedOrderRelevantState.isMakerSide; if (sidedOrderRelevantState.isOrderCancelled) { - throw new Error(ExchangeContractErrs.OrderAlreadyCancelledOrFilled); + return { isValid: false, error: ExchangeContractErrs.OrderCancelled }; } const availableTakerAssetAmount = signedOrder.takerAssetAmount.minus( sidedOrderRelevantState.filledTakerAssetAmount, ); if (availableTakerAssetAmount.eq(0)) { - throw new Error(ExchangeContractErrs.OrderRemainingFillAmountZero); + return { isValid: false, error: ExchangeContractErrs.OrderRemainingFillAmountZero }; } if (sidedOrderRelevantState.traderBalance.eq(0)) { - throw new Error( - isMakerSide - ? ExchangeContractErrs.InsufficientMakerBalance - : ExchangeContractErrs.InsufficientTakerBalance, - ); + const error = isMakerSide + ? ExchangeContractErrs.InsufficientMakerBalance + : ExchangeContractErrs.InsufficientTakerBalance; + return { isValid: false, error }; } if (sidedOrderRelevantState.traderProxyAllowance.eq(0)) { - throw new Error( - isMakerSide - ? ExchangeContractErrs.InsufficientMakerAllowance - : ExchangeContractErrs.InsufficientTakerAllowance, - ); + const error = isMakerSide + ? ExchangeContractErrs.InsufficientMakerAllowance + : ExchangeContractErrs.InsufficientTakerAllowance; + return { isValid: false, error }; } if (!signedOrder.makerFee.eq(0)) { if (sidedOrderRelevantState.traderFeeBalance.eq(0)) { - throw new Error( - isMakerSide - ? ExchangeContractErrs.InsufficientMakerFeeBalance - : ExchangeContractErrs.InsufficientTakerFeeBalance, - ); + const error = isMakerSide + ? ExchangeContractErrs.InsufficientMakerFeeBalance + : ExchangeContractErrs.InsufficientTakerFeeBalance; + return { isValid: false, error }; } if (sidedOrderRelevantState.traderFeeProxyAllowance.eq(0)) { - throw new Error( - isMakerSide - ? ExchangeContractErrs.InsufficientMakerFeeAllowance - : ExchangeContractErrs.InsufficientTakerFeeAllowance, - ); + const error = isMakerSide + ? ExchangeContractErrs.InsufficientMakerFeeAllowance + : ExchangeContractErrs.InsufficientTakerFeeAllowance; + return { isValid: false, error }; } } const remainingTakerAssetAmount = signedOrder.takerAssetAmount.minus( @@ -83,8 +87,9 @@ export class OrderStateUtils { signedOrder.makerAssetAmount, ); if (isRoundingError) { - throw new Error(ExchangeContractErrs.OrderFillRoundingError); + return { isValid: false, error: ExchangeContractErrs.OrderFillRoundingError }; } + return { isValid: true }; } constructor( balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher, @@ -107,19 +112,19 @@ export class OrderStateUtils { remainingFillableAssetAmount: orderRelevantState.remainingFillableMakerAssetAmount, isOrderCancelled, }; - try { - OrderStateUtils._validateIfOrderIsValid(signedOrder, sidedOrderRelevantState); + const orderValidationResult = OrderStateUtils._validateIfOrderIsValid(signedOrder, sidedOrderRelevantState); + if (orderValidationResult.isValid) { const orderState: OrderStateValid = { isValid: true, orderHash, orderRelevantState, }; return orderState; - } catch (err) { + } else { const orderState: OrderStateInvalid = { isValid: false, orderHash, - error: err.message, + error: orderValidationResult.error, }; return orderState; } diff --git a/packages/order-utils/test/order_state_utils_test.ts b/packages/order-utils/test/order_state_utils_test.ts index 9292c40a4..91ef23b69 100644 --- a/packages/order-utils/test/order_state_utils_test.ts +++ b/packages/order-utils/test/order_state_utils_test.ts @@ -2,7 +2,9 @@ import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import 'mocha'; -import { AbstractBalanceAndProxyAllowanceFetcher, AbstractOrderFilledCancelledFetcher, OrderStateUtils } from '../src'; +import { AbstractBalanceAndProxyAllowanceFetcher } from '../src/abstract/abstract_balance_and_proxy_allowance_fetcher'; +import { AbstractOrderFilledCancelledFetcher } from '../src/abstract/abstract_order_filled_cancelled_fetcher'; +import { OrderStateUtils } from '../src/order_state_utils'; import { chaiSetup } from './utils/chai_setup'; import { testOrderFactory } from './utils/test_order_factory'; diff --git a/packages/order-watcher/test/order_watcher_test.ts b/packages/order-watcher/test/order_watcher_test.ts index 0f4caeb63..38bfde7ef 100644 --- a/packages/order-watcher/test/order_watcher_test.ts +++ b/packages/order-watcher/test/order_watcher_test.ts @@ -501,7 +501,7 @@ describe('OrderWatcher', () => { expect(orderState.isValid).to.be.false(); const invalidOrderState = orderState as OrderStateInvalid; expect(invalidOrderState.orderHash).to.be.equal(orderHash); - expect(invalidOrderState.error).to.be.equal(ExchangeContractErrs.OrderAlreadyCancelledOrFilled); + expect(invalidOrderState.error).to.be.equal(ExchangeContractErrs.OrderCancelled); }); orderWatcher.subscribe(callback); await contractWrappers.exchange.cancelOrderAsync(signedOrder); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index fa634420d..04480d093 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -62,8 +62,7 @@ export interface ValidatorSignature { export enum ExchangeContractErrs { OrderFillExpired = 'ORDER_FILL_EXPIRED', OrderCancelExpired = 'ORDER_CANCEL_EXPIRED', - OrderCancelAmountZero = 'ORDER_CANCEL_AMOUNT_ZERO', - OrderAlreadyCancelledOrFilled = 'ORDER_ALREADY_CANCELLED_OR_FILLED', + OrderCancelled = 'ORDER_CANCELLED', OrderFillAmountZero = 'ORDER_FILL_AMOUNT_ZERO', OrderRemainingFillAmountZero = 'ORDER_REMAINING_FILL_AMOUNT_ZERO', OrderFillRoundingError = 'ORDER_FILL_ROUNDING_ERROR', diff --git a/packages/website/ts/utils/utils.ts b/packages/website/ts/utils/utils.ts index 39bbd404c..32b07473c 100644 --- a/packages/website/ts/utils/utils.ts +++ b/packages/website/ts/utils/utils.ts @@ -247,12 +247,9 @@ export const utils = { } = { [ExchangeContractErrs.OrderFillExpired]: 'This order has expired', [ExchangeContractErrs.OrderCancelExpired]: 'This order has expired', - [ExchangeContractErrs.OrderCancelAmountZero]: "Order cancel amount can't be 0", - [ExchangeContractErrs.OrderAlreadyCancelledOrFilled]: - 'This order has already been completely filled or cancelled', + [ExchangeContractErrs.OrderCancelled]: 'This order has been cancelled', [ExchangeContractErrs.OrderFillAmountZero]: "Order fill amount can't be 0", - [ExchangeContractErrs.OrderRemainingFillAmountZero]: - 'This order has already been completely filled or cancelled', + [ExchangeContractErrs.OrderRemainingFillAmountZero]: 'This order has already been completely filled', [ExchangeContractErrs.OrderFillRoundingError]: 'Rounding error will occur when filling this order. Please try filling a different amount.', [ExchangeContractErrs.InsufficientTakerBalance]: -- cgit v1.2.3