Merge pull request #1235 from 0xProject/fixOrderValidation

[order-utils] Fix order validation method

5 files changed, 60 insertions, 6 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);