It is unclear why MISOTokenFactory contract needs its own TOKEN_MINTER_ROLE when similar role is already defined in MISOAccessControls:
bytes32 public constant TOKEN_MINTER_ROLE = keccak256("TOKEN_MINTER_ROLE"); // MISOTokenFactory
bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); // MISOAccessControls
Especially considering that both of these roles have the same privileges, that is to deployToken when the contract is locked:
/// @dev If the contract is locked, only admin and minters can deploy.
if (locked) {
require(accessControls.hasAdminRole(msg.sender)
|| accessControls.hasMinterRole(msg.sender)
|| hasTokenMinterRole(msg.sender),
"MISOTokenFactory: Sender must be minter if locked"
);
}
Similarly with MARKET_MINTER_ROLE in MISOMarket and other contracts.
Recommended Mitigation Steps
Consider removing this surplus role to eliminate the confusion and improve gas usage.
Handle
pauliax
Vulnerability details
Impact
It is unclear why MISOTokenFactory contract needs its own TOKEN_MINTER_ROLE when similar role is already defined in MISOAccessControls: bytes32 public constant TOKEN_MINTER_ROLE = keccak256("TOKEN_MINTER_ROLE"); // MISOTokenFactory bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); // MISOAccessControls Especially considering that both of these roles have the same privileges, that is to deployToken when the contract is locked:
Similarly with MARKET_MINTER_ROLE in MISOMarket and other contracts.
Recommended Mitigation Steps
Consider removing this surplus role to eliminate the confusion and improve gas usage.