diff --git a/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol b/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol index e2a0b25efe..7a73203615 100644 --- a/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol +++ b/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol @@ -25,7 +25,8 @@ import "./interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, - MAssetProxyDispatcher + MAssetProxyDispatcher, + MRichErrors { // Mapping from Asset Proxy Id's to their respective Asset Proxy mapping (bytes4 => address) public assetProxies; @@ -64,17 +65,17 @@ contract MixinAssetProxyDispatcher is } /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. + /// @param orderHash Hash of the order associated with this transfer. /// @param assetData Byte array encoded for the asset. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. - /// @param orderHash Order hash, used for rich reverts. function dispatchTransferFrom( + bytes32 orderHash, bytes memory assetData, address from, address to, - uint256 amount, - bytes32 orderHash + uint256 amount ) internal { @@ -82,14 +83,13 @@ contract MixinAssetProxyDispatcher is if (amount > 0 && from != to) { // Ensure assetData length is valid if (assetData.length <= 3) { - rrevert(AssetProxyDispatchError(AssetProxyDispatchErrorCodes.INVALID_ASSET_DATA_LENGTH)); + rrevert(AssetProxyDispatchError( + orderHash, + assetData, + AssetProxyDispatchErrorCodes.INVALID_ASSET_DATA_LENGTH + )); } - require( - assetData.length > 3, - "LENGTH_GREATER_THAN_3_REQUIRED" - ); - // Lookup assetProxy. We do not use `LibBytes.readBytes4` for gas efficiency reasons. bytes4 assetProxyId; assembly { @@ -102,13 +102,17 @@ contract MixinAssetProxyDispatcher is // Ensure that assetProxy exists if (assetProxy == address(0)) { - rrevert(AssetProxyDispatchError(AssetProxyDispatchErrorCodes.UNKNOWN_ASSET_PROXY)); + rrevert(AssetProxyDispatchError( + orderHash, + assetData, + AssetProxyDispatchErrorCodes.UNKNOWN_ASSET_PROXY + )); } // Whether the AssetProxy transfer succeeded. bool didSucceed; // On failure, the revert message returned by the asset proxy. - bytes revertMessage = new bytes(0); + bytes revertMessage; // We construct calldata for the `assetProxy.transferFrom` ABI. // The layout of this calldata is in the table below. @@ -129,8 +133,8 @@ contract MixinAssetProxyDispatcher is /////// Setup State /////// // `cdStart` is the start of the calldata for `assetProxy.transferFrom` (equal to free memory ptr). let cdStart := mload(64) - // We reserve 256 bytes from `cdStart` because we'll reuse it later for return data. - mstore(64, add(cdStart, 256)) + // We reserve 288 bytes from `cdStart` because we'll reuse it later for return data. + mstore(64, add(cdStart, 288)) // `dataAreaLength` is the total number of words needed to store `assetData` // As-per the ABI spec, this value is padded up to the nearest multiple of 32, // and includes 32-bytes for length. @@ -172,7 +176,7 @@ contract MixinAssetProxyDispatcher is cdStart, // pointer to start of input sub(cdEnd, cdStart), // length of input cdStart, // write output over input - 256 // reserve 256 bytes for output + 288 // reserve 288 bytes for output ) if iszero(didSucceed) { // Call reverted. @@ -196,12 +200,12 @@ contract MixinAssetProxyDispatcher is let selector := and(mload(sub(cdStart, 28), 0xffffffff) cdStart := add(cdStart, 4) if eq(selector, 0x08c379a) { - let dataLength := mload(cdStart); + // Set revertMessage to the start of Data. revertMessage := add(cdStart, mload(cdStart)) // Truncate the data length if it's larger than our buffer - // size (256 - 32 = 224) - if gt(dataLength, 224) { - mstore(revertMessage, 224) + // size (288 - 32 = 256) + if gt(mload(revertMessage), 256) { + mstore(revertMessage, 256) } } } diff --git a/contracts/exchange/contracts/src/MixinRichErrors.sol b/contracts/exchange/contracts/src/MixinRichErrors.sol index ac1eecfb93..7e7eec9681 100644 --- a/contracts/exchange/contracts/src/MixinRichErrors.sol +++ b/contracts/exchange/contracts/src/MixinRichErrors.sol @@ -135,7 +135,7 @@ contract MixinRichErrors is } function AssetProxyExistsError( - address proxy + address proxyAddress ) internal pure @@ -143,11 +143,12 @@ contract MixinRichErrors is { return abi.encodeWithSelector( ASSET_PROXY_EXISTS_ERROR_SELECTOR, - proxy + proxyAddress ); } function AssetProxyDispatchError( + bytes32 orderHash, bytes memory assetData, AssetProxyDispatchErrorCodes error ) @@ -157,6 +158,7 @@ contract MixinRichErrors is { return abi.encodeWithSelector( ASSET_PROXY_DISPATCH_ERROR_SELECTOR, + orderHash, assetData, uint8(error) ); diff --git a/contracts/exchange/contracts/src/mixins/MRichErrors.sol b/contracts/exchange/contracts/src/mixins/MRichErrors.sol index b92bb461f1..57ccb797f3 100644 --- a/contracts/exchange/contracts/src/mixins/MRichErrors.sol +++ b/contracts/exchange/contracts/src/mixins/MRichErrors.sol @@ -18,9 +18,12 @@ pragma solidity ^0.5.5; +import "@0x/contracts-utils/contracs/src/mixins/MLibRichErrors.sol"; -contract MRichErrors { +contract MRichErrors is + MLibRichErrors +{ enum FillErrorCodes { INVALID_TAKER_AMOUNT, TAKER_OVERPAY, @@ -139,9 +142,10 @@ contract MRichErrors { returns (bytes memory); bytes4 internal constant ASSET_PROXY_DISPATCH_ERROR_SELECTOR = - bytes4(keccak256("AssetProxyDispatchError(bytes,uint8)")); + bytes4(keccak256("AssetProxyDispatchError(bytes32,bytes,uint8)")); function AssetProxyDispatchError( + bytes32 orderHash, bytes memory assetData, AssetProxyDispatchErrorCodes error ) diff --git a/contracts/utils/contracts/src/LibRichErrors.sol b/contracts/utils/contracts/src/LibRichErrors.sol index 0d6d89bcc5..5f66ccd250 100644 --- a/contracts/utils/contracts/src/LibRichErrors.sol +++ b/contracts/utils/contracts/src/LibRichErrors.sol @@ -18,12 +18,13 @@ pragma solidity ^0.5.5; -contract LibRichErrors { +import "./mixins/MLibRichErrors.sol"; + + +contract LibRichErrors is + MLibRichErrors +{ // solhint-disable func-name-mixedcase - - bytes4 private constant STANDARD_ERROR_SELECTOR = - bytes4(keccak256("Error(string)")); - function StandardError( string memory message ) diff --git a/contracts/utils/contracts/src/mixins/MLibRichErrors.sol b/contracts/utils/contracts/src/mixins/MLibRichErrors.sol new file mode 100644 index 0000000000..24b36d126e --- /dev/null +++ b/contracts/utils/contracts/src/mixins/MLibRichErrors.sol @@ -0,0 +1,41 @@ +/* + + 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 MRichErrors { + // solhint-disable func-name-mixedcase + + bytes4 internal constant STANDARD_ERROR_SELECTOR = + bytes4(keccak256("Error(string)")); + + function StandardError( + string memory message + ) + internal + pure + returns (bytes memory); + + // solhint-enable func-name-mixedcase + + /// @dev Reverts an encoded rich revert reason `errorData`. + /// @param errorData ABI encoded error data. + function rrevert(bytes memory errorData) + internal + pure; +}