feat: use internal acl (#94)

Author: sakulstra

src/contracts/treasury/Collector.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
+import {AccessControlUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/access/AccessControlUpgradeable.sol';
+import {ReentrancyGuardUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol';
 import {ICollector} from './ICollector.sol';
 import {IAccessControl} from '../dependencies/openzeppelin/contracts/IAccessControl.sol';
-import {ReentrancyGuard} from '../dependencies/openzeppelin/ReentrancyGuard.sol';
-import {VersionedInitializable} from '../misc/aave-upgradeability/VersionedInitializable.sol';
 import {IERC20} from '../dependencies/openzeppelin/contracts/IERC20.sol';
 import {SafeERC20} from '../dependencies/openzeppelin/contracts/SafeERC20.sol';
 import {Address} from '../dependencies/openzeppelin/contracts/Address.sol';
@@ -22,25 +22,19 @@ import {Address} from '../dependencies/openzeppelin/contracts/Address.sol';
  * - Same as with creation, on Sablier the `sender` and `recipient` can cancel a stream. Here, only fund admin and recipient
  * @author BGD Labs
  **/
-contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
+contract Collector is AccessControlUpgradeable, ReentrancyGuardUpgradeable, ICollector {
   using SafeERC20 for IERC20;
   using Address for address payable;
 
   /*** Storage Properties ***/
-
-  /**
-   * @notice Current revision of the contract.
-   */
-  uint256 public constant REVISION = 6;
-
   /// @inheritdoc ICollector
   address public constant ETH_MOCK_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
 
   /// @inheritdoc ICollector
   bytes32 public constant FUNDS_ADMIN_ROLE = 'FUNDS_ADMIN';
 
-  /// @inheritdoc ICollector
-  address public immutable ACL_MANAGER;
+  // Reserved storage space to account for deprecated inherited storage
+  uint256[53] private ______gap;
 
   /**
    * @notice Counter for new stream ids.
@@ -83,33 +77,29 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
     _;
   }
 
-  constructor(address aclManager) {
-    if (aclManager == address(0)) revert InvalidZeroAddress();
-    ACL_MANAGER = aclManager;
+  constructor() {
+    _disableInitializers();
   }
 
   /*** Contract Logic Starts Here */
 
-  /// @inheritdoc ICollector
-  function initialize(uint256 nextStreamId) external virtual initializer {
+  /** @notice Initializes the contracts
+   * @param nextStreamId StreamId to set, applied if greater than 0
+   * @param admin The default admin managing the FundsAdmins
+   **/
+  function initialize(uint256 nextStreamId, address admin) external virtual initializer {
+    __AccessControl_init();
+    __ReentrancyGuard_init();
+    _grantRole(DEFAULT_ADMIN_ROLE, admin);
     if (nextStreamId != 0) {
       _nextStreamId = nextStreamId;
     }
-
-    _initGuard();
-    _setFundsAdmin(fundsAdmin);
   }
 
   /*** View Functions ***/
-
-  /// @inheritdoc VersionedInitializable
-  function getRevision() internal pure override returns (uint256) {
-    return REVISION;
-  }
-
   /// @inheritdoc ICollector
   function isFundsAdmin(address admin) external view returns (bool) {
-    return IAccessControl(ACL_MANAGER).hasRole(FUNDS_ADMIN_ROLE, admin);
+    return hasRole(FUNDS_ADMIN_ROLE, admin);
   }
 
   /// @inheritdoc ICollector
@@ -214,7 +204,7 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
   }
 
   function _onlyFundsAdmin() internal view returns (bool) {
-    return IAccessControl(ACL_MANAGER).hasRole(FUNDS_ADMIN_ROLE, msg.sender);
+    return hasRole(FUNDS_ADMIN_ROLE, msg.sender);
   }
 
   struct CreateStreamLocalVars {

src/contracts/treasury/ICollector.sol

@@ -119,21 +119,11 @@ interface ICollector {
    **/
   function FUNDS_ADMIN_ROLE() external view returns (bytes32);
 
-  /**
-   * @notice Address of the current ACL Manager.
-   **/
-  function ACL_MANAGER() external view returns (address);
-
   /** @notice Returns the mock ETH reference address
    * @return address The address
    **/
   function ETH_MOCK_ADDRESS() external pure returns (address);
 
-  /** @notice Initializes the contracts
-   * @param nextStreamId StreamId to set, applied if greater than 0
-   **/
-  function initialize(uint256 nextStreamId) external;
-
   /**
    * @notice Checks if address is funds admin
    * @return bool If the address has the funds admin role

src/deployments/contracts/procedures/AaveV3SetupProcedure.sol

@@ -36,7 +36,7 @@ contract AaveV3SetupProcedure {
     address treasuryPartner;
     uint16 treasurySplitPercent;
     address proxyAdmin;
-    address aclManager;
+    address aclAdmin;
     bytes32 salt;
   }
 
@@ -100,7 +100,7 @@ contract AaveV3SetupProcedure {
         treasuryPartner: config.treasuryPartner,
         treasurySplitPercent: config.treasurySplitPercent,
         proxyAdmin: proxyAdmin,
-        aclManager: report.aclManager,
+        aclAdmin: roles.poolAdmin,
         salt: config.salt
       })
     );
@@ -210,29 +210,27 @@ contract AaveV3SetupProcedure {
 
   function _deployAaveV3Treasury(
     address deployedProxyAdmin,
-    address aclManager,
+    address aclAdmin,
     bytes32 salt
   ) internal returns (address treasuryProxy, address treasuryImplementation) {
     if (salt != '') {
-      treasuryImplementation = address(new Collector{salt: salt}(aclManager));
-      Collector(treasuryImplementation).initialize(0);
+      treasuryImplementation = address(new Collector{salt: salt}());
 
       treasuryProxy = address(
         new TransparentUpgradeableProxy{salt: salt}(
           treasuryImplementation,
           ProxyAdmin(deployedProxyAdmin),
-          abi.encodeWithSelector(Collector.initialize.selector, 100_000)
+          abi.encodeWithSelector(Collector.initialize.selector, 100_000, aclAdmin)
         )
       );
     } else {
-      treasuryImplementation = address(new Collector(aclManager));
-      Collector(treasuryImplementation).initialize(0);
+      treasuryImplementation = address(new Collector());
 
       treasuryProxy = address(
         new TransparentUpgradeableProxy(
           treasuryImplementation,
           ProxyAdmin(deployedProxyAdmin),
-          abi.encodeWithSelector(Collector.initialize.selector, 100_000)
+          abi.encodeWithSelector(Collector.initialize.selector, 100_000, aclAdmin)
         )
       );
     }
@@ -247,7 +245,7 @@ contract AaveV3SetupProcedure {
     if (input.treasuryProxy == address(0)) {
       (treasuryProxy, treasuryImplementation) = _deployAaveV3Treasury(
         input.proxyAdmin,
-        input.aclManager,
+        input.aclAdmin,
         input.salt
       );
     } else {

tests/deployments/AaveV3PermissionsTest.t.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.0;
 
 import 'forge-std/Test.sol';
+import {IAccessControl} from 'openzeppelin-contracts/contracts/access/IAccessControl.sol';
 import {Ownable} from '../../src/contracts/dependencies/openzeppelin/contracts/Ownable.sol';
 import {IPoolAddressesProvider} from '../../src/contracts/interfaces/IPoolAddressesProvider.sol';
 import '../../src/deployments/interfaces/IMarketReportTypes.sol';
@@ -305,7 +306,7 @@ contract AaveV3PermissionsTest is BatchTestProcedures {
         report.proxyAdmin,
         'Treasury proxy admin does not match with report.proxyAdmin'
       );
-      assertEq(ICollector(report.treasury).ACL_MANAGER(), report.aclManager);
+      assertEq(IAccessControl(report.treasury).hasRole(emptyBytes, roles.poolAdmin), true);
     }
     {
       address proxyAdminOwner = Ownable(report.proxyAdmin).owner();

tests/treasury/Collector.t.sol

@@ -4,12 +4,13 @@ pragma solidity ^0.8.0;
 import {Test} from 'forge-std/Test.sol';
 import {StdUtils} from 'forge-std/StdUtils.sol';
 
-import {IERC20} from 'src/contracts/dependencies/openzeppelin/contracts/IERC20.sol';
-import {IAccessControl} from 'src/contracts/dependencies/openzeppelin/contracts/IAccessControl.sol';
-import {ACLManager} from 'src/contracts/protocol/configuration/ACLManager.sol';
-import {PoolAddressesProvider} from 'src/contracts/protocol/configuration/PoolAddressesProvider.sol';
-import {Collector} from 'src/contracts/treasury/Collector.sol';
-import {ICollector} from 'src/contracts/treasury/ICollector.sol';
+import {ProxyAdmin} from 'solidity-utils/contracts/transparent-proxy/ProxyAdmin.sol';
+import {TransparentUpgradeableProxy} from 'solidity-utils/contracts/transparent-proxy/TransparentUpgradeableProxy.sol';
+import {IERC20} from '../../src/contracts/dependencies/openzeppelin/contracts/IERC20.sol';
+import {IAccessControl} from '../../src/contracts/dependencies/openzeppelin/contracts/IAccessControl.sol';
+import {PoolAddressesProvider} from '../../src/contracts/protocol/configuration/PoolAddressesProvider.sol';
+import {Collector} from '../../src/contracts/treasury/Collector.sol';
+import {ICollector} from '../../src/contracts/treasury/ICollector.sol';
 
 contract CollectorTest is StdUtils, Test {
   Collector public collector;
@@ -25,7 +26,7 @@ contract CollectorTest is StdUtils, Test {
 
   uint256 public streamStartTime;
   uint256 public streamStopTime;
-  uint256 public nextStreamID;
+  uint256 public nextStreamID = 100_000;
 
   event StreamIdChanged(uint256 indexed streamId);
   event CreateStream(
@@ -56,23 +57,29 @@ contract CollectorTest is StdUtils, Test {
     vm.prank(OWNER);
     provider.setACLAdmin(EXECUTOR_LVL_1);
 
-    ACLManager aclManager = new ACLManager(provider);
-
     tokenA = IERC20(address(deployMockERC20('Token A', 'TK_A', 18)));
     tokenB = IERC20(address(deployMockERC20('Token B', 'TK_B', 6)));
 
     streamStartTime = block.timestamp + 10;
     streamStopTime = block.timestamp + 70;
     nextStreamID = 0;
 
-    collector = new Collector(address(aclManager));
-    collector.initialize(nextStreamID);
+    address collectorImpl = address(new Collector());
+    collector = Collector(
+      address(
+        new TransparentUpgradeableProxy(
+          collectorImpl,
+          new ProxyAdmin(address(this)), // mock proxy admin
+          abi.encodeWithSelector(Collector.initialize.selector, nextStreamID, EXECUTOR_LVL_1)
+        )
+      )
+    );
 
     deal(address(tokenA), address(collector), 100 ether);
 
     vm.startPrank(EXECUTOR_LVL_1);
-    IAccessControl(address(aclManager)).grantRole(collector.FUNDS_ADMIN_ROLE(), FUNDS_ADMIN);
-    IAccessControl(address(aclManager)).grantRole(collector.FUNDS_ADMIN_ROLE(), EXECUTOR_LVL_1);
+    IAccessControl(address(collector)).grantRole(collector.FUNDS_ADMIN_ROLE(), FUNDS_ADMIN);
+    IAccessControl(address(collector)).grantRole(collector.FUNDS_ADMIN_ROLE(), EXECUTOR_LVL_1);
     vm.stopPrank();
   }
 
@@ -417,12 +424,6 @@ contract StreamsTest is CollectorTest {
   }
 }
 
-contract GetRevision is CollectorTest {
-  function test_successful() public view {
-    assertEq(collector.REVISION(), 6);
-  }
-}
-
 contract FundsAdminRoleBytesTest is CollectorTest {
   function test_successful() public view {
     assertEq(collector.FUNDS_ADMIN_ROLE(), 'FUNDS_ADMIN');