From 44f11442424c88d1130ee398d0714636c5ade045 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:21:01 +0200 Subject: Add comment for fillOrderAsync method, rename fillAmount to fillTakerAmountInBaseUnits and remove default value for shouldCheckTransfer --- src/contract_wrappers/exchange_wrapper.ts | 17 +++++++++++------ test/exchange_wrapper_test.ts | 7 ++++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dbb427d2c..40f22bd28 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -57,18 +57,23 @@ export class ExchangeWrapper extends ContractWrapper { ); return isValidSignature; } - public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, - shouldCheckTransfer: boolean = true): Promise { + /** + * Fills a signed order with a specified amount in baseUnits of the taker token. The caller can + * decide whether they want the call to throw if the balance/allowance checks fail by setting + * shouldCheckTransfer to false. If true, the call will fail without throwing, preserving gas costs. + */ + public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, + shouldCheckTransfer: boolean): Promise { assert.doesConformToSchema('signedOrder', SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); - assert.isBigNumber('fillAmount', fillAmount); + assert.isBigNumber('fillTakerAmountInBaseUnits', fillTakerAmountInBaseUnits); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); - this.validateFillOrder(signedOrder, fillAmount, senderAddress, shouldCheckTransfer); + this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress, shouldCheckTransfer); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -88,7 +93,7 @@ export class ExchangeWrapper extends ContractWrapper { const gas = await exchangeInstance.fill.estimateGas( orderAddresses, orderValues, - fillAmount, + fillTakerAmountInBaseUnits, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -100,7 +105,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - fillAmount, + fillTakerAmountInBaseUnits, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index c5cbc58be..f324c5492 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -107,6 +107,7 @@ describe('ExchangeWrapper', () => { let tokens: Token[]; const addressBySymbol: {[symbol: string]: string} = {}; let networkId: number; + const shouldCheckTransfer = false; const setBalance = async (toAddress: string, amountInBaseUnits: BigNumber.BigNumber|number, tokenAddress: string) => { @@ -133,7 +134,7 @@ describe('ExchangeWrapper', () => { const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker, 5, addressBySymbol.MLN, 5, addressBySymbol.GNT); const fillAmount = new BigNumber(0); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount)) + expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount, shouldCheckTransfer)) .to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); }); it('should throw when sender is not a taker', async () => { @@ -142,7 +143,7 @@ describe('ExchangeWrapper', () => { const signedOrder = await orderFactory.createSignedOrderAsync(zeroEx, networkId, maker, taker, 5, addressBySymbol.MLN, 5, addressBySymbol.GNT); const fillAmount = new BigNumber(5); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount)) + expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount, shouldCheckTransfer)) .to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); }); }); @@ -160,7 +161,7 @@ describe('ExchangeWrapper', () => { 5, addressBySymbol.MLN, 5, addressBySymbol.GNT); const fillAmount = new BigNumber(5); zeroEx.setDefaultAccount(taker); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount); + await zeroEx.exchange.fillOrderAsync(signedOrder, fillAmount, shouldCheckTransfer); expect(await zeroEx.token.getBalanceAsync(addressBySymbol.MLN, taker)).to.be.bignumber.equal(5); expect(await zeroEx.token.getBalanceAsync(addressBySymbol.GNT, taker)).to.be.bignumber.equal(0); }); -- cgit v1.2.3