Address feedback, rename contract to AssetProxyOwner

6 files changed, 37 insertions, 33 deletions

diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index c36d991e1..e4e5b131d 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -28,7 +28,7 @@
         "test:circleci": "yarn test"
     },
     "config": {
-        "abis": "../migrations/artifacts/2.0.0/@(DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|WETH9|ZRXToken).json"
+        "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock||TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|WETH9|ZRXToken).json"
     },
     "repository": {
         "type": "git",
diff --git a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol
index cc5808813..1b0f9c34f 100644
--- a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol
@@ -18,16 +18,19 @@
 
 pragma solidity ^0.4.10;
 
-import "../MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol";
+import "../../multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol";
 
-contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is
+contract AssetProxyOwner is
     MultiSigWalletWithTimeLock
 {
+
     event AssetProxyRegistration(address assetProxyContract, bool isRegistered);
 
     // Mapping of AssetProxy contract address => approved to execute removeAuthorizedAddress without time lock.
     mapping (address => bool) public isAssetProxyRegistered;
 
+    bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)"));
+
     modifier validRemoveAuthorizedAddressTx(uint256 transactionId) {
         Transaction storage tx = transactions[transactionId];
         require(isAssetProxyRegistered[tx.destination]);
@@ -35,12 +38,13 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is
         _;
     }
 
-    /// @dev Contract constructor sets initial owners, required number of confirmations, time lock, and tokenTransferProxy address.
+    /// @dev Contract constructor sets initial owners, required number of confirmations,
+    ///      time lock, and list of AssetProxy addresses.
     /// @param _owners List of initial owners.
     /// @param _required Number of required confirmations.
     /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
     /// @param _assetProxyContracts Array of AssetProxy contract addresses.
-    function MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress(
+    function AssetProxyOwner(
         address[] memory _owners,
         uint256 _required,
         uint256 _secondsTimeLocked,
@@ -54,7 +58,8 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is
         }
     }
 
-    /// @dev Sets approval for calling removeAuthorizedAddress on an AssetProxy contract without a timelock.
+    /// @dev Registers or deregisters an AssetProxy to be able to execute
+    ///      removeAuthorizedAddress without a timelock.
     /// @param assetProxyContract Address of AssetProxy contract.
     /// @param isRegistered Status of approval for AssetProxy contract.
     function registerAssetProxy(address assetProxyContract, bool isRegistered)
@@ -92,12 +97,14 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is
         pure
         returns (bool)
     {
-        bytes4 removeAuthorizedAddressSelector = bytes4(keccak256("removeAuthorizedAddress(address)"));
         bytes4 first4Bytes = readFirst4(data);
-        require(removeAuthorizedAddressSelector == first4Bytes);
+        require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes);
         return true;
     }
 
+    /// @dev Reads the first 4 bytes from a byte array of arbitrary length.
+    /// @param data Byte array to read first 4 bytes from.
+    /// @return First 4 bytes of data.
     function readFirst4(bytes memory data)
         public
         pure
diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts
index caf5b94fd..fe74ea072 100644
--- a/packages/contracts/src/utils/artifacts.ts
+++ b/packages/contracts/src/utils/artifacts.ts
@@ -1,5 +1,6 @@
 import { ContractArtifact } from '@0xproject/sol-compiler';
 
+import * as AssetProxyOwner from '../artifacts/AssetProxyOwner.json';
 import * as DummyERC20Token from '../artifacts/DummyERC20Token.json';
 import * as DummyERC721Token from '../artifacts/DummyERC721Token.json';
 import * as ERC20Proxy from '../artifacts/ERC20Proxy.json';
@@ -8,7 +9,6 @@ import * as Exchange from '../artifacts/Exchange.json';
 import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json';
 import * as MultiSigWallet from '../artifacts/MultiSigWallet.json';
 import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json';
-import * as MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress from '../artifacts/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.json';
 import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json';
 import * as TestLibBytes from '../artifacts/TestLibBytes.json';
 import * as TestLibs from '../artifacts/TestLibs.json';
