Merge pull request #793 from 0xProject/feature/contracts/transferFromFallback

Optimize transferFrom (saves ~2000 gas /fill)

8 files changed, 299 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..520569185 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
@@ -182,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/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/assertions.ts b/packages/contracts/src/utils/assertions.ts
index e702a3200..baba892d3 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.awaitTransactionMinedAsync(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/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),