diff options
| -rw-r--r-- | src/contract_wrappers/exchange_wrapper.ts | 21 | ||||
| -rw-r--r-- | test/exchange_wrapper_test.ts | 6 | ||||
| -rw-r--r-- | 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<void> { - 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, |