Remove transferFrom, implement in fallback function

6 files changed, 267 insertions, 290 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index b4780b52b..aed62f54f 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -20,138 +20,103 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "../../utils/LibBytes/LibBytes.sol";
-import "./interfaces/IAssetProxy.sol";
 import "./MixinAuthorizable.sol";
 
 contract ERC20Proxy is
-    IAssetProxy,
     MixinAuthorizable
 {
     // Id of this proxy.
     bytes4 constant PROXY_ID = bytes4(keccak256("ERC20Token(address)"));
     
-    /// @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 transferFrom(
-        bytes assetData,
-        address from,
-        address to,
-        uint256 amount
-    )
+    function () 
         external
     {
-        require(
-            authorized[msg.sender],
-            "SENDER_NOT_AUTHORIZED"
-        );
-
-        // `transferFrom`.
-        // The function is marked `external`, so no abi decodeding is done for
-        // us. Instead, we expect the `calldata` memory to contain the
-        // following:
-        //
-        // | Area     | Offset | Length  | Contents                            |
-        // |----------|--------|---------|-------------------------------------|
-        // | Header   | 0      | 4       | function selector                   |
-        // | Params   |        | 4 * 32  | function parameters:                |
-        // |          | 4      |         |   1. offset to assetData (*)        |
-        // |          | 36     |         |   2. from                           |
-        // |          | 68     |         |   3. to                             |
-        // |          | 100    |         |   4. amount                         |
-        // | Data     |        |         | assetData:                          |
-        // |          | 132    | 32      | assetData Length                    |
-        // |          | 164    | **      | assetData Contents                  |
-        //
-        // (*): offset is computed from start of function parameters, so offset
-        //      by an additional 4 bytes in the calldata.
-        //
-        // WARNING: The ABIv2 specification allows additional padding between
-        //          the Params and Data section. This will result in a larger
-        //          offset to assetData.
-        
-        // Asset data itself is encoded as follows:
-        //
-        // | Area     | Offset | Length  | Contents                            |
-        // |----------|--------|---------|-------------------------------------|
-        // | Header   | 0      | 4       | function selector                   |
-        // | Params   |        | 1 * 32  | function parameters:                |
-        // |          | 4      | 12 + 20 |   1. token address                  |
-        
-        // Transfer tokens.
-        // We do a raw call so we can check the success separate
-        // from the return data.
-        // We construct calldata for the `token.transferFrom` ABI.
-        // The layout of this calldata is in the table below.
-        // 
-        // | Area     | Offset | Length  | Contents                            |
-        // |----------|--------|---------|-------------------------------------|
-        // | Header   | 0      | 4       | function selector                   |
-        // | Params   |        | 3 * 32  | function parameters:                |
-        // |          | 4      |         |   1. from                           |
-        // |          | 36     |         |   2. to                             |
-        // |          | 68     |         |   3. amount                         |
-        
         assembly {
-            /////// Token contract address ///////
-            // The token address is found as follows:
-            // * It is stored at offset 4 in `assetData` contents.
-            // * This is stored at offset 32 from `assetData`.
-            // * The offset to `assetData` from Params is stored at offset
-            //   4 in calldata.
-            // * The offset of Params in calldata is 4.
-            // So we read location 4 and add 32 + 4 + 4 to it.
-            let token := calldataload(add(calldataload(4), 40))
-            
-            /////// Setup Header Area ///////
-            // This area holds the 4-byte `transferFrom` selector.
-            // Any trailing data in transferFromSelector will be
-            // overwritten in the next `mstore` call.
-            mstore(0, 0x23b872dd00000000000000000000000000000000000000000000000000000000)
-            
-            /////// Setup Params Area ///////
-            // We copy the fields `from`, `to` and `amount` in bulk
-            // from our own calldata to the new calldata.
-            calldatacopy(4, 36, 96)
+            // The first 4 bytes of calldata holds the function selector
+            let selector := and(calldataload(0), 0xffffffff00000000000000000000000000000000000000000000000000000000)
+
+            // `transferFrom` will be called with the following parameters:
+            // assetData Encoded byte array.
+            // from Address to transfer asset from.
+            // to Address to transfer asset to.
+            // amount Amount of asset to transfer.
+            // bytes4(keccak256("transferFrom(bytes,address,address,uint256)")) = 0xa85e59e4
+            if eq(selector, 0xa85e59e400000000000000000000000000000000000000000000000000000000) {
 
-            /////// Call `token.transferFrom` using the calldata ///////
-            let success := call(
-                gas,            // forward all gas
-                token,          // call address of token contract
-                0,              // don't send any ETH
-                0,              // pointer to start of input
-                100,            // length of input
-                0,              // write output over input
-                32              // output size should be 32 bytes
-            )
+                // To lookup a value in a mapping, we load from the storage location keccak256(k, p),
+                // where k is the key left padded to 32 bytes and p is the storage slot
+                let start := mload(64)
+                mstore(start, and(caller, 0xffffffffffffffffffffffffffffffffffffffff))
+                mstore(add(start, 32), authorized_slot)
 
-            /////// 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
-            // nonzero 32 bytes value.
-            // So the transfer succeeded if the call succeeded and either
-            // returned nothing, or returned a non-zero 32 byte value. 
-            success := and(success, or(
-                iszero(returndatasize),
-                and(
-                    eq(returndatasize, 32),
-                    gt(mload(0), 0)
+                // Revert if authorized[msg.sender] == false
+                if iszero(sload(keccak256(start, 64))) {
+                    // Revert with `Error("SENDER_NOT_AUTHORIZED")`
+                    mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                    mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                    mstore(64, 0x0000001553454e4445525f4e4f545f415554484f52495a454400000000000000)
+                    mstore(96, 0)
+                    revert(0, 100)
+                }
+                
+                /////// Token contract address ///////
+                // The token address is found as follows:
+                // * It is stored at offset 4 in `assetData` contents.
+                // * This is stored at offset 32 from `assetData`.
+                // * The offset to `assetData` from Params is stored at offset
+                //   4 in calldata.
+                // * The offset of Params in calldata is 4.
+                // So we read location 4 and add 32 + 4 + 4 to it.
+                let token := calldataload(add(calldataload(4), 40))
+                
+                /////// Setup Header Area ///////
+                // This area holds the 4-byte `transferFrom` selector.
+                // Any trailing data in transferFromSelector will be
+                // overwritten in the next `mstore` call.
+                mstore(0, 0x23b872dd00000000000000000000000000000000000000000000000000000000)
+                
+                /////// Setup Params Area ///////
+                // We copy the fields `from`, `to` and `amount` in bulk
+                // from our own calldata to the new calldata.
+                calldatacopy(4, 36, 96)
+
+                /////// Call `token.transferFrom` using the calldata ///////
+                let success := call(
+                    gas,            // forward all gas
+                    token,          // call address of token contract
+                    0,              // don't send any ETH
+                    0,              // pointer to start of input
+                    100,            // length of input
+                    0,              // write output over input
+                    32              // output size should be 32 bytes
                 )
-            ))
-            if success {
-                return(0, 0)
+
+                /////// 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
+                // nonzero 32 bytes value.
+                // So the transfer succeeded if the call succeeded and either
+                // returned nothing, or returned a non-zero 32 byte value. 
+                success := and(success, or(
+                    iszero(returndatasize),
+                    and(
+                        eq(returndatasize, 32),
+                        gt(mload(0), 0)
+                    )
+                ))
+                if success {
+                    return(0, 0)
+                }
+                
+                // Revert with `Error("TRANSFER_FAILED")`
+                mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000)
+                mstore(96, 0)
+                revert(0, 100)
             }
-            
-            // Revert with `Error("TRANSFER_FAILED")`
-            mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
-            mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
-            mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000)
-            mstore(96, 0)
-            revert(0, 100)
         }
     }
 
