From 898213bb855de51e8762d2481eedc5c328df4761 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Wed, 31 Jul 2019 16:47:41 -0400 Subject: [PATCH] `@0x/contracts-exchange`: Update tests for moved types. --- contracts/exchange/CHANGELOG.json | 8 ++ .../contracts/test/TestIsolatedExchange.sol | 7 +- contracts/exchange/test/internal.ts | 59 ++++--------- .../exchange/test/isolated_fill_order.ts | 62 ++++++++++---- contracts/exchange/test/transactions.ts | 3 +- .../exchange/test/utils/exchange_wrapper.ts | 11 ++- .../utils/fill_order_combinatorial_utils.ts | 4 +- .../test/utils/fill_order_scenarios.ts | 7 -- .../test/utils/fill_order_simulator.ts | 3 +- .../test/utils/isolated_exchange_wrapper.ts | 21 ++++- .../exchange/test/utils/match_order_tester.ts | 17 ++-- .../test/utils/reference_functions.ts | 83 +------------------ contracts/exchange/test/wrapper.ts | 3 +- 13 files changed, 121 insertions(+), 167 deletions(-) diff --git a/contracts/exchange/CHANGELOG.json b/contracts/exchange/CHANGELOG.json index fd52350d72..cf51699254 100644 --- a/contracts/exchange/CHANGELOG.json +++ b/contracts/exchange/CHANGELOG.json @@ -117,6 +117,14 @@ { "note": "Rewrote _dispatchTransferFrom in Solidity", "pr": 2020 + }, + { + "note": "Add `TestIsolatedExchange` contract and `IsolatedExchangeWrapper` test class", + "pr": "TODO" + }, + { + "note": "Add `test/utils/reference_functions.ts` for `calculateFillResults`", + "pr": "TODO" } ] }, diff --git a/contracts/exchange/contracts/test/TestIsolatedExchange.sol b/contracts/exchange/contracts/test/TestIsolatedExchange.sol index 04c531a57b..9ebcee373a 100644 --- a/contracts/exchange/contracts/test/TestIsolatedExchange.sol +++ b/contracts/exchange/contracts/test/TestIsolatedExchange.sol @@ -42,7 +42,7 @@ contract TestIsolatedExchange is Exchange(1337) {} - /// @dev Overriden to only log arguments. + /// @dev Overriden to only log arguments and revert on certain assetDatas. function _dispatchTransferFrom( bytes32 orderHash, bytes memory assetData, @@ -59,6 +59,11 @@ contract TestIsolatedExchange is 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. diff --git a/contracts/exchange/test/internal.ts b/contracts/exchange/test/internal.ts index 6a544a2caa..4a55ee5cb1 100644 --- a/contracts/exchange/test/internal.ts +++ b/contracts/exchange/test/internal.ts @@ -1,28 +1,18 @@ import { + blockchainTests, bytes32Values, - chaiSetup, constants, - FillResults, - provider, + expect, testCombinatoriallyWithReferenceFuncAsync, - txDefaults, 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 { FillResults, Order, RevertReason, SignedOrder } from '@0x/types'; import { BigNumber, providerUtils, SafeMathRevertErrors } from '@0x/utils'; -import * as chai from 'chai'; import * as _ from 'lodash'; import { artifacts, TestExchangeInternalsContract, TestExchangeMathContract } from '../src'; -chaiSetup.configure(); -const expect = chai.expect; - -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - const { MAX_UINT256 } = constants; const emptyOrder: Order = { @@ -53,35 +43,28 @@ const emptySignedOrder: SignedOrder = { const safeMathErrorForCall = new SafeMathRevertErrors.SafeMathError(); -describe('Exchange math internal functions', () => { +// TODO(dorothy-zbornak): Move this to `exchange-libs` and `utils`. +blockchainTests.resets('Exchange math internal functions', env => { let chainId: number; let testExchange: TestExchangeMathContract; let divisionByZeroErrorForCall: Error | undefined; let roundingErrorForCall: Error | undefined; before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - chainId = await providerUtils.getChainIdAsync(provider); + chainId = await env.getChainIdAsync(); emptyOrder.domain.chainId = chainId; emptySignedOrder.domain.chainId = chainId; testExchange = await TestExchangeMathContract.deployFrom0xArtifactAsync( artifacts.TestExchangeMath, - provider, - txDefaults, + env.provider, + env.txDefaults, ); divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); roundingErrorForCall = new Error(RevertReason.RoundingError); 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! async function referenceIsRoundingErrorFloorAsync( numerator: BigNumber, @@ -314,7 +297,8 @@ describe('Exchange math internal functions', () => { }); }); -describe('Exchange core internal functions', () => { +// TODO(dorothy-zbornak): Add _settleOrder, _dispatchTransferFrom +blockchainTests.resets('Exchange core internal functions', env => { let chainId: number; let testExchange: TestExchangeInternalsContract; let safeMathErrorForSendTransaction: Error | undefined; @@ -322,20 +306,14 @@ describe('Exchange core internal functions', () => { let roundingErrorForCall: Error | undefined; before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - chainId = await providerUtils.getChainIdAsync(provider); + chainId = await providerUtils.getChainIdAsync(env.provider); emptyOrder.domain.chainId = chainId; emptySignedOrder.domain.chainId = chainId; testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync( artifacts.TestExchangeInternals, - provider, - txDefaults, + env.provider, + env.txDefaults, new BigNumber(chainId), ); divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); @@ -393,6 +371,7 @@ describe('Exchange core internal functions', () => { return product.dividedToIntegerBy(denominator); } + // TODO(dorothy-zbornak): Move this to `exchange-libs`. describe('addFillResults', async () => { function makeFillResults(value: BigNumber): FillResults { return { @@ -505,15 +484,7 @@ describe('Exchange core internal functions', () => { ); }); - 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(); - }); + blockchainTests.resets('updateFilledState', async ({ web3Wrapper }) => { async function referenceUpdateFilledStateAsync( takerAssetFilledAmount: BigNumber, orderTakerAssetFilledAmount: BigNumber, diff --git a/contracts/exchange/test/isolated_fill_order.ts b/contracts/exchange/test/isolated_fill_order.ts index 9ee94f613c..8baf1baaa8 100644 --- a/contracts/exchange/test/isolated_fill_order.ts +++ b/contracts/exchange/test/isolated_fill_order.ts @@ -2,19 +2,24 @@ import { blockchainTests, constants, expect, - FillResults, hexRandom, } from '@0x/contracts-test-utils'; +import { FillResults } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; -import { AssetBalances, IsolatedExchangeWrapper, Orderish } from './utils/isolated_exchange_wrapper'; +import { + AssetBalances, + createGoodAssetData, + IsolatedExchangeWrapper, + Orderish, +} from './utils/isolated_exchange_wrapper'; import { calculateFillResults } from './utils/reference_functions'; blockchainTests.resets.only('Isolated fillOrder() tests', env => { const { ZERO_AMOUNT } = constants; const TOMORROW = Math.floor(_.now() / 1000) + 60 * 60 * 24; - const ERC20_ASSET_DATA_LENGTH = 24; + const ONE_ETHER = new BigNumber(10).pow(18); const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH); const DEFAULT_ORDER: Orderish = { senderAddress: constants.NULL_ADDRESS, @@ -22,15 +27,15 @@ blockchainTests.resets.only('Isolated fillOrder() tests', env => { takerAddress: constants.NULL_ADDRESS, makerFee: ZERO_AMOUNT, takerFee: ZERO_AMOUNT, - makerAssetAmount: ZERO_AMOUNT, - takerAssetAmount: ZERO_AMOUNT, + makerAssetAmount: ONE_ETHER, + takerAssetAmount: ONE_ETHER.times(2), salt: ZERO_AMOUNT, feeRecipientAddress: constants.NULL_ADDRESS, expirationTimeSeconds: new BigNumber(TOMORROW), - makerAssetData: hexRandom(ERC20_ASSET_DATA_LENGTH), - takerAssetData: hexRandom(ERC20_ASSET_DATA_LENGTH), - makerFeeAssetData: hexRandom(ERC20_ASSET_DATA_LENGTH), - takerFeeAssetData: hexRandom(ERC20_ASSET_DATA_LENGTH), + makerAssetData: createGoodAssetData(), + takerAssetData: createGoodAssetData(), + makerFeeAssetData: createGoodAssetData(), + takerFeeAssetData: createGoodAssetData(), }; let takerAddress: string; let exchange: IsolatedExchangeWrapper; @@ -107,11 +112,40 @@ blockchainTests.resets.only('Isolated fillOrder() tests', env => { return balances; } - it('can fully fill an order', async () => { - const order = createOrder({ - makerAssetAmount: new BigNumber(1), - takerAssetAmount: new BigNumber(2), + describe('full fills', () => { + it('can fully fill an order', async () => { + const order = createOrder(); + return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + }); + + it('can\'t overfill an order', async () => { + const order = createOrder(); + return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount.times(1.01)); + }); + + it('pays maker and taker fees', async () => { + const order = createOrder({ + makerFee: ONE_ETHER.times(0.025), + takerFee: ONE_ETHER.times(0.035), + }); + return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); + }); + }); + + describe('partial fills', () => { + const takerAssetFillAmount = DEFAULT_ORDER.takerAssetAmount.dividedToIntegerBy(2); + + it('can partially fill an order', async () => { + const order = createOrder(); + return fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); + }); + + it('pays maker and taker fees', async () => { + const order = createOrder({ + makerFee: ONE_ETHER.times(0.025), + takerFee: ONE_ETHER.times(0.035), + }); + return fillOrderAndAssertResultsAsync(order, takerAssetFillAmount); }); - return fillOrderAndAssertResultsAsync(order, order.takerAssetAmount); }); }); 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..3152cd2fda 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -2,16 +2,19 @@ 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 { - BatchMatchedFillResults, BatchMatchOrder, - FillResults, LogDecoder, - MatchedFillResults, OrderInfo, orderUtils, Web3ProviderEngine, } from '@0x/contracts-test-utils'; -import { SignedOrder, SignedZeroExTransaction } from '@0x/types'; +import { + BatchMatchedFillResults, + FillResults, + MatchedFillResults, + 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 9cd360d360..de2b4adaba 100644 --- a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts +++ b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts @@ -5,9 +5,9 @@ import { ERC721Wrapper, MultiAssetProxyContract, } from '@0x/contracts-asset-proxy'; -import { constants, expect, 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 { LogWithDecodedArgs, TxData } from 'ethereum-types'; diff --git a/contracts/exchange/test/utils/fill_order_scenarios.ts b/contracts/exchange/test/utils/fill_order_scenarios.ts index 111068d5a2..d120560c17 100644 --- a/contracts/exchange/test/utils/fill_order_scenarios.ts +++ b/contracts/exchange/test/utils/fill_order_scenarios.ts @@ -82,13 +82,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 index 64580a6e27..e887839a67 100644 --- a/contracts/exchange/test/utils/isolated_exchange_wrapper.ts +++ b/contracts/exchange/test/utils/isolated_exchange_wrapper.ts @@ -1,8 +1,9 @@ -import { constants, FillResults, filterLogsToArguments, LogDecoder, txDefaults as testTxDefaults } from '@0x/contracts-test-utils'; +import { constants, filterLogsToArguments, LogDecoder, txDefaults as testTxDefaults } from '@0x/contracts-test-utils'; import { orderHashUtils } from '@0x/order-utils'; -import { OrderWithoutDomain, SignatureType } from '@0x/types'; +import { FillResults, 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 * as _ from 'lodash'; @@ -143,6 +144,22 @@ export function createBadSignature(type: SignatureType = SignatureType.EIP712): return `0x00${Buffer.from([type]).toString('hex')}`; } +const ERC20_ASSET_DATA_LENGTH = 24; + +/** + * Create asset data for the `TestIsolatedExchange` 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 `TestIsolatedExchange` 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: [] }; } diff --git a/contracts/exchange/test/utils/match_order_tester.ts b/contracts/exchange/test/utils/match_order_tester.ts index 9d8572e6b0..16f21f4cc9 100644 --- a/contracts/exchange/test/utils/match_order_tester.ts +++ b/contracts/exchange/test/utils/match_order_tester.ts @@ -1,16 +1,18 @@ import { ERC1155ProxyWrapper, ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy'; import { - BatchMatchedFillResults, - chaiSetup, ERC1155HoldingsByOwner, - FillResults, - MatchedFillResults, + 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 +20,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/utils/reference_functions.ts b/contracts/exchange/test/utils/reference_functions.ts index 5092215113..8fe0b53bdd 100644 --- a/contracts/exchange/test/utils/reference_functions.ts +++ b/contracts/exchange/test/utils/reference_functions.ts @@ -1,83 +1,8 @@ -import { constants, FillResults } from '@0x/contracts-test-utils'; -import { LibMathRevertErrors } from '@0x/order-utils'; -import { OrderWithoutDomain } from '@0x/types'; -import { AnyRevertError, BigNumber, SafeMathRevertErrors } from '@0x/utils'; +import { ReferenceFunctions as ExchangeLibsReferenceFunctions } from '@0x/contracts-exchange-libs'; +import { FillResults, OrderWithoutDomain } from '@0x/types'; +import { BigNumber } from '@0x/utils'; -const { MAX_UINT256 } = constants; - -export function isRoundingErrorFloor( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): boolean { - if (denominator.eq(0)) { - throw new LibMathRevertErrors.DivisionByZeroError(); - } - 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 (remainderTimes1000.isGreaterThan(MAX_UINT256)) { - // Solidity implementation won't actually throw. - throw new AnyRevertError(); - } - return isError; -} - -export function IsRoundingErrorCeil( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): boolean { - if (denominator.eq(0)) { - throw new LibMathRevertErrors.DivisionByZeroError(); - } - 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 (errorTimes1000.isGreaterThan(MAX_UINT256)) { - // Solidity implementation won't actually throw. - throw new AnyRevertError(); - } - return isError; -} - -export function safeGetPartialAmountFloor( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, -): BigNumber { - if (denominator.eq(0)) { - throw new LibMathRevertErrors.DivisionByZeroError(); - } - const isRoundingError = isRoundingErrorFloor(numerator, denominator, target); - if (isRoundingError) { - throw new LibMathRevertErrors.RoundingError(numerator, denominator, target); - } - const product = numerator.multipliedBy(target); - if (product.isGreaterThan(MAX_UINT256)) { - throw new SafeMathRevertErrors.SafeMathError( - SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow, - numerator, - denominator, - ); - } - return product.dividedToIntegerBy(denominator); -} +const { safeGetPartialAmountFloor } = ExchangeLibsReferenceFunctions; export function calculateFillResults( order: OrderWithoutDomain, 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';