From 4159e45aff14c04f2d799ec8a7b6e7f1a4894064 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 23 Aug 2018 12:54:13 -0700 Subject: Update tests --- packages/contracts/test/exchange/internal.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 67d1d2d2c..0231cc3f1 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -63,6 +63,7 @@ describe('Exchange core internal functions', () => { let testExchange: TestExchangeInternalsContract; let invalidOpcodeErrorForCall: Error | undefined; let overflowErrorForSendTransaction: Error | undefined; + let divisionByZeroErrorForCall: Error | undefined; before(async () => { await blockchainLifecycle.startAsync(); @@ -79,6 +80,9 @@ describe('Exchange core internal functions', () => { overflowErrorForSendTransaction = new Error( await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); + divisionByZeroErrorForCall = new Error( + await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.DivisionByZero), + ); invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); }); // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset @@ -215,26 +219,26 @@ describe('Exchange core internal functions', () => { denominator: BigNumber, target: BigNumber, ): Promise { - const product = numerator.mul(target); if (denominator.eq(0)) { - throw invalidOpcodeErrorForCall; + throw divisionByZeroErrorForCall; } - const remainder = product.mod(denominator); - if (remainder.eq(0)) { + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { return false; } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const remainderTimes1000 = remainder.mul('1000'); + const isError = remainderTimes1000.gt(product); if (product.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - if (product.eq(0)) { - throw invalidOpcodeErrorForCall; - } - const remainderTimes1000000 = remainder.mul('1000000'); - if (remainderTimes1000000.greaterThan(MAX_UINT256)) { + if (remainderTimes1000.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - const errPercentageTimes1000000 = remainderTimes1000000.dividedToIntegerBy(product); - return errPercentageTimes1000000.greaterThan('1000'); + return isError; } async function testIsRoundingErrorAsync( numerator: BigNumber, -- cgit v1.2.3 From 3dad6ee55e9f51daa66cfe3c83dd17abc31f23f5 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 22 Aug 2018 12:23:32 -0700 Subject: Add tests for getPartialAmountCeil --- packages/contracts/test/exchange/internal.ts | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 0231cc3f1..69625ee1d 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -1,6 +1,7 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { Order, RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import * as chai from 'chai'; import * as _ from 'lodash'; import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals'; @@ -16,6 +17,8 @@ import { FillResults } from '../utils/types'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); +const expect = chai.expect; + const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); @@ -213,6 +216,43 @@ describe('Exchange core internal functions', () => { ); }); + describe.only('getPartialAmountCeil', async () => { + async function referenceGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ) { + if (denominator.eq(0)) { + throw new Error('revert DIVISION_BY_ZERO'); + } + const product = numerator.mul(target); + const offset = product.add(denominator.sub(1)); + if (offset.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + const result = offset.dividedToIntegerBy(denominator); + if (product.mod(denominator).eq(0)) { + expect(result.mul(denominator)).to.be.bignumber.eq(product); + } else { + expect(result.mul(denominator)).to.be.bignumber.gt(product); + } + return result; + } + async function testGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + return testExchange.publicGetPartialAmountCeil.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'getPartialAmountCeil', + referenceGetPartialAmountCeilAsync, + testGetPartialAmountCeilAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + describe('isRoundingError', async () => { async function referenceIsRoundingErrorAsync( numerator: BigNumber, -- cgit v1.2.3 From c109d1f5451c43afd92dd0fb4bebb48cba65c661 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 23 Aug 2018 13:33:05 -0700 Subject: Remove .only --- packages/contracts/test/exchange/internal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 69625ee1d..2f25b1708 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -216,7 +216,7 @@ describe('Exchange core internal functions', () => { ); }); - describe.only('getPartialAmountCeil', async () => { + describe('getPartialAmountCeil', async () => { async function referenceGetPartialAmountCeilAsync( numerator: BigNumber, denominator: BigNumber, -- cgit v1.2.3 From 4219af1430f1cfc105d3521616941b7947fde4e3 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 23 Aug 2018 14:22:59 -0700 Subject: Add DIVISION_BY_ZERO to getPartialAmount for consistency --- packages/contracts/test/exchange/internal.ts | 31 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 2f25b1708..48e1e620c 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -46,22 +46,6 @@ const emptySignedOrder: SignedOrder = { const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); -async function referenceGetPartialAmountAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): Promise { - const invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); - const product = numerator.mul(target); - if (product.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - if (denominator.eq(0)) { - throw invalidOpcodeErrorForCall; - } - return product.dividedToIntegerBy(denominator); -} - describe('Exchange core internal functions', () => { let testExchange: TestExchangeInternalsContract; let invalidOpcodeErrorForCall: Error | undefined; @@ -91,6 +75,21 @@ describe('Exchange core internal functions', () => { // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset // the blockchain state for any tests which modify it! + async function referenceGetPartialAmountAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + const product = numerator.mul(target); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return product.dividedToIntegerBy(denominator); + } + describe('addFillResults', async () => { function makeFillResults(value: BigNumber): FillResults { return { -- cgit v1.2.3 From 7f78d7da9dd13d1f0068a292bcd1ee3c5439d5a8 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 23 Aug 2018 15:19:29 -0700 Subject: Add tests --- packages/contracts/test/exchange/internal.ts | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 48e1e620c..0c6ab3707 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -294,6 +294,49 @@ describe('Exchange core internal functions', () => { ); }); + describe('isRoundingErrorCeil', async () => { + async function referenceIsRoundingErrorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { + return false; + } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const error = denominator.sub(remainder).mod(denominator); + const errorTimes1000 = error.mul('1000'); + const isError = errorTimes1000.gt(product); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (errorTimes1000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return isError; + } + async function testIsRoundingErrorCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + return testExchange.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'isRoundingErrorCeil', + referenceIsRoundingErrorAsync, + testIsRoundingErrorCeilAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + describe('updateFilledState', async () => { // Note(albrow): Since updateFilledState modifies the state by calling // sendTransaction, we must reset the state after each test. -- cgit v1.2.3 From 80291caf7cef16f0b300484755960d92d6750a6a Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Fri, 24 Aug 2018 16:16:44 -0700 Subject: Append -Floor to getPartialAmount and isRoundingError --- packages/contracts/test/exchange/internal.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 0c6ab3707..14c0ff4cb 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -75,7 +75,7 @@ describe('Exchange core internal functions', () => { // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset // the blockchain state for any tests which modify it! - async function referenceGetPartialAmountAsync( + async function referenceGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, @@ -165,18 +165,18 @@ describe('Exchange core internal functions', () => { // implementation or the Solidity implementation of // calculateFillResults. return { - makerAssetFilledAmount: await referenceGetPartialAmountAsync( + makerAssetFilledAmount: await referenceGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, ), takerAssetFilledAmount, - makerFeePaid: await referenceGetPartialAmountAsync( + makerFeePaid: await referenceGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, ), - takerFeePaid: await referenceGetPartialAmountAsync( + takerFeePaid: await referenceGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, @@ -199,18 +199,18 @@ describe('Exchange core internal functions', () => { ); }); - describe('getPartialAmount', async () => { - async function testGetPartialAmountAsync( + describe('getPartialAmountFloor', async () => { + async function testGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise { - return testExchange.publicGetPartialAmount.callAsync(numerator, denominator, target); + return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( 'getPartialAmount', - referenceGetPartialAmountAsync, - testGetPartialAmountAsync, + referenceGetPartialAmountFloorAsync, + testGetPartialAmountFloorAsync, [uint256Values, uint256Values, uint256Values], ); }); @@ -284,7 +284,7 @@ describe('Exchange core internal functions', () => { denominator: BigNumber, target: BigNumber, ): Promise { - return testExchange.publicIsRoundingError.callAsync(numerator, denominator, target); + return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( 'isRoundingError', -- cgit v1.2.3 From cc1fac9bbee2656bdb327490de42922abfc5125a Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 17:02:42 -0700 Subject: Fix linting errors --- packages/contracts/test/exchange/internal.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 14c0ff4cb..2521665c2 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -220,9 +220,9 @@ describe('Exchange core internal functions', () => { numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ) { + ): Promise { if (denominator.eq(0)) { - throw new Error('revert DIVISION_BY_ZERO'); + throw divisionByZeroErrorForCall; } const product = numerator.mul(target); const offset = product.add(denominator.sub(1)); -- cgit v1.2.3 From ff4f86f1d67c29656976d5230ce9aaff0d3166ca Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 27 Aug 2018 11:19:25 -0700 Subject: fix(contracts): Use correct error message for division by zero --- packages/contracts/test/exchange/internal.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 2521665c2..de381fca3 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -67,9 +67,7 @@ describe('Exchange core internal functions', () => { overflowErrorForSendTransaction = new Error( await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); - divisionByZeroErrorForCall = new Error( - await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.DivisionByZero), - ); + divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); }); // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset -- cgit v1.2.3 From 14fdb71a716cb95bc1f6933db9057d23f3c41909 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 28 Aug 2018 13:00:49 -0700 Subject: safeGetPartialAmount (#1035) * Added Test "Should transfer correct amounts when left order is fully filled and values pass isRoundingErrorCeil but fail isRoundingErrorFloor" * Added RoundingError exception to reference function for getPartialAmount * Added RoundingError exception to reference function for getPartialAmount * Added isRoundingErrorCeil to getPartialAmountCeil reference funtion * Computed new values for "Should give right maker a better buy price when correct price is not integral" that does not have a rounding error * Almost all tests for match orders are passing after adding isRoundingErrorCeil check * WIP commit: Added rounding error checks to getPartialAmount * WIP commit: Added rounding error checks to getPartialAmount * Use safe versions of getPartialAmount * Update Exchange internals tests * Run linter * Found new values for "Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`" * Fixed merge conflicts * Run all tests * Cleaned up some comments on match Orders tests * Fix tests for geth --- packages/contracts/test/exchange/internal.ts | 181 +++++++++++++++++++-------- 1 file changed, 129 insertions(+), 52 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index de381fca3..c5d2fc58f 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -48,9 +48,9 @@ const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); describe('Exchange core internal functions', () => { let testExchange: TestExchangeInternalsContract; - let invalidOpcodeErrorForCall: Error | undefined; let overflowErrorForSendTransaction: Error | undefined; let divisionByZeroErrorForCall: Error | undefined; + let roundingErrorForCall: Error | undefined; before(async () => { await blockchainLifecycle.startAsync(); @@ -68,12 +68,67 @@ describe('Exchange core internal functions', () => { await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); - invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); + roundingErrorForCall = new Error(RevertReason.RoundingError); }); // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset // the blockchain state for any tests which modify it! - async function referenceGetPartialAmountFloorAsync( + async function referenceIsRoundingErrorFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { + return false; + } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const remainderTimes1000 = remainder.mul('1000'); + const isError = remainderTimes1000.gte(product); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (remainderTimes1000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return isError; + } + + async function referenceIsRoundingErrorCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + if (numerator.eq(0)) { + return false; + } + if (target.eq(0)) { + return false; + } + const product = numerator.mul(target); + const remainder = product.mod(denominator); + const error = denominator.sub(remainder).mod(denominator); + const errorTimes1000 = error.mul('1000'); + const isError = errorTimes1000.gte(product); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (errorTimes1000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return isError; + } + + async function referenceSafeGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, @@ -81,6 +136,10 @@ describe('Exchange core internal functions', () => { if (denominator.eq(0)) { throw divisionByZeroErrorForCall; } + const isRoundingError = await referenceIsRoundingErrorFloorAsync(numerator, denominator, target); + if (isRoundingError) { + throw roundingErrorForCall; + } const product = numerator.mul(target); if (product.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; @@ -163,18 +222,18 @@ describe('Exchange core internal functions', () => { // implementation or the Solidity implementation of // calculateFillResults. return { - makerAssetFilledAmount: await referenceGetPartialAmountFloorAsync( + makerAssetFilledAmount: await referenceSafeGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, ), takerAssetFilledAmount, - makerFeePaid: await referenceGetPartialAmountFloorAsync( + makerFeePaid: await referenceSafeGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, ), - takerFeePaid: await referenceGetPartialAmountFloorAsync( + takerFeePaid: await referenceSafeGetPartialAmountFloorAsync( takerAssetFilledAmount, orderTakerAssetAmount, otherAmount, @@ -198,6 +257,20 @@ describe('Exchange core internal functions', () => { }); describe('getPartialAmountFloor', async () => { + async function referenceGetPartialAmountFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + const product = numerator.mul(target); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return product.dividedToIntegerBy(denominator); + } async function testGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, @@ -206,7 +279,7 @@ describe('Exchange core internal functions', () => { return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'getPartialAmount', + 'getPartialAmountFloor', referenceGetPartialAmountFloorAsync, testGetPartialAmountFloorAsync, [uint256Values, uint256Values, uint256Values], @@ -250,76 +323,80 @@ describe('Exchange core internal functions', () => { ); }); - describe('isRoundingError', async () => { - async function referenceIsRoundingErrorAsync( + describe('safeGetPartialAmountFloor', async () => { + async function testSafeGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ): Promise { + ): Promise { + return testExchange.publicSafeGetPartialAmountFloor.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'safeGetPartialAmountFloor', + referenceSafeGetPartialAmountFloorAsync, + testSafeGetPartialAmountFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('safeGetPartialAmountCeil', async () => { + async function referenceSafeGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise { if (denominator.eq(0)) { throw divisionByZeroErrorForCall; } - if (numerator.eq(0)) { - return false; - } - if (target.eq(0)) { - return false; + const isRoundingError = await referenceIsRoundingErrorCeilAsync(numerator, denominator, target); + if (isRoundingError) { + throw roundingErrorForCall; } const product = numerator.mul(target); - const remainder = product.mod(denominator); - const remainderTimes1000 = remainder.mul('1000'); - const isError = remainderTimes1000.gt(product); - if (product.greaterThan(MAX_UINT256)) { + const offset = product.add(denominator.sub(1)); + if (offset.greaterThan(MAX_UINT256)) { throw overflowErrorForCall; } - if (remainderTimes1000.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; + const result = offset.dividedToIntegerBy(denominator); + if (product.mod(denominator).eq(0)) { + expect(result.mul(denominator)).to.be.bignumber.eq(product); + } else { + expect(result.mul(denominator)).to.be.bignumber.gt(product); } - return isError; + return result; } - async function testIsRoundingErrorAsync( + async function testSafeGetPartialAmountCeilAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, - ): Promise { - return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + ): Promise { + return testExchange.publicSafeGetPartialAmountCeil.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( - 'isRoundingError', - referenceIsRoundingErrorAsync, - testIsRoundingErrorAsync, + 'safeGetPartialAmountCeil', + referenceSafeGetPartialAmountCeilAsync, + testSafeGetPartialAmountCeilAsync, [uint256Values, uint256Values, uint256Values], ); }); - describe('isRoundingErrorCeil', async () => { - async function referenceIsRoundingErrorAsync( + describe('isRoundingErrorFloor', async () => { + async function testIsRoundingErrorFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - if (numerator.eq(0)) { - return false; - } - if (target.eq(0)) { - return false; - } - const product = numerator.mul(target); - const remainder = product.mod(denominator); - const error = denominator.sub(remainder).mod(denominator); - const errorTimes1000 = error.mul('1000'); - const isError = errorTimes1000.gt(product); - if (product.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - if (errorTimes1000.greaterThan(MAX_UINT256)) { - throw overflowErrorForCall; - } - return isError; + return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); } + await testCombinatoriallyWithReferenceFuncAsync( + 'isRoundingErrorFloor', + referenceIsRoundingErrorFloorAsync, + testIsRoundingErrorFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('isRoundingErrorCeil', async () => { async function testIsRoundingErrorCeilAsync( numerator: BigNumber, denominator: BigNumber, @@ -329,7 +406,7 @@ describe('Exchange core internal functions', () => { } await testCombinatoriallyWithReferenceFuncAsync( 'isRoundingErrorCeil', - referenceIsRoundingErrorAsync, + referenceIsRoundingErrorCeilAsync, testIsRoundingErrorCeilAsync, [uint256Values, uint256Values, uint256Values], ); -- cgit v1.2.3 From f225f9e7c8f59a0ea04f6c9b07493a0458c4f502 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 28 Aug 2018 13:25:05 -0700 Subject: Making rounding consistent in calculateFillResults --- packages/contracts/test/exchange/internal.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'packages/contracts/test/exchange/internal.ts') diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index c5d2fc58f..dc2c5fbe0 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -221,16 +221,19 @@ describe('Exchange core internal functions', () => { // in any mathematical operation in either the reference TypeScript // implementation or the Solidity implementation of // calculateFillResults. + const makerAssetFilledAmount = await referenceSafeGetPartialAmountFloorAsync( + takerAssetFilledAmount, + orderTakerAssetAmount, + otherAmount, + ); + const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount); + const orderMakerAssetAmount = order.makerAssetAmount; return { - makerAssetFilledAmount: await referenceSafeGetPartialAmountFloorAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, - otherAmount, - ), + makerAssetFilledAmount, takerAssetFilledAmount, makerFeePaid: await referenceSafeGetPartialAmountFloorAsync( - takerAssetFilledAmount, - orderTakerAssetAmount, + makerAssetFilledAmount, + orderMakerAssetAmount, otherAmount, ), takerFeePaid: await referenceSafeGetPartialAmountFloorAsync( -- cgit v1.2.3