Optimize like in PR #767

3 files changed, 157 insertions, 34 deletions

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"
+            );
         }
     }
 }