Make AssetProxyId last byte of assetData

18 files changed, 74 insertions, 37 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index ee0c66fdc..7c25c9770 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -47,21 +47,23 @@ contract ERC20Proxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
         require(
-            uint8(assetMetadata[0]) == PROXY_ID,
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
         );
 
         // Decode metadata.
         require(
-            assetMetadata.length == 21,
+            length == 21,
             INVALID_METADATA_LENGTH
         );
-        address token = readAddress(assetMetadata, 1);
+        address token = readAddress(assetMetadata, 0);
 
         // Transfer tokens.
         bool success = IERC20Token(token).transferFrom(from, to, amount);
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 94aab9139..59045f73a 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -47,12 +47,14 @@ contract ERC721Proxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
         require(
-            uint8(assetMetadata[0]) == PROXY_ID,
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
         );
 
@@ -64,11 +66,11 @@ contract ERC721Proxy is
 
         // Decode metadata
         require(
-            assetMetadata.length == 53,
+            length == 53,
             INVALID_METADATA_LENGTH
         );
-        address token = readAddress(assetMetadata, 1);
-        uint256 tokenId = readUint256(assetMetadata, 21);
+        address token = readAddress(assetMetadata, 0);
+        uint256 tokenId = readUint256(assetMetadata, 20);
 
         // Transfer token.
         // Either succeeds or throws.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
index 4ec31304f..4bd533148 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
@@ -36,7 +36,8 @@ contract MixinAssetProxy is
         bytes assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         external
         onlyAuthorized
     {
@@ -57,7 +58,8 @@ contract MixinAssetProxy is
         bytes[] memory assetMetadata,
         address[] memory from,
         address[] memory to,
-        uint256[] memory amounts)
+        uint256[] memory amounts
+    )
         public
         onlyAuthorized
     {
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 0bbd3b318..4aaeb66d1 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -87,7 +87,10 @@ contract MixinAuthorizable is
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     /// @param index Index of target in authorities array.
-    function removeAuthorizedAddressAtIndex(address target, uint256 index)
+    function removeAuthorizedAddressAtIndex(
+        address target,
+        uint256 index
+    )
         external
     {
         require(
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
index 8b30dfabb..7e1848889 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -34,7 +34,8 @@ contract IAssetProxy is
         bytes assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         external;
     
     /// @dev Makes multiple transfers of assets. Either succeeds or throws.
@@ -46,7 +47,8 @@ contract IAssetProxy is
         bytes[] memory assetMetadata,
         address[] memory from,
         address[] memory to,
-        uint256[] memory amounts)
+        uint256[] memory amounts
+    )
         public;
 
     /// @dev Gets the proxy id associated with the proxy address.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
index d6fe03898..cedd1744c 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
@@ -45,6 +45,9 @@ contract IAuthorizable is
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     /// @param index Index of target in authorities array.
-    function removeAuthorizedAddressAtIndex(address target, uint256 index)
+    function removeAuthorizedAddressAtIndex(
+        address target,
+        uint256 index
+    )
         external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
index 3800bf04c..de9d65a53 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
@@ -34,6 +34,7 @@ contract MAssetProxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
index cdf60bdee..6f35bd7ec 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
@@ -38,5 +38,5 @@ contract MAuthorizable is
     );
 
     /// @dev Only authorized addresses can invoke functions with this modifier.
