From c8ef10baaf53e2e60f8d053b18a6e8e6432ffbc8 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Thu, 31 Oct 2019 10:10:23 -0400 Subject: [PATCH] `@0x/contracts-utils`: Use simple assembly instead of `abi.decode()` in `LibERC20Token._callWithOptionalBooleanResult()`. `@0x/contracts-exchange-forwarder`: Use `LibERC20Token` in `MixinAssets`. `@0x/order-utils`: Remove `TransferFailedError` from `ForwarderRevertErrors`. --- contracts/exchange-forwarder/CHANGELOG.json | 8 +++++ .../contracts/src/MixinAssets.sol | 33 ++----------------- .../src/libs/LibForwarderRichErrors.sol | 17 ---------- .../utils/contracts/src/LibERC20Token.sol | 3 +- .../src/forwarder_revert_errors.ts | 6 ---- 5 files changed, 12 insertions(+), 55 deletions(-) diff --git a/contracts/exchange-forwarder/CHANGELOG.json b/contracts/exchange-forwarder/CHANGELOG.json index 59f2d977e5..608ecd4e0b 100644 --- a/contracts/exchange-forwarder/CHANGELOG.json +++ b/contracts/exchange-forwarder/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "changes": [ + { + "note": "Use `LibERC20Token` in `MixinAssets`", + "pr": 2309 + } + ] + }, { "version": "3.1.0-beta.0", "changes": [ diff --git a/contracts/exchange-forwarder/contracts/src/MixinAssets.sol b/contracts/exchange-forwarder/contracts/src/MixinAssets.sol index cb62359f08..2fef16220b 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinAssets.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinAssets.sol @@ -19,6 +19,7 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-utils/contracts/src/LibERC20Token.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"; @@ -105,38 +106,8 @@ contract MixinAssets is internal { address token = assetData.readAddress(16); - // Transfer tokens. - // We do a raw call so we can check the success separate - // from the return data. - (bool success, bytes memory returnData) = token.call(abi.encodeWithSelector( - ERC20_TRANSFER_SELECTOR, - msg.sender, - amount - )); - if (!success) { - LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError(returnData)); - } - - // 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) - } - } - } - if (!success) { - LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError(returnData)); - } + LibERC20Token.transfer(token, msg.sender, amount); } /// @dev Decodes ERC721 assetData and transfers given amount to sender. diff --git a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol index 94a76a1d46..07418a4dfd 100644 --- a/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol +++ b/contracts/exchange-forwarder/contracts/src/libs/LibForwarderRichErrors.sol @@ -51,10 +51,6 @@ library LibForwarderRichErrors { bytes4 internal constant OVERSPENT_WETH_ERROR_SELECTOR = 0xcdcbed5d; - // bytes4(keccak256("TransferFailedError(bytes)")) - bytes4 internal constant TRANSFER_FAILED_ERROR_SELECTOR = - 0x5e7eb60f; - // bytes4(keccak256("DefaultFunctionWethContractOnlyError(address)")) bytes4 internal constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR = 0x08b18698; @@ -160,19 +156,6 @@ library LibForwarderRichErrors { ); } - function TransferFailedError( - bytes memory errorData - ) - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - TRANSFER_FAILED_ERROR_SELECTOR, - errorData - ); - } - function DefaultFunctionWethContractOnlyError( address senderAddress ) diff --git a/contracts/utils/contracts/src/LibERC20Token.sol b/contracts/utils/contracts/src/LibERC20Token.sol index 1826ec8ac4..1d14f57f80 100644 --- a/contracts/utils/contracts/src/LibERC20Token.sol +++ b/contracts/utils/contracts/src/LibERC20Token.sol @@ -101,7 +101,8 @@ library LibERC20Token { return; } if (resultData.length == 32) { - uint256 result = abi.decode(resultData, (uint256)); + uint256 result; + assembly { result := mload(add(resultData, 0x20)) } if (result == 1) { return; } diff --git a/packages/order-utils/src/forwarder_revert_errors.ts b/packages/order-utils/src/forwarder_revert_errors.ts index 106187a431..add49a5ab8 100644 --- a/packages/order-utils/src/forwarder_revert_errors.ts +++ b/packages/order-utils/src/forwarder_revert_errors.ts @@ -60,12 +60,6 @@ export class OverspentWethError extends RevertError { } } -export class TransferFailedError extends RevertError { - constructor(errorData?: string) { - super('TransferFailedError', 'TransferFailedError(bytes errorData)', { errorData }); - } -} - export class DefaultFunctionWethContractOnlyError extends RevertError { constructor(senderAddress?: string) { super('DefaultFunctionWethContractOnlyError', 'DefaultFunctionWethContractOnlyError(address senderAddress)', {