Merge branch 'v2-prototype' into feature/combinatorial-testing

* 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, 45 insertions, 75 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 f99f03875..7960e46d2 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -1,6 +1,6 @@
 import { BlockchainLifecycle } from '@0xproject/dev-utils';
 import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils';
-import { RevertReasons } from '@0xproject/types';
+import { RevertReason } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import * as chai from 'chai';
 import { LogWithDecodedArgs } from 'ethereum-types';
@@ -62,10 +62,7 @@ describe('Asset Transfer Proxies', () => {
         erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner);
         erc721Wrapper = new ERC721Wrapper(provider, usedAddresses, owner);
 
-        [zrxToken] = await erc20Wrapper.deployDummyTokensAsync(
-            constants.NUM_DUMMY_ERC20_TO_DEPLOY,
-            constants.DUMMY_TOKEN_DECIMALS,
-        );
+        [zrxToken] = await erc20Wrapper.deployDummyTokensAsync();
         erc20Proxy = await erc20Wrapper.deployProxyAsync();
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         await web3Wrapper.awaitTransactionSuccessAsync(
@@ -103,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,
@@ -130,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,
@@ -175,27 +170,21 @@ describe('Asset Transfer Proxies', () => {
                         transferAmount,
                         { from: exchangeAddress },
                     ),
-                    RevertReasons.TransferFailed,
+                    RevertReason.TransferFailed,
                 );
             });
 
             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,
-                        },
-                    ),
-                    RevertReasons.SenderNotAuthorized,
+                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                        from: notAuthorized,
+                    }),
+                    RevertReason.SenderNotAuthorized,
                 );
             });
         });
@@ -205,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);
@@ -237,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);
@@ -249,14 +236,15 @@ describe('Asset Transfer Proxies', () => {
                     erc20Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, {
                         from: notAuthorized,
                     }),
-                    RevertReasons.SenderNotAuthorized,
+                    RevertReason.SenderNotAuthorized,
                 );
             });
         });
 
-        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);
         });
     });
 
@@ -265,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);
@@ -273,7 +260,7 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(1);
                 await web3Wrapper.awaitTransactionSuccessAsync(
                     await erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
@@ -289,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,
@@ -325,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,
@@ -361,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);
@@ -369,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,
@@ -381,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);
@@ -389,20 +372,19 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(0);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
                         { from: exchangeAddress },
                     ),
-                    RevertReasons.InvalidAmount,
+                    RevertReason.InvalidAmount,
                 );
             });
 
             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);
@@ -410,20 +392,19 @@ describe('Asset Transfer Proxies', () => {
                 const amount = new BigNumber(500);
                 return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
-                        encodedAssetDataWithoutProxyId,
+                        encodedAssetData,
                         makerAddress,
                         takerAddress,
                         amount,
                         { from: exchangeAddress },
                     ),
-                    RevertReasons.InvalidAmount,
+                    RevertReason.InvalidAmount,
                 );
             });
 
             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, {
@@ -434,34 +415,27 @@ 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,
-                        },
-                    ),
-                    RevertReasons.TransferFailed,
+                    erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+                        from: exchangeAddress,
+                    }),
+                    RevertReason.TransferFailed,
                 );
             });
 
             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,
                         { from: notAuthorized },
                     ),
-                    RevertReasons.SenderNotAuthorized,
+                    RevertReason.SenderNotAuthorized,
                 );
             });
         });
@@ -473,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);
@@ -491,8 +465,7 @@ describe('Asset Transfer Proxies', () => {
                     txHash,
                     constants.AWAIT_TRANSACTION_MINED_MS,
                 );
-                const numApproveEvents = 2;
-                expect(res.logs.length).to.equal(numTransfers + numApproveEvents);
+                expect(res.logs.length).to.equal(numTransfers);
 
                 const newOwnerMakerAssetA = await erc721Token.ownerOf.callAsync(makerTokenIdA);
                 const newOwnerMakerAssetB = await erc721Token.ownerOf.callAsync(makerTokenIdB);
@@ -517,14 +490,15 @@ describe('Asset Transfer Proxies', () => {
                     erc721Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, {
                         from: notAuthorized,
                     }),
-                    RevertReasons.SenderNotAuthorized,
+                    RevertReason.SenderNotAuthorized,
                 );
             });
         });
 
-        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 63cbc155a..52bddf996 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -88,7 +88,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 59d80c6d6..41ea88d4e 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -282,14 +282,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,
@@ -310,13 +309,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 044108bfb..7c4ad165e 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -99,7 +99,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 6f820e673..cfabe0435 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -82,7 +82,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 504b78f5c..7e3203291 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -83,7 +83,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);