Merge pull request #79 from bancorprotocol/access-control

Ensure that every proxy contract implements AccessControl + cleanups

Author: lbeder

packages/v3/components/Contracts.ts

@@ -15,15 +15,16 @@ import {
     TestERC20Token__factory,
     TestMathEx__factory,
     TestNetworkTokenPool__factory,
-    TestOwnedUpgradeable__factory,
+    TestOwned__factory,
     TestPendingWithdrawals__factory,
     TestPoolAverageRate__factory,
     TestPoolCollection__factory,
     TestReserveToken__factory,
     TestSafeERC20Ex__factory,
     TestSystemToken__factory,
     TestTokenGovernance__factory,
-    TokenHolderUpgradeable__factory,
+    TestUpgradeable__factory,
+    TokenHolder__factory,
     TransparentUpgradeableProxy__factory
 } from '../typechain';
 
@@ -90,13 +91,14 @@ const getContracts = (signer?: Signer) => ({
     TestPoolCollection: deployOrAttach<TestPoolCollection__factory>('TestPoolCollection', signer),
     TestNetworkTokenPool: deployOrAttach<TestNetworkTokenPool__factory>('TestNetworkTokenPool', signer),
     TestMathEx: deployOrAttach<TestMathEx__factory>('TestMathEx', signer),
-    TestOwnedUpgradeable: deployOrAttach<TestOwnedUpgradeable__factory>('TestOwnedUpgradeable', signer),
+    TestOwned: deployOrAttach<TestOwned__factory>('TestOwned', signer),
     TestPendingWithdrawals: deployOrAttach<TestPendingWithdrawals__factory>('TestPendingWithdrawals', signer),
     TestReserveToken: deployOrAttach<TestReserveToken__factory>('TestReserveToken', signer),
     TestSafeERC20Ex: deployOrAttach<TestSafeERC20Ex__factory>('TestSafeERC20Ex', signer),
     TestSystemToken: deployOrAttach<TestSystemToken__factory>('TestSystemToken', signer),
     TestTokenGovernance: deployOrAttach<TestTokenGovernance__factory>('TestTokenGovernance', signer),
-    TokenHolderUpgradeable: deployOrAttach<TokenHolderUpgradeable__factory>('TokenHolderUpgradeable', signer),
+    TestUpgradeable: deployOrAttach<TestUpgradeable__factory>('TestUpgradeable', signer),
+    TokenHolder: deployOrAttach<TokenHolder__factory>('TokenHolder', signer),
     TransparentUpgradeableProxy: deployOrAttach<TransparentUpgradeableProxy__factory>(
         'TransparentUpgradeableProxy',
         signer

packages/v3/contracts/helpers/TestOwned.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: SEE LICENSE IN LICENSE
+pragma solidity 0.7.6;
+
+import { Owned } from "../utility/Owned.sol";
+
+contract TestOwned is Owned {}

packages/v3/contracts/helpers/TestOwnedUpgradeable.sol

@@ -4,22 +4,16 @@ pragma solidity 0.7.6;
 import { IMintableToken } from "@bancor/token-governance/0.7.6/contracts/IMintableToken.sol";
 import { IClaimable } from "@bancor/token-governance/0.7.6/contracts/IClaimable.sol";
 
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
+import { Owned } from "../utility/Owned.sol";
 
 import { TestERC20Token } from "./TestERC20Token.sol";
 
-contract TestSystemToken is IMintableToken, OwnedUpgradeable, TestERC20Token {
+contract TestSystemToken is IMintableToken, Owned, TestERC20Token {
     constructor(
         string memory name,
         string memory symbol,
         uint256 totalSupply
-    ) TestERC20Token(name, symbol, totalSupply) {
-        __Owned_init();
-    }
-
-    function version() external pure override returns (uint16) {
-        return 1;
-    }
+    ) TestERC20Token(name, symbol, totalSupply) {}
 
     function issue(address to, uint256 amount) external override {
         _mint(to, amount);
@@ -29,15 +23,15 @@ contract TestSystemToken is IMintableToken, OwnedUpgradeable, TestERC20Token {
         _burn(from, amount);
     }
 
-    function owner() public view override(IClaimable, OwnedUpgradeable) returns (address) {
+    function owner() public view override(IClaimable, Owned) returns (address) {
         return super.owner();
     }
 
-    function transferOwnership(address newOwner) public override(IClaimable, OwnedUpgradeable) {
+    function transferOwnership(address newOwner) public override(IClaimable, Owned) {
         super.transferOwnership(newOwner);
     }
 
-    function acceptOwnership() public override(IClaimable, OwnedUpgradeable) {
+    function acceptOwnership() public override(IClaimable, Owned) {
         super.acceptOwnership();
     }
 }

packages/v3/contracts/helpers/TestSystemToken.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: SEE LICENSE IN LICENSE
+pragma solidity 0.7.6;
+
+import { Upgradeable } from "../utility/Upgradeable.sol";
+
+contract TestUpgradeable is Upgradeable {
+    function initialize() external initializer {
+        __TestUpgradeable_init();
+    }
+
+    // solhint-disable func-name-mixedcase
+
+    function __TestUpgradeable_init() internal initializer {
+        __Upgradeable_init();
+
+        __TestUpgradeable_init_unchained();
+    }
+
+    function __TestUpgradeable_init_unchained() internal initializer {}
+
+    function version() external pure override returns (uint16) {
+        return 1;
+    }
+
+    function restricted() external view onlyOwner {}
+}

packages/v3/contracts/helpers/TestUpgradeable.sol

@@ -12,7 +12,6 @@ import { EnumerableSetUpgradeable } from "@openzeppelin/contracts-upgradeable/ut
 import { ITokenGovernance } from "@bancor/token-governance/0.7.6/contracts/TokenGovernance.sol";
 
 import { ITokenHolder } from "../utility/interfaces/ITokenHolder.sol";
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
 import { Upgradeable } from "../utility/Upgradeable.sol";
 import { Time } from "../utility/Time.sol";
 import { Utils } from "../utility/Utils.sol";
@@ -32,7 +31,7 @@ import { IBancorVault } from "./interfaces/IBancorVault.sol";
 /**
  * @dev Bancor Network contract
  */
-contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, ReentrancyGuardUpgradeable, Time, Utils {
+contract BancorNetwork is IBancorNetwork, Upgradeable, ReentrancyGuardUpgradeable, Time, Utils {
     using Address for address payable;
     using SafeMath for uint256;
     using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;
@@ -276,7 +275,7 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         internal
         initializer
     {
-        __Owned_init();
+        __Upgradeable_init();
         __ReentrancyGuard_init();
 
         __BancorNetwork_init_unchained(initNetworkTokenPool, initPendingWithdrawals);

packages/v3/contracts/network/BancorNetwork.sol

@@ -4,7 +4,6 @@ pragma solidity 0.7.6;
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/SafeERC20.sol";
 
-import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
 import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
 
@@ -19,14 +18,7 @@ import { IBancorVault } from "./interfaces/IBancorVault.sol";
 /**
  * @dev Bancor Vault contract
  */
-contract BancorVault is
-    IBancorVault,
-    Upgradeable,
-    AccessControlUpgradeable,
-    PausableUpgradeable,
-    ReentrancyGuardUpgradeable,
-    Utils
-{
+contract BancorVault is IBancorVault, Upgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, Utils {
     using SafeERC20 for IERC20;
     using ReserveToken for IReserveToken;
 
@@ -70,7 +62,7 @@ contract BancorVault is
      * @dev initializes the contract and its parents
      */
     function __BancorVault_init() internal initializer {
-        __AccessControl_init();
+        __Upgradeable_init();
         __Pausable_init();
         __ReentrancyGuard_init();
 
@@ -99,10 +91,6 @@ contract BancorVault is
         _;
     }
 
-    function _hasRole(bytes32 role, address account) internal view {
-        require(hasRole(role, account), "ERR_ACCESS_DENIED");
-    }
-
     receive() external payable override {}
 
     /**

packages/v3/contracts/network/BancorVault.sol

@@ -4,7 +4,6 @@ pragma solidity 0.7.6;
 import { EnumerableSetUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/EnumerableSetUpgradeable.sol";
 
 import { ITokenHolder } from "../utility/interfaces/ITokenHolder.sol";
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
 import { Upgradeable } from "../utility/Upgradeable.sol";
 import { Utils } from "../utility/Utils.sol";
 
@@ -15,7 +14,7 @@ import { INetworkSettings } from "./interfaces/INetworkSettings.sol";
 /**
  * @dev Network Settings contract
  */
-contract NetworkSettings is INetworkSettings, Upgradeable, OwnedUpgradeable, Utils {
+contract NetworkSettings is INetworkSettings, Upgradeable, Utils {
     using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;
 
     // a set of tokens which are eligeble for protection
@@ -101,7 +100,7 @@ contract NetworkSettings is INetworkSettings, Upgradeable, OwnedUpgradeable, Uti
      * @dev initializes the contract and its parents
      */
     function __NetworkSettings_init() internal initializer {
-        __Owned_init();
+        __Upgradeable_init();
 
         __NetworkSettings_init_unchained();
     }

packages/v3/contracts/network/NetworkSettings.sol

@@ -10,7 +10,6 @@ import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol";
 
 import { IReserveToken } from "../token/interfaces/IReserveToken.sol";
 
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
 import { Upgradeable } from "../utility/Upgradeable.sol";
 import { Utils } from "../utility/Utils.sol";
 import { Time } from "../utility/Time.sol";
@@ -23,14 +22,7 @@ import { IPendingWithdrawals, WithdrawalRequest, CompletedWithdrawal } from "./i
 /**
  * @dev Pending Withdrawals contract
  */
-contract PendingWithdrawals is
-    IPendingWithdrawals,
-    Upgradeable,
-    OwnedUpgradeable,
-    ReentrancyGuardUpgradeable,
-    Time,
-    Utils
-{
+contract PendingWithdrawals is IPendingWithdrawals, Upgradeable, ReentrancyGuardUpgradeable, Time, Utils {
     using SafeMath for uint32;
     using SafeMath for uint256;
     using SafeERC20 for IPoolToken;
@@ -134,8 +126,8 @@ contract PendingWithdrawals is
      * @dev initializes the contract and its parents
      */
     function __PendingWithdrawals_init() internal initializer {
+        __Upgradeable_init();
         __ReentrancyGuard_init();
-        __Owned_init();
 
         __PendingWithdrawals_init_unchained();
     }

packages/v3/contracts/network/PendingWithdrawals.sol

@@ -14,7 +14,7 @@ import { ReserveToken } from "../token/ReserveToken.sol";
 
 import { Fraction } from "../utility/Types.sol";
 import { MAX_UINT128, PPM_RESOLUTION } from "../utility/Constants.sol";
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
+import { Owned } from "../utility/Owned.sol";
 import { Utils } from "../utility/Utils.sol";
 import { MathEx } from "../utility/MathEx.sol";
 
@@ -34,7 +34,7 @@ import { PoolAverageRate, AverageRate } from "./PoolAverageRate.sol";
  *
  * - in Bancor V3, the address of reserve token serves as the pool unique ID in both contract functions and events
  */
-contract PoolCollection is IPoolCollection, OwnedUpgradeable, ReentrancyGuardUpgradeable, Utils {
+contract PoolCollection is IPoolCollection, Owned, ReentrancyGuardUpgradeable, Utils {
     using SafeMath for uint256;
     using SafeCast for uint256;
     using ReserveToken for IReserveToken;
@@ -132,7 +132,6 @@ contract PoolCollection is IPoolCollection, OwnedUpgradeable, ReentrancyGuardUpg
         validAddress(address(initNetwork))
         validAddress(address(initPoolTokenFactory))
     {
-        __Owned_init();
         __ReentrancyGuard_init();
 
         _network = initNetwork;

packages/v3/contracts/pools/PoolCollection.sol

@@ -7,15 +7,15 @@ import { ERC20Permit } from "@openzeppelin/contracts/drafts/ERC20Permit.sol";
 import { IReserveToken } from "../token/interfaces/IReserveToken.sol";
 import { ERC20Burnable } from "../token/ERC20Burnable.sol";
 
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
+import { Owned } from "../utility/Owned.sol";
 import { Utils } from "../utility/Utils.sol";
 
 import { IPoolToken } from "./interfaces/IPoolToken.sol";
 
 /**
  * @dev Pool Token contract
  */
-contract PoolToken is IPoolToken, ERC20Permit, ERC20Burnable, OwnedUpgradeable, Utils {
+contract PoolToken is IPoolToken, ERC20Permit, ERC20Burnable, Owned, Utils {
     IReserveToken private immutable _reserveToken;
 
     /**
@@ -30,8 +30,6 @@ contract PoolToken is IPoolToken, ERC20Permit, ERC20Burnable, OwnedUpgradeable,
         _setupDecimals(decimals);
 
         _reserveToken = initReserveToken;
-
-        __Owned_init();
     }
 
     /**

packages/v3/contracts/pools/PoolToken.sol

@@ -6,7 +6,6 @@ import { ReserveToken } from "../token/ReserveToken.sol";
 
 import { Upgradeable } from "../utility/Upgradeable.sol";
 import { Utils } from "../utility/Utils.sol";
-import { OwnedUpgradeable } from "../utility/OwnedUpgradeable.sol";
 
 import { IPoolTokenFactory } from "./interfaces/IPoolTokenFactory.sol";
 import { IPoolToken } from "./interfaces/IPoolToken.sol";
@@ -15,7 +14,7 @@ import { PoolToken } from "./PoolToken.sol";
 /**
  * @dev Pool Token Factory contract
  */
-contract PoolTokenFactory is IPoolTokenFactory, Upgradeable, OwnedUpgradeable, Utils {
+contract PoolTokenFactory is IPoolTokenFactory, Upgradeable, Utils {
     using ReserveToken for IReserveToken;
 
     string private constant POOL_TOKEN_SYMBOL_PREFIX = "bn";
@@ -49,7 +48,7 @@ contract PoolTokenFactory is IPoolTokenFactory, Upgradeable, OwnedUpgradeable, U
      * @dev initializes the contract and its parents
      */
     function __PoolTokenFactory_init() internal initializer {
-        __Owned_init();
+        __Upgradeable_init();
 
         __PoolTokenFactory_init_unchained();
     }

packages/v3/contracts/pools/PoolTokenFactory.sol

@@ -7,12 +7,13 @@ import { IERC20Permit } from "@openzeppelin/contracts/drafts/IERC20Permit.sol";
 import { IReserveToken } from "../../token/interfaces/IReserveToken.sol";
 import { IERC20Burnable } from "../../token/interfaces/IERC20Burnable.sol";
 
+import { IVersioned } from "../../utility/interfaces/IVersioned.sol";
 import { IOwned } from "../../utility/interfaces/IOwned.sol";
 
 /**
  * @dev Pool Token interface
  */
-interface IPoolToken is IOwned, IERC20, IERC20Permit, IERC20Burnable {
+interface IPoolToken is IVersioned, IOwned, IERC20, IERC20Permit, IERC20Burnable {
     /**
      * @dev returns the address of the reserve token
      */

packages/v3/contracts/pools/interfaces/IPoolToken.sol

@@ -3,18 +3,13 @@ pragma solidity 0.7.6;
 
 import { IOwned } from "./interfaces/IOwned.sol";
 
-import { Upgradeable } from "./Upgradeable.sol";
-
 /**
  * @dev this contract provides support and utilities for contract ownership
  */
-abstract contract OwnedUpgradeable is IOwned, Upgradeable {
+abstract contract Owned is IOwned {
     address private _owner;
     address private _newOwner;
 
-    // upgrade forward-compatibility storage gap
-    uint256[MAX_GAP - 2] private __gap;
-
     /**
      * @dev triggered when the owner is updated
      */
@@ -23,16 +18,9 @@ abstract contract OwnedUpgradeable is IOwned, Upgradeable {
     // solhint-disable func-name-mixedcase
 
     /**
-     * @dev initializes the contract and its parents
-     */
-    function __Owned_init() internal initializer {
-        __Owned_init_unchained();
-    }
-
-    /**
-     * @dev performs contract-specific initialization
+     * @dev initializes the contract
      */
-    function __Owned_init_unchained() internal initializer {
+    constructor() {
         _setOwner(msg.sender);
     }