contracts icon indicating copy to clipboard operation
contracts copied to clipboard

hash the roleId with orchestrator address in getOwnerRole and getManagerRole

Open 0xNuggan opened this issue 2 years ago • 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

0xNuggan avatar Feb 02 '24 13:02 0xNuggan

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

0xNuggan avatar Feb 05 '24 09:02 0xNuggan

Is this still relevant @0xNuggan?

marvinkruse avatar Jun 06 '24 11:06 marvinkruse