From 8da42b12f4461f646378a32a5d910cdaa0069821 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 28 Jun 2018 18:27:53 -0700
Subject: Remove transferFrom, implement in fallback function

---
 .../current/protocol/AssetProxy/ERC20Proxy.sol     | 201 ++++++-------
 .../current/protocol/AssetProxy/ERC721Proxy.sol    | 325 +++++++++++----------
 .../protocol/AssetProxy/interfaces/IAssetData.sol  |  12 +-
 .../protocol/AssetProxy/interfaces/IAssetProxy.sol |   2 +-
 .../Exchange/MixinAssetProxyDispatcher.sol         |   3 +-
 .../current/tokens/ERC721Token/ERC721Token.sol     |  14 +-
 6 files changed, 267 insertions(+), 290 deletions(-)

(limited to 'packages')

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);
     }
-- 
cgit v1.2.3


From 762a6199b25391bde65ea2f1f91532c3c1d5badf Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 28 Jun 2018 18:28:09 -0700
Subject: Fix tests

---
 packages/contracts/compiler.json                   |   1 +
 packages/contracts/package.json                    |   2 +-
 .../current/tokens/ERC721Token/ERC721Token.sol     |  14 +-
 packages/contracts/src/utils/artifacts.ts          |   2 +
 packages/contracts/src/utils/constants.ts          |   1 +
 packages/contracts/test/asset_proxy/proxies.ts     | 205 ++++++++++++++-------
 6 files changed, 146 insertions(+), 79 deletions(-)

(limited to 'packages')

diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index 1ee58081a..56c8b1761 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -29,6 +29,7 @@
         "Exchange",
         "ExchangeWrapper",
         "IAssetData",
+        "IAssetProxy",
         "IValidator",
         "IWallet",
         "MixinAuthorizable",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index b5a599fe5..1a26f3364 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -35,7 +35,7 @@
     },
     "config": {
         "abis":
-            "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|IAssetData|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+            "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
     },
     "repository": {
         "type": "git",
diff --git a/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol b/packages/contracts/src/contracts/current/tokens/ERC721Token/ERC721Token.sol
index cff34e99e..41ba149e3 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), "3");
+        require(isApprovedOrOwner(msg.sender, _tokenId));
         _;
     }
 
@@ -222,8 +222,8 @@ contract ERC721Token is
         public
         canTransfer(_tokenId)
     {
-        require(_from != address(0), "1");
-        require(_to != address(0), "2");
+        require(_from != address(0));
+        require(_to != address(0));
 
         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), "7");
