From 9756aa86b051940b88f87fbecb313bf07590eca3 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:34:01 +0200 Subject: Add getZRXTokenAddressAsync --- src/contract_wrappers/exchange_wrapper.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index f0f6e79f7..0c7f27507 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -78,7 +78,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - const zrxTokenAddress = await exchangeInstance.ZRX.call(); + const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); await this.validateFillOrderAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ @@ -226,4 +226,7 @@ export class ExchangeWrapper extends ContractWrapper { this.exchangeContractIfExists = contractInstance as ExchangeContract; return this.exchangeContractIfExists; } + private async getZRXTokenAddressAsync(exchangeInstance: ExchangeContract): Promise { + return exchangeInstance.ZRX.call(); + } } -- cgit From ef3a27ed0545e2e30e965b928ae13c8a53e16e8d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:35:05 +0200 Subject: Rename validation functions --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 0c7f27507..965b31710 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -79,7 +79,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); - await this.validateFillOrderAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -123,8 +123,8 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string): Promise { + private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, zrxTokenAddress: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -135,7 +135,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.EXPIRED); } - await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmount, + await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmount, @@ -143,10 +143,10 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); } } - private async validateFillOrderBalancesAndAllowancesAsync(signedOrder: SignedOrder, - fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, - zrxTokenAddress: string): Promise { + private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, + zrxTokenAddress: string): Promise { // TODO: There is a possibility that the user might have enough funds // to fulfill the order or pay fees but not both. This will happen if // makerToken === zrxToken || makerToken === zrxToken -- cgit From 0f7bd0597234dbeafcee31e6f715f3c2004696df Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:37:28 +0200 Subject: Don't pass zrxTokenAsign --- src/contract_wrappers/exchange_wrapper.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 965b31710..d7d56c276 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -78,8 +78,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -123,8 +122,9 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string): Promise { + private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -134,7 +134,7 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); } - + const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); @@ -226,7 +226,8 @@ export class ExchangeWrapper extends ContractWrapper { this.exchangeContractIfExists = contractInstance as ExchangeContract; return this.exchangeContractIfExists; } - private async getZRXTokenAddressAsync(exchangeInstance: ExchangeContract): Promise { + private async getZRXTokenAddressAsync(): Promise { + const exchangeInstance = await this.getExchangeContractAsync(); return exchangeInstance.ZRX.call(); } } -- cgit From e5cc0e0562eccf67eff9cf0521c4884ffc3b0f3b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:38:47 +0200 Subject: Rename NOT_A_TAKER to TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/types.ts | 2 +- test/exchange_wrapper_test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index d7d56c276..ac15faff4 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -129,7 +129,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { - throw new Error(FillOrderValidationErrs.NOT_A_TAKER); + throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); diff --git a/src/types.ts b/src/types.ts index 57a9c721a..6e1499a7c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -94,7 +94,7 @@ export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; export const FillOrderValidationErrs = strEnum([ 'FILL_AMOUNT_IS_ZERO', - 'NOT_A_TAKER', + 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', 'EXPIRED', 'NOT_ENOUGH_TAKER_BALANCE', 'NOT_ENOUGH_TAKER_ALLOWANCE', diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 76601d1b3..d8934d409 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -147,7 +147,7 @@ describe('ExchangeWrapper', () => { ); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); + )).to.be.rejectedWith(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(42); -- cgit From f5158eebf335c9fbcfb7cfb6f32a0ecd1161391d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:39:21 +0200 Subject: Assign timestamp to a variable --- 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 ac15faff4..4c46404bb 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -131,7 +131,8 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } - if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { + const currentUnixTimestampSec = Date.now() / 1000; + if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { throw new Error(FillOrderValidationErrs.EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); -- cgit From 7fd84e29ab8c27cb872cf8cd0f631d4b78abba0e Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:40:06 +0200 Subject: Rename EXPIRED to FILL_ORDER_EXPIRED --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/types.ts | 2 +- test/exchange_wrapper_test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 4c46404bb..e957530ae 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -133,7 +133,7 @@ export class ExchangeWrapper extends ContractWrapper { } const currentUnixTimestampSec = Date.now() / 1000; if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { - throw new Error(FillOrderValidationErrs.EXPIRED); + throw new Error(FillOrderValidationErrs.FILL_ORDER_EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, diff --git a/src/types.ts b/src/types.ts index 6e1499a7c..e4a7da5d7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -95,7 +95,7 @@ export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; export const FillOrderValidationErrs = strEnum([ 'FILL_AMOUNT_IS_ZERO', 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', - 'EXPIRED', + 'FILL_ORDER_EXPIRED', 'NOT_ENOUGH_TAKER_BALANCE', 'NOT_ENOUGH_TAKER_ALLOWANCE', 'NOT_ENOUGH_MAKER_BALANCE', diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index d8934d409..e83134909 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -158,7 +158,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.EXPIRED); + )).to.be.rejectedWith(FillOrderValidationErrs.FILL_ORDER_EXPIRED); }); describe('should throw when not enough balance or allowance to fulfill the order', () => { const fillableAmount = new BigNumber(5); -- cgit From 54e8bf773091eb2c8587c72e50892afec42abb9d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:41:23 +0200 Subject: Assign wouldRoundingErrorOccur to a variable --- src/contract_wrappers/exchange_wrapper.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e957530ae..828994829 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -139,8 +139,10 @@ export class ExchangeWrapper extends ContractWrapper { await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); - if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmount, - signedOrder.makerTokenAmount)) { + const wouldRoundingErrorOccur = await this.isRoundingErrorAsync( + signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, + ); + if (wouldRoundingErrorOccur) { throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); } } -- cgit From abf2cf4c5e2ea6d09d862871adfa16ca108907e2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:00:37 +0200 Subject: Move FillOrderValidatinErrs to ExchangeContractErrs --- src/contract_wrappers/exchange_wrapper.ts | 25 ++++++++++++------------- src/types.ts | 12 +++--------- test/exchange_wrapper_test.ts | 26 +++++++++++++------------- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 828994829..716f9c77a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -5,7 +5,6 @@ import { ExchangeContract, ExchangeContractErrCodes, ExchangeContractErrs, - FillOrderValidationErrs, OrderValues, OrderAddresses, SignedOrder, @@ -126,14 +125,14 @@ export class ExchangeWrapper extends ContractWrapper { fillTakerAmount: BigNumber.BigNumber, senderAddress: string): Promise { if (fillTakerAmount.eq(0)) { - throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + throw new Error(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { - throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); + throw new Error(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } const currentUnixTimestampSec = Date.now() / 1000; if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { - throw new Error(FillOrderValidationErrs.FILL_ORDER_EXPIRED); + throw new Error(ExchangeContractErrs.ORDER_FILL_EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, @@ -143,7 +142,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, ); if (wouldRoundingErrorOccur) { - throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } } private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, @@ -168,16 +167,16 @@ export class ExchangeWrapper extends ContractWrapper { const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); } if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); } const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, @@ -189,16 +188,16 @@ export class ExchangeWrapper extends ContractWrapper { senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); } if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); } if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); } if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { diff --git a/src/types.ts b/src/types.ts index e4a7da5d7..6cd5a93fe 100644 --- a/src/types.ts +++ b/src/types.ts @@ -89,13 +89,6 @@ export const ExchangeContractErrs = strEnum([ 'ORDER_REMAINING_FILL_AMOUNT_ZERO', 'ORDER_FILL_ROUNDING_ERROR', 'FILL_BALANCE_ALLOWANCE_ERROR', -]); -export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; - -export const FillOrderValidationErrs = strEnum([ - 'FILL_AMOUNT_IS_ZERO', - 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', - 'FILL_ORDER_EXPIRED', 'NOT_ENOUGH_TAKER_BALANCE', 'NOT_ENOUGH_TAKER_ALLOWANCE', 'NOT_ENOUGH_MAKER_BALANCE', @@ -104,9 +97,10 @@ export const FillOrderValidationErrs = strEnum([ 'NOT_ENOUGH_TAKER_FEE_ALLOWANCE', 'NOT_ENOUGH_MAKER_FEE_BALANCE', 'NOT_ENOUGH_MAKER_FEE_ALLOWANCE', - 'ROUNDING_ERROR', + 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', + ]); -export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs; +export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; export interface ContractResponse { logs: ContractEvent[]; diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index e83134909..c05c5c26f 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -9,7 +9,7 @@ import promisify = require('es6-promisify'); import {web3Factory} from './utils/web3_factory'; import {ZeroEx} from '../src/0x.js'; import {BlockchainLifecycle} from './utils/blockchain_lifecycle'; -import {FillOrderValidationErrs, SignedOrder, Token} from '../src/types'; +import {ExchangeContractErrs, SignedOrder, Token} from '../src/types'; import {FillScenarios} from './utils/fill_scenarios'; import {TokenUtils} from './utils/token_utils'; @@ -138,7 +138,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, zeroFillAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); }); it('should throw when sender is not a taker', async () => { const fillableAmount = new BigNumber(5); @@ -147,7 +147,7 @@ describe('ExchangeWrapper', () => { ); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); + )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(42); @@ -158,7 +158,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.FILL_ORDER_EXPIRED); + )).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); @@ -176,7 +176,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); }); it('should throw when taker allowance is less than fill amount', async () => { const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); @@ -185,14 +185,14 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); }); it('should throw when maker balance is less than maker fill amount', async () => { await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, lackingBalance); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); }); it('should throw when maker allowance is less than maker fill amount', async () => { const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); @@ -201,7 +201,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); }); }); it('should throw when there would be a rounding error', async () => { @@ -215,7 +215,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.ROUNDING_ERROR); + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); describe('should raise when not enough balance or allowance to pay fees', () => { const fillableAmount = new BigNumber(5); @@ -234,7 +234,7 @@ describe('ExchangeWrapper', () => { await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, lackingBalance); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); }); it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { const newAllowanceWhichIsLessThanFees = makerFee.minus(1); @@ -242,14 +242,14 @@ describe('ExchangeWrapper', () => { newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); }); it('should throw when taker doesn\'t have enough balance to pay fees', async () => { const lackingBalance = new BigNumber(1); await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, lackingBalance); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); }); it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { const newAllowanceWhichIsLessThanFees = makerFee.minus(1); @@ -257,7 +257,7 @@ describe('ExchangeWrapper', () => { newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); }); }); }); -- cgit From 89d93494788f06d227628dfcfbd712e26ef02b77 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:05:27 +0200 Subject: Rewrite comment --- src/contract_wrappers/exchange_wrapper.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 716f9c77a..ba3b05758 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -65,7 +65,7 @@ export class ExchangeWrapper extends ContractWrapper { * 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 foregoes this check and causes the smart contract to throw instead. + * false forgoes this check and causes the smart contract to throw instead. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean): Promise { @@ -145,14 +145,20 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } } + + /** + * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used + * to pay fees (ZRX). It is possible for them to have enough for fees and the transfer but not both. + * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient + * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts + * will throw for these edge-cases. + * TODO: Throw errors before calling the smart contract for these edge-cases + * TODO: in order to minimize the callers gas costs. + */ private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise { - // TODO: There is a possibility that the user might have enough funds - // to fulfill the order or pay fees but not both. This will happen if - // makerToken === zrxToken || makerToken === zrxToken - // We don't check it for now. The contract checks it and throws. const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); -- cgit From 45fadeb690e27acccc776515ada8d39e3633194a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:06:06 +0200 Subject: Allign arguments --- src/contract_wrappers/exchange_wrapper.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ba3b05758..55ddcc46a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -161,12 +161,12 @@ export class ExchangeWrapper extends ContractWrapper { zrxTokenAddress: string): Promise { const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); + senderAddress); // How many taker tokens would you get for 1 maker token; const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); -- cgit From 76280dd5dbc5a5a2fb5f7230fa73d594f822c7e6 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:10:06 +0200 Subject: Address feedback --- src/contract_wrappers/exchange_wrapper.ts | 24 ++++++++++++------------ src/types.ts | 16 ++++++++-------- test/exchange_wrapper_test.ts | 16 ++++++++-------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 55ddcc46a..4aa532bdd 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -168,42 +168,42 @@ export class ExchangeWrapper extends ContractWrapper { const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); - // How many taker tokens would you get for 1 maker token; + // exchangeRate is the price of one maker token denominated in taker tokens const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); } if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); } const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); const makerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); + senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); } if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); } if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); } if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { diff --git a/src/types.ts b/src/types.ts index 6cd5a93fe..f58fa035c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -89,14 +89,14 @@ export const ExchangeContractErrs = strEnum([ 'ORDER_REMAINING_FILL_AMOUNT_ZERO', 'ORDER_FILL_ROUNDING_ERROR', 'FILL_BALANCE_ALLOWANCE_ERROR', - 'NOT_ENOUGH_TAKER_BALANCE', - 'NOT_ENOUGH_TAKER_ALLOWANCE', - 'NOT_ENOUGH_MAKER_BALANCE', - 'NOT_ENOUGH_MAKER_ALLOWANCE', - 'NOT_ENOUGH_TAKER_FEE_BALANCE', - 'NOT_ENOUGH_TAKER_FEE_ALLOWANCE', - 'NOT_ENOUGH_MAKER_FEE_BALANCE', - 'NOT_ENOUGH_MAKER_FEE_ALLOWANCE', + 'INSUFFICIENT_TAKER_BALANCE', + 'INSUFFICIENT_TAKER_ALLOWANCE', + 'INSUFFICIENT_MAKER_BALANCE', + 'INSUFFICIENT_MAKER_ALLOWANCE', + 'INSUFFICIENT_TAKER_FEE_BALANCE', + 'INSUFFICIENT_TAKER_FEE_ALLOWANCE', + 'INSUFFICIENT_MAKER_FEE_BALANCE', + 'INSUFFICIENT_MAKER_FEE_ALLOWANCE', 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', ]); diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index c05c5c26f..87a6fbb85 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -176,7 +176,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); }); it('should throw when taker allowance is less than fill amount', async () => { const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); @@ -185,14 +185,14 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + )).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, lackingBalance); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); + )).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); @@ -201,7 +201,7 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); }); }); it('should throw when there would be a rounding error', async () => { @@ -234,7 +234,7 @@ describe('ExchangeWrapper', () => { await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, lackingBalance); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + )).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); @@ -242,14 +242,14 @@ describe('ExchangeWrapper', () => { newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); }); it('should throw when taker doesn\'t have enough balance to pay fees', async () => { const lackingBalance = new BigNumber(1); await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, lackingBalance); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + )).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); @@ -257,7 +257,7 @@ describe('ExchangeWrapper', () => { newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, - )).to.be.rejectedWith(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); }); }); }); -- cgit From 199843718fba26db72db6196be5036339e0cf9f9 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:13:22 +0200 Subject: Remove spaces --- test/exchange_wrapper_test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 87a6fbb85..ed6f68954 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -115,7 +115,7 @@ describe('ExchangeWrapper', () => { let zrxTokenAddress: string; const fillTakerAmount = new BigNumber(5); const shouldCheckTransfer = false; - before('fetch tokens', async () => { + before(async () => { [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; tokens = await zeroEx.tokenRegistry.getTokensAsync(); const tokenUtils = new TokenUtils(tokens); @@ -171,7 +171,6 @@ describe('ExchangeWrapper', () => { ); }); it('should throw when taker balance is less than fill amount', async () => { - await zeroEx.token.transferAsync(takerTokenAddress, takerAddress, coinbase, lackingBalance); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( @@ -267,7 +266,6 @@ describe('ExchangeWrapper', () => { 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)) -- cgit From 16c296be1461a3ccc76fbdf68e78d8c8ccd3ed83 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:14:51 +0200 Subject: Add ZRX_TOKEN_NOT_IN_REGISTRY --- src/types.ts | 11 ++++++----- test/utils/token_utils.ts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/types.ts b/src/types.ts index f58fa035c..3da24abc1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,11 +10,12 @@ function strEnum(values: string[]): {[key: string]: string} { } export const ZeroExError = strEnum([ - 'CONTRACT_DOES_NOT_EXIST', - 'UNHANDLED_ERROR', - 'USER_HAS_NO_ASSOCIATED_ADDRESSES', - 'INVALID_SIGNATURE', - 'CONTRACT_NOT_DEPLOYED_ON_NETWORK', + 'CONTRACT_DOES_NOT_EXIST', + 'UNHANDLED_ERROR', + 'USER_HAS_NO_ASSOCIATED_ADDRESSES', + 'INVALID_SIGNATURE', + 'CONTRACT_NOT_DEPLOYED_ON_NETWORK', + 'ZRX_NOT_IN_TOKEN_REGISTRY', ]); export type ZeroExError = keyof typeof ZeroExError; diff --git a/test/utils/token_utils.ts b/test/utils/token_utils.ts index 11d334619..3d2faa959 100644 --- a/test/utils/token_utils.ts +++ b/test/utils/token_utils.ts @@ -11,7 +11,7 @@ export class TokenUtils { public getProtocolTokenOrThrow(): Token { const zrxToken = _.find(this.tokens, {symbol: PROTOCOL_TOKEN_SYMBOL}); if (_.isUndefined(zrxToken)) { - throw new Error(ZeroExError.CONTRACT_NOT_DEPLOYED_ON_NETWORK); + throw new Error(ZeroExError.ZRX_NOT_IN_TOKEN_REGISTRY); } return zrxToken; } -- cgit From a52f834debc1727cee97c28f59e0973ba33bde7c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:16:55 +0200 Subject: Fix test name --- 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 ed6f68954..f445e1202 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -203,7 +203,7 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); }); }); - it('should throw when there would be a rounding error', async () => { + it('should throw when there a rounding error would have occurred', async () => { const makerFillableAmount = new BigNumber(3); const takerFillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createAsymetricFillableSignedOrderAsync( -- cgit From 7cf60b589b280c578bcde4b9209e70d07c92c241 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:20:17 +0200 Subject: Address feedback --- test/exchange_wrapper_test.ts | 10 +++++----- test/utils/fill_scenarios.ts | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index f445e1202..842d6de32 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -204,11 +204,11 @@ describe('ExchangeWrapper', () => { }); }); it('should throw when there a rounding error would have occurred', async () => { - const makerFillableAmount = new BigNumber(3); - const takerFillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAsymetricFillableSignedOrderAsync( + const makerAmount = new BigNumber(3); + const takerAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, - makerFillableAmount, takerFillableAmount, + makerAmount, takerAmount, ); const fillTakerAmountThatCausesRoundingError = new BigNumber(3); zeroEx.setTransactionSenderAccount(takerAddress); @@ -216,7 +216,7 @@ describe('ExchangeWrapper', () => { signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); - describe('should raise when not enough balance or allowance to pay fees', () => { + 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); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 8260b76ab..f95b06663 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -22,7 +22,7 @@ export class FillScenarios { fillableAmount: BigNumber.BigNumber, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { - return this.createAsymetricFillableSignedOrderAsync( + return this.createAsymmetricFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, fillableAmount, expirationUnixTimestampSec, ); @@ -34,24 +34,24 @@ export class FillScenarios { fillableAmount: BigNumber.BigNumber, feeRecepient: string, expirationUnixTimestampSec?: BigNumber.BigNumber, ): Promise { - return this.createAsymetricFillableSignedOrderWithFeesAsync( + return this.createAsymmetricFillableSignedOrderWithFeesAsync( makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, fillableAmount, fillableAmount, feeRecepient, expirationUnixTimestampSec, ); } - public async createAsymetricFillableSignedOrderAsync( + public async createAsymmetricFillableSignedOrderAsync( makerTokenAddress: string, takerTokenAddress: string, makerAddress: string, takerAddress: string, makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { const makerFee = new BigNumber(0); const takerFee = new BigNumber(0); const feeRecepient = constants.NULL_ADDRESS; - return this.createAsymetricFillableSignedOrderWithFeesAsync( + return this.createAsymmetricFillableSignedOrderWithFeesAsync( makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, makerFillableAmount, takerFillableAmount, feeRecepient, expirationUnixTimestampSec, ); } - private async createAsymetricFillableSignedOrderWithFeesAsync( + private async createAsymmetricFillableSignedOrderWithFeesAsync( makerTokenAddress: string, takerTokenAddress: string, makerFee: BigNumber.BigNumber, takerFee: BigNumber.BigNumber, makerAddress: string, takerAddress: string, -- cgit From cc6b27cbb179de3e5ac124bfddac63f66e16737a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:24:24 +0200 Subject: Address feedback --- test/exchange_wrapper_test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 842d6de32..0fa2f93e0 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -162,7 +162,7 @@ describe('ExchangeWrapper', () => { }); describe('should throw when not enough balance or allowance to fulfill the order', () => { const fillableAmount = new BigNumber(5); - const lackingBalance = new BigNumber(3); + const balanceToSubtractFromMaker = new BigNumber(3); const lackingAllowance = new BigNumber(3); let signedOrder: SignedOrder; beforeEach('create fillable signed order', async () => { @@ -171,7 +171,7 @@ describe('ExchangeWrapper', () => { ); }); it('should throw when taker balance is less than fill amount', async () => { - await zeroEx.token.transferAsync(takerTokenAddress, takerAddress, coinbase, lackingBalance); + await zeroEx.token.transferAsync(takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, @@ -187,7 +187,7 @@ describe('ExchangeWrapper', () => { )).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, lackingBalance); + await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, @@ -229,8 +229,8 @@ describe('ExchangeWrapper', () => { zeroEx.setTransactionSenderAccount(takerAddress); }); it('should throw when maker doesn\'t have enough balance to pay fees', async () => { - const lackingBalance = new BigNumber(1); - await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, lackingBalance); + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync(zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); @@ -244,8 +244,8 @@ describe('ExchangeWrapper', () => { )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); }); it('should throw when taker doesn\'t have enough balance to pay fees', async () => { - const lackingBalance = new BigNumber(1); - await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, lackingBalance); + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync(zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, fillTakerAmount, shouldCheckTransfer, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); @@ -261,7 +261,7 @@ describe('ExchangeWrapper', () => { }); }); describe('successful fills', () => { - it('should fill the valid order', async () => { + it('should fill a valid order', async () => { const fillableAmount = new BigNumber(5); const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, -- cgit