@@ -159,7 +124,7 @@ contract ERC20Proxy is
     /// @return Proxy id.
     function getProxyId()
         external
-        view
+        pure
         returns (bytes4)
     {
         return PROXY_ID;
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 8d7cea717..b73dc36cc 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -20,178 +20,189 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "../../utils/LibBytes/LibBytes.sol";
-import "./interfaces/IAssetProxy.sol";
 import "./MixinAuthorizable.sol";
 
 contract ERC721Proxy is
-    IAssetProxy,
     MixinAuthorizable
 {
     // Id of this proxy.
     bytes4 constant PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)"));
 
-    /// @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 transferFrom(
-        bytes assetData,
-        address from,
-        address to,
-        uint256 amount
-    )
+    function () 
         external
     {
-        require(
-            authorized[msg.sender],
-            "SENDER_NOT_AUTHORIZED"
-        );
-    
-        // `transferFrom`.
-        // The function is marked `external`, so no abi decodeding is done for
-        // us. Instead, we expect the `calldata` memory to contain the
-        // following:
-        //
-        // | Area     | Offset | Length  | Contents                            |
-        // |----------|--------|---------|-------------------------------------|
-        // | Header   | 0      | 4       | function selector                   |
-        // | Params   |        | 4 * 32  | function parameters:                |
-        // |          | 4      |         |   1. offset to assetData (*)        |
-        // |          | 36     |         |   2. from                           |
-        // |          | 68     |         |   3. to                             |
-        // |          | 100    |         |   4. amount                         |
-        // | Data     |        |         | assetData:                          |
-        // |          | 132    | 32      | assetData Length                    |
-        // |          | 164    | **      | assetData Contents                  |
-        //
-        // (*): offset is computed from start of function parameters, so offset
-        //      by an additional 4 bytes in the calldata.
-        //
-        // WARNING: The ABIv2 specification allows additional padding between
-        //          the Params and Data section. This will result in a larger
-        //          offset to assetData.
-        
-        // Asset data itself is encoded as follows:
-        //
-        // | Area     | Offset | Length  | Contents                            |
-        // |----------|--------|---------|-------------------------------------|
-        // | Header   | 0      | 4       | function selector                   |
-        // | Params   |        | 3 * 32  | function parameters:                |
-        // |          | 4      | 12 + 20 |   1. token address                  |
-        // |          | 36     |         |   2. tokenId                        |
-        // |          | 68     |         |   3. offset to receiverData (*)     |
-        // | Data     |        |         | receiverData:                       |
-        // |          | 100    | 32      | receiverData Length                 |
-        // |          | 132    | **      | receiverData Contents               |
-        
-        // 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               |
-
         assembly {
-            // There exists only 1 of each token.
-            // require(amount == 1, "INVALID_AMOUNT")
-            if sub(calldataload(100), 1) {
-                // Revert with `Error("INVALID_AMOUNT")`
+            // The first 4 bytes of calldata holds the function selector
+            let selector := and(calldataload(0), 0xffffffff00000000000000000000000000000000000000000000000000000000)
+
+            // `transferFrom` will be called with the following parameters:
+            // assetData Encoded byte array.
+            // from Address to transfer asset from.
+            // to Address to transfer asset to.
+            // amount Amount of asset to transfer.
+            // bytes4(keccak256("transferFrom(bytes,address,address,uint256)")) = 0xa85e59e4
+            if eq(selector, 0xa85e59e400000000000000000000000000000000000000000000000000000000) {
+
+                // To lookup a value in a mapping, we load from the storage location keccak256(k, p),
+                // where k is the key left padded to 32 bytes and p is the storage slot
+                let start := mload(64)
+                mstore(start, and(caller, 0xffffffffffffffffffffffffffffffffffffffff))
+                mstore(add(start, 32), authorized_slot)
+
+                // Revert if authorized[msg.sender] == false
+                if iszero(sload(keccak256(start, 64))) {
+                    // Revert with `Error("SENDER_NOT_AUTHORIZED")`
+                    mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                    mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                    mstore(64, 0x0000001553454e4445525f4e4f545f415554484f52495a454400000000000000)
+                    mstore(96, 0)
+                    revert(0, 100)
+                }
+
+                // `transferFrom`.
+                // The function is marked `external`, so no abi decodeding is done for
+                // us. Instead, we expect the `calldata` memory to contain the
+                // following:
+                //
+                // | Area     | Offset | Length  | Contents                            |
+                // |----------|--------|---------|-------------------------------------|
+                // | Header   | 0      | 4       | function selector                   |
+                // | Params   |        | 4 * 32  | function parameters:                |
+                // |          | 4      |         |   1. offset to assetData (*)        |
+                // |          | 36     |         |   2. from                           |
+                // |          | 68     |         |   3. to                             |
+                // |          | 100    |         |   4. amount                         |
+                // | Data     |        |         | assetData:                          |
+                // |          | 132    | 32      | assetData Length                    |
+                // |          | 164    | **      | assetData Contents                  |
+                //
+                // (*): offset is computed from start of function parameters, so offset
+                //      by an additional 4 bytes in the calldata.
+                //
+                // WARNING: The ABIv2 specification allows additional padding between
+                //          the Params and Data section. This will result in a larger
+                //          offset to assetData.
+                
+                // Asset data itself is encoded as follows:
+                //
+                // | Area     | Offset | Length  | Contents                            |
+                // |----------|--------|---------|-------------------------------------|
+                // | Header   | 0      | 4       | function selector                   |
+                // | Params   |        | 3 * 32  | function parameters:                |
+                // |          | 4      | 12 + 20 |   1. token address                  |
+                // |          | 36     |         |   2. tokenId                        |
+                // |          | 68     |         |   3. offset to receiverData (*)     |
+                // | Data     |        |         | receiverData:                       |
+                // |          | 100    | 32      | receiverData Length                 |
+                // |          | 132    | **      | receiverData Contents               |
+                
+                // 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               |
+
+                // There exists only 1 of each token.
+                // require(amount == 1, "INVALID_AMOUNT")
+                if sub(calldataload(100), 1) {
+                    // Revert with `Error("INVALID_AMOUNT")`
+                    mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                    mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                    mstore(64, 0x0000000e494e56414c49445f414d4f554e540000000000000000000000000000)
+                    mstore(96, 0)
+                    revert(0, 100)
+                }
+                
+                // Require assetData to be at least 132 bytes
+                let offset := calldataload(4)
+                if lt(calldataload(add(offset, 4)), 132) {
+                    // Revert with `Error("LENGTH_GREATER_THAN_131_REQUIRED")`
+                    mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                    mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                    mstore(64, 0x000000204c454e4754485f475245415445525f5448414e5f3133315f52455155)
+                    mstore(96, 0x4952454400000000000000000000000000000000000000000000000000000000)
+                    revert(0, 100)
+                }
+                
+                /////// 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 convert rounding down to rounding up.
+                //        Combined with the previous and this is `63`.
+                //      - Round down to nearest multiple of 32 by clearing
+                //        bits 0x1F. This is done with `and` and a mask.
+
+                /////// Setup Header Area ///////
+                // This area holds the 4-byte `transferFromSelector`.
+                // Any trailing data in transferFromSelector will be
+                // overwritten in the next `mstore` call.
+                mstore(cdStart, 0xb88d4fde00000000000000000000000000000000000000000000000000000000)
+                
+                /////// 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).
+                
+                let length := calldataload(add(offset, 136))
+                let token := calldataload(add(offset, 40))
+                
+                // Round length up to multiple of 32
+                length := and(add(length, 31), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0)
+                
+                // Copy `from` and `to`
+                calldatacopy(add(cdStart, 4), 36, 64)
+                
+                // TokenId
+                mstore(add(cdStart, 68), calldataload(add(offset, 72)))
+                
+                // Offset to receiverData
+                mstore(add(cdStart, 100), 128)
+                
+                // receiverData (including length)
+                calldatacopy(add(cdStart, 132), add(offset, 136), add(length, 32))
+                
+                /////// Call `token.safeTransferFrom` using the calldata ///////
+                let success := call(
+                    gas,                    // forward all gas
+                    token,                  // call address of token contract
+                    0,                      // don't send any ETH
+                    cdStart,                // pointer to start of input
+                    add(length, 164),       // length of input
+                    0,                      // write output to null
+                    0                       // output size is 0 bytes
+                )
+                if success {
+                    return(0, 0)
+                }
+                
+                // Revert with `Error("TRANSFER_FAILED")`
                 mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
                 mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
-                mstore(64, 0x0000000e494e56414c49445f414d4f554e540000000000000000000000000000)
+                mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000)
                 mstore(96, 0)
                 revert(0, 100)
             }
-            
-            // Require assetData to be at least 132 bytes
-            let offset := calldataload(4)
-            if lt(calldataload(add(offset, 4)), 132) {
-                // Revert with `Error("LENGTH_GREATER_THAN_131_REQUIRED")`
-                mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
-                mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
-                mstore(64, 0x000000204c454e4754485f475245415445525f5448414e5f3133315f52455155)
-                mstore(96, 0x4952454400000000000000000000000000000000000000000000000000000000)
-                revert(0, 100)
-            }
-            
-            /////// 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 convert rounding down to rounding up.
-            //        Combined with the previous and this is `63`.
-            //      - Round down to nearest multiple of 32 by clearing
-            //        bits 0x1F. This is done with `and` and a mask.
-
-            /////// Setup Header Area ///////
-            // This area holds the 4-byte `transferFromSelector`.
-            // Any trailing data in transferFromSelector will be
-            // overwritten in the next `mstore` call.
-            mstore(cdStart, 0xb88d4fde00000000000000000000000000000000000000000000000000000000)
-            
-            /////// 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).
-            
-            let length := calldataload(add(offset, 136))
-            let token := calldataload(add(offset, 40))
-            
-            // Round length up to multiple of 32
-            length := and(add(length, 31), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0)
-            
-            // Copy `from` and `to`
-            calldatacopy(add(cdStart, 4), 36, 64)
-            
-            // TokenId
-            mstore(add(cdStart, 68), calldataload(add(offset, 72)))
-            
-            // Offset to receiverData
-            mstore(add(cdStart, 100), 128)
-            
-            // receiverData (including length)
-            calldatacopy(add(cdStart, 132), add(offset, 136), add(length, 32))
-            
-            /////// Call `token.safeTransferFrom` using the calldata ///////
-            let success := call(
-                gas,                    // forward all gas
-                token,                  // call address of token contract
-                0,                      // don't send any ETH
-                cdStart,                // pointer to start of input
-                add(length, 164),       // length of input
-                0,                      // write output to null
-                0                       // output size is 0 bytes
-            )
-            if success {
-                return(0, 0)
-            }
-            
-            // Revert with `Error("TRANSFER_FAILED")`
-            mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
-            mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
-            mstore(64, 0x0000000f5452414e534645525f4641494c454400000000000000000000000000)
-            mstore(96, 0)
-            revert(0, 100)
         }
     }
 
