Merge branch 'v2-prototype' into refactor/check-revert-reasons

* v2-prototype: (21 commits)
  Don't use variables for revert reasons
  Add comments to dispatchTransferFrom
  Document IAssetData
  Compute bytes4 proxyid constants
  Return inplace bytes in LibBytes.readBytesWithLength
  Add Greg's documentation to MixinErc721Transfer
  Add Greg's documentation to MixinErc20Transfer
  Gas golf the transfer success logic
  Optimize like in PR #767
  Run prettier and fix linting errors
  Fix tests
  Update tests
  Fix AssetProxyUtils
  Update tests
  Fix ERC721Proxy TokenId
  Update AssetProxyUtils
  Add IAssetData
  WIP
  ABI encoded asset data
  Remove proxyId argument from dispatchTransferFrom
  ...

# Conflicts:
#	packages/contracts/test/asset_proxy/proxies.ts

7 files changed, 34 insertions, 60 deletions

diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts
index 98d18aa38..902255caf 100644
--- a/packages/contracts/test/asset_proxy/decoder.ts
+++ b/packages/contracts/test/asset_proxy/decoder.ts
@@ -44,7 +44,6 @@ describe('TestAssetDataDecoders', () => {
         it('should correctly decode ERC721 asset data', async () => {
             const tokenId = generatePseudoRandomSalt();
             const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId);
-            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
             const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
             let decodedTokenAddress: string;
             let decodedTokenId: BigNumber;
@@ -53,7 +52,7 @@ describe('TestAssetDataDecoders', () => {
                 decodedTokenAddress,
                 decodedTokenId,
                 decodedData,
-            ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetDataWithoutProxyId);
+            ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData);
             expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
             expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId);
             expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData);
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index d07d82235..0457eb3ef 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -100,13 +100,12 @@ describe('Asset Transfer Proxies', () => {
             it('should successfully transfer tokens', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(10);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -127,13 +126,12 @@ describe('Asset Transfer Proxies', () => {
             it('should do nothing if transferring 0 amount of a token', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
                 const amount = new BigNumber(0);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -179,19 +177,13 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if requesting address is not authorized', async () => {
                 // Construct ERC20 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
+
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(10);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        {
-                            from: notAuthorized,
-                        },
-                    ),
+                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                        from: notAuthorized,
+                    }),
                     RevertReasons.SenderNotAuthorized,
                 );
             });
@@ -202,10 +194,9 @@ describe('Asset Transfer Proxies', () => {
                 const erc20Balances = await erc20Wrapper.getBalancesAsync();
 
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 const amount = new BigNumber(10);
                 const numTransfers = 2;
-                const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
+                const assetData = _.times(numTransfers, () => encodedAssetData);
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
                 const amounts = _.times(numTransfers, () => amount);
@@ -234,10 +225,9 @@ describe('Asset Transfer Proxies', () => {
 
             it('should throw if not called by an authorized address', async () => {
                 const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 const amount = new BigNumber(10);
                 const numTransfers = 2;
-                const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
+                const assetData = _.times(numTransfers, () => encodedAssetData);
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
                 const amounts = _.times(numTransfers, () => amount);
@@ -251,9 +241,10 @@ describe('Asset Transfer Proxies', () => {
             });
         });
 
-        it('should have an id of 1', async () => {
+        it('should have an id of 0xf47261b0', async () => {
             const proxyId = await erc20Proxy.getProxyId.callAsync();
-            expect(proxyId).to.equal(1);
+            const expectedProxyId = '0xf47261b0';
+            expect(proxyId).to.equal(expectedProxyId);
         });
     });
 
@@ -262,7 +253,6 @@ describe('Asset Transfer Proxies', () => {
             it('should successfully transfer tokens', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -270,7 +260,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(1);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -286,14 +276,13 @@ describe('Asset Transfer Proxies', () => {
             it('should call onERC721Received when transferring to a smart contract without receiver data', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 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(
-                    encodedAssetDataWithoutProxyId,
+                    encodedAssetData,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
@@ -322,14 +311,13 @@ describe('Asset Transfer Proxies', () => {
                     erc721MakerTokenId,
                     receiverData,
                 );
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 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(
-                    encodedAssetDataWithoutProxyId,
+                    encodedAssetData,
                     makerAddress,
                     erc721Receiver.address,
                     amount,
@@ -358,7 +346,6 @@ describe('Asset Transfer Proxies', () => {
                     erc721MakerTokenId,
                     receiverData,
                 );
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -366,7 +353,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(1);
                 return expectRevertOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
                         amount,
@@ -378,7 +365,6 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if transferring 0 amount of a token', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -386,7 +372,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(0);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -399,7 +385,6 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if transferring > 1 amount of a token', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Verify pre-condition
                 const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -407,7 +392,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(500);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -420,7 +405,6 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if allowances are too low', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Remove transfer approval for makerAddress.
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, false, {
@@ -431,15 +415,9 @@ describe('Asset Transfer Proxies', () => {
                 // Perform a transfer; expect this to fail.
                 const amount = new BigNumber(1);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                    erc20Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
-                        makerAddress,
-                        takerAddress,
-                        amount,
-                        {
-                            from: exchangeAddress,
-                        },
-                    ),
+                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                        from: exchangeAddress,
+                    }),
                     RevertReasons.TransferFailed,
                 );
             });
@@ -447,12 +425,11 @@ describe('Asset Transfer Proxies', () => {
             it('should throw if requesting address is not authorized', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
-                const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -470,8 +447,8 @@ describe('Asset Transfer Proxies', () => {
 
                 const numTransfers = 2;
                 const assetData = [
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2),
-                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA),
+                    assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB),
                 ];
                 const fromAddresses = _.times(numTransfers, () => makerAddress);
                 const toAddresses = _.times(numTransfers, () => takerAddress);
@@ -518,9 +495,10 @@ describe('Asset Transfer Proxies', () => {
             });
         });
 
-        it('should have an id of 2', async () => {
+        it('should have an id of 0x08e937fa', async () => {
             const proxyId = await erc721Proxy.getProxyId.callAsync();
-            expect(proxyId).to.equal(2);
+            const expectedProxyId = '0x08e937fa';
+            expect(proxyId).to.equal(expectedProxyId);
         });
     });
 });
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 3b8a369b4..c83041638 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -89,7 +89,7 @@ describe('Exchange core', () => {
             artifacts.Exchange,
             provider,
             txDefaults,
-            zrxToken.address,
+            assetProxyUtils.encodeERC20AssetData(zrxToken.address),
         );
         exchangeWrapper = new ExchangeWrapper(exchange, provider);
         await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
index cf73d274b..495f6d3c8 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -279,14 +279,13 @@ describe('AssetProxyDispatcher', () => {
             );
             // Construct metadata for ERC20 proxy
             const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
+
             // Perform a transfer from makerAddress to takerAddress
             const erc20Balances = await erc20Wrapper.getBalancesAsync();
             const amount = new BigNumber(10);
             await web3Wrapper.awaitTransactionSuccessAsync(
                 await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
-                    encodedAssetDataWithoutProxyId,
-                    AssetProxyId.ERC20,
+                    encodedAssetData,
                     makerAddress,
                     takerAddress,
                     amount,
@@ -307,13 +306,11 @@ describe('AssetProxyDispatcher', () => {
         it('should throw if dispatching to unregistered proxy', async () => {
             // Construct metadata for ERC20 proxy
             const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
-            const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
             // Perform a transfer from makerAddress to takerAddress
             const amount = new BigNumber(10);
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
-                    encodedAssetDataWithoutProxyId,
-                    AssetProxyId.ERC20,
+                    encodedAssetData,
                     makerAddress,
                     takerAddress,
                     amount,
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index 324cf319f..6f4feb8b7 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -96,7 +96,7 @@ describe('matchOrders', () => {
             artifacts.Exchange,
             provider,
             txDefaults,
-            zrxToken.address,
+            assetProxyUtils.encodeERC20AssetData(zrxToken.address),
         );
         exchangeWrapper = new ExchangeWrapper(exchange, provider);
         await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index 8d4799109..44ed98c53 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -79,7 +79,7 @@ describe('Exchange transactions', () => {
             artifacts.Exchange,
             provider,
             txDefaults,
-            zrxToken.address,
+            assetProxyUtils.encodeERC20AssetData(zrxToken.address),
         );
         exchangeWrapper = new ExchangeWrapper(exchange, provider);
         await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index 3ea197395..a4ba54904 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -80,7 +80,7 @@ describe('Exchange wrappers', () => {
             artifacts.Exchange,
             provider,
             txDefaults,
-            zrxToken.address,
+            assetProxyUtils.encodeERC20AssetData(zrxToken.address),
         );
         exchangeWrapper = new ExchangeWrapper(exchange, provider);
         await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);