Refactor to use OrderCancellationRequest

4 files changed, 45 insertions, 34 deletions

diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts
index fec75e6df..f67a125aa 100644
--- a/src/contract_wrappers/exchange_wrapper.ts
+++ b/src/contract_wrappers/exchange_wrapper.ts
@@ -18,7 +18,7 @@ import {
     CreateContractEvent,
     ContractEventObj,
     EventCallback,
-    ContractResponse,
+    ContractResponse, OrderCancellationRequest,
 } from '../types';
 import {assert} from '../utils/assert';
 import {utils} from '../utils/utils';
@@ -208,29 +208,31 @@ export class ExchangeWrapper extends ContractWrapper {
     /**
      * Batch version of cancelOrderAsync. Atomically cancels multiple orders in a single transaction.
      */
-    public async batchCancelOrderAsync(
-        orders: Array<Order|SignedOrder>, takerTokenCancelAmounts: BigNumber.BigNumber[]): Promise<void> {
-        const makers = _.map(orders, order => order.maker);
-        assert.isSameLength('orders', orders, 'takerTokenCancelAmounts', takerTokenCancelAmounts);
-        assert.assert(!_.isEmpty(orders), 'Can not cancel an empty batch');
+    public async batchCancelOrderAsync(cancellationRequestsBatch: OrderCancellationRequest[]): Promise<void> {
+        const makers = _.map(cancellationRequestsBatch, cancellationRequest => cancellationRequest.order.maker);
+        assert.assert(!_.isEmpty(cancellationRequestsBatch), 'Can not cancel an empty batch');
         assert.assert(_.uniq(makers).length === 1, 'Can not cancel orders from multiple makers in a single batch');
         const maker = makers[0];
-        // _.zip doesn't type check if values have different types :'(
-        const ordersAndTakerTokenCancelAmounts = _.zip<any>(orders, takerTokenCancelAmounts);
-        _.forEach(ordersAndTakerTokenCancelAmounts,
-            async ([order, takerTokenCancelAmount]: [Order|SignedOrder, BigNumber.BigNumber]) => {
+        _.forEach(cancellationRequestsBatch,
+            async (cancellationRequest: OrderCancellationRequest) => {
             assert.doesConformToSchema('order',
-                SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), orderSchema);
-            assert.isBigNumber('takerTokenCancelAmount', takerTokenCancelAmount);
-            await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'order.maker', order.maker);
-            await this.validateCancelOrderAndThrowIfInvalidAsync(order, takerTokenCancelAmount);
+                SchemaValidator.convertToJSONSchemaCompatibleObject(cancellationRequest.order as object), orderSchema);
+            assert.isBigNumber('takerTokenCancelAmount', cancellationRequest.takerTokenCancelAmount);
+            await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'order.maker',
+                cancellationRequest.order.maker);
+            await this.validateCancelOrderAndThrowIfInvalidAsync(
+                cancellationRequest.order, cancellationRequest.takerTokenCancelAmount);
         });
         const exchangeInstance = await this.getExchangeContractAsync();
-        const orderAddressesAndValues = _.map(orders, order => {
-            return ExchangeWrapper.getOrderAddressesAndValues(order);
+        const orderAddressesValuesAndTakerTokenCancelAmounts = _.map(cancellationRequestsBatch, cancellationRequest => {
+            return [
+                ...ExchangeWrapper.getOrderAddressesAndValues(cancellationRequest.order),
+                cancellationRequest.takerTokenCancelAmount,
+            ];
         });
         // _.unzip doesn't type check if values have different types :'(
-        const [orderAddresses, orderValues] = _.unzip<any>(orderAddressesAndValues);
+        const [orderAddresses, orderValues, takerTokenCancelAmounts] =
+            _.unzip<any>(orderAddressesValuesAndTakerTokenCancelAmounts);
         const gas = await exchangeInstance.batchCancel.estimateGas(
             orderAddresses,
             orderValues,
diff --git a/src/types.ts b/src/types.ts
index 6c217fe51..1034893a4 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -222,3 +222,8 @@ export interface SubscriptionOpts {
 }
 
 export type DoneCallback = (err?: Error) => void;
+
+export interface OrderCancellationRequest {
+    order: Order|SignedOrder;
+    takerTokenCancelAmount: BigNumber.BigNumber;
+}
diff --git a/src/utils/assert.ts b/src/utils/assert.ts
index 61b7527e6..4dc6945a2 100644
--- a/src/utils/assert.ts
+++ b/src/utils/assert.ts
@@ -42,12 +42,6 @@ export const assert = {
         const availableAddresses = await web3Wrapper.getAvailableAddressesAsync();
         this.assert(!_.isEmpty(availableAddresses), 'No addresses were available on the provided web3 instance');
     },
-    isSameLength(variableName1: string, value1: any[], variableName2: string, value2: any[]) {
-        const length1 = value1.length;
-        const length2 = value2.length;
-        this.assert(length1 === length2, `${variableName1} and ${variableName2} length mismatch. \
-${length1} != ${length2}`);
-    },
     isNumber(variableName: string, value: number): void {
         this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value));
     },
diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts
index 6487ba98c..68224a68c 100644
--- a/test/exchange_wrapper_test.ts
+++ b/test/exchange_wrapper_test.ts
@@ -16,7 +16,7 @@ import {
     ExchangeEvents,
     ContractEvent,
     DoneCallback,
-    ExchangeContractErrs,
+    ExchangeContractErrs, OrderCancellationRequest,
 } from '../src/types';
 import {FillScenarios} from './utils/fill_scenarios';
 import {TokenUtils} from './utils/token_utils';
@@ -377,34 +377,44 @@ describe('ExchangeWrapper', () => {
         describe('#batchCancelOrderAsync', () => {
             let anotherSignedOrder: SignedOrder;
             let anotherOrderHashHex: string;
+            let cancelBatch: OrderCancellationRequest[];
             beforeEach(async () => {
                 anotherSignedOrder = await fillScenarios.createFillableSignedOrderAsync(
                     makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
                 );
                 anotherOrderHashHex = await zeroEx.getOrderHashHexAsync(anotherSignedOrder);
+                cancelBatch = [
+                    {
+                        order: signedOrder,
+                        takerTokenCancelAmount: cancelAmount,
+                    },
+                    {
+                        order: anotherSignedOrder,
+                        takerTokenCancelAmount: cancelAmount,
+                    },
+                ];
             });
             describe('failed batch cancels', () => {
-                it('should throw when length of orders and cancelAmounts mismatch', async () => {
-                    return expect(zeroEx.exchange.batchCancelOrderAsync([signedOrder], []))
-                        .to.be.rejectedWith('orders and takerTokenCancelAmounts length mismatch. 1 != 0');
-                });
                 it('should throw when orders are empty', async () => {
-                    return expect(zeroEx.exchange.batchCancelOrderAsync([], []))
+                    return expect(zeroEx.exchange.batchCancelOrderAsync([]))
                         .to.be.rejectedWith('Can not cancel an empty batch');
                 });
                 it.only('should throw when orders have different makers', async () => {
                     const signedOrderWithADifferentMaker = await fillScenarios.createFillableSignedOrderAsync(
                         makerTokenAddress, takerTokenAddress, takerAddress, takerAddress, fillableAmount,
                     );
-                    return expect(zeroEx.exchange.batchCancelOrderAsync(
-                        [signedOrder, signedOrderWithADifferentMaker], [cancelAmount, cancelAmount]))
-                        .to.be.rejectedWith('Can not cancel orders from multiple makers in a single batch');
+                    return expect(zeroEx.exchange.batchCancelOrderAsync([
+                        cancelBatch[0],
+                        {
+                            order: signedOrderWithADifferentMaker,
+                            takerTokenCancelAmount: cancelAmount,
+                        },
+                    ])).to.be.rejectedWith('Can not cancel orders from multiple makers in a single batch');
                 });
             });
             describe('successful batch cancels', () => {
                 it('should cancel a batch of orders', async () => {
-                    await zeroEx.exchange.batchCancelOrderAsync(
-                        [signedOrder, anotherSignedOrder], [cancelAmount, cancelAmount]);
+                    await zeroEx.exchange.batchCancelOrderAsync(cancelBatch);
                     const cancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(orderHashHex);
                     const anotherCancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(
                         anotherOrderHashHex);