Mint restriction: Role-based permissions in the smart contract

[NicolasMahe]

Implement the mint restriction using Role-based permissions in the smart contract as described in the initial post.

Initial post: rarible/protocol#11 (comment)

[Hadrienlc]

Following our discussion with Anthony, since the rarible contracts are designed around the Ownable contract, one of the challenge of the proposed implementation will be to keep the owner in sync in both Ownableand AccessControl.
To be able to set the roles, the sender must be added to the admin role.
The implementation would result in a class like this:

// SPDX-License-Identifier: MIT

pragma solidity >=0.6.0 <0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";

contract MintAccess is Initializable, AccessControlUpgradeable {
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bool public hasMinterControl;

    function __MintAccess_init() internal initializer {
        __AccessControl_init_unchained();
        __MintAccess_init_unchained();
    }

    function __MintAccess_init_unchained() internal initializer {
        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
    }

    modifier validateMinter() {
        require(
            !hasMinterControl || hasRole(MINTER_ROLE, _msgSender()),
            "Caller is not a minter"
        );
        _;
    }

    function enableMinterControl() external {
        require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()));
        require(!hasMinterControl, "MintAccess: Already enabled");
        hasMinterControl = true;
    }

    function disableMinterControl() external {
        require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()));
        require(hasMinterControl, "MintAccess: Already disabled");
        hasMinterControl = false;
    }

    function transferOwnership(address newOwner) public virtual {
        require(newOwner != address(0), "MintAccess: new owner is the zero address");
        require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()));
        grantRole(DEFAULT_ADMIN_ROLE, newOwner);
        revokeRole(DEFAULT_ADMIN_ROLE, _msgSender());
    }
}

One of the major downside is that we need to "intercept" the call to transferOwnership() to update the admin role.
By implementing the same signature, the inheriting class would be required to resolve the propagation. ie:

  function transferOwnership(address newOwner) public override(OwnableUpgradeable, MintAccess) {
      OwnableUpgradeable.transferOwnership(newOwner);
      MintAccess.transferOwnership(newOwner);
  }

I'm not sure the benefits from inheriting AccessControl are greater than the down side in this case.

---

The other solution, would be to skip AccessControl and inherit Ownable from MintAccess.
The class will manage a mapping of allowed minter addresses on our side. The implementation could be:

// SPDX-License-Identifier: MIT

pragma solidity >=0.6.0 <0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

contract MintAccess is Initializable, OwnableUpgradeable {
    mapping (address => bool) private _minters;
    bool public hasMinterControl;

    function __MintAccess_init() internal initializer {
        __Ownable_init_unchained();
        __MintAccess_init_unchained();
    }

    function __MintAccess_init_unchained() internal initializer {
    }

    modifier validateMinter() {
        require(
            !hasMinterControl || _minters[_msgSender],
            "Caller is not a minter"
        );
        _;
    }

    function enableMinterControl() external onlyOwner {
        require(!hasMinterControl, "MintAccess: Already enabled");
        hasMinterControl = true;
    }

    function disableMinterControl() external onlyOwner  {
        require(hasMinterControl, "MintAccess: Already disabled");
        hasMinterControl = false;
    }

    function grantMinter(address _minter) external onlyOwner {
        require(!_minters[_minter], 'Already minter');
        _minters[_minter] = true;

        // TODO : Add event ?
    }

    function revokeMinter(address _minter) external onlyOwner {
        require(_minters[_minter], 'Not minter');
        _minters[_minter] = false;

        // TODO : Add event ?
    }
}

[NicolasMahe]

• @Hadrienlc please implement feedback from Minter Access Control rarible/protocol-contracts#105

[NicolasMahe]

New feedback on the PR, @Hadrienlc please update when you have time:

• Minter Access Control rarible/protocol-contracts#105 (comment)

---

Waiting for review