Tons of tests around nested byte arrays and ERC721 receiver

2 files changed, 45 insertions, 17 deletions

diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts
index 4416334d1..0f1413ff1 100644
--- a/packages/contracts/test/asset_proxy/decoder.ts
+++ b/packages/contracts/test/asset_proxy/decoder.ts
@@ -56,21 +56,45 @@ describe('LibAssetProxyDecoder', () => {
             expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress);
         });
 
-        it('should correctly decode ERC721 proxy data)', async () => {
+        it('should correctly decode ERC721 proxy data', async () => {
             const tokenId = ZeroEx.generatePseudoRandomSalt();
             const encodedProxyData = assetProxyUtils.encodeERC721ProxyData(testAddress, tokenId);
             const expectedDecodedProxyData = assetProxyUtils.decodeERC721ProxyData(encodedProxyData);
             let decodedAssetProxyId: number;
             let decodedTokenAddress: string;
             let decodedTokenId: BigNumber;
+            let decodedData: string;
             [
                 decodedAssetProxyId,
                 decodedTokenAddress,
                 decodedTokenId,
+                decodedData,
             ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedProxyData);
             expect(decodedAssetProxyId).to.be.equal(expectedDecodedProxyData.assetProxyId);
             expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress);
             expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedProxyData.tokenId);
+            expect(decodedData).to.be.equal(expectedDecodedProxyData.data);
+        });
+
+        it('should correctly decode ERC721 proxy data with receiver data', async () => {
+            const tokenId = ZeroEx.generatePseudoRandomSalt();
+            const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())) + 'FFFF';
+            const encodedProxyData = assetProxyUtils.encodeERC721ProxyData(testAddress, tokenId, data);
+            const expectedDecodedProxyData = assetProxyUtils.decodeERC721ProxyData(encodedProxyData);
+            let decodedAssetProxyId: number;
+            let decodedTokenAddress: string;
+            let decodedTokenId: BigNumber;
+            let decodedData: string;
+            [
+                decodedAssetProxyId,
+                decodedTokenAddress,
+                decodedTokenId,
+                decodedData,
+            ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedProxyData);
+            expect(decodedAssetProxyId).to.be.equal(expectedDecodedProxyData.assetProxyId);
+            expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress);
+            expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedProxyData.tokenId);
+            expect(decodedData).to.be.equal(expectedDecodedProxyData.data);
         });
     });
 });
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index d7f27deb4..4995e95a0 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -261,7 +261,7 @@ describe('Asset Transfer Proxies', () => {
         });
     });
 
-    describe.only('Transfer Proxy - ERC721', () => {
+    describe('Transfer Proxy - ERC721', () => {
         describe('transferFrom', () => {
             it('should successfully transfer tokens', async () => {
                 // Construct metadata for ERC721 proxy
@@ -289,7 +289,7 @@ describe('Asset Transfer Proxies', () => {
                 expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress);
             });
 
-            it.only('should call onERC721Received when transferring to a smart contract', async () => {
+            it('should call onERC721Received when transferring to a smart contract', async () => {
                 // Construct metadata for ERC721 proxy
                 const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData(
                     erc721Token.address,
@@ -326,9 +326,11 @@ describe('Asset Transfer Proxies', () => {
 
             it('should call onERC721Received when transferring to a smart contract and receive extra data', async () => {
                 // Construct metadata for ERC721 proxy
+                const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()));
                 const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData(
                     erc721Token.address,
                     erc721MakerTokenId,
+                    data,
                 );
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -348,14 +350,17 @@ describe('Asset Transfer Proxies', () => {
                 tx.logs = _.filter(tx.logs, log => log.address === erc721Receiver.address);
                 const logDecoder = new LogDecoder(constants.TESTRPC_NETWORK_ID);
                 tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log));
-                // Verify erc721 receiver log emitted
-                console.log(tx.logs);
+                // Validate log emitted  by erc721 receiver
+                expect(tx.logs.length).to.be.equal(1);
+                const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>;
+                expect(tokenReceivedLog.args.from).to.be.equal(makerAddress);
+                expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId);
+                expect(tokenReceivedLog.args.data).to.be.equal(data);
                 // Verify transfer was successful
                 const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address);
             });
 
-            /*
             it('should throw if receiving contract does not have onERC721Received', async () => {
                 // Construct metadata for ERC721 proxy
                 const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData(
@@ -368,17 +373,16 @@ describe('Asset Transfer Proxies', () => {
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(1);
-                await erc721Proxy.transferFrom.sendTransactionAsync(
-                    encodedProxyMetadata,
-                    makerAddress,
-                    takerAddress,
-                    amount,
-                    { from: exchangeAddress },
-                );
-                // Verify transfer was successful
-                const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
-                expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress);
-            });*/
+                return expect(
+                    erc721Proxy.transferFrom.sendTransactionAsync(
+                        encodedProxyMetadata,
+                        makerAddress,
+                        erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
+                        amount,
+                        { from: exchangeAddress },
+                    ),
+                ).to.be.rejectedWith(constants.REVERT);
+            });
 
             it('should throw if transferring 0 amount of a token', async () => {
                 // Construct metadata for ERC721 proxy