From e1ee6b84945e729d894f6535be02f3541e43dbf0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:57:21 +0200 Subject: Add check for ROUNDING_ERROR and test for it --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++++++++++ src/types.ts | 5 +++++ test/exchange_wrapper_test.ts | 31 ++++++++++++++++++++++--------- test/utils/fill_scenarios.ts | 27 ++++++++++++++++++--------- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 3fb187de2..fe5fc3d78 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -154,6 +154,10 @@ export class ExchangeWrapper extends ContractWrapper { if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); } + if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits, + signedOrder.makerTokenAmount)) { + throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); @@ -163,6 +167,18 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(errMessage); } } + private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, + fillTakerAmountInBaseUnits: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber): Promise { + const exchangeInstance = await this.getExchangeContractAsync(); + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + const isRoundingError = await exchangeInstance.isRoundingError.call( + takerTokenAmount, fillTakerAmountInBaseUnits, makerTokenAmount, { + from: senderAddress, + }, + ); + return isRoundingError; + } private async getExchangeContractAsync(): Promise { if (!_.isUndefined(this.exchangeContractIfExists)) { return this.exchangeContractIfExists; diff --git a/src/types.ts b/src/types.ts index f80f98dc4..73c448b85 100644 --- a/src/types.ts +++ b/src/types.ts @@ -34,6 +34,10 @@ export type OrderValues = [BigNumber.BigNumber, BigNumber.BigNumber, BigNumber.B export interface ExchangeContract { isValidSignature: any; + isRoundingError: { + call: (takerTokenAmount: BigNumber.BigNumber, fillTakerAmountInBaseUnits: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber, txOpts: TxOpts) => Promise; + }; fill: { (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts: TxOpts): ContractResponse; @@ -93,6 +97,7 @@ export const FillOrderValidationErrs = strEnum([ 'NOT_ENOUGH_TAKER_ALLOWANCE', 'NOT_ENOUGH_MAKER_BALANCE', 'NOT_ENOUGH_MAKER_ALLOWANCE', + 'ROUNDING_ERROR', ]); export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs; diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 66ba167cc..7f8bad377 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -127,7 +127,7 @@ describe('ExchangeWrapper', () => { describe('failed fills', () => { it('should throw when the fill amount is zero', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const zeroFillAmount = new BigNumber(0); @@ -138,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( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); return expect(zeroEx.exchange.fillOrderAsync( @@ -148,7 +148,7 @@ describe('ExchangeWrapper', () => { it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(42); const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); zeroEx.setTransactionSenderAccount(takerAddress); @@ -158,7 +158,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( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); zeroEx.setTransactionSenderAccount(takerAddress); @@ -169,7 +169,7 @@ describe('ExchangeWrapper', () => { }); it('should throw when taker allowance is less than fill amount', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1); @@ -182,7 +182,7 @@ describe('ExchangeWrapper', () => { }); it('should throw when maker balance is less than maker fill amount', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const lackingMakerBalance = new BigNumber(3); @@ -194,7 +194,7 @@ describe('ExchangeWrapper', () => { }); it('should throw when maker allowance is less than maker fill amount', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1); @@ -205,11 +205,24 @@ describe('ExchangeWrapper', () => { signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); }); + it('should throw when there would be a rounding error', async () => { + const makerFillableAmount = new BigNumber(3); + const takerFillableAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAsymetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + makerFillableAmount, takerFillableAmount, + ); + const fillTakerAmountInBaseUnitsThatCausesRoundingError = new BigNumber(3); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnitsThatCausesRoundingError, shouldCheckTransfer, + )).to.be.rejectedWith(FillOrderValidationErrs.ROUNDING_ERROR); + }); }); describe('successful fills', () => { it('should fill the valid order', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); @@ -234,7 +247,7 @@ describe('ExchangeWrapper', () => { }); it('should partially fill the valid order', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const partialFillAmount = new BigNumber(3); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 3b66937e6..568b11ef8 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -14,20 +14,29 @@ export class FillScenarios { this.tokens = tokens; this.coinBase = userAddresses[0]; } - public async createAFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, - makerAddress: string, takerAddress: string, - fillableAmount: BigNumber.BigNumber, - expirationUnixTimestampSec?: BigNumber.BigNumber): + public async createFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, + makerAddress: string, takerAddress: string, + fillableAmount: BigNumber.BigNumber, + expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { - await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, fillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, fillableAmount); - await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, fillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount); + return this.createAsymetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + fillableAmount, fillableAmount, expirationUnixTimestampSec, + ); + } + public async createAsymetricFillableSignedOrderAsync( + makerTokenAddress: string, takerTokenAddress: string, makerAddress: string, takerAddress: string, + makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber, + expirationUnixTimestampSec?: BigNumber.BigNumber): Promise { + await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, makerFillableAmount); + await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, makerFillableAmount); + await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, takerFillableAmount); + await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, takerFillableAmount); const transactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); this.zeroEx.setTransactionSenderAccount(makerAddress); const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, - takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress, + takerAddress, makerFillableAmount, makerTokenAddress, takerFillableAmount, takerTokenAddress, expirationUnixTimestampSec); this.zeroEx.setTransactionSenderAccount(transactionSenderAccount as string); return signedOrder; -- cgit v1.2.3