Split transfer impl and AssetProxyMixin

4 files changed, 156 insertions, 95 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index eeddb9707..7ca823d1f 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -22,69 +22,16 @@ pragma experimental ABIEncoderV2;
 import "../../utils/LibBytes/LibBytes.sol";
 import "./MixinAssetProxy.sol";
 import "./MixinAuthorizable.sol";
-import "../../tokens/ERC20Token/IERC20Token.sol";
+import "./MixinERC20Transfer.sol";
 
 contract ERC20Proxy is
-    LibBytes,
     MixinAssetProxy,
-    MixinAuthorizable
+    MixinAuthorizable,
+    MixinERC20Transfer
 {
-
     // Id of this proxy.
     uint8 constant PROXY_ID = 1;
 
-    /// @dev Internal version of `transferFrom`.
-    /// @param assetData Encoded byte array.
-    /// @param from Address to transfer asset from.
-    /// @param to Address to transfer asset to.
-    /// @param amount Amount of asset to transfer.
-    function transferFromInternal(
-        bytes memory assetData,
-        address from,
-        address to,
-        uint256 amount
-    )
-        internal
-    {
-        // Decode asset data.
-        address token = readAddress(assetData, 0);
-
-        // 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.
-        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)
-                }
-            }
-        }
-        require(
-            success,
-            TRANSFER_FAILED
-        );
-    }
-
     /// @dev Gets the proxy id associated with the proxy address.
     /// @return Proxy id.
     function getProxyId()
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 861fac2c1..2b280add4 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -22,52 +22,16 @@ pragma experimental ABIEncoderV2;
 import "../../utils/LibBytes/LibBytes.sol";
 import "./MixinAssetProxy.sol";
 import "./MixinAuthorizable.sol";
-import "../../tokens/ERC721Token/ERC721Token.sol";
+import "./MixinERC721Transfer.sol";
 
 contract ERC721Proxy is
-    LibBytes,
     MixinAssetProxy,
-    MixinAuthorizable
+    MixinAuthorizable,
+    MixinERC721Transfer
 {
-
     // Id of this proxy.
     uint8 constant PROXY_ID = 2;
 
-    /// @dev Internal version of `transferFrom`.
-    /// @param assetData Encoded byte array.
-    /// @param from Address to transfer asset from.
-    /// @param to Address to transfer asset to.
-    /// @param amount Amount of asset to transfer.
-    function transferFromInternal(
-        bytes memory assetData,
-        address from,
-        address to,
-        uint256 amount
-    )
-        internal
-    {
-        // There exists only 1 of each token.
-        require(
-            amount == 1,
-            INVALID_AMOUNT
-        );
-    
-        // Decode asset data.
-        (
-            address token,
-            uint256 tokenId,
-            bytes memory receiverData
-        ) = decodeERC721AssetData(assetData);
-
-        // Transfer token. Saves gas by calling safeTransferFrom only
-        // when there is receiverData present. Either succeeds or throws.
-        if (receiverData.length > 0) {
-            ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData);
-        } else {
-            ERC721Token(token).transferFrom(from, to, tokenId);
-        }
-    }
-
     /// @dev Gets the proxy id associated with the proxy address.
     /// @return Proxy id.
     function getProxyId()
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
new file mode 100644
index 000000000..b0c32db59
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
@@ -0,0 +1,71 @@
+/*
+
+  Copyright 2018 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.4.24;
+pragma experimental ABIEncoderV2;
+
+import "../../utils/LibBytes/LibBytes.sol";
+import "../../tokens/ERC20Token/IERC20Token.sol";
+
+contract MixinERC20Transfer is
+    LibBytes
+{
+    // Id of this proxy.
+    uint8 constant PROXY_ID = 1;
+    // Revert reasons
+    string constant INVALID_METADATA_LENGTH = "Metadata must have a length of 21.";
+    string constant TRANSFER_FAILED = "Transfer failed.";
+    string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id.";
+
+    /// @dev Internal version of `transferFrom`.
+    /// @param assetMetadata Encoded byte array.
+    /// @param from Address to transfer asset from.
+    /// @param to Address to transfer asset to.
+    /// @param amount Amount of asset to transfer.
+    function transferFromInternal(
+        bytes memory assetMetadata,
+        address from,
+        address to,
+        uint256 amount
+    )
+        internal
+    {
+        // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
+
+        require(
+            length == 21,
+            INVALID_METADATA_LENGTH
+        );
+
+        require(
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
+            PROXY_ID_MISMATCH
+        );
+
+        // Decode metadata.
+        address token = readAddress(assetMetadata, 0);
+
+        // Transfer tokens.
+        bool success = IERC20Token(token).transferFrom(from, to, amount);
+        require(
+            success == true,
+            TRANSFER_FAILED
+        );
+    }
+}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
new file mode 100644
index 000000000..bbb807956
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
@@ -0,0 +1,79 @@
+/*
+
+  Copyright 2018 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.4.24;
+pragma experimental ABIEncoderV2;
+
+import "../../utils/LibBytes/LibBytes.sol";
+import "../../tokens/ERC721Token/ERC721Token.sol";
+
+contract MixinERC721Transfer is
+    LibBytes
+{
+
+    // Id of this proxy.
+    uint8 constant PROXY_ID = 2;
+
+    // Revert reasons
+    string constant INVALID_TRANSFER_AMOUNT = "Transfer amount must equal 1.";
+    string constant INVALID_METADATA_LENGTH = "Metadata must have a length of 53.";
+    string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id.";
+
+    /// @dev Internal version of `transferFrom`.
+    /// @param assetMetadata Encoded byte array.
+    /// @param from Address to transfer asset from.
+    /// @param to Address to transfer asset to.
+    /// @param amount Amount of asset to transfer.
+    function transferFromInternal(
+        bytes memory assetMetadata,
+        address from,
+        address to,
+        uint256 amount
+    )
+        internal
+    {
+        // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
+
+        require(
+            length == 53,
+            INVALID_METADATA_LENGTH
+        );
+
+        require(
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
+            PROXY_ID_MISMATCH
+        );
+
+        // There exists only 1 of each token.
+        require(
+            amount == 1,
+            INVALID_TRANSFER_AMOUNT
+        );
+
+        // Decode metadata
+        address token = readAddress(assetMetadata, 0);
+        uint256 tokenId = readUint256(assetMetadata, 20);
+
+        // Transfer token.
+        // Either succeeds or throws.
+        // @TODO: Call safeTransferFrom if there is additional
+        //        data stored in `assetMetadata`.
+        ERC721Token(token).transferFrom(from, to, tokenId);
+    }
+}