diff options
Diffstat (limited to 'packages')
18 files changed, 392 insertions, 115 deletions
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol index 218713d3c..a7ff400b9 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol @@ -163,7 +163,7 @@ contract MixinExchangeWrapper is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -231,7 +231,7 @@ contract MixinExchangeWrapper is // Convert the remaining amount of ZRX to buy into remaining amount // of WETH to sell, assuming entire amount can be sold in the current order. - uint256 remainingWethSellAmount = getPartialAmount( + uint256 remainingWethSellAmount = getPartialAmountFloor( orders[i].takerAssetAmount, safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees remainingZrxBuyAmount diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol index 42cec4d36..14f191879 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol @@ -87,7 +87,7 @@ contract MixinForwarderCore is uint256 makerAssetAmountPurchased; if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // Calculate amount of WETH that won't be spent on ETH fees. - wethSellAmount = getPartialAmount( + wethSellAmount = getPartialAmountFloor( PERCENTAGE_DENOMINATOR, safeAdd(PERCENTAGE_DENOMINATOR, feePercentage), msg.value @@ -103,7 +103,7 @@ contract MixinForwarderCore is makerAssetAmountPurchased = safeSub(orderFillResults.makerAssetFilledAmount, orderFillResults.takerFeePaid); } else { // 5% of WETH is reserved for filling feeOrders and paying feeRecipient. - wethSellAmount = getPartialAmount( + wethSellAmount = getPartialAmountFloor( MAX_WETH_FILL_PERCENTAGE, PERCENTAGE_DENOMINATOR, msg.value diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol index 93e85e599..5863b522d 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol @@ -82,7 +82,7 @@ contract MixinWeth is uint256 wethRemaining = safeSub(msg.value, wethSold); // Calculate ETH fee to pay to feeRecipient. - uint256 ethFee = getPartialAmount( + uint256 ethFee = getPartialAmountFloor( feePercentage, PERCENTAGE_DENOMINATOR, wethSoldExcludingFeeOrders diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index ab5c6e507..f3a0591b2 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -320,7 +320,7 @@ contract MixinExchangeCore is // Validate fill order rounding require( - !isRoundingError( + !isRoundingErrorFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerAssetAmount @@ -376,17 +376,17 @@ contract MixinExchangeCore is { // Compute proportional transfer amounts fillResults.takerAssetFilledAmount = takerAssetFilledAmount; - fillResults.makerAssetFilledAmount = getPartialAmount( + fillResults.makerAssetFilledAmount = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerAssetAmount ); - fillResults.makerFeePaid = getPartialAmount( + fillResults.makerFeePaid = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerFee ); - fillResults.takerFeePaid = getPartialAmount( + fillResults.takerFeePaid = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.takerFee diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 56b309a1b..20f4b1c33 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -183,7 +183,7 @@ contract MixinMatchOrders is leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; // The right order receives an amount proportional to how much was spent. - rightTakerAssetFilledAmount = getPartialAmount( + rightTakerAssetFilledAmount = getPartialAmountFloor( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, leftTakerAssetFilledAmount @@ -193,7 +193,7 @@ contract MixinMatchOrders is rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; // The left order receives an amount proportional to how much was spent. - leftTakerAssetFilledAmount = getPartialAmount( + leftTakerAssetFilledAmount = getPartialAmountFloor( rightOrder.makerAssetAmount, rightOrder.takerAssetAmount, rightTakerAssetFilledAmount diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index 86194f461..1b77cfbcb 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -298,7 +298,7 @@ contract MixinWrapperFunctions is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -350,7 +350,7 @@ contract MixinWrapperFunctions is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol index fa09da6ac..0e0fba5d2 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol @@ -25,12 +25,12 @@ contract LibMath is SafeMath { - /// @dev Calculates partial value given a numerator and denominator. + /// @dev Calculates partial value given a numerator and denominator rounded down. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to calculate partial of. - /// @return Partial value of target. - function getPartialAmount( + /// @return Partial value of target rounded down. + function getPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -39,19 +39,56 @@ contract LibMath is pure returns (uint256 partialAmount) { + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + partialAmount = safeDiv( safeMul(numerator, target), denominator ); return partialAmount; } - - /// @dev Checks if rounding error > 0.1%. + + /// @dev Calculates partial value given a numerator and denominator rounded down. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target rounded up. + function getPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + internal + pure + returns (uint256 partialAmount) + { + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + + // safeDiv computes `floor(a / b)`. We use the identity (a, b integer): + // ceil(a / b) = floor((a + b - 1) / b) + // To implement `ceil(a / b)` using safeDiv. + partialAmount = safeDiv( + safeAdd( + safeMul(numerator, target), + safeSub(denominator, 1) + ), + denominator + ); + return partialAmount; + } + + /// @dev Checks if rounding error >= 0.1% when rounding down. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to multiply with numerator/denominator. /// @return Rounding error is present. - function isRoundingError( + function isRoundingErrorFloor( uint256 numerator, uint256 denominator, uint256 target @@ -60,16 +97,73 @@ contract LibMath is pure returns (bool isError) { - uint256 remainder = mulmod(target, numerator, denominator); - if (remainder == 0) { - return false; // No rounding error. + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + + // The absolute rounding error is the difference between the rounded + // value and the ideal value. The relative rounding error is the + // absolute rounding error divided by the absolute value of the + // ideal value. This is undefined when the ideal value is zero. + // + // The ideal value is `numerator * target / denominator`. + // Let's call `numerator * target % denominator` the remainder. + // The absolute error is `remainder / denominator`. + // + // When the ideal value is zero, we require the absolute error to + // be zero. Fortunately, this is always the case. The ideal value is + // zero iff `numerator == 0` and/or `target == 0`. In this case the + // remainder and absolute error are also zero. + if (target == 0 || numerator == 0) { + return false; } - - uint256 errPercentageTimes1000000 = safeDiv( - safeMul(remainder, 1000000), - safeMul(numerator, target) + + // Otherwise, we want the relative rounding error to be strictly + // less than 0.1%. + // The relative error is `remainder / (numerator * target)`. + // We want the relative error less than 1 / 1000: + // remainder / (numerator * denominator) < 1 / 1000 + // or equivalently: + // 1000 * remainder < numerator * target + // so we have a rounding error iff: + // 1000 * remainder >= numerator * target + uint256 remainder = mulmod(target, numerator, denominator); + isError = safeMul(1000, remainder) >= safeMul(numerator, target); + return isError; + } + + /// @dev Checks if rounding error >= 0.1% when rounding up. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function isRoundingErrorCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + internal + pure + returns (bool isError) + { + require( + denominator > 0, + "DIVISION_BY_ZERO" ); - isError = errPercentageTimes1000000 > 1000; + + // See the comments in `isRoundingError`. + if (target == 0 || numerator == 0) { + // When either is zero, the ideal value and rounded value are zero + // and there is no rounding error. (Although the relative error + // is undefined.) + return false; + } + // Compute remainder as before + uint256 remainder = mulmod(target, numerator, denominator); + // TODO: safeMod + remainder = safeSub(denominator, remainder) % denominator; + isError = safeMul(1000, remainder) >= safeMul(numerator, target); return isError; } } diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol index d9cec9edc..da9313e02 100644 --- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol +++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol @@ -67,7 +67,7 @@ contract TestExchangeInternals is /// @param denominator Denominator. /// @param target Value to calculate partial of. /// @return Partial value of target. - function publicGetPartialAmount( + function publicGetPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -76,15 +76,49 @@ contract TestExchangeInternals is pure returns (uint256 partialAmount) { - return getPartialAmount(numerator, denominator, target); + return getPartialAmountFloor(numerator, denominator, target); } - /// @dev Checks if rounding error > 0.1%. + /// @dev Calculates partial value given a numerator and denominator. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target. + function publicGetPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + return getPartialAmountCeil(numerator, denominator, target); + } + + /// @dev Checks if rounding error >= 0.1%. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function publicIsRoundingErrorFloor( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (bool isError) + { + return isRoundingErrorFloor(numerator, denominator, target); + } + + /// @dev Checks if rounding error >= 0.1%. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to multiply with numerator/denominator. /// @return Rounding error is present. - function publicIsRoundingError( + function publicIsRoundingErrorCeil( uint256 numerator, uint256 denominator, uint256 target @@ -93,7 +127,7 @@ contract TestExchangeInternals is pure returns (bool isError) { - return isRoundingError(numerator, denominator, target); + return isRoundingErrorCeil(numerator, denominator, target); } /// @dev Updates state with results of a fill order. diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol index 4a99dd9c1..c8c58545f 100644 --- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol +++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol @@ -49,7 +49,7 @@ contract TestLibs is return fillOrderCalldata; } - function publicGetPartialAmount( + function publicGetPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -58,7 +58,7 @@ contract TestLibs is pure returns (uint256 partialAmount) { - partialAmount = getPartialAmount( + partialAmount = getPartialAmountFloor( numerator, denominator, target @@ -66,7 +66,41 @@ contract TestLibs is return partialAmount; } - function publicIsRoundingError( + function publicGetPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + partialAmount = getPartialAmountCeil( + numerator, + denominator, + target + ); + return partialAmount; + } + + function publicIsRoundingErrorFloor( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (bool isError) + { + isError = isRoundingErrorFloor( + numerator, + denominator, + target + ); + return isError; + } + + function publicIsRoundingErrorCeil( uint256 numerator, uint256 denominator, uint256 target @@ -75,7 +109,7 @@ contract TestLibs is pure returns (bool isError) { - isError = isRoundingError( + isError = isRoundingErrorCeil( numerator, denominator, target diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 67d1d2d2c..2521665c2 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); @@ -43,26 +46,11 @@ const emptySignedOrder: SignedOrder = { const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); -async function referenceGetPartialAmountAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): Promise<BigNumber> { - 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; let overflowErrorForSendTransaction: Error | undefined; + let divisionByZeroErrorForCall: Error | undefined; before(async () => { await blockchainLifecycle.startAsync(); @@ -79,11 +67,29 @@ 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 // the blockchain state for any tests which modify it! + async function referenceGetPartialAmountFloorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + 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 { @@ -159,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, @@ -193,18 +199,55 @@ describe('Exchange core internal functions', () => { ); }); - describe('getPartialAmount', async () => { - async function testGetPartialAmountAsync( + describe('getPartialAmountFloor', async () => { + async function testGetPartialAmountFloorAsync( numerator: BigNumber, denominator: BigNumber, target: BigNumber, ): Promise<BigNumber> { - return testExchange.publicGetPartialAmount.callAsync(numerator, denominator, target); + return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( 'getPartialAmount', - referenceGetPartialAmountAsync, - testGetPartialAmountAsync, + referenceGetPartialAmountFloorAsync, + testGetPartialAmountFloorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('getPartialAmountCeil', async () => { + async function referenceGetPartialAmountCeilAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + if (denominator.eq(0)) { + throw divisionByZeroErrorForCall; + } + 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<BigNumber> { + return testExchange.publicGetPartialAmountCeil.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'getPartialAmountCeil', + referenceGetPartialAmountCeilAsync, + testGetPartialAmountCeilAsync, [uint256Values, uint256Values, uint256Values], ); }); @@ -215,33 +258,33 @@ describe('Exchange core internal functions', () => { denominator: BigNumber, target: BigNumber, ): Promise<boolean> { - 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, denominator: BigNumber, target: BigNumber, ): Promise<boolean> { - return testExchange.publicIsRoundingError.callAsync(numerator, denominator, target); + return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); } await testCombinatoriallyWithReferenceFuncAsync( 'isRoundingError', @@ -251,6 +294,49 @@ describe('Exchange core internal functions', () => { ); }); + describe('isRoundingErrorCeil', async () => { + async function referenceIsRoundingErrorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + 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<boolean> { + 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. diff --git a/packages/contracts/test/exchange/libs.ts b/packages/contracts/test/exchange/libs.ts index 6c3305d1d..37234489e 100644 --- a/packages/contracts/test/exchange/libs.ts +++ b/packages/contracts/test/exchange/libs.ts @@ -71,29 +71,57 @@ describe('Exchange libs', () => { // combinatorial tests in test/exchange/internal. They test specific edge // cases that are not covered by the combinatorial tests. describe('LibMath', () => { - it('should return false if there is a rounding error of 0.1%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(999); - const target = new BigNumber(50); - // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - it('should return false if there is a rounding of 0.09%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9991); - const target = new BigNumber(500); - // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); + describe('isRoundingError', () => { + it('should return true if there is a rounding error of 0.1%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(999); + const target = new BigNumber(50); + // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); + it('should return false if there is a rounding of 0.09%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9991); + const target = new BigNumber(500); + // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return true if there is a rounding error of 0.11%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9989); + const target = new BigNumber(500); + // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% + const isRoundingError = await libs.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); }); - it('should return true if there is a rounding error of 0.11%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9989); - const target = new BigNumber(500); - // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.true(); + describe('isRoundingErrorCeil', () => { + it('should return true if there is a rounding error of 0.1%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(1001); + const target = new BigNumber(50); + // rounding error = (ceil(20*50/1001) - (20*50/1001)) / (20*50/1001) = 0.1% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); + it('should return false if there is a rounding of 0.09%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(10009); + const target = new BigNumber(500); + // rounding error = (ceil(20*500/10009) - (20*500/10009)) / (20*500/10009) = 0.09% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return true if there is a rounding error of 0.11%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(10011); + const target = new BigNumber(500); + // rounding error = (ceil(20*500/10011) - (20*500/10011)) / (20*500/10011) = 0.11% + const isRoundingError = await libs.publicIsRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); }); }); diff --git a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts index a9318571c..92d0f4003 100644 --- a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts +++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts @@ -467,17 +467,17 @@ export class FillOrderCombinatorialUtils { ? remainingTakerAmountToFill : alreadyFilledTakerAmount.add(takerAssetFillAmount); - const expFilledMakerAmount = orderUtils.getPartialAmount( + const expFilledMakerAmount = orderUtils.getPartialAmountFloor( expFilledTakerAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, ); - const expMakerFeePaid = orderUtils.getPartialAmount( + const expMakerFeePaid = orderUtils.getPartialAmountFloor( expFilledTakerAmount, signedOrder.takerAssetAmount, signedOrder.makerFee, ); - const expTakerFeePaid = orderUtils.getPartialAmount( + const expTakerFeePaid = orderUtils.getPartialAmountFloor( expFilledTakerAmount, signedOrder.takerAssetAmount, signedOrder.takerFee, @@ -668,7 +668,7 @@ export class FillOrderCombinatorialUtils { signedOrder: SignedOrder, takerAssetFillAmount: BigNumber, ): Promise<void> { - const makerAssetFillAmount = orderUtils.getPartialAmount( + const makerAssetFillAmount = orderUtils.getPartialAmountFloor( takerAssetFillAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, @@ -705,7 +705,7 @@ export class FillOrderCombinatorialUtils { ); } - const makerFee = orderUtils.getPartialAmount( + const makerFee = orderUtils.getPartialAmountFloor( takerAssetFillAmount, signedOrder.takerAssetAmount, signedOrder.makerFee, @@ -829,7 +829,7 @@ export class FillOrderCombinatorialUtils { ); } - const takerFee = orderUtils.getPartialAmount( + const takerFee = orderUtils.getPartialAmountFloor( takerAssetFillAmount, signedOrder.takerAssetAmount, signedOrder.takerFee, diff --git a/packages/contracts/test/utils/order_utils.ts b/packages/contracts/test/utils/order_utils.ts index 019f6e74b..444e27c44 100644 --- a/packages/contracts/test/utils/order_utils.ts +++ b/packages/contracts/test/utils/order_utils.ts @@ -5,7 +5,7 @@ import { constants } from './constants'; import { CancelOrder, MatchOrder } from './types'; export const orderUtils = { - getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + getPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { const partialAmount = numerator .mul(target) .div(denominator) diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index a0e24acf0..8398776aa 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -81,7 +81,7 @@ export class OrderStateUtils { const remainingTakerAssetAmount = signedOrder.takerAssetAmount.minus( sidedOrderRelevantState.filledTakerAssetAmount, ); - const isRoundingError = OrderValidationUtils.isRoundingError( + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor( remainingTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, @@ -191,7 +191,7 @@ export class OrderStateUtils { ); const remainingFillableTakerAssetAmountGivenMakersStatus = signedOrder.makerAssetAmount.eq(0) ? new BigNumber(0) - : utils.getPartialAmount( + : utils.getPartialAmountFloor( orderRelevantMakerState.remainingFillableAssetAmount, signedOrder.makerAssetAmount, signedOrder.takerAssetAmount, diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index 972e6f6d6..8227fb07c 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -24,7 +24,7 @@ export class OrderValidationUtils { * @param denominator Denominator value. When used to check an order, pass in `order.takerAssetAmount` * @param target Target value. When used to check an order, pass in `order.makerAssetAmount` */ - public static isRoundingError(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { + public static isRoundingErrorFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { // Solidity's mulmod() in JS // Source: https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions if (denominator.eq(0)) { @@ -58,7 +58,7 @@ export class OrderValidationUtils { zrxAssetData: string, ): Promise<void> { try { - const fillMakerTokenAmount = utils.getPartialAmount( + const fillMakerTokenAmount = utils.getPartialAmountFloor( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, @@ -79,7 +79,7 @@ export class OrderValidationUtils { TradeSide.Taker, TransferType.Trade, ); - const makerFeeAmount = utils.getPartialAmount( + const makerFeeAmount = utils.getPartialAmountFloor( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.makerFee, @@ -92,7 +92,7 @@ export class OrderValidationUtils { TradeSide.Maker, TransferType.Fee, ); - const takerFeeAmount = utils.getPartialAmount( + const takerFeeAmount = utils.getPartialAmountFloor( fillTakerAssetAmount, signedOrder.takerAssetAmount, signedOrder.takerFee, @@ -218,7 +218,7 @@ export class OrderValidationUtils { zrxAssetData, ); - const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingError( + const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingErrorFloor( desiredFillTakerTokenAmount, signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, diff --git a/packages/order-utils/src/utils.ts b/packages/order-utils/src/utils.ts index 7aaaf0609..0ff05e8ed 100644 --- a/packages/order-utils/src/utils.ts +++ b/packages/order-utils/src/utils.ts @@ -12,7 +12,7 @@ export const utils = { const milisecondsInSecond = 1000; return new BigNumber(Date.now() / milisecondsInSecond).round(); }, - getPartialAmount(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + getPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { const fillMakerTokenAmount = numerator .mul(target) .div(denominator) diff --git a/packages/order-utils/test/order_validation_utils_test.ts b/packages/order-utils/test/order_validation_utils_test.ts index d3ff867d7..d3133c0a6 100644 --- a/packages/order-utils/test/order_validation_utils_test.ts +++ b/packages/order-utils/test/order_validation_utils_test.ts @@ -16,7 +16,7 @@ describe('OrderValidationUtils', () => { const denominator = new BigNumber(999); const target = new BigNumber(50); // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.false(); }); @@ -25,7 +25,7 @@ describe('OrderValidationUtils', () => { const denominator = new BigNumber(9991); const target = new BigNumber(500); // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.false(); }); @@ -34,7 +34,7 @@ describe('OrderValidationUtils', () => { const denominator = new BigNumber(9989); const target = new BigNumber(500); // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.true(); }); @@ -43,7 +43,7 @@ describe('OrderValidationUtils', () => { const denominator = new BigNumber(7); const target = new BigNumber(10); // rounding error = ((3*10/7) - floor(3*10/7)) / (3*10/7) = 6.67% - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.true(); }); @@ -52,7 +52,7 @@ describe('OrderValidationUtils', () => { const denominator = new BigNumber(2); const target = new BigNumber(10); - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.false(); }); @@ -63,7 +63,7 @@ describe('OrderValidationUtils', () => { const target = new BigNumber(105762562); // rounding error = ((76564*105762562/676373677) - floor(76564*105762562/676373677)) / // (76564*105762562/676373677) = 0.0007% - const isRoundingError = OrderValidationUtils.isRoundingError(numerator, denominator, target); + const isRoundingError = OrderValidationUtils.isRoundingErrorFloor(numerator, denominator, target); expect(isRoundingError).to.be.false(); }); }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 4375fc631..f07d7f756 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -172,6 +172,7 @@ export enum RevertReason { InvalidSender = 'INVALID_SENDER', InvalidOrderSignature = 'INVALID_ORDER_SIGNATURE', InvalidTakerAmount = 'INVALID_TAKER_AMOUNT', + DivisionByZero = 'DIVISION_BY_ZERO', RoundingError = 'ROUNDING_ERROR', InvalidSignature = 'INVALID_SIGNATURE', SignatureIllegal = 'SIGNATURE_ILLEGAL', |