Remove authorizable from unnecessary contracts (#233)

* Remove authorizable from unnecessary contracts

contracts/core/exchange-wrappers/KyberNetworkWrapper.sol

@@ -18,7 +18,6 @@ pragma solidity 0.4.24;
 pragma experimental "ABIEncoderV2";
 
 import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
-import { Authorizable } from "../../lib/Authorizable.sol";
 import { ERC20Wrapper as ERC20 } from "../../lib/ERC20Wrapper.sol";
 import { KyberNetworkProxyInterface } from "../../external/KyberNetwork/KyberNetworkProxyInterface.sol";
 import { LibBytes } from "../../external/0x/LibBytes.sol";
@@ -30,14 +29,13 @@ import { LibBytes } from "../../external/0x/LibBytes.sol";
  *
  * The KyberNetworkWrapper contract wrapper to interface with KyberNetwork for reserve liquidity
  */
-contract KyberNetworkWrapper is
-    Authorizable
-{
+contract KyberNetworkWrapper {
     using LibBytes for bytes;
     using SafeMath for uint256;
 
     /* ============ State Variables ============ */
 
+    address public core;
     address public kyberNetworkProxy;
     address public setTransferProxy;
 
@@ -56,16 +54,18 @@ contract KyberNetworkWrapper is
     /**
      * Initialize exchange wrapper with required addresses to facilitate Kyber trades
      *
+     * @param  _core                 Authorized Core contract that sends Kyber trades
      * @param  _kyberNetworkProxy    KyberNetwork contract for filling orders
      * @param  _setTransferProxy     Set Protocol transfer proxy contract
      */
     constructor(
+        address _core,
         address _kyberNetworkProxy,
         address _setTransferProxy
     )
         public
-        Authorizable(2592000) // about 4 weeks
     {
+        core = _core;
         kyberNetworkProxy = _kyberNetworkProxy;
         setTransferProxy = _setTransferProxy;
     }
@@ -129,9 +129,10 @@ contract KyberNetworkWrapper is
         bytes _tradesData
     )
         external
-        onlyAuthorized
         returns (address[], uint256[])
     {
+        require(msg.sender == core, "ONLY_CORE_CAN_EXCHANGE_KYBER");
+
         // Ensure the issuance order maker token is allowed to be transferred by KyberNetworkProxy as the source token
         ERC20.ensureAllowance(
             _makerToken,

contracts/core/exchange-wrappers/TakerWalletWrapper.sol

@@ -18,7 +18,6 @@ pragma solidity 0.4.24;
 pragma experimental "ABIEncoderV2";
 
 import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
-import { Authorizable } from "../../lib/Authorizable.sol";
 import { ERC20Wrapper } from "../../lib/ERC20Wrapper.sol";
 import { ITransferProxy } from "../interfaces/ITransferProxy.sol";
 import { LibBytes } from "../../external/0x/LibBytes.sol";
@@ -30,30 +29,30 @@ import { LibBytes } from "../../external/0x/LibBytes.sol";
  *
  * The TakerWalletWrapper contract wrapper to transfer tokens directly from order taker
  */
-contract TakerWalletWrapper is
-    Authorizable
-{
+contract TakerWalletWrapper {
     using LibBytes for bytes;
     using SafeMath for uint256;
 
     /* ============ State Variables ============ */
 
+    address public core;
     address public transferProxy;
 
     /* ============ Constructor ============ */
 
     /**
      * Sets the transferProxy address for the contract
      *
-     * @param  _transferProxy    Address of current transferProxy
+     * @param  _core            Authorized Core contract that sends Taker transfers orders
+     * @param _transferProxy    Address of current transferProxy
      */
     constructor(
+        address _core,
         address _transferProxy
     )
         public
-        Authorizable(2592000) // about 4 weeks
     {
-        // Set transferProxy address
+        core = _core;
         transferProxy = _transferProxy;
     }
 
@@ -81,9 +80,10 @@ contract TakerWalletWrapper is
         bytes _transfersData
     )
         public
-        onlyAuthorized
         returns(address[], uint256[])
     {
+        require(msg.sender == core, "ONLY_CORE_CAN_EXCHANGE_TAKER");
+
         address[] memory takerTokens = new address[](_orderCount);
         uint256[] memory takerTokenAmounts = new uint256[](_orderCount);
 

contracts/core/exchange-wrappers/ZeroExExchangeWrapper.sol

@@ -17,7 +17,6 @@
 pragma solidity 0.4.24;
 pragma experimental "ABIEncoderV2";
 
-import { Authorizable } from "../../lib/Authorizable.sol";
 import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
 import { CommonMath } from "../../lib/CommonMath.sol";
 import { ERC20Wrapper as ERC20 } from "../../lib/ERC20Wrapper.sol";
@@ -35,14 +34,13 @@ import { ZeroExOrderDataHandler as OrderHandler } from "./lib/ZeroExOrderDataHan
  *
  * The ZeroExExchangeWrapper contract wrapper to interface with 0x V2
  */
-contract ZeroExExchangeWrapper is
-    Authorizable
-{
+contract ZeroExExchangeWrapper {
     using LibBytes for bytes;
     using SafeMath for uint256;
 
     /* ============ State Variables ============ */
 
+    address public core;
     address public zeroExExchange;
     address public zeroExProxy;
     address public zeroExToken;
@@ -53,19 +51,22 @@ contract ZeroExExchangeWrapper is
     /**
      * Initialize exchange wrapper with required addresses to facilitate 0x orders
      *
-     * @param  _zeroExExchange     0x Exchange contract for filling orders
-     * @param  _zeroExProxy        0x Proxy contract for transferring
-     * @param  _setTransferProxy   Set Protocol transfer proxy contract
+     * @param _core               Authorized Core contract that sends 0x orders
+     * @param _zeroExExchange     0x Exchange contract for filling orders
+     * @param _zeroExProxy        0x Proxy contract for transferring
+     * @param _zeroExToken        ZRX token contract addressed used for 0x relayer fees
+     * @param _setTransferProxy   Set Protocol transfer proxy contract
      */
     constructor(
+        address _core,
         address _zeroExExchange,
         address _zeroExProxy,
         address _zeroExToken,
         address _setTransferProxy
     )
         public
-        Authorizable(2592000) // about 4 weeks
     {
+        core = _core;
         zeroExExchange = _zeroExExchange;
         zeroExProxy = _zeroExProxy;
         zeroExToken = _zeroExToken;
@@ -105,9 +106,10 @@ contract ZeroExExchangeWrapper is
         bytes _ordersData
     )
         external
-        onlyAuthorized
         returns (address[], uint256[])
     {
+        require(msg.sender == core, "ONLY_CORE_CAN_EXCHANGE_0X");
+
         // Ensure the taker token is allowed to be transferred by ZeroEx Proxy
         ERC20.ensureAllowance(
             _makerToken,

migrations/2_core.js

@@ -86,12 +86,13 @@ async function deployCoreContracts(deployer, network) {
   }
 
   // Taker Wallet Wrapper
-  await deployer.deploy(TakerWalletWrapper, TransferProxy.address);
+  await deployer.deploy(TakerWalletWrapper, Core.address, TransferProxy.address);
 
   // Kyber Wrapper
   if (kyberNetworkProxyAddress) {
     await deployer.deploy(
       KyberNetworkWrapper,
+      Core.address,
       kyberNetworkProxyAddress,
       TransferProxy.address
     );
@@ -101,6 +102,7 @@ async function deployCoreContracts(deployer, network) {
   if (zeroExExchangeAddress && zeroExERC20ProxyAddress) {
     await deployer.deploy(
       ZeroExExchangeWrapper,
+      Core.address,
       zeroExExchangeAddress,
       zeroExERC20ProxyAddress,
       TransferProxy.address
@@ -133,14 +135,11 @@ async function addAuthorizations(deployer, network) {
   if (network === 'kovan' || network === 'development') {
     await core.registerExchange(EXCHANGES.ZERO_EX, ZeroExExchangeWrapper.address);
     const zeroExExchangeWrapper = await ZeroExExchangeWrapper.deployed();
-    await zeroExExchangeWrapper.addAuthorizedAddress(Core.address);
   };
 
   await core.registerExchange(EXCHANGES.KYBER, KyberNetworkWrapper.address);
   const kyberNetworkWrapper = await KyberNetworkWrapper.deployed();  
-  await kyberNetworkWrapper.addAuthorizedAddress(Core.address);
 
   await core.registerExchange(EXCHANGES.TAKER_WALLET, TakerWalletWrapper.address);
   const takerWalletWrapper = await TakerWalletWrapper.deployed();
-  await takerWalletWrapper.addAuthorizedAddress(Core.address);
 };

test/core/exchange-wrappers/kyberNetworkWrapper.spec.ts

@@ -28,7 +28,7 @@ const blockchain = new Blockchain(web3);
 contract('KyberNetworkWrapper', accounts => {
   const [
     deployerAccount,
-    authorizedAddress,
+    deployedCoreAddress,
     unauthorizedAddress,
     issuanceOrderMakerAccount,
     takerAccount,
@@ -45,12 +45,11 @@ contract('KyberNetworkWrapper', accounts => {
     await blockchain.saveSnapshotAsync();
 
     transferProxy = await coreWrapper.deployTransferProxyAsync();
-
     kyberNetworkWrapper = await exchangeWrapper.deployKyberNetworkWrapper(
+      deployedCoreAddress,
       SetTestUtils.KYBER_NETWORK_PROXY_ADDRESS,
       transferProxy
     );
-    await coreWrapper.addAuthorizationAsync(kyberNetworkWrapper, authorizedAddress);
   });
 
   afterEach(async () => {
@@ -70,7 +69,7 @@ contract('KyberNetworkWrapper', accounts => {
       makerToken = erc20Wrapper.kyberReserveToken(SetTestUtils.KYBER_RESERVE_SOURCE_TOKEN_ADDRESS);
       componentToken = erc20Wrapper.kyberReserveToken(SetTestUtils.KYBER_RESERVE_DESTINATION_TOKEN_ADDRESS);
 
-      subjectCaller = authorizedAddress;
+      subjectCaller = deployedCoreAddress;
       subjectMakerToken = makerToken.address;
       subjectComponentToken = componentToken.address;
       subjectQuantity = ether(5);
@@ -144,7 +143,7 @@ contract('KyberNetworkWrapper', accounts => {
         maxDestinationQuantity: maxDestinationQuantity,
       } as KyberTrade;
 
-      subjectCaller = authorizedAddress;
+      subjectCaller = deployedCoreAddress;
       subjectMaker = issuanceOrderMakerAccount;
       subjectTaker = takerAccount;
       subjectMakerTokenAddress = sourceToken.address;
@@ -197,7 +196,7 @@ contract('KyberNetworkWrapper', accounts => {
       expect(newBalance).to.be.bignumber.equal(expectedNewAllowance);
     });
 
-    describe('when the caller is not authorized', async () => {
+    describe('when the caller is not the initialized core address', async () => {
       beforeEach(async () => {
         subjectCaller = unauthorizedAddress;
       });

test/core/exchange-wrappers/takerWalletWrapper.spec.ts

@@ -60,8 +60,7 @@ contract('TakerWalletWrapper', accounts => {
 
     transferProxy = await coreWrapper.deployTransferProxyAsync();
 
-    takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(transferProxy);
-    await coreWrapper.addAuthorizationAsync(takerWalletWrapper, authorizedAddress);
+    takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(authorizedAddress, transferProxy);
     await coreWrapper.addAuthorizationAsync(transferProxy, takerWalletWrapper.address);
 
     components = await erc20Wrapper.deployTokensAsync(componentCount, takerAccount);
@@ -142,7 +141,7 @@ contract('TakerWalletWrapper', accounts => {
       expect(newBalance).to.be.bignumber.equal(expectedNewAllowance);
     });
 
-    describe('when the caller is not authorized', async () => {
+    describe('when the caller is not the deployed core address', async () => {
       beforeEach(async () => {
         subjectCaller = unauthorizedAddress;
       });

test/core/exchange-wrappers/zeroExExchangeWrapper.spec.ts

@@ -38,6 +38,8 @@ contract('ZeroExExchangeWrapper', accounts => {
     issuanceOrderAndZeroExOrderTakerAccount,
     secondZeroExOrderMakerAccount,
     feeRecipientAccount,
+    coreContractAddress,
+    unauthorizedAddress,
   ] = accounts;
 
   const coreWrapper = new CoreWrapper(deployerAccount, deployerAccount);
@@ -57,12 +59,12 @@ contract('ZeroExExchangeWrapper', accounts => {
     transferProxy = await coreWrapper.deployTransferProxyAsync();
 
     zeroExExchangeWrapper = await exchangeWrapper.deployZeroExExchangeWrapper(
+      coreContractAddress,
       SetTestUtils.ZERO_EX_EXCHANGE_ADDRESS,
       SetTestUtils.ZERO_EX_ERC20_PROXY_ADDRESS,
       SetTestUtils.ZERO_EX_TOKEN_ADDRESS,
       transferProxy,
     );
-    await coreWrapper.addAuthorizationAsync(zeroExExchangeWrapper, deployerAccount);
 
     // ZRX token is already deployed to zrxTokenOwnerAccount via the test snapshot
     zrxToken = erc20Wrapper.zrxToken();
@@ -101,6 +103,7 @@ contract('ZeroExExchangeWrapper', accounts => {
     let subjectMakerTokenAmount: BigNumber;
     let subjectOrderCount: BigNumber;
     let subjectOrderData: Bytes;
+    let subjectCaller: Address;
 
     let zeroExOrder: ZeroExOrder;
     let senderAddress: Address;
@@ -158,6 +161,7 @@ contract('ZeroExExchangeWrapper', accounts => {
       subjectMakerTokenAmount = takerAssetAmount;
       subjectOrderCount = new BigNumber(1);
       subjectOrderData = zeroExExchangeWrapperOrder;
+      subjectCaller = coreContractAddress;
     });
 
     async function subject(): Promise<any> {
@@ -168,7 +172,7 @@ contract('ZeroExExchangeWrapper', accounts => {
         subjectMakerTokenAmount,
         subjectOrderCount,
         subjectOrderData,
-        { from: deployerAccount, gas: DEFAULT_GAS },
+        { from: subjectCaller, gas: DEFAULT_GAS },
       );
     }
 
@@ -280,6 +284,16 @@ contract('ZeroExExchangeWrapper', accounts => {
       });
     });
 
+    describe('when the caller is not the initialized core address', async () => {
+      beforeEach(async () => {
+        subjectCaller = unauthorizedAddress;
+      });
+
+      it('should revert', async () => {
+        await expectRevertError(subject());
+      });
+    });
+
     describe('when the order is already expired', async() => {
       before(async () => {
         expirationTimeSeconds = SetTestUtils.generateTimestamp(0);
@@ -416,7 +430,7 @@ contract('ZeroExExchangeWrapper', accounts => {
           subjectMakerTokenAmount,
           subjectOrderCount,
           subjectOrderData,
-          { from: deployerAccount, gas: DEFAULT_GAS },
+          { from: subjectCaller, gas: DEFAULT_GAS },
         );
       }
 

test/core/extensions/coreIssuanceOrder.spec.ts

@@ -129,18 +129,18 @@ contract('CoreIssuanceOrder', accounts => {
     let kyberConversionRatePower: BigNumber;
 
     beforeEach(async () => {
-      await exchangeWrapper.deployAndAuthorizeTakerWalletExchangeWrapper(transferProxy, core);
+      await exchangeWrapper.deployAndAuthorizeTakerWalletExchangeWrapper(core, transferProxy);
       await exchangeWrapper.deployAndAuthorizeZeroExExchangeWrapper(
+        core,
         SetTestUtils.ZERO_EX_EXCHANGE_ADDRESS,
         SetTestUtils.ZERO_EX_ERC20_PROXY_ADDRESS,
         SetTestUtils.ZERO_EX_TOKEN_ADDRESS,
-        transferProxy,
-        core
+        transferProxy
       );
       await exchangeWrapper.deployAndAuthorizeKyberNetworkWrapper(
+        core,
         SetTestUtils.KYBER_NETWORK_PROXY_ADDRESS,
-        transferProxy,
-        core
+        transferProxy
       );
 
       const firstComponent = await erc20Wrapper.deployTokenAsync(issuanceOrderTaker);

test/core/extensions/coreIssuanceOrderScenarios.spec.ts

@@ -75,8 +75,7 @@ contract('CoreIssuanceOrder::Scenarios', accounts => {
     setTokenFactory = await coreWrapper.deploySetTokenFactoryAsync(core.address);
     await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory);
 
-    takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(transferProxy);
-    await coreWrapper.addAuthorizationAsync(takerWalletWrapper, core.address);
+    takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(core.address, transferProxy);
     await coreWrapper.addAuthorizationAsync(transferProxy, takerWalletWrapper.address);
   });