-    modifier onlyAuthorized { _; }
+    modifier onlyAuthorized { revert(); _; }
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index 3b38d1f37..d996f933c 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -39,7 +39,8 @@ contract MixinAssetProxyDispatcher is
     function registerAssetProxy(
         uint8 assetProxyId,
         address newAssetProxy,
-        address oldAssetProxy)
+        address oldAssetProxy
+    )
         external
         onlyOwner
     {
@@ -86,17 +87,20 @@ contract MixinAssetProxyDispatcher is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Do nothing if no amount should be transferred.
         if (amount > 0) {
+
             // Lookup asset proxy
+            uint256 length = assetMetadata.length;
             require(
-                assetMetadata.length >= 1,
+                length > 0,
                 GT_ZERO_LENGTH_REQUIRED
             );
-            uint8 assetProxyId = uint8(assetMetadata[0]);
+            uint8 assetProxyId = uint8(assetMetadata[length - 1]);
             IAssetProxy assetProxy = assetProxies[assetProxyId];
 
             // transferFrom will either succeed or throw.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index e2d4808c3..96a1f578a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -105,7 +105,7 @@ contract MixinSignatureValidator is
         // also the initialization value for the enum type.
         if (signatureType == SignatureType.Illegal) {
             // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051
-            revert("Illegal signature type.");
+            revert(ILLEGAL_SIGNATURE_TYPE);
 
         // Always invalid signature.
         // Like Illegal, this is always implicitly available and therefore
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
index 65ff45f7b..72f15a2a4 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
@@ -27,6 +27,16 @@ contract ISignatureValidator {
     function preSign(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
+        external;
+    
+    /// @dev Approves a Validator contract to verify signatures on signer's behalf.
+    /// @param validator Address of Validator contract.
+    /// @param approval Approval or disapproval of  Validator contract.
+    function approveSignatureValidator(
+        address validator,
+        bool approval
+    )
         external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
index 53c41d331..0186ad490 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
@@ -26,7 +26,8 @@ contract ISigner {
     /// @return Validity of order signature.
     function isValidSignature(
         bytes32 hash,
-        bytes signature)
+        bytes signature
+    )
         external
         view
         returns (bool isValid);
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
index aed725066..3e5ccc190 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
@@ -28,7 +28,8 @@ contract IValidator {
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
         external
         view
         returns (bool isValid);
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 ccc960d6e..87c5f6361 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -41,6 +41,7 @@ contract MAssetProxyDispatcher is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
index 57600f508..dc08c6d27 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -44,7 +44,8 @@ contract MSignatureValidator is
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes memory signature)
+        bytes memory signature
+    )
         internal
         view
         returns (bool isValid);
diff --git a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
index 15d9ca189..e2bc75b37 100644
--- a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
@@ -26,7 +26,8 @@ contract TestSignatureValidator is MixinSignatureValidator {
     function publicIsValidSignature(
         bytes32 hash,
         address signer,
-        bytes memory signature)
+        bytes memory signature
+    )
         public
         view
         returns (bool isValid)
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 034222915..8b9763c65 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -50,7 +50,10 @@ contract Whitelist is
     /// @dev Adds or removes an address from the whitelist.
     /// @param target Address to add or remove from whitelist.
     /// @param isApproved Whitelist status to assign to address.
-    function updateWhitelistStatus(address target, bool isApproved)
+    function updateWhitelistStatus(
+        address target,
+        bool isApproved
+    )
         external
         onlyOwner
     {
diff --git a/packages/contracts/src/utils/asset_proxy_utils.ts b/packages/contracts/src/utils/asset_proxy_utils.ts
index c042da5d0..a17d4cdfa 100644
--- a/packages/contracts/src/utils/asset_proxy_utils.ts
+++ b/packages/contracts/src/utils/asset_proxy_utils.ts
@@ -39,7 +39,7 @@ export const assetProxyUtils = {
     encodeERC20ProxyData(tokenAddress: string): string {
         const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC20);
         const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
-        const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress]);
+        const encodedMetadata = Buffer.concat([encodedAddress, encodedAssetProxyId]);
         const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata);
         return encodedMetadataHex;
     },
@@ -52,7 +52,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         if (assetProxyId !== AssetProxyId.ERC20) {
             throw new Error(
@@ -61,7 +61,7 @@ export const assetProxyUtils = {
                 }), but got ${assetProxyId}`,
             );
         }
-        const encodedTokenAddress = encodedProxyMetadata.slice(1, 21);
+        const encodedTokenAddress = encodedProxyMetadata.slice(0, 20);
         const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
         const erc20ProxyData = {
             assetProxyId,
@@ -73,7 +73,7 @@ export const assetProxyUtils = {
         const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721);
         const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
         const encodedTokenId = assetProxyUtils.encodeUint256(tokenId);
-        const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress, encodedTokenId]);
+        const encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId, encodedAssetProxyId]);
         const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata);
         return encodedMetadataHex;
     },
@@ -86,7 +86,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         if (assetProxyId !== AssetProxyId.ERC721) {
             throw new Error(
@@ -95,9 +95,9 @@ export const assetProxyUtils = {
                 }), but got ${assetProxyId}`,
             );
         }
-        const encodedTokenAddress = encodedProxyMetadata.slice(1, 21);
+        const encodedTokenAddress = encodedProxyMetadata.slice(0, 20);
         const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
-        const encodedTokenId = encodedProxyMetadata.slice(21, 53);
+        const encodedTokenId = encodedProxyMetadata.slice(20, 52);
         const tokenId = assetProxyUtils.decodeUint256(encodedTokenId);
         const erc721ProxyData = {
             assetProxyId,
@@ -115,7 +115,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         return assetProxyId;
     },