Remove proxyId argument from dispatchTransferFrom

2018-06-21 18:46:59 +02:00
parent 9341afe764
commit f7337c1a05
8 changed files with 20 additions and 41 deletions
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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);
    }
 }
--- 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);
--- 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,