Move Authorizable to AssetProxy dir, rename to MixinAuthorizable

10 files changed, 97 insertions, 63 deletions

diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 48019fe8c..2c5ee872e 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -30,9 +30,9 @@
     },
     "config": {
         "abis":
-            "../migrations/src/artifacts/@(DummyERC20Token|Exchange|TokenRegistry|MultiSigWallet|MultiSigWalletWithTimeLock|MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress|TokenRegistry|ZRXToken|TestAssetProxyDispatcher|TestLibs|TestSignatureValidator|ERC20Proxy|ERC721Proxy|DummyERC721Token|TestLibBytes|Authorizable).json",
+            "../migrations/src/artifacts/@(DummyERC20Token|Exchange|TokenRegistry|MultiSigWallet|MultiSigWalletWithTimeLock|MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress|TokenRegistry|ZRXToken|TestAssetProxyDispatcher|TestLibs|TestSignatureValidator|ERC20Proxy|ERC721Proxy|DummyERC721Token|TestLibBytes|MixinAuthorizable).json",
         "contracts":
-            "Exchange,DummyERC20Token,ZRXToken,WETH9,MultiSigWallet,MultiSigWalletWithTimeLock,MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,TokenRegistry,TestAssetProxyDispatcher,TestLibs,TestSignatureValidator,ERC20Proxy,ERC721Proxy,DummyERC721Token,TestLibBytes,Authorizable"
+            "Exchange,DummyERC20Token,ZRXToken,WETH9,MultiSigWallet,MultiSigWalletWithTimeLock,MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,TokenRegistry,TestAssetProxyDispatcher,TestLibs,TestSignatureValidator,ERC20Proxy,ERC721Proxy,DummyERC721Token,TestLibBytes,MixinAuthorizable"
     },
     "repository": {
         "type": "git",
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index 5ebdb5095..c02536d67 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -22,10 +22,12 @@ pragma experimental ABIEncoderV2;
 import "../../utils/LibBytes/LibBytes.sol";
 import "../../tokens/ERC20Token/IERC20Token.sol";
 import "./MixinAssetProxy.sol";
+import "./MixinAuthorizable.sol";
 
 contract ERC20Proxy is
     LibBytes,
-    MixinAssetProxy
+    MixinAssetProxy,
+    MixinAuthorizable
 {
 
     // Id of this proxy.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index cc1a60f2e..475359087 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -22,10 +22,12 @@ pragma experimental ABIEncoderV2;
 import "../../utils/LibBytes/LibBytes.sol";
 import "../../tokens/ERC721Token/ERC721Token.sol";
 import "./MixinAssetProxy.sol";
+import "./MixinAuthorizable.sol";
 
 contract ERC721Proxy is
     LibBytes,
-    MixinAssetProxy
+    MixinAssetProxy,
+    MixinAuthorizable
 {
 
     // Id of this proxy.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
index 0535559ae..d58cfc2dd 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
@@ -20,10 +20,10 @@ pragma solidity ^0.4.23;
 pragma experimental ABIEncoderV2;
 
 import "./mixins/MAssetProxy.sol";
-import "../../utils/Authorizable/Authorizable.sol";
+import "./mixins/MAuthorizable.sol";
 
 contract MixinAssetProxy is 
-    Authorizable,
+    MAuthorizable,
     MAssetProxy
 {
 
diff --git a/packages/contracts/src/contracts/current/utils/Authorizable/Authorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 6b84d7aca..b66b783ea 100644
--- a/packages/contracts/src/contracts/current/utils/Authorizable/Authorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -19,35 +19,26 @@
 pragma solidity ^0.4.23;
 pragma experimental ABIEncoderV2;
 
-import "./IAuthorizable.sol";
-import "../Ownable/Ownable.sol";
+import "./mixins/MAuthorizable.sol";
+import "../../utils/Ownable/Ownable.sol";
 
-contract Authorizable is
+contract MixinAuthorizable is
     Ownable,
-    IAuthorizable
+    MAuthorizable
 {
 
+    // Revert reasons
+    string constant SENDER_NOT_AUTHORIZED = "Sender not authorized to call this method.";
+    string constant TARGET_NOT_AUTHORIZED = "Target address must be authorized.";
+    string constant TARGET_ALREADY_AUTHORIZED = "Target must not already be authorized.";
+    string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
+    string constant INDEX_ADDRESS_MISMATCH = "Address found at index does not match target address.";
+
     /// @dev Only authorized addresses can invoke functions with this modifier.
     modifier onlyAuthorized {
         require(
             authorized[msg.sender],
-            "Sender not authorized to call this method."
-        );
-        _;
-    }
-
-    modifier targetAuthorized(address target) {
-        require(
-            authorized[target],
-            "Target address not authorized to call this method."
-        );
-        _;
-    }
-
-    modifier targetNotAuthorized(address target) {
-        require(
-            !authorized[target],
-            "Target must not already be authorized to call this method."
+            SENDER_NOT_AUTHORIZED
         );
         _;
     }
@@ -55,17 +46,17 @@ contract Authorizable is
     mapping (address => bool) public authorized;
     address[] public authorities;
 
-    /*
-     * Public functions
-     */
-
     /// @dev Authorizes an address.
     /// @param target Address to authorize.
     function addAuthorizedAddress(address target)
-        public
+        external
         onlyOwner
-        targetNotAuthorized(target)
     {
+        require(
+            !authorized[target],
+            TARGET_ALREADY_AUTHORIZED
+        );
+
         authorized[target] = true;
         authorities.push(target);
         emit AuthorizedAddressAdded(target, msg.sender);
@@ -74,10 +65,14 @@ contract Authorizable is
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     function removeAuthorizedAddress(address target)
-        public
+        external
         onlyOwner
-        targetAuthorized(target)
     {
+        require(
+            authorized[target],
+            TARGET_NOT_AUTHORIZED
+        );
+
         delete authorized[target];
         for (uint i = 0; i < authorities.length; i++) {
             if (authorities[i] == target) {
@@ -93,30 +88,28 @@ contract Authorizable is
     /// @param target Address to remove authorization from.
     /// @param index Index of target in authorities array.
     function removeAuthorizedAddressAtIndex(address target, uint256 index)
-        public
+        external
     {
         require(
             index < authorities.length,
-            "Specified index is out of bounds."
+            INDEX_OUT_OF_BOUNDS
         );
         require(
             authorities[index] == target,
-            "Address found at index does not match target address."
+            INDEX_ADDRESS_MISMATCH
         );
+
         delete authorized[target];
         authorities[index] = authorities[authorities.length - 1];
         authorities.length -= 1;
         emit AuthorizedAddressRemoved(target, msg.sender);
     }
 
-    /*
-     * Public constant functions
-     */
-
     /// @dev Gets all authorized addresses.
     /// @return Array of authorized addresses.
     function getAuthorizedAddresses()
-        public view
+        external
+        view
         returns (address[] memory)
     {
         return authorities;
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 1cdace0b5..eca6524e5 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -19,7 +19,7 @@
 pragma solidity ^0.4.23;
 pragma experimental ABIEncoderV2;
 
-import "../../../utils/Authorizable/IAuthorizable.sol";
+import "./IAuthorizable.sol";
 
 contract IAssetProxy is
     IAuthorizable
diff --git a/packages/contracts/src/contracts/current/utils/Authorizable/IAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
index b14cfba22..3120be7ec 100644
--- a/packages/contracts/src/contracts/current/utils/Authorizable/IAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
@@ -19,37 +19,32 @@
 pragma solidity ^0.4.23;
 pragma experimental ABIEncoderV2;
 
-contract IAuthorizable {
+import "../../../utils/Ownable/IOwnable.sol";
+
+contract IAuthorizable is
+    IOwnable
+{
 
     /// @dev Gets all authorized addresses.
     /// @return Array of authorized addresses.
     function getAuthorizedAddresses()
-        public view
+        external
+        view
         returns (address[]);
 
     /// @dev Authorizes an address.
     /// @param target Address to authorize.
     function addAuthorizedAddress(address target)
-        public;
+        external;
 
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     function removeAuthorizedAddress(address target)
-        public;
+        external;
 
     /// @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)
-        public;
-
-    event AuthorizedAddressAdded(
-        address indexed target,
-        address indexed caller
-    );
-
-    event AuthorizedAddressRemoved(
-        address indexed target,
-        address indexed caller
-    );
+        external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
new file mode 100644
index 000000000..71d1910e5
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
@@ -0,0 +1,42 @@
+/*
+
+  Copyright 2018 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.4.23;
+pragma experimental ABIEncoderV2;
+
+import "../interfaces/IAuthorizable.sol";
+
+contract MAuthorizable is
+    IAuthorizable
+{
+
+    // Event logged when a new address is authorized.
+    event AuthorizedAddressAdded(
+        address indexed target,
+        address indexed caller
+    );
+
+    // Event logged when a currently authorized address is unauthorized.
+    event AuthorizedAddressRemoved(
+        address indexed target,
+        address indexed caller
+    );
+
+    /// @dev Only authorized addresses can invoke functions with this modifier.
+    modifier onlyAuthorized { _; }
+}
diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
index e9b4d6a3b..933aa168a 100644
--- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
+++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
@@ -8,7 +8,7 @@ pragma experimental ABIEncoderV2;
  * Provides onlyOwner modifier, which prevents function from running if it is called by anyone other than the owner.
  */
 
-import "../Ownable/IOwnable.sol";
+import "./IOwnable.sol";
 
 contract Ownable is IOwnable {
     address public owner;
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index cc4f78a31..9cea263e7 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -3,7 +3,7 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper';
 import * as chai from 'chai';
 import * as Web3 from 'web3';
 
-import { AuthorizableContract } from '../../src/contract_wrappers/generated/authorizable';
+import { MixinAuthorizableContract } from '../../src/contract_wrappers/generated/mixin_authorizable';
 import { constants } from '../../src/utils/constants';
 import { ContractName } from '../../src/utils/types';
 import { chaiSetup } from '../utils/chai_setup';
@@ -18,13 +18,13 @@ describe('Authorizable', () => {
     let owner: string;
     let notOwner: string;
     let address: string;
-    let authorizable: AuthorizableContract;
+    let authorizable: MixinAuthorizableContract;
     before(async () => {
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         owner = address = accounts[0];
         notOwner = accounts[1];
         const authorizableInstance = await deployer.deployAsync(ContractName.Authorizable);
-        authorizable = new AuthorizableContract(authorizableInstance.abi, authorizableInstance.address, provider);
+        authorizable = new MixinAuthorizableContract(authorizableInstance.abi, authorizableInstance.address, provider);
     });
     beforeEach(async () => {
         await blockchainLifecycle.startAsync();