@@ -18,6 +18,7 @@ import * as EtherToken from '../artifacts/WETH9.json';
 import * as ZRX from '../artifacts/ZRXToken.json';
 
 export const artifacts = {
+    AssetProxyOwner: (AssetProxyOwner as any) as ContractArtifact,
     DummyERC20Token: (DummyERC20Token as any) as ContractArtifact,
     DummyERC721Token: (DummyERC721Token as any) as ContractArtifact,
     ERC20Proxy: (ERC20Proxy as any) as ContractArtifact,
@@ -27,7 +28,6 @@ export const artifacts = {
     MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact,
     MultiSigWallet: (MultiSigWallet as any) as ContractArtifact,
     MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact,
-    MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress: (MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress as any) as ContractArtifact,
     TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact,
     TestLibBytes: (TestLibBytes as any) as ContractArtifact,
     TestLibs: (TestLibs as any) as ContractArtifact,
diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts
index 9a2b26d77..c33e7bb47 100644
--- a/packages/contracts/src/utils/multi_sig_wrapper.ts
+++ b/packages/contracts/src/utils/multi_sig_wrapper.ts
@@ -3,8 +3,8 @@ import { BigNumber } from '@0xproject/utils';
 import * as _ from 'lodash';
 import * as Web3 from 'web3';
 
+import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_proxy_owner';
 import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet';
-import { MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract } from '../contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address';
 
 import { constants } from './constants';
 import { LogDecoder } from './log_decoder';
@@ -45,10 +45,7 @@ export class MultiSigWrapper {
         from: string,
     ): Promise<TransactionReceiptWithDecodedLogs> {
         const txHash = await (this
-            ._multiSig as MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract).executeRemoveAuthorizedAddress.sendTransactionAsync(
-            txId,
-            { from },
-        );
+            ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from });
         const tx = await this._getTxWithDecodedMultiSigLogs(txHash);
         return tx;
     }
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index ef86b4f38..05cd7ec16 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -99,7 +99,7 @@ export enum ContractName {
     ZRXToken = 'ZRXToken',
     DummyERC20Token = 'DummyERC20Token',
     EtherToken = 'WETH9',
-    MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress = 'MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress',
+    AssetProxyOwner = 'AssetProxyOwner',
     AccountLevels = 'AccountLevels',
     EtherDelta = 'EtherDelta',
     Arbitrage = 'Arbitrage',
diff --git a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts b/packages/contracts/test/asset_proxy_owner.ts
index e384e093b..7b470e82e 100644
--- a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts
+++ b/packages/contracts/test/asset_proxy_owner.ts
@@ -6,14 +6,14 @@ import * as _ from 'lodash';
 import 'make-promises-safe';
 import * as Web3 from 'web3';
 
-import { AuthorizableContract } from '../src/contract_wrappers/generated/authorizable';
 import {
+    AssetProxyOwnerContract,
     AssetProxyRegistrationContractEventArgs,
     ExecutionContractEventArgs,
     ExecutionFailureContractEventArgs,
-    MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract,
     SubmissionContractEventArgs,
-} from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address';
+} from '../src/contract_wrappers/generated/asset_proxy_owner';
+import { MixinAuthorizableContract } from '../src/contract_wrappers/generated/mixin_authorizable';
 import { artifacts } from '../src/utils/artifacts';
 import { chaiSetup } from '../src/utils/chai_setup';
 import { constants } from '../src/utils/constants';
@@ -25,37 +25,37 @@ const expect = chai.expect;
 const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
 const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID });
 
-describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => {
+describe('AssetProxyOwner', () => {
     let owners: string[];
     let authorized: string;
     const requiredApprovals = new BigNumber(2);
     const SECONDS_TIME_LOCKED = new BigNumber(1000000);
 
-    let erc20Proxy: AuthorizableContract;
-    let erc721Proxy: AuthorizableContract;
-    let multiSig: MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract;
+    let erc20Proxy: MixinAuthorizableContract;
+    let erc721Proxy: MixinAuthorizableContract;
+    let multiSig: AssetProxyOwnerContract;
     let multiSigWrapper: MultiSigWrapper;
 
     before(async () => {
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         owners = [accounts[0], accounts[1]];
         const initialOwner = (authorized = accounts[0]);
-        erc20Proxy = await AuthorizableContract.deployFrom0xArtifactAsync(artifacts.Authorizable, provider, txDefaults);
-        erc721Proxy = await AuthorizableContract.deployFrom0xArtifactAsync(
-            artifacts.Authorizable,
+        erc20Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync(artifacts.MixinAuthorizable, provider, txDefaults);
+        erc721Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync(
+            artifacts.MixinAuthorizable,
             provider,
             txDefaults,
         );
         const defaultAssetProxyContractAddresses: string[] = [];
-        multiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync(
-            artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,
+        multiSig = await AssetProxyOwnerContract.deployFrom0xArtifactAsync(
+            artifacts.AssetProxyOwner,
             provider,
             txDefaults,
             owners,
             requiredApprovals,
             SECONDS_TIME_LOCKED,
             defaultAssetProxyContractAddresses,
-        );
+        ]);
         multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx);
         await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner });
         await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner });
@@ -70,8 +70,8 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => {
     describe('constructor', () => {
         it('should register passed in assetProxyContracts', async () => {
             const assetProxyContractAddresses = [erc20Proxy.address, erc721Proxy.address];
-            const newMultiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync(
-                artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,
+            const newMultiSig = await AssetProxyOwnerContract.deployFrom0xArtifactAsync(
+                artifacts.AssetProxyOwner,
                 provider,
                 txDefaults,
                 owners,
@@ -87,8 +87,8 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => {
         it('should throw if a null address is included in assetProxyContracts', async () => {
             const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS];
             expect(
-                MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync(
-                    artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,
+                AssetProxyOwnerContract.deployFrom0xArtifactAsync(
+                    artifacts.AssetProxyOwner,
                     provider,
                     txDefaults,
                     owners,