diff options
Diffstat (limited to 'packages/contracts/src')
29 files changed, 519 insertions, 323 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index dd25bf41a..7ca823d1f 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -22,50 +22,16 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; +import "./MixinERC20Transfer.sol"; contract ERC20Proxy is - LibBytes, MixinAssetProxy, - MixinAuthorizable + MixinAuthorizable, + MixinERC20Transfer { - // Id of this proxy. uint8 constant PROXY_ID = 1; - /// @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. - ( - uint8 proxyId, - address token - ) = decodeERC20AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); - - // Transfer tokens. - bool success = IERC20Token(token).transferFrom(from, to, amount); - require( - success, - TRANSFER_FAILED - ); - } - /// @dev Gets the proxy id associated with the proxy address. /// @return Proxy id. function getProxyId() @@ -75,30 +41,4 @@ contract ERC20Proxy is { return PROXY_ID; } - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function decodeERC20AssetData(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token - ) - { - // Validate encoded data length - uint256 length = assetData.length; - require( - length == 21, - LENGTH_21_REQUIRED - ); - - // Decode data - token = readAddress(assetData, 0); - proxyId = uint8(assetData[length - 1]); - - return (proxyId, token); - } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 25136133d..7ff25aea3 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -22,60 +22,16 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; -import "../../tokens/ERC721Token/ERC721Token.sol"; +import "./MixinERC721Transfer.sol"; contract ERC721Proxy is - LibBytes, MixinAssetProxy, - MixinAuthorizable + MixinAuthorizable, + MixinERC721Transfer { - // Id of this proxy. uint8 constant PROXY_ID = 2; - /// @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. - ( - uint8 proxyId, - address token, - uint256 tokenId, - bytes memory receiverData - ) = decodeERC721AssetData(assetData); - - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); - - // There exists only 1 of each token. - require( - amount == 1, - INVALID_AMOUNT - ); - - // Transfer token. Saves gas by calling safeTransferFrom only - // when there is receiverData present. Either succeeds or throws. - if(receiverData.length > 0) { - ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); - } else { - ERC721Token(token).transferFrom(from, to, tokenId); - } - } - /// @dev Gets the proxy id associated with the proxy address. /// @return Proxy id. function getProxyId() @@ -85,44 +41,4 @@ contract ERC721Proxy is { return PROXY_ID; } - - /// @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 ( - uint8 proxyId, - address token, - uint256 tokenId, - bytes memory receiverData - ) - { - // Validate encoded data length - uint256 length = assetData.length; - require( - length >= 53, - LENGTH_AT_LEAST_53_REQUIRED - ); - - // Decode asset data. - token = readAddress(assetData, 0); - tokenId = readUint256(assetData, 20); - if (length > 53) { - receiverData = readBytes(assetData, 52); - } - proxyId = uint8(assetData[length - 1]); - - return ( - proxyId, - token, - tokenId, - receiverData - ); - } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol new file mode 100644 index 000000000..4af39a00b --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol @@ -0,0 +1,81 @@ +/* + + 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 new file mode 100644 index 000000000..d09aba599 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -0,0 +1,94 @@ +/* + + 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); + + // Transfer token. Saves gas by calling safeTransferFrom only + // when there is receiverData present. Either succeeds or throws. + if (receiverData.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); + } else { + ERC721Token(token).transferFrom(from, to, tokenId); + } + } + + /// @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/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index 80180a0d9..dca4f400f 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -25,13 +25,4 @@ contract LibAssetProxyErrors { 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. - - /// AssetProxy errors /// - string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id. - string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. - string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. - - /// Length validation errors /// - string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol new file mode 100644 index 000000000..ba784ab22 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol @@ -0,0 +1,25 @@ +/* + + 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 LibTransferErrors { + /// Transfer errors /// + string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. + string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index e77d81c06..9e0246303 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,12 +19,14 @@ 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 { @@ -81,11 +83,13 @@ 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 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 @@ -94,16 +98,8 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { - - // Lookup asset proxy - uint256 length = assetData.length; - require( - length > 0, - LENGTH_GREATER_THAN_0_REQUIRED - ); - uint8 assetProxyId = uint8(assetData[length - 1]); + // Lookup assetProxy IAssetProxy assetProxy = assetProxies[assetProxyId]; - // transferFrom will either succeed or throw. assetProxy.transferFrom(assetData, from, to, amount); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 12b57d99f..0a0f0209a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -108,9 +108,6 @@ contract MixinExchangeCore is // Compute proportional fill amounts fillResults = calculateFillResults(order, takerAssetFilledAmount); - // Settle order - settleOrder(order, takerAddress, fillResults); - // Update exchange internal state updateFilledState( order, @@ -119,6 +116,10 @@ contract MixinExchangeCore is orderInfo.orderTakerAssetFilledAmount, fillResults ); + + // Settle order + settleOrder(order, takerAddress, fillResults); + return fillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index ed76287e0..517b743fe 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,7 +14,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -25,7 +24,6 @@ import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; contract MixinMatchOrders is - LibBytes, LibMath, LibExchangeErrors, MExchangeCore, @@ -94,14 +92,6 @@ contract MixinMatchOrders is rightSignature ); - // Settle matched orders. Succeeds or throws. - settleMatchedOrders( - leftOrder, - rightOrder, - takerAddress, - matchedFillResults - ); - // Update exchange state updateFilledState( leftOrder, @@ -117,6 +107,14 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); + + // Settle matched orders. Succeeds or throws. + settleMatchedOrders( + leftOrder, + rightOrder, + takerAddress, + matchedFillResults + ); return matchedFillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 83e9dfdf4..29a9c87bd 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -19,6 +19,7 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; @@ -28,25 +29,18 @@ import "./mixins/MSettlement.sol"; import "./mixins/MAssetProxyDispatcher.sol"; contract MixinSettlement is + LibBytes, LibMath, LibExchangeErrors, MMatchOrders, MSettlement, MAssetProxyDispatcher { - // ZRX metadata used for fee transfers. + // ZRX address encoded as a byte array. // This will be constant throughout the life of the Exchange contract, // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_PROXY_DATA; - - /// @dev Gets the ZRX metadata used for fee transfers. - function zrxAssetData() - external - view - returns (bytes memory) - { - return ZRX_PROXY_DATA; - } + bytes internal ZRX_ASSET_DATA; + uint8 constant ZRX_PROXY_ID = 1; /// TODO: _zrxAssetData should be a constant in production. /// @dev Constructor sets the metadata that will be used for paying ZRX fees. @@ -54,7 +48,7 @@ contract MixinSettlement is constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxAssetData; + ZRX_ASSET_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. @@ -68,26 +62,33 @@ contract MixinSettlement is ) internal { + 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( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -107,21 +108,27 @@ contract MixinSettlement is ) internal { + 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 @@ -129,13 +136,15 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -144,7 +153,8 @@ contract MixinSettlement is // Taker fees if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -154,13 +164,15 @@ contract MixinSettlement is ); } else { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + 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 48a0c5552..8ad15aaff 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -82,7 +82,7 @@ contract MixinSignatureValidator is address signer, bytes memory signature ) - internal + public view returns (bool isValid) { @@ -93,7 +93,7 @@ contract MixinSignatureValidator is ); // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popByte(signature))); + SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); // Variables are not scoped in Solidity. uint8 v; @@ -183,7 +183,7 @@ 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 validator = popAddress(signature); + address validator = popLast20Bytes(signature); // Ensure signer has approved validator. if (!allowedValidators[signer][validator]) { return false; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index e09f80bcc..724f95518 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,7 +19,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -27,7 +26,6 @@ import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; contract MixinWrapperFunctions is - LibBytes, LibMath, LibFillResults, LibExchangeErrors, @@ -174,6 +172,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of <order.makerAssetData> + sourceOffset := mload(add(order, 0x140)) // makerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -193,6 +192,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of <order.takerAssetData> + sourceOffset := mload(add(order, 0x160)) // takerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -263,40 +263,50 @@ contract MixinWrapperFunctions is /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. + /// NOTE: makerAssetFilledAmount and takerAssetFilledAmount may include amounts filled of different assets. function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) public + returns (FillResults memory totalFillResults) { for (uint256 i = 0; i < orders.length; i++) { - fillOrder( + FillResults memory singleFillResults = fillOrder( orders[i], takerAssetFillAmounts[i], signatures[i] ); + addFillResults(totalFillResults, singleFillResults); } + return totalFillResults; } /// @dev Synchronously executes multiple calls of fillOrKill. /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. + /// NOTE: makerAssetFilledAmount and takerAssetFilledAmount may include amounts filled of different assets. function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) public + returns (FillResults memory totalFillResults) { for (uint256 i = 0; i < orders.length; i++) { - fillOrKillOrder( + FillResults memory singleFillResults = fillOrKillOrder( orders[i], takerAssetFillAmounts[i], signatures[i] ); + addFillResults(totalFillResults, singleFillResults); } + return totalFillResults; } /// @dev Fills an order with specified parameters and ECDSA signature. @@ -304,20 +314,25 @@ contract MixinWrapperFunctions is /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. + /// NOTE: makerAssetFilledAmount and takerAssetFilledAmount may include amounts filled of different assets. function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) public + returns (FillResults memory totalFillResults) { for (uint256 i = 0; i < orders.length; i++) { - fillOrderNoThrow( + FillResults memory singleFillResults = fillOrderNoThrow( orders[i], takerAssetFillAmounts[i], signatures[i] ); + addFillResults(totalFillResults, singleFillResults); } + return totalFillResults; } /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. @@ -333,14 +348,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -352,6 +366,14 @@ 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); @@ -377,14 +399,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -420,14 +441,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -447,6 +467,14 @@ 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); @@ -472,14 +500,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol index 26e360c91..02aa9776e 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol @@ -39,4 +39,18 @@ contract ISignatureValidator { bool approval ) external; + + /// @dev Verifies that a signature is valid. + /// @param hash Message hash that is signed. + /// @param signer Address of signer. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signer, + bytes memory signature + ) + public + view + returns (bool isValid); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol index acd4f359c..84bb683bc 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol @@ -22,10 +22,7 @@ pragma experimental ABIEncoderV2; import "../libs/LibOrder.sol"; import "../libs/LibFillResults.sol"; -contract IWrapperFunctions is - LibOrder, - LibFillResults -{ +contract IWrapperFunctions { /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @param order LibOrder.Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. @@ -56,35 +53,41 @@ contract IWrapperFunctions is /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) - public; + public + returns (LibFillResults.FillResults memory totalFillResults); /// @dev Synchronously executes multiple calls of fillOrKill. /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) - public; + public + returns (LibFillResults.FillResults memory totalFillResults); /// @dev Fills an order with specified parameters and ECDSA signature. /// Returns false if the transaction would otherwise revert. /// @param orders Array of order specifications. /// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders. /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, bytes[] memory signatures ) - public; + public + returns (LibFillResults.FillResults memory totalFillResults); /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. /// @param orders Array of order specifications. 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 2b4bbeec4..48dd0f8be 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -25,7 +25,6 @@ contract LibExchangeErrors { string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress. string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`. string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed. - string constant ASSET_DATA_MISMATCH = "ASSET_DATA_MISMATCH"; // Asset data must be the same for each order. /// fillOrder validation errors /// string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. 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 82eafb529..d16a830f4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -34,11 +34,13 @@ contract MAssetProxyDispatcher 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 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/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index 7eed453ff..5e286e43a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -35,18 +35,4 @@ contract MSignatureValidator is PreSigned, // 0x07 Trezor // 0x08 } - - /// @dev Verifies that a signature is valid. - /// @param hash Message hash that is signed. - /// @param signer Address of signer. - /// @param signature Proof of signing. - /// @return Validity of order signature. - function isValidSignature( - bytes32 hash, - address signer, - bytes memory signature - ) - internal - view - returns (bool isValid); } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 2c6a8fdb0..5987291d2 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -23,26 +23,8 @@ import "../../protocol/AssetProxy/ERC20Proxy.sol"; import "../../protocol/AssetProxy/ERC721Proxy.sol"; contract TestAssetDataDecoders is - ERC20Proxy, ERC721Proxy { - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function publicDecodeERC20Data(bytes memory assetData) - public - pure - returns ( - uint8 proxyId, - address token - ) - { - (proxyId, token) = decodeERC20AssetData(assetData); - return (proxyId, token); - } - /// @dev Decodes ERC721 Asset data. /// @param assetData Encoded byte array. /// @return proxyId Intended ERC721 proxy id. @@ -54,21 +36,18 @@ contract TestAssetDataDecoders is public pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { ( - proxyId, token, tokenId, receiverData ) = decodeERC721AssetData(assetData); return ( - proxyId, 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 2ae69e0ef..d469a07f0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -24,11 +24,12 @@ 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, from, to, amount); + dispatchTransferFrom(assetData, assetProxyId, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 22c84504c..6f1898acd 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -28,24 +28,24 @@ contract TestLibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function publicPopByte(bytes memory b) + function publicPopLastByte(bytes memory b) public pure returns (bytes memory, bytes1 result) { - result = popByte(b); + result = popLastByte(b); return (b, result); } /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function publicPopAddress(bytes memory b) + function publicPopLast20Bytes(bytes memory b) public pure returns (bytes memory, address result) { - result = popAddress(b); + result = popLast20Bytes(b); return (b, result); } @@ -62,6 +62,21 @@ contract TestLibBytes is 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. + function publicDeepCopyBytes( + bytes memory dest, + bytes memory source + ) + public + pure + returns (bytes memory) + { + deepCopyBytes(dest, source); + return dest; + } + /// @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. diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index d1d10476f..10d7ce41a 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -30,11 +30,12 @@ contract LibBytes is 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 Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function popByte(bytes memory b) + function popLastByte(bytes memory b) internal pure returns (bytes1 result) @@ -58,7 +59,7 @@ contract LibBytes is /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function popAddress(bytes memory b) + function popLast20Bytes(bytes memory b) internal pure returns (address result) @@ -79,41 +80,6 @@ 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 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 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. @@ -346,4 +312,62 @@ contract LibBytes is 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. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // 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 + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of <dest> + getMemAddress(source) + 32, // +32 to skip length of <source> + sourceLen + ); + } } diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index af3f26d82..ec3c8fd36 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -24,10 +24,14 @@ export const constants = { 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.', TESTRPC_NETWORK_ID: 50, - AWAIT_TRANSACTION_MINED_MS: 100, + // 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 + // ensure we always use the minimum interval. + AWAIT_TRANSACTION_MINED_MS: 0, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, diff --git a/packages/contracts/src/utils/coverage.ts b/packages/contracts/src/utils/coverage.ts index 41c83f703..de29a3ecc 100644 --- a/packages/contracts/src/utils/coverage.ts +++ b/packages/contracts/src/utils/coverage.ts @@ -14,7 +14,8 @@ export const coverage = { _getCoverageSubprovider(): CoverageSubprovider { const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); - const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress); + const isVerbose = true; + const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); return subprovider; }, }; diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index a8ca5183e..4cc8f0b89 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -165,14 +165,14 @@ export class ExchangeWrapper { public async marketBuyOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { makerAssetFillAmount: BigNumber }, + opts: { makerAssetFillAmount: BigNumber; gas?: number }, ): Promise<TransactionReceiptWithDecodedLogs> { const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount); const txHash = await this._exchange.marketBuyOrdersNoThrow.sendTransactionAsync( params.orders, params.makerAssetFillAmount, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index 1035f2d7c..32e4787d6 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -2,6 +2,7 @@ import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; +import { constants } from './constants'; import { orderUtils } from './order_utils'; import { BatchCancelOrders, BatchFillOrders, MarketBuyOrders, MarketSellOrders } from './types'; @@ -28,8 +29,11 @@ export const formatters = { signatures: [], takerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.takerAssetData = constants.NULL_BYTES; + } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); }); @@ -41,8 +45,11 @@ export const formatters = { signatures: [], makerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.makerAssetData = constants.NULL_BYTES; + } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); }); diff --git a/packages/contracts/src/utils/order_utils.ts b/packages/contracts/src/utils/order_utils.ts index 2a8791e4c..a9f994d80 100644 --- a/packages/contracts/src/utils/order_utils.ts +++ b/packages/contracts/src/utils/order_utils.ts @@ -1,6 +1,7 @@ import { OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { constants } from './constants'; import { CancelOrder, MatchOrder } from './types'; export const orderUtils = { @@ -43,6 +44,8 @@ export const orderUtils = { leftSignature: signedOrderLeft.signature, rightSignature: signedOrderRight.signature, }; + fill.right.makerAssetData = constants.NULL_BYTES; + fill.right.takerAssetData = constants.NULL_BYTES; return fill; }, }; diff --git a/packages/contracts/src/utils/profiler.ts b/packages/contracts/src/utils/profiler.ts new file mode 100644 index 000000000..85ee24f22 --- /dev/null +++ b/packages/contracts/src/utils/profiler.ts @@ -0,0 +1,27 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { ProfilerSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let profilerSubprovider: ProfilerSubprovider; + +export const profiler = { + start(): void { + profiler.getProfilerSubproviderSingleton().start(); + }, + stop(): void { + profiler.getProfilerSubproviderSingleton().stop(); + }, + getProfilerSubproviderSingleton(): ProfilerSubprovider { + if (_.isUndefined(profilerSubprovider)) { + profilerSubprovider = profiler._getProfilerSubprovider(); + } + return profilerSubprovider; + }, + _getProfilerSubprovider(): ProfilerSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new ProfilerSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/revert_trace.ts b/packages/contracts/src/utils/revert_trace.ts new file mode 100644 index 000000000..0bf8384bc --- /dev/null +++ b/packages/contracts/src/utils/revert_trace.ts @@ -0,0 +1,21 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { RevertTraceSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let revertTraceSubprovider: RevertTraceSubprovider; + +export const revertTrace = { + getRevertTraceSubproviderSingleton(): RevertTraceSubprovider { + if (_.isUndefined(revertTraceSubprovider)) { + revertTraceSubprovider = revertTrace._getRevertTraceSubprovider(); + } + return revertTraceSubprovider; + }, + _getRevertTraceSubprovider(): RevertTraceSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new RevertTraceSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index df9bf88c8..c9d83a02d 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -1,8 +1,12 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; +import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as _ from 'lodash'; import { coverage } from './coverage'; +import { profiler } from './profiler'; +import { revertTrace } from './revert_trace'; enum ProviderType { Ganache = 'ganache', @@ -45,9 +49,34 @@ const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); +const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); +const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); +const enabledSubproviderCount = _.filter([isCoverageEnabled, isProfilerEnabled, isRevertTraceEnabled], _.identity) + .length; +if (enabledSubproviderCount > 1) { + throw new Error(`Only one of coverage, profiler, or revert trace subproviders can be enabled at a time`); +} if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); prependSubprovider(provider, coverageSubprovider); } +if (isProfilerEnabled) { + if (testProvider === ProviderType.Ganache) { + logUtils.warn( + "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", + ); + process.exit(1); + } + const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + logUtils.log( + "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", + ); + profilerSubprovider.stop(); + prependSubprovider(provider, profilerSubprovider); +} +if (isRevertTraceEnabled) { + const revertTraceSubprovider = revertTrace.getRevertTraceSubproviderSingleton(); + prependSubprovider(provider, revertTraceSubprovider); +} export const web3Wrapper = new Web3Wrapper(provider); |