From 1f0ac47bd97b88071a6380a728cfdb1457c2590c Mon Sep 17 00:00:00 2001
From: Fabio Berger <me@fabioberger.com>
Date: Sat, 10 Nov 2018 00:14:48 +0100
Subject: Move signature validation into
 OrderValidationUtils.validateOrderFillableOrThrowAsync

---
 .../src/contract_wrappers/exchange_wrapper.ts             | 15 ++-------------
 .../test/utils/fill_order_combinatorial_utils.ts          |  2 +-
 packages/order-utils/src/order_validation_utils.ts        | 14 +++++++++++++-
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
index 902fc755c..d3a4e9098 100644
--- a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
@@ -1120,17 +1120,6 @@ export class ExchangeWrapper extends ContractWrapper {
         assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema);
         assert.doesConformToSchema('opts', opts, validateOrderFillableOptsSchema);
 
-        const orderHash = await orderHashUtils.getOrderHashHex(signedOrder);
-        const isValidSignature = await signatureUtils.isValidSignatureAsync(
-            this._web3Wrapper.getProvider(),
-            orderHash,
-            signedOrder.signature,
-            signedOrder.makerAddress,
-        );
-        if (!isValidSignature) {
-            throw new Error('INVALID_ORDER_SIGNATURE');
-        }
-
         const balanceAllowanceFetcher = new AssetBalanceAndProxyAllowanceFetcher(
             this._erc20TokenWrapper,
             this._erc721TokenWrapper,
@@ -1141,7 +1130,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,
@@ -1169,7 +1158,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/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/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts
index 2150e643f..ca4c32543 100644
--- a/packages/order-utils/src/order_validation_utils.ts
+++ b/packages/order-utils/src/order_validation_utils.ts
@@ -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
@@ -116,8 +117,9 @@ 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
@@ -137,6 +139,16 @@ export class OrderValidationUtils {
         expectedFillTakerTokenAmount?: BigNumber,
     ): Promise<void> {
         const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
+        const isValidSignature = await signatureUtils.isValidSignatureAsync(
+            this._provider,
+            orderHash,
+            signedOrder.signature,
+            signedOrder.makerAddress,
+        );
+        if (!isValidSignature) {
+            throw new Error('INVALID_ORDER_SIGNATURE');
+        }
+
         const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
         if (isCancelled) {
             throw new Error('CANCELLED');
-- 
cgit