InverterNetwork/contracts

hash the roleId with orchestrator address in getOwnerRole and getManagerRole

Opened this issue · 2 comments

For consistency. Also make the role definition constants private in the process.
Right now it's quite the footgun at best, bugged at worst, since the modules ask for the unhashed ID when checking authoritzation, so grantGlobalRole would be giving out a role that modules are not checking. Also, going through the normal grantRole() doens't work since that uses the separate roleAdmin adress, and does not scale together with granting the "owner" role.

Proposed changes:

  • Role ID definition constants become private.
  • getOwnerRole and getManagerRole return the RoleID hashed with the orchestrator address
  • All external modules ues those funcs to check authorization
  • People with "OWNER" role can grant and revoke global roles freely.

This issue also ties into the OZ V5 check of the authorizationManager

Also: Review the hasModuleRole / hasRole functionalities to make it more intuitive. Maybe hasModuleRole takes address + role, so it can be called by users too.

In general, even thouhg everything works as expected, the developer UX has proven to be bad, so we need to rename/rethink some parts to make it easier

Is this still relevant @0xNuggan?