Byte-Masons / Reliquary

50 stars 20 forks source link

Switch from Ownable to AccessControlEnumerable. #20

Closed tess3rac7 closed 2 years ago

tess3rac7 commented 2 years ago

DEFAULT_ADMIN_ROLE == super user. Deployer (presumably our multisig--but if not, multisig can be added to this role by deployer later) will get this role. Can add/remove people from all roles.

OPERATOR == allowed to add pools and modify existing pool properties.

Tests have been updated to include a new signer for operator. I also ran prettier on the test files.

~This is just a 1:1 PR whereby DEFAULT_ADMIN_ROLE is equivalent to OWNER. I added a GOVERNANCE role to demonstrate what adding new roles would look like (although it's currently not used anywhere).~

~Functions included with AccessControlEnumerable pretty much take care of all the role administration. We just now have to add roles we want, and enforce them on functions as we see fit.~

~I feel this would go hand-in-hand very powerfully with a UUPS proxy as that would allow us to deploy upgrades with modified roles (roles are constants, not storage) and seamlessly update the capabilities of each role. We can make it so that only GOVERNANCE or some other appropriate role is allowed to do the actual upgrade.~

Here are some more examples of how we use AccessControlEnumerable in our strategy contracts:

Ex 1

    /**
     * @dev Updates the current strategistRemitter.
     *      If there is only one strategist this function may be called by
     *      that strategist. However if there are multiple strategists
     *      this function may only be called by the STRATEGIST_MULTISIG role.
     */
    function updateStrategistRemitter(address _newStrategistRemitter) external {
        if (getRoleMemberCount(STRATEGIST) == 1) {
            _checkRole(STRATEGIST, msg.sender);
        } else {
            _checkRole(STRATEGIST_MULTISIG, msg.sender);
        }

        require(_newStrategistRemitter != address(0), "!0");
        strategistRemitter = _newStrategistRemitter;
        emit StrategistRemitterUpdated(_newStrategistRemitter);
    }

Ex 2

    /**
     * @dev Only strategist or owner can edit the log cadence.
     */
    function updateHarvestLogCadence(uint256 _newCadenceInSeconds) external {
        _onlyStrategistOrOwner();
        harvestLogCadence = _newCadenceInSeconds;
    }

    /**
     * @dev Only allow access to strategist or owner
     */
    function _onlyStrategistOrOwner() internal view {
        require(hasRole(STRATEGIST, msg.sender) || hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Not authorized");
    }