feat: Collector revision-6 Upgrade. (#82)

src/contracts/treasury/Collector.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.0;
 
 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';
@@ -27,15 +28,19 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
 
   /*** Storage Properties ***/
 
-  /**
-   * @notice Address of the current funds admin.
-   */
-  address internal _fundsAdmin;
-
   /**
    * @notice Current revision of the contract.
    */
-  uint256 public constant REVISION = 5;
+  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;
 
   /**
    * @notice Counter for new stream ids.
@@ -47,16 +52,15 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
    */
   mapping(uint256 => Stream) private _streams;
 
-  /// @inheritdoc ICollector
-  address public constant ETH_MOCK_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
-
   /*** Modifiers ***/
 
   /**
-   * @dev Throws if the caller is not the funds admin.
+   * @dev Throws if the caller does not have the FUNDS_ADMIN role
    */
   modifier onlyFundsAdmin() {
-    require(msg.sender == _fundsAdmin, 'ONLY_BY_FUNDS_ADMIN');
+    if (_onlyFundsAdmin() == false) {
+      revert OnlyFundsAdmin();
+    }
     _;
   }
 
@@ -65,30 +69,32 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
    * @param streamId The id of the stream to query.
    */
   modifier onlyAdminOrRecipient(uint256 streamId) {
-    require(
-      msg.sender == _fundsAdmin || msg.sender == _streams[streamId].recipient,
-      'caller is not the funds admin or the recipient of the stream'
-    );
+    if (_onlyFundsAdmin() == false && msg.sender != _streams[streamId].recipient) {
+      revert OnlyFundsAdminOrRceipient();
+    }
     _;
   }
 
   /**
    * @dev Throws if the provided id does not point to a valid stream.
    */
   modifier streamExists(uint256 streamId) {
-    require(_streams[streamId].isEntity, 'stream does not exist');
+    if (!_streams[streamId].isEntity) revert StreamDoesNotExist();
     _;
   }
 
+  constructor(address aclManager) {
+    if (aclManager == address(0)) revert InvalidZeroAddress();
+    ACL_MANAGER = aclManager;
+  }
+
   /*** Contract Logic Starts Here */
 
   /// @inheritdoc ICollector
-  function initialize(address fundsAdmin, uint256 nextStreamId) external initializer {
+  function initialize(uint256 nextStreamId) external virtual initializer {
     if (nextStreamId != 0) {
       _nextStreamId = nextStreamId;
     }
-
-    _setFundsAdmin(fundsAdmin);
   }
 
   /*** View Functions ***/
@@ -99,8 +105,8 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
   }
 
   /// @inheritdoc ICollector
-  function getFundsAdmin() external view returns (address) {
-    return _fundsAdmin;
+  function isFundsAdmin(address admin) external view returns (bool) {
+    return IAccessControl(ACL_MANAGER).hasRole(FUNDS_ADMIN_ROLE, admin);
   }
 
   /// @inheritdoc ICollector
@@ -195,7 +201,7 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
 
   /// @inheritdoc ICollector
   function transfer(IERC20 token, address recipient, uint256 amount) external onlyFundsAdmin {
-    require(recipient != address(0), 'INVALID_0X_RECIPIENT');
+    if (recipient == address(0)) revert InvalidZeroAddress();
 
     if (address(token) == ETH_MOCK_ADDRESS) {
       payable(recipient).sendValue(amount);
@@ -204,18 +210,8 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
     }
   }
 
-  /// @inheritdoc ICollector
-  function setFundsAdmin(address admin) external onlyFundsAdmin {
-    _setFundsAdmin(admin);
-  }
-
-  /**
-   * @dev Transfer the ownership of the funds administrator role.
-   * @param admin The address of the new funds administrator
-   */
-  function _setFundsAdmin(address admin) internal {
-    _fundsAdmin = admin;
-    emit NewFundsAdmin(admin);
+  function _onlyFundsAdmin() internal view returns (bool) {
+    return IAccessControl(ACL_MANAGER).hasRole(FUNDS_ADMIN_ROLE, msg.sender);
   }
 
   struct CreateStreamLocalVars {
@@ -244,21 +240,21 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
     uint256 startTime,
     uint256 stopTime
   ) external onlyFundsAdmin returns (uint256) {
-    require(recipient != address(0), 'stream to the zero address');
-    require(recipient != address(this), 'stream to the contract itself');
-    require(recipient != msg.sender, 'stream to the caller');
-    require(deposit > 0, 'deposit is zero');
-    require(startTime >= block.timestamp, 'start time before block.timestamp');
-    require(stopTime > startTime, 'stop time before the start time');
+    if (recipient == address(0)) revert InvalidZeroAddress();
+    if (recipient == address(this)) revert InvalidRecipient();
+    if (recipient == msg.sender) revert InvalidRecipient();
+    if (deposit == 0) revert InvalidZeroAmount();
+    if (startTime < block.timestamp) revert InvalidStartTime();
+    if (stopTime <= startTime) revert InvalidStopTime();
 
     CreateStreamLocalVars memory vars;
     vars.duration = stopTime - startTime;
 
     /* Without this, the rate per second would be zero. */
-    require(deposit >= vars.duration, 'deposit smaller than time delta');
+    if (deposit < vars.duration) revert DepositSmallerTimeDelta();
 
     /* This condition avoids dealing with remainders */
-    require(deposit % vars.duration == 0, 'deposit not multiple of time delta');
+    if (deposit % vars.duration > 0) revert DepositNotMultipleTimeDelta();
 
     vars.ratePerSecond = deposit / vars.duration;
 
@@ -302,11 +298,11 @@ contract Collector is VersionedInitializable, ICollector, ReentrancyGuard {
     uint256 streamId,
     uint256 amount
   ) external nonReentrant streamExists(streamId) onlyAdminOrRecipient(streamId) returns (bool) {
-    require(amount > 0, 'amount is zero');
+    if (amount == 0) revert InvalidZeroAmount();
     Stream memory stream = _streams[streamId];
 
     uint256 balance = balanceOf(streamId, stream.recipient);
-    require(balance >= amount, 'amount exceeds the available balance');
+    if (balance < amount) revert BalanceExceeded();
 
     _streams[streamId].remainingBalance = stream.remainingBalance - amount;
 

src/contracts/treasury/ICollector.sol

@@ -16,10 +16,60 @@ interface ICollector {
     bool isEntity;
   }
 
-  /** @notice Emitted when the funds admin changes
-   * @param fundsAdmin The new funds admin.
-   **/
-  event NewFundsAdmin(address indexed fundsAdmin);
+  /**
+   * @dev Withdraw amount exceeds available balance
+   */
+  error BalanceExceeded();
+
+  /**
+   * @dev Deposit smaller than time delta
+   */
+  error DepositSmallerTimeDelta();
+
+  /**
+   * @dev Deposit not multiple of time delta
+   */
+  error DepositNotMultipleTimeDelta();
+
+  /**
+   * @dev Recipient cannot be the contract itself or msg.sender
+   */
+  error InvalidRecipient();
+
+  /**
+   * @dev Start time cannot be before block.timestamp
+   */
+  error InvalidStartTime();
+
+  /**
+   * @dev Stop time must be greater than startTime
+   */
+  error InvalidStopTime();
+
+  /**
+   * @dev Provided address cannot be the zero-address
+   */
+  error InvalidZeroAddress();
+
+  /**
+   * @dev Amount cannot be zero
+   */
+  error InvalidZeroAmount();
+
+  /**
+   * @dev Only caller with FUNDS_ADMIN role can call
+   */
+  error OnlyFundsAdmin();
+
+  /**
+   * @dev Only caller with FUNDS_ADMIN role or stream recipient can call
+   */
+  error OnlyFundsAdminOrRceipient();
+
+  /**
+   * @dev The provided ID does not belong to an existing stream
+   */
+  error StreamDoesNotExist();
 
   /** @notice Emitted when the new stream is created
    * @param streamId The identifier of the stream.
@@ -64,29 +114,38 @@ interface ICollector {
     uint256 recipientBalance
   );
 
+  /**
+   * @notice FUNDS_ADMIN role granted by ACL Manager
+   **/
+  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 fundsAdmin Funds admin address
    * @param nextStreamId StreamId to set, applied if greater than 0
    **/
-  function initialize(address fundsAdmin, uint256 nextStreamId) external;
+  function initialize(uint256 nextStreamId) external;
 
   /**
-   * @notice Return the funds admin, only entity to be able to interact with this contract (controller of reserve)
-   * @return address The address of the funds admin
+   * @notice Checks if address is funds admin
+   * @return bool If the address has the funds admin role
    **/
-  function getFundsAdmin() external view returns (address);
+  function isFundsAdmin(address admin) external view returns (bool);
 
   /**
    * @notice Returns the available funds for the given stream id and address.
    * @param streamId The id of the stream for which to query the balance.
    * @param who The address for which to query the balance.
    * @notice Returns the total funds allocated to `who` as uint256.
-   */
+   **/
   function balanceOf(uint256 streamId, address who) external view returns (uint256 balance);
 
   /**
@@ -105,13 +164,6 @@ interface ICollector {
    **/
   function transfer(IERC20 token, address recipient, uint256 amount) external;
 
-  /**
-   * @dev Transfer the ownership of the funds administrator role.
-          This function should only be callable by the current funds administrator.
-   * @param admin The address of the new funds administrator
-   */
-  function setFundsAdmin(address admin) external;
-
   /**
    * @notice Creates a new stream funded by this contracts itself and paid towards `recipient`.
    * @param recipient The address towards which the money is streamed.