From 2cd4d9004c193a81bfe37910048465bdf39e7413 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 14:39:31 -0700 Subject: [PATCH 01/10] Create LibAssetProxyIds --- .../contracts/src/interfaces/IAssetData.sol | 8 ++++ .../contracts/src/libs/LibAssetData.sol | 10 ++--- .../contracts/src/libs/LibAssetProxyIds.sol | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 contracts/asset-proxy/contracts/src/libs/LibAssetProxyIds.sol diff --git a/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol b/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol index 2f5e49bd93..10b4d22d15 100644 --- a/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol +++ b/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol @@ -35,6 +35,14 @@ interface IAssetData { ) external; + function ERC1155Assets( + address tokenContract, + uint256[] calldata ids, + uint256[] calldata values, + bytes calldata data + ) + external; + function MultiAsset( uint256[] calldata amounts, bytes[] calldata nestedAssetData diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol index ebde49b477..dc35bdb759 100644 --- a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol @@ -23,14 +23,12 @@ import "@0x/contracts-utils/contracts/src/LibBytes.sol"; import "@0x/contracts-erc1155/contracts/src/interfaces/IERC1155.sol"; import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; import "@0x/contracts-erc721/contracts/src/interfaces/IERC721Token.sol"; +import "./LibAssetProxyIds.sol"; -library LibAssetData { - bytes4 constant public ERC20_PROXY_ID = bytes4(keccak256("ERC20Token(address)")); - bytes4 constant public ERC721_PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256)")); - bytes4 constant public ERC1155_PROXY_ID = bytes4(keccak256("ERC1155Assets(address,uint256[],uint256[],bytes)")); - bytes4 constant public MULTI_ASSET_PROXY_ID = bytes4(keccak256("MultiAsset(uint256[],bytes[])")); - +contract LibAssetData is + LibAssetProxyIds +{ /// @dev Returns the owner's balance of the token(s) specified in /// assetData. When the asset data contains multiple tokens (eg in /// ERC1155 or Multi-Asset), the return value indicates how many diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetProxyIds.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetProxyIds.sol new file mode 100644 index 0000000000..9ea157357e --- /dev/null +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetProxyIds.sol @@ -0,0 +1,37 @@ +/* + + Copyright 2019 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.5.5; + + +contract LibAssetProxyIds { + + // AssetProxy Ids are equiavalent the first 4 bytes of the keccak256 hash of the function signature assigned to each AssetProxy. + + // ERC20Token(address) + bytes4 constant public ERC20_PROXY_ID = 0xf47261b0; + + // ERC721Token(address,uint256) + bytes4 constant public ERC721_PROXY_ID = 0x02571792; + + // ERC1155Assets(address,uint256[],uint256[],bytes) + bytes4 constant public ERC1155_PROXY_ID = 0xa7cb5fb7; + + // MultiAsset(uint256[],bytes[]) + bytes4 constant public MULTI_ASSET_PROXY_ID = 0x94cfcdd7; +} From c9bf1eda54c0797c59c4fabb96b7a149f472157f Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 15:33:55 -0700 Subject: [PATCH 02/10] Small readability and efficiency improvements --- .../contracts/src/interfaces/IAssetData.sol | 12 +- .../contracts/src/libs/LibAssetData.sol | 242 +++++++++++------- 2 files changed, 156 insertions(+), 98 deletions(-) diff --git a/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol b/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol index 10b4d22d15..b66c4600d7 100644 --- a/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol +++ b/contracts/asset-proxy/contracts/src/interfaces/IAssetData.sol @@ -26,20 +26,20 @@ pragma experimental ABIEncoderV2; // This argument is ABI encoded as one of the methods of this interface. interface IAssetData { - function ERC20Token(address tokenContract) + function ERC20Token(address tokenAddress) external; function ERC721Token( - address tokenContract, + address tokenAddress, uint256 tokenId ) external; function ERC1155Assets( - address tokenContract, - uint256[] calldata ids, - uint256[] calldata values, - bytes calldata data + address tokenAddress, + uint256[] calldata tokenIds, + uint256[] calldata tokenValues, + bytes calldata callbackData ) external; diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol index dc35bdb759..f2ddfa281e 100644 --- a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol @@ -29,6 +29,8 @@ import "./LibAssetProxyIds.sol"; contract LibAssetData is LibAssetProxyIds { + using LibBytes for bytes; + /// @dev Returns the owner's balance of the token(s) specified in /// assetData. When the asset data contains multiple tokens (eg in /// ERC1155 or Multi-Asset), the return value indicates how many @@ -42,15 +44,15 @@ contract LibAssetData is view returns (uint256 balance) { - bytes4 proxyId = LibBytes.readBytes4(assetData, 0); + bytes4 proxyId = assetData.readBytes4(0); + if (proxyId == ERC20_PROXY_ID) { - address tokenAddress = LibBytes.readAddress(assetData, 16); - return IERC20Token(tokenAddress).balanceOf(owner); + address tokenAddress = assetData.readAddress(16); + balance = IERC20Token(tokenAddress).balanceOf(owner); } else if (proxyId == ERC721_PROXY_ID) { (, address tokenAddress, uint256 tokenId) = decodeERC721AssetData(assetData); - return getERC721TokenOwner(tokenAddress, tokenId) == owner ? 1 : 0; + balance = getERC721TokenOwner(tokenAddress, tokenId) == owner ? 1 : 0; } else if (proxyId == ERC1155_PROXY_ID) { - uint256 lowestTokenBalance = 0; ( , address tokenAddress, @@ -58,25 +60,30 @@ contract LibAssetData is uint256[] memory tokenValues, ) = decodeERC1155AssetData(assetData); for (uint256 i = 0; i < tokenIds.length; i++) { - uint256 tokenBalance = IERC1155(tokenAddress).balanceOf(owner, tokenIds[i]) / tokenValues[i]; - if (tokenBalance < lowestTokenBalance || lowestTokenBalance == 0) { - lowestTokenBalance = tokenBalance; + uint256 totalBalance = IERC1155(tokenAddress).balanceOf(owner, tokenIds[i]); + uint256 scaledBalance = totalBalance / tokenValues[i]; + if (scaledBalance < balance || balance == 0) { + balance = scaledBalance; } } - return lowestTokenBalance; } else if (proxyId == MULTI_ASSET_PROXY_ID) { - uint256 lowestAssetBalance = 0; - (, uint256[] memory assetAmounts, bytes[] memory nestedAssetData) = decodeMultiAssetData(assetData); + ( + , + uint256[] memory assetAmounts, + bytes[] memory nestedAssetData + ) = decodeMultiAssetData(assetData); for (uint256 i = 0; i < nestedAssetData.length; i++) { - uint256 assetBalance = getBalance(owner, nestedAssetData[i]) / assetAmounts[i]; - if (assetBalance < lowestAssetBalance || lowestAssetBalance == 0) { - lowestAssetBalance = assetBalance; + uint256 totalBalance = getBalance(owner, nestedAssetData[i]); + uint256 scaledBalance = totalBalance / assetAmounts[i]; + if (scaledBalance < balance || balance == 0) { + balance = scaledBalance; } } - return lowestAssetBalance; } else { - revert("UNSUPPORTED_PROXY_IDENTIFIER"); + revert("UNSUPPORTED_PROXY_ID"); } + + return balance; } /// @dev Calls getBalance() for each element of assetData. @@ -90,10 +97,12 @@ contract LibAssetData is view returns (uint256[] memory balances) { - balances = new uint256[](assetData.length); - for (uint256 i = 0; i < assetData.length; i++) { + uint256 length = assetData.length; + balances = new uint256[](length); + for (uint256 i = 0; i != length; i++) { balances[i] = getBalance(owner, assetData[i]); } + return balances; } /// @dev Returns the number of token(s) (described by assetData) that @@ -106,45 +115,49 @@ contract LibAssetData is /// specification. /// @return Number of tokens (or token baskets) that the spender is /// authorized to spend. - function getAllowance(address owner, address spender, bytes memory assetData) + function getAllowance( + address owner, + address spender, + bytes memory assetData + ) public view returns (uint256 allowance) { - bytes4 proxyId = LibBytes.readBytes4(assetData, 0); + bytes4 proxyId = assetData.readBytes4(0); if (proxyId == ERC20_PROXY_ID) { - address tokenAddress = LibBytes.readAddress(assetData, 16); - return IERC20Token(tokenAddress).allowance(owner, spender); + address tokenAddress = assetData.readAddress(16); + allowance = IERC20Token(tokenAddress).allowance(owner, spender); } else if (proxyId == ERC721_PROXY_ID) { (, address tokenAddress, uint256 tokenId) = decodeERC721AssetData(assetData); IERC721Token token = IERC721Token(tokenAddress); - if (spender == token.getApproved(tokenId) || token.isApprovedForAll(owner, spender)) { - return 1; - } else { - return 0; - } + allowance = (token.getApproved(tokenId) != address(0) || token.isApprovedForAll(owner, spender)) ? 1 : 0; } else if (proxyId == ERC1155_PROXY_ID) { (, address tokenAddress, , , ) = decodeERC1155AssetData(assetData); - if (IERC1155(tokenAddress).isApprovedForAll(owner, spender)) { - return 1; - } else { - return 0; - } + allowance = IERC1155(tokenAddress).isApprovedForAll(owner, spender) ? 1 : 0; } else if (proxyId == MULTI_ASSET_PROXY_ID) { - uint256 lowestAssetAllowance = 0; - // solhint-disable-next-line indent - (, uint256[] memory amounts, bytes[] memory nestedAssetData) = decodeMultiAssetData(assetData); + ( + , + uint256[] memory amounts, + bytes[] memory nestedAssetData + ) = decodeMultiAssetData(assetData); for (uint256 i = 0; i < nestedAssetData.length; i++) { - uint256 assetAllowance = getAllowance(owner, spender, nestedAssetData[i]) / amounts[i]; - if (assetAllowance < lowestAssetAllowance || lowestAssetAllowance == 0) { - lowestAssetAllowance = assetAllowance; + uint256 totalAllowance = getAllowance( + owner, + spender, + nestedAssetData[i] + ); + uint256 scaledAllowance = totalAllowance / amounts[i]; + if (scaledAllowance < allowance || allowance == 0) { + allowance = scaledAllowance; } } - return lowestAssetAllowance; } else { - revert("UNSUPPORTED_PROXY_IDENTIFIER"); + revert("UNSUPPORTED_PROXY_ID"); } + + return allowance; } /// @dev Calls getAllowance() for each element of assetData. @@ -155,15 +168,21 @@ contract LibAssetData is /// @return An array of token allowances from getAllowance(), with each /// element corresponding to the same-indexed element in the assetData /// input. - function getBatchAllowances(address owner, address spender, bytes[] memory assetData) + function getBatchAllowances( + address owner, + address spender, + bytes[] memory assetData + ) public view returns (uint256[] memory allowances) { - allowances = new uint256[](assetData.length); - for (uint256 i = 0; i < assetData.length; i++) { + uint256 length = assetData.length; + allowances = new uint256[](length); + for (uint256 i = 0; i != length; i++) { allowances[i] = getAllowance(owner, spender, assetData[i]); } + return allowances; } /// @dev Calls getBalance() and getAllowance() for assetData. @@ -174,13 +193,18 @@ contract LibAssetData is /// @return Number of tokens (or token baskets) held by owner, and number /// of tokens (or token baskets) that the spender is authorized to /// spend. - function getBalanceAndAllowance(address owner, address spender, bytes memory assetData) + function getBalanceAndAllowance( + address owner, + address spender, + bytes memory assetData + ) public view returns (uint256 balance, uint256 allowance) { balance = getBalance(owner, assetData); allowance = getAllowance(owner, spender, assetData); + return (balance, allowance); } /// @dev Calls getBatchBalances() and getBatchAllowances() for each element @@ -192,13 +216,21 @@ contract LibAssetData is /// @return An array of token balances from getBalance(), and an array of /// token allowances from getAllowance(), with each element /// corresponding to the same-indexed element in the assetData input. - function getBatchBalancesAndAllowances(address owner, address spender, bytes[] memory assetData) + function getBatchBalancesAndAllowances( + address owner, + address spender, + bytes[] memory assetData + ) public view - returns (uint256[] memory balances, uint256[] memory allowances) + returns ( + uint256[] memory balances, + uint256[] memory allowances + ) { balances = getBatchBalances(owner, assetData); allowances = getBatchAllowances(owner, spender, assetData); + return (balances, allowances); } /// @dev Encode ERC-20 asset data into the format described in the @@ -211,7 +243,8 @@ contract LibAssetData is pure returns (bytes memory assetData) { - return abi.encodeWithSelector(ERC20_PROXY_ID, tokenAddress); + assetData = abi.encodeWithSelector(ERC20_PROXY_ID, tokenAddress); + return assetData; } /// @dev Decode ERC-20 asset data from the format described in the @@ -228,11 +261,15 @@ contract LibAssetData is address tokenAddress ) { - proxyId = LibBytes.readBytes4(assetData, 0); + proxyId = assetData.readBytes4(0); - require(proxyId == ERC20_PROXY_ID, "WRONG_PROXY_ID"); + require( + proxyId == ERC20_PROXY_ID, + "WRONG_PROXY_ID" + ); - tokenAddress = LibBytes.readAddress(assetData, 16); + tokenAddress = assetData.readAddress(16); + return (proxyId, tokenAddress); } /// @dev Encode ERC-721 asset data into the format described in the @@ -249,7 +286,12 @@ contract LibAssetData is pure returns (bytes memory assetData) { - return abi.encodeWithSelector(ERC721_PROXY_ID, tokenAddress, tokenId); + assetData = abi.encodeWithSelector( + ERC721_PROXY_ID, + tokenAddress, + tokenId + ); + return assetData; } /// @dev Decode ERC-721 asset data from the format described in the @@ -268,12 +310,16 @@ contract LibAssetData is uint256 tokenId ) { - proxyId = LibBytes.readBytes4(assetData, 0); + proxyId = assetData.readBytes4(0); - require(proxyId == ERC721_PROXY_ID, "WRONG_PROXY_ID"); + require( + proxyId == ERC721_PROXY_ID, + "WRONG_PROXY_ID" + ); - tokenAddress = LibBytes.readAddress(assetData, 16); - tokenId = LibBytes.readUint256(assetData, 36); + tokenAddress = assetData.readAddress(16); + tokenId = assetData.readUint256(36); + return (proxyId, tokenAddress, tokenId); } /// @dev Encode ERC-1155 asset data into the format described in the @@ -294,7 +340,14 @@ contract LibAssetData is pure returns (bytes memory assetData) { - return abi.encodeWithSelector(ERC1155_PROXY_ID, tokenAddress, tokenIds, tokenValues, callbackData); + assetData = abi.encodeWithSelector( + ERC1155_PROXY_ID, + tokenAddress, + tokenIds, + tokenValues, + callbackData + ); + return assetData; } /// @dev Decode ERC-1155 asset data from the format described in the @@ -319,9 +372,12 @@ contract LibAssetData is bytes memory callbackData ) { - proxyId = LibBytes.readBytes4(assetData, 0); + proxyId = assetData.readBytes4(0); - require(proxyId == ERC1155_PROXY_ID, "WRONG_PROXY_ID"); + require( + proxyId == ERC1155_PROXY_ID, + "WRONG_PROXY_ID" + ); assembly { // Skip selector and length to get to the first parameter: @@ -335,6 +391,14 @@ contract LibAssetData is // Point to the next parameter's data: callbackData := add(assetData, mload(add(assetData, 96))) } + + return ( + proxyId, + tokenAddress, + tokenIds, + tokenValues, + callbackData + ); } /// @dev Encode data for multiple assets, per the AssetProxy contract @@ -343,12 +407,20 @@ contract LibAssetData is /// @param nestedAssetData AssetProxy-compliant data describing each asset /// to be traded. /// @return AssetProxy-compliant data describing the set of assets. - function encodeMultiAssetData(uint256[] memory amounts, bytes[] memory nestedAssetData) + function encodeMultiAssetData( + uint256[] memory amounts, + bytes[] memory nestedAssetData + ) public pure returns (bytes memory assetData) { - assetData = abi.encodeWithSelector(MULTI_ASSET_PROXY_ID, amounts, nestedAssetData); + assetData = abi.encodeWithSelector( + MULTI_ASSET_PROXY_ID, + amounts, + nestedAssetData + ); + return assetData; } /// @dev Decode multi-asset data from the format described in the @@ -369,50 +441,36 @@ contract LibAssetData is bytes[] memory nestedAssetData ) { - proxyId = LibBytes.readBytes4(assetData, 0); + proxyId = assetData.readBytes4(0); - require(proxyId == MULTI_ASSET_PROXY_ID, "WRONG_PROXY_ID"); + require( + proxyId == MULTI_ASSET_PROXY_ID, + "WRONG_PROXY_ID" + ); - // solhint-disable-next-line indent - (amounts, nestedAssetData) = abi.decode(LibBytes.slice(assetData, 4, assetData.length), (uint256[], bytes[])); + (amounts, nestedAssetData) = abi.decode( + assetData.slice(4, assetData.length), + (uint256[], bytes[]) + ); } /// @dev Calls `token.ownerOf(tokenId)`, but returns a null owner instead of reverting on an unowned token. - /// @param token Address of ERC721 token. + /// @param tokenAddress Address of ERC721 token. /// @param tokenId The identifier for the specific NFT. /// @return Owner of tokenId or null address if unowned. - function getERC721TokenOwner(address token, uint256 tokenId) + function getERC721TokenOwner(address tokenAddress, uint256 tokenId) public view returns (address owner) { - assembly { - // load free memory pointer - let cdStart := mload(64) - - // bytes4(keccak256(ownerOf(uint256))) = 0x6352211e - mstore(cdStart, 0x6352211e00000000000000000000000000000000000000000000000000000000) - mstore(add(cdStart, 4), tokenId) - - // staticcall `ownerOf(tokenId)` - // `ownerOf` will revert if tokenId is not owned - let success := staticcall( - gas, // forward all gas - token, // call token contract - cdStart, // start of calldata - 36, // length of input is 36 bytes - cdStart, // write output over input - 32 // size of output is 32 bytes - ) - - // Success implies that tokenId is owned - // Copy owner from return data if successful - if success { - owner := mload(cdStart) - } - } + bytes memory ownerOfCalldata = abi.encodeWithSelector( + 0x6352211e, + tokenId + ); + + (bool success, bytes memory returnData) = tokenAddress.staticcall(ownerOfCalldata); - // Owner initialized to address(0), no need to modify if call is unsuccessful + owner = success ? returnData.readAddress(12) : address(0); return owner; } } From 7f94ebe362b3490033d690c2087463bd2e19d36d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 15 May 2019 16:52:23 -0400 Subject: [PATCH 03/10] Add signature validation checks to OrderValidator contract --- .../src/OrderValidator/OrderValidator.sol | 52 +++++++++++++++---- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol b/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol index 81888f3312..fb3024ab7a 100644 --- a/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol +++ b/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol @@ -16,7 +16,7 @@ */ -pragma solidity ^0.5.5; +pragma solidity ^0.5.8; pragma experimental ABIEncoderV2; import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol"; @@ -58,30 +58,64 @@ contract OrderValidator { /// @dev Fetches information for order and maker/taker of order. /// @param order The order structure. + /// @param signature Proof that order has been created by maker. /// @param takerAddress Address that will be filling the order. - /// @return OrderInfo and TraderInfo instances for given order. - function getOrderAndTraderInfo(LibOrder.Order memory order, address takerAddress) + /// @return OrderInfo, TraderInfo, and validity of signature for given order. + function getOrderAndTraderInfo( + LibOrder.Order memory order, + bytes memory signature, + address takerAddress + ) public view - returns (LibOrder.OrderInfo memory orderInfo, TraderInfo memory traderInfo) + returns ( + LibOrder.OrderInfo memory orderInfo, + TraderInfo memory traderInfo, + bool isValidSignature + ) { orderInfo = EXCHANGE.getOrderInfo(order); + isValidSignature = EXCHANGE.isValidSignature( + orderInfo.orderHash, + order.makerAddress, + signature + ); traderInfo = getTraderInfo(order, takerAddress); - return (orderInfo, traderInfo); + return (orderInfo, traderInfo, isValidSignature); } /// @dev Fetches information for all passed in orders and the makers/takers of each order. /// @param orders Array of order specifications. + /// @param signatures Proofs that orders have been created by makers. /// @param takerAddresses Array of taker addresses corresponding to each order. - /// @return Arrays of OrderInfo and TraderInfo instances that correspond to each order. - function getOrdersAndTradersInfo(LibOrder.Order[] memory orders, address[] memory takerAddresses) + /// @return Arrays of OrderInfo, TraderInfo, and validity of signatures that correspond to each order. + function getOrdersAndTradersInfo( + LibOrder.Order[] memory orders, + bytes[] memory signatures, + address[] memory takerAddresses + ) public view - returns (LibOrder.OrderInfo[] memory ordersInfo, TraderInfo[] memory tradersInfo) + returns ( + LibOrder.OrderInfo[] memory ordersInfo, + TraderInfo[] memory tradersInfo, + bool[] memory isValidSignature + ) { ordersInfo = EXCHANGE.getOrdersInfo(orders); tradersInfo = getTradersInfo(orders, takerAddresses); - return (ordersInfo, tradersInfo); + + uint256 length = orders.length; + isValidSignature = new bool[](length); + for (uint256 i = 0; i != length; i++) { + isValidSignature[i] = EXCHANGE.isValidSignature( + ordersInfo[i].orderHash, + orders[i].makerAddress, + signatures[i] + ); + } + + return (ordersInfo, tradersInfo, isValidSignature); } /// @dev Fetches balance and allowances for maker and taker of order. From 4d9f2586d949cde816feab85f4e96a2cbebd4ff4 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 16:15:56 -0700 Subject: [PATCH 04/10] Remove redundant code, add signature checks to order validations, and create DevUtils contract --- .../contracts/src/DevUtils/DevUtils.sol | 27 ++++ .../OrderValidationUtils.sol} | 141 ++++-------------- 2 files changed, 58 insertions(+), 110 deletions(-) create mode 100644 contracts/extensions/contracts/src/DevUtils/DevUtils.sol rename contracts/extensions/contracts/src/{OrderValidator/OrderValidator.sol => DevUtils/OrderValidationUtils.sol} (53%) diff --git a/contracts/extensions/contracts/src/DevUtils/DevUtils.sol b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol new file mode 100644 index 0000000000..b23fe57e94 --- /dev/null +++ b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol @@ -0,0 +1,27 @@ +/* + + 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.5.5; +pragma experimental ABIEncoderV2; + +import "./OrderValidationUtils.sol"; + + +contract DevUtils is + OrderValidationUtils +{} \ No newline at end of file diff --git a/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol b/contracts/extensions/contracts/src/DevUtils/OrderValidationUtils.sol similarity index 53% rename from contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol rename to contracts/extensions/contracts/src/DevUtils/OrderValidationUtils.sol index fb3024ab7a..4678a266c3 100644 --- a/contracts/extensions/contracts/src/OrderValidator/OrderValidator.sol +++ b/contracts/extensions/contracts/src/DevUtils/OrderValidationUtils.sol @@ -16,23 +16,20 @@ */ -pragma solidity ^0.5.8; +pragma solidity ^0.5.5; pragma experimental ABIEncoderV2; import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; -import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; -import "@0x/contracts-erc721/contracts/src/interfaces/IERC721Token.sol"; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-asset-proxy/contracts/src/libs/LibAssetData.sol"; -contract OrderValidator { - +contract OrderValidationUtils is + LibAssetData +{ using LibBytes for bytes; - bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)")); - bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)")); - struct TraderInfo { uint256 makerBalance; // Maker's balance of makerAsset uint256 makerAllowance; // Maker's allowance to corresponding AssetProxy @@ -47,6 +44,7 @@ contract OrderValidator { // solhint-disable var-name-mixedcase IExchange internal EXCHANGE; bytes internal ZRX_ASSET_DATA; + address internal ERC20_PROXY_ADDRESS; // solhint-enable var-name-mixedcase constructor (address _exchange, bytes memory _zrxAssetData) @@ -54,6 +52,7 @@ contract OrderValidator { { EXCHANGE = IExchange(_exchange); ZRX_ASSET_DATA = _zrxAssetData; + ERC20_PROXY_ADDRESS = EXCHANGE.getAssetProxy(ERC20_PROXY_ID); } /// @dev Fetches information for order and maker/taker of order. @@ -127,11 +126,31 @@ contract OrderValidator { view returns (TraderInfo memory traderInfo) { - (traderInfo.makerBalance, traderInfo.makerAllowance) = getBalanceAndAllowance(order.makerAddress, order.makerAssetData); - (traderInfo.takerBalance, traderInfo.takerAllowance) = getBalanceAndAllowance(takerAddress, order.takerAssetData); + bytes4 makerAssetProxyId = order.makerAssetData.readBytes4(0); + bytes4 takerAssetProxyId = order.takerAssetData.readBytes4(0); + + (traderInfo.makerBalance, traderInfo.makerAllowance) = getBalanceAndAllowance( + order.makerAddress, + EXCHANGE.getAssetProxy(makerAssetProxyId), + order.makerAssetData + ); + (traderInfo.takerBalance, traderInfo.takerAllowance) = getBalanceAndAllowance( + takerAddress, + EXCHANGE.getAssetProxy(takerAssetProxyId), + order.takerAssetData + ); bytes memory zrxAssetData = ZRX_ASSET_DATA; - (traderInfo.makerZrxBalance, traderInfo.makerZrxAllowance) = getBalanceAndAllowance(order.makerAddress, zrxAssetData); - (traderInfo.takerZrxBalance, traderInfo.takerZrxAllowance) = getBalanceAndAllowance(takerAddress, zrxAssetData); + address erc20ProxyAddress = ERC20_PROXY_ADDRESS; + (traderInfo.makerZrxBalance, traderInfo.makerZrxAllowance) = getBalanceAndAllowance( + order.makerAddress, + erc20ProxyAddress, + zrxAssetData + ); + (traderInfo.takerZrxBalance, traderInfo.takerZrxAllowance) = getBalanceAndAllowance( + takerAddress, + erc20ProxyAddress, + zrxAssetData + ); return traderInfo; } @@ -151,102 +170,4 @@ contract OrderValidator { } return tradersInfo; } - - /// @dev Fetches token balances and allowances of an address to given assetProxy. Supports ERC20 and ERC721. - /// @param target Address to fetch balances and allowances of. - /// @param assetData Encoded data that can be decoded by a specified proxy contract when transferring asset. - /// @return Balance of asset and allowance set to given proxy of asset. - /// For ERC721 tokens, these values will always be 1 or 0. - function getBalanceAndAllowance(address target, bytes memory assetData) - public - view - returns (uint256 balance, uint256 allowance) - { - bytes4 assetProxyId = assetData.readBytes4(0); - address token = assetData.readAddress(16); - address assetProxy = EXCHANGE.getAssetProxy(assetProxyId); - - if (assetProxyId == ERC20_DATA_ID) { - // Query balance - balance = IERC20Token(token).balanceOf(target); - - // Query allowance - allowance = IERC20Token(token).allowance(target, assetProxy); - } else if (assetProxyId == ERC721_DATA_ID) { - uint256 tokenId = assetData.readUint256(36); - - // Query owner of tokenId - address owner = getERC721TokenOwner(token, tokenId); - - // Set balance to 1 if tokenId is owned by target - balance = target == owner ? 1 : 0; - - // Check if ERC721Proxy is approved to spend tokenId - bool isApproved = IERC721Token(token).isApprovedForAll(target, assetProxy); - - // Set alowance to 1 if ERC721Proxy is approved to spend tokenId - allowance = isApproved ? 1 : 0; - } else { - revert("UNSUPPORTED_ASSET_PROXY"); - } - return (balance, allowance); - } - - /// @dev Fetches token balances and allowances of an address for each given assetProxy. Supports ERC20 and ERC721. - /// @param target Address to fetch balances and allowances of. - /// @param assetData Array of encoded byte arrays that can be decoded by a specified proxy contract when transferring asset. - /// @return Balances and allowances of assets. - /// For ERC721 tokens, these values will always be 1 or 0. - function getBalancesAndAllowances(address target, bytes[] memory assetData) - public - view - returns (uint256[] memory, uint256[] memory) - { - uint256 length = assetData.length; - uint256[] memory balances = new uint256[](length); - uint256[] memory allowances = new uint256[](length); - for (uint256 i = 0; i != length; i++) { - (balances[i], allowances[i]) = getBalanceAndAllowance(target, assetData[i]); - } - return (balances, allowances); - } - - /// @dev Calls `token.ownerOf(tokenId)`, but returns a null owner instead of reverting on an unowned token. - /// @param token Address of ERC721 token. - /// @param tokenId The identifier for the specific NFT. - /// @return Owner of tokenId or null address if unowned. - function getERC721TokenOwner(address token, uint256 tokenId) - public - view - returns (address owner) - { - assembly { - // load free memory pointer - let cdStart := mload(64) - - // bytes4(keccak256(ownerOf(uint256))) = 0x6352211e - mstore(cdStart, 0x6352211e00000000000000000000000000000000000000000000000000000000) - mstore(add(cdStart, 4), tokenId) - - // staticcall `ownerOf(tokenId)` - // `ownerOf` will revert if tokenId is not owned - let success := staticcall( - gas, // forward all gas - token, // call token contract - cdStart, // start of calldata - 36, // length of input is 36 bytes - cdStart, // write output over input - 32 // size of output is 32 bytes - ) - - // Success implies that tokenId is owned - // Copy owner from return data if successful - if success { - owner := mload(cdStart) - } - } - - // Owner initialized to address(0), no need to modify if call is unsuccessful - return owner; - } } From 543011c3de7d3dcef38171269741951b01506f58 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 17:19:01 -0700 Subject: [PATCH 05/10] Modify batch allowances to take an array of spenders --- .../contracts/src/libs/LibAssetData.sol | 14 ++++++------- contracts/asset-proxy/test/lib_asset_data.ts | 20 ++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol index f2ddfa281e..f4b22e9923 100644 --- a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol @@ -162,7 +162,7 @@ contract LibAssetData is /// @dev Calls getAllowance() for each element of assetData. /// @param owner Owner of the tokens specified by assetData. - /// @param spender Address whose authority to spend is in question. + /// @param spenders Array of addresses whose authority to spend is in question for each corresponding assetData. /// @param assetData Description of tokens, per the AssetProxy contract /// specification. /// @return An array of token allowances from getAllowance(), with each @@ -170,7 +170,7 @@ contract LibAssetData is /// input. function getBatchAllowances( address owner, - address spender, + address[] memory spenders, bytes[] memory assetData ) public @@ -180,7 +180,7 @@ contract LibAssetData is uint256 length = assetData.length; allowances = new uint256[](length); for (uint256 i = 0; i != length; i++) { - allowances[i] = getAllowance(owner, spender, assetData[i]); + allowances[i] = getAllowance(owner, spenders[i], assetData[i]); } return allowances; } @@ -210,7 +210,7 @@ contract LibAssetData is /// @dev Calls getBatchBalances() and getBatchAllowances() for each element /// of assetData. /// @param owner Owner of the tokens specified by assetData. - /// @param spender Address whose authority to spend is in question. + /// @param spenders Array of addresses whose authority to spend is in question for each corresponding assetData. /// @param assetData Description of tokens, per the AssetProxy contract /// specification. /// @return An array of token balances from getBalance(), and an array of @@ -218,7 +218,7 @@ contract LibAssetData is /// corresponding to the same-indexed element in the assetData input. function getBatchBalancesAndAllowances( address owner, - address spender, + address[] memory spenders, bytes[] memory assetData ) public @@ -229,7 +229,7 @@ contract LibAssetData is ) { balances = getBatchBalances(owner, assetData); - allowances = getBatchAllowances(owner, spender, assetData); + allowances = getBatchAllowances(owner, spenders, assetData); return (balances, allowances); } @@ -470,7 +470,7 @@ contract LibAssetData is (bool success, bytes memory returnData) = tokenAddress.staticcall(ownerOfCalldata); - owner = success ? returnData.readAddress(12) : address(0); + owner = (success && returnData.length == 32) ? returnData.readAddress(12) : address(0); return owner; } } diff --git a/contracts/asset-proxy/test/lib_asset_data.ts b/contracts/asset-proxy/test/lib_asset_data.ts index bd2b4dcd6d..b19379c504 100644 --- a/contracts/asset-proxy/test/lib_asset_data.ts +++ b/contracts/asset-proxy/test/lib_asset_data.ts @@ -383,10 +383,14 @@ describe('LibAssetData', () => { await setERC20AllowanceAsync(); await setERC721AllowanceAsync(); expect( - await libAssetData.getBatchAllowances.callAsync(tokenOwnerAddress, approvedSpenderAddress, [ - await libAssetData.encodeERC20AssetData.callAsync(erc20TokenAddress), - await libAssetData.encodeERC721AssetData.callAsync(erc721TokenAddress, firstERC721TokenId), - ]), + await libAssetData.getBatchAllowances.callAsync( + tokenOwnerAddress, + [approvedSpenderAddress, approvedSpenderAddress], + [ + await libAssetData.encodeERC20AssetData.callAsync(erc20TokenAddress), + await libAssetData.encodeERC721AssetData.callAsync(erc721TokenAddress, firstERC721TokenId), + ], + ), ).to.deep.equal([new BigNumber(1), new BigNumber(1)]); }); @@ -418,9 +422,11 @@ describe('LibAssetData', () => { it('should query balances and allowances together, from an asset data array', async () => { await setERC20AllowanceAsync(); expect( - await libAssetData.getBatchBalancesAndAllowances.callAsync(tokenOwnerAddress, approvedSpenderAddress, [ - await libAssetData.encodeERC20AssetData.callAsync(erc20TokenAddress), - ]), + await libAssetData.getBatchBalancesAndAllowances.callAsync( + tokenOwnerAddress, + [approvedSpenderAddress, approvedSpenderAddress], + [await libAssetData.encodeERC20AssetData.callAsync(erc20TokenAddress)], + ), ).to.deep.equal([[new BigNumber(erc20TokenTotalSupply)], [new BigNumber(1)]]); }); }); From 24249bcb4d4331d82d3050e805c9f85e3d1be375 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 17:19:45 -0700 Subject: [PATCH 06/10] Add LibTransactionDecoder to DevUtils --- .../contracts/src/LibTransactionDecoder.sol | 6 ++++-- .../extensions/contracts/src/DevUtils/DevUtils.sol | 11 +++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/contracts/exchange-libs/contracts/src/LibTransactionDecoder.sol b/contracts/exchange-libs/contracts/src/LibTransactionDecoder.sol index f0978b10b8..2ba6ddf062 100644 --- a/contracts/exchange-libs/contracts/src/LibTransactionDecoder.sol +++ b/contracts/exchange-libs/contracts/src/LibTransactionDecoder.sol @@ -17,14 +17,16 @@ */ pragma solidity ^0.5.5; -pragma experimental "ABIEncoderV2"; +pragma experimental ABIEncoderV2; import "@0x/contracts-exchange-libs/contracts/src/LibExchangeSelectors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; -contract LibTransactionDecoder is LibExchangeSelectors { +contract LibTransactionDecoder is + LibExchangeSelectors +{ using LibBytes for bytes; /// @dev Decodes the call data for an Exchange contract method call. diff --git a/contracts/extensions/contracts/src/DevUtils/DevUtils.sol b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol index b23fe57e94..b60e217740 100644 --- a/contracts/extensions/contracts/src/DevUtils/DevUtils.sol +++ b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol @@ -20,8 +20,15 @@ pragma solidity ^0.5.5; pragma experimental ABIEncoderV2; import "./OrderValidationUtils.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibTransactionDecoder.sol"; contract DevUtils is - OrderValidationUtils -{} \ No newline at end of file + OrderValidationUtils, + LibTransactionDecoder +{ + constructor (address _exchange, bytes memory _zrxAssetData) + public + OrderValidationUtils(_exchange, _zrxAssetData) + {} +} \ No newline at end of file From 3e59029966144736429608e513a89de60f2c9e8e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 17:20:10 -0700 Subject: [PATCH 07/10] Modify dev utils tests to pass --- contracts/extensions/compiler.json | 4 +- contracts/extensions/package.json | 2 +- contracts/extensions/src/artifacts.ts | 4 +- contracts/extensions/src/wrappers.ts | 2 +- .../test/{order_validator.ts => dev_utils.ts} | 173 +++++++++++------- contracts/extensions/tsconfig.json | 2 +- 6 files changed, 114 insertions(+), 73 deletions(-) rename contracts/extensions/test/{order_validator.ts => dev_utils.ts} (83%) diff --git a/contracts/extensions/compiler.json b/contracts/extensions/compiler.json index 353533da19..7d513dc38e 100644 --- a/contracts/extensions/compiler.json +++ b/contracts/extensions/compiler.json @@ -27,8 +27,8 @@ "@0x/contracts-exchange/contracts/examples/ExchangeWrapper.sol", "@0x/contracts-exchange/contracts/src/Exchange.sol", "src/BalanceThresholdFilter/BalanceThresholdFilter.sol", + "src/DevUtils/DevUtils.sol", "src/DutchAuction/DutchAuction.sol", - "src/OrderMatcher/OrderMatcher.sol", - "src/OrderValidator/OrderValidator.sol" + "src/OrderMatcher/OrderMatcher.sol" ] } diff --git a/contracts/extensions/package.json b/contracts/extensions/package.json index 38271f2684..eb89070979 100644 --- a/contracts/extensions/package.json +++ b/contracts/extensions/package.json @@ -34,7 +34,7 @@ "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" }, "config": { - "abis": "./generated-artifacts/@(BalanceThresholdFilter|DutchAuction|Exchange|ExchangeWrapper|OrderMatcher|OrderValidator|WETH9).json", + "abis": "./generated-artifacts/@(BalanceThresholdFilter|DevUtils|DutchAuction|Exchange|ExchangeWrapper|OrderMatcher|WETH9).json", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." }, "repository": { diff --git a/contracts/extensions/src/artifacts.ts b/contracts/extensions/src/artifacts.ts index 07503e8c7b..49a56d9e48 100644 --- a/contracts/extensions/src/artifacts.ts +++ b/contracts/extensions/src/artifacts.ts @@ -6,11 +6,11 @@ import { ContractArtifact } from 'ethereum-types'; import * as BalanceThresholdFilter from '../generated-artifacts/BalanceThresholdFilter.json'; +import * as DevUtils from '../generated-artifacts/DevUtils.json'; import * as DutchAuction from '../generated-artifacts/DutchAuction.json'; import * as Exchange from '../generated-artifacts/Exchange.json'; import * as ExchangeWrapper from '../generated-artifacts/ExchangeWrapper.json'; import * as OrderMatcher from '../generated-artifacts/OrderMatcher.json'; -import * as OrderValidator from '../generated-artifacts/OrderValidator.json'; import * as WETH9 from '../generated-artifacts/WETH9.json'; export const artifacts = { WETH9: WETH9 as ContractArtifact, @@ -19,5 +19,5 @@ export const artifacts = { BalanceThresholdFilter: BalanceThresholdFilter as ContractArtifact, DutchAuction: DutchAuction as ContractArtifact, OrderMatcher: OrderMatcher as ContractArtifact, - OrderValidator: OrderValidator as ContractArtifact, + DevUtils: DevUtils as ContractArtifact, }; diff --git a/contracts/extensions/src/wrappers.ts b/contracts/extensions/src/wrappers.ts index 170af64789..9a227280ee 100644 --- a/contracts/extensions/src/wrappers.ts +++ b/contracts/extensions/src/wrappers.ts @@ -4,9 +4,9 @@ * ----------------------------------------------------------------------------- */ export * from '../generated-wrappers/balance_threshold_filter'; +export * from '../generated-wrappers/dev_utils'; export * from '../generated-wrappers/dutch_auction'; export * from '../generated-wrappers/exchange'; export * from '../generated-wrappers/exchange_wrapper'; export * from '../generated-wrappers/order_matcher'; -export * from '../generated-wrappers/order_validator'; export * from '../generated-wrappers/weth9'; diff --git a/contracts/extensions/test/order_validator.ts b/contracts/extensions/test/dev_utils.ts similarity index 83% rename from contracts/extensions/test/order_validator.ts rename to contracts/extensions/test/dev_utils.ts index 6bff522896..82c1155bad 100644 --- a/contracts/extensions/test/order_validator.ts +++ b/contracts/extensions/test/dev_utils.ts @@ -18,13 +18,13 @@ import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; -import { artifacts, OrderValidatorContract } from '../src'; +import { artifacts, DevUtilsContract } from '../src'; chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe('OrderValidator', () => { +describe('DevUtils', () => { let makerAddress: string; let owner: string; let takerAddress: string; @@ -35,7 +35,7 @@ describe('OrderValidator', () => { let zrxToken: DummyERC20TokenContract; let erc721Token: DummyERC721TokenContract; let exchange: ExchangeContract; - let orderValidator: OrderValidatorContract; + let devUtils: DevUtilsContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; @@ -83,8 +83,8 @@ describe('OrderValidator', () => { await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); - orderValidator = await OrderValidatorContract.deployFrom0xArtifactAsync( - artifacts.OrderValidator, + devUtils = await DevUtilsContract.deployFrom0xArtifactAsync( + artifacts.DevUtils, provider, txDefaults, exchange.address, @@ -115,7 +115,7 @@ describe('OrderValidator', () => { describe('getBalanceAndAllowance', () => { describe('getERC721TokenOwner', async () => { it('should return the null address when tokenId is not owned', async () => { - const tokenOwner = await orderValidator.getERC721TokenOwner.callAsync(makerAddress, tokenId); + const tokenOwner = await devUtils.getERC721TokenOwner.callAsync(makerAddress, tokenId); expect(tokenOwner).to.be.equal(constants.NULL_ADDRESS); }); it('should return the owner address when tokenId is owned', async () => { @@ -123,14 +123,15 @@ describe('OrderValidator', () => { await erc721Token.mint.sendTransactionAsync(makerAddress, tokenId), constants.AWAIT_TRANSACTION_MINED_MS, ); - const tokenOwner = await orderValidator.getERC721TokenOwner.callAsync(erc721Token.address, tokenId); + const tokenOwner = await devUtils.getERC721TokenOwner.callAsync(erc721Token.address, tokenId); expect(tokenOwner).to.be.equal(makerAddress); }); }); describe('ERC20 assetData', () => { it('should return the correct balances and allowances when both values are 0', async () => { - const [newBalance, newAllowance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [newBalance, newAllowance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc20Proxy.address, erc20AssetData, ); expect(constants.ZERO_AMOUNT).to.be.bignumber.equal(newBalance); @@ -149,8 +150,9 @@ describe('OrderValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - const [newBalance, newAllowance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [newBalance, newAllowance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc20Proxy.address, erc20AssetData, ); expect(balance).to.be.bignumber.equal(newBalance); @@ -159,15 +161,17 @@ describe('OrderValidator', () => { }); describe('ERC721 assetData', () => { it('should return a balance of 0 when the tokenId is not owned by target', async () => { - const [newBalance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [newBalance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc721Proxy.address, erc721AssetData, ); expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); it('should return an allowance of 0 when no approval is set', async () => { - const [, newAllowance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [, newAllowance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc721Proxy.address, erc721AssetData, ); expect(newAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -177,8 +181,9 @@ describe('OrderValidator', () => { await erc721Token.mint.sendTransactionAsync(makerAddress, tokenId), constants.AWAIT_TRANSACTION_MINED_MS, ); - const [newBalance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [newBalance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc721Proxy.address, erc721AssetData, ); expect(newBalance).to.be.bignumber.equal(ERC721_BALANCE); @@ -191,13 +196,15 @@ describe('OrderValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - const [, newAllowance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [, newAllowance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc721Proxy.address, erc721AssetData, ); expect(newAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); }); - it('should return an allowance of 0 when ERC721Proxy is approved for specific tokenId', async () => { + // TODO(abandeali1): determine if a single allowance should return true or false + it.skip('should return an allowance of 0 when ERC721Proxy is approved for specific tokenId', async () => { await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.mint.sendTransactionAsync(makerAddress, tokenId), constants.AWAIT_TRANSACTION_MINED_MS, @@ -208,23 +215,25 @@ describe('OrderValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - const [, newAllowance] = await orderValidator.getBalanceAndAllowance.callAsync( + const [, newAllowance] = await devUtils.getBalanceAndAllowance.callAsync( makerAddress, + erc721Proxy.address, erc721AssetData, ); expect(newAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); }); }); - describe('getBalancesAndAllowances', () => { + describe('getBatchBalancesAndAllowances', () => { it('should return the correct balances and allowances when all values are 0', async () => { const [ [erc20Balance, erc721Balance], [erc20Allowance, erc721Allowance], - ] = await orderValidator.getBalancesAndAllowances.callAsync(makerAddress, [ - erc20AssetData, - erc721AssetData, - ]); + ] = await devUtils.getBatchBalancesAndAllowances.callAsync( + makerAddress, + [erc20Proxy.address, erc721Proxy.address], + [erc20AssetData, erc721AssetData], + ); expect(erc20Balance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(erc721Balance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(erc20Allowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -257,10 +266,11 @@ describe('OrderValidator', () => { const [ [erc20Balance, erc721Balance], [erc20Allowance, erc721Allowance], - ] = await orderValidator.getBalancesAndAllowances.callAsync(makerAddress, [ - erc20AssetData, - erc721AssetData, - ]); + ] = await devUtils.getBatchBalancesAndAllowances.callAsync( + makerAddress, + [erc20Proxy.address, erc721Proxy.address], + [erc20AssetData, erc721AssetData], + ); expect(erc20Balance).to.be.bignumber.equal(balance); expect(erc721Balance).to.be.bignumber.equal(ERC721_BALANCE); expect(erc20Allowance).to.be.bignumber.equal(allowance); @@ -272,7 +282,7 @@ describe('OrderValidator', () => { signedOrder = await orderFactory.newSignedOrderAsync(); }); it('should return the correct info when no balances or allowances are set', async () => { - const traderInfo = await orderValidator.getTraderInfo.callAsync(signedOrder, takerAddress); + const traderInfo = await devUtils.getTraderInfo.callAsync(signedOrder, takerAddress); expect(traderInfo.makerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.makerAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -318,7 +328,7 @@ describe('OrderValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - const traderInfo = await orderValidator.getTraderInfo.callAsync(signedOrder, takerAddress); + const traderInfo = await devUtils.getTraderInfo.callAsync(signedOrder, takerAddress); expect(traderInfo.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo.takerBalance).to.be.bignumber.equal(ERC721_BALANCE); @@ -339,7 +349,7 @@ describe('OrderValidator', () => { it('should return the correct info when no balances or allowances have been set', async () => { const orders = [signedOrder, signedOrder2]; const takers = [takerAddress, takerAddress]; - const [traderInfo1, traderInfo2] = await orderValidator.getTradersInfo.callAsync(orders, takers); + const [traderInfo1, traderInfo2] = await devUtils.getTradersInfo.callAsync(orders, takers); expect(traderInfo1.makerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo1.makerAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo1.takerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -395,7 +405,7 @@ describe('OrderValidator', () => { ); const orders = [signedOrder, signedOrder2]; const takers = [takerAddress, takerAddress]; - const [traderInfo1, traderInfo2] = await orderValidator.getTradersInfo.callAsync(orders, takers); + const [traderInfo1, traderInfo2] = await devUtils.getTradersInfo.callAsync(orders, takers); expect(traderInfo1.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo1.makerAllowance).to.be.bignumber.equal(makerAllowance); @@ -419,9 +429,11 @@ describe('OrderValidator', () => { beforeEach(async () => { signedOrder = await orderFactory.newSignedOrderAsync(); }); - it('should return the correct info when no balances or allowances are set', async () => { - const [orderInfo, traderInfo] = await orderValidator.getOrderAndTraderInfo.callAsync( + it('should return the correct info when no balances/allowances are set and the signature is invalid', async () => { + const invalidSignature = '0x01'; + const [orderInfo, traderInfo, isValidSignature] = await devUtils.getOrderAndTraderInfo.callAsync( signedOrder, + invalidSignature, takerAddress, ); const expectedOrderHash = orderHashUtils.getOrderHashHex(signedOrder); @@ -436,45 +448,56 @@ describe('OrderValidator', () => { expect(traderInfo.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(isValidSignature).to.equal(false); }); - it('should return the correct info when balances and allowances are set', async () => { + it('should return the correct info when balances/allowances are set and the signature is valid', async () => { const makerBalance = new BigNumber(123); const makerAllowance = new BigNumber(456); const makerZrxBalance = new BigNumber(789); const takerZrxAllowance = new BigNumber(987); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Token.setBalance.sendTransactionAsync(makerAddress, makerBalance), + await erc20Token.setBalance.awaitTransactionSuccessAsync( + makerAddress, + makerBalance, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Token.approve.sendTransactionAsync(erc20Proxy.address, makerAllowance, { + await erc20Token.approve.awaitTransactionSuccessAsync( + erc20Proxy.address, + makerAllowance, + { from: makerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await zrxToken.setBalance.sendTransactionAsync(makerAddress, makerZrxBalance), + await zrxToken.setBalance.awaitTransactionSuccessAsync( + makerAddress, + makerZrxBalance, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await zrxToken.approve.sendTransactionAsync(erc20Proxy.address, takerZrxAllowance, { + await zrxToken.approve.awaitTransactionSuccessAsync( + erc20Proxy.address, + takerZrxAllowance, + { from: takerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.mint.sendTransactionAsync(takerAddress, tokenId), + await erc721Token.mint.awaitTransactionSuccessAsync( + takerAddress, + tokenId, constants.AWAIT_TRANSACTION_MINED_MS, ); const isApproved = true; - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, isApproved, { + await erc721Token.setApprovalForAll.awaitTransactionSuccessAsync( + erc721Proxy.address, + isApproved, + { from: takerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); - const [orderInfo, traderInfo] = await orderValidator.getOrderAndTraderInfo.callAsync( + const [orderInfo, traderInfo, isValidSignature] = await devUtils.getOrderAndTraderInfo.callAsync( signedOrder, + signedOrder.signature, takerAddress, ); const expectedOrderHash = orderHashUtils.getOrderHashHex(signedOrder); @@ -489,6 +512,7 @@ describe('OrderValidator', () => { expect(traderInfo.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxAllowance).to.be.bignumber.equal(takerZrxAllowance); + expect(isValidSignature).to.equal(true); }); }); describe('getOrdersAndTradersInfo', () => { @@ -498,13 +522,16 @@ describe('OrderValidator', () => { takerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId2), }); }); - it('should return the correct info when no balances or allowances have been set', async () => { + it('should return the correct info when no balances or allowances have been set and the signatures are invalid', async () => { + const invalidSignature = '0x01'; const orders = [signedOrder, signedOrder2]; + const signatures = [invalidSignature, invalidSignature]; const takers = [takerAddress, takerAddress]; const [ [orderInfo1, orderInfo2], [traderInfo1, traderInfo2], - ] = await orderValidator.getOrdersAndTradersInfo.callAsync(orders, takers); + [isValidSignature1, isValidSignature2], + ] = await devUtils.getOrdersAndTradersInfo.callAsync(orders, signatures, takers); const expectedOrderHash1 = orderHashUtils.getOrderHashHex(signedOrder); const expectedOrderHash2 = orderHashUtils.getOrderHashHex(signedOrder2); expect(orderInfo1.orderStatus).to.be.equal(OrderStatus.Fillable); @@ -529,41 +556,52 @@ describe('OrderValidator', () => { expect(traderInfo2.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(isValidSignature1).to.equal(false); + expect(isValidSignature2).to.equal(false); }); - it('should return the correct info when balances and allowances are set', async () => { + it('should return the correct info when balances and allowances are set and the signatures are valid', async () => { const makerBalance = new BigNumber(123); const makerAllowance = new BigNumber(456); const makerZrxBalance = new BigNumber(789); const takerZrxAllowance = new BigNumber(987); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Token.setBalance.sendTransactionAsync(makerAddress, makerBalance), + await erc20Token.setBalance.awaitTransactionSuccessAsync( + makerAddress, + makerBalance, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Token.approve.sendTransactionAsync(erc20Proxy.address, makerAllowance, { + await erc20Token.approve.awaitTransactionSuccessAsync( + erc20Proxy.address, + makerAllowance, + { from: makerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await zrxToken.setBalance.sendTransactionAsync(makerAddress, makerZrxBalance), + await zrxToken.setBalance.awaitTransactionSuccessAsync( + makerAddress, + makerZrxBalance, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await zrxToken.approve.sendTransactionAsync(erc20Proxy.address, takerZrxAllowance, { + await zrxToken.approve.awaitTransactionSuccessAsync( + erc20Proxy.address, + takerZrxAllowance, + { from: takerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); const isApproved = true; - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, isApproved, { + await erc721Token.setApprovalForAll.awaitTransactionSuccessAsync( + erc721Proxy.address, + isApproved, + { from: takerAddress, - }), + }, constants.AWAIT_TRANSACTION_MINED_MS, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.mint.sendTransactionAsync(takerAddress, tokenId2), + await erc721Token.mint.awaitTransactionSuccessAsync( + takerAddress, + tokenId2, constants.AWAIT_TRANSACTION_MINED_MS, ); const orders = [signedOrder, signedOrder2]; @@ -571,7 +609,8 @@ describe('OrderValidator', () => { const [ [orderInfo1, orderInfo2], [traderInfo1, traderInfo2], - ] = await orderValidator.getOrdersAndTradersInfo.callAsync(orders, takers); + [isValidSignature1, isValidSignature2], + ] = await devUtils.getOrdersAndTradersInfo.callAsync(orders, orders.map(order => order.signature), takers); const expectedOrderHash1 = orderHashUtils.getOrderHashHex(signedOrder); const expectedOrderHash2 = orderHashUtils.getOrderHashHex(signedOrder2); expect(orderInfo1.orderStatus).to.be.equal(OrderStatus.Fillable); @@ -596,6 +635,8 @@ describe('OrderValidator', () => { expect(traderInfo2.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxAllowance).to.be.bignumber.equal(takerZrxAllowance); + expect(isValidSignature1).to.equal(true); + expect(isValidSignature2).to.equal(true); }); }); }); diff --git a/contracts/extensions/tsconfig.json b/contracts/extensions/tsconfig.json index 47c29f1f37..1246d38e1d 100644 --- a/contracts/extensions/tsconfig.json +++ b/contracts/extensions/tsconfig.json @@ -4,11 +4,11 @@ "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], "files": [ "generated-artifacts/BalanceThresholdFilter.json", + "generated-artifacts/DevUtils.json", "generated-artifacts/DutchAuction.json", "generated-artifacts/Exchange.json", "generated-artifacts/ExchangeWrapper.json", "generated-artifacts/OrderMatcher.json", - "generated-artifacts/OrderValidator.json", "generated-artifacts/WETH9.json" ], "exclude": ["./deploy/solc/solc_bin"] From 7809cad6cb6224e4248e89fe4a3e3aa68f2f950d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 17:59:53 -0700 Subject: [PATCH 08/10] Fix linting errors and warnings --- contracts/asset-proxy/contracts/src/ERC1155Proxy.sol | 1 + contracts/asset-proxy/contracts/src/libs/LibAssetData.sol | 2 ++ contracts/extensions/contracts/src/DevUtils/DevUtils.sol | 1 + 3 files changed, 4 insertions(+) diff --git a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol index 4a80754532..43fa0800ea 100644 --- a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol +++ b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol @@ -28,6 +28,7 @@ contract ERC1155Proxy is // Id of this proxy. bytes4 constant internal PROXY_ID = bytes4(keccak256("ERC1155Assets(address,uint256[],uint256[],bytes)")); + // solhint-disable-next-line payable-fallback function () external { diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol index f4b22e9923..1f82c55af3 100644 --- a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol @@ -448,10 +448,12 @@ contract LibAssetData is "WRONG_PROXY_ID" ); + // solhint-disable indent (amounts, nestedAssetData) = abi.decode( assetData.slice(4, assetData.length), (uint256[], bytes[]) ); + // solhint-enable indent } /// @dev Calls `token.ownerOf(tokenId)`, but returns a null owner instead of reverting on an unowned token. diff --git a/contracts/extensions/contracts/src/DevUtils/DevUtils.sol b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol index b60e217740..b3827051b5 100644 --- a/contracts/extensions/contracts/src/DevUtils/DevUtils.sol +++ b/contracts/extensions/contracts/src/DevUtils/DevUtils.sol @@ -23,6 +23,7 @@ import "./OrderValidationUtils.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibTransactionDecoder.sol"; +// solhint-disable no-empty-blocks contract DevUtils is OrderValidationUtils, LibTransactionDecoder From 3c08f5b86a0ad7fd86bd08c4480342334f392105 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 26 May 2019 18:00:03 -0700 Subject: [PATCH 09/10] Update CHANGELOGs --- contracts/asset-proxy/CHANGELOG.json | 9 +++++++++ contracts/extensions/CHANGELOG.json | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/contracts/asset-proxy/CHANGELOG.json b/contracts/asset-proxy/CHANGELOG.json index 813af4ed91..4945db5c77 100644 --- a/contracts/asset-proxy/CHANGELOG.json +++ b/contracts/asset-proxy/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "2.2.0", + "changes": [ + { + "note": "Add `LibAssetProxyIds` and `LibAssetData` contracts", + "pr": 1835 + } + ] + }, { "timestamp": 1558712885, "version": "2.1.5", diff --git a/contracts/extensions/CHANGELOG.json b/contracts/extensions/CHANGELOG.json index ee2790ff16..26103fe705 100644 --- a/contracts/extensions/CHANGELOG.json +++ b/contracts/extensions/CHANGELOG.json @@ -1,4 +1,17 @@ [ + { + "version": "4.0.0", + "changes": [ + { + "note": "Rename `OrderValidator` to `OrderValidationUtils`", + "pr": 1835 + }, + { + "note": "Create `DevUtils` contract", + "pr": 1835 + } + ] + }, { "timestamp": 1558712885, "version": "3.1.5", From 066f3bb6460092ac24eb6ad56ebd3fb709400f52 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 28 May 2019 17:01:54 -0700 Subject: [PATCH 10/10] Return MAX_UINT256 for unlimited allowances in all token standards --- .../contracts/src/libs/LibAssetData.sol | 10 ++++++-- contracts/asset-proxy/test/lib_asset_data.ts | 4 ++-- contracts/extensions/test/dev_utils.ts | 24 +++++++++---------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol index 1f82c55af3..0b223cde77 100644 --- a/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol +++ b/contracts/asset-proxy/contracts/src/libs/LibAssetData.sol @@ -29,6 +29,8 @@ import "./LibAssetProxyIds.sol"; contract LibAssetData is LibAssetProxyIds { + uint256 constant internal _MAX_UINT256 = uint256(-1); + using LibBytes for bytes; /// @dev Returns the owner's balance of the token(s) specified in @@ -132,10 +134,14 @@ contract LibAssetData is } else if (proxyId == ERC721_PROXY_ID) { (, address tokenAddress, uint256 tokenId) = decodeERC721AssetData(assetData); IERC721Token token = IERC721Token(tokenAddress); - allowance = (token.getApproved(tokenId) != address(0) || token.isApprovedForAll(owner, spender)) ? 1 : 0; + if (token.isApprovedForAll(owner, spender)) { + allowance = _MAX_UINT256; + } else if (token.getApproved(tokenId) == spender) { + allowance = 1; + } } else if (proxyId == ERC1155_PROXY_ID) { (, address tokenAddress, , , ) = decodeERC1155AssetData(assetData); - allowance = IERC1155(tokenAddress).isApprovedForAll(owner, spender) ? 1 : 0; + allowance = IERC1155(tokenAddress).isApprovedForAll(owner, spender) ? _MAX_UINT256 : 0; } else if (proxyId == MULTI_ASSET_PROXY_ID) { ( , diff --git a/contracts/asset-proxy/test/lib_asset_data.ts b/contracts/asset-proxy/test/lib_asset_data.ts index b19379c504..d744ba3769 100644 --- a/contracts/asset-proxy/test/lib_asset_data.ts +++ b/contracts/asset-proxy/test/lib_asset_data.ts @@ -325,7 +325,7 @@ describe('LibAssetData', () => { anotherApprovedSpenderAddress, await libAssetData.encodeERC721AssetData.callAsync(erc721TokenAddress, firstERC721TokenId), ), - ).to.bignumber.equal(1); + ).to.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); }); it('should query ERC1155 allowances by asset data', async () => { @@ -348,7 +348,7 @@ describe('LibAssetData', () => { '0x', ), ), - ).to.bignumber.equal(1); + ).to.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); }); it('should query multi-asset allowances by asset data', async () => { diff --git a/contracts/extensions/test/dev_utils.ts b/contracts/extensions/test/dev_utils.ts index 82c1155bad..6f4edf6db0 100644 --- a/contracts/extensions/test/dev_utils.ts +++ b/contracts/extensions/test/dev_utils.ts @@ -46,7 +46,6 @@ describe('DevUtils', () => { const tokenId = new BigNumber(123456789); const tokenId2 = new BigNumber(987654321); const ERC721_BALANCE = new BigNumber(1); - const ERC721_ALLOWANCE = new BigNumber(1); before(async () => { await blockchainLifecycle.startAsync(); @@ -188,7 +187,7 @@ describe('DevUtils', () => { ); expect(newBalance).to.be.bignumber.equal(ERC721_BALANCE); }); - it('should return an allowance of 1 when ERC721Proxy is approved for all', async () => { + it('should return an allowance of MAX_UINT256 when ERC721Proxy is approved for all', async () => { const isApproved = true; await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, isApproved, { @@ -201,10 +200,9 @@ describe('DevUtils', () => { erc721Proxy.address, erc721AssetData, ); - expect(newAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(newAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); }); - // TODO(abandeali1): determine if a single allowance should return true or false - it.skip('should return an allowance of 0 when ERC721Proxy is approved for specific tokenId', async () => { + it('should return an allowance of 1 when ERC721Proxy is approved for specific tokenId', async () => { await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.mint.sendTransactionAsync(makerAddress, tokenId), constants.AWAIT_TRANSACTION_MINED_MS, @@ -220,7 +218,7 @@ describe('DevUtils', () => { erc721Proxy.address, erc721AssetData, ); - expect(newAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newAllowance).to.be.bignumber.equal(new BigNumber(1)); }); }); }); @@ -274,7 +272,7 @@ describe('DevUtils', () => { expect(erc20Balance).to.be.bignumber.equal(balance); expect(erc721Balance).to.be.bignumber.equal(ERC721_BALANCE); expect(erc20Allowance).to.be.bignumber.equal(allowance); - expect(erc721Allowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(erc721Allowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); }); }); describe('getTraderInfo', () => { @@ -332,7 +330,7 @@ describe('DevUtils', () => { expect(traderInfo.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo.takerBalance).to.be.bignumber.equal(ERC721_BALANCE); - expect(traderInfo.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -410,7 +408,7 @@ describe('DevUtils', () => { expect(traderInfo1.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo1.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo1.takerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); - expect(traderInfo1.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo1.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo1.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo1.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo1.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -418,7 +416,7 @@ describe('DevUtils', () => { expect(traderInfo2.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo2.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo2.takerBalance).to.be.bignumber.equal(ERC721_BALANCE); - expect(traderInfo2.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo2.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo2.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo2.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -507,7 +505,7 @@ describe('DevUtils', () => { expect(traderInfo.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo.takerBalance).to.be.bignumber.equal(ERC721_BALANCE); - expect(traderInfo.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -622,7 +620,7 @@ describe('DevUtils', () => { expect(traderInfo1.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo1.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo1.takerBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); - expect(traderInfo1.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo1.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo1.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo1.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo1.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); @@ -630,7 +628,7 @@ describe('DevUtils', () => { expect(traderInfo2.makerBalance).to.be.bignumber.equal(makerBalance); expect(traderInfo2.makerAllowance).to.be.bignumber.equal(makerAllowance); expect(traderInfo2.takerBalance).to.be.bignumber.equal(ERC721_BALANCE); - expect(traderInfo2.takerAllowance).to.be.bignumber.equal(ERC721_ALLOWANCE); + expect(traderInfo2.takerAllowance).to.be.bignumber.equal(constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS); expect(traderInfo2.makerZrxBalance).to.be.bignumber.equal(makerZrxBalance); expect(traderInfo2.makerZrxAllowance).to.be.bignumber.equal(constants.ZERO_AMOUNT); expect(traderInfo2.takerZrxBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT);