From e954e9ca20b51edf5ea519774a11b158d739c27c Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Thu, 26 Sep 2019 15:59:12 -0700 Subject: [PATCH] update tests --- .../dev-utils/test/order_validation_utils.ts | 2 +- .../contracts/src/MixinExchangeWrapper.sol | 41 +-- .../contracts/src/MixinWeth.sol | 20 +- .../contracts/src/libs/LibConstants.sol | 1 - .../src/libs/LibForwarderRichErrors.sol | 24 +- .../src/test/TestProtocolFeeCollector.sol | 70 +++++ contracts/exchange-forwarder/package.json | 3 +- contracts/exchange-forwarder/src/artifacts.ts | 2 + contracts/exchange-forwarder/src/wrappers.ts | 1 + .../exchange-forwarder/test/forwarder.ts | 153 +++++---- .../test/utils/forwarder_test_factory.ts | 297 +++++++++--------- contracts/exchange-forwarder/tsconfig.json | 3 +- contracts/exchange/test/core.ts | 2 +- contracts/exchange/test/match_orders.ts | 2 +- contracts/exchange/test/transactions.ts | 2 +- .../exchange/test/utils/exchange_wrapper.ts | 95 +++--- .../utils/fill_order_combinatorial_utils.ts | 4 +- contracts/exchange/test/wrapper.ts | 2 +- .../test/balance_threshold_filter.ts | 2 +- contracts/extensions/test/dutch_auction.ts | 2 +- contracts/extensions/test/order_matcher.ts | 2 +- contracts/test-utils/src/order_factory.ts | 2 +- .../src/forwarder_revert_errors.ts | 16 +- packages/utils/src/revert_error.ts | 2 +- 24 files changed, 413 insertions(+), 337 deletions(-) create mode 100644 contracts/exchange-forwarder/contracts/src/test/TestProtocolFeeCollector.sol diff --git a/contracts/dev-utils/test/order_validation_utils.ts b/contracts/dev-utils/test/order_validation_utils.ts index c222131a01..8d4087fb75 100644 --- a/contracts/dev-utils/test/order_validation_utils.ts +++ b/contracts/dev-utils/test/order_validation_utils.ts @@ -94,7 +94,7 @@ describe('OrderValidationUtils/OrderTransferSimulatorUtils', () => { txDefaults, artifacts, ); - const exchangeWrapper = new ExchangeWrapper(exchange, provider); + const exchangeWrapper = new ExchangeWrapper(exchange); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(multiAssetProxy.address, owner); diff --git a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol index f2f7caf66d..46c35d10bc 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol @@ -57,26 +57,12 @@ contract MixinExchangeWrapper is ); address exchange = address(EXCHANGE); - - // Call `fillOrder` and handle any exceptions gracefully - assembly { - let success := call( - gas, // forward all gas - exchange, // call address of Exchange contract - 0, // transfer 0 wei - add(fillOrderCalldata, 32), // pointer to start of input (skip array length in first 32 bytes) - mload(fillOrderCalldata), // length of input - fillOrderCalldata, // write output over input - 160 // output size is 160 bytes - ) - if success { - mstore(fillResults, mload(fillOrderCalldata)) - mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32))) - mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64))) - mstore(add(fillResults, 96), mload(add(fillOrderCalldata, 96))) - mstore(add(fillResults, 128), mload(add(fillOrderCalldata, 128))) - } + (bool didSucceed, bytes memory returnData) = exchange.call(fillOrderCalldata); + if (didSucceed) { + assert(returnData.length == 160); + fillResults = abi.decode(returnData, (LibFillResults.FillResults)); } + // fillResults values will be 0 by default if call was unsuccessful return fillResults; } @@ -99,12 +85,14 @@ contract MixinExchangeWrapper is uint256 makerAssetAcquiredAmount ) { - // No fee or percentage fee + uint256 protocolFee = tx.gasprice.safeMul(EXCHANGE.protocolFeeMultiplier()); + + // No taker fee or percentage fee if (order.takerFee == 0 || order.takerFeeAssetData.equals(order.makerAssetData)) { // Attempt to sell the remaining amount of WETH LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow( order, - remainingTakerAssetFillAmount, + remainingTakerAssetFillAmount.safeSub(protocolFee), signature ); @@ -118,14 +106,13 @@ contract MixinExchangeWrapper is ); // WETH fee } else if (order.takerFeeAssetData.equals(order.takerAssetData)) { - uint256 protocolFee = tx.gasprice.safeMul(EXCHANGE.protocolFeeMultiplier()); // We will first sell WETH as the takerAsset, then use it to pay the takerFee. - // This ensures that we reserve enough to pay the fee. + // This ensures that we reserve enough to pay the taker and protocol fees. uint256 takerAssetFillAmount = LibMath.getPartialAmountCeil( order.takerAssetAmount, - order.takerAssetAmount.safeAdd(order.takerFee).safeAdd(protocolFee), - remainingTakerAssetFillAmount + order.takerAssetAmount.safeAdd(order.takerFee), + remainingTakerAssetFillAmount.safeSub(protocolFee) ); LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow( @@ -222,7 +209,7 @@ contract MixinExchangeWrapper is uint256 makerAssetAcquiredAmount ) { - // No fee or WETH fee + // No taker fee or WETH fee if (order.takerFee == 0 || order.takerFeeAssetData.equals(order.takerAssetData)) { // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = LibMath.getPartialAmountCeil( @@ -238,7 +225,7 @@ contract MixinExchangeWrapper is signature ); - // WETH is also spent on the taker fee, so we add it here. + // WETH is also spent on the protocol and taker fees, so we add it here. wethSpentAmount = singleFillResults.takerAssetFilledAmount.safeAdd( singleFillResults.takerFeePaid ).safeAdd( diff --git a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol index 1b521c680e..13f15e6f46 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol @@ -47,19 +47,19 @@ contract MixinWeth is internal { if (msg.value == 0) { - LibRichErrors.rrevert(LibForwarderRichErrors.MsgValueCantEqualZeroError()); + LibRichErrors.rrevert(LibForwarderRichErrors.MsgValueCannotEqualZeroError()); } ETHER_TOKEN.deposit.value(msg.value)(); } /// @dev Transfers feePercentage of WETH spent on primary orders to feeRecipient. /// Refunds any excess ETH to msg.sender. - /// @param wethSold Amount of WETH sold when filling primary orders. + /// @param wethSpent Amount of WETH spent when filling orders. /// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient. /// @param feeRecipient Address that will receive ETH when orders are filled. /// @return ethFee Amount paid to feeRecipient as a percentage fee on the total WETH sold. function _transferEthFeeAndRefund( - uint256 wethSold, + uint256 wethSpent, uint256 feePercentage, address payable feeRecipient ) @@ -73,22 +73,22 @@ contract MixinWeth is )); } - // Ensure that no extra WETH owned by this contract has been sold. - if (wethSold > msg.value) { - LibRichErrors.rrevert(LibForwarderRichErrors.OversoldWethError( - wethSold, + // Ensure that no extra WETH owned by this contract has been spent. + if (wethSpent > msg.value) { + LibRichErrors.rrevert(LibForwarderRichErrors.OverspentWethError( + wethSpent, msg.value )); } - // Calculate amount of WETH that hasn't been sold. - uint256 wethRemaining = msg.value.safeSub(wethSold); + // Calculate amount of WETH that hasn't been spent. + uint256 wethRemaining = msg.value.safeSub(wethSpent); // Calculate ETH fee to pay to feeRecipient. ethFee = LibMath.getPartialAmountFloor( feePercentage, PERCENTAGE_DENOMINATOR, - wethSold + wethSpent ); // Ensure fee is less than amount of WETH remaining. diff --git a/contracts/exchange-forwarder/contracts/src/libs/LibConstants.sol b/contracts/exchange-forwarder/contracts/src/libs/LibConstants.sol index 39c132e07e..8be43049d5 100644 --- a/contracts/exchange-forwarder/contracts/src/libs/LibConstants.sol +++ b/contracts/exchange-forwarder/contracts/src/libs/LibConstants.sol @@ -21,7 +21,6 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol"; import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; -import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; contract LibConstants { diff --git a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol index c1013a547f..3210fb2892 100644 --- a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol +++ b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol @@ -51,9 +51,9 @@ library LibForwarderRichErrors { bytes4 internal constant INSUFFICIENT_ETH_FOR_FEE_ERROR_SELECTOR = 0xecf40fd9; - // bytes4(keccak256("OversoldWethError(uint256,uint256)")) - bytes4 internal constant OVERSOLD_WETH_ERROR_SELECTOR = - 0x5cc555c8; + // bytes4(keccak256("OverspentWethError(uint256,uint256)")) + bytes4 internal constant OVERSPENT_WETH_ERROR_SELECTOR = + 0xcdcbed5d; // bytes4(keccak256("TransferFailedError(bytes)")) bytes4 internal constant TRANSFER_FAILED_ERROR_SELECTOR = @@ -63,9 +63,9 @@ library LibForwarderRichErrors { bytes4 internal constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR = 0x08b18698; - // bytes4(keccak256("MsgValueCantEqualZeroError()")) - bytes4 internal constant MSG_VALUE_CANT_EQUAL_ZERO_ERROR_SELECTOR = - 0x1213e1d6; + // bytes4(keccak256("MsgValueCannotEqualZeroError()")) + bytes4 internal constant MSG_VALUE_CANNOT_EQUAL_ZERO_ERROR_SELECTOR = + 0x8c0e562b; // bytes4(keccak256("Erc721AmountMustEqualOneError(uint256)")) bytes4 internal constant ERC721_AMOUNT_MUST_EQUAL_ONE_ERROR_SELECTOR = @@ -164,8 +164,8 @@ library LibForwarderRichErrors { ); } - function OversoldWethError( - uint256 wethSold, + function OverspentWethError( + uint256 wethSpent, uint256 msgValue ) internal @@ -173,8 +173,8 @@ library LibForwarderRichErrors { returns (bytes memory) { return abi.encodeWithSelector( - OVERSOLD_WETH_ERROR_SELECTOR, - wethSold, + OVERSPENT_WETH_ERROR_SELECTOR, + wethSpent, msgValue ); } @@ -205,12 +205,12 @@ library LibForwarderRichErrors { ); } - function MsgValueCantEqualZeroError() + function MsgValueCannotEqualZeroError() internal pure returns (bytes memory) { - return abi.encodeWithSelector(MSG_VALUE_CANT_EQUAL_ZERO_ERROR_SELECTOR); + return abi.encodeWithSelector(MSG_VALUE_CANNOT_EQUAL_ZERO_ERROR_SELECTOR); } function Erc721AmountMustEqualOneError( diff --git a/contracts/exchange-forwarder/contracts/src/test/TestProtocolFeeCollector.sol b/contracts/exchange-forwarder/contracts/src/test/TestProtocolFeeCollector.sol new file mode 100644 index 0000000000..9db7296212 --- /dev/null +++ b/contracts/exchange-forwarder/contracts/src/test/TestProtocolFeeCollector.sol @@ -0,0 +1,70 @@ +/* + + 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.9; +pragma experimental ABIEncoderV2; + +import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol"; +import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol"; +import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; + + + // solhint-disable no-unused-vars +contract TestProtocolFeeCollector { + + address private _wethAddress; + address private _wethAssetProxyAddress; + + constructor ( + address wethAddress, + address wethAssetProxyAddress + ) + public + { + _wethAddress = wethAddress; + _wethAssetProxyAddress = wethAssetProxyAddress; + } + + /// @dev Pays a protocol fee in WETH (Forwarder orders will always pay protocol fees in WETH). + /// @param makerAddress The address of the order's maker. + /// @param payerAddress The address of the protocol fee payer. + /// @param protocolFeePaid The protocol fee that should be paid. + function payProtocolFee( + address makerAddress, + address payerAddress, + uint256 protocolFeePaid + ) + external + payable + { + assert(msg.value == 0); + + IAssetProxy wethAssetProxy = IAssetProxy(_wethAssetProxyAddress); + bytes memory wethAssetData = abi.encodeWithSelector( + IAssetData(address(0)).ERC20Token.selector, + _wethAddress + ); + // Transfer the protocol fee to this address in WETH. + wethAssetProxy.transferFrom( + wethAssetData, + payerAddress, + address(this), + protocolFeePaid + ); + } +} diff --git a/contracts/exchange-forwarder/package.json b/contracts/exchange-forwarder/package.json index 33c072824f..e2d9729f86 100644 --- a/contracts/exchange-forwarder/package.json +++ b/contracts/exchange-forwarder/package.json @@ -11,6 +11,7 @@ }, "scripts": { "build": "yarn pre_build && tsc -b", + "build:ts": "tsc -b", "build:ci": "yarn build", "pre_build": "run-s compile contracts:gen generate_contract_wrappers", "test": "yarn run_mocha", @@ -34,7 +35,7 @@ "compile:truffle": "truffle compile" }, "config": { - "abis": "./generated-artifacts/@(Forwarder|IAssets|IForwarder|IForwarderCore|LibConstants|LibForwarderRichErrors|MixinAssets|MixinExchangeWrapper|MixinForwarderCore|MixinWeth).json", + "abis": "./generated-artifacts/@(Forwarder|IAssets|IForwarder|IForwarderCore|LibConstants|LibForwarderRichErrors|MixinAssets|MixinExchangeWrapper|MixinForwarderCore|MixinWeth|TestProtocolFeeCollector).json", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." }, "repository": { diff --git a/contracts/exchange-forwarder/src/artifacts.ts b/contracts/exchange-forwarder/src/artifacts.ts index 9643be939a..de0fa3fe35 100644 --- a/contracts/exchange-forwarder/src/artifacts.ts +++ b/contracts/exchange-forwarder/src/artifacts.ts @@ -15,6 +15,7 @@ import * as MixinAssets from '../generated-artifacts/MixinAssets.json'; import * as MixinExchangeWrapper from '../generated-artifacts/MixinExchangeWrapper.json'; import * as MixinForwarderCore from '../generated-artifacts/MixinForwarderCore.json'; import * as MixinWeth from '../generated-artifacts/MixinWeth.json'; +import * as TestProtocolFeeCollector from '../generated-artifacts/TestProtocolFeeCollector.json'; export const artifacts = { Forwarder: Forwarder as ContractArtifact, MixinAssets: MixinAssets as ContractArtifact, @@ -26,4 +27,5 @@ export const artifacts = { IForwarderCore: IForwarderCore as ContractArtifact, LibConstants: LibConstants as ContractArtifact, LibForwarderRichErrors: LibForwarderRichErrors as ContractArtifact, + TestProtocolFeeCollector: TestProtocolFeeCollector as ContractArtifact, }; diff --git a/contracts/exchange-forwarder/src/wrappers.ts b/contracts/exchange-forwarder/src/wrappers.ts index 23213da16e..1d9bd5b205 100644 --- a/contracts/exchange-forwarder/src/wrappers.ts +++ b/contracts/exchange-forwarder/src/wrappers.ts @@ -13,3 +13,4 @@ export * from '../generated-wrappers/mixin_assets'; export * from '../generated-wrappers/mixin_exchange_wrapper'; export * from '../generated-wrappers/mixin_forwarder_core'; export * from '../generated-wrappers/mixin_weth'; +export * from '../generated-wrappers/test_protocol_fee_collector'; diff --git a/contracts/exchange-forwarder/test/forwarder.ts b/contracts/exchange-forwarder/test/forwarder.ts index 992218ff95..025a491a84 100644 --- a/contracts/exchange-forwarder/test/forwarder.ts +++ b/contracts/exchange-forwarder/test/forwarder.ts @@ -14,21 +14,24 @@ import { import { assetDataUtils, ForwarderRevertErrors } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; -import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; -import { artifacts, ForwarderContract, ForwarderTestFactory, ForwarderWrapper } from '../src'; +import { + artifacts, + ForwarderContract, + ForwarderTestFactory, + ForwarderWrapper, + TestProtocolFeeCollectorContract, +} from '../src'; const DECIMALS_DEFAULT = 18; blockchainTests(ContractName.Forwarder, env => { - let chainId: number; - let makerAddress: string; let owner: string; + let makerAddress: string; let takerAddress: string; let orderFeeRecipientAddress: string; let forwarderFeeRecipientAddress: string; let defaultMakerAssetAddress: string; - let wethAssetData: string; let weth: DummyERC20TokenContract; let erc20Token: DummyERC20TokenContract; @@ -36,22 +39,26 @@ blockchainTests(ContractName.Forwarder, env => { let erc721Token: DummyERC721TokenContract; let forwarderContract: ForwarderContract; let wethContract: WETH9Contract; + let exchangeContract: ExchangeContract; + let protocolFeeCollector: TestProtocolFeeCollectorContract; + let forwarderWrapper: ForwarderWrapper; let exchangeWrapper: ExchangeWrapper; + let erc20Wrapper: ERC20Wrapper; let orderFactory: OrderFactory; let forwarderTestFactory: ForwarderTestFactory; - let erc20Wrapper: ERC20Wrapper; - let tx: TransactionReceiptWithDecodedLogs; + let chainId: number; + let wethAssetData: string; let erc721MakerAssetIds: BigNumber[]; - const gasPrice = new BigNumber(constants.DEFAULT_GAS_PRICE); + + const GAS_PRICE = new BigNumber(env.txDefaults.gasPrice || constants.DEFAULT_GAS_PRICE); + const PROTOCOL_FEE_MULTIPLIER = new BigNumber(150); + const PROTOCOL_FEE = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER); before(async () => { - await env.blockchainLifecycle.startAsync(); - - chainId = await env.getChainIdAsync(); - + // Set up addresses const accounts = await env.getAccountAddressesAsync(); const usedAddresses = ([ owner, @@ -61,24 +68,27 @@ blockchainTests(ContractName.Forwarder, env => { forwarderFeeRecipientAddress, ] = accounts); - const erc721Wrapper = new ERC721Wrapper(env.provider, usedAddresses, owner); - erc20Wrapper = new ERC20Wrapper(env.provider, usedAddresses, owner); - - const numDummyErc20ToDeploy = 2; - [erc20Token, secondErc20Token] = await erc20Wrapper.deployDummyTokensAsync( - numDummyErc20ToDeploy, - constants.DUMMY_TOKEN_DECIMALS, + // Set up Exchange + chainId = await env.getChainIdAsync(); + exchangeContract = await ExchangeContract.deployFrom0xArtifactAsync( + exchangeArtifacts.Exchange, + env.provider, + env.txDefaults, + {}, + new BigNumber(chainId), ); + exchangeWrapper = new ExchangeWrapper(exchangeContract); + // Set up ERC20 + erc20Wrapper = new ERC20Wrapper(env.provider, usedAddresses, owner); + [erc20Token, secondErc20Token] = await erc20Wrapper.deployDummyTokensAsync(2, constants.DUMMY_TOKEN_DECIMALS); const erc20Proxy = await erc20Wrapper.deployProxyAsync(); - await erc20Wrapper.setBalancesAndAllowancesAsync(); - - [erc721Token] = await erc721Wrapper.deployDummyTokensAsync(); - const erc721Proxy = await erc721Wrapper.deployProxyAsync(); - await erc721Wrapper.setBalancesAndAllowancesAsync(); - const erc721Balances = await erc721Wrapper.getBalancesAsync(); - erc721MakerAssetIds = erc721Balances[makerAddress][erc721Token.address]; + await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); + await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeContract.address, { + from: owner, + }); + // Set up WETH wethContract = await WETH9Contract.deployFrom0xArtifactAsync( erc20Artifacts.WETH9, env.provider, @@ -86,59 +96,68 @@ blockchainTests(ContractName.Forwarder, env => { {}, ); weth = new DummyERC20TokenContract(wethContract.address, env.provider); - erc20Wrapper.addDummyTokenContract(weth); - wethAssetData = assetDataUtils.encodeERC20AssetData(wethContract.address); - const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( - exchangeArtifacts.Exchange, + erc20Wrapper.addDummyTokenContract(weth); + await erc20Wrapper.setBalancesAndAllowancesAsync(); + + // Set up ERC721 + const erc721Wrapper = new ERC721Wrapper(env.provider, usedAddresses, owner); + [erc721Token] = await erc721Wrapper.deployDummyTokensAsync(); + const erc721Proxy = await erc721Wrapper.deployProxyAsync(); + await erc721Wrapper.setBalancesAndAllowancesAsync(); + const erc721Balances = await erc721Wrapper.getBalancesAsync(); + erc721MakerAssetIds = erc721Balances[makerAddress][erc721Token.address]; + await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); + await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeContract.address, { + from: owner, + }); + + // Set up Protocol Fee Collector + protocolFeeCollector = await TestProtocolFeeCollectorContract.deployFrom0xArtifactAsync( + artifacts.TestProtocolFeeCollector, env.provider, env.txDefaults, {}, - new BigNumber(chainId), + wethContract.address, + erc20Proxy.address, ); - exchangeWrapper = new ExchangeWrapper(exchangeInstance, env.provider); - await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); - await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); - - await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeInstance.address, { - from: owner, - }); - await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeInstance.address, { + await exchangeContract.setProtocolFeeMultiplier.awaitTransactionSuccessAsync(PROTOCOL_FEE_MULTIPLIER); + await exchangeContract.setProtocolFeeCollectorAddress.awaitTransactionSuccessAsync( + protocolFeeCollector.address, + ); + await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(protocolFeeCollector.address, { from: owner, }); + // Set defaults defaultMakerAssetAddress = erc20Token.address; - const defaultTakerAssetAddress = wethContract.address; const defaultOrderParams = { makerAddress, feeRecipientAddress: orderFeeRecipientAddress, - makerAssetData: assetDataUtils.encodeERC20AssetData(defaultMakerAssetAddress), - takerAssetData: assetDataUtils.encodeERC20AssetData(defaultTakerAssetAddress), makerAssetAmount: Web3Wrapper.toBaseUnitAmount(200, DECIMALS_DEFAULT), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(10, DECIMALS_DEFAULT), - makerFeeAssetData: assetDataUtils.encodeERC20AssetData(defaultMakerAssetAddress), - takerFeeAssetData: assetDataUtils.encodeERC20AssetData(defaultMakerAssetAddress), makerFee: Web3Wrapper.toBaseUnitAmount(0, DECIMALS_DEFAULT), takerFee: Web3Wrapper.toBaseUnitAmount(0, DECIMALS_DEFAULT), - exchangeAddress: exchangeInstance.address, + exchangeAddress: exchangeContract.address, chainId, }; - const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; - orderFactory = new OrderFactory(privateKey, defaultOrderParams); + // Set up Forwarder forwarderContract = await ForwarderContract.deployFrom0xArtifactAsync( artifacts.Forwarder, env.provider, env.txDefaults, {}, - exchangeInstance.address, + exchangeContract.address, wethAssetData, ); forwarderWrapper = new ForwarderWrapper(forwarderContract, env.provider); - await forwarderWrapper.approveMakerAssetProxyAsync(defaultOrderParams.makerAssetData, { from: takerAddress }); erc20Wrapper.addTokenOwnerAddress(forwarderContract.address); + // Set up factories + const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; + orderFactory = new OrderFactory(privateKey, defaultOrderParams); forwarderTestFactory = new ForwarderTestFactory( exchangeWrapper, forwarderWrapper, @@ -146,16 +165,18 @@ blockchainTests(ContractName.Forwarder, env => { forwarderContract.address, makerAddress, takerAddress, + protocolFeeCollector.address, orderFeeRecipientAddress, forwarderFeeRecipientAddress, weth.address, - gasPrice, + GAS_PRICE, + PROTOCOL_FEE_MULTIPLIER, ); }); blockchainTests.resets('constructor', () => { it('should revert if assetProxy is unregistered', async () => { - const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( + const exchange = await ExchangeContract.deployFrom0xArtifactAsync( exchangeArtifacts.Exchange, env.provider, env.txDefaults, @@ -168,7 +189,7 @@ blockchainTests(ContractName.Forwarder, env => { env.provider, env.txDefaults, {}, - exchangeInstance.address, + exchange.address, wethAssetData, ) as any) as sendTransactionResult; @@ -223,7 +244,7 @@ blockchainTests(ContractName.Forwarder, env => { const takerEthBalanceBefore = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); // Execute test case - tx = await forwarderWrapper.marketSellOrdersWithEthAsync([order], { + const tx = await forwarderWrapper.marketSellOrdersWithEthAsync([order], { value: ethValue, from: takerAddress, }); @@ -231,7 +252,7 @@ blockchainTests(ContractName.Forwarder, env => { const takerEthBalanceAfter = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); const forwarderEthBalance = await env.web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); const newBalances = await erc20Wrapper.getBalancesAsync(); - const totalEthSpent = gasPrice.times(tx.gasUsed); + const totalEthSpent = GAS_PRICE.times(tx.gasUsed); // Validate test case expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); @@ -273,15 +294,15 @@ blockchainTests(ContractName.Forwarder, env => { }); it('should refund remaining ETH if amount is greater than takerAssetAmount', async () => { const order = await orderFactory.newSignedOrderAsync(); - const ethValue = order.takerAssetAmount.plus(2); + const ethValue = order.takerAssetAmount.plus(PROTOCOL_FEE).plus(2); const takerEthBalanceBefore = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); - tx = await forwarderWrapper.marketSellOrdersWithEthAsync([order], { + const tx = await forwarderWrapper.marketSellOrdersWithEthAsync([order], { value: ethValue, from: takerAddress, }); const takerEthBalanceAfter = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); - const totalEthSpent = order.takerAssetAmount.plus(gasPrice.times(tx.gasUsed)); + const totalEthSpent = order.takerAssetAmount.plus(PROTOCOL_FEE).plus(GAS_PRICE.times(tx.gasUsed)); expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); }); @@ -527,7 +548,7 @@ blockchainTests(ContractName.Forwarder, env => { const fillableOrder = await orderFactory.newSignedOrderAsync(); await forwarderTestFactory.marketBuyTestAsync([cancelledOrder, fillableOrder], 1.5, erc20Token); }); - it('Should buy slightly greater MakerAsset when exchange rate is rounded', async () => { + it('Should buy slightly greater makerAsset when exchange rate is rounded', async () => { // The 0x Protocol contracts round the exchange rate in favor of the Maker. // In this case, the taker must round up how much they're going to spend, which // in turn increases the amount of MakerAsset being purchased. @@ -553,13 +574,14 @@ blockchainTests(ContractName.Forwarder, env => { }); const desiredMakerAssetFillAmount = new BigNumber('5'); const makerAssetFillAmount = new BigNumber('6'); - const ethValue = new BigNumber('4'); + const primaryTakerAssetFillAmount = new BigNumber('4'); + const ethValue = primaryTakerAssetFillAmount.plus(PROTOCOL_FEE); const erc20Balances = await erc20Wrapper.getBalancesAsync(); const takerEthBalanceBefore = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); // Execute test case - tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([order], desiredMakerAssetFillAmount, { + const tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([order], desiredMakerAssetFillAmount, { value: ethValue, from: takerAddress, }); @@ -567,8 +589,7 @@ blockchainTests(ContractName.Forwarder, env => { const takerEthBalanceAfter = await env.web3Wrapper.getBalanceInWeiAsync(takerAddress); const forwarderEthBalance = await env.web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); const newBalances = await erc20Wrapper.getBalancesAsync(); - const primaryTakerAssetFillAmount = ethValue; - const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + const totalEthSpent = ethValue.plus(GAS_PRICE.times(tx.gasUsed)); // Validate test case expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); @@ -588,6 +609,8 @@ blockchainTests(ContractName.Forwarder, env => { expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); it('Should buy slightly greater MakerAsset when exchange rate is rounded (Regression Test)', async () => { + // Disable protocol fees for regression test + await exchangeContract.setProtocolFeeCollectorAddress.awaitTransactionSuccessAsync(constants.NULL_ADDRESS); // Order taken from a transaction on mainnet that failed due to a rounding error. const order = await orderFactory.newSignedOrderAsync({ makerAssetAmount: new BigNumber('268166666666666666666'), @@ -608,7 +631,7 @@ blockchainTests(ContractName.Forwarder, env => { .times(order.makerAssetAmount) .dividedToIntegerBy(order.takerAssetAmount); // Execute test case - tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([order], desiredMakerAssetFillAmount, { + const tx = await forwarderWrapper.marketBuyOrdersWithEthAsync([order], desiredMakerAssetFillAmount, { value: ethValue, from: takerAddress, }); @@ -617,7 +640,7 @@ blockchainTests(ContractName.Forwarder, env => { const forwarderEthBalance = await env.web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); const newBalances = await erc20Wrapper.getBalancesAsync(); const primaryTakerAssetFillAmount = ethValue; - const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + const totalEthSpent = primaryTakerAssetFillAmount.plus(GAS_PRICE.times(tx.gasUsed)); // Validate test case expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); @@ -661,7 +684,7 @@ blockchainTests(ContractName.Forwarder, env => { const order = await orderFactory.newSignedOrderAsync(); const forwarderFeePercentage = new BigNumber(2); const ethFee = ForwarderTestFactory.getPercentageOfValue( - order.takerAssetAmount.times(0.5), + order.takerAssetAmount.times(0.5).plus(PROTOCOL_FEE), forwarderFeePercentage, ); diff --git a/contracts/exchange-forwarder/test/utils/forwarder_test_factory.ts b/contracts/exchange-forwarder/test/utils/forwarder_test_factory.ts index b97124e145..2d7380621a 100644 --- a/contracts/exchange-forwarder/test/utils/forwarder_test_factory.ts +++ b/contracts/exchange-forwarder/test/utils/forwarder_test_factory.ts @@ -2,118 +2,31 @@ import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; import { DummyERC20TokenContract } from '@0x/contracts-erc20'; import { DummyERC721TokenContract } from '@0x/contracts-erc721'; import { ExchangeWrapper } from '@0x/contracts-exchange'; -import { chaiSetup, constants, ERC20BalancesByOwner, OrderStatus, web3Wrapper } from '@0x/contracts-test-utils'; +import { constants, ERC20BalancesByOwner, expect, OrderStatus, web3Wrapper } from '@0x/contracts-test-utils'; import { OrderInfo, SignedOrder } from '@0x/types'; import { BigNumber, RevertError } from '@0x/utils'; -import * as chai from 'chai'; import * as _ from 'lodash'; import { ForwarderWrapper } from './forwarder_wrapper'; -chaiSetup.configure(); -const expect = chai.expect; - // Necessary bookkeeping to validate Forwarder results interface ForwarderFillState { takerAssetFillAmount: BigNumber; makerAssetFillAmount: BigNumber; + protocolFees: BigNumber; wethFees: BigNumber; percentageFees: BigNumber; maxOversoldWeth: BigNumber; maxOverboughtMakerAsset: BigNumber; } -// Simulates filling some orders via the Forwarder contract. For example, if -// orders = [A, B, C, D] and fractionalNumberOfOrdersToFill = 2.3, then -// we simulate A and B being completely filled, and 0.3 * C being filled. -function computeExpectedResults( - orders: SignedOrder[], - ordersInfoBefore: OrderInfo[], - fractionalNumberOfOrdersToFill: number, -): ForwarderFillState { - const currentState = { - takerAssetFillAmount: constants.ZERO_AMOUNT, - makerAssetFillAmount: constants.ZERO_AMOUNT, - wethFees: constants.ZERO_AMOUNT, - percentageFees: constants.ZERO_AMOUNT, - maxOversoldWeth: constants.ZERO_AMOUNT, - maxOverboughtMakerAsset: constants.ZERO_AMOUNT, - }; - let remainingOrdersToFill = fractionalNumberOfOrdersToFill; - - for (const [i, order] of orders.entries()) { - if (remainingOrdersToFill === 0) { - break; - } - - if (ordersInfoBefore[i].orderStatus !== OrderStatus.Fillable) { - // If the order is not fillable, skip over it but still count it towards fractionalNumberOfOrdersToFill - remainingOrdersToFill = Math.max(remainingOrdersToFill - 1, 0); - continue; - } - - let makerAssetAmount; - let takerAssetAmount; - let takerFee; - if (remainingOrdersToFill < 1) { - makerAssetAmount = order.makerAssetAmount.times(remainingOrdersToFill).integerValue(); - takerAssetAmount = order.takerAssetAmount.times(remainingOrdersToFill).integerValue(); - takerFee = order.takerFee.times(remainingOrdersToFill).integerValue(); - - // Up to 1 wei worth of WETH will be oversold on the last order due to rounding - currentState.maxOversoldWeth = new BigNumber(1); - // Equivalently, up to 1 wei worth of maker asset will be overbought - currentState.maxOverboughtMakerAsset = currentState.maxOversoldWeth - .times(order.makerAssetAmount) - .dividedToIntegerBy(order.takerAssetAmount); - } else { - makerAssetAmount = order.makerAssetAmount; - takerAssetAmount = order.takerAssetAmount; - takerFee = order.takerFee; - } - - // Accounting for partially filled orders - // As with unfillable orders, these still count as 1 towards fractionalNumberOfOrdersToFill - const takerAssetFilled = ordersInfoBefore[i].orderTakerAssetFilledAmount; - const makerAssetFilled = takerAssetFilled - .times(order.makerAssetAmount) - .dividedToIntegerBy(order.takerAssetAmount); - takerAssetAmount = BigNumber.max(takerAssetAmount.minus(takerAssetFilled), constants.ZERO_AMOUNT); - makerAssetAmount = BigNumber.max(makerAssetAmount.minus(makerAssetFilled), constants.ZERO_AMOUNT); - - currentState.takerAssetFillAmount = currentState.takerAssetFillAmount.plus(takerAssetAmount); - currentState.makerAssetFillAmount = currentState.makerAssetFillAmount.plus(makerAssetAmount); - - if (order.takerFeeAssetData === order.makerAssetData) { - currentState.percentageFees = currentState.percentageFees.plus(takerFee); - } else if (order.takerFeeAssetData === order.takerAssetData) { - currentState.wethFees = currentState.wethFees.plus(takerFee); - } - - remainingOrdersToFill = Math.max(remainingOrdersToFill - 1, 0); - } - - return currentState; -} - // Since bignumber is not compatible with chai's within -function expectBalanceWithin(balance: BigNumber, low: BigNumber, high: BigNumber): void { - expect(balance).to.be.bignumber.gte(low); - expect(balance).to.be.bignumber.lte(high); +function expectBalanceWithin(balance: BigNumber, low: BigNumber, high: BigNumber, message?: string): void { + expect(balance, message).to.be.bignumber.gte(low); + expect(balance, message).to.be.bignumber.lte(high); } export class ForwarderTestFactory { - private readonly _exchangeWrapper: ExchangeWrapper; - private readonly _forwarderWrapper: ForwarderWrapper; - private readonly _erc20Wrapper: ERC20Wrapper; - private readonly _forwarderAddress: string; - private readonly _makerAddress: string; - private readonly _takerAddress: string; - private readonly _orderFeeRecipientAddress: string; - private readonly _forwarderFeeRecipientAddress: string; - private readonly _wethAddress: string; - private readonly _gasPrice: BigNumber; - public static getPercentageOfValue(value: BigNumber, percentage: BigNumber): BigNumber { const numerator = constants.PERCENTAGE_DENOMINATOR.times(percentage).dividedToIntegerBy(100); const newValue = value.times(numerator).dividedToIntegerBy(constants.PERCENTAGE_DENOMINATOR); @@ -121,28 +34,19 @@ export class ForwarderTestFactory { } constructor( - exchangeWrapper: ExchangeWrapper, - forwarderWrapper: ForwarderWrapper, - erc20Wrapper: ERC20Wrapper, - forwarderAddress: string, - makerAddress: string, - takerAddress: string, - orderFeeRecipientAddress: string, - forwarderFeeRecipientAddress: string, - wethAddress: string, - gasPrice: BigNumber, - ) { - this._exchangeWrapper = exchangeWrapper; - this._forwarderWrapper = forwarderWrapper; - this._erc20Wrapper = erc20Wrapper; - this._forwarderAddress = forwarderAddress; - this._makerAddress = makerAddress; - this._takerAddress = takerAddress; - this._orderFeeRecipientAddress = orderFeeRecipientAddress; - this._forwarderFeeRecipientAddress = forwarderFeeRecipientAddress; - this._wethAddress = wethAddress; - this._gasPrice = gasPrice; - } + private readonly _exchangeWrapper: ExchangeWrapper, + private readonly _forwarderWrapper: ForwarderWrapper, + private readonly _erc20Wrapper: ERC20Wrapper, + private readonly _forwarderAddress: string, + private readonly _makerAddress: string, + private readonly _takerAddress: string, + private readonly _protocolFeeCollectorAddress: string, + private readonly _orderFeeRecipientAddress: string, + private readonly _forwarderFeeRecipientAddress: string, + private readonly _wethAddress: string, + private readonly _gasPrice: BigNumber, + private readonly _protocolFeeMultiplier: BigNumber, + ) {} public async marketBuyTestAsync( orders: SignedOrder[], @@ -167,22 +71,18 @@ export class ForwarderTestFactory { const ordersInfoBefore = await Promise.all(orders.map(order => this._exchangeWrapper.getOrderInfoAsync(order))); const orderStatusesBefore = ordersInfoBefore.map(orderInfo => orderInfo.orderStatus); - const expectedResults = computeExpectedResults(orders, ordersInfoBefore, fractionalNumberOfOrdersToFill); - const ethSpentOnForwarderFee = ForwarderTestFactory.getPercentageOfValue( - expectedResults.takerAssetFillAmount, - forwarderFeePercentage, - ); + const expectedResults = this._computeExpectedResults(orders, ordersInfoBefore, fractionalNumberOfOrdersToFill); + const wethSpent = expectedResults.takerAssetFillAmount + .plus(expectedResults.protocolFees) + .plus(expectedResults.wethFees) + .plus(expectedResults.maxOversoldWeth); + const ethSpentOnForwarderFee = ForwarderTestFactory.getPercentageOfValue(wethSpent, forwarderFeePercentage); + const ethValue = wethSpent.plus(ethSpentOnForwarderFee).plus(ethValueAdjustment); + const feePercentage = ForwarderTestFactory.getPercentageOfValue( constants.PERCENTAGE_DENOMINATOR, forwarderFeePercentage, ); - - const ethValue = expectedResults.takerAssetFillAmount - .plus(expectedResults.wethFees) - .plus(expectedResults.maxOversoldWeth) - .plus(ethSpentOnForwarderFee) - .plus(ethValueAdjustment); - const tx = this._forwarderWrapper.marketBuyOrdersWithEthAsync( orders, expectedResults.makerAssetFillAmount.minus(expectedResults.percentageFees), @@ -240,21 +140,20 @@ export class ForwarderTestFactory { const ordersInfoBefore = await Promise.all(orders.map(order => this._exchangeWrapper.getOrderInfoAsync(order))); const orderStatusesBefore = ordersInfoBefore.map(orderInfo => orderInfo.orderStatus); - const expectedResults = computeExpectedResults(orders, ordersInfoBefore, fractionalNumberOfOrdersToFill); - const ethSpentOnForwarderFee = ForwarderTestFactory.getPercentageOfValue( - expectedResults.takerAssetFillAmount, - forwarderFeePercentage, - ); + const expectedResults = this._computeExpectedResults(orders, ordersInfoBefore, fractionalNumberOfOrdersToFill); + const wethSpent = expectedResults.takerAssetFillAmount + .plus(expectedResults.protocolFees) + .plus(expectedResults.wethFees) + .plus(expectedResults.maxOversoldWeth); + + const ethSpentOnForwarderFee = ForwarderTestFactory.getPercentageOfValue(wethSpent, forwarderFeePercentage); + const ethValue = wethSpent.plus(ethSpentOnForwarderFee); + const feePercentage = ForwarderTestFactory.getPercentageOfValue( constants.PERCENTAGE_DENOMINATOR, forwarderFeePercentage, ); - const ethValue = expectedResults.takerAssetFillAmount - .plus(expectedResults.wethFees) - .plus(expectedResults.maxOversoldWeth) - .plus(ethSpentOnForwarderFee); - const tx = this._forwarderWrapper.marketSellOrdersWithEthAsync( orders, { @@ -268,10 +167,9 @@ export class ForwarderTestFactory { await expect(tx).to.revertWith(options.revertError); } else { const gasUsed = (await tx).gasUsed; - const ordersInfoAfter = await Promise.all( - orders.map(order => this._exchangeWrapper.getOrderInfoAsync(order)), + const orderStatusesAfter = await Promise.all( + orders.map(async order => (await this._exchangeWrapper.getOrderInfoAsync(order)).orderStatus), ); - const orderStatusesAfter = ordersInfoAfter.map(orderInfo => orderInfo.orderStatus); await this._checkResultsAsync( fractionalNumberOfOrdersToFill, @@ -303,6 +201,7 @@ export class ForwarderTestFactory { .minus(expectedResults.makerAssetFillAmount) .minus(expectedResults.maxOverboughtMakerAsset), oldBalances[this._makerAddress][makerAssetAddress].minus(expectedResults.makerAssetFillAmount), + 'Maker makerAsset balance', ); expectBalanceWithin( newBalances[this._takerAddress][makerAssetAddress], @@ -313,11 +212,18 @@ export class ForwarderTestFactory { .plus(expectedResults.makerAssetFillAmount) .minus(expectedResults.percentageFees) .plus(expectedResults.maxOverboughtMakerAsset), + 'Taker makerAsset balance', ); - expect(newBalances[this._orderFeeRecipientAddress][makerAssetAddress]).to.be.bignumber.equal( + expect( + newBalances[this._orderFeeRecipientAddress][makerAssetAddress], + 'Order fee recipient makerAsset balance', + ).to.be.bignumber.equal( oldBalances[this._orderFeeRecipientAddress][makerAssetAddress].plus(expectedResults.percentageFees), ); - expect(newBalances[this._forwarderAddress][makerAssetAddress]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect( + newBalances[this._forwarderAddress][makerAssetAddress], + 'Forwarder contract makerAsset balance', + ).to.be.bignumber.equal(constants.ZERO_AMOUNT); } private async _checkResultsAsync( @@ -340,18 +246,17 @@ export class ForwarderTestFactory { if (fractionalNumberOfOrdersToFill >= i + 1 && orderStatusesBefore[i] === OrderStatus.Fillable) { expectedOrderStatus = OrderStatus.FullyFilled; } - - expect(orderStatus).to.equal(expectedOrderStatus); + expect(orderStatus, ` Order ${i} status`).to.equal(expectedOrderStatus); } + const wethSpent = expectedResults.takerAssetFillAmount + .plus(expectedResults.protocolFees) + .plus(expectedResults.wethFees); const ethSpentOnForwarderFee = ForwarderTestFactory.getPercentageOfValue( - expectedResults.takerAssetFillAmount, + wethSpent, options.forwarderFeePercentage || constants.ZERO_AMOUNT, ); - const totalEthSpent = expectedResults.takerAssetFillAmount - .plus(expectedResults.wethFees) - .plus(ethSpentOnForwarderFee) - .plus(this._gasPrice.times(gasUsed)); + const totalEthSpent = wethSpent.plus(ethSpentOnForwarderFee).plus(this._gasPrice.times(gasUsed)); const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(this._takerAddress); const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(this._forwarderAddress); @@ -361,12 +266,13 @@ export class ForwarderTestFactory { takerEthBalanceAfter, takerEthBalanceBefore.minus(totalEthSpent).minus(expectedResults.maxOversoldWeth), takerEthBalanceBefore.minus(totalEthSpent), + 'Taker ETH balance', ); if (options.forwarderFeeRecipientEthBalanceBefore !== undefined) { const fowarderFeeRecipientEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync( this._forwarderFeeRecipientAddress, ); - expect(fowarderFeeRecipientEthBalanceAfter).to.be.bignumber.equal( + expect(fowarderFeeRecipientEthBalanceAfter, 'Forwarder fee recipient ETH balance').to.be.bignumber.equal( options.forwarderFeeRecipientEthBalanceBefore.plus(ethSpentOnForwarderFee), ); } @@ -375,7 +281,7 @@ export class ForwarderTestFactory { this._checkErc20Balances(erc20Balances, newBalances, expectedResults, makerAssetContract); } else if (options.makerAssetId !== undefined) { const newOwner = await makerAssetContract.ownerOf.callAsync(options.makerAssetId); - expect(newOwner).to.be.bignumber.equal(this._takerAddress); + expect(newOwner, 'New ERC721 owner').to.be.bignumber.equal(this._takerAddress); } expectBalanceWithin( @@ -384,12 +290,97 @@ export class ForwarderTestFactory { erc20Balances[this._makerAddress][this._wethAddress] .plus(expectedResults.takerAssetFillAmount) .plus(expectedResults.maxOversoldWeth), + 'Maker WETH balance', ); - expect(newBalances[this._orderFeeRecipientAddress][this._wethAddress]).to.be.bignumber.equal( + expect( + newBalances[this._orderFeeRecipientAddress][this._wethAddress], + 'Order fee recipient WETH balance', + ).to.be.bignumber.equal( erc20Balances[this._orderFeeRecipientAddress][this._wethAddress].plus(expectedResults.wethFees), ); - - expect(newBalances[this._forwarderAddress][this._wethAddress]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect( + newBalances[this._forwarderAddress][this._wethAddress], + 'Forwarder contract WETH balance', + ).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); } + + // Simulates filling some orders via the Forwarder contract. For example, if + // orders = [A, B, C, D] and fractionalNumberOfOrdersToFill = 2.3, then + // we simulate A and B being completely filled, and 0.3 * C being filled. + private _computeExpectedResults( + orders: SignedOrder[], + ordersInfoBefore: OrderInfo[], + fractionalNumberOfOrdersToFill: number, + ): ForwarderFillState { + const currentState = { + takerAssetFillAmount: constants.ZERO_AMOUNT, + makerAssetFillAmount: constants.ZERO_AMOUNT, + protocolFees: constants.ZERO_AMOUNT, + wethFees: constants.ZERO_AMOUNT, + percentageFees: constants.ZERO_AMOUNT, + maxOversoldWeth: constants.ZERO_AMOUNT, + maxOverboughtMakerAsset: constants.ZERO_AMOUNT, + }; + let remainingOrdersToFill = fractionalNumberOfOrdersToFill; + + for (const [i, order] of orders.entries()) { + if (remainingOrdersToFill === 0) { + break; + } + + if (ordersInfoBefore[i].orderStatus !== OrderStatus.Fillable) { + // If the order is not fillable, skip over it but still count it towards fractionalNumberOfOrdersToFill + remainingOrdersToFill = Math.max(remainingOrdersToFill - 1, 0); + continue; + } + + let makerAssetAmount; + let takerAssetAmount; + let takerFee; + if (remainingOrdersToFill < 1) { + makerAssetAmount = order.makerAssetAmount.times(remainingOrdersToFill).integerValue(); + takerAssetAmount = order.takerAssetAmount.times(remainingOrdersToFill).integerValue(); + takerFee = order.takerFee.times(remainingOrdersToFill).integerValue(); + + // Up to 1 wei worth of WETH will be oversold on the last order due to rounding + currentState.maxOversoldWeth = new BigNumber(1); + // Equivalently, up to 1 wei worth of maker asset will be overbought + currentState.maxOverboughtMakerAsset = currentState.maxOversoldWeth + .times(order.makerAssetAmount) + .dividedToIntegerBy(order.takerAssetAmount); + } else { + makerAssetAmount = order.makerAssetAmount; + takerAssetAmount = order.takerAssetAmount; + takerFee = order.takerFee; + } + + // Accounting for partially filled orders + // As with unfillable orders, these still count as 1 towards fractionalNumberOfOrdersToFill + const takerAssetFilled = ordersInfoBefore[i].orderTakerAssetFilledAmount; + const makerAssetFilled = takerAssetFilled + .times(order.makerAssetAmount) + .dividedToIntegerBy(order.takerAssetAmount); + takerAssetAmount = BigNumber.max(takerAssetAmount.minus(takerAssetFilled), constants.ZERO_AMOUNT); + makerAssetAmount = BigNumber.max(makerAssetAmount.minus(makerAssetFilled), constants.ZERO_AMOUNT); + + currentState.takerAssetFillAmount = currentState.takerAssetFillAmount.plus(takerAssetAmount); + currentState.makerAssetFillAmount = currentState.makerAssetFillAmount.plus(makerAssetAmount); + + if (this._protocolFeeCollectorAddress !== constants.NULL_ADDRESS) { + currentState.protocolFees = currentState.protocolFees.plus( + this._gasPrice.times(this._protocolFeeMultiplier), + ); + } + if (order.takerFeeAssetData === order.makerAssetData) { + currentState.percentageFees = currentState.percentageFees.plus(takerFee); + } else if (order.takerFeeAssetData === order.takerAssetData) { + currentState.wethFees = currentState.wethFees.plus(takerFee); + } + + remainingOrdersToFill = Math.max(remainingOrdersToFill - 1, 0); + } + + return currentState; + } } diff --git a/contracts/exchange-forwarder/tsconfig.json b/contracts/exchange-forwarder/tsconfig.json index e5d468a5ce..1fc2108a56 100644 --- a/contracts/exchange-forwarder/tsconfig.json +++ b/contracts/exchange-forwarder/tsconfig.json @@ -12,7 +12,8 @@ "generated-artifacts/MixinAssets.json", "generated-artifacts/MixinExchangeWrapper.json", "generated-artifacts/MixinForwarderCore.json", - "generated-artifacts/MixinWeth.json" + "generated-artifacts/MixinWeth.json", + "generated-artifacts/TestProtocolFeeCollector.json" ], "exclude": ["./deploy/solc/solc_bin"] } diff --git a/contracts/exchange/test/core.ts b/contracts/exchange/test/core.ts index 1e55c3ec74..397ead2eb6 100644 --- a/contracts/exchange/test/core.ts +++ b/contracts/exchange/test/core.ts @@ -172,7 +172,7 @@ blockchainTests.resets('Exchange core', () => { await multiAssetProxy.registerAssetProxy.awaitTransactionSuccessAsync(staticCallProxy.address, { from: owner }); // Configure Exchange - exchangeWrapper = new ExchangeWrapper(exchange, provider); + exchangeWrapper = new ExchangeWrapper(exchange); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc1155Proxy.address, owner); diff --git a/contracts/exchange/test/match_orders.ts b/contracts/exchange/test/match_orders.ts index ce2a899897..3c74932c8c 100644 --- a/contracts/exchange/test/match_orders.ts +++ b/contracts/exchange/test/match_orders.ts @@ -135,7 +135,7 @@ describe('matchOrders', () => { {}, new BigNumber(chainId), ); - exchangeWrapper = new ExchangeWrapper(exchange, provider); + exchangeWrapper = new ExchangeWrapper(exchange); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc1155Proxy.address, owner); diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index d5d757afdc..13379a4ba0 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -96,7 +96,7 @@ blockchainTests.resets('Exchange transactions', env => { {}, new BigNumber(chainId), ); - exchangeWrapper = new ExchangeWrapper(exchangeInstance, env.provider); + exchangeWrapper = new ExchangeWrapper(exchangeInstance); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await erc20Proxy.addAuthorizedAddress.awaitTransactionSuccessAsync(exchangeInstance.address, { from: owner }); diff --git a/contracts/exchange/test/utils/exchange_wrapper.ts b/contracts/exchange/test/utils/exchange_wrapper.ts index 69b64da172..dead507440 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -1,4 +1,4 @@ -import { BatchMatchOrder, orderUtils, Web3ProviderEngine } from '@0x/contracts-test-utils'; +import { BatchMatchOrder, orderUtils } from '@0x/contracts-test-utils'; import { BatchMatchedFillResults, FillResults, @@ -8,7 +8,7 @@ import { SignedZeroExTransaction, } from '@0x/types'; import { AbiEncoder, BigNumber } from '@0x/utils'; -import { MethodAbi, TransactionReceiptWithDecodedLogs, ZeroExProvider } from 'ethereum-types'; +import { MethodAbi, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; import { ExchangeContract } from '../../src'; @@ -16,18 +16,15 @@ import { ExchangeContract } from '../../src'; import { AbiDecodedFillOrderData } from './types'; export class ExchangeWrapper { - private readonly _exchange: ExchangeContract; - // tslint:disable no-unused-variable - constructor(exchangeContract: ExchangeContract, provider: Web3ProviderEngine | ZeroExProvider) { - this._exchange = exchangeContract; - } + constructor(public readonly exchangeContract: ExchangeContract) {} + public async fillOrderAsync( signedOrder: SignedOrder, from: string, opts: { takerAssetFillAmount?: BigNumber } = {}, ): Promise { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); - const txReceipt = await this._exchange.fillOrder.awaitTransactionSuccessAsync( + const txReceipt = await this.exchangeContract.fillOrder.awaitTransactionSuccessAsync( params.order, params.takerAssetFillAmount, params.signature, @@ -37,7 +34,7 @@ export class ExchangeWrapper { } public async cancelOrderAsync(signedOrder: SignedOrder, from: string): Promise { const params = orderUtils.createCancel(signedOrder); - const txReceipt = await this._exchange.cancelOrder.awaitTransactionSuccessAsync(params.order, { from }); + const txReceipt = await this.exchangeContract.cancelOrder.awaitTransactionSuccessAsync(params.order, { from }); return txReceipt; } public async fillOrKillOrderAsync( @@ -46,7 +43,7 @@ export class ExchangeWrapper { opts: { takerAssetFillAmount?: BigNumber; gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); - const txReceipt = await this._exchange.fillOrKillOrder.awaitTransactionSuccessAsync( + const txReceipt = await this.exchangeContract.fillOrKillOrder.awaitTransactionSuccessAsync( params.order, params.takerAssetFillAmount, params.signature, @@ -59,7 +56,7 @@ export class ExchangeWrapper { from: string, opts: { takerAssetFillAmounts?: BigNumber[]; gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.batchFillOrders.awaitTransactionSuccessAsync( + return this.exchangeContract.batchFillOrders.awaitTransactionSuccessAsync( orders, opts.takerAssetFillAmounts === undefined ? orders.map(signedOrder => signedOrder.takerAssetAmount) @@ -73,7 +70,7 @@ export class ExchangeWrapper { from: string, opts: { takerAssetFillAmounts?: BigNumber[]; gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.batchFillOrKillOrders.awaitTransactionSuccessAsync( + return this.exchangeContract.batchFillOrKillOrders.awaitTransactionSuccessAsync( orders, opts.takerAssetFillAmounts === undefined ? orders.map(signedOrder => signedOrder.takerAssetAmount) @@ -87,7 +84,7 @@ export class ExchangeWrapper { from: string, opts: { takerAssetFillAmounts?: BigNumber[]; gas?: number; gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.batchFillOrdersNoThrow.awaitTransactionSuccessAsync( + return this.exchangeContract.batchFillOrdersNoThrow.awaitTransactionSuccessAsync( orders, opts.takerAssetFillAmounts === undefined ? orders.map(signedOrder => signedOrder.takerAssetAmount) @@ -101,7 +98,7 @@ export class ExchangeWrapper { from: string, opts: { takerAssetFillAmount: BigNumber; gas?: number; gasPrice?: BigNumber }, ): Promise { - return this._exchange.marketSellOrdersNoThrow.awaitTransactionSuccessAsync( + return this.exchangeContract.marketSellOrdersNoThrow.awaitTransactionSuccessAsync( orders, opts.takerAssetFillAmount, orders.map(signedOrder => signedOrder.signature), @@ -113,7 +110,7 @@ export class ExchangeWrapper { from: string, opts: { makerAssetFillAmount: BigNumber; gas?: number; gasPrice?: BigNumber }, ): Promise { - return this._exchange.marketBuyOrdersNoThrow.awaitTransactionSuccessAsync( + return this.exchangeContract.marketBuyOrdersNoThrow.awaitTransactionSuccessAsync( orders, opts.makerAssetFillAmount, orders.map(signedOrder => signedOrder.signature), @@ -125,7 +122,7 @@ export class ExchangeWrapper { from: string, opts: { takerAssetFillAmount: BigNumber; gas?: number }, ): Promise { - return this._exchange.marketSellOrdersFillOrKill.awaitTransactionSuccessAsync( + return this.exchangeContract.marketSellOrdersFillOrKill.awaitTransactionSuccessAsync( orders, opts.takerAssetFillAmount, orders.map(signedOrder => signedOrder.signature), @@ -137,7 +134,7 @@ export class ExchangeWrapper { from: string, opts: { makerAssetFillAmount: BigNumber; gas?: number }, ): Promise { - return this._exchange.marketBuyOrdersFillOrKill.awaitTransactionSuccessAsync( + return this.exchangeContract.marketBuyOrdersFillOrKill.awaitTransactionSuccessAsync( orders, opts.makerAssetFillAmount, orders.map(signedOrder => signedOrder.signature), @@ -148,19 +145,22 @@ export class ExchangeWrapper { orders: SignedOrder[], from: string, ): Promise { - return this._exchange.batchCancelOrders.awaitTransactionSuccessAsync(orders, { from }); + return this.exchangeContract.batchCancelOrders.awaitTransactionSuccessAsync(orders, { from }); } public async cancelOrdersUpToAsync(salt: BigNumber, from: string): Promise { - const txReceipt = await this._exchange.cancelOrdersUpTo.awaitTransactionSuccessAsync(salt, { from }); + const txReceipt = await this.exchangeContract.cancelOrdersUpTo.awaitTransactionSuccessAsync(salt, { from }); return txReceipt; } public async registerAssetProxyAsync( assetProxyAddress: string, from: string, ): Promise { - const txReceipt = await this._exchange.registerAssetProxy.awaitTransactionSuccessAsync(assetProxyAddress, { - from, - }); + const txReceipt = await this.exchangeContract.registerAssetProxy.awaitTransactionSuccessAsync( + assetProxyAddress, + { + from, + }, + ); return txReceipt; } public async executeTransactionAsync( @@ -168,7 +168,7 @@ export class ExchangeWrapper { from: string, opts: { gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.executeTransaction.awaitTransactionSuccessAsync( + return this.exchangeContract.executeTransaction.awaitTransactionSuccessAsync( signedTransaction, signedTransaction.signature, { from, gasPrice: opts.gasPrice }, @@ -180,25 +180,29 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const signatures = signedTransactions.map(signedTransaction => signedTransaction.signature); - return this._exchange.batchExecuteTransactions.awaitTransactionSuccessAsync(signedTransactions, signatures, { - from, - gasPrice: opts.gasPrice, - }); + return this.exchangeContract.batchExecuteTransactions.awaitTransactionSuccessAsync( + signedTransactions, + signatures, + { + from, + gasPrice: opts.gasPrice, + }, + ); } public async getTakerAssetFilledAmountAsync(orderHashHex: string): Promise { - const filledAmount = await this._exchange.filled.callAsync(orderHashHex); + const filledAmount = await this.exchangeContract.filled.callAsync(orderHashHex); return filledAmount; } public async isCancelledAsync(orderHashHex: string): Promise { - const isCancelled = await this._exchange.cancelled.callAsync(orderHashHex); + const isCancelled = await this.exchangeContract.cancelled.callAsync(orderHashHex); return isCancelled; } public async getOrderEpochAsync(makerAddress: string, senderAddress: string): Promise { - const orderEpoch = await this._exchange.orderEpoch.callAsync(makerAddress, senderAddress); + const orderEpoch = await this.exchangeContract.orderEpoch.callAsync(makerAddress, senderAddress); return orderEpoch; } public async getOrderInfoAsync(signedOrder: SignedOrder): Promise { - const orderInfo = await this._exchange.getOrderInfo.callAsync(signedOrder); + const orderInfo = await this.exchangeContract.getOrderInfo.callAsync(signedOrder); return orderInfo; } public async batchMatchOrdersAsync( @@ -208,7 +212,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createBatchMatchOrders(signedOrdersLeft, signedOrdersRight); - return this._exchange.batchMatchOrders.awaitTransactionSuccessAsync( + return this.exchangeContract.batchMatchOrders.awaitTransactionSuccessAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -221,7 +225,7 @@ export class ExchangeWrapper { from: string, opts: { gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.batchMatchOrders.awaitTransactionSuccessAsync( + return this.exchangeContract.batchMatchOrders.awaitTransactionSuccessAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -236,7 +240,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createBatchMatchOrders(signedOrdersLeft, signedOrdersRight); - const batchMatchedFillResults = await this._exchange.batchMatchOrders.callAsync( + const batchMatchedFillResults = await this.exchangeContract.batchMatchOrders.callAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -252,7 +256,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createBatchMatchOrders(signedOrdersLeft, signedOrdersRight); - return this._exchange.batchMatchOrdersWithMaximalFill.awaitTransactionSuccessAsync( + return this.exchangeContract.batchMatchOrdersWithMaximalFill.awaitTransactionSuccessAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -265,7 +269,7 @@ export class ExchangeWrapper { from: string, opts: { gasPrice?: BigNumber } = {}, ): Promise { - return this._exchange.batchMatchOrdersWithMaximalFill.awaitTransactionSuccessAsync( + return this.exchangeContract.batchMatchOrdersWithMaximalFill.awaitTransactionSuccessAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -280,7 +284,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createBatchMatchOrders(signedOrdersLeft, signedOrdersRight); - const batchMatchedFillResults = await this._exchange.batchMatchOrdersWithMaximalFill.callAsync( + const batchMatchedFillResults = await this.exchangeContract.batchMatchOrdersWithMaximalFill.callAsync( params.leftOrders, params.rightOrders, params.leftSignatures, @@ -296,7 +300,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createMatchOrders(signedOrderLeft, signedOrderRight); - const txReceipt = await this._exchange.matchOrders.awaitTransactionSuccessAsync( + const txReceipt = await this.exchangeContract.matchOrders.awaitTransactionSuccessAsync( params.left, params.right, params.leftSignature, @@ -312,7 +316,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createMatchOrders(signedOrderLeft, signedOrderRight); - const matchedFillResults = await this._exchange.matchOrders.callAsync( + const matchedFillResults = await this.exchangeContract.matchOrders.callAsync( params.left, params.right, params.leftSignature, @@ -328,7 +332,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createMatchOrders(signedOrderLeft, signedOrderRight); - return this._exchange.matchOrdersWithMaximalFill.awaitTransactionSuccessAsync( + return this.exchangeContract.matchOrdersWithMaximalFill.awaitTransactionSuccessAsync( params.left, params.right, params.leftSignature, @@ -343,7 +347,7 @@ export class ExchangeWrapper { opts: { gasPrice?: BigNumber }, ): Promise { const params = orderUtils.createMatchOrders(signedOrderLeft, signedOrderRight); - const matchedFillResults = await this._exchange.matchOrdersWithMaximalFill.callAsync( + const matchedFillResults = await this.exchangeContract.matchOrdersWithMaximalFill.callAsync( params.left, params.right, params.leftSignature, @@ -358,7 +362,7 @@ export class ExchangeWrapper { opts: { takerAssetFillAmount?: BigNumber; gasPrice?: BigNumber } = {}, ): Promise { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); - const fillResults = await this._exchange.fillOrder.callAsync( + const fillResults = await this.exchangeContract.fillOrder.callAsync( params.order, params.takerAssetFillAmount, params.signature, @@ -368,7 +372,7 @@ export class ExchangeWrapper { } public abiEncodeFillOrder(signedOrder: SignedOrder, opts: { takerAssetFillAmount?: BigNumber } = {}): string { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); - const data = this._exchange.fillOrder.getABIEncodedTransactionData( + const data = this.exchangeContract.fillOrder.getABIEncodedTransactionData( params.order, params.takerAssetFillAmount, params.signature, @@ -377,13 +381,10 @@ export class ExchangeWrapper { } public abiDecodeFillOrder(data: string): AbiDecodedFillOrderData { // Lookup fillOrder ABI in exchange abi - const fillOrderAbi = _.find(this._exchange.abi, { name: 'fillOrder' }) as MethodAbi; + const fillOrderAbi = _.find(this.exchangeContract.abi, { name: 'fillOrder' }) as MethodAbi; // Decode input data const abiEncoder = new AbiEncoder.Method(fillOrderAbi); const decodedData = abiEncoder.decode(data) as AbiDecodedFillOrderData; return decodedData; } - public getExchangeAddress(): string { - return this._exchange.address; - } } diff --git a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts index c53a51ead2..89cf4e801e 100644 --- a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts +++ b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts @@ -117,7 +117,7 @@ export async function fillOrderCombinatorialUtilsFactoryAsync( ); const logDecoder = new LogDecoder(web3Wrapper, artifacts); - const exchangeWrapper = new ExchangeWrapper(exchangeContract, provider); + const exchangeWrapper = new ExchangeWrapper(exchangeContract); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, ownerAddress); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, ownerAddress); await exchangeWrapper.registerAssetProxyAsync(erc1155Proxy.address, ownerAddress); @@ -646,7 +646,7 @@ export class FillOrderCombinatorialUtils { const exchangeLogs = _.filter( txReceipt.logs, - txLog => txLog.address === this.exchangeWrapper.getExchangeAddress(), + txLog => txLog.address === this.exchangeWrapper.exchangeContract.address, ); expect(exchangeLogs.length).to.be.equal(1, 'logs length'); // tslint:disable-next-line:no-unnecessary-type-assertion diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 9ecfe73add..22487d6b15 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -78,7 +78,7 @@ blockchainTests.resets('Exchange wrappers', env => { from: owner, }); - exchangeWrapper = new ExchangeWrapper(exchange, env.provider); + exchangeWrapper = new ExchangeWrapper(exchange); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await erc20Proxy.addAuthorizedAddress.awaitTransactionSuccessAsync(exchange.address, { diff --git a/contracts/extensions/test/balance_threshold_filter.ts b/contracts/extensions/test/balance_threshold_filter.ts index 1a90ae975a..1c5872fdaa 100644 --- a/contracts/extensions/test/balance_threshold_filter.ts +++ b/contracts/extensions/test/balance_threshold_filter.ts @@ -136,7 +136,7 @@ describe(ContractName.BalanceThresholdFilter, () => { zrxAssetData, new BigNumber(chainId), ); - exchangeWrapper = new ExchangeWrapper(exchangeInstance, provider); + exchangeWrapper = new ExchangeWrapper(exchangeInstance); // Register proxies await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeInstance.address, { diff --git a/contracts/extensions/test/dutch_auction.ts b/contracts/extensions/test/dutch_auction.ts index 82b2b4c659..e395bd21e2 100644 --- a/contracts/extensions/test/dutch_auction.ts +++ b/contracts/extensions/test/dutch_auction.ts @@ -95,7 +95,7 @@ describe(ContractName.DutchAuction, () => { zrxAssetData, new BigNumber(chainId), ); - const exchangeWrapper = new ExchangeWrapper(exchangeInstance, provider); + const exchangeWrapper = new ExchangeWrapper(exchangeInstance); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); diff --git a/contracts/extensions/test/order_matcher.ts b/contracts/extensions/test/order_matcher.ts index cdc5d44a66..4b7359fb91 100644 --- a/contracts/extensions/test/order_matcher.ts +++ b/contracts/extensions/test/order_matcher.ts @@ -115,7 +115,7 @@ describe('OrderMatcher', () => { assetDataUtils.encodeERC20AssetData(zrxToken.address), new BigNumber(chainId), ); - exchangeWrapper = new ExchangeWrapper(exchange, provider); + exchangeWrapper = new ExchangeWrapper(exchange); await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); // Authorize ERC20 trades by exchange diff --git a/contracts/test-utils/src/order_factory.ts b/contracts/test-utils/src/order_factory.ts index b0f435ff2f..1a2e1187e6 100644 --- a/contracts/test-utils/src/order_factory.ts +++ b/contracts/test-utils/src/order_factory.ts @@ -20,10 +20,10 @@ export class OrderFactory { const tenMinutesInSeconds = 10 * 60; const currentBlockTimestamp = await getLatestBlockTimestampAsync(); const order = ({ + takerAddress: constants.NULL_ADDRESS, senderAddress: constants.NULL_ADDRESS, expirationTimeSeconds: new BigNumber(currentBlockTimestamp).plus(tenMinutesInSeconds), salt: generatePseudoRandomSalt(), - takerAddress: constants.NULL_ADDRESS, ...this._defaultOrderParams, ...customOrderParams, } as any) as Order; diff --git a/packages/order-utils/src/forwarder_revert_errors.ts b/packages/order-utils/src/forwarder_revert_errors.ts index d23075721f..b88eb857b8 100644 --- a/packages/order-utils/src/forwarder_revert_errors.ts +++ b/packages/order-utils/src/forwarder_revert_errors.ts @@ -64,10 +64,10 @@ export class InsufficientEthForFeeError extends RevertError { } } -export class OversoldWethError extends RevertError { - constructor(wethSold?: BigNumber | number | string, msgValue?: BigNumber | number | string) { - super('OversoldWethError', 'OversoldWethError(uint256 wethSold, uint256 msgValue)', { - wethSold, +export class OverspentWethError extends RevertError { + constructor(wethSpent?: BigNumber | number | string, msgValue?: BigNumber | number | string) { + super('OverspentWethError', 'OverspentWethError(uint256 wethSpent, uint256 msgValue)', { + wethSpent, msgValue, }); } @@ -87,9 +87,9 @@ export class DefaultFunctionWethContractOnlyError extends RevertError { } } -export class MsgValueCantEqualZeroError extends RevertError { +export class MsgValueCannotEqualZeroError extends RevertError { constructor() { - super('MsgValueCantEqualZeroError', 'MsgValueCantEqualZeroError()', {}); + super('MsgValueCannotEqualZeroError', 'MsgValueCannotEqualZeroError()', {}); } } @@ -109,10 +109,10 @@ const types = [ UnsupportedFeeError, FeePercentageTooLargeError, InsufficientEthForFeeError, - OversoldWethError, + OverspentWethError, TransferFailedError, DefaultFunctionWethContractOnlyError, - MsgValueCantEqualZeroError, + MsgValueCannotEqualZeroError, Erc721AmountMustEqualOneError, ]; diff --git a/packages/utils/src/revert_error.ts b/packages/utils/src/revert_error.ts index 0df9302a59..515950850c 100644 --- a/packages/utils/src/revert_error.ts +++ b/packages/utils/src/revert_error.ts @@ -351,7 +351,7 @@ export function getThrownErrorRevertErrorBytes(error: Error | GanacheTransaction // so we do nothing. } } - throw new Error(`Cannot decode thrown Errror "${error.message}" as a RevertError`); + throw new Error(`Cannot decode thrown Error "${error.message}" as a RevertError`); } function isGanacheTransactionRevertError(