Remove proxyId argument from dispatchTransferFrom

8 files changed, 20 insertions, 41 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index b8d6c0722..1d07587e9 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -19,6 +19,7 @@
 pragma solidity ^0.4.24;
 
 import "../../utils/Ownable/Ownable.sol";
+import "../../utils/LibBytes/LibBytes.sol";
 import "./libs/LibExchangeErrors.sol";
 import "./mixins/MAssetProxyDispatcher.sol";
 import "../AssetProxy/interfaces/IAssetProxy.sol";
@@ -28,6 +29,8 @@ contract MixinAssetProxyDispatcher is
     LibExchangeErrors,
     MAssetProxyDispatcher
 {
+    using LibBytes for bytes;
+    
     // Mapping from Asset Proxy Id's to their respective Asset Proxy
     mapping (uint8 => IAssetProxy) public assetProxies;
 
@@ -83,14 +86,12 @@ contract MixinAssetProxyDispatcher is
     }
 
     /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.
-    /// @param assetData Byte array encoded for the respective asset proxy.
-    /// @param assetProxyId Id of assetProxy to dispach to.
+    /// @param assetData Byte array encoded for the asset.
     /// @param from Address to transfer token from.
     /// @param to Address to transfer token to.
     /// @param amount Amount of token to transfer.
     function dispatchTransferFrom(
         bytes memory assetData,
-        uint8 assetProxyId,
         address from,
         address to,
         uint256 amount
@@ -100,6 +101,15 @@ contract MixinAssetProxyDispatcher is
         // Do nothing if no amount should be transferred.
         if (amount > 0) {
             // Lookup assetProxy
+            uint8 assetProxyId = uint8(assetData[assetData.length - 1]);
+            
+            bytes memory assetDataP = new bytes(assetData.length - 1);
+            LibBytes.memCopy(
+                assetDataP.contentAddress(),
+                assetData.contentAddress(),
+                assetDataP.length
+            );
+            
             IAssetProxy assetProxy = assetProxies[assetProxyId];
             // Ensure that assetProxy exists
             require(
@@ -108,7 +118,7 @@ contract MixinAssetProxyDispatcher is
             );
             // transferFrom will either succeed or throw.
             assetProxy.transferFrom(
-                assetData,
+                assetDataP,
                 from,
                 to,
                 amount
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index b207b3e57..38a73d885 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -20,7 +20,6 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "./libs/LibConstants.sol";
-import "../../utils/LibBytes/LibBytes.sol";
 import "./libs/LibFillResults.sol";
 import "./libs/LibOrder.sol";
 import "./libs/LibMath.sol";
@@ -41,8 +40,6 @@ contract MixinExchangeCore is
     MSignatureValidator,
     MTransactions
 {
-    using LibBytes for bytes;
-    
     // Mapping of orderHash => amount of takerAsset already bought by maker
     mapping (bytes32 => uint256) public filled;
 
@@ -412,33 +409,27 @@ contract MixinExchangeCore is
     )
         private
     {
-        uint8 makerAssetProxyId = uint8(order.makerAssetData.popLastByte());
-        uint8 takerAssetProxyId = uint8(order.takerAssetData.popLastByte());
         bytes memory zrxAssetData = ZRX_ASSET_DATA;
         dispatchTransferFrom(
             order.makerAssetData,
-            makerAssetProxyId,
             order.makerAddress,
             takerAddress,
             fillResults.makerAssetFilledAmount
         );
         dispatchTransferFrom(
             order.takerAssetData,
-            takerAssetProxyId,
             takerAddress,
             order.makerAddress,
             fillResults.takerAssetFilledAmount
         );
         dispatchTransferFrom(
             zrxAssetData,
-            ZRX_PROXY_ID,
             order.makerAddress,
             order.feeRecipientAddress,
             fillResults.makerFeePaid
         );
         dispatchTransferFrom(
             zrxAssetData,
-            ZRX_PROXY_ID,
             takerAddress,
             order.feeRecipientAddress,
             fillResults.takerFeePaid
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index e36fcc2c5..a3e8d8dde 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -15,7 +15,6 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "./libs/LibConstants.sol";
-import "../../utils/LibBytes/LibBytes.sol";
 import "./libs/LibMath.sol";
 import "./libs/LibOrder.sol";
 import "./libs/LibFillResults.sol";
@@ -34,8 +33,6 @@ contract MixinMatchOrders is
     MMatchOrders,
     MTransactions
 {
-    using LibBytes for bytes;
-
     /// @dev Match two complementary orders that have a profitable spread.
     ///      Each order is filled at their respective price point. However, the calculations are
     ///      carried out as though the orders are both being filled at the right order's price point.
@@ -242,27 +239,22 @@ contract MixinMatchOrders is
     )
         private
     {
-        uint8 leftMakerAssetProxyId = uint8(leftOrder.makerAssetData.popLastByte());
-        uint8 rightMakerAssetProxyId = uint8(rightOrder.makerAssetData.popLastByte());
         bytes memory zrxAssetData = ZRX_ASSET_DATA;
         // Order makers and taker
         dispatchTransferFrom(
             leftOrder.makerAssetData,
-            leftMakerAssetProxyId,
             leftOrder.makerAddress,
             rightOrder.makerAddress,
             matchedFillResults.right.takerAssetFilledAmount
         );
         dispatchTransferFrom(
             rightOrder.makerAssetData,
-            rightMakerAssetProxyId,
             rightOrder.makerAddress,
             leftOrder.makerAddress,
             matchedFillResults.left.takerAssetFilledAmount
         );
         dispatchTransferFrom(
             leftOrder.makerAssetData,
-            leftMakerAssetProxyId,
             leftOrder.makerAddress,
             takerAddress,
             matchedFillResults.leftMakerAssetSpreadAmount
@@ -271,14 +263,12 @@ contract MixinMatchOrders is
         // Maker fees
         dispatchTransferFrom(
             zrxAssetData,
-            ZRX_PROXY_ID,
             leftOrder.makerAddress,
             leftOrder.feeRecipientAddress,
             matchedFillResults.left.makerFeePaid
         );
         dispatchTransferFrom(
             zrxAssetData,
-            ZRX_PROXY_ID,
             rightOrder.makerAddress,
             rightOrder.feeRecipientAddress,
             matchedFillResults.right.makerFeePaid
@@ -288,7 +278,6 @@ contract MixinMatchOrders is
         if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) {
             dispatchTransferFrom(
                 zrxAssetData,
-                ZRX_PROXY_ID,
                 takerAddress,
                 leftOrder.feeRecipientAddress,
                 safeAdd(
@@ -299,14 +288,12 @@ contract MixinMatchOrders is
         } else {
             dispatchTransferFrom(
                 zrxAssetData,
-                ZRX_PROXY_ID,
                 takerAddress,
                 leftOrder.feeRecipientAddress,
                 matchedFillResults.left.takerFeePaid
             );
             dispatchTransferFrom(
                 zrxAssetData,
-                ZRX_PROXY_ID,
                 takerAddress,
                 rightOrder.feeRecipientAddress,
                 matchedFillResults.right.takerFeePaid
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
index 4a9452448..488ca956c 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
@@ -25,9 +25,6 @@ contract LibConstants {
     //        not constant to make testing easier.
     bytes public ZRX_ASSET_DATA;
 
-    // Proxy Id for ZRX token.
-    uint8 constant ZRX_PROXY_ID = 1;
-
     // @TODO: Remove when we deploy.
     constructor (bytes memory zrxAssetData)
         public
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
index 788f42c60..970082a61 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -33,14 +33,12 @@ contract MAssetProxyDispatcher is
     );
 
     /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.
-    /// @param assetData Byte array encoded for the respective asset proxy.
-    /// @param assetProxyId Id of assetProxy to dispach to.
+    /// @param assetData Byte array encoded for the asset.
     /// @param from Address to transfer token from.
     /// @param to Address to transfer token to.
     /// @param amount Amount of token to transfer.
     function dispatchTransferFrom(
         bytes memory assetData,
-        uint8 assetProxyId,
         address from,
         address to,
         uint256 amount
diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
index d469a07f0..2ae69e0ef 100644
--- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
@@ -24,12 +24,11 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol";
 contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher {
     function publicDispatchTransferFrom(
         bytes memory assetData,
-        uint8 assetProxyId,
         address from,
         address to,
         uint256 amount)
         public
     {
-        dispatchTransferFrom(assetData, assetProxyId, from, to, amount);
+        dispatchTransferFrom(assetData, from, to, amount);
     }
 }
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index ff652d3aa..5b2d4590f 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -86,7 +86,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 abbfd7ac7..f96f15f16 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -276,14 +276,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,
@@ -304,13 +303,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 expectRevertOrAlwaysFailingTransactionAsync(
                 assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
-                    encodedAssetDataWithoutProxyId,
-                    AssetProxyId.ERC20,
+                    encodedAssetData,
                     makerAddress,
                     takerAddress,
                     amount,