@@ -199,7 +210,7 @@ contract ERC721Proxy is
     /// @return Proxy id.
     function getProxyId()
         external
-        view
+        pure
         returns (bytes4)
     {
         return PROXY_ID;
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol
index 8c78ee8c4..7ebd6acf0 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol
@@ -23,14 +23,16 @@ pragma solidity ^0.4.23;
 // This argument is ABI encoded as one of the methods of this interface.
 interface IAssetData {
     
-    function ERC20Token(
-        address tokenContract)
-        external pure;
+    function ERC20Token(address tokenContract)
+        external
+        pure;
     
     function ERC721Token(
         address tokenContract,
         uint256 tokenId,
-        bytes receiverData)
-        external pure;
+        bytes receiverData
+    )
+        external
+        pure;
     
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
index ae8e195da..eacd5a412 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -42,6 +42,6 @@ contract IAssetProxy is
     /// @return Proxy id.
     function getProxyId()
         external
-        view
+        pure
         returns (bytes4);
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index f7086f543..643ccef81 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -135,7 +135,6 @@ contract MixinAssetProxyDispatcher is
             // |          | 132    | 32      | assetData Length                            |
             // |          | 164    | **      | assetData Contents                          |
 
-            bool success;
             assembly {
                 /////// Setup State ///////
                 // `cdStart` is the start of the calldata for `assetProxy.transferFrom` (equal to free memory ptr).
@@ -173,7 +172,7 @@ contract MixinAssetProxyDispatcher is
                 }
 
                 /////// Call `assetProxy.transferFrom` using the constructed calldata ///////
-                success := call(
+                let success := call(
                     gas,                    // forward all gas
                     assetProxy,             // call address of asset proxy
                     0,                      // don't send any ETH
diff --git a/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol b/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol
index 41ba149e3..cff34e99e 100644
--- a/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol
+++ b/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol
@@ -68,7 +68,7 @@ contract ERC721Token is
     * @param _tokenId uint256 ID of the token to validate
     */
     modifier canTransfer(uint256 _tokenId) {
-        require(isApprovedOrOwner(msg.sender, _tokenId));
+        require(isApprovedOrOwner(msg.sender, _tokenId), "3");
         _;
     }
 
@@ -222,8 +222,8 @@ contract ERC721Token is
         public
         canTransfer(_tokenId)
     {
-        require(_from != address(0));
-        require(_to != address(0));
+        require(_from != address(0), "1");
+        require(_to != address(0), "2");
 
         clearApproval(_from, _tokenId);
         removeTokenFrom(_from, _tokenId);
@@ -276,7 +276,7 @@ contract ERC721Token is
     {
         transferFrom(_from, _to, _tokenId);
         // solium-disable-next-line arg-overflow
-        require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data));
+        require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data), "7");
     }
 
     /**
@@ -331,7 +331,7 @@ contract ERC721Token is
     function clearApproval(address _owner, uint256 _tokenId)
         internal
     {
-        require(ownerOf(_tokenId) == _owner);
+        require(ownerOf(_tokenId) == _owner, "4");
         if (tokenApprovals[_tokenId] != address(0)) {
             tokenApprovals[_tokenId] = address(0);
             emit Approval(_owner, address(0), _tokenId);
@@ -346,7 +346,7 @@ contract ERC721Token is
     function addTokenTo(address _to, uint256 _tokenId)
         internal
     {
-        require(tokenOwner[_tokenId] == address(0));
+        require(tokenOwner[_tokenId] == address(0), "6");
         tokenOwner[_tokenId] = _to;
         ownedTokensCount[_to] = safeAdd(ownedTokensCount[_to], 1);
     }
@@ -359,7 +359,7 @@ contract ERC721Token is
     function removeTokenFrom(address _from, uint256 _tokenId)
         internal
     {
-        require(ownerOf(_tokenId) == _from);
+        require(ownerOf(_tokenId) == _from, "5");
         ownedTokensCount[_from] = safeSub(ownedTokensCount[_from], 1);
         tokenOwner[_tokenId] = address(0);
     }