OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.96k stars 11.8k forks source link

AccessManager restricted should not default to admin role #5001

Open Ramarti opened 7 months ago

Ramarti commented 7 months ago

Right now, when I deploy an AccessManaged contract and use the restricted modifier, it automatically gives the admin role the permission to call it (since getTargetFunctionRole() will return 0, which is the admin role).

I think that can lead to situations where projects can give the admin role too much power without explicitly declaring it through setTargetFunctionRole, by messing up a deploy script for example (with AccessControl, onlyRole was more explicit about which role protects the method).

Why is this the behavior, instead of reverting if not explicitly set?

ernestognw commented 7 months ago

The intention of the manager is to provide a way to progressively decentralize a protocol, so deploying is like the very first step where things are still in deployer's control. Next steps would be to assign permissions and finally renounce its own admin role.

I agree granting the admin role to the deployer may be dangerous, but it's designed so that configuration of the manager can be done within the same account and finally renounce the ownership.

See this access-manager-demo example.

What would you say would be the best alternative?