From 1d4506427fd3eb277f42c02887f33265dac6a783 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 27 Sep 2017 13:28:11 -0700 Subject: Add OrderTransactionOpts to enable optional validation to exchange_wrapper --- src/contract_wrappers/exchange_wrapper.ts | 91 ++++++++++++++++++++++--------- src/types.ts | 8 +++ 2 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 6d4d90441..c7d095ac7 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -29,6 +29,7 @@ import { LogWithDecodedArgs, MethodOpts, ValidateOrderFillableOpts, + OrderTransactionOpts, } from '../types'; import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; @@ -154,19 +155,24 @@ export class ExchangeWrapper extends ContractWrapper { * @param takerAddress The user Ethereum address who would like to fill this order. * Must be available via the supplied Web3.Provider * passed to 0x.js. - * @return Transaction hash. + * @param orderTransactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler public async fillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber.BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, - takerAddress: string): Promise { + takerAddress: string, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema); assert.isBigNumber('fillTakerTokenAmount', fillTakerTokenAmount); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - await this.validateFillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await this.validateFillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); + } const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(signedOrder); @@ -211,12 +217,14 @@ export class ExchangeWrapper extends ContractWrapper { * @param takerAddress The user Ethereum address who would like to fill these * orders. Must be available via the supplied Web3.Provider * passed to 0x.js. - * @return Transaction hash. + * @param orderTransactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler public async fillOrdersUpToAsync(signedOrders: SignedOrder[], fillTakerTokenAmount: BigNumber.BigNumber, shouldThrowOnInsufficientBalanceOrAllowance: boolean, - takerAddress: string): Promise { + takerAddress: string, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('signedOrders', signedOrders, schemas.signedOrdersSchema); const takerTokenAddresses = _.map(signedOrders, signedOrder => signedOrder.takerTokenAddress); assert.hasAtMostOneUniqueValue(takerTokenAddresses, @@ -227,10 +235,13 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBigNumber('fillTakerTokenAmount', fillTakerTokenAmount); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - for (const signedOrder of signedOrders) { - await this.validateFillOrderThrowIfInvalidAsync( - signedOrder, fillTakerTokenAmount, takerAddress); + + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await Promise.all(signedOrders.map(signedOrder => this.validateFillOrderThrowIfInvalidAsync( + signedOrder, fillTakerTokenAmount, takerAddress))); } + if (_.isEmpty(signedOrders)) { throw new Error(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); } @@ -292,12 +303,14 @@ export class ExchangeWrapper extends ContractWrapper { * @param takerAddress The user Ethereum address who would like to fill * these orders. Must be available via the supplied * Web3.Provider passed to 0x.js. - * @return Transaction hash. + * @param orderTransactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler public async batchFillOrdersAsync(orderFillRequests: OrderFillRequest[], shouldThrowOnInsufficientBalanceOrAllowance: boolean, - takerAddress: string): Promise { + takerAddress: string, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('orderFillRequests', orderFillRequests, schemas.orderFillRequestsSchema); const exchangeContractAddresses = _.map( orderFillRequests, @@ -307,9 +320,10 @@ export class ExchangeWrapper extends ContractWrapper { ExchangeContractErrs.BatchOrdersMustHaveSameExchangeAddress); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - for (const orderFillRequest of orderFillRequests) { - await this.validateFillOrderThrowIfInvalidAsync( - orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress); + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await Promise.all(orderFillRequests.map(orderFillRequest => this.validateFillOrderThrowIfInvalidAsync( + orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress))); } if (_.isEmpty(orderFillRequests)) { throw new Error(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); @@ -365,18 +379,23 @@ export class ExchangeWrapper extends ContractWrapper { * @param fillTakerTokenAmount The total amount of the takerTokens you would like to fill. * @param takerAddress The user Ethereum address who would like to fill this order. * Must be available via the supplied Web3.Provider passed to 0x.js. - * @return Transaction hash. + * @param orderTransactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler public async fillOrKillOrderAsync(signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber.BigNumber, - takerAddress: string): Promise { + takerAddress: string, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema); assert.isBigNumber('fillTakerTokenAmount', fillTakerTokenAmount); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - await this.validateFillOrKillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await this.validateFillOrKillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); + } const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(signedOrder); @@ -411,11 +430,13 @@ export class ExchangeWrapper extends ContractWrapper { * @param orderFillOrKillRequests An array of objects that conform to the OrderFillOrKillRequest interface. * @param takerAddress The user Ethereum address who would like to fill there orders. * Must be available via the supplied Web3.Provider passed to 0x.js. - * @return Transaction hash. + * @param orderTransactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler public async batchFillOrKillAsync(orderFillOrKillRequests: OrderFillOrKillRequest[], - takerAddress: string): Promise { + takerAddress: string, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('orderFillOrKillRequests', orderFillOrKillRequests, schemas.orderFillOrKillRequestsSchema); const exchangeContractAddresses = _.map( @@ -429,9 +450,11 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); } const exchangeInstance = await this._getExchangeContractAsync(); - for (const request of orderFillOrKillRequests) { - await this.validateFillOrKillOrderThrowIfInvalidAsync( - request.signedOrder, request.fillTakerAmount, takerAddress); + + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await Promise.all(orderFillOrKillRequests.map(request => this.validateFillOrKillOrderThrowIfInvalidAsync( + request.signedOrder, request.fillTakerAmount, takerAddress))); } const orderAddressesValuesAndTakerTokenFillAmounts = _.map(orderFillOrKillRequests, request => { @@ -478,17 +501,23 @@ export class ExchangeWrapper extends ContractWrapper { * @param order An object that conforms to the Order or SignedOrder interface. * The order you would like to cancel. * @param cancelTakerTokenAmount The amount (specified in taker tokens) that you would like to cancel. - * @return Transaction hash. + * @param transactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler - public async cancelOrderAsync( - order: Order|SignedOrder, cancelTakerTokenAmount: BigNumber.BigNumber): Promise { + public async cancelOrderAsync(order: Order|SignedOrder, + cancelTakerTokenAmount: BigNumber.BigNumber, + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('order', order, schemas.orderSchema); assert.isBigNumber('takerTokenCancelAmount', cancelTakerTokenAmount); await assert.isSenderAddressAsync('order.maker', order.maker, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - await this.validateCancelOrderThrowIfInvalidAsync(order, cancelTakerTokenAmount); + + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await this.validateCancelOrderThrowIfInvalidAsync(order, cancelTakerTokenAmount); + } const [orderAddresses, orderValues] = ExchangeWrapper._getOrderAddressesAndValues(order); const gas = await exchangeInstance.cancelOrder.estimateGasAsync( @@ -515,10 +544,12 @@ export class ExchangeWrapper extends ContractWrapper { * All orders must be from the same maker. * @param orderCancellationRequests An array of objects that conform to the OrderCancellationRequest * interface. - * @return Transaction hash. + * @param transactionOpts Optional arguments this method accepts. + * @return Transaction hash. */ @decorators.contractCallErrorHandler - public async batchCancelOrdersAsync(orderCancellationRequests: OrderCancellationRequest[]): Promise { + public async batchCancelOrdersAsync(orderCancellationRequests: OrderCancellationRequest[], + orderTransactionOpts?: OrderTransactionOpts): Promise { assert.doesConformToSchema('orderCancellationRequests', orderCancellationRequests, schemas.orderCancellationRequestsSchema); const exchangeContractAddresses = _.map( @@ -531,6 +562,12 @@ export class ExchangeWrapper extends ContractWrapper { assert.hasAtMostOneUniqueValue(makers, ExchangeContractErrs.MultipleMakersInSingleCancelBatchDisallowed); const maker = makers[0]; await assert.isSenderAddressAsync('maker', maker, this._web3Wrapper); + const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + if (shouldValidate) { + await Promise.all(orderCancellationRequests.map(cancellationRequest => + this.validateCancelOrderThrowIfInvalidAsync( + cancellationRequest.order, cancellationRequest.takerTokenCancelAmount))); + } for (const cancellationRequest of orderCancellationRequests) { await this.validateCancelOrderThrowIfInvalidAsync( cancellationRequest.order, cancellationRequest.takerTokenCancelAmount, diff --git a/src/types.ts b/src/types.ts index 2d069f596..02230b0ab 100644 --- a/src/types.ts +++ b/src/types.ts @@ -452,3 +452,11 @@ export interface ValidateOrderFillableOpts { export interface MethodOpts { defaultBlock?: Web3.BlockParam; } + +/* + * shouldValidate: Flag indicating whether the library should make attempts to validate a transaction before + * broadcasting it. For example, order has a valid signature, maker has sufficient funds, etc. + */ +export interface OrderTransactionOpts { + shouldValidate: boolean; +} -- cgit v1.2.3 From 333665370fda26753e137f5b11464493b7d5880c Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 27 Sep 2017 16:34:01 -0700 Subject: Add tests --- src/contract_wrappers/exchange_wrapper.ts | 5 - test/exchange_wrapper_test.ts | 234 +++++++++++++++++++++++++++--- 2 files changed, 216 insertions(+), 23 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index c7d095ac7..2ab1635c1 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -568,11 +568,6 @@ export class ExchangeWrapper extends ContractWrapper { this.validateCancelOrderThrowIfInvalidAsync( cancellationRequest.order, cancellationRequest.takerTokenCancelAmount))); } - for (const cancellationRequest of orderCancellationRequests) { - await this.validateCancelOrderThrowIfInvalidAsync( - cancellationRequest.order, cancellationRequest.takerTokenCancelAmount, - ); - } if (_.isEmpty(orderCancellationRequests)) { throw new Error(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); } diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 9c0617671..ec0a05a76 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -18,6 +18,7 @@ import { OrderCancellationRequest, OrderFillRequest, LogFillContractEventArgs, + OrderFillOrKillRequest, } from '../src'; import {DoneCallback} from '../src/types'; import {FillScenarios} from './utils/fill_scenarios'; @@ -93,14 +94,51 @@ describe('ExchangeWrapper', () => { ]; await zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress); }); + describe('order transaction options', () => { + let signedOrder: SignedOrder; + let orderFillOrKillRequests: OrderFillOrKillRequest[]; + const fillableAmount = new BigNumber(5); + beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + orderFillOrKillRequests = [ + { + signedOrder, + fillTakerAmount: new BigNumber(0), + }, + ]; + }); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress)) + .to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + }); }); describe('#fillOrKillOrderAsync', () => { + let signedOrder: SignedOrder; + const fillableAmount = new BigNumber(5); + beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); describe('successful fills', () => { it('should fill a valid order', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) .to.be.bignumber.equal(fillableAmount); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) @@ -120,10 +158,6 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); }); it('should partially fill a valid order', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); const partialFillAmount = new BigNumber(3); await zeroEx.exchange.fillOrKillOrderAsync(signedOrder, partialFillAmount, takerAddress); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) @@ -136,6 +170,27 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); }); }); + describe('order transaction options', () => { + const emptyFillableAmount = new BigNumber(0); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress)) + .to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + }); }); }); describe('fill order(s)', () => { @@ -212,6 +267,34 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(makerFee.plus(takerFee)); }); }); + describe('order transaction options', () => { + let signedOrder: SignedOrder; + const emptyFillTakerAmount = new BigNumber(0); + beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, emptyFillTakerAmount, false, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.fillOrderAsync(signedOrder, emptyFillTakerAmount, false, takerAddress, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.fillOrderAsync(signedOrder, emptyFillTakerAmount, false, takerAddress, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + }); }); describe('#batchFillOrdersAsync', () => { let signedOrder: SignedOrder; @@ -228,18 +311,20 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); anotherOrderHashHex = ZeroEx.getOrderHashHex(anotherSignedOrder); - orderFillBatch = [ - { - signedOrder, - takerTokenFillAmount: fillTakerAmount, - }, - { - signedOrder: anotherSignedOrder, - takerTokenFillAmount: fillTakerAmount, - }, - ]; }); describe('successful batch fills', () => { + beforeEach(() => { + orderFillBatch = [ + { + signedOrder, + takerTokenFillAmount: fillTakerAmount, + }, + { + signedOrder: anotherSignedOrder, + takerTokenFillAmount: fillTakerAmount, + }, + ]; + }); it('should throw if a batch is empty', async () => { return expect(zeroEx.exchange.batchFillOrdersAsync( [], shouldThrowOnInsufficientBalanceOrAllowance, takerAddress), @@ -255,6 +340,42 @@ describe('ExchangeWrapper', () => { expect(anotherFilledAmount).to.be.bignumber.equal(fillTakerAmount); }); }); + describe('order transaction options', () => { + beforeEach(async () => { + const emptyFillTakerAmount = new BigNumber(0); + orderFillBatch = [ + { + signedOrder, + takerTokenFillAmount: emptyFillTakerAmount, + }, + { + signedOrder: anotherSignedOrder, + takerTokenFillAmount: emptyFillTakerAmount, + }, + ]; + }); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.batchFillOrdersAsync( + orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress), + ).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.batchFillOrdersAsync( + orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.batchFillOrdersAsync( + orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + }); }); describe('#fillOrdersUpTo', () => { let signedOrder: SignedOrder; @@ -292,6 +413,30 @@ describe('ExchangeWrapper', () => { expect(anotherFilledAmount).to.be.bignumber.equal(remainingFillAmount); }); }); + describe('order transaction options', () => { + const emptyFillUpToAmount = new BigNumber(0); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.fillOrdersUpToAsync( + signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.fillOrdersUpToAsync( + signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.fillOrdersUpToAsync( + signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + }); + }); }); }); describe('cancel order(s)', () => { @@ -323,6 +468,26 @@ describe('ExchangeWrapper', () => { expect(cancelledAmount).to.be.bignumber.equal(cancelAmount); }); }); + describe('order transaction options', () => { + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0))) + .to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0), + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0), + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + }); }); describe('#batchCancelOrdersAsync', () => { let anotherSignedOrder: SignedOrder; @@ -369,6 +534,39 @@ describe('ExchangeWrapper', () => { expect(anotherCancelledAmount).to.be.bignumber.equal(cancelAmount); }); }); + describe('order transaction options', () => { + beforeEach(async () => { + const emptyTakerTokenCancelAmount = new BigNumber(0); + cancelBatch = [ + { + order: signedOrder, + takerTokenCancelAmount: emptyTakerTokenCancelAmount, + }, + { + order: anotherSignedOrder, + takerTokenCancelAmount: emptyTakerTokenCancelAmount, + }, + ]; + }); + it('should validate when orderTransactionOptions are not present', async () => { + return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch)) + .to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + it('should validate when orderTransactionOptions specify to validate', async () => { + return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, + { + shouldValidate: true, + }, + )).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + it('should not validate when orderTransactionOptions specify not to validate', async () => { + return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, + { + shouldValidate: false, + }, + )).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + }); + }); }); }); describe('tests that require partially filled order', () => { -- cgit v1.2.3 From d21fbbc4c83cd7dd434f7e8389d48ed35fe0a5b0 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 28 Sep 2017 08:44:29 -0700 Subject: Fixed nits --- src/contract_wrappers/exchange_wrapper.ts | 36 +++++++--- test/exchange_wrapper_test.ts | 107 ++++++++++++------------------ 2 files changed, 68 insertions(+), 75 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 2ab1635c1..d02a6e642 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -41,6 +41,8 @@ import {TokenWrapper} from './token_wrapper'; import {decorators} from '../utils/decorators'; import {artifacts} from '../artifacts'; +const SHOULD_VALIDATE_BY_DEFAULT = true; + /** * This class includes all the functionality related to calling methods and subscribing to * events of the 0x Exchange smart contract. @@ -169,7 +171,9 @@ export class ExchangeWrapper extends ContractWrapper { await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await this.validateFillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); } @@ -236,7 +240,9 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await Promise.all(signedOrders.map(signedOrder => this.validateFillOrderThrowIfInvalidAsync( signedOrder, fillTakerTokenAmount, takerAddress))); @@ -320,10 +326,13 @@ export class ExchangeWrapper extends ContractWrapper { ExchangeContractErrs.BatchOrdersMustHaveSameExchangeAddress); assert.isBoolean('shouldThrowOnInsufficientBalanceOrAllowance', shouldThrowOnInsufficientBalanceOrAllowance); await assert.isSenderAddressAsync('takerAddress', takerAddress, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await Promise.all(orderFillRequests.map(orderFillRequest => this.validateFillOrderThrowIfInvalidAsync( - orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress))); + orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress)), + ); } if (_.isEmpty(orderFillRequests)) { throw new Error(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); @@ -392,7 +401,9 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await this.validateFillOrKillOrderThrowIfInvalidAsync(signedOrder, fillTakerTokenAmount, takerAddress); } @@ -451,10 +462,13 @@ export class ExchangeWrapper extends ContractWrapper { } const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await Promise.all(orderFillOrKillRequests.map(request => this.validateFillOrKillOrderThrowIfInvalidAsync( - request.signedOrder, request.fillTakerAmount, takerAddress))); + request.signedOrder, request.fillTakerAmount, takerAddress)), + ); } const orderAddressesValuesAndTakerTokenFillAmounts = _.map(orderFillOrKillRequests, request => { @@ -514,7 +528,9 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this._getExchangeContractAsync(); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await this.validateCancelOrderThrowIfInvalidAsync(order, cancelTakerTokenAmount); } @@ -562,7 +578,9 @@ export class ExchangeWrapper extends ContractWrapper { assert.hasAtMostOneUniqueValue(makers, ExchangeContractErrs.MultipleMakersInSingleCancelBatchDisallowed); const maker = makers[0]; await assert.isSenderAddressAsync('maker', maker, this._web3Wrapper); - const shouldValidate = _.isUndefined(orderTransactionOpts) ? true : orderTransactionOpts.shouldValidate; + const shouldValidate = _.isUndefined(orderTransactionOpts) ? + SHOULD_VALIDATE_BY_DEFAULT : + orderTransactionOpts.shouldValidate; if (shouldValidate) { await Promise.all(orderCancellationRequests.map(cancellationRequest => this.validateCancelOrderThrowIfInvalidAsync( diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index ec0a05a76..45a2d3907 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -114,18 +114,14 @@ describe('ExchangeWrapper', () => { .to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should validate when orderTransactionOptions specify to validate', async () => { - return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, - { - shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, { + shouldValidate: true, + })).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { - return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, - { - shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + return expect(zeroEx.exchange.batchFillOrKillAsync(orderFillOrKillRequests, takerAddress, { + shouldValidate: false, + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); }); @@ -177,18 +173,14 @@ describe('ExchangeWrapper', () => { .to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should validate when orderTransactionOptions specify to validate', async () => { - return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, - { - shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, { + shouldValidate: true, + })).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { - return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, - { - shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + return expect(zeroEx.exchange.fillOrKillOrderAsync(signedOrder, emptyFillableAmount, takerAddress, { + shouldValidate: false, + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); }); @@ -277,22 +269,20 @@ describe('ExchangeWrapper', () => { }); it('should validate when orderTransactionOptions are not present', async () => { return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, emptyFillTakerAmount, false, takerAddress, + signedOrder, emptyFillTakerAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should validate when orderTransactionOptions specify to validate', async () => { - return expect(zeroEx.exchange.fillOrderAsync(signedOrder, emptyFillTakerAmount, false, takerAddress, - { + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, emptyFillTakerAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { - return expect(zeroEx.exchange.fillOrderAsync(signedOrder, emptyFillTakerAmount, false, takerAddress, - { + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, emptyFillTakerAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); }); @@ -361,19 +351,15 @@ describe('ExchangeWrapper', () => { }); it('should validate when orderTransactionOptions specify to validate', async () => { return expect(zeroEx.exchange.batchFillOrdersAsync( - orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, - { + orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.batchFillOrdersAsync( - orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, - { + orderFillBatch, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); }); @@ -422,19 +408,15 @@ describe('ExchangeWrapper', () => { }); it('should validate when orderTransactionOptions specify to validate', async () => { return expect(zeroEx.exchange.fillOrdersUpToAsync( - signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, - { + signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { return expect(zeroEx.exchange.fillOrdersUpToAsync( - signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, - { + signedOrders, emptyFillUpToAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress, { shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderFillAmountZero); }); }); }); @@ -469,23 +451,20 @@ describe('ExchangeWrapper', () => { }); }); describe('order transaction options', () => { + const emptyCancelTakerTokenAmount = new BigNumber(0); it('should validate when orderTransactionOptions are not present', async () => { - return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0))) + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, emptyCancelTakerTokenAmount)) .to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); it('should validate when orderTransactionOptions specify to validate', async () => { - return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0), - { - shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, emptyCancelTakerTokenAmount, { + shouldValidate: true, + })).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { - return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, new BigNumber(0), - { - shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, emptyCancelTakerTokenAmount, { + shouldValidate: false, + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); }); @@ -553,18 +532,14 @@ describe('ExchangeWrapper', () => { .to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); it('should validate when orderTransactionOptions specify to validate', async () => { - return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, - { - shouldValidate: true, - }, - )).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, { + shouldValidate: true, + })).to.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); it('should not validate when orderTransactionOptions specify not to validate', async () => { - return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, - { - shouldValidate: false, - }, - )).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); + return expect(zeroEx.exchange.batchCancelOrdersAsync(cancelBatch, { + shouldValidate: false, + })).to.not.be.rejectedWith(ExchangeContractErrs.OrderCancelAmountZero); }); }); }); -- cgit v1.2.3