From 8b81ea162fd32df0dc2dcbdbc47be0e1bb09ea0f Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 16 Jan 2018 11:08:17 +0100 Subject: Add a regression test for fillUpToValidation --- packages/0x.js/test/exchange_wrapper_test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index 39a5be61d..32f8514d9 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -536,7 +536,24 @@ describe('ExchangeWrapper', () => { ), ).to.be.rejectedWith(ExchangeContractErrs.BatchOrdersMustHaveAtLeastOneItem); }); - it('should successfully fill up to specified amount', async () => { + it('should successfully fill up to specified amount when all orders are fully funded', async () => { + const txHash = await zeroEx.exchange.fillOrdersUpToAsync( + signedOrders, + fillUpToAmount, + shouldThrowOnInsufficientBalanceOrAllowance, + takerAddress, + ); + await zeroEx.awaitTransactionMinedAsync(txHash); + const filledAmount = await zeroEx.exchange.getFilledTakerAmountAsync(signedOrderHashHex); + const anotherFilledAmount = await zeroEx.exchange.getFilledTakerAmountAsync(anotherOrderHashHex); + expect(filledAmount).to.be.bignumber.equal(fillableAmount); + const remainingFillAmount = fillableAmount.minus(1); + expect(anotherFilledAmount).to.be.bignumber.equal(remainingFillAmount); + }); + it('should successfully fill up to specified amount even if filling all orders will fail', async () => { + const missingBalance = new BigNumber(1); // User will still have enough balance to fill up to 9, + // but won't have 10 to fully fill all orders in a batch. + await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, missingBalance); const txHash = await zeroEx.exchange.fillOrdersUpToAsync( signedOrders, fillUpToAmount, -- cgit From 89d6326a83afc386d7d388776ab0a419ece20739 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 16 Jan 2018 11:22:04 +0100 Subject: Fix fillOrdersUpTo balances validation --- packages/0x.js/src/contract_wrappers/exchange_wrapper.ts | 6 ++++-- packages/0x.js/test/exchange_wrapper_test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts index e1d80e01a..be88cdb20 100644 --- a/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts +++ b/packages/0x.js/src/contract_wrappers/exchange_wrapper.ts @@ -258,16 +258,18 @@ export class ExchangeWrapper extends ContractWrapper { ? SHOULD_VALIDATE_BY_DEFAULT : orderTransactionOpts.shouldValidate; if (shouldValidate) { + let filledTakerTokenAmount = new BigNumber(0); const zrxTokenAddress = this.getZRXTokenAddress(); const exchangeTradeEmulator = new ExchangeTransferSimulator(this._tokenWrapper, BlockParamLiteral.Latest); for (const signedOrder of signedOrders) { - await this._orderValidationUtils.validateFillOrderThrowIfInvalidAsync( + const singleFilledTakerTokenAmount = await this._orderValidationUtils.validateFillOrderThrowIfInvalidAsync( exchangeTradeEmulator, signedOrder, - fillTakerTokenAmount, + fillTakerTokenAmount.minus(filledTakerTokenAmount), takerAddress, zrxTokenAddress, ); + filledTakerTokenAmount = filledTakerTokenAmount.plus(singleFilledTakerTokenAmount); } } diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index 32f8514d9..3e4c40b15 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -568,6 +568,20 @@ describe('ExchangeWrapper', () => { expect(anotherFilledAmount).to.be.bignumber.equal(remainingFillAmount); }); }); + describe('failed batch fills', () => { + it("should fail validation if user doesn't have enough balance wo fill up to", async () => { + const missingBalance = new BigNumber(2); // User will only have anough balance to fill up to 8 + await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, missingBalance); + return expect( + zeroEx.exchange.fillOrdersUpToAsync( + signedOrders, + fillUpToAmount, + shouldThrowOnInsufficientBalanceOrAllowance, + takerAddress, + ), + ).to.be.rejectedWith(ExchangeContractErrs.InsufficientMakerBalance); + }); + }); describe('order transaction options', () => { const emptyFillUpToAmount = new BigNumber(0); it('should validate when orderTransactionOptions are not present', async () => { -- cgit From 771f65c8582c48095d812b49e58ee5563f3329a9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 16 Jan 2018 11:25:31 +0100 Subject: Update the CHANGELOG --- packages/0x.js/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index ab2537879..150d57aaa 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -4,6 +4,7 @@ * Add an error parameter to the order watcher callback (#312) * Fix the bug making it impossible to catch some errors from awaitTransactionMinedAsync (#312) + * Fix the bug in fillOrdersUpTo validation making it impossible to fill up to if user doesn't have enough balance to fully fill all the orders (#321) ## v0.29.1 - _January 11, 2018_ -- cgit From 278f68ae770b810960b761bd9eb7b57a24975253 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 17 Jan 2018 16:22:47 +0100 Subject: Fix the typos --- packages/0x.js/CHANGELOG.md | 4 ++-- packages/0x.js/test/exchange_wrapper_test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/0x.js/CHANGELOG.md b/packages/0x.js/CHANGELOG.md index 150d57aaa..0a3b45513 100644 --- a/packages/0x.js/CHANGELOG.md +++ b/packages/0x.js/CHANGELOG.md @@ -3,8 +3,8 @@ ## v0.x.x - _TBD, 2018_ * Add an error parameter to the order watcher callback (#312) - * Fix the bug making it impossible to catch some errors from awaitTransactionMinedAsync (#312) - * Fix the bug in fillOrdersUpTo validation making it impossible to fill up to if user doesn't have enough balance to fully fill all the orders (#321) + * Fix a bug making it impossible to catch some errors from awaitTransactionMinedAsync (#312) + * Fix a bug in fillOrdersUpTo validation making it impossible to fill up to if user doesn't have enough balance to fully fill all the orders (#321) ## v0.29.1 - _January 11, 2018_ diff --git a/packages/0x.js/test/exchange_wrapper_test.ts b/packages/0x.js/test/exchange_wrapper_test.ts index 3e4c40b15..d2a2149a0 100644 --- a/packages/0x.js/test/exchange_wrapper_test.ts +++ b/packages/0x.js/test/exchange_wrapper_test.ts @@ -550,7 +550,7 @@ describe('ExchangeWrapper', () => { const remainingFillAmount = fillableAmount.minus(1); expect(anotherFilledAmount).to.be.bignumber.equal(remainingFillAmount); }); - it('should successfully fill up to specified amount even if filling all orders will fail', async () => { + it('should successfully fill up to specified amount even if filling all orders would fail', async () => { const missingBalance = new BigNumber(1); // User will still have enough balance to fill up to 9, // but won't have 10 to fully fill all orders in a batch. await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, missingBalance); @@ -569,8 +569,8 @@ describe('ExchangeWrapper', () => { }); }); describe('failed batch fills', () => { - it("should fail validation if user doesn't have enough balance wo fill up to", async () => { - const missingBalance = new BigNumber(2); // User will only have anough balance to fill up to 8 + it("should fail validation if user doesn't have enough balance without fill up to", async () => { + const missingBalance = new BigNumber(2); // User will only have enough balance to fill up to 8 await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, missingBalance); return expect( zeroEx.exchange.fillOrdersUpToAsync( -- cgit