diff options
Diffstat (limited to 'packages/contracts')
25 files changed, 533 insertions, 154 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index f66114e87..cdf49dcf6 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -23,6 +23,7 @@ "DummyERC20Token", "DummyERC721Receiver", "DummyERC721Token", + "DummyMultipleReturnERC20Token", "DummyNoReturnERC20Token", "ERC20Proxy", "ERC20Token", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 5d2f290ac..283991cee 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -11,8 +11,7 @@ "test": "test" }, "scripts": { - "watch_without_deps": "yarn pre_build && tsc -w", - "build": "yarn pre_build && tsc", + "build": "yarn pre_build && tsc -b", "pre_build": "run-s compile copy_artifacts generate_contract_wrappers", "copy_artifacts": "copyfiles -u 4 '../migrations/artifacts/2.0.0/**/*' ./lib/artifacts;", "test": "yarn run_mocha", @@ -20,11 +19,14 @@ "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov", "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html", "test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha", - "run_mocha": "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit", + "run_mocha": + "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit", "compile": "sol-compiler --contracts-dir src", "clean": "shx rm -rf lib generated_contract_wrappers", - "generate_contract_wrappers": "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers", - "lint": "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts", + "generate_contract_wrappers": + "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers", + "lint": + "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts", "coverage:report:text": "istanbul report text", "coverage:report:html": "istanbul report html && open coverage/index.html", "profiler:report:html": "istanbul report html && open coverage/index.html", @@ -33,7 +35,8 @@ "lint-contracts": "solhint src/2.0.0/**/**/**/**/*.sol" }, "config": { - "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "abis": + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyMultipleReturnERC20Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol index 785f3aba9..258443bca 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol @@ -18,7 +18,6 @@ pragma solidity 0.4.24; -import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAuthorizable.sol"; diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol index caf05b0dd..65b664b8b 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol @@ -18,7 +18,6 @@ pragma solidity 0.4.24; -import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAuthorizable.sol"; diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol index 7da3d87e2..4d00e92d3 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -16,15 +16,18 @@ */ -pragma solidity 0.4.10; +pragma solidity 0.4.24; import "../../multisig/MultiSigWalletWithTimeLock.sol"; +import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is MultiSigWalletWithTimeLock { + using LibBytes for bytes; + event AssetProxyRegistration(address assetProxyContract, bool isRegistered); // Mapping of AssetProxy contract address => @@ -37,8 +40,14 @@ contract AssetProxyOwner is /// on an approved AssetProxy contract. modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; - require(isAssetProxyRegistered[tx.destination]); - require(readBytes4(tx.data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); + require( + isAssetProxyRegistered[tx.destination], + "UNREGISTERED_ASSET_PROXY" + ); + require( + tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR, + "INVALID_FUNCTION_SELECTOR" + ); _; } @@ -48,7 +57,7 @@ contract AssetProxyOwner is /// @param _assetProxyContracts Array of AssetProxy contract addresses. /// @param _required Number of required confirmations. /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. - function AssetProxyOwner( + constructor ( address[] memory _owners, address[] memory _assetProxyContracts, uint256 _required, @@ -59,7 +68,10 @@ contract AssetProxyOwner is { for (uint256 i = 0; i < _assetProxyContracts.length; i++) { address assetProxy = _assetProxyContracts[i]; - require(assetProxy != address(0)); + require( + assetProxy != address(0), + "INVALID_ASSET_PROXY" + ); isAssetProxyRegistered[assetProxy] = true; } } @@ -74,7 +86,7 @@ contract AssetProxyOwner is notNull(assetProxyContract) { isAssetProxyRegistered[assetProxyContract] = isRegistered; - AssetProxyRegistration(assetProxyContract, isRegistered); + emit AssetProxyRegistration(assetProxyContract, isRegistered); } /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock. @@ -89,36 +101,10 @@ contract AssetProxyOwner is tx.executed = true; // solhint-disable-next-line avoid-call-value if (tx.destination.call.value(tx.value)(tx.data)) - Execution(transactionId); + emit Execution(transactionId); else { - ExecutionFailure(transactionId); + emit ExecutionFailure(transactionId); tx.executed = false; } } - - /// @dev Reads an unpadded bytes4 value from a position in a byte array. - /// @param b Byte array containing a bytes4 value. - /// @param index Index in byte array of bytes4 value. - /// @return bytes4 value from byte array. - function readBytes4( - bytes memory b, - uint256 index - ) - internal - returns (bytes4 result) - { - require(b.length >= index + 4); - - // Arrays are prefixed by a 32 byte length field - index += 32; - - // Read the bytes4 from array memory - assembly { - result := mload(add(b, index)) - // Solidity does not require us to clean the trailing bytes. - // We do it anyway - result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000) - } - return result; - } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol index 80475e6e3..e90b62f19 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,7 +19,6 @@ pragma solidity 0.4.24; import "../../utils/Ownable/Ownable.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; @@ -28,7 +27,6 @@ contract MixinAssetProxyDispatcher is Ownable, MAssetProxyDispatcher { - using LibBytes for bytes; // Mapping from Asset Proxy Id's to their respective Asset Proxy mapping (bytes4 => IAssetProxy) public assetProxies; @@ -90,7 +88,7 @@ contract MixinAssetProxyDispatcher is "LENGTH_GREATER_THAN_3_REQUIRED" ); - // Lookup assetProxy + // Lookup assetProxy. We do not use `LibBytes.readBytes4` for gas efficiency reasons. bytes4 assetProxyId; assembly { assetProxyId := and(mload( diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index ff908917d..11bbe40fb 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -107,14 +107,7 @@ contract MixinExchangeCore is public nonReentrant { - // Fetch current order status - OrderInfo memory orderInfo = getOrderInfo(order); - - // Validate context - assertValidCancel(order, orderInfo); - - // Perform cancel - updateCancelledState(order, orderInfo.orderHash); + cancelOrderInternal(order); } /// @dev Gets information about an order: status, hash, and amount filled. @@ -236,6 +229,22 @@ contract MixinExchangeCore is return fillResults; } + /// @dev After calling, the order can not be filled anymore. + /// Throws if order is invalid or sender does not have permission to cancel. + /// @param order Order to cancel. Order must be OrderStatus.FILLABLE. + function cancelOrderInternal(Order memory order) + internal + { + // Fetch current order status + OrderInfo memory orderInfo = getOrderInfo(order); + + // Validate context + assertValidCancel(order, orderInfo); + + // Perform cancel + updateCancelledState(order, orderInfo.orderHash); + } + /// @dev Updates state with results of a fill order. /// @param order that was filled. /// @param takerAddress Address of taker who filled the order. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index a5459a21e..a149f95c9 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -377,10 +377,11 @@ contract MixinWrapperFunctions is /// @param orders Array of order specifications. function batchCancelOrders(LibOrder.Order[] memory orders) public + nonReentrant { uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { - cancelOrder(orders[i]); + cancelOrderInternal(orders[i]); } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol index d85913e0f..742499568 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol @@ -72,6 +72,11 @@ contract MExchangeCore is internal returns (LibFillResults.FillResults memory fillResults); + /// @dev After calling, the order can not be filled anymore. + /// @param order Order struct containing order specifications. + function cancelOrderInternal(LibOrder.Order memory order) + internal; + /// @dev Updates state with results of a fill order. /// @param order that was filled. /// @param takerAddress Address of taker who filled the order. diff --git a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol new file mode 100644 index 000000000..8a8aecae8 --- /dev/null +++ b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol @@ -0,0 +1,70 @@ +/* + + Copyright 2018 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; + +import "./DummyERC20Token.sol"; + + +// solhint-disable no-empty-blocks +contract DummyMultipleReturnERC20Token is + DummyERC20Token +{ + + constructor ( + string _name, + string _symbol, + uint256 _decimals, + uint256 _totalSupply + ) + public + DummyERC20Token( + _name, + _symbol, + _decimals, + _totalSupply + ) + {} + + /// @dev send `value` token to `to` from `from` on the condition it is approved by `from` + /// @param _from The address of the sender + /// @param _to The address of the recipient + /// @param _value The amount of token to be transferred + function transferFrom( + address _from, + address _to, + uint256 _value + ) + external + returns (bool) + { + emit Transfer( + _from, + _to, + _value + ); + + // HACK: This contract will not compile if we remove `returns (bool)`, so we manually return 64 bytes (equiavalent to true, true) + assembly { + mstore(0, 1) + mstore(32, 1) + return(0, 64) + } + } +} + diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol index 4f4e28302..e3311c703 100644 --- a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol +++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol @@ -51,6 +51,7 @@ contract ReentrantERC20Token is MARKET_SELL_ORDERS, MATCH_ORDERS, CANCEL_ORDER, + BATCH_CANCEL_ORDERS, CANCEL_ORDERS_UP_TO, SET_SIGNATURE_VALIDATOR_APPROVAL } @@ -150,6 +151,11 @@ contract ReentrantERC20Token is EXCHANGE.cancelOrder.selector, order ); + } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_CANCEL_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.batchCancelOrders.selector, + orders + ); } else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDERS_UP_TO)) { calldata = abi.encodeWithSelector( EXCHANGE.cancelOrdersUpTo.selector, diff --git a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol index 75e782d43..38ec42a72 100644 --- a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol +++ b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -16,7 +16,7 @@ */ -pragma solidity 0.4.10; +pragma solidity 0.4.24; import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol"; @@ -26,7 +26,7 @@ contract TestAssetProxyOwner is AssetProxyOwner { - function TestAssetProxyOwner( + constructor ( address[] memory _owners, address[] memory _assetProxyContracts, uint256 _required, @@ -38,6 +38,7 @@ contract TestAssetProxyOwner is function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id) public + view validRemoveAuthorizedAddressAtIndexTx(id) returns (bool) { @@ -50,23 +51,9 @@ contract TestAssetProxyOwner is /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`. function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data) public + pure returns (bool) { - return readBytes4(data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; - } - - /// @dev Reads an unpadded bytes4 value from a position in a byte array. - /// @param b Byte array containing a bytes4 value. - /// @param index Index in byte array of bytes4 value. - /// @return bytes4 value from byte array. - function publicReadBytes4( - bytes memory b, - uint256 index - ) - public - returns (bytes4 result) - { - result = readBytes4(b, index); - return result; + return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; } } diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index e99c6cee3..0c0da46b3 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -2,6 +2,7 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; +import * as _ from 'lodash'; import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mixin_authorizable'; import { artifacts } from '../utils/artifacts'; @@ -28,8 +29,7 @@ describe('Authorizable', () => { }); before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); - owner = address = accounts[0]; - notOwner = accounts[1]; + [owner, address, notOwner] = _.slice(accounts, 0, 3); authorizable = await MixinAuthorizableContract.deployFrom0xArtifactAsync( artifacts.MixinAuthorizable, provider, diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 6031e554d..4d70031d1 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -8,6 +8,8 @@ import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; import { DummyERC721ReceiverContract } from '../../generated_contract_wrappers/dummy_erc721_receiver'; import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; +import { DummyMultipleReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_multiple_return_erc20_token'; +import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy'; @@ -42,6 +44,8 @@ describe('Asset Transfer Proxies', () => { let erc721Receiver: DummyERC721ReceiverContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; + let noReturnErc20Token: DummyNoReturnERC20TokenContract; + let multipleReturnErc20Token: DummyMultipleReturnERC20TokenContract; let erc20Wrapper: ERC20Wrapper; let erc721Wrapper: ERC721Wrapper; @@ -91,6 +95,51 @@ describe('Asset Transfer Proxies', () => { provider, txDefaults, ); + noReturnErc20Token = await DummyNoReturnERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyNoReturnERC20Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + constants.DUMMY_TOKEN_DECIMALS, + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.setBalance.sendTransactionAsync(makerAddress, constants.INITIAL_ERC20_BALANCE), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.approve.sendTransactionAsync( + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + multipleReturnErc20Token = await DummyMultipleReturnERC20TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyMultipleReturnERC20Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + constants.DUMMY_TOKEN_DECIMALS, + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await multipleReturnErc20Token.setBalance.sendTransactionAsync( + makerAddress, + constants.INITIAL_ERC20_BALANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await multipleReturnErc20Token.approve.sendTransactionAsync( + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -141,6 +190,65 @@ describe('Asset Transfer Proxies', () => { ); }); + it('should successfully transfer tokens that do not return a value', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address); + // Perform a transfer from makerAddress to takerAddress + const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance.minus(amount)); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance.plus(amount)); + }); + + it('should successfully transfer tokens and ignore extra assetData', async () => { + // Construct ERC20 asset data + const extraData = '0102030405060708'; + const encodedAssetData = `${assetDataUtils.encodeERC20AssetData(zrxToken.address)}${extraData}`; + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(amount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].add(amount), + ); + }); + it('should do nothing if transferring 0 amount of a token', async () => { // Construct ERC20 asset data const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); @@ -189,6 +297,7 @@ describe('Asset Transfer Proxies', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); + const erc20Balances = await erc20Wrapper.getBalancesAsync(); // Perform a transfer; expect this to fail. await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ @@ -198,6 +307,43 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.TransferFailed, ); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should throw if allowances are too low and token does not return a value', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address); + // Create allowance less than transfer amount. Set allowance on proxy. + const allowance = new BigNumber(0); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await noReturnErc20Token.approve.sendTransactionAsync(erc20Proxy.address, allowance, { + from: makerAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + // Perform a transfer; expect this to fail. + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + RevertReason.TransferFailed, + ); + const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance); }); it('should throw if requesting address is not authorized', async () => { @@ -211,6 +357,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); + const erc20Balances = await erc20Wrapper.getBalancesAsync(); await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc20Proxy.address, @@ -219,6 +366,35 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.SenderNotAuthorized, ); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should throw if token returns more than 32 bytes', async () => { + // Construct ERC20 asset data + const encodedAssetData = assetDataUtils.encodeERC20AssetData(multipleReturnErc20Token.address); + const amount = new BigNumber(10); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + const initialMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress); + const initialTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress); + // Perform a transfer; expect this to fail. + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + to: erc20Proxy.address, + data, + from: exchangeAddress, + }), + RevertReason.TransferFailed, + ); + const newMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress); + const newTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress); + expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance); + expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance); }); }); @@ -247,7 +423,38 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); + // Perform a transfer from makerAddress to takerAddress + const amount = new BigNumber(1); + const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( + encodedAssetData, + makerAddress, + takerAddress, + amount, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + to: erc721Proxy.address, + data, + from: exchangeAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Verify transfer was successful + const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); + }); + + it('should successfully transfer tokens and ignore extra assetData', async () => { + // Construct ERC721 asset data + const extraData = '0102030405060708'; + const encodedAssetData = `${assetDataUtils.encodeERC721AssetData( + erc721Token.address, + erc721MakerTokenId, + )}${extraData}`; + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -274,7 +481,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -304,7 +511,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(0); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -313,7 +520,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -321,6 +528,8 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.InvalidAmount, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if transferring > 1 amount of a token', async () => { @@ -328,7 +537,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(500); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -337,7 +546,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -345,11 +554,16 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.InvalidAmount, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if allowances are too low', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Remove transfer approval for makerAddress. await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.approve.sendTransactionAsync(constants.NULL_ADDRESS, erc721MakerTokenId, { @@ -365,7 +579,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -373,11 +587,16 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.TransferFailed, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); it('should throw if requesting address is not authorized', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + // Verify pre-condition + const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(ownerMakerAsset).to.be.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( @@ -386,7 +605,7 @@ describe('Asset Transfer Proxies', () => { takerAddress, amount, ); - return expectTransactionFailedAsync( + await expectTransactionFailedAsync( web3Wrapper.sendTransactionAsync({ to: erc721Proxy.address, data, @@ -394,6 +613,8 @@ describe('Asset Transfer Proxies', () => { }), RevertReason.SenderNotAuthorized, ); + const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); + expect(newOwner).to.be.equal(ownerMakerAsset); }); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 3bb71b58f..acb99eed1 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -8,7 +8,10 @@ import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; +import { + DummyERC20TokenContract, + DummyERC20TokenTransferEventArgs, +} from '../../generated_contract_wrappers/dummy_erc20_token'; import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; @@ -243,6 +246,40 @@ describe('Exchange core', () => { RevertReason.ValidatorError, ); }); + + it('should not emit transfer events for transfers where from == to', async () => { + const txReceipt = await exchangeWrapper.fillOrderAsync(signedOrder, makerAddress); + const logs = txReceipt.logs; + const transferLogs = _.filter( + logs, + log => (log as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).event === 'Transfer', + ); + expect(transferLogs.length).to.be.equal(2); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal( + zrxToken.address, + ); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal( + makerAddress, + ); + expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal( + feeRecipientAddress, + ); + expect( + (transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value, + ).to.be.bignumber.equal(signedOrder.makerFee); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal( + zrxToken.address, + ); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal( + makerAddress, + ); + expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal( + feeRecipientAddress, + ); + expect( + (transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value, + ).to.be.bignumber.equal(signedOrder.takerFee); + }); }); describe('Testing exchange of ERC20 tokens with no return values', () => { diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index 81871a680..a8ae897a8 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -205,6 +205,60 @@ describe('AssetProxyDispatcher', () => { ); }); + it('should not dispatch a transfer if amount == 0', async () => { + // Register ERC20 proxy + await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Construct metadata for ERC20 proxy + const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = constants.ZERO_AMOUNT; + const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( + encodedAssetData, + makerAddress, + takerAddress, + amount, + { from: owner }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + expect(txReceipt.logs.length).to.be.equal(0); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + + it('should not dispatch a transfer if from == to', async () => { + // Register ERC20 proxy + await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Construct metadata for ERC20 proxy + const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + + // Perform a transfer from makerAddress to takerAddress + const erc20Balances = await erc20Wrapper.getBalancesAsync(); + const amount = new BigNumber(10); + const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( + await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( + encodedAssetData, + makerAddress, + makerAddress, + amount, + { from: owner }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + expect(txReceipt.logs.length).to.be.equal(0); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.deep.equal(erc20Balances); + }); + it('should throw if dispatching to unregistered proxy', async () => { // Construct metadata for ERC20 proxy const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index da2febfd8..b25483c4b 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -14,7 +14,7 @@ import { ValidatorContract } from '../../generated_contract_wrappers/validator'; import { WalletContract } from '../../generated_contract_wrappers/wallet'; import { addressUtils } from '../utils/address_utils'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions'; +import { expectContractCallFailedAsync, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { LogDecoder } from '../utils/log_decoder'; @@ -119,7 +119,7 @@ describe('MixinSignatureValidator', () => { it('should revert when signature is empty', async () => { const emptySignature = '0x'; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -133,7 +133,7 @@ describe('MixinSignatureValidator', () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -146,7 +146,7 @@ describe('MixinSignatureValidator', () => { it('should revert when SignatureType=Illegal', async () => { const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -173,7 +173,7 @@ describe('MixinSignatureValidator', () => { const signatureBuffer = Buffer.concat([fillerData, signatureType]); const signatureHex = ethUtil.bufferToHex(signatureBuffer); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailed( + return expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, @@ -339,7 +339,7 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.Wallet}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - await expectContractCallFailed( + await expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, maliciousWallet.address, @@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => { const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - await expectContractCallFailed( + await expectContractCallFailedAsync( signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), RevertReason.ValidatorError, ); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index efdfa13a2..13640a761 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -9,7 +9,7 @@ import * as _ from 'lodash'; import { TestLibBytesContract } from '../../generated_contract_wrappers/test_lib_bytes'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed } from '../utils/assertions'; +import { expectContractCallFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { typeEncodingUtils } from '../utils/type_encoding_utils'; @@ -103,7 +103,7 @@ describe('LibBytes', () => { describe('popLastByte', () => { it('should revert if length is 0', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES), RevertReason.LibBytesGreaterThanZeroLengthRequired, ); @@ -125,7 +125,7 @@ describe('LibBytes', () => { describe('popLast20Bytes', () => { it('should revert if length is less than 20', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -200,7 +200,7 @@ describe('LibBytes', () => { describe('deepCopyBytes', () => { it('should revert if dest is shorter than source', async () => { - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes), RevertReason.LibBytesGreaterOrEqualToSourceBytesLengthRequired, ); @@ -253,7 +253,7 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold an address', async () => { const shortByteArray = '0xabcdef'; const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadAddress.callAsync(shortByteArray, offset), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -261,7 +261,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { const byteArray = testAddress; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadAddress.callAsync(byteArray, badOffset), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -297,7 +297,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold an address', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -305,7 +305,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress), RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); @@ -329,14 +329,14 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a bytes32', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes32.callAsync(testBytes32, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -372,7 +372,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a bytes32', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -380,7 +380,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -408,7 +408,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a uint256', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -418,7 +418,7 @@ describe('LibBytes', () => { const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); const byteArray = ethUtil.bufferToHex(testUint256AsBuffer); const badOffset = new BigNumber(testUint256AsBuffer.byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadUint256.callAsync(byteArray, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -458,7 +458,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold a uint256', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -466,7 +466,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { const byteArray = byteArrayLongerThan32Bytes; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -478,7 +478,7 @@ describe('LibBytes', () => { it('should revert if byte array has a length < 4', async () => { const byteArrayLessThan4Bytes = '0x010101'; const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo4LengthRequired, ); @@ -504,7 +504,7 @@ describe('LibBytes', () => { }); it('should fail if the length between the offset and end of the byte array is too short to hold a bytes4', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes4).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytes4.callAsync(testBytes4, badOffset), RevertReason.LibBytesGreaterOrEqualTo4LengthRequired, ); @@ -554,28 +554,28 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { // The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read. const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if we store a nested byte array length, without a nested byte array', async () => { const offset = new BigNumber(0); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset), RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); @@ -687,7 +687,7 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { const offset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(1)); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); @@ -695,7 +695,7 @@ describe('LibBytes', () => { it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); - return expectContractCallFailed( + return expectContractCallFailedAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData), RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts index 9515941ff..bb2b3b1a3 100644 --- a/packages/contracts/test/multisig/asset_proxy_owner.ts +++ b/packages/contracts/test/multisig/asset_proxy_owner.ts @@ -1,4 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; @@ -14,9 +15,11 @@ import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mix import { TestAssetProxyOwnerContract } from '../../generated_contract_wrappers/test_asset_proxy_owner'; import { artifacts } from '../utils/artifacts'; import { - expectContractCallFailedWithoutReasonAsync, - expectContractCreationFailedWithoutReason, + expectContractCallFailedAsync, + expectContractCreationFailedAsync, + expectTransactionFailedAsync, expectTransactionFailedWithoutReasonAsync, + sendTransactionResult, } from '../utils/assertions'; import { increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; import { chaiSetup } from '../utils/chai_setup'; @@ -109,8 +112,8 @@ describe('AssetProxyOwner', () => { }); it('should throw if a null address is included in assetProxyContracts', async () => { const assetProxyContractAddresses = [erc20Proxy.address, constants.NULL_ADDRESS]; - return expectContractCreationFailedWithoutReason( - AssetProxyOwnerContract.deployFrom0xArtifactAsync( + return expectContractCreationFailedAsync( + (AssetProxyOwnerContract.deployFrom0xArtifactAsync( artifacts.AssetProxyOwner, provider, txDefaults, @@ -118,7 +121,8 @@ describe('AssetProxyOwner', () => { assetProxyContractAddresses, REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, - ), + ) as any) as sendTransactionResult, + RevertReason.InvalidAssetProxy, ); }); }); @@ -148,25 +152,6 @@ describe('AssetProxyOwner', () => { }); }); - describe('readBytes4', () => { - it('should revert if byte array has a length < 4', async () => { - const byteArrayLessThan4Bytes = '0x010101'; - return expectContractCallFailedWithoutReasonAsync( - testAssetProxyOwner.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)), - ); - }); - it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const byteArrayLongerThan32Bytes = - '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; - const first4Bytes = await testAssetProxyOwner.publicReadBytes4.callAsync( - byteArrayLongerThan32Bytes, - new BigNumber(0), - ); - const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); - expect(first4Bytes).to.equal(expectedFirst4Bytes); - }); - }); - describe('registerAssetProxy', () => { it('should throw if not called by multisig', async () => { const isRegistered = true; @@ -300,8 +285,9 @@ describe('AssetProxyOwner', () => { ); const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>; const txId = log.args.transactionId; - return expectContractCallFailedWithoutReasonAsync( + return expectContractCallFailedAsync( testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + RevertReason.InvalidFunctionSelector, ); }); @@ -335,8 +321,9 @@ describe('AssetProxyOwner', () => { ); const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>; const txId = log.args.transactionId; - return expectContractCallFailedWithoutReasonAsync( + return expectContractCallFailedAsync( testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + RevertReason.UnregisteredAssetProxy, ); }); }); @@ -377,10 +364,11 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - return expectTransactionFailedWithoutReasonAsync( + return expectTransactionFailedAsync( testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { from: owners[1], }), + RevertReason.UnregisteredAssetProxy, ); }); @@ -399,10 +387,11 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - return expectTransactionFailedWithoutReasonAsync( + return expectTransactionFailedAsync( testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { from: owners[1], }), + RevertReason.InvalidFunctionSelector, ); }); diff --git a/packages/contracts/test/tokens/unlimited_allowance_token.ts b/packages/contracts/test/tokens/unlimited_allowance_token.ts index f2725b408..63680fe9b 100644 --- a/packages/contracts/test/tokens/unlimited_allowance_token.ts +++ b/packages/contracts/test/tokens/unlimited_allowance_token.ts @@ -5,7 +5,7 @@ import * as chai from 'chai'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed } from '../utils/assertions'; +import { expectContractCallFailedAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; @@ -54,7 +54,7 @@ describe('UnlimitedAllowanceToken', () => { it('should throw if owner has insufficient balance', async () => { const ownerBalance = await token.balanceOf.callAsync(owner); const amountToTransfer = ownerBalance.plus(1); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transfer.callAsync(spender, amountToTransfer, { from: owner }), RevertReason.Erc20InsufficientBalance, ); @@ -93,7 +93,7 @@ describe('UnlimitedAllowanceToken', () => { await token.approve.sendTransactionAsync(spender, amountToTransfer, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), @@ -109,7 +109,7 @@ describe('UnlimitedAllowanceToken', () => { const isSpenderAllowanceInsufficient = spenderAllowance.cmp(amountToTransfer) < 0; expect(isSpenderAllowanceInsufficient).to.be.true(); - return expectContractCallFailed( + return expectContractCallFailedAsync( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts index 5ddb5cc7f..53f2a4e4e 100644 --- a/packages/contracts/test/utils/artifacts.ts +++ b/packages/contracts/test/utils/artifacts.ts @@ -4,6 +4,7 @@ import * as AssetProxyOwner from '../../artifacts/AssetProxyOwner.json'; import * as DummyERC20Token from '../../artifacts/DummyERC20Token.json'; import * as DummyERC721Receiver from '../../artifacts/DummyERC721Receiver.json'; import * as DummyERC721Token from '../../artifacts/DummyERC721Token.json'; +import * as DummyMultipleReturnERC20Token from '../../artifacts/DummyMultipleReturnERC20Token.json'; import * as DummyNoReturnERC20Token from '../../artifacts/DummyNoReturnERC20Token.json'; import * as ERC20Proxy from '../../artifacts/ERC20Proxy.json'; import * as ERC721Proxy from '../../artifacts/ERC721Proxy.json'; @@ -37,6 +38,7 @@ export const artifacts = { DummyERC20Token: (DummyERC20Token as any) as ContractArtifact, DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact, DummyERC721Token: (DummyERC721Token as any) as ContractArtifact, + DummyMultipleReturnERC20Token: (DummyMultipleReturnERC20Token as any) as ContractArtifact, DummyNoReturnERC20Token: (DummyNoReturnERC20Token as any) as ContractArtifact, ERC20Proxy: (ERC20Proxy as any) as ContractArtifact, ERC721Proxy: (ERC721Proxy as any) as ContractArtifact, diff --git a/packages/contracts/test/utils/assertions.ts b/packages/contracts/test/utils/assertions.ts index 61df800c8..3361a751a 100644 --- a/packages/contracts/test/utils/assertions.ts +++ b/packages/contracts/test/utils/assertions.ts @@ -159,7 +159,7 @@ export async function expectTransactionFailedWithoutReasonAsync(p: sendTransacti * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export async function expectContractCallFailed<T>(p: Promise<T>, reason: RevertReason): Promise<void> { +export async function expectContractCallFailedAsync<T>(p: Promise<T>, reason: RevertReason): Promise<void> { return expect(p).to.be.rejectedWith(reason); } @@ -180,7 +180,20 @@ export async function expectContractCallFailedWithoutReasonAsync<T>(p: Promise<T * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export async function expectContractCreationFailedWithoutReason<T>(p: Promise<T>): Promise<void> { +export async function expectContractCreationFailedAsync<T>( + p: sendTransactionResult, + reason: RevertReason, +): Promise<void> { + return expectTransactionFailedAsync(p, reason); +} + +/** + * Resolves if the contract creation/deployment fails without a revert reason. + * @param p a Promise resulting from a contract creation/deployment + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ +export async function expectContractCreationFailedWithoutReasonAsync<T>(p: Promise<T>): Promise<void> { const errMessage = await _getTransactionFailedErrorMessageAsync(); return expect(p).to.be.rejectedWith(errMessage); } diff --git a/packages/contracts/test/utils/constants.ts b/packages/contracts/test/utils/constants.ts index ee4378d2e..b9ba8ccb9 100644 --- a/packages/contracts/test/utils/constants.ts +++ b/packages/contracts/test/utils/constants.ts @@ -60,6 +60,7 @@ export const constants = { 'MARKET_SELL_ORDERS', 'MATCH_ORDERS', 'CANCEL_ORDER', + 'BATCH_CANCEL_ORDERS', 'CANCEL_ORDERS_UP_TO', 'SET_SIGNATURE_VALIDATOR_APPROVAL', ], diff --git a/packages/contracts/test/utils/web3_wrapper.ts b/packages/contracts/test/utils/web3_wrapper.ts index acb3103b7..d1cd3d387 100644 --- a/packages/contracts/test/utils/web3_wrapper.ts +++ b/packages/contracts/test/utils/web3_wrapper.ts @@ -1,5 +1,5 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; -import { prependSubprovider } from '@0xproject/subproviders'; +import { prependSubprovider, Web3ProviderEngine } from '@0xproject/subproviders'; import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; @@ -47,7 +47,7 @@ const ganacheConfigs = { }; const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : gethConfigs; -export const provider = web3Factory.getRpcProvider(providerConfigs); +export const provider: Web3ProviderEngine = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); diff --git a/packages/contracts/tsconfig.json b/packages/contracts/tsconfig.json index 86b33ede7..5fd4c5331 100644 --- a/packages/contracts/tsconfig.json +++ b/packages/contracts/tsconfig.json @@ -2,13 +2,11 @@ "extends": "../../tsconfig", "compilerOptions": { "outDir": "lib", - "baseUrl": ".", - "declaration": false, - "allowJs": true + "rootDir": "." }, "include": [ "./globals.d.ts", - "./contract_wrappers", + "./generated_contract_wrappers", "./src/**/*", "./utils/**/*", "./test/**/*", |