diff options
author | Fabio Berger <me@fabioberger.com> | 2018-06-27 16:59:37 +0800 |
---|---|---|
committer | Fabio Berger <me@fabioberger.com> | 2018-06-27 16:59:37 +0800 |
commit | c308359f59cc24510ff3d42ae777226a599db376 (patch) | |
tree | 1b5523e4fbc3434abd4f66f87a6bb5b59ca7c52b /packages/contracts | |
parent | 375d667144e26253f8ff6b0ad1beb45d5b3ef228 (diff) | |
parent | 84bd54ba4025a33f4e311ec66d81d58b45e54d6b (diff) | |
download | dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar.gz dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar.bz2 dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar.lz dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar.xz dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.tar.zst dexon-sol-tools-c308359f59cc24510ff3d42ae777226a599db376.zip |
Merge branch 'v2-prototype' into feature/combinatorial-testing
* v2-prototype:
Improve 'web3-provider-engine' typings
Fix build
Update artifacts
Add more revert reasons to tests
Make dispatchTransferFrom revert with reason from AssetProxy on failure
Remove redundant files, hard code function selector in dispatchTransferFrom, and modify revert reason
Updated offset of receiverData length to reflect new assetData encoding.
ERC20 Custom storage layout
Golf the authorization check
ERC20 manual memory layout
ERC20 inline return
ERC721 inline return
Golf ERC721 transferFrom
Inline ERC20 tranferFrom selector constant
Optimize ERC20 transferFrom
Polish asset picker layout
Update relayer grid fallback icon
Change metamask install link based on browser
Use ethereumjs-tx@1.3.5 instead of our fork
# Conflicts:
# packages/contracts/test/asset_proxy/proxies.ts
# packages/contracts/test/exchange/core.ts
# packages/migrations/artifacts/2.0.0/DummyERC721Token.json
Diffstat (limited to 'packages/contracts')
18 files changed, 363 insertions, 736 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index e1ea401b3..1ee58081a 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -24,15 +24,17 @@ "DummyERC721Receiver", "DummyERC721Token", "ERC20Proxy", + "ERC20Token", "ERC721Proxy", "Exchange", "ExchangeWrapper", "IAssetData", + "IValidator", + "IWallet", "MixinAuthorizable", "MultiSigWallet", "MultiSigWalletWithTimeLock", "TestAssetProxyOwner", - "TestAssetDataDecoders", "TestAssetProxyDispatcher", "TestLibBytes", "TestLibs", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 95f2dbb4e..b5a599fe5 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -20,7 +20,8 @@ "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", "clean": "shx rm -rf lib src/generated_contract_wrappers", "generate_contract_wrappers": @@ -34,7 +35,7 @@ }, "config": { "abis": - "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|IAssetData|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|IAssetData|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index b74d2c231..b4780b52b 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,17 +20,140 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "./MixinAssetProxy.sol"; +import "./interfaces/IAssetProxy.sol"; import "./MixinAuthorizable.sol"; -import "./MixinERC20Transfer.sol"; contract ERC20Proxy is - MixinAssetProxy, - MixinAuthorizable, - MixinERC20Transfer + IAssetProxy, + MixinAuthorizable { // Id of this proxy. bytes4 constant PROXY_ID = bytes4(keccak256("ERC20Token(address)")); + + /// @dev Internal version of `transferFrom`. + /// @param assetData Encoded byte array. + /// @param from Address to transfer asset from. + /// @param to Address to transfer asset to. + /// @param amount Amount of asset to transfer. + function transferFrom( + bytes assetData, + address from, + address to, + uint256 amount + ) + external + { + require( + authorized[msg.sender], + "SENDER_NOT_AUTHORIZED" + ); + + // `transferFrom`. + // The function is marked `external`, so no abi decodeding is done for + // us. Instead, we expect the `calldata` memory to contain the + // following: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 4 * 32 | function parameters: | + // | | 4 | | 1. offset to assetData (*) | + // | | 36 | | 2. from | + // | | 68 | | 3. to | + // | | 100 | | 4. amount | + // | Data | | | assetData: | + // | | 132 | 32 | assetData Length | + // | | 164 | ** | assetData Contents | + // + // (*): offset is computed from start of function parameters, so offset + // by an additional 4 bytes in the calldata. + // + // WARNING: The ABIv2 specification allows additional padding between + // the Params and Data section. This will result in a larger + // offset to assetData. + + // Asset data itself is encoded as follows: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 1 * 32 | function parameters: | + // | | 4 | 12 + 20 | 1. token address | + + // Transfer tokens. + // We do a raw call so we can check the success separate + // from the return data. + // We construct calldata for the `token.transferFrom` ABI. + // The layout of this calldata is in the table below. + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 3 * 32 | function parameters: | + // | | 4 | | 1. from | + // | | 36 | | 2. to | + // | | 68 | | 3. amount | + + assembly { + /////// Token contract address /////// + // The token address is found as follows: + // * It is stored at offset 4 in `assetData` contents. + // * This is stored at offset 32 from `assetData`. + // * The offset to `assetData` from Params is stored at offset + // 4 in calldata. + // * The offset of Params in calldata is 4. + // So we read location 4 and add 32 + 4 + 4 to it. + let token := calldataload(add(calldataload(4), 40)) + + /////// Setup Header Area /////// + // This area holds the 4-byte `transferFrom` selector. + // Any trailing data in transferFromSelector will be + // overwritten in the next `mstore` call. + mstore(0, 0x23b872dd00000000000000000000000000000000000000000000000000000000) + + /////// Setup Params Area /////// + // We copy the fields `from`, `to` and `amount` in bulk + // from our own calldata to the new calldata. + calldatacopy(4, 36, 96) + + /////// Call `token.transferFrom` using the calldata /////// + let success := call( + gas, // forward all gas + token, // call address of token contract + 0, // don't send any ETH + 0, // pointer to start of input + 100, // length of input + 0, // write output over input + 32 // output size should be 32 bytes + ) + + /////// Check return data. /////// + // If there is no return data, we assume the token incorrectly + // does not return a bool. In this case we expect it to revert + // on failure, which was handled above. + // If the token does return data, we require that it is a single + // nonzero 32 bytes value. + // So the transfer succeeded if the call succeeded and either + // returned nothing, or returned a non-zero 32 byte value. + success := and(success, or( + iszero(returndatasize), + and( + eq(returndatasize, 32), + gt(mload(0), 0) + ) + )) + if success { + return(0, 0) + } + + // Revert with `Error("TRANSFER_FAILED")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + } /// @dev Gets the proxy id associated with the proxy address. /// @return Proxy id. diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index f6293f97d..8d7cea717 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,18 +20,181 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "./MixinAssetProxy.sol"; +import "./interfaces/IAssetProxy.sol"; import "./MixinAuthorizable.sol"; -import "./MixinERC721Transfer.sol"; contract ERC721Proxy is - MixinAssetProxy, - MixinAuthorizable, - MixinERC721Transfer + IAssetProxy, + MixinAuthorizable { // Id of this proxy. bytes4 constant PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)")); + /// @dev Internal version of `transferFrom`. + /// @param assetData Encoded byte array. + /// @param from Address to transfer asset from. + /// @param to Address to transfer asset to. + /// @param amount Amount of asset to transfer. + function transferFrom( + bytes assetData, + address from, + address to, + uint256 amount + ) + external + { + require( + authorized[msg.sender], + "SENDER_NOT_AUTHORIZED" + ); + + // `transferFrom`. + // The function is marked `external`, so no abi decodeding is done for + // us. Instead, we expect the `calldata` memory to contain the + // following: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 4 * 32 | function parameters: | + // | | 4 | | 1. offset to assetData (*) | + // | | 36 | | 2. from | + // | | 68 | | 3. to | + // | | 100 | | 4. amount | + // | Data | | | assetData: | + // | | 132 | 32 | assetData Length | + // | | 164 | ** | assetData Contents | + // + // (*): offset is computed from start of function parameters, so offset + // by an additional 4 bytes in the calldata. + // + // WARNING: The ABIv2 specification allows additional padding between + // the Params and Data section. This will result in a larger + // offset to assetData. + + // Asset data itself is encoded as follows: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 3 * 32 | function parameters: | + // | | 4 | 12 + 20 | 1. token address | + // | | 36 | | 2. tokenId | + // | | 68 | | 3. offset to receiverData (*) | + // | Data | | | receiverData: | + // | | 100 | 32 | receiverData Length | + // | | 132 | ** | receiverData Contents | + + // We construct calldata for the `token.safeTransferFrom` ABI. + // The layout of this calldata is in the table below. + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 4 * 32 | function parameters: | + // | | 4 | | 1. from | + // | | 36 | | 2. to | + // | | 68 | | 3. tokenId | + // | | 100 | | 4. offset to receiverData (*) | + // | Data | | | receiverData: | + // | | 132 | 32 | receiverData Length | + // | | 164 | ** | receiverData Contents | + + assembly { + // There exists only 1 of each token. + // require(amount == 1, "INVALID_AMOUNT") + if sub(calldataload(100), 1) { + // Revert with `Error("INVALID_AMOUNT")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000e494e56414c49445f414d4f554e540000000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + + // Require assetData to be at least 132 bytes + let offset := calldataload(4) + if lt(calldataload(add(offset, 4)), 132) { + // Revert with `Error("LENGTH_GREATER_THAN_131_REQUIRED")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x000000204c454e4754485f475245415445525f5448414e5f3133315f52455155) + mstore(96, 0x4952454400000000000000000000000000000000000000000000000000000000) + revert(0, 100) + } + + /////// Setup State /////// + // `cdStart` is the start of the calldata for + // `token.safeTransferFrom` (equal to free memory ptr). + let cdStart := mload(64) + // `dataAreaLength` is the total number of words + // needed to store `receiverData` + // As-per the ABI spec, this value is padded up to + // the nearest multiple of 32, + // and includes 32-bytes for length. + // It's calculated as folows: + // - Unpadded length in bytes = `mload(receiverData) + 32` + // - Add 31 to convert rounding down to rounding up. + // Combined with the previous and this is `63`. + // - Round down to nearest multiple of 32 by clearing + // bits 0x1F. This is done with `and` and a mask. + + /////// Setup Header Area /////// + // This area holds the 4-byte `transferFromSelector`. + // Any trailing data in transferFromSelector will be + // overwritten in the next `mstore` call. + mstore(cdStart, 0xb88d4fde00000000000000000000000000000000000000000000000000000000) + + /////// Setup Params Area /////// + // Each parameter is padded to 32-bytes. + // The entire Params Area is 128 bytes. + // Notes: + // 1. A 20-byte mask is applied to addresses + // to zero-out the unused bytes. + // 2. The offset to `receiverData` is the length + // of the Params Area (128 bytes). + + let length := calldataload(add(offset, 136)) + let token := calldataload(add(offset, 40)) + + // Round length up to multiple of 32 + length := and(add(length, 31), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0) + + // Copy `from` and `to` + calldatacopy(add(cdStart, 4), 36, 64) + + // TokenId + mstore(add(cdStart, 68), calldataload(add(offset, 72))) + + // Offset to receiverData + mstore(add(cdStart, 100), 128) + + // receiverData (including length) + calldatacopy(add(cdStart, 132), add(offset, 136), add(length, 32)) + + /////// Call `token.safeTransferFrom` using the calldata /////// + let success := call( + gas, // forward all gas + token, // call address of token contract + 0, // don't send any ETH + cdStart, // pointer to start of input + add(length, 164), // length of input + 0, // write output to null + 0 // output size is 0 bytes + ) + if success { + return(0, 0) + } + + // Revert with `Error("TRANSFER_FAILED")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + } + /// @dev Gets the proxy id associated with the proxy address. /// @return Proxy id. function getProxyId() diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol deleted file mode 100644 index 9032658e7..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol +++ /dev/null @@ -1,75 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "./mixins/MAuthorizable.sol"; -import "./mixins/MAssetProxy.sol"; - -contract MixinAssetProxy is - MAuthorizable, - MAssetProxy -{ - - /// @dev Transfers assets. Either succeeds or throws. - /// @param assetData Encoded byte array. - /// @param from Address to transfer asset from. - /// @param to Address to transfer asset to. - /// @param amount Amount of asset to transfer. - function transferFrom( - bytes assetData, - address from, - address to, - uint256 amount - ) - external - onlyAuthorized - { - transferFromInternal( - assetData, - from, - to, - amount - ); - } - - /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetData Array of byte arrays encoded for the respective asset proxy. - /// @param from Array of addresses to transfer assets from. - /// @param to Array of addresses to transfer assets to. - /// @param amounts Array of amounts of assets to transfer. - function batchTransferFrom( - bytes[] memory assetData, - address[] memory from, - address[] memory to, - uint256[] memory amounts - ) - public - onlyAuthorized - { - for (uint256 i = 0; i < assetData.length; i++) { - transferFromInternal( - assetData[i], - from[i], - to[i], - amounts[i] - ); - } - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol deleted file mode 100644 index a09db43bd..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ /dev/null @@ -1,113 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "../../utils/LibBytes/LibBytes.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; - -contract MixinERC20Transfer { - - using LibBytes for bytes; - - /// @dev Internal version of `transferFrom`. - /// @param assetData Encoded byte array. - /// @param from Address to transfer asset from. - /// @param to Address to transfer asset to. - /// @param amount Amount of asset to transfer. - function transferFromInternal( - bytes memory assetData, - address from, - address to, - uint256 amount - ) - internal - { - // Decode asset data. - address token = assetData.readAddress(16); - - // Transfer tokens. - // We do a raw call so we can check the success separate - // from the return data. - // We construct calldata for the `token.transferFrom` ABI. - // The layout of this calldata is in the table below. - // - // | Area | Offset | Length | Contents | - // |----------|--------|---------|-------------------------------------| - // | Header | 0 | 4 | function selector | - // | Params | | 3 * 32 | function parameters: | - // | | 4 | | 1. from | - // | | 36 | | 2. to | - // | | 68 | | 3. amount | - - bytes4 transferFromSelector = IERC20Token(token).transferFrom.selector; - bool success; - assembly { - /////// Setup State /////// - // `cdStart` is the start of the calldata for `token.transferFrom` (equal to free memory ptr). - let cdStart := mload(64) - - /////// Setup Header Area /////// - // This area holds the 4-byte `transferFromSelector`. - // Any trailing data in transferFromSelector will be - // overwritten in the next `mstore` call. - mstore(cdStart, transferFromSelector) - - /////// Setup Params Area /////// - // Each parameter is padded to 32-bytes. - // The entire Params Area is 96 bytes. - // A 20-byte mask is applied to addresses to - // zero-out the unused bytes. - mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) - mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) - mstore(add(cdStart, 68), amount) - - /////// Call `token.transferFrom` using the calldata /////// - success := call( - gas, // forward all gas - token, // call address of token contract - 0, // don't send any ETH - cdStart, // pointer to start of input - 100, // length of input - cdStart, // write output over input - 32 // output size should be 32 bytes - ) - - /////// Check return data. /////// - // If there is no return data, we assume the token incorrectly - // does not return a bool. In this case we expect it to revert - // on failure, which was handled above. - // If the token does return data, we require that it is a single - // nonzero 32 bytes value. - // So the transfer succeeded if the call succeeded and either - // returned nothing, or returned a non-zero 32 byte value. - success := and(success, or( - iszero(returndatasize), - and( - eq(returndatasize, 32), - gt(mload(cdStart), 0) - ) - )) - } - require( - success, - "TRANSFER_FAILED" - ); - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol deleted file mode 100644 index c170917ea..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ /dev/null @@ -1,168 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "../../utils/LibBytes/LibBytes.sol"; -import "../../tokens/ERC721Token/ERC721Token.sol"; -import "./libs/LibTransferErrors.sol"; - -contract MixinERC721Transfer is - LibTransferErrors -{ - using LibBytes for bytes; - - bytes4 constant SAFE_TRANSFER_FROM_SELECTOR = bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)")); - - /// @dev Internal version of `transferFrom`. - /// @param assetData Encoded byte array. - /// @param from Address to transfer asset from. - /// @param to Address to transfer asset to. - /// @param amount Amount of asset to transfer. - function transferFromInternal( - bytes memory assetData, - address from, - address to, - uint256 amount - ) - internal - { - // There exists only 1 of each token. - require( - amount == 1, - INVALID_AMOUNT - ); - - // Decode asset data. - ( - address token, - uint256 tokenId, - bytes memory receiverData - ) = decodeERC721AssetData(assetData); - - // We construct calldata for the `token.safeTransferFrom` ABI. - // The layout of this calldata is in the table below. - // - // | Area | Offset | Length | Contents | - // |----------|--------|---------|-------------------------------------| - // | Header | 0 | 4 | function selector | - // | Params | | 4 * 32 | function parameters: | - // | | 4 | | 1. from | - // | | 36 | | 2. to | - // | | 68 | | 3. tokenId | - // | | 100 | | 4. offset to receiverData (*) | - // | Data | | | receiverData: | - // | | 132 | 32 | receiverData Length | - // | | 164 | ** | receiverData Contents | - - bytes4 safeTransferFromSelector = SAFE_TRANSFER_FROM_SELECTOR; - bool success; - assembly { - /////// Setup State /////// - // `cdStart` is the start of the calldata for - // `token.safeTransferFrom` (equal to free memory ptr). - let cdStart := mload(64) - // `dataAreaLength` is the total number of words - // needed to store `receiverData` - // As-per the ABI spec, this value is padded up to - // the nearest multiple of 32, - // and includes 32-bytes for length. - // It's calculated as folows: - // - Unpadded length in bytes = `mload(receiverData) + 32` - // - Add 31 to convert rounding down to rounding up. - // Combined with the previous and this is `63`. - // - Round down to nearest multiple of 32 by clearing - // bits 0x1F. This is done with `and` and a mask. - let dataAreaLength := and(add(mload(receiverData), 63), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0) - // `cdEnd` is the end of the calldata for `token.safeTransferFrom`. - let cdEnd := add(cdStart, add(132, dataAreaLength)) - - /////// Setup Header Area /////// - // This area holds the 4-byte `transferFromSelector`. - // Any trailing data in transferFromSelector will be - // overwritten in the next `mstore` call. - mstore(cdStart, safeTransferFromSelector) - - /////// Setup Params Area /////// - // Each parameter is padded to 32-bytes. - // The entire Params Area is 128 bytes. - // Notes: - // 1. A 20-byte mask is applied to addresses - // to zero-out the unused bytes. - // 2. The offset to `receiverData` is the length - // of the Params Area (128 bytes). - mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) - mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) - mstore(add(cdStart, 68), tokenId) - mstore(add(cdStart, 100), 128) - - /////// Setup Data Area /////// - // This area holds `receiverData`. - let dataArea := add(cdStart, 132) - for {} lt(dataArea, cdEnd) {} { - mstore(dataArea, mload(receiverData)) - dataArea := add(dataArea, 32) - receiverData := add(receiverData, 32) - } - - /////// Call `token.safeTransferFrom` using the calldata /////// - success := call( - gas, // forward all gas - token, // call address of token contract - 0, // don't send any ETH - cdStart, // pointer to start of input - sub(cdEnd, cdStart), // length of input - cdStart, // write output over input - 0 // output size is 0 bytes - ) - } - require( - success, - TRANSFER_FAILED - ); - } - - /// @dev Decodes ERC721 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC721 proxy id. - /// @return token ERC721 token address. - /// @return tokenId ERC721 token id. - /// @return receiverData Additional data with no specific format, which - /// is passed to the receiving contract's onERC721Received. - function decodeERC721AssetData(bytes memory assetData) - internal - pure - returns ( - address token, - uint256 tokenId, - bytes memory receiverData - ) - { - // Decode asset data. - token = assetData.readAddress(16); - tokenId = assetData.readUint256(36); - receiverData = assetData.readBytesWithLength(100); - - return ( - token, - tokenId, - receiverData - ); - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol index d12d23610..ae8e195da 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol @@ -38,19 +38,6 @@ contract IAssetProxy is ) external; - /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetData Array of byte arrays encoded for the respective asset proxy. - /// @param from Array of addresses to transfer assets from. - /// @param to Array of addresses to transfer assets to. - /// @param amounts Array of amounts of assets to transfer. - function batchTransferFrom( - bytes[] memory assetData, - address[] memory from, - address[] memory to, - uint256[] memory amounts - ) - public; - /// @dev Gets the proxy id associated with the proxy address. /// @return Proxy id. function getProxyId() diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index b0b20c044..338cb12e2 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -23,9 +23,14 @@ pragma solidity ^0.4.24; contract LibAssetProxyErrors { /// Authorizable errors /// - string constant SENDER_NOT_AUTHORIZED = "SENDER_NOT_AUTHORIZED"; // Sender not authorized to call this method. - string constant TARGET_NOT_AUTHORIZED = "TARGET_NOT_AUTHORIZED"; // Target address not authorized to call this method. - string constant TARGET_ALREADY_AUTHORIZED = "TARGET_ALREADY_AUTHORIZED"; // Target address must not already be authorized. - string constant INDEX_OUT_OF_BOUNDS = "INDEX_OUT_OF_BOUNDS"; // Specified array index is out of bounds. - string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address. + string constant SENDER_NOT_AUTHORIZED = "SENDER_NOT_AUTHORIZED"; // Sender not authorized to call this method. + string constant TARGET_NOT_AUTHORIZED = "TARGET_NOT_AUTHORIZED"; // Target address not authorized to call this method. + string constant TARGET_ALREADY_AUTHORIZED = "TARGET_ALREADY_AUTHORIZED"; // Target address must not already be authorized. + string constant INDEX_OUT_OF_BOUNDS = "INDEX_OUT_OF_BOUNDS"; // Specified array index is out of bounds. + string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address. + + /// Transfer errors /// + string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. + string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. + string constant LENGTH_GREATER_THAN_131_REQUIRED = "LENGTH_GREATER_THAN_131_REQUIRED"; // Byte array must have a length greater than 0. } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol deleted file mode 100644 index 88187f196..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol +++ /dev/null @@ -1,28 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; - -/// @dev This contract documents the revert reasons used in the `transferFrom` methods of different AssetProxy contracts. -/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons. -contract LibTransferErrors { - - /// Transfer errors /// - string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. - string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol deleted file mode 100644 index a52cb56f9..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol +++ /dev/null @@ -1,40 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "../interfaces/IAssetProxy.sol"; - -contract MAssetProxy is - IAssetProxy -{ - - /// @dev Internal version of `transferFrom`. - /// @param assetData Encoded byte array. - /// @param from Address to transfer asset from. - /// @param to Address to transfer asset to. - /// @param amount Amount of asset to transfer. - function transferFromInternal( - bytes memory assetData, - address from, - address to, - uint256 amount - ) - internal; -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 3203322aa..f7086f543 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -135,7 +135,6 @@ contract MixinAssetProxyDispatcher is // | | 132 | 32 | assetData Length | // | | 164 | ** | assetData Contents | - bytes4 transferFromSelector = IAssetProxy(assetProxy).transferFrom.selector; bool success; assembly { /////// Setup State /////// @@ -151,7 +150,8 @@ contract MixinAssetProxyDispatcher is /////// Setup Header Area /////// // This area holds the 4-byte `transferFromSelector`. - mstore(cdStart, transferFromSelector) + // bytes4(keccak256("transferFrom(bytes,address,address,uint256)")) = 0xa85e59e4 + mstore(cdStart, 0xa85e59e400000000000000000000000000000000000000000000000000000000) /////// Setup Params Area /////// // Each parameter is padded to 32-bytes. The entire Params Area is 128 bytes. @@ -180,13 +180,12 @@ contract MixinAssetProxyDispatcher is cdStart, // pointer to start of input sub(cdEnd, cdStart), // length of input cdStart, // write output over input - 0 // output size is 0 bytes + 512 // reserve 512 bytes for output ) + if eq(success, 0) { + revert(cdStart, returndatasize()) + } } - require( - success, - "TRANSFER_FAILED" - ); } } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol deleted file mode 100644 index 5987291d2..000000000 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ /dev/null @@ -1,56 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "../../protocol/AssetProxy/ERC20Proxy.sol"; -import "../../protocol/AssetProxy/ERC721Proxy.sol"; - -contract TestAssetDataDecoders is - ERC721Proxy -{ - /// @dev Decodes ERC721 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC721 proxy id. - /// @return token ERC721 token address. - /// @return tokenId ERC721 token id. - /// @return receiverData Additional data with no specific format, which - /// is passed to the receiving contract's onERC721Received. - function publicDecodeERC721Data(bytes memory assetData) - public - pure - returns ( - address token, - uint256 tokenId, - bytes memory receiverData - ) - { - ( - token, - tokenId, - receiverData - ) = decodeERC721AssetData(assetData); - - return ( - token, - tokenId, - receiverData - ); - } -} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index a46bab9ae..d58e7973c 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,7 +11,6 @@ import * as ExchangeWrapper from '../artifacts/ExchangeWrapper.json'; import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; -import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetProxyOwner from '../artifacts/TestAssetProxyOwner.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; @@ -39,7 +38,6 @@ export const artifacts = { MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyOwner: (TestAssetProxyOwner as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, - TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index c2295dda6..8c9d0495d 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -5,10 +5,7 @@ import * as chai from 'chai'; import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable'; import { artifacts } from '../../src/utils/artifacts'; -import { - expectRevertOrAlwaysFailingTransactionAsync, - expectRevertReasonOrAlwaysFailingTransactionAsync, -} from '../../src/utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; @@ -118,10 +115,11 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: notOwner, }), + RevertReason.OnlyContractOwner, ); }); it('should throw if index is >= authorities.length', async () => { @@ -130,18 +128,20 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const index = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner, }), + RevertReason.IndexOutOfBounds, ); }); it('should throw if owner attempts to remove an address that is not authorized', async () => { const index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner, }), + RevertReason.TargetNotAuthorized, ); }); it('should throw if address at index does not match target', async () => { @@ -156,10 +156,11 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const address1Index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, { from: owner, }), + RevertReason.AuthorizedAddressMismatch, ); }); it('should allow owner to remove an authorized address', async () => { diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts deleted file mode 100644 index 902255caf..000000000 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; -import { BigNumber } from '@0xproject/utils'; -import * as chai from 'chai'; -import ethUtil = require('ethereumjs-util'); - -import { TestAssetDataDecodersContract } from '../../src/generated_contract_wrappers/test_asset_data_decoders'; -import { artifacts } from '../../src/utils/artifacts'; -import { chaiSetup } from '../../src/utils/chai_setup'; -import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; - -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - -describe('TestAssetDataDecoders', () => { - let testAssetProxyDecoder: TestAssetDataDecodersContract; - let testAddress: string; - - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - // Setup accounts & addresses - const accounts = await web3Wrapper.getAvailableAddressesAsync(); - testAddress = accounts[0]; - testAssetProxyDecoder = await TestAssetDataDecodersContract.deployFrom0xArtifactAsync( - artifacts.TestAssetDataDecoders, - provider, - txDefaults, - ); - }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - - describe('Asset Data Decoders', () => { - it('should correctly decode ERC721 asset data', async () => { - const tokenId = generatePseudoRandomSalt(); - const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId); - const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedTokenAddress: string; - let decodedTokenId: BigNumber; - let decodedData: string; - [ - decodedTokenAddress, - decodedTokenId, - decodedData, - ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); - expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); - expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData); - }); - - it('should correctly decode ERC721 asset data with receiver data', async () => { - const tokenId = generatePseudoRandomSalt(); - const receiverDataFirst32Bytes = ethUtil.bufferToHex( - assetProxyUtils.encodeUint256(generatePseudoRandomSalt()), - ); - const receiverDataExtraBytes = 'FFFF'; - // We add extra bytes to generate a value that doesn't fit perfectly into one word - const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes; - const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData); - const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedTokenAddress: string; - let decodedTokenId: BigNumber; - let decodedReceiverData: string; - [ - decodedTokenAddress, - decodedTokenId, - decodedReceiverData, - ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); - expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); - expect(decodedReceiverData).to.be.equal(expectedDecodedAssetData.receiverData); - }); - }); -}); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 869ba4b2a..6303a50fa 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -5,7 +5,6 @@ import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); -import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c20_token'; import { @@ -16,10 +15,7 @@ import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c20_proxy'; import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { artifacts } from '../../src/utils/artifacts'; -import { - expectRevertOrAlwaysFailingTransactionAsync, - expectRevertReasonOrAlwaysFailingTransactionAsync, -} from '../../src/utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -192,58 +188,6 @@ describe('Asset Transfer Proxies', () => { }); }); - describe('batchTransferFrom ERC20', () => { - it('should succesfully make multiple token transfers', async () => { - const erc20Balances = await erc20Wrapper.getBalancesAsync(); - - const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); - const amount = new BigNumber(10); - const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => amount); - - const txHash = await erc20Proxy.batchTransferFrom.sendTransactionAsync( - assetData, - fromAddresses, - toAddresses, - amounts, - { from: exchangeAddress }, - ); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(res.logs.length).to.equal(numTransfers); - expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[makerAddress][zrxToken.address].minus(amount.times(numTransfers)), - ); - expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][zrxToken.address].add(amount.times(numTransfers)), - ); - }); - - it('should throw if not called by an authorized address', async () => { - const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); - const amount = new BigNumber(10); - const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => amount); - - return expectRevertReasonOrAlwaysFailingTransactionAsync( - erc20Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { - from: notAuthorized, - }), - RevertReason.SenderNotAuthorized, - ); - }); - }); - it('should have an id of 0xf47261b0', async () => { const proxyId = await erc20Proxy.getProxyId.callAsync(); const expectedProxyId = '0xf47261b0'; @@ -354,7 +298,7 @@ describe('Asset Transfer Proxies', () => { expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -362,6 +306,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), + RevertReason.TransferFailed, ); }); @@ -443,64 +388,6 @@ describe('Asset Transfer Proxies', () => { }); }); - describe('batchTransferFrom ERC721', () => { - it('should succesfully make multiple token transfers', async () => { - const erc721TokensById = await erc721Wrapper.getBalancesAsync(); - const [makerTokenIdA, makerTokenIdB] = erc721TokensById[makerAddress][erc721Token.address]; - - const numTransfers = 2; - const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB), - ]; - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => new BigNumber(1)); - - const txHash = await erc721Proxy.batchTransferFrom.sendTransactionAsync( - assetData, - fromAddresses, - toAddresses, - amounts, - { from: exchangeAddress }, - ); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - // During an ERC721 transfer enabled by a specific token approval, - // the approval is cleared, emitting an event - const numApprovals = 2; - expect(res.logs.length).to.equal(numTransfers + numApprovals); - - const newOwnerMakerAssetA = await erc721Token.ownerOf.callAsync(makerTokenIdA); - const newOwnerMakerAssetB = await erc721Token.ownerOf.callAsync(makerTokenIdB); - expect(newOwnerMakerAssetA).to.be.bignumber.equal(takerAddress); - expect(newOwnerMakerAssetB).to.be.bignumber.equal(takerAddress); - }); - - it('should throw if not called by an authorized address', async () => { - const erc721TokensById = await erc721Wrapper.getBalancesAsync(); - const [makerTokenIdA, makerTokenIdB] = erc721TokensById[makerAddress][erc721Token.address]; - - const numTransfers = 2; - const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2), - ]; - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => new BigNumber(1)); - - return expectRevertReasonOrAlwaysFailingTransactionAsync( - erc721Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { - from: notAuthorized, - }), - RevertReason.SenderNotAuthorized, - ); - }); - }); - it('should have an id of 0x08e937fa', async () => { const proxyId = await erc721Proxy.getProxyId.callAsync(); const expectedProxyId = '0x08e937fa'; diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index b3c3a891c..76d8063f8 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -13,10 +13,7 @@ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c2 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { CancelContractEventArgs, ExchangeContract } from '../../src/generated_contract_wrappers/exchange'; import { artifacts } from '../../src/utils/artifacts'; -import { - expectRevertOrAlwaysFailingTransactionAsync, - expectRevertReasonOrAlwaysFailingTransactionAsync, -} from '../../src/utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -364,8 +361,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), + RevertReason.TransferFailed, ); }); @@ -386,8 +384,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.not.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), + RevertReason.TransferFailed, ); }); @@ -410,7 +409,7 @@ describe('Exchange core', () => { const takerAssetFillAmount = signedOrder.takerAssetAmount; return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - RevertReason.TransferFailed, + RevertReason.InvalidAmount, ); }); @@ -433,7 +432,7 @@ describe('Exchange core', () => { const takerAssetFillAmount = signedOrder.takerAssetAmount; return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - RevertReason.TransferFailed, + RevertReason.InvalidAmount, ); }); @@ -453,6 +452,32 @@ describe('Exchange core', () => { RevertReason.RoundingError, ); }); + + it('should throw if assetData has a length < 132', async () => { + // Construct Exchange parameters + const makerAssetId = erc721MakerAssetIds[0]; + const takerAssetId = erc721TakerAssetIds[0]; + const makerAssetData = assetProxyUtils + .encodeERC721AssetData(erc721Token.address, makerAssetId) + .slice(0, -2); + signedOrder = orderFactory.newSignedOrder({ + makerAssetAmount: new BigNumber(1), + takerAssetAmount: new BigNumber(1), + makerAssetData, + takerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, takerAssetId), + }); + // Verify pre-conditions + const initialOwnerMakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); + expect(initialOwnerMakerAsset).to.be.bignumber.equal(makerAddress); + const initialOwnerTakerAsset = await erc721Token.ownerOf.callAsync(takerAssetId); + expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); + // Call Exchange + const takerAssetFillAmount = signedOrder.takerAssetAmount; + return expectRevertReasonOrAlwaysFailingTransactionAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), + RevertReason.LengthGreaterThan131Required, + ); + }); }); }); // tslint:disable:max-file-line-count |