From c3cd5812e61359b18c469f70c9f0240db777e67a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 14:40:28 +0200 Subject: Refactor to use OrderCancellationRequest --- src/contract_wrappers/exchange_wrapper.ts | 36 ++++++++++++++++--------------- src/types.ts | 5 +++++ src/utils/assert.ts | 6 ------ test/exchange_wrapper_test.ts | 32 +++++++++++++++++---------- 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, takerTokenCancelAmounts: BigNumber.BigNumber[]): Promise { - 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 { + 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(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(orderAddressesAndValues); + const [orderAddresses, orderValues, takerTokenCancelAmounts] = + _.unzip(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); -- cgit v1.2.3