+        require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data));
     }
 
     /**
@@ -331,7 +331,7 @@ contract ERC721Token is
     function clearApproval(address _owner, uint256 _tokenId)
         internal
     {
-        require(ownerOf(_tokenId) == _owner, "4");
+        require(ownerOf(_tokenId) == _owner);
         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), "6");
+        require(tokenOwner[_tokenId] == address(0));
         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, "5");
+        require(ownerOf(_tokenId) == _from);
         ownedTokensCount[_from] = safeSub(ownedTokensCount[_from], 1);
         tokenOwner[_tokenId] = address(0);
     }
diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts
index d58e7973c..b25266409 100644
--- a/packages/contracts/src/utils/artifacts.ts
+++ b/packages/contracts/src/utils/artifacts.ts
@@ -8,6 +8,7 @@ import * as ERC20Proxy from '../artifacts/ERC20Proxy.json';
 import * as ERC721Proxy from '../artifacts/ERC721Proxy.json';
 import * as Exchange from '../artifacts/Exchange.json';
 import * as ExchangeWrapper from '../artifacts/ExchangeWrapper.json';
+import * as IAssetProxy from '../artifacts/IAssetProxy.json';
 import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json';
 import * as MultiSigWallet from '../artifacts/MultiSigWallet.json';
 import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json';
@@ -33,6 +34,7 @@ export const artifacts = {
     Exchange: (Exchange as any) as ContractArtifact,
     ExchangeWrapper: (ExchangeWrapper as any) as ContractArtifact,
     EtherToken: (EtherToken as any) as ContractArtifact,
+    IAssetProxy: (IAssetProxy as any) as ContractArtifact,
     MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact,
     MultiSigWallet: (MultiSigWallet as any) as ContractArtifact,
     MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact,
diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts
index 2b2bd2425..8e68f376d 100644
--- a/packages/contracts/src/utils/constants.ts
+++ b/packages/contracts/src/utils/constants.ts
@@ -27,6 +27,7 @@ export const constants = {
     MAX_ETHERTOKEN_WITHDRAW_GAS: 43000,
     MAX_TOKEN_TRANSFERFROM_GAS: 80000,
     MAX_TOKEN_APPROVE_GAS: 60000,
+    TRANSFER_FROM_GAS: 150000,
     DUMMY_TOKEN_NAME: '',
     DUMMY_TOKEN_SYMBOL: '',
     DUMMY_TOKEN_DECIMALS: new BigNumber(18),
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 39121d95c..8c9dc9997 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -15,6 +15,7 @@ import {
 import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c721_token';
 import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c20_proxy';
 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy';
+import { IAssetProxyContract } from '../../src/generated_contract_wrappers/i_asset_proxy';
 import { artifacts } from '../../src/utils/artifacts';
 import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions';
 import { chaiSetup } from '../../src/utils/chai_setup';
@@ -41,6 +42,7 @@ describe('Asset Transfer Proxies', () => {
     let erc721Receiver: DummyERC721ReceiverContract;
     let erc20Proxy: ERC20ProxyContract;
     let erc721Proxy: ERC721ProxyContract;
+    let assetProxyInterface: IAssetProxyContract;
 
     let erc20Wrapper: ERC20Wrapper;
     let erc721Wrapper: ERC721Wrapper;
@@ -90,6 +92,11 @@ describe('Asset Transfer Proxies', () => {
             provider,
             txDefaults,
         );
+        assetProxyInterface = await IAssetProxyContract.deployFrom0xArtifactAsync(
+            artifacts.IAssetProxy,
+            provider,
+            txDefaults,
+        );
     });
     beforeEach(async () => {
         await blockchainLifecycle.startAsync();
@@ -105,14 +112,18 @@ describe('Asset Transfer Proxies', () => {
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(10);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 await web3Wrapper.awaitTransactionSuccessAsync(
-                    await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    await web3Wrapper.sendTransactionAsync({
+                        to: erc20Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     constants.AWAIT_TRANSACTION_MINED_MS,
                 );
                 // Verify transfer was successful
@@ -131,14 +142,18 @@ describe('Asset Transfer Proxies', () => {
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(0);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 await web3Wrapper.awaitTransactionSuccessAsync(
-                    await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    await web3Wrapper.sendTransactionAsync({
+                        to: erc20Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     constants.AWAIT_TRANSACTION_MINED_MS,
                 );
                 // Verify transfer was successful
@@ -156,7 +171,13 @@ describe('Asset Transfer Proxies', () => {
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
                 // Create allowance less than transfer amount. Set allowance on proxy.
                 const allowance = new BigNumber(0);
-                const transferAmount = new BigNumber(10);
+                const amount = new BigNumber(10);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await zrxToken.approve.sendTransactionAsync(erc20Proxy.address, allowance, {
                         from: makerAddress,
@@ -165,13 +186,11 @@ describe('Asset Transfer Proxies', () => {
                 );
                 // Perform a transfer; expect this to fail.
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        transferAmount,
-                        { from: exchangeAddress },
-                    ),
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc20Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     RevertReason.TransferFailed,
                 );
             });
@@ -179,11 +198,18 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if requesting address is not authorized', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(10);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc20Proxy.address,
+                        data,
                         from: notAuthorized,
                     }),
                     RevertReason.SenderNotAuthorized,
@@ -208,14 +234,18 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 await web3Wrapper.awaitTransactionSuccessAsync(
-                    await erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    await web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     constants.AWAIT_TRANSACTION_MINED_MS,
                 );
                 // Verify transfer was successful
@@ -231,17 +261,21 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
-                const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
                     encodedAssetData,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
-                    { from: exchangeAddress },
                 );
-                await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS);
-                // Parse transaction logs
                 const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address);
-                const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash);
+                const tx = await logDecoder.getTxWithDecodedLogsAsync(
+                    await web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                        gas: constants.TRANSFER_FROM_GAS,
+                    }),
+                );
                 // Verify that no log was emitted by erc721 receiver
                 expect(tx.logs.length).to.be.equal(1);
                 const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>;
@@ -266,17 +300,21 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
-                const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
                     encodedAssetData,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
-                    { from: exchangeAddress },
                 );
-                await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS);
-                // Parse transaction logs
                 const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address);
-                const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash);
+                const tx = await logDecoder.getTxWithDecodedLogsAsync(
+                    await web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                        gas: constants.TRANSFER_FROM_GAS,
+                    }),
+                );
                 // Validate log emitted by erc721 receiver
                 expect(tx.logs.length).to.be.equal(1);
                 const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>;
@@ -301,14 +339,19 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                        gas: constants.TRANSFER_FROM_GAS,
+                    }),
                     RevertReason.TransferFailed,
                 );
             });
@@ -321,14 +364,18 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(0);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     RevertReason.InvalidAmount,
                 );
             });
@@ -341,14 +388,18 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(500);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: exchangeAddress },
-                    ),
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: exchangeAddress,
+                    }),
                     RevertReason.InvalidAmount,
                 );
             });
@@ -358,15 +409,23 @@ describe('Asset Transfer Proxies', () => {
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
                 // Remove transfer approval for makerAddress.
                 await web3Wrapper.awaitTransactionSuccessAsync(
-                    await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, false, {
+                    await erc721Token.approve.sendTransactionAsync(constants.NULL_ADDRESS, erc721MakerTokenId, {
                         from: makerAddress,
                     }),
                     constants.AWAIT_TRANSACTION_MINED_MS,
                 );
                 // Perform a transfer; expect this to fail.
                 const amount = new BigNumber(1);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
                         from: exchangeAddress,
                     }),
                     RevertReason.TransferFailed,
@@ -378,14 +437,18 @@ describe('Asset Transfer Proxies', () => {
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
+                const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+                    encodedAssetData,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                );
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetData,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        { from: notAuthorized },
-                    ),
+                    web3Wrapper.sendTransactionAsync({
+                        to: erc721Proxy.address,
+                        data,
+                        from: notAuthorized,
+                    }),
                     RevertReason.SenderNotAuthorized,
                 );
             });
-- 
cgit v1.2.3


From 44b6adaa2912e4c21ae93441fde410610f817b1d Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Fri, 29 Jun 2018 10:02:07 -0700
Subject: Fix deployment to geth

---
 .../current/protocol/Exchange/MixinAssetProxyDispatcher.sol   |  2 +-
 packages/contracts/test/asset_proxy/proxies.ts                | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

(limited to 'packages')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index 643ccef81..520569185 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -181,7 +181,7 @@ contract MixinAssetProxyDispatcher is
                     cdStart,                // write output over input
                     512                     // reserve 512 bytes for output
                 )
-                if eq(success, 0) {
+                if iszero(success) {
                     revert(cdStart, returndatasize())
                 }
             }
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 8c9dc9997..08026331f 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -28,6 +28,11 @@ import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'
 chaiSetup.configure();
 const expect = chai.expect;
 const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
+const assetProxyInterface = new IAssetProxyContract(
+    artifacts.IAssetProxy.compilerOutput.abi,
+    constants.NULL_ADDRESS,
+    provider,
+);
 
 // tslint:disable:no-unnecessary-type-assertion
 describe('Asset Transfer Proxies', () => {
@@ -42,7 +47,6 @@ describe('Asset Transfer Proxies', () => {
     let erc721Receiver: DummyERC721ReceiverContract;
     let erc20Proxy: ERC20ProxyContract;
     let erc721Proxy: ERC721ProxyContract;
-    let assetProxyInterface: IAssetProxyContract;
 
     let erc20Wrapper: ERC20Wrapper;
     let erc721Wrapper: ERC721Wrapper;
@@ -92,11 +96,6 @@ describe('Asset Transfer Proxies', () => {
             provider,
             txDefaults,
         );
-        assetProxyInterface = await IAssetProxyContract.deployFrom0xArtifactAsync(
-            artifacts.IAssetProxy,
-            provider,
-            txDefaults,
-        );
     });
     beforeEach(async () => {
         await blockchainLifecycle.startAsync();
-- 
cgit v1.2.3


From 6cf39896f1106f414f83cda1dc42472117e872ae Mon Sep 17 00:00:00 2001
From: Alex Browne <stephenalexbrowne@gmail.com>
Date: Fri, 29 Jun 2018 13:12:46 -0700
Subject: Update expectRevertReasonOrAlwaysFailingTransactionAsync to check
 status codes

---
 packages/contracts/src/utils/assertions.ts       | 41 ++++++++++++++++++++----
 packages/contracts/test/exchange/match_orders.ts | 24 ++------------
 2 files changed, 38 insertions(+), 27 deletions(-)

(limited to 'packages')

diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts
index e702a3200..2ba8f9fb1 100644
--- a/packages/contracts/src/utils/assertions.ts
+++ b/packages/contracts/src/utils/assertions.ts
@@ -1,8 +1,10 @@
 import { RevertReason } from '@0xproject/types';
 import * as chai from 'chai';
+import { TransactionReceipt, TransactionReceiptWithDecodedLogs } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { constants } from './constants';
+import { web3Wrapper } from './web3_wrapper';
 
 const expect = chai.expect;
 
@@ -53,18 +55,45 @@ export function expectRevertOrAlwaysFailingTransactionAsync<T>(p: Promise<T>): P
 }
 
 /**
- * Rejects if the given Promise does not reject with the given revert reason or "always
- * failing transaction" error.
+ * Rejects if at least one the following conditions is not met:
+ * 1) The given Promise rejects with the given revert reason.
+ * 2) The given Promise rejects with an error containing "always failing transaction"
+ * 3) The given Promise fulfills with a txReceipt that has a status of 0 or '0', indicating the transaction failed.
+ * 4) The given Promise fulfills with a txHash and corresponding txReceipt has a status of 0 or '0'.
  * @param p the Promise which is expected to reject
  * @param reason a specific revert reason
  * @returns a new Promise which will reject if the conditions are not met and
  * otherwise resolve with no value.
  */
