diff --git a/contracts/exchange-forwarder/contracts/src/MixinAssets.sol b/contracts/exchange-forwarder/contracts/src/MixinAssets.sol index 9e53a35901..42f74a2c95 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinAssets.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinAssets.sol @@ -19,10 +19,12 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/Ownable.sol"; import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; import "@0x/contracts-erc721/contracts/src/interfaces/IERC721Token.sol"; import "./libs/LibConstants.sol"; +import "./libs/LibForwarderRichErrors.sol"; import "./interfaces/IAssets.sol"; @@ -63,10 +65,9 @@ contract MixinAssets is // For now we only care about ERC20, since percentage fees on ERC721 tokens are invalid. if (proxyId == ERC20_DATA_ID) { address proxyAddress = EXCHANGE.getAssetProxy(ERC20_DATA_ID); - require( - proxyAddress != address(0), - "UNREGISTERED_ASSET_PROXY" - ); + if (proxyAddress == address(0)) { + LibRichErrors._rrevert(LibForwarderRichErrors.UnregisteredAssetProxyError()); + } IERC20Token assetToken = IERC20Token(assetData.readAddress(16)); if (assetToken.allowance(address(this), proxyAddress) != MAX_UINT) { assetToken.approve(proxyAddress, MAX_UINT); @@ -90,7 +91,9 @@ contract MixinAssets is } else if (proxyId == ERC721_DATA_ID) { _transferERC721Token(assetData, amount); } else { - revert("UNSUPPORTED_ASSET_PROXY"); + LibRichErrors._rrevert(LibForwarderRichErrors.UnsupportedAssetProxyError( + proxyId + )); } } @@ -113,10 +116,9 @@ contract MixinAssets is msg.sender, amount )); - require( - success, - "TRANSFER_FAILED" - ); + if (!success) { + LibRichErrors._rrevert(LibForwarderRichErrors.TransferFailedError()); + } // Check return data. // If there is no return data, we assume the token incorrectly @@ -134,10 +136,9 @@ contract MixinAssets is } } } - require( - success, - "TRANSFER_FAILED" - ); + if (!success) { + LibRichErrors._rrevert(LibForwarderRichErrors.TransferFailedError()); + } } /// @dev Decodes ERC721 assetData and transfers given amount to sender. @@ -149,10 +150,11 @@ contract MixinAssets is ) internal { - require( - amount == 1, - "INVALID_AMOUNT" - ); + if (amount != 1) { + LibRichErrors._rrevert(LibForwarderRichErrors.InvalidErc721AmountError( + amount + )); + } // Decode asset data. address token = assetData.readAddress(16); uint256 tokenId = assetData.readUint256(36); diff --git a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol index 24ba8295cb..62a76d39b6 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol @@ -19,11 +19,12 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; -import "./libs/LibConstants.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts/exchange/contracts/src/interfaces/IExchange.sol"; +import "./libs/LibForwarderRichErrors.sol"; contract MixinExchangeWrapper is @@ -98,10 +99,12 @@ contract MixinExchangeWrapper is uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { - require( - orders[i].makerAssetData.equals(orders[0].makerAssetData), - "MAKER_ASSET_MISMATCH" - ); + if (!orders[i].makerAssetData.equals(orders[0].makerAssetData)) { + LibRichErrors._rrevert(LibForwarderRichErrors.MakerAssetMismatchError( + orders[0].makerAssetData, + orders[i].makerAssetData + )); + } // The remaining amount of WETH to sell uint256 remainingTakerAssetFillAmount = _safeSub( @@ -189,10 +192,12 @@ contract MixinExchangeWrapper is { uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { - require( - orders[i].makerAssetData.equals(orders[0].makerAssetData), - "MAKER_ASSET_MISMATCH" - ); + if (!orders[i].makerAssetData.equals(orders[0].makerAssetData)) { + LibRichErrors._rrevert(LibForwarderRichErrors.MakerAssetMismatchError( + orders[0].makerAssetData, + orders[i].makerAssetData + )); + } // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = _getPartialAmountCeil( diff --git a/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol b/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol index 64ea5b7840..8d6df3fb34 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinForwarderCore.sol @@ -20,10 +20,12 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "./libs/LibConstants.sol"; +import "./libs/LibForwarderRichErrors.sol"; import "./interfaces/IAssets.sol"; import "./interfaces/IForwarderCore.sol"; import "./MixinAssets.sol"; @@ -48,10 +50,9 @@ contract MixinForwarderCore is public { address proxyAddress = EXCHANGE.getAssetProxy(ERC20_DATA_ID); - require( - proxyAddress != address(0), - "UNREGISTERED_ASSET_PROXY" - ); + if (proxyAddress == address(0)) { + LibRichErrors._rrevert(LibForwarderRichErrors.UnregisteredAssetProxyError()); + } ETHER_TOKEN.approve(proxyAddress, MAX_UINT); } @@ -112,7 +113,7 @@ contract MixinForwarderCore is /// @dev Attempt to fill makerAssetFillAmount of makerAsset by selling ETH provided with transaction. /// The Forwarder may spend some amount of the makerAsset filled to pay takerFees where - /// takerFeeAssetData == makerAssetData (i.e. percentage fees). + /// takerFeeAssetData == makerAssetData (i.e. percentage fees). /// Any ETH not spent will be refunded to sender. /// @param orders Array of order specifications used containing desired makerAsset and WETH as takerAsset. /// @param makerAssetFillAmount Desired amount of makerAsset to purchase. diff --git a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol index f431600a26..2ef1daa361 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinWeth.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinWeth.sol @@ -18,8 +18,10 @@ pragma solidity ^0.5.9; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "./libs/LibConstants.sol"; +import "./libs/LibForwarderRichErrors.sol"; contract MixinWeth is @@ -31,20 +33,20 @@ contract MixinWeth is external payable { - require( - msg.sender == address(ETHER_TOKEN), - "DEFAULT_FUNCTION_WETH_CONTRACT_ONLY" - ); + if (msg.sender != address(ETHER_TOKEN)) { + LibRichErrors._rrevert(LibForwarderRichErrors.DefaultFunctionWethContractOnlyError( + msg.sender + )); + } } /// @dev Converts message call's ETH value into WETH. function _convertEthToWeth() internal { - require( - msg.value > 0, - "INVALID_MSG_VALUE" - ); + if (msg.value <= 0) { + LibRichErrors._rrevert(LibForwarderRichErrors.InvalidMsgValueError()); + } ETHER_TOKEN.deposit.value(msg.value)(); } @@ -61,16 +63,19 @@ contract MixinWeth is internal { // Ensure feePercentage is less than 5%. - require( - feePercentage <= MAX_FEE_PERCENTAGE, - "FEE_PERCENTAGE_TOO_LARGE" - ); + if (feePercentage > MAX_FEE_PERCENTAGE) { + LibRichErrors._rrevert(LibForwarderRichErrors.FeePercentageTooLargeError( + feePercentage + )); + } // Ensure that no extra WETH owned by this contract has been sold. - require( - wethSold <= msg.value, - "OVERSOLD_WETH" - ); + if (wethSold > msg.value) { + LibRichErrors._rrevert(LibForwarderRichErrors.OversoldWethError( + wethSold, + msg.value + )); + } // Calculate amount of WETH that hasn't been sold. uint256 wethRemaining = _safeSub(msg.value, wethSold); @@ -83,10 +88,12 @@ contract MixinWeth is ); // Ensure fee is less than amount of WETH remaining. - require( - ethFee <= wethRemaining, - "INSUFFICIENT_ETH_REMAINING" - ); + if (ethFee > wethRemaining) { + LibRichErrors._rrevert(LibForwarderRichErrors.InsufficientEthRemainingError( + ethFee, + wethRemaining + )); + } // Do nothing if no WETH remaining if (wethRemaining > 0) { diff --git a/contracts/exchange-forwarder/contracts/src/interfaces/IForwardeRichErrors.sol b/contracts/exchange-forwarder/contracts/src/interfaces/IForwardeRichErrors.sol deleted file mode 100644 index 9dffeabaf6..0000000000 --- a/contracts/exchange-forwarder/contracts/src/interfaces/IForwardeRichErrors.sol +++ /dev/null @@ -1,10 +0,0 @@ -pragma solidity ^0.5.9; - - -contract IForwarderRichErrors { - - enum AssetProxyErrorCodes { - UNREGISTERED_ASSET_PROXY, - UNSUPPORTED_ASSET_PROXY - } -} diff --git a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol index dcb447fe19..bdfc5d0786 100644 --- a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol +++ b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol @@ -19,15 +19,21 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; -import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; -import "./interfaces/IExchangeRichErrors.sol"; -library LibExchangeRichErrors { +library LibForwarderRichErrors { - // bytes4(keccak256("AssetProxyError(bytes4)")) - bytes4 internal constant ASEST_PROXY_ERROR_SELECTOR = - 0xaa3ff166; + // bytes4(keccak256("UnregisteredAssetProxyError()")) + bytes4 internal constant UNREGISTERED_ASSET_PROXY_ERROR_SELECTOR = + 0xf3b96b8d; + + // bytes4(keccak256("UnregisteredAssetProxyError()")) + bytes internal constant UNREGISTERED_ASSET_PROXY_ERROR = + hex"f3b96b8d"; + + // bytes4(keccak256("UnsupportedAssetProxyError(bytes4)")) + bytes4 internal constant UNSUPPORTED_ASSET_PROXY_ERROR_SELECTOR = + 0x7996a271; // bytes4(keccak256("CompleteFillFailedError(uint256,uint256)")) bytes4 internal constant COMPLETE_FILL_FAILED_ERROR_SELECTOR = @@ -53,6 +59,10 @@ library LibExchangeRichErrors { bytes4 internal constant TRANSFER_FAILED_ERROR_SELECTOR = 0x570f1df4; + // bytes4(keccak256("TransferFailedError()")) + bytes internal constant TRANSFER_FAILED_ERROR = + hex"570f1df4"; + // bytes4(keccak256("DefaultFunctionWethContractOnlyError(address)")) bytes4 internal constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR = 0x08b18698; @@ -61,13 +71,24 @@ library LibExchangeRichErrors { bytes4 internal constant INVALID_MSG_VALUE_ERROR_SELECTOR = 0xb0658a43; + // bytes4(keccak256("InvalidMsgValueError()")) + bytes internal constant INVALID_MSG_VALUE_ERROR = + hex"b0658a43"; + // bytes4(keccak256("InvalidErc721AmountError(uint256)")) bytes4 internal constant INVALID_ERC721_AMOUNT_ERROR_SELECTOR = 0x27ed87bf; // solhint-disable func-name-mixedcase - function AssetProxyError( - IForwarderRichErrors.AssetProxyErrorCodes errorCode, + function UnregisteredAssetProxyError() + internal + pure + returns (bytes memory) + { + return UNREGISTERED_ASSET_PROXY_ERROR; + } + + function UnsupportedAssetProxyError( bytes4 proxyId ) internal @@ -75,7 +96,7 @@ library LibExchangeRichErrors { returns (bytes memory) { return abi.encodeWithSelector( - ASSET_PROXY_ERROR_SELECTOR, + UNSUPPORTED_ASSET_PROXY_ERROR_SELECTOR, proxyId ); } @@ -158,7 +179,7 @@ library LibExchangeRichErrors { pure returns (bytes memory) { - return TRANSFER_FAILED_ERROR_SELECTOR; + return TRANSFER_FAILED_ERROR; } function DefaultFunctionWethContractOnlyError( @@ -179,7 +200,7 @@ library LibExchangeRichErrors { pure returns (bytes memory) { - return INVALID_MSG_VALUE_ERROR_SELECTOR; + return INVALID_MSG_VALUE_ERROR; } function InvalidErc721AmountError(