diff options
Diffstat (limited to 'packages/contracts/src')
38 files changed, 923 insertions, 874 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 7ca823d1f..b4780b52b 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,24 +20,147 @@ 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. - uint8 constant PROXY_ID = 1; + 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. function getProxyId() external view - returns (uint8) + returns (bytes4) { 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 7ff25aea3..8d7cea717 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,24 +20,187 @@ 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. - uint8 constant PROXY_ID = 2; + 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() external view - returns (uint8) + returns (bytes4) { return PROXY_ID; } 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/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol index 8cb4254c5..3b9584a44 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol @@ -19,12 +19,10 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "./libs/LibAssetProxyErrors.sol"; import "../../utils/Ownable/Ownable.sol"; import "./mixins/MAuthorizable.sol"; contract MixinAuthorizable is - LibAssetProxyErrors, Ownable, MAuthorizable { @@ -33,7 +31,7 @@ contract MixinAuthorizable is modifier onlyAuthorized { require( authorized[msg.sender], - SENDER_NOT_AUTHORIZED + "SENDER_NOT_AUTHORIZED" ); _; } @@ -49,7 +47,7 @@ contract MixinAuthorizable is { require( !authorized[target], - TARGET_ALREADY_AUTHORIZED + "TARGET_ALREADY_AUTHORIZED" ); authorized[target] = true; @@ -65,11 +63,11 @@ contract MixinAuthorizable is { require( authorized[target], - TARGET_NOT_AUTHORIZED + "TARGET_NOT_AUTHORIZED" ); delete authorized[target]; - for (uint i = 0; i < authorities.length; i++) { + for (uint256 i = 0; i < authorities.length; i++) { if (authorities[i] == target) { authorities[i] = authorities[authorities.length - 1]; authorities.length -= 1; @@ -87,14 +85,19 @@ contract MixinAuthorizable is uint256 index ) external + onlyOwner { require( + authorized[target], + "TARGET_NOT_AUTHORIZED" + ); + require( index < authorities.length, - INDEX_OUT_OF_BOUNDS + "INDEX_OUT_OF_BOUNDS" ); require( authorities[index] == target, - AUTHORIZED_ADDRESS_MISMATCH + "AUTHORIZED_ADDRESS_MISMATCH" ); delete authorized[target]; 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 4af39a00b..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ /dev/null @@ -1,81 +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"; -import "./libs/LibTransferErrors.sol"; - -contract MixinERC20Transfer is - LibBytes, - LibTransferErrors -{ - /// @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 = readAddress(assetData, 0); - - // Transfer tokens. - // We do a raw call so we can check the success separate - // from the return data. - bool success = token.call(abi.encodeWithSelector( - IERC20Token(token).transferFrom.selector, - from, - to, - amount - )); - require( - success, - TRANSFER_FAILED - ); - - // 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 - // value that evaluates to true. - assembly { - if returndatasize { - success := 0 - if eq(returndatasize, 32) { - // First 64 bytes of memory are reserved scratch space - returndatacopy(0, 0, 32) - success := mload(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 6e3156e8a..000000000 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ /dev/null @@ -1,93 +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 - LibBytes, - LibTransferErrors -{ - /// @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); - - ERC721Token(token).safeTransferFrom( - from, - to, - tokenId, - receiverData - ); - } - - /// @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 = readAddress(assetData, 0); - tokenId = readUint256(assetData, 20); - if (assetData.length > 52) { - receiverData = readBytes(assetData, 52); - } - - return ( - token, - tokenId, - receiverData - ); - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol index ba784ab22..8c78ee8c4 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol @@ -16,10 +16,21 @@ */ -pragma solidity ^0.4.24; +pragma solidity ^0.4.23; -contract LibTransferErrors { - /// Transfer errors /// - string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. - string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. +// @dev Interface of the asset proxy's assetData. +// The asset proxies take an ABI encoded `bytes assetData` as argument. +// This argument is ABI encoded as one of the methods of this interface. +interface IAssetData { + + function ERC20Token( + address tokenContract) + external pure; + + function ERC721Token( + address tokenContract, + uint256 tokenId, + bytes receiverData) + external pure; + } 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 22f43b12f..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,24 +38,10 @@ 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() external view - returns (uint8); + returns (bytes4); } - 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 dca4f400f..338cb12e2 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -18,11 +18,19 @@ pragma solidity ^0.4.24; +/// @dev This contract documents the revert reasons used in the AssetProxy contracts. +/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons. 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/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/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 7f5f056b5..eb58b3374 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -22,24 +22,24 @@ import "../../multisig/MultiSigWalletWithTimeLock.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is - LibBytes, MultiSigWalletWithTimeLock { + using LibBytes for bytes; event AssetProxyRegistration(address assetProxyContract, bool isRegistered); // Mapping of AssetProxy contract address => - // if this contract is allowed to call the AssetProxy's removeAuthorizedAddress method without a time lock. + // if this contract is allowed to call the AssetProxy's `removeAuthorizedAddressAtIndex` method without a time lock. mapping (address => bool) public isAssetProxyRegistered; - bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); + bytes4 constant REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR = bytes4(keccak256("removeAuthorizedAddressAtIndex(address,uint256)")); - /// @dev Function will revert if the transaction does not call `removeAuthorizedAddress` + /// @dev Function will revert if the transaction does not call `removeAuthorizedAddressAtIndex` /// on an approved AssetProxy contract. - modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { + modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); - require(isFunctionRemoveAuthorizedAddress(tx.data)); + require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); _; } @@ -66,7 +66,7 @@ contract AssetProxyOwner is } /// @dev Registers or deregisters an AssetProxy to be able to execute - /// removeAuthorizedAddress without a timelock. + /// `removeAuthorizedAddressAtIndex` without a timelock. /// @param assetProxyContract Address of AssetProxy contract. /// @param isRegistered Status of approval for AssetProxy contract. function registerAssetProxy(address assetProxyContract, bool isRegistered) @@ -78,13 +78,13 @@ contract AssetProxyOwner is AssetProxyRegistration(assetProxyContract, isRegistered); } - /// @dev Allows execution of removeAuthorizedAddress without time lock. + /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock. /// @param transactionId Transaction ID. - function executeRemoveAuthorizedAddress(uint256 transactionId) + function executeRemoveAuthorizedAddressAtIndex(uint256 transactionId) public notExecuted(transactionId) fullyConfirmed(transactionId) - validRemoveAuthorizedAddressTx(transactionId) + validRemoveAuthorizedAddressAtIndexTx(transactionId) { Transaction storage tx = transactions[transactionId]; tx.executed = true; @@ -95,17 +95,4 @@ contract AssetProxyOwner is tx.executed = false; } } - - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. - /// @param data Transaction data. - /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes memory data) - public - pure - returns (bool) - { - bytes4 first4Bytes = readFirst4(data); - require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); - return true; - } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index f85019012..f7086f543 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -20,18 +20,17 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, - LibBytes, - LibExchangeErrors, MAssetProxyDispatcher { + using LibBytes for bytes; + // Mapping from Asset Proxy Id's to their respective Asset Proxy - mapping (uint8 => IAssetProxy) public assetProxies; + mapping (bytes4 => IAssetProxy) public assetProxies; /// @dev Registers an asset proxy to an asset proxy id. /// An id can only be assigned to a single proxy at a given time. @@ -39,7 +38,7 @@ contract MixinAssetProxyDispatcher is /// @param newAssetProxy Address of new asset proxy to register, or 0x0 to unset assetProxyId. /// @param oldAssetProxy Existing asset proxy to overwrite, or 0x0 if assetProxyId is currently unused. function registerAssetProxy( - uint8 assetProxyId, + bytes4 assetProxyId, address newAssetProxy, address oldAssetProxy ) @@ -50,17 +49,17 @@ contract MixinAssetProxyDispatcher is address currentAssetProxy = assetProxies[assetProxyId]; require( oldAssetProxy == currentAssetProxy, - ASSET_PROXY_MISMATCH + "ASSET_PROXY_MISMATCH" ); IAssetProxy assetProxy = IAssetProxy(newAssetProxy); // Ensure that the id of newAssetProxy matches the passed in assetProxyId, unless it is being reset to 0. if (newAssetProxy != address(0)) { - uint8 newAssetProxyId = assetProxy.getProxyId(); + bytes4 newAssetProxyId = assetProxy.getProxyId(); require( newAssetProxyId == assetProxyId, - ASSET_PROXY_ID_MISMATCH + "ASSET_PROXY_ID_MISMATCH" ); } @@ -76,7 +75,7 @@ contract MixinAssetProxyDispatcher is /// @dev Gets an asset proxy. /// @param assetProxyId Id of the asset proxy. /// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered. - function getAssetProxy(uint8 assetProxyId) + function getAssetProxy(bytes4 assetProxyId) external view returns (address) @@ -85,14 +84,12 @@ contract MixinAssetProxyDispatcher is } /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetData Byte array encoded for the respective asset proxy. - /// @param assetProxyId Id of assetProxy to dispach to. + /// @param assetData Byte array encoded for the asset. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, - uint8 assetProxyId, address from, address to, uint256 amount @@ -101,20 +98,94 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { + // Ensure assetData length is valid + require( + assetData.length > 3, + "LENGTH_GREATER_THAN_3_REQUIRED" + ); + // Lookup assetProxy + bytes4 assetProxyId; + assembly { + assetProxyId := and(mload( + add(assetData, 32)), + 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000 + ) + } IAssetProxy assetProxy = assetProxies[assetProxyId]; + // Ensure that assetProxy exists require( assetProxy != address(0), - ASSET_PROXY_DOES_NOT_EXIST - ); - // transferFrom will either succeed or throw. - assetProxy.transferFrom( - assetData, - from, - to, - amount + "ASSET_PROXY_DOES_NOT_EXIST" ); + + // We construct calldata for the `assetProxy.transferFrom` 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. offset to assetData (*) | + // | | 36 | | 2. from | + // | | 68 | | 3. to | + // | | 100 | | 4. amount | + // | Data | | | assetData: | + // | | 132 | 32 | assetData Length | + // | | 164 | ** | assetData Contents | + + bool success; + assembly { + /////// Setup State /////// + // `cdStart` is the start of the calldata for `assetProxy.transferFrom` (equal to free memory ptr). + let cdStart := mload(64) + // `dataAreaLength` is the total number of words needed to store `assetData` + // As-per the ABI spec, this value is padded up to the nearest multiple of 32, + // and includes 32-bytes for length. + let dataAreaLength := and(add(mload(assetData), 63), 0xFFFFFFFFFFFE0) + // `cdEnd` is the end of the calldata for `assetProxy.transferFrom`. + let cdEnd := add(cdStart, add(132, dataAreaLength)) + + + /////// Setup Header Area /////// + // This area holds the 4-byte `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. + // Notes: + // 1. The offset to `assetData` is the length of the Params Area (128 bytes). + // 2. A 20-byte mask is applied to addresses to zero-out the unused bytes. + mstore(add(cdStart, 4), 128) + mstore(add(cdStart, 36), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 68), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 100), amount) + + /////// Setup Data Area /////// + // This area holds `assetData`. + let dataArea := add(cdStart, 132) + for {} lt(dataArea, cdEnd) {} { + mstore(dataArea, mload(assetData)) + dataArea := add(dataArea, 32) + assetData := add(assetData, 32) + } + + /////// Call `assetProxy.transferFrom` using the constructed calldata /////// + success := call( + gas, // forward all gas + assetProxy, // call address of asset proxy + 0, // don't send any ETH + cdStart, // pointer to start of input + sub(cdEnd, cdStart), // length of input + cdStart, // write output over input + 512 // reserve 512 bytes for output + ) + if eq(success, 0) { + revert(cdStart, returndatasize()) + } + } } } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c227d210f..c0ed023ac 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -20,11 +20,9 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "./libs/LibConstants.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; import "./libs/LibMath.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; @@ -32,11 +30,9 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is LibConstants, - LibBytes, LibMath, LibOrder, LibFillResults, - LibExchangeErrors, MAssetProxyDispatcher, MExchangeCore, MSignatureValidator, @@ -72,7 +68,7 @@ contract MixinExchangeCore is // Ensure orderEpoch is monotonically increasing require( newOrderEpoch > oldOrderEpoch, - INVALID_NEW_ORDER_EPOCH + "INVALID_NEW_ORDER_EPOCH" ); // Update orderEpoch @@ -284,20 +280,20 @@ contract MixinExchangeCore is // An order can only be filled if its status is FILLABLE. require( orderInfo.orderStatus == uint8(OrderStatus.FILLABLE), - ORDER_UNFILLABLE + "ORDER_UNFILLABLE" ); // Revert if fill amount is invalid require( takerAssetFillAmount != 0, - INVALID_TAKER_AMOUNT + "INVALID_TAKER_AMOUNT" ); // Validate sender is allowed to fill this order if (order.senderAddress != address(0)) { require( order.senderAddress == msg.sender, - INVALID_SENDER + "INVALID_SENDER" ); } @@ -305,7 +301,7 @@ contract MixinExchangeCore is if (order.takerAddress != address(0)) { require( order.takerAddress == takerAddress, - INVALID_TAKER + "INVALID_TAKER" ); } @@ -317,7 +313,7 @@ contract MixinExchangeCore is order.makerAddress, signature ), - INVALID_ORDER_SIGNATURE + "INVALID_ORDER_SIGNATURE" ); } @@ -328,7 +324,7 @@ contract MixinExchangeCore is order.takerAssetAmount, order.makerAssetAmount ), - ROUNDING_ERROR + "ROUNDING_ERROR" ); } @@ -346,14 +342,14 @@ contract MixinExchangeCore is // An order can only be cancelled if its status is FILLABLE. require( orderInfo.orderStatus == uint8(OrderStatus.FILLABLE), - ORDER_UNFILLABLE + "ORDER_UNFILLABLE" ); // Validate sender is allowed to cancel this order if (order.senderAddress != address(0)) { require( order.senderAddress == msg.sender, - INVALID_SENDER + "INVALID_SENDER" ); } @@ -361,7 +357,7 @@ contract MixinExchangeCore is address makerAddress = getCurrentContextAddress(); require( order.makerAddress == makerAddress, - INVALID_MAKER + "INVALID_MAKER" ); } @@ -411,33 +407,27 @@ contract MixinExchangeCore is ) private { - uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; dispatchTransferFrom( order.makerAssetData, - makerAssetProxyId, order.makerAddress, takerAddress, fillResults.makerAssetFilledAmount ); dispatchTransferFrom( order.takerAssetData, - takerAssetProxyId, takerAddress, order.makerAddress, fillResults.takerAssetFilledAmount ); dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 82325a29f..1a43eec79 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -15,11 +15,9 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "./libs/LibConstants.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MMatchOrders.sol"; import "./mixins/MTransactions.sol"; @@ -27,15 +25,12 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is LibConstants, - LibBytes, LibMath, - LibExchangeErrors, MAssetProxyDispatcher, MExchangeCore, MMatchOrders, MTransactions { - /// @dev Match two complementary orders that have a profitable spread. /// Each order is filled at their respective price point. However, the calculations are /// carried out as though the orders are both being filled at the right order's price point. @@ -144,7 +139,7 @@ contract MixinMatchOrders is require( safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) >= safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount), - NEGATIVE_SPREAD_REQUIRED + "NEGATIVE_SPREAD_REQUIRED" ); } @@ -242,27 +237,22 @@ contract MixinMatchOrders is ) private { - uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; // Order makers and taker dispatchTransferFrom( leftOrder.makerAssetData, - leftMakerAssetProxyId, leftOrder.makerAddress, rightOrder.makerAddress, matchedFillResults.right.takerAssetFilledAmount ); dispatchTransferFrom( rightOrder.makerAssetData, - rightMakerAssetProxyId, rightOrder.makerAddress, leftOrder.makerAddress, matchedFillResults.left.takerAssetFilledAmount ); dispatchTransferFrom( leftOrder.makerAssetData, - leftMakerAssetProxyId, leftOrder.makerAddress, takerAddress, matchedFillResults.leftMakerAssetSpreadAmount @@ -271,14 +261,12 @@ contract MixinMatchOrders is // Maker fees dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -288,7 +276,6 @@ contract MixinMatchOrders is if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -299,14 +286,12 @@ contract MixinMatchOrders is } else { dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( zrxAssetData, - ZRX_PROXY_ID, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 881d6e7b3..29172057a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -19,22 +19,17 @@ pragma solidity ^0.4.24; import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; import "./interfaces/IWallet.sol"; import "./interfaces/IValidator.sol"; contract MixinSignatureValidator is - LibBytes, - LibExchangeErrors, MSignatureValidator, MTransactions { - // Personal message headers - string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32"; - string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20"; - + using LibBytes for bytes; + // Mapping of hash => signer => signed mapping (bytes32 => mapping (address => bool)) public preSigned; @@ -58,7 +53,7 @@ contract MixinSignatureValidator is signerAddress, signature ), - INVALID_SIGNATURE + "INVALID_SIGNATURE" ); preSigned[hash][signerAddress] = true; } @@ -98,14 +93,14 @@ contract MixinSignatureValidator is // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.) require( signature.length > 0, - LENGTH_GREATER_THAN_0_REQUIRED + "LENGTH_GREATER_THAN_0_REQUIRED" ); // Ensure signature is supported - uint8 signatureTypeRaw = uint8(popLastByte(signature)); + uint8 signatureTypeRaw = uint8(signature.popLastByte()); require( signatureTypeRaw < uint8(SignatureType.NSignatureTypes), - SIGNATURE_UNSUPPORTED + "SIGNATURE_UNSUPPORTED" ); // Pop last byte off of signature byte array. @@ -123,7 +118,7 @@ contract MixinSignatureValidator is // it an explicit option. This aids testing and analysis. It is // also the initialization value for the enum type. if (signatureType == SignatureType.Illegal) { - revert(SIGNATURE_ILLEGAL); + revert("SIGNATURE_ILLEGAL"); // Always invalid signature. // Like Illegal, this is always implicitly available and therefore @@ -132,7 +127,7 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.Invalid) { require( signature.length == 0, - LENGTH_0_REQUIRED + "LENGTH_0_REQUIRED" ); isValid = false; return isValid; @@ -141,11 +136,11 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.EIP712) { require( signature.length == 65, - LENGTH_65_REQUIRED + "LENGTH_65_REQUIRED" ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover(hash, v, r, s); isValid = signerAddress == recovered; return isValid; @@ -154,13 +149,16 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.EthSign) { require( signature.length == 65, - LENGTH_65_REQUIRED + "LENGTH_65_REQUIRED" ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( - keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)), + keccak256(abi.encodePacked( + "\x19Ethereum Signed Message:\n32", + hash + )), v, r, s @@ -179,7 +177,7 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.Caller) { require( signature.length == 0, - LENGTH_0_REQUIRED + "LENGTH_0_REQUIRED" ); isValid = signerAddress == msg.sender; return isValid; @@ -199,7 +197,9 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validatorAddress = popLast20Bytes(signature); + + address validatorAddress = signature.popLast20Bytes(); + // Ensure signer has approved validator. if (!allowedValidators[signerAddress][validatorAddress]) { return false; @@ -227,13 +227,16 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.Trezor) { require( signature.length == 65, - LENGTH_65_REQUIRED + "LENGTH_65_REQUIRED" ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( - keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)), + keccak256(abi.encodePacked( + "\x19Ethereum Signed Message:\n\x20", + hash + )), v, r, s @@ -247,6 +250,6 @@ contract MixinSignatureValidator is // that we currently support. In this case returning false // may lead the caller to incorrectly believe that the // signature was invalid.) - revert(SIGNATURE_UNSUPPORTED); + revert("SIGNATURE_UNSUPPORTED"); } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index d3d22d48f..e0f450d0a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -20,12 +20,10 @@ pragma solidity ^0.4.24; import "./libs/LibExchangeErrors.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; -import "./libs/LibExchangeErrors.sol"; import "./libs/LibEIP712.sol"; contract MixinTransactions is LibEIP712, - LibExchangeErrors, MSignatureValidator, MTransactions { @@ -51,17 +49,27 @@ contract MixinTransactions is /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @return EIP712 hash of the Transaction. - function hashZeroExTransaction(uint256 salt, address signerAddress, bytes data) + function hashZeroExTransaction( + uint256 salt, + address signerAddress, + bytes memory data + ) internal pure - returns (bytes32) + returns (bytes32 result) { - return keccak256(abi.encode( - EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, - salt, - signerAddress, - keccak256(data) - )); + bytes32 schemaHash = EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH; + bytes32 dataHash = keccak256(data); + assembly { + let memPtr := mload(64) + mstore(memPtr, schemaHash) + mstore(add(memPtr, 32), salt) + mstore(add(memPtr, 64), and(signerAddress, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(memPtr, 96), dataHash) + result := keccak256(memPtr, 128) + } + + return result; } /// @dev Executes an exchange method call in the context of signer. @@ -80,7 +88,7 @@ contract MixinTransactions is // Prevent reentrancy require( currentContextAddress == address(0), - REENTRANCY_ILLEGAL + "REENTRANCY_ILLEGAL" ); bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction( @@ -92,7 +100,7 @@ contract MixinTransactions is // Validate transaction has not been executed require( !transactions[transactionHash], - INVALID_TX_HASH + "INVALID_TX_HASH" ); // Transaction always valid if signer is sender of transaction @@ -104,7 +112,7 @@ contract MixinTransactions is signerAddress, signature ), - INVALID_TX_SIGNATURE + "INVALID_TX_SIGNATURE" ); // Set the current transaction signer @@ -115,7 +123,7 @@ contract MixinTransactions is transactions[transactionHash] = true; require( address(this).delegatecall(data), - FAILED_EXECUTION + "FAILED_EXECUTION" ); // Reset current transaction signer diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 724f95518..00668ca43 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -22,13 +22,11 @@ pragma experimental ABIEncoderV2; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; -import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; contract MixinWrapperFunctions is LibMath, LibFillResults, - LibExchangeErrors, MExchangeCore { /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. @@ -50,7 +48,7 @@ contract MixinWrapperFunctions is ); require( fillResults.takerAssetFilledAmount == takerAssetFillAmount, - COMPLETE_FILL_FAILED + "COMPLETE_FILL_FAILED" ); return fillResults; } @@ -366,14 +364,6 @@ contract MixinWrapperFunctions is signatures[i] ); - // HACK: the proxyId is "popped" from the byte array before a fill is settled - // by subtracting from the length of the array. Since the popped byte is - // still in memory, we can "unpop" it by incrementing the length of the byte array. - assembly { - let len := mload(takerAssetData) - mstore(takerAssetData, add(len, 1)) - } - // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -467,14 +457,6 @@ contract MixinWrapperFunctions is signatures[i] ); - // HACK: the proxyId is "popped" from the byte array before a fill is settled - // by subtracting from the length of the array. Since the popped byte is - // still in memory, we can "unpop" it by incrementing the length of the byte array. - assembly { - let len := mload(makerAssetData) - mstore(makerAssetData, add(len, 1)) - } - // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol index 2c331dc34..fa55dff00 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol @@ -26,7 +26,7 @@ contract IAssetProxyDispatcher { /// @param newAssetProxy Address of new asset proxy to register, or 0x0 to unset assetProxyId. /// @param oldAssetProxy Existing asset proxy to overwrite, or 0x0 if assetProxyId is currently unused. function registerAssetProxy( - uint8 assetProxyId, + bytes4 assetProxyId, address newAssetProxy, address oldAssetProxy ) @@ -35,7 +35,7 @@ contract IAssetProxyDispatcher { /// @dev Gets an asset proxy. /// @param assetProxyId Id of the asset proxy. /// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered. - function getAssetProxy(uint8 assetProxyId) + function getAssetProxy(bytes4 assetProxyId) external view returns (address); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol index 4a9452448..488ca956c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol @@ -25,9 +25,6 @@ contract LibConstants { // not constant to make testing easier. bytes public ZRX_ASSET_DATA; - // Proxy Id for ZRX token. - uint8 constant ZRX_PROXY_ID = 1; - // @TODO: Remove when we deploy. constructor (bytes memory zrxAssetData) public diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index a43f0f927..e37f41ada 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -18,7 +18,10 @@ pragma solidity ^0.4.24; +/// @dev This contract documents the revert reasons used in the Exchange contract. +/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons. contract LibExchangeErrors { + /// Order validation errors /// string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled. string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress. @@ -56,9 +59,11 @@ contract LibExchangeErrors { /// dispatchTransferFrom errors /// string constant ASSET_PROXY_DOES_NOT_EXIST = "ASSET_PROXY_DOES_NOT_EXIST"; // No assetProxy registered at given id. + string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Asset transfer unsuccesful. /// Length validation errors /// string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0. + string constant LENGTH_GREATER_THAN_3_REQUIRED = "LENGTH_GREATER_THAN_3_REQUIRED"; // Byte array must have a length greater than 3. string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0. string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol index 233547b9f..bfe2fd33f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol @@ -23,7 +23,6 @@ import "../../../utils/SafeMath/SafeMath.sol"; contract LibMath is SafeMath { - string constant ROUNDING_ERROR_ON_PARTIAL_AMOUNT = "A rounding error occurred when calculating partial transfer amounts."; /// @dev Calculates partial value given a numerator and denominator. /// @param numerator Numerator. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index 788f42c60..c2b506dcf 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -27,20 +27,18 @@ contract MAssetProxyDispatcher is // Logs registration of new asset proxy event AssetProxySet( - uint8 id, // Id of new registered AssetProxy. + bytes4 id, // Id of new registered AssetProxy. address newAssetProxy, // Address of new registered AssetProxy. address oldAssetProxy // Address of AssetProxy that was overwritten at given id (or null address). ); /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetData Byte array encoded for the respective asset proxy. - /// @param assetProxyId Id of assetProxy to dispach to. + /// @param assetData Byte array encoded for the asset. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, - uint8 assetProxyId, address from, address to, uint256 amount 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/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index d469a07f0..2ae69e0ef 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -24,12 +24,11 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( bytes memory assetData, - uint8 assetProxyId, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetData, assetProxyId, from, to, amount); + dispatchTransferFrom(assetData, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol new file mode 100644 index 000000000..2abcd17a0 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -0,0 +1,56 @@ +/* + + 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 "../../protocol/AssetProxyOwner/AssetProxyOwner.sol"; + +contract TestAssetProxyOwner is + AssetProxyOwner +{ + constructor( + address[] memory _owners, + address[] memory _assetProxyContracts, + uint256 _required, + uint256 _secondsTimeLocked + ) + public + AssetProxyOwner(_owners, _assetProxyContracts, _required, _secondsTimeLocked) + { + } + + function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id) + public + validRemoveAuthorizedAddressAtIndexTx(id) + returns (bool) + { + // Do nothing. We expect reverts through the modifier + return true; + } + + /// @dev Compares first 4 bytes of byte array to `removeAuthorizedAddressAtIndex` function selector. + /// @param data Transaction data. + /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`. + function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data) + public + pure + returns (bool) + { + return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 6f1898acd..f45faaf36 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -21,9 +21,9 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -contract TestLibBytes is - LibBytes -{ +contract TestLibBytes { + + using LibBytes for bytes; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -33,7 +33,7 @@ contract TestLibBytes is pure returns (bytes memory, bytes1 result) { - result = popLastByte(b); + result = b.popLastByte(); return (b, result); } @@ -45,7 +45,7 @@ contract TestLibBytes is pure returns (bytes memory, address result) { - result = popLast20Bytes(b); + result = b.popLast20Bytes(); return (b, result); } @@ -53,12 +53,23 @@ contract TestLibBytes is /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. /// @return True if arrays are the same. False otherwise. - function publicAreBytesEqual(bytes memory lhs, bytes memory rhs) + function publicEquals(bytes memory lhs, bytes memory rhs) public pure returns (bool equal) { - equal = areBytesEqual(lhs, rhs); + equal = lhs.equals(rhs); + return equal; + } + + function publicEqualsPop1(bytes memory lhs, bytes memory rhs) + public + pure + returns (bool equal) + { + lhs.popLastByte(); + rhs.popLastByte(); + equal = lhs.equals(rhs); return equal; } @@ -73,7 +84,7 @@ contract TestLibBytes is pure returns (bytes memory) { - deepCopyBytes(dest, source); + LibBytes.deepCopyBytes(dest, source); return dest; } @@ -89,7 +100,7 @@ contract TestLibBytes is pure returns (address result) { - result = readAddress(b, index); + result = b.readAddress(index); return result; } @@ -106,7 +117,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeAddress(b, index, input); + b.writeAddress(index, input); return b; } @@ -122,7 +133,7 @@ contract TestLibBytes is pure returns (bytes32 result) { - result = readBytes32(b, index); + result = b.readBytes32(index); return result; } @@ -139,7 +150,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes32(b, index, input); + b.writeBytes32(index, input); return b; } @@ -155,7 +166,7 @@ contract TestLibBytes is pure returns (uint256 result) { - result = readUint256(b, index); + result = b.readUint256(index); return result; } @@ -172,19 +183,23 @@ contract TestLibBytes is pure returns (bytes memory) { - writeUint256(b, index, input); + b.writeUint256(index, input); return b; } - /// @dev Reads the first 4 bytes from a byte array of arbitrary length. - /// @param b Byte array to read first 4 bytes from. - /// @return First 4 bytes of data. - function publicReadFirst4(bytes memory b) + /// @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 pure returns (bytes4 result) { - result = readFirst4(b); + result = b.readBytes4(index); return result; } @@ -192,7 +207,7 @@ contract TestLibBytes is /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function publicReadBytes( + function publicReadBytesWithLength( bytes memory b, uint256 index ) @@ -200,7 +215,7 @@ contract TestLibBytes is pure returns (bytes memory result) { - result = readBytes(b, index); + result = b.readBytesWithLength(index); return result; } @@ -209,7 +224,7 @@ contract TestLibBytes is /// @param index Index in byte array of <input>. /// @param input bytes to insert. /// @return b Updated input byte array - function publicWriteBytes( + function publicWriteBytesWithLength( bytes memory b, uint256 index, bytes memory input @@ -218,7 +233,37 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes(b, index, input); + b.writeBytesWithLength(index, input); return b; } + + /// @dev Copies a block of memory from one location to another. + /// @param mem Memory contents we want to apply memCopy to + /// @param dest Destination offset into <mem>. + /// @param source Source offset into <mem>. + /// @param length Length of bytes to copy from <source> to <dest> + /// @return mem Memory contents after calling memCopy. + function testMemcpy( + bytes mem, + uint256 dest, + uint256 source, + uint256 length + ) + public // not external, we need input in memory + pure + returns (bytes) + { + // Sanity check. Overflows are not checked. + require(source + length <= mem.length); + require(dest + length <= mem.length); + + // Get pointer to memory contents + uint256 offset = mem.contentAddress(); + + // Execute memCopy adjusted for memory array location + LibBytes.memCopy(offset + dest, offset + source, length); + + // Return modified memory contents + return mem; + } } diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol deleted file mode 100644 index b7e2e06b8..000000000 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.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; - -import "../../utils/LibMem/LibMem.sol"; - -contract TestLibMem is - LibMem -{ - - /// @dev Copies a block of memory from one location to another. - /// @param mem Memory contents we want to apply memCopy to - /// @param dest Destination offset into <mem>. - /// @param source Source offset into <mem>. - /// @param length Length of bytes to copy from <source> to <dest> - /// @return mem Memory contents after calling memCopy. - function testMemcpy( - bytes mem, - uint256 dest, - uint256 source, - uint256 length - ) - public // not external, we need input in memory - pure - returns (bytes) - { - // Sanity check. Overflows are not checked. - require(source + length <= mem.length); - require(dest + length <= mem.length); - - // Get pointer to memory contents - uint256 offset = getMemAddress(mem) + 32; - - // Execute memCopy adjusted for memory array location - memCopy(offset + dest, offset + source, length); - - // Return modified memory contents - return mem; - } -} diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol index aca74b409..17dee9e9c 100644 --- a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -22,9 +22,9 @@ import "../../protocol/Exchange/interfaces/IWallet.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract TestWallet is - IWallet, - LibBytes + IWallet { + using LibBytes for bytes; string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; @@ -56,8 +56,8 @@ contract TestWallet is ); uint8 v = uint8(eip712Signature[0]); - bytes32 r = readBytes32(eip712Signature, 1); - bytes32 s = readBytes32(eip712Signature, 33); + bytes32 r = eip712Signature.readBytes32(1); + bytes32 s = eip712Signature.readBytes32(33); address recoveredAddress = ecrecover(hash, v, r, s); isValid = WALLET_OWNER == recoveredAddress; return isValid; diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol index d35815474..8b52858b1 100644 --- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -23,13 +23,13 @@ import "../../protocol/Exchange/interfaces/IExchange.sol"; import "../../protocol/Exchange/libs/LibOrder.sol"; import "../../utils/Ownable/Ownable.sol"; -contract Whitelist is +contract Whitelist is Ownable { // Revert reasons - string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted."; - string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted."; - string constant INVALID_SENDER = "Sender must equal transaction origin."; + string constant MAKER_NOT_WHITELISTED = "MAKER_NOT_WHITELISTED"; // Maker address not whitelisted. + string constant TAKER_NOT_WHITELISTED = "TAKER_NOT_WHITELISTED"; // Taker address not whitelisted. + string constant INVALID_SENDER = "INVALID_SENDER"; // Sender must equal transaction origin. // Mapping of address => whitelist status. mapping (address => bool) public isWhitelisted; @@ -77,7 +77,7 @@ contract Whitelist is public { address takerAddress = msg.sender; - + // This contract must be the entry point for the transaction. require( takerAddress == tx.origin, diff --git a/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol index f0bcdafef..b6961a6ec 100644 --- a/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol +++ b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol @@ -23,8 +23,8 @@ import "./IERC20Token.sol"; contract ERC20Token is IERC20Token { - string constant INSUFFICIENT_BALANCE = "Insufficient balance to complete transfer."; - string constant INSUFFICIENT_ALLOWANCE = "Insufficient allowance to complete transfer."; + string constant INSUFFICIENT_BALANCE = "ERC20_INSUFFICIENT_BALANCE"; + string constant INSUFFICIENT_ALLOWANCE = "ERC20_INSUFFICIENT_ALLOWANCE"; string constant OVERFLOW = "Transfer would result in an overflow."; mapping (address => uint256) balances; @@ -97,4 +97,3 @@ contract ERC20Token is IERC20Token { return allowed[_owner][_spender]; } } - diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 10d7ce41a..78b1ddf7c 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,19 +18,200 @@ pragma solidity ^0.4.24; -import "../LibMem/LibMem.sol"; +library LibBytes { -contract LibBytes is - LibMem -{ + using LibBytes for bytes; - // Revert reasons - string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; + /// @dev Gets the memory address for a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of byte array. This + /// points to the header of the byte array which contains + /// the length. + function rawAddress(bytes memory input) + internal + pure + returns (uint256 memoryAddress) + { + assembly { + memoryAddress := input + } + return memoryAddress; + } + + /// @dev Gets the memory address for the contents of a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of the contents of the byte array. + function contentAddress(bytes memory input) + internal + pure + returns (uint256 memoryAddress) + { + assembly { + memoryAddress := add(input, 32) + } + return memoryAddress; + } + + /// @dev Copies `length` bytes from memory location `source` to `dest`. + /// @param dest memory address to copy bytes to. + /// @param source memory address to copy bytes from. + /// @param length number of bytes to copy. + function memCopy( + uint256 dest, + uint256 source, + uint256 length + ) + internal + pure + { + if (length < 32) { + // Handle a partial word by reading destination and masking + // off the bits we are interested in. + // This correctly handles overlap, zero lengths and source == dest + assembly { + let mask := sub(exp(256, sub(32, length)), 1) + let s := and(mload(source), not(mask)) + let d := and(mload(dest), mask) + mstore(dest, or(s, d)) + } + } else { + // Skip the O(length) loop when source == dest. + if (source == dest) { + return; + } + + // For large copies we copy whole words at a time. The final + // word is aligned to the end of the range (instead of after the + // previous) to handle partial words. So a copy will look like this: + // + // #### + // #### + // #### + // #### + // + // We handle overlap in the source and destination range by + // changing the copying direction. This prevents us from + // overwriting parts of source that we still need to copy. + // + // This correctly handles source == dest + // + if (source > dest) { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because it + // is easier to compare with in the loop, and these + // are also the addresses we need for copying the + // last bytes. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the last 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the last bytes in + // source already due to overlap. + let last := mload(sEnd) + + // Copy whole words front to back + // Note: the first check is always true, + // this could have been a do-while loop. + for {} lt(source, sEnd) {} { + mstore(dest, mload(source)) + source := add(source, 32) + dest := add(dest, 32) + } + + // Write the last 32 bytes + mstore(dEnd, last) + } + } else { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because those + // are the starting points when copying a word at the end. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the first 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the first bytes in + // source already due to overlap. + let first := mload(source) + + // Copy whole words back to front + // We use a signed comparisson here to allow dEnd to become + // negative (happens when source and dest < 32). Valid + // addresses in local memory will never be larger than + // 2**255, so they can be safely re-interpreted as signed. + // Note: the first check is always true, + // this could have been a do-while loop. + for {} slt(dest, dEnd) {} { + mstore(dEnd, mload(sEnd)) + sEnd := sub(sEnd, 32) + dEnd := sub(dEnd, 32) + } + + // Write the first 32 bytes + mstore(dest, first) + } + } + } + } + + /// @dev Returns a slices from a byte array. + /// @param b The byte array to take a slice from. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + function slice(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require( + from <= to, + "FROM_LESS_THAN_TO_REQUIRED" + ); + require( + to < b.length, + "TO_LESS_THAN_LENGTH_REQUIRED" + ); + + // Create a new bytes structure and copy contents + result = new bytes(to - from); + memCopy( + result.contentAddress(), + b.contentAddress() + from, + result.length); + return result; + } + + /// @dev Returns a slice from a byte array without preserving the input. + /// @param b The byte array to take a slice from. Will be destroyed in the process. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + /// @dev When `from == 0`, the original array will match the slice. In other cases its state will be corrupted. + function sliceDestructive(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require( + from <= to, + "FROM_LESS_THAN_TO_REQUIRED" + ); + require( + to < b.length, + "TO_LESS_THAN_LENGTH_REQUIRED" + ); + + // Create a new bytes structure around [from, to) in-place. + assembly { + result := add(b, from) + mstore(result, sub(to, from)) + } + return result; + } /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -42,7 +223,7 @@ contract LibBytes is { require( b.length > 0, - GREATER_THAN_ZERO_LENGTH_REQUIRED + "GREATER_THAN_ZERO_LENGTH_REQUIRED" ); // Store last byte. @@ -66,7 +247,7 @@ contract LibBytes is { require( b.length >= 20, - GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED" ); // Store last 20 bytes. @@ -80,6 +261,24 @@ contract LibBytes is return result; } + /// @dev Tests equality of two byte arrays. + /// @param lhs First byte array to compare. + /// @param rhs Second byte array to compare. + /// @return True if arrays are the same. False otherwise. + function equals( + bytes memory lhs, + bytes memory rhs + ) + internal + pure + returns (bool equal) + { + // Keccak gas cost is 30 + numWords * 6. This is a cheap way to compare. + // We early exit on unequal lengths, but keccak would also correctly + // handle this. + return lhs.length == rhs.length && keccak256(lhs) == keccak256(rhs); + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. @@ -94,7 +293,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED" ); // Add offset to index: @@ -126,7 +325,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED" ); // Add offset to index: @@ -145,6 +344,10 @@ contract LibBytes is // 2. Load 32-byte word from memory // 3. Apply 12-byte mask to obtain extra bytes occupying word of memory where we'll store the address let neighbors := and(mload(add(b, index)), 0xffffffffffffffffffffffff0000000000000000000000000000000000000000) + + // Make sure input address is clean. + // (Solidity does not guarantee this) + input := and(input, 0xffffffffffffffffffffffffffffffffffffffff) // Store the neighbors and address into memory mstore(add(b, index), xor(input, neighbors)) @@ -165,7 +368,7 @@ contract LibBytes is { require( b.length >= index + 32, - GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED" ); // Arrays are prefixed by a 256 bit length parameter @@ -192,7 +395,7 @@ contract LibBytes is { require( b.length >= index + 32, - GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED" ); // Arrays are prefixed by a 256 bit length parameter @@ -234,29 +437,37 @@ contract LibBytes is writeBytes32(b, index, bytes32(input)); } - /// @dev Reads the first 4 bytes from a byte array of arbitrary length. - /// @param b Byte array to read first 4 bytes from. - /// @return First 4 bytes of data. - function readFirst4(bytes memory b) + /// @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 pure returns (bytes4 result) { require( - b.length >= 4, - GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED + b.length >= index + 4, + "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED" ); assembly { result := mload(add(b, 32)) + // Solidity does not require us to clean the trailing bytes. + // We do it anyway + result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000) } return result; } /// @dev Reads nested bytes from a specific position. + /// @dev NOTE: the returned value overlaps with the input value. + /// Both should be treated as immutable. /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function readBytes( + function readBytesWithLength( bytes memory b, uint256 index ) @@ -272,17 +483,13 @@ contract LibBytes is // length of nested bytes require( b.length >= index + nestedBytesLength, - GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED - ); - - // Allocate memory and copy value to result - result = new bytes(nestedBytesLength); - memCopy( - getMemAddress(result) + 32, // +32 skips array length - getMemAddress(b) + index + 32, - nestedBytesLength + "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED" ); - + + // Return a pointer to the byte array as it exists inside `b` + assembly { + result := add(b, index) + } return result; } @@ -290,7 +497,7 @@ contract LibBytes is /// @param b Byte array to insert <input> into. /// @param index Index in byte array of <input>. /// @param input bytes to insert. - function writeBytes( + function writeBytesWithLength( bytes memory b, uint256 index, bytes memory input @@ -302,52 +509,17 @@ contract LibBytes is // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, - GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED" ); // Copy <input> into <b> memCopy( - getMemAddress(b) + 32 + index, // +32 to skip length of <b> - getMemAddress(input), // includes length of <input> - input.length + 32 // +32 bytes to store <input> length + b.contentAddress() + index, + input.rawAddress(), // includes length of <input> + input.length + 32 // +32 bytes to store <input> length ); } - /// @dev Tests equality of two byte arrays. - /// @param lhs First byte array to compare. - /// @param rhs Second byte array to compare. - /// @return True if arrays are the same. False otherwise. - function areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by <lhs> - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; - } - /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. /// @param dest Byte array that will be overwritten with source bytes. /// @param source Byte array to copy onto dest bytes. @@ -362,11 +534,11 @@ contract LibBytes is // Dest length must be >= source length, or some bytes would not be copied. require( dest.length >= sourceLen, - GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED" ); memCopy( - getMemAddress(dest) + 32, // +32 to skip length of <dest> - getMemAddress(source) + 32, // +32 to skip length of <source> + dest.contentAddress(), + source.contentAddress(), sourceLen ); } diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol deleted file mode 100644 index 97fb5fb0f..000000000 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ /dev/null @@ -1,142 +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; - -contract LibMem -{ - - /// @dev Gets the memory address for a byte array. - /// @param input Byte array to lookup. - /// @return memoryAddress Memory address of byte array. - function getMemAddress(bytes memory input) - internal - pure - returns (uint256 memoryAddress) - { - assembly { - memoryAddress := input - } - return memoryAddress; - } - - /// @dev Copies `length` bytes from memory location `source` to `dest`. - /// @param dest memory address to copy bytes to. - /// @param source memory address to copy bytes from. - /// @param length number of bytes to copy. - function memCopy( - uint256 dest, - uint256 source, - uint256 length - ) - internal - pure - { - if (length < 32) { - // Handle a partial word by reading destination and masking - // off the bits we are interested in. - // This correctly handles overlap, zero lengths and source == dest - assembly { - let mask := sub(exp(256, sub(32, length)), 1) - let s := and(mload(source), not(mask)) - let d := and(mload(dest), mask) - mstore(dest, or(s, d)) - } - } else { - // Skip the O(length) loop when source == dest. - if (source == dest) { - return; - } - - // For large copies we copy whole words at a time. The final - // word is aligned to the end of the range (instead of after the - // previous) to handle partial words. So a copy will look like this: - // - // #### - // #### - // #### - // #### - // - // We handle overlap in the source and destination range by - // changing the copying direction. This prevents us from - // overwriting parts of source that we still need to copy. - // - // This correctly handles source == dest - // - if (source > dest) { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because it - // is easier to compare with in the loop, and these - // are also the addresses we need for copying the - // last bytes. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the last 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the last bytes in - // source already due to overlap. - let last := mload(sEnd) - - // Copy whole words front to back - // Note: the first check is always true, - // this could have been a do-while loop. - for {} lt(source, sEnd) {} { - mstore(dest, mload(source)) - source := add(source, 32) - dest := add(dest, 32) - } - - // Write the last 32 bytes - mstore(dEnd, last) - } - } else { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because those - // are the starting points when copying a word at the end. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the first 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the first bytes in - // source already due to overlap. - let first := mload(source) - - // Copy whole words back to front - // We use a signed comparisson here to allow dEnd to become - // negative (happens when source and dest < 32). Valid - // addresses in local memory will never be larger than - // 2**255, so they can be safely re-interpreted as signed. - // Note: the first check is always true, - // this could have been a do-while loop. - for {} slt(dest, dEnd) {} { - mstore(dEnd, mload(sEnd)) - sEnd := sub(sEnd, 32) - dEnd := sub(dEnd, 32) - } - - // Write the first 32 bytes - mstore(dest, first) - } - } - } - } -} diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol index 296c6c856..6f5761cc7 100644 --- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol +++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol @@ -22,7 +22,7 @@ contract Ownable is IOwnable { modifier onlyOwner() { require( msg.sender == owner, - "Only contract owner is allowed to call this method." + "ONLY_CONTRACT_OWNER" ); _; } diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 4375d87c6..d58e7973c 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,10 +11,9 @@ 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'; -import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; import * as TestValidator from '../artifacts/TestValidator.json'; @@ -37,10 +36,9 @@ export const artifacts = { MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact, MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, 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, - TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, TestValidator: (TestValidator as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 615e648f3..e702a3200 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -1,3 +1,4 @@ +import { RevertReason } from '@0xproject/types'; import * as chai from 'chai'; import * as _ from 'lodash'; @@ -52,6 +53,21 @@ export function expectRevertOrAlwaysFailingTransactionAsync<T>(p: Promise<T>): P } /** + * Rejects if the given Promise does not reject with the given revert reason or "always + * failing transaction" error. + * @param p the Promise which is expected to reject + * @param reason a specific revert reason + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ +export function expectRevertReasonOrAlwaysFailingTransactionAsync<T>( + p: Promise<T>, + reason: RevertReason, +): PromiseLike<void> { + return _expectEitherErrorAsync(p, 'always failing transaction', reason); +} + +/** * Rejects if the given Promise does not reject with a "revert" or "Contract * call failed" error. * @param p the Promise which is expected to reject diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index f21b8c7a0..2b2bd2425 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,19 +19,6 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED: 'GREATER_THAN_ZERO_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', - ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', - ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', - EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', - EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', - EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', - EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', - EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index f0098bd5e..b0d4fa8ab 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -40,13 +40,15 @@ export class MultiSigWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } - public async executeRemoveAuthorizedAddressAsync( + public async executeRemoveAuthorizedAddressAtIndexAsync( txId: BigNumber, from: string, ): Promise<TransactionReceiptWithDecodedLogs> { // tslint:disable-next-line:no-unnecessary-type-assertion const txHash = await (this - ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); + ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from, + }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 7a1f92afd..5dfac64fc 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -92,7 +92,6 @@ export enum ContractName { Arbitrage = 'Arbitrage', TestAssetDataDecoders = 'TestAssetDataDecoders', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', - TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', |