-export function expectRevertReasonOrAlwaysFailingTransactionAsync<T>(
-    p: Promise<T>,
+export async function expectRevertReasonOrAlwaysFailingTransactionAsync(
+    p: Promise<string | TransactionReceiptWithDecodedLogs | TransactionReceipt>,
     reason: RevertReason,
-): PromiseLike<void> {
-    return _expectEitherErrorAsync(p, 'always failing transaction', reason);
+): Promise<void> {
+    return p
+        .then(async result => {
+            let txReceiptStatus: string | 0 | 1 | null;
+            if (typeof result === 'string') {
+                // Result is a txHash. We need to make a web3 call to get the receipt.
+                const txReceipt = await web3Wrapper.getTransactionReceiptAsync(result);
+                txReceiptStatus = txReceipt.status;
+            } else if ('status' in result) {
+                // Result is a TransactionReceiptWithDecodedLogs or TransactionReceipt
+                // and status is a field of result.
+                txReceiptStatus = result.status;
+            } else {
+                throw new Error('Unexpected result type');
+            }
+            expect(_.toString(txReceiptStatus)).to.equal(
+                '0',
+                'transactionReceipt had a non-zero status, indicating success',
+            );
+        })
+        .catch(err => {
+            expect(err.message).to.satisfy(
+                (msg: string) => _.includes(msg, reason) || _.includes(msg, 'always failing transaction'),
+                `Expected ${reason} or 'always failing transaction' but error message was ${err.message}`,
+            );
+        });
 }
 
 /**
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index e23ab9851..7bdb9c385 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -651,13 +651,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 RevertReason.NegativeSpreadRequired,
             );
         });
@@ -680,13 +674,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 // We are assuming assetData fields of the right order are the
                 // reverse of the left order, rather than checking equality. This
                 // saves a bunch of gas, but as a result if the assetData fields are
@@ -715,13 +703,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 RevertReason.InvalidOrderSignature,
             );
         });
-- 
cgit v1.2.3


From 1e4c3ed22b2409c0f9581cbd763bd1abc4ddcc33 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Fri, 29 Jun 2018 15:21:00 -0700
Subject: Change getTransactionReceipt to awaitTransactionMined

---
 packages/contracts/src/utils/assertions.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'packages')

diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts
index 2ba8f9fb1..baba892d3 100644
--- a/packages/contracts/src/utils/assertions.ts
+++ b/packages/contracts/src/utils/assertions.ts
@@ -74,7 +74,7 @@ export async function expectRevertReasonOrAlwaysFailingTransactionAsync(
             let txReceiptStatus: string | 0 | 1 | null;
             if (typeof result === 'string') {
                 // Result is a txHash. We need to make a web3 call to get the receipt.
-                const txReceipt = await web3Wrapper.getTransactionReceiptAsync(result);
+                const txReceipt = await web3Wrapper.awaitTransactionMinedAsync(result);
                 txReceiptStatus = txReceipt.status;
             } else if ('status' in result) {
                 // Result is a TransactionReceiptWithDecodedLogs or TransactionReceipt
-- 
cgit v1.2.3