From a740498c8061bc7a3dafae82929b358e61bac2d4 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 11:23:39 +0200 Subject: Introduce coinBase account Add makerAccount parameter Adjust tests Add more assertions to success test --- src/0x.js.ts | 6 ++++++ src/web3_wrapper.ts | 3 +++ test/exchange_wrapper_test.ts | 33 ++++++++++++++++++++------------- test/utils/fill_scenarios.ts | 12 +++++++++--- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index a613d5f48..6713ec885 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -123,6 +123,12 @@ export class ZeroEx { public setTransactionSenderAccount(account: string): void { this.web3Wrapper.setDefaultAccount(account); } + /** + * Gets default account for sending transactions. + */ + public getTransactionSenderAccount(): string { + return this.web3Wrapper.getDefaultAccount(); + } /** * Computes the orderHash given the order parameters and returns it as a hex encoded string. */ diff --git a/src/web3_wrapper.ts b/src/web3_wrapper.ts index c1263222a..900c127d5 100644 --- a/src/web3_wrapper.ts +++ b/src/web3_wrapper.ts @@ -17,6 +17,9 @@ export class Web3Wrapper { public isAddress(address: string): boolean { return this.web3.isAddress(address); } + public getDefaultAccount(): string { + return this.web3.eth.defaultAccount; + } public setDefaultAccount(address: string): void { this.web3.eth.defaultAccount = address; } diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 77c78470b..716adb63a 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -108,16 +108,14 @@ describe('ExchangeWrapper', () => { let makerTokenAddress: string; let takerTokenAddress: string; let fillScenarios: FillScenarios; + let coinBase: string; + let makerAddress: string; let takerAddress: string; const fillTakerAmountInBaseUnits = new BigNumber(5); - const addressBySymbol: {[symbol: string]: string} = {}; const shouldCheckTransfer = false; before('fetch tokens', async () => { - takerAddress = userAddresses[1]; + [coinBase, makerAddress, takerAddress] = userAddresses; tokens = await zeroEx.tokenRegistry.getTokensAsync(); - _.forEach(tokens, token => { - addressBySymbol[token.symbol] = token.address; - }); const [makerToken, takerToken] = tokens; makerTokenAddress = makerToken.address; takerTokenAddress = takerToken.address; @@ -130,7 +128,7 @@ describe('ExchangeWrapper', () => { it('should throw when the fill amount is zero', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const zeroFillAmount = new BigNumber(0); zeroEx.setTransactionSenderAccount(takerAddress); @@ -140,7 +138,7 @@ describe('ExchangeWrapper', () => { it('should throw when sender is not a taker', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer)) .to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); @@ -149,7 +147,7 @@ describe('ExchangeWrapper', () => { const expirationInPast = new BigNumber(42); const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, expirationInPast, + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); zeroEx.setTransactionSenderAccount(takerAddress); expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer)) @@ -158,7 +156,7 @@ describe('ExchangeWrapper', () => { it('should throw when taker balance is less than fill amount', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); zeroEx.setTransactionSenderAccount(takerAddress); const moreThanTheBalance = new BigNumber(6); @@ -170,18 +168,27 @@ describe('ExchangeWrapper', () => { it('should fill the valid order', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, takerAddress, fillableAmount, + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - zeroEx.setTransactionSenderAccount(takerAddress); + + 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(fillTakerAmountInBaseUnits); + .to.be.bignumber.equal(fillableAmount); + zeroEx.setTransactionSenderAccount(takerAddress); await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer); + expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmountInBaseUnits)); + expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) + .to.be.bignumber.equal(fillTakerAmountInBaseUnits); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) .to.be.bignumber.equal(fillTakerAmountInBaseUnits); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) - .to.be.bignumber.equal(0); + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmountInBaseUnits)); }); }); }); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index ac233ad50..26840bb0c 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -7,23 +7,29 @@ export class FillScenarios { private zeroEx: ZeroEx; private userAddresses: string[]; private tokens: Token[]; + private coinBase: string; constructor(zeroEx: ZeroEx, userAddresses: string[], tokens: Token[]) { this.zeroEx = zeroEx; this.userAddresses = userAddresses; this.tokens = tokens; + this.coinBase = userAddresses[0]; } public async createAFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, - takerAddress: string, fillableAmount: BigNumber.BigNumber, + makerAddress: string, takerAddress: string, + fillableAmount: BigNumber.BigNumber, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { - const [makerAddress] = this.userAddresses; + await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, fillableAmount); await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, fillableAmount); - await this.zeroEx.token.transferAsync(takerTokenAddress, makerAddress, takerAddress, fillableAmount); + await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, fillableAmount); await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount); + const transactionSenderAccount = this.zeroEx.getTransactionSenderAccount(); + this.zeroEx.setTransactionSenderAccount(makerAddress); const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress, expirationUnixTimestampSec); + this.zeroEx.setTransactionSenderAccount(transactionSenderAccount); return signedOrder; } } -- cgit v1.2.3 From 5155f4980440f1c91006b124406c8b4a7a90e300 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:03:52 +0200 Subject: Add test for insufficient balance and make all async tests async --- src/contract_wrappers/exchange_wrapper.ts | 3 ++ test/exchange_wrapper_test.ts | 47 +++++++++++++++++++++---------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index eace55f4d..e2ac07b55 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -141,6 +141,9 @@ export class ExchangeWrapper extends ContractWrapper { if (fillAmount.greaterThan(takerBalance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); } + if (fillAmount.greaterThan(takerAllowance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 716adb63a..8d39c9e57 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -50,37 +50,37 @@ describe('ExchangeWrapper', () => { r: signature.r, s: signature.s, }; - expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) + return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) .to.be.rejected(); }); - it('r lacks 0x prefix', () => { + it('r lacks 0x prefix', async () => { const malformedR = signature.r.replace('0x', ''); const malformedSignature = { v: signature.v, r: malformedR, s: signature.s, }; - expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) + return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) .to.be.rejected(); }); - it('r is too short', () => { + 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'), }; - expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) + return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) .to.be.rejected(); }); - it('s is not hex', () => { + it('s is not hex', async () => { const malformedS = signature.s.replace('0', 'z'); const malformedSignature = { v: signature.v, r: signature.r, s: malformedS, }; - expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) + return expect(zeroEx.exchange.isValidSignatureAsync(dataHex, malformedSignature, address)) .to.be.rejected(); }); }); @@ -132,16 +132,18 @@ describe('ExchangeWrapper', () => { ); const zeroFillAmount = new BigNumber(0); zeroEx.setTransactionSenderAccount(takerAddress); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, zeroFillAmount, shouldCheckTransfer)) - .to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, zeroFillAmount, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); }); it('should throw when sender is not a taker', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer)) - .to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); }); it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(42); @@ -150,8 +152,9 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); zeroEx.setTransactionSenderAccount(takerAddress); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer)) - .to.be.rejectedWith(FillOrderValidationErrs.EXPIRED); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.EXPIRED); }); it('should throw when taker balance is less than fill amount', async () => { const fillableAmount = new BigNumber(5); @@ -160,8 +163,22 @@ describe('ExchangeWrapper', () => { ); zeroEx.setTransactionSenderAccount(takerAddress); const moreThanTheBalance = new BigNumber(6); - expect(zeroEx.exchange.fillOrderAsync(signedOrder, moreThanTheBalance, shouldCheckTransfer)) - .to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, moreThanTheBalance, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + }); + it('should throw when taker allowance is less than fill amount', async () => { + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1); + await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFillAmount); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); }); }); describe('successful fills', () => { -- cgit v1.2.3 From 4caf1b009ea253d08e1cd6ecc4e914606e2d31c1 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:08:25 +0200 Subject: Add test for valid partial fill --- test/exchange_wrapper_test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 8d39c9e57..0c1f552b6 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -207,6 +207,23 @@ describe('ExchangeWrapper', () => { expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmountInBaseUnits)); }); + it('should partially fill the valid order', async () => { + const fillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + const partialFillAmount = new BigNumber(3); + zeroEx.setTransactionSenderAccount(takerAddress); + await zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer); + 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)); + }); }); }); }); -- cgit v1.2.3 From fc7592d73f1613b11e4b48904bfb041cccca5800 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 2 Jun 2017 12:15:50 +0200 Subject: Fix getTransactionSenderAccountIfExistsAsync to return true senderAddress or undefined if non available --- src/0x.js.ts | 7 ++++--- src/web3_wrapper.ts | 2 +- test/utils/fill_scenarios.ts | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 6713ec885..679d748e7 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -124,10 +124,11 @@ export class ZeroEx { this.web3Wrapper.setDefaultAccount(account); } /** - * Gets default account for sending transactions. + * Get the default account set for sending transactions. */ - public getTransactionSenderAccount(): string { - return this.web3Wrapper.getDefaultAccount(); + public async getTransactionSenderAccountIfExistsAsync(): Promise { + const senderAccountIfExists = await this.web3Wrapper.getSenderAddressIfExistsAsync(); + return senderAccountIfExists; } /** * Computes the orderHash given the order parameters and returns it as a hex encoded string. diff --git a/src/web3_wrapper.ts b/src/web3_wrapper.ts index 900c127d5..49bd8b67d 100644 --- a/src/web3_wrapper.ts +++ b/src/web3_wrapper.ts @@ -69,7 +69,7 @@ export class Web3Wrapper { const {timestamp} = await promisify(this.web3.eth.getBlock)(blockHash); return timestamp; } - private async getSenderAddressIfExistsAsync(): Promise { + public async getSenderAddressIfExistsAsync(): Promise { const defaultAccount = this.web3.eth.defaultAccount; if (!_.isUndefined(defaultAccount)) { return defaultAccount; diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 26840bb0c..3b66937e6 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -24,12 +24,12 @@ export class FillScenarios { await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, fillableAmount); await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount); - const transactionSenderAccount = this.zeroEx.getTransactionSenderAccount(); + const transactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); this.zeroEx.setTransactionSenderAccount(makerAddress); const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress, expirationUnixTimestampSec); - this.zeroEx.setTransactionSenderAccount(transactionSenderAccount); + this.zeroEx.setTransactionSenderAccount(transactionSenderAccount as string); return signedOrder; } } -- cgit v1.2.3