From 491a6e3c04c3d9a63930eaeba6fb4632f3ae9cb1 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 11:50:23 +0200 Subject: Add initial implementation of batchCancelAsync --- src/contract_wrappers/exchange_wrapper.ts | 64 +++++++++++++++++++++---------- src/types.ts | 6 +++ src/utils/assert.ts | 6 +++ 3 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index fe03dbd16..bd057eacd 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -134,37 +134,59 @@ export class ExchangeWrapper extends ContractWrapper { */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, takerAddress: string): Promise { - assert.doesConformToSchema('signedOrder', - SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), - signedOrderSchema); - assert.isBigNumber('fillTakerAmount', fillTakerAmount); + await this.batchFillOrderAsync([signedOrder], [fillTakerAmount], shouldCheckTransfer, takerAddress); + } + /** + * Batched version of fillOrderAsync. Executes fills atomically in a single transaction. + */ + public async batchFillOrderAsync(signedOrders: SignedOrder[], fillTakerAmounts: BigNumber.BigNumber[], + shouldCheckTransfer: boolean, takerAddress: string): Promise { + assert.isSameLength('signedOrders', signedOrders, 'fillTakerAmounts', fillTakerAmounts); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); - + // _.zip doesn't type check if values have different types :'( + const ordersAndAmounts = _.zip(signedOrders, fillTakerAmounts); + _.forEach(ordersAndAmounts, + async ([signedOrder, fillTakerAmount]: [SignedOrder, BigNumber.BigNumber]) => { + assert.doesConformToSchema('signedOrder', + SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); + assert.isBigNumber('fillTakerAmount', fillTakerAmount); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); + }); const exchangeInstance = await this.getExchangeContractAsync(); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); - const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(signedOrder); - const gas = await exchangeInstance.fill.estimateGas( - orderAddresses, - orderValues, - fillTakerAmount, + const orderAddressesValuesAndSignatureArray = _.map(signedOrders, signedOrder => { + return [ + ...ExchangeWrapper.getOrderAddressesAndValues(signedOrder), + signedOrder.ecSignature.v, + signedOrder.ecSignature.r, + signedOrder.ecSignature.s, + ]; + }); + // _.unzip doesn't type check if values have different types :'( + const [orderAddressesArray, orderValuesArray, vArray, rArray, sArray] = _.unzip( + orderAddressesValuesAndSignatureArray, + ); + const gas = await exchangeInstance.batchFill.estimateGas( + orderAddressesArray, + orderValuesArray, + fillTakerAmounts, shouldCheckTransfer, - signedOrder.ecSignature.v, - signedOrder.ecSignature.r, - signedOrder.ecSignature.s, + vArray, + rArray, + sArray, { from: takerAddress, }, ); - const response: ContractResponse = await exchangeInstance.fill( - orderAddresses, - orderValues, - fillTakerAmount, + const response: ContractResponse = await exchangeInstance.batchFill( + orderAddressesArray, + orderValuesArray, + fillTakerAmounts, shouldCheckTransfer, - signedOrder.ecSignature.v, - signedOrder.ecSignature.r, - signedOrder.ecSignature.s, + vArray, + rArray, + sArray, { from: takerAddress, gas, diff --git a/src/types.ts b/src/types.ts index aa699110b..0623ec6a7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -71,6 +71,12 @@ export interface ExchangeContract extends ContractInstance { estimateGas: (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts?: TxOpts) => number; }; + batchFill: { + (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmount: BigNumber.BigNumber[], + shouldCheckTransfer: boolean, v: number[], r: string[], s: string[], txOpts?: TxOpts): ContractResponse; + estimateGas: (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmount: BigNumber.BigNumber[], + shouldCheckTransfer: boolean, v: number[], r: string[], s: string[], txOpts?: TxOpts) => number; + }; cancel: { (orderAddresses: OrderAddresses, orderValues: OrderValues, cancelAmount: BigNumber.BigNumber, txOpts?: TxOpts): ContractResponse; diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 4dc6945a2..3842185bb 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -42,6 +42,12 @@ 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)); }, -- cgit From 879b2870e153d71d75c0cd8943a287e360faba6f Mon Sep 17 00:00:00 2001 From: Leonid Date: Wed, 7 Jun 2017 12:14:25 +0200 Subject: Update 0x.js.ts --- src/0x.js.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index eb7698bd4..fa52d4ad8 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -14,8 +14,7 @@ import {ExchangeWrapper} from './contract_wrappers/exchange_wrapper'; import {TokenRegistryWrapper} from './contract_wrappers/token_registry_wrapper'; import {ecSignatureSchema} from './schemas/ec_signature_schema'; import {TokenWrapper} from './contract_wrappers/token_wrapper'; -import {ECSignature, ZeroExError} from './types'; -import {Order, SignedOrder} from './types'; +import {ECSignature, ZeroExError, Order, SignedOrder} from './types'; import * as ExchangeArtifacts from './artifacts/Exchange.json'; // Customize our BigNumber instances -- cgit From 65beef5b9cf3a859ec2d24a7b68d02e6d8425fba Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 12:16:07 +0200 Subject: Use ContractInstance from globals --- src/globals.d.ts | 4 +++- src/types.ts | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/globals.d.ts b/src/globals.d.ts index 164fc2386..567ba016d 100644 --- a/src/globals.d.ts +++ b/src/globals.d.ts @@ -47,7 +47,9 @@ declare module 'ethereumjs-util' { } // truffle-contract declarations -declare interface ContractInstance {} +declare interface ContractInstance { + address: string; +} declare interface ContractFactory { setProvider: (providerObj: any) => void; deployed: () => ContractInstance; diff --git a/src/types.ts b/src/types.ts index 0623ec6a7..877864f49 100644 --- a/src/types.ts +++ b/src/types.ts @@ -44,9 +44,6 @@ export interface ContractEventObj { } export type CreateContractEvent = (indexFilterValues: IndexFilterValues, subscriptionOpts: SubscriptionOpts) => ContractEventObj; -export interface ContractInstance { - address: string; -} export interface ExchangeContract extends ContractInstance { isValidSignature: { call: (signerAddressHex: string, dataHex: string, v: number, r: string, s: string, -- cgit From 8c44b102fc48a4ba8a662be3b603ce2af0e8acf6 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 12:20:28 +0200 Subject: Remove assertions from utils methods --- src/0x.js.ts | 4 ++++ src/contract_wrappers/exchange_wrapper.ts | 5 +---- src/utils/utils.ts | 7 ------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index fa52d4ad8..7b53b70ea 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -16,6 +16,8 @@ import {ecSignatureSchema} from './schemas/ec_signature_schema'; import {TokenWrapper} from './contract_wrappers/token_wrapper'; import {ECSignature, ZeroExError, Order, SignedOrder} from './types'; import * as ExchangeArtifacts from './artifacts/Exchange.json'; +import {SchemaValidator} from './utils/schema_validator'; +import {orderSchema} from './schemas/order_schemas'; // Customize our BigNumber instances bigNumberConfigs.configure(); @@ -128,6 +130,8 @@ export class ZeroEx { * Computes the orderHash for a given order and returns it as a hex encoded string. */ public async getOrderHashHexAsync(order: Order|SignedOrder): Promise { + assert.doesConformToSchema( + 'order', SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), orderSchema); const exchangeContractAddr = await this.getExchangeAddressAsync(); const hashHex = utils.getOrderHashHex(order, exchangeContractAddr); return hashHex; diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index bd057eacd..408d3deed 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -253,10 +253,7 @@ export class ExchangeWrapper extends ContractWrapper { logEventObj.watch(callback); this.exchangeLogEventObjs.push(logEventObj); } - /** - * Computes the orderHash for a given order and returns it as a hex encoded string. - */ - public async getOrderHashAsync(order: Order|SignedOrder): Promise { + private async getOrderHashAsync(order: Order|SignedOrder): Promise { const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const exchangeInstance = await this.getExchangeContractAsync(); const orderHash = utils.getOrderHashHex(order, exchangeInstance.address); diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 0da83c366..5786bab07 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -2,10 +2,7 @@ import * as _ from 'lodash'; import * as BN from 'bn.js'; import * as ethABI from 'ethereumjs-abi'; import * as ethUtil from 'ethereumjs-util'; -import {orderSchema} from '../schemas/order_schemas'; -import {SchemaValidator} from './schema_validator'; import {Order, SignedOrder, SolidityTypes} from '../types'; -import {assert} from './assert'; import * as BigNumber from 'bignumber.js'; export const utils = { @@ -33,10 +30,6 @@ export const utils = { return new Error(`Unexpected switch value: ${value} encountered for ${name}`); }, getOrderHashHex(order: Order|SignedOrder, exchangeContractAddr: string): string { - assert.doesConformToSchema('order', - SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), - orderSchema); - const orderParts = [ {value: exchangeContractAddr, type: SolidityTypes.address}, {value: order.maker, type: SolidityTypes.address}, -- cgit From 87e0eceb9b5b4460e0875bedcd3a5864ab870e12 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 12:21:53 +0200 Subject: Add comments for dates --- test/exchange_wrapper_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 3a3ec1ca7..c0068c58a 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -158,7 +158,7 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { - const expirationInPast = new BigNumber(1496826058); + const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, @@ -350,7 +350,7 @@ describe('ExchangeWrapper', () => { .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); }); it('should throw when order is expired', async () => { - const expirationInPast = new BigNumber(1496826058); + const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 const expiredSignedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); -- cgit From d55213e1aa1bf5d934afaef3f28d4b5c7a5cc7f9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 12:39:44 +0200 Subject: Add initial batchCancelAsync implementation --- src/contract_wrappers/exchange_wrapper.ts | 45 +++++++++++++++++++++++++++++++ src/types.ts | 6 +++++ 2 files changed, 51 insertions(+) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 408d3deed..1a675ee8e 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -227,6 +227,51 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } + /** + * 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'); + 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]) => { + 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); + }); + const exchangeInstance = await this.getExchangeContractAsync(); + const orderAddressesAndValues = _.map(orders, order => { + return ExchangeWrapper.getOrderAddressesAndValues(order); + }); + // _.unzip doesn't type check if values have different types :'( + const [orderAddresses, orderValues] = _.unzip(orderAddressesAndValues); + const gas = await exchangeInstance.batchCancel.estimateGas( + orderAddresses, + orderValues, + takerTokenCancelAmounts, + { + from: maker, + }, + ); + const response: ContractResponse = await exchangeInstance.batchCancel( + orderAddresses, + orderValues, + takerTokenCancelAmounts, + { + from: maker, + gas, + }, + ); + this.throwErrorLogsAsErrors(response.logs); + } /** * Subscribe to an event type emitted by the Exchange smart contract */ diff --git a/src/types.ts b/src/types.ts index 877864f49..a1bd6d19e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -80,6 +80,12 @@ export interface ExchangeContract extends ContractInstance { estimateGas: (orderAddresses: OrderAddresses, orderValues: OrderValues, cancelAmount: BigNumber.BigNumber, txOpts?: TxOpts) => number; }; + batchCancel: { + (orderAddresses: OrderAddresses[], orderValues: OrderValues[], cancelAmount: BigNumber.BigNumber[], + txOpts?: TxOpts): ContractResponse; + estimateGas: (orderAddresses: OrderAddresses[], orderValues: OrderValues[], cancelAmount: BigNumber.BigNumber[], + txOpts?: TxOpts) => number; + }; filled: { call: (orderHash: string) => BigNumber.BigNumber; }; -- cgit From 2947f55acf932703a54d3177b0c4d9e115c861c2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:20:02 +0200 Subject: Add tests for batchCancelAsync --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/utils/assert.ts | 4 +- test/exchange_wrapper_test.ts | 88 +++++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 26 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 1a675ee8e..690f90360 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -234,7 +234,7 @@ export class ExchangeWrapper extends ContractWrapper { 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'); + assert.assert(!_.isEmpty(orders), '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 :'( diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 3842185bb..61b7527e6 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -45,8 +45,8 @@ export const assert = { 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}`); + 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 c0068c58a..6487ba98c 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -323,7 +323,7 @@ describe('ExchangeWrapper', () => { }); }); }); - describe('#cancelOrderAsync', () => { + describe('cancel order(s)', () => { let makerTokenAddress: string; let takerTokenAddress: string; let coinbase: string; @@ -343,32 +343,74 @@ describe('ExchangeWrapper', () => { ); orderHashHex = await zeroEx.getOrderHashHexAsync(signedOrder); }); - describe('failed cancels', () => { - it('should throw when cancel amount is zero', async () => { - const zeroCancelAmount = new BigNumber(0); - return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, zeroCancelAmount)) - .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); + describe('#cancelOrderAsync', () => { + describe('failed cancels', () => { + it('should throw when cancel amount is zero', async () => { + const zeroCancelAmount = new BigNumber(0); + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, zeroCancelAmount)) + .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); + }); + it('should throw when order is expired', async () => { + const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 + const expiredSignedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + fillableAmount, expirationInPast, + ); + orderHashHex = await zeroEx.getOrderHashHexAsync(expiredSignedOrder); + return expect(zeroEx.exchange.cancelOrderAsync(expiredSignedOrder, cancelAmount)) + .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_EXPIRED); + }); + it('should throw when order is already cancelled or filled', async () => { + await zeroEx.exchange.cancelOrderAsync(signedOrder, fillableAmount); + return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, fillableAmount)) + .to.be.rejectedWith(ExchangeContractErrs.ORDER_ALREADY_CANCELLED_OR_FILLED); + }); }); - it('should throw when order is expired', async () => { - const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 - const expiredSignedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, + describe('successful cancels', () => { + it('should cancel an order', async () => { + await zeroEx.exchange.cancelOrderAsync(signedOrder, cancelAmount); + const cancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(orderHashHex); + expect(cancelledAmount).to.be.bignumber.equal(cancelAmount); + }); + }); + }); + describe('#batchCancelOrderAsync', () => { + let anotherSignedOrder: SignedOrder; + let anotherOrderHashHex: string; + beforeEach(async () => { + anotherSignedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - orderHashHex = await zeroEx.getOrderHashHexAsync(expiredSignedOrder); - return expect(zeroEx.exchange.cancelOrderAsync(expiredSignedOrder, cancelAmount)) - .to.be.rejectedWith(ExchangeContractErrs.ORDER_CANCEL_EXPIRED); + anotherOrderHashHex = await zeroEx.getOrderHashHexAsync(anotherSignedOrder); }); - it('should throw when order is already cancelled or filled', async () => { - await zeroEx.exchange.cancelOrderAsync(signedOrder, fillableAmount); - return expect(zeroEx.exchange.cancelOrderAsync(signedOrder, fillableAmount)) - .to.be.rejectedWith(ExchangeContractErrs.ORDER_ALREADY_CANCELLED_OR_FILLED); + 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([], [])) + .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'); + }); }); - }); - describe('successful cancels', () => { - it('should cancel an order', async () => { - await zeroEx.exchange.cancelOrderAsync(signedOrder, cancelAmount); - const cancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(orderHashHex); - expect(cancelledAmount).to.be.bignumber.equal(cancelAmount); + describe('successful batch cancels', () => { + it('should cancel a batch of orders', async () => { + await zeroEx.exchange.batchCancelOrderAsync( + [signedOrder, anotherSignedOrder], [cancelAmount, cancelAmount]); + const cancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync(orderHashHex); + const anotherCancelledAmount = await zeroEx.exchange.getCanceledTakerAmountAsync( + anotherOrderHashHex); + expect(cancelledAmount).to.be.bignumber.equal(cancelAmount); + expect(anotherCancelledAmount).to.be.bignumber.equal(cancelAmount); + }); }); }); }); -- cgit From 9fd128c47636e991ce2cf10cbb22c4b8fe79cc1b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:26:54 +0200 Subject: Fix linter errors --- src/contract_wrappers/exchange_wrapper.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 690f90360..11cfd134f 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -197,7 +197,8 @@ export class ExchangeWrapper extends ContractWrapper { /** * Cancel a given fill amount of an order. Cancellations are cumulative. */ - public async cancelOrderAsync(order: Order|SignedOrder, takerTokenCancelAmount: BigNumber.BigNumber): Promise { + public async cancelOrderAsync( + order: Order|SignedOrder, takerTokenCancelAmount: BigNumber.BigNumber): Promise { assert.doesConformToSchema('order', SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), orderSchema); -- cgit From 282219a70723351a681ac3c20156eeef14a758c9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:27:48 +0200 Subject: Add assertion for empty batch --- src/contract_wrappers/exchange_wrapper.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 11cfd134f..1ae5b260b 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -142,6 +142,7 @@ export class ExchangeWrapper extends ContractWrapper { public async batchFillOrderAsync(signedOrders: SignedOrder[], fillTakerAmounts: BigNumber.BigNumber[], shouldCheckTransfer: boolean, takerAddress: string): Promise { assert.isSameLength('signedOrders', signedOrders, 'fillTakerAmounts', fillTakerAmounts); + assert.assert(!_.isEmpty(signedOrders), 'Can not cancel an empty batch'); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); // _.zip doesn't type check if values have different types :'( -- cgit From d5767428e598e9355786dd30c3ebb17a89b088ba Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:32:05 +0200 Subject: Move fillOrderAsync to fill order describe block --- test/exchange_wrapper_test.ts | 364 +++++++++++++++++++++--------------------- 1 file changed, 185 insertions(+), 179 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 6487ba98c..309791518 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -120,206 +120,212 @@ describe('ExchangeWrapper', () => { expect(isValid).to.be.true(); }); }); - describe('#fillOrderAsync', () => { - let makerTokenAddress: string; - let takerTokenAddress: string; - let coinbase: string; - let makerAddress: string; - let takerAddress: string; - let feeRecipient: string; - const fillTakerAmount = new BigNumber(5); - const shouldCheckTransfer = false; - before(async () => { - [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; - tokens = await zeroEx.tokenRegistry.getTokensAsync(); - const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); - makerTokenAddress = makerToken.address; - takerTokenAddress = takerToken.address; - }); - describe('failed fills', () => { - it('should throw when the fill amount is zero', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - const zeroFillAmount = new BigNumber(0); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, zeroFillAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); - }); - it('should throw when sender is not a taker', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - const nonExistentSenderAddress = userAddresses[6]; - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, nonExistentSenderAddress, - )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); + describe('fill order', () => { + describe('#fillOrderAsync', () => { + let makerTokenAddress: string; + let takerTokenAddress: string; + let coinbase: string; + let makerAddress: string; + let takerAddress: string; + let feeRecipient: string; + const fillTakerAmount = new BigNumber(5); + const shouldCheckTransfer = false; + before(async () => { + [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; + tokens = await zeroEx.tokenRegistry.getTokensAsync(); + const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); + makerTokenAddress = makerToken.address; + takerTokenAddress = takerToken.address; }); - it('should throw when order is expired', async () => { - const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, - ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_EXPIRED); - }); - describe('should throw when not enough balance or allowance to fulfill the order', () => { - const fillableAmount = new BigNumber(5); - const balanceToSubtractFromMaker = new BigNumber(3); - const lackingAllowance = new BigNumber(3); - let signedOrder: SignedOrder; - beforeEach('create fillable signed order', async () => { - signedOrder = await fillScenarios.createFillableSignedOrderAsync( + describe('failed fills', () => { + it('should throw when the fill amount is zero', async () => { + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); + const zeroFillAmount = new BigNumber(0); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, zeroFillAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); }); - it('should throw when taker balance is less than fill amount', async () => { - await zeroEx.token.transferAsync( - takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, + it('should throw when sender is not a taker', async () => { + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); + const nonExistentSenderAddress = userAddresses[6]; return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); + signedOrder, fillTakerAmount, shouldCheckTransfer, nonExistentSenderAddress, + )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); - it('should throw when taker allowance is less than fill amount', async () => { - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); - await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFillAmount); + it('should throw when order is expired', async () => { + const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + fillableAmount, expirationInPast, + ); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_EXPIRED); + }); + describe('should throw when not enough balance or allowance to fulfill the order', () => { + const fillableAmount = new BigNumber(5); + const balanceToSubtractFromMaker = new BigNumber(3); + const lackingAllowance = new BigNumber(3); + let signedOrder: SignedOrder; + beforeEach('create fillable signed order', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); + it('should throw when taker balance is less than fill amount', async () => { + await zeroEx.token.transferAsync( + takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); + }); + it('should throw when taker allowance is less than fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFillAmount); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); + }); + it('should throw when maker balance is less than maker fill amount', async () => { + await zeroEx.token.transferAsync( + makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); + }); + it('should throw when maker allowance is less than maker fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFillAmount); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); + }); }); - it('should throw when maker balance is less than maker fill amount', async () => { - await zeroEx.token.transferAsync( - makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + it('should throw when there a rounding error would have occurred', async () => { + const makerAmount = new BigNumber(3); + const takerAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + makerAmount, takerAmount, ); + const fillTakerAmountThatCausesRoundingError = new BigNumber(3); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); + signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); - it('should throw when maker allowance is less than maker fill amount', async () => { - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); - await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, - newAllowanceWhichIsLessThanFillAmount); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); + describe('should throw when not enough balance or allowance to pay fees', () => { + const fillableAmount = new BigNumber(5); + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach('setup', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + }); + it('should throw when maker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); + }); + it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); + }); + it('should throw when taker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); + }); + it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); + }); }); }); - it('should throw when there a rounding error would have occurred', async () => { - const makerAmount = new BigNumber(3); - const takerAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, - makerAmount, takerAmount, - ); - const fillTakerAmountThatCausesRoundingError = new BigNumber(3); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); - }); - describe('should throw when not enough balance or allowance to pay fees', () => { - const fillableAmount = new BigNumber(5); - const makerFee = new BigNumber(2); - const takerFee = new BigNumber(2); - let signedOrder: SignedOrder; - beforeEach('setup', async () => { - signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - makerTokenAddress, takerTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, + 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)) + .to.be.bignumber.equal(0); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) + .to.be.bignumber.equal(0); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) + .to.be.bignumber.equal(fillableAmount); + await zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) + .to.be.bignumber.equal(fillTakerAmount); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) + .to.be.bignumber.equal(fillTakerAmount); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); }); - it('should throw when maker doesn\'t have enough balance to pay fees', async () => { - const balanceToSubtractFromMaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + it('should partially fill the valid order', async () => { + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); + const partialFillAmount = new BigNumber(3); + await zeroEx.exchange.fillOrderAsync( + signedOrder, partialFillAmount, shouldCheckTransfer, takerAddress); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) + .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) + .to.be.bignumber.equal(partialFillAmount); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) + .to.be.bignumber.equal(partialFillAmount); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) + .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); }); - it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { - const newAllowanceWhichIsLessThanFees = makerFee.minus(1); - await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, - newAllowanceWhichIsLessThanFees); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); - }); - it('should throw when taker doesn\'t have enough balance to pay fees', async () => { - const balanceToSubtractFromTaker = new BigNumber(1); - await zeroEx.token.transferAsync( - zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + it('should fill the valid orders with fees', async () => { + const fillableAmount = new BigNumber(5); + const makerFee = new BigNumber(1); + const takerFee = new BigNumber(2); + const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, ); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); + await zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); + expect(await zeroEx.token.getBalanceAsync(zrxTokenAddress, feeRecipient)) + .to.be.bignumber.equal(makerFee.plus(takerFee)); }); - it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { - const newAllowanceWhichIsLessThanFees = makerFee.minus(1); - await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFees); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, - )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); - }); - }); - }); - 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)) - .to.be.bignumber.equal(0); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) - .to.be.bignumber.equal(0); - expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillableAmount); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); - expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) - .to.be.bignumber.equal(fillTakerAmount); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillTakerAmount); - expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); - }); - it('should partially fill the 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.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer, takerAddress); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); - expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) - .to.be.bignumber.equal(partialFillAmount); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) - .to.be.bignumber.equal(partialFillAmount); - expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); - }); - it('should fill the valid orders with fees', async () => { - const fillableAmount = new BigNumber(5); - const makerFee = new BigNumber(1); - const takerFee = new BigNumber(2); - const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( - makerTokenAddress, takerTokenAddress, makerFee, takerFee, - makerAddress, takerAddress, fillableAmount, feeRecipient, - ); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); - expect(await zeroEx.token.getBalanceAsync(zrxTokenAddress, feeRecipient)) - .to.be.bignumber.equal(makerFee.plus(takerFee)); }); }); }); -- cgit From 426f43391e0b762f03eb2aaf11e0a8d6c6aab301 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:33:04 +0200 Subject: Move initialization up --- test/exchange_wrapper_test.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 309791518..21d54e061 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -121,22 +121,22 @@ describe('ExchangeWrapper', () => { }); }); describe('fill order', () => { + let makerTokenAddress: string; + let takerTokenAddress: string; + let coinbase: string; + let makerAddress: string; + let takerAddress: string; + let feeRecipient: string; + const fillTakerAmount = new BigNumber(5); + const shouldCheckTransfer = false; + before(async () => { + [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; + tokens = await zeroEx.tokenRegistry.getTokensAsync(); + const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); + makerTokenAddress = makerToken.address; + takerTokenAddress = takerToken.address; + }); describe('#fillOrderAsync', () => { - let makerTokenAddress: string; - let takerTokenAddress: string; - let coinbase: string; - let makerAddress: string; - let takerAddress: string; - let feeRecipient: string; - const fillTakerAmount = new BigNumber(5); - const shouldCheckTransfer = false; - before(async () => { - [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; - tokens = await zeroEx.tokenRegistry.getTokensAsync(); - const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); - makerTokenAddress = makerToken.address; - takerTokenAddress = takerToken.address; - }); describe('failed fills', () => { it('should throw when the fill amount is zero', async () => { const fillableAmount = new BigNumber(5); -- cgit From efbd81df0273a2b29b072b737bd04c4b3c065913 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 13:36:21 +0200 Subject: Move fillableAmount to initialization section --- test/exchange_wrapper_test.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 21d54e061..43f8683c8 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -127,6 +127,7 @@ describe('ExchangeWrapper', () => { let makerAddress: string; let takerAddress: string; let feeRecipient: string; + const fillableAmount = new BigNumber(5); const fillTakerAmount = new BigNumber(5); const shouldCheckTransfer = false; before(async () => { @@ -139,7 +140,6 @@ describe('ExchangeWrapper', () => { describe('#fillOrderAsync', () => { describe('failed fills', () => { it('should throw when the fill amount is zero', async () => { - const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); @@ -149,7 +149,6 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); }); it('should throw when sender is not a taker', async () => { - const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); @@ -160,7 +159,6 @@ describe('ExchangeWrapper', () => { }); it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(1496826058); // 7th Jun 2017 - const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, @@ -170,7 +168,6 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_EXPIRED); }); describe('should throw when not enough balance or allowance to fulfill the order', () => { - const fillableAmount = new BigNumber(5); const balanceToSubtractFromMaker = new BigNumber(3); const lackingAllowance = new BigNumber(3); let signedOrder: SignedOrder; @@ -225,7 +222,6 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); describe('should throw when not enough balance or allowance to pay fees', () => { - const fillableAmount = new BigNumber(5); const makerFee = new BigNumber(2); const takerFee = new BigNumber(2); let signedOrder: SignedOrder; @@ -273,7 +269,6 @@ describe('ExchangeWrapper', () => { }); 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, ); @@ -297,7 +292,6 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); }); it('should partially fill the valid order', async () => { - const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); @@ -314,7 +308,6 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); }); it('should fill the valid orders with fees', async () => { - const fillableAmount = new BigNumber(5); const makerFee = new BigNumber(1); const takerFee = new BigNumber(2); const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( @@ -328,6 +321,22 @@ describe('ExchangeWrapper', () => { }); }); }); + describe('#batchFillOrderAsync', () => { + let anotherSignedOrder: SignedOrder; + let anotherOrderHashHex: string; + beforeEach(async () => { + anotherSignedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + anotherOrderHashHex = await zeroEx.getOrderHashHexAsync(anotherSignedOrder); + }); + describe('failed batch fills', () => { + + }); + describe('successful batch fills', () => { + + }); + }); }); describe('cancel order(s)', () => { let makerTokenAddress: string; -- cgit From 31b10973509ac1925a4cc6f5731a6c4db65b444c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 16:51:58 +0200 Subject: Remove only --- test/exchange_wrapper_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 683c4b17d..b0aa351a4 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -414,7 +414,7 @@ describe('ExchangeWrapper', () => { return expect(zeroEx.exchange.batchCancelOrderAsync([])) .to.be.rejectedWith('Can not cancel an empty batch'); }); - it.only('should throw when orders have different makers', async () => { + it('should throw when orders have different makers', async () => { const signedOrderWithADifferentMaker = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, takerAddress, takerAddress, fillableAmount, ); -- cgit From 3e7d6a0013f4b4a382bd068b2223cada3ffe06d9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 17:11:01 +0200 Subject: Refactor to use OrderFillRequest --- src/contract_wrappers/exchange_wrapper.ts | 48 +++++++++++++++++-------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e824108de..20e3a6abc 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -18,7 +18,7 @@ import { CreateContractEvent, ContractEventObj, EventCallback, - ContractResponse, OrderCancellationRequest, + ContractResponse, OrderCancellationRequest, OrderFillRequest, } from '../types'; import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; @@ -125,53 +125,57 @@ export class ExchangeWrapper extends ContractWrapper { return cancelledAmountInBaseUnits; } /** - * Fills a signed order with a fillAmount denominated in baseUnits of the taker token. + * Fills a signed order with a takerTokenFillAmount denominated in baseUnits of the taker token. * Since the order in which transactions are included in the next block is indeterminate, race-conditions * could arise where a users balance or allowance changes before the fillOrder executes. Because of this, * we allow you to specify `shouldCheckTransfer`. If true, the smart contract will not throw if while * executing, the parties do not have sufficient balances/allowances, preserving gas costs. Setting it to * false forgoes this check and causes the smart contract to throw instead. */ - public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + public async fillOrderAsync(signedOrder: SignedOrder, takerTokenFillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, takerAddress: string): Promise { - await this.batchFillOrderAsync([signedOrder], [fillTakerAmount], shouldCheckTransfer, takerAddress); + await this.batchFillOrderAsync([{ + signedOrder, + takerTokenFillAmount, + }], shouldCheckTransfer, takerAddress); } /** * Batched version of fillOrderAsync. Executes fills atomically in a single transaction. */ - public async batchFillOrderAsync(signedOrders: SignedOrder[], fillTakerAmounts: BigNumber.BigNumber[], + public async batchFillOrderAsync(orderFillRequests: OrderFillRequest[], shouldCheckTransfer: boolean, takerAddress: string): Promise { - assert.isSameLength('signedOrders', signedOrders, 'fillTakerAmounts', fillTakerAmounts); - assert.assert(!_.isEmpty(signedOrders), 'Can not cancel an empty batch'); + assert.assert(!_.isEmpty(orderFillRequests), 'Cannot fill an empty batch'); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); // _.zip doesn't type check if values have different types :'( - const ordersAndAmounts = _.zip(signedOrders, fillTakerAmounts); - _.forEach(ordersAndAmounts, - async ([signedOrder, fillTakerAmount]: [SignedOrder, BigNumber.BigNumber]) => { + _.forEach(orderFillRequests, + async (orderFillRequest: OrderFillRequest) => { assert.doesConformToSchema('signedOrder', - SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); - assert.isBigNumber('fillTakerAmount', fillTakerAmount); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); + SchemaValidator.convertToJSONSchemaCompatibleObject(orderFillRequest.signedOrder as object), + signedOrderSchema); + assert.isBigNumber('takerTokenFillAmount', orderFillRequest.takerTokenFillAmount); + await this.validateFillOrderAndThrowIfInvalidAsync( + orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress); }); const exchangeInstance = await this.getExchangeContractAsync(); - const orderAddressesValuesAndSignatureArray = _.map(signedOrders, signedOrder => { + const orderAddressesValuesAmountsAndSignatureArray = _.map(orderFillRequests, orderFillRequest => { return [ - ...ExchangeWrapper.getOrderAddressesAndValues(signedOrder), - signedOrder.ecSignature.v, - signedOrder.ecSignature.r, - signedOrder.ecSignature.s, + ...ExchangeWrapper.getOrderAddressesAndValues(orderFillRequest.signedOrder), + orderFillRequest.takerTokenFillAmount, + orderFillRequest.signedOrder.ecSignature.v, + orderFillRequest.signedOrder.ecSignature.r, + orderFillRequest.signedOrder.ecSignature.s, ]; }); // _.unzip doesn't type check if values have different types :'( - const [orderAddressesArray, orderValuesArray, vArray, rArray, sArray] = _.unzip( - orderAddressesValuesAndSignatureArray, + const [orderAddressesArray, orderValuesArray, takerTokenFillAmountArray, vArray, rArray, sArray] = _.unzip( + orderAddressesValuesAmountsAndSignatureArray, ); const gas = await exchangeInstance.batchFill.estimateGas( orderAddressesArray, orderValuesArray, - fillTakerAmounts, + takerTokenFillAmountArray, shouldCheckTransfer, vArray, rArray, @@ -183,7 +187,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.batchFill( orderAddressesArray, orderValuesArray, - fillTakerAmounts, + takerTokenFillAmountArray, shouldCheckTransfer, vArray, rArray, -- cgit From f3d9f554d9916af48038ae810d75aaf3f78676c9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 18:09:11 +0200 Subject: Remove old comment --- src/contract_wrappers/exchange_wrapper.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 20e3a6abc..c304aeb08 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -147,7 +147,6 @@ export class ExchangeWrapper extends ContractWrapper { assert.assert(!_.isEmpty(orderFillRequests), 'Cannot fill an empty batch'); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); - // _.zip doesn't type check if values have different types :'( _.forEach(orderFillRequests, async (orderFillRequest: OrderFillRequest) => { assert.doesConformToSchema('signedOrder', -- cgit From edbbf5a215209bcb67052ca6cb83bb40e149d9a7 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 19:11:21 +0200 Subject: Add success test --- test/exchange_wrapper_test.ts | 30 +++++++++++++++++++++++++++--- test/utils/fill_scenarios.ts | 8 ++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index b0aa351a4..748816949 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -16,7 +16,9 @@ import { ExchangeEvents, ContractEvent, DoneCallback, - ExchangeContractErrs, OrderCancellationRequest, + ExchangeContractErrs, + OrderCancellationRequest, + OrderFillRequest, } from '../src/types'; import {FillScenarios} from './utils/fill_scenarios'; import {TokenUtils} from './utils/token_utils'; @@ -132,7 +134,6 @@ describe('ExchangeWrapper', () => { const shouldCheckTransfer = false; before(async () => { [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; - tokens = await zeroEx.tokenRegistry.getTokensAsync(); const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); makerTokenAddress = makerToken.address; takerTokenAddress = takerToken.address; @@ -322,19 +323,42 @@ describe('ExchangeWrapper', () => { }); }); describe('#batchFillOrderAsync', () => { + let signedOrder: SignedOrder; + let signedOrderHashHex: string; let anotherSignedOrder: SignedOrder; let anotherOrderHashHex: string; + let orderFillBatch: OrderFillRequest[]; beforeEach(async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + signedOrderHashHex = await zeroEx.getOrderHashHexAsync(signedOrder); anotherSignedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); anotherOrderHashHex = await zeroEx.getOrderHashHexAsync(anotherSignedOrder); + orderFillBatch = [ + { + signedOrder, + takerTokenFillAmount: fillTakerAmount, + }, + { + signedOrder: anotherSignedOrder, + takerTokenFillAmount: fillTakerAmount, + }, + ]; }); describe('failed batch fills', () => { }); describe('successful batch fills', () => { - + it('should successfully fill multiple orders', async () => { + await zeroEx.exchange.batchFillOrderAsync(orderFillBatch, shouldCheckTransfer, takerAddress); + const filledAmount = await zeroEx.exchange.getFilledTakerAmountAsync(signedOrderHashHex); + const anotherFilledAmount = await zeroEx.exchange.getFilledTakerAmountAsync(anotherOrderHashHex); + expect(filledAmount).to.be.bignumber.equal(fillTakerAmount); + expect(anotherFilledAmount).to.be.bignumber.equal(fillTakerAmount); + }); }); }); }); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index d8d6cd0b9..8ff27ceb1 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -70,9 +70,13 @@ export class FillScenarios { makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber, feeRecepient: string, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinbase, makerAddress, makerFillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, makerFillableAmount); + const oldMakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(makerTokenAddress, makerAddress); + await this.zeroEx.token.setProxyAllowanceAsync( + makerTokenAddress, makerAddress, oldMakerAllowance.plus(makerFillableAmount)); await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinbase, takerAddress, takerFillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, takerFillableAmount); + const oldTakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(takerTokenAddress, takerAddress); + await this.zeroEx.token.setProxyAllowanceAsync( + takerTokenAddress, takerAddress, oldTakerAllowance.plus(takerFillableAmount)); if (!makerFee.isZero()) { await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, makerAddress, makerFee); -- cgit From 01c33ef8b7929ae936fd4bb84bb5d3aee3e63e8d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 19:12:43 +0200 Subject: Add no-op test --- src/contract_wrappers/exchange_wrapper.ts | 4 +++- test/exchange_wrapper_test.ts | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index c304aeb08..a15df5a77 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -144,7 +144,9 @@ export class ExchangeWrapper extends ContractWrapper { */ public async batchFillOrderAsync(orderFillRequests: OrderFillRequest[], shouldCheckTransfer: boolean, takerAddress: string): Promise { - assert.assert(!_.isEmpty(orderFillRequests), 'Cannot fill an empty batch'); + if (_.isEmpty(orderFillRequests)) { + return; + } assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); _.forEach(orderFillRequests, diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 748816949..bd2536741 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -347,11 +347,11 @@ describe('ExchangeWrapper', () => { takerTokenFillAmount: fillTakerAmount, }, ]; - }); - describe('failed batch fills', () => { - }); describe('successful batch fills', () => { + it('should no-op for an empty batch', async () => { + await zeroEx.exchange.batchFillOrderAsync([], shouldCheckTransfer, takerAddress); + }); it('should successfully fill multiple orders', async () => { await zeroEx.exchange.batchFillOrderAsync(orderFillBatch, shouldCheckTransfer, takerAddress); const filledAmount = await zeroEx.exchange.getFilledTakerAmountAsync(signedOrderHashHex); -- cgit From 5d464d87e07003c231fca55e87b95cc7e1e9e978 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 11:00:36 +0200 Subject: Don't use batchFillOrderAsync for fillOrderAsync --- src/contract_wrappers/exchange_wrapper.ts | 40 ++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index a15df5a77..dffb9df13 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -134,10 +134,44 @@ export class ExchangeWrapper extends ContractWrapper { */ public async fillOrderAsync(signedOrder: SignedOrder, takerTokenFillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, takerAddress: string): Promise { - await this.batchFillOrderAsync([{ - signedOrder, + assert.doesConformToSchema('signedOrder', + SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), + signedOrderSchema); + assert.isBigNumber('takerTokenFillAmount', takerTokenFillAmount); + assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); + await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); + + const exchangeInstance = await this.getExchangeContractAsync(); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, takerTokenFillAmount, takerAddress); + + const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(signedOrder); + + const gas = await exchangeInstance.fill.estimateGas( + orderAddresses, + orderValues, takerTokenFillAmount, - }], shouldCheckTransfer, takerAddress); + shouldCheckTransfer, + signedOrder.ecSignature.v, + signedOrder.ecSignature.r, + signedOrder.ecSignature.s, + { + from: takerAddress, + }, + ); + const response: ContractResponse = await exchangeInstance.fill( + orderAddresses, + orderValues, + takerTokenFillAmount, + shouldCheckTransfer, + signedOrder.ecSignature.v, + signedOrder.ecSignature.r, + signedOrder.ecSignature.s, + { + from: takerAddress, + gas, + }, + ); + this.throwErrorLogsAsErrors(response.logs); } /** * Batched version of fillOrderAsync. Executes fills atomically in a single transaction. -- cgit From 931f0f08a52a14c97aa939d98ed3dcf6c72ff6b9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 11:34:29 +0200 Subject: Fix spacing --- src/contract_wrappers/exchange_wrapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 8d95c9fae..107a8f618 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -137,8 +137,8 @@ export class ExchangeWrapper extends ContractWrapper { public async fillOrderAsync(signedOrder: SignedOrder, takerTokenFillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, takerAddress: string): Promise { assert.doesConformToSchema('signedOrder', - SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), - signedOrderSchema); + SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), + signedOrderSchema); assert.isBigNumber('takerTokenFillAmount', takerTokenFillAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); -- cgit From 6d560f1be1f317cbf4352e37d5a7259b2218065c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 11:36:40 +0200 Subject: Address old feedback --- src/contract_wrappers/exchange_wrapper.ts | 5 +++-- src/utils/assert.ts | 6 ------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 107a8f618..57becde24 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -181,7 +181,7 @@ export class ExchangeWrapper extends ContractWrapper { public async batchFillOrderAsync(orderFillRequests: OrderFillRequest[], shouldCheckTransfer: boolean, takerAddress: string): Promise { if (_.isEmpty(orderFillRequests)) { - return; + return; // no-op } assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); @@ -205,10 +205,11 @@ export class ExchangeWrapper extends ContractWrapper { orderFillRequest.signedOrder.ecSignature.s, ]; }); - // _.unzip doesn't type check if values have different types :'( + // We use _.unzip because _.unzip doesn't type check if values have different types :'( const [orderAddressesArray, orderValuesArray, takerTokenFillAmountArray, vArray, rArray, sArray] = _.unzip( orderAddressesValuesAmountsAndSignatureArray, ); + const gas = await exchangeInstance.batchFill.estimateGas( orderAddressesArray, orderValuesArray, 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)); }, -- cgit From a3140c86416fde624e92db745c70ebc72d443a00 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 8 Jun 2017 11:45:35 +0200 Subject: rename decimals to numDecimals for clarity --- src/0x.js.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 8f1178b2a..d06069294 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -83,11 +83,11 @@ export class ZeroEx { * E.g: If a currency has 18 decimal places, 1e18 or one quintillion of the currency is equivalent * to 1 unit. */ - public static toUnitAmount(amount: BigNumber.BigNumber, decimals: number): BigNumber.BigNumber { + public static toUnitAmount(amount: BigNumber.BigNumber, numDecimals: number): BigNumber.BigNumber { assert.isBigNumber('amount', amount); - assert.isNumber('decimals', decimals); + assert.isNumber('numDecimals', numDecimals); - const aUnit = new BigNumber(10).pow(decimals); + const aUnit = new BigNumber(10).pow(numDecimals); const unit = amount.div(aUnit); return unit; } @@ -96,11 +96,11 @@ export class ZeroEx { * is the amount expressed in the smallest denomination. * E.g: 1 unit of a token with 18 decimal places is expressed in baseUnits as 1000000000000000000 */ - public static toBaseUnitAmount(amount: BigNumber.BigNumber, decimals: number): BigNumber.BigNumber { + public static toBaseUnitAmount(amount: BigNumber.BigNumber, numDecimals: number): BigNumber.BigNumber { assert.isBigNumber('amount', amount); - assert.isNumber('decimals', decimals); + assert.isNumber('numDecimals', numDecimals); - const unit = new BigNumber(10).pow(decimals); + const unit = new BigNumber(10).pow(numDecimals); const baseUnitAmount = amount.times(unit); return baseUnitAmount; } -- cgit From 2b50f8178ecbed98ff28b3adb29b87356ff48a3f Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 11:45:54 +0200 Subject: Use isSenderAddressAsync --- src/contract_wrappers/exchange_wrapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 57becde24..94de45dd8 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -296,7 +296,7 @@ export class ExchangeWrapper extends ContractWrapper { SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), orderSchema); assert.isBigNumber('takerTokenCancelAmount', takerTokenCancelAmount); - await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'order.maker', order.maker); + await assert.isSenderAddressAsync('order.maker', order.maker, this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); await this.validateCancelOrderAndThrowIfInvalidAsync(order, takerTokenCancelAmount); @@ -332,7 +332,7 @@ export class ExchangeWrapper extends ContractWrapper { const makers = _.map(orderCancellationRequests, cancellationRequest => cancellationRequest.order.maker); assert.assert(_.uniq(makers).length === 1, ExchangeContractErrs.MULTIPLE_MAKERS_IN_SINGLE_CANCEL_BATCH); const maker = makers[0]; - await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'maker', maker); + await assert.isSenderAddressAsync('maker', maker, this.web3Wrapper); _.forEach(orderCancellationRequests, async (cancellationRequest: OrderCancellationRequest, i: number) => { assert.doesConformToSchema(`orderCancellationRequests[${i}].order`, -- cgit From 00782d6d68c08f3aec4f3dc8f8aee6054ac022e3 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 8 Jun 2017 12:05:48 +0200 Subject: Fix comments --- src/0x.js.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index d06069294..2bf8cad5e 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -33,7 +33,7 @@ export class ZeroEx { private web3Wrapper: Web3Wrapper; /** * Verifies that the elliptic curve signature `signature` was generated - * by signing `data` with the private key corresponding to the `signerAddressHex` address. + * by signing `dataHex` with the private key corresponding to the `signerAddressHex` address. */ public static isValidSignature(dataHex: string, signature: ECSignature, signerAddressHex: string): boolean { assert.isHexString('dataHex', dataHex); @@ -138,7 +138,7 @@ export class ZeroEx { return orderHashHex; } /** - * Signs an orderHash and returns it's elliptic curve signature + * Signs an orderHash and returns it's elliptic curve signature. * This method currently supports TestRPC, Geth and Parity above and below V1.6.6 */ public async signOrderHashAsync(orderHashHex: string, signerAddress: string): Promise { -- cgit From 196130ff96cd20bd13c7bada0ba5eb8d62a2cdc5 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 8 Jun 2017 12:06:04 +0200 Subject: Make web3Wrapper protected instead of public --- src/contract_wrappers/contract_wrapper.ts | 2 +- test/0x.js_test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/contract_wrappers/contract_wrapper.ts b/src/contract_wrappers/contract_wrapper.ts index 9f4cd8039..c3067f613 100644 --- a/src/contract_wrappers/contract_wrapper.ts +++ b/src/contract_wrappers/contract_wrapper.ts @@ -5,7 +5,7 @@ import {ZeroExError} from '../types'; import {utils} from '../utils/utils'; export class ContractWrapper { - public web3Wrapper: Web3Wrapper; + protected web3Wrapper: Web3Wrapper; constructor(web3Wrapper: Web3Wrapper) { this.web3Wrapper = web3Wrapper; } diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 58f259a11..9cba93488 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -35,9 +35,9 @@ describe('ZeroEx library', () => { // Check that all nested web3 instances return the updated provider const nestedWeb3WrapperProvider = (zeroEx as any).web3Wrapper.getCurrentProvider(); expect((nestedWeb3WrapperProvider as any).zeroExTestId).to.be.a('number'); - const exchangeWeb3WrapperProvider = zeroEx.exchange.web3Wrapper.getCurrentProvider(); + const exchangeWeb3WrapperProvider = (zeroEx.exchange as any).web3Wrapper.getCurrentProvider(); expect((exchangeWeb3WrapperProvider as any).zeroExTestId).to.be.a('number'); - const tokenRegistryWeb3WrapperProvider = zeroEx.tokenRegistry.web3Wrapper.getCurrentProvider(); + const tokenRegistryWeb3WrapperProvider = (zeroEx.tokenRegistry as any).web3Wrapper.getCurrentProvider(); expect((tokenRegistryWeb3WrapperProvider as any).zeroExTestId).to.be.a('number'); }); }); -- cgit From 8311203f73bf6eff8bcac8d1fb72f9cf8b65c45b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 12:14:35 +0200 Subject: Refactor isValidSignature --- src/0x.js.ts | 14 +------------ src/contract_wrappers/exchange_wrapper.ts | 4 ++-- src/utils/utils.ts | 16 +++++++++++++++ test/exchange_wrapper_test.ts | 34 +++++++++++++++++++------------ 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 8f1178b2a..f275d9fbd 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -40,19 +40,7 @@ export class ZeroEx { assert.doesConformToSchema('signature', signature, ecSignatureSchema); assert.isETHAddressHex('signerAddressHex', signerAddressHex); - const dataBuff = ethUtil.toBuffer(dataHex); - const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); - try { - const pubKey = ethUtil.ecrecover( - msgHashBuff, - signature.v, - ethUtil.toBuffer(signature.r), - ethUtil.toBuffer(signature.s)); - const retrievedAddress = ethUtil.bufferToHex(ethUtil.pubToAddress(pubKey)); - return retrievedAddress === signerAddressHex; - } catch (err) { - return false; - } + return utils.isValidSignature(dataHex, signature, signerAddressHex); } /** * Generates pseudo-random 256 bit salt. diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index d144d8aad..5b5d1e914 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -70,8 +70,8 @@ export class ExchangeWrapper extends ContractWrapper { await this.stopWatchingExchangeLogEventsAsync(); delete this.exchangeContractIfExists; } - public async isValidSignatureAsync(dataHex: string, ecSignature: ECSignature, - signerAddressHex: string): Promise { + private async isValidSignatureUsingContractCallAsync(dataHex: string, ecSignature: ECSignature, + signerAddressHex: string): Promise { assert.isHexString('dataHex', dataHex); assert.doesConformToSchema('ecSignature', ecSignature, ecSignatureSchema); assert.isETHAddressHex('signerAddressHex', signerAddressHex); diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 5786bab07..ea3d8c7e1 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -4,6 +4,7 @@ import * as ethABI from 'ethereumjs-abi'; import * as ethUtil from 'ethereumjs-util'; import {Order, SignedOrder, SolidityTypes} from '../types'; import * as BigNumber from 'bignumber.js'; +import {ECSignature} from '../types'; export const utils = { /** @@ -50,6 +51,21 @@ export const utils = { const hashHex = ethUtil.bufferToHex(hashBuff); return hashHex; }, + isValidSignature(dataHex: string, signature: ECSignature, signerAddressHex: string): boolean { + const dataBuff = ethUtil.toBuffer(dataHex); + const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); + try { + const pubKey = ethUtil.ecrecover( + msgHashBuff, + signature.v, + ethUtil.toBuffer(signature.r), + ethUtil.toBuffer(signature.s)); + const retrievedAddress = ethUtil.bufferToHex(ethUtil.pubToAddress(pubKey)); + return retrievedAddress === signerAddressHex; + } catch (err) { + return false; + } + }, getCurrentUnixTimestamp(): BigNumber.BigNumber { return new BigNumber(Date.now() / 1000); }, diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 08936f1d2..aed3c3c0b 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -51,7 +51,7 @@ describe('ExchangeWrapper', () => { afterEach(async () => { await blockchainLifecycle.revertAsync(); }); - describe('#isValidSignatureAsync', () => { + describe('#isValidSignatureUsingContractCallAsync', () => { // The Exchange smart contract `isValidSignature` method only validates orderHashes and assumes // the length of the data is exactly 32 bytes. Thus for these tests, we use data of this size. const dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0'; @@ -68,8 +68,9 @@ describe('ExchangeWrapper', () => { r: signature.r, s: signature.s, }; - return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) - .to.be.rejected(); + return expect((zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), + ).to.be.rejected(); }); it('r lacks 0x prefix', async () => { const malformedR = signature.r.replace('0x', ''); @@ -78,8 +79,9 @@ describe('ExchangeWrapper', () => { r: malformedR, s: signature.s, }; - return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) - .to.be.rejected(); + return expect((zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), + ).to.be.rejected(); }); it('r is too short', async () => { const malformedR = signature.r.substr(10); @@ -88,8 +90,9 @@ describe('ExchangeWrapper', () => { r: malformedR, s: signature.s.replace('0', 'z'), }; - return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) - .to.be.rejected(); + return expect((zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), + ).to.be.rejected(); }); it('s is not hex', async () => { const malformedS = signature.s.replace('0', 'z'); @@ -98,26 +101,31 @@ describe('ExchangeWrapper', () => { r: signature.r, s: malformedS, }; - return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) - .to.be.rejected(); + return expect((zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), + ).to.be.rejected(); }); }); it('should return false if the data doesn\'t pertain to the signature & address', async () => { - const isValid = await zeroEx.exchange.isValidSignatureAsync('0x0', signature, address); + const isValid = await (zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync('0x0', signature, address); expect(isValid).to.be.false(); }); it('should return false if the address doesn\'t pertain to the signature & dataHex', async () => { const validUnrelatedAddress = '0x8b0292B11a196601eD2ce54B665CaFEca0347D42'; - const isValid = await zeroEx.exchange.isValidSignatureAsync(dataHex, signature, validUnrelatedAddress); + const isValid = await (zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, signature, validUnrelatedAddress); expect(isValid).to.be.false(); }); it('should return false if the signature doesn\'t pertain to the dataHex & address', async () => { const wrongSignature = {...signature, v: 28}; - const isValid = await zeroEx.exchange.isValidSignatureAsync(dataHex, wrongSignature, address); + const isValid = await (zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, wrongSignature, address); expect(isValid).to.be.false(); }); it('should return true if the signature does pertain to the dataHex & address', async () => { - const isValid = await zeroEx.exchange.isValidSignatureAsync(dataHex, signature, address); + const isValid = await (zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, signature, address); expect(isValid).to.be.true(); }); }); -- cgit From a328f0c8053173fd8e1f75b808228c3edb617bcc Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 12:19:02 +0200 Subject: Move isValidSignature back from utils --- src/0x.js.ts | 14 +++++++++++++- src/utils/utils.ts | 15 --------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index f275d9fbd..8f1178b2a 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -40,7 +40,19 @@ export class ZeroEx { assert.doesConformToSchema('signature', signature, ecSignatureSchema); assert.isETHAddressHex('signerAddressHex', signerAddressHex); - return utils.isValidSignature(dataHex, signature, signerAddressHex); + const dataBuff = ethUtil.toBuffer(dataHex); + const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); + try { + const pubKey = ethUtil.ecrecover( + msgHashBuff, + signature.v, + ethUtil.toBuffer(signature.r), + ethUtil.toBuffer(signature.s)); + const retrievedAddress = ethUtil.bufferToHex(ethUtil.pubToAddress(pubKey)); + return retrievedAddress === signerAddressHex; + } catch (err) { + return false; + } } /** * Generates pseudo-random 256 bit salt. diff --git a/src/utils/utils.ts b/src/utils/utils.ts index ea3d8c7e1..ded2d31fd 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -51,21 +51,6 @@ export const utils = { const hashHex = ethUtil.bufferToHex(hashBuff); return hashHex; }, - isValidSignature(dataHex: string, signature: ECSignature, signerAddressHex: string): boolean { - const dataBuff = ethUtil.toBuffer(dataHex); - const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); - try { - const pubKey = ethUtil.ecrecover( - msgHashBuff, - signature.v, - ethUtil.toBuffer(signature.r), - ethUtil.toBuffer(signature.s)); - const retrievedAddress = ethUtil.bufferToHex(ethUtil.pubToAddress(pubKey)); - return retrievedAddress === signerAddressHex; - } catch (err) { - return false; - } - }, getCurrentUnixTimestamp(): BigNumber.BigNumber { return new BigNumber(Date.now() / 1000); }, -- cgit From cb68f9c9ec52463a1c9a865a446bc9acbc4b703d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 12:20:01 +0200 Subject: Remove tests for isValidSignatureUsingContractCallAsync --- test/exchange_wrapper_test.ts | 78 ------------------------------------------- 1 file changed, 78 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index aed3c3c0b..bd52f155a 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -51,84 +51,6 @@ describe('ExchangeWrapper', () => { afterEach(async () => { await blockchainLifecycle.revertAsync(); }); - describe('#isValidSignatureUsingContractCallAsync', () => { - // The Exchange smart contract `isValidSignature` method only validates orderHashes and assumes - // the length of the data is exactly 32 bytes. Thus for these tests, we use data of this size. - const dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0'; - const signature = { - v: 27, - r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33', - s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254', - }; - const address = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; - describe('should throw if passed a malformed signature', () => { - it('malformed v', async () => { - const malformedSignature = { - v: 34, - r: signature.r, - s: signature.s, - }; - return expect((zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), - ).to.be.rejected(); - }); - it('r lacks 0x prefix', async () => { - const malformedR = signature.r.replace('0x', ''); - const malformedSignature = { - v: signature.v, - r: malformedR, - s: signature.s, - }; - return expect((zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), - ).to.be.rejected(); - }); - it('r is too short', async () => { - const malformedR = signature.r.substr(10); - const malformedSignature = { - v: signature.v, - r: malformedR, - s: signature.s.replace('0', 'z'), - }; - return expect((zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), - ).to.be.rejected(); - }); - it('s is not hex', async () => { - const malformedS = signature.s.replace('0', 'z'); - const malformedSignature = { - v: signature.v, - r: signature.r, - s: malformedS, - }; - return expect((zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, malformedSignature, address), - ).to.be.rejected(); - }); - }); - it('should return false if the data doesn\'t pertain to the signature & address', async () => { - const isValid = await (zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync('0x0', signature, address); - expect(isValid).to.be.false(); - }); - it('should return false if the address doesn\'t pertain to the signature & dataHex', async () => { - const validUnrelatedAddress = '0x8b0292B11a196601eD2ce54B665CaFEca0347D42'; - const isValid = await (zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, signature, validUnrelatedAddress); - expect(isValid).to.be.false(); - }); - it('should return false if the signature doesn\'t pertain to the dataHex & address', async () => { - const wrongSignature = {...signature, v: 28}; - const isValid = await (zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, wrongSignature, address); - expect(isValid).to.be.false(); - }); - it('should return true if the signature does pertain to the dataHex & address', async () => { - const isValid = await (zeroEx.exchange as any) - .isValidSignatureUsingContractCallAsync(dataHex, signature, address); - expect(isValid).to.be.true(); - }); - }); describe('#fillOrKillOrderAsync', () => { let makerTokenAddress: string; let takerTokenAddress: string; -- cgit From a0fd89aeebc41b52b845674fd83edeccbb7f7bd8 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 14:24:12 +0200 Subject: Rewrite isValidSignature tests --- package.json | 2 +- test/0x.js_test.ts | 76 ++++++++++++++++++------------------------------------ yarn.lock | 10 +++---- 3 files changed, 31 insertions(+), 57 deletions(-) diff --git a/package.json b/package.json index 26ae8a504..9d774ed7b 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "bignumber.js": "^4.0.2", "chai": "^4.0.1", "chai-as-promised": "^6.0.0", - "chai-as-promised-typescript-typings": "0.0.2", + "chai-as-promised-typescript-typings": "0.0.3", "chai-bignumber": "git+ssh://git@github.com:0xProject/chai-bignumber.git", "chai-typescript-typings": "^0.0.0", "copyfiles": "^1.2.0", diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 58f259a11..78032006b 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -6,8 +6,9 @@ import * as BigNumber from 'bignumber.js'; import * as Sinon from 'sinon'; import {ZeroEx} from '../src/0x.js'; import {constants} from './utils/constants'; -import {web3Factory} from './utils/web3_factory'; import {Order} from '../src/types'; +import {ECSignature} from '../src/types'; +import {web3Factory} from './utils/web3_factory'; chaiSetup.configure(); const expect = chai.expect; @@ -51,60 +52,33 @@ describe('ZeroEx library', () => { s: '0x2d887fd3b17bfdce3481f10bea41f45ba9f709d39ce8325427b57afcfc994cee', }; const address = '0x9b2055d370f73ec7d8a03e965129118dc8f5bf83'; - describe('should throw if passed a malformed signature', () => { - it('malformed v', () => { - const malformedSignature = { - v: 34, - r: signature.r, - s: signature.s, - }; - expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); - }); - it('r lacks 0x prefix', () => { - const malformedR = signature.r.replace('0x', ''); - const malformedSignature = { - v: signature.v, - r: malformedR, - s: signature.s, - }; - expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); - }); - it('r is too short', () => { - const malformedR = signature.r.substr(10); - const malformedSignature = { - v: signature.v, - r: malformedR, - s: signature.s.replace('0', 'z'), - }; - expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); - }); - it('s is not hex', () => { - const malformedS = signature.s.replace('0', 'z'); - const malformedSignature = { - v: signature.v, - r: signature.r, - s: malformedS, - }; - expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); - }); - }); - it('should return false if the data doesn\'t pertain to the signature & address', () => { - const isValid = ZeroEx.isValidSignature('0x0', signature, address); - expect(isValid).to.be.false(); + const web3 = web3Factory.create(); + const zeroEx = new ZeroEx(web3); + it('should return false if the data doesn\'t pertain to the signature & address', async () => { + expect(ZeroEx.isValidSignature('0x0', signature, address)).to.be.false(); + return expect( + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync('0x0', signature, address), + ).to.become(false); }); - it('should return false if the address doesn\'t pertain to the signature & data', () => { + it('should return false if the address doesn\'t pertain to the signature & data', async () => { const validUnrelatedAddress = '0x8b0292B11a196601eD2ce54B665CaFEca0347D42'; - const isValid = ZeroEx.isValidSignature(data, signature, validUnrelatedAddress); - expect(isValid).to.be.false(); + expect(ZeroEx.isValidSignature(data, signature, validUnrelatedAddress)).to.be.false(); + return expect( + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, validUnrelatedAddress) + ).to.become(false); }); - it('should return false if the signature doesn\'t pertain to the data & address', () => { + it('should return false if the signature doesn\'t pertain to the data & address', async () => { const wrongSignature = _.assign({}, signature, {v: 28}); - const isValid = ZeroEx.isValidSignature(data, wrongSignature, address); - expect(isValid).to.be.false(); - }); - it('should return true if the signature does pertain to the data & address', () => { - const isValid = ZeroEx.isValidSignature(data, signature, address); - expect(isValid).to.be.true(); + expect(ZeroEx.isValidSignature(data, wrongSignature, address)).to.be.false(); + return expect( + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, wrongSignature, address) + ).to.become(false); + }); + it('should return true if the signature does pertain to the data & address', async () => { + expect(ZeroEx.isValidSignature(data, signature, address)).to.be.true(); + return expect( + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, address) + ).to.become(true); }); }); describe('#generateSalt', () => { diff --git a/yarn.lock b/yarn.lock index 24104702b..abe7ca550 100644 --- a/yarn.lock +++ b/yarn.lock @@ -902,9 +902,9 @@ center-align@^0.1.1: align-text "^0.1.3" lazy-cache "^1.0.3" -chai-as-promised-typescript-typings@0.0.2: - version "0.0.2" - resolved "https://registry.yarnpkg.com/chai-as-promised-typescript-typings/-/chai-as-promised-typescript-typings-0.0.2.tgz#5df99c418917a78eb314e5f83f306cb95ae846cb" +chai-as-promised-typescript-typings@0.0.3: + version "0.0.3" + resolved "https://registry.yarnpkg.com/chai-as-promised-typescript-typings/-/chai-as-promised-typescript-typings-0.0.3.tgz#8694287ebe2dd6c18a96667c38151d714d6ecbb6" dependencies: chai-typescript-typings "^0.0.0" @@ -1169,13 +1169,13 @@ debug-log@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/debug-log/-/debug-log-1.0.1.tgz#2307632d4c04382b8df8a32f70b895046d52745f" -debug@2.6.0, debug@^2.1.1, debug@^2.2.0: +debug@2.6.0: version "2.6.0" resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.0.tgz#bc596bcabe7617f11d9fa15361eded5608b8499b" dependencies: ms "0.7.2" -debug@^2.6.3: +debug@^2.1.1, debug@^2.2.0, debug@^2.6.3: version "2.6.8" resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.8.tgz#e731531ca2ede27d188222427da17821d68ff4fc" dependencies: -- cgit From 807e62e76e87ed8eb12ebf8d7b6f1a92e9d34f3b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 14:31:05 +0200 Subject: Fix linter error --- test/0x.js_test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 78032006b..d41cbefd7 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -64,20 +64,20 @@ describe('ZeroEx library', () => { const validUnrelatedAddress = '0x8b0292B11a196601eD2ce54B665CaFEca0347D42'; expect(ZeroEx.isValidSignature(data, signature, validUnrelatedAddress)).to.be.false(); return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, validUnrelatedAddress) + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, validUnrelatedAddress), ).to.become(false); }); it('should return false if the signature doesn\'t pertain to the data & address', async () => { const wrongSignature = _.assign({}, signature, {v: 28}); expect(ZeroEx.isValidSignature(data, wrongSignature, address)).to.be.false(); return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, wrongSignature, address) + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, wrongSignature, address), ).to.become(false); }); it('should return true if the signature does pertain to the data & address', async () => { expect(ZeroEx.isValidSignature(data, signature, address)).to.be.true(); return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, address) + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, address), ).to.become(true); }); }); -- cgit From 96e52ea3cc8efbbff1c43ed87b45e2fc3f57348b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 15:02:43 +0200 Subject: Fix names in types --- src/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types.ts b/src/types.ts index 90e64bc46..edd7f2d33 100644 --- a/src/types.ts +++ b/src/types.ts @@ -69,9 +69,9 @@ export interface ExchangeContract extends ContractInstance { shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts?: TxOpts) => number; }; batchFill: { - (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmount: BigNumber.BigNumber[], + (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmounts: BigNumber.BigNumber[], shouldCheckTransfer: boolean, v: number[], r: string[], s: string[], txOpts?: TxOpts): ContractResponse; - estimateGas: (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmount: BigNumber.BigNumber[], + estimateGas: (orderAddresses: OrderAddresses[], orderValues: OrderValues[], fillAmounts: BigNumber.BigNumber[], shouldCheckTransfer: boolean, v: number[], r: string[], s: string[], txOpts?: TxOpts) => number; }; cancel: { -- cgit From 49d8b5b18b48604f852038662a0bb0ea598671e0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 16:28:55 +0200 Subject: Address feedback --- src/contract_wrappers/exchange_wrapper.ts | 21 ++++++++++++--------- test/exchange_wrapper_test.ts | 6 +++--- test/utils/fill_scenarios.ts | 18 ++++++++++++++---- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 94de45dd8..5caa1da2d 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -127,7 +127,7 @@ export class ExchangeWrapper extends ContractWrapper { return cancelledAmountInBaseUnits; } /** - * Fills a signed order with a takerTokenFillAmount denominated in baseUnits of the taker token. + * Fills a signed order with an amount denominated in baseUnits of the taker token. * Since the order in which transactions are included in the next block is indeterminate, race-conditions * could arise where a users balance or allowance changes before the fillOrder executes. Because of this, * we allow you to specify `shouldCheckTransfer`. If true, the smart contract will not throw if while @@ -176,25 +176,27 @@ export class ExchangeWrapper extends ContractWrapper { this.throwErrorLogsAsErrors(response.logs); } /** - * Batched version of fillOrderAsync. Executes fills atomically in a single transaction. + * Batch version of fillOrderAsync. + * Executes multiple fills atomically in a single transaction. + * If shouldCheckTransfer is set to true, it will continue filling subsequent orders even when earlier ones fail. + * When shouldCheckTransfer is set to false, if any fill fails, the entire batch fails. */ public async batchFillOrderAsync(orderFillRequests: OrderFillRequest[], shouldCheckTransfer: boolean, takerAddress: string): Promise { - if (_.isEmpty(orderFillRequests)) { - return; // no-op - } assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); _.forEach(orderFillRequests, - async (orderFillRequest: OrderFillRequest) => { - assert.doesConformToSchema('signedOrder', + async (orderFillRequest: OrderFillRequest, i: number) => { + assert.doesConformToSchema(`orderFillRequests[${i}].signedOrder`, SchemaValidator.convertToJSONSchemaCompatibleObject(orderFillRequest.signedOrder as object), signedOrderSchema); - assert.isBigNumber('takerTokenFillAmount', orderFillRequest.takerTokenFillAmount); + assert.isBigNumber(`orderFillRequests[${i}].takerTokenFillAmount`, orderFillRequest.takerTokenFillAmount); await this.validateFillOrderAndThrowIfInvalidAsync( orderFillRequest.signedOrder, orderFillRequest.takerTokenFillAmount, takerAddress); }); - const exchangeInstance = await this.getExchangeContractAsync(); + if (_.isEmpty(orderFillRequests)) { + return; // no-op + } const orderAddressesValuesAmountsAndSignatureArray = _.map(orderFillRequests, orderFillRequest => { return [ @@ -210,6 +212,7 @@ export class ExchangeWrapper extends ContractWrapper { orderAddressesValuesAmountsAndSignatureArray, ); + const exchangeInstance = await this.getExchangeContractAsync(); const gas = await exchangeInstance.batchFill.estimateGas( orderAddressesArray, orderValuesArray, diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 2d7810bcc..5cc864d12 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -229,9 +229,9 @@ describe('ExchangeWrapper', () => { const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - const nonExistentSenderAddress = userAddresses[6]; + const nonTakerAddress = userAddresses[6]; return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, nonExistentSenderAddress, + signedOrder, fillTakerAmount, shouldCheckTransfer, nonTakerAddress, )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { @@ -520,7 +520,7 @@ describe('ExchangeWrapper', () => { order: signedOrderWithDifferentMaker, takerTokenCancelAmount: cancelAmount, }, - ])).to.be.rejectedWith('Can not cancel orders from multiple makers in a single batch'); + ])).to.be.rejectedWith(ExchangeContractErrs.MULTIPLE_MAKERS_IN_SINGLE_CANCEL_BATCH); }); }); describe('successful batch cancels', () => { diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 8ff27ceb1..539333b9b 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -72,19 +72,29 @@ export class FillScenarios { await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinbase, makerAddress, makerFillableAmount); const oldMakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(makerTokenAddress, makerAddress); await this.zeroEx.token.setProxyAllowanceAsync( - makerTokenAddress, makerAddress, oldMakerAllowance.plus(makerFillableAmount)); + makerTokenAddress, makerAddress, oldMakerAllowance.plus(makerFillableAmount), + ); await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinbase, takerAddress, takerFillableAmount); const oldTakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(takerTokenAddress, takerAddress); await this.zeroEx.token.setProxyAllowanceAsync( - takerTokenAddress, takerAddress, oldTakerAllowance.plus(takerFillableAmount)); + takerTokenAddress, takerAddress, oldTakerAllowance.plus(takerFillableAmount), + ); if (!makerFee.isZero()) { await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, makerAddress, makerFee); - await this.zeroEx.token.setProxyAllowanceAsync(this.zrxTokenAddress, makerAddress, makerFee); + const oldMakerFeeAllowance = + await this.zeroEx.token.getProxyAllowanceAsync(this.zrxTokenAddress, makerAddress); + await this.zeroEx.token.setProxyAllowanceAsync( + this.zrxTokenAddress, makerAddress, oldMakerFeeAllowance.plus(makerFee), + ); } if (!takerFee.isZero()) { await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, takerAddress, takerFee); - await this.zeroEx.token.setProxyAllowanceAsync(this.zrxTokenAddress, takerAddress, takerFee); + const oldTakerFeeAllowance = + await this.zeroEx.token.getProxyAllowanceAsync(this.zrxTokenAddress, takerAddress); + await this.zeroEx.token.setProxyAllowanceAsync( + this.zrxTokenAddress, takerAddress, oldTakerFeeAllowance.plus(takerFee), + ); } const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, -- cgit From ea4cf16eae4bab31c5da522bb582a5c83af53705 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 17:02:18 +0200 Subject: Remove unused import --- src/utils/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/utils.ts b/src/utils/utils.ts index ded2d31fd..5786bab07 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -4,7 +4,6 @@ import * as ethABI from 'ethereumjs-abi'; import * as ethUtil from 'ethereumjs-util'; import {Order, SignedOrder, SolidityTypes} from '../types'; import * as BigNumber from 'bignumber.js'; -import {ECSignature} from '../types'; export const utils = { /** -- cgit From 6629a716b9dccbc5f07f60830dfe356c4c434f52 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 17:18:00 +0200 Subject: Don't pass expressions as parameters --- test/utils/fill_scenarios.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 539333b9b..a90fc5f57 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -71,13 +71,15 @@ export class FillScenarios { feeRecepient: string, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinbase, makerAddress, makerFillableAmount); const oldMakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(makerTokenAddress, makerAddress); + const newMakerAllowance = oldMakerAllowance.plus(makerFillableAmount); await this.zeroEx.token.setProxyAllowanceAsync( - makerTokenAddress, makerAddress, oldMakerAllowance.plus(makerFillableAmount), + makerTokenAddress, makerAddress, newMakerAllowance, ); await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinbase, takerAddress, takerFillableAmount); const oldTakerAllowance = await this.zeroEx.token.getProxyAllowanceAsync(takerTokenAddress, takerAddress); + const newTakerAllowance = oldTakerAllowance.plus(takerFillableAmount); await this.zeroEx.token.setProxyAllowanceAsync( - takerTokenAddress, takerAddress, oldTakerAllowance.plus(takerFillableAmount), + takerTokenAddress, takerAddress, newTakerAllowance, ); if (!makerFee.isZero()) { -- cgit From f7953511bb1886c6fe3810462926bdbdd0088781 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 8 Jun 2017 17:18:55 +0200 Subject: Don't pass expressions as parameters --- test/utils/fill_scenarios.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index a90fc5f57..2860f1472 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -86,16 +86,18 @@ export class FillScenarios { await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, makerAddress, makerFee); const oldMakerFeeAllowance = await this.zeroEx.token.getProxyAllowanceAsync(this.zrxTokenAddress, makerAddress); + const newMakerFeeAllowance = oldMakerFeeAllowance.plus(makerFee); await this.zeroEx.token.setProxyAllowanceAsync( - this.zrxTokenAddress, makerAddress, oldMakerFeeAllowance.plus(makerFee), + this.zrxTokenAddress, makerAddress, newMakerFeeAllowance, ); } if (!takerFee.isZero()) { await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, takerAddress, takerFee); const oldTakerFeeAllowance = await this.zeroEx.token.getProxyAllowanceAsync(this.zrxTokenAddress, takerAddress); + const newTakerFeeAllowance = oldTakerFeeAllowance.plus(takerFee); await this.zeroEx.token.setProxyAllowanceAsync( - this.zrxTokenAddress, takerAddress, oldTakerFeeAllowance.plus(takerFee), + this.zrxTokenAddress, takerAddress, newTakerFeeAllowance, ); } -- cgit From 31cc75bd6d2651466ebf50e9374d5cd19de6dd5e Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 8 Jun 2017 18:02:31 +0200 Subject: Fix isValidSignature tests --- test/0x.js_test.ts | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index d33480c6b..1349d6360 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -43,15 +43,15 @@ describe('ZeroEx library', () => { }); }); describe('#isValidSignature', () => { - // This test data was borrowed from the JSON RPC documentation - // Source: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign - const data = '0xdeadbeaf'; + // The Exchange smart contract `isValidSignature` method only validates orderHashes and assumes + // the length of the data is exactly 32 bytes. Thus for these tests, we use data of this size. + const dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0'; const signature = { v: 27, - r: '0xa3f20717a250c2b0b729b7e5becbff67fdaef7e0699da4de7ca5895b02a170a1', - s: '0x2d887fd3b17bfdce3481f10bea41f45ba9f709d39ce8325427b57afcfc994cee', + r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33', + s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254', }; - const address = '0x9b2055d370f73ec7d8a03e965129118dc8f5bf83'; + const address = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; const web3 = web3Factory.create(); const zeroEx = new ZeroEx(web3); it('should return false if the data doesn\'t pertain to the signature & address', async () => { @@ -62,23 +62,25 @@ describe('ZeroEx library', () => { }); it('should return false if the address doesn\'t pertain to the signature & data', async () => { const validUnrelatedAddress = '0x8b0292B11a196601eD2ce54B665CaFEca0347D42'; - expect(ZeroEx.isValidSignature(data, signature, validUnrelatedAddress)).to.be.false(); + expect(ZeroEx.isValidSignature(dataHex, signature, validUnrelatedAddress)).to.be.false(); return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, validUnrelatedAddress), + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(dataHex, signature, + validUnrelatedAddress), ).to.become(false); }); - it('should return false if the signature doesn\'t pertain to the data & address', async () => { + it('should return false if the signature doesn\'t pertain to the dataHex & address', async () => { const wrongSignature = _.assign({}, signature, {v: 28}); - expect(ZeroEx.isValidSignature(data, wrongSignature, address)).to.be.false(); + expect(ZeroEx.isValidSignature(dataHex, wrongSignature, address)).to.be.false(); return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, wrongSignature, address), + (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(dataHex, wrongSignature, address), ).to.become(false); }); - it('should return true if the signature does pertain to the data & address', async () => { - expect(ZeroEx.isValidSignature(data, signature, address)).to.be.true(); - return expect( - (zeroEx.exchange as any).isValidSignatureUsingContractCallAsync(data, signature, address), - ).to.become(true); + it('should return true if the signature does pertain to the dataHex & address', async () => { + const isValidSignatureLocal = ZeroEx.isValidSignature(dataHex, signature, address); + expect(isValidSignatureLocal).to.be.true(); + const isValidSignatureOnContract = await (zeroEx.exchange as any) + .isValidSignatureUsingContractCallAsync(dataHex, signature, address); + return expect(isValidSignatureOnContract).to.be.true(); }); }); describe('#generateSalt', () => { -- cgit