From 4df66a4802fc75cd9d3e0c01a92e1cc1ae3dfd58 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Sat, 23 Jun 2018 18:03:49 +0200 Subject: Optimize like in PR #767 --- .../protocol/AssetProxy/MixinERC20Transfer.sol | 64 ++++++++++-------- .../protocol/AssetProxy/MixinERC721Transfer.sol | 77 ++++++++++++++++++++-- .../Exchange/MixinAssetProxyDispatcher.sol | 50 +++++++++++++- 3 files changed, 157 insertions(+), 34 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol index deac59f4d..b5f1f9480 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol @@ -43,35 +43,47 @@ contract MixinERC20Transfer is { // Decode asset data. 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 = 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. + bytes4 transferFromSelector = IERC20Token(token).transferFrom.selector; + bool success; 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) + /////// Setup State /////// + // `cdStart` is the start of the calldata for `token.transferFrom` (equal to free memory ptr). + let cdStart := mload(64) + // `cdEnd` is the end of the calldata for `token.transferFrom`. + let cdEnd := add(cdStart, 100) + + /////// Setup Header Area /////// + // This area holds the 4-byte `transferFromSelector`. + mstore(cdStart, transferFromSelector) + + /////// Setup Params Area /////// + // Each parameter is padded to 32-bytes. The entire Params Area is 96 bytes. + // A 20-byte mask is applied to addresses to zero-out the unused bytes. + mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 68), amount) + + /////// Call `token.transferFrom` using the constructed calldata /////// + success := call( + gas, + token, + 0, + cdStart, + sub(cdEnd, cdStart), + cdStart, + 32 + ) + if success { + 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( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index ebc879644..af986adfa 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -27,7 +27,7 @@ contract MixinERC721Transfer is LibTransferErrors { using LibBytes for bytes; - + bytes4 constant SAFE_TRANSFER_FROM_SELECTOR = bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)")); /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -54,11 +54,76 @@ contract MixinERC721Transfer is bytes memory receiverData ) = decodeERC721AssetData(assetData); - ERC721Token(token).safeTransferFrom( - from, - to, - tokenId, - receiverData + // We construct calldata for the `token.safeTransferFrom` ABI. + // The layout of this calldata is in the table below. + // + // | Area | Offset | Length | Contents | + // | -------- |--------|---------|-------------------------------------------- | + // | Header | 0 | 4 | function selector | + // | Params | | 4 * 32 | function parameters: | + // | | 4 | | 1. from | + // | | 36 | | 2. to | + // | | 68 | | 3. tokenId | + // | | 100 | | 4. offset to receiverData (*) | + // | Data | | | receiverData: | + // | | 132 | 32 | receiverData Length | + // | | 164 | ** | receiverData Contents | + + bytes4 safeTransferFromSelector = SAFE_TRANSFER_FROM_SELECTOR; + bool success; + assembly { + /////// Setup State /////// + // `cdStart` is the start of the calldata for `token.safeTransferFrom` (equal to free memory ptr). + let cdStart := mload(64) + // `dataAreaLength` is the total number of words needed to store `receiverData` + // As-per the ABI spec, this value is padded up to the nearest multiple of 32, + // and includes 32-bytes for length. + // It's calculated as folows: + // - Unpadded length in bytes = `mload(receiverData) + 32` + // - Add 31 to this value then divide by 32 to get the length in words. + // - Multiply this value by 32 to get the padded length in bytes. + let dataAreaLength := mul(div(add(mload(receiverData), 63), 32), 32) + // `cdEnd` is the end of the calldata for `token.safeTransferFrom`. + let cdEnd := add(cdStart, add(132, dataAreaLength)) + + /////// Setup Header Area /////// + // This area holds the 4-byte `transferFromSelector`. + mstore(cdStart, safeTransferFromSelector) + + /////// Setup Params Area /////// + // Each parameter is padded to 32-bytes. The entire Params Area is 128 bytes. + // Notes: + // 1. A 20-byte mask is applied to addresses to zero-out the unused bytes. + // 2. The offset to `receiverData` is the length of the Params Area (128 bytes). + mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 68), tokenId) + mstore(add(cdStart, 100), 128) + + /////// Setup Data Area /////// + // This area holds `receiverData`. + let dataArea := add(cdStart, 132) + for {} lt(dataArea, cdEnd) {} { + mstore(dataArea, mload(receiverData)) + dataArea := add(dataArea, 32) + receiverData := add(receiverData, 32) + } + + /////// Call `token.safeTransferFrom` using the constructed calldata /////// + success := call( + gas, + token, + 0, + cdStart, + sub(cdEnd, cdStart), + cdStart, + 0 + ) + } + + require( + success, + 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 da0beefde..96763970c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -100,21 +100,67 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { + require(assetData.length >= 4, "ASSET_DATA_LENGTH"); + // Lookup assetProxy - bytes4 assetProxyId = assetData.readBytes4(0); + bytes4 assetProxyId; + assembly { + assetProxyId := and(mload(add(assetData, 32)), +0xFFFFFFFF00000000000000000000000000000000000000000000000000000000 + ) + } IAssetProxy assetProxy = assetProxies[assetProxyId]; // Ensure that assetProxy exists require( assetProxy != address(0), ASSET_PROXY_DOES_NOT_EXIST ); - // transferFrom will either succeed or throw. + + /* assetProxy.transferFrom( assetData, from, to, amount ); + return; + */ + + bytes4 transferFromSelector = IAssetProxy(assetProxy).transferFrom.selector; + bool success; + assembly { + + let cdStart := mload(0x40) + let dataAreaLength := mul(div(add(mload(assetData), 63), 32), 32) + let cdEnd := add(cdStart, add(132, dataAreaLength)) + + mstore(cdStart, transferFromSelector) + mstore(add(cdStart, 4), 128) + mstore(add(cdStart, 36), and(from, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 68), and(to, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(cdStart, 100), amount) + + let dataArea := add(cdStart, 132) + for {} lt(dataArea, cdEnd) {} { + mstore(dataArea, mload(assetData)) + dataArea := add(dataArea, 32) + assetData := add(assetData, 32) + } + + success := call( + gas, + assetProxy, + 0, + cdStart, + sub(cdEnd, cdStart), + cdStart, + 0 + ) + } + require( + success, + "TRANSFER_FAILED" + ); } } } -- cgit v1.2.3