Style updates to ERC721 onReceiver

7 files changed, 27 insertions, 16 deletions

diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index e436ae15e..b533a22ce 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -30,7 +30,7 @@
         "test:circleci": "yarn test"
     },
     "config": {
-        "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|DummyERC721Receiver|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+        "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
     },
     "repository": {
         "type": "git",
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index c8e8f4587..50632400e 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -20,7 +20,6 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "../../utils/LibBytes/LibBytes.sol";
-import "../../tokens/ERC20Token/IERC20Token.sol";
 import "./MixinAssetProxy.sol";
 import "./MixinAuthorizable.sol";
 import "../../tokens/ERC20Token/IERC20Token.sol";
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 7de7968cc..21e5518c6 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -33,8 +33,6 @@ contract ERC721Proxy is
     // Id of this proxy.
     uint8 constant PROXY_ID = 2;
 
-    string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id.";
-
     /// @dev Internal version of `transferFrom`.
     /// @param assetData Encoded byte array.
     /// @param from Address to transfer asset from.
@@ -60,7 +58,7 @@ contract ERC721Proxy is
         // Data must be intended for this proxy.
         require(
             proxyId == PROXY_ID,
-            PROXY_ID_MISMATCH
+            ASSET_PROXY_ID_MISMATCH
         );
 
         // There exists only 1 of each token.
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 5610c47b3..8f6647d20 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -332,7 +332,8 @@ contract LibBytes is
         internal
         pure
     {
-        // Read length of nested bytes
+        // Assert length of <b> is valid, given
+        // length of input
         require(
             b.length >= index + 32 /* 32 bytes to store length */ + input.length,
             GTE_32_LENGTH_REQUIRED
@@ -340,9 +341,9 @@ contract LibBytes is
 
         // Copy <input> into <b>
         memcpy(
-            getMemAddress(b) + index + 32,  // +32 to skip length of <b>
-            getMemAddress(input),           // include length of byte array
-            input.length + 32               // +32 bytes to store length
+            getMemAddress(b) + 32 + index,  // +32 to skip length of <b>
+            getMemAddress(input),           // includes length of <input>
+            input.length + 32               // +32 bytes to store <input> length
         );
     }
 }
diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts
index f2668dfe6..6a60c07bb 100644
--- a/packages/contracts/test/asset_proxy/decoder.ts
+++ b/packages/contracts/test/asset_proxy/decoder.ts
@@ -25,6 +25,12 @@ describe('TestAssetDataDecoders', () => {
     let testAddress: string;
 
     before(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    after(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+    before(async () => {
         // Setup accounts & addresses
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         owner = accounts[0];
@@ -78,8 +84,12 @@ describe('TestAssetDataDecoders', () => {
 
         it('should correctly decode ERC721 asset data with receiver data', async () => {
             const tokenId = generatePseudoRandomSalt();
-            const receiverData =
-                ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())) + 'FFFF';
+            const receiverDataFirst32Bytes = ethUtil.bufferToHex(
+                assetProxyUtils.encodeUint256(generatePseudoRandomSalt()),
+            );
+            const receiverDataExtraBytes = 'FFFF';
+            // We add extra bytes to generate a value that doesn't fit perfectly into one word
+            const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes;
             const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData);
             const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
             let decodedAssetProxyId: number;
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 52a92718b..a7dc54efc 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -287,7 +287,7 @@ describe('Asset Transfer Proxies', () => {
                     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);
@@ -319,6 +319,7 @@ describe('Asset Transfer Proxies', () => {
                     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);
diff --git a/packages/order-utils/src/asset_proxy_utils.ts b/packages/order-utils/src/asset_proxy_utils.ts
index 61a9b12e9..c7b301c1a 100644
--- a/packages/order-utils/src/asset_proxy_utils.ts
+++ b/packages/order-utils/src/asset_proxy_utils.ts
@@ -119,10 +119,12 @@ export const assetProxyUtils = {
         const tokenId = assetProxyUtils.decodeUint256(encodedTokenId);
         const nullData = '0x';
         let receiverData = nullData;
-        if (encodedAssetData.byteLength > receiverDataLengthOffset + 1) {
+        const lengthUpToReceiverDataLength = receiverDataLengthOffset + 1;
+        if (encodedAssetData.byteLength > lengthUpToReceiverDataLength) {
             const encodedReceiverDataLength = encodedAssetData.slice(receiverDataLengthOffset, receiverDataOffset);
             const receiverDataLength = assetProxyUtils.decodeUint256(encodedReceiverDataLength);
-            const expectedReceiverDataLength = new BigNumber(encodedAssetData.byteLength - (receiverDataOffset + 1));
+            const lengthUpToReceiverData = receiverDataOffset + 1;
+            const expectedReceiverDataLength = new BigNumber(encodedAssetData.byteLength - lengthUpToReceiverData);
             if (!receiverDataLength.equals(expectedReceiverDataLength)) {
                 throw new Error(
                     `Data length (${receiverDataLength}) does not match actual length of data (${expectedReceiverDataLength})`,
@@ -167,12 +169,12 @@ export const assetProxyUtils = {
                 return generalizedERC20AssetData;
             case AssetProxyId.ERC721:
                 const erc721AssetData = assetProxyUtils.decodeERC721AssetData(assetData);
-                const generaliedERC721AssetData = {
+                const generalizedERC721AssetData = {
                     assetProxyId,
                     tokenAddress: erc721AssetData.tokenAddress,
                     data: erc721AssetData.tokenId,
                 };
-                return generaliedERC721AssetData;
+                return generalizedERC721AssetData;
             default:
                 throw new Error(`Unrecognized asset proxy id: ${assetProxyId}`);
         }