Merge pull request #1235 from 0xProject/fixOrderValidation

[order-utils] Fix order validation method

15 files changed, 217 insertions, 124 deletions

diff --git a/packages/contract-wrappers/CHANGELOG.json b/packages/contract-wrappers/CHANGELOG.json
index 0346b79b2..0ab5e5d08 100644
--- a/packages/contract-wrappers/CHANGELOG.json
+++ b/packages/contract-wrappers/CHANGELOG.json
@@ -1,5 +1,25 @@
 [
     {
+        "version": "4.0.0",
+        "changes": [
+            {
+                "note":
+                    "Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync`",
+                "pr": 1235
+            },
+            {
+                "note":
+                    "Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive",
+                "pr": 1235
+            },
+            {
+                "note":
+                    "Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218)",
+                "pr": 1235
+            }
+        ]
+    },
+    {
         "version": "3.0.1",
         "changes": [
             {
diff --git a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
index 2e978f35b..c76e51eee 100644
--- a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
@@ -18,6 +18,7 @@ import { OrderFilledCancelledFetcher } from '../fetchers/order_filled_cancelled_
 import { methodOptsSchema } from '../schemas/method_opts_schema';
 import { orderTxOptsSchema } from '../schemas/order_tx_opts_schema';
 import { txOptsSchema } from '../schemas/tx_opts_schema';
+import { validateOrderFillableOptsSchema } from '../schemas/validate_order_fillable_opts_schema';
 import {
     BlockRange,
     EventCallback,
@@ -1114,6 +1115,9 @@ export class ExchangeWrapper extends ContractWrapper {
         signedOrder: SignedOrder,
         opts: ValidateOrderFillableOpts = {},
     ): Promise<void> {
+        assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema);
+        assert.doesConformToSchema('opts', opts, validateOrderFillableOptsSchema);
+
         const balanceAllowanceFetcher = new AssetBalanceAndProxyAllowanceFetcher(
             this._erc20TokenWrapper,
             this._erc721TokenWrapper,
@@ -1124,7 +1128,7 @@ export class ExchangeWrapper extends ContractWrapper {
 
         const expectedFillTakerTokenAmountIfExists = opts.expectedFillTakerTokenAmount;
         const filledCancelledFetcher = new OrderFilledCancelledFetcher(this, BlockParamLiteral.Latest);
-        const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher);
+        const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher, this._web3Wrapper.getProvider());
         await orderValidationUtils.validateOrderFillableOrThrowAsync(
             exchangeTradeSimulator,
             signedOrder,
@@ -1152,7 +1156,7 @@ export class ExchangeWrapper extends ContractWrapper {
         const exchangeTradeSimulator = new ExchangeTransferSimulator(balanceAllowanceStore);
 
         const filledCancelledFetcher = new OrderFilledCancelledFetcher(this, BlockParamLiteral.Latest);
-        const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher);
+        const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher, this._web3Wrapper.getProvider());
         await orderValidationUtils.validateFillOrderThrowIfInvalidAsync(
             exchangeTradeSimulator,
             this._web3Wrapper.getProvider(),
diff --git a/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts b/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
index acf7038fa..5d350916c 100644
--- a/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
+++ b/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
@@ -1,5 +1,6 @@
 // tslint:disable:no-unnecessary-type-assertion
-import { AbstractOrderFilledCancelledFetcher } from '@0x/order-utils';
+import { AbstractOrderFilledCancelledFetcher, orderHashUtils } from '@0x/order-utils';
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import { BlockParamLiteral } from 'ethereum-types';
 
@@ -18,9 +19,18 @@ export class OrderFilledCancelledFetcher implements AbstractOrderFilledCancelled
         });
         return filledTakerAmount;
     }
-    public async isOrderCancelledAsync(orderHash: string): Promise<boolean> {
+    public async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean> {
+        const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
         const isCancelled = await this._exchange.isCancelledAsync(orderHash);
-        return isCancelled;
+        const orderEpoch = await this._exchange.getOrderEpochAsync(
+            signedOrder.makerAddress,
+            signedOrder.senderAddress,
+            {
+                defaultBlock: this._stateLayer,
+            },
+        );
+        const isCancelledByOrderEpoch = orderEpoch > signedOrder.salt;
+        return isCancelled || isCancelledByOrderEpoch;
     }
     public getZRXAssetData(): string {
         const zrxAssetData = this._exchange.getZRXAssetData();
diff --git a/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts b/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts
new file mode 100644
index 000000000..2e111af04
--- /dev/null
+++ b/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts
@@ -0,0 +1,7 @@
+export const validateOrderFillableOptsSchema = {
+    id: '/ValidateOrderFillableOpts',
+    properties: {
+        expectedFillTakerTokenAmount: { $ref: '/wholeNumberSchema' },
+    },
+    type: 'object',
+};
diff --git a/packages/contract-wrappers/test/exchange_wrapper_test.ts b/packages/contract-wrappers/test/exchange_wrapper_test.ts
index 0e537bd83..73ce6c743 100644
--- a/packages/contract-wrappers/test/exchange_wrapper_test.ts
+++ b/packages/contract-wrappers/test/exchange_wrapper_test.ts
@@ -1,7 +1,7 @@
 import { BlockchainLifecycle, callbackErrorReporter } from '@0x/dev-utils';
 import { FillScenarios } from '@0x/fill-scenarios';
 import { assetDataUtils, orderHashUtils } from '@0x/order-utils';
-import { DoneCallback, SignedOrder } from '@0x/types';
+import { DoneCallback, RevertReason, SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import * as chai from 'chai';
 import { BlockParamLiteral } from 'ethereum-types';
@@ -282,6 +282,19 @@ describe('ExchangeWrapper', () => {
             expect(ordersInfo[1].orderHash).to.be.equal(anotherOrderHash);
         });
     });
+    describe('#validateOrderFillableOrThrowAsync', () => {
+        it('should throw if signature is invalid', async () => {
+            const signedOrderWithInvalidSignature = {
+                ...signedOrder,
+                signature:
+                    '0x1b61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403',
+            };
+
+            expect(
+                contractWrappers.exchange.validateOrderFillableOrThrowAsync(signedOrderWithInvalidSignature),
+            ).to.eventually.to.be.rejectedWith(RevertReason.InvalidOrderSignature);
+        });
+    });
     describe('#isValidSignature', () => {
         it('should check if the signature is valid', async () => {
             const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
diff --git a/packages/contracts/test/utils/exchange_wrapper.ts b/packages/contracts/test/utils/exchange_wrapper.ts
index 29dba690a..c28989d3f 100644
--- a/packages/contracts/test/utils/exchange_wrapper.ts
+++ b/packages/contracts/test/utils/exchange_wrapper.ts
@@ -212,13 +212,17 @@ export class ExchangeWrapper {
         return tx;
     }
     public async getTakerAssetFilledAmountAsync(orderHashHex: string): Promise<BigNumber> {
-        const filledAmount = new BigNumber(await this._exchange.filled.callAsync(orderHashHex));
+        const filledAmount = await this._exchange.filled.callAsync(orderHashHex);
         return filledAmount;
     }
     public async isCancelledAsync(orderHashHex: string): Promise<boolean> {
         const isCancelled = await this._exchange.cancelled.callAsync(orderHashHex);
         return isCancelled;
     }
+    public async getOrderEpochAsync(makerAddress: string, senderAddress: string): Promise<BigNumber> {
+        const orderEpoch = await this._exchange.orderEpoch.callAsync(makerAddress, senderAddress);
+        return orderEpoch;
+    }
     public async getOrderInfoAsync(signedOrder: SignedOrder): Promise<OrderInfo> {
         const orderInfo = (await this._exchange.getOrderInfo.callAsync(signedOrder)) as OrderInfo;
         return orderInfo;
diff --git a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
index 81bb33318..8046771f9 100644
--- a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
+++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
@@ -392,7 +392,7 @@ export class FillOrderCombinatorialUtils {
         );
 
         // 5. If I fill it by X, what are the resulting balances/allowances/filled amounts expected?
-        const orderValidationUtils = new OrderValidationUtils(orderFilledCancelledFetcher);
+        const orderValidationUtils = new OrderValidationUtils(orderFilledCancelledFetcher, provider);
         const lazyStore = new BalanceAndProxyAllowanceLazyStore(balanceAndProxyAllowanceFetcher);
         const exchangeTransferSimulator = new ExchangeTransferSimulator(lazyStore);
 
diff --git a/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts b/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
index 5f5575c7b..af959e00e 100644
--- a/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
+++ b/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
@@ -1,4 +1,5 @@
-import { AbstractOrderFilledCancelledFetcher } from '@0x/order-utils';
+import { AbstractOrderFilledCancelledFetcher, orderHashUtils } from '@0x/order-utils';
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 
 import { ExchangeWrapper } from './exchange_wrapper';
@@ -14,9 +15,15 @@ export class SimpleOrderFilledCancelledFetcher implements AbstractOrderFilledCan
         const filledTakerAmount = new BigNumber(await this._exchangeWrapper.getTakerAssetFilledAmountAsync(orderHash));
         return filledTakerAmount;
     }
-    public async isOrderCancelledAsync(orderHash: string): Promise<boolean> {
+    public async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean> {
+        const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
         const isCancelled = await this._exchangeWrapper.isCancelledAsync(orderHash);
-        return isCancelled;
+        const orderEpoch = await this._exchangeWrapper.getOrderEpochAsync(
+            signedOrder.makerAddress,
+            signedOrder.senderAddress,
+        );
+        const isCancelledByOrderEpoch = orderEpoch > signedOrder.salt;
+        return isCancelled || isCancelledByOrderEpoch;
     }
     public getZRXAssetData(): string {
         return this._zrxAssetData;
diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json
index abcf3d392..0ed6117df 100644
--- a/packages/order-utils/CHANGELOG.json
+++ b/packages/order-utils/CHANGELOG.json
@@ -1,5 +1,30 @@
 [
     {
+        "version": "3.0.0",
+        "changes": [
+            {
+                "note":
+                    "Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync`",
+                "pr": 1235
+            },
+            {
+                "note":
+                    "Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive",
+                "pr": 1235
+            },
+            {
+                "note":
+                    "Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218)",
+                "pr": 1235
+            },
+            {
+                "note":
+                    "Modified the `AbstractOrderFilledCancelledFetcher` interface slightly such that `isOrderCancelledAsync` accepts a `signedOrder` instead of an `orderHash` param",
+                "pr": 1235
+            }
+        ]
+    },
+    {
         "version": "2.0.1",
         "changes": [
             {
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 de096b7d9..9e240f9ef 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
@@ -1,3 +1,4 @@
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 
 /**
@@ -17,6 +18,6 @@ export abstract class AbstractOrderFilledCancelledFetcher {
      * @param orderHash OrderHash of order we are interested in
      * @return Whether or not the order is cancelled
      */
-    public abstract async isOrderCancelledAsync(orderHash: string): Promise<boolean>;
+    public abstract async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean>;
     public abstract getZRXAssetData(): string;
 }
diff --git a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts
index d9e66db06..186521401 100644
--- a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts
+++ b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts
@@ -1,8 +1,9 @@
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 
 export abstract class AbstractOrderFilledCancelledLazyStore {
     public abstract async getFilledTakerAmountAsync(orderHash: string): Promise<BigNumber>;
-    public abstract async getIsCancelledAsync(orderHash: string): Promise<boolean>;
+    public abstract async getIsCancelledAsync(signedOrder: SignedOrder): Promise<boolean>;
     public abstract setFilledTakerAmount(orderHash: string, balance: BigNumber): void;
     public abstract deleteFilledTakerAmount(orderHash: string): void;
     public abstract setIsCancelled(orderHash: string, isCancelled: boolean): void;
diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts
index 159aeeb09..fe0d6c773 100644
--- a/packages/order-utils/src/order_state_utils.ts
+++ b/packages/order-utils/src/order_state_utils.ts
@@ -117,7 +117,7 @@ export class OrderStateUtils {
     public async getOpenOrderStateAsync(signedOrder: SignedOrder, transactionHash?: string): Promise<OrderState> {
         const orderRelevantState = await this.getOpenOrderRelevantStateAsync(signedOrder);
         const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
-        const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash);
+        const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
         const sidedOrderRelevantState = {
             isMakerSide: true,
             traderBalance: orderRelevantState.makerBalance,
@@ -256,7 +256,7 @@ export class OrderStateUtils {
         const filledTakerAssetAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash);
         const totalMakerAssetAmount = signedOrder.makerAssetAmount;
         const totalTakerAssetAmount = signedOrder.takerAssetAmount;
-        const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash);
+        const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
         const remainingTakerAssetAmount = isOrderCancelled
             ? new BigNumber(0)
             : totalTakerAssetAmount.minus(filledTakerAssetAmount);
diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts
index a40069f63..ae4291ea8 100644
--- a/packages/order-utils/src/order_validation_utils.ts
+++ b/packages/order-utils/src/order_validation_utils.ts
@@ -1,4 +1,4 @@
-import { RevertReason, SignedOrder } from '@0x/types';
+import { ExchangeContractErrs, RevertReason, SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import { Provider } from 'ethereum-types';
 import * as _ from 'lodash';
@@ -17,6 +17,7 @@ import { utils } from './utils';
  */
 export class OrderValidationUtils {
     private readonly _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher;
+    private readonly _provider: Provider;
     /**
      * A Typescript implementation mirroring the implementation of isRoundingError in the
      * Exchange smart contract
@@ -57,65 +58,53 @@ export class OrderValidationUtils {
         senderAddress: string,
         zrxAssetData: string,
     ): Promise<void> {
-        try {
-            const fillMakerTokenAmount = utils.getPartialAmountFloor(
-                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.getPartialAmountFloor(
-                fillTakerAssetAmount,
-                signedOrder.takerAssetAmount,
-                signedOrder.makerFee,
-            );
-            await exchangeTradeEmulator.transferFromAsync(
-                zrxAssetData,
-                signedOrder.makerAddress,
-                signedOrder.feeRecipientAddress,
-                makerFeeAmount,
-                TradeSide.Maker,
-                TransferType.Fee,
-            );
-            const takerFeeAmount = utils.getPartialAmountFloor(
-                fillTakerAssetAmount,
-                signedOrder.takerAssetAmount,
-                signedOrder.takerFee,
-            );
-            await exchangeTradeEmulator.transferFromAsync(
-                zrxAssetData,
-                senderAddress,
-                signedOrder.feeRecipientAddress,
-                takerFeeAmount,
-                TradeSide.Taker,
-                TransferType.Fee,
-            );
-        } catch (err) {
-            throw new Error(RevertReason.TransferFailed);
-        }
-    }
-    private static _validateRemainingFillAmountNotZeroOrThrow(
-        takerAssetAmount: BigNumber,
-        filledTakerTokenAmount: BigNumber,
-    ): void {
-        if (takerAssetAmount.eq(filledTakerTokenAmount)) {
-            throw new Error(RevertReason.OrderUnfillable);
-        }
+        const fillMakerTokenAmount = utils.getPartialAmountFloor(
+            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.getPartialAmountFloor(
+            fillTakerAssetAmount,
+            signedOrder.takerAssetAmount,
+            signedOrder.makerFee,
+        );
+        await exchangeTradeEmulator.transferFromAsync(
+            zrxAssetData,
+            signedOrder.makerAddress,
+            signedOrder.feeRecipientAddress,
+            makerFeeAmount,
+            TradeSide.Maker,
+            TransferType.Fee,
+        );
+        const takerFeeAmount = utils.getPartialAmountFloor(
+            fillTakerAssetAmount,
+            signedOrder.takerAssetAmount,
+            signedOrder.takerFee,
+        );
+        await exchangeTradeEmulator.transferFromAsync(
+            zrxAssetData,
+            senderAddress,
+            signedOrder.feeRecipientAddress,
+            takerFeeAmount,
+            TradeSide.Taker,
+            TransferType.Fee,
+        );
     }
     private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void {
         const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec();
@@ -128,9 +117,13 @@ export class OrderValidationUtils {
      * @param orderFilledCancelledFetcher A module that implements the AbstractOrderFilledCancelledFetcher
      * @return An instance of OrderValidationUtils
      */
-    constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) {
+    constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher, provider: Provider) {
         this._orderFilledCancelledFetcher = orderFilledCancelledFetcher;
+        this._provider = provider;
     }
+    // TODO(fabio): remove this method once the smart contracts have been refactored
+    // to return helpful revert reasons instead of ORDER_UNFILLABLE. Instruct devs
+    // to make "calls" to validate order fillability + getOrderInfo for fillable amount.
     /**
      * Validate if the supplied order is fillable, and throw if it isn't
      * @param exchangeTradeEmulator ExchangeTradeEmulator instance
@@ -146,12 +139,29 @@ export class OrderValidationUtils {
         expectedFillTakerTokenAmount?: BigNumber,
     ): Promise<void> {
         const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
-        const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash);
-        OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow(
-            signedOrder.takerAssetAmount,
-            filledTakerTokenAmount,
+        const isValidSignature = await signatureUtils.isValidSignatureAsync(
+            this._provider,
+            orderHash,
+            signedOrder.signature,
+            signedOrder.makerAddress,
         );
-        OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds);
+        if (!isValidSignature) {
+            throw new Error(RevertReason.InvalidOrderSignature);
+        }
+
+        const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
+        if (isCancelled) {
+            throw new Error('CANCELLED');
+        }
+        const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash);
+        if (signedOrder.takerAssetAmount.eq(filledTakerTokenAmount)) {
+            throw new Error('FULLY_FILLED');
+        }
+        try {
+            OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds);
+        } catch (err) {
+            throw new Error('EXPIRED');
+        }
         let fillTakerAssetAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount);
         if (!_.isUndefined(expectedFillTakerTokenAmount)) {
             fillTakerAssetAmount = expectedFillTakerTokenAmount;
@@ -198,10 +208,9 @@ export class OrderValidationUtils {
             throw new Error(OrderError.InvalidSignature);
         }
         const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash);
-        OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow(
-            signedOrder.takerAssetAmount,
-            filledTakerTokenAmount,
-        );
+        if (signedOrder.takerAssetAmount.eq(filledTakerTokenAmount)) {
+            throw new Error(RevertReason.OrderUnfillable);
+        }
         if (signedOrder.takerAddress !== constants.NULL_ADDRESS && signedOrder.takerAddress !== takerAddress) {
             throw new Error(RevertReason.InvalidTaker);
         }
@@ -210,13 +219,30 @@ export class OrderValidationUtils {
         const desiredFillTakerTokenAmount = remainingTakerTokenAmount.lessThan(fillTakerAssetAmount)
             ? remainingTakerTokenAmount
             : fillTakerAssetAmount;
-        await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync(
-            exchangeTradeEmulator,
-            signedOrder,
-            desiredFillTakerTokenAmount,
-            takerAddress,
-            zrxAssetData,
-        );
+        try {
+            await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync(
+                exchangeTradeEmulator,
+                signedOrder,
+                desiredFillTakerTokenAmount,
+                takerAddress,
+                zrxAssetData,
+            );
+        } catch (err) {
+            const transferFailedErrorMessages = [
+                ExchangeContractErrs.InsufficientMakerBalance,
+                ExchangeContractErrs.InsufficientMakerFeeBalance,
+                ExchangeContractErrs.InsufficientTakerBalance,
+                ExchangeContractErrs.InsufficientTakerFeeBalance,
+                ExchangeContractErrs.InsufficientMakerAllowance,
+                ExchangeContractErrs.InsufficientMakerFeeAllowance,
+                ExchangeContractErrs.InsufficientTakerAllowance,
+                ExchangeContractErrs.InsufficientTakerFeeAllowance,
+            ];
+            if (_.includes(transferFailedErrorMessages, err.message)) {
+                throw new Error(RevertReason.TransferFailed);
+            }
+            throw err;
+        }
 
         const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingErrorFloor(
             desiredFillTakerTokenAmount,
@@ -228,33 +254,4 @@ export class OrderValidationUtils {
         }
         return filledTakerTokenAmount;
     }
-    /**
-     * Validate a call to fillOrKillOrder and throw if it would fail
-     * @param exchangeTradeEmulator ExchangeTradeEmulator to use
-     * @param provider Web3 provider to use for JSON RPC requests
-     * @param signedOrder SignedOrder of interest
-     * @param fillTakerAssetAmount Amount we'd like to fill the order for
-     * @param takerAddress The taker of the order
-     * @param zrxAssetData ZRX asset data
-     */
-    public async validateFillOrKillOrderThrowIfInvalidAsync(
-        exchangeTradeEmulator: ExchangeTransferSimulator,
-        provider: Provider,
-        signedOrder: SignedOrder,
-        fillTakerAssetAmount: BigNumber,
-        takerAddress: string,
-        zrxAssetData: string,
-    ): Promise<void> {
-        const filledTakerTokenAmount = await this.validateFillOrderThrowIfInvalidAsync(
-            exchangeTradeEmulator,
-            provider,
-            signedOrder,
-            fillTakerAssetAmount,
-            takerAddress,
-            zrxAssetData,
-        );
-        if (filledTakerTokenAmount !== fillTakerAssetAmount) {
-            throw new Error(RevertReason.OrderUnfillable);
-        }
-    }
 }
diff --git a/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts b/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts
index 1d84ffdaa..afd6f1108 100644
--- a/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts
+++ b/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts
@@ -1,8 +1,10 @@
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
 import { AbstractOrderFilledCancelledFetcher } from '../abstract/abstract_order_filled_cancelled_fetcher';
 import { AbstractOrderFilledCancelledLazyStore } from '../abstract/abstract_order_filled_cancelled_lazy_store';
+import { orderHashUtils } from '../order_hash';
 
 /**
  * Copy on read store for balances/proxyAllowances of tokens/accounts
@@ -58,9 +60,10 @@ export class OrderFilledCancelledLazyStore implements AbstractOrderFilledCancell
      * @param orderHash OrderHash from order of interest
      * @return Whether the order has been cancelled
      */
-    public async getIsCancelledAsync(orderHash: string): Promise<boolean> {
+    public async getIsCancelledAsync(signedOrder: SignedOrder): Promise<boolean> {
+        const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
         if (_.isUndefined(this._isCancelled[orderHash])) {
-            const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash);
+            const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
             this.setIsCancelled(orderHash, isCancelled);
         }
         const cachedIsCancelled = this._isCancelled[orderHash]; // tslint:disable-line:boolean-naming
diff --git a/packages/order-utils/test/order_state_utils_test.ts b/packages/order-utils/test/order_state_utils_test.ts
index 39c4c362f..42acd54c6 100644
--- a/packages/order-utils/test/order_state_utils_test.ts
+++ b/packages/order-utils/test/order_state_utils_test.ts
@@ -1,3 +1,4 @@
+import { SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import * as chai from 'chai';
 import 'mocha';
@@ -33,7 +34,7 @@ describe('OrderStateUtils', () => {
                 async getFilledTakerAmountAsync(_orderHash: string): Promise<BigNumber> {
                     return filledAmount;
                 },
-                async isOrderCancelledAsync(_orderHash: string): Promise<boolean> {
+                async isOrderCancelledAsync(_signedOrder: SignedOrder): Promise<boolean> {
                     return cancelled;
                 },
                 getZRXAssetData(): string {