diff --git a/contracts/exchange-libs/CHANGELOG.json b/contracts/exchange-libs/CHANGELOG.json index f5f95a1b70..9e0b18542c 100644 --- a/contracts/exchange-libs/CHANGELOG.json +++ b/contracts/exchange-libs/CHANGELOG.json @@ -1,29 +1,7 @@ [ { - "timestamp": 1563193019, - "version": "3.0.2", - "changes": [ - { - "note": "Dependencies updated" - } - ] - }, - { - "timestamp": 1563047529, - "version": "3.0.1", - "changes": [ - { - "note": "Dependencies updated" - } - ] - }, - { - "version": "3.0.0", + "version": "3.1.0", "changes": [ - { - "note": "Move `LibTransactionDecoder` to contracts/dev-utils package", - "pr": 1848 - }, { "note": "Break up `LibEIP712` into reusable components", "pr": 1742 @@ -75,6 +53,53 @@ { "note": "Add `expirationTimeSeconds` to `ZeroExTransaction` struct", "pr": 1823 + }, + { + "note": "Add reference functions for `LibMath` and `LibFillResults`", + "pr": 2031 + }, + { + "note": "Move in revamped `LibMath` tests from the `contracts-exchange` package.", + "pr": 2031 + }, + { + "note": "Move in revamped `LibFillResults` tests from the `contracts-exchange` package.", + "pr": 2031 + }, + { + "note": "Remove unecessary zero-denominator checks in `LibMath`.", + "pr": 2031 + }, + { + "note": "Fix coverage hooks.", + "pr": 2031 + } + ] + }, + { + "timestamp": 1563193019, + "version": "3.0.2", + "changes": [ + { + "note": "Dependencies updated" + } + ] + }, + { + "timestamp": 1563047529, + "version": "3.0.1", + "changes": [ + { + "note": "Dependencies updated" + } + ] + }, + { + "version": "3.0.0", + "changes": [ + { + "note": "Move `LibTransactionDecoder` to contracts/dev-utils package", + "pr": 1848 } ], "timestamp": 1563006338 diff --git a/contracts/exchange-libs/contracts/src/LibMath.sol b/contracts/exchange-libs/contracts/src/LibMath.sol index 6cc082b052..bc145499af 100644 --- a/contracts/exchange-libs/contracts/src/LibMath.sol +++ b/contracts/exchange-libs/contracts/src/LibMath.sol @@ -41,10 +41,6 @@ contract LibMath is pure returns (uint256 partialAmount) { - if (denominator == 0) { - LibRichErrors._rrevert(LibMathRichErrors.DivisionByZeroError()); - } - if (_isRoundingErrorFloor( numerator, denominator, @@ -79,10 +75,6 @@ contract LibMath is pure returns (uint256 partialAmount) { - if (denominator == 0) { - LibRichErrors._rrevert(LibMathRichErrors.DivisionByZeroError()); - } - if (_isRoundingErrorCeil( numerator, denominator, @@ -122,10 +114,6 @@ contract LibMath is pure returns (uint256 partialAmount) { - if (denominator == 0) { - LibRichErrors._rrevert(LibMathRichErrors.DivisionByZeroError()); - } - partialAmount = _safeDiv( _safeMul(numerator, target), denominator @@ -147,10 +135,6 @@ contract LibMath is pure returns (uint256 partialAmount) { - if (denominator == 0) { - LibRichErrors._rrevert(LibMathRichErrors.DivisionByZeroError()); - } - // _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. diff --git a/contracts/exchange-libs/contracts/test/TestLibs.sol b/contracts/exchange-libs/contracts/test/TestLibs.sol index 8f2925af83..43beae3161 100644 --- a/contracts/exchange-libs/contracts/test/TestLibs.sol +++ b/contracts/exchange-libs/contracts/test/TestLibs.sol @@ -73,6 +73,40 @@ contract TestLibs is return partialAmount; } + function safeGetPartialAmountFloor( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + partialAmount = _safeGetPartialAmountFloor( + numerator, + denominator, + target + ); + return partialAmount; + } + + function safeGetPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + partialAmount = _safeGetPartialAmountCeil( + numerator, + denominator, + target + ); + return partialAmount; + } + function isRoundingErrorFloor( uint256 numerator, uint256 denominator, diff --git a/contracts/exchange-libs/package.json b/contracts/exchange-libs/package.json index 5e9b69a81f..cd8d1ec81c 100644 --- a/contracts/exchange-libs/package.json +++ b/contracts/exchange-libs/package.json @@ -53,6 +53,7 @@ "@0x/contracts-test-utils": "^3.1.10", "@0x/dev-utils": "^2.2.4", "@0x/sol-compiler": "^3.1.9", + "@0x/subproviders": "^4.1.1", "@0x/tslint-config": "^3.0.1", "@types/lodash": "4.14.104", "@types/node": "*", diff --git a/contracts/exchange-libs/src/index.ts b/contracts/exchange-libs/src/index.ts index d55f08ea2d..0233c608d1 100644 --- a/contracts/exchange-libs/src/index.ts +++ b/contracts/exchange-libs/src/index.ts @@ -1,2 +1,5 @@ export * from './artifacts'; export * from './wrappers'; + +import * as ReferenceFunctionsToExport from './reference_functions'; +export import ReferenceFunctions = ReferenceFunctionsToExport; diff --git a/contracts/exchange-libs/src/reference_functions.ts b/contracts/exchange-libs/src/reference_functions.ts new file mode 100644 index 0000000000..e3894adf13 --- /dev/null +++ b/contracts/exchange-libs/src/reference_functions.ts @@ -0,0 +1,89 @@ +import { ReferenceFunctions } from '@0x/contracts-utils'; +import { LibMathRevertErrors } from '@0x/order-utils'; +import { FillResults } from '@0x/types'; +import { BigNumber } from '@0x/utils'; + +const { safeAdd, safeSub, safeMul, safeDiv } = ReferenceFunctions; + +/** + * Checks if rounding error >= 0.1% when rounding down. + */ +export function isRoundingErrorFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { + if (denominator.eq(0)) { + throw new LibMathRevertErrors.DivisionByZeroError(); + } + if (numerator.eq(0) || target.eq(0)) { + return false; + } + const remainder = numerator.times(target).mod(denominator); + // Need to do this separately because solidity evaluates RHS of the comparison expression first. + const rhs = safeMul(numerator, target); + const lhs = safeMul(new BigNumber(1000), remainder); + return lhs.gte(rhs); +} + +/** + * Checks if rounding error >= 0.1% when rounding up. + */ +export function isRoundingErrorCeil(numerator: BigNumber, denominator: BigNumber, target: BigNumber): boolean { + if (denominator.eq(0)) { + throw new LibMathRevertErrors.DivisionByZeroError(); + } + if (numerator.eq(0) || target.eq(0)) { + return false; + } + let remainder = numerator.times(target).mod(denominator); + remainder = safeSub(denominator, remainder).mod(denominator); + // Need to do this separately because solidity evaluates RHS of the comparison expression first. + const rhs = safeMul(numerator, target); + const lhs = safeMul(new BigNumber(1000), remainder); + return lhs.gte(rhs); +} + +/** + * Calculates partial value given a numerator and denominator rounded down. + * Reverts if rounding error is >= 0.1% + */ +export function safeGetPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + if (isRoundingErrorFloor(numerator, denominator, target)) { + throw new LibMathRevertErrors.RoundingError(numerator, denominator, target); + } + return safeDiv(safeMul(numerator, target), denominator); +} + +/** + * Calculates partial value given a numerator and denominator rounded down. + * Reverts if rounding error is >= 0.1% + */ +export function safeGetPartialAmountCeil(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + if (isRoundingErrorCeil(numerator, denominator, target)) { + throw new LibMathRevertErrors.RoundingError(numerator, denominator, target); + } + return safeDiv(safeAdd(safeMul(numerator, target), safeSub(denominator, new BigNumber(1))), denominator); +} + +/** + * Calculates partial value given a numerator and denominator rounded down. + */ +export function getPartialAmountFloor(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + return safeDiv(safeMul(numerator, target), denominator); +} + +/** + * Calculates partial value given a numerator and denominator rounded down. + */ +export function getPartialAmountCeil(numerator: BigNumber, denominator: BigNumber, target: BigNumber): BigNumber { + return safeDiv(safeAdd(safeMul(numerator, target), safeSub(denominator, new BigNumber(1))), denominator); +} + +/** + * Adds properties of two `FillResults`. + */ +export function addFillResults(a: FillResults, b: FillResults): FillResults { + return { + makerAssetFilledAmount: safeAdd(a.makerAssetFilledAmount, b.makerAssetFilledAmount), + takerAssetFilledAmount: safeAdd(a.takerAssetFilledAmount, b.takerAssetFilledAmount), + makerFeePaid: safeAdd(a.makerFeePaid, b.makerFeePaid), + takerFeePaid: safeAdd(a.takerFeePaid, b.takerFeePaid), + }; +} diff --git a/contracts/exchange-libs/test/global_hooks.ts b/contracts/exchange-libs/test/global_hooks.ts index 2ca47d433b..91d8693e15 100644 --- a/contracts/exchange-libs/test/global_hooks.ts +++ b/contracts/exchange-libs/test/global_hooks.ts @@ -1,18 +1,28 @@ -import { env, EnvVars } from '@0x/dev-utils'; - import { coverage, profiler, provider } from '@0x/contracts-test-utils'; +import { env, EnvVars } from '@0x/dev-utils'; +import { prependSubprovider } from '@0x/subproviders'; import { providerUtils } from '@0x/utils'; +const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); +const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + +if (env.parseBoolean(EnvVars.SolidityCoverage)) { + prependSubprovider(provider, coverageSubprovider); + provider.stop(); +} +if (env.parseBoolean(EnvVars.SolidityProfiler)) { + prependSubprovider(provider, profilerSubprovider); + provider.stop(); +} + before('start web3 provider', () => { providerUtils.startProviderEngine(provider); }); after('generate coverage report', async () => { if (env.parseBoolean(EnvVars.SolidityCoverage)) { - const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); await coverageSubprovider.writeCoverageAsync(); } if (env.parseBoolean(EnvVars.SolidityProfiler)) { - const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); await profilerSubprovider.writeProfilerOutputAsync(); } provider.stop(); diff --git a/contracts/exchange-libs/test/lib_fill_results.ts b/contracts/exchange-libs/test/lib_fill_results.ts new file mode 100644 index 0000000000..a57db9bb38 --- /dev/null +++ b/contracts/exchange-libs/test/lib_fill_results.ts @@ -0,0 +1,90 @@ +import { blockchainTests, constants, describe, expect } from '@0x/contracts-test-utils'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import * as _ from 'lodash'; + +import { artifacts, ReferenceFunctions, TestLibsContract } from '../src'; + +blockchainTests('LibFillResults', env => { + const CHAIN_ID = 1337; + const { ONE_ETHER, MAX_UINT256 } = constants; + let libsContract: TestLibsContract; + + before(async () => { + libsContract = await TestLibsContract.deployFrom0xArtifactAsync( + artifacts.TestLibs, + env.provider, + env.txDefaults, + new BigNumber(CHAIN_ID), + ); + }); + + describe('addFillResults', () => { + describe('explicit tests', () => { + const DEFAULT_FILL_RESULTS = [ + { + makerAssetFilledAmount: ONE_ETHER, + takerAssetFilledAmount: ONE_ETHER.times(2), + makerFeePaid: ONE_ETHER.times(0.001), + takerFeePaid: ONE_ETHER.times(0.002), + }, + { + makerAssetFilledAmount: ONE_ETHER.times(0.01), + takerAssetFilledAmount: ONE_ETHER.times(2).times(0.01), + makerFeePaid: ONE_ETHER.times(0.001).times(0.01), + takerFeePaid: ONE_ETHER.times(0.002).times(0.01), + }, + ]; + + it('matches the output of the reference function', async () => { + const [a, b] = DEFAULT_FILL_RESULTS; + const expected = ReferenceFunctions.addFillResults(a, b); + const actual = await libsContract.addFillResults.callAsync(a, b); + expect(actual).to.deep.equal(expected); + }); + + it('reverts if computing `makerAssetFilledAmount` overflows', async () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.makerAssetFilledAmount = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.makerAssetFilledAmount, + b.makerAssetFilledAmount, + ); + return expect(libsContract.addFillResults.callAsync(a, b)).to.revertWith(expectedError); + }); + + it('reverts if computing `takerAssetFilledAmount` overflows', async () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.takerAssetFilledAmount = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.takerAssetFilledAmount, + b.takerAssetFilledAmount, + ); + return expect(libsContract.addFillResults.callAsync(a, b)).to.revertWith(expectedError); + }); + + it('reverts if computing `makerFeePaid` overflows', async () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.makerFeePaid = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.makerFeePaid, + b.makerFeePaid, + ); + return expect(libsContract.addFillResults.callAsync(a, b)).to.revertWith(expectedError); + }); + + it('reverts if computing `takerFeePaid` overflows', async () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.takerFeePaid = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.takerFeePaid, + b.takerFeePaid, + ); + return expect(libsContract.addFillResults.callAsync(a, b)).to.revertWith(expectedError); + }); + }); + }); +}); diff --git a/contracts/exchange-libs/test/lib_math.ts b/contracts/exchange-libs/test/lib_math.ts new file mode 100644 index 0000000000..a25d23fa6f --- /dev/null +++ b/contracts/exchange-libs/test/lib_math.ts @@ -0,0 +1,422 @@ +import { + blockchainTests, + constants, + describe, + expect, + testCombinatoriallyWithReferenceFunc, + uint256Values, +} from '@0x/contracts-test-utils'; +import { LibMathRevertErrors } from '@0x/order-utils'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; + +import { artifacts, ReferenceFunctions, TestLibsContract } from '../src'; + +blockchainTests('LibMath', env => { + const CHAIN_ID = 1337; + const { ONE_ETHER, MAX_UINT256, MAX_UINT256_ROOT, ZERO_AMOUNT } = constants; + let libsContract: TestLibsContract; + + before(async () => { + libsContract = await TestLibsContract.deployFrom0xArtifactAsync( + artifacts.TestLibs, + env.provider, + env.txDefaults, + new BigNumber(CHAIN_ID), + ); + }); + + // Wrap a reference function with identical arguments in a promise. + function createAsyncReferenceFunction(ref: (...args: any[]) => T): (...args: any[]) => Promise { + return async (...args: any[]): Promise => { + return ref(...args); + }; + } + + function createContractTestFunction(name: string): (...args: any[]) => Promise { + return async (...args: any[]): Promise => { + const method = (libsContract as any)[name] as { callAsync: (...args: any[]) => Promise }; + return method.callAsync(...args); + }; + } + + describe('getPartialAmountFloor', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'getPartialAmountFloor', + createAsyncReferenceFunction(ReferenceFunctions.getPartialAmountFloor), + createContractTestFunction('getPartialAmountFloor'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + const expected = ReferenceFunctions.getPartialAmountFloor(numerator, denominator, target); + const actual = await libsContract.getPartialAmountFloor.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('rounds down when computing the partial amount', async () => { + const numerator = ONE_ETHER.times(0.6); + const denominator = ONE_ETHER.times(1.8); + const target = ONE_ETHER; + const expected = ONE_ETHER.dividedToIntegerBy(3); + const actual = await libsContract.getPartialAmountFloor.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero, + numerator.times(target), + denominator, + ); + return expect( + libsContract.getPartialAmountFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect( + libsContract.getPartialAmountFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + }); + }); + + describe('getPartialAmountCeil', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'getPartialAmountCeil', + createAsyncReferenceFunction(ReferenceFunctions.getPartialAmountCeil), + createContractTestFunction('getPartialAmountCeil'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + const expected = ReferenceFunctions.getPartialAmountCeil(numerator, denominator, target); + const actual = await libsContract.getPartialAmountCeil.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('rounds up when computing the partial amount', async () => { + const numerator = ONE_ETHER.times(0.6); + const denominator = ONE_ETHER.times(1.8); + const target = ONE_ETHER; + const expected = ONE_ETHER.dividedToIntegerBy(3).plus(1); + const actual = await libsContract.getPartialAmountCeil.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + // This will actually manifest as a subtraction underflow. + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256SubtractionUnderflow, + denominator, + new BigNumber(1), + ); + return expect( + libsContract.getPartialAmountCeil.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect( + libsContract.getPartialAmountCeil.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + }); + }); + + describe('safeGetPartialAmountFloor', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'safeGetPartialAmountFloor', + createAsyncReferenceFunction(ReferenceFunctions.safeGetPartialAmountFloor), + createContractTestFunction('safeGetPartialAmountFloor'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + const expected = ReferenceFunctions.safeGetPartialAmountFloor(numerator, denominator, target); + const actual = await libsContract.safeGetPartialAmountFloor.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('rounds down when computing the partial amount', async () => { + const numerator = ONE_ETHER.times(0.6); + const denominator = ONE_ETHER.times(1.8); + const target = ONE_ETHER; + const expected = ONE_ETHER.dividedToIntegerBy(3); + const actual = await libsContract.safeGetPartialAmountFloor.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts for a rounding error', async () => { + const numerator = new BigNumber(1e3); + const denominator = new BigNumber(1e4); + const target = new BigNumber(333); + const expectedError = new LibMathRevertErrors.RoundingError(numerator, denominator, target); + return expect( + libsContract.safeGetPartialAmountFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect( + libsContract.safeGetPartialAmountFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect( + libsContract.safeGetPartialAmountFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + }); + }); + + describe('safeGetPartialAmountCeil', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'safeGetPartialAmountCeil', + createAsyncReferenceFunction(ReferenceFunctions.safeGetPartialAmountCeil), + createContractTestFunction('safeGetPartialAmountCeil'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + const expected = ReferenceFunctions.safeGetPartialAmountCeil(numerator, denominator, target); + const actual = await libsContract.safeGetPartialAmountCeil.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('rounds up when computing the partial amount', async () => { + const numerator = ONE_ETHER.times(0.6); + const denominator = ONE_ETHER.times(1.8); + const target = ONE_ETHER; + const expected = ONE_ETHER.dividedToIntegerBy(3).plus(1); + const actual = await libsContract.safeGetPartialAmountCeil.callAsync(numerator, denominator, target); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts for a rounding error', async () => { + const numerator = new BigNumber(1e3); + const denominator = new BigNumber(1e4); + const target = new BigNumber(333); + const expectedError = new LibMathRevertErrors.RoundingError(numerator, denominator, target); + return expect( + libsContract.safeGetPartialAmountCeil.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect( + libsContract.safeGetPartialAmountCeil.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect( + libsContract.safeGetPartialAmountCeil.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + }); + }); + + describe('isRoundingErrorFloor', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'isRoundingErrorFloor', + createAsyncReferenceFunction(ReferenceFunctions.isRoundingErrorFloor), + createContractTestFunction('isRoundingErrorFloor'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('returns true when `numerator * target / denominator` produces an error >= 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(102); + const target = new BigNumber(52); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(actual).to.eq(true); + }); + + it('returns false when `numerator * target / denominator` produces an error < 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(101); + const target = new BigNumber(92); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(actual).to.eq(false); + }); + + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + // tslint:disable-next-line: boolean-naming + const expected = ReferenceFunctions.isRoundingErrorFloor(numerator, denominator, target); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorFloor.callAsync(numerator, denominator, target); + expect(actual).to.eq(expected); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect( + libsContract.isRoundingErrorFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect( + libsContract.isRoundingErrorFloor.callAsync(numerator, denominator, target), + ).to.revertWith(expectedError); + }); + }); + }); + + describe('isRoundingErrorCeil', () => { + describe.optional('combinatorial tests', () => { + testCombinatoriallyWithReferenceFunc( + 'isRoundingErrorCeil', + createAsyncReferenceFunction(ReferenceFunctions.isRoundingErrorCeil), + createContractTestFunction('isRoundingErrorCeil'), + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('explicit tests', () => { + it('returns true when `numerator * target / (denominator - 1)` produces an error >= 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(101); + const target = new BigNumber(92); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(actual).to.eq(true); + }); + + it('returns false when `numerator * target / (denominator - 1)` produces an error < 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(102); + const target = new BigNumber(52); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(actual).to.eq(false); + }); + + it('matches the reference function output', async () => { + const numerator = ONE_ETHER; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = ONE_ETHER.times(0.01); + // tslint:disable-next-line: boolean-naming + const expected = ReferenceFunctions.isRoundingErrorCeil(numerator, denominator, target); + // tslint:disable-next-line: boolean-naming + const actual = await libsContract.isRoundingErrorCeil.callAsync(numerator, denominator, target); + expect(actual).to.eq(expected); + }); + + it('reverts if `denominator` is zero', async () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(libsContract.isRoundingErrorCeil.callAsync(numerator, denominator, target)).to.revertWith( + expectedError, + ); + }); + + it('reverts if `numerator * target` overflows', async () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(libsContract.isRoundingErrorCeil.callAsync(numerator, denominator, target)).to.revertWith( + expectedError, + ); + }); + }); + }); +}); diff --git a/contracts/exchange-libs/test/reference_functions.ts b/contracts/exchange-libs/test/reference_functions.ts new file mode 100644 index 0000000000..623582f884 --- /dev/null +++ b/contracts/exchange-libs/test/reference_functions.ts @@ -0,0 +1,315 @@ +import { constants, describe, expect } from '@0x/contracts-test-utils'; +import { LibMathRevertErrors } from '@0x/order-utils'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import * as _ from 'lodash'; + +import { + addFillResults, + getPartialAmountCeil, + getPartialAmountFloor, + isRoundingErrorCeil, + isRoundingErrorFloor, + safeGetPartialAmountCeil, + safeGetPartialAmountFloor, +} from '../src/reference_functions'; + +describe('Reference Functions', () => { + const { ONE_ETHER, MAX_UINT256, MAX_UINT256_ROOT, ZERO_AMOUNT } = constants; + describe('LibFillResults', () => { + describe('addFillResults', () => { + const DEFAULT_FILL_RESULTS = [ + { + makerAssetFilledAmount: ONE_ETHER, + takerAssetFilledAmount: ONE_ETHER.times(2), + makerFeePaid: ONE_ETHER.times(0.001), + takerFeePaid: ONE_ETHER.times(0.002), + }, + { + makerAssetFilledAmount: ONE_ETHER.times(0.01), + takerAssetFilledAmount: ONE_ETHER.times(2).times(0.01), + makerFeePaid: ONE_ETHER.times(0.001).times(0.01), + takerFeePaid: ONE_ETHER.times(0.002).times(0.01), + }, + ]; + + it('reverts if computing `makerAssetFilledAmount` overflows', () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.makerAssetFilledAmount = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.makerAssetFilledAmount, + b.makerAssetFilledAmount, + ); + expect(() => addFillResults(a, b)).to.throw(expectedError.message); + }); + + it('reverts if computing `takerAssetFilledAmount` overflows', () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.takerAssetFilledAmount = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.takerAssetFilledAmount, + b.takerAssetFilledAmount, + ); + expect(() => addFillResults(a, b)).to.throw(expectedError.message); + }); + + it('reverts if computing `makerFeePaid` overflows', () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.makerFeePaid = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.makerFeePaid, + b.makerFeePaid, + ); + expect(() => addFillResults(a, b)).to.throw(expectedError.message); + }); + + it('reverts if computing `takerFeePaid` overflows', () => { + const [a, b] = _.cloneDeep(DEFAULT_FILL_RESULTS); + b.takerFeePaid = MAX_UINT256; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a.takerFeePaid, + b.takerFeePaid, + ); + expect(() => addFillResults(a, b)).to.throw(expectedError.message); + }); + }); + }); + + describe('LibMath', () => { + describe('getPartialAmountFloor', () => { + describe('explicit tests', () => { + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero, + numerator.times(target), + denominator, + ); + return expect(() => getPartialAmountFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => getPartialAmountFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + + describe('getPartialAmountCeil', () => { + describe('explicit tests', () => { + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + // This will actually manifest as a subtraction underflow. + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256SubtractionUnderflow, + denominator, + new BigNumber(1), + ); + return expect(() => getPartialAmountCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => getPartialAmountCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + + describe('safeGetPartialAmountFloor', () => { + describe('explicit tests', () => { + it('reverts for a rounding error', () => { + const numerator = new BigNumber(1e3); + const denominator = new BigNumber(1e4); + const target = new BigNumber(333); + const expectedError = new LibMathRevertErrors.RoundingError(numerator, denominator, target); + return expect(() => safeGetPartialAmountFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => safeGetPartialAmountFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => safeGetPartialAmountFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + + describe('safeGetPartialAmountCeil', () => { + describe('explicit tests', () => { + it('reverts for a rounding error', () => { + const numerator = new BigNumber(1e3); + const denominator = new BigNumber(1e4); + const target = new BigNumber(333); + const expectedError = new LibMathRevertErrors.RoundingError(numerator, denominator, target); + return expect(() => safeGetPartialAmountCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => safeGetPartialAmountCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => safeGetPartialAmountCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + + describe('isRoundingErrorFloor', () => { + describe('explicit tests', () => { + it('returns true when `numerator * target / denominator` produces an error >= 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(102); + const target = new BigNumber(52); + // tslint:disable-next-line: boolean-naming + const actual = isRoundingErrorFloor(numerator, denominator, target); + expect(actual).to.eq(true); + }); + + it('returns false when `numerator * target / denominator` produces an error < 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(101); + const target = new BigNumber(92); + // tslint:disable-next-line: boolean-naming + const actual = isRoundingErrorFloor(numerator, denominator, target); + expect(actual).to.eq(false); + }); + + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => isRoundingErrorFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => isRoundingErrorFloor(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + + describe('isRoundingErrorCeil', () => { + describe('explicit tests', () => { + it('returns true when `numerator * target / (denominator - 1)` produces an error >= 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(101); + const target = new BigNumber(92); + // tslint:disable-next-line: boolean-naming + const actual = isRoundingErrorCeil(numerator, denominator, target); + expect(actual).to.eq(true); + }); + + it('returns false when `numerator * target / (denominator - 1)` produces an error < 0.1%', async () => { + const numerator = new BigNumber(100); + const denominator = new BigNumber(102); + const target = new BigNumber(52); + // tslint:disable-next-line: boolean-naming + const actual = isRoundingErrorCeil(numerator, denominator, target); + expect(actual).to.eq(false); + }); + + it('reverts if `denominator` is zero', () => { + const numerator = ONE_ETHER; + const denominator = ZERO_AMOUNT; + const target = ONE_ETHER.times(0.01); + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => isRoundingErrorCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + + it('reverts if `numerator * target` overflows', () => { + const numerator = MAX_UINT256; + const denominator = ONE_ETHER.dividedToIntegerBy(2); + const target = MAX_UINT256_ROOT.times(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + numerator, + target, + ); + return expect(() => isRoundingErrorCeil(numerator, denominator, target)).to.throw( + expectedError.message, + ); + }); + }); + }); + }); +}); diff --git a/contracts/exchange/CHANGELOG.json b/contracts/exchange/CHANGELOG.json index fd52350d72..8ec36624f4 100644 --- a/contracts/exchange/CHANGELOG.json +++ b/contracts/exchange/CHANGELOG.json @@ -117,6 +117,26 @@ { "note": "Rewrote _dispatchTransferFrom in Solidity", "pr": 2020 + }, + { + "note": "Add `TestIsolatedExchange` contract and `IsolatedExchangeWrapper` test class", + "pr": 2031 + }, + { + "note": "Add `ReferenceFunctions` as package export.", + "pr": 2031 + }, + { + "note": "Remove `TestExchangeMath.sol`. Exchange math functions are now tested in the `exchange-libs` package and reference implementations are available there as well.", + "pr": 2031 + }, + { + "note": "Remove functions from `TestExchangeInternals.sol` that are no longer tested in this package.", + "pr": 2031 + }, + { + "note": "Remove `_assertValidFill()`", + "pr": 2031 } ] }, diff --git a/contracts/exchange/compiler.json b/contracts/exchange/compiler.json index 167f1f515a..2f7baf36f1 100644 --- a/contracts/exchange/compiler.json +++ b/contracts/exchange/compiler.json @@ -33,13 +33,13 @@ "src/interfaces/IExchangeCore.sol", "src/interfaces/IMatchOrders.sol", "src/interfaces/ISignatureValidator.sol", + "test/IsolatedExchange.sol", "src/interfaces/ITransactions.sol", "src/interfaces/IWallet.sol", "src/interfaces/IWrapperFunctions.sol", "test/ReentrantERC20Token.sol", "test/TestAssetProxyDispatcher.sol", "test/TestExchangeInternals.sol", - "test/TestExchangeMath.sol", "test/TestLibExchangeRichErrorDecoder.sol", "test/TestSignatureValidator.sol", "test/TestValidatorWallet.sol" diff --git a/contracts/exchange/contracts/src/MixinExchangeCore.sol b/contracts/exchange/contracts/src/MixinExchangeCore.sol index 656b31806f..844419ec70 100644 --- a/contracts/exchange/contracts/src/MixinExchangeCore.sol +++ b/contracts/exchange/contracts/src/MixinExchangeCore.sol @@ -210,15 +210,6 @@ contract MixinExchangeCore is uint256 remainingTakerAssetAmount = _safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount); uint256 takerAssetFilledAmount = _min256(takerAssetFillAmount, remainingTakerAssetAmount); - // Validate context - _assertValidFill( - order, - orderInfo, - takerAssetFillAmount, - takerAssetFilledAmount, - fillResults.makerAssetFilledAmount - ); - // Compute proportional fill amounts fillResults = _calculateFillResults(order, takerAssetFilledAmount); @@ -389,78 +380,6 @@ contract MixinExchangeCore is } } - /// @dev Validates context for fillOrder. Succeeds or throws. - /// @param order to be filled. - /// @param orderInfo OrderStatus, orderHash, and amount already filled of order. - /// @param takerAssetFillAmount Desired amount of order to fill by taker. - /// @param takerAssetFilledAmount Amount of takerAsset that will be filled. - /// @param makerAssetFilledAmount Amount of makerAsset that will be transfered. - function _assertValidFill( - Order memory order, - OrderInfo memory orderInfo, - uint256 takerAssetFillAmount, // TODO: use FillResults - uint256 takerAssetFilledAmount, - uint256 makerAssetFilledAmount - ) - internal - pure - { - // Revert if fill amount is invalid - // TODO: reconsider necessity for v2.1 - if (takerAssetFillAmount == 0) { - LibRichErrors._rrevert(LibExchangeRichErrors.FillError( - FillErrorCodes.INVALID_TAKER_AMOUNT, - orderInfo.orderHash - )); - } - - // Make sure taker does not pay more than desired amount - // NOTE: This assertion should never fail, it is here - // as an extra defence against potential bugs. - if (takerAssetFilledAmount > takerAssetFillAmount) { - LibRichErrors._rrevert(LibExchangeRichErrors.FillError( - FillErrorCodes.TAKER_OVERPAY, - orderInfo.orderHash - )); - } - - // Make sure order is not overfilled - // NOTE: This assertion should never fail, it is here - // as an extra defence against potential bugs. - if (_safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount) - > order.takerAssetAmount) { - LibRichErrors._rrevert(LibExchangeRichErrors.FillError( - FillErrorCodes.OVERFILL, - orderInfo.orderHash - )); - } - - // Make sure order is filled at acceptable price. - // The order has an implied price from the makers perspective: - // order price = order.makerAssetAmount / order.takerAssetAmount - // i.e. the number of makerAsset maker is paying per takerAsset. The - // maker is guaranteed to get this price or a better (lower) one. The - // actual price maker is getting in this fill is: - // fill price = makerAssetFilledAmount / takerAssetFilledAmount - // We need `fill price <= order price` for the fill to be fair to maker. - // This amounts to: - // makerAssetFilledAmount order.makerAssetAmount - // ------------------------ <= ----------------------- - // takerAssetFilledAmount order.takerAssetAmount - // or, equivalently: - // makerAssetFilledAmount * order.takerAssetAmount <= - // order.makerAssetAmount * takerAssetFilledAmount - // NOTE: This assertion should never fail, it is here - // as an extra defence against potential bugs. - if (_safeMul(makerAssetFilledAmount, order.takerAssetAmount) - > _safeMul(order.makerAssetAmount, takerAssetFilledAmount)) { - LibRichErrors._rrevert(LibExchangeRichErrors.FillError( - FillErrorCodes.INVALID_FILL_PRICE, - orderInfo.orderHash - )); - } - } - /// @dev Validates context for cancelOrder. Succeeds or throws. /// @param order to be cancelled. /// @param orderInfo OrderStatus, orderHash, and amount already filled of order. @@ -529,7 +448,7 @@ contract MixinExchangeCore is address takerAddress, LibFillResults.FillResults memory fillResults ) - private + internal { // Transfer taker -> maker _dispatchTransferFrom( diff --git a/contracts/exchange/contracts/src/MixinMatchOrders.sol b/contracts/exchange/contracts/src/MixinMatchOrders.sol index 04ddb08a9b..17eeeaf453 100644 --- a/contracts/exchange/contracts/src/MixinMatchOrders.sol +++ b/contracts/exchange/contracts/src/MixinMatchOrders.sol @@ -680,22 +680,6 @@ contract MixinMatchOrders is shouldMaximallyFillOrders ); - // Validate fill contexts - _assertValidFill( - leftOrder, - leftOrderInfo, - matchedFillResults.left.takerAssetFilledAmount, - matchedFillResults.left.takerAssetFilledAmount, - matchedFillResults.left.makerAssetFilledAmount - ); - _assertValidFill( - rightOrder, - rightOrderInfo, - matchedFillResults.right.takerAssetFilledAmount, - matchedFillResults.right.takerAssetFilledAmount, - matchedFillResults.right.makerAssetFilledAmount - ); - // Update exchange state _updateFilledState( leftOrder, diff --git a/contracts/exchange/contracts/test/IsolatedExchange.sol b/contracts/exchange/contracts/test/IsolatedExchange.sol new file mode 100644 index 0000000000..00ca515957 --- /dev/null +++ b/contracts/exchange/contracts/test/IsolatedExchange.sol @@ -0,0 +1,86 @@ +/* + + Copyright 2019 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.5.5; +pragma experimental ABIEncoderV2; + +import "../src/Exchange.sol"; + + +/// @dev A version of the Exchange contract with simplified signature validation +/// and a `_dispatchTransferFrom()` that only logs arguments. +/// See the `IsolatedExchangeWrapper` and `isolated_fill_order` tests +/// for example usage. +contract IsolatedExchange is + Exchange +{ + // solhint-disable no-unused-vars + event DispatchTransferFromCalled( + bytes32 orderHash, + bytes assetData, + address from, + address to, + uint256 amount + ); + + // solhint-disable no-empty-blocks + constructor () + public + Exchange(1337) + {} + + /// @dev Overriden to only log arguments and revert on certain assetDatas. + function _dispatchTransferFrom( + bytes32 orderHash, + bytes memory assetData, + address from, + address to, + uint256 amount + ) + internal + { + emit DispatchTransferFromCalled( + orderHash, + assetData, + from, + to, + amount + ); + + // Fail if the first byte is 0. + if (assetData.length > 0 && assetData[0] == 0x00) { + revert("TRANSFER_FAILED"); + } + } + + /// @dev Overriden to simplify signature validation. + /// Unfortunately, this is `view`, so it can't log arguments. + function _isValidOrderWithHashSignature( + Order memory order, + bytes32 orderHash, + address signerAddress, + bytes memory signature + ) + internal + view + returns (bool isValid) + { + // '0x01' in the first byte is valid. + return signature.length == 2 && signature[0] == 0x01; + } +} diff --git a/contracts/exchange/contracts/test/TestExchangeInternals.sol b/contracts/exchange/contracts/test/TestExchangeInternals.sol index f40ca21797..151a2858de 100644 --- a/contracts/exchange/contracts/test/TestExchangeInternals.sol +++ b/contracts/exchange/contracts/test/TestExchangeInternals.sol @@ -26,31 +26,19 @@ import "../src/Exchange.sol"; contract TestExchangeInternals is Exchange { + event DispatchTransferFromCalled( + bytes32 orderHash, + bytes assetData, + address from, + address to, + uint256 amount + ); + constructor (uint256 chainId) public Exchange(chainId) {} - /// @dev Adds properties of both FillResults instances. - /// Modifies the first FillResults instance specified. - /// Note that this function has been modified from the original - // internal version to return the FillResults. - /// @param totalFillResults Fill results instance that will be added onto. - /// @param singleFillResults Fill results instance that will be added to totalFillResults. - /// @return newTotalFillResults The result of adding singleFillResults to totalFilResults. - function addFillResults(FillResults memory totalFillResults, FillResults memory singleFillResults) - public - pure - returns (FillResults memory) - { - _addFillResults(totalFillResults, singleFillResults); - return totalFillResults; - } - - /// @dev Calculates amounts filled and fees paid by maker and taker. - /// @param order to be filled. - /// @param takerAssetFilledAmount Amount of takerAsset that will be filled. - /// @return fillResults Amounts filled and fees paid by maker and taker. function calculateFillResults( Order memory order, uint256 takerAssetFilledAmount @@ -62,12 +50,9 @@ contract TestExchangeInternals is return _calculateFillResults(order, takerAssetFilledAmount); } - /// @dev Updates state with results of a fill order. - /// @param order that was filled. - /// @param takerAddress Address of taker who filled the order. - /// @param orderTakerAssetFilledAmount Amount of order already filled. - /// @return fillResults Amounts filled and fees paid by maker and taker. - function updateFilledState( + /// @dev Call `_updateFilledState()` but first set `filled[order]` to + /// `orderTakerAssetFilledAmount`. + function testUpdateFilledState( Order memory order, address takerAddress, bytes32 orderHash, @@ -76,6 +61,7 @@ contract TestExchangeInternals is ) public { + filled[getOrderHash(order)] = orderTakerAssetFilledAmount; _updateFilledState( order, takerAddress, @@ -84,4 +70,34 @@ contract TestExchangeInternals is fillResults ); } + + function settleOrder( + bytes32 orderHash, + LibOrder.Order memory order, + address takerAddress, + LibFillResults.FillResults memory fillResults + ) + public + { + _settleOrder(orderHash, order, takerAddress, fillResults); + } + + /// @dev Overidden to only log arguments so we can test `_settleOrder()`. + function _dispatchTransferFrom( + bytes32 orderHash, + bytes memory assetData, + address from, + address to, + uint256 amount + ) + internal + { + emit DispatchTransferFromCalled( + orderHash, + assetData, + from, + to, + amount + ); + } } diff --git a/contracts/exchange/contracts/test/TestExchangeMath.sol b/contracts/exchange/contracts/test/TestExchangeMath.sol deleted file mode 100644 index 832e4c5db7..0000000000 --- a/contracts/exchange/contracts/test/TestExchangeMath.sol +++ /dev/null @@ -1,131 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; -pragma experimental ABIEncoderV2; - -import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; - - -contract TestExchangeMath is - LibMath -{ - /// @dev Calculates partial value given a numerator and denominator. - /// Reverts if rounding error is >= 0.1% - /// @param numerator Numerator. - /// @param denominator Denominator. - /// @param target Value to calculate partial of. - /// @return Partial value of target. - function safeGetPartialAmountFloor( - uint256 numerator, - uint256 denominator, - uint256 target - ) - public - pure - returns (uint256 partialAmount) - { - return _safeGetPartialAmountFloor(numerator, denominator, target); - } - - /// @dev Calculates partial value given a numerator and denominator. - /// Reverts if rounding error is >= 0.1% - /// @param numerator Numerator. - /// @param denominator Denominator. - /// @param target Value to calculate partial of. - /// @return Partial value of target. - function safeGetPartialAmountCeil( - uint256 numerator, - uint256 denominator, - uint256 target - ) - public - pure - returns (uint256 partialAmount) - { - return _safeGetPartialAmountCeil(numerator, denominator, target); - } - - /// @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 getPartialAmountFloor( - uint256 numerator, - uint256 denominator, - uint256 target - ) - public - pure - returns (uint256 partialAmount) - { - return _getPartialAmountFloor(numerator, denominator, target); - } - - /// @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 getPartialAmountCeil( - 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 isRoundingErrorFloor( - 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 isRoundingErrorCeil( - uint256 numerator, - uint256 denominator, - uint256 target - ) - public - pure - returns (bool isError) - { - return _isRoundingErrorCeil(numerator, denominator, target); - } -} diff --git a/contracts/exchange/package.json b/contracts/exchange/package.json index 83dc128500..a73ab1b0ac 100644 --- a/contracts/exchange/package.json +++ b/contracts/exchange/package.json @@ -34,7 +34,7 @@ "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" }, "config": { - "abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IWallet|IWrapperFunctions|ReentrantERC20Token|TestAssetProxyDispatcher|TestExchangeInternals|TestExchangeMath|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|Whitelist).json", + "abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|IWallet|IWrapperFunctions|IsolatedExchange|ReentrantERC20Token|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|Whitelist).json", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." }, "repository": { diff --git a/contracts/exchange/src/artifacts.ts b/contracts/exchange/src/artifacts.ts index b0aa72ec35..e25db9a76d 100644 --- a/contracts/exchange/src/artifacts.ts +++ b/contracts/exchange/src/artifacts.ts @@ -16,10 +16,10 @@ import * as ISignatureValidator from '../generated-artifacts/ISignatureValidator import * as ITransactions from '../generated-artifacts/ITransactions.json'; import * as IWallet from '../generated-artifacts/IWallet.json'; import * as IWrapperFunctions from '../generated-artifacts/IWrapperFunctions.json'; +import * as IsolatedExchange from '../generated-artifacts/IsolatedExchange.json'; import * as ReentrantERC20Token from '../generated-artifacts/ReentrantERC20Token.json'; import * as TestAssetProxyDispatcher from '../generated-artifacts/TestAssetProxyDispatcher.json'; import * as TestExchangeInternals from '../generated-artifacts/TestExchangeInternals.json'; -import * as TestExchangeMath from '../generated-artifacts/TestExchangeMath.json'; import * as TestLibExchangeRichErrorDecoder from '../generated-artifacts/TestLibExchangeRichErrorDecoder.json'; import * as TestSignatureValidator from '../generated-artifacts/TestSignatureValidator.json'; import * as TestValidatorWallet from '../generated-artifacts/TestValidatorWallet.json'; @@ -37,10 +37,10 @@ export const artifacts = { ITransactions: ITransactions as ContractArtifact, IWallet: IWallet as ContractArtifact, IWrapperFunctions: IWrapperFunctions as ContractArtifact, + IsolatedExchange: IsolatedExchange as ContractArtifact, ReentrantERC20Token: ReentrantERC20Token as ContractArtifact, TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact, TestExchangeInternals: TestExchangeInternals as ContractArtifact, - TestExchangeMath: TestExchangeMath as ContractArtifact, TestLibExchangeRichErrorDecoder: TestLibExchangeRichErrorDecoder as ContractArtifact, TestSignatureValidator: TestSignatureValidator as ContractArtifact, TestValidatorWallet: TestValidatorWallet as ContractArtifact, diff --git a/contracts/exchange/src/index.ts b/contracts/exchange/src/index.ts index ba813e7caf..28c617a1a2 100644 --- a/contracts/exchange/src/index.ts +++ b/contracts/exchange/src/index.ts @@ -1,3 +1,6 @@ export * from './artifacts'; export * from './wrappers'; export * from '../test/utils'; + +import * as ReferenceFunctionsToExport from './reference_functions'; +export import ReferenceFunctions = ReferenceFunctionsToExport; diff --git a/contracts/exchange/src/reference_functions.ts b/contracts/exchange/src/reference_functions.ts new file mode 100644 index 0000000000..91a96999ad --- /dev/null +++ b/contracts/exchange/src/reference_functions.ts @@ -0,0 +1,24 @@ +import { ReferenceFunctions as ExchangeLibsReferenceFunctions } from '@0x/contracts-exchange-libs'; +import { FillResults, OrderWithoutDomain } from '@0x/types'; +import { BigNumber } from '@0x/utils'; + +const { safeGetPartialAmountFloor } = ExchangeLibsReferenceFunctions; + +/** + * Calculates amounts filled and fees paid by maker and taker. + */ +export function calculateFillResults(order: OrderWithoutDomain, takerAssetFilledAmount: BigNumber): FillResults { + const makerAssetFilledAmount = safeGetPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const makerFeePaid = safeGetPartialAmountFloor(makerAssetFilledAmount, order.makerAssetAmount, order.makerFee); + const takerFeePaid = safeGetPartialAmountFloor(takerAssetFilledAmount, order.takerAssetAmount, order.takerFee); + return { + makerAssetFilledAmount, + takerAssetFilledAmount, + makerFeePaid, + takerFeePaid, + }; +} diff --git a/contracts/exchange/src/wrappers.ts b/contracts/exchange/src/wrappers.ts index 719df58807..4b5500fd68 100644 --- a/contracts/exchange/src/wrappers.ts +++ b/contracts/exchange/src/wrappers.ts @@ -14,10 +14,10 @@ export * from '../generated-wrappers/i_signature_validator'; export * from '../generated-wrappers/i_transactions'; export * from '../generated-wrappers/i_wallet'; export * from '../generated-wrappers/i_wrapper_functions'; +export * from '../generated-wrappers/isolated_exchange'; export * from '../generated-wrappers/reentrant_erc20_token'; export * from '../generated-wrappers/test_asset_proxy_dispatcher'; export * from '../generated-wrappers/test_exchange_internals'; -export * from '../generated-wrappers/test_exchange_math'; export * from '../generated-wrappers/test_lib_exchange_rich_error_decoder'; export * from '../generated-wrappers/test_signature_validator'; export * from '../generated-wrappers/test_validator_wallet'; diff --git a/contracts/exchange/test/fill_order.ts b/contracts/exchange/test/fill_order.ts index 34ddafdff2..dee68e6a57 100644 --- a/contracts/exchange/test/fill_order.ts +++ b/contracts/exchange/test/fill_order.ts @@ -189,14 +189,6 @@ blockchainTests.resets('FillOrder Tests', ({ web3Wrapper, txDefaults }) => { await fillOrderCombinatorialUtils.testFillOrderScenarioFailureAsync(fillScenario); }); - it('should revert if takerAssetFillAmount is 0', async () => { - const fillScenario = { - ...defaultFillScenario, - takerAssetFillAmountScenario: TakerAssetFillAmountScenario.Zero, - }; - await fillOrderCombinatorialUtils.testFillOrderScenarioFailureAsync(fillScenario); - }); - it('should revert if an order is expired', async () => { const fillScenario = { ...defaultFillScenario, diff --git a/contracts/exchange/test/internal.ts b/contracts/exchange/test/internal.ts index 48aec23621..4467beaf2b 100644 --- a/contracts/exchange/test/internal.ts +++ b/contracts/exchange/test/internal.ts @@ -1,561 +1,443 @@ +import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs'; import { - bytes32Values, - chaiSetup, + blockchainTests, constants, - FillResults, - provider, - testCombinatoriallyWithReferenceFuncAsync, - txDefaults, + describe, + expect, + hexRandom, + LogDecoder, + testCombinatoriallyWithReferenceFunc, uint256Values, - web3Wrapper, } from '@0x/contracts-test-utils'; -import { BlockchainLifecycle } from '@0x/dev-utils'; import { LibMathRevertErrors } from '@0x/order-utils'; -import { Order, RevertReason, SignedOrder } from '@0x/types'; -import { BigNumber, providerUtils, SafeMathRevertErrors } from '@0x/utils'; -import * as chai from 'chai'; +import { FillResults, OrderWithoutDomain as Order } from '@0x/types'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import { LogWithDecodedArgs } from 'ethereum-types'; import * as _ from 'lodash'; -import { artifacts, TestExchangeInternalsContract, TestExchangeMathContract } from '../src'; - -chaiSetup.configure(); -const expect = chai.expect; - -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - -const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); - -const emptyOrder: Order = { - senderAddress: constants.NULL_ADDRESS, - makerAddress: constants.NULL_ADDRESS, - takerAddress: constants.NULL_ADDRESS, - makerFee: new BigNumber(0), - takerFee: new BigNumber(0), - makerAssetAmount: new BigNumber(0), - takerAssetAmount: new BigNumber(0), - makerAssetData: '0x', - takerAssetData: '0x', - makerFeeAssetData: '0x', - takerFeeAssetData: '0x', - salt: new BigNumber(0), - feeRecipientAddress: constants.NULL_ADDRESS, - expirationTimeSeconds: new BigNumber(0), - domain: { - verifyingContractAddress: constants.NULL_ADDRESS, - chainId: 0, // To be filled in later. - }, -}; - -const emptySignedOrder: SignedOrder = { - ...emptyOrder, - signature: '', -}; - -const safeMathErrorForCall = new SafeMathRevertErrors.SafeMathError(); +import { + artifacts, + ReferenceFunctions, + TestExchangeInternalsContract, + TestExchangeInternalsDispatchTransferFromCalledEventArgs, + TestExchangeInternalsFillEventArgs, +} from '../src'; -describe('Exchange math internal functions', () => { - let chainId: number; - let testExchange: TestExchangeMathContract; - let divisionByZeroErrorForCall: Error | undefined; - let roundingErrorForCall: Error | undefined; +blockchainTests('Exchange core internal functions', env => { + const CHAIN_ID = 1337; + const ONE_ETHER = constants.ONE_ETHER; + const EMPTY_ORDER: Order = { + senderAddress: constants.NULL_ADDRESS, + makerAddress: constants.NULL_ADDRESS, + takerAddress: constants.NULL_ADDRESS, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + makerAssetAmount: constants.ZERO_AMOUNT, + takerAssetAmount: constants.ZERO_AMOUNT, + makerAssetData: constants.NULL_BYTES, + takerAssetData: constants.NULL_BYTES, + makerFeeAssetData: constants.NULL_BYTES, + takerFeeAssetData: constants.NULL_BYTES, + salt: constants.ZERO_AMOUNT, + feeRecipientAddress: constants.NULL_ADDRESS, + expirationTimeSeconds: constants.ZERO_AMOUNT, + }; + const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH); + const randomHash = () => hexRandom(constants.WORD_LENGTH); + const randomAssetData = () => hexRandom(36); + let testExchange: TestExchangeInternalsContract; + let logDecoder: LogDecoder; + let senderAddress: string; before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - chainId = await providerUtils.getChainIdAsync(provider); - emptyOrder.domain.chainId = chainId; - emptySignedOrder.domain.chainId = chainId; - - testExchange = await TestExchangeMathContract.deployFrom0xArtifactAsync( - artifacts.TestExchangeMath, - provider, - txDefaults, + [senderAddress] = await env.getAccountAddressesAsync(); + testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync( + artifacts.TestExchangeInternals, + env.provider, + env.txDefaults, + new BigNumber(CHAIN_ID), ); - divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); - roundingErrorForCall = new Error(RevertReason.RoundingError); - divisionByZeroErrorForCall = new LibMathRevertErrors.DivisionByZeroError(); - roundingErrorForCall = new LibMathRevertErrors.RoundingError(); + logDecoder = new LogDecoder(env.web3Wrapper, artifacts); }); - // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset - // the blockchain state for any tests which modify it! - 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.multipliedBy(target); - const remainder = product.mod(denominator); - const remainderTimes1000 = remainder.multipliedBy('1000'); - const isError = remainderTimes1000.gte(product); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - if (remainderTimes1000.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - 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.multipliedBy(target); - const remainder = product.mod(denominator); - const error = denominator.minus(remainder).mod(denominator); - const errorTimes1000 = error.multipliedBy('1000'); - const isError = errorTimes1000.gte(product); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - if (errorTimes1000.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - return isError; - } - - async function referenceSafeGetPartialAmountFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - const isRoundingError = await referenceIsRoundingErrorFloorAsync(numerator, denominator, target); - if (isRoundingError) { - throw roundingErrorForCall; - } - const product = numerator.multipliedBy(target); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - return product.dividedToIntegerBy(denominator); - } - - describe('getPartialAmountFloor', async () => { - async function referenceGetPartialAmountFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - const product = numerator.multipliedBy(target); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; + blockchainTests('calculateFillResults', () => { + describe.optional('combinatorial tests', () => { + function makeOrder( + makerAssetAmount: BigNumber, + takerAssetAmount: BigNumber, + makerFee: BigNumber, + takerFee: BigNumber, + ): Order { + return { + ...EMPTY_ORDER, + makerAssetAmount, + takerAssetAmount, + makerFee, + takerFee, + }; } - return product.dividedToIntegerBy(denominator); - } - async function testGetPartialAmountFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.getPartialAmountFloor.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'getPartialAmountFloor', - referenceGetPartialAmountFloorAsync, - testGetPartialAmountFloorAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); - describe('getPartialAmountCeil', async () => { - async function referenceGetPartialAmountCeilAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - const product = numerator.multipliedBy(target); - const offset = product.plus(denominator.minus(1)); - if (offset.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; + async function referenceCalculateFillResultsAsync( + orderTakerAssetAmount: BigNumber, + takerAssetFilledAmount: BigNumber, + otherAmount: BigNumber, + ): Promise { + // Note(albrow): Here we are re-using the same value (otherAmount) + // for order.makerAssetAmount, order.makerFee, and order.takerFee. + // This should be safe because they are never used with each other + // in any mathematical operation in either the reference TypeScript + // implementation or the Solidity implementation of + // calculateFillResults. + return ReferenceFunctions.calculateFillResults( + makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount), + takerAssetFilledAmount, + ); } - const result = offset.dividedToIntegerBy(denominator); - if (product.mod(denominator).eq(0)) { - expect(result.multipliedBy(denominator)).to.be.bignumber.eq(product); - } else { - expect(result.multipliedBy(denominator)).to.be.bignumber.gt(product); + + async function testCalculateFillResultsAsync( + orderTakerAssetAmount: BigNumber, + takerAssetFilledAmount: BigNumber, + otherAmount: BigNumber, + ): Promise { + const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount); + return testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount); } - return result; - } - async function testGetPartialAmountCeilAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.getPartialAmountCeil.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'getPartialAmountCeil', - referenceGetPartialAmountCeilAsync, - testGetPartialAmountCeilAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); - describe('safeGetPartialAmountFloor', async () => { - async function testSafeGetPartialAmountFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.safeGetPartialAmountFloor.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'safeGetPartialAmountFloor', - referenceSafeGetPartialAmountFloorAsync, - testSafeGetPartialAmountFloorAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); + testCombinatoriallyWithReferenceFunc( + 'calculateFillResults', + referenceCalculateFillResultsAsync, + testCalculateFillResultsAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); - describe('safeGetPartialAmountCeil', async () => { - async function referenceSafeGetPartialAmountCeilAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - const isRoundingError = await referenceIsRoundingErrorCeilAsync(numerator, denominator, target); - if (isRoundingError) { - throw roundingErrorForCall; - } - const product = numerator.multipliedBy(target); - const offset = product.plus(denominator.minus(1)); - if (offset.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - const result = offset.dividedToIntegerBy(denominator); - if (product.mod(denominator).eq(0)) { - expect(result.multipliedBy(denominator)).to.be.bignumber.eq(product); - } else { - expect(result.multipliedBy(denominator)).to.be.bignumber.gt(product); + describe('explicit tests', () => { + const MAX_UINT256_ROOT = constants.MAX_UINT256_ROOT; + function makeOrder(details?: Partial): Order { + return _.assign({}, EMPTY_ORDER, details); } - return result; - } - async function testSafeGetPartialAmountCeilAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.safeGetPartialAmountCeil.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'safeGetPartialAmountCeil', - referenceSafeGetPartialAmountCeilAsync, - testSafeGetPartialAmountCeilAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); - - describe('isRoundingErrorFloor', async () => { - async function testIsRoundingErrorFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.isRoundingErrorFloor.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'isRoundingErrorFloor', - referenceIsRoundingErrorFloorAsync, - testIsRoundingErrorFloorAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); - describe('isRoundingErrorCeil', async () => { - async function testIsRoundingErrorCeilAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - return testExchange.isRoundingErrorCeil.callAsync(numerator, denominator, target); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'isRoundingErrorCeil', - referenceIsRoundingErrorCeilAsync, - testIsRoundingErrorCeilAsync, - [uint256Values, uint256Values, uint256Values], - ); - }); -}); + it('matches the output of the reference function', async () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER.times(2), + makerFee: ONE_ETHER.times(0.0023), + takerFee: ONE_ETHER.times(0.0025), + }); + const takerAssetFilledAmount = ONE_ETHER.dividedToIntegerBy(3); + const expected = ReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount); + const actual = await testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount); + expect(actual).to.deep.eq(expected); + }); -describe('Exchange core internal functions', () => { - let chainId: number; - let testExchange: TestExchangeInternalsContract; - let safeMathErrorForSendTransaction: Error | undefined; - let divisionByZeroErrorForCall: Error | undefined; - let roundingErrorForCall: Error | undefined; + it('reverts if computing `fillResults.makerAssetFilledAmount` overflows', async () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT.times(2), + takerAssetAmount: MAX_UINT256_ROOT, + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFilledAmount, + order.makerAssetAmount, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - chainId = await providerUtils.getChainIdAsync(provider); - emptyOrder.domain.chainId = chainId; - emptySignedOrder.domain.chainId = chainId; + it('reverts if computing `fillResults.makerFeePaid` overflows', async () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + makerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + makerAssetFilledAmount, + order.makerFee, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync( - artifacts.TestExchangeInternals, - provider, - txDefaults, - new BigNumber(chainId), - ); - divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); - roundingErrorForCall = new Error(RevertReason.RoundingError); - safeMathErrorForSendTransaction = safeMathErrorForCall; - divisionByZeroErrorForCall = new LibMathRevertErrors.DivisionByZeroError(); - roundingErrorForCall = new LibMathRevertErrors.RoundingError(); - }); - // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset - // the blockchain state for any tests which modify it! + it('reverts if computing `fillResults.takerFeePaid` overflows', async () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + takerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFilledAmount, + order.takerFee, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - 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.multipliedBy(target); - const remainder = product.mod(denominator); - const remainderTimes1000 = remainder.multipliedBy('1000'); - const isError = remainderTimes1000.gte(product); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - if (remainderTimes1000.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - return isError; - } + it('reverts if `order.makerAssetAmount` is 0', async () => { + const order = makeOrder({ + makerAssetAmount: constants.ZERO_AMOUNT, + takerAssetAmount: ONE_ETHER, + }); + const takerAssetFilledAmount = ONE_ETHER; + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - async function referenceSafeGetPartialAmountFloorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - if (denominator.eq(0)) { - throw divisionByZeroErrorForCall; - } - const isRoundingError = await referenceIsRoundingErrorFloorAsync(numerator, denominator, target); - if (isRoundingError) { - throw roundingErrorForCall; - } - const product = numerator.multipliedBy(target); - if (product.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - return product.dividedToIntegerBy(denominator); - } + it('reverts if `order.takerAssetAmount` is 0', async () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: constants.ZERO_AMOUNT, + }); + const takerAssetFilledAmount = ONE_ETHER; + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - describe('addFillResults', async () => { - function makeFillResults(value: BigNumber): FillResults { - return { - makerAssetFilledAmount: value, - takerAssetFilledAmount: value, - makerFeePaid: value, - takerFeePaid: value, - }; - } - async function referenceAddFillResultsAsync( - totalValue: BigNumber, - singleValue: BigNumber, - ): Promise { - // Note(albrow): Here, each of totalFillResults and - // singleFillResults will consist of fields with the same values. - // This should be safe because none of the fields in a given - // FillResults are ever used together in a mathemetical operation. - // They are only used with the corresponding field from *the other* - // FillResults, which are different. - const totalFillResults = makeFillResults(totalValue); - const singleFillResults = makeFillResults(singleValue); - // HACK(albrow): _.mergeWith mutates the first argument! To - // workaround this we use _.cloneDeep. - return _.mergeWith( - _.cloneDeep(totalFillResults), - singleFillResults, - (totalVal: BigNumber, singleVal: BigNumber) => { - const newTotal = totalVal.plus(singleVal); - if (newTotal.isGreaterThan(MAX_UINT256)) { - throw safeMathErrorForCall; - } - return newTotal; - }, - ); - } - async function testAddFillResultsAsync(totalValue: BigNumber, singleValue: BigNumber): Promise { - const totalFillResults = makeFillResults(totalValue); - const singleFillResults = makeFillResults(singleValue); - return testExchange.addFillResults.callAsync(totalFillResults, singleFillResults); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'addFillResults', - referenceAddFillResultsAsync, - testAddFillResultsAsync, - [uint256Values, uint256Values], - ); - }); + it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', async () => { + const order = makeOrder({ + makerAssetAmount: new BigNumber(100), + takerAssetAmount: ONE_ETHER, + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const expectedError = new LibMathRevertErrors.RoundingError( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); - describe('calculateFillResults', async () => { - function makeOrder( - makerAssetAmount: BigNumber, - takerAssetAmount: BigNumber, - makerFee: BigNumber, - takerFee: BigNumber, - ): Order { - return { - ...emptyOrder, - makerAssetAmount, - takerAssetAmount, - makerFee, - takerFee, - }; - } - async function referenceCalculateFillResultsAsync( - orderTakerAssetAmount: BigNumber, - takerAssetFilledAmount: BigNumber, - otherAmount: BigNumber, - ): Promise { - // Note(albrow): Here we are re-using the same value (otherAmount) - // for order.makerAssetAmount, order.makerFee, and order.takerFee. - // This should be safe because they are never used with each other - // 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, - takerAssetFilledAmount, - makerFeePaid: await referenceSafeGetPartialAmountFloorAsync( + it('reverts if there is a rounding error computing `makerFeePaid`', async () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER, + makerFee: new BigNumber(100), + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new LibMathRevertErrors.RoundingError( makerAssetFilledAmount, - orderMakerAssetAmount, - otherAmount, - ), - takerFeePaid: await referenceSafeGetPartialAmountFloorAsync( + order.makerAssetAmount, + order.makerFee, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); + + it('reverts if there is a rounding error computing `takerFeePaid`', async () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER, + takerFee: new BigNumber(100), + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( takerAssetFilledAmount, - orderTakerAssetAmount, - otherAmount, - ), - }; - } - async function testCalculateFillResultsAsync( - orderTakerAssetAmount: BigNumber, - takerAssetFilledAmount: BigNumber, - otherAmount: BigNumber, - ): Promise { - const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount); - return testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount); - } - await testCombinatoriallyWithReferenceFuncAsync( - 'calculateFillResults', - referenceCalculateFillResultsAsync, - testCalculateFillResultsAsync, - [uint256Values, uint256Values, uint256Values], - ); + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new LibMathRevertErrors.RoundingError( + makerAssetFilledAmount, + order.makerAssetAmount, + order.takerFee, + ); + return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith( + expectedError, + ); + }); + }); }); - describe('updateFilledState', async () => { - // Note(albrow): Since updateFilledState modifies the state by calling - // sendTransaction, we must reset the state after each test. - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - async function referenceUpdateFilledStateAsync( - takerAssetFilledAmount: BigNumber, - orderTakerAssetFilledAmount: BigNumber, - // tslint:disable-next-line:no-unused-variable - orderHash: string, - ): Promise { - const totalFilledAmount = takerAssetFilledAmount.plus(orderTakerAssetFilledAmount); - if (totalFilledAmount.isGreaterThan(MAX_UINT256)) { - // FIXME throw safeMathErrorForSendTransaction(takerAssetFilledAmount, orderTakerAssetFilledAmount); - throw safeMathErrorForSendTransaction; - } - return totalFilledAmount; + blockchainTests.resets('updateFilledState', async () => { + const ORDER_DEFAULTS = { + senderAddress: randomAddress(), + makerAddress: randomAddress(), + takerAddress: randomAddress(), + makerFee: ONE_ETHER.times(0.001), + takerFee: ONE_ETHER.times(0.003), + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER.times(0.5), + makerAssetData: randomAssetData(), + takerAssetData: randomAssetData(), + makerFeeAssetData: randomAssetData(), + takerFeeAssetData: randomAssetData(), + salt: new BigNumber(_.random(0, 1e8)), + feeRecipientAddress: randomAddress(), + expirationTimeSeconds: new BigNumber(_.random(0, 1e8)), + }; + + function makeOrder(details?: Partial): Order { + return _.assign({}, ORDER_DEFAULTS, details); } + async function testUpdateFilledStateAsync( - takerAssetFilledAmount: BigNumber, + order: Order, orderTakerAssetFilledAmount: BigNumber, - orderHash: string, - ): Promise { - const fillResults = { - makerAssetFilledAmount: new BigNumber(0), - takerAssetFilledAmount, - makerFeePaid: new BigNumber(0), - takerFeePaid: new BigNumber(0), - }; - await web3Wrapper.awaitTransactionSuccessAsync( - await testExchange.updateFilledState.sendTransactionAsync( - emptySignedOrder, - constants.NULL_ADDRESS, + takerAddress: string, + takerAssetFillAmount: BigNumber, + ): Promise { + const orderHash = randomHash(); + const fillResults = ReferenceFunctions.calculateFillResults(order, takerAssetFillAmount); + const expectedFilledState = orderTakerAssetFilledAmount.plus(takerAssetFillAmount); + // CAll `testUpdateFilledState()`, which will set the `filled` + // state for this order to `orderTakerAssetFilledAmount` before + // calling `_updateFilledState()`. + const receipt = await logDecoder.getTxWithDecodedLogsAsync( + await testExchange.testUpdateFilledState.sendTransactionAsync( + order, + takerAddress, orderHash, orderTakerAssetFilledAmount, fillResults, ), - constants.AWAIT_TRANSACTION_MINED_MS, ); - return testExchange.filled.callAsync(orderHash); + // Grab the new `filled` state for this order. + const actualFilledState = await testExchange.filled.callAsync(orderHash); + // Assert the `filled` state for this order. + expect(actualFilledState).to.bignumber.eq(expectedFilledState); + // Assert the logs. + // tslint:disable-next-line: no-unnecessary-type-assertion + const fillEvent = receipt.logs[0] as LogWithDecodedArgs; + expect(fillEvent.event).to.eq('Fill'); + expect(fillEvent.args.makerAddress).to.eq(order.makerAddress); + expect(fillEvent.args.feeRecipientAddress).to.eq(order.feeRecipientAddress); + expect(fillEvent.args.makerAssetData).to.eq(order.makerAssetData); + expect(fillEvent.args.takerAssetData).to.eq(order.takerAssetData); + expect(fillEvent.args.makerFeeAssetData).to.eq(order.makerFeeAssetData); + expect(fillEvent.args.takerFeeAssetData).to.eq(order.takerFeeAssetData); + expect(fillEvent.args.makerAssetFilledAmount).to.bignumber.eq(fillResults.makerAssetFilledAmount); + expect(fillEvent.args.takerAssetFilledAmount).to.bignumber.eq(fillResults.takerAssetFilledAmount); + expect(fillEvent.args.makerFeePaid).to.bignumber.eq(fillResults.makerFeePaid); + expect(fillEvent.args.takerFeePaid).to.bignumber.eq(fillResults.takerFeePaid); + expect(fillEvent.args.takerAddress).to.eq(takerAddress); + expect(fillEvent.args.senderAddress).to.eq(senderAddress); + expect(fillEvent.args.orderHash).to.eq(orderHash); } - await testCombinatoriallyWithReferenceFuncAsync( - 'updateFilledState', - referenceUpdateFilledStateAsync, - testUpdateFilledStateAsync, - [uint256Values, uint256Values, bytes32Values], - ); + + it('emits a `Fill` event and updates `filled` state correctly', async () => { + const order = makeOrder(); + return testUpdateFilledStateAsync( + order, + order.takerAssetAmount.times(0.1), + randomAddress(), + order.takerAssetAmount.times(0.25), + ); + }); + + it('reverts if `leftOrderTakerAssetFilledAmount + fillResults.takerAssetFilledAmount` overflows', async () => { + const order = makeOrder(); + const orderTakerAssetFilledAmount = constants.MAX_UINT256.dividedToIntegerBy(2); + const takerAssetFillAmount = constants.MAX_UINT256.dividedToIntegerBy(2).plus(2); + const fillResults = { + makerAssetFilledAmount: constants.ZERO_AMOUNT, + takerAssetFilledAmount: takerAssetFillAmount, + makerFeePaid: constants.ZERO_AMOUNT, + takerFeePaid: constants.ZERO_AMOUNT, + }; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + orderTakerAssetFilledAmount, + takerAssetFillAmount, + ); + return expect( + testExchange.testUpdateFilledState.awaitTransactionSuccessAsync( + order, + randomAddress(), + randomHash(), + orderTakerAssetFilledAmount, + fillResults, + ), + ).to.revertWith(expectedError); + }); + }); + + blockchainTests('settleOrder', () => { + const DEFAULT_ORDER = { + senderAddress: randomAddress(), + makerAddress: randomAddress(), + takerAddress: randomAddress(), + makerFee: ONE_ETHER.times(0.001), + takerFee: ONE_ETHER.times(0.003), + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER.times(0.5), + makerAssetData: randomAssetData(), + takerAssetData: randomAssetData(), + makerFeeAssetData: randomAssetData(), + takerFeeAssetData: randomAssetData(), + salt: new BigNumber(_.random(0, 1e8)), + feeRecipientAddress: randomAddress(), + expirationTimeSeconds: new BigNumber(_.random(0, 1e8)), + }; + + it('calls `_dispatchTransferFrom()` in the right order with the correct arguments', async () => { + const order = DEFAULT_ORDER; + const orderHash = randomHash(); + const takerAddress = randomAddress(); + const fillResults = { + makerAssetFilledAmount: ONE_ETHER.times(2), + takerAssetFilledAmount: ONE_ETHER.times(10), + makerFeePaid: ONE_ETHER.times(0.01), + takerFeePaid: ONE_ETHER.times(0.025), + }; + const receipt = await logDecoder.getTxWithDecodedLogsAsync( + await testExchange.settleOrder.sendTransactionAsync(orderHash, order, takerAddress, fillResults), + ); + const logs = receipt.logs as Array< + LogWithDecodedArgs + >; + expect(logs.length === 4); + expect(_.every(logs, log => log.event === 'DispatchTransferFromCalled')).to.be.true(); + // taker -> maker + expect(logs[0].args.orderHash).to.eq(orderHash); + expect(logs[0].args.assetData).to.eq(order.takerAssetData); + expect(logs[0].args.from).to.eq(takerAddress); + expect(logs[0].args.to).to.eq(order.makerAddress); + expect(logs[0].args.amount).to.bignumber.eq(fillResults.takerAssetFilledAmount); + // maker -> taker + expect(logs[1].args.orderHash).to.eq(orderHash); + expect(logs[1].args.assetData).to.eq(order.makerAssetData); + expect(logs[1].args.from).to.eq(order.makerAddress); + expect(logs[1].args.to).to.eq(takerAddress); + expect(logs[1].args.amount).to.bignumber.eq(fillResults.makerAssetFilledAmount); + // taker fee -> feeRecipient + expect(logs[2].args.orderHash).to.eq(orderHash); + expect(logs[2].args.assetData).to.eq(order.takerFeeAssetData); + expect(logs[2].args.from).to.eq(takerAddress); + expect(logs[2].args.to).to.eq(order.feeRecipientAddress); + expect(logs[2].args.amount).to.bignumber.eq(fillResults.takerFeePaid); + // maker fee -> feeRecipient + expect(logs[3].args.orderHash).to.eq(orderHash); + expect(logs[3].args.assetData).to.eq(order.makerFeeAssetData); + expect(logs[3].args.from).to.eq(order.makerAddress); + expect(logs[3].args.to).to.eq(order.feeRecipientAddress); + expect(logs[3].args.amount).to.bignumber.eq(fillResults.makerFeePaid); + }); }); }); // tslint:disable-line:max-file-line-count diff --git a/contracts/exchange/test/isolated_fill_order.ts b/contracts/exchange/test/isolated_fill_order.ts new file mode 100644 index 0000000000..31300fcac1 --- /dev/null +++ b/contracts/exchange/test/isolated_fill_order.ts @@ -0,0 +1,583 @@ +import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs'; +import { blockchainTests, constants, expect, hexRandom } from '@0x/contracts-test-utils'; +import { ExchangeRevertErrors, LibMathRevertErrors } from '@0x/order-utils'; +import { FillResults, OrderInfo, OrderStatus, SignatureType } from '@0x/types'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import * as _ from 'lodash'; + +import { calculateFillResults } from '../src/reference_functions'; + +import { + AssetBalances, + createBadAssetData, + createBadSignature, + createGoodAssetData, + createGoodSignature, + IsolatedExchangeWrapper, + Order, +} from './utils/isolated_exchange_wrapper'; + +blockchainTests('Isolated fillOrder() tests', env => { + const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH); + const getCurrentTime = () => Math.floor(_.now() / 1000); + const { ZERO_AMOUNT, ONE_ETHER, MAX_UINT256_ROOT } = constants; + const ONE_DAY = 60 * 60 * 24; + const TOMORROW = getCurrentTime() + ONE_DAY; + const DEFAULT_ORDER: Order = { + senderAddress: constants.NULL_ADDRESS, + makerAddress: randomAddress(), + takerAddress: constants.NULL_ADDRESS, + feeRecipientAddress: randomAddress(), + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER.times(2), + makerFee: ONE_ETHER.times(0.0015), + takerFee: ONE_ETHER.times(0.0025), + salt: ZERO_AMOUNT, + expirationTimeSeconds: new BigNumber(TOMORROW), + makerAssetData: createGoodAssetData(), + takerAssetData: createGoodAssetData(), + makerFeeAssetData: createGoodAssetData(), + takerFeeAssetData: createGoodAssetData(), + }; + let takerAddress: string; + let notTakerAddress: string; + let exchange: IsolatedExchangeWrapper; + let nextSaltValue = 1; + + before(async () => { + [takerAddress, notTakerAddress] = await env.getAccountAddressesAsync(); + exchange = await IsolatedExchangeWrapper.deployAsync(env.web3Wrapper, { + ...env.txDefaults, + from: takerAddress, + }); + }); + + function createOrder(details: Partial = {}): Order { + return { + ...DEFAULT_ORDER, + salt: new BigNumber(nextSaltValue++), + ...details, + }; + } + + interface FillOrderAndAssertResultsResults { + fillResults: FillResults; + orderInfo: OrderInfo; + } + + async function fillOrderAndAssertResultsAsync( + order: Order, + takerAssetFillAmount: BigNumber, + signature?: string, + ): Promise { + const orderInfo = await exchange.getOrderInfoAsync(order); + const efr = calculateExpectedFillResults(order, orderInfo, takerAssetFillAmount); + const eoi = calculateExpectedOrderInfo(order, orderInfo, efr); + const efb = calculateExpectedFillBalances(order, efr); + // Fill the order. + const fillResults = await exchange.fillOrderAsync(order, takerAssetFillAmount, signature); + const newOrderInfo = await exchange.getOrderInfoAsync(order); + // Check returned fillResults. + expect(fillResults.makerAssetFilledAmount).to.bignumber.eq(efr.makerAssetFilledAmount); + expect(fillResults.takerAssetFilledAmount).to.bignumber.eq(efr.takerAssetFilledAmount); + expect(fillResults.makerFeePaid).to.bignumber.eq(efr.makerFeePaid); + expect(fillResults.takerFeePaid).to.bignumber.eq(efr.takerFeePaid); + // Check balances. + for (const assetData of Object.keys(efb)) { + for (const address of Object.keys(efb[assetData])) { + expect( + exchange.getBalanceChange(assetData, address), + `checking balance of assetData: ${assetData}, address: ${address}`, + ).to.bignumber.eq(efb[assetData][address]); + } + } + // Check order info. + expect(newOrderInfo.orderStatus).to.eq(eoi.orderStatus); + expect(newOrderInfo.orderTakerAssetFilledAmount).to.bignumber.eq(eoi.orderTakerAssetFilledAmount); + // Check that there wasn't an overfill. + expect(newOrderInfo.orderTakerAssetFilledAmount.lte(order.takerAssetAmount), 'checking for overfill').to.be.ok( + '', + ); + return { + fillResults, + orderInfo: newOrderInfo, + }; + } + + function calculateExpectedFillResults( + order: Order, + orderInfo: OrderInfo, + takerAssetFillAmount: BigNumber, + ): FillResults { + const remainingTakerAssetAmount = order.takerAssetAmount.minus(orderInfo.orderTakerAssetFilledAmount); + return calculateFillResults(order, BigNumber.min(takerAssetFillAmount, remainingTakerAssetAmount)); + } + + function calculateExpectedOrderInfo(order: Order, orderInfo: OrderInfo, fillResults: FillResults): OrderInfo { + const orderTakerAssetFilledAmount = orderInfo.orderTakerAssetFilledAmount.plus( + fillResults.takerAssetFilledAmount, + ); + const orderStatus = orderTakerAssetFilledAmount.gte(order.takerAssetAmount) + ? OrderStatus.FullyFilled + : OrderStatus.Fillable; + return { + orderHash: exchange.getOrderHash(order), + orderStatus, + orderTakerAssetFilledAmount, + }; + } + + function calculateExpectedFillBalances(order: Order, fillResults: FillResults): AssetBalances { + const balances: AssetBalances = {}; + const addBalance = (assetData: string, address: string, amount: BigNumber) => { + balances[assetData] = balances[assetData] || {}; + const balance = balances[assetData][address] || ZERO_AMOUNT; + balances[assetData][address] = balance.plus(amount); + }; + addBalance(order.makerAssetData, order.makerAddress, fillResults.makerAssetFilledAmount.negated()); + addBalance(order.makerAssetData, takerAddress, fillResults.makerAssetFilledAmount); + addBalance(order.takerAssetData, order.makerAddress, fillResults.takerAssetFilledAmount); + addBalance(order.takerAssetData, takerAddress, fillResults.takerAssetFilledAmount.negated()); + addBalance(order.makerFeeAssetData, order.makerAddress, fillResults.makerFeePaid.negated()); + addBalance(order.makerFeeAssetData, order.feeRecipientAddress, fillResults.makerFeePaid); + addBalance(order.takerFeeAssetData, takerAddress, fillResults.takerFeePaid.negated()); + addBalance(order.takerFeeAssetData, order.feeRecipientAddress, fillResults.takerFeePaid); + return balances; + } + + function splitAmount(total: BigNumber, n: number = 2): BigNumber[] { + const splitSize = total.dividedToIntegerBy(n); + const splits = _.times(n - 1, () => splitSize); + splits.push(total.minus(splitSize.times(n - 1))); + return splits; + } + + describe('full fills', () => { + it('can fully fill an order', async () => { + const order = createOrder(); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it("can't overfill an order", async () => { + const order = createOrder(); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, order.takerAssetAmount.times(1.01)); + expect(orderInfo.orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('no fees', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + takerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('only maker fees', async () => { + const order = createOrder({ + takerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('only taker fees', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.FullyFilled); + }); + }); + + describe('partial fills', () => { + const takerAssetFillAmount = DEFAULT_ORDER.takerAssetAmount.dividedToIntegerBy(2); + + it('can partially fill an order', async () => { + const order = createOrder(); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.Fillable); + }); + + it('no fees', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + takerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.Fillable); + }); + + it('only maker fees', async () => { + const order = createOrder({ + takerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.Fillable); + }); + + it('only taker fees', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + }); + const { orderInfo } = await fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); + expect(orderInfo.orderStatus).to.eq(OrderStatus.Fillable); + }); + }); + + describe('multiple fills', () => { + it('can fully fill an order in two fills', async () => { + const order = createOrder(); + const fillAmounts = splitAmount(order.takerAssetAmount); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('can partially fill an order in two fills', async () => { + const order = createOrder(); + const fillAmounts = splitAmount(order.takerAssetAmount.dividedToIntegerBy(2)); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.Fillable); + }); + + it("can't overfill an order in two fills", async () => { + const order = createOrder(); + const fillAmounts = splitAmount(order.takerAssetAmount); + fillAmounts[0] = fillAmounts[0].times(1.01); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('can fully fill an order with no fees in two fills', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + takerFee: ZERO_AMOUNT, + }); + const fillAmounts = splitAmount(order.takerAssetAmount); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('can fully fill an order with only maker fees in two fills', async () => { + const order = createOrder({ + takerFee: ZERO_AMOUNT, + }); + const fillAmounts = splitAmount(order.takerAssetAmount); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.FullyFilled); + }); + + it('can fully fill an order with only taker fees in two fills', async () => { + const order = createOrder({ + makerFee: ZERO_AMOUNT, + }); + const fillAmounts = splitAmount(order.takerAssetAmount); + const orderInfos = [ + (await fillOrderAndAssertResultsAsync(order, fillAmounts[0])).orderInfo, + (await fillOrderAndAssertResultsAsync(order, fillAmounts[1])).orderInfo, + ]; + expect(orderInfos[0].orderStatus).to.eq(OrderStatus.Fillable); + expect(orderInfos[1].orderStatus).to.eq(OrderStatus.FullyFilled); + }); + }); + + describe('bad fills', () => { + it("can't fill an order with zero takerAssetAmount", async () => { + const order = createOrder({ + takerAssetAmount: ZERO_AMOUNT, + }); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.InvalidTakerAssetAmount, + ); + return expect(exchange.fillOrderAsync(order, ONE_ETHER)).to.revertWith(expectedError); + }); + + it("can't fill an order with zero makerAssetAmount", async () => { + const order = createOrder({ + makerAssetAmount: ZERO_AMOUNT, + }); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.InvalidMakerAssetAmount, + ); + return expect(exchange.fillOrderAsync(order, ONE_ETHER)).to.revertWith(expectedError); + }); + + it("can't fill an order that is fully filled", async () => { + const order = createOrder(); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.FullyFilled, + ); + await exchange.fillOrderAsync(order, order.takerAssetAmount); + return expect(exchange.fillOrderAsync(order, 1)).to.revertWith(expectedError); + }); + + it("can't fill an order that is expired", async () => { + const order = createOrder({ + expirationTimeSeconds: new BigNumber(getCurrentTime() - 60), + }); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.Expired, + ); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order that is cancelled by `cancelOrder()`", async () => { + const order = createOrder({ + makerAddress: notTakerAddress, + }); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.Cancelled, + ); + await exchange.cancelOrderAsync(order, { from: notTakerAddress }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order that is cancelled by `cancelOrdersUpTo()`", async () => { + const order = createOrder({ + makerAddress: notTakerAddress, + }); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + exchange.getOrderHash(order), + OrderStatus.Cancelled, + ); + await exchange.cancelOrdersUpToAsync(order.salt, { from: notTakerAddress }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order if taker is not `takerAddress`", async () => { + const order = createOrder({ + takerAddress: randomAddress(), + }); + const expectedError = new ExchangeRevertErrors.InvalidTakerError( + exchange.getOrderHash(order), + takerAddress, + ); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order if sender is not `senderAddress`", async () => { + const order = createOrder({ + senderAddress: randomAddress(), + }); + const expectedError = new ExchangeRevertErrors.InvalidSenderError( + exchange.getOrderHash(order), + takerAddress, + ); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order with a taker amount that results in a maker asset rounding error", async () => { + const order = createOrder({ + makerAssetAmount: new BigNumber(100), + takerAssetAmount: ONE_ETHER, + }); + const takerAssetFillAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const expectedError = new LibMathRevertErrors.RoundingError( + takerAssetFillAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order with a taker amount that results in a maker fee rounding error", async () => { + const order = createOrder({ + makerAssetAmount: ONE_ETHER.times(2), + takerAssetAmount: ONE_ETHER, + makerFee: new BigNumber(100), + }); + const takerAssetFillAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const expectedError = new LibMathRevertErrors.RoundingError( + takerAssetFillAmount.times(2), + order.makerAssetAmount, + order.makerFee, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order with a taker amount that results in a taker fee rounding error", async () => { + const order = createOrder({ + makerAssetAmount: ONE_ETHER.times(2), + takerAssetAmount: ONE_ETHER, + takerFee: new BigNumber(100), + }); + const takerAssetFillAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const expectedError = new LibMathRevertErrors.RoundingError( + takerAssetFillAmount, + order.takerAssetAmount, + order.takerFee, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order that results in a `makerAssetFilledAmount` overflow.", async () => { + // All values need to be large to ensure we don't trigger a Rounding. + const order = createOrder({ + makerAssetAmount: MAX_UINT256_ROOT.times(2), + takerAssetAmount: MAX_UINT256_ROOT, + }); + const takerAssetFillAmount = MAX_UINT256_ROOT; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFillAmount, + order.makerAssetAmount, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order that results in a `makerFeePaid` overflow.", async () => { + // All values need to be large to ensure we don't trigger a Rounding. + const order = createOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + makerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFillAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFillAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + makerAssetFilledAmount, + order.makerFee, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order that results in a `takerFeePaid` overflow.", async () => { + // All values need to be large to ensure we don't trigger a Rounding. + const order = createOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + takerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFillAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFillAmount, + order.takerFee, + ); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmount)).to.revertWith(expectedError); + }); + + it("can't fill an order with a bad signature", async () => { + const order = createOrder(); + const signature = createBadSignature(); + const expectedError = new ExchangeRevertErrors.SignatureError( + ExchangeRevertErrors.SignatureErrorCode.BadSignature, + exchange.getOrderHash(order), + order.makerAddress, + signature, + ); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount, signature)).to.revertWith( + expectedError, + ); + }); + + it("can't complementary fill an order with a bad signature that is always checked", async () => { + const order = createOrder(); + const takerAssetFillAmounts = splitAmount(order.takerAssetAmount); + const goodSignature = createGoodSignature(SignatureType.Wallet); + const badSignature = createBadSignature(SignatureType.Wallet); + const expectedError = new ExchangeRevertErrors.SignatureError( + ExchangeRevertErrors.SignatureErrorCode.BadSignature, + exchange.getOrderHash(order), + order.makerAddress, + badSignature, + ); + await exchange.fillOrderAsync(order, takerAssetFillAmounts[0], goodSignature); + return expect(exchange.fillOrderAsync(order, takerAssetFillAmounts[1], badSignature)).to.revertWith( + expectedError, + ); + }); + + const TRANSFER_ERROR = 'TRANSFER_FAILED'; + + it("can't fill an order with a maker asset that fails to transfer", async () => { + const order = createOrder({ + makerAssetData: createBadAssetData(), + }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(TRANSFER_ERROR); + }); + + it("can't fill an order with a taker asset that fails to transfer", async () => { + const order = createOrder({ + takerAssetData: createBadAssetData(), + }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(TRANSFER_ERROR); + }); + + it("can't fill an order with a maker fee asset that fails to transfer", async () => { + const order = createOrder({ + makerFeeAssetData: createBadAssetData(), + }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(TRANSFER_ERROR); + }); + + it("can't fill an order with a taker fee asset that fails to transfer", async () => { + const order = createOrder({ + takerFeeAssetData: createBadAssetData(), + }); + return expect(exchange.fillOrderAsync(order, order.takerAssetAmount)).to.revertWith(TRANSFER_ERROR); + }); + }); + + describe('permitted fills', () => { + it('should allow takerAssetFillAmount to be zero', async () => { + const order = createOrder(); + return fillOrderAndAssertResultsAsync(order, constants.ZERO_AMOUNT); + }); + + it('can fill an order if taker is `takerAddress`', async () => { + const order = createOrder({ + takerAddress, + }); + return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + }); + + it('can fill an order if sender is `senderAddress`', async () => { + const order = createOrder({ + senderAddress: takerAddress, + }); + return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + }); + + it('can complementary fill an order with a bad signature that is checked only once', async () => { + const order = createOrder(); + const takerAssetFillAmounts = splitAmount(order.takerAssetAmount); + const goodSignature = createGoodSignature(SignatureType.EthSign); + const badSignature = createBadSignature(SignatureType.EthSign); + await fillOrderAndAssertResultsAsync(order, takerAssetFillAmounts[0], goodSignature); + await fillOrderAndAssertResultsAsync(order, takerAssetFillAmounts[1], badSignature); + }); + }); +}); +// tslint:disable: max-file-line-count diff --git a/contracts/exchange/test/lib_exchange_rich_error_decoder.ts b/contracts/exchange/test/lib_exchange_rich_error_decoder.ts index 09aa40501a..8c3ce7e660 100644 --- a/contracts/exchange/test/lib_exchange_rich_error_decoder.ts +++ b/contracts/exchange/test/lib_exchange_rich_error_decoder.ts @@ -1,37 +1,16 @@ -import { - addressUtils, - chaiSetup, - OrderStatus, - orderUtils, - provider, - txDefaults, - web3Wrapper, -} from '@0x/contracts-test-utils'; -import { BlockchainLifecycle } from '@0x/dev-utils'; +import { addressUtils, blockchainTests, expect, hexRandom, OrderStatus, orderUtils } from '@0x/contracts-test-utils'; import { ExchangeRevertErrors, generatePseudoRandomSalt } from '@0x/order-utils'; import { RevertError } from '@0x/utils'; -import * as chai from 'chai'; -import * as crypto from 'crypto'; import * as _ from 'lodash'; import { artifacts, TestLibExchangeRichErrorDecoderContract } from '../src'; -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - -describe('LibExchangeRichErrorDecoder', () => { +blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) => { const SIGNATURE_LENGTH = 66; const ASSET_DATA_LENGTH = 36; const ERROR_DATA_LENGTH = 100; let decoder: TestLibExchangeRichErrorDecoderContract; - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); before(async () => { decoder = await TestLibExchangeRichErrorDecoderContract.deployFrom0xArtifactAsync( artifacts.TestLibExchangeRichErrorDecoder, @@ -40,11 +19,6 @@ describe('LibExchangeRichErrorDecoder', () => { ); }); - function generateRandomBytes(length: number): string { - const bytes = crypto.randomBytes(length).toString('hex'); - return `0x${bytes}`; - } - function createDecodeTest(revertType: new (...args: any[]) => RevertError, parameters: any[]): void { const revert = new revertType(...parameters); const encoded = revert.encode(); @@ -76,8 +50,8 @@ describe('LibExchangeRichErrorDecoder', () => { const orderHash = orderUtils.generatePseudoRandomOrderHash(); const signer = addressUtils.generatePseudoRandomAddress(); const validator = addressUtils.generatePseudoRandomAddress(); - const signature = generateRandomBytes(SIGNATURE_LENGTH); - const errorData = generateRandomBytes(ERROR_DATA_LENGTH); + const signature = hexRandom(SIGNATURE_LENGTH); + const errorData = hexRandom(ERROR_DATA_LENGTH); createDecodeTest(ExchangeRevertErrors.SignatureError, [errorCode, orderHash, signer, signature]); createDecodeTest(ExchangeRevertErrors.SignatureValidatorNotApprovedError, [signer, validator]); createDecodeTest(ExchangeRevertErrors.SignatureValidatorError, [ @@ -125,14 +99,14 @@ describe('LibExchangeRichErrorDecoder', () => { (() => { const errorCode = ExchangeRevertErrors.AssetProxyDispatchErrorCode.UnknownAssetProxy; const orderHash = orderUtils.generatePseudoRandomOrderHash(); - const assetData = generateRandomBytes(ASSET_DATA_LENGTH); + const assetData = hexRandom(ASSET_DATA_LENGTH); createDecodeTest(ExchangeRevertErrors.AssetProxyDispatchError, [errorCode, orderHash, assetData]); })(); (() => { const orderHash = orderUtils.generatePseudoRandomOrderHash(); - const assetData = generateRandomBytes(ASSET_DATA_LENGTH); - const errorData = generateRandomBytes(ERROR_DATA_LENGTH); + const assetData = hexRandom(ASSET_DATA_LENGTH); + const errorData = hexRandom(ERROR_DATA_LENGTH); createDecodeTest(ExchangeRevertErrors.AssetProxyTransferError, [orderHash, assetData, errorData]); })(); @@ -151,13 +125,13 @@ describe('LibExchangeRichErrorDecoder', () => { (() => { const transactionHash = orderUtils.generatePseudoRandomOrderHash(); const signer = addressUtils.generatePseudoRandomAddress(); - const signature = generateRandomBytes(SIGNATURE_LENGTH); + const signature = hexRandom(SIGNATURE_LENGTH); createDecodeTest(ExchangeRevertErrors.TransactionSignatureError, [transactionHash, signer, signature]); })(); (() => { const transactionHash = orderUtils.generatePseudoRandomOrderHash(); - const errorData = generateRandomBytes(ERROR_DATA_LENGTH); + const errorData = hexRandom(ERROR_DATA_LENGTH); createDecodeTest(ExchangeRevertErrors.TransactionExecutionError, [transactionHash, errorData]); })(); diff --git a/contracts/exchange/test/match_orders.ts b/contracts/exchange/test/match_orders.ts index 74edbd9acd..4a908aa045 100644 --- a/contracts/exchange/test/match_orders.ts +++ b/contracts/exchange/test/match_orders.ts @@ -11,6 +11,7 @@ import { import { ERC1155Contract as ERC1155TokenContract, Erc1155Wrapper as ERC1155Wrapper } from '@0x/contracts-erc1155'; import { DummyERC20TokenContract } from '@0x/contracts-erc20'; import { DummyERC721TokenContract } from '@0x/contracts-erc721'; +import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs'; import { chaiSetup, constants, @@ -34,7 +35,6 @@ import { ExchangeContract, ExchangeWrapper, ReentrantERC20TokenContract, - TestExchangeMathContract, } from '../src'; import { MatchOrderTester, TokenBalances } from './utils/match_order_tester'; @@ -42,6 +42,7 @@ import { MatchOrderTester, TokenBalances } from './utils/match_order_tester'; const ZERO = new BigNumber(0); const ONE = new BigNumber(1); const TWO = new BigNumber(2); +const { isRoundingErrorCeil, isRoundingErrorFloor } = LibReferenceFunctions; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); chaiSetup.configure(); @@ -88,8 +89,6 @@ describe('matchOrders', () => { let matchOrderTester: MatchOrderTester; - let testExchangeMath: TestExchangeMathContract; - before(async () => { await blockchainLifecycle.startAsync(); }); @@ -240,11 +239,6 @@ describe('matchOrders', () => { orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); - testExchangeMath = await TestExchangeMathContract.deployFrom0xArtifactAsync( - artifacts.TestExchangeMath, - provider, - txDefaults, - ); // Create match order tester matchOrderTester = new MatchOrderTester(exchangeWrapper, erc20Wrapper, erc721Wrapper, erc1155ProxyWrapper); tokenBalances = await matchOrderTester.getBalancesAsync(); @@ -276,18 +270,10 @@ describe('matchOrders', () => { const numerator = signedOrderLeft.makerAssetAmount; const denominator = signedOrderLeft.takerAssetAmount; const target = signedOrderRight.makerAssetAmount; - const isRoundingErrorCeil = await testExchangeMath.isRoundingErrorCeil.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorCeil).to.be.true(); - const isRoundingErrorFloor = await testExchangeMath.isRoundingErrorFloor.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorFloor).to.be.false(); + const _isRoundingErrorCeil = isRoundingErrorCeil(numerator, denominator, target); + expect(_isRoundingErrorCeil).to.be.true(); + const _isRoundingErrorFloor = isRoundingErrorFloor(numerator, denominator, target); + expect(_isRoundingErrorFloor).to.be.false(); // Match signedOrderLeft with signedOrderRight // Note that the left maker received a slightly better sell price. // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. @@ -342,20 +328,11 @@ describe('matchOrders', () => { const numerator = signedOrderRight.takerAssetAmount; const denominator = signedOrderRight.makerAssetAmount; const target = signedOrderLeft.takerAssetAmount; - const isRoundingErrorFloor = await testExchangeMath.isRoundingErrorFloor.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorFloor).to.be.true(); - const isRoundingErrorCeil = await testExchangeMath.isRoundingErrorCeil.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorCeil).to.be.false(); - // Match signedOrderLeft with signedOrderRight - // Note that the right maker received a slightly better purchase price. + const _isRoundingErrorFloor = isRoundingErrorFloor(numerator, denominator, target); + expect(_isRoundingErrorFloor).to.be.true(); + const _isRoundingErrorCeil = isRoundingErrorCeil(numerator, denominator, target); + expect(_isRoundingErrorCeil).to.be.false(); + // Match signedOrderLeft isRoundingErrorFloor right maker received a slightly better purchase price. // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. // Because the right maker received a slightly more favorable buy price, the fee // paid by the right taker is slightly higher than that paid by the right maker. @@ -1421,18 +1398,10 @@ describe('matchOrders', () => { const numerator = signedOrderLeft.makerAssetAmount; const denominator = signedOrderLeft.takerAssetAmount; const target = signedOrderRight.makerAssetAmount; - const isRoundingErrorCeil = await testExchangeMath.isRoundingErrorCeil.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorCeil).to.be.true(); - const isRoundingErrorFloor = await testExchangeMath.isRoundingErrorFloor.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorFloor).to.be.false(); + const _isRoundingErrorCeil = isRoundingErrorCeil(numerator, denominator, target); + expect(_isRoundingErrorCeil).to.be.true(); + const _isRoundingErrorFloor = isRoundingErrorFloor(numerator, denominator, target); + expect(_isRoundingErrorFloor).to.be.false(); // Match signedOrderLeft with signedOrderRight // Note that the left maker received a slightly better sell price. // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. @@ -1487,18 +1456,10 @@ describe('matchOrders', () => { const numerator = signedOrderRight.makerAssetAmount; const denominator = signedOrderRight.takerAssetAmount; const target = signedOrderLeft.makerAssetAmount; - const isRoundingErrorCeil = await testExchangeMath.isRoundingErrorCeil.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorCeil).to.be.false(); - const isRoundingErrorFloor = await testExchangeMath.isRoundingErrorFloor.callAsync( - numerator, - denominator, - target, - ); - expect(isRoundingErrorFloor).to.be.false(); + const _isRoundingErrorCeil = isRoundingErrorCeil(numerator, denominator, target); + expect(_isRoundingErrorCeil).to.be.false(); + const _isRoundingErrorFloor = isRoundingErrorFloor(numerator, denominator, target); + expect(_isRoundingErrorFloor).to.be.false(); // Match signedOrderLeft with signedOrderRight // Note that the right maker received a slightly better purchase price. // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults. diff --git a/contracts/exchange/test/reference_functions.ts b/contracts/exchange/test/reference_functions.ts new file mode 100644 index 0000000000..5e961d51c6 --- /dev/null +++ b/contracts/exchange/test/reference_functions.ts @@ -0,0 +1,162 @@ +import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs'; +import { constants, describe, expect } from '@0x/contracts-test-utils'; +import { LibMathRevertErrors } from '@0x/order-utils'; +import { OrderWithoutDomain as Order } from '@0x/types'; +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import * as _ from 'lodash'; + +import { calculateFillResults } from '../src/reference_functions'; + +describe('Reference functions', () => { + const ONE_ETHER = constants.ONE_ETHER; + const EMPTY_ORDER: Order = { + senderAddress: constants.NULL_ADDRESS, + makerAddress: constants.NULL_ADDRESS, + takerAddress: constants.NULL_ADDRESS, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + makerAssetAmount: constants.ZERO_AMOUNT, + takerAssetAmount: constants.ZERO_AMOUNT, + makerAssetData: constants.NULL_BYTES, + takerAssetData: constants.NULL_BYTES, + makerFeeAssetData: constants.NULL_BYTES, + takerFeeAssetData: constants.NULL_BYTES, + salt: constants.ZERO_AMOUNT, + feeRecipientAddress: constants.NULL_ADDRESS, + expirationTimeSeconds: constants.ZERO_AMOUNT, + }; + + describe('calculateFillResults', () => { + const MAX_UINT256_ROOT = constants.MAX_UINT256_ROOT; + function makeOrder(details?: Partial): Order { + return _.assign({}, EMPTY_ORDER, details); + } + + it('reverts if computing `fillResults.makerAssetFilledAmount` overflows', () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT.times(2), + takerAssetAmount: MAX_UINT256_ROOT, + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFilledAmount, + order.makerAssetAmount, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if computing `fillResults.makerFeePaid` overflows', () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + makerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + makerAssetFilledAmount, + order.makerFee, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if computing `fillResults.takerFeePaid` overflows', () => { + // All values need to be large to ensure we don't trigger a RoundingError. + const order = makeOrder({ + makerAssetAmount: MAX_UINT256_ROOT, + takerAssetAmount: MAX_UINT256_ROOT, + takerFee: MAX_UINT256_ROOT.times(11), + }); + const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + takerAssetFilledAmount, + order.takerFee, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if `order.makerAssetAmount` is 0', () => { + const order = makeOrder({ + makerAssetAmount: constants.ZERO_AMOUNT, + takerAssetAmount: ONE_ETHER, + }); + const takerAssetFilledAmount = ONE_ETHER; + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if `order.takerAssetAmount` is 0', () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: constants.ZERO_AMOUNT, + }); + const takerAssetFilledAmount = ONE_ETHER; + const expectedError = new LibMathRevertErrors.DivisionByZeroError(); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', () => { + const order = makeOrder({ + makerAssetAmount: new BigNumber(100), + takerAssetAmount: ONE_ETHER, + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const expectedError = new LibMathRevertErrors.RoundingError( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if there is a rounding error computing `makerFeePaid`', () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER, + makerFee: new BigNumber(100), + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new LibMathRevertErrors.RoundingError( + makerAssetFilledAmount, + order.makerAssetAmount, + order.makerFee, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + + it('reverts if there is a rounding error computing `takerFeePaid`', () => { + const order = makeOrder({ + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER, + takerFee: new BigNumber(100), + }); + const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3); + const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor( + takerAssetFilledAmount, + order.takerAssetAmount, + order.makerAssetAmount, + ); + const expectedError = new LibMathRevertErrors.RoundingError( + makerAssetFilledAmount, + order.makerAssetAmount, + order.takerFee, + ); + return expect(() => calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message); + }); + }); +}); +// tslint:disable-line:max-file-line-count diff --git a/contracts/exchange/test/signature_validator.ts b/contracts/exchange/test/signature_validator.ts index ba5b84822d..3a19671a2d 100644 --- a/contracts/exchange/test/signature_validator.ts +++ b/contracts/exchange/test/signature_validator.ts @@ -1,17 +1,15 @@ import { addressUtils, - chaiSetup, + blockchainTests, constants, + expect, hexConcat, + hexRandom, LogDecoder, OrderFactory, orderUtils, - provider, TransactionFactory, - txDefaults, - web3Wrapper, } from '@0x/contracts-test-utils'; -import { BlockchainLifecycle } from '@0x/dev-utils'; import { assetDataUtils, ExchangeRevertErrors, @@ -20,9 +18,7 @@ import { transactionHashUtils, } from '@0x/order-utils'; import { SignatureType, SignedOrder, SignedZeroExTransaction } from '@0x/types'; -import { BigNumber, providerUtils, StringRevertError } from '@0x/utils'; -import * as chai from 'chai'; -import * as crypto from 'crypto'; +import { BigNumber, StringRevertError } from '@0x/utils'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -35,11 +31,8 @@ import { import { ValidatorWalletAction, ValidatorWalletDataType } from './utils'; -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); // tslint:disable:no-unnecessary-type-assertion -describe('MixinSignatureValidator', () => { +blockchainTests.resets('MixinSignatureValidator', env => { let chainId: number; let signatureValidator: TestSignatureValidatorContract; let validatorWallet: TestValidatorWalletContract; @@ -49,26 +42,20 @@ describe('MixinSignatureValidator', () => { let notSignerAddress: string; before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - chainId = await providerUtils.getChainIdAsync(provider); - const accounts = await web3Wrapper.getAvailableAddressesAsync(); + chainId = await env.getChainIdAsync(); + const accounts = await env.getAccountAddressesAsync(); signerAddress = accounts[0]; notSignerAddress = accounts[1]; signatureValidator = await TestSignatureValidatorContract.deployFrom0xArtifactAsync( artifacts.TestSignatureValidator, - provider, - txDefaults, + env.provider, + env.txDefaults, new BigNumber(chainId), ); validatorWallet = await TestValidatorWalletContract.deployFrom0xArtifactAsync( artifacts.TestValidatorWallet, - provider, - txDefaults, + env.provider, + env.txDefaults, signatureValidator.address, ); validatorWalletRevertReason = await validatorWallet.REVERT_REASON.callAsync(); @@ -87,16 +74,8 @@ describe('MixinSignatureValidator', () => { signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(signerAddress)]; }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - const SIGNATURE_LENGTH = 65; - const generateRandomBytes = (count: number): string => ethUtil.bufferToHex(crypto.randomBytes(count)); - const generateRandomSignature = (): string => generateRandomBytes(SIGNATURE_LENGTH); + const generateRandomSignature = (): string => hexRandom(SIGNATURE_LENGTH); const hashBytes = (bytesHex: string): string => ethUtil.bufferToHex(ethUtil.sha3(ethUtil.toBuffer(bytesHex))); const signDataHex = (dataHex: string, privateKey: Buffer): string => { const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(dataHex), signerPrivateKey); @@ -609,7 +588,7 @@ describe('MixinSignatureValidator', () => { // We don't actually do anything with the transaction so we can just // fill it with random data. signedTransaction = await transactionFactory.newSignedTransactionAsync({ - data: generateRandomBytes(TRANSACTION_DATA_LENGTH), + data: hexRandom(TRANSACTION_DATA_LENGTH), }); }); @@ -807,7 +786,7 @@ describe('MixinSignatureValidator', () => { let signatureValidatorLogDecoder: LogDecoder; before(async () => { - signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, artifacts); + signatureValidatorLogDecoder = new LogDecoder(env.web3Wrapper, artifacts); }); it('should emit a SignatureValidatorApprovalSet with correct args when a validator is approved', async () => { diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index d9073f71e8..78ca9540e8 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -4,7 +4,6 @@ import { DummyERC20TokenContract } from '@0x/contracts-erc20'; import { chaiSetup, constants, - FillResults, getLatestBlockTimestampAsync, LogDecoder, OrderFactory, @@ -21,7 +20,7 @@ import { orderHashUtils, transactionHashUtils, } from '@0x/order-utils'; -import { EIP712DomainWithDefaultSchema, OrderStatus, RevertReason } from '@0x/types'; +import { EIP712DomainWithDefaultSchema, FillResults, OrderStatus, RevertReason } from '@0x/types'; import { AbiEncoder, BigNumber, providerUtils } from '@0x/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs, MethodAbi } from 'ethereum-types'; diff --git a/contracts/exchange/test/utils/exchange_wrapper.ts b/contracts/exchange/test/utils/exchange_wrapper.ts index fc07fc1534..da244e35e3 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -1,17 +1,15 @@ import { artifacts as erc1155Artifacts } from '@0x/contracts-erc1155'; import { artifacts as erc20Artifacts } from '@0x/contracts-erc20'; import { artifacts as erc721Artifacts } from '@0x/contracts-erc721'; +import { BatchMatchOrder, LogDecoder, orderUtils, Web3ProviderEngine } from '@0x/contracts-test-utils'; import { BatchMatchedFillResults, - BatchMatchOrder, FillResults, - LogDecoder, MatchedFillResults, OrderInfo, - orderUtils, - Web3ProviderEngine, -} from '@0x/contracts-test-utils'; -import { SignedOrder, SignedZeroExTransaction } from '@0x/types'; + SignedOrder, + SignedZeroExTransaction, +} from '@0x/types'; import { AbiEncoder, BigNumber } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import { MethodAbi, TransactionReceiptWithDecodedLogs, ZeroExProvider } from 'ethereum-types'; diff --git a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts index f14aa630d0..de2b4adaba 100644 --- a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts +++ b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts @@ -5,12 +5,11 @@ import { ERC721Wrapper, MultiAssetProxyContract, } from '@0x/contracts-asset-proxy'; -import { chaiSetup, constants, FillResults, orderUtils, signingUtils } from '@0x/contracts-test-utils'; +import { constants, expect, orderUtils, signingUtils } from '@0x/contracts-test-utils'; import { BalanceAndProxyAllowanceLazyStore, ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils'; -import { Order, SignatureType, SignedOrder } from '@0x/types'; +import { FillResults, Order, SignatureType, SignedOrder } from '@0x/types'; import { BigNumber, errorUtils, providerUtils, RevertError, StringRevertError } from '@0x/utils'; import { SupportedProvider, Web3Wrapper } from '@0x/web3-wrapper'; -import * as chai from 'chai'; import { LogWithDecodedArgs, TxData } from 'ethereum-types'; import * as _ from 'lodash'; import 'make-promises-safe'; @@ -36,9 +35,6 @@ import { FillOrderError, FillOrderSimulator } from './fill_order_simulator'; import { OrderFactoryFromScenario } from './order_factory_from_scenario'; import { SimpleAssetBalanceAndProxyAllowanceFetcher } from './simple_asset_balance_and_proxy_allowance_fetcher'; -chaiSetup.configure(); -const expect = chai.expect; - const EMPTY_FILL_RESULTS = { takerAssetFilledAmount: constants.ZERO_AMOUNT, makerAssetFilledAmount: constants.ZERO_AMOUNT, diff --git a/contracts/exchange/test/utils/fill_order_scenarios.ts b/contracts/exchange/test/utils/fill_order_scenarios.ts index 111068d5a2..e80015d6e8 100644 --- a/contracts/exchange/test/utils/fill_order_scenarios.ts +++ b/contracts/exchange/test/utils/fill_order_scenarios.ts @@ -1,5 +1,3 @@ -import { BigNumber } from '@0x/utils'; - export enum FeeRecipientAddressScenario { BurnAddress = 'BURN_ADDRESS', EthUserAddress = 'ETH_USER_ADDRESS', @@ -82,13 +80,6 @@ export interface FillScenario { takerStateScenario: TraderStateScenario; } -export interface FillResults { - makerAssetFilledAmount: BigNumber; - takerAssetFilledAmount: BigNumber; - makerFeePaid: BigNumber; - takerFeePaid: BigNumber; -} - export interface OrderScenario { takerScenario: TakerScenario; feeRecipientScenario: FeeRecipientAddressScenario; diff --git a/contracts/exchange/test/utils/fill_order_simulator.ts b/contracts/exchange/test/utils/fill_order_simulator.ts index 887595c6e1..c2b4224656 100644 --- a/contracts/exchange/test/utils/fill_order_simulator.ts +++ b/contracts/exchange/test/utils/fill_order_simulator.ts @@ -1,4 +1,4 @@ -import { constants, FillResults, orderUtils } from '@0x/contracts-test-utils'; +import { constants, orderUtils } from '@0x/contracts-test-utils'; import { AbstractBalanceAndProxyAllowanceLazyStore as LazyStore, ExchangeTransferSimulator, @@ -6,6 +6,7 @@ import { TradeSide, TransferType, } from '@0x/order-utils'; +import { FillResults } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; diff --git a/contracts/exchange/test/utils/isolated_exchange_wrapper.ts b/contracts/exchange/test/utils/isolated_exchange_wrapper.ts new file mode 100644 index 0000000000..d1dfb22078 --- /dev/null +++ b/contracts/exchange/test/utils/isolated_exchange_wrapper.ts @@ -0,0 +1,195 @@ +import { + constants, + filterLogsToArguments, + MutatorContractFunction, + TransactionHelper, + txDefaults as testTxDefaults, +} from '@0x/contracts-test-utils'; +import { orderHashUtils } from '@0x/order-utils'; +import { FillResults, OrderInfo, OrderWithoutDomain, SignatureType } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { TxData, Web3Wrapper } from '@0x/web3-wrapper'; +import * as crypto from 'crypto'; +import { LogEntry } from 'ethereum-types'; + +import { + artifacts, + IsolatedExchangeContract, + IsolatedExchangeDispatchTransferFromCalledEventArgs as DispatchTransferFromCallArgs, + IsolatedExchangeFillEventArgs as FillEventArgs, +} from '../../src'; + +export interface AssetBalances { + [assetData: string]: { [address: string]: BigNumber }; +} + +export interface IsolatedExchangeEvents { + fillEvents: FillEventArgs[]; + transferFromCalls: DispatchTransferFromCallArgs[]; +} + +export type Order = OrderWithoutDomain; +export type Numberish = string | number | BigNumber; + +export const DEFAULT_GOOD_SIGNATURE = createGoodSignature(); +export const DEFAULT_BAD_SIGNATURE = createBadSignature(); + +/** + * @dev Convenience wrapper for the `IsolatedExchange` contract. + */ +export class IsolatedExchangeWrapper { + public static readonly CHAIN_ID = 1337; + public readonly instance: IsolatedExchangeContract; + public readonly txHelper: TransactionHelper; + public lastTxEvents: IsolatedExchangeEvents = createEmptyEvents(); + public lastTxBalanceChanges: AssetBalances = {}; + + public static async deployAsync( + web3Wrapper: Web3Wrapper, + txDefaults: Partial = testTxDefaults, + ): Promise { + const provider = web3Wrapper.getProvider(); + const instance = await IsolatedExchangeContract.deployFrom0xArtifactAsync( + artifacts.IsolatedExchange, + provider, + txDefaults, + ); + return new IsolatedExchangeWrapper(web3Wrapper, instance); + } + + public static fromAddress( + address: string, + web3Wrapper: Web3Wrapper, + txDefaults: Partial = testTxDefaults, + ): IsolatedExchangeWrapper { + const provider = web3Wrapper.getProvider(); + const instance = new IsolatedExchangeContract(address, provider, txDefaults); + return new IsolatedExchangeWrapper(web3Wrapper, instance); + } + + public constructor(web3Wrapper: Web3Wrapper, instance: IsolatedExchangeContract) { + this.instance = instance; + this.txHelper = new TransactionHelper(web3Wrapper, artifacts); + } + + public async getTakerAssetFilledAmountAsync(order: Order): Promise { + return this.instance.filled.callAsync(this.getOrderHash(order)); + } + + public async cancelOrderAsync(order: Order, txOpts?: TxData): Promise { + await this.instance.cancelOrder.awaitTransactionSuccessAsync(order, txOpts); + } + + public async cancelOrdersUpToAsync(epoch: BigNumber, txOpts?: TxData): Promise { + await this.instance.cancelOrdersUpTo.awaitTransactionSuccessAsync(epoch, txOpts); + } + + public async fillOrderAsync( + order: Order, + takerAssetFillAmount: Numberish, + signature: string = DEFAULT_GOOD_SIGNATURE, + txOpts?: TxData, + ): Promise { + return this._runFillContractFunctionAsync( + this.instance.fillOrder, + order, + new BigNumber(takerAssetFillAmount), + signature, + txOpts, + ); + } + + public getOrderHash(order: Order): string { + const domain = { + verifyingContractAddress: this.instance.address, + chainId: IsolatedExchangeWrapper.CHAIN_ID, + }; + return orderHashUtils.getOrderHashHex({ ...order, domain }); + } + + public async getOrderInfoAsync(order: Order): Promise { + return this.instance.getOrderInfo.callAsync(order); + } + + public getBalanceChange(assetData: string, address: string): BigNumber { + if (assetData in this.lastTxBalanceChanges) { + const balances = this.lastTxBalanceChanges[assetData]; + if (address in balances) { + return balances[address]; + } + } + return constants.ZERO_AMOUNT; + } + + protected async _runFillContractFunctionAsync< + TCallAsyncArgs extends any[], + TAwaitTransactionSuccessAsyncArgs extends any[], + TResult + >( + contractFunction: MutatorContractFunction, + // tslint:disable-next-line: trailing-comma + ...args: TAwaitTransactionSuccessAsyncArgs + ): Promise { + this.lastTxEvents = createEmptyEvents(); + this.lastTxBalanceChanges = {}; + const [result, receipt] = await this.txHelper.getResultAndReceiptAsync(contractFunction, ...args); + this.lastTxEvents = extractEvents(receipt.logs); + this.lastTxBalanceChanges = getBalanceChangesFromTransferFromCalls(this.lastTxEvents.transferFromCalls); + return result; + } +} + +/** + * Create a signature for the `IsolatedExchange` contract that will pass. + */ +export function createGoodSignature(type: SignatureType = SignatureType.EIP712): string { + return `0x01${Buffer.from([type]).toString('hex')}`; +} + +/** + * Create a signature for the `IsolatedExchange` contract that will fail. + */ +export function createBadSignature(type: SignatureType = SignatureType.EIP712): string { + return `0x00${Buffer.from([type]).toString('hex')}`; +} + +const ERC20_ASSET_DATA_LENGTH = 36; + +/** + * Create asset data for the `IsolatedExchange` contract that will pass. + */ +export function createGoodAssetData(length: number = ERC20_ASSET_DATA_LENGTH): string { + return `0x01${crypto.randomBytes(length - 1).toString('hex')}`; +} + +/** + * Create asset data for the `IsolatedExchange` contract that will fail. + */ +export function createBadAssetData(length: number = ERC20_ASSET_DATA_LENGTH): string { + return `0x00${crypto.randomBytes(length - 1).toString('hex')}`; +} + +function createEmptyEvents(): IsolatedExchangeEvents { + return { fillEvents: [], transferFromCalls: [] }; +} + +function extractEvents(logs: LogEntry[]): IsolatedExchangeEvents { + return { + fillEvents: filterLogsToArguments(logs, 'Fill'), + transferFromCalls: filterLogsToArguments(logs, 'DispatchTransferFromCalled'), + }; +} + +// Executes transferFrom calls to compute relative balances for addresses. +function getBalanceChangesFromTransferFromCalls(calls: DispatchTransferFromCallArgs[]): AssetBalances { + const changes: AssetBalances = {}; + for (const call of calls) { + const { assetData, from, to, amount } = call; + const balances = (changes[assetData] = changes[assetData] || {}); + const fromBalance = balances[from] || constants.ZERO_AMOUNT; + const toBalance = balances[to] || constants.ZERO_AMOUNT; + balances[from] = fromBalance.minus(amount); + balances[to] = toBalance.plus(amount); + } + return changes; +} diff --git a/contracts/exchange/test/utils/match_order_tester.ts b/contracts/exchange/test/utils/match_order_tester.ts index 9d8572e6b0..d9e997d601 100644 --- a/contracts/exchange/test/utils/match_order_tester.ts +++ b/contracts/exchange/test/utils/match_order_tester.ts @@ -1,16 +1,8 @@ import { ERC1155ProxyWrapper, ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy'; -import { - BatchMatchedFillResults, - chaiSetup, - ERC1155HoldingsByOwner, - FillResults, - MatchedFillResults, - OrderStatus, -} from '@0x/contracts-test-utils'; +import { ERC1155HoldingsByOwner, expect, OrderStatus } from '@0x/contracts-test-utils'; import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; -import { AssetProxyId, SignedOrder } from '@0x/types'; +import { AssetProxyId, BatchMatchedFillResults, FillResults, MatchedFillResults, SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; -import * as chai from 'chai'; import { LogWithDecodedArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; @@ -18,9 +10,6 @@ import { ExchangeWrapper } from './exchange_wrapper'; const ZERO = new BigNumber(0); -chaiSetup.configure(); -const expect = chai.expect; - export interface IndividualERC1155Holdings { fungible: { [tokenId: string]: BigNumber; diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 3b07144d00..9dcd4e0501 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -5,7 +5,6 @@ import { chaiSetup, constants, ERC20BalancesByOwner, - FillResults, getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync, OrderFactory, @@ -15,7 +14,7 @@ import { } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { assetDataUtils, ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils'; -import { OrderStatus, SignedOrder } from '@0x/types'; +import { FillResults, OrderStatus, SignedOrder } from '@0x/types'; import { BigNumber, providerUtils, ReentrancyGuardRevertErrors } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; diff --git a/contracts/exchange/tsconfig.json b/contracts/exchange/tsconfig.json index 6d8ad9c993..319109caae 100644 --- a/contracts/exchange/tsconfig.json +++ b/contracts/exchange/tsconfig.json @@ -14,10 +14,10 @@ "generated-artifacts/ITransactions.json", "generated-artifacts/IWallet.json", "generated-artifacts/IWrapperFunctions.json", + "generated-artifacts/IsolatedExchange.json", "generated-artifacts/ReentrantERC20Token.json", "generated-artifacts/TestAssetProxyDispatcher.json", "generated-artifacts/TestExchangeInternals.json", - "generated-artifacts/TestExchangeMath.json", "generated-artifacts/TestLibExchangeRichErrorDecoder.json", "generated-artifacts/TestSignatureValidator.json", "generated-artifacts/TestValidatorWallet.json", diff --git a/contracts/exchange/tslint.json b/contracts/exchange/tslint.json index 1bb3ac2a22..2a304eb78b 100644 --- a/contracts/exchange/tslint.json +++ b/contracts/exchange/tslint.json @@ -2,5 +2,8 @@ "extends": ["@0x/tslint-config"], "rules": { "custom-no-magic-numbers": false + }, + "linterOptions": { + "exclude": ["src/artifacts.ts"] } } diff --git a/contracts/test-utils/CHANGELOG.json b/contracts/test-utils/CHANGELOG.json index 21eb15b419..e16993f703 100644 --- a/contracts/test-utils/CHANGELOG.json +++ b/contracts/test-utils/CHANGELOG.json @@ -37,6 +37,30 @@ { "note": "Introduce Mocha blockchain extensions", "pr": 2007 + }, + { + "note": "Move `*FillResults`, `OrderInfo` types to `@0x/types`", + "pr": 2031 + }, + { + "note": "Add `log_utils.ts`", + "pr": 2031 + }, + { + "note": "Add `haxRandom()` to `hex_utils.ts`", + "pr": 2031 + }, + { + "note": "Add the constants: `MAX_UINT256`, `ADDRESS_LENGTH`, `MAX_UINT256_ROOT`, `ONE_ETHER`", + "pr": 2031 + }, + { + "note": "Make `testCombinatoriallyWithReferenceFuncAsync` non-async", + "pr": 2031 + }, + { + "note": "Update `testWithReferenceFuncAsync` to work with `RevertErrors`", + "pr": 2031 } ] }, diff --git a/contracts/test-utils/src/combinatorial_utils.ts b/contracts/test-utils/src/combinatorial_utils.ts index bb1b55b4dd..a50190a6eb 100644 --- a/contracts/test-utils/src/combinatorial_utils.ts +++ b/contracts/test-utils/src/combinatorial_utils.ts @@ -36,30 +36,30 @@ export const bytes32Values = [ '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', ]; -export async function testCombinatoriallyWithReferenceFuncAsync( +export function testCombinatoriallyWithReferenceFunc( name: string, referenceFunc: (p0: P0, p1: P1) => Promise, testFunc: (p0: P0, p1: P1) => Promise, allValues: [P0[], P1[]], -): Promise; -export async function testCombinatoriallyWithReferenceFuncAsync( +): void; +export function testCombinatoriallyWithReferenceFunc( name: string, referenceFunc: (p0: P0, p1: P1, p2: P2) => Promise, testFunc: (p0: P0, p1: P1, p2: P2) => Promise, allValues: [P0[], P1[], P2[]], -): Promise; -export async function testCombinatoriallyWithReferenceFuncAsync( +): void; +export function testCombinatoriallyWithReferenceFunc( name: string, referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise, testFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise, allValues: [P0[], P1[], P2[], P3[]], -): Promise; -export async function testCombinatoriallyWithReferenceFuncAsync( +): void; +export function testCombinatoriallyWithReferenceFunc( name: string, referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise, testFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise, allValues: [P0[], P1[], P2[], P3[], P4[]], -): Promise; +): void; /** * Uses combinatorics to test the behavior of a test function by comparing it to @@ -96,12 +96,12 @@ export async function testCombinatoriallyWithReferenceFuncAsync Promise, testFuncAsync: (...args: any[]) => Promise, allValues: any[], -): Promise { +): void { const testCases = combinatorics.cartesianProduct(...allValues); let counter = 0; testCases.forEach(async testCase => { diff --git a/contracts/test-utils/src/constants.ts b/contracts/test-utils/src/constants.ts index 3ec8ff987d..7a2740da2a 100644 --- a/contracts/test-utils/src/constants.ts +++ b/contracts/test-utils/src/constants.ts @@ -16,6 +16,8 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ '0x23cb7121166b9a2f93ae0b7c05bde02eae50d64449b2cbb42bc84e9d38d6cc89', ]; +const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); + export const constants = { BASE_16: 16, INVALID_OPCODE: 'invalid opcode', @@ -43,7 +45,8 @@ export const constants = { NUM_ERC1155_NONFUNGIBLE_TOKENS_MINT: 4, NULL_ADDRESS: '0x0000000000000000000000000000000000000000', NULL_BYTES32: '0x0000000000000000000000000000000000000000000000000000000000000000', - UNLIMITED_ALLOWANCE_IN_BASE_UNITS: new BigNumber(2).pow(256).minus(1), + UNLIMITED_ALLOWANCE_IN_BASE_UNITS: MAX_UINT256, + MAX_UINT256, TESTRPC_PRIVATE_KEYS: _.map(TESTRPC_PRIVATE_KEYS_STRINGS, privateKeyString => ethUtil.toBuffer(privateKeyString)), INITIAL_ERC20_BALANCE: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 18), INITIAL_ERC20_ALLOWANCE: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 18), @@ -56,8 +59,11 @@ export const constants = { takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 18), }, WORD_LENGTH: 32, + ADDRESS_LENGTH: 20, ZERO_AMOUNT: new BigNumber(0), PERCENTAGE_DENOMINATOR: new BigNumber(10).pow(18), TIME_BUFFER: new BigNumber(1000), KECCAK256_NULL: ethUtil.addHexPrefix(ethUtil.bufferToHex(ethUtil.SHA3_NULL)), + MAX_UINT256_ROOT: new BigNumber('340282366920938463463374607431768211456'), + ONE_ETHER: new BigNumber(1e18), }; diff --git a/contracts/test-utils/src/hex_utils.ts b/contracts/test-utils/src/hex_utils.ts index 95c2435b1f..d1a70442c2 100644 --- a/contracts/test-utils/src/hex_utils.ts +++ b/contracts/test-utils/src/hex_utils.ts @@ -1,3 +1,4 @@ +import * as crypto from 'crypto'; import * as ethUtil from 'ethereumjs-util'; /** @@ -6,3 +7,9 @@ import * as ethUtil from 'ethereumjs-util'; export function hexConcat(...args: Array): string { return ethUtil.bufferToHex(Buffer.concat(args.map(h => ethUtil.toBuffer(h)))); } +/** + * Generate a random hex string. + */ +export function hexRandom(size: number = 32): string { + return ethUtil.bufferToHex(crypto.randomBytes(size)); +} diff --git a/contracts/test-utils/src/index.ts b/contracts/test-utils/src/index.ts index 3abd3f6070..ea1ff0839c 100644 --- a/contracts/test-utils/src/index.ts +++ b/contracts/test-utils/src/index.ts @@ -15,6 +15,7 @@ export { export { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from './block_timestamp'; export { provider, txDefaults, web3Wrapper } from './web3_wrapper'; export { LogDecoder } from './log_decoder'; +export { filterLogs, filterLogsToArguments } from './log_utils'; export { signingUtils } from './signing_utils'; export { orderUtils } from './order_utils'; export { typeEncodingUtils } from './type_encoding_utils'; @@ -23,12 +24,12 @@ export { coverage } from './coverage'; export { Web3ProviderEngine } from '@0x/subproviders'; export { addressUtils } from './address_utils'; export { OrderFactory } from './order_factory'; -export { bytes32Values, testCombinatoriallyWithReferenceFuncAsync, uint256Values } from './combinatorial_utils'; +export { bytes32Values, testCombinatoriallyWithReferenceFunc, uint256Values } from './combinatorial_utils'; export { TransactionFactory } from './transaction_factory'; +export { MutatorContractFunction, TransactionHelper } from './transaction_helper'; export { testWithReferenceFuncAsync } from './test_with_reference'; -export { hexConcat } from './hex_utils'; +export { hexConcat, hexRandom } from './hex_utils'; export { - BatchMatchedFillResults, BatchMatchOrder, ContractName, ERC20BalancesByOwner, @@ -36,11 +37,8 @@ export { ERC1155HoldingsByOwner, ERC1155NonFungibleHoldingsByOwner, ERC721TokenIdsByOwner, - FillResults, MarketBuyOrders, MarketSellOrders, - MatchedFillResults, - OrderInfo, OrderStatus, Token, TransactionDataParams, diff --git a/contracts/test-utils/src/log_decoder.ts b/contracts/test-utils/src/log_decoder.ts index f380f5cef2..215e942381 100644 --- a/contracts/test-utils/src/log_decoder.ts +++ b/contracts/test-utils/src/log_decoder.ts @@ -7,6 +7,7 @@ import { LogEntry, LogWithDecodedArgs, RawLog, + TransactionReceipt, TransactionReceiptWithDecodedLogs, } from 'ethereum-types'; import * as _ from 'lodash'; @@ -44,8 +45,14 @@ export class LogDecoder { return logWithDecodedArgsOrLog; } public async getTxWithDecodedLogsAsync(txHash: string): Promise { - const tx = await this._web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); - tx.logs = _.map(tx.logs, log => this.decodeLogOrThrow(log)); - return tx; + const receipt = await this._web3Wrapper.awaitTransactionSuccessAsync( + txHash, + constants.AWAIT_TRANSACTION_MINED_MS, + ); + return this.decodeReceiptLogs(receipt); + } + public decodeReceiptLogs(receipt: TransactionReceipt): TransactionReceiptWithDecodedLogs { + const decodedLogs = (receipt.logs as LogEntry[]).map(log => this.decodeLogOrThrow(log)); + return _.merge({}, receipt, { logs: decodedLogs }); } } diff --git a/contracts/test-utils/src/log_utils.ts b/contracts/test-utils/src/log_utils.ts new file mode 100644 index 0000000000..ae95ace2f8 --- /dev/null +++ b/contracts/test-utils/src/log_utils.ts @@ -0,0 +1,17 @@ +import { LogEntry, LogWithDecodedArgs } from 'ethereum-types'; + +// tslint:disable no-unnecessary-type-assertion + +/** + * Filter logs by event name/type. + */ +export function filterLogs(logs: LogEntry[], event: string): Array> { + return (logs as Array>).filter(log => log.event === event); +} + +/** + * Filter logs by event name/type and convert to arguments. + */ +export function filterLogsToArguments(logs: LogEntry[], event: string): TEventArgs[] { + return filterLogs(logs, event).map(log => log.args); +} diff --git a/contracts/test-utils/src/test_with_reference.ts b/contracts/test-utils/src/test_with_reference.ts index 7585f8bd4e..fde39852e6 100644 --- a/contracts/test-utils/src/test_with_reference.ts +++ b/contracts/test-utils/src/test_with_reference.ts @@ -1,35 +1,8 @@ +import { BigNumber, RevertError } from '@0x/utils'; import * as _ from 'lodash'; import { expect } from './chai_setup'; -class Value { - public value: T; - constructor(value: T) { - this.value = value; - } -} - -// tslint:disable-next-line: max-classes-per-file -class ErrorMessage { - public error: string; - constructor(message: string) { - this.error = message; - } -} - -type PromiseResult = Value | ErrorMessage; - -// TODO(albrow): This seems like a generic utility function that could exist in -// lodash. We should replace it by a library implementation, or move it to our -// own. -async function evaluatePromiseAsync(promise: Promise): Promise> { - try { - return new Value(await promise); - } catch (e) { - return new ErrorMessage(e.message); - } -} - export async function testWithReferenceFuncAsync( referenceFunc: (p0: P0) => Promise, testFunc: (p0: P0) => Promise, @@ -88,36 +61,80 @@ export async function testWithReferenceFuncAsync( testFuncAsync: (...args: any[]) => Promise, values: any[], ): Promise { - // Measure correct behaviour - const expected = await evaluatePromiseAsync(referenceFuncAsync(...values)); - - // Measure actual behaviour - const actual = await evaluatePromiseAsync(testFuncAsync(...values)); + // Measure correct behavior + let expected: any; + let expectedError: Error | undefined; + try { + expected = await referenceFuncAsync(...values); + } catch (err) { + expectedError = err; + } + // Measure actual behavior + let actual: any; + let actualError: Error | undefined; + try { + actual = await testFuncAsync(...values); + } catch (err) { + actualError = err; + } - // Compare behaviour - if (expected instanceof ErrorMessage) { - // If we expected an error, check if the actual error message contains the - // expected error message. - if (!(actual instanceof ErrorMessage)) { - throw new Error( - `Expected error containing ${expected.error} but got no error\n\tTest case: ${_getTestCaseString( - referenceFuncAsync, - values, - )}`, - ); + const testCaseString = _getTestCaseString(referenceFuncAsync, values); + // Compare behavior + if (expectedError !== undefined) { + // Expecting an error. + if (actualError === undefined) { + return expect.fail(actualError, expectedError, `${testCaseString}: expected failure but instead succeeded`); + } else { + if (expectedError instanceof RevertError) { + // Expecting a RevertError. + if (actualError instanceof RevertError) { + if (!actualError.equals(expectedError)) { + return expect.fail( + actualError, + expectedError, + `${testCaseString}: expected error ${actualError.toString()} to equal ${expectedError.toString()}`, + ); + } + } else { + return expect.fail( + actualError, + expectedError, + `${testCaseString}: expected a RevertError but received an Error`, + ); + } + } else { + // Expecing any Error type. + if (actualError.message !== expectedError.message) { + return expect.fail( + actualError, + expectedError, + `${testCaseString}: expected error message '${actualError.message}' to equal '${ + expectedError.message + }'`, + ); + } + } } - expect(actual.error).to.contain( - expected.error, - `${actual.error}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`, - ); } else { - // If we do not expect an error, compare actual and expected directly. - expect(actual).to.deep.equal(expected, `Test case ${_getTestCaseString(referenceFuncAsync, values)}`); + // Not expecting an error. + if (actualError !== undefined) { + return expect.fail(actualError, expectedError, `${testCaseString}: expected success but instead failed`); + } + if (expected instanceof BigNumber) { + // Technically we can do this with `deep.eq`, but this prints prettier + // error messages for BigNumbers. + expect(actual).to.bignumber.eq(expected, testCaseString); + } else { + expect(actual).to.deep.eq(expected, testCaseString); + } } } function _getTestCaseString(referenceFuncAsync: (...args: any[]) => Promise, values: any[]): string { const paramNames = _getParameterNames(referenceFuncAsync); + while (paramNames.length < values.length) { + paramNames.push(`${paramNames.length}`); + } return JSON.stringify(_.zipObject(paramNames, values)); } diff --git a/contracts/test-utils/src/transaction_helper.ts b/contracts/test-utils/src/transaction_helper.ts new file mode 100644 index 0000000000..e861e389f7 --- /dev/null +++ b/contracts/test-utils/src/transaction_helper.ts @@ -0,0 +1,56 @@ +import { Web3Wrapper } from '@0x/web3-wrapper'; +import { ContractArtifact, TransactionReceipt } from 'ethereum-types'; + +import { LogDecoder } from './log_decoder'; + +type AsyncFunction = (...args: TArgs) => Promise; + +interface ContractArtifacts { + [contractName: string]: ContractArtifact; +} + +export interface MutatorContractFunction< + TCallAsyncArgs extends any[], + TAwaitTransactionSuccessAsyncArgs extends any[], + TCallAsyncResult +> { + callAsync: AsyncFunction; + sendTransactionAsync: AsyncFunction; +} + +/** + * Helper class for performing non-constant contract functions calls. + */ +export class TransactionHelper { + public readonly logDecoder: LogDecoder; + + constructor(web3Wrapper: Web3Wrapper, artifacts: ContractArtifacts) { + this.logDecoder = new LogDecoder(web3Wrapper, artifacts); + } + + /** + * Call a non-constant contract function `contractFunction`, passing `args`. + * This will first perform an 'eth_call' (read-only) call in order to + * retrieve the return value, then a 'sendTransaction' to actually modify + * the state. Returns a tuple of the return value amd receipt, with decoded + * logs. + */ + public async getResultAndReceiptAsync< + TCallAsyncArgs extends any[], + TAwaitTransactionSuccessAsyncArgs extends any[], + TCallAsyncResult + >( + contractFunction: MutatorContractFunction, + // tslint:disable-next-line: trailing-comma + ...args: TAwaitTransactionSuccessAsyncArgs + ): Promise<[TCallAsyncResult, TransactionReceipt]> { + // HACK(dorothy-zbornak): We take advantage of the general rule that + // the parameters for `callAsync()` are a subset of the + // parameters for `sendTransactionAsync()`. + const result = await contractFunction.callAsync(...((args as any) as TCallAsyncArgs)); + const receipt = await this.logDecoder.getTxWithDecodedLogsAsync( + await contractFunction.sendTransactionAsync(...args), + ); + return [result, receipt]; + } +} diff --git a/contracts/test-utils/src/types.ts b/contracts/test-utils/src/types.ts index b085a34d0c..84fdd18b52 100644 --- a/contracts/test-utils/src/types.ts +++ b/contracts/test-utils/src/types.ts @@ -119,12 +119,6 @@ export enum ContractName { BalanceThresholdFilter = 'BalanceThresholdFilter', } -export interface OrderInfo { - orderStatus: number; - orderHash: string; - orderTakerAssetFilledAmount: BigNumber; -} - export interface CancelOrder { order: OrderWithoutDomain; takerAssetCancelAmount: BigNumber; @@ -143,24 +137,3 @@ export interface MatchOrder { leftSignature: string; rightSignature: string; } - -export interface FillResults { - makerAssetFilledAmount: BigNumber; - takerAssetFilledAmount: BigNumber; - makerFeePaid: BigNumber; - takerFeePaid: BigNumber; -} - -export interface MatchedFillResults { - left: FillResults; - right: FillResults; - profitInLeftMakerAsset: BigNumber; - profitInRightMakerAsset: BigNumber; -} - -export interface BatchMatchedFillResults { - left: FillResults[]; - right: FillResults[]; - profitInLeftMakerAsset: BigNumber; - profitInRightMakerAsset: BigNumber; -} diff --git a/contracts/test-utils/test/test_with_reference.ts b/contracts/test-utils/test/test_with_reference.ts index 1c1211003e..7407bf0c93 100644 --- a/contracts/test-utils/test/test_with_reference.ts +++ b/contracts/test-utils/test/test_with_reference.ts @@ -1,11 +1,8 @@ -import * as chai from 'chai'; +import { AnyRevertError, StringRevertError } from '@0x/utils'; -import { chaiSetup } from '../src/chai_setup'; +import { expect } from '../src'; import { testWithReferenceFuncAsync } from '../src/test_with_reference'; -chaiSetup.configure(); -const expect = chai.expect; - async function divAsync(x: number, y: number): Promise { if (y === 0) { throw new Error('MathError: divide by zero'); @@ -18,46 +15,74 @@ function alwaysValueFunc(value: number): (x: number, y: number) => Promise value; } -// returns an async function which always throws/rejects with the given error -// message. -function alwaysFailFunc(errMessage: string): (x: number, y: number) => Promise { +// returns an async function which always throws/rejects with the given error. +function alwaysFailFunc(error: Error): (x: number, y: number) => Promise { return async (x: number, y: number) => { - throw new Error(errMessage); + throw error; }; } describe('testWithReferenceFuncAsync', () => { - it('passes when both succeed and actual === expected', async () => { - await testWithReferenceFuncAsync(alwaysValueFunc(0.5), divAsync, [1, 2]); + it('passes when both succeed and actual == expected', async () => { + return testWithReferenceFuncAsync(alwaysValueFunc(0.5), divAsync, [1, 2]); }); - it('passes when both fail and actual error contains expected error', async () => { - await testWithReferenceFuncAsync(alwaysFailFunc('divide by zero'), divAsync, [1, 0]); + it('fails when both succeed and actual != expected', async () => { + return expect(testWithReferenceFuncAsync(alwaysValueFunc(3), divAsync, [1, 2])).to.be.rejectedWith( + '{"x":1,"y":2}: expected 0.5 to deeply equal 3', + ); }); - it('fails when both succeed and actual !== expected', async () => { - expect(testWithReferenceFuncAsync(alwaysValueFunc(3), divAsync, [1, 2])).to.be.rejectedWith( - 'Test case {"x":1,"y":2}: expected { value: 0.5 } to deeply equal { value: 3 }', - ); + it('passes when both fail and error messages are the same', async () => { + const err = new Error('whoopsie'); + return testWithReferenceFuncAsync(alwaysFailFunc(err), alwaysFailFunc(err), [1, 1]); + }); + + it('fails when both fail and error messages are not identical', async () => { + const errorMessage = 'whoopsie'; + const notErrorMessage = 'not whoopsie'; + const error = new Error(errorMessage); + const notError = new Error(notErrorMessage); + return expect( + testWithReferenceFuncAsync(alwaysFailFunc(notError), alwaysFailFunc(error), [1, 2]), + ).to.be.rejectedWith(`{"x":1,"y":2}: expected error message '${errorMessage}' to equal '${notErrorMessage}'`); }); - it('fails when both fail and actual error does not contain expected error', async () => { - expect( - testWithReferenceFuncAsync(alwaysFailFunc('Unexpected math error'), divAsync, [1, 0]), + it('passes when both fail with compatible RevertErrors', async () => { + const error1 = new StringRevertError('whoopsie'); + const error2 = new AnyRevertError(); + return testWithReferenceFuncAsync(alwaysFailFunc(error1), alwaysFailFunc(error2), [1, 1]); + }); + + it('fails when both fail with incompatible RevertErrors', async () => { + const error1 = new StringRevertError('whoopsie'); + const error2 = new StringRevertError('not whoopsie'); + return expect( + testWithReferenceFuncAsync(alwaysFailFunc(error1), alwaysFailFunc(error2), [1, 1]), ).to.be.rejectedWith( - 'MathError: divide by zero\n\tTest case: {"x":1,"y":0}: expected \'MathError: divide by zero\' to include \'Unexpected math error\'', + `{"x":1,"y":1}: expected error StringRevertError({ message: 'not whoopsie' }) to equal StringRevertError({ message: 'whoopsie' })`, ); }); + it('fails when reference function fails with a RevertError but test function fails with a regular Error', async () => { + const error1 = new StringRevertError('whoopsie'); + const error2 = new Error('whoopsie'); + return expect( + testWithReferenceFuncAsync(alwaysFailFunc(error1), alwaysFailFunc(error2), [1, 1]), + ).to.be.rejectedWith(`{"x":1,"y":1}: expected a RevertError but received an Error`); + }); + it('fails when referenceFunc succeeds and testFunc fails', async () => { - expect(testWithReferenceFuncAsync(alwaysValueFunc(0), divAsync, [1, 0])).to.be.rejectedWith( - 'Test case {"x":1,"y":0}: expected { error: \'MathError: divide by zero\' } to deeply equal { value: 0 }', + const error = new Error('whoopsie'); + return expect(testWithReferenceFuncAsync(alwaysValueFunc(0), alwaysFailFunc(error), [1, 2])).to.be.rejectedWith( + `{"x":1,"y":2}: expected success but instead failed`, ); }); it('fails when referenceFunc fails and testFunc succeeds', async () => { - expect(testWithReferenceFuncAsync(alwaysFailFunc('divide by zero'), divAsync, [1, 2])).to.be.rejectedWith( - 'Expected error containing divide by zero but got no error\n\tTest case: {"x":1,"y":2}', + const error = new Error('whoopsie'); + return expect(testWithReferenceFuncAsync(alwaysFailFunc(error), divAsync, [1, 2])).to.be.rejectedWith( + '{"x":1,"y":2}: expected failure but instead succeeded', ); }); }); diff --git a/contracts/utils/CHANGELOG.json b/contracts/utils/CHANGELOG.json index 8cf9dbdc51..ff9ced6e96 100644 --- a/contracts/utils/CHANGELOG.json +++ b/contracts/utils/CHANGELOG.json @@ -33,6 +33,14 @@ { "note": "Updated Ownable to revert when the owner attempts to transfer ownership to the zero address", "pr": 2019 + }, + { + "note": "Add reference functions for `SafeMath` functions.", + "pr": 2031 + }, + { + "note": "Throw a `SafeMathError` in `SafeMath._safeDiv()` when denominator is zero.", + "pr": 2031 } ] }, diff --git a/contracts/utils/contracts/src/LibSafeMathRichErrors.sol b/contracts/utils/contracts/src/LibSafeMathRichErrors.sol index a1f39cc41d..0b13af3706 100644 --- a/contracts/utils/contracts/src/LibSafeMathRichErrors.sol +++ b/contracts/utils/contracts/src/LibSafeMathRichErrors.sol @@ -10,7 +10,8 @@ library LibSafeMathRichErrors { enum SafeMathErrorCodes { UINT256_ADDITION_OVERFLOW, UINT256_MULTIPLICATION_OVERFLOW, - UINT256_SUBTRACTION_UNDERFLOW + UINT256_SUBTRACTION_UNDERFLOW, + UINT256_DIVISION_BY_ZERO } // solhint-disable func-name-mixedcase diff --git a/contracts/utils/contracts/src/SafeMath.sol b/contracts/utils/contracts/src/SafeMath.sol index ed22105a9a..1edc1bac93 100644 --- a/contracts/utils/contracts/src/SafeMath.sol +++ b/contracts/utils/contracts/src/SafeMath.sol @@ -30,6 +30,13 @@ contract SafeMath { pure returns (uint256) { + if (b == 0) { + LibRichErrors._rrevert(LibSafeMathRichErrors.SafeMathError( + LibSafeMathRichErrors.SafeMathErrorCodes.UINT256_DIVISION_BY_ZERO, + a, + b + )); + } uint256 c = a / b; return c; } diff --git a/contracts/utils/src/index.ts b/contracts/utils/src/index.ts index d55f08ea2d..0233c608d1 100644 --- a/contracts/utils/src/index.ts +++ b/contracts/utils/src/index.ts @@ -1,2 +1,5 @@ export * from './artifacts'; export * from './wrappers'; + +import * as ReferenceFunctionsToExport from './reference_functions'; +export import ReferenceFunctions = ReferenceFunctionsToExport; diff --git a/contracts/utils/src/reference_functions.ts b/contracts/utils/src/reference_functions.ts new file mode 100644 index 0000000000..0049d661bb --- /dev/null +++ b/contracts/utils/src/reference_functions.ts @@ -0,0 +1,62 @@ +import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; + +const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); + +/** + * Add two `uint256` values. Reverts on overflow. + */ +export function safeAdd(a: BigNumber, b: BigNumber): BigNumber { + const r = a.plus(b); + if (r.isGreaterThan(MAX_UINT256)) { + throw new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a, + b, + ); + } + return r; +} + +/** + * Subract two `uint256` values. Reverts on overflow. + */ +export function safeSub(a: BigNumber, b: BigNumber): BigNumber { + const r = a.minus(b); + if (r.isLessThan(0)) { + throw new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256SubtractionUnderflow, + a, + b, + ); + } + return r; +} + +/** + * Multiplies two `uint256` values. Reverts on overflow. + */ +export function safeMul(a: BigNumber, b: BigNumber): BigNumber { + const r = a.times(b); + if (r.isGreaterThan(MAX_UINT256)) { + throw new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + a, + b, + ); + } + return r; +} + +/** + * Divides two `uint256` values. Reverts on division by zero. + */ +export function safeDiv(a: BigNumber, b: BigNumber): BigNumber { + if (b.isEqualTo(0)) { + throw new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero, + a, + b, + ); + } + return a.dividedToIntegerBy(b); +} diff --git a/contracts/utils/test/reference_functions.ts b/contracts/utils/test/reference_functions.ts new file mode 100644 index 0000000000..047ecfd185 --- /dev/null +++ b/contracts/utils/test/reference_functions.ts @@ -0,0 +1,93 @@ +import { constants, describe, expect } from '@0x/contracts-test-utils'; +import { SafeMathRevertErrors } from '@0x/utils'; + +import { safeAdd, safeDiv, safeMul, safeSub } from '../src/reference_functions'; + +describe('Reference Functions', () => { + const { ONE_ETHER, MAX_UINT256, ZERO_AMOUNT } = constants; + const DEFAULT_VALUES = { + a: ONE_ETHER.times(2), + b: ONE_ETHER, + }; + describe('SafeMath', () => { + describe('safeAdd', () => { + it('adds two numbers', () => { + const { a, b } = DEFAULT_VALUES; + const expected = a.plus(b); + const actual = safeAdd(a, b); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts on overflow', () => { + const a = MAX_UINT256.dividedToIntegerBy(2); + const b = MAX_UINT256.dividedToIntegerBy(2).plus(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256AdditionOverflow, + a, + b, + ); + expect(() => safeAdd(a, b)).to.throw(expectedError.message); + }); + }); + + describe('safeSub', () => { + it('subracts two numbers', () => { + const { a, b } = DEFAULT_VALUES; + const expected = a.minus(b); + const actual = safeSub(a, b); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts on underflow', () => { + const a = MAX_UINT256.dividedToIntegerBy(2); + const b = MAX_UINT256.dividedToIntegerBy(2).plus(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256SubtractionUnderflow, + a, + b, + ); + expect(() => safeSub(a, b)).to.throw(expectedError.message); + }); + }); + + describe('safeMul', () => { + it('multiplies two numbers', () => { + const { a, b } = DEFAULT_VALUES; + const expected = a.times(b); + const actual = safeMul(a, b); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts on overflow', () => { + const a = MAX_UINT256.dividedToIntegerBy(2); + const b = MAX_UINT256.dividedToIntegerBy(2).plus(2); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, + a, + b, + ); + expect(() => safeMul(a, b)).to.throw(expectedError.message); + }); + }); + + describe('safeDiv', () => { + it('multiplies two numbers', () => { + const { a, b } = DEFAULT_VALUES; + const expected = a.times(b); + const actual = safeMul(a, b); + expect(actual).to.bignumber.eq(expected); + }); + + it('reverts if denominator is zero', () => { + const a = MAX_UINT256.dividedToIntegerBy(2); + const b = ZERO_AMOUNT; + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero, + a, + b, + ); + expect(() => safeDiv(a, b)).to.throw(expectedError.message); + }); + }); + }); +}); diff --git a/contracts/utils/test/safe_math.ts b/contracts/utils/test/safe_math.ts index 51ac43662b..f8c4b7466d 100644 --- a/contracts/utils/test/safe_math.ts +++ b/contracts/utils/test/safe_math.ts @@ -1,33 +1,36 @@ -import { chaiSetup, constants, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; -import { BlockchainLifecycle } from '@0x/dev-utils'; +import { blockchainTests, constants, describe, expect } from '@0x/contracts-test-utils'; import { BigNumber, SafeMathRevertErrors } from '@0x/utils'; -import * as chai from 'chai'; import * as _ from 'lodash'; import { artifacts, TestSafeMathContract } from '../src'; - -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +import * as ReferenceFunctions from '../src/reference_functions'; function toBigNumber(a: number | string): BigNumber { return new BigNumber(a); } -describe('SafeMath', () => { +blockchainTests('SafeMath', env => { + const { ONE_ETHER } = constants; let safeMath: TestSafeMathContract; before(async () => { - await blockchainLifecycle.startAsync(); // Deploy SafeMath - safeMath = await TestSafeMathContract.deployFrom0xArtifactAsync(artifacts.TestSafeMath, provider, txDefaults); - }); - - after(async () => { - await blockchainLifecycle.revertAsync(); + safeMath = await TestSafeMathContract.deployFrom0xArtifactAsync( + artifacts.TestSafeMath, + env.provider, + env.txDefaults, + ); }); describe('_safeMul', () => { + it('should match the output of the reference function', async () => { + const a = ONE_ETHER; + const b = ONE_ETHER.times(2); + const expected = ReferenceFunctions.safeMul(a, b); + const actual = await safeMath.externalSafeMul.callAsync(a, b); + expect(actual).bignumber.to.be.eq(expected); + }); + it('should return zero if first argument is zero', async () => { const result = await safeMath.externalSafeMul.callAsync(constants.ZERO_AMOUNT, toBigNumber(1)); expect(result).bignumber.to.be.eq(constants.ZERO_AMOUNT); @@ -56,6 +59,14 @@ describe('SafeMath', () => { }); describe('_safeDiv', () => { + it('should match the output of the reference function', async () => { + const a = ONE_ETHER; + const b = ONE_ETHER.times(2); + const expected = ReferenceFunctions.safeDiv(a, b); + const actual = await safeMath.externalSafeDiv.callAsync(a, b); + expect(actual).bignumber.to.be.eq(expected); + }); + it('should return the correct value if both values are the same', async () => { const result = await safeMath.externalSafeDiv.callAsync(toBigNumber(1), toBigNumber(1)); expect(result).bignumber.to.be.eq(toBigNumber(1)); @@ -77,14 +88,26 @@ describe('SafeMath', () => { }); it('should revert if second argument is zero', async () => { - const errMessage = 'VM Exception while processing transaction: invalid opcode'; - return expect(safeMath.externalSafeDiv.callAsync(toBigNumber(1), constants.ZERO_AMOUNT)).to.be.rejectedWith( - errMessage, + const a = toBigNumber(1); + const b = toBigNumber(0); + const expectedError = new SafeMathRevertErrors.SafeMathError( + SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero, + a, + b, ); + return expect(safeMath.externalSafeDiv.callAsync(a, b)).to.revertWith(expectedError); }); }); describe('_safeSub', () => { + it('should match the output of the reference function', async () => { + const a = ONE_ETHER; + const b = ONE_ETHER.dividedToIntegerBy(2); + const expected = ReferenceFunctions.safeSub(a, b); + const actual = await safeMath.externalSafeSub.callAsync(a, b); + expect(actual).bignumber.to.be.eq(expected); + }); + it('should revert if the subtraction underflows', async () => { const a = toBigNumber(0); const b = toBigNumber(1); @@ -108,6 +131,14 @@ describe('SafeMath', () => { }); describe('_safeAdd', () => { + it('should match the output of the reference function', async () => { + const a = ONE_ETHER; + const b = ONE_ETHER.dividedToIntegerBy(2); + const expected = ReferenceFunctions.safeAdd(a, b); + const actual = await safeMath.externalSafeAdd.callAsync(a, b); + expect(actual).bignumber.to.be.eq(expected); + }); + it('should revert if the addition overflows', async () => { const a = toBigNumber('0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'); // The largest uint256 number const b = toBigNumber(1); diff --git a/packages/dev-utils/CHANGELOG.json b/packages/dev-utils/CHANGELOG.json index 81f7fe017e..c420baa714 100644 --- a/packages/dev-utils/CHANGELOG.json +++ b/packages/dev-utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "2.3.1", + "changes": [ + { + "note": "`revertWith` mocha extensions now accept Promise-like objects instead of just Promises", + "pr": 2031 + } + ] + }, { "version": "2.3.0", "changes": [ diff --git a/packages/dev-utils/src/chai_revert_error.ts b/packages/dev-utils/src/chai_revert_error.ts index a0fcd31103..a470ae3970 100644 --- a/packages/dev-utils/src/chai_revert_error.ts +++ b/packages/dev-utils/src/chai_revert_error.ts @@ -34,7 +34,7 @@ export function revertErrorHelper(_chai: Chai): void { return async function(this: ChaiAssertionInstance, expected: any, ...rest: any[]): Promise { const maybePromise = this._obj; // Make sure we're working with a promise. - chaiAssert(_chai, maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); + assertIsPromiseLike(_chai, maybePromise); // Wait for the promise to reject. let resolveValue; let rejectValue: any; @@ -58,7 +58,7 @@ export function revertErrorHelper(_chai: Chai): void { return async function(this: ChaiAssertionInstance, expected: any, ...rest: any[]): Promise { const maybePromise = this._obj; // Make sure we're working with a promise. - chaiAssert(_chai, maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); + assertIsPromiseLike(_chai, maybePromise); // Wait for the promise to resolve. if (!compareRevertErrors.call(this, _chai, await maybePromise, expected)) { // Wasn't handled by the comparison function so call the previous handler. @@ -133,3 +133,10 @@ function chaiFail(_chai: Chai, failMessage?: string, expected?: any, actual?: an const assert = new _chai.Assertion(); assert.assert(false, failMessage, undefined, expected, actual); } + +function assertIsPromiseLike(_chai: Chai, maybePromise: any): void { + if (maybePromise.then instanceof Function && maybePromise.catch instanceof Function) { + return; + } + chaiFail(_chai, `Expected ${maybePromise} to be a promise`, Promise.resolve(), maybePromise); +} diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index f47eea8aec..2dda41a7a3 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Add `OrderStatus` type", "pr": 1761 + }, + { + "note": "Add `OrderInfo`, `FillResults`, `MatchedFillResults`, `BatchMatchedFillResults` types", + "pr": 2031 } ] }, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 57ce884d27..f9c2205d83 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -810,3 +810,30 @@ export enum OrderTransferResults { MakerFeeAssetDataFailed, TransfersSuccessful, } + +export interface FillResults { + makerAssetFilledAmount: BigNumber; + takerAssetFilledAmount: BigNumber; + makerFeePaid: BigNumber; + takerFeePaid: BigNumber; +} + +export interface MatchedFillResults { + left: FillResults; + right: FillResults; + profitInLeftMakerAsset: BigNumber; + profitInRightMakerAsset: BigNumber; +} + +export interface BatchMatchedFillResults { + left: FillResults[]; + right: FillResults[]; + profitInLeftMakerAsset: BigNumber; + profitInRightMakerAsset: BigNumber; +} + +export interface OrderInfo { + orderStatus: number; + orderHash: string; + orderTakerAssetFilledAmount: BigNumber; +} diff --git a/packages/utils/CHANGELOG.json b/packages/utils/CHANGELOG.json index 9ac05c4b9e..7ed47a6ffc 100644 --- a/packages/utils/CHANGELOG.json +++ b/packages/utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "4.5.0", + "changes": [ + { + "note": "Add `SafeMathRevertErrors.SafeMathErrorCodes.Uint256DivisionByZero`", + "pr": 2031 + } + ] + }, { "version": "4.4.0", "changes": [ diff --git a/packages/utils/src/safe_math_revert_errors.ts b/packages/utils/src/safe_math_revert_errors.ts index 42cbf1e355..023e85e4b8 100644 --- a/packages/utils/src/safe_math_revert_errors.ts +++ b/packages/utils/src/safe_math_revert_errors.ts @@ -5,6 +5,7 @@ export enum SafeMathErrorCodes { Uint256AdditionOverflow, Uint256MultiplicationOverflow, Uint256SubtractionUnderflow, + Uint256DivisionByZero, } export class SafeMathError extends RevertError { diff --git a/yarn.lock b/yarn.lock index 69342ef56c..bc11be3614 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8653,7 +8653,8 @@ got@^6.7.1: graceful-fs@4.1.15, graceful-fs@^3.0.0, graceful-fs@^4.0.0, graceful-fs@^4.1.10, graceful-fs@^4.1.11, graceful-fs@^4.1.15, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.1.9, graceful-fs@^4.2.0, graceful-fs@~1.2.0: version "4.1.15" - resolved "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.15.tgz#ffb703e1066e8a0eeaa4c8b80ba9253eeefbfb00" + resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.1.15.tgz#ffb703e1066e8a0eeaa4c8b80ba9253eeefbfb00" + integrity sha512-6uHUhOPEBgQ24HM+r6b/QwWfZq+yiFcipKFrOFiBEnWdy5sdzYoi+pJeQaPI5qOLRFqWmAXUPQNsielzdLoecA== "graceful-readlink@>= 1.0.0": version "1.0.1" @@ -14531,16 +14532,6 @@ react-dom@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" -react-dom@^16.4.2: - version "16.8.6" - resolved "https://registry.npmjs.org/react-dom/-/react-dom-16.8.6.tgz#71d6303f631e8b0097f56165ef608f051ff6e10f" - integrity sha512-1nL7PIq9LTL3fthPqwkvr2zY7phIPjYrT0jp4HjyEQrEROnw4dG41VVwi/wfoCneoleqrNX7iAD+pXebJZwrwA== - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.13.6" - react-dom@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.5.2.tgz#b69ee47aa20bab5327b2b9d7c1fe2a30f2cfa9d7" @@ -14598,8 +14589,6 @@ react-highlight@0xproject/react-highlight#react-peer-deps: dependencies: highlight.js "^9.11.0" highlightjs-solidity "^1.0.5" - react "^16.4.2" - react-dom "^16.4.2" react-hot-loader@^4.3.3: version "4.3.4" @@ -14844,16 +14833,6 @@ react@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" -react@^16.4.2: - version "16.8.6" - resolved "https://registry.npmjs.org/react/-/react-16.8.6.tgz#ad6c3a9614fd3a4e9ef51117f54d888da01f2bbe" - integrity sha512-pC0uMkhLaHm11ZSJULfOBqV4tIZkx87ZLvbbQYunNixAAvjnC+snJCg0XQXn9VIsttVsbZP/H/ewzgsd5fxKXw== - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.13.6" - react@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react/-/react-16.5.2.tgz#19f6b444ed139baa45609eee6dc3d318b3895d42" @@ -15751,14 +15730,6 @@ schedule@^0.5.0: dependencies: object-assign "^4.1.1" -scheduler@^0.13.6: - version "0.13.6" - resolved "https://registry.npmjs.org/scheduler/-/scheduler-0.13.6.tgz#466a4ec332467b31a91b9bf74e5347072e4cd889" - integrity sha512-IWnObHt413ucAYKsD9J1QShUKkbKLQQHdxRyw73sw4FN26iWr3DY/H34xGPe4nmL1DwXyWmSWmMrA9TfQbE/XQ== - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - schema-utils@^0.4.4: version "0.4.7" resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-0.4.7.tgz#ba74f597d2be2ea880131746ee17d0a093c68187"